From d9d193c75766308f023625e13d37cb49b7d16cc5 Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Tue, 15 Sep 2026 00:00:10 +1200 Subject: [PATCH 1/5] battery: say whose charge it is on all seven surfaces, not three Refs #2208, reported by @pipiche38 from a code read. Every claim in that report checks out at the lines given. Three surfaces were routed through LiveConsoleReadout in #2076. Five more kept reading the raw field, and two of those had no gate at all. The mechanism is one shared mutable field. LiveState.batteryPct is the WHOOP's and is never cleared, deliberately; clearBiometrics nils ouraBatteryPct and leaves it. `connected` goes true the moment ANY source streams, including the first live HR from a ring. So both halves of the old gate pass and the screen draws the strap's charge under an active ring. Seven surfaces now, in one pass rather than a third: Liquid Today header ring Liquid Today sources row Classic Today battery row Android Today header ring Android Today sources pill macOS sidebar macOS menu-bar stat The last two are the ones the report flagged and left out, and they are worse than the five it filed rather than equivalent. Neither gated on `connected` at all, so they showed the strap's last percentage with nothing connected to anything, needing no ring to reproduce. The sidebar is the sharper case: its only honest branch is the nil one, and since batteryPct is never cleared that branch stopped being reachable the first time a WHOOP ever connected. "Strap not connected" was dead text on any install that had paired a strap once. Today cannot ask the device registry. It observes BLEManager and NOT AppModel on purpose, because AppModel publishes on the 1 Hz heart-rate tick and observing it would re-render the whole screen every second. Repository.activeDeviceIsOura exists but answers the wrong question: a Polar or Garmin strap is neither Oura nor WHOOP, and the strap's charge is just as wrong under one. So activeIsWhoop is published on LiveState, beside the batteryPct it qualifies, written in ONE place from the same activeDeviceId transition that decides which live source runs. It is set before the Apple Watch branch returns, or a watch would read as a WHOOP. It defaults to true, matching the seam's WHOOP-first default, and clearBiometrics does not touch it: which device is active is not a biometric and does not stop being true when a link drops. StrapBatteryDisplay.resolve takes activeIsWhoop with NO default value. A defaulted flag is one a future call site can forget, and forgetting it reinstates this bug in a form that still compiles. charging and the #713 runtime estimate are gated the same way. Both are strap facts: charging is the strap's BATTERY_LEVEL event and the estimate is banked from strap SoC samples. Tests: four new rows on the Swift truth table, two vectors on the Kotlin seam, and every existing Swift row carries activeIsWhoop: true so the old meaning is preserved rather than rewritten. Negative control: forcing the Kotlin seam to ignore the flag fails 4, including two that predate this change. com.noop.ui 1054 tests 0 failures, compileFullDebugKotlin clean. Apple targets compile on CI. Left as the report left them: the Settings strap card and the onboarding line, which sit under explicitly WHOOP-labelled headings. --- Strand/App/RootView.swift | 8 +++- Strand/BLE/LiveState.swift | 18 ++++++++ Strand/BLE/SourceCoordinator.swift | 5 +++ Strand/Liquid/LiquidTodayView.swift | 15 ++++++- Strand/MenuBar/MenuBarContent.swift | 13 ++++-- Strand/Screens/TodayView.swift | 3 +- StrandTests/LiquidBatteryDisplayTests.swift | 43 ++++++++++++++++--- .../src/main/java/com/noop/ui/TodayScreen.kt | 16 +++++-- .../com/noop/ui/LiveConsoleReadoutTest.kt | 18 ++++++++ 9 files changed, 122 insertions(+), 17 deletions(-) diff --git a/Strand/App/RootView.swift b/Strand/App/RootView.swift index b5bb0222f8..c2a5ca9d5a 100644 --- a/Strand/App/RootView.swift +++ b/Strand/App/RootView.swift @@ -545,7 +545,13 @@ private struct SidebarStatus: View { Text(statusText) .font(StrandFont.rounded(12, weight: .medium)) .foregroundStyle(StrandPalette.textPrimary) - Text(live.batteryPct.map { String(localized: "Battery \(Int($0))%") } ?? String(localized: "Strap not connected")) + // #2208: gated on BOTH the link and whose device it is. This read had NO gate at all, so + // it showed the strap's last charge with nothing connected: `batteryPct` is never cleared, + // which made the honest `nil` branch below unreachable on any install that had paired a + // strap once. "Strap not connected" was dead text. + Text(live.connected && live.activeIsWhoop + ? live.batteryPct.map { String(localized: "Battery \(Int($0))%") } ?? String(localized: "Strap not connected") + : String(localized: "Strap not connected")) .font(StrandFont.rounded(11)) .foregroundStyle(StrandPalette.textTertiary) } diff --git a/Strand/BLE/LiveState.swift b/Strand/BLE/LiveState.swift index 92cfd508a0..e962a8108d 100644 --- a/Strand/BLE/LiveState.swift +++ b/Strand/BLE/LiveState.swift @@ -124,6 +124,24 @@ public final class LiveState: ObservableObject { /// That is #2075, where a ring on 93% displayed the strap's 72%. Nil when no ring has reported. @Published public var ouraBatteryPct: Int? = nil + /// Whether the ACTIVE device is a WHOOP, published so a readout can answer "whose charge is this" + /// without observing `AppModel`. + /// + /// It sits here because [batteryPct] does, and the two are only meaningful together. `batteryPct` is + /// the strap's and is never cleared, deliberately, so on its own it cannot say whether it describes + /// the device the wearer is currently looking at. Every surface that reads it needs this alongside, + /// and `Today` in particular cannot reach the device registry: it observes `BLEManager` rather than + /// `AppModel` on purpose, because `AppModel` publishes on the 1 Hz heart-rate tick and observing it + /// would re-render the whole screen every second. + /// + /// Written in ONE place, `SourceCoordinator.activeDeviceChanged`, from the same `activeDeviceId` + /// transition that decides which live source runs. Defaults to true, matching + /// `LiveConsoleReadout.activeIsWhoop`'s WHOOP-first default for an unresolvable row. + /// + /// Not cleared by [clearBiometrics]: which device is active is not a biometric and does not stop + /// being true when a link drops. (#2208) + @Published public var activeIsWhoop: Bool = true + // MARK: - Battery runtime estimate (#713) /// Rolling buffer of `(unix-seconds, SoC%)` battery readings banked from the live link, the twin of diff --git a/Strand/BLE/SourceCoordinator.swift b/Strand/BLE/SourceCoordinator.swift index a8f2fb1870..99ac55a5c7 100644 --- a/Strand/BLE/SourceCoordinator.swift +++ b/Strand/BLE/SourceCoordinator.swift @@ -158,6 +158,11 @@ final class SourceCoordinator: ObservableObject { /// • WHOOP active after a strap → stop the strap source + resume WHOOP. /// • A generic strap → pause WHOOP + (re)start `StandardHRSource` for that strap's id. func activeDeviceChanged(to id: String) { + // #2208: publish whose device this is BEFORE any branch returns. Readouts that show the strap's + // charge need it to know the number is not the active device's, and the Apple Watch path below + // short-circuits, so setting it inside the WHOOP/strap split would leave a watch reading `true`. + live.activeIsWhoop = isWhoop(id) + // The Apple Watch is a HealthKit source with `peripheralId: nil` (see `AppleWatchDevice`): there is // no BLE peripheral to connect, and the M1 live read happens entirely in `HealthKitBridge`'s // observers + sync, off this BLE coordinator. Short-circuit BEFORE the WHOOP branch so we never diff --git a/Strand/Liquid/LiquidTodayView.swift b/Strand/Liquid/LiquidTodayView.swift index ad137e383e..e9963b2067 100644 --- a/Strand/Liquid/LiquidTodayView.swift +++ b/Strand/Liquid/LiquidTodayView.swift @@ -2527,8 +2527,17 @@ extension LiquidTodayView { /// A reading from the current link. case charge(pct: Double, charging: Bool) - static func resolve(connected: Bool, batteryPct: Double?, charging: Bool?) -> StrapBatteryDisplay { + /// #2208: `activeIsWhoop` is required, not defaulted. `connected` alone was never enough: it is + /// true the moment ANY source streams, `batteryPct` is the strap's and is never cleared, so under + /// an active ring both halves of the old gate passed and this drew the strap's charge. Charging + /// is strap-only for the same reason, so a non-WHOOP active device reports neither. + /// + /// No default value on purpose. A defaulted flag is one a future call site can forget, and + /// forgetting it reinstates exactly this bug in a form that still compiles. + static func resolve(activeIsWhoop: Bool, connected: Bool, + batteryPct: Double?, charging: Bool?) -> StrapBatteryDisplay { guard connected else { return .offline } + guard activeIsWhoop else { return .offline } guard let pct = batteryPct else { return .pending(charging: charging == true) } return .charge(pct: pct, charging: charging == true) } @@ -2656,6 +2665,7 @@ private struct LiquidBatteryButton: View { } #endif return .resolve( + activeIsWhoop: live.activeIsWhoop, connected: live.connected, batteryPct: live.batteryPct, charging: live.charging @@ -2871,7 +2881,8 @@ private struct LiquidSyncStatusRow: View { private struct LiquidStrapBatteryRow: View { @EnvironmentObject var live: LiveState var body: some View { - if live.connected, let pct = live.batteryPct { + // #2208: the strap's charge only when the strap is the active device. + if live.connected, live.activeIsWhoop, let pct = live.batteryPct { HStack { Text("Strap battery").font(StrandFont.subhead).foregroundStyle(StrandPalette.textSecondary) Spacer() diff --git a/Strand/MenuBar/MenuBarContent.swift b/Strand/MenuBar/MenuBarContent.swift index 21d589740d..f9eca49445 100644 --- a/Strand/MenuBar/MenuBarContent.swift +++ b/Strand/MenuBar/MenuBarContent.swift @@ -105,8 +105,15 @@ public struct MenuBarContent: View { : live.connected ? String(localized: "CONNECTED") : String(localized: "OFFLINE") } + /// #2208: the strap's charge, or nil when it is not the active device's to report. This surface read + /// `live.batteryPct` with NO gate, so it showed the strap's last percentage with nothing connected and + /// under an active ring alike. One accessor so the number and its tint cannot disagree. + private var strapBatteryPct: Double? { + (live.connected && live.activeIsWhoop) ? live.batteryPct : nil + } + private var batteryTone: StrandTone { - guard let pct = live.batteryPct else { return .neutral } + guard let pct = strapBatteryPct else { return .neutral } switch pct { case ..<15: return .critical case ..<35: return .warning @@ -212,8 +219,8 @@ public struct MenuBarContent: View { HStack(spacing: 0) { statCell( "BATTERY", - live.batteryPct.map { "\(Int($0.rounded()))%" } ?? "—", - tint: live.batteryPct == nil ? StrandPalette.textPrimary : toneColor(batteryTone) + strapBatteryPct.map { "\(Int($0.rounded()))%" } ?? "—", + tint: strapBatteryPct == nil ? StrandPalette.textPrimary : toneColor(batteryTone) ) cellDivider statCell( diff --git a/Strand/Screens/TodayView.swift b/Strand/Screens/TodayView.swift index f227ff24cb..d92d314947 100644 --- a/Strand/Screens/TodayView.swift +++ b/Strand/Screens/TodayView.swift @@ -5787,7 +5787,8 @@ private struct StrapBatteryRow: View { } var body: some View { - if live.connected, let pct = live.batteryPct { + // #2208: the strap's charge only when the strap is the active device. + if live.connected, live.activeIsWhoop, let pct = live.batteryPct { Divider().overlay(StrandPalette.hairline) HStack(spacing: 10) { SourceBadge("Strap battery", tint: tint(pct)) diff --git a/StrandTests/LiquidBatteryDisplayTests.swift b/StrandTests/LiquidBatteryDisplayTests.swift index bd13f7a1d8..0dd1d6bae4 100644 --- a/StrandTests/LiquidBatteryDisplayTests.swift +++ b/StrandTests/LiquidBatteryDisplayTests.swift @@ -19,7 +19,7 @@ final class LiquidBatteryDisplayTests: XCTestCase { /// ever) telling us a number. The old view nested the bolt inside `if let pct`, making this state /// unrenderable — it drew `bolt.slash` at a wearer who was sitting on the charger. func testChargingIsReportedEvenWithNoChargeReadingYet() { - let d = Display.resolve(connected: true, batteryPct: nil, charging: true) + let d = Display.resolve(activeIsWhoop: true, connected: true, batteryPct: nil, charging: true) XCTAssertEqual(d, .pending(charging: true), "a known charging state must survive a missing % — it is the wearer's live question") } @@ -28,7 +28,7 @@ final class LiquidBatteryDisplayTests: XCTestCase { /// `.pending(charging: false)` and `.offline` must stay distinguishable so the view can render one as /// a pending ellipsis and the other as a crossed-out bolt. func testConnectedWithNoReadingIsPendingNotOffline() { - let d = Display.resolve(connected: true, batteryPct: nil, charging: nil) + let d = Display.resolve(activeIsWhoop: true, connected: true, batteryPct: nil, charging: nil) XCTAssertEqual(d, .pending(charging: false)) XCTAssertNotEqual(d, .offline, "connected-but-silent is not the same claim as no link") } @@ -40,28 +40,57 @@ final class LiquidBatteryDisplayTests: XCTestCase { /// forever, and a view keying off `batteryPct` alone shows a dead strap's stale charge as if live. /// During the incident that rendered a 21 h old 11% identically to a fresh one. func testStaleChargeIsNotShownOnceTheLinkIsGone() { - let d = Display.resolve(connected: false, batteryPct: 11, charging: false) + let d = Display.resolve(activeIsWhoop: true, connected: false, batteryPct: 11, charging: false) XCTAssertEqual(d, .offline, "a % with no link behind it must not render as a live reading") } /// Disconnect must also drop a charging bit — nothing about the old link is still true. func testStaleChargingFlagIsNotShownOnceTheLinkIsGone() { - XCTAssertEqual(Display.resolve(connected: false, batteryPct: nil, charging: true), .offline) + XCTAssertEqual(Display.resolve(activeIsWhoop: true, connected: false, batteryPct: nil, charging: true), .offline) } // MARK: - The normal path still reads normally func testConnectedReadingCarriesPctAndChargingThrough() { - XCTAssertEqual(Display.resolve(connected: true, batteryPct: 87.4, charging: true), + XCTAssertEqual(Display.resolve(activeIsWhoop: true, connected: true, batteryPct: 87.4, charging: true), .charge(pct: 87.4, charging: true)) - XCTAssertEqual(Display.resolve(connected: true, batteryPct: 87.4, charging: false), + XCTAssertEqual(Display.resolve(activeIsWhoop: true, connected: true, batteryPct: 87.4, charging: false), .charge(pct: 87.4, charging: false)) } /// `charging` is `Bool?` — nil means "the strap hasn't said" (no BATTERY_LEVEL event this session), /// which must read as not-charging, never as charging. Same `== true` posture as the rest of the app. func testUnknownChargingReadsAsNotCharging() { - XCTAssertEqual(Display.resolve(connected: true, batteryPct: 50, charging: nil), + XCTAssertEqual(Display.resolve(activeIsWhoop: true, connected: true, batteryPct: 50, charging: nil), .charge(pct: 50, charging: false)) } + + // MARK: - #2208 whose charge is this + + /// The reported bug. `connected` goes true the moment ANY source streams, and the strap's percentage + /// is never cleared, so under an active ring both halves of the old gate passed and Today drew the + /// strap's charge. A non-WHOOP active device must show nothing rather than someone else's number. + func testRingActiveShowsNothingEvenWithAStalestrapCharge() { + XCTAssertEqual(Display.resolve(activeIsWhoop: false, connected: true, batteryPct: 72.4, charging: false), + .offline) + } + + /// Charging is the strap's BATTERY_LEVEL event, so it is strap-only for the same reason. A ring must + /// not inherit the strap's charger state either. + func testRingActiveShowsNothingWhileTheStrapCharges() { + XCTAssertEqual(Display.resolve(activeIsWhoop: false, connected: true, batteryPct: 88, charging: true), + .offline) + } + + /// A ring active with no strap charge ever recorded is the same answer, reached the other way. + func testRingActiveWithNoStrapChargeIsAlsoOffline() { + XCTAssertEqual(Display.resolve(activeIsWhoop: false, connected: true, batteryPct: nil, charging: nil), + .offline) + } + + /// The WHOOP path is unchanged: this fix must not blank a strap that IS the active device. + func testWhoopActiveStillReportsItsCharge() { + XCTAssertEqual(Display.resolve(activeIsWhoop: true, connected: true, batteryPct: 61, charging: false), + .charge(pct: 61, charging: false)) + } } diff --git a/android/app/src/main/java/com/noop/ui/TodayScreen.kt b/android/app/src/main/java/com/noop/ui/TodayScreen.kt index 480f6b1cca..d5ebeec5ff 100644 --- a/android/app/src/main/java/com/noop/ui/TodayScreen.kt +++ b/android/app/src/main/java/com/noop/ui/TodayScreen.kt @@ -333,6 +333,11 @@ fun TodayScreen( val days by viewModel.recentDays.collectAsStateWithLifecycle() val activeDayCycle by viewModel.activeDayCycle.collectAsStateWithLifecycle() val spo2CandidateByDay by viewModel.spo2CandidateByDay.collectAsStateWithLifecycle() + // #2208: `connected` alone never said WHOSE charge liveSnap.batteryPct is. It goes true the moment any + // source streams, and the strap's percentage is never cleared, so under an active ring both halves of + // the old gate passed and Today drew the strap's charge. Same seam the Devices list already uses. + val activeIsWhoop by viewModel.activeIsWhoop.collectAsStateWithLifecycle() + val ouraBatteryPct by viewModel.ouraBatteryPct.collectAsStateWithLifecycle() val v5Signals by viewModel.v5Signals.collectAsStateWithLifecycle() val cycleEnabled by viewModel.cycleTrackingEnabled.collectAsStateWithLifecycle() val cycleHidden by viewModel.cycleAwarenessHidden.collectAsStateWithLifecycle() @@ -1391,7 +1396,7 @@ fun TodayScreen( dayTitle = dayTitle, humanDate = humanDate, selectedDay = selectedDay, - batteryPct = if (liveSnap.connected) liveSnap.batteryPct else null, + batteryPct = if (liveSnap.connected && activeIsWhoop) liveSnap.batteryPct else null, backfilling = liveSnap.backfilling, syncChunksThisSession = liveSnap.syncChunksThisSession, lastSyncAt = liveSnap.lastSyncAt, @@ -1901,8 +1906,13 @@ fun TodayScreen( item { TodaySourcesSection( footer, - strapBatteryPct = if (liveSnap.connected) liveSnap.batteryPct?.roundToInt() else null, - strapBatteryEstimate = if (liveSnap.connected) batteryEstimateText else null, + strapBatteryPct = if (liveSnap.connected) LiveConsoleReadout.batteryPercent( + activeIsWhoop = activeIsWhoop, + whoopPct = liveSnap.batteryPct, + ringPct = ouraBatteryPct, + ) else null, + // The runtime estimate is banked from strap SoC samples, so it is the strap's alone. + strapBatteryEstimate = if (liveSnap.connected && activeIsWhoop) batteryEstimateText else null, expanded = sourcesExpanded, onToggle = { sourcesExpanded = !sourcesExpanded }, ) diff --git a/android/app/src/test/java/com/noop/ui/LiveConsoleReadoutTest.kt b/android/app/src/test/java/com/noop/ui/LiveConsoleReadoutTest.kt index be578af183..c9bc34f41e 100644 --- a/android/app/src/test/java/com/noop/ui/LiveConsoleReadoutTest.kt +++ b/android/app/src/test/java/com/noop/ui/LiveConsoleReadoutTest.kt @@ -109,4 +109,22 @@ class LiveConsoleReadoutTest { // Symmetry: a ring that reported earlier must not leak into a strap's readout either. assertNull(LiveConsoleReadout.batteryPercent(true, null, 93)) } + + /** + * #2208: Today used to gate on `connected` alone, which goes true the moment ANY source streams, + * while the strap's percentage is never cleared. Both halves passed under an active ring and the + * screen drew the strap's charge. The seam's answer for a non-WHOOP active device is the ring's own + * number, or nothing, never the strap's. + */ + @Test + fun ringActiveNeverFallsBackToTheStrapCharge() { + assertEquals(93, LiveConsoleReadout.batteryPercent(false, 72.4, 93)) + assertNull(LiveConsoleReadout.batteryPercent(false, 72.4, null)) + } + + /** And the strap keeps reporting when it IS the active device. */ + @Test + fun whoopActiveStillReportsItsOwnCharge() { + assertEquals(72, LiveConsoleReadout.batteryPercent(true, 72.4, 93)) + } } From 3cba13fa0e0d4705f041a79c945f15ae1e356319 Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Tue, 15 Sep 2026 00:14:05 +1200 Subject: [PATCH 2/5] battery: carry the flag through the demo harness call site too CI caught a second StrapBatteryDisplay.resolve call, inside `#if DEBUG` for DemoSyncHarness. It stands in for a connected WHOOP, so it answers true. Worth recording why this was missed and why it still failed loudly. Missed because my sweep grepped `.resolve(connected:` on one line, and this call puts `connected:` on the line after the open paren. A single-line pattern cannot see a multi-line call, and I used one while fixing a bug whose whole shape is "the previous sweep missed some readers". Re-swept with a multi-line matcher; the only other hit is RecordingState.resolve, a different type. Failed loudly because the parameter has no default value. That choice was made so a future call site could not forget the flag and silently reinstate the bug, and the first thing it caught was me forgetting it. A defaulted parameter would have compiled and left the demo harness on the old behaviour. --- Strand/Liquid/LiquidTodayView.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Strand/Liquid/LiquidTodayView.swift b/Strand/Liquid/LiquidTodayView.swift index e9963b2067..1913f1c3fa 100644 --- a/Strand/Liquid/LiquidTodayView.swift +++ b/Strand/Liquid/LiquidTodayView.swift @@ -2657,7 +2657,9 @@ private struct LiquidBatteryButton: View { private var batteryDisplay: LiquidTodayView.StrapBatteryDisplay { #if DEBUG if DemoSyncHarness.active { + // The harness stands in for a connected WHOOP, so it answers this the way one would. return .resolve( + activeIsWhoop: true, connected: true, batteryPct: DemoSyncHarness.batteryPercent, charging: DemoSyncHarness.charging From 768844f8909281286668a310af5e7dcaab5710ec Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Tue, 15 Sep 2026 00:31:26 +1200 Subject: [PATCH 3/5] battery: show nothing in the WHOOP row, not the ring's charge Second re-review pass on #2216, and this one was a bug I had introduced rather than one I was fixing. I routed the Android sources pill through LiveConsoleReadout.batteryPercent, which SUBSTITUTES the ring's charge when the active device is not a WHOOP. That is right for a readout naming the active device, which is what the Live Console and the Devices list are, and wrong here. This value lands in the SourceRow badged `today_source_whoop`, and it also feeds that row's `present` flag: val whoopPresent = (footer.whoopDays ?: 0) > 0 || strapBatteryPct != null So the substitution would have put the ring's percentage under a WHOOP label and asserted a WHOOP source that is not there. That is worse than the stale reading being fixed: the old bug at least showed a number that really was the strap's. Gated instead, matching the six other surfaces and the iOS row beside it, which is labelled "Strap battery" and hides for the same reason. Nothing is the honest answer for a strap that is not the active device. The report suggested the substitution too, and was wrong for the same reason mine was: the seam is correct for a readout that names whichever device is active, and a WHOOP-labelled row is not one. Applying a seam mechanically is what both of us did; what the destination MEANS is the part that has to be read. ouraBatteryPct is no longer collected in TodayScreen, since nothing there reads it now. com.noop.ui 1054 tests 0 failures, compileFullDebugKotlin clean. --- .../app/src/main/java/com/noop/ui/TodayScreen.kt | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/android/app/src/main/java/com/noop/ui/TodayScreen.kt b/android/app/src/main/java/com/noop/ui/TodayScreen.kt index d5ebeec5ff..3faff55167 100644 --- a/android/app/src/main/java/com/noop/ui/TodayScreen.kt +++ b/android/app/src/main/java/com/noop/ui/TodayScreen.kt @@ -337,7 +337,6 @@ fun TodayScreen( // source streams, and the strap's percentage is never cleared, so under an active ring both halves of // the old gate passed and Today drew the strap's charge. Same seam the Devices list already uses. val activeIsWhoop by viewModel.activeIsWhoop.collectAsStateWithLifecycle() - val ouraBatteryPct by viewModel.ouraBatteryPct.collectAsStateWithLifecycle() val v5Signals by viewModel.v5Signals.collectAsStateWithLifecycle() val cycleEnabled by viewModel.cycleTrackingEnabled.collectAsStateWithLifecycle() val cycleHidden by viewModel.cycleAwarenessHidden.collectAsStateWithLifecycle() @@ -1906,11 +1905,14 @@ fun TodayScreen( item { TodaySourcesSection( footer, - strapBatteryPct = if (liveSnap.connected) LiveConsoleReadout.batteryPercent( - activeIsWhoop = activeIsWhoop, - whoopPct = liveSnap.batteryPct, - ringPct = ouraBatteryPct, - ) else null, + // NOT routed through LiveConsoleReadout.batteryPercent, which substitutes the RING's charge + // for a non-WHOOP active device. That is right for a readout that names the active device, + // and wrong here: this value lands in the SourceRow badged `today_source_whoop`, and it + // also feeds that row's `present` flag. Substituting would put the ring's number under a + // WHOOP label and assert a WHOOP source that is not there, which is worse than the stale + // reading being fixed. Nothing is the honest answer for a strap that is not active. + strapBatteryPct = if (liveSnap.connected && activeIsWhoop) + liveSnap.batteryPct?.roundToInt() else null, // The runtime estimate is banked from strap SoC samples, so it is the strap's alone. strapBatteryEstimate = if (liveSnap.connected && activeIsWhoop) batteryEstimateText else null, expanded = sourcesExpanded, From ee00683bf9b9a90fd60c0f6b42534972d1872277 Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Tue, 15 Sep 2026 00:45:50 +1200 Subject: [PATCH 4/5] docs: warn at the field the whole class keeps tripping over Third re-review pass on #2216. No behaviour change. Two things came out of it. First, the sweep is now enumerated rather than asserted. Every reader of the strap battery field, both platforms: seven fixed here, six already correct because they route through LiveConsoleReadout, four deliberately out of scope under WHOOP-labelled headings (Settings and onboarding on each platform), one diagnostic in Test Centre where the raw strap SoC is the thing being inspected, and three that are not readouts at all (an internal BLEManager read and two LaunchedEffect keys). Nothing is left unaccounted for. Second, and the reason this keeps happening: `batteryPct` carried NO documentation at all. A field that is the strap's alone, is never cleared by design, and cannot be qualified by the `connected` flag sitting next to it, was a bare declaration. That is how it ended up read raw by seven surfaces across two separate reports. It now says all three things, and says which of the two correct readings a surface needs: LiveConsoleReadout.batteryPercent for a readout naming the ACTIVE device, or pairing with activeIsWhoop and showing nothing for a readout labelled as the strap's. Getting that choice wrong is the bug the previous pass found in my own change, so the distinction is written where a reader meets it rather than in a commit message nobody will look up. --- Strand/BLE/LiveState.swift | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/Strand/BLE/LiveState.swift b/Strand/BLE/LiveState.swift index e962a8108d..4130cbed8c 100644 --- a/Strand/BLE/LiveState.swift +++ b/Strand/BLE/LiveState.swift @@ -72,6 +72,22 @@ public final class LiveState: ObservableObject { /// separate short history to render an actually-moving R-R strip / rolling RMSSD. Appended (never /// replaced) by `setRRIntervals(_:)`; emptied by `clearBiometrics()`. @Published public private(set) var rrRecent: [Int] = [] + /// The WHOOP strap's last reported charge. NOT the active device's. + /// + /// Two properties make this dangerous to read on its own, and both are deliberate. It is the + /// strap's alone, with no other source writing it. And it is never cleared: [clearBiometrics] + /// blanks the ring's charge beside it and leaves this, so a strap's last percentage outlives its + /// link on purpose, which is what lets a reconnect show a number before the first fresh reading. + /// + /// So `connected` does not qualify it. That flag goes true the moment ANY source streams, including + /// a ring's first live heart rate, at which point a stale strap percentage satisfies both halves of + /// the obvious gate. That is #2076 and #2208, the same bug found twice, across ten surfaces. + /// + /// Any readout naming the ACTIVE device must go through `LiveConsoleReadout.batteryPercent`, which + /// substitutes the ring's own charge. Any readout labelled as the strap's must pair this with + /// [activeIsWhoop] and show nothing when it is false: substituting there would put a ring's number + /// under a WHOOP heading. Which of the two a surface needs depends on what it claims to be showing, + /// and that is a question about the label rather than about this field. @Published public var batteryPct: Double? = nil /// 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 the From e63ff260ac81f29755ad9cab3616f3f913151c7e Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Tue, 15 Sep 2026 00:58:25 +1200 Subject: [PATCH 5/5] battery: draw nothing under a ring, rather than claiming the strap is offline @pipiche38's review of #2216, and they are right. The previous shape traded one false claim for another on the same screen. StrapBatteryDisplay.resolve returned .offline for a non-WHOOP active device, so a wearer with a ring streaming 93% saw the crossed-out bolt on the Today header and VoiceOver read "Strap battery, strap not connected". That is a different wrong statement from the one reported in #2208, not a repair of it, and a ring-only wearer would meet it every day, which is exactly who the Ring 5 report came from. .notActiveDevice is now its own case, and the header control is not drawn at all when it applies. Adding a case rather than reusing .offline means the compiler required both switches to answer, which is the property that already caught a missed call site earlier in this PR. Keeping .offline for a strap that IS active and disconnected, because that is a real claim worth making. The two answers were being collapsed, and a test now asserts they differ. Android matches: LiquidTodayHeader takes strapIsActiveDevice separately from batteryPct, and skips the ring entirely when it is false. Separate parameters on purpose: batteryPct says what the control reads, the flag says whether the control should exist. A null percentage while the strap IS active means "connected, no reading yet" and is worth drawing; a strap that is not the active device has nothing to say. Not taking the other option on offer. Showing the RING's charge on these surfaces would need "Ring battery" in nine locales and a ring-charging source, and @pipiche38 did not argue for it either. Hiding is what the two Today rows already do, so this is now one answer across all of them. com.noop.ui 1054 tests 0 failures, compileFullDebugKotlin clean. --- Strand/Liquid/LiquidTodayView.swift | 49 +++++++++++++------ StrandTests/LiquidBatteryDisplayTests.swift | 16 ++++-- .../src/main/java/com/noop/ui/TodayScreen.kt | 14 +++++- 3 files changed, 58 insertions(+), 21 deletions(-) diff --git a/Strand/Liquid/LiquidTodayView.swift b/Strand/Liquid/LiquidTodayView.swift index 1913f1c3fa..839caab4f2 100644 --- a/Strand/Liquid/LiquidTodayView.swift +++ b/Strand/Liquid/LiquidTodayView.swift @@ -2526,6 +2526,13 @@ extension LiquidTodayView { case pending(charging: Bool) /// A reading from the current link. case charge(pct: Double, charging: Bool) + /// The strap is not the active device, so this control has nothing to say and is not drawn. + /// + /// Distinct from [offline], which asserts a strap that IS active is not connected. Collapsing the + /// two put a crossed-out bolt and "strap not connected" on the header of a wearer whose ring was + /// streaming, which is a different false claim from the one #2208 is about rather than a fix for + /// it, and the only one a ring-only wearer would see every day. (@pipiche38 on #2216) + case notActiveDevice /// #2208: `activeIsWhoop` is required, not defaulted. `connected` alone was never enough: it is /// true the moment ANY source streams, `batteryPct` is the strap's and is never cleared, so under @@ -2536,8 +2543,8 @@ extension LiquidTodayView { /// forgetting it reinstates exactly this bug in a form that still compiles. static func resolve(activeIsWhoop: Bool, connected: Bool, batteryPct: Double?, charging: Bool?) -> StrapBatteryDisplay { + guard activeIsWhoop else { return .notActiveDevice } guard connected else { return .offline } - guard activeIsWhoop else { return .offline } guard let pct = batteryPct else { return .pending(charging: charging == true) } return .charge(pct: pct, charging: charging == true) } @@ -2676,7 +2683,7 @@ private struct LiquidBatteryButton: View { private var indicatorState: ChargeSyncIndicator.BatteryState { switch batteryDisplay { - case .offline: + case .offline, .notActiveDevice: return .offline case .pending(let charging): return .pending(charging: charging) @@ -2686,21 +2693,29 @@ private struct LiquidBatteryButton: View { } var body: some View { - Button { router.openDevices() } label: { - ChargeSyncIndicator( - batteryState: indicatorState, - syncing: syncing, - chunks: syncChunks - ) + // Not drawn at all when the strap is not the active device. The alternative is a glyph that + // has to say SOMETHING about a strap nobody is wearing, and every option is a claim: a charge + // that is not the active device's, or a crossed-out bolt asserting a disconnection that is not + // the interesting fact. The two Today rows already resolve it this way. (#2208) + if case .notActiveDevice = batteryDisplay { + EmptyView() + } else { + Button { router.openDevices() } label: { + ChargeSyncIndicator( + batteryState: indicatorState, + syncing: syncing, + chunks: syncChunks + ) + } + .nativeLiquidGlassSyncButton() + .accessibilityLabel(batteryAccessibility) + .debouncedSyncSignal(syncingRaw, into: $syncing) + // DEBUG-gated at the CALL SITE too, not just in the body: in Release the harness must cost + // literally nothing, rather than an async task created and immediately returned per appearance. + #if DEBUG + .task { await runDemoSyncCycleIfNeeded() } + #endif } - .nativeLiquidGlassSyncButton() - .accessibilityLabel(batteryAccessibility) - .debouncedSyncSignal(syncingRaw, into: $syncing) - // DEBUG-gated at the CALL SITE too, not just in the body: in Release the harness must cost - // literally nothing, rather than an async task created and immediately returned per appearance. - #if DEBUG - .task { await runDemoSyncCycleIfNeeded() } - #endif } /// DEBUG `--demo-sync` only: loop the syncing signal so the charge→sync morph plays in both @@ -2767,6 +2782,8 @@ private struct LiquidBatteryButton: View { } switch batteryDisplay { + case .notActiveDevice: + return "" // not drawn; the label is unreachable and must not claim anything case .offline: return String(localized: "Strap battery, strap not connected") case .pending(let charging): diff --git a/StrandTests/LiquidBatteryDisplayTests.swift b/StrandTests/LiquidBatteryDisplayTests.swift index 0dd1d6bae4..352ea98174 100644 --- a/StrandTests/LiquidBatteryDisplayTests.swift +++ b/StrandTests/LiquidBatteryDisplayTests.swift @@ -72,19 +72,29 @@ final class LiquidBatteryDisplayTests: XCTestCase { /// strap's charge. A non-WHOOP active device must show nothing rather than someone else's number. func testRingActiveShowsNothingEvenWithAStalestrapCharge() { XCTAssertEqual(Display.resolve(activeIsWhoop: false, connected: true, batteryPct: 72.4, charging: false), - .offline) + .notActiveDevice) } /// Charging is the strap's BATTERY_LEVEL event, so it is strap-only for the same reason. A ring must /// not inherit the strap's charger state either. func testRingActiveShowsNothingWhileTheStrapCharges() { XCTAssertEqual(Display.resolve(activeIsWhoop: false, connected: true, batteryPct: 88, charging: true), - .offline) + .notActiveDevice) } /// A ring active with no strap charge ever recorded is the same answer, reached the other way. - func testRingActiveWithNoStrapChargeIsAlsoOffline() { + func testRingActiveWithNoStrapChargeIsAlsoNotDrawn() { XCTAssertEqual(Display.resolve(activeIsWhoop: false, connected: true, batteryPct: nil, charging: nil), + .notActiveDevice) + } + + /// The distinction @pipiche38 asked for on #2216. A strap that IS active and disconnected is offline, + /// a real claim worth making. A strap that is not the active device is a different answer entirely, + /// and collapsing the two told a wearer with a streaming ring that their strap was not connected. + func testNotActiveIsNotTheSameAnswerAsOffline() { + XCTAssertNotEqual(Display.resolve(activeIsWhoop: false, connected: true, batteryPct: 72, charging: false), + Display.resolve(activeIsWhoop: true, connected: false, batteryPct: 72, charging: false)) + XCTAssertEqual(Display.resolve(activeIsWhoop: true, connected: false, batteryPct: 72, charging: false), .offline) } diff --git a/android/app/src/main/java/com/noop/ui/TodayScreen.kt b/android/app/src/main/java/com/noop/ui/TodayScreen.kt index 3faff55167..bcba5b975d 100644 --- a/android/app/src/main/java/com/noop/ui/TodayScreen.kt +++ b/android/app/src/main/java/com/noop/ui/TodayScreen.kt @@ -1395,7 +1395,8 @@ fun TodayScreen( dayTitle = dayTitle, humanDate = humanDate, selectedDay = selectedDay, - batteryPct = if (liveSnap.connected && activeIsWhoop) liveSnap.batteryPct else null, + batteryPct = if (liveSnap.connected) liveSnap.batteryPct else null, + strapIsActiveDevice = activeIsWhoop, backfilling = liveSnap.backfilling, syncChunksThisSession = liveSnap.syncChunksThisSession, lastSyncAt = liveSnap.lastSyncAt, @@ -2511,6 +2512,11 @@ private fun LiquidTodayHeader( humanDate: String, selectedDay: LocalDate, batteryPct: Double?, + /** Whether the STRAP is the active device. Separate from [batteryPct] on purpose: that says what + * the control reads, this says whether the control should exist. A null percentage while the strap + * IS active means "connected, no reading yet" and is worth drawing; a strap that is not the active + * device has nothing to say and is not drawn at all. (#2208) */ + strapIsActiveDevice: Boolean, // #245: sync state for the compact header chip (twin of iOS SyncStatusChip). backfilling: Boolean = false, syncChunksThisSession: Int = 0, @@ -2640,7 +2646,11 @@ private fun LiquidTodayHeader( // disc → the quick-actions menu). Sized to match the rest of the liquid cluster (shared HeaderClusterControl). QuickActionDisc(onClick = onQuickActions) // (c) Strap battery ring showing the % (iOS LiquidBatteryButton). Tap → Devices. - LiquidBatteryRing(batteryPct = batteryPct, onClick = onOpenDevices) + // Not drawn when the strap is not the active device: an empty "Strap battery" ring under a + // streaming ring is a control asserting something about a strap nobody is wearing. + if (strapIsActiveDevice) { + LiquidBatteryRing(batteryPct = batteryPct, onClick = onOpenDevices) + } } } }