Bug 394945 - Sometimes screen brightness gets raised at critical battery level if current brightness is lower than "low battery" profile's brightness
Summary: Sometimes screen brightness gets raised at critical battery level if current ...
Status: RESOLVED FIXED
Alias: None
Product: Powerdevil
Classification: Plasma
Component: general (show other bugs)
Version: 5.12.5
Platform: openSUSE Linux
: NOR normal
Target Milestone: ---
Assignee: Plasma Bugs List
URL:
Keywords: usability
: 437258 (view as bug list)
Depends on:
Blocks:
 
Reported: 2018-06-02 09:57 UTC by Rolf Eike Beer
Modified: 2022-09-01 18:57 UTC (History)
5 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.24.7


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Rolf Eike Beer 2018-06-02 09:57:54 UTC
I have set the screen brightness to ~30% when on critical battery level. When the screen is at a lower brightness level before and the battery goes below 10%, then the screen brightness is _raised_, which is totally pointless.
Comment 1 Patrick Silva 2018-09-15 15:24:22 UTC
Same behavior on neon dev unstable.
Comment 2 Bruno Guedes 2021-01-21 13:45:34 UTC
Can Confirm this. beyond it being pointless, it burns my eyes when I am using my notebook in a dark place with 5% brightness and it suddenly jumps to 30%.

The correct thing to do was a check the current brightness level and just change it if it is greater that the power saving level.

Obs.: It would be nice as well if there were a percentage meter at the side of the slider(the one written "Level") on the screen brightness option of the the power saving configuration.

Operating System: Manjaro Linux
KDE Plasma Version: 5.20.80
KDE Frameworks Version: 5.79.0
Qt Version: 5.15.2
Kernel Version: 5.9.16-1-MANJARO
OS Type: 64-bit
Processors: 4 × Intel® Core™ i5-6200U CPU @ 2.30GHz
Memory: 7,2 GiB of RAM
Graphics Processor: Mesa Intel® HD Graphics 520
Comment 3 Nate Graham 2022-05-14 20:05:52 UTC
*** Bug 437258 has been marked as a duplicate of this bug. ***
Comment 4 Nate Graham 2022-05-14 21:20:06 UTC
I decided to look into this today and found that powerdevil has code to specifically handle this situation. When I tried to reproduce the issue I can't reproduce it: I set the low or critical battery threshold to 1% below the current charge state, then reduce the brightness to below the level specified in the low battery config, let the battery drain by 1%, and the brightness stays where it is and does not increase.

So the code is definitely trying to handle this situation, and it's working for me. There must be some subtle bug affecting the people who can reproduce the issue.

Would be good to get more information about what state the machine was in when the issue happened.
Comment 5 Louis Moureaux 2022-05-14 22:43:59 UTC
I could reproduce it with powerdevil 4:5.24.4-0xneon+20.04+focal+release+build46. I used the following settings (translating labels back from French):

* In the "Advanced" page:
    * Weak level = 90%
    * Critical level = 80%
* In the "Energy saving" page:
    * Check "Screen brightness" and set the level to about 30% (which is the default anyway)

Then I set the brightness to 10% and let the battery drain from its initial 93%. When it reached 90% the brightness went up to 30% as specified in the configuration. I reproduced it again setting 85% as the weak level.

Note that there is no OSD notification of the new luminosity level, so you need to set something visibly different or look in the applet.

I'll try to gdb in and understand what's going on at https://invent.kde.org/plasma/powerdevil/-/blob/master/daemon/actions/bundled/brightnesscontrol.cpp#L84. This code appears to be duplicated for keyboard brightness, so both will need a fix.
Comment 6 Louis Moureaux 2022-05-15 00:35:43 UTC
Finally got a breakpoint set at the right place. Here is what I see:

Thread 1 "org_kde_powerde" hit Breakpoint 2, PowerDevil::BundledActions::BrightnessControl::onProfileLoad (this=0x55cba3ef1910) at ./daemon/actions/bundled/brightnesscontrol.cpp:80
80      in ./daemon/actions/bundled/brightnesscontrol.cpp
(gdb) print m_lastProfile
$9 = "LowBattery"
(gdb) print m_currentProfile
$10 = "LowBattery"

