| Summary: | Sometimes screen brightness gets raised at critical battery level if current brightness is lower than "low battery" profile's brightness | ||
|---|---|---|---|
| Product: | [Plasma] Powerdevil | Reporter: | Rolf Eike Beer <kde> |
| Component: | general | Assignee: | Plasma Bugs List <plasma-bugs> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | brunoguedes.nave, bugseforuns, m_louis30, nate, putr4.s |
| Priority: | NOR | Keywords: | usability |
| Version: | 5.12.5 | ||
| Target Milestone: | --- | ||
| Platform: | openSUSE | ||
| OS: | Linux | ||
| Latest Commit: | https://invent.kde.org/plasma/powerdevil/commit/35fb7f501e6e0057d6fef8a0f9da425d58dfd27a | Version Fixed In: | 5.24.7 |
| Sentry Crash Report: | |||
|
Description
Rolf Eike Beer
2018-06-02 09:57:54 UTC
Same behavior on neon dev unstable. 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 *** Bug 437258 has been marked as a duplicate of this bug. *** 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. 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.
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. 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? A possibly relevant merge request was started @ https://invent.kde.org/plasma/powerdevil/-/merge_requests/90 A possibly relevant merge request was started @ https://invent.kde.org/plasma/powerdevil/-/merge_requests/90 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 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 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 |