Skip to content

Commit fc5d610

Browse files
committed
Stats: Resume the open charge session across a process restart
A process death while plugged in restarted the live card's "Since …" at process-launch time: startup repair sealed every open row unconditionally, so the next tick opened a fresh session stamped with the current time and history gained a duplicate row for one physical charge. Android exposes no plug-in timestamp, but the row was already persisted. Startup repair now probes current battery state (via the existing BatteryReader sticky read) and reattaches the newest open row when the evidence is consistent with the same plug event, else seals as before. Continuity is inferred, not observed, so the guards are best-effort and biased toward merging: a replug at an unchanged level during the gap is indistinguishable from an uninterrupted plug. Two things keep that honest — a resumed row is always flagged partial, and its last power/temperature readings are dropped so the unobserved gap is never credited to the aggregates. A wrong merge costs an over-long duration, never invented averages. The probe runs synchronously in startup repair rather than on the first watcher tick: a row held open awaiting a tick renders as a live session and suppresses the "couldn't start capture" retry when the foreground service fails to start. Also fixed, all reachable before this change: - BootIdSource's -1 sentinel compares equal across two different boots, so an unknown boot id is now explicitly disqualifying rather than merely degraded — otherwise a resume could splice two boots' elapsed-realtime readings into one bogus duration. - ChargeStatsRepository.currentSession() keyed distinctUntilChangedBy on the row id alone while capturing the row inside flatMapLatest, so a partial flip on the same row was suppressed for the lifetime of the subscription. - StatsCardPresentation preferred Live whenever a row existed. A resumed row stays open by design, so a failed service start would show a frozen live card and hide the retry. A failed start now wins. - The lastCapture stamp is written after the session row is committed, so a death in between left an open row that startup repair skipped entirely, stranding it and letting the next tick open a second row. ChargeStatsRecorder gains an injected dispatcher (@StatsDispatcher) so the new Robolectric test can drive its command loop. The stats test classes now use per-test temp-file DataStores instead of sharing the app's real path. Verified on a Pixel 8 (resume across force-stop, unplugged reject, level-drop reject, and BOOT_MISMATCH plus REBOOT seal across a real reboot).
1 parent 01bafb0 commit fc5d610

12 files changed

Lines changed: 715 additions & 38 deletions

