devices: attribute firmware to the strap it came from, not the last to connect by ryanbr · Pull Request #1634 · ryanbr/noop

ryanbr · 2026-08-26T01:24:39Z

The report

A WHOOP 5/MG shows 41.17.6.0 in the Devices window — the 4.0's firmware.

Cause

The persisted firmware lives under one global key, noop.lastFirmware, written on any WHOOP connect and read back for whichever strap is active:

liveFirmware = if (device.status == active) (live.strapFirmware ?: NoopPrefs.lastFirmware(context))

On a single-strap install those are the same strap, which is why it went unnoticed. With two paired, the fallback reports the other one. The comment above that line states the assumption plainly — "fall back to the last-known persisted firmware … when the live value is momentarily null" — sound for one strap, wrong for two.

The strap log had it worse: its header printed that global key with no live value and no device qualification at all, so a 5/MG log named a 4.0's firmware outright.

The rule

One shared resolution, twinned:

live handshake value — it came from this connection.
else this device's own persisted value.
else the legacy global, only when a single device is paired — where it cannot have come from anything else. This is what stops an existing install regressing to "unknown" on upgrade.
else nothing. "Unknown" is a better answer than another strap's number.

The reported bug is a test: two straps, no per-device value, global present → null.

Android: wired end to end

The write is keyed by the address this connection used — mirroring #1527's rule that a stamp belongs to the identity the strap adopted, not to whatever is active now, which is the same mis-mapping in a different coat. The Devices card, the strap-log header and the #1633 inventory all read per device, and the inventory line gains fw=:

device id=my-whoop status=ACTIVE brand=WHOOP model=WHOOP 5.0 / MG lastSeen=3h 10m ago fw=50.1.0.0
device id=whoop-B  status=paired brand=WHOOP model=WHOOP 4.0      lastSeen=10m ago    fw=41.17.6.0

Apple: correct, not yet complete — and why

FrameRouter has no peripheral identity to key a write on, and LiveState carries none either. Giving it one is a BLE-path change that wants its own review and hardware verification, so it is not folded in here.

Until then Apple writes nothing per-device, and the same rule simply refuses the global key whenever more than one device is paired. So Apple stops mis-attributing today without yet being able to say more — the failure mode becomes "unknown", never "the other strap's firmware".

One place still reads the raw global: Swift's strapStateLines header, which is sync and prefs-only by contract (the scheduled export calls it with no store) while the rule needs a registry read for pairedCount. It is marked in place and superseded by the Devices block in the same export.

Verification

Both platforms byte-identical across nine attribution cases and the inventory line, diffed rather than eyeballed.
7 attribution + 5 inventory tests per platform, confirmed present on the pushed branch, not just locally.
compileFullDebugKotlin clean; 138 JVM tests green; doc_comment_lint and i18n_audit --ci main clean.
Needs app-build: Swift edits are app-target.

