Bug 467797

*** Bug 463884 has been marked as a duplicate of this bug. ***

Git commit 127f6aef4ec9626e95c200d5600655bc781b45d6 by Jakob Petsovits.
Committed on 15/12/2023 at 07:21.
Pushed by jpetso into branch 'master'.

Drop "Advanced Power Settings" KCM in favor of a new QML page

This commit removes the QWidgets-based KCM in kcmodule/global,
and instead adds a new QML page accessible via header action
in the existing "Energy Saving" (QML) KCM a.k.a. profiles config.
Renaming folder and class names of the profiles KCM to reflect
its combined scope is left for a later commit.

I modified the "Energy Saving" KCM's JSON file to declare it
on the top level of System Settings, and merged extra keywords
from the now deleted kcm_powerdevilglobalconfig.json.
We'll have to remove the "power-management" category from
System Settings after this, as only a single KCM is left.

New C++ KCM backend code is mostly moved from GeneralPage.{h,cpp}:

* PowerDevil::GlobalSettings provides kcm.settings.global for QML.
* KAuth-powered ChargeThresholdHelper settings are accessible via
  kcm.externalServiceSettings & left out of non-default highlighting.
* "Supports $X" properties are added to the KCM class directly.

All in all, it's a fairly straight port that doesn't change the
underlying data representation apart from the obvious C++/QML split.
Only the magic value -1 gets replaced by a constant named
ChargeThresholdUnsupported, plus functions for checking support.

The UI is also roughly the same. That said, there are some differences:

* I replaced the "Configure Notifications" button with a column of
  two ToolButtons in the style of Quick Settings' "Most used pages".

* The start-charging threshold, i.e. the lower bound, previously
  had a weirdly long spinbox field because it needed to fit the
  "special value" text "Always charge when plugged in".
  This looks awkward and is not easily discoverable. Furthermore,
  QQC2.SpinBox doesn't have a concept of a "special value" and
  doesn't pre-allocate item width for any given space. I decided to
  represent this state with numeric percentages only.
    * "Special value" means display text for the minimum value
      ("from") of a QSpinBox. The start-charging threshold QSpinBox
      had no minimum set in generalPage.ui, so its "special value"
      would have been 0. This makes little sense, "Always charge"
      for the start threshold is conceptually more similar to
      "start threshold equals stop threshold".
    * Hence, the start threshold SpinBox now starts at 1, not 0,
      and the maximum allowed value is that of the stop threshold
      from the other field above. If the maximum value is selected,
      we write the "special value" 0 to the backend. If the backend
      value changes to 0, we set the UI to the stop threshold value.
Related: bug 449254, bug 450276, bug 459081

M  +0    -2    kcmodule/CMakeLists.txt
M  +0    -1    kcmodule/common/CMakeLists.txt
D  +0    -90   kcmodule/common/ErrorOverlay.cpp
D  +0    -28   kcmodule/common/ErrorOverlay.h
D  +0    -19   kcmodule/global/CMakeLists.txt
D  +0    -266  kcmodule/global/GeneralPage.cpp
D  +0    -47   kcmodule/global/GeneralPage.h
D  +0    -4    kcmodule/global/Messages.sh
D  +0    -277  kcmodule/global/generalPage.ui
D  +0    -126  kcmodule/global/kcm_powerdevilglobalconfig.json
M  +3    -1    kcmodule/profiles/CMakeLists.txt
A  +190  -0    kcmodule/profiles/ExternalServiceSettings.cpp     [License: GPL(v2.0+)]
A  +66   -0    kcmodule/profiles/ExternalServiceSettings.h     [License: GPL(v2.0+)]
M  +0    -2    kcmodule/profiles/Messages.sh
M  +107  -5    kcmodule/profiles/ProfilesConfigKCM.cpp
M  +41   -2    kcmodule/profiles/ProfilesConfigKCM.h
M  +32   -31   kcmodule/profiles/kcm_powerdevilprofilesconfig.json
A  +364  -0    kcmodule/profiles/ui/GlobalConfig.qml     [License: GPL(3+eV) GPL(v3.0) GPL(v2.0)]
A  +53   -0    kcmodule/profiles/ui/MostUsedItem.qml     [License: LGPL(v2.0)]
M  +8    -1    kcmodule/profiles/ui/main.qml

https://invent.kde.org/plasma/powerdevil/-/commit/127f6aef4ec9626e95c200d5600655bc781b45d6

To be honest, this sounds more like a system issue or perhaps an issue with the PowerDevil charge threshold helper. That said, Plasma 6 Beta 2 has new UI code for setting charge limits, so if the UI itself had an issue, there's a high chance this is fixed now. Please retest once you have access to a Plasma 6 setup. Thanks!

Plasma 6 has been out for a while now, please retest to confirm the fix (or let the bot autoclose this bug).

Created attachment 172822 [details]
[video] Stop Charing value stuck at 50%

Here's a video with the current behavior. At the first glace that the Start Charging is saved, but the Stop Charging isn't.

