[Bug 29981] Support power saving interface

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Thu Sep 9 04:15:32 CEST 2010


https://bugs.freedesktop.org/show_bug.cgi?id=29981

Eitan Isaacson <eitan.isaacson at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Status Whiteboard|review-                     |
           Keywords|                            |patch

--- Comment #3 from Eitan Isaacson <eitan.isaacson at collabora.co.uk> 2010-09-08 19:15:32 PDT ---
Ok, now it is ready for review!

(In reply to comment #1)
> It's better not to use _gabble_connection_send_with_reply() in new code. Please
> call wocky_porter_send_iq_async() on the porter you get back from
> gabble_connection_get_porter(). Maybe we should add a
> gabble_connection_send_iq_async() wrapper around it, but we should avoid adding
> new code that uses the Lm compatibility stuff. (See below for a wrapper I've
> already written around _finish().)
> 

This branch depends on another one in bug 30094. I made slick
wocky_porter_send_iq_async wrappers to make life easier. I also stole some of
your code.

> -  /* We can only cork presence updates on Google Talk. Of course, the Google
> -   * Talk server doesn't advertise support for google:queue. So let's use the
> -   * roster again... */
> -  if (!(conn->features & GABBLE_CONNECTION_FEATURES_GOOGLE_ROSTER))
> -    return;
> 
> This check appears to have been lost, so we're sending commands to the server
> without checking if it supports them. What we should do instead is not include
> Connection.Interface.PowerSave in Connection.Interfaces if we're not on a
> Google connection (so that MC knows not to bother telling Gabble the activity
> status has changed), and probably also return an error from the method on
> PowerSave() if someone decides to call it anyway.
> 

I am not sure why that comment is there. "google:queue" does not get advertised
in the connection disco. So the only way to know is trial and error (ie.
SetPowerSaving and catch NotAvailable exception).

This is probably not such a bad thing. I need to amend the spec documentation,
but I made it possible to set power save mode when the connection is offline.
The method will always succeed, once we go online, if we encounter an error
PowerSavingChanged(false) is emitted.

On second thought, maybe that is not very useful, since MC will know when a
connection goes online, and could explicitly set the mode each time.

> The test should check what happens if Gabble's still waiting for a reply to a
> <queue/> request when it's disconnected. I think the contact search tests have
> an example of how to do this: you pass some EventPatterns to a helper function
> that deals with the disconnection; so here we should presumably expect the
> Cancelled error or something.
> 

This would not really be regarded as an error, so if the connection drops
before we get an ack, we will reënable presence queuing the next time it
connects.

> The test could also check that no-op calls to TogglePowerSaving return without
> any network traffic.

Added that.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.


More information about the telepathy-bugs mailing list