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 24184f29d6..8144480562 100644 --- a/android/app/src/main/java/com/noop/ble/PuffinExperiment.kt +++ b/android/app/src/main/java/com/noop/ble/PuffinExperiment.kt @@ -223,6 +223,15 @@ class PuffinExperiment(private val prefs: SharedPreferences) { editor.apply() } + /** + * "Clear a stale phone pairing" - may NOOP call removeBond() when the OS holds a pairing the strap + * no longer honours? Default OFF, like every other switch here that changes hardware or OS state. + * See [shouldRemoveStaleBond] for the gate and why the threshold is above the guide's. + */ + var clearStaleBond: Boolean + get() = prefs.getBoolean(KEY_CLEAR_STALE_BOND, false) + set(v) { prefs.edit().putBoolean(KEY_CLEAR_STALE_BOND, v).apply() } + companion object { /** Persisted preferences file. Internal so a UI screen can observe external writes to it. */ internal const val PREFS = "noop_experiments" @@ -247,6 +256,9 @@ class PuffinExperiment(private val prefs: SharedPreferences) { * so no macOS key to mirror. */ const val KEY_EXPLICIT_BOND = "noopWhoop5ExplicitBond" + /** "Clear a stale phone pairing" opt-in — see [shouldRemoveStaleBond]. */ + const val KEY_CLEAR_STALE_BOND = "noopWhoop5ClearStaleBond" + /** "Try history sync without pairing" opt-in — the unbonded offload probe (#1635). Android-only, * so no macOS key to mirror. */ const val KEY_UNBONDED_OFFLOAD = "noopWhoop5UnbondedOffload" @@ -258,7 +270,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_UNBONDED_OFFLOAD, KEY_CLEAR_STALE_BOND) /** "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/StaleBondRemoval.kt b/android/app/src/main/java/com/noop/ble/StaleBondRemoval.kt new file mode 100644 index 0000000000..e39f38d8ef --- /dev/null +++ b/android/app/src/main/java/com/noop/ble/StaleBondRemoval.kt @@ -0,0 +1,77 @@ +package com.noop.ble + +/** + * When the app may clear a STALE platform pairing on the user's behalf. + * + * A bonded fast-path connect that drops before it ever reaches a session means the OS is holding a bond + * the strap no longer honours — a firmware update, or the official WHOOP app re-bonding, leaves Android + * with a pairing the band has forgotten. NOOP has always detected this ([WhoopBleClient] logs "stale OS + * bond") and, from the second failure, shown the forget-and-re-pair guide. Asking is the right first + * move, and it stays: this is what happens when asking did not work. + * + * OFF by default and behind its own switch, per the rule every strap-affecting probe in + * [PuffinExperiment] follows. `removeBond()` deletes a pairing from the phone's Bluetooth settings — + * at least as consequential as `createBond()` forming one, which [shouldRequestExplicitBond] already + * gates the same way — so it must not ride in on consent given for something else. + * + * Independently arrived at by OpenStrap/edge, whose gen5 bootstrap removes the platform bond once after + * five consecutive exchange failures and clears the counter only when a connect reaches READY. The + * threshold and the once-only shape are taken from that behaviour as a fact; none of its code is. + */ + +/** + * Consecutive stale-bond failures before the pairing is cleared automatically. + * + * Five, not the two at which the guide appears: the guide asks the user to do this themselves, and they + * deserve several reconnect cycles to act before the app does it for them. A strap whose pairing really + * is stale reaches five within a couple of minutes, and every one of those cycles is a failed connect. + */ +internal const val STALE_BOND_REMOVAL_THRESHOLD = 5 + +/** + * Should this disconnect clear the platform pairing? + * + * Every clause is load-bearing: + * - [optedIn]: default-off, see the file note. No switch, no bond removal, ever. + * - [isWhoop5]: a 4.0 bonds by the route it always has. This failure mode is the 5/MG fast-path one, + * and removing a WHOOP 4 pairing would break a link that works. + * - [osBonded]: nothing to remove otherwise, and calling it anyway would log a failure that means + * nothing. + * - [consecutiveStaleFailures]: the streak, not a single drop — one stale-looking disconnect is a + * transient, and this is not an action to take on a transient. + * - [alreadyRemovedThisRun]: exactly ONCE per streak. Removing a bond and immediately failing again + * means the bond was not the problem, and repeating it would delete a pairing per reconnect for a + * cause it cannot fix. Cleared only by a genuine bond, which is the event that proves the strap and + * the phone agree again. + */ +internal fun shouldRemoveStaleBond( + optedIn: Boolean, + isWhoop5: Boolean, + osBonded: Boolean, + consecutiveStaleFailures: Int, + alreadyRemovedThisRun: Boolean, + threshold: Int = STALE_BOND_REMOVAL_THRESHOLD, +): Boolean { + if (!optedIn) return false + if (!isWhoop5) return false + if (!osBonded) return false + if (alreadyRemovedThisRun) return false + return consecutiveStaleFailures >= threshold +} + +/** + * The one line that says what was done and whether the OS accepted it. + * + * `removeBond()` is not public API and is reached by reflection, so "we asked" and "it happened" are + * genuinely different outcomes and the log must not conflate them. No PII: no MAC, no serial. + */ +internal fun staleBondRemovalLine(failures: Int, accepted: Boolean): String = + if (accepted) { + "Stale pairing: cleared the phone's Bluetooth pairing for this strap after $failures failed " + + "bonded connects. Android accepted the request; the next connect starts from an unpaired " + + "state, which is what the re-pair guide was asking you to do by hand (#1635)." + } else { + "Stale pairing: asked Android to clear this strap's pairing after $failures failed bonded " + + "connects and it REFUSED (removeBond is not public API and can be denied). Nothing has " + + "changed - clear it yourself in Settings > Bluetooth, as the re-pair guide describes." + } 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 15d73b2a50..f687c0e40a 100644 --- a/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt +++ b/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt @@ -5533,6 +5533,10 @@ class WhoopBleClient( * nag the user. Reset to 0 on any genuine bond. (5/MG firmware reset parity, 2026-06) */ private var staleDirectFailures = 0 + /** #1635: the stale-pairing clear is once per streak. Cleared by a genuine bond, the event that + * proves the phone and the strap agree again. */ + private var staleBondRemoved = false + /** * Which of OUR OWN paths last tore the link down, and when the current session started (#1020). * @@ -6506,6 +6510,7 @@ class WhoopBleClient( noteGenuineBond(g.device.address) // #52: this strap bonds fine; clears any pin-refusal streak clearPairingHint() // #78: a genuine bond means the pairing guidance no longer applies staleDirectFailures = 0 // genuine bond — clear the wiped-bond counter (#84 parity) + staleBondRemoved = false // ...and re-arm the one-shot stale-pairing clear } _state.update { it.copy(bonded = true, encryptedBond = encrypted) } bondedAtMs = System.currentTimeMillis() // #617: stamp the bond so handleDisconnect can spot a bond-then-quick-timeout loop @@ -9730,6 +9735,15 @@ class WhoopBleClient( // MARK: Disconnect / teardown (port of didDisconnectPeripheral) // ==================================================================================== + /** + * Ask Android to delete this device's pairing. Reflection because `removeBond()` is not public API; + * a throw or a `false` is a REFUSAL, not a crash, and the caller reports which happened. + */ + @SuppressLint("MissingPermission") + private fun removeOsBond(device: BluetoothDevice): Boolean = runCatching { + device.javaClass.getMethod("removeBond").invoke(device) as? Boolean ?: false + }.getOrDefault(false) + @SuppressLint("MissingPermission") private fun handleDisconnect(status: Int) { val helloWasUnacked = clientHelloWriteAtMs > 0L @@ -9980,6 +9994,26 @@ class WhoopBleClient( } if (staleDirectBond) { staleDirectFailures++ + // Before `lastDevice = null` below: that handle is the only device left to act on here. + val staleDevice = lastDevice + if (staleDevice != null && shouldRemoveStaleBond( + optedIn = puffinExperiment.clearStaleBond, + // Belt and braces. `staleDirectBond` can only be set by the Easy-connect path, + // whose helpers match a 5/MG and which pins selectedModel to it — so this is + // already implied. Kept because `connectedFamily` fails CLOSED when it is stale + // (it holds the previous link's value), and the direction that matters is never + // removing a pairing we should not. + isWhoop5 = connectedFamily == DeviceFamily.WHOOP5, + osBonded = runCatching { + staleDevice.bondState == BluetoothDevice.BOND_BONDED + }.getOrDefault(false), + consecutiveStaleFailures = staleDirectFailures, + alreadyRemovedThisRun = staleBondRemoved, + ) + ) { + staleBondRemoved = true + log(staleBondRemovalLine(staleDirectFailures, removeOsBond(staleDevice))) + } log("Disconnected (status=$status) before the bonded fast-path reached a session — stale OS bond (attempt $staleDirectFailures); falling back to a scan") lastDevice = null // Two consecutive wiped-bond failures = the strap really reset its pairing (firmware diff --git a/android/app/src/main/java/com/noop/ui/TestCentreScreen.kt b/android/app/src/main/java/com/noop/ui/TestCentreScreen.kt index 84595a09e9..3b17c54cf8 100644 --- a/android/app/src/main/java/com/noop/ui/TestCentreScreen.kt +++ b/android/app/src/main/java/com/noop/ui/TestCentreScreen.kt @@ -105,6 +105,7 @@ fun TestCentreScreen(vm: AppViewModel, onOpenGroundTruthCollector: () -> Unit = var broadcastHr by remember { mutableStateOf(puffinExperiment.broadcastHr) } var explicitBond by remember { mutableStateOf(puffinExperiment.explicitBond) } var unbondedOffload by remember { mutableStateOf(puffinExperiment.unbondedOffload) } + var clearStaleBond by remember { mutableStateOf(puffinExperiment.clearStaleBond) } var ecgRawData by remember { mutableStateOf(puffinExperiment.ecgRawData) } val r22DisableReport by vm.ble.r22DisableReport.collectAsStateWithLifecycle() val ecgGateReport by vm.ble.ecgRawDataGate.collectAsStateWithLifecycle() @@ -259,6 +260,20 @@ fun TestCentreScreen(vm: AppViewModel, onOpenGroundTruthCollector: () -> Unit = puffinExperiment.unbondedOffload = it }, ) + DeveloperToggleRow( + title = stringResource(R.string.raw_diag_clear_stale_bond), + detail = "When a bonded fast-path connect keeps dropping before it reaches a " + + "session, the phone is holding a pairing the strap no longer honours. NOOP " + + "already shows the forget-and-re-pair guide at two failures; with this on it " + + "does that step for you at five, once, and only until the strap bonds again. " + + "It cannot make a strap that refuses pairing pair. Leave it off unless you " + + "are testing #1635.", + checked = clearStaleBond, + onCheckedChange = { + clearStaleBond = it + puffinExperiment.clearStaleBond = it + }, + ) DeveloperToggleRow( title = stringResource(R.string.raw_diag_r22), detail = "Accepted writes have not been shown to enable a separate live stream. Not required for normal sync or raw capture.", diff --git a/android/app/src/main/res/values-de/strings.xml b/android/app/src/main/res/values-de/strings.xml index 29cf15fda3..1006182fd1 100644 --- a/android/app/src/main/res/values-de/strings.xml +++ b/android/app/src/main/res/values-de/strings.xml @@ -2628,6 +2628,7 @@ Herzfrequenz vom Band übertragen Android um Kopplung bitten Verlaufssynchronisierung ohne Kopplung versuchen + Veraltete Kopplung entfernen Altes R22-Feature-Flag-Experiment Alte R22-Aktivierungssequenz senden R22-Flags auf dem Band löschen diff --git a/android/app/src/main/res/values-es/strings.xml b/android/app/src/main/res/values-es/strings.xml index b6f211b2ec..2abd505303 100644 --- a/android/app/src/main/res/values-es/strings.xml +++ b/android/app/src/main/res/values-es/strings.xml @@ -2613,6 +2613,7 @@ Transmitir frecuencia cardíaca desde la pulsera Pedir a Android que vincule Probar la sincronización del historial sin vinculación + Borrar vinculación obsoleta Experimento antiguo de indicadores R22 Enviar secuencia de activación R22 antigua Borrar indicadores R22 de la pulsera diff --git a/android/app/src/main/res/values-fr/strings.xml b/android/app/src/main/res/values-fr/strings.xml index 65a5d2533d..bc0790e792 100644 --- a/android/app/src/main/res/values-fr/strings.xml +++ b/android/app/src/main/res/values-fr/strings.xml @@ -2612,6 +2612,7 @@ Diffuser la fréquence cardiaque du bracelet Demander l’appairage à Android Essayer la synchronisation de l\'historique sans appairage + Supprimer un appairage obsolète Ancienne expérience d’indicateurs R22 Envoyer l’ancienne séquence d’activation R22 Effacer les indicateurs R22 du bracelet diff --git a/android/app/src/main/res/values-pl/strings.xml b/android/app/src/main/res/values-pl/strings.xml index c79b1d06c1..134280a652 100644 --- a/android/app/src/main/res/values-pl/strings.xml +++ b/android/app/src/main/res/values-pl/strings.xml @@ -2617,6 +2617,7 @@ Transmituj tętno z opaski Poproś Androida o sparowanie Spróbuj synchronizacji historii bez parowania + Usuń nieaktualne parowanie Starszy eksperyment flag R22 Wyślij starszą sekwencję włączenia R22 Wyczyść flagi R22 na opasce diff --git a/android/app/src/main/res/values-pt-rPT/strings.xml b/android/app/src/main/res/values-pt-rPT/strings.xml index 039cd3d5ad..00da9bb722 100644 --- a/android/app/src/main/res/values-pt-rPT/strings.xml +++ b/android/app/src/main/res/values-pt-rPT/strings.xml @@ -2605,6 +2605,7 @@ Transmitir frequência cardíaca da bracelete Pedir ao Android para emparelhar Tentar sincronizar o histórico sem emparelhamento + Limpar emparelhamento obsoleto Experiência antiga de indicadores R22 Enviar sequência antiga de ativação R22 Limpar indicadores R22 da bracelete diff --git a/android/app/src/main/res/values-zh/strings.xml b/android/app/src/main/res/values-zh/strings.xml index 7c3c1d274b..a98ee0e47b 100644 --- a/android/app/src/main/res/values-zh/strings.xml +++ b/android/app/src/main/res/values-zh/strings.xml @@ -2526,6 +2526,7 @@ 从手环广播心率 请求 Android 配对 尝试在未配对状态下同步历史数据 + 清除过期配对 旧版 R22 功能标志实验 发送旧版 R22 启用序列 清除手环上的 R22 标志 diff --git a/android/app/src/main/res/values/strings.xml b/android/app/src/main/res/values/strings.xml index fde717148a..d32df2df16 100644 --- a/android/app/src/main/res/values/strings.xml +++ b/android/app/src/main/res/values/strings.xml @@ -2641,6 +2641,7 @@ Broadcast heart rate from the strap Ask Android to pair Try history sync without pairing + Clear a stale phone pairing Legacy R22 feature-flag experiment Send legacy R22 enable sequence Clear legacy R22 flags on strap diff --git a/android/app/src/test/java/com/noop/ble/StaleBondRemovalTest.kt b/android/app/src/test/java/com/noop/ble/StaleBondRemovalTest.kt new file mode 100644 index 0000000000..e62b94970e --- /dev/null +++ b/android/app/src/test/java/com/noop/ble/StaleBondRemovalTest.kt @@ -0,0 +1,120 @@ +package com.noop.ble + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * #1635: when NOOP may delete a pairing from the user's phone. + * + * Every clause is a separate way to get this wrong, and getting it wrong means removing a working + * pairing rather than a stale one — so each is pinned on its own rather than trusting one happy path. + */ +class StaleBondRemovalTest { + + private fun ok( + optedIn: Boolean = true, + isWhoop5: Boolean = true, + osBonded: Boolean = true, + failures: Int = STALE_BOND_REMOVAL_THRESHOLD, + already: Boolean = false, + ) = shouldRemoveStaleBond(optedIn, isWhoop5, osBonded, failures, already) + + @Test + fun `the streak at threshold, opted in, on a bonded 5MG, clears the pairing`() { + assertTrue(ok()) + } + + @Test + fun `no switch, no bond removal - deleting a pairing never rides in on other consent`() { + assertFalse(ok(optedIn = false)) + // ...not even far past the threshold. + assertFalse(ok(optedIn = false, failures = 500)) + } + + @Test + fun `a WHOOP 4 is never touched - it bonds by the route it always has`() { + assertFalse(ok(isWhoop5 = false)) + } + + @Test + fun `nothing to remove when the OS holds no bond`() { + assertFalse(ok(osBonded = false)) + } + + @Test + fun `one drop is a transient, not a stale pairing`() { + for (n in 0 until STALE_BOND_REMOVAL_THRESHOLD) { + assertFalse("failures=$n must not remove", ok(failures = n)) + } + assertTrue(ok(failures = STALE_BOND_REMOVAL_THRESHOLD)) + } + + @Test + fun `exactly once per streak - a second failure does not delete a second pairing`() { + assertFalse(ok(already = true)) + assertFalse(ok(already = true, failures = 500)) + } + + @Test + fun `it waits longer than the guide, which asks the user first`() { + // The re-pair guide appears at 2. Acting before the user has had cycles to act themselves would + // take the choice away from them; this is what happens when asking did not work. + assertTrue(STALE_BOND_REMOVAL_THRESHOLD > 2) + } + + @Test + fun `the line separates asking from happening, and carries no PII`() { + val accepted = staleBondRemovalLine(5, accepted = true) + val refused = staleBondRemovalLine(5, accepted = false) + assertTrue(accepted.contains("Android accepted")) + assertTrue(refused.contains("REFUSED")) + assertTrue("a refusal must say nothing changed", refused.contains("Nothing has changed")) + assertEquals(2, listOf(accepted, refused).count { it.contains("5 failed") }) + // A bare ":" is prose punctuation here, so match the SHAPE of a MAC rather than the character. + val macLike = Regex("[0-9A-Fa-f]{2}:[0-9A-Fa-f]{2}") + for (line in listOf(accepted, refused)) { + assertFalse("no MAC in a shared log: $line", macLike.containsMatchIn(line)) + assertFalse("no serial prefix", line.contains("MGB")) + } + } + + /** + * The switch has to be REACHABLE, and nothing else in this suite can tell you that. + * + * The first cut of this feature put its row in SettingsScreen inside `if (false && ...)` — a block + * whose own comment says developer 5/MG controls moved to Test Centre and that it must never render. + * Every test passed and CI was green with a toggle no user could turn on. It looked right because + * the sibling "Ask Android to pair" row sits in the same dead block and appears to work only + * because its pref persists from before that block was disabled. + * + * So this reads the source: the switch must be wired in Test Centre, and Test Centre must not have + * grown a disabled block of its own. + */ + @Test + fun `the toggle is wired somewhere a user can actually reach`() { + val testCentre = uiSource("TestCentreScreen.kt") + assertTrue("the toggle must be wired in Test Centre, which is where 5/MG switches live", + testCentre.contains("R.string.raw_diag_clear_stale_bond")) + assertTrue("and bound to the experiment it gates", + testCentre.contains("puffinExperiment.clearStaleBond = it")) + assertFalse("Test Centre must not grow a disabled block the way Settings did", + testCentre.contains("if (false")) + + // And it must NOT have been added to the retained-but-dead Settings copy: a row that never + // shipped there needs no compatibility copy, and one would only be more to delete later. + assertFalse("the dead Settings block must not gain new switches", + uiSource("SettingsScreen.kt").contains("clearStaleBond")) + } + + private fun uiSource(name: String): String { + val rel = "android/app/src/main/java/com/noop/ui/$name" + val start = java.io.File(System.getProperty("user.dir") ?: ".").absoluteFile + val f = generateSequence(start) { it.parentFile }.map { java.io.File(it, rel) } + .firstOrNull { it.isFile } + assertTrue("$name not found walking up from $start — this check cannot run, and one that " + + "silently skips is worse than none", f != null) + return f!!.readText() + } +}