Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion android/app/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
51 changes: 49 additions & 2 deletions android/app/src/main/java/com/noop/ble/UnbondedOffloadProbe.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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)"

Expand Down Expand Up @@ -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.
*
Expand Down
29 changes: 29 additions & 0 deletions android/app/src/main/java/com/noop/ble/WhoopBleClient.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
72 changes: 68 additions & 4 deletions android/app/src/test/java/com/noop/ble/UnbondedOffloadProbeTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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"))
}

/**
Expand Down
2 changes: 1 addition & 1 deletion project.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
Loading