diff --git a/Packages/WhoopProtocol/Sources/WhoopProtocol/FeatureFlagProbe.swift b/Packages/WhoopProtocol/Sources/WhoopProtocol/FeatureFlagProbe.swift index 30cad0e336..5442ac93b5 100644 --- a/Packages/WhoopProtocol/Sources/WhoopProtocol/FeatureFlagProbe.swift +++ b/Packages/WhoopProtocol/Sources/WhoopProtocol/FeatureFlagProbe.swift @@ -75,6 +75,113 @@ public enum FeatureFlagProbe { /// a mis-anchored record, not a key. public static let maxKeyLength = 32 + /// START_DEVICE_CONFIG_KEY_EXCHANGE (115 / 0x73) — the device-config twin of 117. Read-only. + public static let startDeviceConfigKeyExchangeCmd: UInt8 = 115 + /// SEND_NEXT_DEVICE_CONFIG (116 / 0x74) — the device-config twin of 118. Read-only. + public static let sendNextDeviceConfigCmd: UInt8 = 116 + + /// How many consecutive `validKey = 0` replies the walk steps over before it gives up on the + /// EMPTY-SLOT reading (see `NextResponse.isEmptySlot`). + /// + /// Calibrated on the one 117/118 walk this project has: that strap announced 16 keys and served + /// indices **1–10 then 13–18** — a two-slot hole it handled itself, without ever sending + /// `validKey = 0`. A firmware that instead exposes its holes would show them as runs of that size, + /// so eight is four times the largest hole ever observed and still a hard bound: the empty-slot + /// experiment can cost at most eight extra round-trips beyond where the walk used to stop. + public static let maxConsecutiveEmptySlots = 8 + + /// Extra `SEND_NEXT` replies the walk takes AFTER the strap's announced count is satisfied, so the + /// strap — not our arithmetic — gets to end its own list. + /// + /// This exists because of what the announced count actually did on hardware: the 117/118 walk stopped + /// at exactly `steps == count`, having never seen `index = 0xFF` **or** `validKey = 0`. Every entry + /// came back valid and named. So the list was not observed to end; the client stopped asking. The + /// same strap served indices up to **18** while announcing **16**, which is the calibration: an + /// allowance of four is twice that observed excess, and still bounded by `maxFlags`. + public static let countOvershootAllowance = 4 + + /// Ceiling on record bytes rendered as hex in one trace line. Records are short (`revision`, `index`, + /// `validKey`, a ≤32-byte name, padding), so anything past this is already evidence that the record + /// is not the documented layout — and the byte COUNT is always reported even when the tail is elided, + /// so the finding survives the elision. + public static let maxRawHexBytes = 64 + + // MARK: - Namespaces + + /// One enumerate pair — the opcodes the walk drives and the words the report prints for them. + /// + /// The protocol's own `CommandNumber` table names two symmetric enumerate pairs (117/118 for feature + /// flags, 115/116 for device config). Both are walked by the SAME code here, so a fix to the walk's + /// terminator handling cannot apply to one namespace and miss the other. + /// + /// Reusing this decoder for 115/116 ASSUMES the two share a record layout — an inference from the + /// naming symmetry in that table, not an observation. It fails closed: a mismatch surfaces as + /// `.truncated` or as a count `countIsPlausible` rejects. + public struct Namespace: Equatable, Sendable { + public let startCmd: UInt8 + public let nextCmd: UInt8 + public let startLabel: String + public let nextLabel: String + /// Report title, e.g. `FEATURE-FLAG ENUMERATION PROBE`. + public let title: String + /// What the namespace is called in prose, e.g. `feature-flag`. + public let noun: String + /// What ONE entry is called in prose, e.g. `flag`. + public let entryNoun: String + /// Heading for the collected list, e.g. `Flags`. + public let listHeading: String + public init(startCmd: UInt8, nextCmd: UInt8, startLabel: String, nextLabel: String, + title: String, noun: String, entryNoun: String, listHeading: String) { + self.startCmd = startCmd + self.nextCmd = nextCmd + self.startLabel = startLabel + self.nextLabel = nextLabel + self.title = title + self.noun = noun + self.entryNoun = entryNoun + self.listHeading = listHeading + } + } + + /// 117/118 — the feature-flag key list. + public static let featureFlagNamespace = Namespace( + startCmd: startKeyExchangeCmd, nextCmd: sendNextFlagCmd, + startLabel: "START_FF_KEY_EXCHANGE", nextLabel: "SEND_NEXT_FF", + title: "FEATURE-FLAG ENUMERATION PROBE", noun: "feature-flag", entryNoun: "flag", + listHeading: "Flags") + + /// 115/116 — the device-config key list. + public static let deviceConfigNamespace = Namespace( + startCmd: startDeviceConfigKeyExchangeCmd, nextCmd: sendNextDeviceConfigCmd, + startLabel: "START_DEVICE_CONFIG_KEY_EXCHANGE", nextLabel: "SEND_NEXT_DEVICE_CONFIG", + title: "DEVICE-CONFIG ENUMERATION PROBE", noun: "device-config", entryNoun: "config key", + listHeading: "Keys") + + /// Why a walk stopped. A STRING reason reads well in a log and a CODE can be asserted on, and both + /// are always set together: a truncated walk that reports no reason at all is precisely how a partial + /// key list gets mistaken for a complete one. + public enum StopCode: String, Equatable, Sendable { + /// The strap served `index = 0xFF`. The one terminator this project has ever seen unambiguously. + case endMarker + /// A `validKey = 0` reply repeated the previous reply's index: the cursor did not advance, so + /// this walk cannot reach anything beyond that point. Note what this does NOT establish — a + /// firmware whose cursor parks on an empty slot emits the identical frame, so "the list ends + /// here" and "the walk is stuck on a hole" are not separable by this observation. + case emptySlotCursorParked + /// `maxConsecutiveEmptySlots` consecutive `validKey = 0` replies, with the index still advancing. + case emptySlotRunCap + /// `maxFlags` replies. A client-side bound: says nothing about where the strap's list ends. + case stepCap + /// The announced count plus `countOvershootAllowance` was spent. Also a client-side bound. + case announcedCountOvershoot + /// The strap answered nothing inside the probe's window. + case timeout + /// The strap refused the verb (result `UNSUPPORTED`). + case unsupported + /// A reply did not decode. Our limitation, and labelled as ours. + case parseFailure + } + // MARK: - Decoded responses /// Why a frame was not decoded. Distinct cases so the probe can say *what* went wrong in the log @@ -98,15 +205,36 @@ public enum FeatureFlagProbe { public let resultCode: Int? /// Record `revision` — the layout version the firmware answers with. public let revision: Int - /// `numberOfFeatureFlags` as reported by the strap. NOT trusted as a loop bound on its own. + /// `numberOfFeatureFlags` read as u16 LE (`record[1] | record[2] << 8`). NOT trusted as a loop + /// bound on its own, and NOT the only defensible reading of those bytes — see `singleByteCount`. public let count: Int - public init(resultCode: Int?, revision: Int, count: Int) { + /// The raw record bytes this response was decoded from, kept so the report can print them. + /// Parsed fields are a claim about the layout; these bytes are the evidence for it. + public let record: [UInt8] + public init(resultCode: Int?, revision: Int, count: Int, record: [UInt8] = []) { self.resultCode = resultCode self.revision = revision self.count = count + self.record = record } /// True when the count is inside the range a real key list could plausibly occupy. public var countIsPlausible: Bool { count > 0 && count <= FeatureFlagProbe.maxFlags } + + /// The same field read as a SINGLE byte — i.e. on the reading where `record[2]` is padding + /// rather than the count's high byte. + /// + /// The layout this decoder implements calls the field u16 LE. Nothing observed here settles that: + /// every count seen so far has had `record[2] == 0x00`, where a u16 read and a single-byte read + /// return the SAME number, so no capture yet distinguishes them. Both are therefore carried, and + /// the report says plainly when they agree (no evidence either way) and when they differ (which + /// is itself the finding). Guessing one and discarding the other is how a walk gets bounded by a + /// number nobody checked. + public var singleByteCount: Int? { record.count >= 2 ? Int(record[1]) : nil } + /// The byte a u16 reading treats as the count's high half. `0x00` means the two readings agree. + public var countHighByte: Int? { record.count >= 3 ? Int(record[2]) : nil } + /// True when the u16 and single-byte readings return the same number, i.e. this reply is silent + /// on the field's width. + public var countReadingsAgree: Bool { countHighByte == 0 } } /// Decoded `SEND_NEXT_FF` reply. @@ -119,21 +247,45 @@ public enum FeatureFlagProbe { public let validKey: Bool /// The key name, when one decodes as printable ASCII; nil when the entry carries none. public let key: String? - public init(resultCode: Int?, revision: Int, index: Int, validKey: Bool, key: String?) { + /// The raw record bytes this response was decoded from, kept so the report can print them. + public let record: [UInt8] + public init(resultCode: Int?, revision: Int, index: Int, validKey: Bool, key: String?, + record: [UInt8] = []) { self.resultCode = resultCode self.revision = revision self.index = index self.validKey = validKey self.key = key + self.record = record } - /// The STRAP said stop: it returned the 0xFF end marker, or flagged the entry as not a real key. - /// Both are the firmware's own signal, so both are trusted. + + /// The strap's UNAMBIGUOUS end marker: `index = 0xFF`. + /// + /// This used to also mean `validKey = 0`, on the reading that the firmware sets that flag to + /// signal the end of its list. That reading is not established. The file's own header note says + /// the record layout is **unverified on 5/MG** — it was derived from a WHOOP 4.0 with an R19-era + /// key list — and the terminator semantics came with the layout. + /// + /// What a 5/MG (WS50_r03) actually served, on the only walks this project has: + /// + /// - 117/118: sixteen replies, every one `validKey = 1` with a decodable name, indices 1–10 then + /// 13–18. The strap handled its own two-slot hole internally. **Neither** `validKey = 0` **nor** + /// `index = 0xFF` was ever served — the walk ended because the client stopped asking at the + /// announced count. + /// - 115/116: the final reply carried `index = 255` **and** `validKey = 0` **together**. Both + /// conditions fired on the same frame, so that run cannot say which one the firmware meant. /// - /// Deliberately does NOT include `key == nil`. That is OUR parser declining a name — a byte outside - /// printable ASCII, or one longer than `maxKeyLength` — and it says nothing about whether the strap - /// has more entries. Treating it as a terminator meant one undecodable name threw away every entry - /// after it: a list of forty with a bad byte at seven yielded six keys. See `isSkippable`. - public var isExhausted: Bool { !validKey || index == 0xFF } + /// So on this hardware the disjunction has never been separated. If `validKey = 0` in fact marks + /// an EMPTY or RETIRED SLOT with the list continuing past it, then stopping there truncates — + /// the identical failure `isSkippable` exists to prevent, one condition over, and one that would + /// silently shorten every published key list including this project's own. `isEmptySlot` is that + /// case, and the walk now steps over it and records what comes next. + public var isExhausted: Bool { index == 0xFF } + + /// `validKey = 0` WITHOUT the 0xFF end marker — the ambiguous case, and the one this probe exists + /// to resolve. Recorded, stepped over, and bounded by `maxConsecutiveEmptySlots`; what the strap + /// serves next is the evidence that separates "end of list" from "empty slot". + public var isEmptySlot: Bool { !validKey && index != 0xFF } /// The firmware calls this a real key but the name did not decode. Record it and KEEP WALKING. /// @@ -148,19 +300,30 @@ public enum FeatureFlagProbe { /// Decode a `START_FF_KEY_EXCHANGE` COMMAND_RESPONSE. CRC-gated: a frame whose checksums fail is /// rejected before any field is read. - public static func parseStart(frame: [UInt8], family: DeviceFamily) -> Result { - switch record(frame: frame, family: family, expecting: startKeyExchangeCmd) { + /// + /// `expecting` defaults to 117 and exists so the DEVICE-CONFIG twin + /// `START_DEVICE_CONFIG_KEY_EXCHANGE` (115) is decoded by this same code — the reuse contract + /// documented on `Namespace`. + public static func parseStart(frame: [UInt8], family: DeviceFamily, + expecting: UInt8 = startKeyExchangeCmd) + -> Result { + switch record(frame: frame, family: family, expecting: expecting) { case .failure(let f): return .failure(f) case .success(let r): guard r.record.count >= 3 else { return .failure(.truncated) } let count = Int(r.record[1]) | (Int(r.record[2]) << 8) - return .success(StartResponse(resultCode: r.resultCode, revision: Int(r.record[0]), count: count)) + return .success(StartResponse(resultCode: r.resultCode, revision: Int(r.record[0]), + count: count, record: r.record)) } } /// Decode a `SEND_NEXT_FF` COMMAND_RESPONSE. CRC-gated like `parseStart`. - public static func parseNext(frame: [UInt8], family: DeviceFamily) -> Result { - switch record(frame: frame, family: family, expecting: sendNextFlagCmd) { + /// `expecting` defaults to 118 and carries the same reuse contract: 116 + /// (`SEND_NEXT_DEVICE_CONFIG`) walks the device-config namespace through this decoder. + public static func parseNext(frame: [UInt8], family: DeviceFamily, + expecting: UInt8 = sendNextFlagCmd) + -> Result { + switch record(frame: frame, family: family, expecting: expecting) { case .failure(let f): return .failure(f) case .success(let r): // revision + index are the minimum: the 0xFF end marker arrives with nothing after it. @@ -170,7 +333,7 @@ public enum FeatureFlagProbe { let validKey = r.record.count >= 3 ? r.record[2] != 0 : false let key = r.record.count >= 4 ? asciiKey(Array(r.record[3...])) : nil return .success(NextResponse(resultCode: r.resultCode, revision: revision, index: index, - validKey: validKey, key: key)) + validKey: validKey, key: key, record: r.record)) } } @@ -223,6 +386,21 @@ public enum FeatureFlagProbe { return out.isEmpty ? nil : String(decoding: out, as: UTF8.self) } + /// Lower-case space-separated hex, for putting the RAW bytes of a reply in the log next to the + /// fields decoded from them. + /// + /// Parsed output alone has repeatedly turned out to be insufficient evidence here: a claim about the + /// record layout cannot be re-checked, or contradicted, from the fields that layout produced. Over + /// `maxRawHexBytes` the tail is elided and the true length is printed, so the elision itself is + /// visible and a suspiciously long record still reads as one. + public static func hex(_ bytes: [UInt8]) -> String { + if bytes.isEmpty { return "(empty)" } + let shown = bytes.prefix(maxRawHexBytes) + var out = shown.map { String(format: "%02x", $0) }.joined(separator: " ") + if bytes.count > maxRawHexBytes { out += " … (\(bytes.count) bytes total)" } + return out + } + /// 5/MG result-code label (0 FAILURE / 1 SUCCESS / 2 PENDING / 3 UNSUPPORTED), matching /// `BodyLocationProbe`. Unknown codes keep their number rather than being coerced. static func resultLabel(_ code: Int) -> String { @@ -247,86 +425,195 @@ public struct FeatureFlagProbeReport: Equatable, Sendable { /// Which family the probe ran against (labels only). public let family: DeviceFamily + /// Which enumerate pair this walk drives — 117/118 or 115/116. The walk logic is identical; only + /// the opcodes and the words differ, so neither namespace can drift from the other's terminator rules. + public let namespace: FeatureFlagProbe.Namespace /// Key names collected, in the order the strap reported them. public private(set) var keys: [String] = [] /// Trace lines: one per reply plus any failure notes. public private(set) var trace: [String] = [] /// SEND_NEXT replies seen. Bounds the walk (see `noteNext`). public private(set) var steps = 0 - /// The count the strap reported for `START_FF_KEY_EXCHANGE`, when it answered. + /// The count the strap reported for the START verb, when it answered. public private(set) var reportedCount: Int? + /// The same count read as a single byte (`record[1]`), when the record carried one. + public private(set) var reportedCountSingleByte: Int? + /// `record[2]` — the byte a u16 reading treats as the count's high half. + public private(set) var reportedCountHighByte: Int? /// Result code of the START reply on 5/MG (nil on 4.0, or before it lands). public private(set) var startResult: Int? /// Entries the strap flagged as real keys whose NAME did not decode, and which the walk stepped over /// rather than stopping at. Surfaced in the report so a dump with holes never reads as a complete list. public private(set) var skipped = 0 + /// Replies carrying `validKey = 0` WITHOUT the 0xFF end marker, which the walk stepped over. + public private(set) var emptySlots = 0 + /// True once any `validKey = 0` reply arrived without the 0xFF marker. + public private(set) var sawEmptySlot = false + /// True once `index = 0xFF` arrived. + public private(set) var sawEndMarker = false + /// True when the 0xFF reply ALSO carried `validKey = 0` — i.e. both terminator conditions fired on + /// one frame, and that run cannot say which one the firmware meant. This is what a 5/MG served on + /// 115/116, and the reason the two have never been separated. + public private(set) var endMarkerAlsoCarriedInvalidFlag = false + /// Replies the strap served AFTER the first `validKey = 0`. Any value above zero is the decisive + /// observation: the list did not end there. + public private(set) var repliesAfterFirstEmptySlot = 0 + /// Key names collected after the first `validKey = 0` — keys a walk that stopped there would have lost. + public private(set) var keysAfterFirstEmptySlot = 0 + /// Replies after the first `validKey = 0` that the FIRMWARE flagged as real entries, whether or not + /// this parser could decode their names. + /// + /// This — not the decoded-key count — is what makes the empty-slot reading decisive. #874's rule + /// applies to the conclusion as much as to the walk: if the strap named an entry past the hole and our + /// ASCII filter declined it, the list still continued, and reporting "inconclusive" there would be our + /// parser's limitation deciding a question about the firmware. + public private(set) var validEntriesAfterFirstEmptySlot = 0 + /// Replies taken past the strap's announced count, under `countOvershootAllowance`. + public private(set) var repliesPastAnnouncedCount = 0 /// Set once the walk stopped for a reason we can name. public private(set) var stopReason: String? + /// The same reason as a code, so a caller (or a test) can branch on it rather than on prose. + public private(set) var stopCode: FeatureFlagProbe.StopCode? + /// True once the walk has a reason to stop. The BLE driver consults this after EVERY reply, so a + /// refusal recorded on the START reply ends the run instead of being stepped over into the next verb. + public var hasStopped: Bool { stopCode != nil } + /// The index the previous reply carried, for spotting a cursor that stopped advancing. + private var lastIndex: Int? + /// Length of the current unbroken run of `validKey = 0` replies, reset by any valid entry. + private var consecutiveEmptySlots = 0 - public init(family: DeviceFamily) { self.family = family } + public init(family: DeviceFamily, + namespace: FeatureFlagProbe.Namespace = FeatureFlagProbe.featureFlagNamespace) { + self.family = family + self.namespace = namespace + } /// Record the START reply. public mutating func noteStart(_ r: FeatureFlagProbe.StartResponse) { reportedCount = r.count + reportedCountSingleByte = r.singleByteCount + reportedCountHighByte = r.countHighByte startResult = r.resultCode - var line = "START_FF_KEY_EXCHANGE(117) → revision=\(r.revision) count=\(r.count)" + var line = "\(namespace.startLabel)(\(namespace.startCmd)) → revision=\(r.revision) count=\(r.count)" if let c = r.resultCode { line += " result=\(FeatureFlagProbe.resultLabel(c))(\(c))" } + line += " raw=\(FeatureFlagProbe.hex(r.record))" if !r.countIsPlausible { line += " ⚠︎ count outside 1…\(FeatureFlagProbe.maxFlags) — treated as unknown, the walk still stops on the strap's own end marker" } trace.append(line) + if let c = r.resultCode, c == 3 { + stopReason = "strap refused \(namespace.startLabel)(\(namespace.startCmd)) with UNSUPPORTED(3)" + stopCode = .unsupported + } } /// Record one SEND_NEXT reply. Returns true when the walk should continue. + /// + /// The decision order IS the experiment. `index = 0xFF` ends the walk, because that marker is the one + /// thing a strap has served here unambiguously. `validKey = 0` on its own does NOT end it: the walk + /// steps over the entry, sends the next-record verb again, and records what comes back — which is the + /// only way to tell "end of list" from "empty slot" apart. Everything past that point is a + /// CLIENT-side bound (`maxConsecutiveEmptySlots`, `maxFlags`, the announced count plus its + /// overshoot), and each one names itself in `stopCode` so a walk that ended on our arithmetic can + /// never be read as a walk the strap ended. @discardableResult public mutating func noteNext(_ r: FeatureFlagProbe.NextResponse) -> Bool { steps += 1 - var line = "SEND_NEXT_FF(118) → index=\(r.index) validKey=\(r.validKey)" + if sawEmptySlot { repliesAfterFirstEmptySlot += 1 } + var line = "\(namespace.nextLabel)(\(namespace.nextCmd)) → index=\(r.index) validKey=\(r.validKey)" if let k = r.key { line += " key=\"\(k)\"" } if let c = r.resultCode { line += " result=\(FeatureFlagProbe.resultLabel(c))(\(c))" } + line += " raw=\(FeatureFlagProbe.hex(r.record))" trace.append(line) + + // 0. An explicit refusal. UNSUPPORTED(3) says the firmware does not serve this verb, so nothing + // in the record is meaningful and there is nothing to walk. Checked FIRST, before any field is + // acted on, and named as the strap's answer rather than as one of our bounds. + if let c = r.resultCode, c == 3 { + stopReason = "strap refused \(namespace.nextLabel)(\(namespace.nextCmd)) with UNSUPPORTED(3)" + stopCode = .unsupported + return false + } + + // 1. The strap's own unambiguous end marker. Nothing overrides this. if r.isExhausted { - if r.index == 0xFF { - stopReason = "cursor exhausted (index 0xFF)" - } else { - // `validKey = 0` is documented as an end marker alongside 0xFF, and it is the firmware's - // own flag, so it is trusted here. But nothing observed so far rules out the other - // reading — that it marks an EMPTY SLOT and the list continues past it. If so this stops - // on the first hole, which is the same truncation `isSkippable` exists to prevent, one - // condition over. It cannot be settled without a strap, so instead of guessing we make - // the discrepancy loud: stopping here well short of the announced count is exactly the - // evidence that would settle it. - stopReason = "firmware reported validKey=false" - if let count = reportedCount, count > steps { - stopReason = "firmware reported validKey=false at index \(r.index) after \(steps) of " - + "\(count) announced entries — if validKey=0 marks an empty slot rather than the " - + "end of the list, the remainder was NOT walked (see #872 review)" - } + sawEndMarker = true + if !r.validKey { + endMarkerAlsoCarriedInvalidFlag = true + trace[trace.count - 1] += " (index=0xFF AND validKey=0 on the same reply — both terminator conditions at once)" } + stopReason = "cursor exhausted (index 0xFF)" + stopCode = .endMarker return false } - if r.isSkippable { - // Our decode declined the name; the strap still says the entry is real and may have more after - // it. Count it so a partial dump describes itself instead of looking complete. - skipped += 1 - trace[trace.count - 1] += " (name did not decode — skipped, walk continues)" + + // 2. `validKey = 0` with no end marker: the ambiguous case. Step over it and keep asking. + if r.isEmptySlot { + let parked = (lastIndex == r.index) + lastIndex = r.index + emptySlots += 1 + consecutiveEmptySlots += 1 + let runLength = consecutiveEmptySlots + sawEmptySlot = true + trace[trace.count - 1] += " (validKey=0 without the 0xFF marker — treated as an EMPTY SLOT, walk continues)" + if parked { + // The cursor did not move. A firmware that means "end of list" parks there and repeats + // itself, so this is the observation that settles the ambiguity the other way — and it + // costs two round-trips rather than a full cap's worth. + stopReason = "validKey=0 repeated at index \(r.index) without advancing — the cursor is " + + "parked, so on this firmware validKey=0 IS a terminator" + stopCode = .emptySlotCursorParked + return false + } + if runLength >= FeatureFlagProbe.maxConsecutiveEmptySlots { + stopReason = "\(runLength) consecutive validKey=0 replies (cap " + + "\(FeatureFlagProbe.maxConsecutiveEmptySlots)) — a CLIENT-side bound, not the strap's" + stopCode = .emptySlotRunCap + return false + } + } else { + lastIndex = r.index + consecutiveEmptySlots = 0 + if sawEmptySlot { validEntriesAfterFirstEmptySlot += 1 } + if r.isSkippable { + // Our decode declined the name; the strap still says the entry is real and may have more + // after it. Count it so a partial dump describes itself instead of looking complete. + skipped += 1 + trace[trace.count - 1] += " (name did not decode — skipped, walk continues)" + } + if let k = r.key, !keys.contains(k) { + keys.append(k) + if sawEmptySlot { keysAfterFirstEmptySlot += 1 } + } } - if let k = r.key, !keys.contains(k) { keys.append(k) } - // Bound the walk on REPLIES, not on distinct keys: a firmware whose cursor never advances would - // repeat one name forever, and a key-count bound would never stop writing 118 to the strap. + + // 3. Bound the walk on REPLIES, not on distinct keys: a firmware whose cursor never advances would + // repeat one name forever, and a key-count bound would never stop writing the verb to the strap. if steps >= FeatureFlagProbe.maxFlags { stopReason = "safety cap of \(FeatureFlagProbe.maxFlags) replies reached" + stopCode = .stepCap return false } - if let count = reportedCount, count > 0, count <= FeatureFlagProbe.maxFlags, steps >= count { - stopReason = "walked the \(count) entries the strap announced" - return false + // 4. The announced count, PLUS an overshoot, so the strap gets to end its own list. On the one + // 117/118 walk this project has, `steps >= count` was the only thing that stopped it — no marker, + // no validKey=0 — which means that list was never observed to end at all. + if let count = reportedCount, count > 0, count <= FeatureFlagProbe.maxFlags { + if steps > count { repliesPastAnnouncedCount = steps - count } + if steps >= count + FeatureFlagProbe.countOvershootAllowance { + stopReason = "walked the \(count) entries the strap announced plus " + + "\(FeatureFlagProbe.countOvershootAllowance) more — a CLIENT-side bound; the strap " + + "never sent an end marker, so the list is not known to end here" + stopCode = .announcedCountOvershoot + return false + } } return true } - /// Record a reply that could not be decoded. - public mutating func noteFailure(_ f: FeatureFlagProbe.ParseFailure, command: Int) { + /// Record a reply that could not be decoded. `frame`, when supplied, is logged in full: a frame that + /// failed its CRC is still the only evidence of what the strap actually put on the wire. + public mutating func noteFailure(_ f: FeatureFlagProbe.ParseFailure, command: Int, + frame: [UInt8] = []) { let why: String switch f { case .crc: why = "CRC failed — frame rejected (never decoded)" @@ -334,27 +621,113 @@ public struct FeatureFlagProbeReport: Equatable, Sendable { case .wrongCommand: why = "COMMAND_RESPONSE for a different command" case .truncated: why = "record too short for the documented layout" } - trace.append("cmd \(command) reply not decoded: \(why)") - stopReason = stopReason ?? why + var line = "cmd \(command) reply not decoded: \(why)" + if !frame.isEmpty { line += " raw frame=\(FeatureFlagProbe.hex(frame))" } + trace.append(line) + if stopReason == nil { + stopReason = why + stopCode = .parseFailure + } } /// Record the strap answering nothing at all within the probe's window. public mutating func noteTimeout(command: Int, seconds: Int) { trace.append("no COMMAND_RESPONSE for opcode \(command) within \(seconds)s") - stopReason = stopReason ?? "strap served no reply to opcode \(command) within \(seconds)s" + if stopReason == nil { + stopReason = "strap served no reply to opcode \(command) within \(seconds)s" + stopCode = .timeout + } + } + + // MARK: - Findings + + /// What this run established about the two terminator conditions — the question the probe exists to + /// answer, stated in the report rather than left for a reader to reconstruct from the trace. + public var terminatorFinding: String { + if sawEmptySlot && (validEntriesAfterFirstEmptySlot > 0 || sawEndMarker) { + var s = "DECISIVE — validKey=0 is an EMPTY/RETIRED SLOT on this firmware, not the end of the " + s += "list: the strap served \(repliesAfterFirstEmptySlot) further repl(ies) after the first " + s += "validKey=0" + if validEntriesAfterFirstEmptySlot > 0 { + s += ", \(validEntriesAfterFirstEmptySlot) of them flagged validKey=1" + } + if keysAfterFirstEmptySlot > 0 { s += ", naming \(keysAfterFirstEmptySlot) more key(s)" } + if sawEndMarker { s += ", and ended on the index=0xFF marker" } + s += ". A walk that stopped on validKey=0 would have been truncated here." + return s + } + if stopCode == .emptySlotCursorParked { + // The DECISIVE label is kept, narrowed to what the run actually decides. "There is nothing + // past it" is a step beyond the observation, and TWO firmwares emit this identical frame: + // a list that genuinely ends at a parked sentinel, and a cursor that advances only on a valid + // record — where validKey=0 is a SLOT, the walk is stuck on a hole, and keys may sit behind + // it. The second is precisely the reading this probe exists to make testable, so printing the + // first as decided would hand a reader the conclusion the probe was written to question. + return "DECISIVE — validKey=0 is a TERMINATOR on this firmware: the next request returned the " + + "same index with validKey=0 again, so the cursor does not advance past it, so this walk " + + "cannot see anything beyond. Whether the list truly ends here is not separable from a " + + "firmware whose cursor parks on an empty slot." + } + if sawEmptySlot { + return "INCONCLUSIVE — validKey=0 was served and the walk continued past it, but a client-side " + + "bound ended the run before the strap did. Nothing here separates 'end of list' from " + + "'empty slot'." + } + if sawEndMarker && endMarkerAlsoCarriedInvalidFlag { + return "AMBIGUOUS — the walk ended on a reply carrying index=0xFF AND validKey=0 together, so " + + "both terminator conditions fired at once and this run cannot say which one the firmware " + + "meant." + } + if sawEndMarker { + return "index=0xFF ended the walk with validKey still true on the same reply, so the 0xFF " + + "marker alone terminates. validKey=0 was never served, so it is untested here." + } + return "NO TERMINATOR OBSERVED — neither validKey=0 nor index=0xFF was served in \(steps) " + + "repl(ies); the walk ended on a CLIENT-side bound, so the strap's list is not known to end " + + "where this report stops." + } + + /// What the announced count actually says, including the reading it does not settle. + public var countFinding: String? { + guard let count = reportedCount else { return nil } + var s = "u16 LE read = \(count)" + if let single = reportedCountSingleByte, let high = reportedCountHighByte { + s += "; single-byte read = \(single) (high byte " + + String(format: "0x%02x", high) + ")" + s += high == 0 + ? " — the two readings AGREE, so this reply does not establish the field's width" + : " — the two readings DISAGREE; the walk uses the u16 value and plausibility-checks it" + } + s += ". Keys yielded: \(keys.count)" + if skipped > 0 { s += " (+\(skipped) undecodable)" } + if emptySlots > 0 { s += " (+\(emptySlots) empty slot(s))" } + let accounted = keys.count + skipped + emptySlots + if accounted != count { + s += " — MISMATCH against the announced \(count)" + } + if repliesPastAnnouncedCount > 0 { + s += ". The strap kept answering \(repliesPastAnnouncedCount) repl(ies) past its own announced " + + "count." + } + return s } /// The full copyable report. public func render() -> String { let fam = family == .whoop5 ? "WHOOP 5/MG" : "WHOOP 4.0" - var sb = "#761 FEATURE-FLAG ENUMERATION PROBE — \(fam)\n" - sb += "Read-only: START_FF_KEY_EXCHANGE(117) + SEND_NEXT_FF(118). No value is written; " + var sb = "#761 \(namespace.title) — \(fam)\n" + sb += "Read-only: \(namespace.startLabel)(\(namespace.startCmd)) + " + sb += "\(namespace.nextLabel)(\(namespace.nextCmd)). No value is written; " sb += "SET_FF_VALUE(120) and SET_DEVICE_CONFIG_VALUE(119) are never sent from this path.\n" sb += "\nVerdict: \(verdict)\n" if let stopReason { sb += "Stopped: \(stopReason)\n" } - sb += "\nFlags reported by the strap (\(keys.count)" + if let stopCode { sb += "Stop code: \(stopCode.rawValue)\n" } + sb += "Terminator: \(terminatorFinding)\n" + if let countFinding { sb += "Announced count: \(countFinding)\n" } + sb += "\n\(namespace.listHeading) reported by the strap (\(keys.count)" if let reportedCount { sb += " of \(reportedCount) announced" } if skipped > 0 { sb += ", \(skipped) name(s) did not decode and were skipped" } + if emptySlots > 0 { sb += ", \(emptySlots) validKey=0 slot(s) stepped over" } sb += "):\n" if keys.isEmpty { sb += " (none)\n" @@ -371,16 +744,20 @@ public struct FeatureFlagProbeReport: Equatable, Sendable { /// verdict is the line that gets pasted into an issue, and restating a number the probe itself /// distrusts as bare fact is the same over-claim in a smaller place. A plausible count renders /// exactly as before, so the common report is unchanged. + /// + /// The noun comes from `namespace`, not a literal: #913 wrote this when the probe walked 117/118 only, + /// so "flag(s)" was the only possibility. This PR walks 115/116 through the same code, where the + /// entries are config keys — a hardcoded "flag(s)" would mislabel every device-config report. private var announcedFlags: String { let n = reportedCount ?? 0 let plausible = n > 0 && n <= FeatureFlagProbe.maxFlags - return plausible ? "\(n) flag(s)" : "an implausible \(n) flag(s)" + return plausible ? "\(n) \(namespace.entryNoun)(s)" : "an implausible \(n) \(namespace.entryNoun)(s)" } /// One-line summary of what the probe established. public var verdict: String { if let startResult, startResult == 3 { - return "opcode 117 REJECTED by firmware (UNSUPPORTED) — this strap does not serve the enumerate verb" + return "opcode \(namespace.startCmd) REJECTED by firmware (UNSUPPORTED) — this strap does not serve the enumerate verb" } if keys.isEmpty && reportedCount == nil { return "no usable reply — the enumerate path is unconfirmed on this firmware" @@ -390,7 +767,7 @@ public struct FeatureFlagProbeReport: Equatable, Sendable { // would carry into #103. Same class as `isSkippable`: never report our limitation as the strap's // behaviour. if keys.isEmpty && skipped > 0 { - return "strap named \(skipped) flag(s), none of which decoded as printable ASCII within " + return "strap named \(skipped) \(namespace.entryNoun)(s), none of which decoded as printable ASCII within " + "\(FeatureFlagProbe.maxKeyLength) chars — this is our parser rejecting them, NOT the " + "strap serving blanks; see the trace for the raw replies" } @@ -401,15 +778,24 @@ public struct FeatureFlagProbeReport: Equatable, Sendable { // timer firing on the first 118. What the strap would have named is unknown, and the report has // to say unknown. `stopReason`, rendered directly under this line, names which of the two it was. if keys.isEmpty && steps == 0 { - return "strap announced \(announcedFlags); no SEND_NEXT_FF(118) reply was decoded — " - + "the key list was never read (inconclusive)" + // Namespace-aware for the same reason as `announcedFlags`: #913 could name SEND_NEXT_FF(118) + // literally because 117/118 was the only walk; a 115/116 run must say SEND_NEXT_DEVICE_CONFIG(116). + return "strap announced \(announcedFlags); no \(namespace.nextLabel)(\(namespace.nextCmd)) " + + "reply was decoded — the key list was never read (inconclusive)" } if keys.isEmpty { + // Both halves survive the merge with #913: its doubt about an implausible announced count + // (`announcedFlags`), and this PR's namespace-aware noun. Taking either side alone would drop + // the other — #913's version hardcodes "flag(s)" and would mislabel a device-config walk. return "strap announced \(announcedFlags) but named none" } - if skipped > 0 { - return "enumerated \(keys.count) feature-flag key name(s); \(skipped) further name(s) did not decode" + var s = "enumerated \(keys.count) \(namespace.noun) key name(s)" + if skipped > 0 { s += "; \(skipped) further name(s) did not decode" } + // A walk that ended on OUR bound is not a complete list, and the one-line summary is the line that + // gets pasted into an issue. Say it here, not only four lines further down. + if stopCode == .stepCap || stopCode == .announcedCountOvershoot || stopCode == .emptySlotRunCap { + s += " — INCOMPLETE: the walk ended on a client-side bound, not on the strap's own end marker" } - return "enumerated \(keys.count) feature-flag key name(s)" + return s } } diff --git a/Packages/WhoopProtocol/Tests/WhoopProtocolTests/FeatureFlagProbeTests.swift b/Packages/WhoopProtocol/Tests/WhoopProtocolTests/FeatureFlagProbeTests.swift index c37a34098f..4b21687df5 100644 --- a/Packages/WhoopProtocol/Tests/WhoopProtocolTests/FeatureFlagProbeTests.swift +++ b/Packages/WhoopProtocol/Tests/WhoopProtocolTests/FeatureFlagProbeTests.swift @@ -145,21 +145,146 @@ final class FeatureFlagProbeTests: XCTestCase { XCTAssertEqual(report.stopReason, "cursor exhausted (index 0xFF)") } - func testValidKeyFalseStopsTheWalkEvenWithBytesAfterIt() { - // validKey=0 with a plausible-looking name after it: the firmware's own flag wins. + /// `validKey = 0` alone is NOT classified as the strap's end marker any more — it is an empty slot, + /// and the walk steps over it. The entry itself is still never collected as a key. + func testValidKeyFalseIsAnEmptySlotNotTheEndMarker() { + // validKey=0 with a plausible-looking name after it: still not a key, still not the end. let record: [UInt8] = [0x01, 0x04, 0x00] + keyBytes("stale_buffer_leftover") let frame = whoop4Response(cmd: 118, payload: payload(result: 1, record: record)) guard case .success(let r) = FeatureFlagProbe.parseNext(frame: frame, family: .whoop4) else { return XCTFail("expected a decoded NEXT response") } XCTAssertFalse(r.validKey) - XCTAssertTrue(r.isExhausted) + XCTAssertFalse(r.isExhausted, "only index=0xFF is the strap's unambiguous end marker") + XCTAssertTrue(r.isEmptySlot) var report = FeatureFlagProbeReport(family: .whoop4) - XCTAssertFalse(report.noteNext(r)) - XCTAssertEqual(report.stopReason, "firmware reported validKey=false") + XCTAssertTrue(report.noteNext(r), "validKey=0 without 0xFF must not end the walk") + XCTAssertNil(report.stopReason) + XCTAssertEqual(report.emptySlots, 1) XCTAssertTrue(report.keys.isEmpty, "an invalid entry must never be collected as a flag") } + /// The whole experiment, in one walk: the strap serves `validKey = 0` mid-list, the walk keeps asking, + /// and the strap names MORE keys before ending on 0xFF. That sequence is decisive — under the old rule + /// this list would have been reported as one key long. + func testWalkContinuesPastValidKeyZeroAndCollectsTheKeysAfterIt() { + var report = FeatureFlagProbeReport(family: .whoop5) + report.noteStart(FeatureFlagProbe.StartResponse(resultCode: 1, revision: 1, count: 5, + record: [0x01, 0x05, 0x00])) + func next(_ index: Int, valid: Bool, _ key: String?) -> FeatureFlagProbe.NextResponse { + FeatureFlagProbe.NextResponse(resultCode: 1, revision: 1, index: index, validKey: valid, + key: key, record: [0x01, UInt8(index), valid ? 1 : 0]) + } + XCTAssertTrue(report.noteNext(next(1, valid: true, "enable_r22_packets"))) + XCTAssertTrue(report.noteNext(next(2, valid: false, nil)), "an empty slot must not end the walk") + XCTAssertTrue(report.noteNext(next(3, valid: false, nil)), "nor must a second one") + XCTAssertTrue(report.noteNext(next(4, valid: true, "enable_sig12"))) + XCTAssertFalse(report.noteNext(next(0xFF, valid: false, nil))) + + XCTAssertEqual(report.keys, ["enable_r22_packets", "enable_sig12"]) + XCTAssertEqual(report.emptySlots, 2) + XCTAssertEqual(report.keysAfterFirstEmptySlot, 1) + XCTAssertEqual(report.stopCode, .endMarker) + let finding = report.terminatorFinding + XCTAssertTrue(finding.hasPrefix("DECISIVE — validKey=0 is an EMPTY/RETIRED SLOT"), finding) + XCTAssertTrue(finding.contains("naming 1 more key(s)"), finding) + XCTAssertTrue(report.render().contains("2 validKey=0 slot(s) stepped over")) + } + + /// #874's rule reaches the CONCLUSION too. The strap flagged an entry past the hole as real; our ASCII + /// filter declined its name. The list still continued, so the finding must still be decisive — reading + /// "inconclusive" here would be our parser deciding a question about the firmware. + func testAnUndecodableEntryAfterAnEmptySlotIsStillDecisive() { + var report = FeatureFlagProbeReport(family: .whoop5) + report.noteStart(FeatureFlagProbe.StartResponse(resultCode: 1, revision: 1, count: 30, + record: [0x01, 0x1E, 0x00])) + XCTAssertTrue(report.noteNext(FeatureFlagProbe.NextResponse( + resultCode: 1, revision: 1, index: 1, validKey: false, key: nil, record: [0x01, 0x01, 0x00]))) + XCTAssertTrue(report.noteNext(FeatureFlagProbe.NextResponse( + resultCode: 1, revision: 1, index: 2, validKey: true, key: nil, + record: [0x01, 0x02, 0x01, 0xDE, 0xAD]))) + XCTAssertEqual(report.keys, [], "no name decoded, so no key is invented") + XCTAssertEqual(report.keysAfterFirstEmptySlot, 0) + XCTAssertEqual(report.validEntriesAfterFirstEmptySlot, 1) + let finding = report.terminatorFinding + XCTAssertTrue(finding.hasPrefix("DECISIVE — validKey=0 is an EMPTY/RETIRED SLOT"), finding) + XCTAssertTrue(finding.contains("1 of them flagged validKey=1"), finding) + XCTAssertFalse(finding.contains("naming"), finding) + } + + /// The other decisive outcome, and the one that costs two round-trips: the strap repeats the same + /// index with `validKey = 0`. What that decides is that the cursor does not advance, so this walk + /// cannot see past the point — NOT that the list ends there. A firmware whose cursor parks on an + /// empty slot emits the identical frame, and that is the reading this whole probe exists to make + /// testable, so the verdict must not print it as settled. Both halves are pinned below: the DECISIVE + /// label stays, and the over-claim must not come back. + func testRepeatedEmptySlotAtTheSameIndexIsReportedAsATerminator() { + var report = FeatureFlagProbeReport(family: .whoop5) + report.noteStart(FeatureFlagProbe.StartResponse(resultCode: 1, revision: 1, count: 9, + record: [0x01, 0x09, 0x00])) + let empty = FeatureFlagProbe.NextResponse(resultCode: 1, revision: 1, index: 7, validKey: false, + key: nil, record: [0x01, 0x07, 0x00]) + XCTAssertTrue(report.noteNext(empty), "the first empty slot is stepped over") + XCTAssertFalse(report.noteNext(empty), "a parked cursor ends the walk") + XCTAssertEqual(report.stopCode, .emptySlotCursorParked) + XCTAssertTrue(report.terminatorFinding.hasPrefix("DECISIVE — validKey=0 is a TERMINATOR"), + report.terminatorFinding) + // The narrowing, pinned so it cannot be quietly widened back. "There is nothing past it" is the + // conclusion a reader would paste into an issue as settled, and it is the one this probe exists + // to question — two firmwares emit this identical frame. + XCTAssertTrue(report.terminatorFinding.contains("the cursor does not advance past it"), + report.terminatorFinding) + XCTAssertTrue(report.terminatorFinding.contains( + "not separable from a firmware whose cursor parks on an empty slot"), + report.terminatorFinding) + XCTAssertFalse(report.terminatorFinding.contains("there is nothing past it"), + "the walk observes a stalled cursor, not an empty tail: \(report.terminatorFinding)") + XCTAssertEqual(report.steps, 2, "settling this must cost two round-trips, not a full cap") + } + + /// A firmware that answers `validKey = 0` forever with an ADVANCING index cannot run the walk away: + /// the consecutive-empty cap stops it, and names itself a CLIENT-side bound so the run is never read + /// as a complete list. + func testAnUnendingRunOfEmptySlotsStopsAtTheConsecutiveCap() { + var report = FeatureFlagProbeReport(family: .whoop4) + var index = 0 + var sent = 0 + while report.noteNext(FeatureFlagProbe.NextResponse( + resultCode: 1, revision: 1, index: index, validKey: false, key: nil, + record: [0x01, UInt8(index), 0x00])) { + index += 1 + sent += 1 + XCTAssertLessThan(sent, FeatureFlagProbe.maxConsecutiveEmptySlots + 2, "walk must not run away") + } + XCTAssertEqual(report.steps, FeatureFlagProbe.maxConsecutiveEmptySlots) + XCTAssertEqual(report.stopCode, .emptySlotRunCap) + let why = report.stopReason ?? "" + XCTAssertTrue(why.contains("a CLIENT-side bound, not the strap's"), why) + XCTAssertTrue(report.terminatorFinding.hasPrefix("INCONCLUSIVE"), report.terminatorFinding) + } + + /// A valid entry resets the run, so scattered holes cost nothing against the cap. + func testAValidEntryResetsTheConsecutiveEmptySlotRun() { + var report = FeatureFlagProbeReport(family: .whoop4) + func empty(_ i: Int) -> FeatureFlagProbe.NextResponse { + FeatureFlagProbe.NextResponse(resultCode: 1, revision: 1, index: i, validKey: false, key: nil, + record: [0x01, UInt8(i), 0x00]) + } + func valid(_ i: Int, _ k: String) -> FeatureFlagProbe.NextResponse { + FeatureFlagProbe.NextResponse(resultCode: 1, revision: 1, index: i, validKey: true, key: k, + record: [0x01, UInt8(i), 0x01]) + } + for i in 0..<(FeatureFlagProbe.maxConsecutiveEmptySlots - 1) { + XCTAssertTrue(report.noteNext(empty(i))) + } + XCTAssertTrue(report.noteNext(valid(20, "hr_ch_switching"))) + for i in 21..<(21 + FeatureFlagProbe.maxConsecutiveEmptySlots - 1) { + XCTAssertTrue(report.noteNext(empty(i)), "the run restarted at the valid entry") + } + XCTAssertNil(report.stopCode) + XCTAssertEqual(report.keys, ["hr_ch_switching"]) + } + func testNonPrintableNameIsNotReportedAsAKey() { let record: [UInt8] = [0x01, 0x02, 0x01, 0xDE, 0xAD, 0xBE, 0xEF] let frame = whoop4Response(cmd: 118, payload: payload(result: 1, record: record)) @@ -198,30 +323,75 @@ final class FeatureFlagProbeTests: XCTestCase { "a dump with holes must describe itself rather than look complete") } - /// `validKey = 0` is trusted as an end marker, but the other reading — an EMPTY SLOT with the list - /// continuing past it — is not ruled out by anything observed so far. Stopping well short of the - /// announced count is the evidence that would settle it, so the report has to say so loudly rather - /// than reporting a confident short list. - func testStoppingOnValidKeyFalseShortOfTheAnnouncedCountIsFlagged() { - var report = FeatureFlagProbeReport(family: .whoop4) - report.noteStart(FeatureFlagProbe.StartResponse(resultCode: 1, revision: 1, count: 40)) + /// The reply a WHOOP 5 MG actually served to end its 115/116 walk: `index = 255` and `validKey = 0` + /// on the SAME frame. Both terminator conditions fired at once, so that run cannot say which one the + /// firmware meant — and the report must say exactly that instead of picking one. + func testEndMarkerCarryingValidKeyZeroIsReportedAsAmbiguous() { + var report = FeatureFlagProbeReport(family: .whoop5, + namespace: FeatureFlagProbe.deviceConfigNamespace) + report.noteStart(FeatureFlagProbe.StartResponse(resultCode: 1, revision: 1, count: 7, + record: [0x01, 0x07, 0x00])) XCTAssertTrue(report.noteNext(FeatureFlagProbe.NextResponse( - resultCode: 1, revision: 1, index: 0, validKey: true, key: "enable_r22_packets"))) + resultCode: 1, revision: 1, index: 1, validKey: true, key: "enable_rfid", + record: [0x01, 0x01, 0x01]))) XCTAssertFalse(report.noteNext(FeatureFlagProbe.NextResponse( - resultCode: 1, revision: 1, index: 1, validKey: false, key: nil))) + resultCode: 1, revision: 1, index: 0xFF, validKey: false, key: nil, + record: [0x01, 0xFF, 0x00, 0x00]))) - let why = try! XCTUnwrap(report.stopReason) - XCTAssertTrue(why.contains("2 of 40 announced entries"), why) - XCTAssertTrue(why.contains("the remainder was NOT walked"), why) + XCTAssertTrue(report.endMarkerAlsoCarriedInvalidFlag) + XCTAssertEqual(report.stopCode, .endMarker) + XCTAssertTrue(report.terminatorFinding.hasPrefix("AMBIGUOUS"), report.terminatorFinding) + XCTAssertTrue(report.terminatorFinding.contains("both terminator conditions fired at once"), + report.terminatorFinding) + XCTAssertTrue(report.render().contains("index=0xFF AND validKey=0 on the same reply")) } - /// …and when the count was fully walked, the plain reason stands — no false alarm. - func testValidKeyFalseAtTheAnnouncedEndIsNotFlagged() { - var report = FeatureFlagProbeReport(family: .whoop4) - report.noteStart(FeatureFlagProbe.StartResponse(resultCode: 1, revision: 1, count: 1)) + /// The 0xFF marker with `validKey` still true separates the two conditions the other way, and the + /// report has to be equally careful there: 0xFF alone terminated, and validKey=0 stayed untested. + func testEndMarkerWithValidKeyTrueSaysValidKeyZeroIsUntested() { + var report = FeatureFlagProbeReport(family: .whoop5) + report.noteStart(FeatureFlagProbe.StartResponse(resultCode: 1, revision: 1, count: 2, + record: [0x01, 0x02, 0x00])) + XCTAssertTrue(report.noteNext(FeatureFlagProbe.NextResponse( + resultCode: 1, revision: 1, index: 1, validKey: true, key: "enable_r22_packets", + record: [0x01, 0x01, 0x01]))) XCTAssertFalse(report.noteNext(FeatureFlagProbe.NextResponse( - resultCode: 1, revision: 1, index: 0, validKey: false, key: nil))) - XCTAssertEqual(report.stopReason, "firmware reported validKey=false") + resultCode: 1, revision: 1, index: 0xFF, validKey: true, key: nil, + record: [0x01, 0xFF, 0x01]))) + XCTAssertFalse(report.endMarkerAlsoCarriedInvalidFlag) + XCTAssertTrue(report.terminatorFinding.contains("validKey=0 was never served, so it is untested"), + report.terminatorFinding) + } + + /// What the 117/118 walk on a 5/MG actually did: sixteen valid, named entries and NO terminator of + /// either kind. The walk used to stop dead at the announced count and read as a complete list. It now + /// asks a bounded number of further times, and — whatever comes back — reports in the verdict itself + /// that a client-side bound ended the run. + func testWalkOvershootsTheAnnouncedCountSoTheStrapCanEndItsOwnList() { + var report = FeatureFlagProbeReport(family: .whoop5) + report.noteStart(FeatureFlagProbe.StartResponse(resultCode: 1, revision: 1, count: 3, + record: [0x01, 0x03, 0x00])) + var sent = 0 + var keepGoing = true + while keepGoing { + sent += 1 + keepGoing = report.noteNext(FeatureFlagProbe.NextResponse( + resultCode: 1, revision: 1, index: sent, validKey: true, key: "key_\(sent)", + record: [0x01, UInt8(sent), 0x01])) + } + XCTAssertEqual(sent, 3 + FeatureFlagProbe.countOvershootAllowance, + "the strap gets \(FeatureFlagProbe.countOvershootAllowance) replies past its own count") + XCTAssertEqual(report.stopCode, .announcedCountOvershoot) + XCTAssertEqual(report.repliesPastAnnouncedCount, FeatureFlagProbe.countOvershootAllowance) + XCTAssertTrue(report.verdict.contains("INCOMPLETE: the walk ended on a client-side bound"), + report.verdict) + XCTAssertTrue(report.terminatorFinding.hasPrefix("NO TERMINATOR OBSERVED"), + report.terminatorFinding) + // …and the count line reports the arithmetic that makes it a finding. + let count = try! XCTUnwrap(report.countFinding) + XCTAssertTrue(count.contains("Keys yielded: 7"), count) + XCTAssertTrue(count.contains("MISMATCH against the announced 3"), count) + XCTAssertTrue(count.contains("kept answering 4 repl(ies) past its own announced count"), count) } /// The verdict must never blame the strap for our own decode. A firmware whose names all fail our @@ -341,10 +511,16 @@ final class FeatureFlagProbeTests: XCTestCase { guard case .success(let n) = FeatureFlagProbe.parseNext(frame: frame, family: .whoop5) else { return XCTFail("next \(i)") } - let keepGoing = report.noteNext(n) - XCTAssertEqual(keepGoing, i < names.count - 1, "the announced count bounds the walk") + XCTAssertTrue(report.noteNext(n), "the announced count no longer ends the walk by itself") + } + // The strap's own end marker — the only thing that ends a walk cleanly. + let end = whoop5Response(cmd: 118, payload: payload(result: 1, record: [0x01, 0xFF, 0x00, 0x00])) + guard case .success(let last) = FeatureFlagProbe.parseNext(frame: end, family: .whoop5) else { + return XCTFail("end marker") } + XCTAssertFalse(report.noteNext(last)) XCTAssertEqual(report.keys, names) + XCTAssertEqual(report.stopCode, .endMarker) let text = report.render() XCTAssertTrue(text.contains("#761 FEATURE-FLAG ENUMERATION PROBE — WHOOP 5/MG")) XCTAssertTrue(text.contains("Read-only")) @@ -352,11 +528,220 @@ final class FeatureFlagProbeTests: XCTestCase { XCTAssertTrue(text.contains("Flags reported by the strap (3 of 3 announced)")) XCTAssertTrue(text.contains(" 1. enable_r22_packets")) XCTAssertTrue(text.contains(" 3. wear_detect_bias")) - XCTAssertTrue(text.contains("walked the 3 entries the strap announced")) + XCTAssertTrue(text.contains("Stop code: endMarker")) XCTAssertTrue(text.contains("START_FF_KEY_EXCHANGE(117) → revision=1 count=3 result=SUCCESS(1)")) XCTAssertTrue(text.contains("SEND_NEXT_FF(118) → index=0 validKey=true key=\"enable_r22_packets\"")) } + // MARK: - Raw bytes + + /// Requirement of the whole exercise: the RAW record bytes of every reply are in the report, not only + /// the fields parsed out of them. Earlier findings here had to be re-run because only parsed output + /// survived, and a parsed field cannot contradict the layout that produced it. + func testEveryReplyLogsItsRawRecordBytes() { + var report = FeatureFlagProbeReport(family: .whoop5) + let start = whoop5Response(cmd: 117, payload: payload(result: 1, record: [0x01, 0x02, 0x00])) + guard case .success(let s) = FeatureFlagProbe.parseStart(frame: start, family: .whoop5) else { + return XCTFail("start") + } + report.noteStart(s) + XCTAssertEqual(s.record, [0x01, 0x02, 0x00], "the parse keeps the bytes it decoded from") + let next = whoop5Response(cmd: 118, payload: payload(result: 1, record: [0x01, 0x00, 0x01, 0x78, 0x00])) + guard case .success(let n) = FeatureFlagProbe.parseNext(frame: next, family: .whoop5) else { + return XCTFail("next") + } + XCTAssertTrue(report.noteNext(n)) + let text = report.render() + XCTAssertTrue(text.contains("raw=01 02 00"), text) + XCTAssertTrue(text.contains("raw=01 00 01 78 00"), text) + } + + /// A reply that failed to decode is the one whose raw bytes matter most, so the whole frame goes in. + func testUndecodedReplyLogsTheWholeRawFrame() { + var report = FeatureFlagProbeReport(family: .whoop4) + report.noteFailure(.crc, command: 118, frame: [0xAA, 0x05, 0x00, 0x2B, 0x24]) + XCTAssertTrue(report.render().contains("raw frame=aa 05 00 2b 24"), report.render()) + XCTAssertEqual(report.stopCode, .parseFailure) + } + + /// The hex is bounded so one absurd record cannot flood the log — but the true LENGTH is always + /// printed, because an over-long record is itself evidence that the layout is wrong. + func testOverLongRawRecordIsElidedButItsLengthIsKept() { + let long = [UInt8](repeating: 0xAB, count: FeatureFlagProbe.maxRawHexBytes + 10) + let rendered = FeatureFlagProbe.hex(long) + XCTAssertTrue(rendered.hasSuffix("… (\(FeatureFlagProbe.maxRawHexBytes + 10) bytes total)"), rendered) + XCTAssertEqual(FeatureFlagProbe.hex([]), "(empty)") + } + + // MARK: - The announced count + + /// The count is read as u16 LE, and every count seen so far has had a zero high byte — where a + /// single-byte read returns the SAME number. So no capture distinguishes the two readings, and the + /// report says that rather than asserting a field width nothing has established. + func testCountCarriesBothReadingsAndSaysWhenTheyAgree() { + let frame = whoop5Response(cmd: 117, payload: payload(result: 1, record: [0x01, 0x10, 0x00])) + guard case .success(let r) = FeatureFlagProbe.parseStart(frame: frame, family: .whoop5) else { + return XCTFail("start") + } + XCTAssertEqual(r.count, 16) + XCTAssertEqual(r.singleByteCount, 16) + XCTAssertEqual(r.countHighByte, 0) + XCTAssertTrue(r.countReadingsAgree) + var report = FeatureFlagProbeReport(family: .whoop5) + report.noteStart(r) + let finding = try! XCTUnwrap(report.countFinding) + XCTAssertTrue(finding.contains("u16 LE read = 16; single-byte read = 16 (high byte 0x00)"), finding) + XCTAssertTrue(finding.contains("the two readings AGREE"), finding) + } + + /// A nonzero high byte is the only thing that separates them, and it is a finding either way: on the + /// u16 reading the list is enormous, on the single-byte reading `record[2]` is padding. + func testNonZeroHighByteIsReportedAsADisagreement() { + let frame = whoop5Response(cmd: 117, payload: payload(result: 1, record: [0x01, 0x07, 0x02])) + guard case .success(let r) = FeatureFlagProbe.parseStart(frame: frame, family: .whoop5) else { + return XCTFail("start") + } + XCTAssertEqual(r.count, 0x0207) + XCTAssertEqual(r.singleByteCount, 7) + XCTAssertFalse(r.countReadingsAgree) + XCTAssertFalse(r.countIsPlausible, "an implausible u16 must not become a loop bound") + var report = FeatureFlagProbeReport(family: .whoop5) + report.noteStart(r) + let finding = try! XCTUnwrap(report.countFinding) + XCTAssertTrue(finding.contains("single-byte read = 7 (high byte 0x02)"), finding) + XCTAssertTrue(finding.contains("the two readings DISAGREE"), finding) + } + + /// Announced-count-versus-yielded is itself a result. A strap that announces sixteen and names + /// fourteen has two entries nobody has accounted for, and the report has to raise it rather than + /// print a tidy short list. + func testAnnouncedCountVersusKeysYieldedMismatchIsReported() { + var report = FeatureFlagProbeReport(family: .whoop5) + report.noteStart(FeatureFlagProbe.StartResponse(resultCode: 1, revision: 1, count: 16, + record: [0x01, 0x10, 0x00])) + XCTAssertTrue(report.noteNext(FeatureFlagProbe.NextResponse( + resultCode: 1, revision: 1, index: 1, validKey: true, key: "enable_r22_packets", + record: [0x01, 0x01, 0x01]))) + XCTAssertFalse(report.noteNext(FeatureFlagProbe.NextResponse( + resultCode: 1, revision: 1, index: 0xFF, validKey: true, key: nil, + record: [0x01, 0xFF, 0x01]))) + let finding = try! XCTUnwrap(report.countFinding) + XCTAssertTrue(finding.contains("Keys yielded: 1"), finding) + XCTAssertTrue(finding.contains("MISMATCH against the announced 16"), finding) + } + + /// …and when everything is accounted for — keys plus skipped plus empty slots — there is no alarm. + func testFullyAccountedCountIsNotFlaggedAsAMismatch() { + var report = FeatureFlagProbeReport(family: .whoop5) + report.noteStart(FeatureFlagProbe.StartResponse(resultCode: 1, revision: 1, count: 3, + record: [0x01, 0x03, 0x00])) + _ = report.noteNext(FeatureFlagProbe.NextResponse(resultCode: 1, revision: 1, index: 1, + validKey: true, key: "a", record: [0x01, 0x01, 0x01])) + _ = report.noteNext(FeatureFlagProbe.NextResponse(resultCode: 1, revision: 1, index: 2, + validKey: true, key: nil, record: [0x01, 0x02, 0x01])) + _ = report.noteNext(FeatureFlagProbe.NextResponse(resultCode: 1, revision: 1, index: 3, + validKey: false, key: nil, record: [0x01, 0x03, 0x00])) + let finding = try! XCTUnwrap(report.countFinding) + XCTAssertFalse(finding.contains("MISMATCH"), finding) + XCTAssertTrue(finding.contains("Keys yielded: 1 (+1 undecodable) (+1 empty slot(s))"), finding) + } + + // MARK: - The device-config namespace (115/116) + + /// The SAME walk drives the device-config pair, so the terminator rules cannot be corrected in one + /// namespace and left wrong in the other. Decoded end-to-end from 115/116 frames. + func testDeviceConfigNamespaceWalksThroughTheSameCorrectedRules() { + var report = FeatureFlagProbeReport(family: .whoop5, + namespace: FeatureFlagProbe.deviceConfigNamespace) + let start = whoop5Response(cmd: 115, payload: payload(result: 1, record: [0x01, 0x02, 0x00])) + guard case .success(let s) = FeatureFlagProbe.parseStart(frame: start, family: .whoop5, + expecting: 115) else { + return XCTFail("115 start") + } + report.noteStart(s) + // An empty slot first — under the old rule the walk would have ended here with zero keys. + let hole = whoop5Response(cmd: 116, payload: payload(result: 1, record: [0x01, 0x02, 0x00, 0x00])) + guard case .success(let h) = FeatureFlagProbe.parseNext(frame: hole, family: .whoop5, + expecting: 116) else { + return XCTFail("116 hole") + } + XCTAssertTrue(report.noteNext(h)) + let named = whoop5Response(cmd: 116, payload: payload( + result: 1, record: [0x01, 0x03, 0x01] + keyBytes("enable_raw_data_w_ecg"))) + guard case .success(let n) = FeatureFlagProbe.parseNext(frame: named, family: .whoop5, + expecting: 116) else { + return XCTFail("116 named") + } + XCTAssertTrue(report.noteNext(n)) + XCTAssertEqual(report.keys, ["enable_raw_data_w_ecg"], + "the key AFTER the empty slot is exactly what the old rule discarded") + + let text = report.render() + XCTAssertTrue(text.contains("#761 DEVICE-CONFIG ENUMERATION PROBE — WHOOP 5/MG"), text) + XCTAssertTrue(text.contains("START_DEVICE_CONFIG_KEY_EXCHANGE(115) + SEND_NEXT_DEVICE_CONFIG(116)"), text) + XCTAssertTrue(text.contains("Keys reported by the strap (1 of 2 announced"), text) + XCTAssertTrue(text.contains("SEND_NEXT_DEVICE_CONFIG(116) → index=3"), text) + } + + /// An explicit refusal is the strap's answer, not one of our bounds, and it must end the walk with its + /// own name — on the START reply (where the driver reads `hasStopped` instead of stepping to 118) and + /// on a NEXT reply alike. + func testUnsupportedRefusalEndsTheWalkWithItsOwnStopCode() { + var start = FeatureFlagProbeReport(family: .whoop5) + XCTAssertFalse(start.hasStopped) + start.noteStart(FeatureFlagProbe.StartResponse(resultCode: 3, revision: 0, count: 0, + record: [0x00, 0x00, 0x00])) + XCTAssertTrue(start.hasStopped, "a refused START must not step on to the next verb") + XCTAssertEqual(start.stopCode, .unsupported) + XCTAssertTrue(start.render().contains("Stop code: unsupported")) + + var mid = FeatureFlagProbeReport(family: .whoop5) + mid.noteStart(FeatureFlagProbe.StartResponse(resultCode: 1, revision: 1, count: 4, + record: [0x01, 0x04, 0x00])) + XCTAssertTrue(mid.noteNext(FeatureFlagProbe.NextResponse( + resultCode: 1, revision: 1, index: 1, validKey: true, key: "enable_r22_packets", + record: [0x01, 0x01, 0x01]))) + XCTAssertFalse(mid.noteNext(FeatureFlagProbe.NextResponse( + resultCode: 3, revision: 0, index: 0, validKey: false, key: nil, record: [0x00, 0x00, 0x00]))) + XCTAssertEqual(mid.stopCode, .unsupported) + XCTAssertEqual(mid.emptySlots, 0, "a refusal is never counted as an empty slot") + XCTAssertTrue(mid.stopReason?.contains("UNSUPPORTED(3)") == true, mid.stopReason ?? "") + } + + /// The two platforms must classify the same reply the same way, or a strap log means different things + /// on either side. These are the exact constants the walk is bounded by; the Kotlin twin + /// (`walkBoundsMatchTheSwiftTwin`) asserts the same numbers and the same stop-code spellings. + func testWalkBoundsMatchTheKotlinTwin() { + XCTAssertEqual(FeatureFlagProbe.maxConsecutiveEmptySlots, 8) + XCTAssertEqual(FeatureFlagProbe.countOvershootAllowance, 4) + XCTAssertEqual(FeatureFlagProbe.maxRawHexBytes, 64) + XCTAssertEqual(FeatureFlagProbe.maxFlags, 128) + XCTAssertEqual(FeatureFlagProbe.StopCode.endMarker.rawValue, "endMarker") + XCTAssertEqual(FeatureFlagProbe.StopCode.emptySlotCursorParked.rawValue, "emptySlotCursorParked") + XCTAssertEqual(FeatureFlagProbe.StopCode.emptySlotRunCap.rawValue, "emptySlotRunCap") + XCTAssertEqual(FeatureFlagProbe.StopCode.stepCap.rawValue, "stepCap") + XCTAssertEqual(FeatureFlagProbe.StopCode.announcedCountOvershoot.rawValue, "announcedCountOvershoot") + XCTAssertEqual(FeatureFlagProbe.StopCode.timeout.rawValue, "timeout") + XCTAssertEqual(FeatureFlagProbe.StopCode.unsupported.rawValue, "unsupported") + XCTAssertEqual(FeatureFlagProbe.StopCode.parseFailure.rawValue, "parseFailure") + } + + /// The 115/116 frames must not decode as 117/118 by accident — the opcode is checked, both ways. + func testTheTwoNamespacesDoNotDecodeEachOthersFrames() { + let cfg = whoop5Response(cmd: 116, payload: payload(result: 1, record: [0x01, 0x00, 0x01])) + XCTAssertEqual(FeatureFlagProbe.parseNext(frame: cfg, family: .whoop5), .failure(.wrongCommand)) + let ff = whoop5Response(cmd: 118, payload: payload(result: 1, record: [0x01, 0x00, 0x01])) + XCTAssertEqual(FeatureFlagProbe.parseNext(frame: ff, family: .whoop5, expecting: 116), + .failure(.wrongCommand)) + XCTAssertEqual(FeatureFlagProbe.deviceConfigNamespace.startCmd, 115) + XCTAssertEqual(FeatureFlagProbe.deviceConfigNamespace.nextCmd, 116) + // Still read-only: the SET verbs appear in neither namespace. + for ns in [FeatureFlagProbe.featureFlagNamespace, FeatureFlagProbe.deviceConfigNamespace] { + XCTAssertFalse([119, 120].contains(Int(ns.startCmd))) + XCTAssertFalse([119, 120].contains(Int(ns.nextCmd))) + } + } + func testRepeatedKeysCannotDriveAnUnboundedWalk() { // A firmware whose cursor never advances repeats one entry: the reply-count cap must stop it. var report = FeatureFlagProbeReport(family: .whoop4) @@ -417,14 +802,17 @@ final class FeatureFlagProbeTests: XCTestCase { Verdict: enumerated 1 feature-flag key name(s) Stopped: cursor exhausted (index 0xFF) + Stop code: endMarker + Terminator: AMBIGUOUS — the walk ended on a reply carrying index=0xFF AND validKey=0 together, so both terminator conditions fired at once and this run cannot say which one the firmware meant. + Announced count: u16 LE read = 2; single-byte read = 2 (high byte 0x00) — the two readings AGREE, so this reply does not establish the field's width. Keys yielded: 1 — MISMATCH against the announced 2 Flags reported by the strap (1 of 2 announced): 1. enable_r22_packets Exchange: - START_FF_KEY_EXCHANGE(117) → revision=1 count=2 result=SUCCESS(1) - SEND_NEXT_FF(118) → index=0 validKey=true key="enable_r22_packets" result=SUCCESS(1) - SEND_NEXT_FF(118) → index=255 validKey=false result=SUCCESS(1) + START_FF_KEY_EXCHANGE(117) → revision=1 count=2 result=SUCCESS(1) raw=01 02 00 + SEND_NEXT_FF(118) → index=0 validKey=true key="enable_r22_packets" result=SUCCESS(1) raw=01 00 01 65 6e 61 62 6c 65 5f 72 32 32 5f 70 61 63 6b 65 74 73 00 00 00 00 00 00 00 00 00 00 + SEND_NEXT_FF(118) → index=255 validKey=false result=SUCCESS(1) raw=01 ff 00 00 00 00 00 (index=0xFF AND validKey=0 on the same reply — both terminator conditions at once) """ XCTAssertEqual(report.render(), golden) diff --git a/Strand/BLE/BLEManager.swift b/Strand/BLE/BLEManager.swift index aaad54f415..df03d82cff 100644 --- a/Strand/BLE/BLEManager.swift +++ b/Strand/BLE/BLEManager.swift @@ -2822,9 +2822,18 @@ public final class BLEManager: NSObject, ObservableObject { switch FeatureFlagProbe.parseStart(frame: frame, family: family) { case .success(let r): featureFlagReport?.noteStart(r) - sendFeatureFlagStep(.sendNextFeatureFlag) + // A firmware that REFUSED 117 has nothing to enumerate; sending 118 anyway would spend a + // round-trip to learn what the refusal already said. `hasStopped` is the report's own + // named reason, so the driver never has to re-derive one. + if featureFlagReport?.hasStopped == true { + finishFeatureFlagProbe() + } else { + sendFeatureFlagStep(.sendNextFeatureFlag) + } case .failure(let f): - featureFlagReport?.noteFailure(f, command: Int(awaiting)) + // Pass the frame: a reply that failed to decode is the one whose RAW bytes matter most, + // and a report that kept only the parsed fields cannot be re-examined later. + featureFlagReport?.noteFailure(f, command: Int(awaiting), frame: frame) finishFeatureFlagProbe() } return 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 be009f1544..deb92ba83b 100644 --- a/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt +++ b/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt @@ -3489,9 +3489,18 @@ class WhoopBleClient( val start = parsed.value if (start != null) { featureFlagReport?.noteStart(start) - sendFeatureFlagStep(CommandNumber.SEND_NEXT_FF) + // A firmware that REFUSED 117 has nothing to enumerate; sending 118 anyway would spend a + // round-trip to learn what the refusal already said. hasStopped is the report's own named + // reason, so the driver never has to re-derive one. + if (featureFlagReport?.hasStopped == true) { + finishFeatureFlagProbe() + } else { + sendFeatureFlagStep(CommandNumber.SEND_NEXT_FF) + } } else { - featureFlagReport?.noteFailure(parsed.failure!!, awaiting) + // Pass the frame: a reply that failed to decode is the one whose RAW bytes matter most, + // and a report that kept only the parsed fields cannot be re-examined later. + featureFlagReport?.noteFailure(parsed.failure!!, awaiting, frame) finishFeatureFlagProbe() } return diff --git a/android/app/src/main/java/com/noop/protocol/FeatureFlagProbe.kt b/android/app/src/main/java/com/noop/protocol/FeatureFlagProbe.kt index 93eef6ded4..21f138ddab 100644 --- a/android/app/src/main/java/com/noop/protocol/FeatureFlagProbe.kt +++ b/android/app/src/main/java/com/noop/protocol/FeatureFlagProbe.kt @@ -63,6 +63,113 @@ object FeatureFlagProbe { /** Longest key name accepted from a reply (the SET side NUL-pads names to 32 bytes). */ const val MAX_KEY_LENGTH = 32 + /** START_DEVICE_CONFIG_KEY_EXCHANGE (115 / 0x73) — the device-config twin of 117. Read-only. */ + const val START_DEVICE_CONFIG_KEY_EXCHANGE_CMD = 115 + + /** SEND_NEXT_DEVICE_CONFIG (116 / 0x74) — the device-config twin of 118. Read-only. */ + const val SEND_NEXT_DEVICE_CONFIG_CMD = 116 + + /** + * How many consecutive `validKey = 0` replies the walk steps over before it gives up on the + * EMPTY-SLOT reading (see [NextResponse.isEmptySlot]). + * + * Calibrated on the one 117/118 walk this project has: that strap announced 16 keys and served + * indices **1–10 then 13–18** — a two-slot hole it handled itself, without ever sending + * `validKey = 0`. A firmware that instead exposes its holes would show them as runs of that size, so + * eight is four times the largest hole ever observed and still a hard bound. + */ + const val MAX_CONSECUTIVE_EMPTY_SLOTS = 8 + + /** + * Extra SEND_NEXT replies the walk takes AFTER the strap's announced count is satisfied, so the + * strap — not our arithmetic — gets to end its own list. The 117/118 walk stopped at exactly + * `steps == count`, having seen neither `index = 0xFF` nor `validKey = 0`; the same strap served + * indices up to 18 while announcing 16, and four is twice that observed excess. + */ + const val COUNT_OVERSHOOT_ALLOWANCE = 4 + + /** + * Ceiling on record bytes rendered as hex in one trace line. The byte COUNT is printed even when the + * tail is elided, so a suspiciously long record still reads as one. + */ + const val MAX_RAW_HEX_BYTES = 64 + + /** + * One enumerate pair — the opcodes the walk drives and the words the report prints for them. + * + * The protocol's own `CommandNumber` table names two symmetric enumerate pairs (117/118 for feature + * flags, 115/116 for device config). Both are walked by the SAME code here, so a fix to the walk's + * terminator handling cannot apply to one namespace and miss the other. Reusing this decoder for + * 115/116 ASSUMES the two share a record layout — an inference from that table's naming symmetry, + * not an observation. It fails closed. + */ + data class Namespace( + val startCmd: Int, + val nextCmd: Int, + val startLabel: String, + val nextLabel: String, + /** Report title, e.g. `FEATURE-FLAG ENUMERATION PROBE`. */ + val title: String, + /** What the namespace is called in prose, e.g. `feature-flag`. */ + val noun: String, + /** What ONE entry is called in prose, e.g. `flag`. */ + val entryNoun: String, + /** Heading for the collected list, e.g. `Flags`. */ + val listHeading: String, + ) + + /** 117/118 — the feature-flag key list. */ + val FEATURE_FLAG_NAMESPACE = Namespace( + startCmd = START_KEY_EXCHANGE_CMD, nextCmd = SEND_NEXT_FLAG_CMD, + startLabel = "START_FF_KEY_EXCHANGE", nextLabel = "SEND_NEXT_FF", + title = "FEATURE-FLAG ENUMERATION PROBE", noun = "feature-flag", entryNoun = "flag", + listHeading = "Flags", + ) + + /** 115/116 — the device-config key list. */ + val DEVICE_CONFIG_NAMESPACE = Namespace( + startCmd = START_DEVICE_CONFIG_KEY_EXCHANGE_CMD, nextCmd = SEND_NEXT_DEVICE_CONFIG_CMD, + startLabel = "START_DEVICE_CONFIG_KEY_EXCHANGE", nextLabel = "SEND_NEXT_DEVICE_CONFIG", + title = "DEVICE-CONFIG ENUMERATION PROBE", noun = "device-config", entryNoun = "config key", + listHeading = "Keys", + ) + + /** + * Why a walk stopped. A STRING reason reads well in a log and a CODE can be asserted on, and both are + * always set together: a truncated walk that reports no reason at all is precisely how a partial key + * list gets mistaken for a complete one. [code] matches the Swift `StopCode` raw values exactly. + */ + enum class StopCode(val code: String) { + /** The strap served `index = 0xFF`. The one terminator this project has seen unambiguously. */ + END_MARKER("endMarker"), + + /** + * A `validKey = 0` reply repeated the previous reply's index: the cursor did not advance, so + * this walk cannot reach anything beyond that point. Note what this does NOT establish — a + * firmware whose cursor parks on an empty slot emits the identical frame, so "the list ends + * here" and "the walk is stuck on a hole" are not separable by this observation. + */ + EMPTY_SLOT_CURSOR_PARKED("emptySlotCursorParked"), + + /** [MAX_CONSECUTIVE_EMPTY_SLOTS] consecutive `validKey = 0` replies, index still advancing. */ + EMPTY_SLOT_RUN_CAP("emptySlotRunCap"), + + /** [MAX_FLAGS] replies. A client-side bound: says nothing about where the strap's list ends. */ + STEP_CAP("stepCap"), + + /** The announced count plus [COUNT_OVERSHOOT_ALLOWANCE] was spent. Also a client-side bound. */ + ANNOUNCED_COUNT_OVERSHOOT("announcedCountOvershoot"), + + /** The strap answered nothing inside the probe's window. */ + TIMEOUT("timeout"), + + /** The strap refused the verb (result `UNSUPPORTED`). */ + UNSUPPORTED("unsupported"), + + /** A reply did not decode. Our limitation, and labelled as ours. */ + PARSE_FAILURE("parseFailure"), + } + /** COMMAND_RESPONSE packet type (36 / 0x24). */ private const val COMMAND_RESPONSE_TYPE = 36 @@ -73,9 +180,39 @@ object FeatureFlagProbe { enum class ParseFailure { CRC, ENVELOPE, WRONG_COMMAND, TRUNCATED } /** Decoded `START_FF_KEY_EXCHANGE` reply. */ - data class StartResponse(val resultCode: Int?, val revision: Int, val count: Int) { + data class StartResponse( + val resultCode: Int?, + val revision: Int, + /** + * `numberOfFeatureFlags` read as u16 LE (`record[1] | record[2] shl 8`). NOT trusted as a loop + * bound on its own, and NOT the only defensible reading of those bytes — see [singleByteCount]. + */ + val count: Int, + /** + * The raw record bytes this response was decoded from, kept so the report can print them. + * Parsed fields are a claim about the layout; these bytes are the evidence for it. + */ + val record: List = emptyList(), + ) { /** True when the count is inside the range a real key list could plausibly occupy. */ val countIsPlausible: Boolean get() = count > 0 && count <= MAX_FLAGS + + /** + * The same field read as a SINGLE byte — i.e. on the reading where `record[2]` is padding rather + * than the count's high byte. + * + * The layout this decoder implements calls the field u16 LE. Nothing observed here settles that: + * every count seen so far has had `record[2] == 0x00`, where a u16 read and a single-byte read + * return the SAME number. Both are therefore carried, and the report says plainly when they + * agree (no evidence either way) and when they differ (which is itself the finding). + */ + val singleByteCount: Int? get() = if (record.size >= 2) record[1].toInt() and 0xFF else null + + /** `record[2]` — the byte a u16 reading treats as the count's high half. */ + val countHighByte: Int? get() = if (record.size >= 3) record[2].toInt() and 0xFF else null + + /** True when the two readings return the same number, i.e. this reply is silent on the width. */ + val countReadingsAgree: Boolean get() = countHighByte == 0 } /** Decoded `SEND_NEXT_FF` reply. */ @@ -86,18 +223,39 @@ object FeatureFlagProbe { val index: Int, val validKey: Boolean, val key: String?, + /** The raw record bytes this response was decoded from, kept so the report can print them. */ + val record: List = emptyList(), ) { /** - * The STRAP said stop: it returned the 0xFF end marker, or flagged the entry as not a real key. - * Both are the firmware's own signal, so both are trusted. + * The strap's UNAMBIGUOUS end marker: `index = 0xFF`. + * + * This used to also mean `validKey = 0`, on the reading that the firmware sets that flag to + * signal the end of its list. That reading is not established. This file's own header note says + * the record layout is UNVERIFIED on 5/MG — it was derived from a WHOOP 4.0 with an R19-era key + * list — and the terminator semantics came with the layout. + * + * What a 5/MG (WS50_r03) actually served, on the only walks this project has: * - * Deliberately does NOT include `key == null`. That is OUR parser declining a name — a byte - * outside printable ASCII, or one longer than [MAX_KEY_LENGTH] — and it says nothing about - * whether the strap has more entries. Treating it as a terminator meant one undecodable name - * threw away every entry after it: a list of forty with a bad byte at seven yielded six keys. - * See [isSkippable]. + * - 117/118: sixteen replies, every one `validKey = 1` with a decodable name, indices 1–10 then + * 13–18. The strap handled its own two-slot hole internally. Neither `validKey = 0` nor + * `index = 0xFF` was ever served — the walk ended because the client stopped asking at the + * announced count. + * - 115/116: the final reply carried `index = 255` AND `validKey = 0` together. Both conditions + * fired on the same frame, so that run cannot say which one the firmware meant. + * + * So on this hardware the disjunction has never been separated. If `validKey = 0` in fact marks + * an EMPTY or RETIRED SLOT with the list continuing past it, then stopping there truncates — the + * identical failure [isSkippable] exists to prevent, one condition over. [isEmptySlot] is that + * case, and the walk now steps over it and records what comes next. + */ + val isExhausted: Boolean get() = index == 0xFF + + /** + * `validKey = 0` WITHOUT the 0xFF end marker — the ambiguous case, and the one this probe exists + * to resolve. Recorded, stepped over, and bounded by [MAX_CONSECUTIVE_EMPTY_SLOTS]; what the + * strap serves next is the evidence that separates "end of list" from "empty slot". */ - val isExhausted: Boolean get() = !validKey || index == 0xFF + val isEmptySlot: Boolean get() = !validKey && index != 0xFF /** * The firmware calls this a real key but the name did not decode. Record it and KEEP WALKING. @@ -113,19 +271,40 @@ object FeatureFlagProbe { /** One decode outcome: exactly one of [value] / [failure] is non-null. */ data class Parsed(val value: T?, val failure: ParseFailure?) - /** Decode a `START_FF_KEY_EXCHANGE` COMMAND_RESPONSE. CRC-gated. */ - fun parseStart(frame: ByteArray, family: DeviceFamily): Parsed { - val e = extract(frame, family, START_KEY_EXCHANGE_CMD) + /** + * Decode a `START_FF_KEY_EXCHANGE` COMMAND_RESPONSE. CRC-gated. + * + * [expecting] defaults to 117 and exists so the DEVICE-CONFIG twin + * `START_DEVICE_CONFIG_KEY_EXCHANGE` (115) is decoded by this same code — the reuse contract + * documented on [Namespace]. + */ + fun parseStart( + frame: ByteArray, + family: DeviceFamily, + expecting: Int = START_KEY_EXCHANGE_CMD, + ): Parsed { + val e = extract(frame, family, expecting) e.failure?.let { return Parsed(null, it) } val r = e.value!! if (r.record.size < 3) return Parsed(null, ParseFailure.TRUNCATED) val count = (r.record[1].toInt() and 0xFF) or ((r.record[2].toInt() and 0xFF) shl 8) - return Parsed(StartResponse(r.resultCode, r.record[0].toInt() and 0xFF, count), null) + return Parsed( + StartResponse(r.resultCode, r.record[0].toInt() and 0xFF, count, r.record.toList()), + null, + ) } - /** Decode a `SEND_NEXT_FF` COMMAND_RESPONSE. CRC-gated like [parseStart]. */ - fun parseNext(frame: ByteArray, family: DeviceFamily): Parsed { - val e = extract(frame, family, SEND_NEXT_FLAG_CMD) + /** + * Decode a `SEND_NEXT_FF` COMMAND_RESPONSE. CRC-gated like [parseStart]. [expecting] defaults to 118 + * and carries the same reuse contract: 116 (`SEND_NEXT_DEVICE_CONFIG`) walks the device-config + * namespace through this decoder. + */ + fun parseNext( + frame: ByteArray, + family: DeviceFamily, + expecting: Int = SEND_NEXT_FLAG_CMD, + ): Parsed { + val e = extract(frame, family, expecting) e.failure?.let { return Parsed(null, it) } val r = e.value!! // revision + index are the minimum: the 0xFF end marker arrives with nothing after it. @@ -134,7 +313,7 @@ object FeatureFlagProbe { val index = r.record[1].toInt() and 0xFF val validKey = if (r.record.size >= 3) r.record[2].toInt() != 0 else false val key = if (r.record.size >= 4) asciiKey(r.record.copyOfRange(3, r.record.size)) else null - return Parsed(NextResponse(r.resultCode, revision, index, validKey, key), null) + return Parsed(NextResponse(r.resultCode, revision, index, validKey, key, r.record.toList()), null) } private class Extracted(val record: ByteArray, val resultCode: Int?) @@ -177,6 +356,25 @@ object FeatureFlagProbe { return if (out.isEmpty()) null else out.toString() } + /** + * Lower-case space-separated hex, for putting the RAW bytes of a reply in the log next to the fields + * decoded from them. + * + * Parsed output alone has repeatedly turned out to be insufficient evidence here: a claim about the + * record layout cannot be re-checked, or contradicted, from the fields that layout produced. Over + * [MAX_RAW_HEX_BYTES] the tail is elided and the true length is printed, so the elision itself is + * visible and a suspiciously long record still reads as one. + */ + fun hex(bytes: List): String { + if (bytes.isEmpty()) return "(empty)" + var out = bytes.take(MAX_RAW_HEX_BYTES).joinToString(" ") { "%02x".format(it.toInt() and 0xFF) } + if (bytes.size > MAX_RAW_HEX_BYTES) out += " … (${bytes.size} bytes total)" + return out + } + + /** [hex] over a raw frame. */ + fun hex(bytes: ByteArray): String = hex(bytes.toList()) + /** 5/MG result-code label, matching the body-location probe's table. */ fun resultLabel(code: Int): String = when (code) { 0 -> "FAILURE" @@ -192,7 +390,14 @@ object FeatureFlagProbe { * the strap log both show. Pure and order-dependent (start → next… → finish). Twin of Swift * `FeatureFlagProbeReport`; [render] is byte-identical across platforms. */ -class FeatureFlagProbeReport(private val family: DeviceFamily) { +class FeatureFlagProbeReport( + private val family: DeviceFamily, + /** + * Which enumerate pair this walk drives — 117/118 or 115/116. The walk logic is identical; only the + * opcodes and the words differ, so neither namespace can drift from the other's terminator rules. + */ + val namespace: FeatureFlagProbe.Namespace = FeatureFlagProbe.FEATURE_FLAG_NAMESPACE, +) { private val _keys = mutableListOf() /** Key names collected, in the order the strap reported them. */ @@ -210,6 +415,14 @@ class FeatureFlagProbeReport(private val family: DeviceFamily) { var reportedCount: Int? = null private set + /** The same count read as a single byte (`record[1]`), when the record carried one. */ + var reportedCountSingleByte: Int? = null + private set + + /** `record[2]` — the byte a u16 reading treats as the count's high half. */ + var reportedCountHighByte: Int? = null + private set + /** Result code of the START reply on 5/MG (null on 4.0, or before it lands). */ var startResult: Int? = null private set @@ -221,89 +434,228 @@ class FeatureFlagProbeReport(private val family: DeviceFamily) { var skipped: Int = 0 private set + /** Replies carrying `validKey = 0` WITHOUT the 0xFF end marker, which the walk stepped over. */ + var emptySlots: Int = 0 + private set + + /** True once any `validKey = 0` reply arrived without the 0xFF marker. */ + var sawEmptySlot: Boolean = false + private set + + /** True once `index = 0xFF` arrived. */ + var sawEndMarker: Boolean = false + private set + + /** + * True when the 0xFF reply ALSO carried `validKey = 0` — i.e. both terminator conditions fired on one + * frame, and that run cannot say which one the firmware meant. This is what a 5/MG served on 115/116. + */ + var endMarkerAlsoCarriedInvalidFlag: Boolean = false + private set + + /** + * Replies the strap served AFTER the first `validKey = 0`. Any value above zero is the decisive + * observation: the list did not end there. + */ + var repliesAfterFirstEmptySlot: Int = 0 + private set + + /** Key names collected after the first `validKey = 0` — keys a walk that stopped there would lose. */ + var keysAfterFirstEmptySlot: Int = 0 + private set + + /** + * Replies after the first `validKey = 0` that the FIRMWARE flagged as real entries, whether or not + * this parser could decode their names. + * + * This — not the decoded-key count — is what makes the empty-slot reading decisive. #874's rule applies + * to the conclusion as much as to the walk: if the strap named an entry past the hole and our ASCII + * filter declined it, the list still continued, and reporting "inconclusive" there would be our + * parser's limitation deciding a question about the firmware. + */ + var validEntriesAfterFirstEmptySlot: Int = 0 + private set + + /** Replies taken past the strap's announced count, under [FeatureFlagProbe.COUNT_OVERSHOOT_ALLOWANCE]. */ + var repliesPastAnnouncedCount: Int = 0 + private set + /** Set once the walk stopped for a reason we can name. */ var stopReason: String? = null private set + /** The same reason as a code, so a caller (or a test) can branch on it rather than on prose. */ + var stopCode: FeatureFlagProbe.StopCode? = null + private set + + /** + * True once the walk has a reason to stop. The BLE driver consults this after EVERY reply, so a + * refusal recorded on the START reply ends the run instead of being stepped over into the next verb. + */ + val hasStopped: Boolean get() = stopCode != null + + /** The index the previous reply carried, for spotting a cursor that stopped advancing. */ + private var lastIndex: Int? = null + + /** Length of the current unbroken run of `validKey = 0` replies, reset by any valid entry. */ + private var consecutiveEmptySlots = 0 + /** Record the START reply. */ fun noteStart(r: FeatureFlagProbe.StartResponse) { reportedCount = r.count + reportedCountSingleByte = r.singleByteCount + reportedCountHighByte = r.countHighByte startResult = r.resultCode - var line = "START_FF_KEY_EXCHANGE(117) → revision=${r.revision} count=${r.count}" + var line = "${namespace.startLabel}(${namespace.startCmd}) → revision=${r.revision} count=${r.count}" r.resultCode?.let { line += " result=${FeatureFlagProbe.resultLabel(it)}($it)" } + line += " raw=${FeatureFlagProbe.hex(r.record)}" if (!r.countIsPlausible) { line += " ⚠︎ count outside 1…${FeatureFlagProbe.MAX_FLAGS} — treated as unknown, the walk " + "still stops on the strap's own end marker" } _trace.add(line) + if (r.resultCode == 3) { + stopReason = "strap refused ${namespace.startLabel}(${namespace.startCmd}) with UNSUPPORTED(3)" + stopCode = FeatureFlagProbe.StopCode.UNSUPPORTED + } } - /** Record one SEND_NEXT reply. Returns true when the walk should continue. */ + /** + * Record one SEND_NEXT reply. Returns true when the walk should continue. + * + * The decision order IS the experiment. `index = 0xFF` ends the walk, because that marker is the one + * thing a strap has served here unambiguously. `validKey = 0` on its own does NOT end it: the walk + * steps over the entry, sends the next-record verb again, and records what comes back — the only way + * to tell "end of list" from "empty slot" apart. Everything past that point is a CLIENT-side bound, + * and each one names itself in [stopCode] so a walk that ended on our arithmetic can never be read as + * a walk the strap ended. + */ fun noteNext(r: FeatureFlagProbe.NextResponse): Boolean { steps += 1 - var line = "SEND_NEXT_FF(118) → index=${r.index} validKey=${r.validKey}" + if (sawEmptySlot) repliesAfterFirstEmptySlot += 1 + var line = "${namespace.nextLabel}(${namespace.nextCmd}) → index=${r.index} validKey=${r.validKey}" r.key?.let { line += " key=\"$it\"" } r.resultCode?.let { line += " result=${FeatureFlagProbe.resultLabel(it)}($it)" } + line += " raw=${FeatureFlagProbe.hex(r.record)}" _trace.add(line) + + // 0. An explicit refusal. UNSUPPORTED(3) says the firmware does not serve this verb, so nothing in + // the record is meaningful and there is nothing to walk. Checked FIRST, before any field is acted + // on, and named as the strap's answer rather than as one of our bounds. + if (r.resultCode == 3) { + stopReason = "strap refused ${namespace.nextLabel}(${namespace.nextCmd}) with UNSUPPORTED(3)" + stopCode = FeatureFlagProbe.StopCode.UNSUPPORTED + return false + } + + // 1. The strap's own unambiguous end marker. Nothing overrides this. if (r.isExhausted) { - if (r.index == 0xFF) { - stopReason = "cursor exhausted (index 0xFF)" - } else { - // `validKey = 0` is documented as an end marker alongside 0xFF, and it is the firmware's - // own flag, so it is trusted here. But nothing observed so far rules out the other - // reading — that it marks an EMPTY SLOT and the list continues past it. If so this stops - // on the first hole, which is the same truncation [isSkippable] exists to prevent, one - // condition over. It cannot be settled without a strap, so instead of guessing we make - // the discrepancy loud: stopping here well short of the announced count is exactly the - // evidence that would settle it. - stopReason = "firmware reported validKey=false" - val count = reportedCount - if (count != null && count > steps) { - stopReason = "firmware reported validKey=false at index ${r.index} after $steps of " + - "$count announced entries — if validKey=0 marks an empty slot rather than the " + - "end of the list, the remainder was NOT walked (see #872 review)" - } + sawEndMarker = true + if (!r.validKey) { + endMarkerAlsoCarriedInvalidFlag = true + _trace[_trace.size - 1] = _trace[_trace.size - 1] + + " (index=0xFF AND validKey=0 on the same reply — both terminator conditions at once)" } + stopReason = "cursor exhausted (index 0xFF)" + stopCode = FeatureFlagProbe.StopCode.END_MARKER return false } - if (r.isSkippable) { - // Our decode declined the name; the strap still says the entry is real and may have more after - // it. Count it so a partial dump describes itself instead of looking complete. - skipped += 1 + + // 2. `validKey = 0` with no end marker: the ambiguous case. Step over it and keep asking. + if (r.isEmptySlot) { + val parked = lastIndex == r.index + lastIndex = r.index + emptySlots += 1 + consecutiveEmptySlots += 1 + val runLength = consecutiveEmptySlots + sawEmptySlot = true _trace[_trace.size - 1] = _trace[_trace.size - 1] + - " (name did not decode — skipped, walk continues)" + " (validKey=0 without the 0xFF marker — treated as an EMPTY SLOT, walk continues)" + if (parked) { + // The cursor did not move. A firmware that means "end of list" parks there and repeats + // itself, so this is the observation that settles the ambiguity the other way — and it + // costs two round-trips rather than a full cap's worth. + stopReason = "validKey=0 repeated at index ${r.index} without advancing — the cursor is " + + "parked, so on this firmware validKey=0 IS a terminator" + stopCode = FeatureFlagProbe.StopCode.EMPTY_SLOT_CURSOR_PARKED + return false + } + if (runLength >= FeatureFlagProbe.MAX_CONSECUTIVE_EMPTY_SLOTS) { + stopReason = "$runLength consecutive validKey=0 replies (cap " + + "${FeatureFlagProbe.MAX_CONSECUTIVE_EMPTY_SLOTS}) — a CLIENT-side bound, not the strap's" + stopCode = FeatureFlagProbe.StopCode.EMPTY_SLOT_RUN_CAP + return false + } + } else { + lastIndex = r.index + consecutiveEmptySlots = 0 + if (sawEmptySlot) validEntriesAfterFirstEmptySlot += 1 + if (r.isSkippable) { + // Our decode declined the name; the strap still says the entry is real and may have more + // after it. Count it so a partial dump describes itself instead of looking complete. + skipped += 1 + _trace[_trace.size - 1] = _trace[_trace.size - 1] + + " (name did not decode — skipped, walk continues)" + } + r.key?.let { + if (!_keys.contains(it)) { + _keys.add(it) + if (sawEmptySlot) keysAfterFirstEmptySlot += 1 + } + } } - r.key?.let { if (!_keys.contains(it)) _keys.add(it) } - // Bound the walk on REPLIES, not on distinct keys: a firmware whose cursor never advances would - // repeat one name forever, and a key-count bound would never stop writing 118 to the strap. + + // 3. Bound the walk on REPLIES, not on distinct keys: a firmware whose cursor never advances would + // repeat one name forever, and a key-count bound would never stop writing the verb to the strap. if (steps >= FeatureFlagProbe.MAX_FLAGS) { stopReason = "safety cap of ${FeatureFlagProbe.MAX_FLAGS} replies reached" + stopCode = FeatureFlagProbe.StopCode.STEP_CAP return false } + // 4. The announced count, PLUS an overshoot, so the strap gets to end its own list. On the one + // 117/118 walk this project has, `steps >= count` was the only thing that stopped it — no marker, + // no validKey=0 — which means that list was never observed to end at all. val count = reportedCount - if (count != null && count > 0 && count <= FeatureFlagProbe.MAX_FLAGS && steps >= count) { - stopReason = "walked the $count entries the strap announced" - return false + if (count != null && count > 0 && count <= FeatureFlagProbe.MAX_FLAGS) { + if (steps > count) repliesPastAnnouncedCount = steps - count + if (steps >= count + FeatureFlagProbe.COUNT_OVERSHOOT_ALLOWANCE) { + stopReason = "walked the $count entries the strap announced plus " + + "${FeatureFlagProbe.COUNT_OVERSHOOT_ALLOWANCE} more — a CLIENT-side bound; the strap " + + "never sent an end marker, so the list is not known to end here" + stopCode = FeatureFlagProbe.StopCode.ANNOUNCED_COUNT_OVERSHOOT + return false + } } return true } - /** Record a reply that could not be decoded. */ - fun noteFailure(f: FeatureFlagProbe.ParseFailure, command: Int) { + /** + * Record a reply that could not be decoded. [frame], when supplied, is logged in full: a frame that + * failed its CRC is still the only evidence of what the strap actually put on the wire. + */ + fun noteFailure(f: FeatureFlagProbe.ParseFailure, command: Int, frame: ByteArray = ByteArray(0)) { val why = when (f) { FeatureFlagProbe.ParseFailure.CRC -> "CRC failed — frame rejected (never decoded)" FeatureFlagProbe.ParseFailure.ENVELOPE -> "not a COMMAND_RESPONSE envelope" FeatureFlagProbe.ParseFailure.WRONG_COMMAND -> "COMMAND_RESPONSE for a different command" FeatureFlagProbe.ParseFailure.TRUNCATED -> "record too short for the documented layout" } - _trace.add("cmd $command reply not decoded: $why") - if (stopReason == null) stopReason = why + var line = "cmd $command reply not decoded: $why" + if (frame.isNotEmpty()) line += " raw frame=${FeatureFlagProbe.hex(frame)}" + _trace.add(line) + if (stopReason == null) { + stopReason = why + stopCode = FeatureFlagProbe.StopCode.PARSE_FAILURE + } } /** Record the strap answering nothing at all within the probe's window. */ fun noteTimeout(command: Int, seconds: Int) { _trace.add("no COMMAND_RESPONSE for opcode $command within ${seconds}s") - if (stopReason == null) stopReason = "strap served no reply to opcode $command within ${seconds}s" + if (stopReason == null) { + stopReason = "strap served no reply to opcode $command within ${seconds}s" + stopCode = FeatureFlagProbe.StopCode.TIMEOUT + } } /** @@ -317,14 +669,89 @@ class FeatureFlagProbeReport(private val family: DeviceFamily) { get() { val n = reportedCount ?: 0 val plausible = n > 0 && n <= FeatureFlagProbe.MAX_FLAGS - return if (plausible) "$n flag(s)" else "an implausible $n flag(s)" + return if (plausible) "$n ${namespace.entryNoun}(s)" + else "an implausible $n ${namespace.entryNoun}(s)" + } + + /** + * What this run established about the two terminator conditions — the question the probe exists to + * answer, stated in the report rather than left for a reader to reconstruct from the trace. + */ + val terminatorFinding: String + get() { + if (sawEmptySlot && (validEntriesAfterFirstEmptySlot > 0 || sawEndMarker)) { + var s = "DECISIVE — validKey=0 is an EMPTY/RETIRED SLOT on this firmware, not the end of " + + "the list: the strap served $repliesAfterFirstEmptySlot further repl(ies) after the " + + "first validKey=0" + if (validEntriesAfterFirstEmptySlot > 0) { + s += ", $validEntriesAfterFirstEmptySlot of them flagged validKey=1" + } + if (keysAfterFirstEmptySlot > 0) s += ", naming $keysAfterFirstEmptySlot more key(s)" + if (sawEndMarker) s += ", and ended on the index=0xFF marker" + s += ". A walk that stopped on validKey=0 would have been truncated here." + return s + } + if (stopCode == FeatureFlagProbe.StopCode.EMPTY_SLOT_CURSOR_PARKED) { + // The DECISIVE label is kept, narrowed to what the run actually decides. "There is nothing + // past it" is a step beyond the observation, and TWO firmwares emit this identical frame: + // a list that genuinely ends at a parked sentinel, and a cursor that advances only on a + // valid record — where validKey=0 is a SLOT, the walk is stuck on a hole, and keys may sit + // behind it. The second is precisely the reading this probe exists to make testable. + return "DECISIVE — validKey=0 is a TERMINATOR on this firmware: the next request returned " + + "the same index with validKey=0 again, so the cursor does not advance past it, so " + + "this walk cannot see anything beyond. Whether the list truly ends here is not " + + "separable from a firmware whose cursor parks on an empty slot." + } + if (sawEmptySlot) { + return "INCONCLUSIVE — validKey=0 was served and the walk continued past it, but a " + + "client-side bound ended the run before the strap did. Nothing here separates 'end of " + + "list' from 'empty slot'." + } + if (sawEndMarker && endMarkerAlsoCarriedInvalidFlag) { + return "AMBIGUOUS — the walk ended on a reply carrying index=0xFF AND validKey=0 " + + "together, so both terminator conditions fired at once and this run cannot say which " + + "one the firmware meant." + } + if (sawEndMarker) { + return "index=0xFF ended the walk with validKey still true on the same reply, so the 0xFF " + + "marker alone terminates. validKey=0 was never served, so it is untested here." + } + return "NO TERMINATOR OBSERVED — neither validKey=0 nor index=0xFF was served in $steps " + + "repl(ies); the walk ended on a CLIENT-side bound, so the strap's list is not known to " + + "end where this report stops." + } + + /** What the announced count actually says, including the reading it does not settle. */ + val countFinding: String? + get() { + val count = reportedCount ?: return null + var s = "u16 LE read = $count" + val single = reportedCountSingleByte + val high = reportedCountHighByte + if (single != null && high != null) { + s += "; single-byte read = $single (high byte 0x%02x".format(high) + ")" + s += if (high == 0) { + " — the two readings AGREE, so this reply does not establish the field's width" + } else { + " — the two readings DISAGREE; the walk uses the u16 value and plausibility-checks it" + } + } + s += ". Keys yielded: ${_keys.size}" + if (skipped > 0) s += " (+$skipped undecodable)" + if (emptySlots > 0) s += " (+$emptySlots empty slot(s))" + if (_keys.size + skipped + emptySlots != count) s += " — MISMATCH against the announced $count" + if (repliesPastAnnouncedCount > 0) { + s += ". The strap kept answering $repliesPastAnnouncedCount repl(ies) past its own " + + "announced count." + } + return s } /** One-line summary of what the probe established. */ val verdict: String get() { if (startResult == 3) { - return "opcode 117 REJECTED by firmware (UNSUPPORTED) — this strap does not serve the enumerate verb" + return "opcode ${namespace.startCmd} REJECTED by firmware (UNSUPPORTED) — this strap does not serve the enumerate verb" } if (_keys.isEmpty() && reportedCount == null) { return "no usable reply — the enumerate path is unconfirmed on this firmware" @@ -334,40 +761,57 @@ class FeatureFlagProbeReport(private val family: DeviceFamily) { // a reader would carry into #103. Same class as [FeatureFlagProbe.NextResponse.isSkippable]: // never report our limitation as the strap's behaviour. if (_keys.isEmpty() && skipped > 0) { - return "strap named $skipped flag(s), none of which decoded as printable ASCII within " + + return "strap named $skipped ${namespace.entryNoun}(s), none of which decoded as printable ASCII within " + "${FeatureFlagProbe.MAX_KEY_LENGTH} chars — this is our parser rejecting them, NOT " + "the strap serving blanks; see the trace for the raw replies" } // Same discipline one condition over: with no 118 reply decoded, the walk never asked for a // single name, so "named none" would blame the strap for OUR timeout (or our parse failure). // [steps] counts decoded SEND_NEXT replies, so `steps == 0` is exactly "the key list was - // never read" — the reachable case being `probeFeatureFlags()` getting its 117 answer and - // then the 8s timer firing on the first 118. What the strap would have named is unknown, and - // the report has to say unknown. [stopReason], rendered directly under this line, names - // which of the two it was. + // never read" — the reachable case being the probe getting its START answer and then the 8s + // timer firing on the first SEND_NEXT. What the strap would have named is unknown, and the + // report has to say unknown. [stopReason], rendered directly under this line, names which of + // the two it was. (#913, kept through this PR's rebase: the opcode names are now taken from + // [namespace] so a 115/116 walk does not report itself as SEND_NEXT_FF(118).) if (_keys.isEmpty() && steps == 0) { - return "strap announced $announcedFlags; no SEND_NEXT_FF(118) reply was decoded — " + - "the key list was never read (inconclusive)" + return "strap announced $announcedFlags; no ${namespace.nextLabel}(${namespace.nextCmd}) " + + "reply was decoded — the key list was never read (inconclusive)" + } + if (_keys.isEmpty()) { + // Both halves survive the merge with #913: its doubt about an implausible announced count + // ([announcedFlags]), and this PR's namespace-aware noun. + return "strap announced $announcedFlags but named none" } - if (_keys.isEmpty()) return "strap announced $announcedFlags but named none" - if (skipped > 0) { - return "enumerated ${_keys.size} feature-flag key name(s); $skipped further name(s) did not decode" + var s = "enumerated ${_keys.size} ${namespace.noun} key name(s)" + if (skipped > 0) s += "; $skipped further name(s) did not decode" + // A walk that ended on OUR bound is not a complete list, and the one-line summary is the line + // that gets pasted into an issue. Say it here, not only four lines further down. + if (stopCode == FeatureFlagProbe.StopCode.STEP_CAP || + stopCode == FeatureFlagProbe.StopCode.ANNOUNCED_COUNT_OVERSHOOT || + stopCode == FeatureFlagProbe.StopCode.EMPTY_SLOT_RUN_CAP + ) { + s += " — INCOMPLETE: the walk ended on a client-side bound, not on the strap's own end marker" } - return "enumerated ${_keys.size} feature-flag key name(s)" + return s } /** The full copyable report (byte-identical to the Swift `render()`). */ fun render(): String { val fam = if (family == DeviceFamily.WHOOP5) "WHOOP 5/MG" else "WHOOP 4.0" val sb = StringBuilder() - sb.append("#761 FEATURE-FLAG ENUMERATION PROBE — $fam\n") - sb.append("Read-only: START_FF_KEY_EXCHANGE(117) + SEND_NEXT_FF(118). No value is written; ") + sb.append("#761 ${namespace.title} — $fam\n") + sb.append("Read-only: ${namespace.startLabel}(${namespace.startCmd}) + ") + sb.append("${namespace.nextLabel}(${namespace.nextCmd}). No value is written; ") sb.append("SET_FF_VALUE(120) and SET_DEVICE_CONFIG_VALUE(119) are never sent from this path.\n") sb.append("\nVerdict: $verdict\n") stopReason?.let { sb.append("Stopped: $it\n") } - sb.append("\nFlags reported by the strap (${_keys.size}") + stopCode?.let { sb.append("Stop code: ${it.code}\n") } + sb.append("Terminator: $terminatorFinding\n") + countFinding?.let { sb.append("Announced count: $it\n") } + sb.append("\n${namespace.listHeading} reported by the strap (${_keys.size}") reportedCount?.let { sb.append(" of $it announced") } if (skipped > 0) sb.append(", $skipped name(s) did not decode and were skipped") + if (emptySlots > 0) sb.append(", $emptySlots validKey=0 slot(s) stepped over") sb.append("):\n") if (_keys.isEmpty()) { sb.append(" (none)\n") diff --git a/android/app/src/test/java/com/noop/protocol/FeatureFlagProbeTest.kt b/android/app/src/test/java/com/noop/protocol/FeatureFlagProbeTest.kt index 86bede8bec..901cc764e4 100644 --- a/android/app/src/test/java/com/noop/protocol/FeatureFlagProbeTest.kt +++ b/android/app/src/test/java/com/noop/protocol/FeatureFlagProbeTest.kt @@ -125,18 +125,161 @@ class FeatureFlagProbeTest { assertEquals("cursor exhausted (index 0xFF)", report.stopReason) } - @Test fun validKeyFalseStopsTheWalkEvenWithBytesAfterIt() { + /** + * `validKey = 0` alone is NOT classified as the strap's end marker any more — it is an empty slot, + * and the walk steps over it. The entry itself is still never collected as a key. + */ + @Test fun validKeyFalseIsAnEmptySlotNotTheEndMarker() { val record = byteArrayOf(0x01, 0x04, 0x00) + keyBytes("stale_buffer_leftover") val frame = whoop4Response(118, payload(1, record)) val r = FeatureFlagProbe.parseNext(frame, DeviceFamily.WHOOP4).value!! assertFalse(r.validKey) - assertTrue(r.isExhausted) + assertFalse("only index=0xFF is the strap's unambiguous end marker", r.isExhausted) + assertTrue(r.isEmptySlot) val report = FeatureFlagProbeReport(DeviceFamily.WHOOP4) - assertFalse(report.noteNext(r)) - assertEquals("firmware reported validKey=false", report.stopReason) + assertTrue("validKey=0 without 0xFF must not end the walk", report.noteNext(r)) + assertNull(report.stopReason) + assertEquals(1, report.emptySlots) assertTrue(report.keys.isEmpty()) } + /** + * The whole experiment, in one walk: the strap serves `validKey = 0` mid-list, the walk keeps asking, + * and the strap names MORE keys before ending on 0xFF. That sequence is decisive — under the old rule + * this list would have been reported as one key long. + */ + @Test fun walkContinuesPastValidKeyZeroAndCollectsTheKeysAfterIt() { + val report = FeatureFlagProbeReport(DeviceFamily.WHOOP5) + report.noteStart(FeatureFlagProbe.StartResponse(1, 1, 5, listOf(0x01, 0x05, 0x00))) + fun next(index: Int, valid: Boolean, key: String?) = FeatureFlagProbe.NextResponse( + 1, 1, index, valid, key, + listOf(0x01, index.toByte(), if (valid) 0x01 else 0x00), + ) + assertTrue(report.noteNext(next(1, true, "enable_r22_packets"))) + assertTrue("an empty slot must not end the walk", report.noteNext(next(2, false, null))) + assertTrue("nor must a second one", report.noteNext(next(3, false, null))) + assertTrue(report.noteNext(next(4, true, "enable_sig12"))) + assertFalse(report.noteNext(next(0xFF, false, null))) + + assertEquals(listOf("enable_r22_packets", "enable_sig12"), report.keys) + assertEquals(2, report.emptySlots) + assertEquals(1, report.keysAfterFirstEmptySlot) + assertEquals(FeatureFlagProbe.StopCode.END_MARKER, report.stopCode) + val finding = report.terminatorFinding + assertTrue(finding, finding.startsWith("DECISIVE — validKey=0 is an EMPTY/RETIRED SLOT")) + assertTrue(finding, finding.contains("naming 1 more key(s)")) + assertTrue(report.render().contains("2 validKey=0 slot(s) stepped over")) + } + + /** + * #874's rule reaches the CONCLUSION too. The strap flagged an entry past the hole as real; our ASCII + * filter declined its name. The list still continued, so the finding must still be decisive — reading + * "inconclusive" here would be our parser deciding a question about the firmware. + */ + @Test fun anUndecodableEntryAfterAnEmptySlotIsStillDecisive() { + val report = FeatureFlagProbeReport(DeviceFamily.WHOOP5) + report.noteStart(FeatureFlagProbe.StartResponse(1, 1, 30, listOf(0x01, 0x1E, 0x00))) + assertTrue( + report.noteNext( + FeatureFlagProbe.NextResponse(1, 1, 1, false, null, listOf(0x01, 0x01, 0x00)), + ), + ) + assertTrue( + report.noteNext( + FeatureFlagProbe.NextResponse( + 1, 1, 2, true, null, listOf(0x01, 0x02, 0x01, 0xDE.toByte(), 0xAD.toByte()), + ), + ), + ) + assertEquals("no name decoded, so no key is invented", emptyList(), report.keys) + assertEquals(0, report.keysAfterFirstEmptySlot) + assertEquals(1, report.validEntriesAfterFirstEmptySlot) + val finding = report.terminatorFinding + assertTrue(finding, finding.startsWith("DECISIVE — validKey=0 is an EMPTY/RETIRED SLOT")) + assertTrue(finding, finding.contains("1 of them flagged validKey=1")) + assertFalse(finding, finding.contains("naming")) + } + + /** + * The other decisive outcome, and the one that costs two round-trips: the strap repeats the same index + * with `validKey = 0`. What that decides is that the cursor does not advance, so this walk cannot see + * past the point — NOT that the list ends there. A firmware whose cursor parks on an empty slot emits + * the identical frame, and that is the reading this whole probe exists to make testable, so the + * verdict must not print it as settled. Both halves are pinned below. + */ + @Test fun repeatedEmptySlotAtTheSameIndexIsReportedAsATerminator() { + val report = FeatureFlagProbeReport(DeviceFamily.WHOOP5) + report.noteStart(FeatureFlagProbe.StartResponse(1, 1, 9, listOf(0x01, 0x09, 0x00))) + val empty = FeatureFlagProbe.NextResponse(1, 1, 7, false, null, listOf(0x01, 0x07, 0x00)) + assertTrue("the first empty slot is stepped over", report.noteNext(empty)) + assertFalse("a parked cursor ends the walk", report.noteNext(empty)) + assertEquals(FeatureFlagProbe.StopCode.EMPTY_SLOT_CURSOR_PARKED, report.stopCode) + assertTrue( + report.terminatorFinding, + report.terminatorFinding.startsWith("DECISIVE — validKey=0 is a TERMINATOR"), + ) + // The narrowing, pinned so it cannot be quietly widened back. "There is nothing past it" is the + // conclusion a reader would paste into an issue as settled, and it is the one this probe exists + // to question — two firmwares emit this identical frame. + assertTrue( + report.terminatorFinding, + report.terminatorFinding.contains("the cursor does not advance past it"), + ) + assertTrue( + report.terminatorFinding, + report.terminatorFinding.contains( + "not separable from a firmware whose cursor parks on an empty slot", + ), + ) + assertFalse( + "the walk observes a stalled cursor, not an empty tail: ${report.terminatorFinding}", + report.terminatorFinding.contains("there is nothing past it"), + ) + assertEquals("settling this must cost two round-trips, not a full cap", 2, report.steps) + } + + /** + * A firmware that answers `validKey = 0` forever with an ADVANCING index cannot run the walk away: the + * consecutive-empty cap stops it, and names itself a CLIENT-side bound so the run is never read as a + * complete list. + */ + @Test fun anUnendingRunOfEmptySlotsStopsAtTheConsecutiveCap() { + val report = FeatureFlagProbeReport(DeviceFamily.WHOOP4) + var index = 0 + var sent = 0 + while (report.noteNext( + FeatureFlagProbe.NextResponse(1, 1, index, false, null, listOf(0x01, index.toByte(), 0x00)), + ) + ) { + index += 1 + sent += 1 + assertTrue("walk must not run away", sent < FeatureFlagProbe.MAX_CONSECUTIVE_EMPTY_SLOTS + 2) + } + assertEquals(FeatureFlagProbe.MAX_CONSECUTIVE_EMPTY_SLOTS, report.steps) + assertEquals(FeatureFlagProbe.StopCode.EMPTY_SLOT_RUN_CAP, report.stopCode) + val why = report.stopReason!! + assertTrue(why, why.contains("a CLIENT-side bound, not the strap's")) + assertTrue(report.terminatorFinding, report.terminatorFinding.startsWith("INCONCLUSIVE")) + } + + /** A valid entry resets the run, so scattered holes cost nothing against the cap. */ + @Test fun aValidEntryResetsTheConsecutiveEmptySlotRun() { + val report = FeatureFlagProbeReport(DeviceFamily.WHOOP4) + fun empty(i: Int) = FeatureFlagProbe.NextResponse(1, 1, i, false, null, listOf(0x01, i.toByte(), 0x00)) + fun valid(i: Int, k: String) = + FeatureFlagProbe.NextResponse(1, 1, i, true, k, listOf(0x01, i.toByte(), 0x01)) + + for (i in 0 until FeatureFlagProbe.MAX_CONSECUTIVE_EMPTY_SLOTS - 1) { + assertTrue(report.noteNext(empty(i))) + } + assertTrue(report.noteNext(valid(20, "hr_ch_switching"))) + for (i in 21 until 21 + FeatureFlagProbe.MAX_CONSECUTIVE_EMPTY_SLOTS - 1) { + assertTrue("the run restarted at the valid entry", report.noteNext(empty(i))) + } + assertNull(report.stopCode) + assertEquals(listOf("hr_ch_switching"), report.keys) + } + @Test fun nonPrintableNameIsNotReportedAsAKey() { val record = byteArrayOf(0x01, 0x02, 0x01, 0xDE.toByte(), 0xAD.toByte(), 0xBE.toByte(), 0xEF.toByte()) val frame = whoop4Response(118, payload(1, record)) @@ -170,28 +313,316 @@ class FeatureFlagProbeTest { } /** - * `validKey = 0` is trusted as an end marker, but the other reading — an EMPTY SLOT with the list - * continuing past it — is not ruled out by anything observed so far. Stopping well short of the - * announced count is the evidence that would settle it, so the report has to say so loudly rather - * than reporting a confident short list. + * The reply a WHOOP 5 MG actually served to end its 115/116 walk: `index = 255` and `validKey = 0` on + * the SAME frame. Both terminator conditions fired at once, so that run cannot say which one the + * firmware meant — and the report must say exactly that instead of picking one. */ - @Test fun stoppingOnValidKeyFalseShortOfTheAnnouncedCountIsFlagged() { - val report = FeatureFlagProbeReport(DeviceFamily.WHOOP4) - report.noteStart(FeatureFlagProbe.StartResponse(1, 1, 40)) - assertTrue(report.noteNext(FeatureFlagProbe.NextResponse(1, 1, 0, true, "enable_r22_packets"))) - assertFalse(report.noteNext(FeatureFlagProbe.NextResponse(1, 1, 1, false, null))) + @Test fun endMarkerCarryingValidKeyZeroIsReportedAsAmbiguous() { + val report = FeatureFlagProbeReport(DeviceFamily.WHOOP5, FeatureFlagProbe.DEVICE_CONFIG_NAMESPACE) + report.noteStart(FeatureFlagProbe.StartResponse(1, 1, 7, listOf(0x01, 0x07, 0x00))) + assertTrue( + report.noteNext( + FeatureFlagProbe.NextResponse(1, 1, 1, true, "enable_rfid", listOf(0x01, 0x01, 0x01)), + ), + ) + assertFalse( + report.noteNext( + FeatureFlagProbe.NextResponse( + 1, 1, 0xFF, false, null, listOf(0x01, 0xFF.toByte(), 0x00, 0x00), + ), + ), + ) - val why = report.stopReason!! - assertTrue(why, why.contains("2 of 40 announced entries")) - assertTrue(why, why.contains("the remainder was NOT walked")) + assertTrue(report.endMarkerAlsoCarriedInvalidFlag) + assertEquals(FeatureFlagProbe.StopCode.END_MARKER, report.stopCode) + assertTrue(report.terminatorFinding, report.terminatorFinding.startsWith("AMBIGUOUS")) + assertTrue( + report.terminatorFinding, + report.terminatorFinding.contains("both terminator conditions fired at once"), + ) + assertTrue(report.render().contains("index=0xFF AND validKey=0 on the same reply")) + } + + /** + * The 0xFF marker with `validKey` still true separates the two conditions the other way, and the + * report has to be equally careful there: 0xFF alone terminated, and validKey=0 stayed untested. + */ + @Test fun endMarkerWithValidKeyTrueSaysValidKeyZeroIsUntested() { + val report = FeatureFlagProbeReport(DeviceFamily.WHOOP5) + report.noteStart(FeatureFlagProbe.StartResponse(1, 1, 2, listOf(0x01, 0x02, 0x00))) + assertTrue( + report.noteNext( + FeatureFlagProbe.NextResponse( + 1, 1, 1, true, "enable_r22_packets", listOf(0x01, 0x01, 0x01), + ), + ), + ) + assertFalse( + report.noteNext( + FeatureFlagProbe.NextResponse(1, 1, 0xFF, true, null, listOf(0x01, 0xFF.toByte(), 0x01)), + ), + ) + assertFalse(report.endMarkerAlsoCarriedInvalidFlag) + assertTrue( + report.terminatorFinding, + report.terminatorFinding.contains("validKey=0 was never served, so it is untested"), + ) + } + + /** + * What the 117/118 walk on a 5/MG actually did: sixteen valid, named entries and NO terminator of + * either kind. The walk used to stop dead at the announced count and read as a complete list. It now + * asks a bounded number of further times, and — whatever comes back — reports in the verdict itself + * that a client-side bound ended the run. + */ + @Test fun walkOvershootsTheAnnouncedCountSoTheStrapCanEndItsOwnList() { + val report = FeatureFlagProbeReport(DeviceFamily.WHOOP5) + report.noteStart(FeatureFlagProbe.StartResponse(1, 1, 3, listOf(0x01, 0x03, 0x00))) + var sent = 0 + var keepGoing = true + while (keepGoing) { + sent += 1 + keepGoing = report.noteNext( + FeatureFlagProbe.NextResponse( + 1, 1, sent, true, "key_$sent", listOf(0x01, sent.toByte(), 0x01), + ), + ) + } + assertEquals(3 + FeatureFlagProbe.COUNT_OVERSHOOT_ALLOWANCE, sent) + assertEquals(FeatureFlagProbe.StopCode.ANNOUNCED_COUNT_OVERSHOOT, report.stopCode) + assertEquals(FeatureFlagProbe.COUNT_OVERSHOOT_ALLOWANCE, report.repliesPastAnnouncedCount) + assertTrue( + report.verdict, + report.verdict.contains("INCOMPLETE: the walk ended on a client-side bound"), + ) + assertTrue(report.terminatorFinding, report.terminatorFinding.startsWith("NO TERMINATOR OBSERVED")) + val count = report.countFinding!! + assertTrue(count, count.contains("Keys yielded: 7")) + assertTrue(count, count.contains("MISMATCH against the announced 3")) + assertTrue(count, count.contains("kept answering 4 repl(ies) past its own announced count")) + } + + // MARK: raw bytes + + /** + * Requirement of the whole exercise: the RAW record bytes of every reply are in the report, not only + * the fields parsed out of them. Earlier findings here had to be re-run because only parsed output + * survived, and a parsed field cannot contradict the layout that produced it. + */ + @Test fun everyReplyLogsItsRawRecordBytes() { + val report = FeatureFlagProbeReport(DeviceFamily.WHOOP5) + val start = whoop5Response(117, payload(1, byteArrayOf(0x01, 0x02, 0x00))) + val s = FeatureFlagProbe.parseStart(start, DeviceFamily.WHOOP5).value!! + report.noteStart(s) + assertEquals(listOf(0x01, 0x02, 0x00), s.record) + val next = whoop5Response(118, payload(1, byteArrayOf(0x01, 0x00, 0x01, 0x78, 0x00))) + val n = FeatureFlagProbe.parseNext(next, DeviceFamily.WHOOP5).value!! + assertTrue(report.noteNext(n)) + val text = report.render() + assertTrue(text, text.contains("raw=01 02 00")) + assertTrue(text, text.contains("raw=01 00 01 78 00")) } - /** …and when the count was fully walked, the plain reason stands — no false alarm. */ - @Test fun validKeyFalseAtTheAnnouncedEndIsNotFlagged() { + /** A reply that failed to decode is the one whose raw bytes matter most, so the whole frame goes in. */ + @Test fun undecodedReplyLogsTheWholeRawFrame() { val report = FeatureFlagProbeReport(DeviceFamily.WHOOP4) - report.noteStart(FeatureFlagProbe.StartResponse(1, 1, 1)) - assertFalse(report.noteNext(FeatureFlagProbe.NextResponse(1, 1, 0, false, null))) - assertEquals("firmware reported validKey=false", report.stopReason) + report.noteFailure( + FeatureFlagProbe.ParseFailure.CRC, 118, + byteArrayOf(0xAA.toByte(), 0x05, 0x00, 0x2B, 0x24), + ) + assertTrue(report.render(), report.render().contains("raw frame=aa 05 00 2b 24")) + assertEquals(FeatureFlagProbe.StopCode.PARSE_FAILURE, report.stopCode) + } + + /** + * The hex is bounded so one absurd record cannot flood the log — but the true LENGTH is always + * printed, because an over-long record is itself evidence that the layout is wrong. + */ + @Test fun overLongRawRecordIsElidedButItsLengthIsKept() { + val long = List(FeatureFlagProbe.MAX_RAW_HEX_BYTES + 10) { 0xAB.toByte() } + val rendered = FeatureFlagProbe.hex(long) + assertTrue( + rendered, + rendered.endsWith("… (${FeatureFlagProbe.MAX_RAW_HEX_BYTES + 10} bytes total)"), + ) + assertEquals("(empty)", FeatureFlagProbe.hex(emptyList())) + } + + // MARK: the announced count + + /** + * The count is read as u16 LE, and every count seen so far has had a zero high byte — where a + * single-byte read returns the SAME number. So no capture distinguishes the two readings, and the + * report says that rather than asserting a field width nothing has established. + */ + @Test fun countCarriesBothReadingsAndSaysWhenTheyAgree() { + val frame = whoop5Response(117, payload(1, byteArrayOf(0x01, 0x10, 0x00))) + val r = FeatureFlagProbe.parseStart(frame, DeviceFamily.WHOOP5).value!! + assertEquals(16, r.count) + assertEquals(16, r.singleByteCount) + assertEquals(0, r.countHighByte) + assertTrue(r.countReadingsAgree) + val report = FeatureFlagProbeReport(DeviceFamily.WHOOP5) + report.noteStart(r) + val finding = report.countFinding!! + assertTrue(finding, finding.contains("u16 LE read = 16; single-byte read = 16 (high byte 0x00)")) + assertTrue(finding, finding.contains("the two readings AGREE")) + } + + /** + * A nonzero high byte is the only thing that separates them, and it is a finding either way: on the + * u16 reading the list is enormous, on the single-byte reading `record[2]` is padding. + */ + @Test fun nonZeroHighByteIsReportedAsADisagreement() { + val frame = whoop5Response(117, payload(1, byteArrayOf(0x01, 0x07, 0x02))) + val r = FeatureFlagProbe.parseStart(frame, DeviceFamily.WHOOP5).value!! + assertEquals(0x0207, r.count) + assertEquals(7, r.singleByteCount) + assertFalse(r.countReadingsAgree) + assertFalse("an implausible u16 must not become a loop bound", r.countIsPlausible) + val report = FeatureFlagProbeReport(DeviceFamily.WHOOP5) + report.noteStart(r) + val finding = report.countFinding!! + assertTrue(finding, finding.contains("single-byte read = 7 (high byte 0x02)")) + assertTrue(finding, finding.contains("the two readings DISAGREE")) + } + + /** + * Announced-count-versus-yielded is itself a result. A strap that announces sixteen and names one has + * fifteen entries nobody has accounted for, and the report has to raise it rather than print a tidy + * short list. + */ + @Test fun announcedCountVersusKeysYieldedMismatchIsReported() { + val report = FeatureFlagProbeReport(DeviceFamily.WHOOP5) + report.noteStart(FeatureFlagProbe.StartResponse(1, 1, 16, listOf(0x01, 0x10, 0x00))) + assertTrue( + report.noteNext( + FeatureFlagProbe.NextResponse( + 1, 1, 1, true, "enable_r22_packets", listOf(0x01, 0x01, 0x01), + ), + ), + ) + assertFalse( + report.noteNext( + FeatureFlagProbe.NextResponse(1, 1, 0xFF, true, null, listOf(0x01, 0xFF.toByte(), 0x01)), + ), + ) + val finding = report.countFinding!! + assertTrue(finding, finding.contains("Keys yielded: 1")) + assertTrue(finding, finding.contains("MISMATCH against the announced 16")) + } + + /** …and when everything is accounted for — keys plus skipped plus empty slots — there is no alarm. */ + @Test fun fullyAccountedCountIsNotFlaggedAsAMismatch() { + val report = FeatureFlagProbeReport(DeviceFamily.WHOOP5) + report.noteStart(FeatureFlagProbe.StartResponse(1, 1, 3, listOf(0x01, 0x03, 0x00))) + report.noteNext(FeatureFlagProbe.NextResponse(1, 1, 1, true, "a", listOf(0x01, 0x01, 0x01))) + report.noteNext(FeatureFlagProbe.NextResponse(1, 1, 2, true, null, listOf(0x01, 0x02, 0x01))) + report.noteNext(FeatureFlagProbe.NextResponse(1, 1, 3, false, null, listOf(0x01, 0x03, 0x00))) + val finding = report.countFinding!! + assertFalse(finding, finding.contains("MISMATCH")) + assertTrue(finding, finding.contains("Keys yielded: 1 (+1 undecodable) (+1 empty slot(s))")) + } + + // MARK: the device-config namespace (115/116) + + /** + * The SAME walk drives the device-config pair, so the terminator rules cannot be corrected in one + * namespace and left wrong in the other. Decoded end-to-end from 115/116 frames. + */ + @Test fun deviceConfigNamespaceWalksThroughTheSameCorrectedRules() { + val report = FeatureFlagProbeReport(DeviceFamily.WHOOP5, FeatureFlagProbe.DEVICE_CONFIG_NAMESPACE) + val start = whoop5Response(115, payload(1, byteArrayOf(0x01, 0x02, 0x00))) + report.noteStart(FeatureFlagProbe.parseStart(start, DeviceFamily.WHOOP5, 115).value!!) + // An empty slot first — under the old rule the walk would have ended here with zero keys. + val hole = whoop5Response(116, payload(1, byteArrayOf(0x01, 0x02, 0x00, 0x00))) + assertTrue(report.noteNext(FeatureFlagProbe.parseNext(hole, DeviceFamily.WHOOP5, 116).value!!)) + val named = whoop5Response( + 116, payload(1, byteArrayOf(0x01, 0x03, 0x01) + keyBytes("enable_raw_data_w_ecg")), + ) + assertTrue(report.noteNext(FeatureFlagProbe.parseNext(named, DeviceFamily.WHOOP5, 116).value!!)) + assertEquals( + "the key AFTER the empty slot is exactly what the old rule discarded", + listOf("enable_raw_data_w_ecg"), report.keys, + ) + + val text = report.render() + assertTrue(text, text.contains("#761 DEVICE-CONFIG ENUMERATION PROBE — WHOOP 5/MG")) + assertTrue(text, text.contains("START_DEVICE_CONFIG_KEY_EXCHANGE(115) + SEND_NEXT_DEVICE_CONFIG(116)")) + assertTrue(text, text.contains("Keys reported by the strap (1 of 2 announced")) + assertTrue(text, text.contains("SEND_NEXT_DEVICE_CONFIG(116) → index=3")) + } + + /** The 115/116 frames must not decode as 117/118 by accident — the opcode is checked, both ways. */ + @Test fun theTwoNamespacesDoNotDecodeEachOthersFrames() { + val cfg = whoop5Response(116, payload(1, byteArrayOf(0x01, 0x00, 0x01))) + assertEquals( + FeatureFlagProbe.ParseFailure.WRONG_COMMAND, + FeatureFlagProbe.parseNext(cfg, DeviceFamily.WHOOP5).failure, + ) + val ff = whoop5Response(118, payload(1, byteArrayOf(0x01, 0x00, 0x01))) + assertEquals( + FeatureFlagProbe.ParseFailure.WRONG_COMMAND, + FeatureFlagProbe.parseNext(ff, DeviceFamily.WHOOP5, 116).failure, + ) + assertEquals(115, FeatureFlagProbe.DEVICE_CONFIG_NAMESPACE.startCmd) + assertEquals(116, FeatureFlagProbe.DEVICE_CONFIG_NAMESPACE.nextCmd) + // Still read-only: the SET verbs appear in neither namespace. + for (ns in listOf(FeatureFlagProbe.FEATURE_FLAG_NAMESPACE, FeatureFlagProbe.DEVICE_CONFIG_NAMESPACE)) { + assertFalse(listOf(119, 120).contains(ns.startCmd)) + assertFalse(listOf(119, 120).contains(ns.nextCmd)) + } + } + + /** + * An explicit refusal is the strap's answer, not one of our bounds, and it must end the walk with its + * own name — on the START reply (where the driver reads [FeatureFlagProbeReport.hasStopped] instead of + * stepping to 118) and on a NEXT reply alike. + */ + @Test fun unsupportedRefusalEndsTheWalkWithItsOwnStopCode() { + val start = FeatureFlagProbeReport(DeviceFamily.WHOOP5) + assertFalse(start.hasStopped) + start.noteStart(FeatureFlagProbe.StartResponse(3, 0, 0, listOf(0x00, 0x00, 0x00))) + assertTrue("a refused START must not step on to the next verb", start.hasStopped) + assertEquals(FeatureFlagProbe.StopCode.UNSUPPORTED, start.stopCode) + assertTrue(start.render().contains("Stop code: unsupported")) + + val mid = FeatureFlagProbeReport(DeviceFamily.WHOOP5) + mid.noteStart(FeatureFlagProbe.StartResponse(1, 1, 4, listOf(0x01, 0x04, 0x00))) + assertTrue( + mid.noteNext( + FeatureFlagProbe.NextResponse( + 1, 1, 1, true, "enable_r22_packets", listOf(0x01, 0x01, 0x01), + ), + ), + ) + assertFalse( + mid.noteNext( + FeatureFlagProbe.NextResponse(3, 0, 0, false, null, listOf(0x00, 0x00, 0x00)), + ), + ) + assertEquals(FeatureFlagProbe.StopCode.UNSUPPORTED, mid.stopCode) + assertEquals("a refusal is never counted as an empty slot", 0, mid.emptySlots) + assertTrue(mid.stopReason!!, mid.stopReason!!.contains("UNSUPPORTED(3)")) + } + + /** + * The two platforms must classify the same reply the same way, or a strap log means different things + * on either side. These are the exact constants the walk is bounded by. + */ + @Test fun walkBoundsMatchTheSwiftTwin() { + assertEquals(8, FeatureFlagProbe.MAX_CONSECUTIVE_EMPTY_SLOTS) + assertEquals(4, FeatureFlagProbe.COUNT_OVERSHOOT_ALLOWANCE) + assertEquals(64, FeatureFlagProbe.MAX_RAW_HEX_BYTES) + assertEquals(128, FeatureFlagProbe.MAX_FLAGS) + assertEquals("endMarker", FeatureFlagProbe.StopCode.END_MARKER.code) + assertEquals("emptySlotCursorParked", FeatureFlagProbe.StopCode.EMPTY_SLOT_CURSOR_PARKED.code) + assertEquals("emptySlotRunCap", FeatureFlagProbe.StopCode.EMPTY_SLOT_RUN_CAP.code) + assertEquals("stepCap", FeatureFlagProbe.StopCode.STEP_CAP.code) + assertEquals("announcedCountOvershoot", FeatureFlagProbe.StopCode.ANNOUNCED_COUNT_OVERSHOOT.code) + assertEquals("timeout", FeatureFlagProbe.StopCode.TIMEOUT.code) + assertEquals("unsupported", FeatureFlagProbe.StopCode.UNSUPPORTED.code) + assertEquals("parseFailure", FeatureFlagProbe.StopCode.PARSE_FAILURE.code) } /** @@ -299,7 +730,7 @@ class FeatureFlagProbeTest { assertEquals(FeatureFlagProbe.ParseFailure.CRC, FeatureFlagProbe.parseStart(ByteArray(0), DeviceFamily.WHOOP4).failure) } - @Test fun fullWalkRendersEveryKeyAndStopsOnTheAnnouncedCount() { + @Test fun fullWalkRendersEveryKeyAndStopsOnTheEndMarker() { val report = FeatureFlagProbeReport(DeviceFamily.WHOOP5) val start = whoop5Response(117, payload(1, byteArrayOf(0x01, 0x03, 0x00))) report.noteStart(FeatureFlagProbe.parseStart(start, DeviceFamily.WHOOP5).value!!) @@ -308,9 +739,13 @@ class FeatureFlagProbeTest { val record = byteArrayOf(0x01, i.toByte(), 0x01) + keyBytes(name) val frame = whoop5Response(118, payload(1, record)) val n = FeatureFlagProbe.parseNext(frame, DeviceFamily.WHOOP5).value!! - assertEquals(i < names.size - 1, report.noteNext(n)) + assertTrue("the announced count no longer ends the walk by itself", report.noteNext(n)) } + // The strap's own end marker — the only thing that ends a walk cleanly. + val end = whoop5Response(118, payload(1, byteArrayOf(0x01, 0xFF.toByte(), 0x00, 0x00))) + assertFalse(report.noteNext(FeatureFlagProbe.parseNext(end, DeviceFamily.WHOOP5).value!!)) assertEquals(names, report.keys) + assertEquals(FeatureFlagProbe.StopCode.END_MARKER, report.stopCode) val text = report.render() assertTrue(text.contains("#761 FEATURE-FLAG ENUMERATION PROBE — WHOOP 5/MG")) assertTrue(text.contains("Read-only")) @@ -318,7 +753,7 @@ class FeatureFlagProbeTest { assertTrue(text.contains("Flags reported by the strap (3 of 3 announced)")) assertTrue(text.contains(" 1. enable_r22_packets")) assertTrue(text.contains(" 3. wear_detect_bias")) - assertTrue(text.contains("walked the 3 entries the strap announced")) + assertTrue(text.contains("Stop code: endMarker")) assertTrue(text.contains("START_FF_KEY_EXCHANGE(117) → revision=1 count=3 result=SUCCESS(1)")) assertTrue(text.contains("SEND_NEXT_FF(118) → index=0 validKey=true key=\"enable_r22_packets\"")) } @@ -372,14 +807,17 @@ class FeatureFlagProbeTest { Verdict: enumerated 1 feature-flag key name(s) Stopped: cursor exhausted (index 0xFF) + Stop code: endMarker + Terminator: AMBIGUOUS — the walk ended on a reply carrying index=0xFF AND validKey=0 together, so both terminator conditions fired at once and this run cannot say which one the firmware meant. + Announced count: u16 LE read = 2; single-byte read = 2 (high byte 0x00) — the two readings AGREE, so this reply does not establish the field's width. Keys yielded: 1 — MISMATCH against the announced 2 Flags reported by the strap (1 of 2 announced): 1. enable_r22_packets Exchange: - START_FF_KEY_EXCHANGE(117) → revision=1 count=2 result=SUCCESS(1) - SEND_NEXT_FF(118) → index=0 validKey=true key="enable_r22_packets" result=SUCCESS(1) - SEND_NEXT_FF(118) → index=255 validKey=false result=SUCCESS(1) + START_FF_KEY_EXCHANGE(117) → revision=1 count=2 result=SUCCESS(1) raw=01 02 00 + SEND_NEXT_FF(118) → index=0 validKey=true key="enable_r22_packets" result=SUCCESS(1) raw=01 00 01 65 6e 61 62 6c 65 5f 72 32 32 5f 70 61 63 6b 65 74 73 00 00 00 00 00 00 00 00 00 00 + SEND_NEXT_FF(118) → index=255 validKey=false result=SUCCESS(1) raw=01 ff 00 00 00 00 00 (index=0xFF AND validKey=0 on the same reply — both terminator conditions at once) """.trimIndent() assertEquals(golden, report.render()) diff --git a/docs/PROTOCOL.md b/docs/PROTOCOL.md index 85be882044..e6a0d4936c 100644 --- a/docs/PROTOCOL.md +++ b/docs/PROTOCOL.md @@ -547,10 +547,21 @@ the same `pay[2]` record start `GET_BATTERY_LEVEL` and `GET_CLOCK` already decod | 117 `START_FF_KEY_EXCHANGE` | `revision u8` · `numberOfFeatureFlags u16 LE` · padding | | 118 `SEND_NEXT_FF` | `revision u8` · `index u8` · `validKey u8` · `key` (ASCII, NUL-terminated) · padding | -The walk stops on the strap's own end marker (`validKey = 0`, or `index = 0xFF`), on the announced count, -or on a hard cap of 128 replies — and each 118 is only sent after the previous reply lands. Both CRCs are -verified before any field is read; a failed CRC, a non-COMMAND_RESPONSE type, or a short record ends the -walk with a named reason instead of a decode. Driven by `BLEManager.probeFeatureFlags()` / +**The two terminator conditions are not interchangeable, and are separated deliberately.** The walk stops +on `index = 0xFF` — the one end marker a strap has served here unambiguously. `validKey = 0` on its own +does NOT stop it: that could equally mark an EMPTY or RETIRED SLOT with the list continuing past it, and +the record layout above is derived from a WHOOP 4.0 and **unverified on 5/MG**. Neither reading is +established, because on the walks this project has, the two have never been separated on the wire: the +117/118 walk on a WS50_r03 served sixteen replies that were all `validKey = 1` with no `0xFF` at all, +and its 115/116 walk ended on a single reply carrying `index = 255` **and** `validKey = 0` together. So a +`validKey = 0` entry is recorded, stepped over, and the next record verb is sent again — what comes back +separates the two readings, and the report states which it observed. Past that the bounds are all +CLIENT-side and each names itself in the report's `Stop code:` line: 8 consecutive `validKey = 0` replies, +a repeated index during such a run (a parked cursor — evidence for the terminator reading), the announced +count plus 4, or a hard cap of 128 replies. Each next-record request is only sent after the previous reply +lands. Both CRCs are verified before any field is read; a failed CRC, a non-COMMAND_RESPONSE type, or a +short record ends the walk with a named reason instead of a decode, and the RAW record bytes of every +reply are logged beside the fields decoded from them. Driven by `BLEManager.probeFeatureFlags()` / `WhoopBleClient.probeFeatureFlags()` (user-triggered, Test Centre → Connection, both families) and allowlisted for 5/MG framing **only while a probe is in flight**; parsed + rendered by the pure `FeatureFlagProbe` / `FeatureFlagProbeReport` twins (Swift↔Kotlin byte-parity, unit-tested on synthetic