From 507fbee919dc318607dcf9fffff91cbd793d8a25 Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Fri, 28 Aug 2026 20:51:31 +1200 Subject: [PATCH 1/2] alarm: attribute the readback to a strap before judging it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported as #1706. The debug export read: Model: WHOOP 5.0 / MG Last arm: sent 2026-08-26 06:30 Strap reports: 2045-06-10 14:03 ⚠️ MISMATCH — strap didn't accept the time None of the thirteen alarm.* preferences is device-scoped, and on Android the readback is written only on the WHOOP 4.0 path. So on an install with a 4.0 and an active 5.0, that verdict compared an arm sent to one strap against a readback from another, then named a strap that was never asked. Not cosmetic on Apple: the same comparison drives alarm.rejectStreak, which raises a warning in SmartAlarmView at two. A cross-strap reading could climb that streak indefinitely. Both halves now record the strap they came from, and a shared AlarmReadback returns MATCHES, MISMATCH, DIFFERENT_STRAP or UNATTRIBUTED. Only a proven same-strap disagreement means refusal. DIFFERENT_STRAP and UNATTRIBUTED deliberately do nothing to the streak rather than resetting it: advancing would warn about a strap that was never asked, clearing would hide a real refusal, and neither reading is evidence either way. The raw readback frame is persisted beside the epoch now. That is what separates a genuinely stored stale alarm from a misdecode of a fixed response field, and the capture built for exactly that question had already rolled out of the live log before the export was taken, which is why the 2045 value is still unexplained. Nothing here decides it; the next export will carry the bytes. Also states what the clock line measures. It reads RECORD timestamps and sat two lines above the 2045 readback, which reads as the two contradicting each other. The verdict lives in a package on the Swift side so both twins are covered by default CI without app-build. Verified: 4,722 Android tests, 0 failures with --rerun-tasks --no-build-cache; the Swift verdict compiled standalone and agreed with the Kotlin case for case. --- .../StrandAnalytics/AlarmReadback.swift | 65 ++++++++++++++++++ .../AlarmReadbackTests.swift | 65 ++++++++++++++++++ Strand/BLE/BLEManager.swift | 6 ++ Strand/BLE/FrameRouter.swift | 28 +++++++- Strand/System/DebugDataDiagnostics.swift | 19 ++++-- .../java/com/noop/analytics/AlarmReadback.kt | 66 +++++++++++++++++++ .../main/java/com/noop/ble/WhoopBleClient.kt | 10 +++ .../com/noop/testcentre/AndroidDiagnostics.kt | 21 ++++-- .../com/noop/analytics/AlarmReadbackTest.kt | 65 ++++++++++++++++++ 9 files changed, 335 insertions(+), 10 deletions(-) create mode 100644 Packages/StrandAnalytics/Sources/StrandAnalytics/AlarmReadback.swift create mode 100644 Packages/StrandAnalytics/Tests/StrandAnalyticsTests/AlarmReadbackTests.swift create mode 100644 android/app/src/main/java/com/noop/analytics/AlarmReadback.kt create mode 100644 android/app/src/test/java/com/noop/analytics/AlarmReadbackTest.kt diff --git a/Packages/StrandAnalytics/Sources/StrandAnalytics/AlarmReadback.swift b/Packages/StrandAnalytics/Sources/StrandAnalytics/AlarmReadback.swift new file mode 100644 index 0000000000..42b760ed58 --- /dev/null +++ b/Packages/StrandAnalytics/Sources/StrandAnalytics/AlarmReadback.swift @@ -0,0 +1,65 @@ +import Foundation + +/// Whether the alarm time a strap reports back can be compared with the one we sent (#1706). +/// +/// The two halves are persisted under flat, device-less keys — `alarm.lastArmSentEpoch` and +/// `alarm.lastReportedEpoch`. On an install with more than one strap registered they can therefore +/// describe DIFFERENT devices, and comparing them then produces a confident "strap didn't accept the +/// time" about a strap that was never asked. On Android the readback is written only on the WHOOP 4.0 +/// path, so a 5.0-active install could only ever be comparing across straps. +/// +/// That verdict is not cosmetic here: it also drives `alarm.rejectStreak`, which raises a warning in +/// SmartAlarmView at two. A cross-strap comparison could climb that streak forever. +/// +/// So attribution is required, not assumed: unless both halves are known to come from the same strap, +/// this refuses to judge. Refusing is the same stance `WindowedStreamPlan` takes — a diagnosis that +/// cannot be proven is worse than none, because it sends the reader after the wrong device. +/// +/// Twin of Kotlin `AlarmReadback`. +public enum AlarmReadback { + + /// Seconds of slack allowed between what we armed and what the strap reports back. + public static let toleranceS = 120 + + public enum Verdict: Equatable { + /// Same strap, and the readback agrees within `toleranceS`. + case matches + /// Same strap, and it does not. This is the only value that means the strap refused. + case mismatch + /// The two halves came from different straps. Nothing can be concluded about either. + case differentStrap + /// One or both halves predate device attribution, so they cannot be tied to a strap. + case unattributed + } + + public static func verdict( + sentEpoch: Int, + reportedEpoch: Int, + sentDeviceId: String?, + reportedDeviceId: String?, + toleranceS: Int = AlarmReadback.toleranceS + ) -> Verdict { + guard let sentDeviceId, !sentDeviceId.isEmpty, + let reportedDeviceId, !reportedDeviceId.isEmpty else { return .unattributed } + guard sentDeviceId == reportedDeviceId else { return .differentStrap } + return abs(reportedEpoch - sentEpoch) > toleranceS ? .mismatch : .matches + } + + /// The suffix the debug export appends after the reported time. Byte-identical to the Kotlin twin. + public static func suffix(_ verdict: Verdict) -> String { + switch verdict { + case .matches: return " ✓ matches" + case .mismatch: return " ⚠️ MISMATCH — strap didn't accept the time" + case .differentStrap: return " (readback is from a different strap — not comparable)" + case .unattributed: return " (no strap recorded for one of these — not comparable)" + } + } + + /// Whether this verdict may advance the consecutive-rejection streak. Only a proven same-strap + /// disagreement counts: an unattributed or cross-strap reading must leave the streak untouched + /// rather than reset it, since neither is evidence either way. + public static func countsAsRejection(_ verdict: Verdict) -> Bool { verdict == .mismatch } + + /// Whether this verdict is evidence the strap DID accept, which clears the streak. + public static func clearsRejectionStreak(_ verdict: Verdict) -> Bool { verdict == .matches } +} diff --git a/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/AlarmReadbackTests.swift b/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/AlarmReadbackTests.swift new file mode 100644 index 0000000000..a54ca57792 --- /dev/null +++ b/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/AlarmReadbackTests.swift @@ -0,0 +1,65 @@ +import XCTest +@testable import StrandAnalytics + +/// #1706. The values here are the ones from the field log that produced the issue: an arm sent +/// 2026-08-26 06:30 against a readback claiming 2045-06-10, on a phone with a 4.0 and a 5.0 registered. +/// Twin of Kotlin `AlarmReadbackTest`. +final class AlarmReadbackTests: XCTestCase { + + private let sent = 1_787_682_600 // 2026-08-26 06:30 +12:00 + private let reported = 2_380_672_980 // 2045-06-10 14:03 +12:00, what the strap reported back + + func testSameStrapAndAgreeing() { + XCTAssertEqual(AlarmReadback.verdict(sentEpoch: sent, reportedEpoch: sent + 5, + sentDeviceId: "whoop-a", reportedDeviceId: "whoop-a"), .matches) + } + + func testSameStrapAtTheToleranceBoundary() { + XCTAssertEqual(AlarmReadback.verdict(sentEpoch: sent, reportedEpoch: sent + 120, + sentDeviceId: "whoop-a", reportedDeviceId: "whoop-a"), .matches) + XCTAssertEqual(AlarmReadback.verdict(sentEpoch: sent, reportedEpoch: sent + 121, + sentDeviceId: "whoop-a", reportedDeviceId: "whoop-a"), .mismatch) + } + + func testSameStrapAndDisagreeingIsTheOnlyRealRefusal() { + let v = AlarmReadback.verdict(sentEpoch: sent, reportedEpoch: reported, + sentDeviceId: "whoop-a", reportedDeviceId: "whoop-a") + XCTAssertEqual(v, .mismatch) + XCTAssertTrue(AlarmReadback.countsAsRejection(v)) + } + + /// The field case: the readback can only come from the 4.0, the arm went to the active 5.0. + func testCrossStrapIsNotJudged() { + let v = AlarmReadback.verdict(sentEpoch: sent, reportedEpoch: reported, + sentDeviceId: "whoop-5mg", reportedDeviceId: "my-whoop") + XCTAssertEqual(v, .differentStrap) + XCTAssertFalse(AlarmReadback.countsAsRejection(v), "a strap that was never asked must not be blamed") + XCTAssertFalse(AlarmReadback.clearsRejectionStreak(v), "nor may it clear a real refusal") + } + + /// Data written before attribution existed. Unknown is not the same as innocent. + func testMissingAttributionIsNotJudged() { + let pairs: [(String?, String?)] = [(nil, "whoop-a"), ("whoop-a", nil), (nil, nil), ("", "whoop-a")] + for (a, b) in pairs { + let v = AlarmReadback.verdict(sentEpoch: sent, reportedEpoch: reported, + sentDeviceId: a, reportedDeviceId: b) + XCTAssertEqual(v, .unattributed, "\(String(describing: a)) / \(String(describing: b))") + XCTAssertFalse(AlarmReadback.countsAsRejection(v)) + XCTAssertFalse(AlarmReadback.clearsRejectionStreak(v)) + } + } + + func testOnlyAProvenAgreementClearsTheStreak() { + XCTAssertTrue(AlarmReadback.clearsRejectionStreak(.matches)) + XCTAssertFalse(AlarmReadback.clearsRejectionStreak(.mismatch)) + XCTAssertFalse(AlarmReadback.clearsRejectionStreak(.differentStrap)) + XCTAssertFalse(AlarmReadback.clearsRejectionStreak(.unattributed)) + } + + func testSuffixShape() { + XCTAssertEqual(AlarmReadback.suffix(.matches), " ✓ matches") + XCTAssertEqual(AlarmReadback.suffix(.mismatch), " ⚠️ MISMATCH — strap didn't accept the time") + XCTAssertEqual(AlarmReadback.suffix(.differentStrap), " (readback is from a different strap — not comparable)") + XCTAssertEqual(AlarmReadback.suffix(.unattributed), " (no strap recorded for one of these — not comparable)") + } +} diff --git a/Strand/BLE/BLEManager.swift b/Strand/BLE/BLEManager.swift index d8a5a005a5..ee5dc7b6ad 100644 --- a/Strand/BLE/BLEManager.swift +++ b/Strand/BLE/BLEManager.swift @@ -1427,6 +1427,7 @@ public final class BLEManager: NSObject, ObservableObject { // and tell the router which decoder to use. Fresh per connection so no stale bytes carry over. reassembler = Reassembler(family: model.deviceFamily) router.family = model.deviceFamily + router.deviceId = deviceId // #1706: attribute this connection's alarm readback // Live 5/MG persistence: point the Collector's decode at the selected family and install the // identity clock ref for a 5/MG (its live timestamps are already real unix). WHOOP 4.0 keeps // the GET_CLOCK correlation flow untouched. Re-applied after bootstrapStore builds the @@ -4302,6 +4303,7 @@ public final class BLEManager: NSObject, ObservableObject { selectedModel = model reassembler = Reassembler(family: model.deviceFamily) router.family = model.deviceFamily + router.deviceId = deviceId // #1706: attribute this connection's alarm readback configureCollectorFamily() central.stopScan() log("Scanning for \(model.displayName)…") @@ -4643,6 +4645,9 @@ public final class BLEManager: NSObject, ObservableObject { private func recordAlarmArm(sentEpoch: Int) { let d = UserDefaults.standard d.set(sentEpoch, forKey: "alarm.lastArmSentEpoch") + // #1706: WHICH strap this arm went to. Without it the export, and the reject streak, compare + // this epoch against a readback that on a multi-strap install may be a different device. + d.set(deviceId, forKey: "alarm.lastArmDeviceId") d.set(Date().timeIntervalSince1970, forKey: "alarm.lastArmAt") d.set(commandChannelReady, forKey: "alarm.lastArmConnected") // #613: true only if the arm actually went out // #34: the strap-clock skew (its own RTC minus wall, seconds) AT THE MOMENT we armed. A wrong RTC @@ -5445,6 +5450,7 @@ extension BLEManager: @preconcurrency CBCentralManagerDelegate { selectedModel = .persisted reassembler = Reassembler(family: selectedModel.deviceFamily) router.family = selectedModel.deviceFamily + router.deviceId = deviceId // #1706: attribute this connection's alarm readback configureCollectorFamily() // Collection only runs post-bond, so a restored link was already bonded; // seed those flags now. `didWriteValueFor` won't re-fire on its own. diff --git a/Strand/BLE/FrameRouter.swift b/Strand/BLE/FrameRouter.swift index f43be26aae..b38c9c2601 100644 --- a/Strand/BLE/FrameRouter.swift +++ b/Strand/BLE/FrameRouter.swift @@ -10,6 +10,11 @@ public final class FrameRouter { /// Called when the strap pushes an EVENT packet (WHOOP's strap-as-clock catch-up signal). The /// BLEManager wires this to a rate-limited requestSync(.strap). nil in pure/unit contexts. var onSyncTrigger: (() -> Void)? + /// #1706: which strap this connection is talking to, so an alarm readback can be attributed to a + /// device. Set per connection by BLEManager alongside `family`; nil in pure/unit contexts, which the + /// verdict treats as unattributed rather than guessing. + var deviceId: String? + /// Which family's framing to decode with. Set per connection by BLEManager. WHOOP 5.0/MG frames /// use the CRC16/offset-8 envelope; the biometric field decode for puffin is still a stub, so /// WHOOP 5 custom frames currently surface only their envelope (live HR/battery come from the @@ -175,15 +180,34 @@ public final class FrameRouter { // #34: persist what the strap reports so the debug export can show sent-vs-reported. let d = UserDefaults.standard d.set(Int(epoch), forKey: "alarm.lastReportedEpoch") + // #1706: the strap this readback came from, and the bytes it came in. The raw + // frame is what separates a genuinely-stored stale alarm from a misdecode of a + // fixed response field, and the live log rolls long before a debug export is + // taken — a 2045 readback went unexplained for exactly that reason. + d.set(deviceId, forKey: "alarm.lastReportedDeviceId") + d.set(raw, forKey: "alarm.lastReportedRaw") d.set(Date().timeIntervalSince1970, forKey: "alarm.lastReportedAt") // #34: count CONSECUTIVE rejections (reported ≠ what we last sent) — the signature of // a corrupted strap alarm register. A matching readback resets it, so a transient // (first read stale, then correct) never trips the warning; only a persistent refusal // climbs. SmartAlarmView surfaces the warning at ≥2; the debug export shows the count. // Observability only — never gates the BLE arm. + // #1706: the streak raises a UI warning at two, so it must only count a + // disagreement PROVEN to be the same strap. A cross-strap or unattributed + // reading is evidence of nothing and leaves the streak untouched — advancing it + // would warn about a strap that was never asked, clearing it would hide a real + // refusal. if let sent = d.object(forKey: "alarm.lastArmSentEpoch") as? Int { - d.set(abs(Int(epoch) - sent) > 120 ? d.integer(forKey: "alarm.rejectStreak") + 1 : 0, - forKey: "alarm.rejectStreak") + let verdict = AlarmReadback.verdict( + sentEpoch: sent, + reportedEpoch: Int(epoch), + sentDeviceId: d.string(forKey: "alarm.lastArmDeviceId"), + reportedDeviceId: d.string(forKey: "alarm.lastReportedDeviceId")) + if AlarmReadback.countsAsRejection(verdict) { + d.set(d.integer(forKey: "alarm.rejectStreak") + 1, forKey: "alarm.rejectStreak") + } else if AlarmReadback.clearsRejectionStreak(verdict) { + d.set(0, forKey: "alarm.rejectStreak") + } } } else if Self.readbackReportsNoAlarm(in: frame) { // #34 (issue comment 2026-07-12): the strap's "nothing armed" sentinel — the epoch diff --git a/Strand/System/DebugDataDiagnostics.swift b/Strand/System/DebugDataDiagnostics.swift index 00ddc7c0b4..7b797caa00 100644 --- a/Strand/System/DebugDataDiagnostics.swift +++ b/Strand/System/DebugDataDiagnostics.swift @@ -391,7 +391,9 @@ enum DebugDataDiagnostics { } else if behind < -3 * 86400 { lines.append("Strap clock: \(-behind / 86400)d AHEAD of wall (future-dated — alarm unreliable; recent sleep may be misdated, #67)") } else { - lines.append("Strap clock: OK") + // #1706: say what this measures. It reads RECORD timestamps, and sat two lines above an + // alarm readback claiming 2045, which reads as the two contradicting. + lines.append("Strap clock: OK (from record timestamps, not the alarm readback)") } } if let sent = d.object(forKey: "alarm.lastArmSentEpoch") as? Int { @@ -413,14 +415,23 @@ enum DebugDataDiagnostics { } lines.append(line) if let reported = d.object(forKey: "alarm.lastReportedEpoch") as? Int { - let mismatch = abs(reported - sent) > 120 - var rline = "Strap reports: \(alarmStamp(reported))" - + (mismatch ? " ⚠️ MISMATCH — strap didn't accept the time" : " ✓ matches") + // #1706: only judge when both halves are known to be the SAME strap, otherwise this + // blames a device that was never asked. + let verdict = AlarmReadback.verdict( + sentEpoch: sent, + reportedEpoch: reported, + sentDeviceId: d.string(forKey: "alarm.lastArmDeviceId"), + reportedDeviceId: d.string(forKey: "alarm.lastReportedDeviceId")) + var rline = "Strap reports: \(alarmStamp(reported))" + AlarmReadback.suffix(verdict) // #34: consecutive rejections — a persistent refusal (vs a one-off) points at a strap whose // alarm register needs a reset, and is what SmartAlarmView warns the user about at ≥2. let streak = d.integer(forKey: "alarm.rejectStreak") if streak >= 2 { rline += " · \(streak) in a row (register likely needs a reset, #34)" } lines.append(rline) + // The bytes the epoch was decoded from: what tells a stored stale alarm from a misdecode. + if let raw = d.string(forKey: "alarm.lastReportedRaw"), !raw.isEmpty { + lines.append("Readback frame: \(raw)") + } } else { lines.append("Strap reports: (no readback)") } diff --git a/android/app/src/main/java/com/noop/analytics/AlarmReadback.kt b/android/app/src/main/java/com/noop/analytics/AlarmReadback.kt new file mode 100644 index 0000000000..a37707fb61 --- /dev/null +++ b/android/app/src/main/java/com/noop/analytics/AlarmReadback.kt @@ -0,0 +1,66 @@ +package com.noop.analytics + +/** + * Whether the alarm time a strap reports back can be compared with the one we sent (#1706). + * + * The two halves are persisted under flat, device-less keys — `alarm.lastArmSentEpoch` and + * `alarm.lastReportedEpoch`. On an install with more than one strap registered they can therefore + * describe DIFFERENT devices, and comparing them then produces a confident "strap didn't accept the + * time" about a strap that was never asked. On Android the readback is written only on the WHOOP 4.0 + * path, so a 5.0-active install could only ever be comparing across straps. + * + * That verdict is not cosmetic on Apple: it also drives `alarm.rejectStreak`, which raises a warning in + * SmartAlarmView at two. A cross-strap comparison could climb that streak forever. + * + * So attribution is required, not assumed: unless both halves are known to come from the same strap, + * this refuses to judge. Refusing is the same stance `WindowedStreamPlan` takes — a diagnosis that + * cannot be proven is worse than none, because it sends the reader after the wrong device. + * + * Twin of Swift `AlarmReadback`. + */ +object AlarmReadback { + + /** Seconds of slack allowed between what we armed and what the strap reports back. */ + const val TOLERANCE_S: Long = 120 + + enum class Verdict { + /** Same strap, and the readback agrees within [TOLERANCE_S]. */ + MATCHES, + /** Same strap, and it does not. This is the only value that means the strap refused. */ + MISMATCH, + /** The two halves came from different straps. Nothing can be concluded about either. */ + DIFFERENT_STRAP, + /** One or both halves predate device attribution, so they cannot be tied to a strap. */ + UNATTRIBUTED, + } + + fun verdict( + sentEpoch: Long, + reportedEpoch: Long, + sentDeviceId: String?, + reportedDeviceId: String?, + toleranceS: Long = TOLERANCE_S, + ): Verdict { + if (sentDeviceId.isNullOrBlank() || reportedDeviceId.isNullOrBlank()) return Verdict.UNATTRIBUTED + if (sentDeviceId != reportedDeviceId) return Verdict.DIFFERENT_STRAP + return if (kotlin.math.abs(reportedEpoch - sentEpoch) > toleranceS) Verdict.MISMATCH else Verdict.MATCHES + } + + /** The suffix the debug export appends after the reported time. Byte-identical to the Swift twin. */ + fun suffix(verdict: Verdict): String = when (verdict) { + Verdict.MATCHES -> " ✓ matches" + Verdict.MISMATCH -> " ⚠️ MISMATCH — strap didn't accept the time" + Verdict.DIFFERENT_STRAP -> " (readback is from a different strap — not comparable)" + Verdict.UNATTRIBUTED -> " (no strap recorded for one of these — not comparable)" + } + + /** + * Whether this verdict may advance the consecutive-rejection streak. Only a proven same-strap + * disagreement counts: an unattributed or cross-strap reading must leave the streak untouched + * rather than reset it, since neither is evidence either way. + */ + fun countsAsRejection(verdict: Verdict): Boolean = verdict == Verdict.MISMATCH + + /** Whether this verdict is evidence the strap DID accept, which clears the streak. */ + fun clearsRejectionStreak(verdict: Verdict): Boolean = verdict == Verdict.MATCHES +} 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 aff7c38b2d..51c212cc0d 100644 --- a/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt +++ b/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt @@ -4574,6 +4574,9 @@ class WhoopBleClient( runCatching { val editor = NoopPrefs.of(context).edit() .putLong("alarm.lastArmSentEpoch", sentEpoch) + // #1706: WHICH strap this arm went to. Without it the export compares this epoch against + // a readback that, on a multi-strap install, may have come from a different device. + .putString("alarm.lastArmDeviceId", deviceId) .putLong("alarm.lastArmAt", System.currentTimeMillis()) .putBoolean("alarm.lastArmConnected", _state.value.connected) // #34: live HR at the moment of the arm, purely to test a hypothesis raised on a reporter's @@ -6460,6 +6463,13 @@ class WhoopBleClient( runCatching { NoopPrefs.of(context).edit() .putLong("alarm.lastReportedEpoch", epoch) + // #1706: the strap this readback came from, and the bytes it came in. + // The raw frame is what separates a genuinely-stored stale alarm from a + // misdecode of a fixed response field, and the live log rolls long before + // a debug export is taken — a 2045 readback went unexplained for exactly + // that reason. + .putString("alarm.lastReportedDeviceId", deviceId) + .putString("alarm.lastReportedRaw", raw) .putLong("alarm.lastReportedAt", System.currentTimeMillis()) .apply() } diff --git a/android/app/src/main/java/com/noop/testcentre/AndroidDiagnostics.kt b/android/app/src/main/java/com/noop/testcentre/AndroidDiagnostics.kt index 1284e2252e..f0c85c62f6 100644 --- a/android/app/src/main/java/com/noop/testcentre/AndroidDiagnostics.kt +++ b/android/app/src/main/java/com/noop/testcentre/AndroidDiagnostics.kt @@ -368,7 +368,9 @@ object AndroidDiagnostics { add(when { behind > 3 * 86400L -> "Strap clock: ${behind / 86400L}d behind wall (reset/stale — alarm unreliable)" behind < -3 * 86400L -> "Strap clock: ${-behind / 86400L}d AHEAD of wall (future-dated — alarm unreliable)" - else -> "Strap clock: OK" + // #1706: say what this is measuring. It reads RECORD timestamps, and sat two lines + // above an alarm readback claiming 2045, which reads as the two contradicting. + else -> "Strap clock: OK (from record timestamps, not the alarm readback)" }) } val sent = p.getLong("alarm.lastArmSentEpoch", 0L) @@ -383,9 +385,20 @@ object AndroidDiagnostics { add(line) val reported = p.getLong("alarm.lastReportedEpoch", 0L) if (reported > 0L) { - val mismatch = kotlin.math.abs(reported - sent) > 120 - add("Strap reports: ${alarmStamp(reported)}" + - if (mismatch) " ⚠️ MISMATCH — strap didn't accept the time" else " ✓ matches") + // #1706: only judge when both halves are known to be the SAME strap. The readback is + // written on the WHOOP 4.0 path alone, so a 5.0-active install comparing these was + // always comparing two devices and then blaming one of them. + val verdict = com.noop.analytics.AlarmReadback.verdict( + sentEpoch = sent, + reportedEpoch = reported, + sentDeviceId = p.getString("alarm.lastArmDeviceId", null), + reportedDeviceId = p.getString("alarm.lastReportedDeviceId", null), + ) + add("Strap reports: ${alarmStamp(reported)}" + com.noop.analytics.AlarmReadback.suffix(verdict)) + // The bytes the epoch was decoded from: what tells a stored stale alarm from a misdecode. + p.getString("alarm.lastReportedRaw", null) + ?.takeIf { it.isNotBlank() } + ?.let { add("Readback frame: $it") } } else add("Strap reports: (no readback)") } else add("Last arm: never") // #1: did the strap actually fire? (STRAP_DRIVEN_ALARM_EXECUTED) diff --git a/android/app/src/test/java/com/noop/analytics/AlarmReadbackTest.kt b/android/app/src/test/java/com/noop/analytics/AlarmReadbackTest.kt new file mode 100644 index 0000000000..1c9bf827a6 --- /dev/null +++ b/android/app/src/test/java/com/noop/analytics/AlarmReadbackTest.kt @@ -0,0 +1,65 @@ +package com.noop.analytics + +import com.noop.analytics.AlarmReadback.Verdict +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * #1706. The values here are the ones from the field log that produced the issue: an arm sent + * 2026-08-26 06:30 against a readback claiming 2045-06-10, on a phone with a 4.0 and a 5.0 registered. + * Twin of Swift `AlarmReadbackTests`. + */ +class AlarmReadbackTest { + + private val sent = 1_787_682_600L // 2026-08-26 06:30 +12:00 + private val reported = 2_380_672_980L // 2045-06-10 14:03 +12:00, what the strap reported back + + @Test fun sameStrapAndAgreeing() { + assertEquals(Verdict.MATCHES, AlarmReadback.verdict(sent, sent + 5, "whoop-a", "whoop-a")) + } + + @Test fun sameStrapAtTheToleranceBoundary() { + assertEquals(Verdict.MATCHES, AlarmReadback.verdict(sent, sent + 120, "whoop-a", "whoop-a")) + assertEquals(Verdict.MISMATCH, AlarmReadback.verdict(sent, sent + 121, "whoop-a", "whoop-a")) + } + + @Test fun sameStrapAndDisagreeingIsTheOnlyRealRefusal() { + val v = AlarmReadback.verdict(sent, reported, "whoop-a", "whoop-a") + assertEquals(Verdict.MISMATCH, v) + assertTrue(AlarmReadback.countsAsRejection(v)) + } + + /** The field case: the readback can only come from the 4.0, the arm went to the active 5.0. */ + @Test fun crossStrapIsNotJudged() { + val v = AlarmReadback.verdict(sent, reported, "whoop-5mg", "my-whoop") + assertEquals(Verdict.DIFFERENT_STRAP, v) + assertFalse("a strap that was never asked must not be blamed", AlarmReadback.countsAsRejection(v)) + assertFalse("nor may it clear a real refusal", AlarmReadback.clearsRejectionStreak(v)) + } + + /** Data written before attribution existed. Unknown is not the same as innocent. */ + @Test fun missingAttributionIsNotJudged() { + for (pair in listOf(null to "whoop-a", "whoop-a" to null, null to null, "" to "whoop-a")) { + val v = AlarmReadback.verdict(sent, reported, pair.first, pair.second) + assertEquals("$pair", Verdict.UNATTRIBUTED, v) + assertFalse("$pair", AlarmReadback.countsAsRejection(v)) + assertFalse("$pair", AlarmReadback.clearsRejectionStreak(v)) + } + } + + @Test fun onlyAProvenAgreementClearsTheStreak() { + assertTrue(AlarmReadback.clearsRejectionStreak(Verdict.MATCHES)) + assertFalse(AlarmReadback.clearsRejectionStreak(Verdict.MISMATCH)) + assertFalse(AlarmReadback.clearsRejectionStreak(Verdict.DIFFERENT_STRAP)) + assertFalse(AlarmReadback.clearsRejectionStreak(Verdict.UNATTRIBUTED)) + } + + @Test fun suffixShape() { + assertEquals(" ✓ matches", AlarmReadback.suffix(Verdict.MATCHES)) + assertEquals(" ⚠️ MISMATCH — strap didn't accept the time", AlarmReadback.suffix(Verdict.MISMATCH)) + assertEquals(" (readback is from a different strap — not comparable)", AlarmReadback.suffix(Verdict.DIFFERENT_STRAP)) + assertEquals(" (no strap recorded for one of these — not comparable)", AlarmReadback.suffix(Verdict.UNATTRIBUTED)) + } +} From b878213e5fcf9e41dfa69970c0d92df8a39b883e Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Fri, 28 Aug 2026 20:58:18 +1200 Subject: [PATCH 2/2] alarm: make stale attribution impossible, and do not strand a legacy streak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-review of the attribution change, three defects in it. The router's deviceId was a separate assignment after family, so a future connection path that set the family and forgot the id would carry the PREVIOUS connection's strap forward — a confident wrong attribution, which is the exact mistake this work exists to prevent. Cleared in family's didSet, already documented as the per-session reset hook, so a forgotten assignment now attributes nothing instead of lying. All three call sites set family then deviceId, in that order. The streak change stranded upgraders. Values standing in alarm.rejectStreak were built by the cross-strap comparison being replaced, and since only a proven match clears the streak now, an install that upgraded mid-streak would hold SmartAlarmView's warning up forever. The first unattributed readback — which can only be pre-upgrade data — discards it once. That fix then made the comment above it false: it still said an unattributed reading leaves the streak untouched, two lines above code clearing it. Rewritten. Also takes the deviceId local directly rather than re-reading from UserDefaults what it wrote three lines earlier. Kotlin is untouched by this pass. The Swift verdict still compiles standalone and behaves identically. --- Strand/BLE/FrameRouter.swift | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/Strand/BLE/FrameRouter.swift b/Strand/BLE/FrameRouter.swift index b38c9c2601..83643d4440 100644 --- a/Strand/BLE/FrameRouter.swift +++ b/Strand/BLE/FrameRouter.swift @@ -11,8 +11,10 @@ public final class FrameRouter { /// BLEManager wires this to a rate-limited requestSync(.strap). nil in pure/unit contexts. var onSyncTrigger: (() -> Void)? /// #1706: which strap this connection is talking to, so an alarm readback can be attributed to a - /// device. Set per connection by BLEManager alongside `family`; nil in pure/unit contexts, which the - /// verdict treats as unattributed rather than guessing. + /// device. Set per connection by BLEManager immediately AFTER `family`, whose didSet clears this — + /// a path that sets the family and forgets the id then attributes nothing rather than carrying the + /// previous connection's strap forward, which is the very mistake this attribution exists to stop. + /// nil in pure/unit contexts, which the verdict treats as unattributed rather than guessing. var deviceId: String? /// Which family's framing to decode with. Set per connection by BLEManager. WHOOP 5.0/MG frames @@ -23,7 +25,7 @@ public final class FrameRouter { // #900: a fresh connection is a fresh capture session — re-arm the per-command raw-frame dump so // each connect can re-capture the disputed COMMAND_RESPONSE prefix once. `family` is set fresh per // connection by BLEManager (connectCore), so this is the per-session reset hook. - didSet { rawDumpedRespCmds.removeAll(); loggedFirmwareGate = nil } + didSet { rawDumpedRespCmds.removeAll(); loggedFirmwareGate = nil; deviceId = nil } } /// #900: resp command names (e.g. "GET_BATTERY_LEVEL(26)") whose raw COMMAND_RESPONSE frame has already @@ -192,21 +194,28 @@ public final class FrameRouter { // (first read stale, then correct) never trips the warning; only a persistent refusal // climbs. SmartAlarmView surfaces the warning at ≥2; the debug export shows the count. // Observability only — never gates the BLE arm. - // #1706: the streak raises a UI warning at two, so it must only count a - // disagreement PROVEN to be the same strap. A cross-strap or unattributed - // reading is evidence of nothing and leaves the streak untouched — advancing it - // would warn about a strap that was never asked, clearing it would hide a real - // refusal. + // #1706: the streak raises a UI warning at two, so it must only COUNT a + // disagreement PROVEN to be the same strap. A cross-strap reading is evidence of + // nothing and leaves the streak alone — advancing would warn about a strap that + // was never asked, clearing would hide a real refusal. An unattributed one is a + // different case, handled below. if let sent = d.object(forKey: "alarm.lastArmSentEpoch") as? Int { let verdict = AlarmReadback.verdict( sentEpoch: sent, reportedEpoch: Int(epoch), sentDeviceId: d.string(forKey: "alarm.lastArmDeviceId"), - reportedDeviceId: d.string(forKey: "alarm.lastReportedDeviceId")) + reportedDeviceId: deviceId) // the local, not a re-read of what we just wrote if AlarmReadback.countsAsRejection(verdict) { d.set(d.integer(forKey: "alarm.rejectStreak") + 1, forKey: "alarm.rejectStreak") } else if AlarmReadback.clearsRejectionStreak(verdict) { d.set(0, forKey: "alarm.rejectStreak") + } else if verdict == .unattributed { + // Any streak standing here was built by the cross-strap comparison this + // replaces, so it cannot be trusted — and since only a proven match clears + // the streak now, leaving it would hold SmartAlarmView's warning up forever + // on an install that upgraded mid-streak. Discard once; the next attributed + // readback rebuilds it honestly. + d.set(0, forKey: "alarm.rejectStreak") } } } else if Self.readbackReportsNoAlarm(in: frame) {