Repository navigation
ble: light the charging pill on a 5/MG, which listens for events nobody sends it - #1826
Merged
Merged
Conversation
…nobody sends it
The Live-screen charging bolt is often dark while a 5/MG is demonstrably on its
battery pack. The pill is driven from CHARGING_ON(7) / CHARGING_OFF(8) and the
BATTERY_LEVEL charging bit. On a 5/MG none of those is a dependable source: the
strap reads battery from the standard 0x2A19 characteristic, a bare percentage
byte with no charge bit, so the flag rests entirely on catching an edge.
Meanwhile BATTERY_PACK_CONNECTED(21) and BATTERY_PACK_REMOVED(22) are declared
in EventNumber and handled nowhere at all.
Measured on WHOOP MG fw 50.39.1.0 over two attach/detach cycles with every pushed
event logged:
01:40:51 BATTERY_PACK_CONNECTED(21) <- attach
01:40:51 CHARGING_ON(7)
01:41:06 CHARGING_OFF(8)
01:41:11 BATTERY_PACK_REMOVED(22) <- detach
Both families fire, and 21/22 lead. So this ADDS a source for the same flag
rather than replacing one: the 7/8 branch is untouched, and a WHOOP 4.0 is
unaffected because it never sends 21/22.
The existing replayedOffload gate covers the new branch, and that matters: the
strap replays these edges during the next historical offload with byte-identical
payloads. An accepted replay would switch the pill back on minutes after the pack
came off. Verified: the replayed copies at 01:43:00 are correctly suppressed.
One correction to the record, since it appears in an earlier draft of this work:
a census over a single detach/reattach cycle saw no CHARGING_ON/CHARGING_OFF and
concluded an MG never sends them. That was wrong -- CHARGING_ON simply trails
event 21 by about 17 seconds, long enough for a short capture window to miss it.
This was referenced Sep 2, 2026
Owner
|
Merged — thanks @Zebsi235. I checked the load-bearing claims against the tree rather than taking them on The self-correction about Two follow-ups it prompted: Apple twin — #1831. Same events, no replay guard: that router is live-only ( If your capture happens to hold the raw frame bytes for a #1832. |
ryanbr
added a commit
that referenced
this pull request
Sep 3, 2026
Apple twin of @Zebsi235's Android change in #1826. BATTERY_PACK_CONNECTED(21) and BATTERY_PACK_REMOVED(22) are declared in the shared protocol schema and were handled on neither platform. On a 5/MG they fire on every attach and detach and lead the CHARGING_ON/OFF edges, so the charging pill responds when a pack goes on rather than waiting to catch a later edge. A WHOOP 4.0 never sends them. No replay guard here, deliberately: this router is live-only — Backfiller holds no reference to it — which is why the CHARGING_ON/OFF branch above carries none either. Android's EVENT routing does see replays, so its gate is load-bearing there and would guard nothing here. Recorded on #1832: event 21 means a pack was ATTACHED, not that charging began, and `charging` also disables the low-battery throttle. A pack that never starts charging holds that off until it is removed. Unchanged here because both platforms now share that shape.
8 of 12 tasks
ryanbr
added a commit
to Zebsi235/noop
that referenced
this pull request
Sep 3, 2026
Fourth time this session, and mine every time. The ryanbr#1833 block went in between `/** Mask MAC addresses and WHOOP serials ... */` and the redactStrapLogPii it documents, so that text bound to nothing and the function took my block instead. The gate reports a per-FILE count rather than line-keyed sites, so one new stacked doc pushed WhoopBleClient.kt from its baselined 8 to 9 and listed all nine — including eight pre-existing ones that were never the problem. That reads like the branch broke something wholesale; it was one insertion. Moved above the neighbour's doc, which is where an inserted declaration belongs. Also merged current main into this branch: it was 33 commits behind, which predated the ryanbr#1826 pack-attach handler landing in the same EVENT block, so merging now rather than at the merge button keeps the resolution visible. Gate clean, compile clean, PiiRedactionTest 14/14.
ryanbr
added a commit
that referenced
this pull request
Sep 3, 2026
versionName/MARKETING_VERSION 11.1.0 -> 11.1.1, versionCode 418 -> 419, CURRENT_PROJECT_VERSION 299 -> 300. 418 was the 11.1.0 RELEASE, so 419 clears that high-water and the testing build can install over it. Carries everything merged since the release, none of which has reached a user yet: - the clock perf regression from #1822, where ~13 one-time DateFormatters became computed properties running an ICU pattern build and a Bundle lookup per access (#1830) - pack-attach events lighting the charging pill on a 5/MG, both platforms (#1826, #1831) - the Test Centre event census (#1825) - redaction for a strap serial that arrives as HEX inside a payload dump, both platforms (#1833). 11.1.0 can still print one through the alarm readback or the clock frame= dump, so this build is the first where "send me a strap log" is safe to ask again The 11.1.0 startup report is NOT known to be addressed by any of this. The clock fix is a real regression but was never established as its cause, and this build changes nothing about that question - it just stops shipping the regression.
ryanbr
added a commit
that referenced
this pull request
Sep 7, 2026
Started as option A from #1935, document the conflation rather than split the state. Re-reviewing the doc before merging showed the argument that option A rested on was wrong, so this now carries the fix as well. ## What the re-review found The doc claimed a spurious `charging = true` was bounded and self-healing: the next `BATTERY_LEVEL` overwrites it from the strap's own gauge, so at worst it is wrong for about one battery cadence. That is true on iOS. It was not true on Android, because `BATTERY_LEVEL` was not the only writer. The pushed pack-info event (109) also wrote `charging = true`, keyed on pack PRESENCE plus a plausible SoC. `displayable` is `present && socPct in 0.0..100.0`, so a flat pack qualifies, and the decoder's own hardware-confirmed note says the strap volunteers 109 every couple of minutes while a pack is attached. That outruns the ~8 min `BATTERY_LEVEL` that would clear the flag. So the gauge never got the last word: a pack attached and not charging read as charging for its whole attachment, not for one cadence. That is not only a wrong pill. `idleThrottleActive` reads the flag and gates three levers: the low-battery offload cadence, the GATT connection-priority throttle, and the continuous-capture pause behind the user's own "Pause HRV capture" percentage. A strap on a flat or badly seated pack kept background capture running at low battery after the wearer asked for it to stop. iOS was never affected. There the pack record is log-only and Test Centre gated, so nothing but the gauge writes the flag. ## The fix 109 now publishes `packSocPct` alone. That is what the anti-staleness half actually wanted, and the neighbouring doc already calls a non-null `packSocPct` the pack-attached signal. `BATTERY_PACK_CONNECTED(21)` still sets the flag on the attach edge, so the latency win that #1826 added survives, and `BATTERY_LEVEL` can now correct it. A missed attach edge costs at most one battery cadence of lag and is then right, rather than being fast and possibly wrong. Android ends up doing what iOS already did. The rule both platforms now state at the field: an EDGE may set this flag (7, 21, 22), a repeating presence signal may not, or the gauge can no longer correct it. ## Tests One new test, pinned against the source, because the write lives inline in a GATT callback no JVM test can construct. It is anchored on the handler's own `info.displayable && soc != null` guard, which is unique in the file. Anchoring on the decode call or the event constant instead swallowed 400k characters and passed on the pre-fix source, so the anchor is checked rather than assumed. Verified as a negative control: re-adding `charging = true` to that one line fails the test, restoring it passes. Full Android suite 5504 green, doc-comment gate clean. Swift is doc-only, so CI is the check there. ## Not included The `charging` flag still means "attached or charging" on the 21 edge, which is the original conflation #1935 raised. That stays documented rather than split: 21 leads `CHARGING_ON(7)` by up to ~17 s in captures, and with the gauge able to correct the flag again, the window is genuinely one cadence. One thing remains unmeasured, and it is noted in the code rather than glossed: how often 109 repeats on a pack that is attached but flat. The fix does not depend on it, since a repeating presence write is wrong regardless, but the severity of what was there before does.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
The Live-screen charging bolt is often dark while a 5/MG is demonstrably sitting on its battery pack.
The pill is driven from
CHARGING_ON(7)/CHARGING_OFF(8)and theBATTERY_LEVELcharging bit. On a 5/MG none of those is a dependable source: the strap reads battery from the standard0x2A19characteristic — a bare percentage byte with no charge bit — so the flag rests entirely on catching an edge. Miss the edge and there is no level-based source to recover from.Meanwhile
BATTERY_PACK_CONNECTED(21)andBATTERY_PACK_REMOVED(22)are declared inEventNumberand handled nowhere at all.This handles them, alongside the existing 7/8 branch. Five lines of behaviour.
The evidence
WHOOP MG, fw
50.39.1.0, two attach/detach cycles with every pushed event logged:Both families fire, and 21/22 lead. They are simply the ones nothing was listening to.
This ADDS a source, it does not replace one
CommandNumbercase, no send-allowlist entry, no polling, no wire traffic at allThe replay gate is load-bearing here
The new branch sits inside the existing
shouldApplyChargingFromBatteryEvent(replayedOffload)gate, and that matters more than it might look. The strap replays these edges during the next historical offload with byte-identical payloads:An accepted replay would switch the pill back on minutes after the pack was physically removed. Verified in the same capture: the replayed copies are correctly suppressed by the existing gate, so the new branch inherits the right behaviour rather than needing its own.
A correction to the record
An earlier draft of this work claimed "a WHOOP MG never emits
CHARGING_ON/CHARGING_OFF", drawn from a census over a single detach/reattach cycle that logged zero of them.That was wrong, and the comment in the diff says so rather than leaving it buried.
CHARGING_ONtrails event 21 by about 17 seconds — long enough for a short capture window to miss it. A negative result from one cycle was read as a property of the firmware.The change is unaffected either way: 21/22 fire reliably and lead 7/8, so handling them improves the pill's latency. It does not repair an absent signal, and this PR does not claim it does.
Type of change
How it was tested
On hardware. WHOOP MG, hw
WS50_r00, fw50.39.1.0(DIS-attested), no subscription, Pixel 9 Pro / Android 15, localfulldebug build. Two deliberate pack attach/detach cycles with every pushed event logged; the trace above is from that run. The pill tracked both cycles, updating in the same second as the physical attach. The replayed copies arrived ~2 minutes later and did not move it.Android unit tests, full suite.
./gradlew testFullDebugUnitTeston Windows 11 / JDK 17, compileSdk 35, againstmain@d6d84cc5: 5,039 tests across 614 classes, 0 failures, 0 errors (10 skipped, pre-existing). Counted from the JUnit XML with the results directory cleared beforehand, so the numbers come from a real run rather than an up-to-date task.No new build warnings.
Tools/doc_comment_lint.py: not run to completion locally — it fails with aPermissionErrorreading a SwiftPM artifact underPackages/StrandAnalytics/.build/on the SMB share this checkout lives on, unrelated to the change. This diff adds no/** */doc comments, only inline//, so its per-file count is unchanged. Left tosource-hygiene.yml.Not run:
swift test. No Apple machine here, and this diff touches no Swift.What this does NOT claim
50.39.1.0. Nothing here says another strap or firmware behaves the same.chargingis still cleared on every disconnect and there is no level-based source to re-read it from, so a pack attached while the app is disconnected is still missed until the next edge. That is a real remaining gap and this PR does not close it.presenton a pack is not the same as actively charging — a full or dead pack is attached but not charging. The edges are what the pill has always used, and that limitation is unchanged.Strand/BLE/FrameRouter.swifthandles 7/8 and not 21/22, so it has the same gap. Deliberately left out — there is no Swift toolchain here to verify it, and it belongs in its own PR.Checklist
./gradlew testFullDebugUnitTest, full suite)StrandDesigntokens — n/a, no UIdocs/CONTRIBUTING.mdRelated issues
None. Does not close anything.
The event logging that produced the trace above is #1825, which is independent of this change — this does not depend on it, but that PR is what makes the capture reproducible.