[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-hardware-devel
Subject: Re: [Kde-hardware-devel] Backlight control support
From: Dario Freddi <drf54321 () gmail ! com>
Date: 2013-06-05 14:01:05
Message-ID: CAFFVnfO=mLWDha2rNrcZnKrNPxDE5cY4ZNZHg29tDmg7sFyebA () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
Yo,
2013/5/25 Kai Uwe Broulik <kde@privat.broulik.de>
>
> I'm currently working on revamping the battery monitor and found that the
> algorithm that is supposed to hide brightness slider in case backlight
> controls are not supported in 4.11 doesn't work and always returns true
> leading the a sort of unchanged situation.
> My new battery monitor tries to be a lot cleaner and so having proper
> backlight support detection is important for that. Especially since it will
> have keyboard backlight support too, and having two useless sliders is bad.
> (Okay, keyboard backlight afaik is properly detected).
>
Much appreciated :)
> My analysis shows:
> Problem 1)
> In powerdevil/daemon/backends/upower/powerdevilupowerbackend.cpp always
> does
> controls.insert(QLatin1String("LVDS1"), Screen);
> no matter whether the checks a bit earlier have failed. In the action
> itself
> the isSupported() function checks whether brightnessControlsList is empty,
> which is never the case because it alawys contains LVDS1.
>
I honestly don't remember why that code was there, and probably it wasn't
even me who wrote it. Anyway. Maybe the work in KScreen might be useful to
identify which output to pilot when dealing with brightness, and might have
the interesting consequence of piloting brightness on different outputs.
Again, I never really took care of that area specifically, so I might be
completely out of track.
> Problem is also, if the brightness controls are not available, PowerDevil
> spam
> you with the "The profile foo tried to load action BrightnessControls
> which is
> a non-existent action.", so this message also needs fixing in this case.
>
This is an interesting consequence of the changed behavior of the
brightness actions, which no longer load upon brightness control being not
supported, and the generator (or previous configs) not being aware of that.
We have two possible fixes here:
* Simply ignore the fact the config file might have bogus action sections,
and skip them over
* Fix the generators and make the loader aware of the fact some actions
are present, but not loaded.
I personally favor the second, and can provide a fix for that. Thoughts?
> Problem 2)
> The power management engine just connects to the
> /org/kde/Solid/PowerManagement/Actions/BrightnessControls object. And it
> seems
> the connect call *always* returns true, even if the object doesn't even
> exist.
>
Yes, unfortunately QDBusConnection::connect gives no information over the
existence of the signal itself, and we have to live with that...
> So, the dataengine itself always says that backlight controls are
> available,
> regardless of Problem 1. (And also it won't notice if the controls become
> unavailable in the mean time or if they become available after plasma
> started
> for some reason)
>
> Any comment or suggestion?
>
The only reliable way to monitor things on DBus is relying on service
names. What we can do is having the actions register a
org.kde.Solid.PowerManagement.{BrightnessControl,
KeyboardBrightnessControl} service on the session bus and the engine
monitor the existence of both of them. I personally quite favor the
solution and the use case seems sound to me, so I'd go for that, if you
agree.
> I did a few hacks and workarounds in the code mentioned above, it's inside
> the
> plasma/broulik/batterymonitor branch of kde-workspace. It does somewhat
> work
> but is definitly not something for production use or merging.
>
I'll definitely have a look.
>
> Greetings,
> Kai Uwe
> _______________________________________________
> Kde-hardware-devel mailing list
> Kde-hardware-devel@kde.org
> https://mail.kde.org/mailman/listinfo/kde-hardware-devel
>
[Attachment #5 (text/html)]
<div dir="ltr">Yo,<div class="gmail_extra"><br><div class="gmail_quote">2013/5/25 Kai \
Uwe Broulik <span dir="ltr"><<a href="mailto:kde@privat.broulik.de" \
target="_blank">kde@privat.broulik.de</a>></span><blockquote class="gmail_quote" \
style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I'm currently working on revamping the battery monitor and found that the<br>
algorithm that is supposed to hide brightness slider in case backlight<br>
controls are not supported in 4.11 doesn't work and always returns true<br>
leading the a sort of unchanged situation.<br>
My new battery monitor tries to be a lot cleaner and so having proper<br>
backlight support detection is important for that. Especially since it will<br>
have keyboard backlight support too, and having two useless sliders is bad.<br>
(Okay, keyboard backlight afaik is properly \
detected).<br></blockquote><div><br></div><div style>Much appreciated :)</div><div> \
</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex">
My analysis shows:<br>
Problem 1)<br>
In powerdevil/daemon/backends/upower/powerdevilupowerbackend.cpp always does<br>
controls.insert(QLatin1String("LVDS1"), Screen);<br>
no matter whether the checks a bit earlier have failed. In the action itself<br>
the isSupported() function checks whether brightnessControlsList is empty,<br>
which is never the case because it alawys contains \
LVDS1.<br></blockquote><div><br></div><div style>I honestly don't remember why \
that code was there, and probably it wasn't even me who wrote it. Anyway. Maybe \
the work in KScreen might be useful to identify which output to pilot when dealing \
with brightness, and might have the interesting consequence of piloting brightness on \
different outputs. Again, I never really took care of that area specifically, so I \
might be completely out of track.</div> <div> </div><blockquote class="gmail_quote" \
style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Problem is \
also, if the brightness controls are not available, PowerDevil spam<br> you with the \
"The profile foo tried to load action BrightnessControls which is<br> a \
non-existent action.", so this message also needs fixing in this \
case.<br></blockquote><div><br></div><div style>This is an interesting consequence of \
the changed behavior of the brightness actions, which no longer load upon brightness \
control being not supported, and the generator (or previous configs) not being aware \
of that. We have two possible fixes here:</div> <div style><br></div><div style> * \
Simply ignore the fact the config file might have bogus action sections, and skip \
them over</div><div style> * Fix the generators and make the loader aware of the fact \
some actions are present, but not loaded.</div> <div style><br></div><div style>I \
personally favor the second, and can provide a fix for that. Thoughts?</div><div> \
</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex">
Problem 2)<br>
The power management engine just connects to the<br>
/org/kde/Solid/PowerManagement/Actions/BrightnessControls object. And it seems<br>
the connect call *always* returns true, even if the object doesn't even \
exist.<br></blockquote><div><br></div><div style>Yes, unfortunately \
QDBusConnection::connect gives no information over the existence of the signal \
itself, and we have to live with that...</div> <div> </div><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> So, the dataengine itself always says that backlight \
controls are available,<br> regardless of Problem 1. (And also it won't notice if \
the controls become<br> unavailable in the mean time or if they become available \
after plasma started<br> for some reason)<br>
<br>
Any comment or suggestion?<br></blockquote><div><br></div><div style>The only \
reliable way to monitor things on DBus is relying on service names. What we can do is \
having the actions register a org.kde.Solid.PowerManagement.{BrightnessControl, \
KeyboardBrightnessControl} service on the session bus and the engine monitor the \
existence of both of them. I personally quite favor the solution and the use case \
seems sound to me, so I'd go for that, if you agree.</div> <div> \
</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> I did a few hacks and workarounds in the code mentioned \
above, it's inside the<br> plasma/broulik/batterymonitor branch of kde-workspace. \
It does somewhat work<br> but is definitly not something for production use or \
merging.<br></blockquote><div><br></div><div style>I'll definitely have a \
look.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 \
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Greetings,<br>
Kai Uwe<br>
_______________________________________________<br>
Kde-hardware-devel mailing list<br>
<a href="mailto:Kde-hardware-devel@kde.org">Kde-hardware-devel@kde.org</a><br>
<a href="https://mail.kde.org/mailman/listinfo/kde-hardware-devel" \
target="_blank">https://mail.kde.org/mailman/listinfo/kde-hardware-devel</a><br> \
</blockquote></div><br></div></div>
_______________________________________________
Kde-hardware-devel mailing list
Kde-hardware-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-hardware-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic