Bug 29914

Introducing Connection.Interface.PowerSave

http://git.collabora.co.uk/?p=user/eitan/telepathy-spec.git;a=shortlog;h=refs/heads/power-save

http://people.freedesktop.org/~eitani/telepathy-spec-power_save/spec/

Some open questions:

1. Is a setter, property and notification change too much? After all, the property won't change without explicitly changing it.

2. Maybe device inactivity is too low level, and MC should be dealing with that. Maybe this interface should speak purely in power saving terms. In this way, the Connection objects state talks in terms of the actual connection, and not in terms of device idleness. For example:

method: SetPowerSaving(b:on)
property: PowerSavingActivated:b
signal: PowerSavingChanged

This would also make possible exceptions in SetPowerSaving clearer. Example NotImplemented for XMPP servers that don't support "google:queue". In the first draft, when setting device inactivity on, NotImplemented would not be as clear.

3. If we are talking about an interface like in 2, is on/off enough, or do we need finer granularity like "medium" power saving?

OK,

I went with power saving terminology.

Same URLs as above.

This looks OK as a first draft, if the following changes are made:

I'd prefer the name SetPowerSaving rather than TogglePowerSaving - its semantics are currently set rather than toggle, because toggle means "let x = !x", which is never what we want on D-Bus (because if two processes toggle at the same time, you lose).

I'd prefer the interface to be named PowerSaving rather than PowerSave.

> +            connection can have it's powersaving mode turned on or off.</p>

have its power saving mode

> +        <tp:error name="org.freedesktop.Telepathy.Error.NotAvailable">
> +          <tp:docstring>
> +            The current connection has no power saving features.

I don't think this is useful. I think SetPowerSaving(TRUE) should mean "consume minimum power" and FALSE should mean "behave normally"; on an XMPP connection that lacks power-saving extensions like GTalk's, minimum power and behaving normally are the same thing :-)

The only cases in which I'd expect Gabble to raise an error from this method would be if sending the stanza to the server failed, e.g. NetworkError (but that's a ridiculous edge-case which should only happen if we're about to drop off the network anyway), or if we sent the stanza to the server and it replied with an error (which might well be mapped to NotAvailable, hence my concern).

--------------------------------------------------

This interface doesn't mention what you lose by enabling power saving. We should say what the power saving flag breaks, so that clients can choose when to set it in an appropriate way.

I believe that in practice, power saving suppresses the sorts of things that you don't care about notifications for until your display is lit - presence updates, and hence the ability to get up-to-date capabilities and avatars - while not suppressing channels or publish requests (which could be thought of as communication types). Is this true?

A more overengineered version of this interface would be to have a bitfield of "things I'm prepared to sacrifice in order to save power", and to have the CM decide what to do based on that, e.g.:

    SetPowerSaving (Presence | Capabilities | Avatars | Calls)

If we did that, Gabble on GTalk would only go to power-saving mode if the first three of those flags were all set, and could stop advertising Jingle capabilities if the Calls flag was set. (Silly hypothetical example, we probably wouldn't actually want to do that - although in principle we could stop being callable while on the last 10% of battery or something...)

I think that's probably overkill, but it seems worth at least mentioning it!

--------------------------------------------------

An alternative approach worth mentioning here would be to have a service name ofdT.PowerSaving (in practice yet another name for MC), have it emit signals, and have CMs listen for those. That would break the general design that CMs are "at the bottom of the stack" (always emit signals and have methods called on them), though.

(In reply to comment #4)
> This looks OK as a first draft, if the following changes are made:
> 
> I'd prefer the name SetPowerSaving rather than TogglePowerSaving - its
> semantics are currently set rather than toggle, because toggle means "let x =
> !x", which is never what we want on D-Bus (because if two processes toggle at
> the same time, you lose).
> 

Yup. b4b2e04

> I'd prefer the interface to be named PowerSaving rather than PowerSave.
> 

7bbfa3c

> > +            connection can have it's powersaving mode turned on or off.</p>
> 
> have its power saving mode
> 
> > +        <tp:error name="org.freedesktop.Telepathy.Error.NotAvailable">
> > +          <tp:docstring>
> > +            The current connection has no power saving features.
> 
> I don't think this is useful. I think SetPowerSaving(TRUE) should mean "consume
> minimum power" and FALSE should mean "behave normally"; on an XMPP connection
> that lacks power-saving extensions like GTalk's, minimum power and behaving
> normally are the same thing :-)

