sleep: several fixlets #25374
Merged
sleep: several fixlets #25374
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bluca
reviewed
Nov 13, 2022
bluca
reviewed
Nov 13, 2022
bluca
reviewed
Nov 13, 2022
bluca
reviewed
Nov 13, 2022
bluca
reviewed
Nov 13, 2022
bluca
added
the
reviewed/needs-rework 🔨
yuwata
force-pushed
the
sleep-fixlets
branch
from
3202437
to
a5011f9
Compare
yuwata
removed
the
reviewed/needs-rework 🔨
|
@bluca Thank you for your review. Updated the commit messages to response the above comments. PTAL. |
bluca
added
the
reviewed/needs-rework 🔨
yuwata
force-pushed
the
sleep-fixlets
branch
from
a5011f9
to
e17d73f
Compare
bluca
approved these changes
Nov 15, 2022
bluca
added
good-to-merge/with-minor-suggestions
and removed
reviewed/needs-rework 🔨
poettering
reviewed
Nov 16, 2022
poettering
reviewed
Nov 16, 2022
poettering
added
reviewed/needs-rework 🔨
|
@yuwata please see Lennart's comments above. |
|
@yuwata I applied this pull request (as well as #25662) on top of Debian's systemd v252.2-1 but did not get the expected result: the system did not go to hibernation after Here is an excerpt of journalctl logs with a call to I saw |
|
@n-peugnet Thank you for testing this PR, and sorry for late to update this PR. I will take a look. |
|
I don't know if it is possible for a regular user to ask for this, but it would be nice to have this PR prioritized, since it heavily affects regular workflows of users with laptops without S3 sleep state, who rely on |
|
Is there anything else needed to do or to discuss in order to get this PR merged? I'm happy to assist in case assistance is needed, since v252 has pretty significant impacts on the UX due to broken configurations (which have been made on purpose and worked for years). I want to add some perspectives to the discussion at #25269:
I think that having both options available in conjunction is the only logical thing to do. |
|
any updates on this? would love to see it get merged. |
|
I also found that this patch did not fix the |
github-actions
bot
added
util-lib
please-review
bluca
force-pushed
the
sleep-fixlets
branch
2 times, most recently
from
412a8f9
to
f7f6af9
Compare
done, ptal |
|
I didn't do a full review, but the changed parts look OK. Let's do this. |
keszybz
added
good-to-merge/waiting-for-ci 👍
|
this doesn't appear to contain the fix I posted |
Before v252, HibernateDelaySec= specifies the maximum timespan that the system in suspend state, and the system hibernate after the timespan. However, after 96d662f, the setting is repurposed as the default interval to measure battery charge level and estimate the battery discharging late. And if the system has enough battery capacity, then the system will stay in suspend state and not hibernate even if the time passed. See issue systemd#25269. To keep the backward compatibility, let's introduce another setting SuspendEstimationSec= for controlling the interval to measure battery charge level, and make HibernateDelaySec= work as of v251. This also drops implementation details from the man page. Fixes systemd#25269.
- use device_get_sysattr_int(), - drop redundant log message.
Also, rename get_battery_identifier() to siphash24_compress_device_sysattr(). This also makes any errors in sd_id128_get_machine() or id128_get_product() ignored. For the machine ID, the failure should not be significant unless the file stored in the discharge level is reused by another system, which is quite unusual. For the product ID, if the firmware provides useless ID (all zero or all 0xFF), then loading/storing the discharge rate becomes completely broken, that should be avoided. Note, now sysattrs are used instead of properties in uevent files, but both provide the same information, hence no functionality should be changed.
The enumerator is now mostly consistent with on_ac_power() in udev-util.c.
|
thanks for everyone involved |
keszybz
removed
the
good-to-merge/waiting-for-ci 👍
@devsnek if something is missing, please open a new pull request. |
|
things work beautifully again, thank you! |
|
@wired meaning that HibernateDelaySec now again sets time after which the machine wakes up from S3 (or similar) and hibernates? This seems to have been backported to 252. I am on Ubuntu 23.04. Doesn't seem this made it into 23.04's systemd (currently |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #25269.