Even if I plug AC between two "LowBattery" events, m_lastProfile remains at "LowBattery". This is because I unchecked "Screen Brightness" in the "AC" and "Battery" tabs. When this is the case, the brightness control action doesn't get notifications for profile changes and thus it doesn't update m_currentProfile accordingly.
Comment 7 Nate Graham 2022-05-17 20:53:15 UTC
That makes sense! Since you've already done such great investigation and you're clearly a talented developer, would you be interested in submitting a merge request to fix it?
Comment 8 Bug Janitor Service 2022-05-18 19:36:28 UTC
A possibly relevant merge request was started @ https://invent.kde.org/plasma/powerdevil/-/merge_requests/90
Comment 9 Bug Janitor Service 2022-05-18 19:36:30 UTC
A possibly relevant merge request was started @ https://invent.kde.org/plasma/powerdevil/-/merge_requests/90
Comment 10 David Edmundson 2022-09-01 18:54:35 UTC
Git commit 6220db1b8a5038ed5e1359b1afdf70c0625f0895 by David Edmundson, on behalf of Louis Moureaux.
Committed on 01/09/2022 at 18:52.
Pushed by ngraham into branch 'master'.

Fix profile switching in the brightness actions

The current code assumes that the action is notified of every profile change,
which is only the case when the action is configured to run in all profiles.
The code that prevents brightness from being raised when switching to a more
conservative profile gets confused if the action didn't run in the "previous"
profile. Make it more robust by always querying the core for the previous
profile, which it luckily hasn't updated yet when the action gets executed.
FIXED-IN: 5.25

M  +8    -7    daemon/actions/bundled/brightnesscontrol.cpp
M  +0    -1    daemon/actions/bundled/brightnesscontrol.h
M  +9    -7    daemon/actions/bundled/keyboardbrightnesscontrol.cpp
M  +0    -1    daemon/actions/bundled/keyboardbrightnesscontrol.h

https://invent.kde.org/plasma/powerdevil/commit/6220db1b8a5038ed5e1359b1afdf70c0625f0895
Comment 11 Nate Graham 2022-09-01 18:55:43 UTC
Git commit 0f548c53dd177235eaf5ce8452c9b44839c12e21 by Nate Graham, on behalf of Louis Moureaux.
Committed on 01/09/2022 at 18:55.
Pushed by ngraham into branch 'Plasma/5.25'.

Fix profile switching in the brightness actions

The current code assumes that the action is notified of every profile change,
which is only the case when the action is configured to run in all profiles.
The code that prevents brightness from being raised when switching to a more
conservative profile gets confused if the action didn't run in the "previous"
profile. Make it more robust by always querying the core for the previous
profile, which it luckily hasn't updated yet when the action gets executed.
FIXED-IN: 5.25


(cherry picked from commit 6220db1b8a5038ed5e1359b1afdf70c0625f0895)

M  +8    -7    daemon/actions/bundled/brightnesscontrol.cpp
M  +0    -1    daemon/actions/bundled/brightnesscontrol.h
M  +9    -7    daemon/actions/bundled/keyboardbrightnesscontrol.cpp
M  +0    -1    daemon/actions/bundled/keyboardbrightnesscontrol.h

https://invent.kde.org/plasma/powerdevil/commit/0f548c53dd177235eaf5ce8452c9b44839c12e21
Comment 12 Nate Graham 2022-09-01 18:57:11 UTC
Git commit 35fb7f501e6e0057d6fef8a0f9da425d58dfd27a by Nate Graham, on behalf of Louis Moureaux.
Committed on 01/09/2022 at 18:57.
Pushed by ngraham into branch 'Plasma/5.24'.

Fix profile switching in the brightness actions

The current code assumes that the action is notified of every profile change,
which is only the case when the action is configured to run in all profiles.
The code that prevents brightness from being raised when switching to a more
conservative profile gets confused if the action didn't run in the "previous"
profile. Make it more robust by always querying the core for the previous
profile, which it luckily hasn't updated yet when the action gets executed.
FIXED-IN: 5.25


(cherry picked from commit 6220db1b8a5038ed5e1359b1afdf70c0625f0895)

M  +8    -7    daemon/actions/bundled/brightnesscontrol.cpp
M  +0    -1    daemon/actions/bundled/brightnesscontrol.h
M  +9    -7    daemon/actions/bundled/keyboardbrightnesscontrol.cpp
M  +0    -1    daemon/actions/bundled/keyboardbrightnesscontrol.h

https://invent.kde.org/plasma/powerdevil/commit/35fb7f501e6e0057d6fef8a0f9da425d58dfd27a