mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-06 03:38:23 +00:00
perf(fitness): load My Fitness progressively instead of on one serial Health Connect read
The My Fitness dashboard sat on a spinner for 2-3s on a phone with a watch connected. Three things compounded: - `readWorkouts` made one `aggregate()` IPC per session, in series. The dashboard's window is 28 days (`WorkoutStats.WINDOW_DAYS`) against the New Workout carousel's 7, so a daily trainer paid 30-60 sequential round trips. - `refresh()` launched on `viewModelScope` (`Dispatchers.Main.immediate`) and `healthConnectStatus()` never left it, so the PackageManager query and the lazy Health Connect service bind ran on the UI thread. `readWorkouts` already hopped to IO with a comment about exactly this hazard; the permission probe in front of it did not. - The screen stayed on `State.Loading` until `healthConnectStatus` went non-null, which happened only after the whole read finished — so the user's published kind 1301 workouts, already indexed in `LocalCache`, waited behind Health Connect for data the screen's own KDoc calls "not a precondition". Fixed all three, and split the read into two stages. The session list is one IPC and already carries activity, title, start, duration and source — enough for the counts, total time, streak, active days, per-activity split, recent list and the longest-duration best. The metrics that need a per-session aggregate (distance, calories, heart rate, steps, elevation) arrive second, now fanned out under a permit cap rather than run one at a time. That partial pass is honest rather than approximate: `WorkoutStats.total` and `bests` already treat an absent metric as contributing nothing instead of zero, and the screen already gates each metric cell on `> 0` / non-null, so an unknown metric reads as a hidden cell, never as "0 km". `State.Ready` carries `metricsPending` so the dashboard says what is still filling in. Aggregation stays per raw session with merging after it, exactly as before: `WorkoutMerger` sums its members' metrics, so aggregating a merged span would fold in the breaks between segments and change the totals. Two flicker guards: the status is published before the read starts (it is cheap, and gating on it is what lets the published log render immediately), but an *empty* report keeps waiting while the session list is in flight, so a user whose workouts are one IPC away never sees the empty state or the connect prompt flash. A resume re-reads in place rather than blanking a dashboard that is already correct, and each refresh cancels the previous one so two reads cannot interleave. Tests: 6 new in `PartialMetricsReportTest` pinning that the first pass is final for everything not metric-derived and absent (not zero) for everything that is; 2 in `WorkoutMergerTest` for skeleton/loaded grouping parity and for why per-session aggregation has to precede the merge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VgfgpDWnu635p2qc2K2yMX
This commit is contained in:
+165
-52
@@ -38,7 +38,15 @@ import com.vitorpamplona.amethyst.commons.fitness.DetectedWorkout
|
||||
import com.vitorpamplona.quartz.utils.Log
|
||||
import kotlinx.coroutines.CancellationException
|
||||
import kotlinx.coroutines.Dispatchers
|
||||
import kotlinx.coroutines.withContext
|
||||
import kotlinx.coroutines.async
|
||||
import kotlinx.coroutines.awaitAll
|
||||
import kotlinx.coroutines.coroutineScope
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
import kotlinx.coroutines.flow.flow
|
||||
import kotlinx.coroutines.flow.flowOn
|
||||
import kotlinx.coroutines.flow.last
|
||||
import kotlinx.coroutines.sync.Semaphore
|
||||
import kotlinx.coroutines.sync.withPermit
|
||||
import java.time.Duration
|
||||
import java.time.Instant
|
||||
import kotlin.math.roundToInt
|
||||
@@ -66,6 +74,16 @@ class HealthConnectManager(
|
||||
/** How far back the New Workout carousel looks for workouts to offer. */
|
||||
const val LOOKBACK_DAYS = 7L
|
||||
|
||||
/**
|
||||
* How many per-session metric aggregations may be in flight at once.
|
||||
*
|
||||
* Each is an independent binder transaction into the Health Connect provider, so running
|
||||
* them in series made the read scale with the window length. Capped rather than unbounded
|
||||
* because the provider serves them from a finite thread/transaction pool, and a four-week
|
||||
* window for a daily trainer would otherwise fire dozens at once.
|
||||
*/
|
||||
private const val MAX_CONCURRENT_AGGREGATES = 6
|
||||
|
||||
private const val DEFAULT_SOURCE = "Health Connect"
|
||||
|
||||
/** Friendly names for well-known writers when their app isn't installed to read a label from. */
|
||||
@@ -124,49 +142,153 @@ class HealthConnectManager(
|
||||
}
|
||||
|
||||
/**
|
||||
* All exercise sessions that ended within [since]..[now], mapped to
|
||||
* [DetectedWorkout]. Sessions whose activity type Amethyst cannot represent
|
||||
* are skipped. Returns an empty list (never throws) if Health Connect is
|
||||
* unavailable or a read fails.
|
||||
* One stage of a progressive Health Connect read. See [readWorkoutsProgressively].
|
||||
*/
|
||||
data class WorkoutRead(
|
||||
val workouts: List<DetectedWorkout>,
|
||||
/**
|
||||
* True while the per-session aggregate metrics (distance, calories, heart rate, steps,
|
||||
* elevation) are still being fetched, so [workouts] currently carries nulls for them.
|
||||
*/
|
||||
val metricsPending: Boolean,
|
||||
)
|
||||
|
||||
/**
|
||||
* All exercise sessions that ended within [since]..[now], mapped to [DetectedWorkout], in two
|
||||
* stages.
|
||||
*
|
||||
* The session list itself is one IPC. Every optional metric — distance, calories, heart rate,
|
||||
* steps, elevation — needs a *separate* [aggregate] round trip per session, and a four-week
|
||||
* window for someone who trains daily is dozens of them. Waiting for all of that before
|
||||
* showing anything is what made the My Fitness dashboard sit on a spinner for seconds.
|
||||
*
|
||||
* So this emits twice:
|
||||
* 1. the sessions with what the read already knows — activity, title, start, duration,
|
||||
* source — and [WorkoutRead.metricsPending] true;
|
||||
* 2. the same workouts with their metrics filled in, and the flag false.
|
||||
*
|
||||
* That first emission already carries everything the dashboard's counts, durations, streak,
|
||||
* active days and per-activity split need. `WorkoutStats` totals and bests treat an absent
|
||||
* metric as contributing nothing rather than zero, so the partial pass is honest rather than
|
||||
* wrong — a distance cell is missing until it is known, not shown as 0.
|
||||
*
|
||||
* A single-emission caller that wants the finished numbers takes [readWorkouts].
|
||||
*
|
||||
* Aggregation is per *raw* session and merging happens after it, exactly as a one-shot read
|
||||
* did: [WorkoutMerger] sums the members' metrics, so aggregating a merged span instead would
|
||||
* fold in the breaks between segments and change the totals.
|
||||
*
|
||||
* Emits a single empty result (never throws) if Health Connect is unavailable or the read
|
||||
* fails.
|
||||
*/
|
||||
fun readWorkoutsProgressively(
|
||||
since: Instant,
|
||||
now: Instant = Instant.now(),
|
||||
): Flow<WorkoutRead> =
|
||||
// flowOn(IO): callers collect from Dispatchers.Main, Health Connect's own calls suspend,
|
||||
// but the PackageManager lookup in resolveSourceName is a blocking binder call — so the
|
||||
// whole read moves off the UI thread rather than relying on each step to behave.
|
||||
flow {
|
||||
if (!isAvailable(context)) {
|
||||
Log.i(TAG) { "readWorkouts: Health Connect unavailable (status=${HealthConnectClient.getSdkStatus(context)})" }
|
||||
emit(WorkoutRead(emptyList(), metricsPending = false))
|
||||
return@flow
|
||||
}
|
||||
|
||||
val sessions =
|
||||
try {
|
||||
client
|
||||
.readRecords(
|
||||
ReadRecordsRequest(
|
||||
recordType = ExerciseSessionRecord::class,
|
||||
timeRangeFilter = TimeRangeFilter.between(since, now),
|
||||
),
|
||||
).records
|
||||
} catch (e: Exception) {
|
||||
if (e is CancellationException) throw e
|
||||
Log.w(TAG, "Failed to read workouts from Health Connect", e)
|
||||
emit(WorkoutRead(emptyList(), metricsPending = false))
|
||||
return@flow
|
||||
}
|
||||
|
||||
Log.i(TAG) { "readWorkouts: ${sessions.size} exercise session(s) in window $since .. $now" }
|
||||
|
||||
// Kept paired with their sessions: stage 2 aggregates over each raw session's own
|
||||
// time range, which the mapped workout no longer carries once merging rewrites it.
|
||||
val skeletons = sessions.mapNotNull { session -> mapSession(session)?.let { session to it } }
|
||||
|
||||
if (skeletons.isEmpty()) {
|
||||
Log.i(TAG) { "readWorkouts: no mappable sessions" }
|
||||
emit(WorkoutRead(emptyList(), metricsPending = false))
|
||||
return@flow
|
||||
}
|
||||
|
||||
emit(WorkoutRead(merge(skeletons.map { it.second }), metricsPending = true))
|
||||
|
||||
// Bounded fan-out rather than one-at-a-time: these are independent IPCs into the
|
||||
// provider, and running them in series is what the window length multiplied. The
|
||||
// permit cap keeps a month of daily training from opening 60 concurrent binder
|
||||
// transactions at a provider that has a finite pool for them.
|
||||
val detailed =
|
||||
coroutineScope {
|
||||
val gate = Semaphore(MAX_CONCURRENT_AGGREGATES)
|
||||
skeletons
|
||||
.map { (session, workout) ->
|
||||
async { gate.withPermit { workout.withMetrics(aggregate(session)) } }
|
||||
}.awaitAll()
|
||||
}
|
||||
|
||||
val merged = merge(detailed)
|
||||
Log.i(TAG) { "readWorkouts: mapped ${detailed.size} -> ${merged.size} workout(s) after type/duration filtering and merging" }
|
||||
emit(WorkoutRead(merged, metricsPending = false))
|
||||
}.flowOn(Dispatchers.IO)
|
||||
|
||||
/**
|
||||
* All exercise sessions in [since]..[now], with their metrics — the finished result of
|
||||
* [readWorkoutsProgressively]. For callers that have nothing useful to show from a partial
|
||||
* read and so gain nothing from the intermediate stage.
|
||||
*/
|
||||
suspend fun readWorkouts(
|
||||
since: Instant,
|
||||
now: Instant = Instant.now(),
|
||||
): List<DetectedWorkout> {
|
||||
if (!isAvailable(context)) {
|
||||
Log.i(TAG) { "readWorkouts: Health Connect unavailable (status=${HealthConnectClient.getSdkStatus(context)})" }
|
||||
return emptyList()
|
||||
}
|
||||
): List<DetectedWorkout> = readWorkoutsProgressively(since, now).last().workouts
|
||||
|
||||
// The callers are composables launching into rememberCoroutineScope(), i.e.
|
||||
// Dispatchers.Main. Health Connect's own calls suspend, but the PackageManager
|
||||
// lookup in resolveSourceName is a blocking binder call, so the whole read
|
||||
// moves off the UI thread rather than relying on each step to behave.
|
||||
return withContext(Dispatchers.IO) {
|
||||
try {
|
||||
val response =
|
||||
client.readRecords(
|
||||
ReadRecordsRequest(
|
||||
recordType = ExerciseSessionRecord::class,
|
||||
timeRangeFilter = TimeRangeFilter.between(since, now),
|
||||
),
|
||||
)
|
||||
Log.i(TAG) { "readWorkouts: ${response.records.size} exercise session(s) in window $since .. $now" }
|
||||
val mapped = response.records.mapNotNull { mapSession(it) }
|
||||
// Fold split-up sessions of the same activity (a long run broken around
|
||||
// breaks) into one suggestion so the composer offers the whole effort.
|
||||
val merged = WorkoutMerger.mergeCloseWorkouts(mapped)
|
||||
Log.i(TAG) { "readWorkouts: mapped ${mapped.size} -> ${merged.size} workout(s) after type/duration filtering and merging" }
|
||||
merged
|
||||
} catch (e: Exception) {
|
||||
if (e is CancellationException) throw e
|
||||
Log.w(TAG, "Failed to read workouts from Health Connect", e)
|
||||
emptyList()
|
||||
}
|
||||
}
|
||||
/**
|
||||
* Folds split-up sessions of the same activity (a long run broken around breaks) into one
|
||||
* workout so the composer offers the whole effort.
|
||||
*/
|
||||
private fun merge(workouts: List<DetectedWorkout>) = WorkoutMerger.mergeCloseWorkouts(workouts)
|
||||
|
||||
/** Copies [totals] — the result of one [aggregate] call — onto a session skeleton. */
|
||||
private fun DetectedWorkout.withMetrics(totals: AggregationResult?): DetectedWorkout {
|
||||
if (totals == null) return this
|
||||
|
||||
return copy(
|
||||
distanceMeters = totals[DistanceRecord.DISTANCE_TOTAL]?.inMeters,
|
||||
// Prefer active calories (what RUNSTR publishes); fall back to total for
|
||||
// sources that only record total energy. Total includes basal burn, so it
|
||||
// over-reports the workout if used as the primary figure.
|
||||
calories =
|
||||
(
|
||||
totals[ActiveCaloriesBurnedRecord.ACTIVE_CALORIES_TOTAL]?.inKilocalories
|
||||
?: totals[TotalCaloriesBurnedRecord.ENERGY_TOTAL]?.inKilocalories
|
||||
)?.roundToInt(),
|
||||
avgHeartRate = totals[HeartRateRecord.BPM_AVG]?.toInt(),
|
||||
maxHeartRate = totals[HeartRateRecord.BPM_MAX]?.toInt(),
|
||||
steps = totals[StepsRecord.COUNT_TOTAL]?.toInt(),
|
||||
elevationGainMeters = totals[ElevationGainedRecord.ELEVATION_GAINED_TOTAL]?.inMeters,
|
||||
)
|
||||
}
|
||||
|
||||
private suspend fun mapSession(session: ExerciseSessionRecord): DetectedWorkout? {
|
||||
/**
|
||||
* The session as the record itself describes it: activity, title, when, how long, who wrote
|
||||
* it. The optional metrics are left null for [withMetrics] to fill in — they each cost their
|
||||
* own IPC, so they are not part of mapping a session.
|
||||
*
|
||||
* Null when the activity type is one Amethyst cannot represent, or the session has no
|
||||
* positive duration.
|
||||
*/
|
||||
private fun mapSession(session: ExerciseSessionRecord): DetectedWorkout? {
|
||||
val exercise = ExerciseTypeMapper.toExerciseType(session.exerciseType)
|
||||
if (exercise == null) {
|
||||
Log.i(TAG) {
|
||||
@@ -187,27 +309,18 @@ class HealthConnectManager(
|
||||
"from ${session.metadata.dataOrigin.packageName}"
|
||||
}
|
||||
|
||||
val totals = aggregate(session)
|
||||
|
||||
return DetectedWorkout(
|
||||
id = session.metadata.id,
|
||||
exercise = exercise,
|
||||
title = session.title?.takeIf { it.isNotBlank() },
|
||||
startTimeEpochSeconds = session.startTime.epochSecond,
|
||||
durationSeconds = durationSeconds,
|
||||
distanceMeters = totals?.get(DistanceRecord.DISTANCE_TOTAL)?.inMeters,
|
||||
// Prefer active calories (what RUNSTR publishes); fall back to total for
|
||||
// sources that only record total energy. Total includes basal burn, so it
|
||||
// over-reports the workout if used as the primary figure.
|
||||
calories =
|
||||
(
|
||||
totals?.get(ActiveCaloriesBurnedRecord.ACTIVE_CALORIES_TOTAL)?.inKilocalories
|
||||
?: totals?.get(TotalCaloriesBurnedRecord.ENERGY_TOTAL)?.inKilocalories
|
||||
)?.roundToInt(),
|
||||
avgHeartRate = totals?.get(HeartRateRecord.BPM_AVG)?.toInt(),
|
||||
maxHeartRate = totals?.get(HeartRateRecord.BPM_MAX)?.toInt(),
|
||||
steps = totals?.get(StepsRecord.COUNT_TOTAL)?.toInt(),
|
||||
elevationGainMeters = totals?.get(ElevationGainedRecord.ELEVATION_GAINED_TOTAL)?.inMeters,
|
||||
distanceMeters = null,
|
||||
calories = null,
|
||||
avgHeartRate = null,
|
||||
maxHeartRate = null,
|
||||
steps = null,
|
||||
elevationGainMeters = null,
|
||||
source = resolveSourceName(session.metadata.dataOrigin.packageName),
|
||||
)
|
||||
}
|
||||
|
||||
+30
@@ -76,6 +76,7 @@ import com.vitorpamplona.amethyst.commons.resources.my_fitness_connect_title
|
||||
import com.vitorpamplona.amethyst.commons.resources.my_fitness_distance
|
||||
import com.vitorpamplona.amethyst.commons.resources.my_fitness_elevation
|
||||
import com.vitorpamplona.amethyst.commons.resources.my_fitness_empty
|
||||
import com.vitorpamplona.amethyst.commons.resources.my_fitness_loading_metrics
|
||||
import com.vitorpamplona.amethyst.commons.resources.my_fitness_max_heart_rate
|
||||
import com.vitorpamplona.amethyst.commons.resources.my_fitness_recent
|
||||
import com.vitorpamplona.amethyst.commons.resources.my_fitness_share
|
||||
@@ -181,6 +182,7 @@ fun MyFitnessScreen(
|
||||
// Only offered when it would actually add something: a device with no
|
||||
// provider gets no banner to act on.
|
||||
showConnectBanner = current.healthConnect == MyFitnessViewModel.HealthConnectStatus.AVAILABLE,
|
||||
metricsPending = current.metricsPending,
|
||||
onDetails = openRationale,
|
||||
onConnect = requestPermissions,
|
||||
) { workout, label ->
|
||||
@@ -226,6 +228,32 @@ private fun ConnectBanner(
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Shown while the per-session metrics are still being read from Health Connect. The dashboard
|
||||
* below it is already real — counts, time, streak, the activity split — but its distance,
|
||||
* calories and heart rate cells appear as each session's metrics land, and a row of numbers
|
||||
* growing on its own needs saying out loud.
|
||||
*/
|
||||
@Composable
|
||||
private fun MetricsPendingNote() {
|
||||
Row(
|
||||
horizontalArrangement = Arrangement.spacedBy(8.dp),
|
||||
verticalAlignment = Alignment.CenterVertically,
|
||||
modifier = Modifier.fillMaxWidth(),
|
||||
) {
|
||||
CircularProgressIndicator(
|
||||
modifier = Modifier.size(14.dp),
|
||||
strokeWidth = 2.dp,
|
||||
color = MaterialTheme.colorScheme.onSurfaceVariant,
|
||||
)
|
||||
Text(
|
||||
text = stringRes(Res.string.my_fitness_loading_metrics),
|
||||
style = MaterialTheme.typography.labelMedium,
|
||||
color = MaterialTheme.colorScheme.onSurfaceVariant,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@Composable
|
||||
private fun ConnectPrompt(
|
||||
onDetails: () -> Unit,
|
||||
@@ -263,6 +291,7 @@ private fun ConnectPrompt(
|
||||
private fun Dashboard(
|
||||
report: WorkoutStats.Report,
|
||||
showConnectBanner: Boolean,
|
||||
metricsPending: Boolean,
|
||||
onDetails: () -> Unit,
|
||||
onConnect: () -> Unit,
|
||||
onShare: (DetectedWorkout, String) -> Unit,
|
||||
@@ -274,6 +303,7 @@ private fun Dashboard(
|
||||
verticalArrangement = Arrangement.spacedBy(18.dp),
|
||||
) {
|
||||
if (showConnectBanner) ConnectBanner(onDetails = onDetails, onConnect = onConnect)
|
||||
if (metricsPending) MetricsPendingNote()
|
||||
ThisWeekCard(report, miles)
|
||||
ConsistencyRow(report)
|
||||
WindowTotalsCard(report, miles)
|
||||
|
||||
+95
-31
@@ -32,6 +32,7 @@ import com.vitorpamplona.amethyst.service.workouts.health.HealthConnectManager
|
||||
import com.vitorpamplona.amethyst.service.workouts.health.publishedWorkoutsOf
|
||||
import kotlinx.coroutines.Dispatchers
|
||||
import kotlinx.coroutines.ExperimentalCoroutinesApi
|
||||
import kotlinx.coroutines.Job
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
import kotlinx.coroutines.flow.MutableStateFlow
|
||||
import kotlinx.coroutines.flow.SharingStarted
|
||||
@@ -42,6 +43,7 @@ import kotlinx.coroutines.flow.flowOf
|
||||
import kotlinx.coroutines.flow.flowOn
|
||||
import kotlinx.coroutines.flow.stateIn
|
||||
import kotlinx.coroutines.launch
|
||||
import kotlinx.coroutines.withContext
|
||||
import java.time.Duration
|
||||
import java.time.Instant
|
||||
|
||||
@@ -84,16 +86,41 @@ class MyFitnessViewModel : ViewModel() {
|
||||
data class Ready(
|
||||
val report: WorkoutStats.Report,
|
||||
val healthConnect: HealthConnectStatus,
|
||||
/**
|
||||
* True while Health Connect's per-session metrics are still arriving. The report is
|
||||
* real and complete in every other respect — counts, time, streak, active days, the
|
||||
* per-activity split — but distance, calories, heart rate, steps and elevation are
|
||||
* still filling in, so the screen says so rather than letting cells appear unexplained.
|
||||
*/
|
||||
val metricsPending: Boolean = false,
|
||||
) : State
|
||||
}
|
||||
|
||||
private val pubkeyHex = MutableStateFlow<String?>(null)
|
||||
|
||||
/**
|
||||
* Health Connect's contribution, and how far along reading it is.
|
||||
*
|
||||
* [sessionsPending] and [metricsPending] are the two stages of
|
||||
* [HealthConnectManager.readWorkoutsProgressively]: the session list costs one IPC, the
|
||||
* metrics cost one per session. They are tracked separately because they mean different
|
||||
* things to the screen — an empty dashboard must not say "nothing logged yet" while the
|
||||
* sessions are still coming, but it can show real counts and times while the metrics are.
|
||||
*/
|
||||
private data class Contribution(
|
||||
val workouts: List<DetectedWorkout> = emptyList(),
|
||||
val sessionsPending: Boolean = false,
|
||||
val metricsPending: Boolean = false,
|
||||
)
|
||||
|
||||
/**
|
||||
* Health Connect's contribution. A push source: the platform has no change feed we can
|
||||
* observe, so [refresh] re-reads it when the screen resumes or a permission is granted.
|
||||
*/
|
||||
private val fromHealthConnect = MutableStateFlow<List<DetectedWorkout>>(emptyList())
|
||||
private val fromHealthConnect = MutableStateFlow(Contribution())
|
||||
|
||||
/** The in-flight [refresh], cancelled by the next one so two reads never interleave. */
|
||||
private var refreshJob: Job? = null
|
||||
|
||||
/** Null until the first [refresh] resolves, which is what keeps the screen on [State.Loading]. */
|
||||
private val healthConnectStatus = MutableStateFlow<HealthConnectStatus?>(null)
|
||||
@@ -112,21 +139,29 @@ class MyFitnessViewModel : ViewModel() {
|
||||
|
||||
val state: StateFlow<State> =
|
||||
combine(fromHealthConnect, fromRelays, healthConnectStatus) { healthConnect, published, status ->
|
||||
if (status == null) {
|
||||
State.Loading
|
||||
} else {
|
||||
val now = Instant.now()
|
||||
val since = now.minus(Duration.ofDays(WorkoutStats.WINDOW_DAYS)).epochSecond
|
||||
// The status check is cheap; reading the workouts is not. Waiting only on the former
|
||||
// is what lets a user's published log render while their watch data is still coming.
|
||||
if (status == null) return@combine State.Loading
|
||||
|
||||
State.Ready(
|
||||
report =
|
||||
WorkoutStats.report(
|
||||
TrainingLog.merge(healthConnect, published.filter { it.startTimeEpochSeconds >= since }),
|
||||
now,
|
||||
),
|
||||
healthConnect = status,
|
||||
val now = Instant.now()
|
||||
val since = now.minus(Duration.ofDays(WorkoutStats.WINDOW_DAYS)).epochSecond
|
||||
|
||||
val report =
|
||||
WorkoutStats.report(
|
||||
TrainingLog.merge(healthConnect.workouts, published.filter { it.startTimeEpochSeconds >= since }),
|
||||
now,
|
||||
)
|
||||
}
|
||||
|
||||
// Nothing to show *yet* is not the same as nothing logged. Going Ready here would
|
||||
// flash the empty state — or the connect prompt — at a user whose sessions are one
|
||||
// IPC away, so an empty report keeps waiting while the session list is in flight.
|
||||
if (report.isEmpty && healthConnect.sessionsPending) return@combine State.Loading
|
||||
|
||||
State.Ready(
|
||||
report = report,
|
||||
healthConnect = status,
|
||||
metricsPending = healthConnect.metricsPending,
|
||||
)
|
||||
}.flowOn(Dispatchers.Default)
|
||||
.stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), State.Loading)
|
||||
|
||||
@@ -141,25 +176,54 @@ class MyFitnessViewModel : ViewModel() {
|
||||
* numbers on display. The published side needs no refresh — it is observed.
|
||||
*/
|
||||
fun refresh(context: Context) {
|
||||
viewModelScope.launch {
|
||||
val status = healthConnectStatus(context)
|
||||
// A resume while the previous read is still running would leave two collectors writing
|
||||
// fromHealthConnect, and the slower one could land a stale list last.
|
||||
refreshJob?.cancel()
|
||||
refreshJob =
|
||||
viewModelScope.launch {
|
||||
val status = healthConnectStatus(context)
|
||||
val hc = manager
|
||||
|
||||
fromHealthConnect.value =
|
||||
if (status == HealthConnectStatus.CONNECTED) {
|
||||
val now = Instant.now()
|
||||
manager?.readWorkouts(now.minus(Duration.ofDays(WorkoutStats.WINDOW_DAYS)), now).orEmpty()
|
||||
} else {
|
||||
emptyList()
|
||||
// Read through the local rather than the field: nothing may set sessionsPending
|
||||
// without a reader that will clear it again, or an empty dashboard waits forever.
|
||||
if (status != HealthConnectStatus.CONNECTED || hc == null) {
|
||||
fromHealthConnect.value = Contribution()
|
||||
healthConnectStatus.value = status
|
||||
return@launch
|
||||
}
|
||||
|
||||
healthConnectStatus.value = status
|
||||
// Published before the read, not after: the status is what the screen is gated on,
|
||||
// and it is now known. The workouts arrive into an already-rendered dashboard.
|
||||
// The previous read's workouts stay up meanwhile, so a resume re-reads in place
|
||||
// rather than blanking a dashboard that is already correct.
|
||||
fromHealthConnect.value = fromHealthConnect.value.copy(sessionsPending = true)
|
||||
healthConnectStatus.value = status
|
||||
|
||||
val now = Instant.now()
|
||||
hc
|
||||
.readWorkoutsProgressively(now.minus(Duration.ofDays(WorkoutStats.WINDOW_DAYS)), now)
|
||||
.collect { read ->
|
||||
fromHealthConnect.value =
|
||||
Contribution(
|
||||
workouts = read.workouts,
|
||||
sessionsPending = false,
|
||||
metricsPending = read.metricsPending,
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Both calls here are binder round trips — a PackageManager query, and a bind to the Health
|
||||
* Connect service that [HealthConnectManager] makes lazily on first use — so they run off the
|
||||
* main thread. [viewModelScope] is `Dispatchers.Main.immediate`, which would otherwise stall
|
||||
* the frame that opens the screen.
|
||||
*/
|
||||
private suspend fun healthConnectStatus(context: Context): HealthConnectStatus =
|
||||
withContext(Dispatchers.IO) {
|
||||
if (!HealthConnectManager.isAvailable(context)) return@withContext HealthConnectStatus.UNAVAILABLE
|
||||
|
||||
val hc = manager ?: HealthConnectManager(context.applicationContext).also { manager = it }
|
||||
if (hc.hasAllPermissions()) HealthConnectStatus.CONNECTED else HealthConnectStatus.AVAILABLE
|
||||
}
|
||||
}
|
||||
|
||||
private suspend fun healthConnectStatus(context: Context): HealthConnectStatus {
|
||||
if (!HealthConnectManager.isAvailable(context)) return HealthConnectStatus.UNAVAILABLE
|
||||
|
||||
val hc = manager ?: HealthConnectManager(context.applicationContext).also { manager = it }
|
||||
return if (hc.hasAllPermissions()) HealthConnectStatus.CONNECTED else HealthConnectStatus.AVAILABLE
|
||||
}
|
||||
}
|
||||
|
||||
+43
@@ -226,6 +226,49 @@ class WorkoutMergerTest {
|
||||
assertEquals(1800, merged.durationSeconds)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun skeletonsWithoutMetricsGroupExactlyAsTheFullyLoadedOnesDo() {
|
||||
// The progressive read merges twice: once over session skeletons, to get something on
|
||||
// screen, and again once each session's metrics have been aggregated. Grouping keys on
|
||||
// type, start and duration only, so both passes must produce the same shape — otherwise
|
||||
// the dashboard would reshuffle its workout list when the metrics land.
|
||||
val loaded =
|
||||
listOf(
|
||||
workout("a", startTimeEpochSeconds = 0, durationSeconds = 600, distanceMeters = 2_000.0, calories = 150, avgHeartRate = 140, steps = 1_800),
|
||||
workout("b", startTimeEpochSeconds = 1200, durationSeconds = 600, distanceMeters = 2_100.0, calories = 160, avgHeartRate = 150, steps = 1_900),
|
||||
workout("c", exercise = ExerciseType.CYCLING, startTimeEpochSeconds = 20_000, durationSeconds = 3600, distanceMeters = 25_000.0),
|
||||
)
|
||||
val skeletons =
|
||||
loaded.map {
|
||||
it.copy(distanceMeters = null, calories = null, avgHeartRate = null, maxHeartRate = null, steps = null, elevationGainMeters = null)
|
||||
}
|
||||
|
||||
val early = WorkoutMerger.mergeCloseWorkouts(skeletons)
|
||||
val late = WorkoutMerger.mergeCloseWorkouts(loaded)
|
||||
|
||||
assertEquals(late.map { it.id }, early.map { it.id })
|
||||
assertEquals(late.map { it.sessionCount }, early.map { it.sessionCount })
|
||||
assertEquals(late.map { it.startTimeEpochSeconds }, early.map { it.startTimeEpochSeconds })
|
||||
assertEquals(late.map { it.durationSeconds }, early.map { it.durationSeconds })
|
||||
}
|
||||
|
||||
@Test
|
||||
fun aggregatingPerSessionBeforeMergingIsWhatSumsASplitEffort() {
|
||||
// Why the progressive read still aggregates each raw session and merges afterwards,
|
||||
// rather than aggregating the merged span once: the merged workout's metrics are the
|
||||
// sum of its members', and the span between them is not part of the effort.
|
||||
val first = workout("a", startTimeEpochSeconds = 0, durationSeconds = 600, distanceMeters = 2_000.0, calories = 150, steps = 1_800)
|
||||
val second = workout("b", startTimeEpochSeconds = 1200, durationSeconds = 600, distanceMeters = 2_100.0, calories = 160, steps = 1_900)
|
||||
|
||||
val merged = WorkoutMerger.mergeCloseWorkouts(listOf(first, second)).single()
|
||||
|
||||
assertEquals(4_100.0, merged.distanceMeters!!, 0.001)
|
||||
assertEquals(310, merged.calories)
|
||||
assertEquals(3_700, merged.steps)
|
||||
// The 10-minute break between them is excluded: duration is the sum, not end minus start.
|
||||
assertEquals(1200, merged.durationSeconds)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun chainMergesEvenWhenAdjacentGapsAreShortButEndsAreFarApart() {
|
||||
// 45-min sessions each starting 50 min apart: consecutive gaps are 5 min,
|
||||
|
||||
+186
@@ -0,0 +1,186 @@
|
||||
/*
|
||||
* Copyright (c) 2025 Vitor Pamplona
|
||||
*
|
||||
* Permission is hereby granted, free of charge, to any person obtaining a copy of
|
||||
* this software and associated documentation files (the "Software"), to deal in
|
||||
* the Software without restriction, including without limitation the rights to use,
|
||||
* copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
|
||||
* Software, and to permit persons to whom the Software is furnished to do so,
|
||||
* subject to the following conditions:
|
||||
*
|
||||
* The above copyright notice and this permission notice shall be included in all
|
||||
* copies or substantial portions of the Software.
|
||||
*
|
||||
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
|
||||
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
|
||||
* FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
|
||||
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
|
||||
* AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
|
||||
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
|
||||
*/
|
||||
package com.vitorpamplona.amethyst.commons.fitness
|
||||
|
||||
import com.vitorpamplona.quartz.experimental.fitness.workout.tags.ExerciseType
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Assert.assertNull
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Test
|
||||
import java.time.Instant
|
||||
import java.time.ZoneId
|
||||
import java.time.ZonedDateTime
|
||||
|
||||
/**
|
||||
* The My Fitness dashboard renders before Health Connect's per-session metrics have arrived —
|
||||
* each of those costs its own IPC, so waiting for all of them is what made the screen sit on a
|
||||
* spinner. These tests pin what that first pass is allowed to look like.
|
||||
*
|
||||
* The contract has two halves, and the screen depends on both:
|
||||
* - everything not derived from a metric must already be final, so no visible number *changes*
|
||||
* when the metrics land — it only gains cells;
|
||||
* - a metric that is not known yet must read as absent, never as zero, so the screen hides its
|
||||
* cell instead of claiming the user ran 0 km.
|
||||
*/
|
||||
class PartialMetricsReportTest {
|
||||
private val zone: ZoneId = ZoneId.of("UTC")
|
||||
|
||||
/** Noon UTC so a test never straddles a day boundary by accident. */
|
||||
private val now: Instant = ZonedDateTime.of(2026, 3, 15, 12, 0, 0, 0, zone).toInstant()
|
||||
|
||||
private var nextId = 0
|
||||
|
||||
private fun workout(
|
||||
daysAgo: Long,
|
||||
exercise: ExerciseType = ExerciseType.RUNNING,
|
||||
durationSeconds: Long = 1800,
|
||||
distanceMeters: Double? = null,
|
||||
calories: Int? = null,
|
||||
avgHeartRate: Int? = null,
|
||||
maxHeartRate: Int? = null,
|
||||
steps: Int? = null,
|
||||
elevationGainMeters: Double? = null,
|
||||
) = DetectedWorkout(
|
||||
id = "w${nextId++}",
|
||||
exercise = exercise,
|
||||
title = null,
|
||||
startTimeEpochSeconds = now.epochSecond - daysAgo * 86_400L,
|
||||
durationSeconds = durationSeconds,
|
||||
distanceMeters = distanceMeters,
|
||||
calories = calories,
|
||||
avgHeartRate = avgHeartRate,
|
||||
maxHeartRate = maxHeartRate,
|
||||
steps = steps,
|
||||
elevationGainMeters = elevationGainMeters,
|
||||
source = "Samsung Health",
|
||||
)
|
||||
|
||||
/** What the first pass has: activity, when, how long. Nothing that needs an aggregate call. */
|
||||
private fun DetectedWorkout.withoutMetrics() =
|
||||
copy(
|
||||
distanceMeters = null,
|
||||
calories = null,
|
||||
avgHeartRate = null,
|
||||
maxHeartRate = null,
|
||||
steps = null,
|
||||
elevationGainMeters = null,
|
||||
)
|
||||
|
||||
private val fullyLoaded =
|
||||
listOf(
|
||||
workout(daysAgo = 0, durationSeconds = 3600, distanceMeters = 10_000.0, calories = 700, avgHeartRate = 150, maxHeartRate = 175, steps = 9_000, elevationGainMeters = 120.0),
|
||||
workout(daysAgo = 1, durationSeconds = 1800, distanceMeters = 5_000.0, calories = 320, avgHeartRate = 140, maxHeartRate = 160, steps = 4_500),
|
||||
workout(daysAgo = 2, exercise = ExerciseType.CYCLING, durationSeconds = 5400, distanceMeters = 40_000.0, calories = 900, avgHeartRate = 130, maxHeartRate = 155, elevationGainMeters = 400.0),
|
||||
workout(daysAgo = 9, exercise = ExerciseType.STRENGTH, durationSeconds = 2700, calories = 250),
|
||||
)
|
||||
|
||||
private val partial = fullyLoaded.map { it.withoutMetrics() }
|
||||
|
||||
@Test
|
||||
fun `counts, time and consistency are already final before the metrics arrive`() {
|
||||
val early = WorkoutStats.report(partial, now, zone)
|
||||
val late = WorkoutStats.report(fullyLoaded, now, zone)
|
||||
|
||||
assertEquals(late.windowTotals.workoutCount, early.windowTotals.workoutCount)
|
||||
assertEquals(late.windowTotals.durationSeconds, early.windowTotals.durationSeconds)
|
||||
assertEquals(late.thisWeek.workoutCount, early.thisWeek.workoutCount)
|
||||
assertEquals(late.thisWeek.durationSeconds, early.thisWeek.durationSeconds)
|
||||
assertEquals(late.previousWeek.workoutCount, early.previousWeek.workoutCount)
|
||||
assertEquals(late.weeklyAverage.durationSeconds, early.weeklyAverage.durationSeconds)
|
||||
assertEquals(late.activeDays, early.activeDays)
|
||||
assertEquals(late.currentStreakDays, early.currentStreakDays)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the activity split is already final, in the same order`() {
|
||||
val early = WorkoutStats.report(partial, now, zone)
|
||||
val late = WorkoutStats.report(fullyLoaded, now, zone)
|
||||
|
||||
assertEquals(late.byActivity.map { it.exercise }, early.byActivity.map { it.exercise })
|
||||
assertEquals(
|
||||
late.byActivity.map { it.totals.workoutCount },
|
||||
early.byActivity.map { it.totals.workoutCount },
|
||||
)
|
||||
assertEquals(
|
||||
late.byActivity.map { it.totals.durationSeconds },
|
||||
early.byActivity.map { it.totals.durationSeconds },
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an unknown metric reads as absent, so the screen hides its cell instead of showing zero`() {
|
||||
val early = WorkoutStats.report(partial, now, zone)
|
||||
|
||||
// The screen gates these cells on `> 0` / a non-null, so absent is what keeps them hidden.
|
||||
assertEquals(0.0, early.windowTotals.distanceMeters, 0.0)
|
||||
assertEquals(0, early.windowTotals.calories)
|
||||
assertEquals(0, early.windowTotals.steps)
|
||||
assertEquals(0.0, early.windowTotals.elevationGainMeters, 0.0)
|
||||
assertNull(early.windowTotals.avgHeartRate)
|
||||
assertNull(early.windowTotals.maxHeartRate)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `only the duration best is offered before the metrics arrive`() {
|
||||
val early = WorkoutStats.report(partial, now, zone)
|
||||
val late = WorkoutStats.report(fullyLoaded, now, zone)
|
||||
|
||||
// A best whose metric is unknown is withheld rather than awarded to whichever workout
|
||||
// happens to have a null — the cycling ride below is the real longest distance.
|
||||
assertEquals(listOf(WorkoutStats.BestKind.LONGEST_DURATION), early.bests.map { it.kind })
|
||||
|
||||
assertTrue(late.bests.map { it.kind }.containsAll(early.bests.map { it.kind }))
|
||||
assertEquals(
|
||||
late.bests
|
||||
.first { it.kind == WorkoutStats.BestKind.LONGEST_DURATION }
|
||||
.workout.id,
|
||||
early.bests
|
||||
.first { it.kind == WorkoutStats.BestKind.LONGEST_DURATION }
|
||||
.workout.id,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a partial pass with workouts in it is not mistaken for an empty log`() {
|
||||
val early = WorkoutStats.report(partial, now, zone)
|
||||
|
||||
// isEmpty drives the "nothing logged yet" copy and the connect prompt. A user with four
|
||||
// workouts whose metrics are still loading must not see either.
|
||||
assertFalse(early.isEmpty)
|
||||
assertEquals(fullyLoaded.size, early.workouts.size)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `merging both sources still dedupes when the health connect copy has no metrics yet`() {
|
||||
val published =
|
||||
fullyLoaded.take(1).map {
|
||||
it.copy(id = "published", origin = WorkoutOrigin.PUBLISHED, alreadyPublished = true)
|
||||
}
|
||||
|
||||
// Dedupe keys on activity and start time, neither of which is a metric, so the first pass
|
||||
// must already collapse the pair rather than double-count it and then halve the count.
|
||||
val merged = TrainingLog.merge(partial, published)
|
||||
|
||||
assertEquals(partial.size, merged.size)
|
||||
assertEquals(1, merged.count { it.alreadyPublished })
|
||||
}
|
||||
}
|
||||
@@ -716,6 +716,7 @@
|
||||
<string name="health_connect_rationale_privacy_policy">Read the full privacy policy</string>
|
||||
<string name="my_fitness_title">My Fitness</string>
|
||||
<string name="my_fitness_window">Last 4 weeks</string>
|
||||
<string name="my_fitness_loading_metrics">Loading distance, calories and heart rate…</string>
|
||||
<string name="my_fitness_window_note">The last four whole weeks of your training: the workouts you have published, plus anything Health Connect records on this phone. Amethyst works the summary out on the device and publishes none of it.</string>
|
||||
<string name="my_fitness_this_week">This week</string>
|
||||
<string name="my_fitness_vs_last_week">vs last week</string>
|
||||
|
||||
Reference in New Issue
Block a user