Make the ECG gate hypothesis testable: an opt-in enable_raw_data_w_ecg write with a mandatory read-back (#891, #103)

[vishk23]

Stacked on #898 — its commit is the first one here. Review only the second commit; once #898 lands this rebases to just that.

What this is

#891 records the standing puzzle: on a WHOOP 5 MG all three TOGGLE_LABRADOR commands answer SUCCESS(1) and produce zero ECG packets in a 30-second listen. This adds the one experiment that can move it, and deliberately nothing else.

The key was not guessed. #898's read-only 115/116 enumeration asked the strap to list its own device-config keys. It answered revision=1 count=7 and walked to a clean index=255 validKey=false terminator:

sigproc_wear_detect        enable_rfid                   max_collection_backlog
cont_collection_mode       whoop_live_hr_in_adv_ind_pkt  whoop_live_2_hrm_devices
enable_raw_data_w_ecg

Six of those seven were unknown to this codebase. On that strap enable_raw_data_w_ecg reads '0'. A key pairing "raw data" with "ecg", sitting at '0' on hardware whose ECG commands accept and emit nothing, is the leading candidate for the gate.

Whether flipping it produces ECG data is UNKNOWN. It is a hypothesis the strap's own enumeration made testable — no claim is made that writing it replicates any other client's behaviour. A confirmed '1' with still no packets removes the leading hypothesis from #891 and is exactly as publishable as a positive.

Config writes on this firmware are demonstrated rather than assumed: running the existing enable_r22_* sequence (#174) moved enable_sig12 from '2' (0x32) to '1' (0x31), confirmed by a 121 read either side.

The read-back is the proof, never the ack

The write's own COMMAND_RESPONSE is recorded and not believed. A SUCCESS result byte is not evidence that state changed — SELECT_WRIST returns SUCCESS for a no-op and FAILURE for a real change. Every write is followed by GET_DEVICE_CONFIG_VALUE(121) on the same key, and only the value that comes back is reported. A SUCCESS ack whose value did not move is reported as unchanged, not as success.

Framing owed to @ryanbr on #891.

What the echoed byte in those toggle replies actually means is still open. Arg-echo is refuted, and blanket payload-echo is refuted, but "read-back of stored state" remains an inference — a per-opcode handler echoing a constant fits the observations equally well. Nothing here relies on settling that.

Safety: the allowlist is key-aware now, which is a tightening

Opcode 119 is shared with the Broadcast-HR flag (#181), so an opcode-only clause cannot express "this key and no other" — and the clause this replaces admitted ANY device-config key whenever the Broadcast-HR opt-in happened to be on.

DeviceConfigWriteGate.admitsSend parses the key name out of the body and admits exactly two keys, each only under its own opt-in:

key	gate
whoop_live_hr_in_adv_ind_pkt	Broadcast-HR opt-in (#181), unchanged, not MG-gated
enable_raw_data_w_ecg	its own opt-in and Whoop5Variant.isMG

.unknown is not MG, so an unattested strap is refused exactly like a plain 5.0 — which has no electrodes. The other five enumerated keys are refused unconditionally; their effects are undocumented and nothing here has any reason to move them. SET_FF_VALUE(120) is refused outright — the R22 sequence keeps its own separate clause.

It is one pure predicate that the send path itself consults, the same discipline DeviceConfigReadProbe.isReadOnlyOpcode established, so the tests prove it about the real wire path rather than a copy of the rule:

• 119-with-this-key is the only admitted pair out of all 256 opcodes
• 120 refused for every key x every opt-in combination
• all five out-of-scope keys refused with both opt-ins on and an attested MG
• near-miss keys, wrong b3 byte, truncated bodies and non-NUL padding all refused

Reversible in one tap — the two directions differ only in the value byte.

Three fixes that decide whether a negative sweep means anything

1. The candidate sweep routed each name to ONE verb. It picked the verb from an author's guess at the name's namespace. But the namespaces are proven separate on hardware: 128 asked for a device-config key answers FAILURE, and 121 asked for a feature-flag key answers FAILURE. So a name asked through the wrong verb answers FAILURE and is indistinguishable from "no such key" — which would have made a negative sweep worthless for exactly the names it most needed to settle. Every candidate now goes through every answering verb; the report shows which verb said what per name; a name counts as absent only when all verbs agree; the arithmetic stays in names rather than round-trips. Step cap 128 -> 320 so a doubled sweep is not silently truncated.

2. The sweep can now run even when enumeration succeeds, as an explicit second menu action. Enumeration lists what the firmware holds, which is not everything it would accept, and it says nothing at all about the feature-flag namespace — where most of the catalogue is aimed, including the sig<N> line the strap's own console tag (SIGPROC: generated a valid SPO2 during sleep) points at. Off by default: one round-trip per name per verb.

3. Device-config values are not all one byte. max_collection_backlog reads "0.0". The single-byte reader reported that as '0' and lost the rest. ValueResponse.stringValue reads the whole NUL-terminated ASCII value (stopping at the first NUL, so envelope padding stays out), and the read-back comparison uses it.

A note on the 16 flag values

The feature-flag values this probe reads back are a round-trip of NOOP's own writes, not WHOOP defaults — enable_sig12 reads '2' because an older build wrote 0x32 before #522 changed it to 0x31. They say nothing about what the strap ships with.

How to run it

Settings -> Experimental - WHOOP 5/MG -> ECG raw-data gate (WHOOP MG only) -> Turn gate on ('1'). NOOP writes the key, reads it back, and reports the value the strap actually stores. Turn gate off ('0') reverses it in one tap.

The full name sweep is Devices -> the device row menu -> Device-config read probe -> Send probe + full name sweep.

Tests

Swift swift test (Packages/WhoopProtocol) and Android testFullDebugUnitTest. New UI copy ships with translations in every locale the i18n gate tracks; Tools/i18n_audit.py --ci passes clean on both platforms.

[ryanbr]

Reviewed the second commit. The gate design is right and I checked the safety claims rather than taking them.

Verified

The claim about the clause this replaces is accurate. On main today it is command == .setDeviceConfig && PuffinExperiment.broadcastHrEnabled — opcode-only, with the key constraint living in the one call site rather than in the gate. Worth stating precisely so it is not overread: it is a latent widening, not a live bug, because nothing currently sends 119 with another key. But the gate did not express the constraint, and now it does. That is a tightening on its own merits and would be worth landing even without the ECG experiment.

The gate is real, not a copy of the rule. admitsSend(opcode:payload:ecgGateOptIn:isMG:broadcastHrOptIn:) is called inside send() against the actual payload argument, so the tests are proving something about the wire path. 120 is unreachable by construction (guard opcode == setDeviceConfigValueCmd), isWritableKey closes with default: return false, and keyName(inSendPayload:) requires the 0x01 b3 byte, printable ASCII, and NUL for every byte after the name — so a padded or truncated body fails to parse rather than falling through. Both platforms carry the predicate, and the Swift one is in Packages/, so swift-packages actually runs its tests.

The read-back-is-the-proof design is the right response to what the SELECT_WRIST pair showed: a SUCCESS ack is not evidence that state moved. Reporting a SUCCESS-with-unmoved-value as unchanged is the correct verdict, and it is the same discipline the #896 verdict fix applies.

Candidate sweep routing (your fix 1) is a genuinely good catch and the same class of defect as the verdict bug: a wrong-verb FAILURE is indistinguishable from "no such key", so a negative sweep was worthless for precisely the names it needed to settle. Asking every verb and calling a name absent only when all agree is the right shape.

Two mechanical things

Six conflicts, all strings.xml. #902 landed 22 keys appended before </resources> in every locale file, and this appends five at the same anchor. Textual and trivial — keep both blocks — but a real rebase, not a stale flag this time. I test-merged to confirm.

No CI has run on the branch at all — gh pr checks reports nothing, so do not read the absence as green. Worth a push after the rebase, and note that android.yml is enabled now, so you will get a real Android build and unit-test run rather than the Swift half only.

The one risk the description does not cover

The persistence warning is good and correctly prominent — "STAYS ON YOUR STRAP … closing NOOP won't undo it" is exactly the right register, and better than most copy in the app.

What it does not address is that the flag is reversible and data recorded while it was on is not. The strap holds history until NOOP acks it, and the ack advances a trim cursor (HISTORY_END carries it, HISTORICAL_DATA_RESULT advances it). Flash is therefore a bounded buffer of unsynced history. A key whose name is literally enable_raw_data_w_ecg is, on the natural reading, asking the strap to store more — and if that competes for the same flash, history that has not been synced yet gets trimmed earlier than it otherwise would.

I cannot show that it does; the flag's effect is exactly what is unknown. But the asymmetry is worth a sentence, because "reversible in one tap" is currently doing work it can only do for the flag itself. Two cheap mitigations, either is fine:

• say in the copy that the experiment should be short — turn on, listen, turn off — rather than left enabled;
• or suggest syncing before turning it on, so there is less unacked history exposed if it does increase storage pressure.

A user who flips this, forgets, and wears the strap for a week is the case worth designing the copy against.

Smaller

• Strings are localised at the point of writing, in both catalogues. Worth noting because #896 is currently red on exactly this and the copy there carries the non-medical framing.
• The byte-13 characterisation is now correct — arg-echo refuted, blanket payload-echo refuted, read-back still an inference because a per-opcode constant fits. Nothing in the PR leans on it, which is the right call.
• Stacked on #898, so this needs that to land first; the description says so and the commit split is clean.