Skip to content

ecg: correct the MG Labrador R17 layout, wrist values and START list #2594

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ryanbr merged 1 commit into from
Sep 29, 2026
Merged

ecg: correct the MG Labrador R17 layout, wrist values and START list #2594

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
312 changes: 238 additions & 74 deletions Packages/WhoopProtocol/Sources/WhoopProtocol/Whoop5Ecg.swift
View file
Open in desktop

Large diffs are not rendered by default.

27 changes: 14 additions & 13 deletions Packages/WhoopProtocol/Sources/WhoopProtocol/Whoop5EcgProbe.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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. "
Expand Down Expand Up @@ -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],
Expand Down Expand Up @@ -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)" } }
Expand Down
439 changes: 298 additions & 141 deletions Packages/WhoopProtocol/Tests/WhoopProtocolTests/Whoop5EcgTests.swift
View file
Open in desktop

Large diffs are not rendered by default.

131 changes: 86 additions & 45 deletions Strand/BLE/BLEManager.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
///
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
Loading
Loading