Skip to content

Commit aaf6d00

Browse files
eriedclaude
andcommitted
fix(metrics): min/max/avg pills work for every metric, not just the legacy 6
The dashboard pills resolved stats from a hardcoded when(key) covering only battery/temp/voltage/current/load/speed, and the dashboard MetricHistory dropped the generic extras buffer - so Max Power, Max GPS Speed, AVG etc. showed "--" even when the data existed. Now the pills fall back to the extras buffer and format it with the same unit conversion as the detail screen, and the sampler allocates a buffer for every supportsStats catalog key. Location/phone-derived metrics (GPS_SPEED, GPS_ALTITUDE, GPS_ACCURACY, PHONE_BATTERY, EXTERNAL_GPS_ BATTERY) are sampled each tick from their sources (Lazy<TripRepository> avoids the DI cycle). Drift-guard test asserts every supportsStats key has a buffer. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GADLyChheAoMX9dQbRgRnH
1 parent fb0b3c8 commit aaf6d00

4 files changed

Lines changed: 221 additions & 24 deletions

File tree

app/src/main/java/com/eried/eucplanet/data/repository/WheelRepository.kt

Lines changed: 80 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,41 @@ internal val EXTRA_HISTORY_METRICS: List<Pair<String, (com.eried.eucplanet.data.
135135
"TIRE_PRESSURE" to { it.tirePressureKpa }
136136
)
137137

