[Kde-hardware-devel] Review Request 116527: Properly check logind suspend-related interfaces

[Timothée Ravier]

> On March 3, 2014, 1:35 p.m., Lukáš Tinkl wrote:
> > powerdevil/daemon/backends/upower/powerdevilupowerbackend.cpp, line 267
> > <https://git.reviewboard.kde.org/r/116527/diff/1/?file=251312#file251312line267>
> >
> >     Why are you checking for this property? Are you sure it was introduced together with the signal below?
> 
> Timothée Ravier wrote:
>     You're right, I should check for the signal been available instead (if I understand correctly what's written here: http://www.freedesktop.org/wiki/Software/systemd/logind/). I'll have a look as I don't know if it's possible to check for a signal availability with DBus (except using introspection).
> 
> Timothée Ravier wrote:
>     Thiago told me on IRC (#dbus) that I should not use introspection and that they are no other way to check if a signal is available except waiting for it. I'm going to have a look in systemd to make sure that this variable was introduced at the same time as the signal.

I asked for clarifications on the systemd mailing list: http://lists.freedesktop.org/archives/systemd-devel/2014-March/017566.html


- Timothée


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


On March 2, 2014, 3:08 a.m., Timothée Ravier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116527/
> -----------------------------------------------------------
> 
> (Updated March 2, 2014, 3:08 a.m.)
> 
> 
> Review request for Solid.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> -------
> 
> Properly check logind suspend-related interfaces by calling DBus methods, getting properties and checking for errors. This is recommended by upstream:
> http://lists.freedesktop.org/archives/systemd-devel/2014-February/017403.html
> http://lists.freedesktop.org/archives/systemd-devel/2014-February/017452.html
>     
> Replace logic introduced in:    
> https://git.reviewboard.kde.org/r/114393/    
> https://git.reviewboard.kde.org/r/115271/
> 
> This also removes the systemd and upstart version checks.
> 
> Note: I do not have any non-systemd system to check the other paths in the code.
> 
> 
> Diffs
> -----
> 
>   powerdevil/daemon/backends/upower/dbus/org.freedesktop.login1.xml PRE-CREATION 
>   powerdevil/daemon/backends/upower/dbus/com.ubuntu.Upstart.xml dc7ae42 
>   powerdevil/daemon/BackendConfig.cmake 41c8812 
>   powerdevil/daemon/backends/upower/login1suspendjob.h e1c2ff4 
>   powerdevil/daemon/backends/upower/login1suspendjob.cpp 54371c8 
>   powerdevil/daemon/backends/upower/powerdevilupowerbackend.h c6563b0 
>   powerdevil/daemon/backends/upower/powerdevilupowerbackend.cpp 94b20f1 
> 
> Diff: https://git.reviewboard.kde.org/r/116527/diff/
> 
> 
> Testing
> -------
> 
> All tests were done running systemd 210 on Arch Linux:
> 
> * With kdebase-workspace 4.11.6-2 from official repositories, suspend is not available;
> * With kdebase-workspace git branch KDE/4.11 with commit 5dc2b9a5c09d95ba9a17ac00056213082feb8d90 reverted, suspend is not available;
> * With kdebase-workspace git branch KDE/4.11 with commit 5dc2b9a5c09d95ba9a17ac00056213082feb8d90, suspend is available;
> * With kdebase-workspace git branch KDE/4.11 with the patch, suspend is available;
> 
> 
> Thanks,
> 
> Timothée Ravier
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-hardware-devel/attachments/20140304/1b25287b/attachment.html>