> 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.
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.
- 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
>
>
_______________________________________________
Kde-hardware-devel mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/kde-hardware-devel
