Only add DPMS to the profile if it is supported

Review Request #107503 - Created Nov. 28, 2012 and submitted

Information
Kai Uwe Broulik
kde-workspace
Reviewers
solid
oliverhenshaw
I found out that the Powerdevilprofilegenerator which is fired on first start of a KDE session which generates the profile and sets the defaults adds the DPMS action no matter if it is compiled or supported. That patch makes it only add it when it is compiled (HAVE_DPMS) and the action returns isSupported(). This will silence the most prominend warning about missing DPMS and having something in the config file which doesn't work is not good.
Combined with the patch by Oliver Henshaw this will make PowerDevil not stick in your face on startup anymore (except for that Battery Low stuff I added :D which I am thinking of a fix)
Deleted my powermanagementrc, Tested *without* DPMS, works flawlessly. Cannot test with since I do not have DPMS support.

Issues

  • 2
  • 0
  • 0
  • 2
Description From Last Updated
This won't work because isSupported() is a non-static member function - it needs an object. It shouldn't compile, but it ... Oliver Henshaw Oliver Henshaw
I don't understand why, but these ifdef's seem to evaluate to false even when dpms support is available. Oliver Henshaw Oliver Henshaw
Dario Freddi
Commit Hook
This review has been submitted with commit bbe593dbb6fb954c718c268d8a6cc31d8fbedd6f by Kai Uwe Broulik to branch master.
Kai Uwe Broulik
Review request changed

Status: Closed (submitted)

Oliver Henshaw
Sorry, this doesn't look right:
This won't work because isSupported() is a non-static member function - it needs an object. It shouldn't compile, but it does because...
I don't understand why, but these ifdef's seem to evaluate to false even when dpms support is available.
So powerdevilprofilegenerator generates configurations with no DPMS action unconditionally.

What's the best thing to do? Maybe move all the code that touches the DPMS extension (and so needs ifdef'ing) out of powerdevildpmsaction (into non-member functions, maybe in a private include file) leaving only the core action logic. And then the profile generator (and handlebuttoneventsconfig, but that's just cosmetic) could use that. I made a start on a half-baked patch series to do the code movement, but it's currently on the shelf as it raised some questions abut xlib error handling and it didn't seem necessary given the approach in review #107257. I hadn't planned to revisit it until after 4.10 branched.

A slightly quicker fix is just to move the isSupported code to a non-member function. You'd still need to make sure that the x error handling is sane in the function and in the action though (I suspect that using kxerrorhandler from kdelibs/kdeui/util is the best way forward).

The most-short-term option, and the one I favour, is to revert this and try the ifdef'd powerdevildpmsaction stub (from review #106863) instead.


P.S. Are you able to test the DPMS code paths in a virtual machine? Why don't you have DPMS support available anyway?
Loading...