diff --git a/Strand/BLE/FrameRouter.swift b/Strand/BLE/FrameRouter.swift index 32d41f09e2..61e7469b8f 100644 --- a/Strand/BLE/FrameRouter.swift +++ b/Strand/BLE/FrameRouter.swift @@ -14,7 +14,17 @@ public final class FrameRouter { /// 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 /// standard 0x2A37/0x2A19 profiles instead). - var family: DeviceFamily = .whoop4 + var family: DeviceFamily = .whoop4 { + // #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() } + } + + /// #900: resp command names (e.g. "GET_BATTERY_LEVEL(26)") whose raw COMMAND_RESPONSE frame has already + /// been dumped this connection. The provenance dump fires once per command per session so a 4.0's + /// per-poll battery reads don't flood the strap log. Reset when `family` is set at connect. + private var rawDumpedRespCmds: Set = [] public init(state: LiveState) { self.state = state @@ -190,6 +200,33 @@ public final class FrameRouter { state.append(log: "HELLO_HARVARD(35) resp raw: \(Self.commandResponsePayloadHex(in: frame) ?? "empty") — locate the strap serial offset (#1303)") } } + // #900: surface a non-SUCCESS COMMAND_RESPONSE on BOTH families (a result=UNSUPPORTED here is how + // the MG haptics rejection #48 would show), and — the key part — annotate a reply that DELIVERED + // ITS VALUE rather than reporting a bare failure. The 4.0 GET_BATTERY_LEVEL replies on record carry + // a zeroed [seq][result] prefix, so a battery read that returned a good percentage logs as + // "FAILURE(0)"; a failure line next to a gauge reading 42% is the artefact that gets quoted as a + // fault that isn't there — that is how #900 started. The line still prints (hiding it would hide the + // anomaly), it just no longer reads as a failure. Twin of the Kotlin WhoopBleClient annotation (#923). + if let result = parsed.parsed["result"]?.stringValue, !result.hasPrefix("SUCCESS") { + let cmdName = parsed.cmdName ?? "?" + let note: String + if let pct = parsed.parsed["battery_pct"]?.doubleValue { + note = " (the reply still carried a value: battery \(String(format: "%.1f", pct))%" + + " — the result byte on this reply is not established, see #900)" + } else { + note = "" + } + state.append(log: "Command response: \(cmdName) → \(result)\(note)") + // #900: dump the FULL raw frame once per command per connection, so a normal (shareable) + // strap-log export carries the disputed [seq][result] prefix bytes with known provenance — the + // one capture the issue is blocked on. Full frame (not the post-prefix payload, which hides + // those very bytes); matches the GET_DATA_RANGE raw-frame line (#451) and the format #900's + // fixtures are quoted in. Rate-limited: a 4.0 hits this branch on every battery poll. + if !rawDumpedRespCmds.contains(cmdName) { + rawDumpedRespCmds.insert(cmdName) + state.append(log: " raw frame (#900 — [seq][result] provenance): \(Self.fullFrameHex(frame))") + } + } case "EVENT": if let ev = parsed.parsed["event"]?.stringValue { @@ -310,6 +347,14 @@ public final class FrameRouter { return payload.map { String(format: "%02x", $0) }.joined(separator: " ") } + /// #900: the entire frame (0xAA SOF through the crc32 trailer) as contiguous lowercase hex — the + /// provenance format #900's fixtures are quoted in (e.g. "aa0f00c324141a0000…"). Unlike + /// `commandResponsePayloadHex`, this keeps the [type,seq,cmd,origin_seq,result] prefix, which is the + /// exact region #900 needs to inspect. Mirrors the Android `frame.joinToString("") { "%02x" }` dump. + nonisolated static func fullFrameHex(_ frame: [UInt8]) -> String { + frame.map { String(format: "%02x", $0) }.joined() + } + /// Plausibility gate for a readback epoch: a real armed alarm is near-now, so anything outside /// 2017..2100 (1_500_000_000 to 4_102_444_800) is garbage or a strap with no alarm armed - the /// caller falls back to the raw-hex line rather than logging a misleading date. Bounds inclusive. diff --git a/StrandTests/BatteryResultProvenanceDumpTests.swift b/StrandTests/BatteryResultProvenanceDumpTests.swift new file mode 100644 index 0000000000..e0efb15700 --- /dev/null +++ b/StrandTests/BatteryResultProvenanceDumpTests.swift @@ -0,0 +1,73 @@ +import XCTest +import StrandAnalytics +import WhoopProtocol +@testable import Strand + +/// #900: the strap log must self-capture the one artefact the issue is blocked on — a real WHOOP 4.0 +/// `GET_BATTERY_LEVEL` COMMAND_RESPONSE of known provenance. FrameRouter annotates a non-SUCCESS reply that +/// still carried a value (the #923 behaviour, brought to Swift here for parity) AND dumps the FULL raw frame +/// once per command per connection — full frame, because the disputed bytes are the `[seq][result]` prefix +/// that the post-prefix payload dump hides. Rate-limited so a 4.0's per-poll battery reads don't flood the +/// log, and re-armed each connection (when `family` is set). Twin of the Kotlin WhoopBleClient dump. +@MainActor +final class BatteryResultProvenanceDumpTests: XCTestCase { + + private func bytes(_ s: String) -> [UInt8] { + var out = [UInt8](); var i = s.startIndex + while i < s.endIndex { + let j = s.index(i, offsetBy: 2) + out.append(UInt8(s[i.. [String] { + live.log.filter { $0.contains("raw frame (#900") } + } + + func testNonSuccessBatteryReplyIsAnnotatedNotReportedAsABareFailure() { + let live = LiveState() + let router = FrameRouter(state: live) + router.family = .whoop4 + router.handle(frame: bytes(batteryFrameHex)) + + XCTAssertTrue(live.log.contains { + $0.contains("Command response: GET_BATTERY_LEVEL") + && $0.contains("battery 42.5%") + && $0.contains("see #900") + }, "a value-carrying non-SUCCESS battery reply must annotate, not read as a failure: \(live.log)") + } + + func testRawFrameProvenanceIsDumpedOncePerConnection() { + let live = LiveState() + let router = FrameRouter(state: live) + router.family = .whoop4 + + router.handle(frame: bytes(batteryFrameHex)) + XCTAssertEqual(rawDumpLines(live).count, 1, "one raw-frame line on the first battery reply") + XCTAssertTrue(rawDumpLines(live).first?.contains(batteryFrameHex) ?? false, + "the dump must carry the FULL frame incl. the [seq][result] prefix: \(rawDumpLines(live))") + + // Every subsequent poll this connection hits the same non-SUCCESS branch — but must NOT re-dump. + router.handle(frame: bytes(batteryFrameHex)) + router.handle(frame: bytes(batteryFrameHex)) + XCTAssertEqual(rawDumpLines(live).count, 1, "rate-limited: no re-dump for the same command this session") + } + + func testSettingFamilyReArmsTheDumpForTheNextConnection() { + let live = LiveState() + let router = FrameRouter(state: live) + router.family = .whoop4 + router.handle(frame: bytes(batteryFrameHex)) + XCTAssertEqual(rawDumpLines(live).count, 1) + + // A fresh connection sets `family` again (BLEManager.connectCore) → fresh capture session. + router.family = .whoop4 + router.handle(frame: bytes(batteryFrameHex)) + XCTAssertEqual(rawDumpLines(live).count, 2, "re-arming on connect lets the next session re-capture") + } +} 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 5971835307..a8399afcd7 100644 --- a/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt +++ b/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt @@ -2670,6 +2670,10 @@ class WhoopBleClient( private val drainCccdRetryRunnable = Runnable { gatt?.let { drainCccdQueue(it) } } /** Set once startSession() has fired the first command, so it runs exactly once per connection. */ private var sessionStarted = false + /** #900: resp_cmd names whose raw COMMAND_RESPONSE frame has already been dumped this connection, so the + * provenance dump fires once per command per session (a 4.0's per-poll battery reads would otherwise + * flood the strap log). Cleared in reset(). */ + private val rawDumpedRespCmds = mutableSetOf() // ==================================================================================== // MARK: Public API (port of BLEManager.connect / disconnect / send + buzz helper) @@ -5638,6 +5642,15 @@ class WhoopBleClient( "" } log("Command response: ${respCmd ?: "?"} → $result$note") + // #900: dump the FULL raw frame once per command per connection, so a normal (shareable) + // strap-log export carries the disputed [seq][result] prefix bytes with known provenance — + // the one capture the issue is blocked on. Full frame (not whoop4CommandResponsePayload, + // which skips those very bytes); matches the GET_DATA_RANGE raw-frame line (#451). Rate- + // limited: a 4.0 hits this branch on every battery poll. Twin of the macOS FrameRouter dump. + if (rawDumpedRespCmds.add(respCmd ?: "?")) { + log(" raw frame (#900 — [seq][result] provenance): " + + frame.joinToString("") { "%02x".format(it) }) + } } // Arm-readback diagnostic (#401 close-out): armStrapAlarm follows every WHOOP 4.0 arm // with GET_ALARM_TIME (67) so the log proves what the STRAP believes is armed, not just @@ -7932,6 +7945,7 @@ class WhoopBleClient( cccdInFlight = false cccdRetries = 0 sessionStarted = false + rawDumpedRespCmds.clear() // #900: re-arm the per-command raw-frame dump for the next connection serviceDiscoveryRunnable?.let { handler.removeCallbacks(it) } serviceDiscoveryRunnable = null // Clear the onMtuChanged dedup (#50) so the first MTU callback of the NEXT connection — even to