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
33 changes: 29 additions & 4 deletions android/app/src/main/java/com/noop/ble/StalledLinkDiagnostics.kt
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,13 @@ internal fun helloDeferredByExplicitBondLine(
* [helloEverWrittenThisLink] earns its place by distinguishing the two ways to arrive here that need
* opposite fixes — a hello that was written and went unanswered (a strap or timing problem) from one that
* was never written at all (a local decision, and the one this log's field capture actually hit).
*
* #1802: when the structural-unreachable case applies AND the unbonded-offload probe is available but not
* opted in, the line names the one action that exists — the "Try history sync without pairing" toggle in
* Test Centre. The probe was written for precisely this strap in precisely this state, and its stage 3 is
* SET_CLOCK, the thing the structural claim says never runs. Without this hint the diagnostic reads as
* hopeless when it is not. The probe stays opt-in and self-limiting; this is about discoverability, not
* about running it automatically.
*/
internal fun backfillDeferredLine(
family: String?,
Expand All @@ -95,6 +102,8 @@ internal fun backfillDeferredLine(
explicitBondRequestedThisLink: Boolean,
deferralsThisLink: Int,
msSinceConnect: Long,
unbondedProbeOptedIn: Boolean = false,
unbondedProbeRetired: Boolean = false,
): String {
val since = if (msSinceConnect >= 0L) "${msSinceConnect / 1000}s" else "?"
// NULL means service discovery has not established the family yet. `connectedFamily` defaults to
Expand All @@ -105,10 +114,26 @@ internal fun backfillDeferredLine(
val why = if (family == "WHOOP5" && !didBond && !helloEverWrittenThisLink) {
// A structural claim, not a guess about the strap: didBond is set only by the hello's own ack,
// so no hello written means this gate cannot open on this link no matter what the strap does.
" No hello was written on this link, so didBond cannot become true and this gate cannot open" +
" for the rest of it. SET_CLOCK rides the same handshake tail, and an un-clocked 5/MG is" +
" hardware-known not to persist sensor data to flash (#78 fork) — so there may also be" +
" nothing banked to offload."
val base = " No hello was written on this link, so didBond cannot become true and this gate" +
" cannot open for the rest of it. SET_CLOCK rides the same handshake tail, and an un-clocked" +
" 5/MG is hardware-known not to persist sensor data to flash (#78 fork) — so there may also" +
" be nothing banked to offload."
// #1802: name the unbonded-offload probe when it is the one action that exists for this state.
// The probe's stage 3 is SET_CLOCK — the thing the structural claim above says never runs — and
// it is designed for precisely this strap. Without this hint the diagnostic reads as hopeless
// when it is not. NOT a recommendation to run it automatically: the probe is opt-in and
// self-limiting, and #1804 showed a local teardown can produce a false negative, so the hint
// says where the toggle is rather than promising an answer.
val probe = when {
unbondedProbeOptedIn -> "" // the probe is on — its own lines say what it found
unbondedProbeRetired -> " The \"Try history sync without pairing\" experiment has retired for" +
" this strap — turn it off and on in Test Centre to retry (#1804 fixed a false negative" +
" that could have latched it)."
else -> " The \"Try history sync without pairing\" experiment in Test Centre can test whether" +
" the offload works without a bond — its stage 3 is SET_CLOCK, the thing this gate" +
" blocks (#1635, #1802)."
}
base + probe
} else {
""
}
Expand Down
8 changes: 8 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 @@ -9856,6 +9856,14 @@ class WhoopBleClient(
explicitBondRequestedThisLink = explicitBondRequestedThisLink,
deferralsThisLink = backfillDeferralsThisLink,
msSinceConnect = if (connectedAtMs > 0L) System.currentTimeMillis() - connectedAtMs else -1L,
// #1802: name the unbonded-offload probe when the structural-unreachable case applies.
// The probe is the one action that exists for this state, and without this hint the
// diagnostic reads as hopeless when it is not.
unbondedProbeOptedIn = puffinExperiment.unbondedOffload,
unbondedProbeRetired = unbondedProbeRetired(
previouslyRefused = unbondedOffloadPreviouslyRefused(lastDeviceAddress),
silentLinksSoFar = unbondedProbeSilentLinks,
),
))
return
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,81 @@ class StalledLinkDiagnosticsTest {
assertTrue(backfillDeferredLine("WHOOP5", false, false, true, 1, 0L).contains("sinceConnect=0s"))
}

// ---- #1802: unbonded-offload probe discoverability ---------------------------------------

/**
* The structural-unreachable case with the probe NOT opted in and NOT retired is the one where the
* hint matters most: the diagnostic reads as hopeless, but the probe can test exactly this state.
* The line must name the toggle and say what it does, without promising an answer.
*/
@Test
fun `the unreachable case names the probe toggle when not opted in`() {
val line = backfillDeferredLine(
"WHOOP5", false, false, true, 3, 42_000L,
unbondedProbeOptedIn = false, unbondedProbeRetired = false,
)
assertTrue(line, line.contains("Try history sync without pairing"))
assertTrue(line, line.contains("Test Centre"))
assertTrue(line, line.contains("SET_CLOCK"))
}

/**
* When the probe IS opted in, its own lines report what it found — the deferral line must not
* duplicate that. The hint is only for the user who has NOT turned it on.
*/
@Test
fun `the probe hint is suppressed when the probe is opted in`() {
val line = backfillDeferredLine(
"WHOOP5", false, false, true, 3, 42_000L,
unbondedProbeOptedIn = true, unbondedProbeRetired = false,
)
assertFalse(line, line.contains("Try history sync without pairing"))
}

/**
* When the probe has retired (latched refusal or spent silence budget), the hint says so and
* points at the off/on retry — #1804 fixed a false negative that could have latched it.
*/
@Test
fun `the probe hint names the retry path when retired`() {
val line = backfillDeferredLine(
"WHOOP5", false, false, true, 3, 42_000L,
unbondedProbeOptedIn = false, unbondedProbeRetired = true,
)
assertTrue(line, line.contains("retired"))
assertTrue(line, line.contains("turn it off and on"))
assertTrue(line, line.contains("#1804"))
}

/**
* The probe hint must NOT appear on the non-unreachable cases — a WHOOP4 or a hello that was
* written and went unanswered is a different problem with a different fix.
*/
@Test
fun `the probe hint only appears on the structural-unreachable case`() {
// WHOOP4: no probe, no hint
assertFalse(backfillDeferredLine(
"WHOOP4", false, false, false, 1, 5_000L,
unbondedProbeOptedIn = false, unbondedProbeRetired = false,
).contains("Try history sync without pairing"))
// Hello written but unanswered: different problem
assertFalse(backfillDeferredLine(
"WHOOP5", false, true, true, 3, 42_000L,
unbondedProbeOptedIn = false, unbondedProbeRetired = false,
).contains("Try history sync without pairing"))
}

/**
* The default values (not opted in, not retired) preserve the old behaviour for callers that
* have not been updated — the hint appears on the unreachable case.
*/
@Test
fun `default probe params preserve the old unreachable behaviour plus the hint`() {
val line = backfillDeferredLine("WHOOP5", false, false, true, 3, 42_000L)
assertTrue(line, line.contains("No hello was written"))
assertTrue(line, line.contains("Try history sync without pairing"))
}

// ---- liveInsertFailedLine -----------------------------------------------------------------

/**
Expand Down