Fix dual-battery handling in upower queries#5958
Conversation
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Thanks for the credit @stefanomainardi, I was waiting for a response from a maintainer on which of the 2 possible fixes I mention in the PR is better. |
`upower -i $(upower -e | grep BAT)` silently breaks on systems with multiple batteries (e.g. ThinkPad T480): `upower -e | grep BAT` returns multiple device paths, but `upower -i` only honors the first one. So the notification ends up reporting BAT0 only, while waybar's own module shows the correct unified value — producing the inconsistency reported in basecamp#5780. Switch all five callers to `/org/freedesktop/UPower/devices/DisplayDevice`, upower's composite virtual device that aggregates every power source (this is the same approach waybar uses internally), so percentage, time-to-empty/full, energy-rate, energy-full and state all reflect the combined battery state. Affected scripts: - bin/omarchy-battery-status (notification on waybar right-click + SUPER+CTRL+ALT+B) - bin/omarchy-battery-remaining-time (called by battery-status) - bin/omarchy-battery-capacity (called by battery-status) - bin/omarchy-battery-remaining (called by battery-monitor) - bin/omarchy-battery-monitor (systemd timer, critical-low alert) The last two are not strictly needed to fix the notification reported in the issue, but they share the same anti-pattern: on a dual-battery system the critical-low alert would also misfire (could trigger when only BAT0 hits 10% even though BAT1 is full, or never trigger if BAT0 lists first and stays charged). Reported and root-caused by @Michael-Steshenko in basecamp#5780. Closes basecamp#5780. Co-authored-by: Michael Steshenko <michael.steshenko@gmail.com>
4be27fe to
3d7e22f
Compare
|
Done in 3d7e22f — amended with Looking forward to the T480 validation, thanks 🙌 |
Summary
Fixes the dual-battery inconsistency reported by @Michael-Steshenko in #5780: the waybar percentage and the right-click notification disagree on multi-battery systems (e.g. ThinkPad T480 with internal + slice).
Root cause
Five scripts share the same anti-pattern:
upower -i $(upower -e | grep BAT)upower -elists every device path. When two batteries exist,grep BATreturns two lines, andupower -iis invoked with multiple positional args — it only honors the first. So the scripts report BAT0 only, while waybar's native battery module shows the correct unified value.Fix
Switch every call to
upower -i /org/freedesktop/UPower/devices/DisplayDevice. That's upower's composite virtual device aggregating all power sources — the same data waybar uses internally. Same fields are exposed (percentage,energy-rate,state,time-to-empty/full,energy-full), so no other code change is needed.Scope
All five scripts in the affected callgraph are fixed in a single commit because they all share the same bug from the same root cause:
bin/omarchy-battery-statusSUPER+CTRL+ALT+Bnotificationbin/omarchy-battery-remaining-timebattery-statusbin/omarchy-battery-capacitybattery-statusbin/omarchy-battery-remainingbattery-monitorbin/omarchy-battery-monitorThe last two aren't strictly required to fix the symptom in the issue, but the same anti-pattern was silently breaking the critical-low alert as well — on dual-battery systems it could misfire (trigger when only BAT0 hits 10% while BAT1 is full) or never trigger (BAT0 stays charged thanks to BAT1 draining first). Reviewers — happy to split if you prefer one PR per concern.
Master backport
The same five scripts exist on
masterwith identical content (verified:git diff origin/master:bin/omarchy-battery-{status,remaining,remaining-time,capacity,monitor} origin/dev:...). If you'd like me to open a parallel PR againstmaster, just say so.Test plan
bash -nclean on all five scriptsReported and root-caused by @Michael-Steshenko in #5780 — credit where it's due.