Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 44 additions & 18 deletions Strand/Liquid/LiquidTodayView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2559,9 +2559,11 @@ extension LiquidTodayView {
case offline
/// Linked, but no charge reading has landed yet. `charging` is still knowable on its own.
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.
/// A reading from the current link. `isRing` says whose: the ring's own charge under an active
/// ring, the strap's under an active strap — the label names the device the number belongs to.
case charge(pct: Double, charging: Bool, isRing: Bool)
/// The active device is neither the strap nor a ring that has reported its charge this link, 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
Expand All @@ -2572,16 +2574,26 @@ extension LiquidTodayView {
/// #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.
/// is strap-only for the same reason, so a non-WHOOP active device reports neither of the strap's.
///
/// No default value on purpose. A defaulted flag is one a future call site can forget, and
/// A ring reports its OWN charge into `ringPct` (`LiveState.ouraBatteryPct`), cleared with the
/// link, so under a non-WHOOP active device a non-nil `ringPct` is a reading from the ring that is
/// live right now and is drawn as such; nil (no ring, or none has reported yet) keeps the control
/// off the header. `ringCharging` is the ring's charger state (`OuraWearState.charging`), the only
/// charging evidence a ring gives. Same resolution `LiveConsoleReadout.batteryPercent` applies.
///
/// No default values 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 activeIsWhoop else { return .notActiveDevice }
batteryPct: Double?, charging: Bool?,
ringPct: Int?, ringCharging: Bool) -> StrapBatteryDisplay {
guard activeIsWhoop else {
guard let ringPct else { return .notActiveDevice }
return .charge(pct: Double(ringPct), charging: ringCharging, isRing: true)
}
guard connected else { return .offline }
guard let pct = batteryPct else { return .pending(charging: charging == true) }
return .charge(pct: pct, charging: charging == true)
return .charge(pct: pct, charging: charging == true, isRing: false)
}
}

Expand Down Expand Up @@ -2666,8 +2678,9 @@ extension LiquidTodayView {
}
}

/// Strap-battery ring. At sync start it briefly expands within the trailing control row, then settles into
/// an in-place spinner; the layered header keeps either state from moving the Today content. Tap → Devices.
/// Active-device battery ring: the strap's charge under an active strap, the ring's own under an active
/// ring. At sync start it briefly expands within the trailing control row, then settles into an in-place
/// spinner; the layered header keeps either state from moving the Today content. Tap → Devices.
private struct LiquidBatteryButton: View {
@EnvironmentObject var live: LiveState
@EnvironmentObject var router: NavRouter
Expand Down Expand Up @@ -2704,15 +2717,19 @@ private struct LiquidBatteryButton: View {
activeIsWhoop: true,
connected: true,
batteryPct: DemoSyncHarness.batteryPercent,
charging: DemoSyncHarness.charging
charging: DemoSyncHarness.charging,
ringPct: nil,
ringCharging: false
)
}
#endif
return .resolve(
activeIsWhoop: live.activeIsWhoop,
connected: live.connected,
batteryPct: live.batteryPct,
charging: live.charging
charging: live.charging,
ringPct: live.ouraBatteryPct,
ringCharging: live.ouraWearState == .charging
)
}

Expand All @@ -2722,16 +2739,18 @@ private struct LiquidBatteryButton: View {
return .offline
case .pending(let charging):
return .pending(charging: charging)
case .charge(let percent, let charging):
case .charge(let percent, let charging, _):
return .charge(percent: percent, charging: charging)
}
}

