Let a user stop a sync that is part-way through by ryanbr · Pull Request #875 · ryanbr/noop

ryanbr · 2026-07-27T07:16:51Z

NOOP has been able to start an offload since day one (SEND_HISTORICAL_DATA, 22) with no way to
stop one. A session ended on HISTORY_COMPLETE, the 15-minute backfill timeout, or a dropped link —
so on a strap with a large banked history the only escape was walking out of Bluetooth range.

ABORT_HISTORICAL_TRANSMITS (20) is the counterpart. It has been named in whoop_protocol.json
since the table was imported and never wired up. Found by comparing against OpenStrap Edge, which
sends it on WHOOP 4.0 as Cmd.abortHistoricalTransmits with body [0x00] — matched here rather than
guessing an empty body, since that is the only hands-on use of the opcode and sendHistoricalData
already proved an empty body does not work on this path.

Nothing is lost

This is a stop, not a trim. The strap frees banked records when NOOP acks a HISTORY_END, so
records already acked are persisted and records not yet acked stay in flash and re-offload on the
next sync. No cursor moves, nothing is deleted.

It works even if the strap ignores it

The local teardown does not depend on the opcode being honoured. exitBackfilling runs either way —
the same path the timeout and disconnect already use, including the drain loop's !backfilling check
that discards queued frames. A firmware that ignores 20 therefore degrades to exactly today's
behaviour (frames keep arriving and are dropped) rather than leaving the UI wedged in "Syncing".

That matters because the opcode is confirmed in use on 4.0 by Edge but is not hardware-confirmed
here, on either family.

Gating

Send ordering is load-bearing, not stylistic: the 5/MG allow-list admits opcode 20 only while
backfilling is true, so the command goes out before the teardown flips it.

The menu entry appears only while that strap is actually offloading. Not Test Centre gated — this is
an ordinary escape hatch, not a probe — and "an offload is in flight" is a tighter gate than a
settings toggle anyway.

canAbortSync sits beside canRequestSync as a pure predicate, for the reason that one exists: the
menu's visibility and the client's own guard must not drift apart. It deliberately does not
require a live link — a user can cancel a session whose link already dropped, and then there is
simply no opcode to send. Two tests, one asserting the start and stop gates are mutually exclusive.

Verification, and its limits

Strings added to the Apple catalog in all eight languages and all six Android locale files;
i18n_audit.py --ci and doc_comment_lint.py both green.

Not verified on hardware, on either family, and no CI job compiles either half — this is
app-target Swift plus Kotlin. Every symbol was checked against its declaration by hand, which is how
a wrong connectivity check (connected vs _state.value.connected) got caught before pushing.
Source-hygiene caught two stacked KDoc blocks on the way: I inserted between an existing doc comment
and its declaration twice in one change, orphaning canRequestSync and exitBackfilling's docs.

Worth a strap log from a 4.0 owner before this is trusted to actually stop the stream, rather than
just stopping NOOP from listening.

NOOP has been able to START an offload since day one (SEND_HISTORICAL_DATA, 22) with no way to stop one. A session ended on HISTORY_COMPLETE, the 15-minute backfill timeout, or a dropped link — so on a strap with a large banked history the only escape was walking out of Bluetooth range. ABORT_HISTORICAL_TRANSMITS (20) is the counterpart, named in whoop_protocol.json since the table was imported and never wired up. Found by comparing against OpenStrap Edge, which sends it on WHOOP 4.0 as `Cmd.abortHistoricalTransmits` with body [0x00] — matched here rather than guessing an empty body, since that is the only hands-on use of the opcode and sendHistoricalData already proved an empty body does not work. NOTHING IS LOST. This is a stop, not a trim: the strap frees banked records when NOOP acks a HISTORY_END, so acked records are persisted and unacked ones stay in flash and re-offload on the next sync. No cursor moves and nothing is deleted. The local teardown does NOT depend on the strap honouring the opcode. exitBackfilling runs either way — the same path the timeout and disconnect already use, including the drain loop's !backfilling check that discards queued frames — so a firmware that ignores 20 degrades to exactly today's behaviour rather than leaving the UI wedged in "Syncing". That matters because the opcode is confirmed in use on 4.0 by Edge but is NOT hardware-confirmed here, on either family. Send ordering is load-bearing, not stylistic: the 5/MG allow-list admits opcode 20 only while `backfilling` is still true, so the command goes out before the teardown flips it. The menu entry appears only while that strap is actually offloading. Not Test Centre gated — this is an ordinary escape hatch, not a probe — and the in-flight condition is a tighter gate than a settings toggle anyway. canAbortSync is a pure predicate beside canRequestSync, for the same reason that one exists: the menu's visibility and the client's own guard must not drift apart. It deliberately does not require a live link — a user can cancel a session whose link already dropped, and then there is simply no opcode to send. Two tests, including one asserting the start and stop gates are mutually exclusive. Strings added to the Apple catalog in all eight languages and to all six Android locale files. NOT VERIFIED ON HARDWARE, on either family, and no CI job compiles either half. Every symbol was checked against its declaration by hand. Two stacked KDoc blocks were caught by source-hygiene on the way — inserting between an existing doc comment and its declaration, twice in one change.

Re-reviewing this PR: the KDoc on canAbortSync says it is pure so that "the menu's visibility and the client's own guard must not drift apart" — and then the menu tested `live.backfilling` directly and never called it. The predicate was used only by the client and its tests, so the stated reason for its existence was not true of the code. That is not how the precedent works. canRequestSync earns the same claim honestly: TodayProvenance.kt calls it from the UI layer and WhoopBleClient calls it internally, so the two genuinely cannot diverge. The menu now calls canAbortSync the same way. Verified while re-reading rather than assumed: the Kotlin allow-list is a NEGATIVE chain (`if (cmd != A && … ) drop`) where the Swift one is a POSITIVE guard, so the two clauses had to be written in opposite polarity — `!(cmd == ABORT && backfilling)` against `|| (command == .abortHistoricalTransmits && backfilling)`. Both reduce to "admit opcode 20 only mid-offload", checked by simulating the reduced expression. backfilling, send() and abortBackfill() are all members of WhoopBleClient; LiveState carries `backfilling`; the WhoopBleClient import was already present and is not duplicated. Comment-and-wiring only, no behaviour change: the menu condition is the same boolean, now routed through the predicate that documents it.

Third re-review pass. The outcome side of exitBackfilling turned out to be correct but ACCIDENTALLY so, which is the same shape as the last finding: right behaviour resting on something unstated. Both platforms classify the teardown reason. Only "HISTORY_COMPLETE" stamps lastSyncAt/lastSyncedAt and clears the error; only "timeout" raises "the strap went quiet"; every other reason falls through untouched. That fall-through is exactly right for an abort — a cancelled sync is neither a success nor a failure, nothing was lost, and the next sync re-offloads what was left. Verified on both sides, including that Swift renders a sensible "aborted by user rows=N" in the Test Centre offload line. But it holds only because the string happens not to match a case. Someone adding a case, or making the else stricter, would silently start reporting a cancelled sync as a completed one — stamping a lastSyncAt the strap never earned. Comment says so at both call sites now. Also checked and fine: historicalKickSent resets at session START, so an abort cannot leave the next offload unable to kick. Comments only.

Follow-up to #872, from its review. Comments only, no behaviour change. Both allow-list comments claimed: allowed ONLY while a probe is actually in flight, so a default install can never form these bytes That is true on 5/MG and only there. The whole send() allow-list sits inside the `deviceFamily == .whoop5` branch, and WHOOP 4.0 has no send allow-list at all — so on a 4.0 the only thing keeping START_FF_KEY_EXCHANGE (117) and SEND_NEXT_FF (118) off the wire is probeFeatureFlags()'s own Test Centre gate. Same practical outcome, and it matches how every other 4.0 command already works, so this is not a hole. But the 4.0 is the family with a published key dump to reproduce, which makes it the likely first runner — and someone auditing the safety story of an opcode nobody has validated on hardware should not have to discover that the reassuring sentence only covers the other family. Also adds a line on the Test Centre guard itself: for 4.0 it is not belt-and-braces, it is the belt. That guard doing its job is load-bearing on that family, and the comment now says so. Both platforms, both comment sites. doc_comment_lint and i18n_audit --ci green. Originally offered as a patch on #872, but the push to the contributor's fork was refused (maintainerCanModify is on, the token is not), so it lands here instead. Verified by trial merge onto current main rather than trusting the mergeable flag: #875 edited the same allow-list region in both files afterwards, and its abort clause survives intact on both platforms with the merged comment block reading coherently.