If the CM cannot switch modes, either because of the protocol (NotImplemented), or because of the service (NotAvailable), I think MC (or whoever manages this) should be made aware. They could ignore it or, in the extreme, be fascist and disconnect the account.

> 
> The only cases in which I'd expect Gabble to raise an error from this method
> would be if sending the stanza to the server failed, e.g. NetworkError (but
> that's a ridiculous edge-case which should only happen if we're about to drop
> off the network anyway), or if we sent the stanza to the server and it replied
> with an error (which might well be mapped to NotAvailable, hence my concern).
> 

Functionally, there is no difference. In the XMPP case, gabble will get a <service-unavailable/> error if the server does not support google's queueing, which will be mapped to a Telepathy NotAvailable error. So we are basically saying the same thing.

You think this should not be explicitly mentioned in the docs?

> --------------------------------------------------
> 
> This interface doesn't mention what you lose by enabling power saving. We
> should say what the power saving flag breaks, so that clients can choose when
> to set it in an appropriate way.
> 
> I believe that in practice, power saving suppresses the sorts of things that
> you don't care about notifications for until your display is lit - presence
> updates, and hence the ability to get up-to-date capabilities and avatars -
> while not suppressing channels or publish requests (which could be thought of
> as communication types). Is this true?
> 

Yeah, I'll make it clear in the docs. Power saving should be used when the user is not interacting with the device, for example when the screen is locked.

> A more overengineered version of this interface would be to have a bitfield of
> "things I'm prepared to sacrifice in order to save power", and to have the CM
> decide what to do based on that, e.g.:
> 
>     SetPowerSaving (Presence | Capabilities | Avatars | Calls)
> 
> If we did that, Gabble on GTalk would only go to power-saving mode if the first
> three of those flags were all set, and could stop advertising Jingle
> capabilities if the Calls flag was set. (Silly hypothetical example, we
> probably wouldn't actually want to do that - although in principle we could
> stop being callable while on the last 10% of battery or something...)
> 
> I think that's probably overkill, but it seems worth at least mentioning it!
> 

I think the encapsulation here is nice. It is up to the CM to be creative and figure out how to take power saving measures with no noticeable effects to the user.

An earlier version of the draft used a two member enum instead of binary on/off. I was thinking it would make it future-proof if we ever invent intermediate power saving modes. But I couldn't think of real world examples, so I dropped it.

> --------------------------------------------------
> 
> An alternative approach worth mentioning here would be to have a service name
> ofdT.PowerSaving (in practice yet another name for MC), have it emit signals,
> and have CMs listen for those. That would break the general design that CMs are
> "at the bottom of the stack" (always emit signals and have methods called on
> them), though.

Yeah, I think that would be a significant pattern change (is there precedent?). Anyway, I think having this as a simple interface on the CM is very elegant.

review+ as a draft. When you merge, please say which version will be the first to have the draft, remove the patch keyword (until you make further changes), but keep the bug open for the revision/undrafting process.

> > > +            connection can have it's powersaving mode turned on or off.</p>
> > 
> > have its power saving mode

Still to be corrected, I think?

