Skip to content

ble: faster history sync — experimental toggle for offload connection priority (#533) - #536

Merged
ryanbr merged 3 commits into
mainfrom
feat/offload-connection-priority-533
Jul 17, 2026
Merged

ble: faster history sync — experimental toggle for offload connection priority (#533)#536
ryanbr merged 3 commits into
mainfrom
feat/offload-connection-priority-533

Conversation

@ryanbr

@ryanbr ryanbr commented Jul 17, 2026

Copy link
Copy Markdown
Owner

The finding

#477 shipped GATT connection-priority management fully implemented but dormant. setConnectionPriorityManagement is never called from anywhere in production (grep: only its own definition), so connectionPriorityEnabled stays false, refreshConnectionPriority() early-returns at every call site, and every historical offload to date has run at the stack default.

The hook is already in exactly the right place — it just was never switched on:

// enterBackfilling
refreshConnectionPriority()   // #477: escalate to HIGH for the offload burst (faster sync). No-op unless enabled.

The source even says "a follow-up wires it to a persisted Settings toggle" — that follow-up never landed. This PR is that follow-up.

Relevant to #533 (historical sync ~10× slower than the official app): before investing in HCI captures to find a hypothetical bulk-offload command, the cheapest experiment is to turn on an escalation we already wrote. On Android CONNECTION_PRIORITY_HIGH is ~11.25 ms vs BALANCED ~30 ms.

What this does

Adds Settings → Strap → "Faster history sync (experimental)", default OFF. When on, it enables the safe half of #477:

Why live-HR is excluded (the battery reasoning)

realtimeArmed is set from screenWantsRealtime || continuousCaptureWantsNow(), and continuousCaptureWantsNow() covers the overnight continuous-HRV window (22:00–07:00 by default) — not just an open Live screen. Escalating it would hold an ~11.25 ms interval for hours every night to carry a 1 Hz HR/RR stream that BALANCED already serves: sustained drain on strap and phone, zero throughput gain. The source itself calls the realtime stream "one of the larger drains."

It's a knob rather than a deletion because the opt-in R22 deep-buffer capture is high-rate and is the one live case that could legitimately want HIGH — so the resolver branch stays live, not dead.

Battery

Why a toggle rather than default-on

BLE behaviour cannot be CI- or Linux-tested, so neither the speedup nor the battery cost can be proven in this PR. An opt-in experimental toggle lets it ship and gather real field reports instead of blocking on a single strap, and anyone who finds it unhelpful can switch it straight back off.

It sits in the Strap card next to "Keep connected in the background" — deliberately not in Power saving, since that card is about saving power and this switch spends it. It's also not gated on the Power-saving master.

Measuring needs no new instrumentation: the strap log already timestamps Backfill: session started / session ended next to Backfill: session persisted N rows, so records/sec is directly comparable from a before/after log on a real backlog.

Re-review catch: turning it off now actually turns it off

refreshConnectionPriority() early-returns on !connectionPriorityEnabled, so setConnectionPriorityManagement(enabled = false) issued no BLE op at all — disabling only stopped future escalations and left a link already pinned at CONNECTION_PRIORITY_HIGH there until the next reconnect.

That broke this toggle's own promise: the Settings copy says to turn it back off if it costs more battery than it's worth, but a user doing exactly that would keep paying — potentially for hours, since "Keep connected in the background" (the switch directly above it) holds the link open.

Now a real on→off edge requests BALANCED once, handing the link back to the stack default. Every other transition still issues nothing — notably the default launch path, which re-applies enabled = false while already off and stays byte-for-byte today's zero-BLE-op behaviour. The edge rule is a pure companion predicate (releasesConnectionPriority), matching this file's idiom, since WhoopBleClient needs a Context and can't be built in the pure-JVM suite.

(The latent flaw is #477's — but nothing could reach it while the feature was dormant. Exposing the toggle is what makes the disable path reachable, so it's fixed here.)

Tests

  • python3 Tools/i18n_audit.py --ciOK, no hardcoded literals, all focus locales complete (strings added to values + de/es/fr).
  • compileFullDebugKotlin + ConnectionPriorityTest11/11 pass — two cases pinning that an armed live/overnight stream stays BALANCED while an offload burst in that same window still escalates, plus two pinning that only the on→off edge releases the link.
  • Not validated on hardware — I have no strap. The toggle's default-off means that's safe to merge, but the escalation itself still needs an on-strap before/after before it could be trusted or ever considered for default-on.

Follow-ups (deliberately not here)

Cross-platform: no Swift twin (deliberate)

Per the parity contract this needs an explicit "why not": CoreBluetooth exposes no app-side API for this. The peripheral proposes the GAP connection parameters and Apple's stack negotiates them (and the PHY) itself — apps get no say. #477 already records the same divergence in-code: "Android-only by necessity … so there is no Swift twin — a deliberate platform divergence, not a parity gap."

Useful corollary: because iOS/macOS have their link parameters chosen for them, they are effectively "Android with these levers permanently on and untunable" — which makes them a control group. If a Mac drains a comparable backlog at a much higher rows/sec than Android (compare Backfill: session started / session ended / persisted N rows, which both platforms log identically), the strap is provably not firmware-paced, since the strap is the constant across the test. If they match, these levers likely will not help and #533's HCI capture is the better investment.

The one lever here that does have a Swift twin is the scheduler (MAX_AUTO_CONTINUES / the 15-min BACKFILL_INTERVAL_MS, mirrored by BLEManager.maxAutoContinues / BackfillPolicy.periodicFloorSeconds) — deliberately untouched in this PR, and it would need both platforms if changed.

ryanbr added 2 commits July 17, 2026 12:39
#477 shipped GATT connection-priority management fully implemented but
DORMANT: setConnectionPriorityManagement was never called from anywhere, so
connectionPriorityEnabled stayed false, refreshConnectionPriority early-returned
at every call site, and EVERY historical offload has run at the stack default.
The escalation hook is already in the right place (enterBackfilling ->
'escalate to HIGH for the offload burst (faster sync)') — nothing switched it on.

Switch on the SAFE half. The offload burst is bounded (HISTORY_COMPLETE / idle
timeout) and bandwidth-hungry, so a shorter interval moves the same bytes in
LESS radio-on wall-clock — the sync gets faster without a proportional energy
cost. The RISKY idle->LOW_POWER half stays at 0 (still dormant, #478).

Live-HR deliberately does NOT escalate (new escalateForLiveHr knob, default
off). realtimeArmed is true for the whole OVERNIGHT continuous-HRV window
(22:00-07:00 by default via continuousCaptureWantsNow), not just while a Live
screen is open — escalating it would hold an ~11.25 ms interval for HOURS to
carry a 1 Hz HR/RR stream that BALANCED already serves: sustained drain on both
strap and phone for no throughput gain. The knob keeps the resolver branch live
rather than dead, since the opt-in R22 deep-buffer capture is the one high-rate
live case that could legitimately want HIGH.

NOT VALIDATED ON HARDWARE — I have no strap. BLE behaviour can't be CI- or
Linux-tested, so the speedup and the battery delta both need an on-strap
measurement before this should be trusted. The existing strap log already
timestamps 'Backfill: session started'/'session ended' next to 'persisted N
rows', so records/sec is measurable from a before/after log with no new
instrumentation.

compileFullDebugKotlin + ConnectionPriorityTest (9) green.
…ggle

Follow-up on the same branch: rather than switching the #477 escalation on for
everyone, gate it behind Settings > Strap > 'Faster history sync (experimental)',
default OFF.

BLE behaviour can't be CI- or Linux-tested, so neither the speedup nor the
battery cost can be proven here — an opt-in toggle lets it ship and collect real
field reports instead of blocking on one person's strap, and lets anyone who
finds it unhelpful (or costly) turn it straight back off. Matches how the other
unvalidated levers are handled.

- NoopPrefs.KEY_FAST_HISTORY_SYNC (default false) + getter/setter
- AppViewModel.setFastHistorySync persists + re-applies immediately, so the next
  offload burst picks it up without a reconnect; applyPowerSaving now reads the
  pref instead of hardcoding enabled=true. NOT gated on the Power-saving master
  (this spends radio for speed; it is not a power-saving lever)
- Settings toggle lives in the Strap card next to 'Keep connected in the
  background', not in Power saving (a card about saving power is the wrong home
  for a switch that spends it)
- strings added to values + de/es/fr (the coverage half of the gate)

i18n audit OK (no hardcoded literals, all focus locales); compileFullDebugKotlin
+ ConnectionPriorityTest (9) green.
@ryanbr ryanbr changed the title ble: escalate connection priority for the offload burst (#533) ble: faster history sync — experimental toggle for offload connection priority (#533) Jul 17, 2026
…ned off

Re-review catch on my own change. refreshConnectionPriority() early-returns on
!connectionPriorityEnabled, so setConnectionPriorityManagement(enabled = false)
issued NO BLE op at all: turning the toggle off only stopped FUTURE escalations
and left a link already pinned at CONNECTION_PRIORITY_HIGH there until the next
reconnect.

That breaks the toggle's own promise. The Settings copy says to turn it back off
if it costs more battery than it's worth — but a user doing exactly that would
keep paying for it, potentially for hours, since 'Keep connected in the
background' (the switch directly above it) holds the link open.

On a real on->off edge, request BALANCED once to hand the link back to the stack
default. Every other transition still issues nothing — notably the default launch
path, which re-applies enabled=false while already off and stays byte-for-byte
today's zero-BLE-op behaviour.

The edge rule is a pure companion predicate (releasesConnectionPriority),
matching this file's existing idiom (connectionPriorityFor / idleThrottleActive /
offloadIntervalMsFor), since WhoopBleClient needs a Context and can't be built in
the pure-JVM suite. Release swallows throws like the refresh path — a priority
hint must never tear the link down.

The latent flaw is #477's, but nothing could reach it while the feature was
dormant; exposing the toggle is what makes the disable path reachable.

ConnectionPriorityTest 11/11 (adds both edge cases); compileFullDebugKotlin +
i18n audit green.
@ryanbr
ryanbr merged commit 5b1b31d into main Jul 17, 2026
1 check passed
@ryanbr
ryanbr deleted the feat/offload-connection-priority-533 branch July 17, 2026 01:12
ryanbr added a commit that referenced this pull request Jul 17, 2026
NOOP has never called setPreferredPhy — grep finds no PHY handling at all — so
every historical offload has run on the 1M PHY. LE 2M doubles the symbol rate:
the same bytes spend HALF the air-time.

That makes this the better-behaved of the two sync-speed levers. #536's
connection-interval lever buys speed with ~2.7x more radio wakeups, so its
battery effect is ambiguous and hinges on whether the offload is link- or
firmware-limited. Halving air-time for the same bytes should cost LESS radio
energy per byte, not more. The two are orthogonal and stack, and are deliberately
SEPARATE toggles: they have opposite battery profiles, so bundling them would
make a field report un-attributable.

Requested at offload START, not on connect, on purpose: the connect handshake is
fragile — an extra GATT op before requestMtu can make it return false, which
skips the MTU bump and caps the very offload this is meant to speed up (#85/#50).

The mask ALWAYS includes 1M, never 2M alone: it is a preference, and keeping 1M
lets the controller fall back rather than cling to a 2M link gone marginal (2M
trades range for speed). Turning the toggle off releases an already-2M link back
to 1M — a PHY persists once negotiated, and the toggle's own copy tells the user
to switch it off if syncing goes flaky at range, which is exactly when 2M is the
suspect. Reuses #536's releasesConnectionPriority edge rule, so the default
launch path issues zero BLE ops.

onPhyUpdate logs the PHY actually negotiated — the only way to learn whether
WHOOP supports 2M at all, and it lands next to the offload's existing session
timestamps + 'persisted N rows' for a before/after records/sec.

Android-only by necessity: CoreBluetooth exposes no app-side PHY API, so there is
no Swift twin — documented in-code, mirroring #477's precedent.

EXPERIMENTAL, default OFF. Off is byte-for-byte today. NOT validated on hardware.

i18n audit OK; compileFullDebugKotlin + the full com.noop.ble suite green
(PreferredPhyTest 6, ConnectionPriorityTest 11, GattCrashSafetyTest 7).
ryanbr added a commit that referenced this pull request Jul 17, 2026
…537)

NOOP has never called setPreferredPhy — grep finds no PHY handling at all — so
every historical offload has run on the 1M PHY. LE 2M doubles the symbol rate:
the same bytes spend HALF the air-time.

That makes this the better-behaved of the two sync-speed levers. #536's
connection-interval lever buys speed with ~2.7x more radio wakeups, so its
battery effect is ambiguous and hinges on whether the offload is link- or
firmware-limited. Halving air-time for the same bytes should cost LESS radio
energy per byte, not more. The two are orthogonal and stack, and are deliberately
SEPARATE toggles: they have opposite battery profiles, so bundling them would
make a field report un-attributable.

Requested at offload START, not on connect, on purpose: the connect handshake is
fragile — an extra GATT op before requestMtu can make it return false, which
skips the MTU bump and caps the very offload this is meant to speed up (#85/#50).

The mask ALWAYS includes 1M, never 2M alone: it is a preference, and keeping 1M
lets the controller fall back rather than cling to a 2M link gone marginal (2M
trades range for speed). Turning the toggle off releases an already-2M link back
to 1M — a PHY persists once negotiated, and the toggle's own copy tells the user
to switch it off if syncing goes flaky at range, which is exactly when 2M is the
suspect. Reuses #536's releasesConnectionPriority edge rule, so the default
launch path issues zero BLE ops.

onPhyUpdate logs the PHY actually negotiated — the only way to learn whether
WHOOP supports 2M at all, and it lands next to the offload's existing session
timestamps + 'persisted N rows' for a before/after records/sec.

Android-only by necessity: CoreBluetooth exposes no app-side PHY API, so there is
no Swift twin — documented in-code, mirroring #477's precedent.

EXPERIMENTAL, default OFF. Off is byte-for-byte today. NOT validated on hardware.

i18n audit OK; compileFullDebugKotlin + the full com.noop.ble suite green
(PreferredPhyTest 6, ConnectionPriorityTest 11, GattCrashSafetyTest 7).
ryanbr added a commit that referenced this pull request Jul 17, 2026
Two follow-ups on #536/#537, found re-reviewing them after merge.

A. The PHY was never released at offload END, unlike its twin lever.
   refreshConnectionPriority() runs at BOTH enterBackfilling and exitBackfilling,
   so the priority escalation is bounded to the burst. applyPreferredPhy() ran only
   at enterBackfilling - and a PHY PERSISTS once negotiated, so a link that went 2M
   for a sync stayed 2M for the WHOLE connection, including the overnight window.
   That is not what the toggle's copy promises (it says "while your strap hands over
   its stored history"), and it left 2M's range trade-off in force long after the
   transfer it was for.

   Release to 1M at exitBackfilling so the preference is bounded to the burst,
   symmetric with the priority lever, and the shipped copy becomes accurate rather
   than aspirational.

   Battery was NOT the reason: 2M costs LESS energy per byte, so leaving it on was
   arguably fine. The reason is that RANGE is 2M's unvalidated trade-off, and an
   experimental lever should confine its unknown to the window the user was told
   about. If field data later shows 2M is good everywhere, widen it deliberately.

   The guard lives at the call site, not inside releasePreferredPhy: that method
   cannot check the flag, because setFastLinkPhy's on->off edge calls it AFTER the
   flag is already false. Guarding at the call site keeps the default path at ZERO
   BLE ops.

B. releasesConnectionPriority was a misnomer the moment #537 reused it for the PHY:
   one predicate, two levers, a name that claimed one. Renamed to releasesOnDisable
   and its doc now says it serves both. My own mess from the reuse.

compileFullDebugKotlin + the full com.noop.ble suite green (PreferredPhyTest 6,
ConnectionPriorityTest 11, GattCrashSafetyTest 7); i18n audit OK. Still not
validated on hardware - both toggles remain default-off.
ryanbr added a commit that referenced this pull request Jul 17, 2026
…le (#538)

Two follow-ups on #536/#537, found re-reviewing them after merge.

A. The PHY was never released at offload END, unlike its twin lever.
   refreshConnectionPriority() runs at BOTH enterBackfilling and exitBackfilling,
   so the priority escalation is bounded to the burst. applyPreferredPhy() ran only
   at enterBackfilling - and a PHY PERSISTS once negotiated, so a link that went 2M
   for a sync stayed 2M for the WHOLE connection, including the overnight window.
   That is not what the toggle's copy promises (it says "while your strap hands over
   its stored history"), and it left 2M's range trade-off in force long after the
   transfer it was for.

   Release to 1M at exitBackfilling so the preference is bounded to the burst,
   symmetric with the priority lever, and the shipped copy becomes accurate rather
   than aspirational.

   Battery was NOT the reason: 2M costs LESS energy per byte, so leaving it on was
   arguably fine. The reason is that RANGE is 2M's unvalidated trade-off, and an
   experimental lever should confine its unknown to the window the user was told
   about. If field data later shows 2M is good everywhere, widen it deliberately.

   The guard lives at the call site, not inside releasePreferredPhy: that method
   cannot check the flag, because setFastLinkPhy's on->off edge calls it AFTER the
   flag is already false. Guarding at the call site keeps the default path at ZERO
   BLE ops.

B. releasesConnectionPriority was a misnomer the moment #537 reused it for the PHY:
   one predicate, two levers, a name that claimed one. Renamed to releasesOnDisable
   and its doc now says it serves both. My own mess from the reuse.

compileFullDebugKotlin + the full com.noop.ble suite green (PreferredPhyTest 6,
ConnectionPriorityTest 11, GattCrashSafetyTest 7); i18n audit OK. Still not
validated on hardware - both toggles remain default-off.
ryanbr added a commit that referenced this pull request Jul 17, 2026
Numbers are main's current 292/203 +1, matching the previous staging commit's
pattern, so the .staging app upgrades cleanly over the last testing build (290)
and stays above main's released high-water.

Carries everything merged since the 9.0.1 release:
 - Workouts read HR under the recording strap, not a hardcoded id (#512 Kotlin,
   #513 Swift twin)
 - Oura 0x60/0x80 IBI decoders corrected to the ring's real byte layout (#511)
 - Whoop5Config enable_sig12 corrected to ASCII '1' (#522)
 - Sleep time edits save as one validated window (#525)
 - "Recompute this night" for deleted sleep windows (#526)
 - Stress marker values stay readable (#507)
 - Faster history sync + faster Bluetooth link, both EXPERIMENTAL and default-off
   (#536/#537/#538) - the whole point of this build is field data on those
 - i18n changelog-title fix + main-push audit (#514/#535)

versionName stays 9.0.2 here for the staging identity only; the real 9.0.2
release still bumps main separately (and must be raised past 293 first).
DX23876 pushed a commit to DX23876/noop that referenced this pull request Jul 22, 2026
… priority (ryanbr#533) (ryanbr#536)

* ble: escalate connection priority for the offload burst (ryanbr#533)

ryanbr#477 shipped GATT connection-priority management fully implemented but
DORMANT: setConnectionPriorityManagement was never called from anywhere, so
connectionPriorityEnabled stayed false, refreshConnectionPriority early-returned
at every call site, and EVERY historical offload has run at the stack default.
The escalation hook is already in the right place (enterBackfilling ->
'escalate to HIGH for the offload burst (faster sync)') — nothing switched it on.

Switch on the SAFE half. The offload burst is bounded (HISTORY_COMPLETE / idle
timeout) and bandwidth-hungry, so a shorter interval moves the same bytes in
LESS radio-on wall-clock — the sync gets faster without a proportional energy
cost. The RISKY idle->LOW_POWER half stays at 0 (still dormant, ryanbr#478).

Live-HR deliberately does NOT escalate (new escalateForLiveHr knob, default
off). realtimeArmed is true for the whole OVERNIGHT continuous-HRV window
(22:00-07:00 by default via continuousCaptureWantsNow), not just while a Live
screen is open — escalating it would hold an ~11.25 ms interval for HOURS to
carry a 1 Hz HR/RR stream that BALANCED already serves: sustained drain on both
strap and phone for no throughput gain. The knob keeps the resolver branch live
rather than dead, since the opt-in R22 deep-buffer capture is the one high-rate
live case that could legitimately want HIGH.

NOT VALIDATED ON HARDWARE — I have no strap. BLE behaviour can't be CI- or
Linux-tested, so the speedup and the battery delta both need an on-strap
measurement before this should be trusted. The existing strap log already
timestamps 'Backfill: session started'/'session ended' next to 'persisted N
rows', so records/sec is measurable from a before/after log with no new
instrumentation.

compileFullDebugKotlin + ConnectionPriorityTest (9) green.

* settings: put the faster-offload escalation behind an experimental toggle

Follow-up on the same branch: rather than switching the ryanbr#477 escalation on for
everyone, gate it behind Settings > Strap > 'Faster history sync (experimental)',
default OFF.

BLE behaviour can't be CI- or Linux-tested, so neither the speedup nor the
battery cost can be proven here — an opt-in toggle lets it ship and collect real
field reports instead of blocking on one person's strap, and lets anyone who
finds it unhelpful (or costly) turn it straight back off. Matches how the other
unvalidated levers are handled.

- NoopPrefs.KEY_FAST_HISTORY_SYNC (default false) + getter/setter
- AppViewModel.setFastHistorySync persists + re-applies immediately, so the next
  offload burst picks it up without a reconnect; applyPowerSaving now reads the
  pref instead of hardcoding enabled=true. NOT gated on the Power-saving master
  (this spends radio for speed; it is not a power-saving lever)
- Settings toggle lives in the Strap card next to 'Keep connected in the
  background', not in Power saving (a card about saving power is the wrong home
  for a switch that spends it)
- strings added to values + de/es/fr (the coverage half of the gate)

i18n audit OK (no hardcoded literals, all focus locales); compileFullDebugKotlin
+ ConnectionPriorityTest (9) green.

* ble: release the link back to default when the sync experiment is turned off

Re-review catch on my own change. refreshConnectionPriority() early-returns on
!connectionPriorityEnabled, so setConnectionPriorityManagement(enabled = false)
issued NO BLE op at all: turning the toggle off only stopped FUTURE escalations
and left a link already pinned at CONNECTION_PRIORITY_HIGH there until the next
reconnect.

That breaks the toggle's own promise. The Settings copy says to turn it back off
if it costs more battery than it's worth — but a user doing exactly that would
keep paying for it, potentially for hours, since 'Keep connected in the
background' (the switch directly above it) holds the link open.

On a real on->off edge, request BALANCED once to hand the link back to the stack
default. Every other transition still issues nothing — notably the default launch
path, which re-applies enabled=false while already off and stays byte-for-byte
today's zero-BLE-op behaviour.

The edge rule is a pure companion predicate (releasesConnectionPriority),
matching this file's existing idiom (connectionPriorityFor / idleThrottleActive /
offloadIntervalMsFor), since WhoopBleClient needs a Context and can't be built in
the pure-JVM suite. Release swallows throws like the refresh path — a priority
hint must never tear the link down.

The latent flaw is ryanbr#477's, but nothing could reach it while the feature was
dormant; exposing the toggle is what makes the disable path reachable.

ConnectionPriorityTest 11/11 (adds both edge cases); compileFullDebugKotlin +
i18n audit green.
DX23876 pushed a commit to DX23876/noop that referenced this pull request Jul 22, 2026
) (ryanbr#537)

NOOP has never called setPreferredPhy — grep finds no PHY handling at all — so
every historical offload has run on the 1M PHY. LE 2M doubles the symbol rate:
the same bytes spend HALF the air-time.

That makes this the better-behaved of the two sync-speed levers. ryanbr#536's
connection-interval lever buys speed with ~2.7x more radio wakeups, so its
battery effect is ambiguous and hinges on whether the offload is link- or
firmware-limited. Halving air-time for the same bytes should cost LESS radio
energy per byte, not more. The two are orthogonal and stack, and are deliberately
SEPARATE toggles: they have opposite battery profiles, so bundling them would
make a field report un-attributable.

Requested at offload START, not on connect, on purpose: the connect handshake is
fragile — an extra GATT op before requestMtu can make it return false, which
skips the MTU bump and caps the very offload this is meant to speed up (ryanbr#85/ryanbr#50).

The mask ALWAYS includes 1M, never 2M alone: it is a preference, and keeping 1M
lets the controller fall back rather than cling to a 2M link gone marginal (2M
trades range for speed). Turning the toggle off releases an already-2M link back
to 1M — a PHY persists once negotiated, and the toggle's own copy tells the user
to switch it off if syncing goes flaky at range, which is exactly when 2M is the
suspect. Reuses ryanbr#536's releasesConnectionPriority edge rule, so the default
launch path issues zero BLE ops.

onPhyUpdate logs the PHY actually negotiated — the only way to learn whether
WHOOP supports 2M at all, and it lands next to the offload's existing session
timestamps + 'persisted N rows' for a before/after records/sec.

Android-only by necessity: CoreBluetooth exposes no app-side PHY API, so there is
no Swift twin — documented in-code, mirroring ryanbr#477's precedent.

EXPERIMENTAL, default OFF. Off is byte-for-byte today. NOT validated on hardware.

i18n audit OK; compileFullDebugKotlin + the full com.noop.ble suite green
(PreferredPhyTest 6, ConnectionPriorityTest 11, GattCrashSafetyTest 7).
DX23876 pushed a commit to DX23876/noop that referenced this pull request Jul 22, 2026
…le (ryanbr#538)

Two follow-ups on ryanbr#536/ryanbr#537, found re-reviewing them after merge.

A. The PHY was never released at offload END, unlike its twin lever.
   refreshConnectionPriority() runs at BOTH enterBackfilling and exitBackfilling,
   so the priority escalation is bounded to the burst. applyPreferredPhy() ran only
   at enterBackfilling - and a PHY PERSISTS once negotiated, so a link that went 2M
   for a sync stayed 2M for the WHOLE connection, including the overnight window.
   That is not what the toggle's copy promises (it says "while your strap hands over
   its stored history"), and it left 2M's range trade-off in force long after the
   transfer it was for.

   Release to 1M at exitBackfilling so the preference is bounded to the burst,
   symmetric with the priority lever, and the shipped copy becomes accurate rather
   than aspirational.

   Battery was NOT the reason: 2M costs LESS energy per byte, so leaving it on was
   arguably fine. The reason is that RANGE is 2M's unvalidated trade-off, and an
   experimental lever should confine its unknown to the window the user was told
   about. If field data later shows 2M is good everywhere, widen it deliberately.

   The guard lives at the call site, not inside releasePreferredPhy: that method
   cannot check the flag, because setFastLinkPhy's on->off edge calls it AFTER the
   flag is already false. Guarding at the call site keeps the default path at ZERO
   BLE ops.

B. releasesConnectionPriority was a misnomer the moment ryanbr#537 reused it for the PHY:
   one predicate, two levers, a name that claimed one. Renamed to releasesOnDisable
   and its doc now says it serves both. My own mess from the reuse.

compileFullDebugKotlin + the full com.noop.ble suite green (PreferredPhyTest 6,
ConnectionPriorityTest 11, GattCrashSafetyTest 7); i18n audit OK. Still not
validated on hardware - both toggles remain default-off.
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