Replace screensaver inhibition by direct dbus calls

Review Request #126627 - Created Jan. 4, 2016 and updated

Information
Martin Klapetek
gwenview
master
334525
Reviewers
gwenview
broulik

This is a better approach to fix bug 334525 than
https://git.reviewboard.kde.org/r/125910/ which forces
a default reason (which is user visible).

With this new patch Gwenview will set a proper reason
why is it inhibitting the screensaver ("Full Screen Presentation")
and will use the DBus directly rather than the obsolete
KNotificationRestrictions API.

PowerDevil correctly shows that Gwenview is inhibitting
the PM because "Full Screen Presentation".

Issues

  • 1
  • 3
  • 1
  • 5
Description From Last Updated
you are not inhibiting the screensaver, but Powermanagment. Similar all the comments are semantically wrong. Martin Flöser Martin Flöser
Martin Klapetek
Kai Uwe Broulik
Martin Klapetek
Review request changed

Change Summary:

Fix issues

Groups:

-kde-baseapps

Diff:

Revision 2 (+58 -7)

Show changes

Albert Astals Cid

Don't we really have something other than this complex code for apps to use? It'd be sad to see people copy&pasting this with subtle mistakes all over :/

  1. There is a thing in solid that is unfinished (there's a big chunk
    of code with "WITH_NEW_POWER_ASYNC_API" cmake switch) and apparently
    no solid maintainer to actually finished it.

    On the other hand, this ain't no complex code, it's a simple dbus
    call with storing the reply and another dbus call sending the stored
    reply. It's a long boring code, but not complex.

  2. we could use your code to turn it into a library ;-)

  3. Like adding a PowerManagement flag to KNotificationRestrictions as well as a new constructor
    KNotificationRestrictions(QString reason, Services control=NonCriticalServices, QObject *parent=0)
    
    Seems a natural solution =)
  4. I actually wanted to get rid of KNotificationRestriction,
    it's used only by Gwenview and somehow really don't belong
    to KNotifications anyway, it's disabling PM after all.

    @Martin G - well there is half finished thing in solid, it
    needs someone to step up to it and decide what to do with it.

  5. => KIdleTime?
    The feature itself could be interesting for okular or some video players as well - esp. in tier1, yesno?

  6. I would have no problem with that. The smaller the lib the
    better for this simple function.

    I'll poke afiestas to see what's missing in that solid thing etc.

Martin Flöser

   
app/mainwindow.cpp (Diff revision 2)
 
 

you are not inhibiting the screensaver, but Powermanagment. Similar all the comments are semantically wrong.

  1. also you might want to also inhibit screensaver?

  2. Hum...and I thought this would do all-in-one. Need to check.

Aurélien Gâteau

As a first step toward turning this into a library, you could move all the dbus code to a separate class.

  1. See https://git.reviewboard.kde.org/r/126650/

  2. Ah cool, looks good!

Loading...