[Bug 29981] Support power saving interface

[bugzilla-daemon at freedesktop.org]

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

--- Comment #14 from Eitan Isaacson <eitan.isaacson at collabora.co.uk> 2010-09-16 09:16:24 PDT ---
(In reply to comment #13)
> (In reply to comment #12)
> > (In reply to comment #10)
> > > (In reply to comment #7)
> > > > I see Conn.I.PowerSaving being useful for offline connections as well, but I am
> > > > not totally convinced about it. You could sway me either way. If we use this
> > > > iface on offline accounts, we don't need to piggyback on top of google:roster,
> > > > we will just always sport the iface, and raise NotAvailable if the service does
> > > > not support it. Added bonus - non-google servers could support google:queue and
> > > > we would interop with them, even if they don't support google:roster.
> > > 
> > > I would make it raise NotAvailable() until Connected, using
> > > TP_BASE_CONNECTION_ERROR_IF_NOT_CONNECTED(). When we get the server features,
> > > check for 'google:queue' or 'google:roster'; if either is present, add the
> > > interface using tp_base_connection_add_interfaces().
> > > 
> > 
> > Great! Made the appropriate changes. 0545f7c
> 
> +  if (base->status != TP_CONNECTION_STATUS_CONNECTED)
> +    {
> +      GError *error = g_error_new_literal (TP_ERRORS, TP_ERROR_NOT_AVAILABLE,
> +          "Cannot enter power saving mode when not connected.");
> +      dbus_g_method_return_error (context, error);
> +      g_error_free (error);
>        return;
>      }
> 
> This is TP_BASE_CONNECTION_ERROR_IF_NOT_CONNECTED(), expanded out. And you
> don't need to heap-allocate a GError with a literal string, you can just say
> GError e = { TP_ERRORS, TP_ERROR_NOT_AVAILABLE, "..." }; if you want to keep
> the more-specific error message.

Yeah, I should make that a habit. 846a938

> 
> +                conn->features |=
> GABBLE_CONNECTION_FEATURES_GOOGLE_MAIL_NOTIFY;              else if (0 ==
> strcmp (var, NS_GOOGLE_QUEUE))
> +                conn->features |= GABBLE_CONNECTION_FEATURES_GOOGLE_QUEUE;
> 
> I think you're missing a newline before the 'else if' here.

846a938

> 
> > > By checking for both features we would do the right thing with the deployed
> > > server that implements this feature, and allow for hypothetical servers to do
> > > the right thing.
> > 
> > This introduces false positives when a server genuinely supports google:roster,
> > but not google:queue. This is all hypothetical, so never mind :)
> 
> Yeah, the worst that happens is spurious updates. If this turns out to be a
> problem... Gabble can remember failures and stop sending them.
> 
> I don't suppose that, while you're touching this code, you could check if
> there's any reference to
> http://mail.jabber.org/pipermail/summit/2010-February/000528.html and if not
> add it?

846a938

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