138+
/**
139+
* The six metrics with their own typed buffer on [FullMetricHistory]. Every
140+
* OTHER supportsStats catalog metric rides in the keyed extras map instead.
141+
*/
142+
internal val LEGACY_STAT_METRIC_KEYS: List<String> =
143+
listOf("BATTERY", "TEMPERATURE", "VOLTAGE", "CURRENT", "LOAD", "SPEED")
144+
145+
/**
146+
* Stat metrics that are NOT on WheelData: they come from the phone location,
147+
* the phone battery, or the paired external GPS box. Sampled from those
148+
* injected sources on the same 1 Hz history tick as the wheel metrics.
149+
* Stored raw so [computeDashboardStatValue] and the tile formatter agree:
150+
* GPS_SPEED in km/h, altitude / accuracy in metres, batteries in percent.
151+
*/
152+
internal val SOURCE_HISTORY_METRIC_KEYS: List<String> =
153+
listOf("GPS_SPEED", "GPS_ALTITUDE", "GPS_ACCURACY", "PHONE_BATTERY", "EXTERNAL_GPS_BATTERY")
154+
155+
/**
156+
* Every catalog key with supportsStats = true that isn't a legacy typed
157+
* buffer gets an extras buffer, so min / max / avg / last resolve for it on
158+
* the dashboard. Wheel-sourced keys fill from [EXTRA_HISTORY_METRICS], the
159+
* phone / GPS keys from [SOURCE_HISTORY_METRIC_KEYS]. Any remaining stat
160+
* metric with no live source yet still gets an (empty) buffer, so its pill
161+
* shows a stable placeholder instead of silently having no stat path. Driven
162+
* off the catalog so a new supportsStats metric can't regress unnoticed.
163+
*/
164+
internal val STAT_BUFFER_EXTRA_KEYS: List<String> =
165+
(EXTRA_HISTORY_METRICS.map { it.first } +
166+
SOURCE_HISTORY_METRIC_KEYS +
167+
com.eried.eucplanet.data.model.MetricCatalog.all
168+
.filter { it.supportsStats }
169+
.map { it.key })
170+
.distinct()
171+
.filter { it !in LEGACY_STAT_METRIC_KEYS }
172+
138173
@Singleton
139174
class WheelRepository @Inject constructor(
140175
@ApplicationContext private val context: Context,
@@ -148,6 +183,11 @@ class WheelRepository @Inject constructor(
148183
private val cheatState: com.eried.eucplanet.cheats.CheatState,
149184
private val phoneSensorRepository: PhoneSensorRepository,
150185
private val externalGpsRepository: ExternalGpsRepository,
186+
// Lazy breaks the Hilt dependency cycle: TripRepository injects this
187+
// repository, so a direct TripRepository here would be circular. Only
188+
// read on the history tick to sample GPS_SPEED / GPS_ALTITUDE /
189+
// GPS_ACCURACY from the current location fix.
190+
private val tripRepositoryLazy: dagger.Lazy<TripRepository>,
151191
private val appNotifier: com.eried.eucplanet.util.AppNotifier
152192
) {
153193
companion object {
@@ -342,11 +382,12 @@ class WheelRepository @Inject constructor(
342382
private val ampsHist = mutableListOf<MetricSample>()
343383
private val loadHist = mutableListOf<MetricSample>()
344384
private val speedHist = mutableListOf<MetricSample>()
345-
// One buffer per entry in EXTRA_HISTORY_METRICS. Keys mirror the
346-
// catalog so the metric-detail screen can fish the right list out
347-
// by name without a hard-coded switch per metric.
385+
// One buffer per non-legacy stat metric (wheel-sourced, phone / GPS
386+
// sourced, plus any not-yet-sourced supportsStats key). Keys mirror the
387+
// catalog so the metric-detail screen and the dashboard corner stats can
388+
// fish the right list out by name without a hard-coded switch per metric.
348389
private val extrasHist: MutableMap<String, MutableList<MetricSample>> =
349-
EXTRA_HISTORY_METRICS.associate { (key, _) -> key to mutableListOf<MetricSample>() }
390+
STAT_BUFFER_EXTRA_KEYS.associateWith { mutableListOf<MetricSample>() }
350391
.toMutableMap()
351392
private var lastHistorySampleMs = 0L
352393

@@ -384,6 +425,35 @@ class WheelRepository @Inject constructor(
384425
_fullHistory.value = FullMetricHistory()
385426
}
386427

428+
/**
429+
* Appends one sample to each phone- / GPS-derived stat buffer
430+
* ([SOURCE_HISTORY_METRIC_KEYS]) from its injected source. Called on the
431+
* wheel history tick so these metrics share the legacy buffers' timeline
432+
* and retention. A source with no fresh value this tick simply skips its
433+
* append (the buffer holds its last window of good samples). Wrapped so a
434+
* transient source read can never break the telemetry path.
435+
*/
436+
private fun sampleSourceHistory(now: Long) {
437+
runCatching {
438+
val loc = tripRepositoryLazy.get().currentLocation.value
439+
if (loc != null) {
440+
if (loc.hasSpeed()) extrasHist["GPS_SPEED"]?.add(MetricSample(now, loc.speed * 3.6f))
441+
if (loc.hasAltitude()) extrasHist["GPS_ALTITUDE"]?.add(MetricSample(now, loc.altitude.toFloat()))
442+
if (loc.hasAccuracy()) extrasHist["GPS_ACCURACY"]?.add(MetricSample(now, loc.accuracy))
443+
}
444+
// Phone battery: cheap in-memory system read, fine at 1 Hz.
445+
val phonePct = (context.getSystemService(Context.BATTERY_SERVICE) as? android.os.BatteryManager)
446+
?.getIntProperty(android.os.BatteryManager.BATTERY_PROPERTY_CAPACITY) ?: -1
447+
if (phonePct in 0..100) extrasHist["PHONE_BATTERY"]?.add(MetricSample(now, phonePct.toFloat()))
448+
// External GPS box battery, freshness-gated to 5 s like the dashboard tile.
449+
val ext = externalGpsRepository.currentSample.value
450+
val extPct = ext?.batteryPercent
451+
if (extPct != null && now - ext.timestamp < 5_000L) {
452+
extrasHist["EXTERNAL_GPS_BATTERY"]?.add(MetricSample(now, extPct.toFloat()))
453+
}
454+
}
455+
}
456+
387457
// Auth state for lock/unlock (V14 requires password verification)
388458
private var authKey: ByteArray? = null
389459
private var pendingAuthKeyDeferred: CompletableDeferred<ByteArray>? = null
@@ -1647,6 +1717,12 @@ class WheelRepository @Inject constructor(
16471717
for ((key, extractor) in EXTRA_HISTORY_METRICS) {
16481718
extrasHist[key]?.add(MetricSample(now, extractor(d)))
16491719
}
1720+
// Phone- and GPS-derived stat metrics aren't on WheelData,
1721+
// so sample them from their injected sources on this same
1722+
// tick. Store raw values (km/h, metres, percent) so the
1723+
// stat math and the tile formatter agree, exactly like the
1724+
// wheel extras above.
1725+
sampleSourceHistory(now)
16501726
// Drop anything older than the 5-min window from every
16511727
// buffer in one pass. List.removeAll touches each list
16521728
// once so this stays linear in buffer size.

app/src/main/java/com/eried/eucplanet/ui/dashboard/DashboardScreen.kt

Lines changed: 69 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,52 @@ private fun openMediaGallery(context: Context, video: Boolean, onNoGalleryApp: (
179179
runCatching { context.startActivity(intent) }.onFailure { onNoGalleryApp() }
180180
}
181181

182+
/**
183+
* Formats a RAW buffered stat value (WheelData canonical units) into the
184+
* compact string a dashboard corner chip or composite cell shows, applying
185+
* the rider's unit conversion so a MAX / AVG chip reads in the same units as
186+
* the live tile. Mirrors displayValueFor's per-key formatting and the
187+
* MetricDetailScreen conversions. Any key without a dedicated unit falls back
188+
* to one decimal. Shared by the standalone-tile corner stats and the
189+
* composite cell renderer so both agree.
190+
*/
191+
private fun formatMetricStatValue(
192+
key: String,
193+
raw: Float,
194+
speedUnit: String,
195+
speedUnitLabel: String,
196+
tempUnit: String,
197+
tempUnitLabel: String,
198+
distanceUnit: String
199+
): String = when (key) {
200+
"BATTERY", "LOAD" -> "${raw.toInt()}%"
201+
"BATTERY_1", "BATTERY_2", "PHONE_BATTERY", "EXTERNAL_GPS_BATTERY" -> "%.0f%%".format(raw)
202+
// Temp buffers store raw °C; convert to the rider's unit like the tile.
203+
"TEMPERATURE" -> "${com.eried.eucplanet.util.Units.temperature(raw, tempUnit).toInt()}°"
204+
"MOTOR_TEMP", "CONTROLLER_TEMP", "BATTERY_TEMP" ->
205+
"%.0f%s".format(com.eried.eucplanet.util.Units.temperature(raw, tempUnit), tempUnitLabel)
206+
"VOLTAGE" -> "%.1fV".format(raw)
207+
"CURRENT", "DYN_CURRENT_LIMIT" -> "%.1fA".format(raw)
208+
// Speed buffers store raw km/h; convert to the rider's speed unit.
209+
"SPEED", "DYN_SPEED_LIMIT", "GPS_SPEED" ->
210+
"%.0f %s".format(com.eried.eucplanet.util.Units.speed(raw, speedUnit), speedUnitLabel)
211+
"MOTOR_POWER", "BATTERY_POWER", "POWER" -> "%.0fW".format(raw)
212+
"PITCH", "ROLL" -> "%.1f°".format(raw)
213+
"G_FORCE", "LATERAL_G", "FORWARD_G" -> "%.2fg".format(raw)
214+
"TORQUE" -> "%.1fNm".format(raw)
215+
// Tire pressure stored raw in kPa; psi for imperial-distance, bar otherwise.
216+
"TIRE_PRESSURE" -> if (distanceUnit == "mi")
217+
"%.1f psi".format(com.eried.eucplanet.util.Units.pressure(raw, "psi"))
218+
else
219+
"%.2f bar".format(com.eried.eucplanet.util.Units.pressure(raw, "bar"))
220+
// Altitude / accuracy stored raw in metres; feet for imperial riders.
221+
"GPS_ALTITUDE", "GPS_ACCURACY" -> if (distanceUnit == "mi")
222+
"%.0fft".format(raw * 3.28084f)
223+
else
224+
"%.0fm".format(raw)
225+
else -> "%.1f".format(raw)
226+
}
227+
182228
@OptIn(ExperimentalMaterial3Api::class, ExperimentalFoundationApi::class)
183229
@Composable
184230
fun DashboardScreen(
@@ -1353,14 +1399,18 @@ fun DashboardScreen(
13531399
if (stat == com.eried.eucplanet.ui.settings.DashboardStat.NONE ||
13541400
stat == com.eried.eucplanet.ui.settings.DashboardStat.CURRENT ||
13551401
stat == com.eried.eucplanet.ui.settings.DashboardStat.EMPTY) return null
1402+
// Legacy six use their typed lists; every other supportsStats
1403+
// metric rides in the keyed extras map (POWER, GPS_SPEED,
1404+
// MOTOR_TEMP, ...). A key with no buffer resolves to an empty
1405+
// list -> placeholder, same as a cold-boot legacy buffer.
13561406
val buf = when (key) {
13571407
"BATTERY" -> history.battery
13581408
"TEMPERATURE" -> history.temperature
13591409
"VOLTAGE" -> history.voltage
13601410
"CURRENT" -> history.current
13611411
"LOAD" -> history.load
13621412
"SPEED" -> history.speed
1363-
else -> return placeholder
1413+
else -> history.extras[key].orEmpty()
13641414
}
13651415
if (buf.isEmpty()) return placeholder
13661416
val samples = buf.mapIndexed { idx, v ->
@@ -1369,20 +1419,9 @@ fun DashboardScreen(
13691419
val raw = com.eried.eucplanet.ui.settings.computeDashboardStatValue(
13701420
stat, samples, fallbackCurrent = buf.last()
13711421
) ?: return placeholder
1372-
return when (key) {
1373-
"BATTERY", "LOAD" -> "${raw.toInt()}%"
1374-
// Buffers store raw °C / km/h, so the corner stat must run
1375-
// the same unit conversion as the tile value or imperial
1376-
// riders see metric numbers under an imperial label.
1377-
"TEMPERATURE" -> "${com.eried.eucplanet.util.Units.temperature(raw, tempUnit).toInt()}°"
1378-
"VOLTAGE" -> "%.1fV".format(raw)
1379-
"CURRENT" -> "%.1fA".format(raw)
1380-
"SPEED" -> "%.0f %s".format(
1381-
com.eried.eucplanet.util.Units.speed(raw, speedUnit),
1382-
speedUnitLabel
1383-
)
1384-
else -> "%.1f".format(raw)
1385-
}
1422+
return formatMetricStatValue(
1423+
key, raw, speedUnit, speedUnitLabel, tempUnit, tempUnitLabel, distanceUnit
1424+
)
13861425
}
13871426

13881427
// Short stat label for the corner chip — "MAX", "MIN", "AVG",
@@ -1841,7 +1880,10 @@ fun DashboardScreen(
18411880
"CURRENT" -> historySnapshot.current
18421881
"LOAD" -> historySnapshot.load
18431882
"SPEED" -> historySnapshot.speed
1844-
else -> emptyList()
1883+
// Any other supportsStats cell
1884+
// (POWER, GPS_SPEED, MOTOR_TEMP,
1885+
// ...) reads its extras buffer.
1886+
else -> historySnapshot.extras[metricKey].orEmpty()
18451887
}
18461888
if (buf.size < 2) return@cellRenderer placeholder
18471889
val samples = buf.mapIndexed { idx, v ->
@@ -1850,7 +1892,13 @@ fun DashboardScreen(
18501892
val value = com.eried.eucplanet.ui.settings.computeDashboardStatValue(
18511893
stat, samples, fallbackCurrent = buf.last()
18521894
) ?: return@cellRenderer placeholder
1853-
"%.1f".format(value)
1895+
// Same unit-correct formatting the
1896+
// standalone corner chip uses so a
1897+
// composite MAX/AVG cell reads right.
1898+
formatMetricStatValue(
1899+
metricKey, value, speedUnit, speedUnitLabel,
1900+
tempUnit, tempUnitLabel, distanceUnit
1901+
)
18541902
}
18551903
// A composite always occupies one standard slot
18561904
// (61 dp), exactly like every other dashboard tile.
@@ -1995,7 +2043,10 @@ fun DashboardScreen(
19952043
},
19962044
value = centerOverride ?: displayValueFor(key),
19972045
accent = spec?.accent?.let { MaterialTheme.appColors.remap(it) } ?: primary,
1998-
sparkData = emptyList(),
2046+
// Generic tiles now get a sparkline too, from
2047+
// the metric's extras buffer (empty until the
2048+
// first sample or for unsourced metrics).
2049+
sparkData = history.extras[key].orEmpty(),
19992050
sparkStyle = spec?.sparkline ?: SparklineStyle.NONE,
20002051
sparklineEnabled = sparklineEnabled,
20012052
bipolarBaseline = spec?.bipolarBaseline ?: 0f,

app/src/main/java/com/eried/eucplanet/ui/dashboard/DashboardViewModel.kt

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,16 @@ data class MetricHistory(
4040
val voltage: List<Float> = emptyList(),
4141
val current: List<Float> = emptyList(),
4242
val load: List<Float> = emptyList(),
43-
val speed: List<Float> = emptyList()
43+
val speed: List<Float> = emptyList(),
44+
/**
45+
* Sparkline / stat window for every non-legacy catalog metric, keyed by
46+
* metric key (MOTOR_POWER, GPS_SPEED, MOTOR_TEMP, ...). Mirrors the six
47+
* typed lists above but data-driven off [FullMetricHistory.extras], so the
48+
* dashboard's corner stats and sparklines work for any supportsStats tile,
49+
* not just the legacy six. Values are raw (WheelData canonical units); the
50+
* tile applies the rider's unit conversion at render time.
51+
*/
52+
val extras: Map<String, List<Float>> = emptyMap()
4453
)
4554

4655
@HiltViewModel
@@ -604,7 +613,12 @@ class DashboardViewModel @Inject constructor(
604613
voltage = full.voltage.takeLast(SPARKLINE_SIZE).map { it.value },
605614
current = full.current.takeLast(SPARKLINE_SIZE).map { it.value },
606615
load = full.load.takeLast(SPARKLINE_SIZE).map { it.value },
607-
speed = full.speed.takeLast(SPARKLINE_SIZE).map { it.value }
616+
speed = full.speed.takeLast(SPARKLINE_SIZE).map { it.value },
617+
// Same window as the legacy six, applied per extras buffer so
618+
// every catalog metric's corner stats / sparkline resolve.
619+
extras = full.extras.mapValues { (_, list) ->
620+
list.takeLast(SPARKLINE_SIZE).map { it.value }
621+
}
608622
)
609623
}
610624
.stateIn(viewModelScope, SharingStarted.WhileSubscribed(5000), MetricHistory())
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
package com.eried.eucplanet.data.repository
2+
3+
import com.eried.eucplanet.data.model.MetricCatalog
4+
import org.junit.Assert.assertEquals
5+
import org.junit.Assert.assertTrue
6+
import org.junit.Test
7+
8+
/**
9+
* Drift guard (CONVENTIONS rule 13) for the dashboard stat buffers.
10+
*
11+
* Every catalog metric that advertises supportsStats = true must resolve to a
12+
* rolling-history buffer, otherwise its min / max / avg / last pills and its
13+
* sparkline have nothing to compute from and render an empty "--". That was
14+
* the exact bug this test defends against: POWER / MAX GPS SPEED and friends
15+
* showed blank pills because their buffer path was missing.
16+
*
17+
* The buffer path is either one of the six legacy typed lists on
18+
* [FullMetricHistory], or an allocated extras buffer in
19+
* [STAT_BUFFER_EXTRA_KEYS]. Since that set is derived from the catalog, a new
20+
* supportsStats metric is covered automatically, and any future change that
21+
* turns it into a hand-maintained list is caught here.
22+
*/
23+
class MetricStatBufferCoverageTest {
24+
25+
private val statKeys: List<String> =
26+
MetricCatalog.all.filter { it.supportsStats }.map { it.key }
27+
28+
@Test
29+
fun everyStatMetricHasABufferPath() {
30+
val covered = (LEGACY_STAT_METRIC_KEYS + STAT_BUFFER_EXTRA_KEYS).toSet()
31+
val missing = statKeys.filterNot { it in covered }
32+
assertTrue(
33+
"supportsStats metrics with no history buffer path: $missing",
34+
missing.isEmpty()
35+
)
36+
}
37+
38+
@Test
39+
fun legacyAndExtrasBuffersAreDisjoint() {
40+
val overlap = STAT_BUFFER_EXTRA_KEYS.filter { it in LEGACY_STAT_METRIC_KEYS }
41+
assertTrue("legacy keys must not also be extras buffers: $overlap", overlap.isEmpty())
42+
}
43+
44+
@Test
45+
fun extractorKeysAllHaveAllocatedBuffers() {
46+
val wheelKeys = EXTRA_HISTORY_METRICS.map { it.first }
47+
val unallocated = (wheelKeys + SOURCE_HISTORY_METRIC_KEYS)
48+
.filterNot { it in STAT_BUFFER_EXTRA_KEYS }
49+
assertTrue("extractor keys with no allocated buffer: $unallocated", unallocated.isEmpty())
50+
}
51+
52+
@Test
53+
fun extrasBufferKeysAreUnique() {
54+
assertEquals(STAT_BUFFER_EXTRA_KEYS.size, STAT_BUFFER_EXTRA_KEYS.toSet().size)
55+
}
56+
}

0 commit comments

Comments
 (0)