Skip to content

Draft: Restore battery charge limits on PowerDevil service startup

Jakob Petsovits requested to merge jpetso/powerdevil:work/jpetso/persistent-charge-limits into master

An alternative approach to MR !253 for fixing BUG: 450551, which makes chargethresholdhelper responsible for persisting charge thresholds (system-wide) instead of PowerDevil::Core and its global .kcfg settings.

[Edit: See !290 (comment 903945) for why this is back to draft, other than not having gone through any notable testing.]

This builds but is entirely untested at the moment. Also because I've had trouble with KAuth for some time. I would be surprised if this patch works without extra testing & fixes. That said, I'm not aware of any missing functionality so maybe we're lucky.

Commit message below:

Restore battery charge limits on PowerDevil service startup

The code so far did not persist any charge limits, because it assumed that the firmware would by itself persist them across reboots. This was true with many ThinkPads, but is not true anymore with a growing number of modern laptops that rely on user-space software to remember and restore threshold values.

This commit modifies the KAuth-powered chargethresholdhelper itself. Calling the setthreshold action will now write a clone of the sysfs power supply battery threshold files to a persistent config path, in addition to writing the settings to sysfs itself.

Secondly, there is a new restorethreshold action which combines parts of getthreshold and setthreshold to (you guessed it):

  • read persisted threshold values,
  • and restore them where currently active thresholds differ.

Because chargethresholdhelper runs as root, threshold values are shared among different users of the same machine. In effect, this makes it idempotent and works very similar to the previous assumptions of firmware-persisted thresholds. Barely any changes to PowerDevil itself are needed as a result.

restorethreshold is tame enough to allow unprivileged/passwordless calls from within PowerDevil::Core service code.

Existing chargethresholdhelper code got refactored a little to facilitate these changes. On a high level:

  • getThresholds() and setThresholds() get an extra argument to specify sysfs vs. persistent config directory access. A more complex config system is not needed.
  • Thresholds are now internally read and set as hashmap of battery to (start or stop) threshold value, instead of flat array. This helps with applying stored thresholds to current ones, and puts in groundwork for per-battery charge limit support.
  • getBatteries() only gets called once per action now. More efficient but also will fail more loudly in the unlikely case of unexpected sysfs directory tree changes.

BUG: 450551

Edited by Jakob Petsovits

Merge request reports