mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-06 03:38:23 +00:00
fix: replace the whole form when a second workout suggestion is picked
Three defects found auditing the Health Connect workout path.
applyPrefill only assigned a field when the incoming route carried a value,
so picking a second suggestion merged into the first instead of replacing it.
Tapping a run (5 km, 380 kcal) and then a gym session left the run's distance
and calories in the form — the user publishes numbers from a workout that
never happened. Every metric is now assigned on both branches. Notes are
deliberately left alone: they are typed by the user, never carried by a route.
The `source` tag published the writing app's display label ("Samsung Health"),
or its raw package name when that app is not installed. SourceTag defines a
vocabulary — gps / manual / health_connect — and the feed badge uppercases
whatever is in the tag, so an imported workout rendered as "SAMSUNG HEALTH"
or "COM.HUAWEI.HEALTH" beside other clients' "GPS". It now publishes
SourceTag.HEALTH_CONNECT, which existed for this and had no callers.
DetectedWorkout.source keeps the friendly name for the UI.
readNewWorkouts ran entirely on Dispatchers.Main: the callers launch into a
composable's rememberCoroutineScope, and nothing in the feature switched
dispatcher. Health Connect's own calls suspend, but resolveSourceName's
PackageManager lookup is a blocking binder call made once per session, so a
week of sessions blocked the UI thread once each. The read now runs on
Dispatchers.IO and the label lookups are memoized per writer package.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019egdJyBHnrATZHjs86up8f
This commit is contained in:
+43
-24
@@ -36,6 +36,8 @@ import androidx.health.connect.client.request.ReadRecordsRequest
|
||||
import androidx.health.connect.client.time.TimeRangeFilter
|
||||
import com.vitorpamplona.quartz.utils.Log
|
||||
import kotlinx.coroutines.CancellationException
|
||||
import kotlinx.coroutines.Dispatchers
|
||||
import kotlinx.coroutines.withContext
|
||||
import java.time.Duration
|
||||
import java.time.Instant
|
||||
import kotlin.math.roundToInt
|
||||
@@ -54,6 +56,9 @@ class HealthConnectManager(
|
||||
) {
|
||||
private val client: HealthConnectClient by lazy { HealthConnectClient.getOrCreate(context) }
|
||||
|
||||
/** Writer package -> display label. See [resolveSourceName]. */
|
||||
private val sourceNames = mutableMapOf<String, String>()
|
||||
|
||||
companion object {
|
||||
private const val TAG = "HealthConnectManager"
|
||||
|
||||
@@ -132,25 +137,31 @@ class HealthConnectManager(
|
||||
return emptyList()
|
||||
}
|
||||
|
||||
return try {
|
||||
val response =
|
||||
client.readRecords(
|
||||
ReadRecordsRequest(
|
||||
recordType = ExerciseSessionRecord::class,
|
||||
timeRangeFilter = TimeRangeFilter.between(since, now),
|
||||
),
|
||||
)
|
||||
Log.i(TAG) { "readNewWorkouts: ${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) { "readNewWorkouts: 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()
|
||||
// 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) { "readNewWorkouts: ${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) { "readNewWorkouts: 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()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -208,11 +219,19 @@ class HealthConnectManager(
|
||||
*/
|
||||
private fun resolveSourceName(packageName: String): String {
|
||||
if (packageName.isBlank()) return DEFAULT_SOURCE
|
||||
runCatching {
|
||||
val pm = context.packageManager
|
||||
return pm.getApplicationLabel(pm.getApplicationInfo(packageName, 0)).toString()
|
||||
}
|
||||
return KNOWN_SOURCES[packageName] ?: packageName
|
||||
// Memoized: getApplicationInfo is a blocking binder call, and a week of sessions
|
||||
// almost always comes from the same one or two writer apps, so an uncached lookup
|
||||
// pays for the same round trip once per session.
|
||||
sourceNames[packageName]?.let { return it }
|
||||
|
||||
val resolved =
|
||||
runCatching {
|
||||
val pm = context.packageManager
|
||||
pm.getApplicationLabel(pm.getApplicationInfo(packageName, 0)).toString()
|
||||
}.getOrNull() ?: KNOWN_SOURCES[packageName] ?: packageName
|
||||
|
||||
sourceNames[packageName] = resolved
|
||||
return resolved
|
||||
}
|
||||
|
||||
/** Aggregates the optional metrics over the session window. Null if aggregation fails. */
|
||||
|
||||
+12
-2
@@ -93,20 +93,30 @@ class NewWorkoutViewModel : ViewModel() {
|
||||
*/
|
||||
fun applyPrefill(route: Route.NewWorkout) {
|
||||
route.exercise?.let { ExerciseType.parse(it) }?.let { exercise = it }
|
||||
route.title?.let { title = it }
|
||||
title = route.title ?: ""
|
||||
|
||||
// Every metric is assigned on both branches. Picking a second suggestion has to
|
||||
// REPLACE the form, not merge into it: a gym session carries no distance, so
|
||||
// leaving the previous run's 5 km in the field would publish a workout that
|
||||
// never happened.
|
||||
if (route.durationSeconds > 0) {
|
||||
hours = (route.durationSeconds / 3600).toString()
|
||||
minutes = ((route.durationSeconds % 3600) / 60).toString()
|
||||
seconds = (route.durationSeconds % 60).toString()
|
||||
} else {
|
||||
hours = ""
|
||||
minutes = ""
|
||||
seconds = ""
|
||||
}
|
||||
if (route.distanceMeters > 0) {
|
||||
val miles = phonePrefersMiles()
|
||||
distanceUnit = if (miles) DistanceTag.MILES else DistanceTag.KILOMETERS
|
||||
val value = if (miles) route.distanceMeters / DistanceTag.METERS_PER_MILE else route.distanceMeters / 1000.0
|
||||
distance = ((value * 100).roundToInt() / 100.0).toString()
|
||||
} else {
|
||||
distance = ""
|
||||
}
|
||||
if (route.calories > 0) calories = route.calories.toString()
|
||||
calories = if (route.calories > 0) route.calories.toString() else ""
|
||||
|
||||
route.source?.let { source = it }
|
||||
avgHeartRate = route.avgHeartRate
|
||||
|
||||
+6
-1
@@ -23,6 +23,7 @@ package com.vitorpamplona.amethyst.ui.screen.loggedIn.workouts.suggestion
|
||||
import android.text.format.DateUtils
|
||||
import com.vitorpamplona.amethyst.service.workouts.health.DetectedWorkout
|
||||
import com.vitorpamplona.amethyst.ui.navigation.routes.Route
|
||||
import com.vitorpamplona.quartz.experimental.fitness.workout.tags.SourceTag
|
||||
|
||||
/** Builds the pre-filled composer route for a detected workout. Shared by the
|
||||
* feed suggestion banner and the New Workout carousel so they never drift. */
|
||||
@@ -38,7 +39,11 @@ internal fun DetectedWorkout.toNewWorkoutRoute(title: String) =
|
||||
steps = steps ?: 0,
|
||||
elevationGainMeters = elevationGainMeters ?: 0.0,
|
||||
startTime = startTimeEpochSeconds,
|
||||
source = source,
|
||||
// The `source` tag carries the NIP-101e vocabulary token, not the writing app's
|
||||
// label: the badge in the feed renders it uppercased next to "GPS" and "MANUAL",
|
||||
// and other clients match on the token. DetectedWorkout.source stays the friendly
|
||||
// name for the UI to show.
|
||||
source = SourceTag.HEALTH_CONNECT,
|
||||
)
|
||||
|
||||
internal fun formatWorkoutDuration(totalSeconds: Long): String {
|
||||
|
||||
+114
@@ -0,0 +1,114 @@
|
||||
/*
|
||||
* 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.ui.screen.loggedIn.workouts
|
||||
|
||||
import com.vitorpamplona.amethyst.service.workouts.health.DetectedWorkout
|
||||
import com.vitorpamplona.amethyst.ui.navigation.routes.Route
|
||||
import com.vitorpamplona.amethyst.ui.screen.loggedIn.workouts.suggestion.toNewWorkoutRoute
|
||||
import com.vitorpamplona.quartz.experimental.fitness.workout.tags.ExerciseType
|
||||
import com.vitorpamplona.quartz.experimental.fitness.workout.tags.SourceTag
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Test
|
||||
|
||||
class NewWorkoutPrefillTest {
|
||||
private fun route(
|
||||
exercise: ExerciseType,
|
||||
title: String,
|
||||
durationSeconds: Long,
|
||||
distanceMeters: Double = 0.0,
|
||||
calories: Int = 0,
|
||||
source: String = "Samsung Health",
|
||||
) = Route.NewWorkout(
|
||||
exercise = exercise.code,
|
||||
title = title,
|
||||
durationSeconds = durationSeconds,
|
||||
distanceMeters = distanceMeters,
|
||||
calories = calories,
|
||||
avgHeartRate = 0,
|
||||
maxHeartRate = 0,
|
||||
steps = 0,
|
||||
elevationGainMeters = 0.0,
|
||||
startTime = 1_700_000_000L,
|
||||
source = source,
|
||||
)
|
||||
|
||||
/**
|
||||
* Picking a second suggestion must replace the form, not merge into it. A gym
|
||||
* session has no distance and no calories; if the run's numbers survive, the
|
||||
* user publishes a workout that never happened.
|
||||
*/
|
||||
@Test
|
||||
fun `picking a second workout clears metrics the new one does not have`() {
|
||||
val vm = NewWorkoutViewModel()
|
||||
|
||||
vm.applyPrefill(route(ExerciseType.RUNNING, "Morning Run", 1800, distanceMeters = 5000.0, calories = 380))
|
||||
assertEquals("380", vm.calories)
|
||||
|
||||
vm.applyPrefill(route(ExerciseType.STRENGTH, "Gym", 2400))
|
||||
|
||||
assertEquals(ExerciseType.STRENGTH, vm.exercise)
|
||||
assertEquals("Gym", vm.title)
|
||||
assertEquals("", vm.distance)
|
||||
assertEquals("", vm.calories)
|
||||
}
|
||||
|
||||
/**
|
||||
* A Health Connect import publishes the NIP-101e vocabulary token, not the writing
|
||||
* app's label: the feed badge uppercases whatever is in the tag, so a raw label
|
||||
* renders as "SAMSUNG HEALTH" (or "COM.HUAWEI.HEALTH" when the app is not installed)
|
||||
* next to other clients' "GPS" and "MANUAL".
|
||||
*/
|
||||
@Test
|
||||
fun `a detected workout carries the health_connect source token`() {
|
||||
val detected =
|
||||
DetectedWorkout(
|
||||
id = "abc",
|
||||
exercise = ExerciseType.RUNNING,
|
||||
title = "Morning Run",
|
||||
startTimeEpochSeconds = 1_700_000_000L,
|
||||
durationSeconds = 1800,
|
||||
distanceMeters = 5000.0,
|
||||
calories = 380,
|
||||
avgHeartRate = 150,
|
||||
maxHeartRate = 172,
|
||||
steps = 6000,
|
||||
elevationGainMeters = 40.0,
|
||||
source = "Samsung Health",
|
||||
)
|
||||
|
||||
assertEquals(SourceTag.HEALTH_CONNECT, detected.toNewWorkoutRoute("Morning Run").source)
|
||||
}
|
||||
|
||||
/** Duration must switch too, not keep the previous workout's clock. */
|
||||
@Test
|
||||
fun `picking a second workout replaces the duration`() {
|
||||
val vm = NewWorkoutViewModel()
|
||||
|
||||
vm.applyPrefill(route(ExerciseType.RUNNING, "Morning Run", 3661))
|
||||
assertEquals("1", vm.hours)
|
||||
|
||||
vm.applyPrefill(route(ExerciseType.YOGA, "Yoga", 600))
|
||||
|
||||
assertEquals("0", vm.hours)
|
||||
assertEquals("10", vm.minutes)
|
||||
assertEquals("0", vm.seconds)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user