Repository navigation
Asking Android to pair must not clear the hello give-up every link (#1635) - #1762
Merged
Merged
Conversation
…1635) A capture on 31 Aug shows the give-up doing its job and being undone. The suppression latched at 17:32:48 — "never acknowledged the secure handshake 5x in a row, leaving the handshake off so live heart rate keeps streaming" — and then 13 more CLIENT_HELLOs went out, each dropping the link ~4.8s in. The line printed when a hello is actually skipped appears zero times in the whole log. With "Ask Android to pair" on, the explicit-bond request cleared the suppression latch. The comment there argued this was safe because it happened "ONCE, here", as against being re-armed by "an OS pairing exists", a condition that never goes away. It guarded the wrong recurrence: shouldRequestExplicitBond is bounded only by alreadyRequestedThisLink, a per-LINK flag, and on a strap answering SMP "Pairing Not Supported" neither bond condition ever becomes true. So createBond runs on every connect — 36 times in that capture — and the clear ran with it. Nor could it recover. recordRefusal() reports the crossing exactly once and stays gaveUp until reset, so the latch is written once per session; a clear on a recurring event does not cost one link, it costs the latch permanently. The designed end state — live HR, handshake off, link stable — was unreachable while that switch was on. The clear is removed. Recovery is the explicit Connect the epitaph already tells the user to tap. The encrypted-bond clear is deliberately not claimed as an alternative: it lives inside the hello write-completion callback, so a suppressed hello cannot reach it. The pairing experiment is untouched — createBond still runs on every link, it just no longer drags the hello along, which helloDeferredByExplicitBond says must never share a link. Two tests pin the contradiction that produced this: the request recurs on every link, and the give-up reports its crossing once.
The count in the previous commit came from grepping "createBond", and half of those lines are the bond-state poll that merely mentions it. The capture holds 18 explicit pairing requests, not 36. The corrected figure is the better evidence anyway: 18 pairing requests and 18 CLIENT_HELLOs, one of each on every link — which is precisely the pairing helloDeferredByExplicitBond exists to forbid, stated as a count rather than as an argument. 13 of those hellos land after the give-up.
The test added for the recurrence called a pure function five times with the same arguments and asserted the same thing each time. That is theatre, not rigour — it pins nothing the neighbouring "an opted-in unbonded 5-MG is asked to pair" does not already pin. The invariant worth holding is that the ONLY bound on this gate is per link. Two assertions say it: the permanent state of a strap answering SMP "Pairing Not Supported" still returns true, and the one input that returns false is the per-link flag every new link clears. Together they are the reason a once-only side effect hung off this request runs on every connect. Carries the corrected count with it.
…ch (#1635) The comment amended on "an existing OS pairing does NOT permanently bypass the latch" said the latch is cleared by "a completed bond or an explicit Connect, both of which are things that actually happened". The second clause is right; the first repeats the overclaim this branch had already corrected once in the production comment, and it is the third time this shape has appeared in two days of work on this file. A genuine bond clears the latch from inside the hello WRITE-COMPLETION callback, so a suppressed hello cannot reach it. Naming it as a live route tells the next reader that suppression resolves itself, which is exactly the wrong expectation to leave behind — that reader would go looking for a bug when the app sat suppressed forever, correctly. Now names all three clears and marks which one suppression puts out of reach: explicit Connect, forgetting the device, and the genuine bond that needs a hello on the wire to be noticed.
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 give-up worked, and was undone
A capture on 31 Aug (build 397, 5/MG, fw 50.41.1.0) has two disconnect bursts
sixteen hours apart, and the gap between them is the point — the link was
perfectly stable all night, so the strap is not what is unstable.
The second burst should have ended itself. At 17:32:48 the give-up fired exactly
as designed:
Then 13 more CLIENT_HELLOs.
CLIENT_HELLO suppressed for this strap— theline printed when a hello is actually skipped — appears zero times in the
entire log.
Why
With "Ask Android to pair" on, the explicit-bond request cleared the suppression
latch. The comment there reasoned it was safe:
It guarded the wrong recurrence.
shouldRequestExplicitBondis bounded only byalreadyRequestedThisLink, a per-link flag; on a strap answering SMP"Pairing Not Supported" neither bond condition ever becomes true. So
createBondfires on every connect and the clear fired with it. The capture holds 18
pairing requests and 18 CLIENT_HELLOs — one of each on every link, which is
exactly the pairing
helloDeferredByExplicitBondexists to forbid. The commentdescribes its own effect and rules it out in the same breath.
It could not recover, either.
recordRefusal()reports the crossing exactly onceand stays
gaveUpuntilreset(), so the latch is written once per session.A clear on a recurring event does not cost one link and self-correct — it costs
the latch permanently. The #1635 end state (live HR, handshake off, link stable)
was simply unreachable while that switch was on.
The change
The clear is removed. Recovery is the explicit Connect the epitaph already names,
cleared through
clearPairingHintForUserConnect().The encrypted-bond clear is deliberately not offered as an automatic
alternative:
clearPairingHint()on a genuine bond lives inside the hellowrite-completion callback, so a suppressed hello cannot reach it. Claiming it
would be the same overclaim the retirement line in #1761 had to be corrected for.
That costs nothing real — the latch only sets after five consecutive refusals, so
"latched, and pairing now works" requires something to have changed (new
firmware, the strap put into pairing mode as on @Zebsi235's MG), and a user who
has changed something taps Connect.
The pairing experiment is untouched:
createBondstill runs on every link. Itjust no longer drags the hello along with it — which is exactly what
helloDeferredByExplicitBondsays must never share a link:Tests
Two pins for the contradiction that produced this, one on each side:
ExplicitBondTest— the request recurs on every link, so nothing may hang aonce-only side effect off it.
HelloSuppressionTest— the give-up reports its crossing once, so the latch iswritten once and a recurring clear is permanent.
A stale comment on
an existing OS pairing does NOT permanently bypass the latchcalled the pairing request the "self-limiting" alternative; it was not, and now
says so.
606 tests in
com.noop.ble, zero failures on a forced clean run.compileFullDebugKotlin,doc_comment_lint.py,i18n_audit.pyclean.What this does not settle
With the hello suppressed,
createBondstill runs once per link on a strap thatrefuses SMP. Nothing in this capture says whether that alone is enough to keep a
link up, because the sixteen stable hours contained no connects at all. If the
next capture still shows a loop, that is the remaining cause and it will be
readable for the first time — with the hello finally out of the way.
Parity
Checked rather than assumed. Swift has the same
BondRefusalGiveUpmirror and thesame latch (
HelloSuppressionStore), and its three writes are all sound: therelease path (twin of Kotlin's forget-device clear), the give-up write, and one
clear gated on
ClientHelloOutcome.isAck— an outcome, not a request. No twinof this bug exists there, and could not: CoreBluetooth has no
createBond, whichis why the offending clear is Android-only in the first place.
Android only. Refs #1635.