D-Bus interface is hard to use

Bug #1333632 reported by Lars Karlitski
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
powerd
New
Undecided
Unassigned
powerd (Ubuntu)
New
Undecided
Unassigned

Bug Description

Powerd's dbus API [1] is unnecessarily hard to use from clients.

Its methods are named with lowerCamelCase instead of following the usual UpperCamelCase naming convention, while the signals are using UpperCamelCase.

Passing integer flags for requestSysState() and requestDisplayState() might make it hard to debug those functions, as you'd always have to remember the values of the flags. Also, the 'Sys' in requestSysState() and the SysPowerStateChange() signal could be spelled out.

requestDisplayState() and updateDisplayState() could be merged by passing an invalid cookie in the first call. What's the reason for having a cookie instead of binding the state to the client's connection? Do we expect some clients to request multiple states?

getBrightnessParams() returns a tuple of parameters. This makes it hard to debug, since you need to know the positions for each parameter. Also, if an item is added to the tuple, all clients will have to be updated to take this into account. This could be solved by returning a dictionary or (even better) using dbus properties.

AutoUserBrightnessEnable() is not named in the same convention as the other setters. It could be named SetAutoUserBrightness(). Having that function return a boolean to denote whether auto-brightness is available at all would save a round-trip, because a client wouldn't need to call getBrightnessParams() first to find out.

[1] https://wiki.ubuntu.com/powerd#API

Revision history for this message
Allison Karlitskaya (desrt) wrote :

On a somewhat higher level, the separate concepts of state requests and state change acknowledgement, and their interaction, are very underspecified.

For example, what happens when a client fails to acknowledge a state change? What if a client is only interested in some types of changes? How are the clients identified (ie: why no cookies on this part of the API)?

It might be useful to look at what logind has done in the same problem space -- it has a far more fleshed-out API. It decided to merge these two concepts and allow establishing two types of locks on each type of event -- one for demanding a particular state and one for notification about that state:

  http://www.freedesktop.org/wiki/Software/systemd/inhibit/

Note that the details of what happens when clients misbehave are well spelled-out.

It also has a very nice mechanism of avoiding cookies and the need to unregister them: it uses file descriptors. When you close the fd (which automatically happens on program crashes, for example) then the request is cancelled.

The additional things that exist on the powerd API could be introduced as additional 'what' type (proximity sensor, etc).

About those things: the interaction between different requests from clients should be better documented. What if one client asks for the display to be on and another asks for the proximity detector to turn the display off? Does it matter which app is in the foreground or does one request always override the other?

One more high level 'flavour' comment: the comment in the API doc about "If this is the last request to keep the screen on, it will turn off. " is not what you want. Presumably you have some sort of hooks into the display server to monitor for user inactivity -- what you want to do is to restart the inactivity timer at the point that the last request is dropped.

Consider that I am playing a video and the video comes to an end. The video player releases its "keep the screen on" lock. I don't want the screen to turn off immediately in this case, but rather I'd like if the usual idle rules came into play. We can't do this just by having idle being considered as a separate thing ,either, because if the video was long enough, the user input has probably already been idle for a long time. We actually need to restart the counter from 0 when the last request drops.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.