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

Felix Geyer debfx-kde at fobos.de
Wed May 5 22:58:52 CEST 2010


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

(Updated 2010-05-05 20:58:51.762208)


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



More information about the Plasma-devel mailing list