From 37bdfda40d6b416c0ccd22481d07a99f9e862b86 Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Wed, 30 Sep 2026 02:15:46 +1300 Subject: [PATCH] ecg: correct the MG Labrador R17 layout, wrist values and START list The WHOOP MG ECG decode in this repo was built from static analysis of the vendor client, with the packet type and several field offsets marked in the source as unattested inferences. A third-party implementation has since closed those questions against the official Android 5.458.0 Labrador parser and the 50.41.1.0 firmware constructor, and run the result on a physical MG. Three of the inferences were wrong. The layout. Fields were read from frame[11] as a 17-byte status block. The record's status fields actually begin six bytes later, and the sample block starts at inner[26]. The structural triage built on that offset required four bytes to be 0 or 1; two of them land on this record's quality byte (0..3) and its flags byte, whose electrode-contact bit is 0x08. Every packet reporting contact, which is every packet carrying a reading, failed the triage and was dropped. That is sufficient on its own to explain the acceptedButSilent verdict the probe keeps returning. What says the correction is right rather than merely different: rawWaveformStart was measured on an MG at 34 before any of this was decoded, and 34 is exactly rawTypeOffset + inner[26]. A test on each platform pins that identity, so an edit to either constant fails. The wrist values. SELECT_WRIST carried right=0/left=1, read off the client enum's declaration order. The wire values are right=1/left=2. This is the one command in the family that writes PERSISTENT strap state, so the old values did not merely fail, they wrote a wrong persistent selection or were refused. The same reading of the same enum's order was already wrong once for ControlSignal (#896), and that lesson was recorded in a comment beside a test that still asserted the wrist bytes through the symbol rather than as literals. The START list. ABORT_HISTORICAL_TRANSMITS (20) belongs to START, immediately ahead of 124, and goes out on every run. It was sent early and only when an offload happened to be draining, which left the common case without the clear the sequence calls for. The send allowlist gates opcode 20 on `backfilling`, which is right for the abort-a-sync button and wrong for a member of this sequence, so both platforms widen it for the one synchronous send. Also replaces FilteredLabradorPacket and its decode with LabradorR17, carrying the flags byte, the unreadable mask, and the terminal and invalid conditions. The probe log now reports sequence, presence and progress, so a strap log shows whether a reading progressed rather than only that packets arrived. EcgStatusHeader survives on the RAW (revision 16) path, whose body layout is still unconfirmed; the comments claiming both records share it are corrected. Not changed here: samplesPerRawRecord is 101 where the wire carries 100, but it feeds the #194 autocorrelation exclude-lag and needs its own change. Android still has no wrist-selection surface, so it sends an incomplete PREPARE list. LabradorOperation restart=3 exists and is not implemented. Verification: WhoopProtocol 812 tests, Android full unit suite, both green with fresh results. doc_comment_lint and i18n_audit --ci clean. BLEManager.swift is app-target Swift that no local or default-CI leg compiles, so app-build is the only thing that covers it. Not exercised on hardware yet. Refs #891, #1100. Tools/parity_twin_map.json moves in this commit rather than a follow-up: the derived sets change because the source does, and rebase-merge puts every commit on main on its own, so splitting them would land one that fails the ratchet. The refresh pairs all four new functions (function_pairs 211 to 215, unpaired_functions 4108 to 4100) rather than suppressing a finding. The probe report is one text on both platforms. The candidate line carried the not-a-diagnosis disclaimer inside it on Apple and appended it at log time on Android, so the two stored different strings for the same packet, and the per-packet notes landed after a closing parenthesis on one side. The report footer and the result sheet each already say it, so the per-line copy is gone and both platforms now store identical text. The verdict headline and the two section headings, which described a shape heuristic that no longer exists, are rewritten on both sides together. --- .../Sources/WhoopProtocol/Whoop5Ecg.swift | 312 ++++++++++--- .../WhoopProtocol/Whoop5EcgProbe.swift | 27 +- .../WhoopProtocolTests/Whoop5EcgTests.swift | 439 +++++++++++------ Strand/BLE/BLEManager.swift | 131 ++++-- Strand/Resources/Localizable.xcstrings | 20 +- Strand/Screens/DevicesView.swift | 12 +- Tools/parity_twin_map.json | 20 +- .../main/java/com/noop/ble/WhoopBleClient.kt | 109 ++++- .../main/java/com/noop/protocol/Whoop5Ecg.kt | 297 +++++++++--- .../java/com/noop/protocol/Whoop5EcgProbe.kt | 16 +- .../java/com/noop/protocol/Whoop5EcgTest.kt | 441 ++++++++++++------ 11 files changed, 1294 insertions(+), 530 deletions(-) diff --git a/Packages/WhoopProtocol/Sources/WhoopProtocol/Whoop5Ecg.swift b/Packages/WhoopProtocol/Sources/WhoopProtocol/Whoop5Ecg.swift index 68fb8d068b..2e30dbdf80 100644 --- a/Packages/WhoopProtocol/Sources/WhoopProtocol/Whoop5Ecg.swift +++ b/Packages/WhoopProtocol/Sources/WhoopProtocol/Whoop5Ecg.swift @@ -5,8 +5,12 @@ import Foundation // The WHOOP MG carries ECG electrodes in its conductive clasp (a plain WHOOP 5.0 does not — see // `Whoop5Variant`). The strap's ECG subsystem is called "Labrador" in the protocol tables, and it is a // SEPARATE realtime data type from the R-numbered `StrapSensorData` layouts this package already decodes: -// there is a FILTERED stream (live, display-ready) and a RAW stream (persisted on the strap for later -// offload). Both carry the same 17-byte status header; they differ only in what follows it. +// there is a FILTERED stream (live, display-ready, data revision 17) and a RAW stream (persisted on the +// strap for later offload, data revision 16). +// +// They do NOT share a status block. The filtered record's layout is `LabradorR17`, read from the inner +// record at the offsets the official parser uses. `EcgStatusHeader` below is the earlier client-derived +// reading; it survives only on the RAW path, whose body layout nothing here can yet confirm. // // Provenance — and the line this file does not cross. // @@ -21,13 +25,6 @@ import Foundation // own style, and no WHOOP code, firmware or asset is present. // // What is NOT established, and is therefore never asserted: -// • The packet TYPE byte these records arrive under. No capture exists, and the repo's `PacketType` -// table has no Labrador entry. So this file decodes a PAYLOAD, and the app layer discovers the type -// empirically by running `plausibleFilteredPayload` over unclassified frames and logging the hits. -// • The `WristSelection` raw values. `right` is listed first in the client's enum, so right=0/left=1 is -// the natural reading — but it is an INFERENCE, not an attested fact, and it is labelled as such -// everywhere it surfaces (including in the UI, because picking the wrong one writes the wrong -// persistent value to the strap). // • The `heartKeyProgress` "timed out" sentinel. The client's type is a union of a percentage and a // timed-out case; the sentinel VALUE is not attested, so an out-of-range byte is carried raw rather // than renamed into a state we cannot prove it means. @@ -127,9 +124,13 @@ public enum EcgHeartKeyProgress: Equatable, Sendable { // MARK: - Shared status header -/// The 17-byte status block both Labrador packets open with, in wire order. +/// The 17-byte status block the RAW (revision 16) record is read with, in wire order. /// /// Multi-byte fields are little-endian, matching every other 5/MG field in this package. +/// +/// NOT the filtered record's layout, though it was written believing it was: see `LabradorR17`. Whether +/// the raw record really opens this way is still unconfirmed, so this stays where it is rather than +/// being corrected to something equally unproven. public struct EcgStatusHeader: Equatable, Sendable { public let signalQuality: EcgSignalQuality /// Raw quality byte, kept so a value outside the known enum is never lost. @@ -182,21 +183,137 @@ public struct EcgStatusHeader: Equatable, Sendable { /// The live ECG stream packet (`toggleRealtimeFilteredECG` / TOGGLE_LABRADOR_FILTERED, 0x8B). /// -/// 17 fields in wire order: the shared status header, then `numberOfECGSamples` signed 16-bit samples, -/// then whatever trailing bytes the envelope carried. The sample UNIT and SCALE are not attested, so the -/// array is named `filteredECGDataRaw` and no µV conversion is applied anywhere. -public struct FilteredLabradorPacket: Equatable, Sendable { - public let header: EcgStatusHeader - public let filteredECGDataRaw: [Int16] - public let padding: [UInt8] +/// Decoded by `Whoop5Ecg.parseR17` into a `LabradorR17`, whose offsets are read from the INNER record +/// (the type byte onwards), not from the payload after it. + +/// R17 `inner[14]` — the on-strap classifier's state/transition bits plus electrode presence. +public struct EcgLabradorFlags: Equatable, Sendable { + public let raw: UInt8 + public init(raw: UInt8) { self.raw = raw } + + /// bit 0 — entering classifier state 1. + public var enteringStateOne: Bool { raw & 0x01 != 0 } + /// bit 1 — the current classifier state IS 1. An ordinary active frame carries this set; a valid + /// active frame with it clear is the explicit-restart case, not a contact loss. + public var currentStateOne: Bool { raw & 0x02 != 0 } + /// bit 2 — the 1 → 2 state transition, set on the terminal frame. + public var stateTransitionOneToTwo: Bool { raw & 0x04 != 0 } + /// bit 3 — electrode contact, debounced on the strap. This is the bit `heartKeyLeadsAreOn` was + /// reaching for at the wrong offset. + public var presence: Bool { raw & 0x08 != 0 } + + /// The set bits by name, in bit order. Bits above 3 are reported as unknown rather than named. + public var tokens: [String] { + var out = [String]() + if enteringStateOne { out.append("entering_state_1") } + if currentStateOne { out.append("state_1") } + if stateTransitionOneToTwo { out.append("transition_1_2") } + if presence { out.append("presence") } + let unknown = raw & ~0x0F + if unknown != 0 { out.append(String(format: "unknown_bits_0x%02x", Int(unknown))) } + return out + } +} - public init(header: EcgStatusHeader, filteredECGDataRaw: [Int16], padding: [UInt8]) { - self.header = header - self.filteredECGDataRaw = filteredECGDataRaw - self.padding = padding +/// R17 `inner[18]` — why the strap called a reading unreadable. Bits above 3 are reported as unknown +/// rather than given a meaning. +public struct EcgUnreadableMask: Equatable, Sendable { + public let raw: UInt8 + public init(raw: UInt8) { self.raw = raw } + + public var lowAmplitude: Bool { raw & 0x01 != 0 } + public var significantNoise: Bool { raw & 0x02 != 0 } + public var unstableSignal: Bool { raw & 0x04 != 0 } + public var notEnoughData: Bool { raw & 0x08 != 0 } + + /// The set bits by name, in bit order. + public var reasons: [String] { + var out = [String]() + if lowAmplitude { out.append("low_amplitude") } + if significantNoise { out.append("significant_noise") } + if unstableSignal { out.append("unstable_signal") } + if notEnoughData { out.append("not_enough_data") } + let unknown = raw & ~0x0F + if unknown != 0 { out.append(String(format: "unknown_bits_0x%02x", Int(unknown))) } + return out } } +/// One Labrador revision-17 packet — the strap's live filtered-ECG cycle. +/// +/// Offsets are into the INNER record, counted from the packet-type byte. On a 5/MG frame that byte is at +/// `Whoop5Ecg.rawTypeOffset` (8), so `inner[k]` is `frame[8 + k]`: fixed fields occupy `inner[0...25]` +/// and the samples start at `inner[26]` — `frame[34]`, which is exactly the waveform offset the +/// type-43 constants in this file were already observing on hardware. +/// +/// `samples` are 100 Hz filtered signed i16 little-endian values as transmitted. No rescaling is applied +/// and no anatomical lead or polarity is claimed. `variabilityRaw` has no proven unit. +public struct LabradorR17: Equatable, Sendable { + /// `inner[0]`: 43 (REALTIME_RAW_DATA, the live path) or 47 (HISTORICAL_DATA, a stored copy). + public let packetType: UInt8 + /// `inner[2]` — a packet-context marker the consumer ignores. Kept raw rather than named. + public let headerSecondary: UInt8 + public let sequence: UInt32 // inner[3...6] u32 LE + public let strapSeconds: UInt32 // inner[7...10] u32 LE + public let subseconds: UInt16 // inner[11...12] u16 LE, 1/32768 s + public let signalQuality: EcgSignalQuality + public let signalQualityRaw: UInt8 // inner[13] + public let flags: EcgLabradorFlags // inner[14] + /// `inner[15]` — the classifier result code. Non-nil when it maps to a known case. + public let arrhythmiaCheckResult: EcgArrhythmiaCheckResult? + public let arrhythmiaCheckResultRaw: UInt8 + public let classifierState: UInt8 // inner[16]; 2 is terminal + public let progress: EcgHeartKeyProgress // inner[17]; 100 terminal, 255 invalid + public let unreadable: EcgUnreadableMask // inner[18] + public let averageHR: UInt8 // inner[19] — the final/stored heart rate + public let liveHR: UInt8 // inner[20] — the current heart rate + /// `inner[21...22]` u16 LE, or nil when the wire carried the unavailable sentinel. + public let variabilityRaw: UInt16? + public let reserved: UInt8 // inner[23] + public let sampleCount: UInt16 // inner[24...25] u16 LE + public let samples: [Int16] + /// Aligned bytes after the sample block, byte-exact, meaning unassigned. + public let tail: [UInt8] + + public init(packetType: UInt8, headerSecondary: UInt8, sequence: UInt32, strapSeconds: UInt32, + subseconds: UInt16, signalQuality: EcgSignalQuality, signalQualityRaw: UInt8, + flags: EcgLabradorFlags, arrhythmiaCheckResult: EcgArrhythmiaCheckResult?, + arrhythmiaCheckResultRaw: UInt8, classifierState: UInt8, progress: EcgHeartKeyProgress, + unreadable: EcgUnreadableMask, averageHR: UInt8, liveHR: UInt8, variabilityRaw: UInt16?, + reserved: UInt8, sampleCount: UInt16, samples: [Int16], tail: [UInt8]) { + self.packetType = packetType + self.headerSecondary = headerSecondary + self.sequence = sequence + self.strapSeconds = strapSeconds + self.subseconds = subseconds + self.signalQuality = signalQuality + self.signalQualityRaw = signalQualityRaw + self.flags = flags + self.arrhythmiaCheckResult = arrhythmiaCheckResult + self.arrhythmiaCheckResultRaw = arrhythmiaCheckResultRaw + self.classifierState = classifierState + self.progress = progress + self.unreadable = unreadable + self.averageHR = averageHR + self.liveHR = liveHR + self.variabilityRaw = variabilityRaw + self.reserved = reserved + self.sampleCount = sampleCount + self.samples = samples + self.tail = tail + } + + /// Electrode contact, from the flags byte. + public var presence: Bool { flags.presence } + + /// The strap's completion condition. + public var isTerminal: Bool { progress.raw == 100 || classifierState == 2 } + + /// The strap's invalid/abort sentinel. + public var isInvalid: Bool { progress.raw == 255 } + +} + /// The persisted ECG record (`toggleSaveRawECG` / TOGGLE_LABRADOR_RAW_SAVE, 0x7D). /// /// 20 fields in wire order: the same status header, then an OPAQUE raw-sample blob, then the leads-off @@ -274,12 +391,19 @@ public enum Whoop5Ecg { /// Which wrist the strap is worn on. /// - /// ⚠️ The raw values are INFERRED, not attested: `right` is listed first in the client's enum, so - /// right=0/left=1 is the natural reading. Since this command writes PERSISTENT strap state, a wrong - /// inference writes a wrong persistent value — so every surface that offers it says so. + /// `right = 1`, `left = 2` — one-based, NOT the client's zero-based declaration order. + /// + /// The previous `right = 0 / left = 1` was read off that order and shipped as an acknowledged + /// inference. It was wrong in exactly the way `ControlSignal`'s was (#896): the wire values for this + /// family start at 1, and 0 is not a member. Corrected against the official Android 5.458.0 Labrador + /// parser and the 50.41.1.0 firmware constructor, cross-checked against a third-party implementation + /// that drives a physical MG through a complete reading. + /// + /// This command writes PERSISTENT strap state, so the old values did not merely fail — they wrote a + /// wrong persistent selection, or were refused outright, on every strap that ran the probe. public enum WristSelection: UInt8, Equatable, Sendable, CaseIterable { - case right = 0 - case left = 1 + case right = 1 + case left = 2 public var token: String { self == .right ? "right" : "left" } } @@ -392,25 +516,63 @@ public enum Whoop5Ecg { // MARK: Filtered decode - /// Decode a `FilteredLabradorPacket` from the inner record's PAYLOAD (i.e. the bytes after - /// `[type][seq][cmd]`). + /// Decode a `LabradorR17` from a complete INNER record — the bytes from the packet-type byte + /// onwards, which on a 5/MG frame means `frame[8...]`. /// - /// Fails closed on a short header, and on a `numberOfECGSamples` the buffer cannot actually hold — - /// a count that disagrees with the bytes present is a decode error, never a truncated best effort. - public static func decodeFiltered(payload: [UInt8]) -> FilteredLabradorPacket? { - guard let header = EcgStatusHeader(payload: payload) else { return nil } - let n = Int(header.numberOfECGSamples) - let end = headerLength + n * 2 - guard end <= payload.count else { return nil } + /// Accepts only a type-43 (or, with `allowStored`, type-47) record of data revision 17 whose declared + /// sample block fits: fixed fields through `inner[25]` present, `sampleCount <= 100`, and + /// `26 + 2 * sampleCount` bytes available. No fixed total length is required; bytes past the sample + /// block land in `tail`. CRC validity is the caller's business — `r17FromFrame` enforces it. + /// Kotlin twin: `Whoop5Ecg.parseR17`. + public static func parseR17(inner: [UInt8], allowStored: Bool = false) -> LabradorR17? { + guard inner.count >= r17FixedLength else { return nil } + let type = inner[0] + guard type == rawRecordType || (allowStored && type == storedRecordType) else { return nil } + guard inner[1] == r17Revision else { return nil } + let count = Int(u16le(inner, 24)) + guard count <= r17MaxSamples else { return nil } + let end = r17SampleStart + count * 2 + guard inner.count >= end else { return nil } + var samples = [Int16]() - samples.reserveCapacity(n) - for i in 0.. UInt16 { + UInt16(b[i]) | (UInt16(b[i + 1]) << 8) + } + + /// Kotlin twin: `Whoop5Ecg.u32le`. + private static func u32le(_ b: [UInt8], _ i: Int) -> UInt32 { + UInt32(b[i]) | (UInt32(b[i + 1]) << 8) | (UInt32(b[i + 2]) << 16) | (UInt32(b[i + 3]) << 24) } /// CRC-gated decode straight off a complete 5/MG frame. The frame must pass BOTH puffin CRCs — a @@ -419,10 +581,13 @@ public enum Whoop5Ecg { /// `payloadStart` defaults to the standard puffin inner-data offset. It is a parameter, not a /// constant, because the packet TYPE these records arrive under is not yet attested (see the file /// header), so a capture may show a different body offset. - public static func decodeFilteredFrame(_ frame: [UInt8], - payloadStart: Int = puffinPayloadStart) -> FilteredLabradorPacket? { - guard let payload = innerPayload(frame, payloadStart: payloadStart) else { return nil } - return decodeFiltered(payload: payload) + /// Kotlin twin: `Whoop5Ecg.r17FromFrame`. + public static func r17FromFrame(_ frame: [UInt8], allowStored: Bool = false) -> LabradorR17? { + // Through `innerPayload` with the INNER record's own start offset, not `frame[rawTypeOffset...]`. + // It is the one seam that both CRC-gates the frame and stops at the CRC32 trailer; slicing to the + // end of the buffer instead would hand four envelope bytes to `tail` and call them record bytes. + guard let inner = innerPayload(frame, payloadStart: rawTypeOffset) else { return nil } + return parseR17(inner: inner, allowStored: allowStored) } // MARK: Raw decode @@ -505,26 +670,6 @@ public enum Whoop5Ecg { // MARK: Discovery - /// A cheap structural triage for "could these bytes be a filtered Labrador payload?". - /// - /// Used by the app layer to hunt for the packet TYPE byte, which is not attested: while an ECG probe - /// is armed, every unclassified 5/MG frame is run through this and the hits are logged with their - /// type. It is a HEURISTIC — four booleans, three enum ranges and a length agreement — not a - /// classifier, and nothing downstream may treat a hit as proof. - public static func plausibleFilteredPayload(_ payload: [UInt8], - maxPadding: Int = defaultMaxPadding) -> Bool { - guard let header = EcgStatusHeader(payload: payload) else { return false } - guard header.signalQualityRaw <= 3, - header.heartKeyArrhythmiaCheckResult != nil, - header.heartKeyArrhythmiaCheckStatus != nil else { return false } - // The four booleans are Bool-typed on the wire, so anything but 0/1 rules the buffer out. - guard payload[2] <= 1, payload[3] <= 1, payload[4] <= 1, payload[5] <= 1 else { return false } - let n = Int(header.numberOfECGSamples) - guard n > 0 else { return false } - let end = headerLength + n * 2 - return end <= payload.count && payload.count - end <= maxPadding - } - // MARK: - The type-43 REALTIME_RAW_DATA record: the live ECG sample carrier (#891/#1100) // // OBSERVED on one WHOOP MG (WS50_r00, fw 50.39.1.0), not attested by any vendor document: once @@ -545,6 +690,33 @@ public enum Whoop5Ecg { /// The inner record type byte for REALTIME_RAW_DATA. public static let rawRecordType: UInt8 = 43 + /// The inner record type byte for HISTORICAL_DATA — a STORED R17, which the live turn-on path never + /// enables. `parseR17` accepts it only when asked to. + public static let storedRecordType: UInt8 = 47 + + // MARK: The revision-17 layout + // + // Offsets into the INNER record, from the packet-type byte. `inner[k]` is `frame[rawTypeOffset + k]`. + // Source-closed against the official Android 5.458.0 Labrador parser and the 50.41.1.0 firmware + // constructor, and corroborated here by `rawWaveformStart`: the waveform offset OBSERVED on an MG + // (34) is exactly `rawTypeOffset + r17SampleStart`, which is what says these two readings of the + // same record agree. + + /// `inner[1]` — the data revision that makes a record an R17. + public static let r17Revision: UInt8 = 17 + + /// Fixed fields occupy `inner[0...25]`; the sample block follows. + public static let r17FixedLength = 26 + + /// First sample byte, `inner[26]`. + public static let r17SampleStart = 26 + + /// Wire capacity: 100 i16 samples per packet. + public static let r17MaxSamples = 100 + + /// `0xffff` at `inner[21...22]` means the variability value is unavailable. + public static let r17VariabilityUnavailable: UInt16 = 0xffff + /// First body byte considered by `realtimeRawBodyNonZeroBytes` — excludes the constant sub-header. public static let rawBodyStart = 24 @@ -603,14 +775,6 @@ public enum Whoop5Ecg { return n > rawBodyActiveNonZeroBytes } - /// The frame-level form of `plausibleFilteredPayload`, CRC-gated. This is what the app layer runs - /// over unclassified 5/MG frames while an ECG probe is armed. - public static func plausibleFilteredFrame(_ frame: [UInt8], - payloadStart: Int = puffinPayloadStart, - maxPadding: Int = defaultMaxPadding) -> Bool { - guard let payload = innerPayload(frame, payloadStart: payloadStart) else { return false } - return plausibleFilteredPayload(payload, maxPadding: maxPadding) - } /// The inner record's payload from a complete 5/MG frame, or nil when the frame fails either CRC or /// is too short. Every frame-level entry point in this file goes through here, so no Labrador field diff --git a/Packages/WhoopProtocol/Sources/WhoopProtocol/Whoop5EcgProbe.swift b/Packages/WhoopProtocol/Sources/WhoopProtocol/Whoop5EcgProbe.swift index d2c1bdc226..3f1528c034 100644 --- a/Packages/WhoopProtocol/Sources/WhoopProtocol/Whoop5EcgProbe.swift +++ b/Packages/WhoopProtocol/Sources/WhoopProtocol/Whoop5EcgProbe.swift @@ -132,11 +132,11 @@ public enum Whoop5EcgProbe { /// The gate verdict, kept separate from the report text so it is assertable in a test. public enum Verdict: Equatable, Sendable { - /// Frames that PASS THE STRUCTURAL TRIAGE arrived. Not proof: the triage is a heuristic over - /// four booleans, three enum ranges and a length agreement (see - /// `Whoop5Ecg.plausibleFilteredPayload`), run against ordinary live 5/MG traffic, so an - /// unrelated packet can match. The verdict says "candidate", and confirming it needs the raw - /// bytes in the log — never this count alone. + /// Revision-17 Labrador records arrived. Stronger than it used to be: the old structural + /// heuristic could match an unrelated packet, whereas `Whoop5Ecg.isLabradorR17Frame` requires + /// both CRCs, `inner[0] == 43`, `inner[1] == 17` and a sample block that fits. Still called a + /// candidate, because what the samples MEAN remains unvalidated here — and confirming a run + /// needs the raw bytes in the log, never this count alone. case ecgCandidatesArrived(packets: Int) /// At least one command that ASKED FOR DATA came back FAILURE: the opcode exists and execution /// was refused. WHY it was refused is not on the wire — the reply carries a result code and @@ -163,9 +163,10 @@ public enum Whoop5EcgProbe { public var headline: String { switch self { case .ecgCandidatesArrived(let packets): - return "\(packets) frame(s) matched the ECG structural triage. That is a CANDIDATE, not proof: " - + "the triage is a shape heuristic and unrelated traffic can match it. Confirm against the " - + "raw bytes below before concluding anything about whether the feature is blocked." + return "\(packets) revision-17 ECG packet(s) decoded. The strap IS producing the filtered " + + "trace: each one arrived as type 43 revision 17 with both CRCs intact. What the samples " + + "MEAN is still unvalidated here — check the per-packet progress and presence below to see " + + "whether a reading actually advanced, rather than reading the count alone." case .dataRequestRefused(let commands): return "DATA REQUEST REFUSED — the firmware returned FAILURE for \(commands.joined(separator: ", ")), " + "which asked it to produce ECG data: it knows the opcode and refused to run it. " @@ -244,9 +245,9 @@ public enum Whoop5EcgProbe { /// The full report: verdict, per-command outcomes, the ECG-packet tally, and the raw replies. /// - /// `candidateFrames` are the type/length lines for frames that passed the structural triage in - /// `Whoop5Ecg.plausibleFilteredPayload` — the empirical answer to "which packet type do these arrive - /// under", which no table in this repo yet holds. + /// `candidateFrames` are the per-packet lines for frames `Whoop5Ecg.r17FromFrame` decoded — the + /// sequence, progress and presence of each accepted record, which is what tells a clean run apart + /// from one the electrodes kept dropping. public static func report(steps: [Step], ecgPacketsSeen: Int, candidateFrames: [String], @@ -282,9 +283,9 @@ public enum Whoop5EcgProbe { + "the zero.\n" } if candidateFrames.isEmpty { - sb += "Candidate packet types: none — no frame passed the structural triage.\n" + sb += "Revision-17 packets decoded: none — no frame arrived as type 43 revision 17.\n" } else { - sb += "Candidate packet types (structural triage only, NOT a confirmed mapping):\n" + sb += "Revision-17 packets decoded (type 43, revision 17, both CRCs checked):\n" for line in candidateFrames { sb += " \(line)\n" } } let replies = steps.compactMap { step in step.replyHex.map { " \(step.label): \($0)" } } diff --git a/Packages/WhoopProtocol/Tests/WhoopProtocolTests/Whoop5EcgTests.swift b/Packages/WhoopProtocol/Tests/WhoopProtocolTests/Whoop5EcgTests.swift index a3c6e0ece6..42c34627af 100644 --- a/Packages/WhoopProtocol/Tests/WhoopProtocolTests/Whoop5EcgTests.swift +++ b/Packages/WhoopProtocol/Tests/WhoopProtocolTests/Whoop5EcgTests.swift @@ -34,6 +34,59 @@ final class Whoop5EcgTests: XCTestCase { UInt8(samples & 0xFF), UInt8(samples >> 8)] } + /// A revision-17 INNER record, in wire order: `inner[0]` type, `inner[1]` revision, fixed fields + /// through `inner[25]`, then the samples. + /// + /// Note what `inner[1]` and `inner[2]` are NOT. On a COMMAND frame those offsets are the sequence and + /// the opcode, which is what the `[8]type [9]seq [10]cmd` comment in the decoder describes. A DATA + /// record reuses the same two bytes for the data revision and a context marker — which is why a + /// decoder that started reading fields at `frame[11]` was six bytes off. + private func r17Inner(type: UInt8 = 43, + revision: UInt8 = 17, + headerSecondary: UInt8 = 0, + sequence: UInt32 = 7, + strapSeconds: UInt32 = 1_700_000_000, + subseconds: UInt16 = 16_384, + quality: UInt8 = 2, + flags: UInt8 = 0x0A, + result: UInt8 = 0, + state: UInt8 = 1, + progress: UInt8 = 42, + unreadable: UInt8 = 0, + averageHR: UInt8 = 61, + liveHR: UInt8 = 63, + variability: UInt16 = 812, + reserved: UInt8 = 17, + samples: UInt16) -> [UInt8] { + var out: [UInt8] = [type, revision, headerSecondary] + out += [UInt8(sequence & 0xFF), UInt8((sequence >> 8) & 0xFF), + UInt8((sequence >> 16) & 0xFF), UInt8((sequence >> 24) & 0xFF)] + out += [UInt8(strapSeconds & 0xFF), UInt8((strapSeconds >> 8) & 0xFF), + UInt8((strapSeconds >> 16) & 0xFF), UInt8((strapSeconds >> 24) & 0xFF)] + out += [UInt8(subseconds & 0xFF), UInt8(subseconds >> 8)] + out += [quality, flags, result, state, progress, unreadable, averageHR, liveHR] + out += [UInt8(variability & 0xFF), UInt8(variability >> 8)] + out += [reserved, UInt8(samples & 0xFF), UInt8(samples >> 8)] + return out + } + + /// "Does this frame decode as an R17?" — a TEST-local predicate over the product entry point. + /// + /// Deliberately not a shipped helper: the probe calls `r17FromFrame` and uses the packet, so a + /// separate boolean wrapper in the package would be a declaration nothing in production runs. + private func r17IsRecognised(_ frame: [UInt8], allowStored: Bool = false) -> Bool { + Whoop5Ecg.r17FromFrame(frame, allowStored: allowStored) != nil + } + + /// Wrap an inner record in a valid puffin envelope. The inner record starts at `frame[8]`, so the + /// builder takes everything from the type byte onwards. + private func r17Frame(_ inner: [UInt8]) -> [UInt8] { + puffinCommandFrame(cmd: inner.count > 2 ? inner[2] : 0, + seq: inner.count > 1 ? inner[1] : 0, + payload: Array(inner.dropFirst(3)), + type: inner[0]) + } + private func i16le(_ values: [Int16]) -> [UInt8] { values.flatMap { v -> [UInt8] in let u = UInt16(bitPattern: v) @@ -51,76 +104,158 @@ final class Whoop5EcgTests: XCTestCase { puffinCommandFrame(cmd: 0x00, seq: 0x01, payload: payload, type: type) } - // MARK: - Filtered packet + // MARK: - Revision-17 packet - func testFilteredDecodesEveryFieldInWireOrder() { + func testR17DecodesEveryFieldInWireOrder() { let samples: [Int16] = [0, 1, -1, 32_767, -32_768, 250, -250] - let payload = header(samples: UInt16(samples.count)) + i16le(samples) - let packet = Whoop5Ecg.decodeFiltered(payload: payload) + let inner = r17Inner(samples: UInt16(samples.count)) + i16le(samples) + let packet = Whoop5Ecg.parseR17(inner: inner) XCTAssertNotNil(packet) guard let packet else { return } - XCTAssertEqual(packet.header.signalQuality, .medium) - XCTAssertEqual(packet.header.statusFlags, 0x05) - XCTAssertTrue(packet.header.heartKeyStarted) - XCTAssertTrue(packet.header.heartKeyIsRunning) - XCTAssertFalse(packet.header.heartKeyIsStoppedAndComplete) - XCTAssertTrue(packet.header.heartKeyLeadsAreOn) - XCTAssertEqual(packet.header.heartKeyArrhythmiaCheckResult, .notComplete) - XCTAssertEqual(packet.header.heartKeyArrhythmiaCheckStatus, .inProgress) - XCTAssertEqual(packet.header.heartKeyProgress, .percent(42)) - XCTAssertEqual(packet.header.heartKeyProgress.percentValue, 42) - XCTAssertEqual(packet.header.heartKeyUnreadableReason, 0) - XCTAssertEqual(packet.header.heartKeyAverageHR, 61) - XCTAssertEqual(packet.header.heartKeyHR, 63) - XCTAssertEqual(packet.header.heartKeyHRV, 812) // u16 LE, not two u8s - XCTAssertEqual(packet.header.heartKeyStressScore, 17) - XCTAssertEqual(packet.header.numberOfECGSamples, 7) - XCTAssertEqual(packet.filteredECGDataRaw, samples) // signed, LE - XCTAssertTrue(packet.padding.isEmpty) - } - - func testFilteredCarriesTrailingPadding() { - let payload = header(samples: 2) + i16le([5, -5]) + [0x00, 0x00, 0x00] - let packet = Whoop5Ecg.decodeFiltered(payload: payload) - XCTAssertEqual(packet?.filteredECGDataRaw, [5, -5]) - XCTAssertEqual(packet?.padding, [0x00, 0x00, 0x00]) - } - - func testFilteredZeroSamplesIsValid() { - // A status-only packet (leads off, nothing captured yet) is legitimate, not an error. - let payload = header(leadsOn: 0, samples: 0) - let packet = Whoop5Ecg.decodeFiltered(payload: payload) + XCTAssertEqual(packet.packetType, 43) + XCTAssertEqual(packet.headerSecondary, 0) + XCTAssertEqual(packet.sequence, 7) + XCTAssertEqual(packet.strapSeconds, 1_700_000_000) + XCTAssertEqual(packet.subseconds, 16_384) + XCTAssertEqual(packet.signalQuality, .medium) + XCTAssertEqual(packet.signalQualityRaw, 2) + XCTAssertEqual(packet.flags.raw, 0x0A) + XCTAssertEqual(packet.arrhythmiaCheckResult, .notComplete) + XCTAssertEqual(packet.arrhythmiaCheckResultRaw, 0) + XCTAssertEqual(packet.classifierState, 1) + XCTAssertEqual(packet.progress, .percent(42)) + XCTAssertEqual(packet.progress.percentValue, 42) + XCTAssertEqual(packet.unreadable.raw, 0) + XCTAssertEqual(packet.averageHR, 61) + XCTAssertEqual(packet.liveHR, 63) + XCTAssertEqual(packet.variabilityRaw, 812) // u16 LE, not two u8s + XCTAssertEqual(packet.reserved, 17) + XCTAssertEqual(packet.sampleCount, 7) + XCTAssertEqual(packet.samples, samples) // signed, LE + XCTAssertTrue(packet.tail.isEmpty) + } + + /// The offset that says the source-closed layout and the layout OBSERVED on hardware are the same + /// record. `rawWaveformStart` was measured on an MG before any of this was decoded; it has to equal + /// where the R17 sample block begins, or one of the two readings is wrong. + func testTheObservedWaveformOffsetEqualsTheR17SampleStart() { + XCTAssertEqual(Whoop5Ecg.rawWaveformStart, + Whoop5Ecg.rawTypeOffset + Whoop5Ecg.r17SampleStart) + } + + func testR17FlagsDecodeBitByBit() { + XCTAssertTrue(EcgLabradorFlags(raw: 0x01).enteringStateOne) + XCTAssertTrue(EcgLabradorFlags(raw: 0x02).currentStateOne) + XCTAssertTrue(EcgLabradorFlags(raw: 0x04).stateTransitionOneToTwo) + XCTAssertTrue(EcgLabradorFlags(raw: 0x08).presence) + XCTAssertFalse(EcgLabradorFlags(raw: 0x07).presence) + // 0x0c is the terminal frame's physically observed value: transition set, presence set. + let terminal = EcgLabradorFlags(raw: 0x0C) + XCTAssertTrue(terminal.stateTransitionOneToTwo) + XCTAssertTrue(terminal.presence) + XCTAssertFalse(terminal.currentStateOne) + } + + /// The bug the whole layout correction is about, pinned as a value. + /// + /// The superseded triage required `payload[4] <= 1 && payload[5] <= 1`. Those two bytes are this + /// record's quality (0...3) and flags. A packet reporting electrode contact has `0x08` set in flags, + /// so it could never satisfy that check — every packet carrying a real reading was discarded. + func testAPresencePositivePacketIsAcceptedNow() { + let inner = r17Inner(quality: 3, flags: 0x0A, samples: 2) + i16le([1, 2]) + let packet = Whoop5Ecg.parseR17(inner: inner) + XCTAssertNotNil(packet, "a contact-positive packet must decode") + XCTAssertTrue(packet?.presence == true) + XCTAssertGreaterThan(Int(packet?.flags.raw ?? 0), 1, "precondition: the old triage capped this at 1") + XCTAssertGreaterThan(Int(packet?.signalQualityRaw ?? 0), 1, "precondition: same for quality") + } + + func testR17UnreadableMaskNamesItsBits() { + XCTAssertEqual(EcgUnreadableMask(raw: 0x00).reasons, []) + XCTAssertEqual(EcgUnreadableMask(raw: 0x01).reasons, ["low_amplitude"]) + XCTAssertEqual(EcgUnreadableMask(raw: 0x0F).reasons, + ["low_amplitude", "significant_noise", "unstable_signal", "not_enough_data"]) + // An unmapped bit is reported as unknown, never folded onto a named reason. + XCTAssertEqual(EcgUnreadableMask(raw: 0x10).reasons, ["unknown_bits_0x10"]) + } + + func testR17TerminalAndInvalidPredicates() { + let running = Whoop5Ecg.parseR17(inner: r17Inner(state: 1, progress: 42, samples: 0)) + XCTAssertEqual(running?.isTerminal, false) + XCTAssertEqual(running?.isInvalid, false) + // Either condition alone is terminal. + XCTAssertEqual(Whoop5Ecg.parseR17(inner: r17Inner(state: 1, progress: 100, samples: 0))?.isTerminal, true) + XCTAssertEqual(Whoop5Ecg.parseR17(inner: r17Inner(state: 2, progress: 42, samples: 0))?.isTerminal, true) + // 255 is the abort sentinel, and it is NOT a completion. + let invalid = Whoop5Ecg.parseR17(inner: r17Inner(state: 1, progress: 255, samples: 0)) + XCTAssertEqual(invalid?.isInvalid, true) + XCTAssertEqual(invalid?.isTerminal, false) + } + + func testR17VariabilitySentinelBecomesNil() { + XCTAssertNil(Whoop5Ecg.parseR17(inner: r17Inner(variability: 0xFFFF, samples: 0))?.variabilityRaw) + XCTAssertEqual(Whoop5Ecg.parseR17(inner: r17Inner(variability: 0xFFFE, samples: 0))?.variabilityRaw, 0xFFFE) + } + + func testR17CarriesTrailingBytesAsTail() { + let inner = r17Inner(samples: 2) + i16le([5, -5]) + [0x00, 0x00, 0x00] + let packet = Whoop5Ecg.parseR17(inner: inner) + XCTAssertEqual(packet?.samples, [5, -5]) + XCTAssertEqual(packet?.tail, [0x00, 0x00, 0x00]) + } + + func testR17ZeroSamplesIsValid() { + // A status-only packet (no contact, nothing captured yet) is legitimate, not an error. + let packet = Whoop5Ecg.parseR17(inner: r17Inner(flags: 0x00, samples: 0)) XCTAssertNotNil(packet) - XCTAssertEqual(packet?.filteredECGDataRaw, []) - XCTAssertEqual(packet?.header.heartKeyLeadsAreOn, false) + XCTAssertEqual(packet?.samples, []) + XCTAssertEqual(packet?.presence, false) } - func testFilteredRejectsShortHeader() { - for count in 0.. 0 { inner[0] = 43 } + if count > 1 { inner[1] = 17 } + XCTAssertNil(Whoop5Ecg.parseR17(inner: inner), "\(count)-byte record must not decode") } } - func testFilteredRejectsSampleCountLongerThanBuffer() { - // numberOfECGSamples says 10; only 3 samples are present. Fail closed — never a partial decode. - let payload = header(samples: 10) + i16le([1, 2, 3]) - XCTAssertNil(Whoop5Ecg.decodeFiltered(payload: payload)) + func testR17RejectsTheWrongTypeOrRevision() { + XCTAssertNil(Whoop5Ecg.parseR17(inner: r17Inner(type: 47, samples: 0)), "stored needs allowStored") + XCTAssertNotNil(Whoop5Ecg.parseR17(inner: r17Inner(type: 47, samples: 0), allowStored: true)) + XCTAssertNil(Whoop5Ecg.parseR17(inner: r17Inner(type: 40, samples: 0), allowStored: true)) + // Revision 16 is the RAW record and shares the type byte. Reading it as an R17 would invent + // fields, so the revision check is what keeps the two apart. + XCTAssertNil(Whoop5Ecg.parseR17(inner: r17Inner(revision: 16, samples: 0))) + } + + func testR17RejectsSampleCountLongerThanBuffer() { + // sampleCount says 10; only 3 samples are present. Fail closed — never a partial decode. + XCTAssertNil(Whoop5Ecg.parseR17(inner: r17Inner(samples: 10) + i16le([1, 2, 3]))) } - func testFilteredRejectsSampleCountOffByOneByte() { + func testR17RejectsSampleCountOffByOneByte() { // One byte short of the declared 4 samples: the array must not be silently truncated to 3. - let payload = header(samples: 4) + i16le([1, 2, 3]) + [0x07] - XCTAssertNil(Whoop5Ecg.decodeFiltered(payload: payload)) + XCTAssertNil(Whoop5Ecg.parseR17(inner: r17Inner(samples: 4) + i16le([1, 2, 3]) + [0x07])) + } + + func testR17RejectsMoreSamplesThanTheWireCanCarry() { + // 100 is the physical capacity. A count above it is a corrupt or misread record, not a big packet. + let over = Int(Whoop5Ecg.r17MaxSamples) + 1 + let inner = r17Inner(samples: UInt16(over)) + [UInt8](repeating: 0, count: over * 2) + XCTAssertNil(Whoop5Ecg.parseR17(inner: inner)) + let atLimit = r17Inner(samples: UInt16(Whoop5Ecg.r17MaxSamples)) + + [UInt8](repeating: 0, count: Whoop5Ecg.r17MaxSamples * 2) + XCTAssertNotNil(Whoop5Ecg.parseR17(inner: atLimit), "exactly 100 is on the wire, not over it") } - func testFilteredExtraSamplesBeyondTheCountBecomePadding() { - // The count is authoritative: bytes past it are padding, never extra samples. - let payload = header(samples: 2) + i16le([9, 9, 9, 9]) - let packet = Whoop5Ecg.decodeFiltered(payload: payload) - XCTAssertEqual(packet?.filteredECGDataRaw.count, 2) - XCTAssertEqual(packet?.padding.count, 4) + func testR17ExtraSamplesBeyondTheCountBecomeTail() { + // The count is authoritative: bytes past it are tail, never extra samples. + let packet = Whoop5Ecg.parseR17(inner: r17Inner(samples: 2) + i16le([9, 9, 9, 9])) + XCTAssertEqual(packet?.samples.count, 2) + XCTAssertEqual(packet?.tail.count, 4) } // MARK: - Enum coverage @@ -137,58 +272,40 @@ final class Whoop5EcgTests: XCTestCase { ] XCTAssertEqual(expected.count, EcgArrhythmiaCheckResult.allCases.count) for (raw, expectedCase, token) in expected { - let payload = header(arrhythmiaResult: raw, samples: 1) + i16le([0]) - let packet = Whoop5Ecg.decodeFiltered(payload: payload) - XCTAssertEqual(packet?.header.heartKeyArrhythmiaCheckResult, expectedCase, "raw \(raw)") - XCTAssertEqual(packet?.header.heartKeyArrhythmiaCheckResultRaw, raw) + let packet = Whoop5Ecg.parseR17(inner: r17Inner(result: raw, samples: 1) + i16le([0])) + XCTAssertEqual(packet?.arrhythmiaCheckResult, expectedCase, "raw \(raw)") + XCTAssertEqual(packet?.arrhythmiaCheckResultRaw, raw) XCTAssertEqual(expectedCase.token, token) } } func testUnknownArrhythmiaResultIsCarriedRawNotCoerced() { // A firmware value outside the table must NOT be folded onto a known case. - let payload = header(arrhythmiaResult: 200, samples: 1) + i16le([0]) - let packet = Whoop5Ecg.decodeFiltered(payload: payload) - XCTAssertNil(packet?.header.heartKeyArrhythmiaCheckResult) - XCTAssertEqual(packet?.header.heartKeyArrhythmiaCheckResultRaw, 200) - } - - func testEveryArrhythmiaCheckStatusCaseDecodes() { - let expected: [(UInt8, EcgArrhythmiaCheckStatus)] = [ - (0, .notRunning), (1, .inProgress), (2, .checkComplete), - ] - XCTAssertEqual(expected.count, EcgArrhythmiaCheckStatus.allCases.count) - for (raw, expectedCase) in expected { - let payload = header(arrhythmiaStatus: raw, samples: 0) - XCTAssertEqual(Whoop5Ecg.decodeFiltered(payload: payload)?.header.heartKeyArrhythmiaCheckStatus, - expectedCase) - } - let bad = header(arrhythmiaStatus: 9, samples: 0) - XCTAssertNil(Whoop5Ecg.decodeFiltered(payload: bad)?.header.heartKeyArrhythmiaCheckStatus) - XCTAssertEqual(Whoop5Ecg.decodeFiltered(payload: bad)?.header.heartKeyArrhythmiaCheckStatusRaw, 9) + let packet = Whoop5Ecg.parseR17(inner: r17Inner(result: 200, samples: 1) + i16le([0])) + XCTAssertNil(packet?.arrhythmiaCheckResult) + XCTAssertEqual(packet?.arrhythmiaCheckResultRaw, 200) } func testEverySignalQualityCaseDecodes() { for quality in EcgSignalQuality.allCases { - let payload = header(signalQuality: quality.rawValue, samples: 0) - XCTAssertEqual(Whoop5Ecg.decodeFiltered(payload: payload)?.header.signalQuality, quality) + XCTAssertEqual(Whoop5Ecg.parseR17(inner: r17Inner(quality: quality.rawValue, samples: 0))?.signalQuality, + quality) } // Out of range falls back to .unknown but keeps the raw byte. - let payload = header(signalQuality: 77, samples: 0) - XCTAssertEqual(Whoop5Ecg.decodeFiltered(payload: payload)?.header.signalQuality, .unknown) - XCTAssertEqual(Whoop5Ecg.decodeFiltered(payload: payload)?.header.signalQualityRaw, 77) + let packet = Whoop5Ecg.parseR17(inner: r17Inner(quality: 77, samples: 0)) + XCTAssertEqual(packet?.signalQuality, .unknown) + XCTAssertEqual(packet?.signalQualityRaw, 77) } func testProgressPercentInRangeAndRawOutside() { for value: UInt8 in [0, 1, 50, 99, 100] { - let payload = header(progress: value, samples: 0) - XCTAssertEqual(Whoop5Ecg.decodeFiltered(payload: payload)?.header.heartKeyProgress, .percent(value)) + XCTAssertEqual(Whoop5Ecg.parseR17(inner: r17Inner(progress: value, samples: 0))?.progress, + .percent(value)) } - // 101...255 is out of percentage range. The source type has a "timed out" case, but its sentinel - // value is not attested, so the byte is carried raw rather than renamed into a state we can't prove. + // 101...255 is out of percentage range. 255 is the strap's abort sentinel; the rest have no + // attested meaning, so the byte is carried raw rather than renamed into a state we can't prove. for value: UInt8 in [101, 200, 255] { - let payload = header(progress: value, samples: 0) - let progress = Whoop5Ecg.decodeFiltered(payload: payload)?.header.heartKeyProgress + let progress = Whoop5Ecg.parseR17(inner: r17Inner(progress: value, samples: 0))?.progress XCTAssertEqual(progress, .unmapped(value)) XCTAssertNil(progress?.percentValue) XCTAssertEqual(progress?.raw, value) @@ -287,45 +404,61 @@ final class Whoop5EcgTests: XCTestCase { // MARK: - Frame level (CRC gating) - func testFilteredFrameDecodesThroughAValidPuffinEnvelope() { + func testR17DecodesThroughAValidPuffinEnvelope() { let samples: [Int16] = [10, -10, 300] - let payload = header(samples: UInt16(samples.count)) + i16le(samples) - let frame = puffinFrame(type: 0x28, payload: payload) + let frame = r17Frame(r17Inner(samples: UInt16(samples.count)) + i16le(samples)) XCTAssertTrue(verifyFrame(frame, family: .whoop5).ok) - let packet = Whoop5Ecg.decodeFilteredFrame(frame) - XCTAssertEqual(packet?.filteredECGDataRaw, samples) - XCTAssertEqual(packet?.header.heartKeyHR, 63) + let packet = Whoop5Ecg.r17FromFrame(frame) + XCTAssertEqual(packet?.samples, samples) + XCTAssertEqual(packet?.liveHR, 63) + // The record starts at frame[8], so the samples land at frame[34] — the offset observed on an MG. + XCTAssertEqual(frame[Whoop5Ecg.rawTypeOffset], 43) + XCTAssertEqual(frame[Whoop5Ecg.rawTypeOffset + 1], 17) + // `tail` is record bytes only. Slicing the frame to its END instead of to the declared length + // would put the four-byte CRC32 trailer in here and call it part of the record. + XCTAssertTrue(packet?.tail.isEmpty == true, "the CRC32 trailer is envelope, not tail") } - func testFilteredFrameRejectsBadCRC32() { - let payload = header(samples: 2) + i16le([1, 2]) - var frame = puffinFrame(type: 0x28, payload: payload) + /// The same guarantee with a tail that genuinely exists, so an empty-tail assertion cannot pass for + /// the wrong reason. + /// + /// Two samples, not one: the inner record is padded to a 4-byte boundary, so an odd sample count + /// puts pad bytes in `tail` too and the exact-bytes assertion stops being about the CRC at all. + /// 26 fixed + 4 sample + 2 trailing = 32, already aligned. + func testTailStopsAtTheCrcTrailer() { + let frame = r17Frame(r17Inner(samples: 2) + i16le([1, 2]) + [0xAB, 0xCD]) + let packet = Whoop5Ecg.r17FromFrame(frame) + XCTAssertEqual(packet?.tail, [0xAB, 0xCD]) + // Decisive: the four CRC32 bytes the envelope ends with are absent from the record's tail. + XCTAssertFalse(packet?.tail.suffix(4).elementsEqual(frame.suffix(4)) == true) + } + + func testR17FrameRejectsBadCRC32() { + var frame = r17Frame(r17Inner(samples: 2) + i16le([1, 2])) frame[frame.count - 1] ^= 0xFF // corrupt the CRC32 trailer XCTAssertFalse(verifyFrame(frame, family: .whoop5).ok) - XCTAssertNil(Whoop5Ecg.decodeFilteredFrame(frame), "a bad CRC must never reach a field read") + XCTAssertNil(Whoop5Ecg.r17FromFrame(frame), "a bad CRC must never reach a field read") } - func testFilteredFrameRejectsBadHeaderCRC16() { - let payload = header(samples: 2) + i16le([1, 2]) - var frame = puffinFrame(type: 0x28, payload: payload) + func testR17FrameRejectsBadHeaderCRC16() { + var frame = r17Frame(r17Inner(samples: 2) + i16le([1, 2])) frame[6] ^= 0xFF // corrupt the CRC16 header check XCTAssertFalse(verifyFrame(frame, family: .whoop5).ok) - XCTAssertNil(Whoop5Ecg.decodeFilteredFrame(frame)) + XCTAssertNil(Whoop5Ecg.r17FromFrame(frame)) } - func testFilteredFrameRejectsCorruptedBodyThatBreaksCRC() { - let payload = header(samples: 2) + i16le([1, 2]) - var frame = puffinFrame(type: 0x28, payload: payload) - frame[12] ^= 0x01 // flip a status bit; CRC32 no longer matches + func testR17FrameRejectsCorruptedBodyThatBreaksCRC() { + var frame = r17Frame(r17Inner(samples: 2) + i16le([1, 2])) + frame[12] ^= 0x01 // flip a field byte; CRC32 no longer matches XCTAssertFalse(verifyFrame(frame, family: .whoop5).ok) - XCTAssertNil(Whoop5Ecg.decodeFilteredFrame(frame)) + XCTAssertNil(Whoop5Ecg.r17FromFrame(frame)) } - func testFrameRejectsGarbageAndShortInput() { - XCTAssertNil(Whoop5Ecg.decodeFilteredFrame([])) - XCTAssertNil(Whoop5Ecg.decodeFilteredFrame([0xAA, 0x01, 0x00])) - XCTAssertNil(Whoop5Ecg.decodeFilteredFrame([UInt8](repeating: 0xFF, count: 64))) + func testR17FrameRejectsGarbageAndShortInput() { + XCTAssertNil(Whoop5Ecg.r17FromFrame([])) + XCTAssertNil(Whoop5Ecg.r17FromFrame([0xAA, 0x01, 0x00])) + XCTAssertNil(Whoop5Ecg.r17FromFrame([UInt8](repeating: 0xFF, count: 64))) } func testRawFrameDecodesThroughAValidPuffinEnvelope() { @@ -344,36 +477,49 @@ final class Whoop5EcgTests: XCTestCase { XCTAssertNil(Whoop5Ecg.decodeRawFrame(frame, bytesPerSample: 2)) } - // MARK: - Structural triage (packet-type discovery) + // MARK: - Packet recognition - func testPlausibleFilteredPayloadAcceptsAWellFormedPacket() { - let payload = header(samples: 3) + i16le([1, 2, 3]) - XCTAssertTrue(Whoop5Ecg.plausibleFilteredPayload(payload)) + func testIsLabradorR17FrameAcceptsAWellFormedRecord() { + XCTAssertTrue(r17IsRecognised(r17Frame(r17Inner(samples: 3) + i16le([1, 2, 3])))) } - func testPlausibleFilteredPayloadRejectsNonBooleanFlagBytes() { - let payload = header(started: 7, samples: 3) + i16le([1, 2, 3]) - XCTAssertFalse(Whoop5Ecg.plausibleFilteredPayload(payload)) + /// What the superseded heuristic got wrong, as a test rather than a comment. + /// + /// It read a status block from `frame[11]` and required four of those bytes to be 0 or 1. Two of them + /// are this record's quality and flags bytes. A packet with electrode contact carries `0x08` in + /// flags, so the triage rejected exactly the packets a reading is made of. The recogniser that + /// replaced it keys on the record's own type and revision instead. + func testRecognitionDoesNotDependOnTheBytesTheOldHeuristicCapped() { + for flags: UInt8 in [0x00, 0x02, 0x08, 0x0A, 0x0C] { + for quality: UInt8 in [0, 1, 2, 3] { + let frame = r17Frame(r17Inner(quality: quality, flags: flags, samples: 2) + i16le([1, 2])) + XCTAssertTrue(r17IsRecognised(frame), + "flags=0x\(String(flags, radix: 16)) quality=\(quality) must be recognised") + } + } } - func testPlausibleFilteredPayloadRejectsOutOfRangeEnums() { - XCTAssertFalse(Whoop5Ecg.plausibleFilteredPayload(header(signalQuality: 9, samples: 2) + i16le([1, 2]))) - XCTAssertFalse(Whoop5Ecg.plausibleFilteredPayload(header(arrhythmiaResult: 9, samples: 2) + i16le([1, 2]))) - XCTAssertFalse(Whoop5Ecg.plausibleFilteredPayload(header(arrhythmiaStatus: 9, samples: 2) + i16le([1, 2]))) + func testIsLabradorR17FrameRejectsOtherRecords() { + // Right type, wrong revision: the RAW (revision 16) record must not be read as filtered. + XCTAssertFalse(r17IsRecognised(r17Frame(r17Inner(revision: 16, samples: 2) + i16le([1, 2])))) + // Right revision, a type nothing in this family uses. + XCTAssertFalse(r17IsRecognised(r17Frame(r17Inner(type: 40, samples: 2) + i16le([1, 2])))) + // Stored records need to be asked for. + let stored = r17Frame(r17Inner(type: 47, samples: 2) + i16le([1, 2])) + XCTAssertFalse(r17IsRecognised(stored)) + XCTAssertTrue(r17IsRecognised(stored, allowStored: true)) } - func testPlausibleFilteredPayloadRejectsLengthDisagreement() { - XCTAssertFalse(Whoop5Ecg.plausibleFilteredPayload(header(samples: 40) + i16le([1, 2]))) - // Far more trailing bytes than the pad4 budget → not this layout. - XCTAssertFalse(Whoop5Ecg.plausibleFilteredPayload(header(samples: 1) + i16le([1]) + [UInt8](repeating: 0, count: 40))) + func testIsLabradorR17FrameIsCrcGated() { + var frame = r17Frame(r17Inner(samples: 2) + i16le([1, 2])) + XCTAssertTrue(r17IsRecognised(frame), "precondition: intact") + frame[frame.count - 1] ^= 0xFF + XCTAssertFalse(r17IsRecognised(frame), "a bad CRC must not pass recognition") } - func testPlausibleFilteredPayloadRejectsEmptyAndZeroSampleBuffers() { - XCTAssertFalse(Whoop5Ecg.plausibleFilteredPayload([])) - // Zero samples is a VALID packet but a useless triage signal (an all-zero buffer would match), so - // the sniffer deliberately requires at least one sample. - XCTAssertFalse(Whoop5Ecg.plausibleFilteredPayload(header(samples: 0))) - XCTAssertFalse(Whoop5Ecg.plausibleFilteredPayload([UInt8](repeating: 0, count: 64))) + func testIsLabradorR17FrameRejectsEmptyAndGarbage() { + XCTAssertFalse(r17IsRecognised([])) + XCTAssertFalse(r17IsRecognised([UInt8](repeating: 0, count: 64))) } // MARK: - Commands @@ -396,8 +542,11 @@ final class Whoop5EcgTests: XCTestCase { } func testCommandPayloadIsRevisionThenArg() { - XCTAssertEqual(Whoop5Ecg.selectWristPayload(.right), [0x01, 0x00]) - XCTAssertEqual(Whoop5Ecg.selectWristPayload(.left), [0x01, 0x01]) + // LITERAL wire bytes, for the same reason the control-signal pin below spells them out: the old + // right=0/left=1 reading of the client's declaration order was wrong, and a test written through + // `.rawValue` would have moved with the enum and stayed green. + XCTAssertEqual(Whoop5Ecg.selectWristPayload(.right), [0x01, 0x01]) + XCTAssertEqual(Whoop5Ecg.selectWristPayload(.left), [0x01, 0x02]) XCTAssertEqual(Whoop5Ecg.togglePayload(on: true), [0x01, 0x01]) XCTAssertEqual(Whoop5Ecg.togglePayload(on: false), [0x01, 0x00]) XCTAssertEqual(Whoop5Ecg.controlPayload(.stop), [0x01, 0x01]) @@ -409,7 +558,7 @@ final class Whoop5EcgTests: XCTestCase { // must produce byte-identical output, so a test pins the wire form without a strap. let seq: UInt8 = 9 XCTAssertEqual(Whoop5Ecg.selectWristFrame(.left, seq: seq), - puffinCommandFrame(cmd: 0x7B, seq: seq, payload: [0x01, 0x01])) + puffinCommandFrame(cmd: 0x7B, seq: seq, payload: [0x01, 0x02])) XCTAssertEqual(Whoop5Ecg.toggleRealtimeFilteredEcgFrame(on: true, seq: seq), puffinCommandFrame(cmd: 0x8B, seq: seq, payload: [0x01, 0x01])) XCTAssertEqual(Whoop5Ecg.toggleSaveRawEcgFrame(on: false, seq: seq), @@ -538,11 +687,19 @@ final class Whoop5EcgTests: XCTestCase { .ecgCandidatesArrived(packets: 12)) } + /// The hedge MOVED; it did not go away, and this repins it where it now belongs. + /// + /// It used to read "CANDIDATE, not proof", because the packets were found by a shape heuristic that + /// unrelated traffic could match. They are now identified by type and data revision with both CRCs + /// checked, so "these are R17 packets" is a fact and hedging it would be false modesty. What is still + /// unvalidated here is what the SAMPLES mean, and that is what the report must keep saying. + /// + /// The two negative assertions are unchanged: neither wording may conclude the feature is unblocked. func testCandidateVerdictIsHedgedNotAssertedAsProof() { let text = Whoop5EcgProbe.report( steps: [sent(124, arg: 2, .success)], - ecgPacketsSeen: 3, candidateFrames: ["type=0x28 len=220"], windowSeconds: 30) - XCTAssertTrue(text.contains("CANDIDATE, not proof")) + ecgPacketsSeen: 3, candidateFrames: ["type=0x2b len=240 seq=7"], windowSeconds: 30) + XCTAssertTrue(text.contains("still unvalidated here")) // The old wording asserted the conclusion outright; it must not come back. XCTAssertFalse(text.contains("Not blocked")) XCTAssertFalse(text.contains("is ACTIVE")) @@ -800,17 +957,17 @@ final class Whoop5EcgTests: XCTestCase { /// classes the verifier newly rejects must stop here rather than being read as ECG samples. func testTheOrdinaryEcgFrameStillYieldsItsPayload() { let samples: [Int16] = [10, -10, 300] - let frame = puffinFrame(type: 0x28, payload: header(samples: UInt16(samples.count)) + i16le(samples)) + let frame = r17Frame(r17Inner(samples: UInt16(samples.count)) + i16le(samples)) XCTAssertEqual(verifyFrame(frame, family: .whoop5).reason, .none, "precondition: intact") XCTAssertNotNil(Whoop5Ecg.innerPayload(frame), "the useful path must still produce a payload") - XCTAssertEqual(Whoop5Ecg.decodeFilteredFrame(frame)?.filteredECGDataRaw, samples) + XCTAssertEqual(Whoop5Ecg.r17FromFrame(frame)?.samples, samples) } func testAnEcgFrameWithTrailingBytesIsRefused() { - let frame = puffinFrame(type: 0x28, payload: header(samples: 2) + i16le([1, 2])) + [0x00] + let frame = r17Frame(r17Inner(samples: 2) + i16le([1, 2])) + [0x00] XCTAssertEqual(verifyFrame(frame, family: .whoop5).reason, .lengthMismatch) XCTAssertNil(Whoop5Ecg.innerPayload(frame), "trailing bytes mean we do not know where the frame ends") - XCTAssertNil(Whoop5Ecg.decodeFilteredFrame(frame)) + XCTAssertNil(Whoop5Ecg.r17FromFrame(frame)) } func testAnEcgFrameBelowTheFamilyMinimumIsRefused() { diff --git a/Strand/BLE/BLEManager.swift b/Strand/BLE/BLEManager.swift index 8ea4f3b17d..33d1d57470 100644 --- a/Strand/BLE/BLEManager.swift +++ b/Strand/BLE/BLEManager.swift @@ -2337,7 +2337,8 @@ public final class BLEManager: NSObject, ObservableObject { // ONLY while one actually is, so a default install can never form these bytes on a 5/MG — // and the gate is the same state the command is about. Non-destructive: the strap frees // records on our HISTORY_END ack, not on this, so an aborted drain re-offloads intact. - || (command == .abortHistoricalTransmits && backfilling) + // The second disjunct is the ECG START list's member: see `ecgSendAbortHistorical()`. + || (command == .abortHistoricalTransmits && (backfilling || ecgAbortOverride)) // GET_DEVICE_CONFIG_VALUE (121) / GET_FF_VALUE (128) over puffin: the READ-ONLY // device-config READ probe (#103) — it asks for a key's VALUE and writes none. Gated the // same way as 117/118: allowed ONLY while a probe is actually in flight, and the opcode @@ -4323,6 +4324,15 @@ public final class BLEManager: NSObject, ObservableObject { /// True while a probe run is listening. Guards the per-frame triage so it costs nothing otherwise. private var ecgProbeArmed: Bool { ecgProbeDeadline != nil } + /// Set for the length of `ecgSendAbortHistorical()`'s one synchronous send, so the START list's + /// opcode 20 reaches the wire when no offload is draining. + /// + /// The allowlist clause for 20 is `backfilling`, which is the right gate for the abort-a-sync button + /// and the wrong one for a member of the ECG start sequence: that member is sent on every run, and + /// most runs have nothing draining. Scoped exactly like `ecgStopOverride` — one call, one opcode, + /// one body — so a default install still cannot form these bytes. + private var ecgAbortOverride = false + /// Set for the duration of `ecgStopCapture()` only, so the OFF path is reachable even after the /// Experimental opt-in has been switched back off. /// @@ -4422,55 +4432,72 @@ public final class BLEManager: NSObject, ObservableObject { /// `ecgStartCapture()`: it is the only command in this family that changes strap state which outlives /// the session, so the user chooses the wrist explicitly and confirms it on its own. /// - /// The raw values are inferred from the client enum's ORDER, not attested — the UI says so, and - /// re-sending with the other wrist is the whole remedy if the inference is backwards. + /// The raw values are right=1/left=2, corrected against the official parser and firmware constructor. + /// Re-sending with the other wrist remains the whole remedy if a strap disagrees. public func ecgSelectWrist(_ wrist: Whoop5Ecg.WristSelection) { guard ecgGatesAllow() else { return } beginEcgProbeRun(clearingSteps: true) - log("ECG probe: SELECT_WRIST=\(wrist.token) (raw \(wrist.rawValue)) — PERSISTENT strap write; the " - + "right=0/left=1 mapping is inferred from the client enum order, not confirmed on hardware") + log("ECG probe: SELECT_WRIST=\(wrist.token) (raw \(wrist.rawValue)) — PERSISTENT strap write") sendEcgCommand(.selectWrist, arg: wrist.rawValue) scheduleEcgProbeVerdict() } - /// The documented turn-on sequence MINUS `selectWrist` (which the user runs separately, above): - /// toggleRealtimeFilteredECG(on) → toggleSaveRawECG(on) → mainControlECGDataGeneration(start). + /// The documented turn-on sequence MINUS `selectWrist` (which the user runs separately, above). + /// + /// Two lists, in the official order. PREPARE is `139 ON` then `125 ON` (`123` having been sent on its + /// own). START is `20` then `124 = start` — opcode 20 belongs to START, immediately ahead of the + /// generation command, and it is sent on EVERY run rather than only when an offload happens to be + /// draining. public func ecgStartCapture() { guard ecgGatesAllow() else { return } ecgMayBeRunning = true // latched BEFORE the sends, so a mid-sequence drop still leaves Stop offered beginEcgProbeRun(clearingSteps: true) log("ECG probe: starting the ECG turn-on sequence on an MG (experimental, unvalidated instrumentation)") - // Clear a historical drain BEFORE asking for a realtime trace. OpenStrap Edge's prepare step - // sends ABORT_HISTORICAL_TRANSMITS (20) ahead of the wrist selector, and the reason is - // mechanical rather than protocol-deep: an offload already in flight is competing for the same - // link, so a filtered trace requested underneath one can be acked and still never arrive. That - // is indistinguishable, from this side, from `acceptedButSilent` — which is the verdict this - // probe has been returning. - // - // Conditioned on `backfilling` rather than sent unconditionally, which keeps the 5/MG allowlist - // clause for opcode 20 exactly as it is: it admits the opcode only while an offload is actually - // running, so a default install still cannot form these bytes. When nothing is draining there is - // nothing to clear and the abort would be noise. - // - // Deliberately NOT recorded as a probe Step. Steps feed the verdict, and a FAILURE here (an - // abort the firmware declines) would classify the run as `commandRefused` and mask the ECG - // outcome the run exists to establish. `abortBackfill`'s own log carries the diagnostic. - // - // Through `abortBackfill()` rather than writing opcode 20 here, which is the whole point: that - // function sends the same `[0x00]` body the allowlist admits AND calls `exitBackfilling`. The - // raw send alone would stop the strap while leaving `backfilling` true on this side, so the - // session would sit waiting for records that are never coming — the stuck sync the opcode - // exists to prevent. - if backfilling { - log("ECG probe: an offload is in flight and would compete with the realtime trace; aborting it first") - abortBackfill() - } sendEcgCommand(.toggleLabradorFiltered, arg: 1) sendEcgCommand(.toggleLabradorRawSave, arg: 1) + ecgSendAbortHistorical() sendEcgCommand(.toggleLabradorDataGeneration, arg: Whoop5Ecg.ControlSignal.start.rawValue) scheduleEcgProbeVerdict() } + /// The START list's first member: ABORT_HISTORICAL_TRANSMITS (20), immediately ahead of `124`. + /// + /// Unconditional, because that is where it sits in the sequence — not a conditional tidy-up. An + /// offload in flight competes for the same link, so a filtered trace requested underneath one can be + /// acked and still never arrive, which from this side is indistinguishable from `acceptedButSilent` + /// — the verdict this probe has been returning. Sending it only when `backfilling` happened to be + /// true left the common case (no drain running, the strap still holding whatever a previous run left + /// behind) without the clear the sequence calls for. + /// + /// Two routes, because the local bookkeeping differs and only one of them is safe to skip: + /// + /// • Draining: through `abortBackfill()`, which sends the same `[0x00]` body AND calls + /// `exitBackfilling`. The raw send alone would stop the strap while leaving `backfilling` true on + /// this side, so the session would sit waiting for records that are never coming — the stuck sync + /// the opcode exists to prevent. + /// • Not draining: the bare send. There is no session to tear down, and `abortBackfill()` would + /// refuse it outright (`guard backfilling`). + /// + /// Deliberately NOT recorded as a probe Step. Steps feed the verdict, and a FAILURE here (an abort + /// the firmware declines) would classify the run as `commandRefused` and mask the ECG outcome the run + /// exists to establish. Its own log line carries the diagnostic instead. + private func ecgSendAbortHistorical() { + if backfilling { + log("ECG probe: → ABORT_HISTORICAL_TRANSMITS (20) — an offload is in flight and would compete " + + "with the realtime trace, so the local session is torn down with it") + abortBackfill() + return + } + // The allowlist admits opcode 20 while `backfilling` OR while this override is set, exactly as + // `ecgStopOverride` widens it for the OFF path. Set for the length of one synchronous call, so + // nothing outside this line can form these bytes. + ecgAbortOverride = true + defer { ecgAbortOverride = false } + log("ECG probe: → ABORT_HISTORICAL_TRANSMITS (20) — no offload in flight; sent because it is the " + + "START list's first member, not as a tidy-up") + send(.abortHistoricalTransmits, payload: [0x00], writeType: .withResponse) + } + /// The explicit OFF path: stop generation first, then drop both streams. /// /// `reportsResult: false` is the Settings-toggle path — it must not pop the Devices result sheet from @@ -4593,23 +4620,37 @@ public final class BLEManager: NSObject, ObservableObject { /// Structural triage for the packet TYPE the ECG records arrive under, which no table in this repo /// holds. A hit is a CANDIDATE, never a confirmed mapping. private func noteEcgProbeCandidate(_ frame: [UInt8]) { - guard frame.count >= 12, Whoop5Ecg.plausibleFilteredFrame(frame) else { return } + guard frame.count >= 12, let packet = Whoop5Ecg.r17FromFrame(frame) else { return } ecgProbePacketsSeen += 1 - guard ecgProbeCandidates.count < BLEManager.ecgProbeMaxCandidates, - let packet = Whoop5Ecg.decodeFilteredFrame(frame) else { return } - let header = packet.header + guard ecgProbeCandidates.count < BLEManager.ecgProbeMaxCandidates else { return } // The classifier byte is logged as a NUMBER, never as its token name: a strap log is a shareable // artefact, and no line in it should read like a clinical finding. The decoder maps the value // offline; the raw byte is lossless. - let line = String(format: "type=0x%02x len=%d samples=%d quality=%d leadsOn=%d hr=%d hrv=%d " - + "classifierRaw=%d statusRaw=%d (unvalidated instrumentation, not a diagnosis)", - Int(frame[8]), frame.count, Int(header.numberOfECGSamples), - Int(header.signalQualityRaw), header.heartKeyLeadsAreOn ? 1 : 0, - Int(header.heartKeyHR), Int(header.heartKeyHRV), - Int(header.heartKeyArrhythmiaCheckResultRaw), - Int(header.heartKeyArrhythmiaCheckStatusRaw)) - ecgProbeCandidates.append(line) - log("ECG probe: candidate packet \(line)") + // + // `seq` and `progress` are here because the reading is a SEQUENCE, not a set of packets: a gap in + // one or a regression in the other is what tells a contact loss apart from a clean run, and + // neither was visible in the old line. + // The disclaimer is NOT repeated per line. `Whoop5EcgProbe.report` already ends with it and the + // result sheet pins it above the text, so a third copy here was noise — and it forced the notes + // below to land after a closing parenthesis. It also made this string differ from the Kotlin + // twin's, which never carried it, so the two platforms' reports quoted different text. + let line = String(format: "type=0x%02x len=%d seq=%d samples=%d quality=%d presence=%d " + + "progress=%d state=%d hr=%d avgHr=%d var=%d classifierRaw=%d unreadable=0x%02x", + Int(packet.packetType), frame.count, Int(packet.sequence), + Int(packet.sampleCount), Int(packet.signalQualityRaw), + packet.presence ? 1 : 0, Int(packet.progress.raw), Int(packet.classifierState), + Int(packet.liveHR), Int(packet.averageHR), + Int(packet.variabilityRaw ?? 0), Int(packet.arrhythmiaCheckResultRaw), + Int(packet.unreadable.raw)) + // Rendered from the record rather than left to whoever reads the hex: the flag byte and the + // unreadable mask are the two fields that say WHY a run went the way it did, and a strap log is + // usually all there is to go on. `terminal`/`invalid` are the strap's own end conditions. + let state = [packet.isTerminal ? "terminal" : nil, + packet.isInvalid ? "invalid" : nil].compactMap { $0 } + let notes = (packet.flags.tokens + packet.unreadable.reasons + state).joined(separator: ",") + let reported = notes.isEmpty ? line : line + " [\(notes)]" + ecgProbeCandidates.append(reported) + log("ECG probe: ← candidate \(reported) (unvalidated instrumentation, not a diagnosis)") // Dump the raw bytes of the FIRST few candidates so a plain strap-log export is enough to redo // the decode offline — the durable frame recorder is a separate opt-in the user may not have on. // Capped hard: these frames are hundreds of bytes and the log is a scrolling UI list. diff --git a/Strand/Resources/Localizable.xcstrings b/Strand/Resources/Localizable.xcstrings index 3f12966fec..779777483a 100644 --- a/Strand/Resources/Localizable.xcstrings +++ b/Strand/Resources/Localizable.xcstrings @@ -88906,60 +88906,60 @@ } } }, - "It is also not fully confirmed. Which value means “left” and which means “right” is read off the order they appear in WHOOP's own app, not verified on a strap — so it may set the opposite wrist. You can send it again with the other choice at any time, and it changes nothing about your recorded data.": { + "Which value means “left” and which means “right” comes from WHOOP's own app and the strap firmware, not from a strap NOOP has tested. You can send it again with the other choice at any time, and it changes nothing about your recorded data.": { "localizations": { "de": { "stringUnit": { "state": "translated", - "value": "Sie ist außerdem nicht vollständig bestätigt. Welcher Wert „links“ und welcher „rechts“ bedeutet, ist aus der Reihenfolge in WHOOPs eigener App abgelesen und nicht an einem Strap überprüft — es kann also das entgegengesetzte Handgelenk einstellen. Du kannst sie jederzeit erneut mit der anderen Auswahl senden, und an deinen aufgezeichneten Daten ändert das nichts." + "value": "Welcher Wert „links“ und welcher „rechts“ bedeutet, stammt aus WHOOPs eigener App und der Firmware des Straps, nicht von einem Strap, den NOOP getestet hat. Du kannst sie jederzeit erneut mit der anderen Auswahl senden, und an deinen aufgezeichneten Daten ändert das nichts." } }, "es": { "stringUnit": { "state": "translated", - "value": "Tampoco está totalmente confirmado. Qué valor significa «izquierda» y cuál «derecha» se ha deducido del orden en que aparecen en la propia app de WHOOP, y no se ha verificado en una pulsera, así que puede ajustar la muñeca contraria. Puedes enviarlo de nuevo con la otra opción cuando quieras, y no cambia nada de tus datos registrados." + "value": "Qué valor significa «izquierda» y cuál «derecha» procede de la propia app de WHOOP y del firmware de la pulsera, no de una pulsera que NOOP haya probado. Puedes enviarlo de nuevo con la otra opción cuando quieras, y no cambia nada de tus datos registrados." } }, "fr": { "stringUnit": { "state": "translated", - "value": "Ce n'est pas non plus totalement confirmé. Quelle valeur signifie « gauche » et laquelle « droite » est déduite de l'ordre dans lequel elles apparaissent dans l'application de WHOOP, sans vérification sur un bracelet — cela peut donc régler le poignet opposé. Vous pouvez le renvoyer avec l'autre choix à tout moment, et cela ne change rien à vos données enregistrées." + "value": "Quelle valeur signifie « gauche » et laquelle « droite » provient de l'application de WHOOP et du micrologiciel du bracelet, pas d'un bracelet testé par NOOP. Vous pouvez le renvoyer avec l'autre choix à tout moment, et cela ne change rien à vos données enregistrées." } }, "it": { "stringUnit": { "state": "translated", - "value": "Inoltre non è del tutto confermato. Quale valore significhi «sinistro» e quale «destro» è dedotto dall'ordine in cui compaiono nell'app di WHOOP, non verificato su una fascia — quindi potrebbe impostare il polso opposto. Puoi inviarlo di nuovo con l'altra scelta in qualsiasi momento, e non cambia nulla dei tuoi dati registrati." + "value": "Quale valore significhi «sinistro» e quale «destro» proviene dall'app di WHOOP e dal firmware della fascia, non da una fascia testata da NOOP. Puoi inviarlo di nuovo con l'altra scelta in qualsiasi momento, e non cambia nulla dei tuoi dati registrati." } }, "pt-PT": { "stringUnit": { "state": "translated", - "value": "Também não está totalmente confirmado. Que valor significa «esquerdo» e qual significa «direito» foi deduzido da ordem por que aparecem na própria app da WHOOP, não foi verificado numa bracelete — por isso pode definir o pulso oposto. Podes enviá-lo novamente com a outra opção a qualquer momento, e não altera nada nos teus dados registados." + "value": "Que valor significa «esquerdo» e qual significa «direito» vem da própria app da WHOOP e do firmware da bracelete, não de uma bracelete testada pela NOOP. Podes enviá-lo novamente com a outra opção a qualquer momento, e não altera nada nos teus dados registados." } }, "ru": { "stringUnit": { "state": "translated", - "value": "Кроме того, это не полностью подтверждено. Какое значение означает «левое», а какое «правое», выведено из порядка их появления в собственном приложении WHOOP и не проверено на браслете — поэтому может быть задано противоположное запястье. Вы можете отправить это ещё раз с другим вариантом в любой момент, и на записанные данные это никак не влияет." + "value": "Какое значение означает «левое», а какое «правое», взято из собственного приложения WHOOP и прошивки браслета, а не из браслета, проверенного NOOP. Вы можете отправить это ещё раз с другим вариантом в любой момент, и на записанные данные это никак не влияет." } }, "zh-Hans": { "stringUnit": { "state": "translated", - "value": "这一项同样未完全确认。哪个值表示「左」、哪个表示「右」，是根据它们在 WHOOP 自家 App 中出现的顺序推断的，并未在手环上验证——因此可能设置成相反的手腕。你随时可以用另一个选项重新发送，这不会改变你已记录的任何数据。" + "value": "哪个值表示「左」、哪个表示「右」，来自 WHOOP 自家 App 和手环固件，而不是 NOOP 测试过的手环。你随时可以用另一个选项重新发送，这不会改变你已记录的任何数据。" } }, "zh-Hant": { "stringUnit": { "state": "translated", - "value": "這一項同樣未完全確認。哪個值表示「左」、哪個表示「右」，是根據它們在 WHOOP 自家 App 中出現的順序推斷的，並未在手環上驗證——因此可能設定成相反的手腕。你隨時可以用另一個選項重新傳送，這不會改變你已記錄的任何資料。" + "value": "哪個值表示「左」、哪個表示「右」，來自 WHOOP 自家 App 與手環韌體，而不是 NOOP 測試過的手環。你隨時可以用另一個選項重新傳送，這不會改變你已記錄的任何資料。" } }, "pl": { "stringUnit": { "state": "translated", - "value": "To również nie jest w pełni potwierdzone. Która wartość oznacza „lewo”, a która „prawo” jest odczytywana w kolejności, w jakiej pojawiają się we własnej aplikacji WHOOP, a nie sprawdzana na pasku — może więc ustawić przeciwny nadgarstek. W dowolnym momencie możesz wysłać go ponownie, wybierając inną opcję, i nie zmienia to niczego w zarejestrowanych danych." + "value": "Która wartość oznacza „lewo”, a która „prawo”, pochodzi z własnej aplikacji WHOOP i z oprogramowania paska, a nie z paska przetestowanego przez NOOP. W dowolnym momencie możesz wysłać go ponownie, wybierając inną opcję, i nie zmienia to niczego w zarejestrowanych danych." } } } diff --git a/Strand/Screens/DevicesView.swift b/Strand/Screens/DevicesView.swift index 12743f287e..151ae456d0 100644 --- a/Strand/Screens/DevicesView.swift +++ b/Strand/Screens/DevicesView.swift @@ -1477,9 +1477,9 @@ private struct FeatureFlagProbeSheets: ViewModifier { /// the result sheet — isolated for the same iOS type-checker reason as `BodyLocationProbeSheets`. /// /// The wrist selection is deliberately a second, independent confirmation rather than a button inside -/// the start flow: `SELECT_WRIST` writes strap state that survives a disconnect, and the right/left -/// mapping is inferred from the client enum's order rather than confirmed on hardware. A persistent -/// write nobody has verified is exactly the kind of thing that should cost a deliberate extra tap. +/// the start flow: `SELECT_WRIST` writes strap state that survives a disconnect. The right/left mapping +/// is no longer a guess (right=1/left=2, from the official parser and the firmware constructor), but a +/// persistent write NOOP has not verified on its own hardware still costs a deliberate extra tap. private struct EcgProbeSheets: ViewModifier { @EnvironmentObject var model: AppModel @EnvironmentObject var live: LiveState @@ -1566,8 +1566,8 @@ private struct FeatureFlagProbeResultView: View { /// The wrist-selection step: the one ECG command that writes strap state outliving the session, so it /// gets its own screen, its own warning, and its own confirmation rather than a button inside the start -/// flow. The copy names both caveats plainly — that the value persists on the strap, and that the -/// left/right mapping is read off the order in WHOOP's own app rather than verified on hardware. +/// flow. The copy names both facts plainly — that the value persists on the strap, and where the +/// left/right mapping comes from. private struct EcgWristSheet: View { let onPick: (Whoop5Ecg.WristSelection) -> Void let onCancel: () -> Void @@ -1581,7 +1581,7 @@ private struct EcgWristSheet: View { .font(StrandFont.subhead) .foregroundStyle(StrandPalette.statusWarning) .fixedSize(horizontal: false, vertical: true) - Text("It is also not fully confirmed. Which value means “left” and which means “right” is read off the order they appear in WHOOP's own app, not verified on a strap — so it may set the opposite wrist. You can send it again with the other choice at any time, and it changes nothing about your recorded data.") + Text("Which value means “left” and which means “right” comes from WHOOP's own app and the strap firmware, not from a strap NOOP has tested. You can send it again with the other choice at any time, and it changes nothing about your recorded data.") .font(StrandFont.caption) .foregroundStyle(StrandPalette.textSecondary) .fixedSize(horizontal: false, vertical: true) diff --git a/Tools/parity_twin_map.json b/Tools/parity_twin_map.json index 9d9b93204f..4fe84922f8 100644 --- a/Tools/parity_twin_map.json +++ b/Tools/parity_twin_map.json @@ -19,16 +19,16 @@ }, "authority": { "files": {"count": 507, "sha256": "efc6950759de76273adccde8b6457f5e8577bdaf6ed9c6538cde79b0eaf0c612"}, - "functions": {"count": 4517, "sha256": "a4f212d6713372a24e3682e7c18908afbe3fecc5a15ecff8cc380ebe15f12428"}, - "properties": {"count": 465, "sha256": "64fcd18c85bf33a4738d154d45ec86967fdc974e78a448d16e9504d9b13609eb"}, - "constants": {"count": 1973, "sha256": "a6a79a02c8d0db218443a6b01a614ddcaaf078af54ee9e8ee3222bc31624edf7"}, - "file_pairs": {"count": 77, "sha256": "76fcf94bbfaa9af56adc8cbc4ec9cc301c08651d209ae8c910362f2f78fa7828"}, - "function_pairs": {"count": 211, "sha256": "21d8e5c3e88e36d79465cdee41a054bffa51b825532050bace5a14a618fe0f60"}, - "property_pairs": {"count": 150, "sha256": "be7168d309c8a9cf5f3f25438c028cc60ab64202bb6e9c09a6c553e3699ad0e2"}, - "constant_pairs": {"count": 687, "sha256": "15266c829ddd529604cbe1c736f6c75857cba62499273159adfd2bca2d6304f6"}, - "unpaired_files": {"count": 375, "sha256": "ba14f7aa2a3967ebc2aa21311917ae3c0c3db218186fac0a6d297f8969580f1a"}, - "unpaired_functions": {"count": 4108, "sha256": "9ea0f30fafe4d91ed62d44e631eafa9fbbb838eacfd7ae687e2b20e1693d3e4f"}, + "functions": {"count": 4517, "sha256": "05dc56778990d1a6313fc8429b783110ef534ebe49693d91ab7da9c736df040c"}, + "properties": {"count": 491, "sha256": "d91dc8fbf47fdfeb7be4ee410281adfd522565f9abf109c6cd101022169f9037"}, + "constants": {"count": 1986, "sha256": "cb08c3e7a3503cf5a0ec9cd95ff78a68d7215328aa532c98f9046be63d6a8ece"}, + "file_pairs": {"count": 78, "sha256": "ec0d88b4e0535c3b1051b5c10205f21174944018669fbe3479a3c21801a6dc38"}, + "function_pairs": {"count": 215, "sha256": "f4ffaafbdf720d27594002cc2337a7b1be5635ec8d02720b7722f01656e8720b"}, + "property_pairs": {"count": 163, "sha256": "b7d4a29086beec661993370657c0bcdae86f79b221fdc0043206161df5e1117f"}, + "constant_pairs": {"count": 694, "sha256": "ae11bdde04d49f9dfe83170f0aa362e6bf18d3fdc381c1bdf5bbc256caa400ab"}, + "unpaired_files": {"count": 373, "sha256": "2b0c9bc717b301037f735ea7cc32f7a7cd9b4a7887d46550b9e727ffd66d3639"}, + "unpaired_functions": {"count": 4100, "sha256": "cc10b9887488dddae0c270071d3fde36a0c6c764df318e18a27e569e181051a9"}, "unpaired_properties": {"count": 165, "sha256": "fe609dd25384cda6de1616d36753fed68431b1a1f9a73b6fd664c7ff35089955"}, - "unpaired_constants": {"count": 599, "sha256": "b7467c65623ecc296604558a639d62eee2e00874c56d21acc7ec6d3b6264fdc7"} + "unpaired_constants": {"count": 598, "sha256": "59f90c169443ea5b3243b0138e125a6e995d115f64f9c4c4860eabbaababdc6d"} } } 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 29d93ed22d..b96646be43 100644 --- a/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt +++ b/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt @@ -4444,11 +4444,17 @@ class WhoopBleClient( ) && groundTruthImuCommandAllowed) && cmd != CommandNumber.RUN_HAPTICS_PATTERN && cmd != CommandNumber.SEND_HISTORICAL_DATA && cmd != CommandNumber.HISTORICAL_DATA_RESULT && - // ABORT_HISTORICAL_TRANSMITS (20) over puffin: stop an offload already in flight. Allowed - // ONLY while one actually is, so a default install can never form these bytes on a 5/MG — - // and the gate is the same state the command is about. Non-destructive: the strap frees - // records on our HISTORY_END ack, not on this, so an aborted drain re-offloads intact. - !(cmd == CommandNumber.ABORT_HISTORICAL_TRANSMITS && backfilling) && + // ABORT_HISTORICAL_TRANSMITS (20) over puffin: stop an offload already in flight, OR the + // first member of the ECG START list (see [ecgSendAbortHistorical]). Allowed while one of + // those two is true, so a default install can never form these bytes on a 5/MG — the + // first gate is the same state the command is about, and the second is the send burst + // itself. Non-destructive: the strap frees records on our HISTORY_END ack, not on this, + // so an aborted drain re-offloads intact. + // + // NOTE [ecgProbeArmed] here is the SEND-BURST flag, not Apple's listen-window property of + // the same name. Apple scopes its equivalent with a dedicated `ecgAbortOverride`; the two + // admit the same one opcode over the same one call, by different mechanisms. + !(cmd == CommandNumber.ABORT_HISTORICAL_TRANSMITS && (backfilling || ecgProbeArmed)) && // The three MG ECG ("Labrador") TOGGLES (124 / 125 / 139). Gated the HARD way, like // GET_BATTERY_PACK_INFO and unlike the 98/84 read probes: allowed ONLY while an ECG probe // run is actually in flight, so a default install cannot form these bytes at all. Unlike @@ -5255,29 +5261,70 @@ class WhoopBleClient( }, ECG_PROBE_WINDOW_SECONDS * 1000L) } + /** + * The START list's first member: ABORT_HISTORICAL_TRANSMITS (20), immediately ahead of `124`. + * Twin of macOS `ecgSendAbortHistorical`. + * + * Unconditional, because that is where it sits in the sequence — not a conditional tidy-up. An + * offload in flight competes for the same link, so a filtered trace requested underneath one can be + * acked and still never arrive, which from this side is indistinguishable from `acceptedButSilent` — + * the verdict this probe has been returning. Sending it only when [backfilling] happened to be true + * left the common case (no drain running) without the clear the sequence calls for. + * + * Two routes, because the local bookkeeping differs and only one of them is safe to skip: + * + * - Draining: through [abortBackfill], which sends the same `[0x00]` body AND ends the session. The + * raw send alone would stop the strap while leaving [backfilling] true on this side, so the + * session would sit waiting for records that are never coming. + * - Not draining: the bare send. There is no session to tear down, and [abortBackfill] would refuse + * it outright. + * + * Deliberately NOT recorded as a probe step. Steps feed the verdict, and a FAILURE here (an abort the + * firmware declines) would classify the run as `commandRefused` and mask the ECG outcome the run + * exists to establish. Its own log line carries the diagnostic instead. + * + * PARITY NOTE: opcode 123 SELECT_WRIST is the one member of the official PREPARE list Android does + * not send, because Android has no wrist-selection surface. It writes PERSISTENT strap state and so + * needs its own confirmation UI, which macOS has and this platform does not yet. + */ + private fun ecgSendAbortHistorical() { + if (backfilling) { + log( + "ECG probe: → ABORT_HISTORICAL_TRANSMITS (20) — an offload is in flight and would " + + "compete with the realtime trace, so the local session is torn down with it", + ) + abortBackfill() + return + } + log( + "ECG probe: → ABORT_HISTORICAL_TRANSMITS (20) — no offload in flight; sent because it " + + "is the START list's first member, not as a tidy-up", + ) + send(CommandNumber.ABORT_HISTORICAL_TRANSMITS, byteArrayOf(0), withResponse = true) + } + /** * Turn ECG generation ON. Twin of macOS `ecgStartCapture`. * * Order is load-bearing: 139 (filtered) is the master gate, 125 enables the raw save, and 124 takes * an OPERATION byte where `start` is 2. A caller sending 1 here would STOP a session it just armed, * which is the shape of every SUCCESS-and-silence report in #891. + * + * Two lists, in the official order. PREPARE is `139 ON` then `125 ON` (`123` is not on Android's + * surface at all — see the parity note on [ecgSendAbortHistorical]). START is `20` then + * `124 = start`: opcode 20 belongs to START, immediately ahead of the generation command, and it is + * sent on EVERY run rather than only when an offload happens to be draining. */ fun ecgStartCapture() { if (!ecgGatesAllow()) return ecgMayBeRunning = true // latched BEFORE the sends, so a mid-sequence drop still offers Stop beginEcgProbeRun() log("ECG probe: starting the ECG turn-on sequence on an MG (experimental, unvalidated instrumentation)") - // Clear a historical drain first: an offload in flight competes for the same link, so a trace - // requested underneath one can be acked and still never arrive. Through abortBackfill() rather - // than opcode 20 directly, because that also tears the local session down. Twin of macOS. - if (backfilling) { - log("ECG probe: an offload is in flight and would compete with the realtime trace; aborting it first") - abortBackfill() - } ecgProbeArmed = true try { sendEcgCommand(CommandNumber.TOGGLE_LABRADOR_FILTERED, 1) sendEcgCommand(CommandNumber.TOGGLE_LABRADOR_RAW_SAVE, 1) + ecgSendAbortHistorical() sendEcgCommand(CommandNumber.TOGGLE_LABRADOR_DATA_GENERATION, Whoop5Ecg.ControlSignal.START.raw) } finally { ecgProbeArmed = false @@ -5330,20 +5377,38 @@ class WhoopBleClient( * artefact and no line in it should read like a clinical finding. */ private fun noteEcgProbeCandidate(frame: ByteArray) { - if (frame.size < 12 || !Whoop5Ecg.plausibleFilteredFrame(frame)) return + if (frame.size < 12) return + val packet = Whoop5Ecg.r17FromFrame(frame) ?: return synchronized(ecgProbeLock) { ecgProbePacketsSeen += 1 if (ecgProbeCandidates.size >= ECG_PROBE_MAX_CANDIDATES) return } - val packet = Whoop5Ecg.decodeFilteredFrame(frame) ?: return - val h = packet.header - val line = "type=0x%02x len=%d samples=%d quality=%d leadsOn=%d hr=%d hrv=%d classifierRaw=%d".format( - frame[8].toInt() and 0xFF, frame.size, h.numberOfECGSamples, h.signalQualityRaw, - if (h.heartKeyLeadsAreOn) 1 else 0, h.heartKeyHR, h.heartKeyHRV, - h.heartKeyArrhythmiaCheckResultRaw, + // The classifier byte is logged as a NUMBER, never as its token name: a strap log is a shareable + // artefact, and no line in it should read like a clinical finding. + // + // `seq` and `progress` are here because the reading is a SEQUENCE, not a set of packets: a gap in + // one or a regression in the other is what tells a contact loss apart from a clean run, and + // neither was visible in the old line. + val line = ( + "type=0x%02x len=%d seq=%d samples=%d quality=%d presence=%d progress=%d state=%d " + + "hr=%d avgHr=%d var=%d classifierRaw=%d unreadable=0x%02x" + ).format( + packet.packetType, frame.size, packet.sequence, packet.sampleCount, packet.signalQualityRaw, + if (packet.presence) 1 else 0, packet.progress.raw, packet.classifierState, + packet.liveHR, packet.averageHR, packet.variabilityRaw ?: 0, + packet.arrhythmiaCheckResultRaw, packet.unreadable.raw, ) - synchronized(ecgProbeLock) { ecgProbeCandidates.add(line) } - log("ECG probe: ← candidate $line (unvalidated instrumentation, not a diagnosis)") + // Rendered from the record rather than left to whoever reads the hex: the flag byte and the + // unreadable mask are the two fields that say WHY a run went the way it did, and a strap log is + // usually all there is to go on. `terminal`/`invalid` are the strap's own end conditions. + val state = buildList { + if (packet.isTerminal) add("terminal") + if (packet.isInvalid) add("invalid") + } + val notes = (packet.flags.tokens + packet.unreadable.reasons + state).joinToString(",") + val reported = if (notes.isEmpty()) line else "$line [$notes]" + synchronized(ecgProbeLock) { ecgProbeCandidates.add(reported) } + log("ECG probe: ← candidate $reported (unvalidated instrumentation, not a diagnosis)") } fun probeFeatureFlags() { @@ -8453,7 +8518,7 @@ class WhoopBleClient( return } - // MG ECG probe: offer every VERIFIED frame to the structural triage while a run is listening. + // MG ECG probe: offer every VERIFIED frame to the R17 decode while a run is listening. // Placed after the verifier so bad bytes can never be counted as a candidate, and gated on the // window so an ordinary frame pays one boolean. Without this the probe reports zero packets // forever and every run reads as "accepted but silent" even while a trace is streaming. diff --git a/android/app/src/main/java/com/noop/protocol/Whoop5Ecg.kt b/android/app/src/main/java/com/noop/protocol/Whoop5Ecg.kt index a7a4254ce5..9597c6b6f4 100644 --- a/android/app/src/main/java/com/noop/protocol/Whoop5Ecg.kt +++ b/android/app/src/main/java/com/noop/protocol/Whoop5Ecg.kt @@ -5,8 +5,12 @@ package com.noop.protocol // // The WHOOP MG carries ECG electrodes in its conductive clasp (a plain WHOOP 5.0 does not — see // Whoop5Variant). The strap's ECG subsystem is called "Labrador" in the protocol tables, and it is a -// SEPARATE realtime data type from the R-numbered StrapSensorData layouts: a FILTERED stream (live) and -// a RAW stream (persisted on the strap for later offload). Both open with the same 17-byte status header. +// SEPARATE realtime data type from the R-numbered StrapSensorData layouts: a FILTERED stream (live, data +// revision 17) and a RAW stream (persisted on the strap for later offload, data revision 16). +// +// They do NOT share a status block. The filtered record's layout is [LabradorR17], read from the inner +// record at the offsets the official parser uses. [EcgStatusHeader] below is the earlier client-derived +// reading; it survives only on the RAW path, whose body layout nothing here can yet confirm. // // Provenance. The four command NUMBERS are already in this repo's protocol table // (WhoopProtocol/Resources/whoop_protocol.json, CommandNumber), from the upstream whoomp/goose work @@ -72,7 +76,14 @@ data class EcgHeartKeyProgress(val raw: Int) { val isMapped: Boolean get() = percentValue != null } -/** The 17-byte status block both Labrador packets open with, in wire order. Multi-byte fields are LE. */ +/** + * The 17-byte status block the RAW (revision 16) record is read with, in wire order. Multi-byte fields + * are LE. + * + * NOT the filtered record's layout, though it was written believing it was: see [LabradorR17]. Whether + * the raw record really opens this way is still unconfirmed, so this stays where it is rather than being + * corrected to something equally unproven. + */ data class EcgStatusHeader( val signalQuality: EcgSignalQuality, /** Raw quality byte, kept so a value outside the known enum is never lost. */ @@ -95,17 +106,110 @@ data class EcgStatusHeader( val numberOfECGSamples: Int, ) +/** R17 `inner[14]` — the on-strap classifier's state/transition bits plus electrode presence. */ +@JvmInline +value class EcgLabradorFlags(val raw: Int) { + /** bit 0 — entering classifier state 1. */ + val enteringStateOne: Boolean get() = raw and 0x01 != 0 + + /** + * bit 1 — the current classifier state IS 1. An ordinary active frame carries this set; a valid + * active frame with it clear is the explicit-restart case, not a contact loss. + */ + val currentStateOne: Boolean get() = raw and 0x02 != 0 + + /** bit 2 — the 1 -> 2 state transition, set on the terminal frame. */ + val stateTransitionOneToTwo: Boolean get() = raw and 0x04 != 0 + + /** + * bit 3 — electrode contact, debounced on the strap. This is the bit `heartKeyLeadsAreOn` was + * reaching for at the wrong offset. + */ + val presence: Boolean get() = raw and 0x08 != 0 + + /** The set bits by name, in bit order. Bits above 3 are reported as unknown rather than named. */ + val tokens: List + get() = buildList { + if (enteringStateOne) add("entering_state_1") + if (currentStateOne) add("state_1") + if (stateTransitionOneToTwo) add("transition_1_2") + if (presence) add("presence") + val unknown = raw and 0x0F.inv() and 0xFF + if (unknown != 0) add("unknown_bits_0x%02x".format(unknown)) + } +} + +/** + * R17 `inner[18]` — why the strap called a reading unreadable. Bits above 3 are reported as unknown + * rather than given a meaning. + */ +@JvmInline +value class EcgUnreadableMask(val raw: Int) { + val lowAmplitude: Boolean get() = raw and 0x01 != 0 + val significantNoise: Boolean get() = raw and 0x02 != 0 + val unstableSignal: Boolean get() = raw and 0x04 != 0 + val notEnoughData: Boolean get() = raw and 0x08 != 0 + + /** The set bits by name, in bit order. */ + val reasons: List + get() = buildList { + if (lowAmplitude) add("low_amplitude") + if (significantNoise) add("significant_noise") + if (unstableSignal) add("unstable_signal") + if (notEnoughData) add("not_enough_data") + val unknown = raw and 0x0F.inv() and 0xFF + if (unknown != 0) add("unknown_bits_0x%02x".format(unknown)) + } +} + /** - * The live ECG stream packet (TOGGLE_LABRADOR_FILTERED, 0x8B). + * One Labrador revision-17 packet — the strap's live filtered-ECG cycle. Twin of Swift `LabradorR17`. * - * The sample UNIT and SCALE are not attested, so the array is named `filteredECGDataRaw` and no µV - * conversion is applied anywhere. + * Offsets are into the INNER record, counted from the packet-type byte. On a 5/MG frame that byte is at + * [Whoop5Ecg.RAW_TYPE_OFFSET] (8), so `inner[k]` is `frame[8 + k]`: fixed fields occupy `inner[0..25]` + * and the samples start at `inner[26]` — `frame[34]`, which is exactly the waveform offset the type-43 + * constants in this file were already observing on hardware. + * + * [samples] are 100 Hz filtered signed i16 little-endian values as transmitted. No rescaling is applied + * and no anatomical lead or polarity is claimed. [variabilityRaw] has no proven unit. */ -data class FilteredLabradorPacket( - val header: EcgStatusHeader, - val filteredECGDataRaw: List, - val padding: List, -) +data class LabradorR17( + /** `inner[0]`: 43 (REALTIME_RAW_DATA, the live path) or 47 (HISTORICAL_DATA, a stored copy). */ + val packetType: Int, + /** `inner[2]` — a packet-context marker the consumer ignores. Kept raw rather than named. */ + val headerSecondary: Int, + val sequence: Long, // inner[3..6] u32 LE + val strapSeconds: Long, // inner[7..10] u32 LE + val subseconds: Int, // inner[11..12] u16 LE, 1/32768 s + val signalQuality: EcgSignalQuality, + val signalQualityRaw: Int, // inner[13] + val flags: EcgLabradorFlags, // inner[14] + /** `inner[15]` — the classifier result code. Null when it maps to no known case. */ + val arrhythmiaCheckResult: EcgArrhythmiaCheckResult?, + val arrhythmiaCheckResultRaw: Int, + val classifierState: Int, // inner[16]; 2 is terminal + val progress: EcgHeartKeyProgress, // inner[17]; 100 terminal, 255 invalid + val unreadable: EcgUnreadableMask, // inner[18] + val averageHR: Int, // inner[19] — the final/stored heart rate + val liveHR: Int, // inner[20] — the current heart rate + /** `inner[21..22]` u16 LE, or null when the wire carried the unavailable sentinel. */ + val variabilityRaw: Int?, + val reserved: Int, // inner[23] + val sampleCount: Int, // inner[24..25] u16 LE + val samples: List, + /** Aligned bytes after the sample block, byte-exact, meaning unassigned. */ + val tail: List, +) { + /** Electrode contact, from the flags byte. */ + val presence: Boolean get() = flags.presence + + /** The strap's completion condition. */ + val isTerminal: Boolean get() = progress.raw == 100 || classifierState == 2 + + /** The strap's invalid/abort sentinel. */ + val isInvalid: Boolean get() = progress.raw == 255 + +} /** * The persisted ECG record (TOGGLE_LABRADOR_RAW_SAVE, 0x7D). @@ -159,11 +263,19 @@ object Whoop5Ecg { /** * Which wrist the strap is worn on. * - * The raw values are INFERRED, not attested: `right` is listed first in the client's enum. Since this - * command writes PERSISTENT strap state, a wrong inference writes a wrong persistent value. + * `RIGHT = 1`, `LEFT = 2` — one-based, NOT the client's zero-based declaration order. + * + * The previous `RIGHT(0) / LEFT(1)` was read off that order and shipped as an acknowledged + * inference. It was wrong in exactly the way [ControlSignal]'s was (#896): the wire values for this + * family start at 1, and 0 is not a member. Corrected against the official Android 5.458.0 Labrador + * parser and the 50.41.1.0 firmware constructor, cross-checked against a third-party implementation + * that drives a physical MG through a complete reading. + * + * This command writes PERSISTENT strap state, so the old values did not merely fail — they wrote a + * wrong persistent selection, or were refused outright, on every strap that ran the probe. */ enum class WristSelection(val raw: Int, val token: String) { - RIGHT(0, "right"), LEFT(1, "left") + RIGHT(1, "right"), LEFT(2, "left") } /** @@ -293,27 +405,78 @@ object Whoop5Ecg { } /** - * Decode a filtered packet from the inner record's PAYLOAD. + * Decode a [LabradorR17] from a complete INNER record — the bytes from the packet-type byte onwards, + * which on a 5/MG frame means `frame[8..]`. Swift twin: `Whoop5Ecg.parseR17`. + * + * Accepts only a type-43 (or, with [allowStored], type-47) record of data revision 17 whose declared + * sample block fits: fixed fields through `inner[25]` present, `sampleCount <= 100`, and + * `26 + 2 * sampleCount` bytes available. No fixed total length is required; bytes past the sample + * block land in `tail`. CRC validity is the caller's business — [r17FromFrame] enforces it. * - * Fails closed on a short header and on a numberOfECGSamples the buffer cannot hold — a count that - * disagrees with the bytes present is a decode error, never a truncated best effort. + * Fails closed throughout: a count that disagrees with the bytes present is a decode error, never a + * truncated best effort. */ - fun decodeFiltered(payload: List): FilteredLabradorPacket? { - val header = decodeHeader(payload) ?: return null - val n = header.numberOfECGSamples - val end = HEADER_LENGTH + n * 2 - if (end > payload.size) return null - val samples = (0 until n).map { i -> - val off = HEADER_LENGTH + i * 2 - val u = payload[off] or (payload[off + 1] shl 8) - if (u >= 0x8000) u - 0x10000 else u // signed 16-bit, LE + fun parseR17(inner: List, allowStored: Boolean = false): LabradorR17? { + if (inner.size < R17_FIXED_LENGTH) return null + for (i in 0 until R17_FIXED_LENGTH) if (inner[i] !in 0..255) return null + val type = inner[0] + if (type != RAW_RECORD_TYPE && !(allowStored && type == STORED_RECORD_TYPE)) return null + if (inner[1] != R17_REVISION) return null + val count = u16le(inner, 24) + if (count > R17_MAX_SAMPLES) return null + val end = R17_SAMPLE_START + count * 2 + if (inner.size < end) return null + for (i in R17_SAMPLE_START until end) if (inner[i] !in 0..255) return null + + val samples = ArrayList(count) + for (i in 0 until count) { + val raw = u16le(inner, R17_SAMPLE_START + i * 2) + samples.add(if (raw >= 0x8000) raw - 0x10000 else raw) } - return FilteredLabradorPacket(header, samples, payload.subList(end, payload.size).toList()) + val variability = u16le(inner, 21) + val quality = inner[13] + return LabradorR17( + packetType = type, + headerSecondary = inner[2], + sequence = u32le(inner, 3), + strapSeconds = u32le(inner, 7), + subseconds = u16le(inner, 11), + signalQuality = EcgSignalQuality.entries.firstOrNull { it.raw == quality } ?: EcgSignalQuality.UNKNOWN, + signalQualityRaw = quality, + flags = EcgLabradorFlags(inner[14]), + arrhythmiaCheckResult = EcgArrhythmiaCheckResult.entries.firstOrNull { it.raw == inner[15] }, + arrhythmiaCheckResultRaw = inner[15], + classifierState = inner[16], + progress = EcgHeartKeyProgress(inner[17]), + unreadable = EcgUnreadableMask(inner[18]), + averageHR = inner[19], + liveHR = inner[20], + variabilityRaw = if (variability == R17_VARIABILITY_UNAVAILABLE) null else variability, + reserved = inner[23], + sampleCount = count, + samples = samples, + tail = inner.subList(end, inner.size).toList(), + ) } - /** CRC-gated decode straight off a complete 5/MG frame. A frame failing either CRC is rejected. */ - fun decodeFilteredFrame(frame: ByteArray, payloadStart: Int = PUFFIN_PAYLOAD_START): FilteredLabradorPacket? = - innerPayload(frame, payloadStart)?.let { decodeFiltered(it) } + /** Swift twin: `Whoop5Ecg.u16le`. */ + private fun u16le(b: List, i: Int): Int = b[i] or (b[i + 1] shl 8) + + /** Swift twin: `Whoop5Ecg.u32le`. */ + private fun u32le(b: List, i: Int): Long = + (b[i].toLong()) or (b[i + 1].toLong() shl 8) or (b[i + 2].toLong() shl 16) or (b[i + 3].toLong() shl 24) + + /** + * [parseR17] straight off a complete 5/MG frame, CRC-gated. The frame must pass the central verifier + * before any field is read, per the BLE safety contract. Swift twin: `Whoop5Ecg.r17FromFrame`. + */ + fun r17FromFrame(frame: ByteArray, allowStored: Boolean = false): LabradorR17? { + // Through [innerPayload] with the INNER record's own start offset, not `frame.drop(8)`. It is the + // one seam that both CRC-gates the frame and stops at the CRC32 trailer; slicing to the end of + // the buffer instead would hand four envelope bytes to `tail` and call them record bytes. + val inner = innerPayload(frame, RAW_TYPE_OFFSET) ?: return null + return parseR17(inner, allowStored) + } // Decode — raw @@ -389,10 +552,12 @@ object Whoop5Ecg { // layer, so the three consumers (live view, signal classifier, waveform export) cannot drift apart and // so the decode is unit-testable without a strap. // - // What this layout is NOT: the documented 17-byte status header ([HEADER_LENGTH]) is the FILTERED - // packet's, and it is a separate question whether it also sits inside this record (see #891 §7). Nothing - // here decodes a status field, an HR, or a rhythm classification — only the sample series and a byte-fill - // count. A record that fails CRC must never reach these helpers: callers gate on the frame's CRC first. + // What this layout is NOT: [HEADER_LENGTH] belongs to the RAW record, not to this one. The record + // these helpers walk IS the filtered R17 — [RAW_WAVEFORM_START] equals `RAW_TYPE_OFFSET + + // R17_SAMPLE_START`, which is how the offset measured here and the source-closed layout were shown to + // describe the same bytes. These helpers stay sample-only on purpose: nothing here decodes a status + // field, an HR, or a rhythm classification, because [parseR17] is where that belongs. A record that + // fails CRC must never reach them: callers gate on the frame's CRC first. /** Every REALTIME_RAW_DATA record OBSERVED on the MG was exactly this long. */ const val RAW_RECORD_LENGTH = 240 @@ -400,6 +565,38 @@ object Whoop5Ecg { /** Frame offset of the inner record's type byte on 5/MG (`[8]type [9]seq [10]cmd`). */ const val RAW_TYPE_OFFSET = 8 + /** The inner record type byte for REALTIME_RAW_DATA. */ + const val RAW_RECORD_TYPE = 43 + + /** + * The inner record type byte for HISTORICAL_DATA — a STORED R17, which the live turn-on path never + * enables. [parseR17] accepts it only when asked to. + */ + const val STORED_RECORD_TYPE = 47 + + // The revision-17 layout. + // + // Offsets into the INNER record, from the packet-type byte. `inner[k]` is `frame[RAW_TYPE_OFFSET + k]`. + // Source-closed against the official Android 5.458.0 Labrador parser and the 50.41.1.0 firmware + // constructor, and corroborated here by [RAW_WAVEFORM_START]: the waveform offset OBSERVED on an MG + // (34) is exactly `RAW_TYPE_OFFSET + R17_SAMPLE_START`, which is what says these two readings of the + // same record agree. + + /** `inner[1]` — the data revision that makes a record an R17. */ + const val R17_REVISION = 17 + + /** Fixed fields occupy `inner[0..25]`; the sample block follows. */ + const val R17_FIXED_LENGTH = 26 + + /** First sample byte, `inner[26]`. */ + const val R17_SAMPLE_START = 26 + + /** Wire capacity: 100 i16 samples per packet. */ + const val R17_MAX_SAMPLES = 100 + + /** `0xffff` at `inner[21..22]` means the variability value is unavailable. */ + const val R17_VARIABILITY_UNAVAILABLE = 0xFFFF + /** First body byte considered by [realtimeRawBodyNonZeroBytes] — excludes the constant sub-header. */ const val RAW_BODY_START = 24 @@ -466,38 +663,6 @@ object Whoop5Ecg { // Discovery - /** - * The frame-level form of [plausibleFilteredPayload], CRC-gated. This is what the app layer runs over - * unclassified 5/MG frames while an ECG probe is armed. Twin of the Swift helper. - */ - fun plausibleFilteredFrame( - frame: ByteArray, - payloadStart: Int = PUFFIN_PAYLOAD_START, - maxPadding: Int = DEFAULT_MAX_PADDING, - ): Boolean { - val payload = innerPayload(frame, payloadStart) ?: return false - return plausibleFilteredPayload(payload, maxPadding) - } - - /** - * A cheap structural triage for "could these bytes be a filtered Labrador payload?". - * - * Used by the app layer to hunt for the packet TYPE byte, which is not attested: while an ECG probe is - * armed, every unclassified 5/MG frame is run through this and the hits are logged with their type. It - * is a HEURISTIC — four booleans, three enum ranges and a length agreement — not a classifier, and - * nothing downstream may treat a hit as proof. - */ - fun plausibleFilteredPayload(payload: List, maxPadding: Int = DEFAULT_MAX_PADDING): Boolean { - val header = decodeHeader(payload) ?: return false - if (header.signalQualityRaw > 3) return false - if (header.heartKeyArrhythmiaCheckResult == null) return false - if (header.heartKeyArrhythmiaCheckStatus == null) return false - if (payload[2] > 1 || payload[3] > 1 || payload[4] > 1 || payload[5] > 1) return false - val n = header.numberOfECGSamples - if (n <= 0) return false - val end = HEADER_LENGTH + n * 2 - return end <= payload.size && payload.size - end <= maxPadding - } /** * The inner record's payload from a complete 5/MG frame, or null when the frame fails the envelope diff --git a/android/app/src/main/java/com/noop/protocol/Whoop5EcgProbe.kt b/android/app/src/main/java/com/noop/protocol/Whoop5EcgProbe.kt index 8fe948f106..9bbd52afe1 100644 --- a/android/app/src/main/java/com/noop/protocol/Whoop5EcgProbe.kt +++ b/android/app/src/main/java/com/noop/protocol/Whoop5EcgProbe.kt @@ -167,9 +167,10 @@ object Whoop5EcgProbe { val headline: String get() = when (this) { is EcgCandidatesArrived -> - "$packets frame(s) matched the ECG structural triage. That is a CANDIDATE, not proof: " + - "the triage is a shape heuristic and unrelated traffic can match it. Confirm against " + - "the raw bytes below before concluding anything about whether the feature is blocked." + "$packets revision-17 ECG packet(s) decoded. The strap IS producing the filtered " + + "trace: each one arrived as type 43 revision 17 with both CRCs intact. What the " + + "samples MEAN is still unvalidated here — check the per-packet progress and presence " + + "below to see whether a reading actually advanced, rather than reading the count alone." is DataRequestRefused -> "DATA REQUEST REFUSED — the firmware returned FAILURE for " + "${commands.joinToString(", ")}, which asked it to produce ECG data: it knows the " + @@ -246,8 +247,9 @@ object Whoop5EcgProbe { /** * The full report: verdict, per-command outcomes, the ECG-packet tally, and the raw replies. * - * [candidateFrames] are the type/length lines for frames that passed the structural triage in - * [Whoop5Ecg.plausibleFilteredPayload]. + * [candidateFrames] are the per-packet lines for frames [Whoop5Ecg.r17FromFrame] decoded — the + * sequence, progress and presence of each accepted record, which is what tells a clean run apart + * from one the electrodes kept dropping. */ fun report( steps: List, @@ -289,9 +291,9 @@ object Whoop5EcgProbe { ) } if (candidateFrames.isEmpty()) { - sb.append("Candidate packet types: none — no frame passed the structural triage.\n") + sb.append("Revision-17 packets decoded: none — no frame arrived as type 43 revision 17.\n") } else { - sb.append("Candidate packet types (structural triage only, NOT a confirmed mapping):\n") + sb.append("Revision-17 packets decoded (type 43, revision 17, both CRCs checked):\n") for (line in candidateFrames) sb.append(" $line\n") } val replies = steps.mapNotNull { step -> step.replyHex?.let { " ${step.label}: $it" } } diff --git a/android/app/src/test/java/com/noop/protocol/Whoop5EcgTest.kt b/android/app/src/test/java/com/noop/protocol/Whoop5EcgTest.kt index faadbfd3ac..dbb979a6f7 100644 --- a/android/app/src/test/java/com/noop/protocol/Whoop5EcgTest.kt +++ b/android/app/src/test/java/com/noop/protocol/Whoop5EcgTest.kt @@ -43,6 +43,66 @@ class Whoop5EcgTest { samples and 0xFF, (samples shr 8) and 0xFF, ) + /** + * A revision-17 INNER record, in wire order: `inner[0]` type, `inner[1]` revision, fixed fields + * through `inner[25]`, then the samples. + * + * Note what `inner[1]` and `inner[2]` are NOT. On a COMMAND frame those offsets are the sequence and + * the opcode, which is what the `[8]type [9]seq [10]cmd` comment in the decoder describes. A DATA + * record reuses the same two bytes for the data revision and a context marker — which is why a + * decoder that started reading fields at `frame[11]` was six bytes off. + */ + private fun r17Inner( + type: Int = 43, + revision: Int = 17, + headerSecondary: Int = 0, + sequence: Long = 7, + strapSeconds: Long = 1_700_000_000, + subseconds: Int = 16_384, + quality: Int = 2, + flags: Int = 0x0A, + result: Int = 0, + state: Int = 1, + progress: Int = 42, + unreadable: Int = 0, + averageHR: Int = 61, + liveHR: Int = 63, + variability: Int = 812, + reserved: Int = 17, + samples: Int, + ): List = buildList { + add(type); add(revision); add(headerSecondary) + for (i in 0 until 4) add(((sequence shr (8 * i)) and 0xFF).toInt()) + for (i in 0 until 4) add(((strapSeconds shr (8 * i)) and 0xFF).toInt()) + add(subseconds and 0xFF); add((subseconds shr 8) and 0xFF) + add(quality); add(flags); add(result); add(state); add(progress); add(unreadable) + add(averageHR); add(liveHR) + add(variability and 0xFF); add((variability shr 8) and 0xFF) + add(reserved) + add(samples and 0xFF); add((samples shr 8) and 0xFF) + } + + /** + * "Does this frame decode as an R17?" — a TEST-local predicate over the product entry point. + * + * Deliberately not a shipped helper: the probe calls [Whoop5Ecg.r17FromFrame] and uses the packet, + * so a separate boolean wrapper in the protocol object would be a declaration nothing runs. + */ + private fun r17IsRecognised(frame: ByteArray, allowStored: Boolean = false): Boolean = + Whoop5Ecg.r17FromFrame(frame, allowStored) != null + + /** + * Wrap an inner record in a valid puffin envelope. The inner record starts at `frame[8]`, so the + * builder takes everything from the type byte onwards. + */ + private fun r17Frame(inner: List): ByteArray = + Framing.puffinCommandFrame( + cmd = if (inner.size > 2) inner[2] else 0, + seq = if (inner.size > 1) inner[1] else 0, + payload = inner.drop(3).map { it.toByte() }.toByteArray(), + type = inner[0], + ) + private fun i16le(values: List): List = values.flatMap { listOf(it and 0xFF, (it shr 8) and 0xFF) } @@ -56,76 +116,168 @@ class Whoop5EcgTest { type = type, ) - // MARK: - Filtered packet + // MARK: - Revision-17 packet @Test - fun filteredDecodesEveryFieldInWireOrder() { + fun r17DecodesEveryFieldInWireOrder() { val samples = listOf(0, 1, -1, 32_767, -32_768, 250, -250) - val payload = header(samples = samples.size) + i16le(samples) - val packet = Whoop5Ecg.decodeFiltered(payload) + val packet = Whoop5Ecg.parseR17(r17Inner(samples = samples.size) + i16le(samples)) assertNotNull(packet) packet!! - assertEquals(EcgSignalQuality.MEDIUM, packet.header.signalQuality) - assertEquals(0x05, packet.header.statusFlags) - assertTrue(packet.header.heartKeyStarted) - assertTrue(packet.header.heartKeyIsRunning) - assertFalse(packet.header.heartKeyIsStoppedAndComplete) - assertTrue(packet.header.heartKeyLeadsAreOn) - assertEquals(EcgArrhythmiaCheckResult.NOT_COMPLETE, packet.header.heartKeyArrhythmiaCheckResult) - assertEquals(EcgArrhythmiaCheckStatus.IN_PROGRESS, packet.header.heartKeyArrhythmiaCheckStatus) - assertEquals(42, packet.header.heartKeyProgress.percentValue) - assertEquals(0, packet.header.heartKeyUnreadableReason) - assertEquals(61, packet.header.heartKeyAverageHR) - assertEquals(63, packet.header.heartKeyHR) - assertEquals(812, packet.header.heartKeyHRV) // u16 LE, not two u8s - assertEquals(17, packet.header.heartKeyStressScore) - assertEquals(7, packet.header.numberOfECGSamples) - assertEquals(samples, packet.filteredECGDataRaw) // signed, LE - assertTrue(packet.padding.isEmpty()) + assertEquals(43, packet.packetType) + assertEquals(0, packet.headerSecondary) + assertEquals(7L, packet.sequence) + assertEquals(1_700_000_000L, packet.strapSeconds) + assertEquals(16_384, packet.subseconds) + assertEquals(EcgSignalQuality.MEDIUM, packet.signalQuality) + assertEquals(2, packet.signalQualityRaw) + assertEquals(0x0A, packet.flags.raw) + assertEquals(EcgArrhythmiaCheckResult.NOT_COMPLETE, packet.arrhythmiaCheckResult) + assertEquals(0, packet.arrhythmiaCheckResultRaw) + assertEquals(1, packet.classifierState) + assertEquals(42, packet.progress.percentValue) + assertEquals(0, packet.unreadable.raw) + assertEquals(61, packet.averageHR) + assertEquals(63, packet.liveHR) + assertEquals(812, packet.variabilityRaw) // u16 LE, not two u8s + assertEquals(17, packet.reserved) + assertEquals(7, packet.sampleCount) + assertEquals(samples, packet.samples) // signed, LE + assertTrue(packet.tail.isEmpty()) + } + + /** + * The offset that says the source-closed layout and the layout OBSERVED on hardware are the same + * record. RAW_WAVEFORM_START was measured on an MG before any of this was decoded; it has to equal + * where the R17 sample block begins, or one of the two readings is wrong. + */ + @Test + fun theObservedWaveformOffsetEqualsTheR17SampleStart() { + assertEquals(Whoop5Ecg.RAW_WAVEFORM_START, Whoop5Ecg.RAW_TYPE_OFFSET + Whoop5Ecg.R17_SAMPLE_START) + } + + @Test + fun r17FlagsDecodeBitByBit() { + assertTrue(EcgLabradorFlags(0x01).enteringStateOne) + assertTrue(EcgLabradorFlags(0x02).currentStateOne) + assertTrue(EcgLabradorFlags(0x04).stateTransitionOneToTwo) + assertTrue(EcgLabradorFlags(0x08).presence) + assertFalse(EcgLabradorFlags(0x07).presence) + // 0x0c is the terminal frame's physically observed value: transition set, presence set. + val terminal = EcgLabradorFlags(0x0C) + assertTrue(terminal.stateTransitionOneToTwo) + assertTrue(terminal.presence) + assertFalse(terminal.currentStateOne) + } + + /** + * The bug the whole layout correction is about, pinned as a value. + * + * The superseded triage required `payload[4] <= 1 && payload[5] <= 1`. Those two bytes are this + * record's quality (0..3) and flags. A packet reporting electrode contact has `0x08` set in flags, so + * it could never satisfy that check — every packet carrying a real reading was discarded. + */ + @Test + fun aPresencePositivePacketIsAcceptedNow() { + val packet = Whoop5Ecg.parseR17(r17Inner(quality = 3, flags = 0x0A, samples = 2) + i16le(listOf(1, 2))) + assertNotNull("a contact-positive packet must decode", packet) + assertTrue(packet!!.presence) + assertTrue("precondition: the old triage capped this at 1", packet.flags.raw > 1) + assertTrue("precondition: same for quality", packet.signalQualityRaw > 1) + } + + @Test + fun r17UnreadableMaskNamesItsBits() { + assertEquals(emptyList(), EcgUnreadableMask(0x00).reasons) + assertEquals(listOf("low_amplitude"), EcgUnreadableMask(0x01).reasons) + assertEquals( + listOf("low_amplitude", "significant_noise", "unstable_signal", "not_enough_data"), + EcgUnreadableMask(0x0F).reasons, + ) + // An unmapped bit is reported as unknown, never folded onto a named reason. + assertEquals(listOf("unknown_bits_0x10"), EcgUnreadableMask(0x10).reasons) } @Test - fun filteredCarriesTrailingPadding() { - val payload = header(samples = 2) + i16le(listOf(5, -5)) + listOf(0, 0, 0) - val packet = Whoop5Ecg.decodeFiltered(payload) - assertEquals(listOf(5, -5), packet?.filteredECGDataRaw) - assertEquals(listOf(0, 0, 0), packet?.padding) + fun r17TerminalAndInvalidPredicates() { + val running = Whoop5Ecg.parseR17(r17Inner(state = 1, progress = 42, samples = 0)) + assertEquals(false, running?.isTerminal) + assertEquals(false, running?.isInvalid) + // Either condition alone is terminal. + assertEquals(true, Whoop5Ecg.parseR17(r17Inner(state = 1, progress = 100, samples = 0))?.isTerminal) + assertEquals(true, Whoop5Ecg.parseR17(r17Inner(state = 2, progress = 42, samples = 0))?.isTerminal) + // 255 is the abort sentinel, and it is NOT a completion. + val invalid = Whoop5Ecg.parseR17(r17Inner(state = 1, progress = 255, samples = 0)) + assertEquals(true, invalid?.isInvalid) + assertEquals(false, invalid?.isTerminal) } @Test - fun filteredZeroSamplesIsValid() { - val packet = Whoop5Ecg.decodeFiltered(header(leadsOn = 0, samples = 0)) + fun r17VariabilitySentinelBecomesNull() { + assertNull(Whoop5Ecg.parseR17(r17Inner(variability = 0xFFFF, samples = 0))?.variabilityRaw) + assertEquals(0xFFFE, Whoop5Ecg.parseR17(r17Inner(variability = 0xFFFE, samples = 0))?.variabilityRaw) + } + + @Test + fun r17CarriesTrailingBytesAsTail() { + val packet = Whoop5Ecg.parseR17(r17Inner(samples = 2) + i16le(listOf(5, -5)) + listOf(0, 0, 0)) + assertEquals(listOf(5, -5), packet?.samples) + assertEquals(listOf(0, 0, 0), packet?.tail) + } + + @Test + fun r17ZeroSamplesIsValid() { + val packet = Whoop5Ecg.parseR17(r17Inner(flags = 0x00, samples = 0)) assertNotNull(packet) - assertEquals(emptyList(), packet?.filteredECGDataRaw) - assertEquals(false, packet?.header?.heartKeyLeadsAreOn) + assertEquals(emptyList(), packet?.samples) + assertEquals(false, packet?.presence) } @Test - fun filteredRejectsShortHeader() { - for (count in 0 until Whoop5Ecg.HEADER_LENGTH) { - assertNull("$count-byte payload must not decode", Whoop5Ecg.decodeFiltered(List(count) { 0 })) + fun r17RejectsShortRecord() { + for (count in 0 until Whoop5Ecg.R17_FIXED_LENGTH) { + val inner = MutableList(count) { 0 } + if (count > 0) inner[0] = 43 + if (count > 1) inner[1] = 17 + assertNull("$count-byte record must not decode", Whoop5Ecg.parseR17(inner)) } } @Test - fun filteredRejectsSampleCountLongerThanBuffer() { - val payload = header(samples = 10) + i16le(listOf(1, 2, 3)) - assertNull(Whoop5Ecg.decodeFiltered(payload)) + fun r17RejectsTheWrongTypeOrRevision() { + assertNull("stored needs allowStored", Whoop5Ecg.parseR17(r17Inner(type = 47, samples = 0))) + assertNotNull(Whoop5Ecg.parseR17(r17Inner(type = 47, samples = 0), allowStored = true)) + assertNull(Whoop5Ecg.parseR17(r17Inner(type = 40, samples = 0), allowStored = true)) + // Revision 16 is the RAW record and shares the type byte. Reading it as an R17 would invent + // fields, so the revision check is what keeps the two apart. + assertNull(Whoop5Ecg.parseR17(r17Inner(revision = 16, samples = 0))) + } + + @Test + fun r17RejectsSampleCountLongerThanBuffer() { + assertNull(Whoop5Ecg.parseR17(r17Inner(samples = 10) + i16le(listOf(1, 2, 3)))) } @Test - fun filteredRejectsSampleCountOffByOneByte() { - val payload = header(samples = 4) + i16le(listOf(1, 2, 3)) + listOf(7) - assertNull(Whoop5Ecg.decodeFiltered(payload)) + fun r17RejectsSampleCountOffByOneByte() { + assertNull(Whoop5Ecg.parseR17(r17Inner(samples = 4) + i16le(listOf(1, 2, 3)) + listOf(7))) } @Test - fun filteredExtraSamplesBeyondTheCountBecomePadding() { - val payload = header(samples = 2) + i16le(listOf(9, 9, 9, 9)) - val packet = Whoop5Ecg.decodeFiltered(payload) - assertEquals(2, packet?.filteredECGDataRaw?.size) - assertEquals(4, packet?.padding?.size) + fun r17RejectsMoreSamplesThanTheWireCanCarry() { + // 100 is the physical capacity. A count above it is a corrupt or misread record, not a big packet. + val over = Whoop5Ecg.R17_MAX_SAMPLES + 1 + assertNull(Whoop5Ecg.parseR17(r17Inner(samples = over) + List(over * 2) { 0 })) + val atLimit = r17Inner(samples = Whoop5Ecg.R17_MAX_SAMPLES) + List(Whoop5Ecg.R17_MAX_SAMPLES * 2) { 0 } + assertNotNull("exactly 100 is on the wire, not over it", Whoop5Ecg.parseR17(atLimit)) + } + + @Test + fun r17ExtraSamplesBeyondTheCountBecomeTail() { + val packet = Whoop5Ecg.parseR17(r17Inner(samples = 2) + i16le(listOf(9, 9, 9, 9))) + assertEquals(2, packet?.samples?.size) + assertEquals(4, packet?.tail?.size) } // MARK: - Enum coverage @@ -143,60 +295,39 @@ class Whoop5EcgTest { ) assertEquals(EcgArrhythmiaCheckResult.entries.size, expected.size) for ((raw, expectedCase, token) in expected) { - val payload = header(arrhythmiaResult = raw, samples = 1) + i16le(listOf(0)) - val packet = Whoop5Ecg.decodeFiltered(payload) - assertEquals("raw $raw", expectedCase, packet?.header?.heartKeyArrhythmiaCheckResult) - assertEquals(raw, packet?.header?.heartKeyArrhythmiaCheckResultRaw) + val packet = Whoop5Ecg.parseR17(r17Inner(result = raw, samples = 1) + i16le(listOf(0))) + assertEquals("raw $raw", expectedCase, packet?.arrhythmiaCheckResult) + assertEquals(raw, packet?.arrhythmiaCheckResultRaw) assertEquals(token, expectedCase.token) } } @Test fun unknownArrhythmiaResultIsCarriedRawNotCoerced() { - val payload = header(arrhythmiaResult = 200, samples = 1) + i16le(listOf(0)) - val packet = Whoop5Ecg.decodeFiltered(payload) - assertNull(packet?.header?.heartKeyArrhythmiaCheckResult) - assertEquals(200, packet?.header?.heartKeyArrhythmiaCheckResultRaw) - } - - @Test - fun everyArrhythmiaCheckStatusCaseDecodes() { - val expected = listOf( - 0 to EcgArrhythmiaCheckStatus.NOT_RUNNING, - 1 to EcgArrhythmiaCheckStatus.IN_PROGRESS, - 2 to EcgArrhythmiaCheckStatus.CHECK_COMPLETE, - ) - assertEquals(EcgArrhythmiaCheckStatus.entries.size, expected.size) - for ((raw, expectedCase) in expected) { - val packet = Whoop5Ecg.decodeFiltered(header(arrhythmiaStatus = raw, samples = 0)) - assertEquals(expectedCase, packet?.header?.heartKeyArrhythmiaCheckStatus) - } - val bad = Whoop5Ecg.decodeFiltered(header(arrhythmiaStatus = 9, samples = 0)) - assertNull(bad?.header?.heartKeyArrhythmiaCheckStatus) - assertEquals(9, bad?.header?.heartKeyArrhythmiaCheckStatusRaw) + val packet = Whoop5Ecg.parseR17(r17Inner(result = 200, samples = 1) + i16le(listOf(0))) + assertNull(packet?.arrhythmiaCheckResult) + assertEquals(200, packet?.arrhythmiaCheckResultRaw) } @Test fun everySignalQualityCaseDecodes() { for (quality in EcgSignalQuality.entries) { - val packet = Whoop5Ecg.decodeFiltered(header(signalQuality = quality.raw, samples = 0)) - assertEquals(quality, packet?.header?.signalQuality) + assertEquals(quality, Whoop5Ecg.parseR17(r17Inner(quality = quality.raw, samples = 0))?.signalQuality) } - val packet = Whoop5Ecg.decodeFiltered(header(signalQuality = 77, samples = 0)) - assertEquals(EcgSignalQuality.UNKNOWN, packet?.header?.signalQuality) - assertEquals(77, packet?.header?.signalQualityRaw) + val packet = Whoop5Ecg.parseR17(r17Inner(quality = 77, samples = 0)) + assertEquals(EcgSignalQuality.UNKNOWN, packet?.signalQuality) + assertEquals(77, packet?.signalQualityRaw) } @Test fun progressPercentInRangeAndRawOutside() { for (value in listOf(0, 1, 50, 99, 100)) { - val packet = Whoop5Ecg.decodeFiltered(header(progress = value, samples = 0)) - assertEquals(value, packet?.header?.heartKeyProgress?.percentValue) + assertEquals(value, Whoop5Ecg.parseR17(r17Inner(progress = value, samples = 0))?.progress?.percentValue) } - // 101..255 is out of percentage range. The source type has a "timed out" case, but its sentinel - // value is not attested, so the byte is carried raw rather than renamed into an unproven state. + // 101..255 is out of percentage range. 255 is the strap's abort sentinel; the rest have no + // attested meaning, so the byte is carried raw rather than renamed into an unproven state. for (value in listOf(101, 200, 255)) { - val progress = Whoop5Ecg.decodeFiltered(header(progress = value, samples = 0))?.header?.heartKeyProgress + val progress = Whoop5Ecg.parseR17(r17Inner(progress = value, samples = 0))?.progress assertNull(progress?.percentValue) assertEquals(value, progress?.raw) assertEquals(false, progress?.isMapped) @@ -286,44 +417,64 @@ class Whoop5EcgTest { // MARK: - Frame level (CRC gating) @Test - fun filteredFrameDecodesThroughAValidPuffinEnvelope() { + fun r17DecodesThroughAValidPuffinEnvelope() { val samples = listOf(10, -10, 300) - val payload = header(samples = samples.size) + i16le(samples) - val frame = puffinFrame(0x28, payload) - val packet = Whoop5Ecg.decodeFilteredFrame(frame) - assertEquals(samples, packet?.filteredECGDataRaw) - assertEquals(63, packet?.header?.heartKeyHR) - } - - @Test - fun filteredFrameRejectsBadCrc32() { - val payload = header(samples = 2) + i16le(listOf(1, 2)) - val frame = puffinFrame(0x28, payload) + val frame = r17Frame(r17Inner(samples = samples.size) + i16le(samples)) + val packet = Whoop5Ecg.r17FromFrame(frame) + assertEquals(samples, packet?.samples) + assertEquals(63, packet?.liveHR) + // The record starts at frame[8], so the samples land at frame[34] — the offset observed on an MG. + assertEquals(43, frame[Whoop5Ecg.RAW_TYPE_OFFSET].toInt() and 0xFF) + assertEquals(17, frame[Whoop5Ecg.RAW_TYPE_OFFSET + 1].toInt() and 0xFF) + // `tail` is record bytes only. Slicing the frame to its END instead of to the declared length + // would put the four-byte CRC32 trailer in here and call it part of the record. + assertTrue("the CRC32 trailer is envelope, not tail", packet?.tail?.isEmpty() == true) + } + + /** + * The same guarantee with a tail that genuinely exists, so an empty-tail assertion cannot pass for + * the wrong reason. + * + * Two samples, not one: the inner record is padded to a 4-byte boundary, so an odd sample count puts + * pad bytes in `tail` too and the exact-bytes assertion stops being about the CRC at all. + * 26 fixed + 4 sample + 2 trailing = 32, already aligned. + */ + @Test + fun tailStopsAtTheCrcTrailer() { + val frame = r17Frame(r17Inner(samples = 2) + i16le(listOf(1, 2)) + listOf(0xAB, 0xCD)) + val packet = Whoop5Ecg.r17FromFrame(frame) + assertEquals(listOf(0xAB, 0xCD), packet?.tail) + // Decisive: the four CRC32 bytes the envelope ends with are absent from the record's tail. + val crc = frame.takeLast(4).map { it.toInt() and 0xFF } + assertFalse(packet!!.tail.takeLast(4) == crc) + } + + @Test + fun r17FrameRejectsBadCrc32() { + val frame = r17Frame(r17Inner(samples = 2) + i16le(listOf(1, 2))) frame[frame.size - 1] = (frame[frame.size - 1].toInt() xor 0xFF).toByte() - assertNull("a bad CRC must never reach a field read", Whoop5Ecg.decodeFilteredFrame(frame)) + assertNull("a bad CRC must never reach a field read", Whoop5Ecg.r17FromFrame(frame)) } @Test - fun filteredFrameRejectsBadHeaderCrc16() { - val payload = header(samples = 2) + i16le(listOf(1, 2)) - val frame = puffinFrame(0x28, payload) + fun r17FrameRejectsBadHeaderCrc16() { + val frame = r17Frame(r17Inner(samples = 2) + i16le(listOf(1, 2))) frame[6] = (frame[6].toInt() xor 0xFF).toByte() - assertNull(Whoop5Ecg.decodeFilteredFrame(frame)) + assertNull(Whoop5Ecg.r17FromFrame(frame)) } @Test - fun filteredFrameRejectsCorruptedBodyThatBreaksCrc() { - val payload = header(samples = 2) + i16le(listOf(1, 2)) - val frame = puffinFrame(0x28, payload) + fun r17FrameRejectsCorruptedBodyThatBreaksCrc() { + val frame = r17Frame(r17Inner(samples = 2) + i16le(listOf(1, 2))) frame[12] = (frame[12].toInt() xor 0x01).toByte() - assertNull(Whoop5Ecg.decodeFilteredFrame(frame)) + assertNull(Whoop5Ecg.r17FromFrame(frame)) } @Test fun frameRejectsGarbageAndShortInput() { - assertNull(Whoop5Ecg.decodeFilteredFrame(ByteArray(0))) - assertNull(Whoop5Ecg.decodeFilteredFrame(byteArrayOf(0xAA.toByte(), 0x01, 0x00))) - assertNull(Whoop5Ecg.decodeFilteredFrame(ByteArray(64) { 0xFF.toByte() })) + assertNull(Whoop5Ecg.r17FromFrame(ByteArray(0))) + assertNull(Whoop5Ecg.r17FromFrame(byteArrayOf(0xAA.toByte(), 0x01, 0x00))) + assertNull(Whoop5Ecg.r17FromFrame(ByteArray(64) { 0xFF.toByte() })) } @Test @@ -344,36 +495,50 @@ class Whoop5EcgTest { assertNull(Whoop5Ecg.decodeRawFrame(frame, bytesPerSample = 2)) } - // MARK: - Structural triage + // MARK: - Packet recognition @Test - fun plausibleFilteredPayloadAcceptsAWellFormedPacket() { - assertTrue(Whoop5Ecg.plausibleFilteredPayload(header(samples = 3) + i16le(listOf(1, 2, 3)))) + fun isLabradorR17FrameAcceptsAWellFormedRecord() { + assertTrue(r17IsRecognised(r17Frame(r17Inner(samples = 3) + i16le(listOf(1, 2, 3))))) } + /** + * What the superseded heuristic got wrong, as a test rather than a comment. + * + * It read a status block from `frame[11]` and required four of those bytes to be 0 or 1. Two of them + * are this record's quality and flags bytes. A packet with electrode contact carries `0x08` in flags, + * so the triage rejected exactly the packets a reading is made of. The recogniser that replaced it + * keys on the record's own type and revision instead. + */ @Test - fun plausibleFilteredPayloadRejectsNonBooleanFlagBytes() { - assertFalse(Whoop5Ecg.plausibleFilteredPayload(header(started = 7, samples = 3) + i16le(listOf(1, 2, 3)))) - } - - @Test - fun plausibleFilteredPayloadRejectsOutOfRangeEnums() { - assertFalse(Whoop5Ecg.plausibleFilteredPayload(header(signalQuality = 9, samples = 2) + i16le(listOf(1, 2)))) - assertFalse(Whoop5Ecg.plausibleFilteredPayload(header(arrhythmiaResult = 9, samples = 2) + i16le(listOf(1, 2)))) - assertFalse(Whoop5Ecg.plausibleFilteredPayload(header(arrhythmiaStatus = 9, samples = 2) + i16le(listOf(1, 2)))) + fun recognitionDoesNotDependOnTheBytesTheOldHeuristicCapped() { + for (flags in listOf(0x00, 0x02, 0x08, 0x0A, 0x0C)) { + for (quality in 0..3) { + val frame = r17Frame(r17Inner(quality = quality, flags = flags, samples = 2) + i16le(listOf(1, 2))) + assertTrue( + "flags=0x%02x quality=%d must be recognised".format(flags, quality), + r17IsRecognised(frame), + ) + } + } } @Test - fun plausibleFilteredPayloadRejectsLengthDisagreement() { - assertFalse(Whoop5Ecg.plausibleFilteredPayload(header(samples = 40) + i16le(listOf(1, 2)))) - assertFalse(Whoop5Ecg.plausibleFilteredPayload(header(samples = 1) + i16le(listOf(1)) + List(40) { 0 })) + fun isLabradorR17FrameRejectsOtherRecords() { + // Right type, wrong revision: the RAW (revision 16) record must not be read as filtered. + assertFalse(r17IsRecognised(r17Frame(r17Inner(revision = 16, samples = 2) + i16le(listOf(1, 2))))) + // Right revision, a type nothing in this family uses. + assertFalse(r17IsRecognised(r17Frame(r17Inner(type = 40, samples = 2) + i16le(listOf(1, 2))))) + // Stored records need to be asked for. + val stored = r17Frame(r17Inner(type = 47, samples = 2) + i16le(listOf(1, 2))) + assertFalse(r17IsRecognised(stored)) + assertTrue(r17IsRecognised(stored, allowStored = true)) } @Test - fun plausibleFilteredPayloadRejectsEmptyAndZeroSampleBuffers() { - assertFalse(Whoop5Ecg.plausibleFilteredPayload(emptyList())) - assertFalse(Whoop5Ecg.plausibleFilteredPayload(header(samples = 0))) - assertFalse(Whoop5Ecg.plausibleFilteredPayload(List(64) { 0 })) + fun isLabradorR17FrameRejectsEmptyAndGarbage() { + assertFalse(r17IsRecognised(ByteArray(0))) + assertFalse(r17IsRecognised(ByteArray(64) { 0 })) } // MARK: - Commands @@ -401,8 +566,11 @@ class Whoop5EcgTest { @Test fun commandPayloadIsRevisionThenArg() { - assertEquals(listOf(0x01, 0x00), Whoop5Ecg.selectWristPayload(Whoop5Ecg.WristSelection.RIGHT)) - assertEquals(listOf(0x01, 0x01), Whoop5Ecg.selectWristPayload(Whoop5Ecg.WristSelection.LEFT)) + // LITERAL wire bytes, for the same reason the control-signal pin below spells them out: the old + // RIGHT(0)/LEFT(1) reading of the client's declaration order was wrong, and a test written + // through `.raw` would have moved with the enum and stayed green. + assertEquals(listOf(0x01, 0x01), Whoop5Ecg.selectWristPayload(Whoop5Ecg.WristSelection.RIGHT)) + assertEquals(listOf(0x01, 0x02), Whoop5Ecg.selectWristPayload(Whoop5Ecg.WristSelection.LEFT)) assertEquals(listOf(0x01, 0x01), Whoop5Ecg.togglePayload(on = true)) assertEquals(listOf(0x01, 0x00), Whoop5Ecg.togglePayload(on = false)) // LITERAL wire bytes, not `ControlSignal.X.raw`. Asserting through the symbol is what let the @@ -419,11 +587,11 @@ class Whoop5EcgTest { assertEquals(35, frame[8].toInt() and 0xFF) assertEquals(0x7B, frame[10].toInt() and 0xFF) assertEquals(Whoop5Ecg.COMMAND_REVISION, frame[11].toInt() and 0xFF) - assertEquals(0x01, frame[12].toInt() and 0xFF) + assertEquals(0x02, frame[12].toInt() and 0xFF) assertEquals(listOf(0, 0, 0), (13..15).map { frame[it].toInt() and 0xFF }) // The builders must agree with the raw framing call the Swift twin is pinned against. assertArrayEquals( - Framing.puffinCommandFrame(cmd = 0x7B, seq = 9, payload = byteArrayOf(0x01, 0x01)), + Framing.puffinCommandFrame(cmd = 0x7B, seq = 9, payload = byteArrayOf(0x01, 0x02)), frame, ) } @@ -435,7 +603,7 @@ class Whoop5EcgTest { Whoop5Ecg.toggleSaveRawEcgFrame(on = false, seq = 9) to (0x7D to 0), Whoop5Ecg.mainControlEcgDataGenerationFrame(Whoop5Ecg.ControlSignal.START, seq = 9) to (0x7C to 2), Whoop5Ecg.mainControlEcgDataGenerationFrame(Whoop5Ecg.ControlSignal.STOP, seq = 9) to (0x7C to 1), - Whoop5Ecg.selectWristFrame(Whoop5Ecg.WristSelection.RIGHT, seq = 9) to (0x7B to 0), + Whoop5Ecg.selectWristFrame(Whoop5Ecg.WristSelection.RIGHT, seq = 9) to (0x7B to 1), ) for ((frame, expected) in cases) { val (cmd, arg) = expected @@ -446,12 +614,11 @@ class Whoop5EcgTest { } @Test - fun plausibleFilteredFrameIsCrcGated() { - val payload = header(samples = 3) + i16le(listOf(1, 2, 3)) - val frame = puffinFrame(0x28, payload) - assertTrue(Whoop5Ecg.plausibleFilteredFrame(frame)) + fun isLabradorR17FrameIsCrcGated() { + val frame = r17Frame(r17Inner(samples = 3) + i16le(listOf(1, 2, 3))) + assertTrue(r17IsRecognised(frame)) frame[frame.size - 1] = (frame[frame.size - 1].toInt() xor 0xFF).toByte() - assertFalse("a bad CRC must not pass the triage", Whoop5Ecg.plausibleFilteredFrame(frame)) + assertFalse("a bad CRC must not pass recognition", r17IsRecognised(frame)) } @Test @@ -459,12 +626,14 @@ class Whoop5EcgTest { // Kotlin's List can express values Swift's [UInt8] cannot. Those inputs must fail the decode // rather than diverge from Swift or throw on a subscript. val negativeHeader = header(samples = 2).toMutableList().also { it[0] = -1 } - assertNull(Whoop5Ecg.decodeFiltered(negativeHeader + i16le(listOf(1, 2)))) assertNull(Whoop5Ecg.decodeHeader(negativeHeader)) - assertFalse(Whoop5Ecg.plausibleFilteredPayload(negativeHeader + i16le(listOf(1, 2)))) - val oversizedHeader = header(samples = 2).toMutableList().also { it[13] = 0x1_0000 } - assertNull(Whoop5Ecg.decodeFiltered(oversizedHeader + i16le(listOf(1, 2)))) + // The same hazard on the R17 path: a negative or oversized element anywhere in the fixed block + // must fail the decode rather than diverge from Swift or throw on a subscript. + val negativeR17 = r17Inner(samples = 2).toMutableList().also { it[13] = -1 } + assertNull(Whoop5Ecg.parseR17(negativeR17 + i16le(listOf(1, 2)))) + val oversizedR17 = r17Inner(samples = 2).toMutableList().also { it[14] = 0x1_0000 } + assertNull(Whoop5Ecg.parseR17(oversizedR17 + i16le(listOf(1, 2)))) // A negative leads-off count would make qEnd negative and throw on subList without the guard. val payload = header(samples = 2) + listOf(0, 0, 0, 0) + listOf(-5) + List(8) { 0 }