Wait AC Power#45
Conversation
| wait_ac_power() { | ||
| local timecount=0 | ||
| local timeout=0 | ||
| local ac_power_device=/sys/class/power_supply/AC/online |
There was a problem hiding this comment.
How reliable is this? The file does not exist on my system. Though there's a user-defined path for that I don't think we should require to go to /sys and let the user find the right one. I don't see any "stable ABI" path defined in the kernel docs so we'd need at least some heuristic for the path of fallback.
There was a problem hiding this comment.
yep, I'm still thinking about a way to grab the information without use external tools (like acpi client) and reading from /sys directly.
There was a problem hiding this comment.
That said, I seem to remember reading that the /sys interface for power was somewhat depreciated...IIRC acpi client or upower were the recommended alternatives. 'could be wrong about that, as the memory is very vague!
|
@sten0, Can you review the proposed code. luigi |
|
@comio No output. On the other hand, |
| wait_ac_power() { | ||
| local timecount=0 | ||
| local timeout=0 | ||
| local ac_power_device=/sys/class/power_supply/AC/online |
There was a problem hiding this comment.
That said, I seem to remember reading that the /sys interface for power was somewhat depreciated...IIRC acpi client or upower were the recommended alternatives. 'could be wrong about that, as the memory is very vague!
| # AC is not online | ||
| [ $timeout -gt 0 ] && [ $timecount -ge $timeout ] && return 1 | ||
| sleep 1s | ||
| timecount=$((timecount+1)) |
There was a problem hiding this comment.
Can we sleep for at least 30sec, so laptops' cpus can stay in deep sleep longer? (or is it appropriate to make this configurable?)
There was a problem hiding this comment.
I'd rather make the timeout longer and not add another config variable.
There was a problem hiding this comment.
how long? I hate hidden timeouts :D
| # | ||
| # Path of AC status flag | ||
| BTRFS_AC_POWER_DEVICE="/sys/class/power_supply/AC/online" | ||
| BTRFS_AC_POWER_DEVICE="/sys/class/power_supply/*/online" |
There was a problem hiding this comment.
This looks like it will do the trick for laptops. I mentioned upower in another comment, because I think that /sys/class/power_supply/*/online will return true even when a server is on UPS battery. (eg: NUT or apcupsd). IIRC it provides a generic interface, and alternatives are upsmon (NUT) and apcaccess -p TONBATT (apcupsd)
There was a problem hiding this comment.
We may want to revisit that later, with support for UPS, but for now I'm fine with the more relaxed check. Meanwhile I verified the path exists on my other systems, so I think there's nothing more left to discuss.
There was a problem hiding this comment.
I had no time to dedicate to search a portable solution yet. @sten0, can you provide some example of upower, upsmon, apsacces (with outputs). I have a very basic system (my home NAS) and I cannot checks all these tools.
|
we can backport this script from OpenRC: and use this implementation if on_ac_power command is not present. This will move all /proc/blablabla logic into an external command/function. |
|
I'm not sure if the on_ac_power is generally available, so we'll need some fallback anyway, but systemd has |
| local timecount=0 | ||
| local timeout=$1 | ||
|
|
||
| while ! check_power_status "$@"; do |
There was a problem hiding this comment.
The timeout value will be propagated to check_power_status, so we need a shift
| # AC is not online | ||
| [ $timeout -gt 0 ] && [ $timecount -ge $timeout ] && return 1 | ||
| sleep 30s | ||
| timecount=$((timecount+1)) |
There was a problem hiding this comment.
If my math is correct, this will wait the 30 times more than the config value.
There was a problem hiding this comment.
It was 1s but i changed just before to commit ("1s is too short" I said).
|
this is the code of on_ac_power: it's almost the same. |
|
So the way you implement it in 14c44e9 it will just wait for AC and if it does not show up, the task continues. Is this desired from the user's POV? Eg. should we make it more configurable:
|
3 participants
NOT TESTED
Only for review and discussion purpose.