Skip to content

Fix dual-battery handling in upower queries#5958

Open
stefanomainardi wants to merge 1 commit into
basecamp:devfrom
stefanomainardi:fix/dual-battery-displaydevice
Open

Fix dual-battery handling in upower queries#5958
stefanomainardi wants to merge 1 commit into
basecamp:devfrom
stefanomainardi:fix/dual-battery-displaydevice

Conversation

@stefanomainardi
Copy link
Copy Markdown

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 -e lists every device path. When two batteries exist, grep BAT returns two lines, and upower -i is 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:

Script Used by
bin/omarchy-battery-status waybar right-click + SUPER+CTRL+ALT+B notification
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 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 master with 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 against master, just say so.

Test plan

  • bash -n clean on all five scripts
  • Smoke-run on a single-battery (well, no-battery — DisplayDevice still exists and returns sensible empty values) system, scripts don't crash and produce parseable output
  • Multi-battery verification — I don't own a ThinkPad T480-class machine; relying on @Michael-Steshenko's root-cause analysis (their issue includes a screen recording showing the exact mismatch). If a reviewer can confirm on real hardware, that would close the loop.

Reported and root-caused by @Michael-Steshenko in #5780 — credit where it's due.

Copilot AI review requested due to automatic review settings May 23, 2026 17:51
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@Michael-Steshenko
Copy link
Copy Markdown

Michael-Steshenko commented May 23, 2026

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.
Thanks for also taking care of all other spots where this issue exists and making a clean PR.
I can validate the fix works across all the script changes when I get to my T480 tomorrow.
Would you mind adding a Co-authored-by: trailer to the commit message?

`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>
@stefanomainardi stefanomainardi force-pushed the fix/dual-battery-displaydevice branch from 4be27fe to 3d7e22f Compare May 24, 2026 05:31
@stefanomainardi
Copy link
Copy Markdown
Author

Done in 3d7e22f — amended with Co-authored-by: Michael Steshenko <michael.steshenko@gmail.com> (using the public email from your GitHub profile; let me know if you'd prefer the noreply alias instead and I'll swap it).

Looking forward to the T480 validation, thanks 🙌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants