Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 46 additions & 1 deletion Strand/BLE/FrameRouter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> = []

public init(state: LiveState) {
self.state = state
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
Expand Down
73 changes: 73 additions & 0 deletions StrandTests/BatteryResultProvenanceDumpTests.swift
Original file line number Diff line number Diff line change
@@ -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..<j], radix: 16)!); i = j
}
return out
}

/// The real 4.0 battery reply at the heart of #900: succeeds (42.5%) yet its `[seq][result]` prefix is
/// zeroed, so `result` decodes to FAILURE(0). Same fixture as `Whoop4ResponseResultTests`.
private let batteryFrameHex = "aa0f00c324141a0000a9010000000052cd1a49"

private func rawDumpLines(_ live: LiveState) -> [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")
}
}
14 changes: 14 additions & 0 deletions android/app/src/main/java/com/noop/ble/WhoopBleClient.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>()

// ====================================================================================
// MARK: Public API (port of BLEManager.connect / disconnect / send + buzz helper)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Loading