Fix: Stop presenting a failed battery poll as the current reading

A throwing read re-emitted the last known readout forever, so a reader
that stayed broken froze the UI on whatever it last saw — still claiming
"Charging · 82%" long after the cable came out. That was tolerable while
the reading was incidental; it is not now that a surface labels it "Now".

The repeat is capped at two consecutive failures (~6s at the default
interval), after which the flow emits BatteryReadout.UNKNOWN and every
field honestly reads "Not reported" until a read succeeds. A transient
blip stays invisible, a sustained failure stops being asserted, and a
recovery emits fresh data rather than a stale copy.

The loop moved to an internal batteryReadouts(interval, read) so the
failure/recovery behaviour is JVM-testable against a scripted reader
instead of a real device. CancellationException still propagates rather
than being absorbed as a failed read.

Author: d4rken

app/src/main/java/eu/darken/amply/battery/core/BatteryReadoutSource.kt

@@ -17,33 +17,73 @@ import javax.inject.Singleton
  * is controlled by the collector (the dashboard's `WhileSubscribed`), which means no polling runs
  * while the UI is gone — there is no background work here at all.
  *
- * Failure isolation: a throwing read emits the last-known readout (or [BatteryReadout.UNKNOWN]
- * before the first success) instead of terminating the flow, so a single bad poll can never tear
- * down the combined dashboard state or strand onboarding. Reads run on [Dispatchers.IO] so the
- * sticky-broadcast query and property reads never touch the main thread.
+ * Failure isolation, bounded: a throwing read repeats the last-known readout instead of terminating
+ * the flow, so a single bad poll can never tear down the combined dashboard state or strand
+ * onboarding. But repeating it *forever* would turn a dead reader into a frozen display that still
+ * says "Charging · 82%" long after the cable came out — the surfaces that render this label it
+ * "Now". So the repeat is capped at [STALE_TOLERANCE_READS] consecutive failures, after which the
+ * flow emits [BatteryReadout.UNKNOWN] and every field honestly reads "Not reported" until a read
+ * succeeds again.
+ *
+ * Reads run on [Dispatchers.IO] so the sticky-broadcast query and property reads never touch the
+ * main thread.
  */
 @Singleton
 class BatteryReadoutSource @Inject constructor(
     private val reader: BatteryReader,
 ) {
-    fun readouts(intervalMillis: Long = DEFAULT_INTERVAL_MILLIS): Flow<BatteryReadout> = flow {
-        var last = BatteryReadout.UNKNOWN
-        while (true) {
-            last = try {
-                reader.read()
-            } catch (e: CancellationException) {
-                throw e
-            } catch (e: Exception) {
-                log(TAG, Logging.Priority.WARN) { "Battery read failed, keeping last value: ${e.message}" }
-                last
-            }
-            emit(last)
-            delay(intervalMillis)
-        }
-    }.flowOn(Dispatchers.IO)
+    fun readouts(intervalMillis: Long = DEFAULT_INTERVAL_MILLIS): Flow<BatteryReadout> =
+        batteryReadouts(intervalMillis, reader::read).flowOn(Dispatchers.IO)
 
     private companion object {
-        val TAG = logTag("Battery", "ReadoutSource")
         const val DEFAULT_INTERVAL_MILLIS = 3_000L
     }
 }
+
+/**
+ * The polling/staleness loop, extracted from the Android-bound [BatteryReadoutSource] so the failure
+ * behaviour above is JVM-testable against a scripted [read] rather than a real device.
+ */
+internal fun batteryReadouts(
+    intervalMillis: Long,
+    read: () -> BatteryReadout,
+): Flow<BatteryReadout> = flow {
+    // The last *successful* read, so a recovery always emits fresh data rather than a stale copy.
+    var lastSuccess: BatteryReadout? = null
+    var consecutiveFailures = 0
+    while (true) {
+        val fresh = try {
+            read()
+        } catch (e: CancellationException) {
+            throw e
+        } catch (e: Exception) {
+            log(TAG, Logging.Priority.WARN) { "Battery read failed (#${consecutiveFailures + 1}): ${e.message}" }
+            null
+        }
+        if (fresh != null) {
+            lastSuccess = fresh
+            consecutiveFailures = 0
+        } else {
+            consecutiveFailures++
+        }
+        emit(
+            when {
+                fresh != null -> fresh
+                // Ride out a blip on the last good reading…
+                consecutiveFailures <= STALE_TOLERANCE_READS -> lastSuccess ?: BatteryReadout.UNKNOWN
+                // …but stop asserting it once the reader is properly broken.
+                else -> BatteryReadout.UNKNOWN
+            },
+        )
+        delay(intervalMillis)
+    }
+}
+
+/**
+ * Consecutive failed reads that may still show the previous reading. At the default interval that is
+ * ~6s of tolerance — long enough to hide a transient failure, short enough that a genuinely stale
+ * value is never presented as the current state for more than a glance.
+ */
+private const val STALE_TOLERANCE_READS = 2
+
+private val TAG = logTag("Battery", "ReadoutSource")

app/src/test/java/eu/darken/amply/battery/core/BatteryReadoutSourceTest.kt

@@ -0,0 +1,84 @@
+package eu.darken.amply.battery.core
+
+import io.kotest.assertions.throwables.shouldThrow
+import io.kotest.matchers.shouldBe
+import kotlinx.coroutines.CancellationException
+import kotlinx.coroutines.flow.first
+import kotlinx.coroutines.flow.take
+import kotlinx.coroutines.flow.toList
+import kotlinx.coroutines.test.runTest
+import org.junit.jupiter.api.Test
+
+class BatteryReadoutSourceTest {
+
+    private val charging = BatteryReadout(levelPercent = 82, plugged = 1, temperatureTenthsC = 310)
+    private val later = BatteryReadout(levelPercent = 60, plugged = 0, temperatureTenthsC = 295)
+
+    /** Replays a scripted sequence of reads; `null` entries throw, standing in for a broken reader. */
+    private fun script(vararg reads: BatteryReadout?): () -> BatteryReadout {
+        var index = 0
+        return {
+            val next = reads[minOf(index, reads.lastIndex)]
+            index++
+            next ?: error("reader is broken")
+        }
+    }
+
+    private suspend fun emissions(read: () -> BatteryReadout, count: Int): List<BatteryReadout> =
+        batteryReadouts(intervalMillis = 0L, read = read).take(count).toList()
+
+    @Test
+    fun `successful reads pass straight through`() = runTest {
+        emissions(script(charging, later), 2) shouldBe listOf(charging, later)
+    }
+
+    @Test
+    fun `a blip keeps the last good reading rather than flashing empty`() = runTest {
+        val states = emissions(script(charging, null, null), 3)
+        states[1] shouldBe charging
+        states[2] shouldBe charging
+    }
+
+    @Test
+    fun `a sustained failure stops asserting a stale reading`() = runTest {
+        // Tolerance is 2 consecutive failures; the third must not still claim "Charging · 82%".
+        val states = emissions(script(charging, null, null, null, null), 5)
+        states[0] shouldBe charging
+        states[1] shouldBe charging
+        states[2] shouldBe charging
+        states[3] shouldBe BatteryReadout.UNKNOWN
+        states[4] shouldBe BatteryReadout.UNKNOWN
+    }
+
+    @Test
+    fun `a failure before any success is unknown, never invented`() = runTest {
+        emissions(script(null), 1) shouldBe listOf(BatteryReadout.UNKNOWN)
+    }
+
+    @Test
+    fun `recovery emits the fresh reading, not the pre-failure copy`() = runTest {
+        val states = emissions(script(charging, null, null, null, later), 5)
+        states[3] shouldBe BatteryReadout.UNKNOWN
+        states[4] shouldBe later
+    }
+
+    @Test
+    fun `cancellation propagates instead of being swallowed as a failed read`() = runTest {
+        // The catch-all that absorbs reader failures must not also absorb collector cancellation, or
+        // the polling loop would survive its own scope being cancelled.
+        shouldThrow<CancellationException> {
+            batteryReadouts(intervalMillis = 0L, read = { throw CancellationException("collector gone") })
+                .first()
+        }
+    }
+
+    @Test
+    fun `the failure budget resets after a success`() = runTest {
+        // fail, fail, succeed, fail, fail — the trailing pair is within budget again, so the last
+        // good reading may still be shown rather than collapsing to unknown.
+        val states = emissions(script(charging, null, null, later, null, null), 6)
+        states[3] shouldBe later
+        states[4] shouldBe later
+        states[5] shouldBe later
+    }
+}