Agreed. Useful link: http://lists.freedesktop.org/archives/dbus/2006-March/004467.html

2006-03-17  Richard Hughes  <richard@hughsie.com>
 * src/gpm-manager.{c|h}, docs/dbus-interface.txt, src/gpm-manager.xml: Add templates of InhibitInactiveSleep and AllowInactiveSleep to begin to add support for #334809. At the moment they don't do much.

Jon, I guess g-s still emits the IdleChanged signal even when "Inhibited"?

Because if it does, we are going to need InhibitInactiveLCDdim and AllowInactiveLCDdim calls also.

Or should InhibitInactiveSleep be expanded with a parameter about what to inhibit?

It seems to me most obvious to make an application (say totem) call one method on g-p-m saying "disable changing-the-screen AND inactiveSleep" and then for g-p-m to inhibit g-s.

This is different to what happens now, that totem talks to g-s saying "InhibitScreensaver" and then has to talk to g-p-m saying "Inhibit DPMS, LCD DIM and Inactive-sleep" as the user is not going to want to stop the screensaver energising, but want dpms and lcd-dim to be enabled.

This could be solved quite easily if g-s was changed not to emit a idle-state-changed signal when inhibited, but this seems a bit of a bodge to me, as somethings might want to screensave, but not to suspend (like the nautilus example).

What you think?

It inhibits idle as well as activation.  The name is a bit misleading I suppose.

The idea is that totem doesn't have to worry about power management.  The only thing it wants to assert is that even though there are no keypresses or mouse movements the session is still being used.

The only applications that should use the g-p-m inhibit method are things that want to assert that the session can be marked as idle but things are still happening so shouldn't power down.

I suppose it is an open question what should happen when the user explicitly requests a suspend.

>It inhibits idle as well as activation.

Ahh, gotcha. That changes things. We should write this up in a document/faq somewhere.

>I suppose it is an open question what should happen when the user explicitly
>requests a suspend.

We should either do the action, or display a warning else I will get lots of bugzillas. :-)
Maybe a libnotify warning might be best "You cannot suspend as application 'Nautilus' is preventing the action. The reason is 'copying files'."

So the inhibit routine in g-p-m should just inhibit the Idle Suspend action, but leave the lcd-dim and dpms alone, as g-s won't ever let the session go idle when inhibited.

That's a good solution. Thanks for the quick answer Jon.

Yeah, I think the user activated suspend should be allowed.  For instance, we allow user activated lock-screen even when inhibited.

This is one reason why I chose to use the term "inhibit" instead of block or prohibit or something else.

Another question about dbus, probably for David:

>Need to do refcounting for multiple callers

Why? Wouldn't each new inhibit request be done from a new connection? Or would two nautilus windows each doing a long copy use the same dbus connection?

> Why? Wouldn't each new inhibit request be done from a new connection? Or would
> two nautilus windows each doing a long copy use the same dbus connection?

That's very likely yes especially if this is implemented from gnome-vfs-daemon rather than Nautilus. 

Saying that you can only inhibit sleep once per dbus connection is just bad design because connections are shared; for example this ruins in-process plug-in architectures. 

Example: for Nautilus someone writes a Nautilus extension that also uses InhibitSleep() but this is the same process and thus uses the same DBusConnection to the session message bus. 

For this reason you want the API to look something like this

 uint32 InhibitSleep(uint32 flagsOnWhatToInhibit) # returns cookie
 void UnInhibitSleep(uint32 cookie)

where the cookie is just a unique number generated on the fly by g-p-m. You need this for UnInhibitSleep to be able to look up what flags to remove when un-inhibiting sleep. Since you may have multiple callers of InhibitSleep stacked you probably want to use the union of the given flags.

Hope this makes sense.

>where the cookie is just a unique number generated on the fly by g-p-m.

Yes, I was wondering this, I think this is something we need to do.

Why do we need flagsOnWhatToInhibit, when all we need to disable is the suspend on idle timeout and the suspend on lid close?

Also, InhibitSleep needs to know the application name (unless we can get this from dbus) and also the reason why it's being inhibited so we can do some clever things if the user clicks suspend and and we need to explain why (in the logs or in a libnotify window) the suspend was cancelled.

Hmm, connection sharing.  That means we need something like this in g-s too. 
And probably libhal.

libhal is a bit different since device locking is exclusive but seems like the
way it is now if a gnome-vfs module took a lock on a device and another module
might be able to unlock it.

Anyway, this API looks good to me.  I guess I'll have to break the g-s API
during the 2.15 series to match this.

>I guess I'll have to break the g-s API

Up to you, but don't do it yet, as the interface to g-p-m is still changing.

It might be worth saying that if an application *knows* it's only ever going to inhibit once, then not to save the cookie, and then to call Allow____ with a cookie of 0 (meaning "all") -- allowing a simple program like totem to not worry about storing a cookie.

Or maybe the API should be robust, where the cookie thing is enforced. 

comment 9:
> Why do we need flagsOnWhatToInhibit, when all we need to disable is the 
> suspend on idle timeout and the suspend on lid close?

Oh, you don't need them then I guess. 

> Also, InhibitSleep needs to know the application name (unless we can get
> this from dbus) 

You can't.

> and also the reason why it's being inhibited so we can
> do some clever things if the user clicks suspend and and we need to
> explain why (in the logs or in a libnotify window) the suspend was
> cancelled.

Right, such as "burning a DVD". Make sure to specify in the API docs that you need to localized reason and this might be presented in the UI. Question; do you want apps to update the reason, e.g. "burning a DVD, 45% complete" so the potential g-p-m UI is updated on the fly? Maybe, maybe not.. (I tend to think maybe not).

Also, several applications might be inhibiting at once (Nautilus copying files, n-c-b burning a CD); for the UI you may or may not want to show this... Maybe you do, I think so...

Btw, said UI should probably contain a button "Do the action anyway, dammit!" as it does work as reported in bug 334806. And users would get pissed of if we didn't provide this action. Maybe needs a small "warning: things may break etc." disclaimer besides the button.

Btw, you only get the UI if you explitcitly press the Suspend button on the computer right? Ie. we don't get it for idleness..

comment 10:
> libhal is a bit different since device locking is exclusive but seems like the
> way it is now if a gnome-vfs module took a lock on a device and another module
> might be able to unlock it.

Yea, that's true in theory but probably I would just say an application shouldn't do that. It would be nice to fix hal too, not sure if it's that big a priority though... and btw.. it's fixable by apps using a private (not shared) connection to the system message bus... But I will fix this when we break API the next time :-)

>Question; do you want apps to update the reason

No, I don't think this should be for that sort of interactive notification.

I've committed this, which works towards what we've discussed so far:

2006-03-17  Richard Hughes  <richard@hughsie.com>
 * src/gpm-manager.{c|h}, docs/dbus-interface.txt, src/gpm-manager.xml, src/Makefile.am: Update with the new code for the inhibit action.
 * src/gpm-inhibit.{c|h}, docs/dbus-test-inhibit.py: Add these new files so we can start to write the inhibit framework further described in #334809. InhibitInactiveSleep and AllowInactiveSleep work, but don't do anything in the current setup. More to come...

Thanks for your quick responses and help today, it's very much appreciated.

Created attachment 61504 [details]
screenshot showing the test script in action

Okay, I had a spare couple of hours today and added near complete inhibit support to g-p-m. There's also a test program, dbus-test-inhibit.py, that you can play with:

2006-03-18  Richard Hughes  <richard@hughsie.com>
 * src/gpm-inhibit.{c|h}: Add gpm_inhibit_check() so we can see if it's okay to do the action. Also add gpm_inhibit_get_message() to get a full description of why the action was inhibited.
 * src/gpm-manager.c: Add a helper function, gpm_manager_is_inhibit_valid () that checks validity, and shows a libnotify type window explaining the problem. Add this helper where we currently use gpm_manager_is_policy_timout_valid(), except the low power battery condition which must be done regardless of any veto.

I still need to track disconnects for crashing programs, but it handles multiple requests from one connection, and lots of other funky error sequences.

I've added some of this information to the g-p-m FAQ page, http://live.gnome.org/GnomePowerManager/Faq

Feel free to edit and correct where required.

>I still need to track disconnects for crashing programs

A bit of refactoring was required:

2006-03-19  Richard Hughes  <richard@hughsie.com>
 * src/gpm-hal-monitor.{c|h}: Remove the DBUS n-o-c functionality.
 * src/gpm-dbus-monitor.{c|h}, src/Makefile.am, src/gpm-marshall.list: Add these files as this functionality has been refactored from gpm-hal-monitor. Add the facility to monitor session name-owner-change as well as system, as we need this for the auto-remove inhibits when the caller crashes or forgets to remove the inhibit request.
 * src/gpm-inhibit.{c|h}: Add gpm_inhibit_remove_dbus which does a forced removal if already existing in the inhibit database.
 * src/gpm-manager.c: Watch the session dbus, and call gpm_inhibit_remove_dbus when a application quits from the session bus.
 * src/gpm-power.{c|h}: Proxy the dbus n-o-c, so we can deal with hal's disappearance from the system bus like we used to.


Before I mark this bug FIXED, is there a better name for InhibitInactiveSleep?

I'll close this bug as it works well with 2.15.0 -- Please open a new bug with any specific issues if there are problems/enhancements. Thanks.

Note to anybody watching this bug:

The InhibitInactiveSleep() method has now become Inhibit() and AllowInactiveSleep() has become UnInhibit(). This is part of on-going standardisation from the XDG people for cross-distro compatability. See http://cvs.gnome.org/viewcvs/*checkout*/gnome-power-manager/docs/gnome-power-manager.html#pm-Compatibility for other API changes that may affect you.

Thanks,

Richard.