diff --git a/android/app/src/main/java/com/noop/ble/PuffinExperiment.kt b/android/app/src/main/java/com/noop/ble/PuffinExperiment.kt index 8144480562..411d54dc9d 100644 --- a/android/app/src/main/java/com/noop/ble/PuffinExperiment.kt +++ b/android/app/src/main/java/com/noop/ble/PuffinExperiment.kt @@ -61,6 +61,17 @@ class PuffinExperiment(private val prefs: SharedPreferences) { get() = prefs.getBoolean(KEY_ECG_RAW_DATA, false) set(v) = prefs.edit().putBoolean(KEY_ECG_RAW_DATA, v).apply() + /** True if the user opted in to the "MG ECG research probe" (#891/#1100) — the gated, hand-run panel + * that SENDS the WHOOP MG ECG ("Labrador") toggle commands (139/125/124) to learn what a real MG does + * with them. SEPARATE from [ecgRawData]: that opt-in writes ONE persistent device-config value; this + * one dispatches session ECG commands. Every one is on the strict allow-list in + * [com.noop.protocol.EcgResearchAllowList] and additionally gated on an attested MG at the send path + * ([com.noop.ble.WhoopBleClient.ecgSendAdmitted]). Reversible, default false — a plain 5.0 or a 4.0 + * never sees these. Mirrors the macOS `PuffinExperiment.ecgEnabled` (`noopWhoop5Ecg`). */ + var ecgProbe: Boolean + get() = prefs.getBoolean(KEY_ECG_PROBE, false) + set(v) = prefs.edit().putBoolean(KEY_ECG_PROBE, v).apply() + /** True if the user opted in to "Experimental sleep staging (V2)": detected nights are re-staged with * [com.noop.analytics.SleepStagerV2] (the transparent cardiorespiratory recipe, reimplemented from * contributor PR #600) instead of the older V1 [com.noop.analytics.SleepStager]. Pure analysis switch @@ -252,6 +263,11 @@ class PuffinExperiment(private val prefs: SharedPreferences) { * `PuffinExperiment.ecgRawDataKey`). (#891) */ const val KEY_ECG_RAW_DATA = "noopEcgRawDataGate" + /** "MG ECG research probe" opt-in — dispatches the Labrador ECG session commands from the gated + * research panel. Same UserDefaults key as macOS `PuffinExperiment.ecgEnabledKey`, so a future + * cross-platform settings sync does not have to reconcile two names. (#891/#1100) */ + const val KEY_ECG_PROBE = "noopWhoop5Ecg" + /** "Ask Android to pair" opt-in — the explicit `createBond()` experiment (#1635). Android-only, * so no macOS key to mirror. */ const val KEY_EXPLICIT_BOND = "noopWhoop5ExplicitBond" @@ -270,7 +286,7 @@ class PuffinExperiment(private val prefs: SharedPreferences) { * SettingsScreen watches exactly these for external writes. Two lists would drift. */ internal val FIVE_MG_GATED_KEYS = listOf(KEY, KEY_CAPTURE, KEY_DEEP_DATA, KEY_BROADCAST_HR, KEY_ECG_RAW_DATA, KEY_EXPLICIT_BOND, - KEY_UNBONDED_OFFLOAD, KEY_CLEAR_STALE_BOND) + KEY_UNBONDED_OFFLOAD, KEY_CLEAR_STALE_BOND, KEY_ECG_PROBE) /** "Experimental sleep staging (V2)" opt-in (mirrors macOS `PuffinExperiment.experimentalSleepV2Key`). */ const val KEY_EXPERIMENTAL_SLEEP_V2 = "noopExperimentalSleepV2" 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 e73de2ffce..2aeb5abe76 100644 --- a/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt +++ b/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt @@ -49,6 +49,7 @@ import com.noop.protocol.DeviceConfigReadProbeReport import com.noop.protocol.DeviceConfigWriteGate import com.noop.protocol.BroadcastHrGateReport import com.noop.protocol.EcgRawDataGateReport +import com.noop.protocol.EcgResearchAllowList import com.noop.protocol.FeatureFlagProbe import com.noop.protocol.FeatureFlagProbeReport import com.noop.protocol.Framing @@ -4397,7 +4398,12 @@ class WhoopBleClient( // verified — same discipline as the R22 read-back above. The write ack is never the proof. // Both the ECG gate (#891) and the Broadcast-HR gate (#1061) read themselves back over this. !(DeviceConfigWriteGate.isReadBackOpcode(cmd.rawValue) && - (ecgGateReport != null || broadcastHrGateReport != null))) { + (ecgGateReport != null || broadcastHrGateReport != null)) && + // MG ECG ("Labrador") research probe (#891/#1100): the ECG opcodes 123/124/125/139, admitted + // ONLY by ecgSendAdmitted — the probe opt-in on, on a connected, positively-attested MG. The + // opcode gate is the positive allow-list EcgResearchAllowList, a closed set of four, so the + // firmware-load family three codes above 139 is not expressible through it. + !ecgSendAdmitted(cmd)) { log("send(${cmd.name}) skipped — no WHOOP 5/MG framing for this command yet") return } @@ -8822,6 +8828,29 @@ class WhoopBleClient( log("ECG gate (#891):\n${report.render()}") } + /** + * THE send allow-list for the WHOOP MG ECG ("Labrador") opcodes (#891/#1100). + * + * True only for an opcode in the positive allow-list [EcgResearchAllowList.PROBE_OPCODES], with the ECG + * probe opt-in on, on a connected, positively-attested MG. Every other opcode, family or state is false + * — the firmware-load family and the rest of [EcgResearchAllowList.FORBIDDEN] are refused because they + * are simply not in that set, not because a deny-list caught them. + * + * Nothing in this change can make it return true in practice: there is no ECG caller on Android yet and + * no Settings row that sets [PuffinExperiment.ecgProbe], so the opcodes this change makes constructible + * are not reachable from any code path. That is the point — the enum widening and the gate that bounds + * it land together, and the probe that needs it lands afterwards against a gate already in `main`. + * + * The run-arming and stop-override conditions belong with that probe, and tighten this further. + */ + private fun ecgSendAdmitted(cmd: CommandNumber): Boolean { + if (!EcgResearchAllowList.isProbeOpcode(cmd.rawValue)) return false + if (connectedFamily != DeviceFamily.WHOOP5) return false + if (!whoop5Variant().isMG) return false + if (!_state.value.connected) return false + return puffinExperiment.ecgProbe + } + /** Clear the #891 result (Settings row dismissed / disconnect). Twin of Swift clearEcgRawDataGate(). */ fun clearEcgRawDataGate() { _ecgRawDataGate.value = null } diff --git a/android/app/src/main/java/com/noop/protocol/Enums.kt b/android/app/src/main/java/com/noop/protocol/Enums.kt index 9e4d348d9a..e315ac6d4c 100644 --- a/android/app/src/main/java/com/noop/protocol/Enums.kt +++ b/android/app/src/main/java/com/noop/protocol/Enums.kt @@ -262,16 +262,29 @@ enum class CommandNumber(val rawValue: Int) { // not add it and deliberately does not remove it either: dropping a pre-existing entry would change // the curated send surface for a reason that has nothing to do with decoding ECG packets, and that is // a separate decision from the one below. - SELECT_WRIST(123); + SELECT_WRIST(123), + // The three WHOOP MG ECG ("Labrador") TOGGLES (124 / 125 / 139). // - // The three WHOOP MG ECG ("Labrador") TOGGLES (124 / 125 / 139) are deliberately ABSENT from this - // enum. This branch originally listed them here so a COMMAND_RESPONSE for one would be labelled - // rather than shown as a bare hex opcode — a reason #893 has since made obsolete, by giving Android - // a read-only `CommandNames` label table that names every opcode the schema names without making any - // of them constructible. Android has no ECG app layer and sends none of them, so growing the - // SENDER enum to buy a label would widen what the command sender can express for nothing. Apple's - // `WhoopCommand` carries them because Apple actually drives the gated probe. See - // `com.noop.protocol.Whoop5Ecg` for the decoder and `Whoop5EcgProbe` for the verdict rules. + // These were ABSENT, and the reason given was: "Android has no ECG app layer and sends none of them, + // so growing the SENDER enum to buy a label would widen what the command sender can express for + // nothing." The label half of that is still right — #893's read-only `CommandNames` table names every + // opcode the schema names without making any constructible, and nothing here is added for a label. + // + // What changed is the premise: the gated MG ECG research probe (#891/#1100) is an Android ECG app + // layer, and it does send them, exactly as Apple's `WhoopCommand` already does for the same probe. + // + // Adding them does not widen the REACHABLE surface, which is the property that matters. Every send + // passes the single `send()` chokepoint, where [WhoopBleClient.ecgSendAdmitted] admits an opcode only + // if it is in the positive allow-list [com.noop.protocol.EcgResearchAllowList.PROBE_OPCODES], the ECG + // probe opt-in is on, and the strap is a POSITIVELY-attested MG. The allow-list is a closed set of + // four, so the firmware-load family three codes above 139 (142/143/144) is not expressible through it + // — asserted in EcgResearchAllowListTest, not merely asserted here. + // + // See `com.noop.protocol.Whoop5Ecg` for the command builders and decoder, and `Whoop5EcgProbe` for + // the verdict rules. Values are the schema numbers. + TOGGLE_LABRADOR_DATA_GENERATION(124), + TOGGLE_LABRADOR_RAW_SAVE(125), + TOGGLE_LABRADOR_FILTERED(139); companion object { private val byRaw = entries.associateBy { it.rawValue } diff --git a/android/app/src/test/java/com/noop/protocol/CommandCatalogueTest.kt b/android/app/src/test/java/com/noop/protocol/CommandCatalogueTest.kt index aeaa431aa8..f480c264f0 100644 --- a/android/app/src/test/java/com/noop/protocol/CommandCatalogueTest.kt +++ b/android/app/src/test/java/com/noop/protocol/CommandCatalogueTest.kt @@ -111,15 +111,34 @@ class CommandCatalogueTest { } } - /** The ECG family #891 turns on. None is sendable; all three are now legible. */ + /** + * The ECG family #891 turns on: all three legible, and — since the gated MG ECG research probe + * (#891/#1100) gave Android an ECG app layer that drives them — all three now constructible too. + * + * This assertion used to be `assertNull`, on the grounds that "Android has no ECG app layer and sends + * none of them". That premise is what changed; the safety property did not. Being in the sender enum + * makes an opcode EXPRESSIBLE, not REACHABLE: every send crosses the one `send()` chokepoint, where + * `WhoopBleClient.ecgSendAdmitted` requires the opcode to be in the closed four-element allow-list + * [EcgResearchAllowList.PROBE_OPCODES], the ECG probe opt-in to be on, and the strap to be a + * positively-attested MG. + * + * `senderEnumStaysCuratedAfterTheLabelFix` above is the other half of this and is deliberately + * unchanged: the destructive families — firmware load, DFU, fuel-gauge reset, and the *_NEW trio at + * 142/143/144 that sits three codes above 139 — are still `assertNull`. Widening the enum's top from + * 123 to 139 did not drag its neighbours in. + */ @Test - fun theMgEcgTogglesAreNamedButNotSendable() { + fun theMgEcgTogglesAreNamedAndNowConstructibleBehindTheSendGate() { assertEquals("TOGGLE_LABRADOR_DATA_GENERATION", CommandNames.byRaw[124]) assertEquals("TOGGLE_LABRADOR_RAW_SAVE", CommandNames.byRaw[125]) assertEquals("TOGGLE_LABRADOR_FILTERED", CommandNames.byRaw[139]) - assertNull(CommandNumber.fromRaw(124)) - assertNull(CommandNumber.fromRaw(125)) - assertNull(CommandNumber.fromRaw(139)) + assertEquals(124, CommandNumber.fromRaw(124)?.rawValue) + assertEquals(125, CommandNumber.fromRaw(125)?.rawValue) + assertEquals(139, CommandNumber.fromRaw(139)?.rawValue) + // SELECT_WRIST(123) was already constructible; what changes for it is the SEND surface: the gate now + // admits it to an attested MG with the probe opt-in on, and unlike the three toggles it is PERSISTENT + // wrist config (see EcgResearchAllowList) — a toggle stops when the session does, this does not. + assertEquals(123, CommandNumber.fromRaw(123)?.rawValue) } /** diff --git a/android/app/src/test/java/com/noop/protocol/EcgResearchAllowListTest.kt b/android/app/src/test/java/com/noop/protocol/EcgResearchAllowListTest.kt index 9be5d00778..78096be96c 100644 --- a/android/app/src/test/java/com/noop/protocol/EcgResearchAllowListTest.kt +++ b/android/app/src/test/java/com/noop/protocol/EcgResearchAllowListTest.kt @@ -2,6 +2,7 @@ package com.noop.protocol import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotNull import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Test @@ -73,16 +74,23 @@ class EcgResearchAllowListTest { } /** - * The companion guard — that every DISPATCHABLE opcode is a constructible [CommandNumber] — is - * deliberately NOT here. It is an invariant of the SEND PATH: `send()` forms a frame from a - * `CommandNumber`, so an allow-listed opcode the enum cannot express could not be dispatched at all. + * The send-path invariant this allow-list depends on: `send()` forms a frame from a [CommandNumber], + * so an allow-listed opcode the enum cannot express could not be dispatched at all. Guards against an + * enum entry being removed and the allow-list silently admitting an opcode the sender cannot form. * - * Nothing in this change dispatches anything, and opcodes 124/125/139 are absent from that enum on - * purpose — upstream excluded them with a comment saying Android has no ECG app layer and sends - * none of them. Asserting their presence here would fail for the correct reason, so the guard lands - * with the send-path wiring that makes it true and gives it something worth protecting. + * This is the assertion the pure-modules split deliberately deferred: 124/125/139 were absent from + * `CommandNumber` on purpose, so it would have failed there for the correct reason. The change that + * widens the enum is the change that gets to make it true. */ @Test + fun everyDispatchableOpcodeIsAConstructibleCommandNumber() { + for (op in EcgResearchAllowList.DISPATCHABLE_OPCODES) { + assertNotNull("CommandNumber must carry opcode $op", CommandNumber.fromRaw(op)) + } + } + + /** The forbidden set and the probe set are disjoint: no never-send opcode is reachable through a probe case. */ + @Test fun noForbiddenOpcodeIsReachableThroughTheProbeCases() { for (op in EcgResearchAllowList.FORBIDDEN.keys) { assertFalse(EcgResearchAllowList.PROBE_OPCODES.contains(op))