But if I do "ls charge_* && cat charge_*" in "/sys/class/power_supply/BAT0/" immediately after what you see in the attached video I get:

.rw-r--r-- 4,1k root 21 Aug 17:36  charge_behaviour
.rw-r--r-- 4,1k root 21 Aug 17:30  charge_control_end_threshold
.rw-r--r-- 4,1k root 21 Aug 17:30  charge_control_start_threshold
.rw-r--r-- 4,1k root 21 Aug 17:36  charge_start_threshold
.rw-r--r-- 4,1k root 21 Aug 17:36  charge_stop_threshold
[auto] inhibit-charge force-discharge
70
70
20
20

So it seems a value is set but isn't properly picked up in the UI but always shows "50%" instead.

Operating System: CachyOS Linux 
KDE Plasma Version: 6.1.4
KDE Frameworks Version: 6.5.0
Qt Version: 6.7.2
Kernel Version: 6.10.6-2-cachyos (64-bit)
Graphics Platform: Wayland
Processors: 8 × Intel® Core™ i5-8250U CPU @ 1.60GHz
Memory: 15,4 GiB of RAM
Graphics Processor: Mesa Intel® UHD Graphics 620
Manufacturer: LENOVO
Product Name: 20KN001QGE
System Version: ThinkPad E480

Created attachment 174561 [details]
Energy panel shows "correct" value.

Still happening on plasma 6.2, but I also noticed that the Battery panel widget shows the actually set value.

Operating System: CachyOS Linux 
KDE Plasma Version: 6.1.5
KDE Frameworks Version: 6.6.0
Qt Version: 6.7.3
Kernel Version: 6.11.2-5-cachyos (64-bit)
Graphics Platform: Wayland
Processors: 8 × Intel® Core™ i5-8250U CPU @ 1.60GHz
Memory: 15,4 GiB of RAM
Graphics Processor: Mesa Intel® UHD Graphics 620
Manufacturer: LENOVO
Product Name: 20KN001QGE
System Version: ThinkPad E480

Thanks for the screen recordings! Unfortunately I am unable to reproduce the issue in Plasma 6.2. Can you?

Also cannot reproduce on my ThinkPad X1 Carbon Gen 7. I'll go back to my initial assumption:

> To be honest, this sounds more like a system issue or perhaps an issue with the PowerDevil charge threshold helper.

So let's try to figure out what's happening. I'm noticing something weird in your sysfs output:

> But if I do "ls charge_* && cat charge_*" in "/sys/class/power_supply/BAT0/" immediately after what you see in the attached video I get:
> 
> .rw-r--r-- 4,1k root 21 Aug 17:36  charge_behaviour
> .rw-r--r-- 4,1k root 21 Aug 17:30  charge_control_end_threshold
> .rw-r--r-- 4,1k root 21 Aug 17:30  charge_control_start_threshold
> .rw-r--r-- 4,1k root 21 Aug 17:36  charge_start_threshold
> .rw-r--r-- 4,1k root 21 Aug 17:36  charge_stop_threshold
> [auto] inhibit-charge force-discharge
> 70
> 70
> 20
> 20

If the order of printed files (with `cat`) is the same order as the one that `ls` gives you, then this is seriously broken. If I set 70/20 on my laptop, it looks like this:

> $ ls -l charge_* && cat charge_*
-rw-r--r-- 1 root root 4096 Oct 16 13:27 charge_behaviour
-rw-r--r-- 1 root root 4096 Oct 16 13:36 charge_control_end_threshold
-rw-r--r-- 1 root root 4096 Oct 16 13:36 charge_control_start_threshold
-rw-r--r-- 1 root root 4096 Oct 16 13:27 charge_start_threshold
-rw-r--r-- 1 root root 4096 Oct 16 13:27 charge_stop_threshold
[auto] inhibit-charge force-discharge
70
20
20
70
```

Notice how in your example, `charge_stop_threshold` at the very bottom holds a value of 20 when it should be 70, and more importantly, `charge_control_start_threshold` holds a value of 70 when it should be 20.

The kernel introduced `charge_control_end_threshold` as newer name for the old `charge_stop_threshold`, as well as charge_control_start_threshold` as newer name for the old `charge_start_threshold`. Each pair should always return the same value, but in your case, it doesn't.

So to me, this suggests that the kernel has a bug for your specific hardware that causes the firmware settings to get exposes by the wrong sysfs file. I figure that all of the other UI confusion is caused by the same underlying root cause.

Please report this bug to the Linux kernel. You can also verify this purely on the command line without involvement by Plasma (I figure that's their preferred method for reproductions) by writing the value manually, e.g.:

> echo 70 | sudo tee charge_control_end_threshold
> echo 20 | sudo tee charge_control_start_threshold

This is what Plasma's charge threshold helper is doing internally, so it should accurately capture the behavior of the Power Management settings page. Except for the fact that PowerDevil and the applet won't know about the change until they restart.

Feel free to reopen this bug if somehow it turns out not to be a kernel bug after all.