Skip to content

Commit 457e94b

Browse files
committed
Settings: Make charge recording a one-time opt-in with a Charging history screen
The battery hub's "Record statistics" card was a toggle card that stayed on screen forever. Since the switch is flipped once and then never again, it occupied the top of the hub permanently for no further purpose. It becomes a one-time opt-in card: same title, subtitle and tone, but with a "Start recording" button and a sentence pointing at the new settings screen for turning it back off. Once recording is on the card is gone entirely and the charge section becomes the hub's first content. The durable on/off control now lives in Settings > Charging history, which also carries a retention slider. Retention is new behavior, not just a new knob. Previously charge_sessions rows were kept forever and only raw samples were purged, on a hardcoded 30-day window. Now whole entries are purged by endedAtWallMillis past a user-set window (3-14 days, default 14) with their samples cascading, while deleteSamplesOlderThan is retained: a sample's wall time being <= its session's end stamp does not put it inside the window, so a long charge that ended recently would otherwise keep its entire curve. The purge also had to be made reachable. It previously sat inside reconcileDanglingSessions() behind a maxOfOrNull{}?.let{}, so with no dangling open rows - the normal state after a clean disable - it never ran at all. It now runs in startupRepair() after reconciliation, on session seal, and immediately on a retention change via an ordered Command.Purge. The recorder's "might we have data" guard moves from the lastCaptureWallMillis timestamp to a stats.db file check. The stamp was written after the first row was committed, so a crash in that gap hid existing data from the guard forever. That also retires the preference outright, removing a DataStore write every ~20s while charging. SettingsSliderItem is new: local drag state committed only on release and only when the value actually changed, so dragging cannot write the shared DataStore per frame or enqueue a purge per frame. Its label formats the live dragged value so the number tracks the thumb. The retention flow is collected at MainActivity's composition root rather than at the settings destination. StatsViewModel is a lazy by viewModels() delegate, so collecting at the destination would start its eager sharing in the very frame the screen appears and render the placeholder default before the stored value landed.
1 parent eab807f commit 457e94b

21 files changed

Lines changed: 882 additions & 214 deletions

app/src/main/java/eu/darken/amply/common/settings/SettingsComponents.kt

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,14 @@ import androidx.compose.material.icons.filled.Palette
1212
import androidx.compose.material3.HorizontalDivider
1313
import androidx.compose.material3.Icon
1414
import androidx.compose.material3.MaterialTheme
15+
import androidx.compose.material3.Slider
1516
import androidx.compose.material3.Switch
1617
import androidx.compose.material3.Text
1718
import androidx.compose.runtime.Composable
19+
import androidx.compose.runtime.getValue
20+
import androidx.compose.runtime.mutableStateOf
21+
import androidx.compose.runtime.remember
22+
import androidx.compose.runtime.setValue
1823
import androidx.compose.ui.Alignment
1924
import androidx.compose.ui.Modifier
2025
import androidx.compose.ui.draw.alpha
@@ -23,6 +28,7 @@ import androidx.compose.ui.text.font.FontWeight
2328
import androidx.compose.ui.unit.dp
2429
import eu.darken.amply.common.compose.AmplyPreview
2530
import eu.darken.amply.common.compose.PreviewWrapper
31+
import kotlin.math.roundToInt
2632

2733
@Composable
2834
fun SettingsBaseItem(
@@ -116,6 +122,79 @@ fun SettingsSwitchItem(
116122
},
117123
)
118124

