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..56672a146f 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 @@ -308,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) } /** @@ -7627,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 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..49811aa3a9 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,24 @@ 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. + // 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 }, @@ -748,6 +786,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 +951,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 9303d36c76..c62d049ade 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,33 @@ 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. + * + * 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( + deviceClockUnix: Long?, + strapNewestUnix: Long?, + batteryPct: Double?, +): Pair { + val row = LiveReadoutRow( + "clockLatched", + "Clock latched", + ConnectionReadout.clockLatchedLabel(deviceClockUnix, strapNewestUnix), + ) + 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 4f3405b0bc..da624c9027 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,9 @@ 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 +import com.noop.analytics.ConnectionReadout /** * Settings -> Test Centre (spec section 7), the Android twin of TestCentreView. Four sections: domain @@ -672,14 +675,52 @@ 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. + // + // 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( + deviceClockUnix = ConnectionReadout.clockCorrelatedDevice(logLines), + strapNewestUnix = live.strapNewestUnix, + batteryPct = linkBatteryPct, + ) + } + } else { + null + } 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..6b8e91c718 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,78 @@ 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(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(deviceClockUnix = null, 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(deviceClockUnix = null, strapNewestUnix = null, batteryPct = null) + assertEquals("no (waiting for the strap clock)", row.value) + 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(deviceClockUnix = null, strapNewestUnix = live.strapNewestUnix, batteryPct = 50.0).first.value) + assertEquals("no (waiting for the strap clock)", + 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" + * 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(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(deviceClockUnix = null, strapNewestUnix = 40_000_000L, batteryPct = 40.0).second + ) + val charged = requireNotNull( + connectionClockReadout(deviceClockUnix = null, strapNewestUnix = 40_000_000L, batteryPct = 100.0).second + ) + assertTrue(flat.contains("Charge the strap")) + assertTrue(charged.contains("already charged")) + } }