From eec90679fcd7163f564c001ba37511f7d8ebf63c Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Fri, 11 Sep 2026 19:41:56 +1200 Subject: [PATCH 1/5] Show Android the strap-clock readout it was already computing Reviewing #2085 (iOS, WHOOP 5.0, live HR fine and no sleep ever) turned up that the two readouts that diagnose it have no Android caller at all. ConnectionReadout.clockLatchedLabel and ConnectionReadout.rtcWarning are both implemented in the Kotlin twin AND covered by ConnectionReadoutTest, and nothing in production calls either. iOS has shown both on Test Centre and Devices since the #987 work. An Android user with the same strap could only find out by exporting a log and reading it themselves. The gap that kept it unshowable was a missing value, not a missing view. A WHOOP 5/MG does not serve GET_CLOCK, so the correlated device clock the WHOOP4 path derives is always null there, and the only evidence of a never-set RTC is how the strap DATED its own banked records. iOS reads that from LiveState.strapRange.newestUnix; Android had no equivalent field, so the readout could only ever have said "waiting for the strap clock" forever, which is the shape #827 describes and #1823 changed on iOS. LiveState now carries strapNewestUnix, published where the GET_DATA_RANGE reply is already parsed. It is set unconditionally rather than inside the Test Centre gate beside it: the readout that needs it is the one a reporter is asked to quote, and gating the value on a diagnostic mode would leave it null for exactly the user who has not turned one on. Rendered the way iOS renders it, in the connection panel rather than through the registry's liveReadout ids. A first attempt added an id there and broke TestModeRegistryParityTest, which is right to fail: that list is a spec both platforms carry byte-for-byte, and iOS's own panel already shows more rows than it names. The pure helper keeps the wiring testable without the Composable. Also carries the #1818 nuance Android was missing. The log-only clock verdict says "charge to 100%" unconditionally, while rtcWarning tells an already-charged strap that charging again will not help and to export a log instead. Android users at 100% were being sent round the loop that copy was written to end. --- .../main/java/com/noop/ble/WhoopBleClient.kt | 15 +++++++ .../com/noop/ui/TestCentreLiveReadouts.kt | 27 ++++++++++++ .../main/java/com/noop/ui/TestCentreScreen.kt | 23 +++++++++- .../com/noop/ui/TestCentreLiveReadoutsTest.kt | 42 +++++++++++++++++++ 4 files changed, 106 insertions(+), 1 deletion(-) 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 75f352d6a6..dbbd4702ed 100644 --- a/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt +++ b/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt @@ -148,6 +148,15 @@ data class LiveState( * short history to render a moving R-R strip / rolling RMSSD. Appended (never replaced) via * [withRRIntervals]; emptied by [clearedBiometrics]. Twin of macOS LiveState.rrRecent (PR#191). */ val rrRecent: List = emptyList(), + /** Newest banked-record timestamp the strap reported in its last GET_DATA_RANGE reply, unix seconds. + * null until a range lands. + * + * This is the ONLY clock evidence a WHOOP 5/MG offers: its GET_CLOCK reply is not served on that + * family (see the Interpreter's capture note), so the correlated device clock the WHOOP4 path + * derives is always null there. Without it the Clock-latched readout can only say "waiting for the strap + * clock" on a 5/MG, however broken the strap's RTC is, which is the #827 shape iOS fixed in #1823 by + * falling back to how the strap DATED its records. Twin of macOS LiveState.strapRange.newestUnix. */ + val strapNewestUnix: Long? = null, val batteryPct: Double? = null, /** Strap battery pack VOLTAGE (mV), decoded from the ~8-min BATTERY_LEVEL event (mv@21/@25) and the * GET_EXTENDED_BATTERY_INFO response (#592). Shown on the Devices card as a "x.xx V" readout beside @@ -7635,6 +7644,12 @@ class WhoopBleClient( // (gate = active(UNIVERSAL) == any mode on), tagged .universal, not just the // Connection mode. Matches the universal dayOwner line. Gated zero-cost; pure // formatter, no behaviour change. Twin of the macOS data-range emit. + // Publish the newest banked stamp for the Clock-latched readout. Set + // UNCONDITIONALLY, not inside the Test Centre gate below: the readout that + // needs it is what a reporter is asked to quote, and gating the value on a + // diagnostic mode would leave it null for exactly the user who has not + // enabled one. + _state.update { s -> s.copy(strapNewestUnix = it) } if (testCentre.active(com.noop.testcentre.TestDomain.UNIVERSAL)) { val line = com.noop.analytics.ConnectionTrace.clockDriftLine( oldestUnix = if (oldestUnix != null && oldestUnix < it) oldestUnix else null, diff --git a/android/app/src/main/java/com/noop/ui/TestCentreLiveReadouts.kt b/android/app/src/main/java/com/noop/ui/TestCentreLiveReadouts.kt index 9303d36c76..83083456e9 100644 --- a/android/app/src/main/java/com/noop/ui/TestCentreLiveReadouts.kt +++ b/android/app/src/main/java/com/noop/ui/TestCentreLiveReadouts.kt @@ -148,3 +148,30 @@ internal object TestCentreLiveReadouts { "\\[(${TestDomain.entries.joinToString("|") { Regex.escape(it.id) }})]\\s", ) } + +/** + * The clock half of the Connection readout: the "Clock latched" row and the plain-words RTC warning. + * + * iOS renders these in `ConnectionReadoutPanel`, hard-coded in the view rather than bound to the + * registry's `liveReadout` ids (that list is a spec both platforms must carry byte-for-byte, and iOS's + * panel already shows more rows than it names). Android mirrors that shape here rather than adding an + * id, so the registry parity test keeps its meaning. + * + * Both values were computable on this platform all along: `clockLatchedLabel` and `rtcWarning` are + * implemented in `ConnectionReadout` and covered by `ConnectionReadoutTest`, and NEITHER had a + * production caller, so an Android user could only learn their strap's clock was never set by exporting + * a log and reading it themselves. Pure and separate from the Composable so the wiring is testable. + */ +internal fun connectionClockReadout( + logLines: List, + strapNewestUnix: Long?, + batteryPct: Double?, +): Pair { + val deviceClock = ConnectionReadout.clockCorrelatedDevice(logLines) + val row = LiveReadoutRow( + "clockLatched", + "Clock latched", + ConnectionReadout.clockLatchedLabel(deviceClock, strapNewestUnix), + ) + return row to ConnectionReadout.rtcWarning(deviceClock, strapNewestUnix, batteryPct) +} diff --git a/android/app/src/main/java/com/noop/ui/TestCentreScreen.kt b/android/app/src/main/java/com/noop/ui/TestCentreScreen.kt index 4f3405b0bc..508e979d3f 100644 --- a/android/app/src/main/java/com/noop/ui/TestCentreScreen.kt +++ b/android/app/src/main/java/com/noop/ui/TestCentreScreen.kt @@ -72,6 +72,8 @@ import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.isActive import kotlinx.coroutines.launch import kotlin.math.roundToInt +import androidx.compose.ui.semantics.contentDescription +import androidx.compose.ui.semantics.semantics /** * Settings -> Test Centre (spec section 7), the Android twin of TestCentreView. Four sections: domain @@ -672,14 +674,33 @@ private fun TestCentreLiveReadoutPanel( gravitySamples = gravitySamples, ), ) + // #987/#1823: the clock half of the Connection readout, the row a reporter is asked to quote when + // nothing syncs plus the sentence that says what to do about it. Rendered here rather than through + // the registry's liveReadout ids because iOS renders it the same way, in ConnectionReadoutPanel: that + // id list is a spec both platforms carry byte-for-byte, and adding to one side alone breaks it. + val clockReadout = connectionClockReadout( + logLines = logLines, + strapNewestUnix = live.strapNewestUnix, + batteryPct = live.batteryPct, + ).takeIf { mode.domain == TestDomain.CONNECTION } Column(verticalArrangement = Arrangement.spacedBy(4.dp), modifier = Modifier.padding(top = 2.dp)) { - rows.forEach { row -> + (rows + listOfNotNull(clockReadout?.first)).forEach { row -> Row(Modifier.fillMaxWidth()) { Text(row.label, style = NoopType.footnote, color = Palette.textTertiary) Spacer(Modifier.weight(1f)) Text(row.value, style = NoopType.mono, color = Palette.textSecondary) } } + clockReadout?.second?.let { warning -> + // Amber and in words, not a bare token: a never-set clock is the most common "no history at + // all" root cause, and the fix is in the sentence. + Text( + warning, + style = NoopType.footnote, + color = Palette.statusWarning, + modifier = Modifier.semantics { contentDescription = warning }, + ) + } } } diff --git a/android/app/src/test/java/com/noop/ui/TestCentreLiveReadoutsTest.kt b/android/app/src/test/java/com/noop/ui/TestCentreLiveReadoutsTest.kt index da6b481f2a..d9e56a5107 100644 --- a/android/app/src/test/java/com/noop/ui/TestCentreLiveReadoutsTest.kt +++ b/android/app/src/test/java/com/noop/ui/TestCentreLiveReadoutsTest.kt @@ -107,4 +107,46 @@ class TestCentreLiveReadoutsTest { TestCentreLiveRefreshPolicy.sources(battery, active = true), ) } + + // -- #987/#1823: the clock half of the Connection readout, computed here and never shown --------- + // + // ConnectionReadout.clockLatchedLabel and rtcWarning were both implemented AND unit-tested on this + // platform with NO production caller, so an Android user could only learn their strap's clock was + // never set by exporting a log and reading it themselves. iOS has shown both on Test Centre and + // Devices throughout. These pin the wiring; the rules themselves are pinned in ConnectionReadoutTest. + + /** The reported shape: a WHOOP 5/MG whose banked records are epoch-era. Its GET_CLOCK reply is not + * served on that family, so how the strap DATED its records is the only evidence there is. */ + @Test fun anEpochDatedStrapReadsNotLatchedAndWarns() { + val (row, warning) = connectionClockReadout(emptyList(), strapNewestUnix = 40_000_000L, batteryPct = 50.0) + assertEquals(LiveReadoutRow("clockLatched", "Clock latched", "no (records dated 1970/71)"), row) + assertTrue(requireNotNull(warning).contains("not banking history")) + } + + @Test fun aSanelyDatedStrapReadsLatchedAndDoesNotWarn() { + val (row, warning) = connectionClockReadout(emptyList(), strapNewestUnix = 1_782_475_000L, batteryPct = 50.0) + assertEquals("yes", row.value) + assertEquals(null, warning) + } + + /** Before any range lands there is nothing to judge. The row says so rather than inventing a + * verdict, and no warning is raised on an absence. */ + @Test fun noEvidenceYetSaysWaitingAndRaisesNoWarning() { + val (row, warning) = connectionClockReadout(emptyList(), strapNewestUnix = null, batteryPct = null) + assertEquals("no (waiting for the strap clock)", row.value) + assertEquals(null, warning) + } + + /** #1818: the remedy is battery-dependent, and an already-charged strap must not be sent round the + * loop it has just run. Wiring the battery through is the whole point of passing it. */ + @Test fun anAlreadyChargedStrapIsNotToldToChargeAgain() { + val flat = requireNotNull( + connectionClockReadout(emptyList(), strapNewestUnix = 40_000_000L, batteryPct = 40.0).second + ) + val charged = requireNotNull( + connectionClockReadout(emptyList(), strapNewestUnix = 40_000_000L, batteryPct = 100.0).second + ) + assertTrue(flat.contains("Charge the strap")) + assertTrue(charged.contains("already charged")) + } } From e8000106300cc77685fa3a94c96de8ade5429db0 Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Fri, 11 Sep 2026 19:45:15 +1200 Subject: [PATCH 2/5] Clear the strap-clock stamp with the link, and unstack a comment Re-review of my own change, two findings. The verdict could outlive the link that produced it. strapNewestUnix was published and never cleared, so after a disconnect the Clock-latched row kept answering from a dead link, and after a device SWITCH it would have answered for the previous strap. macOS clears its strapRange on exactly this path, with the note that a stale clock-drift window must not outlive the link either; clearedBiometrics is that path here, and the field is cleared there now. Pinned by a test that reads the row either side of the clear. The publish line also landed between the CAPTURE-B comment and the block it explains, so that comment read as documentation for my line and the block it belonged to lost its own. Plain slash comments, which the doc lint does not inspect, so it had to be caught by reading. Moved above the block. --- .../main/java/com/noop/ble/WhoopBleClient.kt | 18 +++++++++++------- .../com/noop/ui/TestCentreLiveReadoutsTest.kt | 11 +++++++++++ 2 files changed, 22 insertions(+), 7 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 dbbd4702ed..56672a146f 100644 --- a/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt +++ b/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt @@ -317,7 +317,11 @@ data class LiveState( * strip can't outlive the link. Applied on disconnect alongside the charging/bond clears. Twin of * macOS LiveState.clearBiometrics (PR#191). */ fun clearedBiometrics(): LiveState = copy(heartRate = null, rr = emptyList(), rrRecent = emptyList(), - streamingLiveHR = false) // #56: a dropped link is no longer streaming + streamingLiveHR = false, // #56: a dropped link is no longer streaming + // A stale clock-drift window must not outlive the link either, and + // after a device SWITCH it would answer for the previous strap. + // macOS clears its strapRange on the same path. + strapNewestUnix = null) } /** @@ -7636,6 +7640,12 @@ class WhoopBleClient( log("Strap banked history span: ${fmt.format(java.util.Date(oldestUnix * 1000L))} → newest " + "(~$spanDays day${if (spanDays == 1L) "" else "s"} of backlog, drained oldest-first)") } + // Publish the newest banked stamp for the Clock-latched readout. Set + // UNCONDITIONALLY, not inside the Test Centre gate below: the readout that + // needs it is what a reporter is asked to quote, and gating the value on a + // diagnostic mode would leave it null for exactly the user who has not + // enabled one. Cleared with the rest of the link state on disconnect. + _state.update { s -> s.copy(strapNewestUnix = it) } // CAPTURE-B parity: promote the CLOCK-DRIFT picture from the buried raw frames to // one upfront line in the UNIVERSAL block - the strap-reported [oldest, newest] // window vs wall clock with a FUTURE-DATE flag (#767 / #754 / #72 cluster). A @@ -7644,12 +7654,6 @@ class WhoopBleClient( // (gate = active(UNIVERSAL) == any mode on), tagged .universal, not just the // Connection mode. Matches the universal dayOwner line. Gated zero-cost; pure // formatter, no behaviour change. Twin of the macOS data-range emit. - // Publish the newest banked stamp for the Clock-latched readout. Set - // UNCONDITIONALLY, not inside the Test Centre gate below: the readout that - // needs it is what a reporter is asked to quote, and gating the value on a - // diagnostic mode would leave it null for exactly the user who has not - // enabled one. - _state.update { s -> s.copy(strapNewestUnix = it) } if (testCentre.active(com.noop.testcentre.TestDomain.UNIVERSAL)) { val line = com.noop.analytics.ConnectionTrace.clockDriftLine( oldestUnix = if (oldestUnix != null && oldestUnix < it) oldestUnix else null, diff --git a/android/app/src/test/java/com/noop/ui/TestCentreLiveReadoutsTest.kt b/android/app/src/test/java/com/noop/ui/TestCentreLiveReadoutsTest.kt index d9e56a5107..0537a52f6c 100644 --- a/android/app/src/test/java/com/noop/ui/TestCentreLiveReadoutsTest.kt +++ b/android/app/src/test/java/com/noop/ui/TestCentreLiveReadoutsTest.kt @@ -137,6 +137,17 @@ class TestCentreLiveReadoutsTest { assertEquals(null, warning) } + /** A verdict must not outlive the link that produced it: after a disconnect, and especially after a + * strap SWITCH, a kept stamp would answer for the previous strap. macOS clears its strapRange on + * the same path, and clearedBiometrics is that path here. */ + @Test fun aDroppedLinkStopsReportingTheOldStrapsClock() { + val live = com.noop.ble.LiveState(strapNewestUnix = 40_000_000L) + assertEquals("no (records dated 1970/71)", + connectionClockReadout(emptyList(), live.strapNewestUnix, batteryPct = 50.0).first.value) + assertEquals("no (waiting for the strap clock)", + connectionClockReadout(emptyList(), live.clearedBiometrics().strapNewestUnix, batteryPct = 50.0).first.value) + } + /** #1818: the remedy is battery-dependent, and an already-charged strap must not be sent round the * loop it has just run. Wiring the battery through is the whole point of passing it. */ @Test fun anAlreadyChargedStrapIsNotToldToChargeAgain() { From 653715f9a66e32ba3ed3c3fef3d7b34d82a73cdb Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Fri, 11 Sep 2026 20:13:08 +1200 Subject: [PATCH 3/5] Stop scanning the log for every panel, and never judge charge from a dead link Re-review of my own change, two findings, both in the call I added. The scan ran for every mode, not just Connection. I wrote it as a call followed by takeIf, and takeIf runs on the RESULT, so the log walk happened for every Test Centre panel and was then thrown away. Worse than a wasted call: this panel recomposes once a second off the connection clock, and clockCorrelatedDevice walks the whole log looking for a line a WHOOP 5/MG never emits, so the miss is a full scan, once a second, on the family the readout exists for. Gated before the call now, and remembered on its inputs so a clock tick alone cannot re-run it. The charge could come from a link that had ended. rtcWarning tells its callers in as many words to pass a reading from the CURRENT link and never a last-known one, because a stale 100% withdraws the "charge it" remedy from the very strap that earned it by running flat and resetting its RTC. Android never clears batteryPct on disconnect, where macOS clears its battery samples, so the stale value is exactly what my call passed. It is gated on connected now; null keeps the charge advice, which is the safe direction to be wrong in. A residual stays, and is not this change's to take on: a reconnect that has not yet read battery on the new link still carries the previous one's value. Clearing batteryPct on disconnect is the real fix and belongs with the battery display it would also change. --- .../main/java/com/noop/ui/TestCentreScreen.kt | 29 +++++++++++++++---- .../com/noop/ui/TestCentreLiveReadoutsTest.kt | 11 +++++++ 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/android/app/src/main/java/com/noop/ui/TestCentreScreen.kt b/android/app/src/main/java/com/noop/ui/TestCentreScreen.kt index 508e979d3f..11197f7be4 100644 --- a/android/app/src/main/java/com/noop/ui/TestCentreScreen.kt +++ b/android/app/src/main/java/com/noop/ui/TestCentreScreen.kt @@ -678,11 +678,30 @@ private fun TestCentreLiveReadoutPanel( // nothing syncs plus the sentence that says what to do about it. Rendered here rather than through // the registry's liveReadout ids because iOS renders it the same way, in ConnectionReadoutPanel: that // id list is a spec both platforms carry byte-for-byte, and adding to one side alone breaks it. - val clockReadout = connectionClockReadout( - logLines = logLines, - strapNewestUnix = live.strapNewestUnix, - batteryPct = live.batteryPct, - ).takeIf { mode.domain == TestDomain.CONNECTION } + // + // Gated BEFORE the call and remembered on its inputs, both deliberately. `takeIf` would have run the + // scan for every mode's panel and then discarded it, and this panel recomposes once a SECOND from the + // connection clock, while clockCorrelatedDevice walks the whole log looking for a line a 5/MG never + // emits - so the miss is the full scan, on the family this readout exists for. + // + // The charge is passed ONLY while connected, which rtcWarning requires of its callers in as many + // words: it must be a reading from the CURRENT link, never a last-known one that outlives it, + // because a stale 100% withdraws the "charge it" remedy from the very strap that earned it by + // running flat and resetting its RTC. Android never clears batteryPct on disconnect (macOS clears + // its battery samples), so without this gate the stale value is exactly what would arrive. Null + // keeps the charge advice, which is the safe direction. + val linkBatteryPct = live.batteryPct.takeIf { live.connected } + val clockReadout = if (mode.domain == TestDomain.CONNECTION) { + remember(logLines, live.strapNewestUnix, linkBatteryPct) { + connectionClockReadout( + logLines = logLines, + strapNewestUnix = live.strapNewestUnix, + batteryPct = linkBatteryPct, + ) + } + } else { + null + } Column(verticalArrangement = Arrangement.spacedBy(4.dp), modifier = Modifier.padding(top = 2.dp)) { (rows + listOfNotNull(clockReadout?.first)).forEach { row -> Row(Modifier.fillMaxWidth()) { diff --git a/android/app/src/test/java/com/noop/ui/TestCentreLiveReadoutsTest.kt b/android/app/src/test/java/com/noop/ui/TestCentreLiveReadoutsTest.kt index 0537a52f6c..67a75fdf69 100644 --- a/android/app/src/test/java/com/noop/ui/TestCentreLiveReadoutsTest.kt +++ b/android/app/src/test/java/com/noop/ui/TestCentreLiveReadoutsTest.kt @@ -148,6 +148,17 @@ class TestCentreLiveReadoutsTest { connectionClockReadout(emptyList(), live.clearedBiometrics().strapNewestUnix, batteryPct = 50.0).first.value) } + /** rtcWarning requires a charge from the CURRENT link: a stale 100% would withdraw the "charge it" + * remedy from the very strap that earned it by running flat and resetting its RTC. Android never + * clears batteryPct on disconnect, so a disconnected link must pass null, and null KEEPS the + * charge advice rather than withdrawing it on no evidence. */ + @Test fun aChargeFromADeadLinkDoesNotSuppressTheChargeAdvice() { + val stale = requireNotNull( + connectionClockReadout(emptyList(), strapNewestUnix = 40_000_000L, batteryPct = null).second + ) + assertTrue(stale.contains("Charge the strap")) + } + /** #1818: the remedy is battery-dependent, and an already-charged strap must not be sent round the * loop it has just run. Wiring the battery through is the whole point of passing it. */ @Test fun anAlreadyChargedStrapIsNotToldToChargeAgain() { From 3224d2889172da9a23a09db875140141d7c4025f Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Fri, 11 Sep 2026 20:20:57 +1200 Subject: [PATCH 4/5] Put the strap-clock state on the Devices card, where people actually look Test Centre is a diagnostic mode a user has to switch on. Devices is the screen they already have open, and macOS has shown the clock state on the active card since the #987 work. Surfacing it only in Test Centre left the person most likely to need it still unable to see it. The device card takes the same two values macOS passes it, liveClockLine and liveClockWarning, active card only. The line carries the tokens; the warning carries the sentence, which is the half that says what to do. Two deliberate differences from macOS, both because of where Android keeps things. The line is the clock half only. macOS pairs it with a last-frame freshness that this platform cannot show: it has no lastFrameAtUnix to read. That belongs with the other ConnectionReadout functions Android computes and never surfaces, not smuggled in here. The device clock is not resolved on this screen. It comes from the strap log, which lives in the BLE client here rather than in LiveState as it does on macOS, so reading it would subscribe the Devices screen to the log revision and rebuild the line list several times a second through an offload burst. It is a WHOOP4-only signal anyway, and the strap's own dating of its records is what matters and is the only evidence a 5/MG gives. The cost is wording on a 4.0 with a dead RTC, not the verdict. So the helper now takes the device clock RESOLVED rather than the log to resolve it from, because the two callers legitimately differ, and a test pins that it honours whichever it is handed. --- .../main/java/com/noop/ui/DevicesScreen.kt | 54 +++++++++++++++++++ .../com/noop/ui/TestCentreLiveReadouts.kt | 11 ++-- .../main/java/com/noop/ui/TestCentreScreen.kt | 3 +- .../com/noop/ui/TestCentreLiveReadoutsTest.kt | 26 ++++++--- 4 files changed, 81 insertions(+), 13 deletions(-) diff --git a/android/app/src/main/java/com/noop/ui/DevicesScreen.kt b/android/app/src/main/java/com/noop/ui/DevicesScreen.kt index f419f6de74..bb93915d88 100644 --- a/android/app/src/main/java/com/noop/ui/DevicesScreen.kt +++ b/android/app/src/main/java/com/noop/ui/DevicesScreen.kt @@ -170,6 +170,26 @@ fun DevicesScreen( // #1300 tier 2: recompute the two-strap comparison when the strap set changes. LaunchedEffect(activeDevices) { strapCompare = loadStrapCompare(viewModel.repo, activeDevices) } val removedDevices = all.filter { it.status == DeviceStatus.archived.name } + // #987: the strap-clock readout, computed ONCE for the whole list rather than per card, and only + // while connected. Remembered on its inputs so the log scan does not re-run on every recomposition + // of a screen that redraws for battery, sync and rename state. + // The device clock is deliberately NOT resolved here. It comes from the strap log, which on this + // platform lives in the BLE client rather than in LiveState as it does on macOS, so reading it would + // subscribe this screen to the log revision and rebuild the line list several times a second through + // an offload burst. It is a WHOOP4-only signal in any case, and the strap's own dating of its records + // is the evidence that matters, and the ONLY evidence a 5/MG gives. The cost is wording on a 4.0 with + // a dead RTC ("records dated" rather than "RTC reads"), not the verdict. + val clockState = if (live.connected) { + remember(live.strapNewestUnix, live.batteryPct) { + connectionClockReadout( + deviceClockUnix = null, + strapNewestUnix = live.strapNewestUnix, + batteryPct = live.batteryPct, + ) + } + } else { + null + } val currentActiveName = all.firstOrNull { it.status == DeviceStatus.active.name }?.let { displayName(it) } ?: "Your current strap" @@ -289,6 +309,18 @@ fun DevicesScreen( // Historical record layout from the current backfill, distinct from strap firmware. liveHistoryLayout = if (device.status == DeviceStatus.active.name && live.connected) live.historyLayoutVersion else null, + // #987: the strap-clock state, ACTIVE card only, computed once for both halves (the log + // scan is the cost worth paying once, not twice - the same reason macOS pairs them). + // + // The charge is passed only while connected: rtcWarning requires a reading from the + // CURRENT link, since a stale 100% withdraws the "charge it" remedy from the very strap + // that earned it by running flat and resetting its RTC. + // + // macOS pairs this with a "last frame" half that Android cannot show yet: it has no + // lastFrameAtUnix to read. That belongs with the other ConnectionReadout functions this + // platform computes and does not surface, not smuggled in here. + liveClockLine = clockState?.first?.let { "Clock latched: ${it.value}" }, + liveClockWarning = clockState?.second, onMakeActive = { switchTarget = device }, onRename = { renameTarget = device }, onRemove = { removeTarget = device }, @@ -748,6 +780,14 @@ private fun DeviceCard( liveFirmware: String? = null, /** The active+connected strap's observed banked-history record layout (`hist_version`). */ liveHistoryLayout: Int? = null, + /** #987: "Clock latched: … · last frame …" for the ACTIVE card. A strap whose clock was never set + * banks nothing, which reads to the user as "live HR works but no history ever arrives", and this + * is the line a reporter is asked to quote. null on every other card. Twin of the macOS + * DeviceCard liveClockLine. */ + liveClockLine: String? = null, + /** #987: the plain-words 1970/71 warning that goes with [liveClockLine], when there is one to give. + * Twin of the macOS DeviceCard liveClockWarning. */ + liveClockWarning: String? = null, onMakeActive: () -> Unit, onRename: () -> Unit, onRemove: (() -> Unit)?, @@ -905,6 +945,20 @@ private fun DeviceCard( onDeviceConfigProbe = onDeviceConfigProbe, ) } + // #987: the strap-clock state, on the card the user already looks at rather than only in a + // diagnostic mode they have to switch on. The line carries the tokens; the warning carries + // the sentence, which is the half that says what to do about it. + if (liveClockLine != null) { + Text(liveClockLine, style = NoopType.footnote, color = Palette.textTertiary) + } + if (liveClockWarning != null) { + Text( + liveClockWarning, + style = NoopType.footnote, + color = Palette.statusWarning, + modifier = Modifier.semantics { contentDescription = liveClockWarning }, + ) + } } } diff --git a/android/app/src/main/java/com/noop/ui/TestCentreLiveReadouts.kt b/android/app/src/main/java/com/noop/ui/TestCentreLiveReadouts.kt index 83083456e9..c62d049ade 100644 --- a/android/app/src/main/java/com/noop/ui/TestCentreLiveReadouts.kt +++ b/android/app/src/main/java/com/noop/ui/TestCentreLiveReadouts.kt @@ -157,21 +157,24 @@ internal object TestCentreLiveReadouts { * panel already shows more rows than it names). Android mirrors that shape here rather than adding an * id, so the registry parity test keeps its meaning. * + * Takes the device clock RESOLVED rather than the log to resolve it from, because the two callers get + * it differently: Test Centre already holds the log lines, while the Devices screen does not and must + * not start subscribing to the log revision just for this. See each call site. + * * Both values were computable on this platform all along: `clockLatchedLabel` and `rtcWarning` are * implemented in `ConnectionReadout` and covered by `ConnectionReadoutTest`, and NEITHER had a * production caller, so an Android user could only learn their strap's clock was never set by exporting * a log and reading it themselves. Pure and separate from the Composable so the wiring is testable. */ internal fun connectionClockReadout( - logLines: List, + deviceClockUnix: Long?, strapNewestUnix: Long?, batteryPct: Double?, ): Pair { - val deviceClock = ConnectionReadout.clockCorrelatedDevice(logLines) val row = LiveReadoutRow( "clockLatched", "Clock latched", - ConnectionReadout.clockLatchedLabel(deviceClock, strapNewestUnix), + ConnectionReadout.clockLatchedLabel(deviceClockUnix, strapNewestUnix), ) - return row to ConnectionReadout.rtcWarning(deviceClock, strapNewestUnix, batteryPct) + return row to ConnectionReadout.rtcWarning(deviceClockUnix, strapNewestUnix, batteryPct) } diff --git a/android/app/src/main/java/com/noop/ui/TestCentreScreen.kt b/android/app/src/main/java/com/noop/ui/TestCentreScreen.kt index 11197f7be4..da624c9027 100644 --- a/android/app/src/main/java/com/noop/ui/TestCentreScreen.kt +++ b/android/app/src/main/java/com/noop/ui/TestCentreScreen.kt @@ -74,6 +74,7 @@ import kotlinx.coroutines.launch import kotlin.math.roundToInt import androidx.compose.ui.semantics.contentDescription import androidx.compose.ui.semantics.semantics +import com.noop.analytics.ConnectionReadout /** * Settings -> Test Centre (spec section 7), the Android twin of TestCentreView. Four sections: domain @@ -694,7 +695,7 @@ private fun TestCentreLiveReadoutPanel( val clockReadout = if (mode.domain == TestDomain.CONNECTION) { remember(logLines, live.strapNewestUnix, linkBatteryPct) { connectionClockReadout( - logLines = logLines, + deviceClockUnix = ConnectionReadout.clockCorrelatedDevice(logLines), strapNewestUnix = live.strapNewestUnix, batteryPct = linkBatteryPct, ) diff --git a/android/app/src/test/java/com/noop/ui/TestCentreLiveReadoutsTest.kt b/android/app/src/test/java/com/noop/ui/TestCentreLiveReadoutsTest.kt index 67a75fdf69..6b8e91c718 100644 --- a/android/app/src/test/java/com/noop/ui/TestCentreLiveReadoutsTest.kt +++ b/android/app/src/test/java/com/noop/ui/TestCentreLiveReadoutsTest.kt @@ -118,13 +118,13 @@ class TestCentreLiveReadoutsTest { /** The reported shape: a WHOOP 5/MG whose banked records are epoch-era. Its GET_CLOCK reply is not * served on that family, so how the strap DATED its records is the only evidence there is. */ @Test fun anEpochDatedStrapReadsNotLatchedAndWarns() { - val (row, warning) = connectionClockReadout(emptyList(), strapNewestUnix = 40_000_000L, batteryPct = 50.0) + val (row, warning) = connectionClockReadout(deviceClockUnix = null, strapNewestUnix = 40_000_000L, batteryPct = 50.0) assertEquals(LiveReadoutRow("clockLatched", "Clock latched", "no (records dated 1970/71)"), row) assertTrue(requireNotNull(warning).contains("not banking history")) } @Test fun aSanelyDatedStrapReadsLatchedAndDoesNotWarn() { - val (row, warning) = connectionClockReadout(emptyList(), strapNewestUnix = 1_782_475_000L, batteryPct = 50.0) + val (row, warning) = connectionClockReadout(deviceClockUnix = null, strapNewestUnix = 1_782_475_000L, batteryPct = 50.0) assertEquals("yes", row.value) assertEquals(null, warning) } @@ -132,7 +132,7 @@ class TestCentreLiveReadoutsTest { /** Before any range lands there is nothing to judge. The row says so rather than inventing a * verdict, and no warning is raised on an absence. */ @Test fun noEvidenceYetSaysWaitingAndRaisesNoWarning() { - val (row, warning) = connectionClockReadout(emptyList(), strapNewestUnix = null, batteryPct = null) + val (row, warning) = connectionClockReadout(deviceClockUnix = null, strapNewestUnix = null, batteryPct = null) assertEquals("no (waiting for the strap clock)", row.value) assertEquals(null, warning) } @@ -143,9 +143,9 @@ class TestCentreLiveReadoutsTest { @Test fun aDroppedLinkStopsReportingTheOldStrapsClock() { val live = com.noop.ble.LiveState(strapNewestUnix = 40_000_000L) assertEquals("no (records dated 1970/71)", - connectionClockReadout(emptyList(), live.strapNewestUnix, batteryPct = 50.0).first.value) + connectionClockReadout(deviceClockUnix = null, strapNewestUnix = live.strapNewestUnix, batteryPct = 50.0).first.value) assertEquals("no (waiting for the strap clock)", - connectionClockReadout(emptyList(), live.clearedBiometrics().strapNewestUnix, batteryPct = 50.0).first.value) + connectionClockReadout(deviceClockUnix = null, strapNewestUnix = live.clearedBiometrics().strapNewestUnix, batteryPct = 50.0).first.value) } /** rtcWarning requires a charge from the CURRENT link: a stale 100% would withdraw the "charge it" @@ -154,19 +154,29 @@ class TestCentreLiveReadoutsTest { * charge advice rather than withdrawing it on no evidence. */ @Test fun aChargeFromADeadLinkDoesNotSuppressTheChargeAdvice() { val stale = requireNotNull( - connectionClockReadout(emptyList(), strapNewestUnix = 40_000_000L, batteryPct = null).second + connectionClockReadout(deviceClockUnix = null, strapNewestUnix = 40_000_000L, batteryPct = null).second ) assertTrue(stale.contains("Charge the strap")) } + /** A WHOOP4 correlated clock is the stronger signal and wins over the record dating. The Test Centre + * caller resolves it from the log; the Devices caller deliberately passes null, so this pins that the + * helper honours whichever it is handed. */ + @Test fun aCorrelatedDeviceClockTakesPrecedenceOverTheRecordDating() { + val (row, _) = connectionClockReadout( + deviceClockUnix = 40_000_000L, strapNewestUnix = 1_782_475_000L, batteryPct = 50.0, + ) + assertEquals("no (RTC reads 1970/71)", row.value) + } + /** #1818: the remedy is battery-dependent, and an already-charged strap must not be sent round the * loop it has just run. Wiring the battery through is the whole point of passing it. */ @Test fun anAlreadyChargedStrapIsNotToldToChargeAgain() { val flat = requireNotNull( - connectionClockReadout(emptyList(), strapNewestUnix = 40_000_000L, batteryPct = 40.0).second + connectionClockReadout(deviceClockUnix = null, strapNewestUnix = 40_000_000L, batteryPct = 40.0).second ) val charged = requireNotNull( - connectionClockReadout(emptyList(), strapNewestUnix = 40_000_000L, batteryPct = 100.0).second + connectionClockReadout(deviceClockUnix = null, strapNewestUnix = 40_000_000L, batteryPct = 100.0).second ) assertTrue(flat.contains("Charge the strap")) assertTrue(charged.contains("already charged")) From 0851d42e1910603deb8c296c444cdc4d4f770417 Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Fri, 11 Sep 2026 20:23:33 +1200 Subject: [PATCH 5/5] Keep the clock line on the strap it describes The Devices call site runs per device, and I passed the clock line and warning to every card without gating on which one is active. liveHistoryLayout sits two lines above doing exactly that gate, so on a two-strap install the inactive strap's card would have printed the ACTIVE strap's verdict: a diagnostic answering for a device it never observed, which is the failure this codebase already has a rule against. macOS gates both on device.status == .active; so does this now. --- android/app/src/main/java/com/noop/ui/DevicesScreen.kt | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/android/app/src/main/java/com/noop/ui/DevicesScreen.kt b/android/app/src/main/java/com/noop/ui/DevicesScreen.kt index bb93915d88..49811aa3a9 100644 --- a/android/app/src/main/java/com/noop/ui/DevicesScreen.kt +++ b/android/app/src/main/java/com/noop/ui/DevicesScreen.kt @@ -319,8 +319,14 @@ fun DevicesScreen( // macOS pairs this with a "last frame" half that Android cannot show yet: it has no // lastFrameAtUnix to read. That belongs with the other ConnectionReadout functions this // platform computes and does not surface, not smuggled in here. - liveClockLine = clockState?.first?.let { "Clock latched: ${it.value}" }, - liveClockWarning = clockState?.second, + // ACTIVE card only, like liveHistoryLayout above. clockState describes the ONE strap the + // link belongs to, so on a two-strap install an ungated line would have printed the + // active strap's verdict on the inactive strap's card, which is a diagnostic answering + // for a device it never observed. + liveClockLine = if (device.status == DeviceStatus.active.name) + clockState?.first?.let { "Clock latched: ${it.value}" } else null, + liveClockWarning = if (device.status == DeviceStatus.active.name) + clockState?.second else null, onMakeActive = { switchTarget = device }, onRename = { renameTarget = device }, onRemove = { removeTarget = device },