Skip to content

ble: instrument the actual MTU settle time (follow-up to #1066) #1153

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ryanbr merged 2 commits into from
Aug 8, 2026
Merged

ble: instrument the actual MTU settle time (follow-up to #1066) #1153

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
57a841e
ble: instrument the actual MTU settle time (follow-up to #1066)
ryanbr Aug 8, 2026
a28e530
ble: also log the dedup'd MTU callback's timing (re-review fix)
ryanbr Aug 8, 2026
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
22 changes: 20 additions & 2 deletions android/app/src/main/java/com/noop/ble/WhoopBleClient.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4510,6 +4510,12 @@ class WhoopBleClient(
private var lastMtuValue = -1
private var lastMtuAtMs = 0L

/** #1066 follow-up: wall-clock of the `requestMtu` attempt, so `onMtuChanged` can log how long the MTU
* actually took to settle. `MTU_DISCOVERY_SETTLE_MS` (1.5s) is a deliberately-conservative FIXED wait
* (no provenance bit to end it early), added to EVERY connect's discovery; this measurement is what
* lets that constant be tuned down later with capture DATA rather than a guess. Reset per connection. */
@Volatile private var mtuRequestedAtMs = 0L

/** Start service discovery exactly once for the captured connection. */
@SuppressLint("MissingPermission")
private fun kickServiceDiscovery(g: BluetoothGatt, expectedGeneration: Int, reason: String) {
Expand Down Expand Up @@ -4680,6 +4686,7 @@ class WhoopBleClient(
// follows one fixed settle delay whether the request is accepted or rejected, so a
// stack that ignores requestMtu cannot stall the connect. (PR #85)
val mtuOps = gattOps
mtuRequestedAtMs = System.currentTimeMillis() // #1066: measure the real settle time
val mtuOk = mtuOps != null &&
safeGatt("requestMtu") { mtuOps.requestMtuCompat(GATT_MTU) }
if (mtuOk) {
Expand Down Expand Up @@ -4711,15 +4718,25 @@ class WhoopBleClient(
// callback, but duplicate telemetry is still noise and should not repeat future callback work.
val now = System.currentTimeMillis()
if (now - lastMtuAtMs < DUPLICATE_MTU_WINDOW_MS && mtu == lastMtuValue) {
log("Ignoring duplicate MTU callback (mtu=$mtu) — OnePlus/spurious")
// #1066 follow-up: still log THIS callback's timing before dropping it. A same-value second
// callback is often our requestMtu completing AFTER a fast connection-event MTU — dropping it
// silently would make the settle measurement under-report the real bound the 1.5s must cover.
val dupMs = if (mtuRequestedAtMs > 0L) now - mtuRequestedAtMs else -1L
log("Ignoring duplicate MTU callback (mtu=$mtu) — OnePlus/spurious" +
if (dupMs >= 0L) " (${dupMs}ms after request)" else "")
return
}
lastMtuValue = mtu
lastMtuAtMs = now
// Whatever the strap granted (≤ requested). Telemetry only: Android can emit this callback
// for the connection itself as well as requestMtu, without saying which. Starting discovery
// here can overlap the still-running MTU operation and wedge service discovery.
log("MTU negotiated: $mtu (status=$status)")
// #1066 follow-up: log the ACTUAL settle time vs the fixed MTU_DISCOVERY_SETTLE_MS wait, so a
// capture reveals how much headroom that 1.5s has before discovery. -1 when no request preceded
// this callback (a bare connection-event MTU).
val settledMs = if (mtuRequestedAtMs > 0L) now - mtuRequestedAtMs else -1L
log("MTU negotiated: $mtu (status=$status)" +
if (settledMs >= 0L) " — settled ${settledMs}ms after request (fixed wait ${MTU_DISCOVERY_SETTLE_MS}ms)" else "")
}

/** #533: what the controller and the strap ACTUALLY settled on — the request is only a preference
Expand Down Expand Up @@ -7444,6 +7461,7 @@ class WhoopBleClient(
// the same strap with the same granted mtu — is never mistaken for a duplicate of the last one.
lastMtuValue = -1
lastMtuAtMs = 0L
mtuRequestedAtMs = 0L // #1066: don't measure settle time across a connection boundary
// The strap forgets the realtime-HR toggle across a disconnect; the post-bond branch re-arms it
// from [wantsRealtime]. Clear only the "what we last sent" flag — the screen/preference WANTS
// ([screenWantsRealtime]/[keepStreamForData]/[wantsRealtime]) are intent and must survive a
Expand Down