> > 
> > > +        <tp:error name="org.freedesktop.Telepathy.Error.NotAvailable">
> > > +          <tp:docstring>
> > > +            The current connection has no power saving features.
> > 
> > I don't think this is useful. I think SetPowerSaving(TRUE) should mean "consume
> > minimum power" and FALSE should mean "behave normally"; on an XMPP connection
> > that lacks power-saving extensions like GTalk's, minimum power and behaving
> > normally are the same thing :-)
> 
> If the CM cannot switch modes, either because of the protocol (NotImplemented),
> or because of the service (NotAvailable), I think MC (or whoever manages this)
> should be made aware. They could ignore it or, in the extreme, be fascist and
> disconnect the account.

Your reasoning is persuasive; if you turn it into a <tp:rationale> and add a note like this one, I think this behaviour would make sense:

    Errors raised by this method indicate that power saving could not be
    enabled, which SHOULD NOT generally be treated as fatal.

(Implementation detail: Mission Control will currently g_warning() all over the place whenever it becomes unhappy. Don't treat that as exemplary - I consider it to be a bug, Bug #23486. MC should definitely only DEBUG() on failures to save power.)

> > This interface doesn't mention what you lose by enabling power saving.
[...]
> Yeah, I'll make it clear in the docs. Power saving should be used when the user
> is not interacting with the device, for example when the screen is locked.

Still to be done.

> It is up to the CM to be creative and
> figure out how to take power saving measures with no noticeable effects to the
> user.

Well, what's noticeable to the user depends what the user is looking for :-P but I agree that the bitfield approach seems like overkill, and a simple boolean will be enough in practice.

> An earlier version of the draft used a two member enum instead of binary
> on/off. I was thinking it would make it future-proof if we ever invent
> intermediate power saving modes. But I couldn't think of real world examples,
> so I dropped it.

Yeah, that makes sense.

Merged to maseter (with amendments).

(In reply to comment #6)
> > > > +        <tp:error name="org.freedesktop.Telepathy.Error.NotAvailable">
> > > > +          <tp:docstring>
> > > > +            The current connection has no power saving features.
> > > 
> > > I don't think this is useful. I think SetPowerSaving(TRUE) should mean "consume
> > > minimum power" and FALSE should mean "behave normally"; on an XMPP connection
> > > that lacks power-saving extensions like GTalk's, minimum power and behaving
> > > normally are the same thing :-)
> > 
> > If the CM cannot switch modes, either because of the protocol (NotImplemented),
> > or because of the service (NotAvailable), I think MC (or whoever manages this)
> > should be made aware. They could ignore it or, in the extreme, be fascist and
> > disconnect the account.
> 
> Your reasoning is persuasive; if you turn it into a <tp:rationale> and add a
> note like this one, I think this behaviour would make sense:
> 
>     Errors raised by this method indicate that power saving could not be
>     enabled, which SHOULD NOT generally be treated as fatal.

If the connection doesn't support power saving, this interface should not be listed in <http://telepathy.freedesktop.org/spec-snapshot/Connection.html#org.freedesktop.Telepathy.Connection.Interfaces> in the first place, so MC shouldn't be calling this method. :)

(In reply to comment #6)
> review+ as a draft. When you merge, please say which version will be the first
> to have the draft, remove the patch keyword (until you make further changes),
> but keep the bug open for the revision/undrafting process.
> 
> > > > +            connection can have it's powersaving mode turned on or off.</p>
> > > 
> > > have its power saving mode
> 
> Still to be corrected, I think?

This still wasn't corrected. I just fixed it and pushed to master.

Here are a couple of patches rephrasing some documentation in the interface.

Merged to spec master. Retitling the bug. I think the remaining item is the MC implementation.

Implementation status:

* MC: draft merged, Bug #31263
* Gabble: draft merged, Bug #29981

I don't see anything that would prevent undrafting this. Eitan, Will, any objections?

(In reply to comment #12)
> Eitan, Will, any objections?

... cc'ing Will would help.

Looks good to me!

(In reply to comment #14)
> Looks good to me!

Undrafted for 0.21.5.