125+
/**
126+
* A discrete whole-number slider row. Deliberately not a [SettingsBaseItem]: the row itself is not a
127+
* click target — the track is — so it lays out its own title line (icon + title + [valueLabel]) and
128+
* puts the slider beneath, inset to line up with the title.
129+
*
130+
* The dragged value is local state and is committed **only on release**, and only when it actually
131+
* changed. Material's [Slider] fires `onValueChangeFinished` for a plain press-and-release on the
132+
* thumb too, so without that guard a no-op gesture would write a preference and kick off whatever
133+
* work the change triggers. [value] changing from outside re-syncs the local state.
134+
*
135+
* [valueLabel] formats the **live** dragged value, not [value] — a caller must not pre-format the
136+
* persisted number, or the label would sit frozen on the old value for the whole gesture.
137+
*/
138+
@Composable
139+
fun SettingsSliderItem(
140+
title: String,
141+
valueLabel: @Composable (Int) -> String,
142+
value: Int,
143+
range: IntRange,
144+
onValueChange: (Int) -> Unit,
145+
modifier: Modifier = Modifier,
146+
subtitle: String? = null,
147+
icon: ImageVector? = null,
148+
) {
149+
var dragged by remember(value) { mutableStateOf(value) }
150+
Column(
151+
modifier = modifier
152+
.fillMaxWidth()
153+
.padding(horizontal = 16.dp, vertical = 16.dp),
154+
) {
155+
Row(verticalAlignment = Alignment.CenterVertically) {
156+
if (icon != null) {
157+
Icon(
158+
imageVector = icon,
159+
contentDescription = null,
160+
modifier = Modifier.size(24.dp),
161+
tint = MaterialTheme.colorScheme.onSurfaceVariant,
162+
)
163+
}
164+
Column(
165+
modifier = Modifier
166+
.weight(1f)
167+
.padding(start = if (icon != null) 16.dp else 0.dp),
168+
) {
169+
Text(title, style = MaterialTheme.typography.bodyLarge, color = MaterialTheme.colorScheme.onSurface)
170+
subtitle?.let {
171+
Text(
172+
it,
173+
modifier = Modifier.padding(top = 2.dp),
174+
style = MaterialTheme.typography.bodyMedium,
175+
color = MaterialTheme.colorScheme.onSurfaceVariant,
176+
)
177+
}
178+
}
179+
Text(
180+
valueLabel(dragged),
181+
modifier = Modifier.padding(start = 8.dp),
182+
style = MaterialTheme.typography.titleMedium,
183+
color = MaterialTheme.colorScheme.primary,
184+
)
185+
}
186+
Slider(
187+
value = dragged.toFloat(),
188+
onValueChange = { dragged = it.roundToInt() },
189+
onValueChangeFinished = { if (dragged != value) onValueChange(dragged) },
190+
valueRange = range.first.toFloat()..range.last.toFloat(),
191+
// One stop per whole step, ends excluded.
192+
steps = (range.count() - 2).coerceAtLeast(0),
193+
modifier = Modifier.padding(start = if (icon != null) 40.dp else 0.dp),
194+
)
195+
}
196+
}
197+
119198
@Composable
120199
fun SettingsCategoryHeader(text: String) {
121200
Text(
@@ -161,5 +240,15 @@ private fun SettingsComponentsPreview() = PreviewWrapper {
161240
onCheckedChange = {},
162241
icon = Icons.Default.Palette,
163242
)
243+
SettingsDivider()
244+
SettingsSliderItem(
245+
title = "Keep history for",
246+
valueLabel = { days -> "$days days" },
247+
value = 7,
248+
range = 3..14,
249+
onValueChange = {},
250+
subtitle = "Charges older than this are deleted automatically.",
251+
icon = Icons.Default.Palette,
252+
)
164253
}
165254
}

app/src/main/java/eu/darken/amply/main/ui/MainActivity.kt

