[Kde-hardware-devel] Review Request: Silence "Profile foo could not activate bar which is a non-existent action" message

[Oliver Henshaw]

> On Oct. 24, 2012, 11:09 a.m., Oliver Henshaw wrote:
> > powerdevil/daemon/powerdevilactionpool.cpp, lines 102-109
> > <http://git.reviewboard.kde.org/r/106863/diff/3/?file=90668#file90668line102>
> >
> >     Is isSupported checked everywhere it should be? Remember that this should work when DPMS is configured in but the display doesn't support DPMS (e.g. qxl or vmware displays in a VM)
> 
> Kai Uwe Broulik wrote:
>     I ifdef'd out all the DPMS-related stuff but it would probably also be good to add a
>     if (action) {
>       if (action->isSupported()) {
>         action->onProfileLoad();
>       }
>     } else { …
>     to powerdevilcore so it doesn't load an action if it is not supported.
> 
> Oliver Henshaw wrote:
>     Guarding onProfileLoad won't be enough if/when PowerDevilDPMS action is patched to use KIdleTime timeouts rather than DPMS timeouts. Plus this complicates PowerDevilAction semantics, at least in my view.
>     
>     Sorry I don't have anything more constructive to offer. It would be easier if there were a Solid::Display class to query in the profilegenerator, as in my original suggestion (or maybe the extremely new libkscreen might be more appropriate). Maybe this is more long term work though.
> 
> Oliver Henshaw wrote:
>     Hmm, how about your addition as well as (uncompiled, untested, uncommented):
>     
>     diff --git a/powerdevil/daemon/powerdevilactionpool.cpp b/powerdevil/daemon/powerdevilactionpool.cpp
>     index a9950f1..fae35ee 100644
>     --- a/powerdevil/daemon/powerdevilactionpool.cpp
>     +++ b/powerdevil/daemon/powerdevilactionpool.cpp
>     @@ -125,7 +125,7 @@ Action* ActionPool::loadAction(const QString& actionId, const KConfigGroup& grou
>          if (m_actionPool.contains(actionId)) {
>              Action *retaction = m_actionPool[actionId];
>      
>     -        if (group.isValid()) {
>     +        if (group.isValid() && retaction->isSupported()) {
>      
>                  if (m_activeActions.contains(actionId)) {
>                      // We are reloading the action: let's unload it first then.
>     
>     
>     Which should be right as long as isSupported never changes at run-time and as long as there's not some gotcha I've not seen yet.

Forgot to mention that that diff relies on https://git.reviewboard.kde.org/r/106692/


- Oliver


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106863/#review20781
-----------------------------------------------------------


On Oct. 15, 2012, 4:17 p.m., Kai Uwe Broulik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106863/
> -----------------------------------------------------------
> 
> (Updated Oct. 15, 2012, 4:17 p.m.)
> 
> 
> Review request for Solid.
> 
> 
> Description
> -------
> 
> That message usually appears when starting before the Desktop is up, causing an ugly 1990's passivepopup dialog on the screen, and its contents are not really novice-user-resolvable.
> On my machine it always claims "The profile Battery tried to activate DPMSControl which is a non-existent action.", which is when I compile powerdevil myself that DPMS stuff is not compiled (DPMS build requirements not met here) and so the action floats around in the config but cannot be triggered anyways. (Imho this is a really infamous message, have seen it quite often on other machines *duck*). All the other actions seem to be installed anyways, so this missing action poses no threat. I guess a kWarning would be sufficient for this.
> 
> 
> Diffs
> -----
> 
>   powerdevil/daemon/actions/CMakeLists.txt db9ca47 
>   powerdevil/daemon/actions/dpms/powerdevildpmsaction.cpp a16bf7e 
>   powerdevil/daemon/powerdevilactionpool.cpp 484c271 
> 
> Diff: http://git.reviewboard.kde.org/r/106863/diff/
> 
> 
> Testing
> -------
> 
> Compiles.
> The previous passivepopup does not appear anymore. Did not test whether the kwarning is triggered, though. (Dunno how to get powerdevil debug console output)
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-hardware-devel/attachments/20121024/bed0b46c/attachment-0001.html>