… read probe (#103) (#890) Read-only follow-up to #872. That probe asked a strap for the feature-flag key NAMES its firmware knows, and on a real WHOOP 5 MG (WS50_r03) it worked: the strap enumerated exactly the 16 keys NOOP already writes, serving indices 1-10 then 13-18 and skipping 11/12 itself. This one asks for a named key's VALUE, and reaches a namespace 117/118 never covered. WHY. NOOP writes config through two verbs into two namespaces and reads neither: SET_FF_VALUE (120) into the feature-flag namespace, SET_DEVICE_CONFIG_VALUE (119) into device-config. The 117/118 enumerate pair covers the feature-flag namespace only; device-config has never been enumerated or read here at all. For #103 that matters — if a config value gates the byte at offset 82, it is far likelier to live in device-config than among the 16 flags NOOP already writes. THE PRIMARY DELIVERABLE IS A PER-VERB VERDICT, not a value dump. A number in the CommandNumber table is not a served verb (opcode 96 is the standing example of one nothing sends), so a clean "neither verb is served by this firmware" is a publishable result that closes a branch of the #103 search, and is treated as a success of the probe rather than a failure. One round-trip per verb establishes that first — 128 against a flag NOOP writes, 121 against the hardware-validated Broadcast-HR key — so a FAILURE is evidence about the VERB, not the key. A dead verb is retired, so both-verbs-dead is two round-trips and ~16s, not one per key. READ-ONLY BY CONSTRUCTION. DeviceConfigReadProbe.isReadOnlyOpcode is the SAME predicate the 5/MG send() allow-list consults on both platforms — not a hardcoded pair repeated in the clause — so the exhaustive test looping all 256 opcodes proves the allow-list itself, not a copy of it. readOnlyOpcodes is disjoint from writeOpcodes, and 119/120 keep their own separate opt-in clauses, untouched. Verified on review, along with: the plan bounded three ways (maxSteps, an attempted set keyed opcode:key, monotonic phases), CRC gating before any field read on both platforms, all nine shared constants matching, 25 tests each side, and the golden report byte-identical (the Kotlin concatenated literal reconstructed and diffed against the Swift heredoc rather than trusting the test exists). NO SUBSTRING FALSE-POSITIVE. The candidate list contains both enable_spo2 and enable_spo2_packets, so the value claim was checked specifically: echoOffset requires the remainder of the 32-byte field to be NUL padding, so the shorter key against a record echoing the longer one returns nil rather than reading a bogus byte at +32. A value is claimed only when the strap echoes the requested key in the SET side's own layout; nil means "no value claimed", never "value is zero". WIRE SHAPE IS A DOCUMENTED GUESS THAT FAILS CLOSED. The request body ([0x01] then the key NUL-padded to 32 bytes) is inferred from the SET side, not observed; a wrong shape draws FAILURE or silence, which the report states plainly. Beyond the 2-byte response header no field layout is assumed — the record is reported as raw hex. The guessed oxygen key names are labelled as GUESSES in the RENDERED report, not only in source comments ("Candidate oxygen keys — GUESSES, never observed on a wire or in any table"), so someone pasting the result into #103 carries the caveat with it. NOT HARDWARE-VALIDATED: opcodes 121 and 128 have never been sent to a strap. That is the deliverable rather than a risk being glossed — the probe exists to establish whether they are served at all. Default-off behind Test Centre → Connection, user-triggered, in-flight allow-list, Test Centre re-check in the sender. Trial-merged before landing: #872's enumerate clause, #875's abort clause, #890's config-read clause and #873's 4.0 note all coexist cleanly, with both gates green on the merged tree.

NOOP has been able to START an offload since day one (SEND_HISTORICAL_DATA, 22) with no way to stop one. A session ended on HISTORY_COMPLETE, the 15-minute backfill timeout, or a dropped link — so on a strap with a large banked history the only escape was walking out of Bluetooth range. ABORT_HISTORICAL_TRANSMITS (20) is the counterpart. It has been named in whoop_protocol.json since the table was imported and never wired up. Found by comparing against OpenStrap Edge, which sends it on WHOOP 4.0 as `Cmd.abortHistoricalTransmits` with body [0x00] — matched here rather than guessing an empty body, since that is the only hands-on use of the opcode and sendHistoricalData already proved an empty body does not work on this path. NOTHING IS LOST. A stop, not a trim: the strap frees banked records when NOOP acks a HISTORY_END, so acked records are persisted and unacked ones stay in flash and re-offload on the next sync. No cursor moves and nothing is deleted. IT WORKS EVEN IF THE STRAP IGNORES IT. The local teardown does not depend on the opcode being honoured — exitBackfilling runs either way, the same path the timeout and disconnect already use, including the drain loop's !backfilling check that discards queued frames. A firmware that does not know opcode 20 therefore degrades to exactly today's behaviour rather than wedging the UI in "Syncing". That matters because the opcode is confirmed in use on 4.0 by Edge but is NOT hardware-confirmed here, on either family. GATING. Send ordering is load-bearing, not stylistic: the 5/MG allow-list admits opcode 20 only while `backfilling` is true, so the command goes out before the teardown flips it. The two allow-lists are opposite in polarity — Kotlin is a negative chain (`if (cmd != A && …) drop`), Swift a positive guard — so the clauses read `!(cmd == ABORT && backfilling)` and `|| (command == .abortHistoricalTransmits && backfilling)`. Both reduce to "admit 20 only mid-offload"; checked by simulating the reduced expression rather than by eye. The menu entry appears only while that strap is actually offloading. Not Test Centre gated — an ordinary escape hatch, not a probe — and "an offload is in flight" is a tighter gate than a settings toggle anyway. Both the menu and the client's own guard route through the pure `canAbortSync`, the way TodayProvenance and WhoopBleClient both route through `canRequestSync`, so the two cannot drift. Two tests, one asserting the start and stop gates are mutually exclusive. OUTCOME REPORTING. "aborted by user" is deliberately not a reason either platform classifies: only HISTORY_COMPLETE stamps lastSyncAt and only "timeout" raises a sync error, so an abort leaves both untouched — neither a success nor a failure, which is what a cancelled sync is. Verified identical on both sides, and commented at both call sites so a later edit does not start reporting a cancelled sync as a completed one. historicalKickSent resets at session START, so an abort cannot leave the next offload unable to kick. Strings added to the Apple catalog in all eight languages and all six Android locale files. NOT VERIFIED ON HARDWARE, on either family, and no CI job compiles either half — this is app-target Swift plus Kotlin. Every symbol was checked against its declaration by hand, which is how a wrong connectivity check (`connected` vs `_state.value.connected`, a data-class field rather than an instance property) was caught before pushing. source-hygiene caught two stacked KDoc blocks inserted between an existing doc comment and its declaration, orphaning canRequestSync's and exitBackfilling's docs. Worth a strap log from a 4.0 owner before this is trusted to stop the STREAM, rather than only stopping NOOP from listening.

…#873) Follow-up to ryanbr#872, from its review. Comments only, no behaviour change. Both allow-list comments claimed: allowed ONLY while a probe is actually in flight, so a default install can never form these bytes That is true on 5/MG and only there. The whole send() allow-list sits inside the `deviceFamily == .whoop5` branch, and WHOOP 4.0 has no send allow-list at all — so on a 4.0 the only thing keeping START_FF_KEY_EXCHANGE (117) and SEND_NEXT_FF (118) off the wire is probeFeatureFlags()'s own Test Centre gate. Same practical outcome, and it matches how every other 4.0 command already works, so this is not a hole. But the 4.0 is the family with a published key dump to reproduce, which makes it the likely first runner — and someone auditing the safety story of an opcode nobody has validated on hardware should not have to discover that the reassuring sentence only covers the other family. Also adds a line on the Test Centre guard itself: for 4.0 it is not belt-and-braces, it is the belt. That guard doing its job is load-bearing on that family, and the comment now says so. Both platforms, both comment sites. doc_comment_lint and i18n_audit --ci green. Originally offered as a patch on ryanbr#872, but the push to the contributor's fork was refused (maintainerCanModify is on, the token is not), so it lands here instead. Verified by trial merge onto current main rather than trusting the mergeable flag: ryanbr#875 edited the same allow-list region in both files afterwards, and its abort clause survives intact on both platforms with the merged comment block reading coherently.

… read probe (ryanbr#103) (ryanbr#890) Read-only follow-up to ryanbr#872. That probe asked a strap for the feature-flag key NAMES its firmware knows, and on a real WHOOP 5 MG (WS50_r03) it worked: the strap enumerated exactly the 16 keys NOOP already writes, serving indices 1-10 then 13-18 and skipping 11/12 itself. This one asks for a named key's VALUE, and reaches a namespace 117/118 never covered. WHY. NOOP writes config through two verbs into two namespaces and reads neither: SET_FF_VALUE (120) into the feature-flag namespace, SET_DEVICE_CONFIG_VALUE (119) into device-config. The 117/118 enumerate pair covers the feature-flag namespace only; device-config has never been enumerated or read here at all. For ryanbr#103 that matters — if a config value gates the byte at offset 82, it is far likelier to live in device-config than among the 16 flags NOOP already writes. THE PRIMARY DELIVERABLE IS A PER-VERB VERDICT, not a value dump. A number in the CommandNumber table is not a served verb (opcode 96 is the standing example of one nothing sends), so a clean "neither verb is served by this firmware" is a publishable result that closes a branch of the ryanbr#103 search, and is treated as a success of the probe rather than a failure. One round-trip per verb establishes that first — 128 against a flag NOOP writes, 121 against the hardware-validated Broadcast-HR key — so a FAILURE is evidence about the VERB, not the key. A dead verb is retired, so both-verbs-dead is two round-trips and ~16s, not one per key. READ-ONLY BY CONSTRUCTION. DeviceConfigReadProbe.isReadOnlyOpcode is the SAME predicate the 5/MG send() allow-list consults on both platforms — not a hardcoded pair repeated in the clause — so the exhaustive test looping all 256 opcodes proves the allow-list itself, not a copy of it. readOnlyOpcodes is disjoint from writeOpcodes, and 119/120 keep their own separate opt-in clauses, untouched. Verified on review, along with: the plan bounded three ways (maxSteps, an attempted set keyed opcode:key, monotonic phases), CRC gating before any field read on both platforms, all nine shared constants matching, 25 tests each side, and the golden report byte-identical (the Kotlin concatenated literal reconstructed and diffed against the Swift heredoc rather than trusting the test exists). NO SUBSTRING FALSE-POSITIVE. The candidate list contains both enable_spo2 and enable_spo2_packets, so the value claim was checked specifically: echoOffset requires the remainder of the 32-byte field to be NUL padding, so the shorter key against a record echoing the longer one returns nil rather than reading a bogus byte at +32. A value is claimed only when the strap echoes the requested key in the SET side's own layout; nil means "no value claimed", never "value is zero". WIRE SHAPE IS A DOCUMENTED GUESS THAT FAILS CLOSED. The request body ([0x01] then the key NUL-padded to 32 bytes) is inferred from the SET side, not observed; a wrong shape draws FAILURE or silence, which the report states plainly. Beyond the 2-byte response header no field layout is assumed — the record is reported as raw hex. The guessed oxygen key names are labelled as GUESSES in the RENDERED report, not only in source comments ("Candidate oxygen keys — GUESSES, never observed on a wire or in any table"), so someone pasting the result into ryanbr#103 carries the caveat with it. NOT HARDWARE-VALIDATED: opcodes 121 and 128 have never been sent to a strap. That is the deliverable rather than a risk being glossed — the probe exists to establish whether they are served at all. Default-off behind Test Centre → Connection, user-triggered, in-flight allow-list, Test Centre re-check in the sender. Trial-merged before landing: ryanbr#872's enumerate clause, ryanbr#875's abort clause, ryanbr#890's config-read clause and ryanbr#873's 4.0 note all coexist cleanly, with both gates green on the merged tree.

ryanbr added 3 commits July 27, 2026 00:16

ryanbr merged commit 4dbd319 into main Jul 27, 2026
2 checks passed

ryanbr deleted the ble/abort-historical-transmits branch July 27, 2026 07:27

Navigation Menu

Repository navigation

Let a user stop a sync that is part-way through - #875

Let a user stop a sync that is part-way through#875
ryanbr merged 3 commits into
mainfrom
ble/abort-historical-transmits

ryanbr commented Jul 27, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

ryanbr commented Jul 27, 2026

Nothing is lost

It works even if the strap ignores it

Gating

Verification, and its limits

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant