From 0dd44d9f697491fc5bb6178c8ef07a1b4a18470d Mon Sep 17 00:00:00 2001 From: kavemang <66108693+kavemang@users.noreply.github.com> Date: Thu, 8 Oct 2026 04:31:00 -0500 Subject: [PATCH 1/7] fix(android): fingerprint the full stress curve HR union Refs #2710 --- .../com/noop/widget/StressWidgetProducer.kt | 19 +-- .../noop/widget/StressWidgetUnionCacheTest.kt | 128 ++++++++++++++++++ 2 files changed, 139 insertions(+), 8 deletions(-) create mode 100644 android/app/src/test/java/com/noop/widget/StressWidgetUnionCacheTest.kt diff --git a/android/app/src/main/java/com/noop/widget/StressWidgetProducer.kt b/android/app/src/main/java/com/noop/widget/StressWidgetProducer.kt index 52ffc24630..924c5c863f 100644 --- a/android/app/src/main/java/com/noop/widget/StressWidgetProducer.kt +++ b/android/app/src/main/java/com/noop/widget/StressWidgetProducer.kt @@ -18,10 +18,10 @@ import kotlinx.coroutines.withContext * and putting three reads of that size on a repeating cadence is precisely the pattern this codebase * has had to unpick before (the analyze-pass storm, and the 21-night re-score behind it). * - * So nothing is read until [WhoopRepository.hrFingerprintWindow] says today's heart rate actually - * moved. That fingerprint is a COUNT and a MAX over an indexed column: no rows, no decode. On an idle - * tick, which is almost every tick, this costs one cheap query and returns the previous curve. Stress - * is scored hourly, so even a busy day recomputes about as often as it has new hours. + * So nothing is read until [WhoopRepository.hrUnionFingerprint] says today's heart rate actually + * moved. That fingerprint is a COUNT and a MAX per source over indexed columns: no rows, no decode. + * On an idle tick, which is almost every tick, this costs only those aggregates and returns the previous + * curve. Stress is scored hourly, so even a busy day recomputes about as often as it has new hours. * * SCORING MODE. Background callers keep the DayRelative default: resolving the personal lens reads * trailing days of heart rate, a cost a foreground screen can afford and an unprompted tick cannot. @@ -44,7 +44,7 @@ internal object StressWidgetProducer { * scored against. Only the view model calls this today, so that is a narrow window, but an * immutable holder closes it for free and survives a second caller being added later. */ private data class Memo( - val fingerprint: Pair, + val fingerprint: String, val day: Long, val personalBaseline: Boolean, val points: List, @@ -162,9 +162,12 @@ internal object StressWidgetProducer { val day = window.day.toEpochDay() val from = window.fromEpochSecond - val fingerprint = repo.hrFingerprintWindow(deviceId, from, nowSeconds) - // Same day, same heart rate: nothing can have changed the score, so nothing is read. The day - // is part of the check because a fingerprint that happens to match across midnight would + val fingerprint = repo.hrUnionFingerprint(deviceId, from, nowSeconds) + // Fingerprint the same source union that hrSamplesUnion reads: a backfill under an archived + // strap or canonical import must invalidate the curve even when the active strap is idle. + // The source ids are part of the witness, so switching active sources cannot replay a + // curve whose active-first precedence differs. The day is part of the check because a + // fingerprint that happens to match across midnight would // otherwise serve yesterday's curve as today's. // The foreground personal lens and the background widget can call this producer in either // order. The preference is part of the identity so one surface can never receive the other diff --git a/android/app/src/test/java/com/noop/widget/StressWidgetUnionCacheTest.kt b/android/app/src/test/java/com/noop/widget/StressWidgetUnionCacheTest.kt new file mode 100644 index 0000000000..9f57d7f927 --- /dev/null +++ b/android/app/src/test/java/com/noop/widget/StressWidgetUnionCacheTest.kt @@ -0,0 +1,128 @@ +package com.noop.widget + +import com.noop.data.HrSample +import com.noop.data.PairedDeviceRow +import com.noop.data.WhoopDao +import com.noop.data.WhoopRepository +import java.lang.reflect.Proxy +import java.time.Instant +import java.time.ZoneId +import kotlinx.coroutines.runBlocking +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test + +/** Exercises the producer's actual memo gate with an in-memory DAO, without Room or a strap. */ +class StressWidgetUnionCacheTest { + private val now = Instant.parse("2026-10-01T12:00:00Z").epochSecond + private val active = "whoop-active" + + @Before fun reset() = StressWidgetProducer.resetForTest() + + private class Data { + val rows = mutableMapOf>() + val registered = mutableListOf() + val rowReads = mutableListOf() + + private fun device(id: String) = PairedDeviceRow( + id = id, brand = "WHOOP", model = "4.0", nickname = null, + sourceKind = "strap", capabilities = "hr", status = "archived", + addedAt = 0L, lastSeenAt = 0L, + ) + + val repo: WhoopRepository = WhoopRepository( + Proxy.newProxyInstance( + WhoopDao::class.java.classLoader, arrayOf(WhoopDao::class.java), + ) { _, method, args -> + when (method.name) { + "pairedDevices" -> registered.map(::device) + "countHrInWindow", "maxHrTsInWindow", "hrSamples" -> { + val id = args[0] as String + val from = args[1] as Long + val to = args[2] as Long + val samples = rows[id].orEmpty().filter { it.ts in from..to } + when (method.name) { + "countHrInWindow" -> samples.size + "maxHrTsInWindow" -> samples.maxOfOrNull { it.ts } ?: 0L + else -> { rowReads += id; samples } + } + } + else -> throw UnsupportedOperationException("producer touched ${method.name}") + } + } as WhoopDao, + ) + } + + // Below the scoring minimum deliberately: these tests measure whether a day's streams are read, + // not analytics outputs. A genuine empty curve is cached too and must invalidate on new HR. + private fun curve(data: Data, id: String = active, personal: Boolean = false) = runBlocking { + StressWidgetProducer.todayCurve(data.repo, id, personal, now, ZoneId.of("UTC")) + } + + @Test fun unchangedUnionReusesTheCurveWithoutFetchingRows() { + val data = Data() + assertNotNull(curve(data)) + val reads = data.rowReads.toList() + assertTrue(reads.isNotEmpty()) + assertNotNull(curve(data)) + assertEquals(reads, data.rowReads) + } + + @Test fun canonicalImportInvalidatesAnIdleActiveStrapsCurve() { + val data = Data() + assertNotNull(curve(data)) + data.rowReads.clear() + data.rows["my-whoop"] = listOf(HrSample("my-whoop", now - 60, 65)) + assertNotNull(curve(data)) + assertEquals(listOf(active, "my-whoop"), data.rowReads) + data.rowReads.clear() + assertNotNull(curve(data)) + assertTrue("the refreshed curve must be memoized", data.rowReads.isEmpty()) + } + + @Test fun archivedSourceBackfillInvalidatesEvenWhenNewestTimestampDoesNotMove() { + val data = Data() + val alias = "whoop-archived" + data.registered += alias + data.rows[alias] = listOf(HrSample(alias, now - 60, 65)) + assertNotNull(curve(data)) + data.rowReads.clear() + data.rows[alias] = data.rows.getValue(alias) + HrSample(alias, now - 120, 64) + assertNotNull(curve(data)) + assertEquals(listOf(active, alias, "my-whoop"), data.rowReads) + } + + @Test fun newlyRegisteredSourceInvalidatesThePreviouslyEmptyCurve() { + val data = Data() + assertNotNull(curve(data)) + data.rowReads.clear() + data.registered += "whoop-restored" + data.rows["whoop-restored"] = listOf(HrSample("whoop-restored", now - 60, 65)) + assertNotNull(curve(data)) + assertEquals(listOf(active, "whoop-restored", "my-whoop"), data.rowReads) + } + + @Test fun switchingActiveSourcesWithEqualCountsStillReloads() { + val data = Data() + assertNotNull(curve(data)) + data.rowReads.clear() + assertNotNull(curve(data, id = "whoop-other")) + assertEquals(listOf("whoop-other", "my-whoop"), data.rowReads) + } + + @Test fun bothLensesKeepTheirOwnWarmUnionGate() { + val data = Data() + assertNotNull(curve(data)) + assertNotNull(curve(data, personal = true)) + data.rowReads.clear() + assertNotNull(curve(data)) + assertNotNull(curve(data, personal = true)) + assertTrue(data.rowReads.isEmpty()) + data.rows["my-whoop"] = listOf(HrSample("my-whoop", now - 60, 65)) + assertNotNull(curve(data)) + assertNotNull(curve(data, personal = true)) + assertEquals(listOf(active, "my-whoop", active, "my-whoop"), data.rowReads) + } +} From 4bac7105d4ccdaa7b0795333215831cdeb510a37 Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Fri, 9 Oct 2026 19:54:46 +1300 Subject: [PATCH 2/7] docs(stress): name the producer among the union-witness callers The caller list was written as an exhaustive set, so it goes stale the moment a gate joins. StressWidgetProducer's memo became the fifth. Refs #2710 --- .../app/src/main/java/com/noop/data/WhoopRepository.kt | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/android/app/src/main/java/com/noop/data/WhoopRepository.kt b/android/app/src/main/java/com/noop/data/WhoopRepository.kt index 3551960cde..3eba037006 100644 --- a/android/app/src/main/java/com/noop/data/WhoopRepository.kt +++ b/android/app/src/main/java/com/noop/data/WhoopRepository.kt @@ -1190,9 +1190,11 @@ class WhoopRepository( * * The single union witness for every caller (#2566): the cycle load cache in * [com.noop.analytics.PhysiologicalStepCycleEngine], the daytime stress lens memo in - * [com.noop.ui.selectedDaytimeStressMode], the Stress screen's periodic re-read gate and the widget - * refresh worker's stored gate (#2710). Two of these that were free to disagree is what #2566 - * removed, so route a new caller here rather than adding another. + * [com.noop.ui.selectedDaytimeStressMode], and all three stress gates standing in front of a union + * read (#2710) — [com.noop.widget.StressWidgetProducer]'s memo, the Stress screen's periodic + * re-read, and [com.noop.widget.StressWidgetRefresh.storedFingerprint]'s stored value. Two of these + * that were free to disagree is what #2566 removed, so route a new caller here rather than adding + * another. */ suspend fun hrUnionFingerprint(activeDeviceId: String, from: Long, to: Long): String { // An explicit loop rather than joinToString: the per-id read suspends and that builder's lambda From 9229293b8614973599846a5f8004768d9694026e Mon Sep 17 00:00:00 2001 From: "Jordan.#2139" <65438497+Jordan2139@users.noreply.github.com> Date: Fri, 9 Oct 2026 02:31:08 -0400 Subject: [PATCH 3/7] feat(apple): show battery pack charge Route the validated WHOOP 5/MG event 109 reading into Apple live state so the Devices card matches Android without new BLE traffic. Clear stale readings on pack removal and disconnect. --- Strand/BLE/FrameRouter.swift | 21 +++++ Strand/BLE/LiveState.swift | 13 +++ Strand/Resources/Localizable.xcstrings | 58 +++++++++++++ Strand/Screens/DevicesView.swift | 18 +++- StrandTests/FrameRouterBatteryPackTests.swift | 85 +++++++++++++++++++ 5 files changed, 194 insertions(+), 1 deletion(-) create mode 100644 StrandTests/FrameRouterBatteryPackTests.swift diff --git a/Strand/BLE/FrameRouter.swift b/Strand/BLE/FrameRouter.swift index 73615c3067..ac43f2b3e6 100644 --- a/Strand/BLE/FrameRouter.swift +++ b/Strand/BLE/FrameRouter.swift @@ -513,6 +513,27 @@ public final class FrameRouter { state.charging = true } else if ev.hasPrefix("BATTERY_PACK_REMOVED") { state.charging = false + // A detached pack must not leave its last charge sitting on the device card. + state.packSocPct = nil + } + // The 5/MG pack's own charge. The strap volunteers the full pack record in uncatalogued + // event 109 every couple of minutes while a pack is attached, so this adds no command, + // send-allowlist entry, polling, or other wire traffic. Match the event BYTE rather than + // its rendered "0x6D(109)" label: adding a schema name later must not break the route. + // + // `BatteryPackInfo.displayable` is the one shared safety gate for both transports: + // present, carrying a charge, and inside 0...100. A moved offset therefore yields no + // reading rather than a confident wrong one. Unlike Android's history path, this router is + // live-only, so a replayed offload event cannot move live state. + if family == .whoop5, frame.count > 10, + Int(frame[10]) == BatteryPackInfo.packInfoEvent, + let info = BatteryPackInfo.decodeEventFrame(frame) { + if info.displayable, let soc = info.socPct { + // SoC only: pack presence does not prove that current is flowing (#1935). + state.packSocPct = soc + } else if !info.present { + state.packSocPct = nil + } } // The other physical inputs the strap exposes — live only, as above. The double-tap // was handled before the sync kick. diff --git a/Strand/BLE/LiveState.swift b/Strand/BLE/LiveState.swift index c0122d1f09..266740857a 100644 --- a/Strand/BLE/LiveState.swift +++ b/Strand/BLE/LiveState.swift @@ -127,6 +127,18 @@ public final class LiveState: ObservableObject { /// link. Flag ONLY — the battery % keeps its family-specific source (#77). @Published public var charging: Bool? = nil + /// The attached WHOOP 5/MG battery pack's own charge, in percent with tenths precision. + /// + /// This is separate from [batteryPct], which is the STRAP's charge. The strap volunteers the pack + /// record in pushed event 109 every couple of minutes while a pack is attached; no command or polling + /// is needed. nil until that event lands, and cleared on BATTERY_PACK_REMOVED or disconnect so a stale + /// pack reading can never outlive the accessory or link. A WHOOP 4.0 has no pack fuel-gauge reading; + /// its command-98 voltage remains in [batteryMv], which is a different quantity. + /// + /// Do not infer [charging] from this value. Event 109 proves pack PRESENCE and charge, not that current + /// is flowing; the strap's BATTERY_LEVEL gauge remains the charging authority (#1935). + @Published public var packSocPct: Double? = nil + /// The Oura ring's current wear/charge state (nil for non-Oura straps or before any evidence this /// session). Driven by OuraLiveSource from the live-HR push + the ring's STATE charger strings: a live /// beat only comes from a finger (`.worn`); "chg. detected"/"stopped" bracket `.charging`; a silent @@ -757,6 +769,7 @@ public final class LiveState: ObservableObject { lastFrameAtUnix = nil // #987: a stale "last frame" freshness must not outlive it either ouraWearState = nil // a stale worn/charging badge must not outlive the link either ouraBatteryPct = nil // nor a stale ring charge (#2075) + packSocPct = nil // nor an attached pack's charge } /// Cap on the in-app strap-log ring buffer. Raised from the old ~1h (200 lines) to retain a rolling diff --git a/Strand/Resources/Localizable.xcstrings b/Strand/Resources/Localizable.xcstrings index de94f88c9f..86b8f4b9d3 100644 --- a/Strand/Resources/Localizable.xcstrings +++ b/Strand/Resources/Localizable.xcstrings @@ -38635,6 +38635,64 @@ } } }, + "Battery pack %.1f%%": { + "localizations": { + "de": { + "stringUnit": { + "state": "translated", + "value": "Akkupack %.1f%%" + } + }, + "es": { + "stringUnit": { + "state": "translated", + "value": "Batería externa %.1f%%" + } + }, + "fr": { + "stringUnit": { + "state": "translated", + "value": "Batterie externe %.1f %%" + } + }, + "it": { + "stringUnit": { + "state": "translated", + "value": "Pacco batteria %.1f%%" + } + }, + "pt-PT": { + "stringUnit": { + "state": "translated", + "value": "Bateria externa %.1f%%" + } + }, + "pl": { + "stringUnit": { + "state": "translated", + "value": "Moduł baterii %.1f%%" + } + }, + "ru": { + "stringUnit": { + "state": "translated", + "value": "Аккумуляторный блок %.1f%%" + } + }, + "zh-Hans": { + "stringUnit": { + "state": "translated", + "value": "电池包 %.1f%%" + } + }, + "zh-Hant": { + "stringUnit": { + "state": "translated", + "value": "電池包 %.1f%%" + } + } + } + }, "Battery voltage %.2f volts": { "localizations": { "de": { diff --git a/Strand/Screens/DevicesView.swift b/Strand/Screens/DevicesView.swift index 151ae456d0..c7ed8d96a6 100644 --- a/Strand/Screens/DevicesView.swift +++ b/Strand/Screens/DevicesView.swift @@ -199,6 +199,10 @@ private struct DevicesContent: View { whoopPct: live.batteryPct, ringPct: live.ouraBatteryPct) : nil, liveBatteryMv: (device.status == .active && live.connected) ? live.batteryMv : nil, + livePackSocPct: (device.status == .active && live.connected + && DeviceFamily.isWhoop5Registry(model: device.model, + brand: device.brand)) + ? live.packSocPct : nil, // Firmware version for the ACTIVE strap. It's a STABLE property (NOOP can't change a // strap's firmware), so prefer the live handshake value but fall back to the last-known // persisted firmware (written on connect in FrameRouter) when the live value is momentarily @@ -748,6 +752,9 @@ private struct DeviceCard: View { var liveBatteryPct: Int? = nil /// #592: strap pack voltage (mV) for the active+connected strap; nil otherwise. Shown beside the percent. var liveBatteryMv: Int? = nil + /// The attached WHOOP 5/MG battery pack's own charge. nil before its pushed pack-info event arrives, + /// when no pack is attached, or for every other device family. + var livePackSocPct: Double? = nil /// The active+connected strap's firmware version (from the connect handshake). nil when not the /// active/connected device, or for a source that reports no firmware (e.g. a non-WHOOP strap). var liveFirmware: String? = nil @@ -893,6 +900,14 @@ private struct DeviceCard: View { .foregroundStyle(StrandPalette.textSecondary) .accessibilityLabel("Battery voltage \(Double(mv) / 1000.0, specifier: "%.2f") volts") } + // The pack and strap gauges are distinct facts. Show the pack once, only while the + // live event says it exists; detach/disconnect clears the source value. + if let soc = livePackSocPct { + Text("·").font(StrandFont.footnote).foregroundStyle(StrandPalette.textTertiary) + Text("Battery pack \(soc, specifier: "%.1f")%") + .font(StrandFont.footnote) + .foregroundStyle(StrandPalette.textSecondary) + } if let layout = liveHistoryLayout { Text("·").font(StrandFont.footnote).foregroundStyle(StrandPalette.textTertiary) Text("v\(layout) history") @@ -1760,7 +1775,8 @@ struct DeviceCardCatalog: View { onMakeActive: {}, onRename: {}, onRemove: nil) DeviceCard(device: Self.dev("whoop-5d", "WHOOP", "5.0 MG", Self.whoopCaps.union([.steps])), - isActive: false, isLiveConnected: false, + isActive: true, isLiveConnected: true, + liveBatteryPct: 64, livePackSocPct: 56.9, onMakeActive: {}, onRename: {}, onRemove: {}) // #221: a WHOOP 5/MG that's BLE-connected but whose encrypted bond was refused (#78) — no // data flows despite the link being up. Renders the "Connected · not paired" pill + the diff --git a/StrandTests/FrameRouterBatteryPackTests.swift b/StrandTests/FrameRouterBatteryPackTests.swift new file mode 100644 index 0000000000..012af60003 --- /dev/null +++ b/StrandTests/FrameRouterBatteryPackTests.swift @@ -0,0 +1,85 @@ +import XCTest +@testable import Strand +import WhoopProtocol + +/// End-to-end routing for the WHOOP 5/MG battery pack's own charge: an intact pushed event reaches +/// `LiveState`, the shared displayability gate refuses an impossible value, and both detach and link +/// teardown clear the reading. The event payloads are the same real-hardware vectors pinned by the +/// Swift/Kotlin protocol twins; only the verified WHOOP 5 envelope is assembled here. +@MainActor +final class FrameRouterBatteryPackTests: XCTestCase { + private func bytes(_ hex: String) -> [UInt8] { + stride(from: 0, to: hex.count, by: 2).map { + let start = hex.index(hex.startIndex, offsetBy: $0) + let end = hex.index(start, offsetBy: 2) + return UInt8(hex[start.. [UInt8] { + var inner: [UInt8] = [48, 1, event, 0, 0, 0, 0, 0] + payload + let pad = (4 - inner.count % 4) % 4 + if pad > 0 { inner += [UInt8](repeating: 0, count: pad) } + + let declaredLength = inner.count + 4 + var frame: [UInt8] = [0xAA, 0x01, + UInt8(declaredLength & 0xFF), UInt8((declaredLength >> 8) & 0xFF), + 0x00, 0x01] + let headerCRC = crc16Modbus(Array(frame[0..<6])) + frame += [UInt8(headerCRC & 0xFF), UInt8(headerCRC >> 8)] + frame += inner + let payloadCRC = crc32(inner) + frame += [UInt8(payloadCRC & 0xFF), UInt8((payloadCRC >> 8) & 0xFF), + UInt8((payloadCRC >> 16) & 0xFF), UInt8((payloadCRC >> 24) & 0xFF)] + return frame + } + + /// Real WHOOP MG event-109 payload, fw 50.39.1.0: pack WBB5AP0000001 at 56.9%. + private let packEvent569 = "f5481c000100005e005301574242354150303030303030310000003902010c00" + + private func router(_ live: LiveState) -> FrameRouter { + let router = FrameRouter(state: live) + router.family = .whoop5 + return router + } + + func testIntactPushedPackEventPublishesItsOwnCharge() { + let live = LiveState() + let frame = eventFrame(UInt8(BatteryPackInfo.packInfoEvent), payload: bytes(packEvent569)) + XCTAssertTrue(parseFrame(frame, family: .whoop5).ok, "fixture must pass the full integrity gate") + + router(live).handle(frame: frame) + + XCTAssertEqual(live.packSocPct ?? -1, 56.9, accuracy: 1e-9) + XCTAssertNil(live.charging, "pack presence must not claim that current is flowing") + XCTAssertNil(live.batteryPct, "the pack gauge must not overwrite the strap gauge") + } + + func testImpossiblePackChargeDoesNotReachLiveState() { + var payload = bytes(packEvent569) + payload[27] = 0xE9 // record base 4 + SoC offset 23: 1001 tenths = 100.1% + payload[28] = 0x03 + + let live = LiveState() + router(live).handle(frame: eventFrame(UInt8(BatteryPackInfo.packInfoEvent), payload: payload)) + + XCTAssertNil(live.packSocPct) + } + + func testPackDetachAndDisconnectClearTheReading() { + let live = LiveState() + let r = router(live) + r.handle(frame: eventFrame(UInt8(BatteryPackInfo.packInfoEvent), payload: bytes(packEvent569))) + XCTAssertNotNil(live.packSocPct) + + r.handle(frame: eventFrame(22)) // BATTERY_PACK_REMOVED + XCTAssertNil(live.packSocPct) + XCTAssertEqual(live.charging, false) + + r.handle(frame: eventFrame(UInt8(BatteryPackInfo.packInfoEvent), payload: bytes(packEvent569))) + XCTAssertNotNil(live.packSocPct) + live.clearBiometrics() // the disconnect path's live-state teardown + XCTAssertNil(live.packSocPct) + } +} From b1c76595e726dc27d7d43ad5f5d660b0e7e20aa5 Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Fri, 9 Oct 2026 20:28:15 +1300 Subject: [PATCH 4/7] fix(ecg): let a probe run reach the frame that carries the result The MG ECG probe could not observe a completed reading on any strap or firmware, and its report looked correct while it happened. @meta1971's field timing on #891 is the evidence: across five MG sessions the terminal frame (classifier state 2, progress 100) landed 38 to 39 s after the first live frame, against a 30 s listen window held independently by both platforms. The verdict sheet appearing at 30 s made it worse than a missed deadline, since Stop is the next control a person reaches for. Second defect, same report: the candidate cap kept the FIRST twelve frames and R17 arrives about once a second, so the retained evidence was seconds 1 to 12 while result, average HR and the reason mask populate only on the terminal frame. The packet count stayed honest; the retained evidence did not. Whoop5EcgProbe now owns the window, the caps and the two decisions, with a declared twin on each side, so the numbers cannot drift apart again. A capture run listens 60 s, wrist-select and stop keep 30 s, a terminal frame is retained past the cap within a bounded reserve, and it re-arms the run for 10 s more rather than ending it: the variability field reads 0xffff for 2 to 9 s after that frame, so stopping on it would pin the unset value as the reading's own. A terminal frame also now ends a run EARLY, so the report quotes the time listened rather than the window asked for. The client wiring is BLE code and cannot be covered without a strap. The rules it calls are pure and pinned on both platforms, including the window against the measured 39 s, and a mutation back to the old behaviour kills the two cases that name the two defects on both sides. Tools/parity_twin_map.json carries the derived counts for the four new declarations (functions 4534 to 4538) so the authority keeps reproducing. Refs #891 --- .../WhoopProtocol/Whoop5EcgProbe.swift | 62 +++++++++++ .../Whoop5EcgProbeWindowTests.swift | 57 ++++++++++ Strand/BLE/BLEManager.swift | 65 +++++++++--- Tools/parity_twin_map.json | 12 +-- .../main/java/com/noop/ble/WhoopBleClient.kt | 100 +++++++++++++++--- .../java/com/noop/protocol/Whoop5EcgProbe.kt | 67 ++++++++++++ .../noop/protocol/Whoop5EcgProbeWindowTest.kt | 64 +++++++++++ 7 files changed, 394 insertions(+), 33 deletions(-) create mode 100644 Packages/WhoopProtocol/Tests/WhoopProtocolTests/Whoop5EcgProbeWindowTests.swift create mode 100644 android/app/src/test/java/com/noop/protocol/Whoop5EcgProbeWindowTest.kt diff --git a/Packages/WhoopProtocol/Sources/WhoopProtocol/Whoop5EcgProbe.swift b/Packages/WhoopProtocol/Sources/WhoopProtocol/Whoop5EcgProbe.swift index 3f1528c034..0522a04161 100644 --- a/Packages/WhoopProtocol/Sources/WhoopProtocol/Whoop5EcgProbe.swift +++ b/Packages/WhoopProtocol/Sources/WhoopProtocol/Whoop5EcgProbe.swift @@ -201,6 +201,68 @@ public enum Whoop5EcgProbe { } } + /// How long one run listens, in seconds, chosen by what started it. + /// + /// `capture` is 60 rather than 30 because of @meta1971's field timing in #891: across five MG + /// sessions the strap's terminal frame (classifier state 2, progress 100) landed 38 to 39 s after the + /// first live frame, so a 30 s window could not reach a completed reading on ANY strap or firmware. + /// The verdict sheet landing at 30 s made that worse than a missed deadline: Stop is the next control + /// a person reaches for, so the window trained people to end the session 8 to 9 s before the + /// classifier would have answered, and the probe then reported the silence it had caused. + /// + /// `selectOrStop` stays at 30. Those runs wait for a COMMAND_RESPONSE rather than for a reading, so + /// the longer window would be dead time in front of an answer that already arrived. + /// + /// `terminalGrace` is how long a run keeps listening AFTER the terminal frame instead of ending on + /// it. @meta1971 measured the variability field sitting at `0xffff` for 2 to 9 s past that frame + /// before it settled, so a probe that stopped on the terminal frame would pin the unset value as the + /// reading's own. It also means a run that ends early reports early: a terminal frame at 12 s renders + /// the verdict at 22 s rather than holding the sheet for the rest of the window. + public enum Window { + public static let selectOrStop = 30 + public static let capture = 60 + public static let terminalGrace = 10 + } + + /// Cap on the per-packet candidate lines one run records. Each line is rendered into both the report + /// and the strap log, so a chatty stream must not grow either without bound. + public static let maxCandidateLines = 12 + + /// Slots a run keeps in reserve for TERMINAL frames once `maxCandidateLines` is spent. + /// + /// R17 arrives at about one frame a second, so a flat cap keeps seconds 1 to 12 and discards + /// everything after. Per #891 the result, average HR and reason mask populate only on the terminal + /// frame, which means the flat cap threw away the single frame carrying the answer and kept twelve + /// that could not. A reserve rather than a bigger cap: the point is not more evidence, it is the RIGHT + /// evidence, and the ceiling stays bounded at `maxCandidateLines + maxTerminalCandidateLines`. + public static let maxTerminalCandidateLines = 4 + + /// Whether a decoded frame's line is kept, given what the run has already recorded. + /// + /// A terminal frame inside the first `maxCandidateLines` is kept by the ordinary rule and still spends + /// one reserve slot, so a run can never record more than the ceiling either way. + /// + /// Kotlin twin: `Whoop5EcgProbe.retainsCandidate`. + public static func retainsCandidate(linesKept: Int, + terminalLinesKept: Int, + isTerminal: Bool) -> Bool { + if linesKept < maxCandidateLines { return true } + return isTerminal && terminalLinesKept < maxTerminalCandidateLines + } + + /// Seconds to wait before rendering the verdict once a terminal frame lands, or nil to leave the + /// run's existing deadline alone. + /// + /// ONCE per run (`alreadyExtended`), because the strap sends more than one terminal frame: extending + /// on each would let a terminal stream push the verdict out indefinitely and the report would never + /// render. + /// + /// Kotlin twin: `Whoop5EcgProbe.terminalGraceSeconds`. + public static func terminalGraceSeconds(isTerminal: Bool, alreadyExtended: Bool) -> Int? { + guard isTerminal, !alreadyExtended else { return nil } + return Window.terminalGrace + } + /// Classify the run. /// /// Order matters, and it puts the ATTESTED signals first: the COMMAND_RESPONSE result codes are diff --git a/Packages/WhoopProtocol/Tests/WhoopProtocolTests/Whoop5EcgProbeWindowTests.swift b/Packages/WhoopProtocol/Tests/WhoopProtocolTests/Whoop5EcgProbeWindowTests.swift new file mode 100644 index 0000000000..718b79e0c8 --- /dev/null +++ b/Packages/WhoopProtocol/Tests/WhoopProtocolTests/Whoop5EcgProbeWindowTests.swift @@ -0,0 +1,57 @@ +import XCTest +@testable import WhoopProtocol + +/// #891 — the two rules that decide whether a run can OBSERVE a completed reading at all. +/// +/// Both defects they pin were invisible to every existing test, because the probe's own report looked +/// correct: the packet count was honest, the verdict wording was honest, and the run simply never +/// reached the frame that carries the answer. @meta1971's field timing (terminal frame at 38 to 39 s +/// across five MG sessions, variability unset for 2 to 9 s after it) is the evidence behind the numbers. +/// +/// The Kotlin twin pins the same cases in `Whoop5EcgProbeWindowTest`. +final class Whoop5EcgProbeWindowTests: XCTestCase { + + func testCaptureWindowOutlastsTheMeasuredTerminalFrame() { + // The whole defect in one assertion: 30 s cannot reach a frame that lands at 38 to 39 s. + XCTAssertGreaterThan(Whoop5EcgProbe.Window.capture, 39) + XCTAssertEqual(Whoop5EcgProbe.Window.capture, 60) + // A run waiting on a COMMAND_RESPONSE is not waiting on a reading, so it keeps the short window. + XCTAssertEqual(Whoop5EcgProbe.Window.selectOrStop, 30) + // Long enough to outlast the measured 2-to-9 s unset variability window after the terminal frame. + XCTAssertGreaterThan(Whoop5EcgProbe.Window.terminalGrace, 9) + XCTAssertEqual(Whoop5EcgProbe.Window.terminalGrace, 10) + } + + func testOrdinaryFramesFillTheCapAndThenStop() { + XCTAssertTrue(Whoop5EcgProbe.retainsCandidate(linesKept: 0, terminalLinesKept: 0, + isTerminal: false)) + XCTAssertTrue(Whoop5EcgProbe.retainsCandidate(linesKept: 11, terminalLinesKept: 0, + isTerminal: false)) + XCTAssertFalse(Whoop5EcgProbe.retainsCandidate(linesKept: 12, terminalLinesKept: 0, + isTerminal: false)) + XCTAssertFalse(Whoop5EcgProbe.retainsCandidate(linesKept: 99, terminalLinesKept: 0, + isTerminal: false)) + } + + func testTerminalFramesAreKeptPastTheCapButStayBounded() { + // THE #891 CASE. R17 arrives about once a second, so at the terminal frame the ordinary cap has + // long since filled with seconds 1 to 12 — the twelve frames that carry no result. + XCTAssertTrue(Whoop5EcgProbe.retainsCandidate(linesKept: 12, terminalLinesKept: 0, + isTerminal: true)) + XCTAssertTrue(Whoop5EcgProbe.retainsCandidate(linesKept: 40, terminalLinesKept: 3, + isTerminal: true)) + // Bounded: the reserve is four slots, not an exemption. + XCTAssertFalse(Whoop5EcgProbe.retainsCandidate(linesKept: 40, terminalLinesKept: 4, + isTerminal: true)) + XCTAssertEqual(Whoop5EcgProbe.maxCandidateLines + Whoop5EcgProbe.maxTerminalCandidateLines, 16) + } + + func testTheGraceExtensionHappensOncePerRun() { + XCTAssertEqual(Whoop5EcgProbe.terminalGraceSeconds(isTerminal: true, alreadyExtended: false), 10) + // A terminal STREAM must not be able to postpone the verdict indefinitely. + XCTAssertNil(Whoop5EcgProbe.terminalGraceSeconds(isTerminal: true, alreadyExtended: true)) + // An ordinary frame leaves the run's deadline alone. + XCTAssertNil(Whoop5EcgProbe.terminalGraceSeconds(isTerminal: false, alreadyExtended: false)) + XCTAssertNil(Whoop5EcgProbe.terminalGraceSeconds(isTerminal: false, alreadyExtended: true)) + } +} diff --git a/Strand/BLE/BLEManager.swift b/Strand/BLE/BLEManager.swift index 0feffde48b..25005f7910 100644 --- a/Strand/BLE/BLEManager.swift +++ b/Strand/BLE/BLEManager.swift @@ -1097,6 +1097,15 @@ public final class BLEManager: NSObject, ObservableObject { private var ecgProbeDeadline: Date? /// Supersedes a previous run's pending verdict timer when the user taps again. private var ecgProbeRunToken = 0 + /// When the run in flight began, so the report states the time actually listened rather than the + /// window asked for. A terminal frame ends a run early (`Whoop5EcgProbe.terminalGraceSeconds`), so + /// those two stopped being the same number. + private var ecgProbeStartedAt: Date? + /// Whether this run already pushed its deadline out on a terminal frame. Once only, or a terminal + /// stream would postpone the verdict forever. + private var ecgProbeTerminalExtended = false + /// How many TERMINAL candidate lines this run has kept, against the reserve the cap holds back. + private var ecgProbeTerminalLinesKept = 0 private var clockRequested = false /// #700: retry count for GET_CLOCK when no correlation establishes before backfill. Capped at 3. private var clockRetries = 0 @@ -4299,10 +4308,12 @@ public final class BLEManager: NSObject, ObservableObject { /// Sentinel shown while a probe run is in flight (twin of the #592/#690 constants). public static let ecgProbeWaiting = "__waiting__" - /// How long the probe listens for ECG-shaped packets before rendering its verdict. - private static let ecgProbeWindow: TimeInterval = 30 - /// Cap on recorded candidate-frame lines, so a chatty stream can't grow the report without bound. - private static let ecgProbeMaxCandidates = 12 + /// How long a run that is waiting on a COMMAND_RESPONSE listens before rendering its verdict. + /// Derived from `Whoop5EcgProbe.Window` so the two platforms cannot hold different numbers: that + /// they both held 30 independently is how the window outlived the evidence against it (#891). + private static let ecgProbeWindow = TimeInterval(Whoop5EcgProbe.Window.selectOrStop) + /// The CAPTURE run's window. Longer, because a reading's terminal frame lands at 38 to 39 s. + private static let ecgProbeCaptureWindow = TimeInterval(Whoop5EcgProbe.Window.capture) /// Cap on recorded steps. A run sends at most three, so the headroom is for UNSOLICITED replies — /// which are inbound-driven and therefore need the same bound as the candidate list. private static let ecgProbeMaxSteps = 12 @@ -4446,13 +4457,13 @@ public final class BLEManager: NSObject, ObservableObject { public func ecgStartCapture() { guard ecgGatesAllow() else { return } ecgMayBeRunning = true // latched BEFORE the sends, so a mid-sequence drop still leaves Stop offered - beginEcgProbeRun(clearingSteps: true) + beginEcgProbeRun(clearingSteps: true, window: BLEManager.ecgProbeCaptureWindow) log("ECG probe: starting the ECG turn-on sequence on an MG (experimental, unvalidated instrumentation)") sendEcgCommand(.toggleLabradorFiltered, arg: 1) sendEcgCommand(.toggleLabradorRawSave, arg: 1) ecgSendAbortHistorical() sendEcgCommand(.toggleLabradorDataGeneration, arg: Whoop5Ecg.ControlSignal.start.rawValue) - scheduleEcgProbeVerdict() + scheduleEcgProbeVerdict(after: BLEManager.ecgProbeCaptureWindow) } /// The START list's first member: ABORT_HISTORICAL_TRANSMITS (20), immediately ahead of `124`. @@ -4523,28 +4534,36 @@ public final class BLEManager: NSObject, ObservableObject { /// Clear the probe result (dialog dismissed). public func clearEcgProbe() { state.ecgProbe = nil } - private func beginEcgProbeRun(clearingSteps: Bool) { + private func beginEcgProbeRun(clearingSteps: Bool, + window: TimeInterval = BLEManager.ecgProbeWindow) { if clearingSteps { ecgProbeSteps = [] ecgProbeCandidates = [] ecgProbePacketsSeen = 0 } ecgProbeRunToken &+= 1 - ecgProbeDeadline = Date().addingTimeInterval(BLEManager.ecgProbeWindow) + ecgProbeStartedAt = Date() + ecgProbeTerminalExtended = false + ecgProbeTerminalLinesKept = 0 + ecgProbeDeadline = Date().addingTimeInterval(window) state.ecgProbe = BLEManager.ecgProbeWaiting } /// Render the verdict once the listen window closes. BLE callbacks and this timer both run on the /// main queue, so the token check is race-free without a lock. - private func scheduleEcgProbeVerdict() { + private func scheduleEcgProbeVerdict(after window: TimeInterval = BLEManager.ecgProbeWindow) { let token = ecgProbeRunToken - DispatchQueue.main.asyncAfter(deadline: .now() + BLEManager.ecgProbeWindow) { [weak self] in + DispatchQueue.main.asyncAfter(deadline: .now() + window) { [weak self] in guard let self, self.ecgProbeRunToken == token else { return } self.ecgProbeDeadline = nil + // The time LISTENED, not the window asked for. A terminal frame cuts a run short, and the + // verdict text quotes this number ("no ECG packet arrived in Ns"), so quoting the request + // would have the report claim a window it did not run. + let listened = Int((Date().timeIntervalSince(self.ecgProbeStartedAt ?? Date())).rounded()) let text = Whoop5EcgProbe.report(steps: self.ecgProbeSteps, ecgPacketsSeen: self.ecgProbePacketsSeen, candidateFrames: self.ecgProbeCandidates, - windowSeconds: Int(BLEManager.ecgProbeWindow)) + windowSeconds: listened) self.log("ECG probe:\n\(text)") self.state.ecgProbe = text } @@ -4617,7 +4636,26 @@ public final class BLEManager: NSObject, ObservableObject { private func noteEcgProbeCandidate(_ frame: [UInt8]) { guard frame.count >= 12, let packet = Whoop5Ecg.r17FromFrame(frame) else { return } ecgProbePacketsSeen += 1 - guard ecgProbeCandidates.count < BLEManager.ecgProbeMaxCandidates else { return } + // A TERMINAL frame is kept even past the cap, and ends the run on a grace period rather than on + // the frame itself. Both halves come from #891: the result, average HR and reason mask populate + // only on that frame, so the old flat cap kept seconds 1 to 12 and discarded the one frame that + // carried the answer; and the variability field reads 0xffff for several seconds after it, so + // stopping on the frame would pin the unset value as the reading's own. + if let grace = Whoop5EcgProbe.terminalGraceSeconds(isTerminal: packet.isTerminal, + alreadyExtended: ecgProbeTerminalExtended) { + ecgProbeTerminalExtended = true + // Re-arm rather than cancel: bumping the token strands the pending verdict timer, which is + // the same mechanism a second user tap already uses. + ecgProbeRunToken &+= 1 + ecgProbeDeadline = Date().addingTimeInterval(TimeInterval(grace)) + log("ECG probe: terminal frame (state=\(packet.classifierState) progress=" + + "\(packet.progress.raw)) — listening \(grace)s more before the verdict") + scheduleEcgProbeVerdict(after: TimeInterval(grace)) + } + guard Whoop5EcgProbe.retainsCandidate(linesKept: ecgProbeCandidates.count, + terminalLinesKept: ecgProbeTerminalLinesKept, + isTerminal: packet.isTerminal) else { return } + if packet.isTerminal { ecgProbeTerminalLinesKept += 1 } // The classifier byte is logged as a NUMBER, never as its token name: a strap log is a shareable // artefact, and no line in it should read like a clinical finding. The decoder maps the value // offline; the raw byte is lossless. @@ -6192,6 +6230,9 @@ extension BLEManager: @preconcurrency CBCentralManagerDelegate { ecgProbeSteps = [] ecgProbeCandidates = [] ecgProbePacketsSeen = 0 + ecgProbeStartedAt = nil + ecgProbeTerminalExtended = false + ecgProbeTerminalLinesKept = 0 // The DIS strings belong to the link that just dropped; a stale variant must not keep an MG-only // capability unlocked for whatever connects next. state.whoop5Variant = nil diff --git a/Tools/parity_twin_map.json b/Tools/parity_twin_map.json index f462057855..6d97b4e14d 100644 --- a/Tools/parity_twin_map.json +++ b/Tools/parity_twin_map.json @@ -19,14 +19,14 @@ }, "authority": { "files": {"count": 510, "sha256": "b1c73338e79009d984d1313d8b3166e52672712c68074f5fee190c503e299b18"}, - "functions": {"count": 4534, "sha256": "91a1670dfe86761a907dc0d1fa73b219e9ca8b5b6d90c36310db21c1657dc884"}, + "functions": {"count": 4538, "sha256": "f62a1ca0dfb2122fdca87a2f455ec0ca4435903c24069ae13bc4e75c0c3f32c0"}, "properties": {"count": 488, "sha256": "77d60fe0f6218252233b9e60da8bf5135968e49be3c6d2027d0973ea20ddf792"}, - "constants": {"count": 1991, "sha256": "ab5fad0319f0ee260993445c0388fbcd89801491c316977095e01014888f9228"}, - "file_pairs": {"count": 80, "sha256": "eeeb9e0f4adabed6e6c3c5886e980ea3a3656b65b5bd2a9b3b9e43b5ea3b051c"}, - "function_pairs": {"count": 224, "sha256": "af7d03c31dc065970bbefee6d86f549137dd3acbcb698017df57f63f436098b6"}, + "constants": {"count": 2001, "sha256": "a9c1f36d2acc614dd747fd6cc0b3ea48079b81517afda5591d219f9591e353c3"}, + "file_pairs": {"count": 81, "sha256": "3c94824ccff843da495a212a973d89ce61e536b6795dbee2d0e496a7e7a498f0"}, + "function_pairs": {"count": 226, "sha256": "7c7f2298ad7921101241f39d86ecab774d21eadeb89d52d4d5db0397878905f2"}, "property_pairs": {"count": 163, "sha256": "b7d4a29086beec661993370657c0bcdae86f79b221fdc0043206161df5e1117f"}, - "constant_pairs": {"count": 697, "sha256": "1f02bdcb642db9d6b24a4cdcbe1351fefbf5ec269830900eba0df49104745b5a"}, - "unpaired_files": {"count": 372, "sha256": "37aa705af1be255d3d18adfe912b93bcfeaa3760e1513412aad81388a4c1410b"}, + "constant_pairs": {"count": 702, "sha256": "e3de4c1b84b8500c5700292648bf23dc680e5e14cde055c36c5d1ef4ecc39dc2"}, + "unpaired_files": {"count": 370, "sha256": "441451c8f68b68e1dc3d3556c28952b4bdac243de9d6f909d3c18d6ac9732a1e"}, "unpaired_functions": {"count": 4099, "sha256": "3ee3e74d901e17225e294c309c1b00a7a5b178bb3164bff43eaaa5309d574169"}, "unpaired_properties": {"count": 162, "sha256": "dabff97ba9bce0aebdc8ef84b9c4f107ce574020e8b0455d8f9214082948246b"}, "unpaired_constants": {"count": 597, "sha256": "578c607cc05b1cd1f867c206c9fab70237ac40707cd49d4e0c5cc66b400184c7"} diff --git a/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt b/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt index b7a7a7e7e7..0505d44c43 100644 --- a/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt +++ b/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt @@ -5146,12 +5146,13 @@ class WhoopBleClient( */ // ---- WHOOP MG ECG ("Labrador") turn-on probe — twin of macOS BLEManager.ecg* ---------------- - /** Listen window after the turn-on burst, matching macOS `ecgProbeWindow`. */ - private val ECG_PROBE_WINDOW_SECONDS = 30 + /** Listen window for a run waiting on a COMMAND_RESPONSE, matching macOS `ecgProbeWindow`. + * Derived from [Whoop5EcgProbe.Window] so the two platforms cannot hold different numbers: that + * they both held 30 independently is how the window outlived the evidence against it (#891). */ + private val ECG_PROBE_WINDOW_SECONDS = Whoop5EcgProbe.Window.SELECT_OR_STOP - /** Detailed candidate lines are capped; the PACKET COUNT is not, so the verdict stays complete - * while a chatty stream cannot grow the strap log without bound. Matches macOS. */ - private val ECG_PROBE_MAX_CANDIDATES = 12 + /** The CAPTURE run's window. Longer, because a reading's terminal frame lands at 38 to 39 s. */ + private val ECG_PROBE_CAPTURE_WINDOW_SECONDS = Whoop5EcgProbe.Window.CAPTURE /** * Guards every probe accumulator below. [noteEcgProbeCandidate] runs on the BINDER thread (the @@ -5170,6 +5171,21 @@ class WhoopBleClient( private var ecgProbePacketsSeen = 0 + /** Supersedes a previous run's pending verdict post when a run is re-armed. Guarded by + * [ecgProbeLock], because a terminal frame re-arms from the binder thread. */ + private var ecgProbeRunToken = 0 + + /** When the run in flight began, so the report states the time actually listened rather than the + * window asked for: a terminal frame ends a run early, so those stopped being the same number. */ + private var ecgProbeStartedAtMs = 0L + + /** Whether this run already pushed its deadline out on a terminal frame. Once only, or a terminal + * stream would postpone the verdict forever. */ + private var ecgProbeTerminalExtended = false + + /** How many TERMINAL candidate lines this run has kept, against the reserve the cap holds back. */ + private var ecgProbeTerminalLinesKept = 0 + /** * True only while a probe run is sending. The send() allowlist reads this, so the three toggles are * unformable on a default install even with the opt-in on: permission and in-flight are separate @@ -5247,19 +5263,48 @@ class WhoopBleClient( ecgProbeSteps.clear() ecgProbeCandidates.clear() ecgProbePacketsSeen = 0 + ecgProbeRunToken += 1 + ecgProbeStartedAtMs = System.currentTimeMillis() + ecgProbeTerminalExtended = false + ecgProbeTerminalLinesKept = 0 } ecgProbeListening = true } - private fun scheduleEcgProbeVerdict() { + private fun scheduleEcgProbeVerdict(windowSeconds: Int = ECG_PROBE_WINDOW_SECONDS) { + val token = synchronized(ecgProbeLock) { ecgProbeRunToken } handler.postDelayed({ - ecgProbeListening = false - val (steps, packets, candidates) = synchronized(ecgProbeLock) { - Triple(ecgProbeSteps.toList(), ecgProbePacketsSeen, ecgProbeCandidates.toList()) + // Token check, not removeCallbacks: a terminal frame re-arms the run from the binder thread + // and a second user tap re-arms from main, so a stale post must simply do nothing. Twin of + // the macOS run-token guard. + val (stale, steps, packets, candidates, listened) = synchronized(ecgProbeLock) { + EcgVerdictSnapshot( + ecgProbeRunToken != token, + ecgProbeSteps.toList(), + ecgProbePacketsSeen, + ecgProbeCandidates.toList(), + // Integer rounding rather than a Double hop: this is a whole number of seconds in + // a report line, and it matches the Swift side's `.rounded()` for a positive span. + ((System.currentTimeMillis() - ecgProbeStartedAtMs + 500L) / 1000L).toInt(), + ) } - log(Whoop5EcgProbe.report(steps, packets, candidates, ECG_PROBE_WINDOW_SECONDS)) - }, ECG_PROBE_WINDOW_SECONDS * 1000L) - } + if (stale) return@postDelayed + ecgProbeListening = false + // The time LISTENED, not the window asked for. The verdict text quotes this number ("no ECG + // packet arrived in Ns"), so quoting the request would have the report claim a window it + // did not run. + log(Whoop5EcgProbe.report(steps, packets, candidates, listened)) + }, windowSeconds * 1000L) + } + + /** The verdict's inputs, read under [ecgProbeLock] in one go so the report cannot mix two runs. */ + private data class EcgVerdictSnapshot( + val stale: Boolean, + val steps: List, + val packets: Int, + val candidates: List, + val listenedSeconds: Int, + ) /** * The START list's first member: ABORT_HISTORICAL_TRANSMITS (20), immediately ahead of `124`. @@ -5331,7 +5376,7 @@ class WhoopBleClient( // Scheduled in the finally, because it is what CLOSES the listen window. If a send threw, // an early return would leave `ecgProbeListening` true forever and the triage running on // every frame for the life of the process. - scheduleEcgProbeVerdict() + scheduleEcgProbeVerdict(ECG_PROBE_CAPTURE_WINDOW_SECONDS) } } @@ -5379,10 +5424,35 @@ class WhoopBleClient( private fun noteEcgProbeCandidate(frame: ByteArray) { if (frame.size < 12) return val packet = Whoop5Ecg.r17FromFrame(frame) ?: return - synchronized(ecgProbeLock) { + // A TERMINAL frame is kept even past the cap, and ends the run on a grace period rather than on + // the frame itself. Both halves come from #891: the result, average HR and reason mask populate + // only on that frame, so the old flat cap kept seconds 1 to 12 and discarded the one frame that + // carried the answer; and the variability field reads 0xffff for several seconds after it, so + // stopping on the frame would pin the unset value as the reading's own. + var grace: Int? = null + val keep = synchronized(ecgProbeLock) { ecgProbePacketsSeen += 1 - if (ecgProbeCandidates.size >= ECG_PROBE_MAX_CANDIDATES) return + grace = Whoop5EcgProbe.terminalGraceSeconds(packet.isTerminal, ecgProbeTerminalExtended) + if (grace != null) ecgProbeTerminalExtended = true + val retain = Whoop5EcgProbe.retainsCandidate( + linesKept = ecgProbeCandidates.size, + terminalLinesKept = ecgProbeTerminalLinesKept, + isTerminal = packet.isTerminal, + ) + if (retain && packet.isTerminal) ecgProbeTerminalLinesKept += 1 + retain + } + grace?.let { seconds -> + log( + "ECG probe: terminal frame (state=${packet.classifierState} " + + "progress=${packet.progress.raw}) — listening ${seconds}s more before the verdict", + ) + // Re-arms the run: beginEcgProbeRun bumped the token at the start, and scheduling against a + // fresh token strands the pending verdict post. Same mechanism a second user tap uses. + synchronized(ecgProbeLock) { ecgProbeRunToken += 1 } + scheduleEcgProbeVerdict(seconds) } + if (!keep) return // The classifier byte is logged as a NUMBER, never as its token name: a strap log is a shareable // artefact, and no line in it should read like a clinical finding. // diff --git a/android/app/src/main/java/com/noop/protocol/Whoop5EcgProbe.kt b/android/app/src/main/java/com/noop/protocol/Whoop5EcgProbe.kt index 9bbd52afe1..73bca48483 100644 --- a/android/app/src/main/java/com/noop/protocol/Whoop5EcgProbe.kt +++ b/android/app/src/main/java/com/noop/protocol/Whoop5EcgProbe.kt @@ -206,6 +206,73 @@ object Whoop5EcgProbe { } } + /** + * How long one run listens, in seconds, chosen by what started it. + * + * [CAPTURE] is 60 rather than 30 because of @meta1971's field timing in #891: across five MG sessions + * the strap's terminal frame (classifier state 2, progress 100) landed 38 to 39 s after the first live + * frame, so a 30 s window could not reach a completed reading on ANY strap or firmware. The verdict + * sheet landing at 30 s made that worse than a missed deadline: Stop is the next control a person + * reaches for, so the window trained people to end the session 8 to 9 s before the classifier would + * have answered, and the probe then reported the silence it had caused. + * + * [SELECT_OR_STOP] stays at 30. Those runs wait for a COMMAND_RESPONSE rather than for a reading, so + * the longer window would be dead time in front of an answer that already arrived. + * + * [TERMINAL_GRACE] is how long a run keeps listening AFTER the terminal frame instead of ending on it. + * @meta1971 measured the variability field sitting at `0xffff` for 2 to 9 s past that frame before it + * settled, so a probe that stopped on the terminal frame would pin the unset value as the reading's + * own. It also means a run that ends early reports early: a terminal frame at 12 s renders the verdict + * at 22 s rather than holding the sheet for the rest of the window. + */ + object Window { + const val SELECT_OR_STOP = 30 + const val CAPTURE = 60 + const val TERMINAL_GRACE = 10 + } + + /** Cap on the per-packet candidate lines one run records. Each line is rendered into both the report + * and the strap log, so a chatty stream must not grow either without bound. */ + const val MAX_CANDIDATE_LINES = 12 + + /** + * Slots a run keeps in reserve for TERMINAL frames once [MAX_CANDIDATE_LINES] is spent. + * + * R17 arrives at about one frame a second, so a flat cap keeps seconds 1 to 12 and discards everything + * after. Per #891 the result, average HR and reason mask populate only on the terminal frame, which + * means the flat cap threw away the single frame carrying the answer and kept twelve that could not. A + * reserve rather than a bigger cap: the point is not more evidence, it is the RIGHT evidence, and the + * ceiling stays bounded at [MAX_CANDIDATE_LINES] + [MAX_TERMINAL_CANDIDATE_LINES]. + */ + const val MAX_TERMINAL_CANDIDATE_LINES = 4 + + /** + * Whether a decoded frame's line is kept, given what the run has already recorded. + * + * A terminal frame inside the first [MAX_CANDIDATE_LINES] is kept by the ordinary rule and still spends + * one reserve slot, so a run can never record more than the ceiling either way. + * + * Swift twin: `Whoop5EcgProbe.retainsCandidate`. + */ + fun retainsCandidate(linesKept: Int, terminalLinesKept: Int, isTerminal: Boolean): Boolean { + if (linesKept < MAX_CANDIDATE_LINES) return true + return isTerminal && terminalLinesKept < MAX_TERMINAL_CANDIDATE_LINES + } + + /** + * Seconds to wait before rendering the verdict once a terminal frame lands, or null to leave the run's + * existing deadline alone. + * + * ONCE per run ([alreadyExtended]), because the strap sends more than one terminal frame: extending on + * each would let a terminal stream push the verdict out indefinitely and the report would never render. + * + * Swift twin: `Whoop5EcgProbe.terminalGraceSeconds`. + */ + fun terminalGraceSeconds(isTerminal: Boolean, alreadyExtended: Boolean): Int? { + if (!isTerminal || alreadyExtended) return null + return Window.TERMINAL_GRACE + } + /** * Classify the run. * diff --git a/android/app/src/test/java/com/noop/protocol/Whoop5EcgProbeWindowTest.kt b/android/app/src/test/java/com/noop/protocol/Whoop5EcgProbeWindowTest.kt new file mode 100644 index 0000000000..bf0cea7a8c --- /dev/null +++ b/android/app/src/test/java/com/noop/protocol/Whoop5EcgProbeWindowTest.kt @@ -0,0 +1,64 @@ +package com.noop.protocol + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * #891 — the two rules that decide whether a run can OBSERVE a completed reading at all. + * + * Both defects they pin were invisible to every existing test, because the probe's own report looked + * correct: the packet count was honest, the verdict wording was honest, and the run simply never reached + * the frame that carries the answer. @meta1971's field timing (terminal frame at 38 to 39 s across five + * MG sessions, variability unset for 2 to 9 s after it) is the evidence behind the numbers. + * + * Twin of Swift `Whoop5EcgProbeWindowTests`, pinning the same cases. + */ +class Whoop5EcgProbeWindowTest { + + @Test + fun `capture window outlasts the measured terminal frame`() { + // The whole defect in one assertion: 30 s cannot reach a frame that lands at 38 to 39 s. + assertTrue(Whoop5EcgProbe.Window.CAPTURE > 39) + assertEquals(60, Whoop5EcgProbe.Window.CAPTURE) + // A run waiting on a COMMAND_RESPONSE is not waiting on a reading, so it keeps the short window. + assertEquals(30, Whoop5EcgProbe.Window.SELECT_OR_STOP) + // Long enough to outlast the measured 2-to-9 s unset variability window after the terminal frame. + assertTrue(Whoop5EcgProbe.Window.TERMINAL_GRACE > 9) + assertEquals(10, Whoop5EcgProbe.Window.TERMINAL_GRACE) + } + + @Test + fun `ordinary frames fill the cap and then stop`() { + assertTrue(Whoop5EcgProbe.retainsCandidate(0, 0, isTerminal = false)) + assertTrue(Whoop5EcgProbe.retainsCandidate(11, 0, isTerminal = false)) + assertFalse(Whoop5EcgProbe.retainsCandidate(12, 0, isTerminal = false)) + assertFalse(Whoop5EcgProbe.retainsCandidate(99, 0, isTerminal = false)) + } + + @Test + fun `terminal frames are kept past the cap but stay bounded`() { + // THE #891 CASE. R17 arrives about once a second, so at the terminal frame the ordinary cap has + // long since filled with seconds 1 to 12 — the twelve frames that carry no result. + assertTrue(Whoop5EcgProbe.retainsCandidate(12, 0, isTerminal = true)) + assertTrue(Whoop5EcgProbe.retainsCandidate(40, 3, isTerminal = true)) + // Bounded: the reserve is four slots, not an exemption. + assertFalse(Whoop5EcgProbe.retainsCandidate(40, 4, isTerminal = true)) + assertEquals( + 16, + Whoop5EcgProbe.MAX_CANDIDATE_LINES + Whoop5EcgProbe.MAX_TERMINAL_CANDIDATE_LINES, + ) + } + + @Test + fun `the grace extension happens once per run`() { + assertEquals(10, Whoop5EcgProbe.terminalGraceSeconds(isTerminal = true, alreadyExtended = false)) + // A terminal STREAM must not be able to postpone the verdict indefinitely. + assertNull(Whoop5EcgProbe.terminalGraceSeconds(isTerminal = true, alreadyExtended = true)) + // An ordinary frame leaves the run's deadline alone. + assertNull(Whoop5EcgProbe.terminalGraceSeconds(isTerminal = false, alreadyExtended = false)) + assertNull(Whoop5EcgProbe.terminalGraceSeconds(isTerminal = false, alreadyExtended = true)) + } +} From cd9235dd2c170981a9f7fceaf9320cb0c37a2e17 Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Fri, 9 Oct 2026 20:33:20 +1300 Subject: [PATCH 5/7] fix(ecg): keep a capture's frames when the user stops it Found re-reviewing the window change, which is what created it. A stop re-arms the probe run, and re-arming cleared the candidate lines and the packet count along with the steps. At a 30 s window the verdict sheet had almost always rendered first, so little was lost. At 60 s it has not: the reading completes at 38 to 39 s while the sheet still reads "waiting", so the obvious next action is to stop a session the user can see has finished, and that discarded every R17 line including the terminal frame this change exists to retain. So a stop issued while the capture's window is still open carries the lines, the count and the run's START TIME. The start time travels with them because the report states "packets seen in Ns", and a count from a 47 s capture under a 30 s stop window would attribute those packets to a span that did not collect them. Only while the window is open. Once the verdict has rendered the lines are already out, and a later stop is a separate run. A fresh capture still clears, or a previous reading's frames would surface as the new one's, and wrist select still clears because its report is about a different command. Refs #891 --- Strand/BLE/BLEManager.swift | 30 ++++++++++++++--- .../main/java/com/noop/ble/WhoopBleClient.kt | 33 ++++++++++++++++--- 2 files changed, 54 insertions(+), 9 deletions(-) diff --git a/Strand/BLE/BLEManager.swift b/Strand/BLE/BLEManager.swift index 25005f7910..06f370fdce 100644 --- a/Strand/BLE/BLEManager.swift +++ b/Strand/BLE/BLEManager.swift @@ -4522,7 +4522,15 @@ public final class BLEManager: NSObject, ObservableObject { } ecgStopOverride = true defer { ecgStopOverride = false } - if reportsResult { beginEcgProbeRun(clearingSteps: true) } else { ecgProbeSteps = [] } + // Read BEFORE the run is re-armed: an open window means the capture's verdict has not rendered + // yet, so its evidence has never been reported and must survive into this run. Once the verdict + // has rendered the lines are already out, and a later stop is a genuinely separate run. + let captureStillListening = ecgProbeArmed + if reportsResult { + beginEcgProbeRun(clearingSteps: true, keepingEvidence: captureStillListening) + } else { + ecgProbeSteps = [] + } log("ECG probe: stopping ECG data generation and both streams") sendEcgCommand(.toggleLabradorDataGeneration, arg: Whoop5Ecg.ControlSignal.stop.rawValue) sendEcgCommand(.toggleLabradorRawSave, arg: 0) @@ -4534,15 +4542,29 @@ public final class BLEManager: NSObject, ObservableObject { /// Clear the probe result (dialog dismissed). public func clearEcgProbe() { state.ecgProbe = nil } + /// `keepingEvidence` carries a capture's collected R17 lines, packet count and start time into the + /// STOP run that follows it, instead of clearing them with the steps. + /// + /// Without it the stop discards exactly what this probe exists to retain. The reading completes at + /// 38 to 39 s (#891) while the sheet still reads "waiting", so the natural next action is to stop a + /// session the user can tell has finished — and that wiped every candidate line, the terminal frame + /// among them. The 30 s window hid this by rendering the verdict before most people got there. + /// + /// The START TIME travels with the packets on purpose. The report states "packets seen in Ns", so + /// keeping a count from a 47 s capture under a 30 s stop window would attribute those packets to a + /// span that did not collect them. private func beginEcgProbeRun(clearingSteps: Bool, + keepingEvidence: Bool = false, window: TimeInterval = BLEManager.ecgProbeWindow) { if clearingSteps { ecgProbeSteps = [] - ecgProbeCandidates = [] - ecgProbePacketsSeen = 0 + if !keepingEvidence { + ecgProbeCandidates = [] + ecgProbePacketsSeen = 0 + } } ecgProbeRunToken &+= 1 - ecgProbeStartedAt = Date() + if !keepingEvidence { ecgProbeStartedAt = Date() } ecgProbeTerminalExtended = false ecgProbeTerminalLinesKept = 0 ecgProbeDeadline = Date().addingTimeInterval(window) diff --git a/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt b/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt index 0505d44c43..8fd7e63730 100644 --- a/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt +++ b/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt @@ -5258,13 +5258,28 @@ class WhoopBleClient( send(cmd, Whoop5Ecg.commandPayload(arg).map { it.toByte() }.toByteArray()) } - private fun beginEcgProbeRun() { + /** + * [keepingEvidence] carries a capture's collected R17 lines, packet count and start time into the + * STOP run that follows it, instead of clearing them with the steps. + * + * Without it the stop discards exactly what this probe exists to retain. The reading completes at 38 + * to 39 s (#891) while the sheet still reads "waiting", so the natural next action is to stop a + * session the user can tell has finished, and that wiped every candidate line, the terminal frame + * among them. The 30 s window hid this by rendering the verdict before most people got there. + * + * The START TIME travels with the packets on purpose. The report states "packets seen in Ns", so + * keeping a count from a 47 s capture under a 30 s stop window would attribute those packets to a + * span that did not collect them. + */ + private fun beginEcgProbeRun(keepingEvidence: Boolean = false) { synchronized(ecgProbeLock) { ecgProbeSteps.clear() - ecgProbeCandidates.clear() - ecgProbePacketsSeen = 0 + if (!keepingEvidence) { + ecgProbeCandidates.clear() + ecgProbePacketsSeen = 0 + ecgProbeStartedAtMs = System.currentTimeMillis() + } ecgProbeRunToken += 1 - ecgProbeStartedAtMs = System.currentTimeMillis() ecgProbeTerminalExtended = false ecgProbeTerminalLinesKept = 0 } @@ -5395,7 +5410,15 @@ class WhoopBleClient( } return } - if (reportsResult) beginEcgProbeRun() else synchronized(ecgProbeLock) { ecgProbeSteps.clear() } + // Read BEFORE the run is re-armed: an open window means the capture's verdict has not rendered + // yet, so its evidence has never been reported and must survive into this run. Once the verdict + // has rendered the lines are already out, and a later stop is a genuinely separate run. + val captureStillListening = ecgProbeListening + if (reportsResult) { + beginEcgProbeRun(keepingEvidence = captureStillListening) + } else { + synchronized(ecgProbeLock) { ecgProbeSteps.clear() } + } log("ECG probe: stopping ECG data generation and both streams") ecgProbeArmed = true try { From 4f3baeba47215dab941e4e7cc1a4ffa442583147 Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Fri, 9 Oct 2026 20:36:51 +1300 Subject: [PATCH 6/7] refactor(ecg): keep the listen window in one place Second re-review pass, on my own first pass. `ecgProbeDeadline` is a MARKER: `ecgProbeArmed` tests it for nil and nothing in the file compares the stamp to the clock. So stamping it with `now + window` encoded the window a second time, where only the verdict timer's delay can act on it, and `ecgStartCapture` had to pass the same number twice for the code to read correctly. The window now lives in `scheduleEcgProbeVerdict` alone, which also renews the marker, and `beginEcgProbeRun` goes back to arming the run. Arming stays there rather than moving into the scheduler because the sends in between can draw a COMMAND_RESPONSE and the triage is gated on it. This is the shape Kotlin already had, where `ecgProbeListening` is a plain flag and the window exists only in the posted delay, so the two clients now make the same four window decisions in the same four places. Refs #891 --- Strand/BLE/BLEManager.swift | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/Strand/BLE/BLEManager.swift b/Strand/BLE/BLEManager.swift index 06f370fdce..053813a014 100644 --- a/Strand/BLE/BLEManager.swift +++ b/Strand/BLE/BLEManager.swift @@ -1093,7 +1093,9 @@ public final class BLEManager: NSObject, ObservableObject { /// Structural-triage hits: the empirical search for the packet TYPE these records arrive under. private var ecgProbeCandidates: [String] = [] private var ecgProbePacketsSeen = 0 - /// Non-nil while the listen window is open; drives `ecgProbeArmed`. + /// Non-nil while the listen window is open; drives `ecgProbeArmed`. A MARKER, not a deadline: + /// nothing compares the stamp to the clock, and the window's length lives only in the verdict + /// timer, so do not start reading this as the moment a run ends. private var ecgProbeDeadline: Date? /// Supersedes a previous run's pending verdict timer when the user taps again. private var ecgProbeRunToken = 0 @@ -4457,7 +4459,7 @@ public final class BLEManager: NSObject, ObservableObject { public func ecgStartCapture() { guard ecgGatesAllow() else { return } ecgMayBeRunning = true // latched BEFORE the sends, so a mid-sequence drop still leaves Stop offered - beginEcgProbeRun(clearingSteps: true, window: BLEManager.ecgProbeCaptureWindow) + beginEcgProbeRun(clearingSteps: true) log("ECG probe: starting the ECG turn-on sequence on an MG (experimental, unvalidated instrumentation)") sendEcgCommand(.toggleLabradorFiltered, arg: 1) sendEcgCommand(.toggleLabradorRawSave, arg: 1) @@ -4553,9 +4555,7 @@ public final class BLEManager: NSObject, ObservableObject { /// The START TIME travels with the packets on purpose. The report states "packets seen in Ns", so /// keeping a count from a 47 s capture under a 30 s stop window would attribute those packets to a /// span that did not collect them. - private func beginEcgProbeRun(clearingSteps: Bool, - keepingEvidence: Bool = false, - window: TimeInterval = BLEManager.ecgProbeWindow) { + private func beginEcgProbeRun(clearingSteps: Bool, keepingEvidence: Bool = false) { if clearingSteps { ecgProbeSteps = [] if !keepingEvidence { @@ -4567,13 +4567,20 @@ public final class BLEManager: NSObject, ObservableObject { if !keepingEvidence { ecgProbeStartedAt = Date() } ecgProbeTerminalExtended = false ecgProbeTerminalLinesKept = 0 - ecgProbeDeadline = Date().addingTimeInterval(window) + // Armed from HERE rather than from `scheduleEcgProbeVerdict`, because the sends in between can + // draw a COMMAND_RESPONSE and the triage is gated on this. The VALUE is a marker only: nothing + // compares it to the clock, and the length of the window lives in the verdict timer alone, so + // there is one number to get right instead of two that can disagree. + ecgProbeDeadline = Date() state.ecgProbe = BLEManager.ecgProbeWaiting } /// Render the verdict once the listen window closes. BLE callbacks and this timer both run on the /// main queue, so the token check is race-free without a lock. private func scheduleEcgProbeVerdict(after window: TimeInterval = BLEManager.ecgProbeWindow) { + // Renews the armed marker as well as the timer: a terminal frame re-arms through here, and the + // two must not be able to drift apart. + ecgProbeDeadline = Date() let token = ecgProbeRunToken DispatchQueue.main.asyncAfter(deadline: .now() + window) { [weak self] in guard let self, self.ecgProbeRunToken == token else { return } @@ -4669,7 +4676,6 @@ public final class BLEManager: NSObject, ObservableObject { // Re-arm rather than cancel: bumping the token strands the pending verdict timer, which is // the same mechanism a second user tap already uses. ecgProbeRunToken &+= 1 - ecgProbeDeadline = Date().addingTimeInterval(TimeInterval(grace)) log("ECG probe: terminal frame (state=\(packet.classifierState) progress=" + "\(packet.progress.raw)) — listening \(grace)s more before the verdict") scheduleEcgProbeVerdict(after: TimeInterval(grace)) From eae23433c2948d1df6e39e7607ec94fcf34a28b7 Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Fri, 9 Oct 2026 21:50:40 +1300 Subject: [PATCH 7/7] build: 12.1.0 test beta (555 / 436) Marketing version moves with it, since this beta is of 12.1.0 rather than another 12.0.0 staging build. Refs #891 --- android/app/build.gradle.kts | 4 ++-- project.yml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/android/app/build.gradle.kts b/android/app/build.gradle.kts index 5bb70425cc..03b30dd7c5 100644 --- a/android/app/build.gradle.kts +++ b/android/app/build.gradle.kts @@ -26,8 +26,8 @@ android { applicationId = "com.noop.whoop" minSdk = 26 targetSdk = 34 - versionCode = 554 - versionName = "12.0.0" + versionCode = 555 + versionName = "12.1.0" testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner" vectorDrawables { diff --git a/project.yml b/project.yml index e1a3f6f3ac..beac7b618f 100644 --- a/project.yml +++ b/project.yml @@ -17,11 +17,11 @@ configFiles: Release: Config/BundleId.xcconfig settings: base: - MARKETING_VERSION: "12.0.0" + MARKETING_VERSION: "12.1.0" # iOS build number (CFBundleVersion). GLOBAL so the iOS app AND its widget extension inherit the # SAME value — they must match or iOS warns "extension version must match parent app" (#416). # macOS sets its own CFBundleVersion on the Strand target and is unaffected by this. - CURRENT_PROJECT_VERSION: "435" + CURRENT_PROJECT_VERSION: "436" SWIFT_VERSION: "5.0" # Emit localizable strings so Xcode auto-extracts every LocalizedStringKey into the # String Catalog on build (the English (US) base entries).