Repository navigation
The unbonded offload probe's silence budget must outlive the process (#1635) - #1761
Merged
Merged
Conversation
…1635) A field capture on 31 Aug shows the probe starting 18 times across 24 connects, each start writing the puffin CCCDs and each link dying ~4.8s later — an endless reconnect loop, and a permanent "Reconnecting to your WHOOP" notification. The three-link silence budget was meant to stop exactly this, and it did, within one process. Nothing bounded the processes: the foreground service restarts, the counter went back to zero, and the probe bought three more link-killing attempts on a strap that had already answered the same way. Bounded within a process and endless across them is not bounded. The budget is now persisted per device. It moves onto PuffinExperiment rather than sitting beside the refusal latch in NoopPrefs, because the switch's setter clears these budgets by prefix — it has no device in hand — and a sweep can only reach its own prefs file. Written to one file and swept from the other, re-enabling the switch would have cleared nothing and the probe would have stayed retired forever, silently. Keeping the budget on the object that owns the switch makes that drift unrepresentable rather than merely documented. Turning the experiment on clears every strap's budget, so a user still gets another try by flipping the switch — and the give-up line now says so instead of promising only a session. That intent is sampled in the setter and not at connect: a stable link produces no connects, so a switch flipped off and on while idle would never be seen. Both charge sites — "asked and heard nothing" and "link died mid-probe" — now share one path, since a quiet link and a link torn down by the very subscriptions we wrote are the same evidence about this strap. Android only; there is no Swift probe to twin.
Three findings from re-reading the change. The setter cleared every strap's budget on any write of `true`, not on the off->on edge. Only the two switches write it today, so it was correct by the grace of its callers — and the failure mode is the loop itself: a caller that rewrites the current value re-arms three more link-killing attempts through the mechanism meant to bound them. Now a pure predicate, so a test pins it rather than inspection. Two docs still described the old design. The budget constant claimed silence "deliberately does not" latch per device, which is now precisely what it does; and the switch's own doc promised opting in "costs one link and not a loop" while the field capture shows it costing 18 across 24 connects. Both now say what the code does, the second recording the correction rather than quietly dropping the claim. A new test asserted exactly what an existing one already covered; folded its record of the incident into that test instead. Merging the two doc blocks it left stacked is what the doc-comment lint caught.
…ned (#1635) The give-up line said the strap "serves those characteristics unbonded but does not act on commands written to them". That holds only for links that subscribed and then stayed quiet. Since #1749 a link LOST mid-probe charges the same budget, and those links confirm no subscribes at all — so on the 31 Aug capture, where every charge was a lost link, retiring the probe would have recorded the exact opposite of what that strap demonstrated. It is the wrong line to overclaim on. The retirement line is where a future reader goes to find out whether the offload is possible at all, and reading the stronger evidence off the weaker one is the mistake this file was built to avoid — it is what the false bond of 28 Aug was. The line now names both ways the budget can be spent and asserts neither, since the budget does not distinguish them. Nothing is lost: the per-link lines already record which happened each time, with the confirmed subscribe count. The test that pinned the old wording pinned a false claim. Repinned to the same shape: both possibilities named, neither asserted.
This was referenced Aug 31, 2026
ryanbr
added a commit
that referenced
this pull request
Aug 31, 2026
Asking Android to pair was the only probe in this area with no bound at all. The CLIENT_HELLO has a give-up after five refusals, the unbonded offload probe has a persisted three-link silence budget, and this one had nothing: gated only by optedIn, family, the two bond flags, and alreadyRequestedThisLink - a per-LINK flag. On a strap answering SMP "Pairing Not Supported" neither bond flag ever becomes true, so it fired on every connect indefinitely. That is worse than a quiet loop. Every declined request surfaces a system "Pairing rejected" notice, so the cost lands on the user rather than in a log nobody reads, and it was reported from the field as exactly that. The request now stands down once the bond give-up has latched for the device. The signal is the PERSISTED suppression latch rather than BondRefusalGiveUp.gaveUp, which lives in one process - a bound that dies with the process is the same defect corrected twice already this week, in #1761 and #1762. Gating one experiment on the other is deliberate. BondRefusalGiveUp does not track the hello; it tracks whether this strap will form an encrypted bond, which is the single thing both routes are trying to achieve and neither has. By the time it latches, the pairing request has had its five links and returned BOND_NONE every time, with an HCI capture showing SMP 0x05 underneath. The experiment concludes; it is not cut short. It ends the loop, not the experiment. Both things that clear the latch are user actions that could have changed the answer - tapping Connect, and forgetting the device - and putting a 5/MG into pairing mode then tapping Connect is the one flow known to have worked on real hardware. The trade is that a user who enters pairing mode and waits is no longer asked automatically; with the link now stable after #1762 that path was already unreliable, since it only fired when a new link happened to form. Nothing here touches connecting. The give-up this rides is the suppress branch, which stays connected and keeps live HR streaming; the pause branch belongs to an auth refusal and is untouched. The diff contains no connect, reconnect, scan or pause line. The latch is read once, above both decisions that consult it, because the pairing request is made before the hello is considered. That single read is only correct while nothing between the two writes the latch - true since #1762 - and the code now says so, with what to do instead. A one-shot line reports the stand-down: a switch that is on and silently doing nothing reads as a broken switch, and the user has just been reading Android's rejections. The line claims the retirement rather than a per-connect skip, matching its two siblings, and the branch that emits it asks the gate itself whether the give-up was the reason rather than restating half the gate's conditions - which would drift into blaming the give-up for some later check's decision. Forgetting a device re-arms it, as clearPairingHint already did. Field evidence, build 398, same strap, after the two earlier repairs: refusals climb 1 to 5 across five links, the give-up latches at 18:45:23, the hello is suppressed eight seconds later, and afterwards there are zero hellos and zero disconnects - one further pairing request, which is what this removes. The UI reached "Live HR (not fully paired)", the #1635 end state. ExplicitBondTest +2, 608 tests in com.noop.ble, zero failures on a forced clean run. compileFullDebugKotlin, doc_comment_lint and i18n_audit clean. Parity: none owed, and checked rather than assumed. All three #1635 bond experiments - noopWhoop5ExplicitBond, noopWhoop5UnbondedOffload, noopWhoop5HelloDespiteRefusal - exist only on Android, and Swift has a single consumer of the suppression latch, so there is no second decision to hoist a read for and no gate to add. CoreBluetooth exposes no explicit pairing API, which is why this is permanently one-sided. Refs #1635.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The loop
A field capture on 31 Aug (10.6.1-staging build 393,
whoop-MGB…, fw 50.41.1.0):Every probe start writes the four puffin CCCDs; the strap tears the link down a few seconds
later; we reconnect; the probe starts again. The user saw a permanent "Reconnecting to your
WHOOP…" notification and a strap re-establishing its link every ten seconds.
The control case in the same log is what makes this attributable rather than circumstantial:
the one link whose four CCCD writes were all abandoned locally lived 31.3s and ended in
connectionTimeout. Every link whose writes reached the air died at 10.8s, locallyterminated.
Why the budget did not bound it
UNBONDED_PROBE_MAX_SILENT_LINKSexists for exactly this, and it worked — within one process.Nothing bounded the processes. The foreground service restarts;
unbondedProbeSilentLinkswentback to zero; the probe bought three more link-killing attempts on a strap that had already
answered the same way three times. Bounded within a process and endless across them is not
bounded, and the file's own doc admitted as much ("the honest limit of an in-memory bound")
without following the admission anywhere.
The change
The budget is persisted per device, and it moves onto
PuffinExperimentrather than sittingbeside the refusal latch in
NoopPrefs.That placement is the load-bearing part. Turning the switch on has to hand the budget back, and
the setter has no device in hand, so it clears by prefix — and a
SharedPreferencessweep canonly reach its own file. Written to
NoopPrefsand swept fromnoop_experiments, re-enablingthe switch would have cleared nothing, and it would have done it silently, the give-up line
having already latched. Keeping the budget on the object that owns the switch makes that drift
unrepresentable instead of merely documented; a test pins the key against the prefix as well.
The re-arm is sampled in the setter and not at connect, deliberately: once the probe retires the
link is stable, a stable link produces no connects, and a switch flipped off and on while idle
would never have been seen.
Both charge sites — "asked and heard nothing" and "link died mid-probe" — now share one path. A
quiet link and a link torn down by the very subscriptions we wrote are the same evidence about
this strap, and counting only the first is how 18 starts fit into 24 connects.
The give-up line no longer promises only a session, and names the one thing that undoes it.
Behaviour
Tests
UnbondedOffloadProbeTest+4 (33 total, green). Wholecom.noop.blesuite: 604 tests, 0failures.
compileFullDebugKotlin,doc_comment_lint.pyandi18n_audit.pyclean.The one thing unit tests cannot reach is the prefs file itself — no Robolectric here — which is
why the file choice is structural rather than asserted.
Scope
Android only. There is no Swift probe to twin;
BLEManager.swiftonly carries the comment thatmotivated the experiment.
Refs #1635.