Lines changed: 44 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ import eu.darken.amply.main.ui.dashboard.DashboardViewModel
5555
import eu.darken.amply.main.ui.dashboard.shouldMonitorAccess
5656
import eu.darken.amply.main.ui.onboarding.OnboardingScreen
5757
import eu.darken.amply.main.ui.settings.AcknowledgementsScreen
58+
import eu.darken.amply.main.ui.settings.ChargingHistorySettingsScreen
5859
import eu.darken.amply.main.ui.settings.GeneralSettingsScreen
5960
import eu.darken.amply.main.ui.settings.ReconnectGestureSettingsScreen
6061
import eu.darken.amply.main.ui.settings.SettingsDestination
@@ -96,6 +97,15 @@ class MainActivity : ComponentActivity() {
9697
enableEdgeToEdge()
9798
setContent {
9899
val themeState by settingsViewModel.themeState.collectAsState()
100+
// Collected here at the composition root, NOT down in the CHARGING_HISTORY_SETTINGS branch
101+
// that consumes it. `statsViewModel` is a lazy `by viewModels()` delegate, so collecting at
102+
// the destination would construct the ViewModel — and thus start its "eager" sharing — in
103+
// the very frame the screen appears, rendering the placeholder default before the stored
104+
// value lands (a drag started in that window would also be reset, since the slider re-keys
105+
// its local state on the incoming value). Collecting at the root makes it an already-resolved
106+
// source by the time the user is two levels deep in Settings, the same way `captureEnabled`
107+
// uses the resolved dashboard state instead of its own first emission. Don't move it down.
108+
val retentionDays by statsViewModel.retentionDays.collectAsState()
99109
AmplyTheme(themeState) {
100110
Surface(
101111
modifier = Modifier.fillMaxSize(),
@@ -285,6 +295,10 @@ class MainActivity : ComponentActivity() {
285295
SettingsDestination.SETTINGS -> SettingsScreen(
286296
onBack = { destination = SettingsDestination.DASHBOARD },
287297
onGeneral = { destination = SettingsDestination.GENERAL },
298+
captureEnabled = state.stats.enabled,
299+
onChargingHistory = {
300+
destination = SettingsDestination.CHARGING_HISTORY_SETTINGS
301+
},
288302
// Offered whenever this device is one we want contribution data for (unsupported/lab),
289303
// regardless of whether Shizuku is installed yet — the wizard nudges the install.
290304
showDiagnostics = state.charging.contributionWanted,
@@ -348,33 +362,36 @@ class MainActivity : ComponentActivity() {
348362
onBack = { destination = SettingsDestination.DASHBOARD },
349363
onAnyLevelChange = viewModel::setQuickFullChargeAnyLevel,
350364
)
351-
// The hub reads the battery straight from the dashboard state (already
352-
// collected) and takes only the capture switch from the stats ViewModel —
353-
// deliberately not its history flow, which is what would open stats.db.
354-
SettingsDestination.BATTERY -> {
355-
val capture by statsViewModel.captureState.collectAsState()
356-
BatteryHubScreen(
357-
readout = state.batteryReadout,
358-
// The switch reads from the already-resolved dashboard state, not from
359-
// captureState: that flow is only subscribed when this screen opens, so
360-
// its first emission is the `false` default and the toggle would render
361-
// off for a frame beside a teaser saying a charge is being recorded.
362-
// Same preference, one resolved source, no disagreement on screen.
363-
captureEnabled = state.stats.enabled,
364-
lastCaptureWallMillis = capture.lastCaptureWallMillis,
365-
teaser = ChargeTeaserState.from(state.stats, state.batteryReadout),
366-
onBack = { destination = SettingsDestination.DASHBOARD },
367-
onOpenHistory = { destination = SettingsDestination.CHARGE_HISTORY },
368-
onCaptureEnabledChange = { enabled ->
369-
if (enabled) {
370-
runWithNotifications(NotificationAction.ENABLE_STATS)
371-
} else {
372-
statsViewModel.setCaptureEnabled(false)
373-
}
374-
},
375-
onOpenSession = { id -> openSession(id, SettingsDestination.BATTERY) },
376-
)
377-
}
365+
SettingsDestination.CHARGING_HISTORY_SETTINGS -> ChargingHistorySettingsScreen(
366+
// Both values come from already-resolved sources (the dashboard state and
367+
// the root-collected retention flow), so neither the switch nor the slider
368+
// can render a placeholder for a frame while a first emission lands.
369+
captureEnabled = state.stats.enabled,
370+
retentionDays = retentionDays,
371+
onBack = { destination = SettingsDestination.SETTINGS },
372+
onCaptureEnabledChange = { enabled ->
373+
if (enabled) {
374+
runWithNotifications(NotificationAction.ENABLE_STATS)
375+
} else {
376+
statsViewModel.setCaptureEnabled(false)
377+
}
378+
},
379+
onRetentionChange = statsViewModel::setRetentionDays,
380+
)
381+
// The hub reads the battery and the capture flag straight from the dashboard
382+
// state (already collected and resolved, so the opt-in card can't flash on for
383+
// a frame beside a teaser saying a charge is being recorded) — deliberately not
384+
// the stats ViewModel's history flow, which is what would open stats.db.
385+
SettingsDestination.BATTERY -> BatteryHubScreen(
386+
readout = state.batteryReadout,
387+
captureEnabled = state.stats.enabled,
388+
teaser = ChargeTeaserState.from(state.stats, state.batteryReadout),
389+
onBack = { destination = SettingsDestination.DASHBOARD },
390+
onOpenHistory = { destination = SettingsDestination.CHARGE_HISTORY },
391+
// Enable-only: turning recording back off lives in Settings › Charging history.
392+
onEnableCapture = { runWithNotifications(NotificationAction.ENABLE_STATS) },
393+
onOpenSession = { id -> openSession(id, SettingsDestination.BATTERY) },
394+
)
378395
// The Room-backed session list is collected only here, so the stats DB isn't
379396
// opened just by visiting the hub — a user who never enables statistics never
380397
// creates stats.db by looking at their battery.

app/src/main/java/eu/darken/amply/main/ui/battery/BatteryHubScreen.kt

Lines changed: 20 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,10 @@ import eu.darken.amply.common.compose.AmplyPreview
4040
import eu.darken.amply.common.compose.PreviewWrapper
4141

4242
/**
43-
* The single battery/charging destination: the capture switch, the current or last charge, and the
44-
* full live readout. Replaces the split between a read-only battery-detail screen and a statistics
45-
* screen that each rendered level, current, and temperature through different code.
43+
* The single battery/charging destination: the current or last charge and the full live readout, led
44+
* by the recording opt-in until it has been accepted. Replaces the split between a read-only
45+
* battery-detail screen and a statistics screen that each rendered level, current, and temperature
46+
* through different code.
4647
*
4748
* State-hoisted and previewable — it renders straight from a [BatteryReadout] plus a
4849
* [ChargeTeaserState], so it needs no ViewModel of its own. Fields the platform doesn't report render
@@ -53,11 +54,10 @@ import eu.darken.amply.common.compose.PreviewWrapper
5354
fun BatteryHubScreen(
5455
readout: BatteryReadout?,
5556
captureEnabled: Boolean,
56-
lastCaptureWallMillis: Long?,
5757
teaser: ChargeTeaserState,
5858
onBack: () -> Unit,
5959
onOpenHistory: () -> Unit,
60-
onCaptureEnabledChange: (Boolean) -> Unit,
60+
onEnableCapture: () -> Unit,
6161
onOpenSession: (Long) -> Unit,
6262
) {
6363
Scaffold(
@@ -91,16 +91,15 @@ fun BatteryHubScreen(
9191
contentPadding = PaddingValues(16.dp),
9292
verticalArrangement = Arrangement.spacedBy(12.dp),
9393
) {
94-
item {
95-
CaptureToggleCard(
96-
enabled = captureEnabled,
97-
lastCaptureWallMillis = lastCaptureWallMillis,
98-
onCaptureEnabledChange = onCaptureEnabledChange,
99-
)
94+
// Gated on the authoritative preference, not on the teaser: the opt-in is a one-time
95+
// prompt, so it disappears for good the moment recording is on.
96+
if (!captureEnabled) {
97+
item { CaptureOptInCard(onEnable = onEnableCapture) }
10098
}
10199
// The charge section sits above the raw readout: "how is this charge going" is the question
102-
// that brings most visits here, and the readout below is reference material. With capture
103-
// off there is no section at all — the toggle immediately above already explains why.
100+
// that brings most visits here, and the readout below is reference material. With recording
101+
// on it is the first thing on the screen; with recording off there is no section at all —
102+
// the opt-in card immediately above already explains why.
104103
if (teaser != ChargeTeaserState.CaptureOff) {
105104
item { SectionHeader(stringResource(teaser.sectionTitle())) }
106105
item {
@@ -257,14 +256,14 @@ private val previewCharging = BatteryReadout(
257256
@AmplyPreview
258257
@Composable
259258
private fun BatteryHubScreenLivePreview() = PreviewWrapper {
259+
// Recording on: no opt-in card, so the live charge is the first thing on the screen.
260260
BatteryHubScreen(
261261
readout = previewCharging,
262262
captureEnabled = true,
263-
lastCaptureWallMillis = 0L,
264263
teaser = ChargeTeaserState.Live(previewLiveSession),
265264
onBack = {},
266265
onOpenHistory = {},
267-
onCaptureEnabledChange = {},
266+
onEnableCapture = {},
268267
onOpenSession = {},
269268
)
270269
}
@@ -278,28 +277,26 @@ private fun BatteryHubScreenLastPreview() = PreviewWrapper {
278277
plugged = 0,
279278
),
280279
captureEnabled = true,
281-
lastCaptureWallMillis = 0L,
282280
teaser = ChargeTeaserState.Last(previewLastSession),
283281
onBack = {},
284282
onOpenHistory = {},
285-
onCaptureEnabledChange = {},
283+
onEnableCapture = {},
286284
onOpenSession = {},
287285
)
288286
}
289287

290288
@AmplyPreview
291289
@Composable
292290
private fun BatteryHubScreenCaptureOffPreview() = PreviewWrapper {
293-
// Capture off: no charge section at all, but the full readout is still here — it never depended
294-
// on the opt-in.
291+
// Never opted in: the opt-in card leads, there is no charge section, and the full readout is still
292+
// here — it never depended on recording.
295293
BatteryHubScreen(
296294
readout = previewCharging,
297295
captureEnabled = false,
298-
lastCaptureWallMillis = null,
299296
teaser = ChargeTeaserState.CaptureOff,
300297
onBack = {},
301298
onOpenHistory = {},
302-
onCaptureEnabledChange = {},
299+
onEnableCapture = {},
303300
onOpenSession = {},
304301
)
305302
}
@@ -319,11 +316,10 @@ private fun BatteryHubScreenSparsePreview() = PreviewWrapper {
319316
voltageMillivolts = 3900,
320317
),
321318
captureEnabled = true,
322-
lastCaptureWallMillis = null,
323319
teaser = ChargeTeaserState.None,
324320
onBack = {},
325321
onOpenHistory = {},
326-
onCaptureEnabledChange = {},
322+
onEnableCapture = {},
327323
onOpenSession = {},
328324
)
329325
}
@@ -336,11 +332,10 @@ private fun BatteryHubScreenLargeFontPreview() = PreviewWrapper {
336332
BatteryHubScreen(
337333
readout = previewCharging.copy(technology = "Li-ion polymer (high voltage)"),
338334
captureEnabled = true,
339-
lastCaptureWallMillis = 0L,
340335
teaser = ChargeTeaserState.Last(previewLastSession),
341336
onBack = {},
342337
onOpenHistory = {},
343-
onCaptureEnabledChange = {},
338+
onEnableCapture = {},
344339
onOpenSession = {},
345340
)
346341
}

0 commit comments

Comments
 (0)