Review Request 129651: [dragon] Inhibit powersave

Harald Sitter sitter at kde.org
Fri Dec 16 13:41:59 GMT 2016



> On Dec. 14, 2016, 10:15 a.m., Harald Sitter wrote:
> > Inhibiting is so very bad... 
> > 
> > - org.freedesktop.PowerManagement is deprecated https://www.freedesktop.org/wiki/Specifications/power-management-spec/ there seems to be no information as to what it was deprecated in favor of though
> > - we could use org.kde.Solid.PowerManagement but that would not work on !plasma as I understand it
> > - we could use org.freedesktop.login1 https://www.freedesktop.org/wiki/Software/systemd/inhibit/ which would work everywhere but on plasma, as apparently powerdevil tells logind to shut up
> > - there's this unlanded RR https://git.reviewboard.kde.org/r/126650/ which would kinda solve our problem, albeit also using org.freedesktop.PowerManagement
> > 
> > Where to go from here I do not actually know. I'd rather not use org.freedesktop.PowerManagement given its depreated state, at the same time I am somewhat inclined to not care given every source code I checked is using it anyway. that being said the RR currently doesn't inhibit screensavers. 
> > 
> > ---
> > 
> > As for the merge problem. When you commit to maintenance branches (i.e. Applications/) you need to manually merge them into master or any other relevant branches as well. There is no automatic merge and I am also only merging into master on-demand, when I make a change myself. To fix this it appears you can simply merge Applications/16.04 into Applications/16.12 and then Applications/16.12 into master
> 
> Anthony Fieroni wrote:
>     QDBusInterface iface(QLatin1String("org.freedesktop.login1"),
>                          QLatin1String("/org/freedesktop/login1"),
>                          QLatin1String("org.freedesktop.login1.Manager"),
>                          QDBusConnection::systemBus());
>      reply = iface.call(QLatin1String("Inhibit"),
>                         QLatin1String("sleep:shutdown"),
>                         i18n("Dragon Player"),
>                         i18n("playing a video"),
>                         QLatin1String("block"));
>     does not work

How does it not work? No return value? It seems to be working with qdbus running on neon:

`qdbus --system org.freedesktop.login1 /org/freedesktop/login1 org.freedesktop.login1.Manager.Inhibit sleep:shutdown dragon video block`

results in journalctl

```
Dez 16 13:58:10 smith systemd-logind[3214]: Inhibitor dragon (video) pid=12609 uid=1000 mode=block started.
Dez 16 13:58:10 smith systemd-logind[3214]: Sent message type=signal sender=n/a destination=n/a object=/org/freedesktop/login1 interface=org.freedesktop.DBus.Properties member=PropertiesChanged cookie=193 reply_cookie=0 error=n/a
Dez 16 13:58:10 smith systemd-logind[3214]: Sent message type=method_return sender=n/a destination=:1.318 object=n/a interface=n/a member=n/a cookie=194 reply_cookie=5 error=n/a
Dez 16 13:58:10 smith systemd-logind[3214]: Inhibitor dragon (video) pid=12609 uid=1000 mode=block stopped.
```

(supposedly the block is released when the pid disappears)


> On Dec. 14, 2016, 10:15 a.m., Harald Sitter wrote:
> > src/app/mainWindow.cpp, line 820
> > <https://git.reviewboard.kde.org/r/129651/diff/1/?file=487471#file487471line820>
> >
> >     I am pretty sure the inhibition port is lackluster. Notably KnotificationRestrictions inhibited screensaving which I am assuming doesn't fall into the PowerManagement domain so the new code while inhibiting sleeping it seems ot miss screensaver stuff.
> >     
> >     Fortunately knotificationrestrictions is ported and you can just keep on using it.
> 
> Anthony Fieroni wrote:
>     Can we use only knotificationrestrictions, but sleep/suspend to not handle at all?

We could do that and wait for kidletime to fix the sleep/suspend problem for us via the RR I mentioned (assuming that ever happens).

It depends on how much time you want to spend on this really.
If you want broadest possible support I'd implement a bunch of "Inhibitor" classes (FDOInhibitor, PowerdevilInhibitor, Login1Inhibitor...) and have them contain inhibit logic for all the possible things and then call all of them to inhibit/uninhibit.

That is a lot of code though, so I suggest that you go with what you already posted here and maybe switch to a generated dbus adaptor as mentioned in the techbase tutorial I linked to. Also add back KNotificationRestriction as previously used. That way the code should be covering the majority of desktops/OS combinations.


- Harald


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


On Dec. 14, 2016, 4:49 a.m., Anthony Fieroni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129651/
> -----------------------------------------------------------
> 
> (Updated Dec. 14, 2016, 4:49 a.m.)
> 
> 
> Review request for KDE Multimedia and Harald Sitter.
> 
> 
> Repository: dragon
> 
> 
> Description
> -------
> 
> Since solid power api is not released, i use freedesktop dbus to fit other desktop power managers
> 
> 
> Diffs
> -----
> 
>   src/app/mainWindow.cpp 4f0b3e0 
> 
> Diff: https://git.reviewboard.kde.org/r/129651/diff/
> 
> 
> Testing
> -------
> 
> *Harald master was not merged (16.04 / 16.08 to master) -> it has fresh bugreports for fixed issues.
> Last merge to 16.08
> 2016-04-18	Merge branch 'Applications/16.04'	Harald Sitter
> after that in brach 16.04
> 2016-04-26	Hide toolbar on multiscreen setup	Anthony Fieroni
> 2016-04-20	Don't try to resize maximized window	Anthony Fieroni
> 2016-04-19	MPRIS2 property 'Identity' to match desktop file entry	Anthony Fieroni
> 
> Do you want to merge them or you will ship ?
> 
> 
> Thanks,
> 
> Anthony Fieroni
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-multimedia/attachments/20161216/18d2b5c5/attachment.htm>


More information about the kde-multimedia mailing list