Review Request 121915: Add lidClosedChanged signal to org.kde.Solid.PowerManagement

Àlex Fiestas afiestas at kde.org
Fri Jan 9 16:05:45 UTC 2015



> On gen. 9, 2015, 11:25 a.m., Àlex Fiestas wrote:
> > Where do you plan to use this? We are not maintaining compatibility (so far) in our dbus apis, so why add this at all?
> 
> Daniel Vrátil wrote:
>     KScreen. For now we are listening to UPower, which IMO sucks and we should use PowerDevil instead (as it provides abstraction for alternative backends). This makes it just easier to monitor changes.

This has a few problems:
1-It couples KScreen more to PLasma by adding a runtime dependency (which is ok if you decide to do so)
2-It might create a deadlock since kscreen is a kded module asking to another module (powerdevil) for things.
3-We will depend on Powerdevil if we use kscreen in other places (sddm desperatly needs something like kscreen, or kscreen itself)
4-Adds a dependency to an api that is not stable (so far it has not been), we have changed it many times and I am sure we will change it again

My recommendation for this will be to merge the new power management api in Solid and make kscreen depend on it. Solid already offers backends (so we don't have to implement this in kscreen) and we don't have to expose powerdevil internals.

So this is a -1 from my side (if the only reason to add this is KSCreen).


- Àlex


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/121915/#review73569
-----------------------------------------------------------


On gen. 8, 2015, 12:04 p.m., Daniel Vrátil wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121915/
> -----------------------------------------------------------
> 
> (Updated gen. 8, 2015, 12:04 p.m.)
> 
> 
> Review request for Plasma and Solid.
> 
> 
> Repository: powerdevil
> 
> 
> Description
> -------
> 
> There's no way to detect when lid has been closed other than listening to `changed` signal org.freedesktop.UPower and then polling the Powerdevil for new values. This patch adds a new signal to the PowerDevil interface to notify about the change and provide new value right away. Makes it much easier to use.
> 
> 
> Diffs
> -----
> 
>   daemon/org.kde.Solid.PowerManagement.xml 53f77e5 
>   daemon/powerdevilbackendinterface.h 702b66b 
>   daemon/powerdevilbackendinterface.cpp 6dd8c71 
>   daemon/powerdevilcore.h e255703 
>   daemon/powerdevilcore.cpp d51ab19 
> 
> Diff: https://git.reviewboard.kde.org/r/121915/diff/
> 
> 
> Testing
> -------
> 
> Tested with qdbus-monitor, signal is emitted when laptop lid is closed/opened.
> 
> 
> Thanks,
> 
> Daniel Vrátil
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20150109/60d50fff/attachment.html>


More information about the Plasma-devel mailing list