Summary: | Import Connection.Interface.StoredMessages from rtcom-tp-glib | ||
---|---|---|---|
Product: | Telepathy | Reporter: | Will Thompson <will> |
Component: | tp-spec | Assignee: | Will Thompson <will> |
Status: | RESOLVED MOVED | QA Contact: | Telepathy bugs list <telepathy-bugs> |
Severity: | normal | ||
Priority: | medium | CC: | lassi.syrjala, marco.barisione, smcv |
Version: | unspecified | Keywords: | patch |
Hardware: | Other | ||
OS: | All | ||
URL: | http://git.collabora.co.uk/?p=user/wjt/telepathy-spec-wjt.git;a=shortlog;h=refs/heads/import-stored-messages | ||
Whiteboard: | |||
i915 platform: | i915 features: | ||
Bug Depends on: | 26838 | ||
Bug Blocks: |
Description
Will Thompson
2010-06-24 08:08:51 UTC
CCing Lassi. Is there anything functionally wrong with the Maemo 5 version of this interface that we should change before standardizing it, in your opinion? The first two patches "Add a type synonym for message part IDs", "Allow arrays of Protocol_Message_Token" are obviously OK; please merge. My worry about the new interface is that it's defined in terms of 'message-token', but as I understand it, the unique identifiers in SMS are actually what we'd now call a 'protocol-token' (i.e. they're temporarily unique per contact, but not globally unique). > Persistently-stored messages MUST also contain a stored-message-received > header. The first time the message is announced in MessageReceived, this > SHOULD be identical to the message-token; if the message might have been > presented earlier, the stored-message-received header MAY differ from the > message-token. This could do with rationale. What's it for? If the intention is that it's the original value of message-token, say so; if the intention is that in re-deliveries it's the new protocol-level ID, again, say so. See Bug #26838 for a third sort of message identifier; if this is appropriate for what you need, please use that one. > If the message has been presented earlier in this Connection's lifetime, > a header rescued with boolean value True SHOULD be present. Is this compatible with the usual meaning of 'rescued', which is essentially "a logger that has been paying attention should already have seen this"? (In reply to comment #2) > The first two patches "Add a type synonym for message part IDs", "Allow arrays > of Protocol_Message_Token" are obviously OK; please merge. I have done so. The biggest problem with the original interface support by commhistoryd in Harmattan was the fact that stored messages includes currently pending messages. If such a message is expunged, it's delivered twice. In case of tp-ring, only "rescued" can be used to detect a message delivered via expunging ("message-received" and "stored-message-received" are always identical and they are timestamps, not tokens currently). Do we really need two message tokens for re-delivered messages? At least for inbound messages, it's not used for anything else but expunge checks and we could use just original token. (In reply to comment #4) > The biggest problem with the original interface support by commhistoryd in > Harmattan was the fact that stored messages includes currently pending > messages. If such a message is expunged, it's delivered twice. In case of > tp-ring, only "rescued" can be used to detect a message delivered via expunging > ("message-received" and "stored-message-received" are always identical and they > are timestamps, not tokens currently). I think I've addressed this, by making DeliverStoredMessages() a no-op if messages are already in PendingMessages on some channel. Sound okay? > Do we really need two message tokens for re-delivered messages? At least for > inbound messages, it's not used for anything else but expunge checks and we > could use just original token. Yeah, I just binned stored-message-token entirely. I've also reworded the introduction again, added a 'stored' header that indicates that the message also appears on this interface, and some other stuff. I think it's ready to go. Hello, I am on vacation until August 8th. In urgent project matters, please contact Naba Kumar. Regards, Lassi StoredMessages is a slightly misleading name for this interface (not that I can think of a better one off-hand). Maybe we could call it MessageSpool? That is, after all, what it is. And, if we want to support cancelling outgoing messages after we call SendMessage() but before they're actually sent (maybe we're in a tunnel, so no SMSes or XMPP <message>s can be sent), should this interface be the place to do that, too? (In reply to comment #7) > StoredMessages is a slightly misleading name for this interface (not that I can > think of a better one off-hand). Maybe we could call it MessageSpool? That is, > after all, what it is. This is purely cosmetic, of course. > And, if we want to support cancelling outgoing messages after we call > SendMessage() but before they're actually sent (maybe we're in a tunnel, so no > SMSes or XMPP <message>s can be sent), should this interface be the place to do > that, too? I think that, if we need this, we could add it later. I've rebased the branch, and updated the HTML copy. Just a quick note here as well that there are some issues in implementing this interface with the new ofono-based tpring (without adding another layer of disk spooling to the path). See the long recent thread on ofono list: http://lists.ofono.org/pipermail/ofono/2011-February/008428.html As a result of the recent discussions, we are now implementing a prototype (involving an ofono plugin and matching tpring patch) to verify we can sanely implement the updated StoredMessage interface. I'll follow up to this bug when we have more information. One thing we know already is that it will not be possible to implement SetStorageState(), so it would be good to have a possibility to return 'Not Available' (or similar) for the method. Reasoning why this won't be supported is here: http://lists.ofono.org/pipermail/ofono/2011-February/008455.html ... in theory this might change in some future ofono version, but in any case with some modems this won't be supported, so tp-ring has to prepare for the case (e.g. that it cannot signal to cellular network to stop sending more SMS'es). *** Bug 24904 has been marked as a duplicate of this bug. *** See Bug #24904 for some other discussion on this. -- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/telepathy/telepathy-spec/issues/76. |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.