[Bug 29981] Support power saving interface

[bugzilla-daemon at freedesktop.org]

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

Will Thompson <will.thompson at collabora.co.uk> changed:

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

--- Comment #1 from Will Thompson <will.thompson at collabora.co.uk> 2010-09-08 07:12:28 PDT ---
Did you intend to change the Wocky submodule in the second commit?

Also, these commits could really do with being arranged more readably. I don't
see that preserving the history of the first version of the interface is all
that useful. Similarly, the ‘Return DBUS invocation context.’ commit fixes a
bug introduced in the previous commit; it should be squashed. I think I'd
basically expect this branch to be:

• Add the latest draft XML;
• Replace the code using GabbleSlacker with an implementation of the new
interface, replacing the test;
• Remove GabbleSlacker.

git add -p, git commit --amend, git rebase are all your friends!

-    gboolean is_inactive)
+    gboolean start_queueing)

Much better argument name!

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().)

Oh, there should also be one more patch that replaces
lm_message_build_with_sub_type() with wocky_xmpp_stanza_build(). And, since
conn-slacker.[ch] have been renamed, their introductory comments should be
updated. (I notice that I was a bad person and claim that the .c is a .h in its
comment...)

-  /* 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.

+      g_error_free (error);
+
+      error = g_error_new (TP_ERRORS, TP_ERROR_NOT_AVAILABLE,
+          "Failed to %sable power saving", enable ? "en" : "dis");
+
+      dbus_g_method_return_error (context, error);

It'd be nice to include information on why not in the error message, based on
the error returned from sending the IQ. In a work-in-progress branch
<http://git.collabora.co.uk/?p=user/wjt/telepathy-gabble-wjt.git;a=shortlog;h=refs/heads/search-cleanup>,
I've got a wrapper around wocky_porter_send_iq_finish() that makes this easier
to do.
<http://git.collabora.co.uk/?p=user/wjt/telepathy-gabble-wjt.git;a=commitdiff;h=23455ed61a3994aebaf39032cbc402a2521fe189>


+  gboolean enable =lm_message_node_find_child (

Coding style: space after =.

The configure script should be updated not to check for MCE any more.

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.

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

-- 
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.