GNOME Bugzilla – Bug 611613
get presence notifications from telepathy
Last modified: 2010-05-17 14:05:41 UTC
since we'll be ignoring empathy's notifications about buddy login/logout, we need to subscribe to the right notifications directly via telepathy. These should result in notifications, using the same source as the conversation with that user. At the same time, we should probably fix it to notice errors from the Send() method, so that it can tell if the remote person logged out before receiving the message you sent.
I don't really understand what you are planning to do. Could you be more specific please?
We just want to be able to display "Guillaume Desmottes logged in/out" notifications. Previously we got these for free from empathy via libnotify, but since we're switching to using telepathy directly for messaging, we have to just ignore all the libnotify notifications coming from empathy, which means we don't get the login/logout notifications either. So we have to arrange to get that info from telepathy instead, and them build our own notifications. "using the same source as the conversation" just refers to shell internals. "notice errors from Send" is just that, if the user logs out, and I send an IM, I assume I get back some notification that my message didn't reach the user, but we're not currently doing anything with that.
Maybe Empathy could use notify_notification_set_category so the shell could filter notifications depending on their type?
possibly. one other issue is that we don't want this to depend on what preferences the user has set in empathy. (though of course, we don't want there to be do-nothing checkboxes in empathy's preferences window either. the detailed interactions haven't been fully thought-out.)
Created attachment 158114 [details] account class account class for the python example
Created attachment 158115 [details] account manager class account manager class for the python example
Created attachment 158116 [details] presence example written in python simple example. depends on the other two files, as the telepathy-python bindings are a bit incomplete. hope it's helpful flow goes like this: we create an account manager object which proxies for the actual account manager. we then ask the account manager which protocols are supported, then we ask each of these if it's online. for the ones that are online, we create a connection proxy object. then, for each connection, we ensure that the "subscribe" contact list channel for that connection exists, and grab a list of contacts from that channel. finally, we connect to the PresencesChanged signal, for each connection, and print updates from contacts in our subscribe list.
Additional comments: getting things from telepathy directly is the proper way to do it. using libempathy is deprecated (as far as i know at least). in addition to the above, getting a notification when sending of a message has failed is pretty trivial. we connect to the SendError signal, and everything sorts itself out beautifully. it's very straight forward. Link to spec: http://telepathy.freedesktop.org/spec/org.freedesktop.Telepathy.Channel.Type.Text.html#org.freedesktop.Telepathy.Channel.Type.Text.SendError
Created attachment 160833 [details] [review] TelepathyClient: Simplify the channel filter specifications
Created attachment 160834 [details] [review] TelepathyClient: rename AvatarManager to ContactManager and belated rename info.token to info.tokens
Created attachment 160835 [details] [review] TelepathyClient: track added/removed accounts (prep work for presence tracking)
Created attachment 160836 [details] [review] TelepathyClient: show notifications for presence changes Fetch the names of the user's "subscribed" contacts, and use the SimplePresence interface to watch for available/away/busy/etc messages and create notifications for them. Currently we display notifications when switching between "available" and "offline"/"extended away", but when switching between "available" and "away"/"busy" we just add the information to the chat window without popping up a notification, to avoid spamming the user with "Bob's screensaver activated" messages.
There are simplifications/refactoring that could be made involving ContactManager (to avoid redundantly fetching names/avatar tokens) but I don't think it's really worth doing since we'll eventually move to tp-glib. Currently we don't show presence notifications for users until after you start chatting with them. This is probably pretty definitely wrong... but we probably also don't want to create a tray source for every user who logs in or out while you're around. I'm thinking: - if you get a logout notification for someone you haven't been talking to, we just ignore it - if you get a login notification for someone you haven't been talking to, we create a source for them, but that source disappears automatically after some reasonably short timeout if you don't start a conversation
Review of attachment 160833 [details] [review]: Looks good.
Review of attachment 160834 [details] [review]: Seems like two unrelated patches, but ok.
Review of attachment 160835 [details] [review]: Looks good overall. Ok to commit once you incorporate the comments. You need signals: [ { name: 'AccountValidityChanged', inSignature: 'ob' } ] in telepathy.js as part of this commit. ::: js/ui/telepathyClient.js @@ +80,3 @@ + _accountValidityChanged: function(accountManager, accountPath, valid) { + if (!valid) + delete this._accounts[accountPath]; Some testing showed that 'Closed' signal does get called for the channels of the deleted account, but maybe it is worth a comment that corresponding this._channels entries will be cleared independently.
Review of attachment 160836 [details] [review]: Looks good and works well! Good to commit once you address the minor comments below. ::: js/misc/telepathy.js @@ +328,3 @@ + signals: [ + { name: 'AccountValidityChanged', + inSignature: 'ob' } As mentioned earlier, this should be a part of the previous patch. ::: js/ui/telepathyClient.js @@ +65,3 @@ this._accounts = {}; this._channels = {}; + this._sources = {}; Can we just use this._sources, and not this._channels ? It seems that we always have connPath and targetHandle when we get the channel. It seems that you could have multiple channelPath values for the combination of these two if there are different channel types or if there are multiple channels for the same channel type (not sure if that is possible). In any case, it shouldn't be problematic if they link to the same Source object (at least for now, before we add support for video/voice calls). Otherwise, it'd be good to explain the difference between this._channels and this._sources in a comment because they both have Source objects as values. @@ +196,3 @@ }, + addConnection: function(connPath) { Need to updated _addConnection() call in createAvatar() to be addConnection(). @@ +325,3 @@ info.connectionAvatars.disconnect(info.updatedId); info.connectionAvatars.disconnect(info.retrievedId); + info.contactsGroup.disconnect(info.contactsListChangedId); Do we also need to disconnect info.presenceChangedId for info.connectionPresence? @@ +551,3 @@ + notify = (this._presence != Telepathy.ConnectionPresenceType.OFFLINE); + } else if (presence == Telepathy.ConnectionPresenceType.AWAY) { + msg = _("%s is offline.").format(this.name); is away?
What you are suggesting about the presence notifications for people you haven't talked to sounds fine. Though I think it is ok to notify if the person you haven't talked to goes offline if you subscribed to this type of notifications. Other possible changes after these patches land: - have a different style for displaying presence changes inline (e.g. center them) - don't allow to continue chatting if the contact is offline
pushed with suggested fixes
(In reply to comment #13) > - if you get a login notification for someone you haven't been talking to, > we create a source for them, but that source disappears automatically > after some reasonably short timeout if you don't start a conversation let's make that be part of bug 606920