> On Jan. 9, 2015, 12:25 p.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.
> 
> Àlex Fiestas wrote:
>     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).
> 
> Martin Klapetek wrote:
>     > 2-It might create a deadlock since kscreen is a kded module asking to 
> another module (powerdevil) for things.
>     
>     As dfaure explained in https://git.reviewboard.kde.org/r/121885/ QtDbus 
> is really smart and in-process DBus calls are made into direct methods and 
> should never deadlock. Which is cool :)
> 
> Àlex Fiestas wrote:
>     Oh wow! One less problem to worry about then.
>     We still need to be careful not to deadlock by doing sync calls between 
> the same kded modules (happened already).

That should still work no? I mean if sync QtDBus works in-process, then it 
doesn't matter which modules it calls to as it is all the same process...right?


- Martin


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


On Jan. 8, 2015, 1: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 Jan. 8, 2015, 1: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
> 
>

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to