app/src/main/java/eu/darken/amply/stats/core/BootIdSource.kt

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,14 @@ import javax.inject.Singleton
99
/**
1010
* Reads the device boot count, used to tell a same-boot process restart (resume the open session)
1111
* from a reboot (seal it — elapsed-time continuity can't survive a power-off). The value can only
12-
* change across a reboot, which restarts this process, so it is read once and cached. A device that
13-
* doesn't report [Settings.Global.BOOT_COUNT] yields a constant sentinel; reboots then look like
14-
* same-boot restarts, which degrades to a partial/interrupted seal rather than anything unsafe.
12+
* change across a reboot, which restarts this process, so it is read once and cached.
13+
*
14+
* A device that doesn't report [Settings.Global.BOOT_COUNT] yields the [UNAVAILABLE] sentinel, which
15+
* two different boots would compare *equal* on. So the sentinel is not merely a degraded id — it is
16+
* explicitly disqualifying: [StatsSessionEngine.evaluateResume] refuses to resume an open session
17+
* when either side is [UNAVAILABLE], because a resume across an unnoticed reboot would splice two
18+
* boots' [android.os.SystemClock.elapsedRealtime] readings into one bogus duration. Such a device
19+
* always falls back to sealing, which is lossy but never wrong.
1520
*/
1621
@Singleton
1722
class BootIdSource @Inject constructor(
@@ -22,10 +27,11 @@ class BootIdSource @Inject constructor(
2227
fun current(): Long = cached
2328

2429
private fun read(): Long = runCatching {
25-
Settings.Global.getInt(context.contentResolver, Settings.Global.BOOT_COUNT, UNAVAILABLE).toLong()
26-
}.getOrDefault(UNAVAILABLE.toLong())
30+
Settings.Global.getInt(context.contentResolver, Settings.Global.BOOT_COUNT, UNAVAILABLE.toInt()).toLong()
31+
}.getOrDefault(UNAVAILABLE)
2732

28-
private companion object {
29-
const val UNAVAILABLE = -1
33+
companion object {
34+
/** Boot identity could not be determined — never usable to prove two observations share a boot. */
35+
const val UNAVAILABLE = -1L
3036
}
3137
}

app/src/main/java/eu/darken/amply/stats/core/ChargeStatsRecorder.kt

Lines changed: 69 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package eu.darken.amply.stats.core
22

33
import android.os.BatteryManager
4+
import android.os.SystemClock
45
import androidx.room.withTransaction
56
import dagger.Lazy
67
import eu.darken.amply.battery.core.BatteryReadout
@@ -10,10 +11,11 @@ import eu.darken.amply.common.debug.logging.log
1011
import eu.darken.amply.common.debug.logging.logTag
1112
import eu.darken.amply.stats.core.db.BatterySampleEntity
1213
import eu.darken.amply.stats.core.db.ChargeSessionEntity
14+
import eu.darken.amply.stats.core.db.StatsDao
1315
import eu.darken.amply.stats.core.db.StatsDatabase
1416
import kotlinx.coroutines.CancellationException
17+
import kotlinx.coroutines.CoroutineDispatcher
1518
import kotlinx.coroutines.CoroutineScope
16-
import kotlinx.coroutines.Dispatchers
1719
import kotlinx.coroutines.SupervisorJob
1820
import kotlinx.coroutines.channels.Channel
1921
import kotlinx.coroutines.flow.first
@@ -41,8 +43,9 @@ class ChargeStatsRecorder @Inject constructor(
4143
private val preferences: StatsPreferences,
4244
private val bootIdSource: BootIdSource,
4345
private val batteryReader: BatteryReader,
46+
@StatsDispatcher dispatcher: CoroutineDispatcher,
4447
) {
45-
private val scope = CoroutineScope(SupervisorJob() + Dispatchers.IO)
48+
private val scope = CoroutineScope(SupervisorJob() + dispatcher)
4649
private val commands = Channel<Command>(Channel.UNLIMITED)
4750

4851
// Mutated only by the single consumer coroutine below — no locking needed.
@@ -54,8 +57,8 @@ class ChargeStatsRecorder @Inject constructor(
5457

5558
init {
5659
scope.launch {
57-
// Runs before any command: seals sessions left open by an unclean shutdown, and seeds the
58-
// capturing flag from the durable preference.
60+
// Runs before any command: reconciles sessions left open by an unclean shutdown, and seeds
61+
// the capturing flag from the durable preference.
5962
startupRepair()
6063
for (command in commands) {
6164
try {
@@ -92,10 +95,12 @@ class ChargeStatsRecorder @Inject constructor(
9295
try {
9396
capturing = preferences.isCaptureEnabledNow()
9497
// Only touch the DB when we might have data — avoids creating an empty stats.db for users
95-
// who never enabled statistics. A dangling open row can only exist after prior recording,
96-
// which always stamps lastCapture.
97-
if (preferences.lastCaptureWallMillis.first() == null) return
98-
sealDanglingSessions()
98+
// who never enabled statistics. The lastCapture stamp alone is not a sound existence test:
99+
// it is written after the row is committed, so a process death in between leaves an open
100+
// row with no stamp. Capture being enabled is therefore also sufficient — such a user gets
101+
// a stats.db on the next tick anyway, so opening it here costs nothing.
102+
if (!capturing && preferences.lastCaptureWallMillis.first() == null) return
103+
reconcileDanglingSessions()
99104
} catch (e: CancellationException) {
100105
throw e
101106
} catch (e: Exception) {
@@ -243,24 +248,70 @@ class ChargeStatsRecorder @Inject constructor(
243248
}
244249

245250
/**
246-
* Seal every session left open by an unclean shutdown (process kill / reboot). Never resumes:
247-
* losing the monitor mid-charge breaks curve/elapsed continuity, so the session is closed
248-
* honestly and the next plug opens a fresh one. This also sidesteps having to trust the boot
249-
* count — a resumed session would risk a negative post-reboot duration.
251+
* Reconcile sessions left open by an unclean shutdown (process kill / reboot). The newest row is
252+
* offered to [StatsSessionEngine.evaluateResume] against the battery state observed right now; if
253+
* the same plug event is still underway it is reattached, keeping the true plug-in time and one
254+
* history row per physical charge. Everything else is sealed as before.
255+
*
256+
* The probe happens here rather than on the first watcher tick on purpose. A held-open row would
257+
* be rendered as a live session by the dashboard for as long as no tick arrives — which also
258+
* suppresses the "couldn't start capture" retry when the foreground service fails to start — and
259+
* would need leak guards on the disable/clear paths. Resolving synchronously costs one sticky
260+
* broadcast read and leaves [onSample] untouched.
250261
*/
251-
private suspend fun sealDanglingSessions() {
262+
private suspend fun reconcileDanglingSessions() {
252263
val dao = database.get().statsDao()
253264
val open = dao.openSessions()
254265
if (open.isEmpty()) return
255266
val currentBoot = bootIdSource.current()
256-
open.forEach { row ->
257-
val sealed = sealFromLastKnown(row, currentBoot)
258-
// Drop a dangling row that never captured anything (same rule as a live seal).
259-
if (StatsSessionEngine.isDiscardable(sealed)) dao.deleteSession(sealed.id) else dao.updateSession(sealed)
260-
}
267+
// Capture is off, so no tick is coming to advance a resumed row — seal everything, as before.
268+
// Ordered by id (monotonic), matching the row `openSessionFlow` shows as live; `openSessions`
269+
// orders by elapsed-realtime, which disagrees across a reboot.
270+
val candidate = if (capturing) open.maxByOrNull { it.id } else null
271+
val resumed = candidate?.let { resumeOrNull(it, currentBoot) }
272+
open.filter { it.id != resumed?.id }.forEach { row -> sealDangling(dao, row, currentBoot) }
261273
open.maxOfOrNull { it.runningLastWallMillis ?: it.startedAtWallMillis }?.let { purgeOldSamples(it) }
262274
}
263275

276+
/**
277+
* Reattach [row] if the charge it recorded is still running, else null. In-memory state is
278+
* installed only *after* the durable write returns: command failures are logged and swallowed
279+
* (see the loop above), and a remembered-but-unwritten session would let a later tick open a
280+
* second row against a row the database still has open.
281+
*/
282+
private suspend fun resumeOrNull(row: ChargeSessionEntity, currentBoot: Long): ChargeSessionEntity? {
283+
val readout = batteryReader.read()
284+
val probe = ResumeProbe(
285+
elapsedRealtimeMillis = SystemClock.elapsedRealtime(),
286+
bootId = currentBoot,
287+
plugged = (readout.plugged ?: 0) != 0,
288+
percent = readout.levelPercent,
289+
)
290+
return when (val decision = StatsSessionEngine.evaluateResume(row, probe)) {
291+
is ResumeDecision.Reject -> {
292+
log(TAG) { "Not resuming session ${row.id}: ${decision.reason}" }
293+
null
294+
}
295+
296+
is ResumeDecision.Resume -> {
297+
database.get().statsDao().updateSession(decision.session)
298+
openSession = decision.session
299+
// Continue the existing cadence instead of restarting it, so the resumed session
300+
// doesn't force an extra curve point on top of the one it already has.
301+
lastRecordedElapsed = decision.session.runningLastElapsedRealtimeMillis
302+
lastRecordedPercent = decision.session.runningLastPercent
303+
log(TAG, Logging.Priority.INFO) { "Resumed charge session ${decision.session.id}" }
304+
decision.session
305+
}
306+
}
307+
}
308+
309+
private suspend fun sealDangling(dao: StatsDao, row: ChargeSessionEntity, currentBoot: Long) {
310+
val sealed = sealFromLastKnown(row, currentBoot)
311+
// Drop a dangling row that never captured anything (same rule as a live seal).
312+
if (StatsSessionEngine.isDiscardable(sealed)) dao.deleteSession(sealed.id) else dao.updateSession(sealed)
313+
}
314+
264315
private fun sealFromLastKnown(row: ChargeSessionEntity, currentBoot: Long): ChargeSessionEntity {
265316
val reason = if (row.bootId != currentBoot) StatsSealReason.REBOOT else StatsSealReason.INTERRUPTED
266317
return StatsSessionEngine.seal(

app/src/main/java/eu/darken/amply/stats/core/ChargeStatsRepository.kt

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,17 @@ class ChargeStatsRepository @Inject constructor(
3535
* The in-progress charge session of the current boot, or null when nothing is open, as a live flow
3636
* for the dashboard card. The curve is a bounded recent window (decimated) so a session that stays
3737
* open for days at an OEM charge limit never triggers an unbounded reload on every appended sample.
38-
* `distinctUntilChangedBy(id)` keeps the inner sample flow subscribed across the per-tick session-row
39-
* updates (the open row's start fields are immutable, so only a change of session identity matters),
40-
* avoiding a redundant curve re-query on every fold.
38+
* `distinctUntilChangedBy` keeps the inner sample flow subscribed across the per-tick session-row
39+
* updates, avoiding a redundant curve re-query on every fold. It keys on identity **plus every row
40+
* field this flow actually renders**: `partial` flips when a session is resumed after a process
41+
* restart, and since the row is captured inside `flatMapLatest`, an id-only key would pin the stale
42+
* copy for the lifetime of the subscription. The remaining projected fields (the start columns) are
43+
* immutable once the row exists.
4144
*/
4245
@OptIn(ExperimentalCoroutinesApi::class)
4346
fun currentSession(): Flow<StatsLiveSession?> =
4447
database.get().statsDao().openSessionFlow(bootIdSource.current())
45-
.distinctUntilChangedBy { it?.id }
48+
.distinctUntilChangedBy { row -> row?.let { it.id to it.partial } }
4649
.flatMapLatest { row ->
4750
if (row == null) {
4851
flowOf(null)

app/src/main/java/eu/darken/amply/stats/core/StatsModule.kt

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,14 @@ package eu.darken.amply.stats.core
22

33
import dagger.Binds
44
import dagger.Module
5+
import dagger.Provides
56
import dagger.hilt.InstallIn
67
import dagger.hilt.components.SingletonComponent
78
import dagger.multibindings.IntoSet
89
import eu.darken.amply.monitor.core.ChargeMonitorWatcher
10+
import kotlinx.coroutines.CoroutineDispatcher
11+
import kotlinx.coroutines.Dispatchers
12+
import javax.inject.Qualifier
913

1014
/**
1115
* Contributes [ChargeStatsWatcher] into the shared [Set] of [ChargeMonitorWatcher]s the
@@ -18,4 +22,20 @@ abstract class StatsModule {
1822
@Binds
1923
@IntoSet
2024
abstract fun bindStatsWatcher(impl: ChargeStatsWatcher): ChargeMonitorWatcher
25+
26+
companion object {
27+
@Provides
28+
@StatsDispatcher
29+
fun statsDispatcher(): CoroutineDispatcher = Dispatchers.IO
30+
}
2131
}
32+
33+
/**
34+
* The dispatcher [ChargeStatsRecorder] runs its serialized command loop on. Injected rather than
35+
* hardcoded purely so tests can substitute a deterministic scheduler; production is always
36+
* [Dispatchers.IO]. Qualified (and scoped to the stats feature) so this doesn't become an
37+
* unqualified app-wide `CoroutineDispatcher` binding.
38+
*/
39+
@Qualifier
40+
@Retention(AnnotationRetention.BINARY)
41+
annotation class StatsDispatcher

app/src/main/java/eu/darken/amply/stats/core/StatsSealReason.kt

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,12 @@ enum class StatsSealReason {
99
/** Charger removed — the normal, complete end of a session. */
1010
UNPLUGGED,
1111

12-
/** Reopened an open session after a process restart within the same boot; curve has a gap. */
12+
/**
13+
* Found open after a process restart and *not* resumable — the charge it recorded could not be
14+
* shown to still be running (unplugged since, level dropped, unknown boot). A restart that is
15+
* consistent with the same plug event reattaches the row instead of sealing it, so this reason
16+
* means the evidence failed, not merely that the process died.
17+
*/
1318
INTERRUPTED,
1419

1520
/** Open session found after a reboot; elapsed-time continuity across power-off can't be trusted. */

app/src/main/java/eu/darken/amply/stats/core/StatsSessionEngine.kt

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,49 @@ sealed interface StatsTransition {
1818
data object Ignore : StatsTransition
1919
}
2020

21+
/**
22+
* Current battery state observed at process start, used to reconcile a session left open by a
23+
* process death. Deliberately *not* a [StatsSample]: that type's contract is "built from the exact
24+
* intent the charge-session service evaluated", while this comes from an independent sticky read
25+
* before any service tick exists.
26+
*/
27+
data class ResumeProbe(
28+
val elapsedRealtimeMillis: Long,
29+
val bootId: Long,
30+
val plugged: Boolean,
31+
val percent: Int?,
32+
)
33+
34+
/** Outcome of reconciling a dangling open session against a [ResumeProbe]. */
35+
sealed interface ResumeDecision {
36+
37+
/** Reattach [session] — the same plug event is still in progress, as far as can be told. */
38+
data class Resume(val session: ChargeSessionEntity) : ResumeDecision
39+
40+
/** Seal the row instead; [reason] is for diagnostics only. */
41+
data class Reject(val reason: Reason) : ResumeDecision
42+
43+
enum class Reason {
44+
/** Row is already sealed — nothing to reconcile. */
45+
CLOSED,
46+
47+
/** No external power now, so whatever charge was underway has ended. */
48+
UNPLUGGED,
49+
50+
/** Boot identity is unknown on either side, so a same-boot claim can't be made. */
51+
BOOT_UNKNOWN,
52+
53+
/** A reboot happened; elapsed-realtime readings from two boots can't be compared. */
54+
BOOT_MISMATCH,
55+
56+
/** Probe predates the row's last sample — the clock base can't be the one we recorded against. */
57+
TIME_WENT_BACKWARDS,
58+
59+
/** Charge level fell while we weren't looking, so the device discharged in the gap. */
60+
LEVEL_DROPPED,
61+
}
62+
}
63+
2164
/**
2265
* Pure charge-session segmentation and online aggregation. Holds no state itself: the open session
2366
* is a [ChargeSessionEntity] the recorder loads from (and persists to) Room, so every decision
@@ -69,6 +112,51 @@ object StatsSessionEngine {
69112
return zeroDuration && noLevelGain && sealed.runningSampleCount <= 1
70113
}
71114

115+
/**
116+
* Reconcile a session left open by a process death against the battery state observed at the next
117+
* process start. Resuming keeps the real plug-in time and one history row for one physical charge;
118+
* the alternative (always sealing) restarts the session at process-launch time, which reads as a
119+
* wrong "Since …" and splits one charge into two rows.
120+
*
121+
* Continuity here is **inferred, not observed** — nothing survived the gap to witness it. A replug
122+
* at an equal-or-higher level is indistinguishable from an uninterrupted plug, and a level *drop*
123+
* while plugged is possible (heavy load, weak charger, thermal throttling, an OEM hold). So these
124+
* guards are best-effort and deliberately biased toward merging: an occasional merged plug cycle
125+
* beats fragmenting real sessions. Two things keep that bias honest, both applied on [Resume]:
126+
* the row is flagged [ChargeSessionEntity.partial], and the last power/temperature readings are
127+
* dropped so [fold] credits **nothing** for the unobserved gap (see the null handling in
128+
* [creditInterval]). A wrong merge therefore costs an over-long duration — never invented
129+
* power/temperature averages.
130+
*/
131+
fun evaluateResume(row: ChargeSessionEntity, probe: ResumeProbe): ResumeDecision {
132+
val lastObserved = row.runningLastElapsedRealtimeMillis ?: row.startedElapsedRealtimeMillis
133+
val reason = when {
134+
row.endedAtWallMillis != null -> ResumeDecision.Reason.CLOSED
135+
!probe.plugged -> ResumeDecision.Reason.UNPLUGGED
136+
// Checked before equality: the sentinel compares equal to itself across *different* boots.
137+
probe.bootId == BootIdSource.UNAVAILABLE ||
138+
row.bootId == BootIdSource.UNAVAILABLE -> ResumeDecision.Reason.BOOT_UNKNOWN
139+
probe.bootId != row.bootId -> ResumeDecision.Reason.BOOT_MISMATCH
140+
probe.elapsedRealtimeMillis < lastObserved -> ResumeDecision.Reason.TIME_WENT_BACKWARDS
141+
droppedLevel(row.runningLastPercent, probe.percent) -> ResumeDecision.Reason.LEVEL_DROPPED
142+
else -> null
143+
}
144+
if (reason != null) return ResumeDecision.Reject(reason)
145+
return ResumeDecision.Resume(
146+
row.copy(
147+
// The curve has a hole and the continuity is inferred — never present this as a clean
148+
// plug→unplug history.
149+
partial = true,
150+
runningLastPowerMilliwatts = null,
151+
runningLastTemperatureTenthsC = null,
152+
),
153+
)
154+
}
155+
156+
/** True only when both readings exist and the level fell — a missing reading is not evidence. */
157+
private fun droppedLevel(lastPercent: Int?, probePercent: Int?): Boolean =
158+
lastPercent != null && probePercent != null && probePercent < lastPercent
159+
72160
fun open(sample: StatsSample, partial: Boolean): ChargeSessionEntity = ChargeSessionEntity(
73161
startedAtWallMillis = sample.wallMillis,
74162
startedElapsedRealtimeMillis = sample.elapsedRealtimeMillis,

app/src/main/java/eu/darken/amply/stats/core/db/ChargeSessionEntity.kt

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,10 @@ data class ChargeSessionEntity(
4040

4141
/**
4242
* True when the session does not represent a clean plug→unplug: capture was enabled mid-charge,
43-
* started already at 100%, or the session was sealed by process recovery / reboot. The UI labels
44-
* these as partial rather than presenting them as complete histories.
43+
* started already at 100%, the session was sealed by process recovery / reboot, or it was resumed
44+
* after a process restart (start time and totals are real, but the curve has a gap and continuity
45+
* across that gap is inferred rather than observed). The UI labels these as partial rather than
46+
* presenting them as complete histories.
4547
*/
4648
val partial: Boolean = false,
4749

0 commit comments

Comments
 (0)