…o connect A WHOOP 5/MG shows 41.17.6.0 in the Devices window — the 4.0's firmware. The persisted value lives under ONE global key, "noop.lastFirmware", written on any WHOOP connect and read back for whichever strap is active. On a single-strap install those are the same strap, which is why this went unnoticed; with two paired it reports the other one. The strap log had it worse: its header printed that global key with no live value and no device qualification at all, so a 5/MG log named a 4.0's firmware outright. Resolve per device instead, by one rule shared across both platforms: live handshake value, else THIS device's own persisted value, else the legacy global key ONLY when a single device is paired — where it cannot have come from anything else. Otherwise nothing, because "unknown" is a better answer than another strap's number. The single-device carry is what stops an existing install regressing to "unknown" on upgrade. Android is wired end to end: the write is keyed by the address THIS connection used (mirroring #1527's rule that a stamp belongs to the identity the strap adopted, not to whatever is active now), and the Devices card, the strap-log header and the #1633 inventory all read per device. The inventory line gains fw=, so a two-strap log states each strap's firmware. Apple is CORRECT but not yet complete, and the difference is structural: FrameRouter has no peripheral identity to key a write on, and LiveState carries none either, so giving it one is a BLE-path change wanting its own review and hardware verification. Until then Apple writes nothing per-device and the same rule simply refuses the global key whenever more than one device is paired — so it stops mis-attributing without yet being able to say more. The one place still reading the raw global is Swift's sync prefs-only strapStateLines header, which is marked in place and superseded by the Devices block in the same export. Both platforms, byte-identical across nine attribution cases and the inventory line, diffed rather than eyeballed. 7 attribution tests and 5 inventory tests per platform, including the reported bug as a test: two straps, no per-device value, global present -> null. Verified: compileFullDebugKotlin clean; 138 JVM tests green; doc_comment_lint and i18n_audit clean. Swift edits are app-target and need app-build.

Attribution alone cannot make a firmware appear. The 5/MG version is read as four bytes at pay[93] behind a `pay[93] == 50` guard, and the decoder's own comment says both are anchored to a SINGLE capture at 50.38.1.0: the name+token region ahead of the version is fixed-width "on this firmware", re-verifying the offset across firmwares is left as future work, and the guards deliberately fail closed. So a 5/MG that does not match reports nothing — and from a log, two very different causes look identical: - the byte at 93 is not 50: a different generation marker, offset probably still right; - the name+token region is a different width, so byte 93 is NOT the version and the offset has MOVED. This is the case the decoder comment warns about, and the one a user cannot diagnose. Emit the evidence that separates them: payload length, the byte actually at 93, where the printable-ASCII name run ended (the width that shifts everything after it), and a hex window across the region the version should occupy. A capture from an undecoded strap then locates the real offset without another round trip. The guard is NOT loosened. Widening it blindly would render unrelated bytes as a version, which is worse than "unknown" because it looks authoritative — the same reasoning as leaving BEAT_ACCURACY_MIN_FRACTION alone in #1632. Logged once per connection on both platforms (the value is stable), cleared on the same per-connection boundary each already uses: Android's reset(), Swift's `family` didSet. The decoder half is PURE and lives in WhoopProtocol / Framing, so unlike most of this cluster it is covered by tests that run on Linux: 5 twinned cases each, including a truncated payload that must clamp rather than trap, and a payload ending exactly at the window start. Verified: 638 WhoopProtocol tests green on Linux; 1031 Android tests green; the diagnostic diffed Kotlin-vs-Swift across five cases and byte-identical; doc_comment_lint and i18n_audit clean. The app-side logging is app-target on Swift and needs app-build.

Re-review. I placed the fw_gate logging INSIDE the `(fw_version ?: fw_harvard)?.let { fw -> … }` block — the branch that only runs when a firmware WAS decoded. fw_gate exists only when one was NOT, so the two conditions are mutually exclusive: the line could never be emitted. The whole diagnostic was inert on Android. Nothing caught it. It compiled, and all five twinned formatter tests passed — they exercise the string, not where it is called from. The Swift twin was already correct (its check is a sibling of the `if let fw` block, not nested inside it), so the byte-parity diff passed too while one platform silently produced nothing. Moved out to the same sibling position Swift uses. Also confirmed while re-reviewing: both gates sit inside 5/MG-only paths (respCmd == 145 / cmd == 145, GET_HELLO), so a WHOOP 4.0 — which reports firmware through REPORT_VERSION_INFO as fw_harvard — never reaches them and cannot emit a spurious "no version decoded" line on every connect. compileFullDebugKotlin clean; protocol tests green; doc_comment_lint clean.

app-build failed both legs on one error: DebugDataDiagnostics.swift:131: cannot find 'invDevices' in scope I read the registry inline — `((try? invRegistry.all()) ?? []).map { … }` — and then referenced `invDevices.count` for the paired-count rule, but no such binding ever existed. Bound it before the map. Worth recording how this got through: every local check passed. The pure helpers are byte-parity diffed, the Kotlin side compiles and its tests run, and the WhoopProtocol half has 638 tests green on Linux. None of that touches this file, which needs macOS. app-build is the only thing that compiles it, and it did its job on the first run. Swept the same block for any other name used but never bound: none. doc_comment_lint clean.

Carries the per-device firmware attribution and the 5/MG firmware-gate diagnostic (#1634), so a two-strap install stops reporting one strap's firmware for the other, and a strap that decodes none says why. Version strings stay 10.6.1.

…OS) (#1673) The gap #1672 left open. iOS issued the DIS identity read only inside the post-bond handshake, so a 5/MG that never bonds never read DIS at all - and since the #1635 suppression landed, "never bonded" is a PERMANENT state on these straps rather than a transient one. The Devices screen showed a WHOOP 4.0 with its firmware beside a 5/MG with none, and MG-only capabilities stayed locked because the strap never got to attest what it is. Android has had this since #1647/#1649/#1653. Ported: shouldReadDisUnbonded, disRefusedPrefKey, disReadFailureLine and shouldPublishDisFirmware as a pure Strand/BLE/DisUnbondedRead.swift twinning DisUnbondedRead.kt; the unbonded attempt itself, scheduled from the suppressed path and nowhere else, because that is the only 5/MG state known to be stable; and the four DIS characteristics this platform never read - firmware (0x2A26), manufacturer, model number, software revision. Firmware is the point of it. DIS 0x2A26 is the ONLY source an unbonded strap has, since the puffin report needs the bond that never happens - and #1634 records that the 5/MG hello frequently decodes no firmware even when it DOES bond, so this fills a real void rather than racing a value that is coming. Published as a FALLBACK, never an override. Self-limiting, deliberately. A read is the same class of operation as the write that provokes the teardown, so this could cost the stable link the suppression just bought: once per link, and never again on a strap that has refused once. A refusal is a RESULT - it settles #490, which no capture has ever done - so it is reported with its status rather than swallowed by the generic notify-failure line. Five review passes found five defects, and the last two are the ones worth reading: - the ECG PROBE was blocked only by accident. Its gate checks opt-in, attested MG and connected - but not the bond. On a suppressed strap the MG check blocked it incidentally, because the variant could only resolve from a post-bond read. Reading DIS unbonded removes that accident, and the gate would then have let the probe write puffin frames to fd4b0002 - the very characteristic whose write tears the link down, spending the stable link the suppression exists to buy. send() is no backstop: it guards connected, not bonded. The gate now requires the encrypted bond, as the ECG GATE-WRITE path always did (#269). - the per-device firmware key had readers and NO writer. I first claimed iOS had no FirmwareAttribution twin; that was wrong, and wrong because I grepped Kotlin's names against Swift. It exists, has tests, and is read by DevicesView and the debug export - but the decoded path cannot key on identity, which its own comment concedes, so perDevice always resolved nil and both readers fell back to a global value that is wrong the moment a second strap exists. The DIS site has the peripheral, so it is the one place that can attribute a firmware honestly. That fills a hole predating this change. Also from the passes: the delayed read now checks `connected`, because `peripheral` outlives a disconnect on this platform and the line would otherwise claim an attempt that never happened; three comments still describing the read as post-bond-only were corrected; and the delay is pinned to the twin's actual number (1500 x 2 = 3.0s, not the 4.0 I first guessed). Deliberate divergences, all documented in code: the extras are issued together rather than chained, because CoreBluetooth already serialises reads; the refusal latches only for an UNBONDED attempt, since a transient failure on a bonded link would poison a permanent latch; there is no explicit-bond-deferred call site, that experiment being Android-only; and the post-bond path stays silent about a missing serial, because it is re-entered from the keep-alive tick and would log every cycle. Verification: disReadFailureLine and both firmware log lines proven byte-identical to the Kotlin source by extracting and comparing - which caught a hyphen where Kotlin has an em-dash. All six DIS UUIDs verified against the twin. 14 tests over the pure rules incl. every arm of the attempt gate, the fallback-not-override rule, and that the DIS latch key cannot collide with the hello-suppression one; the pure file also compiled standalone with swiftc and every rule exercised. doc_comment_lint and the i18n gate clean. NOT hardware-validated, and this one is genuinely unproven rather than merely untested: whether a 5/MG answers a standard read on an unencrypted link is the open question #490 asks, and nothing here can settle it without a strap. Either outcome is informative, and a refusal costs one read per strap, once.

Re-review of the port, and it caught a regression I introduced rather than a defect I inherited. Moving the last-sync and #57 write-health stamps to per-device keys removed their only writers, and three readers were left behind: the debug export's "Last sync" and "Data write" lines, and the single-strap fallback in the new seed. Nothing writes those globals any more, so after the upgrade the header would not become per-device — it would freeze at whatever it held before, then read "never" and "no rows ever persisted" forever, for everyone including single-strap users. Worse than the bug being fixed, and my own comment on that line described it as "still global" when the truth after the change was "global and dead". The firmware precedent had this right and I missed it: FrameRouter still writes `noop.lastFirmware` globally while #1634's DIS site writes the per-device key beside it. Both stamps now do the same. Every reader that CAN attribute prefers the per-device key, and the resolve still refuses the global whenever pairedCount says it is ambiguous — so the fix is intact and the prefs-only line keeps working. A stale global is the smaller lie than a dead one, and the debug line now says which it is.

…what it can do (#1753) Both defects Android fixed today were already present here, in the same shapes, and one of them sat eighty lines from its own fix. LAST SYNC. `lastSyncedAt` is ONE defaults key, stamped on any strap's HISTORY_COMPLETE and read back for whichever strap is active. Single-strap installs never notice; with two paired it reports the other one's. DebugDataDiagnostics already called FirmwareAttribution.resolve for firmware and then read last-sync globally eighty lines above it - the pattern was in the file, last-sync just never got it. LastSyncAttribution mirrors FirmwareAttribution one for one: per-BLE-identifier keys, and the legacy global honoured ONLY when exactly one strap is paired, the one case it cannot be ambiguous. The #57 write-health pair is scoped too, both halves, because they are read together ("stalled more recently than ok" is the alarm) and scoping one would compare a strap's own stall against another's success. Both initialisers now seed NOTHING. The obvious cheap seed is the global key, which belongs to whichever strap synced last - on a two-strap install not the one the screens are scoped to. seedLastSyncFromActiveStrap resolves it from the registry's ACTIVE row at bootstrapStore, the first point the registry exists. HISTORY READY. Every manual-sync control gated on `bonded`, which the live-HR path sets for a 5/MG that has never completed a handshake - so the control was offered, accepted, then declined by beginBackfill's connectHandshakeDone guard in silence. HealthView's own comment conceded the gate it was matching. This cannot withhold a sync that would have run: connectHandshakeDone is already that precondition. The card's pill, pulsing and helper line move WITH the button, or gating one of the three makes them contradict - "Ready to sync" above a disabled button. Three review passes, each finding something. One: Today's gate was on the pull's RELEASE, which would have left refreshArmed stuck true for the rest of the gesture since that branch is the only thing clearing it - a worse failure than the silent one being fixed. It gates the ARM now, which also withholds the haptic, reading honestly as unavailable rather than unresponsive. Two: moving the stamps to per-device keys removed their only writers and left three readers behind. The prefs-only debug header would not have become per-device, it would have frozen at its pre-upgrade value and then read "never" and "no rows ever persisted" forever, for everyone. The firmware precedent had this right and it was missed: FrameRouter still writes noop.lastFirmware globally beside #1634's per-device write. Both stamps dual-write now; every reader that CAN attribute still prefers the per-device key. Three: LastSyncAttribution shipped without tests while its Kotlin twin had a full set, next to an existing FirmwareAttributionTests. Parity is not just the helper. NOT ported: the unbonded offload probe, whose answer on the strap that motivated it is no - a twin would port a dead end. The write-completion-is-not-encryption split has no twin either: CoreBluetooth exposes no bond state, so the Apple fix needs the strap's own BLE_BONDED event instead. app-build green on both legs, and its Strand leg RUNS StrandTests. No new copy, so no xcstrings churn. NOT hardware-validated on Apple.

ryanbr added 4 commits August 26, 2026 13:24

ryanbr merged commit 653c44f into main Aug 26, 2026
18 checks passed

ryanbr deleted the fix/firmware-per-device-not-global branch August 26, 2026 02:11

ryanbr mentioned this pull request Aug 30, 2026

Apple parity: last sync belongs to a strap, and sync controls follow what it can do #1753

Merged

bhelm mentioned this pull request Sep 1, 2026

Add fail-closed cross-platform parity governance #1534

Merged

Navigation Menu

Repository navigation

devices: attribute firmware to the strap it came from, not the last to connect - #1634

devices: attribute firmware to the strap it came from, not the last to connect#1634
ryanbr merged 4 commits into
mainfrom
fix/firmware-per-device-not-global

ryanbr commented Aug 26, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

ryanbr commented Aug 26, 2026

The report

Cause

The rule

Android: wired end to end

Apple: correct, not yet complete — and why

Verification

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant