diff --git a/android/app/build.gradle.kts b/android/app/build.gradle.kts index 3d06a39a54..cf5c6bad50 100644 --- a/android/app/build.gradle.kts +++ b/android/app/build.gradle.kts @@ -26,7 +26,7 @@ android { applicationId = "com.noop.whoop" minSdk = 26 targetSdk = 34 - versionCode = 393 + versionCode = 394 versionName = "10.6.1" testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner" diff --git a/android/app/src/main/java/com/noop/ble/UnbondedOffloadProbe.kt b/android/app/src/main/java/com/noop/ble/UnbondedOffloadProbe.kt index 6b3ddfbcd9..cc7a06343b 100644 --- a/android/app/src/main/java/com/noop/ble/UnbondedOffloadProbe.kt +++ b/android/app/src/main/java/com/noop/ble/UnbondedOffloadProbe.kt @@ -139,8 +139,9 @@ internal fun unbondedProbeSupersedesLine(explicitBondOptedIn: Boolean): String = " asks whether the offload needs a bond at all. That question needs a link with no CLIENT_HELLO on" + " it, so the hello is not written here (press Connect to try the handshake instead)." + (if (explicitBondOptedIn) - " NOTE: \"Ask Android to pair\" is also on. A pairing in flight makes a refusal unattributable" + - " to the strap, so turn it off for a clean answer." + " \"Ask Android to pair\" is on but is ALSO skipped on this connect — this branch returns" + + " before the pairing request, so no SMP is in flight and a refusal here is attributable" + + " to the strap." else "") + " (#1635, experimental)" @@ -222,6 +223,52 @@ internal fun puffinSubscribeRefusedLine(uuid: String, status: String): String = " or -encryption status, the puffin notify chars need an encrypted link, the historical offload" + " cannot be reached without a bond, and a 5/MG that refuses SMP can never sync history (#1635)." +/** + * Stage 1 ended because the LINK did, before any subscribe was confirmed or refused. + * + * The outcome the probe had no verdict for, and the field capture is unambiguous about how much that + * matters: 16 probe starts, 0 verdicts of any kind, 0 confirmed subscribes, 0 refusals, and the link + * dying 10.8s into every connect — roughly three seconds after the CCCD writes went out. With no verdict + * the silence budget never advanced, so the probe re-ran on every reconnect indefinitely. That is exactly + * the unbounded retry [shouldProbeUnbondedOffload]'s own doc claims this design prevents, reintroduced by + * an unhandled exit. + * + * It is also a FINDING, not just a gap. No callback and no ATT error, then a teardown about three seconds + * later, is the CLIENT_HELLO's signature — the same silent elevate-and-drop, on the same service. It does + * not prove the puffin characteristics require encryption, but it is what that would look like from here, + * and it says plainly that the offload is not reachable on this strap without a bond. + * + * Charged to the silence budget, because "the link will not survive being asked" is a stronger reason to + * stop asking than a strap that merely stayed quiet. + */ +internal fun unbondedProbeLinkLostLine( + uptimeMs: Long, + confirmedSubscribes: Int, + total: Int, +): String = + "Unbonded offload probe: the link dropped ${uptimeMs}ms into this connect with $confirmedSubscribes" + + " of $total puffin subscribes confirmed and no refusal — no callback and no ATT error, then a" + + " teardown. That is the CLIENT_HELLO's own signature on the same service, so the offload is not" + + " reachable on this strap without a bond (#1635)." + +/** + * Stage 2 ended because the link did, with GET_CLOCK already on the wire. + * + * Deliberately NOT the same line as [unbondedProbeLinkLostLine], and that distinction is the whole point. + * A stage-1 link loss carries a finding — the subscribes drew no callback and no ATT error before the drop, + * which is the CLIENT_HELLO's signature. A stage-2 link loss carries none: the subscribes LANDED, the + * transport was open, and the strap was still within its window to answer when the link went. Reporting + * that as evidence the strap will not answer would be the same conflation this probe keeps having to + * unpick. + * + * It still spends a budget attempt, because an inconclusive link is not a reason to retry forever — that + * is the hole this exists to close, and leaving stage 2 uncounted would reopen it one stage later. + */ +internal fun unbondedProbeLinkLostAskingLine(uptimeMs: Long, waitedMs: Long): String = + "Unbonded offload probe: the link dropped ${uptimeMs}ms into this connect, ${waitedMs}ms after" + + " GET_CLOCK went out. The subscribes had landed, so the transport was open and the strap was still" + + " inside its window to answer — this link settles nothing either way (#1635)." + /** * Stage 2's question, logged so the wait that follows is attributable to it. * 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 369cb3d547..8f2f86d1c0 100644 --- a/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt +++ b/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt @@ -9792,6 +9792,35 @@ class WhoopBleClient( // see the cleared state, which is harmless, but one still queued must not reach the next link. handler.removeCallbacks(unbondedProbeStartRunnable) handler.removeCallbacks(unbondedProbeVerdictRunnable) + // A probe still mid-subscribe when the link goes is stage 1 ending with the LINK, and it is a + // verdict rather than an absence — see [unbondedProbeLinkLostLine]. Without this the probe + // reported nothing at all and its silence budget never advanced, so it re-ran on every reconnect: + // 16 starts and 0 verdicts in one capture. Charged to the budget, because "the link will not + // survive being asked" is a stronger reason to stop asking than a strap that merely stayed quiet. + // + // BOTH stages, because stage 2 has the identical hole: the verdict timer is cancelled just above, + // so a link lost during the GET_CLOCK wait would also report nothing and also fail to spend a + // budget attempt. The two get DIFFERENT lines — stage 1's loss carries the CLIENT_HELLO signature, + // stage 2's carries no finding at all — because conflating them is the mistake this probe keeps + // having to unpick. + if (unbondedProbeSubscribing || unbondedProbeAwaitingReply) { + val uptime = if (connectedAtMs > 0L) System.currentTimeMillis() - connectedAtMs else -1L + log( + if (unbondedProbeSubscribing) unbondedProbeLinkLostLine( + uptimeMs = uptime, + confirmedSubscribes = unbondedProbeSubscribed, + total = WHOOP5_NOTIFY_CHARS.size, + ) else unbondedProbeLinkLostAskingLine( + uptimeMs = uptime, + waitedMs = if (unbondedProbeAskedAtMs > 0L) + System.currentTimeMillis() - unbondedProbeAskedAtMs else -1L, + ), + ) + unbondedProbeSilentLinks++ + if (!unbondedProbeStillWorthAsking(unbondedProbeSilentLinks)) { + log(unbondedProbeGaveUpLine(unbondedProbeSilentLinks)) + } + } unbondedProbeStartedThisLink = false unbondedProbeSubscribed = 0 unbondedProbeSubscribing = false diff --git a/android/app/src/test/java/com/noop/ble/UnbondedOffloadProbeTest.kt b/android/app/src/test/java/com/noop/ble/UnbondedOffloadProbeTest.kt index ceebb95e33..d31e25e60f 100644 --- a/android/app/src/test/java/com/noop/ble/UnbondedOffloadProbeTest.kt +++ b/android/app/src/test/java/com/noop/ble/UnbondedOffloadProbeTest.kt @@ -88,6 +88,70 @@ class UnbondedOffloadProbeTest { assertFalse(probe(silentLinksSoFar = UNBONDED_PROBE_MAX_SILENT_LINKS + 5)) } + /** + * The exit that had no verdict, and it cost the probe its bound. Field capture: 16 probe starts, 0 + * verdicts of any kind, 0 confirmed subscribes, 0 refusals, every link dying 10.8s in — about three + * seconds after the CCCD writes. With nothing concluded the silence budget never advanced, so it + * re-ran on every reconnect forever: the unbounded retry this file's own doc claims to prevent. + */ + @Test + fun `a link lost mid-subscribe is a verdict, not an absence`() { + val line = unbondedProbeLinkLostLine(uptimeMs = 10_800L, confirmedSubscribes = 0, total = 4) + assertTrue(line.contains("10800ms")) + assertTrue(line.contains("0 of 4")) + // It must name the signature rather than just the failure: no callback AND no error, then a drop, + // is what the CLIENT_HELLO does on the same service — that is the finding, not the silence. + assertTrue(line.contains("no ATT error")) + assertTrue(line.contains("CLIENT_HELLO")) + assertTrue(line.contains("not\n reachable") || line.contains("not reachable")) + assertTrue(line.contains("#1635")) + } + + /** + * The same hole, one stage later. The verdict timer is cancelled on teardown, so a link lost during + * the GET_CLOCK wait would also report nothing and also fail to spend a budget attempt — reopening the + * unbounded retry immediately below where it was closed. + * + * It must NOT reuse stage 1's line. Stage 1's loss carries a finding (no callback, no ATT error, then + * a drop — the CLIENT_HELLO's signature). Stage 2's carries none: the subscribes landed, so the + * transport was open and the strap was still inside its window. Conflating them would manufacture + * evidence out of an inconclusive link. + */ + @Test + fun `a link lost while asking settles nothing, and says so`() { + val line = unbondedProbeLinkLostAskingLine(uptimeMs = 9_000L, waitedMs = 2_500L) + assertTrue(line.contains("9000ms")) + assertTrue(line.contains("2500ms")) + assertTrue(line.contains("settles nothing")) + assertTrue(line.contains("#1635")) + // The stage-1 finding must not leak into it. + assertFalse(line.contains("CLIENT_HELLO")) + assertFalse(line.contains("no ATT error")) + } + + /** + * A partial subscribe must report honestly rather than rounding to zero — it is a different fact about + * the strap than "none of them landed". + */ + @Test + fun `a partial subscribe is reported as partial`() { + assertTrue(unbondedProbeLinkLostLine(9_000L, confirmedSubscribes = 2, total = 4).contains("2 of 4")) + } + + /** + * The supersede line must not warn about a pairing the SAME branch prevents. It said "a pairing in + * flight makes a refusal unattributable" while returning before the pairing request — so the one + * capture this exists to produce carried a caveat that could not apply, and it briefly cast doubt on + * a clean result. + */ + @Test + fun `the supersede line does not warn about a pairing it prevents`() { + val clash = unbondedProbeSupersedesLine(explicitBondOptedIn = true) + assertTrue(clash.contains("ALSO skipped")) + assertTrue(clash.contains("attributable to the strap")) + assertFalse(clash.contains("in flight makes a refusal unattributable")) + } + @Test fun `the give-up line says why it stopped, not merely that it did`() { // The CLIENT_HELLO's suppression stopped silently and cost eleven weeks of unreadable captures. @@ -249,16 +313,16 @@ class UnbondedOffloadProbeTest { * attribute a refusal to the strap. */ @Test - fun `the supersede line explains the absence and flags the clash`() { + fun `the supersede line explains the absence and names the other switch`() { val clean = unbondedProbeSupersedesLine(explicitBondOptedIn = false) assertTrue(clean.contains("handshake skipped")) assertTrue(clean.contains("press Connect")) assertFalse(clean.contains("Ask Android to pair")) assertTrue(clean.contains("#1635")) - val clash = unbondedProbeSupersedesLine(explicitBondOptedIn = true) - assertTrue(clash.contains("Ask Android to pair")) - assertTrue(clash.contains("unattributable")) + // Mentioned so a reader knows it is on, but as SKIPPED rather than as interference — the + // attributability claim it used to make is asserted in its own case below. + assertTrue(unbondedProbeSupersedesLine(explicitBondOptedIn = true).contains("Ask Android to pair")) } /** diff --git a/project.yml b/project.yml index b4ecdc5dea..fa4e5a3af9 100644 --- a/project.yml +++ b/project.yml @@ -21,7 +21,7 @@ settings: # iOS build number (CFBundleVersion). GLOBAL so the iOS app AND its widget extension inherit the # SAME value — they must match or iOS warns "extension version must match parent app" (#416). # macOS sets its own CFBundleVersion on the Strand target and is unaffected by this. - CURRENT_PROJECT_VERSION: "274" + CURRENT_PROJECT_VERSION: "275" SWIFT_VERSION: "5.0" # Emit localizable strings so Xcode auto-extracts every LocalizedStringKey into the # String Catalog on build (the English (US) base entries).