BLE (W6): read the 5/MG strap clock before setting it, and the batch's review fixes by ayiskakov · Pull Request #33 · ayiskakov/noop

ayiskakov · 2026-09-26T21:40:48Z

W6 BLE and collection, the clock batch. The 5/MG connect handshake now reads the strap clock before touching it, and sets it only when the reading shows it wrong. This is the owner's 2026-09-25 decision on W06-050, with W06-001's reply decode, which it needs. Two code reviews of the batch added W06-069 … W06-107, and their fixes are here too. Finding rows are in docs/review/workstreams/W06-ble-collect.md, one commit per change.

Why

Every connect used to send SET_CLOCK. The strap applies the value when it processes the command, and on a busy relaunch that came seconds after the phone stamped it. The owner's strap logs hold 12 distinct SET/GET pairs from firmware 50.39.1.0. Measured against the strap's own uptime counter, five of those sets were processed 2.3–3.9 s late, which left a correct clock that far behind, and the next prompt set stepped it forward again. Both observed IMU label gaps (the 11.9.9 and 11.9.10 runs) follow that pattern: a late set, then a prompt relaunch set that moved the IMU arrival delay by the same 3 to 4 s.

What changes

W06-001: StrapClock.decodeReply (WhoopProtocol) decodes the GET_CLOCK reply: origin sequence at byte 11, result at byte 12, u32 LE seconds at byte 13, bounded by the declared length. Seconds are read only from a SUCCESS reply (W06-095).
W06-050: the handshake sends GET_CLOCK only. StrapClock.judge asks for a SET_CLOCK when the reply is refused or carries no reading, when the reading is invalid (at or below 1293840001), or when the clock is provably more than 2 s off across the whole round trip, judged as the open interval it is (W06-089). A long round trip never sets by itself. The round trip is timed on a monotonic clock, and the wall clock is read once, at the reply (W06-074). A first reply that took more than 1 s and cannot be judged is read once more; an unanswered second read leaves the first reading standing (W06-084). With no reading in 10 s the clock is set without one, as before, but only on the link the check began on, and only while that link can take commands (W06-069, W06-091). connectHandshakeDone, historyReady and connectSettled move to the moment the check settles.
Order after a set: a set gets the full 1.5 s before the connect's first offload request, whatever the handshake's age (W06-071). The delay counts on the monotonic clock, is clamped to 0…1.5 s, and a delayed request whose link has gone is dropped (W06-086). Other triggers (a strap event, the Sync Strap shortcut) can still ask sooner, and the docs now say so (W06-099).
W06-085: a Bluetooth power-off before the reply no longer spends the connect. The timeout settles nothing on a gone link: a newer link past its CLIENT_HELLO gets a check of its own, and otherwise the next CLIENT_HELLO acknowledgement runs the handshake again.
W06-075: the 5/MG set stamps the nearest whole second rather than the truncated one.
Log lines (W06-067, W06-079, W06-090, W06-100): routine readouts (the GET_CLOCK sent line, an in-sync verdict, FrameRouter's reading line, the second read) sit behind Test Centre → Connection. Every set, refusal, unresolved verdict, timeout and stray reply stays always-on, and each line states only what happened.
W06-022: the dead ClockPolicy is deleted; StrapClock is the live policy.
GET_HELLO (W06-065, W06-066, W06-076, W06-087, W06-092, W06-103): FrameRouter.commandResponsePayload read the declared length from bytes 1–2, so it returned nil on every real 5/MG reply and the Test Centre GET_HELLO identity probe never printed. The revived probe runs only on a SUCCESS reply, takes its name offset from the decoder, says where its offsets count from, and quotes only the first three characters of a serial-shaped run, as WhoopSerialIdentity.logSafe does. The shared GET_HELLO decoder now decodes only a SUCCESS reply, which also stops the PENDING acknowledgement logging "fw gate: no version decoded" on every connect.
Docs and comments (W06-068, W06-080, W06-081, W06-093, W06-098): counts, the set rule wherever it is stated, the 50.38.1.0 GET_CLOCK claim with its source, and the whoop_setclock tool's text. The stale-record copy said NOOP re-sets the clock on every connect; it now says NOOP checks it and sets a wrong one.

Tests only

W06-073, W06-096: StrapClockCheckTests drive the app half without a strap: an in-sync reading sends no set, an off or invalid one sets and settles, the handshake begins the check once and waits for it, a reply through handleWhoop5Frame settles it, the timeout sets and settles, and the link classifier's table.
W06-097: decodeReply's guards at the exact body edges.
W06-101, W06-107: the clock tests save and restore the host's Test Centre keys and release their managers in tearDown, so an earlier test's scheduled offload cannot fire into a later one.

Recorded, not changed

Deferred, with reasons in their rows: W06-088 (a reply matched by its 8-bit sequence alone across a relaunch; the remedy changes every frame's sequence byte), W06-094 (a clock reply on type 38), and W06-102 (the stuck-strap watchdog's blind SET_CLOCK, deferred to W06-021's rework, with a warning where the flip would arm it).
Phase 5 refactors: W06-077, W06-078, W06-104, W06-105, W06-106. Phase 3: W06-083 (a Bluetooth power-off runs none of the per-link teardown).
Not bugs: W06-070, and W06-072, whose missed half is W06-085.

Verification

V0 before each code fix: the failure reproduced on the old code. V1 after: the fix's test fails when the fix is reverted or mutated. Five mutations of the clock policy each fail the suite: always set (the old behaviour), ignore the request sequence, expire after settling, ignore the round trip, floor at zero.
WhoopProtocol: 784 tests pass. StrandTests: 2,043 pass. Strand (macOS) and NOOPiOS build; NOOPiOS was built without the watch app, since this Mac has no watchOS runtime.
doc_comment_lint, check_source_references, validate_examples, the i18n gate and the linux-capture Python tests pass. The four test_check_source_references failures on macOS are W11-001 (/var against /private/var) and fail on main too.

Not yet tested on hardware

This changes the BLE connect path, so a strap run decides it:

A plain connect sends no SET_CLOCK, and a default log of an in-sync connect holds no clock line. The first offload and the alarm re-arm still run.
A relaunch during a Raw Data Collector session skips no IMU label.
With Test Centre → Connection on, a connect logs one GET_HELLO probe line, from the SUCCESS reply, and a relaunch logs the second read and an in-sync verdict.
Bluetooth off and on inside a connect's first 10 s leaves the next link with a clock verdict of its own.

Two things the run may show, recorded in W06-050: a lost read reply falls through to the blind set after 10 s (12 of 12 replies have arrived so far), and at about 15 ppm of drift an occasional set after about two days connected is expected.

W06-001 — the strap's GET_CLOCK reply was only hex-logged, and the log line said the result offset was unestablished for the clock. - Severity: S4 · Category: truth · Upstream-able: yes - Location: `Packages/WhoopProtocol/.../StrapClock.swift` `decodeReply`; `Strand/BLE/FrameRouter.swift` clock line - V0: 20 SET/GET pairs in 13 strap logs from firmware 50.39.1.0 share one layout: origin sequence at byte 11, result at 12 (SUCCESS in all 20), the strap's unix seconds as a u32 LE at 13, then seven zero bytes. This is the format-1 response layout in docs/PROTOCOL_TRANSPORT.md. - Fix: `StrapClock.decodeReply` reads the origin sequence, result and seconds, bounded by the declared length. The FrameRouter line prints the result and the strap's seconds beside the full frame. - V1: `StrapClockTests` decode three real replies, and refuse a SET_CLOCK reply, a request frame, and seconds past the declared length.

W06-050 — every connect sent SET_CLOCK. The strap applies the value when it processes the command, which on a busy relaunch came about 4 s after the phone stamped it, so a correct clock was left seconds behind. The next prompt set stepped it forward, and during a Raw Data Collector session that step skipped IMU labels, so the session read incomplete with no motion missing. - Severity: S3 · Category: bug · Upstream-able: yes - Location: `Strand/BLE/BLEManager.swift` 5/MG handshake; `Packages/WhoopProtocol/.../StrapClock.swift` `judge`, `Check` - V0: in all 20 SET/GET pairs in the owner's strap logs, the GET_CLOCK readback equals the SET value exactly, including the four answered about four seconds late. Both observed forward steps (11.9.9 and 11.9.10) follow such a late set; the prompt set that corrected it moved the IMU arrival delay by the same 3 to 4 s. - Fix (the owner's 2026-09-25 decision): the handshake sends GET_CLOCK only. `StrapClock.judge` asks for a SET_CLOCK only when the reply is refused, reads at or below the validity floor, or puts the clock more than 2 s off across the whole request round trip; a long round trip never sets by itself. With no reply in 10 s the clock is set without a reading, as before. The handshake counts as done (`connectHandshakeDone`, `historyReady`, `connectSettled`) when the check settles, so a set still precedes the first offload and the alarm re-arm. The stale-record copy now says NOOP checks the clock every connect and sets a wrong one. - W06-022: the dead `ClockPolicy` is deleted; `StrapClock` is the live policy. - V1: 19 `StrapClockTests`. Five mutations (always set, which is the old behaviour; ignore the request sequence; expire after settling; ignore the round trip; floor at zero) each fail the suite. `StrandTests` (2,013) and both app builds pass (NOOPiOS without the watch app). - V3: needs a strap run.

W06-065 — `FrameRouter.commandResponsePayload` took the declared length from bytes 1–2 instead of 2–3. On every real 5/MG reply the computed end lay past the frame, so it returned nil, and the Test Centre GET_HELLO identity probe (ryanbr#1303), its only caller, never printed. - Severity: S4 · Category: truth · Upstream-able: yes - Location: `Strand/BLE/FrameRouter.swift` `commandResponsePayload` - V0: the helper, run verbatim on a real GET_CLOCK reply (28 bytes, declared length 20), reads length 5121 and returns nil; on a SET_CLOCK reply, 3073 and nil. None of the 13 owner strap logs has a ryanbr#1303 line, though they carry Test Centre connection lines and decoded hellos. No other site reads a 5/MG length there. - Fix: the payload ends at the CRC32 trailer, 4 bytes past the declared length. - V1: `FrameRouterCommandPayloadTests` (two real replies, two short frames) pass; with the old offset all three fail. - V3: a connect with Test Centre → Connection on logs the ryanbr#1303 line.

…fset W06-066 — W06-065 revived the Test Centre GET_HELLO probe (ryanbr#1303) with two faults. It fired on the PENDING acknowledgement, whose all-zero block reports "none", which the probe reads as "no ASCII serial in this block". And it labelled the device name at offset 16, the decoder's coordinate (payload from frame byte 11), while this payload starts at byte 13, so the name sits at 14. - Severity: S4 · Category: truth · Upstream-able: yes - Location: `Strand/BLE/FrameRouter.swift` GET_HELLO probe - V0: found by the batch's V2 review with an oracle over the old and new helpers; the owner's logs hold 15 PENDING hello replies. - Fix: the probe runs only on a SUCCESS reply and passes `knownNameOffset: 14`. - V1: two `FrameRouterCommandPayloadTests` on synthetic replies pass, and both fail with the gate and the offset reverted. W06-065's row now says that two of its three tests, not all three, fail with the old offset.

W06-067 — two lines of the new clock check could claim what did not happen. "GET_CLOCK sent" printed even when `send` wrote nothing. After a Bluetooth power-off inside the 10 s window, which never reaches `didDisconnectPeripheral` and so leaves the check's token alive, the timeout logged "setting the clock without a reading" on a dead link. - Severity: S4 · Category: truth · Upstream-able: yes - Location: `Strand/BLE/BLEManager.swift` `beginStrapClockCheck` - V0: found by the batch's V2 review; by construction, since no test seam reaches the private handshake. The flags already matched the old handshake. - Fix: the sent line prints only when a sequence came back. The timeout sets the clock only on a connected link, otherwise says it is neither read nor set, and then settles the flags as before. - V1: both app builds (NOOPiOS without the watch app).

W06-068 — the clock batch's own text overstated or went stale: - "20 replies" where the logs hold 12 distinct ones. - "Seven zero bytes" stated as layout, when each GET ran in the same millisecond as its SET, so they may be sub-second ticks. - The linux-capture tools claimed to mirror `StrapClock`, but they read stream timestamps, and `whoop_setclock` writes by default. - A test comment, two restore-flag comments and CONTRIBUTING still described the old set-every-connect order. - The Backfiller copy still claimed a byte-identical Android twin. - The `validityFloor` doc cited the 146/147 contract as if it covered command 11. - Severity: S4 · Category: truth · Upstream-able: yes - V0: found by the batch's V2 review. Each item was checked before the edit: 12 distinct GET frames, and tool defaults of 0 and 30 s. - V1: `doc_comment_lint`, `check_source_references` and the linux-capture tests pass. The W06-001 and W06-050 rows now carry the 12-reply count and the console evidence that the late sets were processed 2.3–3.9 s after the phone stamped them. W06-050 also records V2's two questions for the strap run: a lost read reply, and drift.

Fourteen findings from a /code-review of review/w06-clock-read-first, each with its V0 result, and W06-083, found while checking W06-069: a Bluetooth power-off runs none of the per-link teardown.

### W06-073 — no test covered the app-layer half of W06-050 - Severity: S3 · Category: test · Upstream-able: yes - Location: `Strand/BLE/BLEManager.swift` `handleStrapClockReply`, `beginStrapClockCheck` - Reproduction: a `.settle` verdict that also sends SET_CLOCK, a handshake that settles at `beginStrapClockCheck`, or a `.set` that never settles, each passed every suite. - Fix: the clock-check state, `beginStrapClockCheck`, `handleStrapClockReply` and the timeout (now `strapClockCheckTimedOut(token:)`) are internal, and `StrapClockCheckTests` drives them on a manager with no strap, where `send` logs "ignored" for each command it is asked to write. - V1: 8 tests pass; each of the three mutations fails at least one.

### W06-069 — the power-off guard read a flag no power-off clears - Severity: S3 · Category: bug · Upstream-able: yes - Location: `Strand/BLE/BLEManager.swift` `strapClockCheckTimedOut` - Reproduction: Bluetooth off inside the 10 s window leaves `state.connected` set (no `didDisconnectPeripheral` on a power-off, W06-083), so the timeout logged "setting the clock without a reading" and asked `send` for a SET_CLOCK it then dropped; a link that came back inside the window would have been set without a reading by the old link's timer. - V0: `testTheTimeoutOnALinkThatIsGoneSetsNothingAndSaysSo` fails on the old check (a SET_CLOCK ask, the "setting" line, no "gone" line). - Fix: the timeout sets only when `peripheral?.state == .connected` (the test `send` uses) and `connectGeneration` still matches the link the check began on; otherwise it says the link is gone and sets nothing. It settles either way (W06-072). - V1: the test passes; `StrapClockCheckTests` 9/9.

### W06-071 — the offload settle was counted from the handshake, not the set - Severity: S3 · Category: bug · Upstream-able: yes - Location: `Strand/BLE/BLEManager.swift` `strapClockSettled` - Reproduction: the delay was `max(0, 1.5 − elapsed since the handshake)`, so a set sent by the 10 s timeout, or after a slow read, was followed by SEND_HISTORICAL_DATA in the same main-queue turn, against the hardware-validated SET_CLOCK → 1.5 s → offload order. - V0: elapsed ≥ 10 s on the timeout gives a delay of 0; so does a `.set` verdict after a 4 s relaunch round trip. - Fix: `strapClockSettled(setJustSent:)` takes its delay from `firstOffloadDelay`: the full 1.5 s after a set, otherwise what remains of it since the handshake. - V1: `FirstOffloadDelayTests` pass; the set case fails with the old formula.

### W06-074 — a phone clock step during the read could set a correct strap - Severity: S4 · Category: bug · Upstream-able: yes - Location: `Packages/WhoopProtocol/.../StrapClock.swift` `judge`, `Check` - Reproduction: phone 4 s fast, stepped back during the read, strap right: `judge(sentAt: T+4, receivedAt: T+0.2)` gave `off(-4.0…-3.0)`, so the strap was set, the step W06-050 exists to prevent. A test pinned the narrowing. - V0: oracle (`swiftc -O StrapClock.swift main.swift`) printed that verdict. - Fix: `judge(_:receivedAt:roundTrip:)` samples the wall clock once, at the reply, and takes the round trip from a monotonic clock. `Check` records its requests on that clock; `BLEManager.monotonicSeconds()` reads `ContinuousClock`, which also counts through sleep. - V1: the V0 case is now in sync (−0.25…+1.0 s); two tests added; WhoopProtocol 769 tests and StrapClockCheckTests pass.

### W06-075 — each set left the strap up to 1 s behind - Severity: S4 · Category: bug · Upstream-able: yes - Location: `Strand/BLE/BLEManager.swift` `setStrapClock` - Reproduction: the set truncated the phone time to whole seconds with zero subseconds, so a set the strap applied late was judged off and set again on the next connect. - V0: oracle over 1,000 stamp phases × 10 read phases, set applied 2.3 s late, next read over a 0.1 s round trip: truncated, off in 6,792 of 10,000; rounded, 2,443. A promptly applied set is never judged off either way. - Fix: `StrapClock.setSeconds(forPhoneTime:)` rounds to the nearest second. The payload keeps zero subseconds (their unit is unverified on 5/MG), and WHOOP 4 is untouched. - V1: `testASetStampsTheNearestSecond`; the oracle counts are pinned in `testARoundedStampIsReSetLessOftenAfterALateApply`.

### W06-082 — `judge` took a threshold that `describe` could not report - Severity: S4 · Category: truth · Upstream-able: yes - Location: `Packages/WhoopProtocol/.../StrapClock.swift` `judge`, `describe` - Reproduction: `judge(…, threshold: 5)` on a strap 7 s off returned `.off`, which `describe` printed as "more than 2 s off". - V0: oracle printed exactly that line. No caller passed a threshold. - Fix: the parameter is gone; `judge` reads `driftThresholdSeconds`, the constant `describe` prints. - V1: StrapClockTests pass; the protocol source-reference check passes.

### W06-076 — a protocol offset was hard-coded in app code - Severity: S4 · Category: design · Upstream-able: yes - Location: `Strand/BLE/FrameRouter.swift` GET_HELLO probe - Reproduction: FrameRouter passed `knownNameOffset: 14`, the decoder's name offset 16 minus the two-byte shift between two payload coordinates, with nothing tying the two; a moved name offset would leave the probe labelling the wrong run. - Fix: the decoder reads the name at `whoop5HelloNameOffset`; `HelloIdentityProbe.whoop5NameOffsetAfterResult` derives from it, the probe's default is the decoder's offset, and FrameRouter passes the derived constant. - V1: `testTheAfterResultNameOffsetIsWhereTheDecoderReadsTheName` and `FrameRouterCommandPayloadTests` pass.

### W06-079 — per-connect clock readouts went into every default log - Severity: S4 · Category: truth · Upstream-able: yes - Location: `Strand/BLE/BLEManager.swift` `beginStrapClockCheck`, `handleStrapClockReply` - Reproduction: "GET_CLOCK sent" and the in-sync verdict were plain `log()` calls on every connect, against the AGENTS.md rule to gate per-connect readouts behind Test Centre and keep rare events always-on. - V0: `testRoutineClockLinesNeedTestCentreAndASetIsAlwaysLogged` fails on the old code (the in-sync line appears without Test Centre). - Fix: both lines go through `clockReadout` (Test Centre → Connection, tagged); the unresolved verdict, every set, the timeout, PENDING and stray replies, the readback and the not-sent line stay always-on. - V1: the test passes; a set is still logged without Test Centre.

### W06-080 — comments described the old clock order or claimed what no code does - Severity: S4 · Category: truth · Upstream-able: yes - Location: `Strand/BLE/BLEManager.swift` `armStrapAlarm` doc, `send()` guard and allowlist; `Packages/WhoopProtocol/.../Interpreter.swift` - Reproduction: the alarm doc said the RTC is set during the connect handshake and the allowlist that SET_CLOCK is mandatory before history, both from the set-every-connect order; the guard comment said a power-off publishes `state.connected = false` (nothing does, W06-083); the decoder comment said 50.38.1.0 does not serve GET_CLOCK, a misreading of the upstream text (W06-070). - Fix: each comment now states what the code does. A sweep found no other set-every-connect claim outside docs/review/.

### W06-081 — "by default it always writes" was false - Severity: S4 · Category: truth · Upstream-able: yes - Location: `Tools/linux-capture/whoop_setclock.py`, `Tools/linux-capture/README.md` - Reproduction: the check is `abs(drift) <= args.if_drift`, so with the default of 0 a strap reading the phone's whole second is left alone, while the docstring, the `--if-drift` help and the README said the tool always writes. - V0: `abs(0) <= 0` is true. - Fix (text): each place states the real default. The README and docstring also say the app reads a 5/MG first but sets a WHOOP 4 on every connect. - V1: the linux-capture unit tests pass.

W06-069 … W06-083: ten fixed, W06-070 and W06-072 not bugs, W06-077 and W06-078 proposed for Phase 5, W06-083 left for Phase 3. WhoopProtocol, StrandTests and both app builds pass.

A max-effort /code-review of review/w06-clock-read-first (one verifier per candidate, then a gap sweep) reported 23 findings, recorded as W06-084 … W06-106. The S3s: W06-084 (on a restored link the read waits behind the re-subscribe burst, so the check cannot judge in sync), W06-085 (a power-off before the reply settles the connect on the dead link), W06-086 (the first-offload delay counts on the wall clock), W06-092 (the revived hello probe prints serial-shaped runs in full) and W06-096 (no test reaches the timeout's set, the dispatch or the handshake call). Earlier rows point at their successors; the board and the log carry the next steps.

W06-085. When Bluetooth powered off before the GET_CLOCK reply, the timeout's gone branch still settled the connect: `connectSettled` fired the alarm re-arm into a send the dead link dropped, then latched, and the reconnect, which inherits `whoop5SessionStarted` and skips the 5/MG handshake (W06-083), got no clock check at all. The old handshake set the clock and bumped `connectSettled` on the live link. The timeout now settles only the link the check began on. A newer link that is up and past its CLIENT_HELLO gets a check of its own, which settles it on a live link; otherwise `whoop5SessionStarted` is cleared, so the next CLIENT_HELLO acknowledgement runs the whole handshake, check included. What the timeout sees of the link is `ClockCheckLink`, which tests pass in, since they cannot connect a peripheral. - Severity: S3 · Category: bug · Upstream-able: no - Location: `Strand/BLE/BLEManager.swift` `strapClockCheckTimedOut` - Reproduction: Bluetooth off after GET_CLOCK and before its reply → the check settles on the dead link (history ready, `connectSettled` bumped, session flag left set); expected no settle there, and a clock check on the next link - V0: `testTheTimeoutOnALinkThatIsGoneSettlesNothingAndLetsTheNextLinkRunTheHandshake` failed five ways on the old handler - V1: it, `testTheTimeoutWithANewerLinkUpChecksThatLinkInstead` and `testTheTimeoutOnTheSameLinkSetsTheClockWithoutAReadingAndSettles` pass, and all three fail with the old logic behind the new signature; removing the timeout's `setStrapClock()` fails the last one. StrandTests (2,029) and both app builds pass (iOS without the watch app). - V3 due: Bluetooth off and on inside a connect's first 10 s, on the strap

W06-084. On a state-restored link the one GET_CLOCK read goes out behind the relaunch's ~20 notify re-subscribe writes, so its reply comes back 3–5 s later (5 of 8 restored-link connects in the owner's strap logs). `judge`'s range is 1 s plus the round trip wide, so above 3 s it cannot say "in sync": an in-sync strap always read unresolved, logged always-on, and a strap 3–5 s off was never set on such a link. A first reply that comes back more than 1 s after its request with an unresolved verdict is now read once more, under a timeout of its own, and that second read decides. If it goes unanswered, the first reading stands and nothing is set. Up to 1 s the range is at most 2 s wide, so a strap exactly in sync is always judged in sync, which is why the second read starts there. The second-read line is a routine readout behind Test Centre. - Severity: S3 · Category: bug · Upstream-able: no - Location: `Packages/WhoopProtocol/.../StrapClock.swift` `Check`; `Strand/BLE/BLEManager.swift` `handleStrapClockReply`, `strapClockCheckTimedOut` - Reproduction: a restored link answers GET_CLOCK after 3.2–4.7 s → an in-sync strap is unresolved 100 of 100, a strap 3, 4 or 5 s behind is unresolved 100 of 100 and never set; expected in sync, and a set - V0: the review's oracle over `judge` and the owner's strap logs - V1: `testASecondReadResolvesWhatARestoredLinksRoundTripCannot` runs that oracle through the check (second reads 0.3 s): in sync 100 of 100, 4 and 5 s behind set 100 of 100, 3 s behind set 74 and unresolved 26. Five more StrapClockTests and three StrapClockCheckTests pin the steps. With the second read disabled, 17 package assertions and all three app tests fail; with the standing verdict ignored, the timeout test fails. WhoopProtocol (778), StrandTests (2,032) and both app builds pass (iOS without the watch app). - V3 due: a relaunch with Test Centre on logs the second read and an in-sync verdict

W06-086. The first-offload delay counted from `whoop5HandshakeAt = Date()` and had no upper bound, so a phone clock stepped back during the clock check postponed the connect's offload by the size of the step: an hour's step gave a delay of 3,601 s. The delayed request also checked no link, so it could run on a later link and spend a parked Sync Strap request before that link's handshake was done. The handshake now stamps `monotonicSeconds()`, the delay is clamped to 0…1.5 s, and the delayed request carries the clock-check token, which after a settle only a disconnect or a later link's check moves. A stale request is dropped with an always-on line, and the scheduling line now states the delay. - Severity: S3 · Category: bug · Upstream-able: no - Location: `Strand/BLE/BLEManager.swift` `strapClockSettled`, `firstOffloadDelay` - Reproduction: elapsed −3,600 s → delay 3,601.5 s, expected at most 1.5 s; a request scheduled on one link running after a later link's check → the parked Sync Strap request spent, expected it to wait - V0: `testANegativeElapsedTimeCannotPostponeTheOffload` and `testAFirstOffloadRequestFromAnEndedLinkIsDropped` failed on the old code - V1: both pass, with `testTheFirstOffloadWaitsWhatRemainsOfTheSettleSinceTheHandshake` and the control `testTheFirstOffloadRequestRunsOnTheLinkThatSettled`; removing the fence, the clamp or the stamp fails its test, and a fence that always drops fails the control. StrandTests (2,036) and both app builds pass (iOS without the watch app). - V3: with the batch's strap run, a connect states its offload delay and the offload follows

…robe W06-092. W06-065 revived the Test Centre GET_HELLO probe (ryanbr#1303), which quotes every 6–20 character alphanumeric run of the identity block in full. If the strap serial sits in that block, which is what ryanbr#1303 is looking for, the whole serial lands in a log a user may export, while the pack-serial line beside it and the DIS line log a serial only as a 3-character prefix (`WhoopSerialIdentity.logSafe`). A serial-shaped run now prints its first three characters and an ellipsis. With the offset and length that still matches a run against the serial on the strap's casing, which is all the probe is for. - Severity: S3 · Category: risk · Upstream-able: no - Location: `Packages/WhoopProtocol/.../HelloIdentityProbe.swift` `candidateLines`; `Strand/BLE/FrameRouter.swift` probe comment - Reproduction: a 14-character alphanumeric run in the block → the log quotes all 14; expected "3A1…" - V0: `testASerialShapedRunPrintsOnlyItsFirstThreeCharacters` printed the whole run on the old code - V1: it passes; the six probe tests that quoted whole runs and `testTheHelloProbeLabelsTheNameWhereTheDecoderReadsIt` now expect the prefix, and the last two also assert the full run is absent. WhoopProtocol (779) and the app's probe tests pass.

W06-096. W06-073's app tests left the half of W06-050 that matters unpinned: no test fed a GET_CLOCK reply through the 5/MG frame loop, told the link a check began on from a later one, or ran the handshake's call to the check. Deleting the dispatch, forcing the generation test true, or settling the handshake at once, as before W06-050, passed every clock test. Two seams, neither changing behaviour: the link the timeout sees is `classifyClockCheckLink`, a pure function behind `clockCheckLink`, and the handshake's once-per-link work moves unchanged into `startWhoop5SessionIfNeeded`, which `didWriteValueFor` calls where the block was. - Severity: S3 · Category: test gap · Upstream-able: no - Location: `StrandTests/StrapClockCheckTests.swift`; `Strand/BLE/BLEManager.swift` `clockCheckLink`, 5/MG handshake - V0: the three mutants together passed all 21 clock tests - V1: each now fails its own test (`testAClockReplyThroughTheFrameLoopSettlesTheCheck`, `testTheTimeoutTellsTheLinkItBeganOnFromALaterOne`, `testTheHandshakeBeginsTheCheckOnceAndWaitsForIt`), and dropping the check from the handshake fails the last. Still unpinned: that `didWriteValueFor` calls `startWhoop5SessionIfNeeded`, since no test can deliver a write acknowledgement. StrandTests (2,039) and both app builds pass (iOS without the watch app).

W06-087. The strap answers GET_HELLO first with PENDING and a body of zeros after the revision (`docs/PROTOCOL_TRANSPORT.md` §Hello). The decoder read that body as the info block, found no version byte, and logged the always-on "fw gate: no version decoded … at93=0" line on every connect, although the SUCCESS reply that follows decodes the firmware. The owner's strap logs hold 15 such lines, each just before "GET_HELLO(145) → PENDING(2)". W06-066 gated only the ryanbr#1303 probe. The 145 branch now decodes only a SUCCESS (1) reply, in the decoder both consumers share. - Severity: S4 · Category: truth · Upstream-able: yes - Location: `Packages/WhoopProtocol/.../Interpreter.swift` `decodeWhoop5CommandResponse` - Reproduction: a PENDING hello with a zeroed body → `fw_gate` set; expected no block decoded - V0: `testThePendingHelloAcknowledgementDecodesNoBlock` reproduced the logged line byte for byte - V1: it passes, and the control `testASuccessHelloThatFailsTheVersionGuardStillReportsTheGate` still reports the gate. The two synthetic hello fixtures had been zero-filled to a FAILURE (0) result; they now carry SUCCESS, with the CRC32 recomputed, so they still reach the name and version guards. WhoopProtocol (781) and StrandTests (2,039) pass.

W06-093. The capture paragraph this branch rewrote kept "`GET_CLOCK` (11) isn't served at all" on 50.38.1.0, after W06-080 removed the same claim from the decoder and W06-070 was closed because no doc said so. The sentence now names its source, the upstream capture that sent command 11 with the empty payload NOOP sends, says no log in this repository confirms it, and states what the clock check does on a firmware that never answers: a 10 s wait, then a set without a reading. - Severity: S4 · Category: truth · Upstream-able: no - Location: `docs/BLE_REVERSE_ENGINEERING.md` COMMAND_RESPONSE section - V0: the sentence quoted against W06-070's premise and W06-080's edit - V1: the source-reference check passes

W06-095. `StrapClock.decodeReply` read the seconds field whenever the body was long enough, whatever the result, so a failure, pending or unsupported reply with a full-length body decoded seconds 0. The FrameRouter clock line then printed "strap=0", a 1970 clock, beside the check's "no reading" verdict, and the readback line said "the strap reads 0": two readouts of one reply disagreeing, the ryanbr#1823 shape. `decodeReply`, which every line that prints a reading reads, now returns seconds only from a SUCCESS reply. - Severity: S4 · Category: truth · Upstream-able: no - Location: `Packages/WhoopProtocol/.../StrapClock.swift` `decodeReply` - Reproduction: a result-0 reply with an 11-byte zero body → seconds 0 and "strap=0"; expected no reading - V0: `testANonSuccessReplyWithAFullBodyDecodesNoSeconds` and `testAFailedClockReplyPrintsNoReading` failed on the old code - V1: both pass. WhoopProtocol (782) and StrandTests (2,040) pass.

W06-100. FrameRouter logged "clock: GET_CLOCK(11) result=SUCCESS(1) strap=…" on every 5/MG connect with only a domain tag, which does not gate, while W06-079 had put BLEManager's in-sync verdict for the same reply behind Test Centre as a routine per-connect readout (AGENTS.md: gate per-connect readouts, keep rare evidence always-on). A GET_CLOCK reply that carries a reading now logs only under Test Centre → Connection. A SET_CLOCK reply, which is how a default log shows every set, and any reply without a reading stay always-on. - Severity: S4 · Category: truth · Upstream-able: no - Location: `Strand/BLE/FrameRouter.swift` clock line - Reproduction: Test Centre off, an in-sync GET_CLOCK reply → the line in the default log; expected none - V0: `testARoutineClockReadingIsATestCentreReadout` failed on the old code - V1: it passes, with the control `testASetClockReplyAndAReplyWithoutAReadingStayAlwaysOn`; both clear the master flag and restore the host's Test Centre keys. StrandTests (2,042) passes.

W06-103. The ryanbr#1303 probe line reports offsets from frame byte 13, the payload after the result byte, and never said so, while the decoder and the probe's own doc count from byte 11. A serial found at probe offset N therefore sits at the decoder's `pay[N + 2]`, an off-by-two trap for whoever promotes a finding to a decoded field. The probe's doc also still claimed a WHOOP 4 GET_HELLO_HARVARD caller that no longer exists. The line now says "offsets from frame byte 13", and the doc explains the two coordinates and drops the dead claim. The probe keeps its own coordinates, which match the PROTOCOL_TRANSPORT response-body tables. - Severity: S4 · Category: truth · Upstream-able: no - Location: `Strand/BLE/FrameRouter.swift` GET_HELLO probe; `Packages/WhoopProtocol/.../HelloIdentityProbe.swift` docs - V0: the probe test, given an assertion on the origin, failed on the old line - V1: it passes; the probe tests and the doc checks pass

W06-089. `judge`'s own doc puts the offset in the open range (low, high), since the strap's clock is below s + 1 and the strap reads it after the request lands and before the reply. `judge` tested the range as closed, so a bound exactly on ±2 s, which already puts the whole range past the threshold, came out unresolved, and a test pinned that. Separately `describe` rounded each bound to the nearest tenth, so an off range ending at −2.04 and an unresolved one ending at −1.96 both printed "-2.0" beside opposite verdicts. `judge` now counts a bound on the threshold as off, and `describe` rounds a range outward to tenths, snapping floating-point noise first and printing zero unsigned, so the printed range always holds the true one. - Severity: S4 · Category: bug · Upstream-able: no - Location: `Packages/WhoopProtocol/.../StrapClock.swift` `judge`, `describe` - Reproduction: s = 1,789,999,997 received at 1,790,000,000.5 over 0.5 s → (−3.5, −2.0) unresolved; expected off - V0: `testARangeWhoseBoundSitsOnTheThresholdIsOff` and `testPrintedRangesRoundOutward` failed on the old code; the old edge test pinned the wrong answer and is replaced - V1: both pass. Three pinned oracles move with the boundary: W06-075's grid puts many ranges exactly on −2 s (7,543 and 3,192, from 6,792 and 2,443; rounding still cuts re-sets by more than half), W06-084's 3 s case gains one phase (75 and 25), and one log line prints −4.3 for −4.25. WhoopProtocol (783) and StrandTests (2,042) pass.

…ands W06-091. The clock check's timeout called its own link up from two of `send`'s three guard terms and left out the command characteristic, so on a link connected without one it logged "setting the clock without a reading" and settled as if a set had gone out, while `send` dropped it. The link counts as `.same` only with the command characteristic. Without it the timeout takes the `.none` path: nothing is set, and the session flag is cleared so the next CLIENT_HELLO acknowledgement runs the handshake, check included. The `.none` line now says no link is ready for the check's commands, which holds for every case it covers. - Severity: S4 · Category: truth · Upstream-able: no - Location: `Strand/BLE/BLEManager.swift` `classifyClockCheckLink`, `strapClockCheckTimedOut` - V0: the classifier table, given a row for that link, returned `.same` on the old code - V1: it passes. StrandTests (2,042) passes.

…PENDING W06-090. The clock check's "sent" line, the `send` allowlist and `armStrapAlarm` comments, `StrapClock`'s header, CONTRIBUTING, the reverse-engineering doc and a PROTOCOL_IMPLEMENTATION table row said a SET_CLOCK follows "only if" the reading is invalid or more than 2 s off. A refused or reading-less reply and the 10 s timeout set too; only the PROTOCOL_IMPLEMENTATION rule section said so. And after a PENDING answer the timeout logged "no GET_CLOCK reply", though the strap had replied. Each site now states the whole rule, the "sent" line says only what was sent, and the timeout lines say no reading came. - Severity: S4 · Category: truth · Upstream-able: no - Location: `Strand/BLE/BLEManager.swift`, `StrapClock.swift` header, `docs/CONTRIBUTING.md`, `docs/BLE_REVERSE_ENGINEERING.md`, `docs/PROTOCOL_IMPLEMENTATION.md` - V0: `testATimeoutAfterAPendingAnswerSaysNoReadingCame` failed on the old wording - V1: it passes; the doc checks and StrandTests (2,043) pass

W06-097. `decodeReply`'s guards sit on reply bodies ending at 12/13 (the result) and 16/17 (the seconds), but every fixture came from `puffinCommandFrame`, which pads to 4 bytes, so bodies ended at 12, 16, 20 or 24; and the "type 35 is our own request" assertion used a body the length guard rejected before the type check ran. `testTheGuardsSitOnTheResultAndTheSeconds` builds exact-length replies with `w5Frame`, and the type-35 assertion uses a body that passes both length guards. - Severity: S4 · Category: test gap · Upstream-able: no - Location: `Packages/WhoopProtocol/Tests/.../StrapClockTests.swift` - V0: removing the type check, and moving the guards to `>= 18`, `> 13` or `> 11`, each passed the old suite - V1: all five mutants, `>= 16` included, now fail. WhoopProtocol (784) passes.

… claims W06-098. W06-081's rewrite of the whoop_setclock text was wrong three ways: it said the phone app sets a WHOOP 4 on every connect, but NOOP never connects to one; it named "the phone's whole second" as the reference, but the tool compares with the computer's clock and needs the phone's Bluetooth off; and the README example below it still called the default run "force-set to now", which writes nothing when the reading is on the second. Three BLEManager comments added with the batch also presented the dead WHOOP 4 settle as a live path. - Severity: S4 · Category: truth · Upstream-able: no - Location: `Tools/linux-capture/README.md`, `Tools/linux-capture/whoop_setclock.py`, `Strand/BLE/BLEManager.swift` comments on `restoreNeedsResubscribe` - V0: each line quoted against `DeviceFamily` and `int(time.time())` - V1: the linux-capture tests (235) and doc_comment_lint pass

…wn request W06-099. W06-071 made the connect's first-offload request wait 1.5 s after a SET_CLOCK, and its row, a test comment, the `firstOffloadDelay` doc and PROTOCOL_IMPLEMENTATION then promised that gap for the offload itself. The settle opens the sync gate in the same turn as the set, so a strap EVENT, the Sync Strap shortcut or Sync now can ask for history sooner, as they could before; an owner log shows an EVENT's offload ahead of the 1.5 s kick, and such offloads served data on main. The text now claims the gap for the connect's own request and says other triggers can come sooner. Gating them waits on evidence that the gap matters. - Severity: S4 · Category: truth · Upstream-able: no - Location: `Strand/BLE/BLEManager.swift` `firstOffloadDelay` doc; `StrandTests/StrapClockCheckTests.swift`; `docs/PROTOCOL_IMPLEMENTATION.md` - V1: the doc checks pass

… earlier tests W06-101. `testRoutineClockLinesNeedTestCentreAndASetIsAlwaysLogged` cleared only Test Centre's connection flag, but the master flag turns on every domain, and both live in the test host's defaults, which the developer's staging app shares. Run on its own with master on, it counted one routine line where it expected none. The class now saves both keys before each test, clears master, and restores both after, as its siblings do. W06-107, found while fixing it. XCTest keeps every test case, and so its manager, alive until the run ends, and a settle schedules the first offload request 1.5 s out. One run of the class alone failed `testTheFirstOffloadRequestRunsOnTheLinkThatSettled` (W06-086): an earlier test's request fired during it and spent its parked Sync Strap request, which also lives in the host's defaults. `tearDown` now releases the manager and its `LiveState`; a released manager deallocates with a request pending, so the request finds nothing. - Severity: S4 each · Category: test gap · Upstream-able: no - Location: `StrandTests/StrapClockCheckTests.swift` setUp/tearDown - V0: W06-101, the test alone with master set failed as predicted; W06-107, the observed failure (timing-dependent: three later runs without the fix passed) - V1: with master set, the class alone passed three times, and the host's defaults were restored after each run; a temporary test confirmed that a released manager deallocates. StrandTests (2,043) passes.

W06-102. The stuck-strap watchdog's recovery sends SET_CLOCK with a truncated stamp and no read or readback, and the 5/MG allowlist admits it. Nothing arms the watchdog on 5/MG today, because `strapNewestTs` is fed only when the data-range handler's `feedsSync` is true and 5/MG passes false, but the comment at that call invites the flip. The comment now says the flip arms a blind set past W06-050's read-first check and must route it through the check first. The fix itself is deferred to W06-021's rework of the detector. - Severity: S4 · Category: risk · Upstream-able: no - Location: `Strand/BLE/BLEManager.swift` 5/MG data-range call - V0: the watchdog, allowlist and `feedsSync` lines, quoted - V1: doc_comment_lint passes

Records the rest of the second review of `review/w06-clock-read-first`: W06-087 … W06-103 fixed one commit each, W06-107 found and fixed with W06-101, W06-088 and W06-094 deferred with reasons (W06-102 was deferred in its own commit), W06-021 corrected, and the checkpoint: WhoopProtocol (784), StrandTests (2,043), both app builds and the hygiene gates. The board lists what the strap run must show.

ayiskakov added 30 commits September 26, 2026 12:22

docs(review): record the clock batch's code review as W06-069 … W06-083

e2d2845

Fourteen findings from a /code-review of review/w06-clock-read-first, each with its V0 result, and W06-083, found while checking W06-069: a Bluetooth power-off runs none of the per-link teardown.

docs(review): log the clock batch's code-review fixes

b1318f2

W06-069 … W06-083: ten fixed, W06-070 and W06-072 not bugs, W06-077 and W06-078 proposed for Phase 5, W06-083 left for Phase 3. WhoopProtocol, StrandTests and both app builds pass.

ayiskakov added 8 commits September 26, 2026 17:28

ayiskakov merged commit bbdf238 into main Sep 26, 2026
15 checks passed

Navigation Menu

Repository navigation

BLE (W6): read the 5/MG strap clock before setting it, and the batch's review fixes - #33

BLE (W6): read the 5/MG strap clock before setting it, and the batch's review fixes#33
ayiskakov merged 38 commits into
mainfrom
review/w06-clock-read-first

ayiskakov commented Sep 26, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

ayiskakov commented Sep 26, 2026

Why

What changes

Tests only

Recorded, not changed

Verification

Not yet tested on hardware

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant