More tuned-ppd adjustments #669
Conversation
zacikpa
force-pushed
the
ppd-adjustments
branch
from
699c5a2
to
673ae32
Compare
|
CI failures fixed. |
zacikpa
force-pushed
the
ppd-adjustments
branch
from
673ae32
to
e0ed8b3
Compare
|
Rebased onto master because the issue resolved by the first commit was fixed by c082797. |
|
Added another change - a new |
Instead of querying TuneD for the active profile and coverting it to a PPD profile using a reverse profile map, this commit makes tuned-ppd keep track of the currently active PPD profile. This allows for more flexible profile mappings, as they do not have to be injective now. Alongside the active PPD profile, tuned-ppd now also keeps track of the "base" PPD profile, which is restored when all profile holds are released or when tuned-ppd is restarted. For the latter purpose, this profile is also saved in a file.
Direct access via two dictionaries (one for AC, one for DC) was clumsy, this commit replaces it with a new class - ProfileMap.
When starting tuned-ppd: 1. check that all TuneD profiles appearing in the profile mapping exist, 2. check that all PPD profiles in the battery section appear in the main mapping. Do not require the battery_detection option in the configuration file. If not set, decide based on whether the battery section exists. Log an error when setting a TuneD profile fails.
This makes tuned-ppd fully API-compatible with the latest power-profiles-daemon. The property will signify the level of API compatibility with power-profiles-daemon.
zacikpa
force-pushed
the
ppd-adjustments
branch
from
6acb2da
to
cb4a882
Compare
|
Moved the first commit into a separate PR: #697. That one should ideally land in Fedora 41 before the final freeze. The rest are more complex and will likely require some discussion. |
|
Just chiming in here to say that technically this is api-incompatible still with PPD. The original PPD cannot return an This can cause a crash in gnome-control-center: see https://bugzilla.redhat.com/show_bug.cgi?id=2324518. Now, I don't really see a way around this, but it would be good to document this. Source: I'm the upstream (co)maintainer of gnome-control-center. |
|
Thanks, @velsinki. I'll adjust the code not to perform this check. |
|
Thanks for the reply. I'm not sure that's ideal either, as it would mean tuned-ppd is lying about which profile is active? We'll anyways fix the crash in gnome-control-center, not against keeping "unknown". I don't know what other consumers of PPD are doing though, GNOME Shell should have a similar issue. |
Exactly. I agree it's not ideal, but it shifts the responsibility to the users when they mess with the TuneD profiles via multiple methods. What's your proposed behavior of gnome-control-center if tuned-ppd returns "unknown"? It would certainly be nicer if we could keep the implementation as it is now. I would check with the other users of PPD to see whether they would be able to deal with it too. |
We currently show three radio buttons (performance, balanced, power-saver). If tuned-ppd would lie about a profile, we would start the UI with that radio button enabled, as we don't know anything else. That would mean that radio button can't actually be clicked anymore, as it's already active. If however we get "unknown", we can start the UI with no radio buttons selected instead. Currently we don't handle "unknown", and crash, but that's not hard to support. |
This sounds like very reasonable behavior.
In that case, I'd prefer to keep this as is and check in with the other consumers. Sorry for the troubles and thanks for looking into this! |
This PR brings another round of tuned-ppd adjustments and bug fixes. The individual commit messages explain why the changes are being made.
The most prominent change is dropping reverse profile mapping, i.e., from TuneD to PPD profiles. Instead, the daemon now keeps track of the currently active profile and the so-called base profile. This improves tuned-ppd in two ways: