Repository navigation
charging conflates "pack attached" with "actually charging", and the low-battery throttle reads it #1935
Description
Flagged by @ryanbr in the #1831 merge note ("worth its own issue if it should be split"); opening it as the issue that note asked for, so the decision has a home. Nothing here is urgent and nothing is broken in the normal case.
What #1826 / #1831 do
On a 5/MG, BATTERY_PACK_CONNECTED(21) sets charging = true and BATTERY_PACK_REMOVED(22) clears it, on both platforms. That was a latency win: 21 leads CHARGING_ON(7) by under a second in one captured cycle and by ~17 s in another, so the pill responds when the pack goes on rather than when charging happens to start.
The edge
21 means a pack was attached. 7 means charging began. They differ when a pack is attached but never charges — a depleted pack, or a poor contact. Then 21 fires, 7 never does, and charging stays true until 22.
Two consequences:
- The pill reads charging while nothing is charging. Cosmetic.
lowPowerThrottleActive = thresholdPct > 0 && !charging && batteryPct <= thresholdPct, so a truechargingdisables the low-battery throttle. A strap at low battery on a dead pack stops stretching its offload interval — the opposite of what that gate exists to do — for as long as the pack stays on.
The second is the one that matters, and it is behaviour outside the Live screen.
What is measured, and what is not
Measured (MG, fw 50.39.1.0, #1825 census): 21 and 22 fire on every attach/detach; 7 follows 21 when the pack is charging; the strap replays all of these during the next offload with byte-identical payloads, and the replay gate correctly suppresses them.
Not measured: a pack that is attached and does not charge. I have not produced that case on hardware, so the size of the edge is unknown. It may be rare enough not to matter.
Options, for your call
- A — leave it. Document the conflation in
charging's doc comment. Cheapest; accepts the throttle edge as rare. - B — split the state. A second field (
packAttachedor similar) set by 21/22, withchargingkept on 7/8 plusBATTERY_LEVEL's charge bit. The pill can show either; the throttle reads onlycharging. Needs both platforms at once to stay in step, and needs a decision about what the pill shows during the 21→7 gap. - C — narrow the throttle only. Keep
chargingas is for the pill; havelowPowerThrottleActiveadditionally require a risingbatteryPcttrend (the strap's own gauge, already sampled every ~30 s) before treatingchargingas true. Level-based, cannot go stale, no new event handling — but it delays the throttle release by a couple of samples after a genuine attach.
I lean C if the edge turns out to be real, because it fixes the consequence that matters without touching the pill or adding state — but I have not measured the edge, so A is entirely defensible.
Happy to do the hardware run that would size it (attach a pack that is known-flat, watch whether 7 ever arrives) if that helps decide.
Activity
ryanbr commentedon Sep 7, 2026
Good issue, and the throttle consequence is the right thing to have led with — that predicate is exactly as you quote it, and the Swift doc says "Charging never engages" in as many words.
Going with A, but for a different reason than the one on offer, because one premise does not hold:
It stays true until the next live
BATTERY_LEVEL. That event carries the strap's own charging bit and writeschargingunconditionally on both platforms — Kotlin at theshouldApplyChargingFromBatteryEventbranch, Swift inFrameRouter. It is a level signal from the gauge, not an edge, arriving "~every 8 min on captured links" per the comment beside it. So a staletruefrom 21 self-corrects at the next battery event.That bounds the exposure to: threshold configured, strap at or below it, a pack attached that never charges, and within ~8 minutes of the attach. Then the throttle stays off for that window and re-engages by itself.
Which is what rules out C. Requiring a rising battery trend would delay throttle release on every genuine attach, for everyone, permanently — and at roughly 1pp per 8 minutes the gauge needs several samples to establish a trend, so that delay is not small. It trades a bounded, self-healing edge for an unbounded, always-on one. B is a real improvement and the conflation is worth naming, but two-platform state plus a decision about the 21→7 gap is a lot for an eight-minute window.
So: document it, and the doc should correct the "until 22" reading specifically, since that premise is what makes B and C look necessary. I will put that up.
On the offered hardware run — thanks, but I would skip it. It would tell us whether 7 ever arrives on a flat pack, which is genuinely interesting, and it no longer changes this decision either way.
Worth saying: you flagged the throttle rather than stopping at the pill, said plainly what you had not measured, and asked instead of shipping. The only thing missed is a level signal that lives nowhere near the event handling this is about.
ryanbr commentedon Sep 7, 2026
Correcting myself: your "stays true until 22" reading was right on Android, and
the premise I replaced it with was wrong.
I said the next live
BATTERY_LEVELwrites the flag unconditionally, so theexposure is bounded by about one battery cadence.
BATTERY_LEVELwas not theonly writer. The pushed pack-info event (109) also wrote
charging = true, keyedon pack PRESENCE plus a plausible SoC.
displayableispresent && socPct in 0.0..100.0, so a flat pack qualifies, and the decoder'shardware-confirmed note has the strap volunteering 109 every couple of minutes
while a pack is attached. That outruns the ~8 min
BATTERY_LEVEL, so the gaugenever got the last word and the flag stayed true toward 22, exactly as you
described it.
So the throttle consequence you led with was larger than I credited, not one
cadence at low battery but the whole attachment, with the user's own capture
pause among the three levers held off.
iOS was never affected: the pack record is log-only there, so nothing but the
gauge writes the flag. That parity gap is what hid this from me, since the
reasoning I checked was the Swift one.
#1945 now carries the change rather than only the doc. 109 publishes
packSocPctalone, which is what the anti-staleness half wanted;BATTERY_PACK_CONNECTED(21)still lights the pill on the attach edge, so thelatency win you measured for #1826 survives and the gauge can correct it again.
The rule now stated at the field on both platforms: an EDGE may set this flag,
a repeating presence signal may not.
On the hardware run I talked you out of, I would take it after all if you still
have the flat pack. One thing stays unmeasured: how often 109 repeats on a pack
that is attached but flat. Nothing here depends on it, since a repeating
presence write is wrong either way, but that is the measurement that would have
caught this, and I told you it would not change anything.
charging: stop pack presence from overriding the strap gauge (#1945)
ryanbr commentedon Sep 7, 2026
Landed in #1945: the pack-info event now publishes the pack SoC alone, so the strap's own gauge gets the last word again. Thanks @Zebsi235 — your original reading of this was the correct one.