Exaile should be able to inhibit suspend/hibernate while playing

[dfalk]

If exaile is playing music, it should prevent suspension of the PC by default. I believe this is the expected behavior for music apps. If not, please make it an option.

[reacocard (reacocard)]

IMO inhibiting sleep by default is a bad idea. However a plugin to do so is perfectly feasible (and easy), feel free to contribute one.

[dfalk (dfalk)]

I attached one. Thanks for the great plugin documentation. The plugin was very easy to make.

I still like the idea and it can sometimes be useful. (Consider the case where I'm playing from a computer which normally suspends on inactivity, but I'd like to listen to music on that computer while I'm doing something else in the room.) AFAIK, totem and rhythmbox both inhibit suspend during playback as well.

[reacocard (reacocard)]

That was fast. :D Thanks, I'll be sure to get it into the next release of Exaile.

[dfalk (dfalk)]

Why was this changed to 0.3.x and unassigned? Is it still expected to go in?

[Mathias Brodala (mathbr)]

Could you port the plugin to Exaile’s new plugin API?

[Ciaran Liedeman (ciaran-liedeman)]

This suspend plugin works for the latest trunk

[Ciaran Liedeman (ciaran-liedeman)]

Here is a small script that checks if Suspension is inhibited

[Mathias Brodala (mathbr)]

Looks very good. You can drop the deferred enable call since you are not accessing any GUI.

One question: do we need to fix the plugin to the GNOME session manager? The original plugin was using the f.d.o PowerManagement interface. Does the GNOME session manager not implement this interface? We either need a general solution or possible fallbacks for other desktops.

[Ciaran Liedeman (ciaran-liedeman)]

Thanks for the feedback

According to https://bugs.launchpad.net/ubuntu/+source/gnome-power-manager/+bug/554899
the interface was removed some time ago -- from Gnome not Ubuntu if I'm not mistaken.
 If someone running a non Ubuntu Linux could confirm that would help as the documentation is scarce.

I will remove the deferred call if no one is interested in suspending idling - preventing the screen saver from running - as some users might want this functionality but most won't, as then I'll need to add check boxes.

Unless check boxes in the plugin options don't need the deferred call?

[Mathias Brodala (mathbr)]

I’ve asked on xfce-dev what kind of interface their power manager offers.

You are accessing the settings in a non-GUI way so you don’t have to bother with the GUI for them.
The deferred enable() call is mainly useful if you actually want to access the GUI main window.

[reacocard (reacocard)]

More specifically, deferred enable is used if you need to access in your enable that would not be present at the point plugins are loaded (ie. if you need state other than settings, events, and providers). This is in practice used mostly for adding things to the UI, but there are other events for the load of individual components of exaile (player_loaded, playlists_loaded, etc.) if such a point would be more appropriate for your plugin. Take a look at xl/main.py for a better idea of how the startup goes and what events are available.

In this particular case, as Mathias says, you don't access anything that would require a deferred handler, so you can just remove that code and enable() directly.

One minor stylistic point - please do not use all caps in the logging messages. E.g. instead of "INHIBIT SUSPEND: Inhibiting", something like "Suspend inhibited" would be more appropriate.

[Ciaran Liedeman (ciaran-liedeman)]

Changed logging style, removed deferred call and added locking - That information related to the deferred call should really be added to the plugin development wiki page

[reacocard (reacocard)]

Sorry for not being clear on this the first time - don't prefix the log messages like that either. When running with --debug the module name is automatically added to the log anyway, and adding a prefix like that makes the style of your logs inconsistent with the rest of the logging output. Also some of those logging messages seem unnecessary - eg. if there's a playback error, we dont need a message from the inhibitor telling us there's an error, the playback system itself will handle that. Logging messages should inform about important events/state-changes in the subsystem they come from, not other subsystems. In this case, I think there are only three things that need to be logged: 1) initialized - connected successfully to the inhibition system 2) inhibited - suspend is now inhibited 3) uninhibited - suspend is now uninhibited.

I also found a bug - if you try to enable the plugin when suspend inhibition is not available, it fails to load with a cryptic message about DBUS interfaces. It would be better if you displayed a proper message explaining the problem (raise NotImplementedError with an appropriate string to do so, see moodbar and lyriciwiki plugins for examples), or just logged the error and enabled anyway. I prefer the second approach so that users switching between, say, GNOME and openbox don't have to reenable the plugin every time they switch, but that's your call.

Feel free to add to the wiki page - that's why its a wiki after all :) There's also a page for style guidelines here: http://exaile.org/wiki/Exaile:CodeGuidelines , I'll try to add some better guidelines for logging to that.

[Mathias Brodala (mathbr)]

To summarize what I have found:

1) The org.freedesktop.PowerManagement interface should still be used. The methods mostly relevant here:

org.freedesktop.PowerManagement.Inhibit.HasInhibit
org.freedesktop.PowerManagement.Inhibit.HasInhibitChanged
org.freedesktop.PowerManagement.Inhibit.Inhibit
org.freedesktop.PowerManagement.Inhibit.UnInhibit

2) Using Xlib as fallback if no compliant power manager is available should also be considered. I’ll attach a small script which shows how I expect it to work, please let me know if it works at all.

[Ciaran Liedeman (ciaran-liedeman)]

I noticed that too when I tested the patch on different desktop environments

The new patch works on Ubuntu, Kubuntu and Xubuntu 10.10.

Xubuntu and Kubuntu use the org.freedesktop.PowerManagement interfaces falling back to the same interfaces at differnt bus names
if org.freedesktop.PowerManagement isn't available, i.e. org.xfce.PowerManager and org.kde.powerdevil.

Gnome uses its own interface and does not have a fall back that I know of.

The dbus interfaces are fairly standard and the PowerManagerAdapter should work on most recent as they rarely change

[Mathias Brodala (mathbr)]

I am willing to add this plugin but first would like to see a few more comments confirming this plugin is working as expected on GNOME, KDE and Xfce.

[Mathias Brodala (mathbr)]

Please add comments whether this works expected on all common desktop environments.

[Dustin Spicuzza (dustin-virtualroadside)]

This plugin looks like it's well written, and I'm inclined to take this for 0.3.3 as is if there are no objections. Presumably it works on the author's platform -- if it doesn't work on a particular desktop environment, presumably someone who cares will complain.

[Dustin Spicuzza (dustin-virtualroadside)]

Since there were no comments, I've committed this in revision r4081. If it doesn't work for someone, then file a bug and we'll get it fixed.