Review Request: Display brightness OSD notification and change brightness when Fn keys are unhandled

[Aaron Seigo]

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


"This is suboptimal but KDE doesn't provide a generic way to show this kind of OSD yet."

if you'd like to work on a shared OSD once this patch set is in, let me know. i have some thoughts on where this could go and how it could be done and would be happy to support you in the goal of getting a desktop OSD service. basically, i think this should be driven by a very simple dbus service (which could eventually be fd.o'd) that is implemented in krunner (which already links against libplasma and hosts other desktop services like triggering the screen saver and launch feedbak) ... it would be a simple matter of being able to define which OSD type to show (volume, brightness, etc) and what value (0.0 -> 1.0) to display in it. then krunner would throw up the appropriate OSD.

all of this could be hidden behind a class in kdelibs if we wished to make it even easier to use for apps.

let me know if you're interested in working on this :)

- Aaron


On 2010-05-05 20:58:51, Felix Geyer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1942/
> -----------------------------------------------------------
> 
> (Updated 2010-05-05 20:58:51)
> 
> 
> Review request for Plasma and Dario Freddi.
> 
> 
> Summary
> -------
> 
> This patch addresses the following issues:
> - KDE doesn't provide an OSD notification when the user changes the
> display brightness.
> - The brightness Fn keys don't have an effect if they aren't handled by
> the kernel/hardware.
> 
> Implementation overview:
> PowerDevil uses a KActionCollection to register the Qt::Key_MonBrightnessUp/Down keys
> and forwards them to Solid. 
> Solid then decides if the brightness should be increased/decreased by comparing the
> current brightness value with the cached one. The brightness is only changed
> if both are equal and the brightness_in_hardware HAL property is false.
> This prevents a double increase/decrease of the brightness and is consistent with
> gnome-power-manager using the HAL backend.
> In any case PowerDevil emits the brightnessChanged() dbus signal.
> The battery applet listens to that signal and displays an OSD.
> It uses an adapted copy of the OSDWidget class from KMix. This is suboptimal but KDE
> doesn't provide a generic way to show this kind of OSD yet.
> 
> 
> This addresses bugs 182400 and 187960.
>     https://bugs.kde.org/show_bug.cgi?id=182400
>     https://bugs.kde.org/show_bug.cgi?id=187960
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdebase/workspace/libs/solid/control/ifaces/powermanager.h 1122901 
>   trunk/KDE/kdebase/workspace/libs/solid/control/powermanager.h 1122901 
>   trunk/KDE/kdebase/workspace/libs/solid/control/powermanager.cpp 1122901 
>   trunk/KDE/kdebase/workspace/plasma/generic/applets/battery/CMakeLists.txt 1122901 
>   trunk/KDE/kdebase/workspace/plasma/generic/applets/battery/battery.h 1122901 
>   trunk/KDE/kdebase/workspace/plasma/generic/applets/battery/battery.cpp 1122901 
>   trunk/KDE/kdebase/workspace/plasma/generic/applets/battery/brightnessosdwidget.h PRE-CREATION 
>   trunk/KDE/kdebase/workspace/plasma/generic/applets/battery/brightnessosdwidget.cpp PRE-CREATION 
>   trunk/KDE/kdebase/workspace/powerdevil/daemon/PowerDevilDaemon.h 1122901 
>   trunk/KDE/kdebase/workspace/powerdevil/daemon/PowerDevilDaemon.cpp 1122901 
>   trunk/KDE/kdebase/workspace/powerdevil/daemon/org.kde.PowerDevil.xml 1122901 
>   trunk/KDE/kdebase/workspace/solid/hal/halpower.h 1122901 
>   trunk/KDE/kdebase/workspace/solid/hal/halpower.cpp 1122901 
> 
> Diff: http://reviewboard.kde.org/r/1942/diff
> 
> 
> Testing
> -------
> 
> This patch in a slightly different implementation (some code moved from PowerDevil to Solid)
> is being shipped with Kubuntu Karmic and Lucid and I think Fedora. I haven't seen any bug reports
> concering the OSD or incorrect brightness changes.
> 
> Apart from that I tested the changes on my laptop and have verified that the code correctly
> respects the HAL property.
> 
> 
> Thanks,
> 
> Felix
> 
>