From 93a360784828c1e6a3a1ccc1900ff6512371a4fc Mon Sep 17 00:00:00 2001 From: ryanbr Date: Sat, 1 Aug 2026 16:58:44 -0700 Subject: [PATCH 1/3] Report what each offload burst actually achieved (#1007) #477 (CONNECTION_PRIORITY_HIGH for the burst) and #533 (LE 2M around it) both ship OFF, gated on a real-strap before/after that has never been done. The reason is visible in the code: nothing measures the thing they change. The onPhyUpdate comment already talks about reading "the negotiated PHY next to the offload's records/sec" - but no records/sec is computed anywhere, which is why #1007's figures had to be counted by hand out of a raw capture. exitBackfilling now logs the burst's frame count, elapsed and achieved rate beside its exit reason, so validating those levers is toggle-sync-compare rather than frame-counting in a capture. Unconditional, one line per offload rather than per frame: behind the test-mode gate a reporter would have to find Test Centre first, and that friction is part of why the levers are still unvalidated. Frames, not records: offloadFramesThisSession counts genuine offload frames (47/48/49/50), which is what the levers move and the same numerator #1007 used. A non-positive elapsed reports the count with NO rate rather than a fabricated one. Locale-fixed so a decimal comma can't follow the phone language into a pasted log. Diagnostic only - no BLE op, no timing, no default changed. Deliberately not flipping either lever's default: "a shorter interval is strictly safer" is a plausible argument, not a measurement, and this is the change that lets it become one. --- .../main/java/com/noop/ble/WhoopBleClient.kt | 31 ++++++++++++ .../com/noop/ble/OffloadThroughputTest.kt | 47 +++++++++++++++++++ 2 files changed, 78 insertions(+) create mode 100644 android/app/src/test/java/com/noop/ble/OffloadThroughputTest.kt diff --git a/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt b/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt index c980ed8c6d..9586bafbbd 100644 --- a/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt +++ b/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt @@ -538,6 +538,25 @@ class WhoopBleClient( else -> BluetoothGatt.CONNECTION_PRIORITY_BALANCED } + /** + * The throughput summary for one offload burst (#1007), pure so it is testable without a BLE stack. + * + * #477 and #533 ship two levers that should shorten the offload — CONNECTION_PRIORITY_HIGH for the + * burst, and LE 2M around it — and both default OFF, gated on validation against a real strap. That + * validation never happened, and this is why: nothing measured the thing they change. The figures in + * #1007 had to be counted by hand out of a raw capture ("3193 frames / 90 s"), which is not something + * a reporter can be asked to do twice. + * + * Frames rather than records: [offloadFramesThisSession] counts genuine offload frames (47/48/49/50), + * which is what the levers actually move, and is the same numerator #1007 measured. A non-positive + * elapsed yields the count WITHOUT a rate rather than a fabricated one. Locale-fixed so a decimal + * comma cannot follow the phone language into a log someone pastes into an issue. + */ + fun offloadThroughputLine(frames: Int, elapsedMs: Long): String = + if (elapsedMs <= 0L) "$frames frame(s)" + else "%d frame(s) in %.1fs (%.1f frame/s)".format( + java.util.Locale.US, frames, elapsedMs / 1000.0, frames * 1000.0 / elapsedMs) + /** * The `reason=` token for a connection-down trace line (#1020), pure so the composition is * unit-testable without a BLE stack. @@ -2212,6 +2231,9 @@ class WhoopBleClient( /** Genuine offload frames seen this session — zero at timeout means the strap never answered * the history request at all (5/MG retry trigger, #78 fork). Main-looper only. */ private var offloadFramesThisSession = 0 + /** Wall time (ms) this offload burst began, for the #1007 throughput line. 0 before the first burst; + * only read inside [exitBackfilling], which cannot run unless [enterBackfilling] stamped it. */ + private var backfillStartedAtMs = 0L /** #174 deep-packet cooldown: wall time (ms) of the most recent offload frame OR HISTORY_COMPLETE. * A type-0x2F arriving just after a backfill ends (backfilling already flipped false) is a TRAILING * historical frame, not the live R22 stream, so it must not be counted as a "live deep packet". @@ -6335,6 +6357,9 @@ class WhoopBleClient( decodedChunksThisSession = 0 consoleChunksThisSession = 0 offloadFramesThisSession = 0 + // #1007: wall time the burst began, for the throughput line at exit. Its own field rather than + // reusing lastBackfillAtMs, which is the BackfillPolicy floor and measures from the last KICK. + backfillStartedAtMs = System.currentTimeMillis() historicalKickSent = false _state.update { it.copy(backfilling = true, syncChunksThisSession = 0) } refreshConnectionPriority() // #477: escalate to HIGH for the offload burst (faster sync). No-op unless enabled. @@ -6562,6 +6587,12 @@ class WhoopBleClient( // setFastLinkPhy's on→off edge calls it AFTER the flag is already false. So the default path still // issues ZERO BLE ops. if (fastLinkPhyEnabled) releasePreferredPhy() + // #1007: what the burst actually achieved. Emitted unconditionally (one line per offload, not per + // frame) so an ordinary exported strap log carries it — behind the test-mode gate it would need a + // reporter to find Test Centre first, which is the friction that left #477/#533 unvalidated. + val offloadElapsedMs = + if (backfillStartedAtMs > 0L) System.currentTimeMillis() - backfillStartedAtMs else -1L + log("Backfill: ${offloadThroughputLine(offloadFramesThisSession, offloadElapsedMs)} ($reason)") // #174: a backfill just ended. Start (or extend) the deep-packet cooldown from this instant so // any type-0x2F records the strap flushes in the seconds after the session aren't miscounted as // the live R22 stream — they're the offload's tail. diff --git a/android/app/src/test/java/com/noop/ble/OffloadThroughputTest.kt b/android/app/src/test/java/com/noop/ble/OffloadThroughputTest.kt new file mode 100644 index 0000000000..c7c8312067 --- /dev/null +++ b/android/app/src/test/java/com/noop/ble/OffloadThroughputTest.kt @@ -0,0 +1,47 @@ +package com.noop.ble + +import org.junit.Assert.assertEquals +import org.junit.Test + +/** + * #1007 — the throughput summary emitted at the end of an offload burst. + * + * This exists so #477's connection-priority escalation and #533's LE 2M preference can be validated + * instead of argued about. Both ship OFF, gated on a real-strap comparison that never happened because + * nothing measured the thing they change: #1007's own figures had to be counted by hand out of a raw + * capture. These pin the format a before/after comparison is read from. + */ +class OffloadThroughputTest { + + /** The reported case, at #1007's own measurement: 3193 frames in 90 s is ~35.5 frame/s on a 5/MG. */ + @Test fun reportsCountElapsedAndRate() { + assertEquals( + "3193 frame(s) in 90.0s (35.5 frame/s)", + WhoopBleClient.offloadThroughputLine(3193, 90_000L), + ) + } + + /** A WHOOP 4.0's ~10 frame/s, the slower half of the same issue. */ + @Test fun reportsTheSlowerFourPointZeroRate() { + assertEquals( + "600 frame(s) in 60.0s (10.0 frame/s)", + WhoopBleClient.offloadThroughputLine(600, 60_000L), + ) + } + + /** A burst the strap never answered still reports honestly — the count is the #78 retry signal. */ + @Test fun aStrapThatSentNothingStillReportsTheElapsed() { + assertEquals("0 frame(s) in 12.0s (0.0 frame/s)", WhoopBleClient.offloadThroughputLine(0, 12_000L)) + } + + /** An unknown start yields the count WITHOUT a rate, never a fabricated one. */ + @Test fun anUnknownElapsedOmitsTheRate() { + assertEquals("42 frame(s)", WhoopBleClient.offloadThroughputLine(42, -1L)) + assertEquals("42 frame(s)", WhoopBleClient.offloadThroughputLine(42, 0L)) + } + + /** Sub-second bursts round to a tenth rather than reading as zero elapsed. */ + @Test fun subSecondBurstsRoundToATenth() { + assertEquals("5 frame(s) in 0.4s (12.5 frame/s)", WhoopBleClient.offloadThroughputLine(5, 400L)) + } +} From 6e73a0dd44c919165ec0876822d4462eaf1a55e2 Mon Sep 17 00:00:00 2001 From: ryanbr Date: Sat, 1 Aug 2026 17:10:01 -0700 Subject: [PATCH 2/3] Also report a burst the disconnect cut short (#1007) exitBackfilling has exactly three call sites - HISTORY_COMPLETE, timeout and user-abort. A burst interrupted by a disconnect goes through reset() instead, which clears backfilling and zeroes the frame counter without passing through any of them, so it emitted no throughput line at all. That absence is ambiguous in the worst way for the question this measurement exists to answer: no offload at all, and an offload that was cut off, look identical in the log - and for battery they are opposite findings, since the interrupted one spent radio and banked nothing. Logged in reset() rather than by calling exitBackfilling there: that method also releases the connection priority and the PHY and records a sync outcome, none of which the teardown path does today. This stays diagnostic. Guarded on backfilling, so a completed burst that already logged on its way through exitBackfilling cannot log twice. --- .../app/src/main/java/com/noop/ble/WhoopBleClient.kt | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt b/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt index 9586bafbbd..a9bde5e5af 100644 --- a/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt +++ b/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt @@ -7256,6 +7256,16 @@ class WhoopBleClient( disRead = false disSerial = null disHwRev = null + // #1007: a burst cut short by a disconnect never reaches exitBackfilling — that path is only + // HISTORY_COMPLETE / timeout / user-abort — so without this the throughput line simply would not + // appear, and its ABSENCE is ambiguous: no offload at all, or one that was interrupted? For a + // battery question those are opposite answers (the interrupted one spent radio for nothing). + // Logged here rather than by calling exitBackfilling, which would also release the connection + // priority and PHY and record a sync outcome — behaviour, on a path that does none of it today. + if (backfilling && backfillStartedAtMs > 0L) { + log("Backfill: ${offloadThroughputLine(offloadFramesThisSession, + System.currentTimeMillis() - backfillStartedAtMs)} (interrupted)") + } backfilling = false backfillDraining = false backfillFrameQueue.clear() From c668c7bdfe545ffc7ef2e6e67c66c9a44785a6b6 Mon Sep 17 00:00:00 2001 From: ryanbr Date: Sat, 1 Aug 2026 17:18:07 -0700 Subject: [PATCH 3/3] Correct the backfillStartedAtMs doc (#1007) The field's doc said it was "only read inside exitBackfilling" - true when it was written, false one commit later when the interrupted-burst case added a read in reset(). Same class of error as #1023's field docs: an invariant stated at the point it happened to hold rather than one anything maintains. Now says what is actually true: stamped by enterBackfilling, never cleared, so it holds the previous burst's start between bursts - which is safe only because both readers pair it with `backfilling`, and that is false until enterBackfilling re-stamps. Deliberately makes NO threading claim. reset() is reached from handleDisconnect, which runs from the GATT callback rather than a handler.post, so "main-looper only" is not something this change verified - and asserting it while fixing an unverified assertion would repeat the mistake. It states only that the two readers are the paths that already own offloadFramesThisSession. --- android/app/src/main/java/com/noop/ble/WhoopBleClient.kt | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt b/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt index a9bde5e5af..ccb2bd5b3f 100644 --- a/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt +++ b/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt @@ -2231,8 +2231,11 @@ class WhoopBleClient( /** Genuine offload frames seen this session — zero at timeout means the strap never answered * the history request at all (5/MG retry trigger, #78 fork). Main-looper only. */ private var offloadFramesThisSession = 0 - /** Wall time (ms) this offload burst began, for the #1007 throughput line. 0 before the first burst; - * only read inside [exitBackfilling], which cannot run unless [enterBackfilling] stamped it. */ + /** Wall time (ms) this offload burst began, for the #1007 throughput line. Stamped by + * [enterBackfilling] and NEVER cleared, so it holds the PREVIOUS burst's start between bursts - both + * readers pair it with `backfilling`, which is only true when [enterBackfilling] has re-stamped it. + * Read in [exitBackfilling] for a classified exit, and in [reset] for a burst a disconnect cut short - + * the same two paths that already own [offloadFramesThisSession], so this adds no new thread. */ private var backfillStartedAtMs = 0L /** #174 deep-packet cooldown: wall time (ms) of the most recent offload frame OR HISTORY_COMPLETE. * A type-0x2F arriving just after a backfill ends (backfilling already flipped false) is a TRAILING