Conversation
3202437 to
a5011f9
Compare
|
@bluca Thank you for your review. Updated the commit messages to response the above comments. PTAL. |
a5011f9 to
e17d73f
Compare
|
@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 |
412a8f9 to
f7f6af9
Compare
done, ptal |
|
I didn't do a full review, but the changed parts look OK. Let's do this. |
|
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 |
@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 |
|
I didn't found in v254 changelog any reference to suspend-then-hibernate. I went down to 252 and didn't found any reference. Sleep-then-hibernate is, for me, an absolute must, as long as my laptop is not my main computer and it can keep without being used few minutes or few weeks. Trading battery percentages for convenience was a good trade off with sleep-then-hibernate. |
Please check the NEWS: Lines 1398 to 1403 in 7c52d52 |
Fixes #25269.