var body: some View {
// 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)
// Not drawn at all when the active device is neither the strap nor a ring with a charge of its
// own to show. 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. (#2208) A ring that HAS
// reported its charge is the active device's own reading, and #2208's fix left it undrawn only
// because the control could not yet tell whose number it held.
if case .notActiveDevice = batteryDisplay {
EmptyView()
} else {
Expand Down Expand Up @@ -2825,8 +2844,15 @@ private struct LiquidBatteryButton: View {
return charging
? String(localized: "Strap battery charging, no reading yet")
: String(localized: "Strap battery, no reading yet")
case .charge(let percent, let charging):
case .charge(let percent, let charging, let isRing):
let n = Int(percent.rounded())
// Named for the device the number belongs to: "Strap battery" over a ring's charge would be
// the #2208 misattribution again, in the label instead of the number.
if isRing {
return charging
? String(localized: "Ring battery \(n) percent, charging")
: String(localized: "Ring battery \(n) percent")
}
return charging
? String(localized: "Strap battery \(n) percent, charging")
: String(localized: "Strap battery \(n) percent")
Expand Down
6 changes: 6 additions & 0 deletions Strand/Resources/Localizable.xcstrings
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
{
"sourceLanguage": "en",
"strings": {
"Ring battery %lld percent": { "localizations": {
"de": {"stringUnit": {"state": "translated", "value": "Ring-Akku %lld Prozent"}}, "es": {"stringUnit": {"state": "translated", "value": "Batería del anillo %lld por ciento"}}, "fr": {"stringUnit": {"state": "translated", "value": "Batterie de la bague %lld pour cent"}}, "it": {"stringUnit": {"state": "translated", "value": "Batteria dell'anello %lld percento"}}, "pl": {"stringUnit": {"state": "translated", "value": "Bateria pierścienia %lld procent"}}, "pt-PT": {"stringUnit": {"state": "translated", "value": "Bateria do anel %lld por cento"}}, "ru": {"stringUnit": {"state": "translated", "value": "Заряд кольца %lld процентов"}}, "zh-Hans": {"stringUnit": {"state": "translated", "value": "戒指电量百分之 %lld"}}, "zh-Hant": {"stringUnit": {"state": "translated", "value": "戒指電量百分之 %lld"}}
} },
"Ring battery %lld percent, charging": { "localizations": {
"de": {"stringUnit": {"state": "translated", "value": "Ring-Akku %lld Prozent, wird geladen"}}, "es": {"stringUnit": {"state": "translated", "value": "Batería del anillo %lld por ciento, cargando"}}, "fr": {"stringUnit": {"state": "translated", "value": "Batterie de la bague %lld pour cent, en charge"}}, "it": {"stringUnit": {"state": "translated", "value": "Batteria dell'anello %lld percento, in carica"}}, "pl": {"stringUnit": {"state": "translated", "value": "Bateria pierścienia %lld procent, ładowanie"}}, "pt-PT": {"stringUnit": {"state": "translated", "value": "Bateria do anel %lld por cento, a carregar"}}, "ru": {"stringUnit": {"state": "translated", "value": "Заряд кольца %lld процентов, заряжается"}}, "zh-Hans": {"stringUnit": {"state": "translated", "value": "戒指电量百分之 %lld,正在充电"}}, "zh-Hant": {"stringUnit": {"state": "translated", "value": "戒指電量百分之 %lld,正在充電"}}
} },
"Oura notification mask ff (experimental)": { "localizations": {
"de": {"stringUnit": {"state": "translated", "value": "Oura-Benachrichtigungsmaske ff (experimentell)"}}, "es": {"stringUnit": {"state": "translated", "value": "Máscara de notificación Oura ff (experimental)"}}, "fr": {"stringUnit": {"state": "translated", "value": "Masque de notification Oura ff (expérimental)"}}, "pt-PT": {"stringUnit": {"state": "translated", "value": "Máscara de notificação Oura ff (experimental)"}}, "it": {"stringUnit": {"state": "translated", "value": "Maschera di notifica Oura ff (sperimentale)"}}, "pl": {"stringUnit": {"state": "translated", "value": "Maska powiadomień Oura ff (eksperymentalne)"}}, "ru": {"stringUnit": {"state": "translated", "value": "Маска уведомлений Oura ff (экспериментально)"}}, "zh-Hans": {"stringUnit": {"state": "translated", "value": "Oura 通知掩码 ff(实验性)"}}, "zh-Hant": {"stringUnit": {"state": "translated", "value": "Oura 通知遮罩 ff(實驗性)"}}
} },
Expand Down
72 changes: 54 additions & 18 deletions StrandTests/LiquidBatteryDisplayTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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(activeIsWhoop: true, connected: true, batteryPct: nil, charging: true)
let d = Display.resolve(activeIsWhoop: true, connected: true, batteryPct: nil, charging: true, ringPct: nil, ringCharging: false)
XCTAssertEqual(d, .pending(charging: true),
"a known charging state must survive a missing % — it is the wearer's live question")
}
Expand All @@ -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(activeIsWhoop: true, connected: true, batteryPct: nil, charging: nil)
let d = Display.resolve(activeIsWhoop: true, connected: true, batteryPct: nil, charging: nil, ringPct: nil, ringCharging: false)
XCTAssertEqual(d, .pending(charging: false))
XCTAssertNotEqual(d, .offline, "connected-but-silent is not the same claim as no link")
}
Expand All @@ -40,29 +40,29 @@ 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(activeIsWhoop: true, connected: false, batteryPct: 11, charging: false)
let d = Display.resolve(activeIsWhoop: true, connected: false, batteryPct: 11, charging: false, ringPct: nil, ringCharging: 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(activeIsWhoop: true, connected: false, batteryPct: nil, charging: true), .offline)
XCTAssertEqual(Display.resolve(activeIsWhoop: true, connected: false, batteryPct: nil, charging: true, ringPct: nil, ringCharging: false), .offline)
}

// MARK: - The normal path still reads normally

func testConnectedReadingCarriesPctAndChargingThrough() {
XCTAssertEqual(Display.resolve(activeIsWhoop: true, connected: true, batteryPct: 87.4, charging: true),
.charge(pct: 87.4, charging: true))
XCTAssertEqual(Display.resolve(activeIsWhoop: true, connected: true, batteryPct: 87.4, charging: false),
.charge(pct: 87.4, charging: false))
XCTAssertEqual(Display.resolve(activeIsWhoop: true, connected: true, batteryPct: 87.4, charging: true, ringPct: nil, ringCharging: false),
.charge(pct: 87.4, charging: true, isRing: false))
XCTAssertEqual(Display.resolve(activeIsWhoop: true, connected: true, batteryPct: 87.4, charging: false, ringPct: nil, ringCharging: false),
.charge(pct: 87.4, charging: false, isRing: 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(activeIsWhoop: true, connected: true, batteryPct: 50, charging: nil),
.charge(pct: 50, charging: false))
XCTAssertEqual(Display.resolve(activeIsWhoop: true, connected: true, batteryPct: 50, charging: nil, ringPct: nil, ringCharging: false),
.charge(pct: 50, charging: false, isRing: false))
}

// MARK: - #2208 whose charge is this
Expand All @@ -71,36 +71,72 @@ final class LiquidBatteryDisplayTests: XCTestCase {
/// 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),
XCTAssertEqual(Display.resolve(activeIsWhoop: false, connected: true, batteryPct: 72.4, charging: false, ringPct: nil, ringCharging: false),
.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),
XCTAssertEqual(Display.resolve(activeIsWhoop: false, connected: true, batteryPct: 88, charging: true, ringPct: nil, ringCharging: false),
.notActiveDevice)
}

/// A ring active with no strap charge ever recorded is the same answer, reached the other way.
func testRingActiveWithNoStrapChargeIsAlsoNotDrawn() {
XCTAssertEqual(Display.resolve(activeIsWhoop: false, connected: true, batteryPct: nil, charging: nil),
XCTAssertEqual(Display.resolve(activeIsWhoop: false, connected: true, batteryPct: nil, charging: nil, ringPct: nil, ringCharging: false),
.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),
XCTAssertNotEqual(Display.resolve(activeIsWhoop: false, connected: true, batteryPct: 72, charging: false, ringPct: nil, ringCharging: false),
Display.resolve(activeIsWhoop: true, connected: false, batteryPct: 72, charging: false, ringPct: nil, ringCharging: false))
XCTAssertEqual(Display.resolve(activeIsWhoop: true, connected: false, batteryPct: 72, charging: false, ringPct: nil, ringCharging: false),
.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))
XCTAssertEqual(Display.resolve(activeIsWhoop: true, connected: true, batteryPct: 61, charging: false, ringPct: nil, ringCharging: false),
.charge(pct: 61, charging: false, isRing: false))
}

// MARK: - The ring's own charge

/// The other half of #2208. Hiding the strap's number under a ring was right; hiding the RING's was
/// only ever a limitation of the control, which could not say whose number it held. A ring that has
/// reported its charge this link is the active device's own reading, and is drawn as the ring's.
func testRingActiveDrawsTheRingsOwnCharge() {
XCTAssertEqual(Display.resolve(activeIsWhoop: false, connected: true, batteryPct: 72.4, charging: true,
ringPct: 93, ringCharging: false),
.charge(pct: 93, charging: false, isRing: true),
"the strap's 72 % and its charger bit must not leak into the ring's reading")
}

/// The ring's charging evidence is its own charger state, never the strap's BATTERY_LEVEL bit.
func testRingChargingComesFromTheRingsChargerState() {
XCTAssertEqual(Display.resolve(activeIsWhoop: false, connected: true, batteryPct: nil, charging: nil,
ringPct: 40, ringCharging: true),
.charge(pct: 40, charging: true, isRing: true))
}

/// `LiveState.ouraBatteryPct` is cleared with the link (#2075), so under a non-WHOOP active device a
/// nil ring charge means "no ring is live, or it has not reported yet" — and a generic HR strap or a
/// machine, which never write it, keep the control off the header exactly as before.
func testRingActiveWithNoRingChargeYetIsStillNotDrawn() {
XCTAssertEqual(Display.resolve(activeIsWhoop: false, connected: true, batteryPct: 72, charging: false,
ringPct: nil, ringCharging: false),
.notActiveDevice)
}

/// A ring charge that somehow sits beside an active STRAP is not the strap's reading and must not
/// replace it: `activeIsWhoop` decides whose number is shown, the same way `batteryPercent` does.
func testStrapActiveIgnoresARingCharge() {
XCTAssertEqual(Display.resolve(activeIsWhoop: true, connected: true, batteryPct: 61, charging: false,
ringPct: 93, ringCharging: true),
.charge(pct: 61, charging: false, isRing: false))
}
}
Loading
Loading