Skip to content

ble: observe the OS pairing flow, which NOOP has never watched - #1639

Merged
ryanbr merged 2 commits into
mainfrom
feat/bond-state-trace
Aug 26, 2026
Merged

ryanbr merged 2 commits into
mainfrom
feat/bond-state-trace

Conversation

@ryanbr

@ryanbr ryanbr commented Aug 26, 2026

Copy link
Copy Markdown
Owner

What #1638 answered, and what it left

The CLIENT_HELLO instrumentation resolved the first question immediately. On a WHOOP 5/MG, every attempt is the same failure:

CLIENT_HELLO outcome: NO write callback after 3153ms …
CLIENT_HELLO outcome: NO write callback after 3158ms …
CLIENT_HELLO outcome: NO write callback after 3160ms …

9 of 9 in a connection bundle, 8 of 8 in a strap log. And the timing is the tell:

n=9   min=3153ms   max=3163ms   mean=3158ms   spread=10ms

A 10 ms spread across nine attempts is a timer, not a link dying of interference. There is no NOOP-side timer at that interval — the only ~3 s constants in the BLE client are RECONNECT_DELAY_MS and RSSI_READ_DELAY_MS, neither of which tears anything down.

Two explanations fit that regularity, and nothing in the log separates them:

  1. The confirmed write to an encryption-requiring characteristic triggers OS pairing, which fails or times out and takes the link with it.
  2. The write never triggers pairing at all, and the cause lies elsewhere entirely.

The change

NOOP has never observed ACTION_BOND_STATE_CHANGED — there is no such receiver anywhere in the codebase, so the OS transition BOND_NONE → BOND_BONDING → BOND_NONE is invisible to us.

bond state: BOND_NONE -> BOND_BONDING device=FD:… 120ms after CLIENT_HELLO
bond state: BOND_BONDING -> BOND_NONE device=FD:… 3158ms after CLIENT_HELLO — pairing did NOT complete

Seeing that across the window answers the question. Seeing no transition at all answers it the other way, and that absence is just as conclusive — which is why the line has to be equally legible about a transition happening and about one not happening.

Registered and unregistered alongside the existing radio receiver, in the same ContextCompat / RECEIVER_NOT_EXPORTED form and under the same already-registered flag, so the pair cannot drift apart or double-register (the flag's doc now says it covers both).

The elapsed time is measured against the outstanding CLIENT_HELLO and omitted when there is none — a transition from an unrelated pairing must not be timed against a write it has nothing to do with. There is a test for that.

Android-only: CoreBluetooth performs pairing opaquely and exposes no equivalent transition, so there is nothing to twin.

What this replaces, and why

I first proposed deferring commands until the handshake completes, having read → DISABLE_ALARM in the log as a second GATT write landing inside the outstanding CLIENT_HELLO window — in 15 of 16 cycles.

That was wrong. That line is logged at enqueue:

enqueueWrite(PendingWrite(frame, withResponse, cmd))
log("→ ${cmd.name} payload=…")

and drainWriteQueue opens with if (writeInFlight) return. The CLIENT_HELLO holds that flag, so the command is queued and held — the one-outstanding-operation rule was never violated, and the fix I proposed was unnecessary. I had counted a log line as a wire event. Corrected on the issue before building this instead.

One real consequence does follow from the bug: because the ACK never arrives, writeInFlight stays true for the life of the link, so the whole command queue is wedged behind it. That is an effect, not a cause.

Verification

  • compileFullDebugKotlin clean; full com.noop.ble suite (438 tests) green.
  • 6 tests on the pure line, including the two that matter for reading a capture correctly: a failed pairing must be called out rather than inferred, and a transition with no outstanding hello must carry no elapsed time.
  • doc_comment_lint and i18n_audit --ci main clean — gated on, not run alongside the commit.
  • No Swift changes, so no app-build dependency.

ryanbr added 2 commits August 26, 2026 17:00
#1638's instrumentation answered the first question and left one: on a
WHOOP 5/MG, every CLIENT_HELLO goes unacknowledged (9 of 9 and 8 of 8
across two captures) and the link is torn down locally on a clockwork
timer — mean 3158ms, 10ms spread over nine attempts. No NOOP-side timer
exists at that interval.

Two explanations fit that regularity equally well, and nothing in the log
separates them:

  - the confirmed write to an encryption-requiring characteristic triggers
    OS pairing, which fails or times out and takes the link with it;
  - the write never triggers pairing at all, and the cause is elsewhere.

NOOP has no ACTION_BOND_STATE_CHANGED receiver anywhere, so the OS
transition BOND_NONE -> BOND_BONDING -> BOND_NONE is invisible to us.
Seeing it across that window answers the question; seeing no transition at
all answers it the other way, and the absence is just as conclusive.

Registered and unregistered alongside the existing radio receiver, in the
same ContextCompat/RECEIVER_NOT_EXPORTED form and under the same
already-registered flag, so the pair cannot drift apart or double-register.

The elapsed time is measured against the outstanding CLIENT_HELLO and is
OMITTED when there is none: a transition belonging to some other pairing
must not be timed against a write it has nothing to do with.

Android-only: CoreBluetooth performs pairing opaquely and exposes no
equivalent transition, so there is nothing to twin.

Worth recording what this REPLACES. I first proposed deferring commands
until the handshake completed, having read "→ DISABLE_ALARM" in the log as
a second write landing inside the outstanding window. It is not: that line
is logged at ENQUEUE, and drainWriteQueue opens with `if (writeInFlight)
return`, so the queue already holds it. The one-outstanding-operation rule
was never violated and that fix was unnecessary. Corrected on the issue.

Verified: compileFullDebugKotlin clean; com.noop.ble suite (438 tests)
green; doc_comment_lint and i18n_audit clean, gated rather than run
alongside the commit.
The receiver hears ACTION_BOND_STATE_CHANGED for every device the phone pairs
with - headphones, a car kit, someone else's keyboard - and unconditionally wrote
a "bond state:" line for each one. Two problems with that in a file people attach
to public issues: unrelated pairings crowd a fixed-size rolling buffer, and a line
that reads as though it concerns the strap while describing a headset is exactly
the confidently-misleading diagnostic this trace was added to avoid.

shouldTraceBondState() gates on the event address matching the strap we are talking
to, compared case-insensitively - a case-sensitive compare would trace nothing at
all, which looks identical to "the pairing never happened", one of the two answers
the trace exists to tell apart. An address-less event is traced only while a
CLIENT_HELLO is outstanding, since outside that window it cannot be attributed to us.

Pure, so the scoping rule is tested without a radio. 10 tests, all green.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant