After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 611613 - get presence notifications from telepathy
get presence notifications from telepathy
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on: 608999
Blocks:
 
 
Reported: 2010-03-02 15:31 UTC by Dan Winship
Modified: 2010-05-17 14:05 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
account class (1.26 KB, text/x-python)
2010-04-07 12:36 UTC, Morten Mjelva
  Details
account manager class (1.33 KB, text/x-python)
2010-04-07 12:37 UTC, Morten Mjelva
  Details
presence example written in python (6.99 KB, text/x-python)
2010-04-07 12:53 UTC, Morten Mjelva
  Details
TelepathyClient: Simplify the channel filter specifications (2.79 KB, patch)
2010-05-11 17:20 UTC, Dan Winship
committed Details | Review
TelepathyClient: rename AvatarManager to ContactManager (3.47 KB, patch)
2010-05-11 17:21 UTC, Dan Winship
committed Details | Review
TelepathyClient: track added/removed accounts (4.07 KB, patch)
2010-05-11 17:21 UTC, Dan Winship
committed Details | Review
TelepathyClient: show notifications for presence changes (15.75 KB, patch)
2010-05-11 17:21 UTC, Dan Winship
committed Details | Review

Description Dan Winship 2010-03-02 15:31:55 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.
Comment 1 Guillaume Desmottes 2010-03-02 15:44:27 UTC
I don't really understand what you are planning to do. Could you be more specific please?
Comment 2 Dan Winship 2010-03-02 15:52:15 UTC
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.
Comment 3 Guillaume Desmottes 2010-03-02 16:04:11 UTC
Maybe Empathy could use notify_notification_set_category so the shell could filter notifications depending on their type?
Comment 4 Dan Winship 2010-03-02 16:16:49 UTC
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.)
Comment 5 Morten Mjelva 2010-04-07 12:36:32 UTC
Created attachment 158114 [details]
account class

account class for the python example
Comment 6 Morten Mjelva 2010-04-07 12:37:12 UTC
Created attachment 158115 [details]
account manager class

account manager class for the python example
Comment 7 Morten Mjelva 2010-04-07 12:53:48 UTC
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.
Comment 8 Morten Mjelva 2010-04-07 13:10:25 UTC
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
Comment 9 Dan Winship 2010-05-11 17:20:59 UTC
Created attachment 160833 [details] [review]
TelepathyClient: Simplify the channel filter specifications
Comment 10 Dan Winship 2010-05-11 17:21:03 UTC
Created attachment 160834 [details] [review]
TelepathyClient: rename AvatarManager to ContactManager

and belated rename info.token to info.tokens
Comment 11 Dan Winship 2010-05-11 17:21:06 UTC
Created attachment 160835 [details] [review]
TelepathyClient: track added/removed accounts

(prep work for presence tracking)
Comment 12 Dan Winship 2010-05-11 17:21:09 UTC
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.
Comment 13 Dan Winship 2010-05-11 17:24:21 UTC
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
Comment 14 Marina Zhurakhinskaya 2010-05-11 20:58:06 UTC
Review of attachment 160833 [details] [review]:

Looks good.
Comment 15 Marina Zhurakhinskaya 2010-05-11 21:09:20 UTC
Review of attachment 160834 [details] [review]:

Seems like two unrelated patches, but ok.
Comment 16 Marina Zhurakhinskaya 2010-05-11 23:02:15 UTC
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.
Comment 17 Marina Zhurakhinskaya 2010-05-14 20:25:50 UTC
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?
Comment 18 Marina Zhurakhinskaya 2010-05-14 20:30:11 UTC
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
Comment 19 Dan Winship 2010-05-17 14:00:31 UTC
pushed with suggested fixes
Comment 20 Dan Winship 2010-05-17 14:05:41 UTC
(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