After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 705269 - power: Add Brightness property for keyboard backlights
power: Add Brightness property for keyboard backlights
Status: RESOLVED FIXED
Product: gnome-settings-daemon
Classification: Core
Component: power
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-settings-daemon-maint
gnome-settings-daemon-maint
Depends on:
Blocks: 705272
 
 
Reported: 2013-08-01 09:07 UTC by Bastien Nocera
Modified: 2013-08-02 09:00 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
power: Add Brightness property for keyboard backlights (10.81 KB, patch)
2013-08-01 09:07 UTC, Bastien Nocera
committed Details | Review
power: don't return negative value when succeeded (1.02 KB, patch)
2013-08-01 13:35 UTC, Cosimo Cecchi
accepted-commit_now Details | Review
power: don't mix printf with GVariant parsing (1.40 KB, patch)
2013-08-01 13:35 UTC, Cosimo Cecchi
accepted-commit_now Details | Review
power: always use absolute values to set the keyboard brightness (1.19 KB, patch)
2013-08-01 13:35 UTC, Cosimo Cecchi
committed Details | Review
power: use correct return value type (1.02 KB, patch)
2013-08-01 13:35 UTC, Cosimo Cecchi
accepted-commit_now Details | Review
media-keys: update for new power plugin DBus API (1.06 KB, patch)
2013-08-01 13:36 UTC, Cosimo Cecchi
committed Details | Review

Description Bastien Nocera 2013-08-01 09:07:16 UTC
Necessary for adding it to the Power panel.
Comment 1 Bastien Nocera 2013-08-01 09:07:18 UTC
Created attachment 250592 [details] [review]
power: Add Brightness property for keyboard backlights

This changest the return values of the StepUp/StepDown methods
to match the return values on the similar screen backlight interface.
Comment 2 Cosimo Cecchi 2013-08-01 09:50:35 UTC
Review of attachment 250592 [details] [review]:

::: plugins/power/gsd-power-manager.c
@@ +2007,3 @@
                         /* succeeded, set to -1 since now no old value */
                         manager->priv->kbd_brightness_old = -1;
+                        value = manager->priv->kbd_brightness_old;

Doesn't this change behavior? Previously in this case you would have returned TRUE, but now you return -1.
Comment 3 Cosimo Cecchi 2013-08-01 13:35:35 UTC
Created attachment 250614 [details] [review]
power: don't return negative value when succeeded

This matches the behavior before the previous patch.
Comment 4 Cosimo Cecchi 2013-08-01 13:35:45 UTC
Created attachment 250615 [details] [review]
power: don't mix printf with GVariant parsing

Or we'll segfault trying to parse a string-typed variant for the %s.
Comment 5 Cosimo Cecchi 2013-08-01 13:35:50 UTC
Created attachment 250616 [details] [review]
power: always use absolute values to set the keyboard brightness

The property is a percentage, but we need to pass the absolute value down to upower.
Comment 6 Cosimo Cecchi 2013-08-01 13:35:57 UTC
Created attachment 250617 [details] [review]
power: use correct return value type

We changed this to signed int but forgot to update one location.
Comment 7 Cosimo Cecchi 2013-08-01 13:36:20 UTC
Created attachment 250619 [details] [review]
media-keys: update for new power plugin DBus API

We use signed integers instead of unsigned integers for power percentage now.
Comment 8 Bastien Nocera 2013-08-01 15:41:07 UTC
Review of attachment 250617 [details] [review]:

Yeah
Comment 9 Bastien Nocera 2013-08-01 15:43:15 UTC
Review of attachment 250614 [details] [review]:

Yeah, please merge into the original patch (and change the authorship ;)
Comment 10 Bastien Nocera 2013-08-01 15:44:29 UTC
Review of attachment 250615 [details] [review]:

Definitely yes, but could you please merge the fix before this patchset? It was already broken before my patch.
Comment 11 Bastien Nocera 2013-08-01 15:44:52 UTC
Review of attachment 250616 [details] [review]:

Yep, please merge into previous patch.
Comment 12 Bastien Nocera 2013-08-01 15:45:33 UTC
Review of attachment 250619 [details] [review]:

Yep.
Comment 13 Cosimo Cecchi 2013-08-02 08:53:12 UTC
Attachment 250592 [details] pushed as 5614e2c - power: Add Brightness property for keyboard backlights
Attachment 250616 [details] pushed as f79014f - power: always use absolute values to set the keyboard brightness
Attachment 250619 [details] pushed as 42485c0 - media-keys: update for new power plugin DBus API

Squashed and pushed to master, thanks