From 7832f8a4a81a9de24b885fd6a00b565a4853a458 Mon Sep 17 00:00:00 2001 From: Gideon Zenz Date: Wed, 27 May 2026 10:20:07 +0200 Subject: [PATCH] Health Connect: Sanitise GPS route to prevent ExerciseRoute crash ExerciseRoute requires strictly increasing timestamps; duplicate-second points from device GPS parsers (e.g. Xiaomi Smart Band 9 Pro outdoor cycling) caused IllegalArgumentException at construction time, aborting the workout sync entirely. Extract route construction into buildSanitisedRoute, which: - drops points outside the workout time window - drops points with non-finite or out-of-range lat/lng (HC's Location constructor enforces lat in [-90, 90], lng in [-180, 180]) - drops points with non-finite or negative hdop/vdop accuracy (HC rejects negative accuracy) - deduplicates points by Instant - returns null if fewer than two usable points remain - catches IllegalArgumentException as a safety net so future HC invariants degrade gracefully (workout saves without route rather than the whole sync aborting) Logs dropped points with the [HC_SYNC] prefix so users can grep. --- .../syncers/RecordedWorkoutSyncer.kt | 100 +++++++++--- .../syncers/RecordedWorkoutSyncerRouteTest.kt | 147 ++++++++++++++++++ 2 files changed, 222 insertions(+), 25 deletions(-) create mode 100644 app/src/test/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/RecordedWorkoutSyncerRouteTest.kt diff --git a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/RecordedWorkoutSyncer.kt b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/RecordedWorkoutSyncer.kt index 66ebae9cc1..e2b72bff88 100644 --- a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/RecordedWorkoutSyncer.kt +++ b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/RecordedWorkoutSyncer.kt @@ -302,32 +302,8 @@ internal object RecordedWorkoutSyncer { ) { val deviceName = device.aliasOrName - // Build GPS route if location data is available and permission is granted val exerciseRoute = if (PERMISSION_WRITE_EXERCISE_ROUTE in grantedPermissions) { - val locationPoints = activityPoints - .filter { it.location != null && it.time != null } - .mapNotNull { point -> - val pointInstant = point.time.toInstant() - if (pointInstant.isBefore(workoutStartInstant) || pointInstant.isAfter(workoutEndInstant)) { - return@mapNotNull null - } - val location = point.location ?: return@mapNotNull null - ExerciseRoute.Location( - time = pointInstant, - latitude = location.latitude, - longitude = location.longitude, - altitude = if (location.hasAltitude()) Length.meters(location.altitude) else null, - horizontalAccuracy = if (location.hasHdop()) Length.meters(location.hdop) else null, - verticalAccuracy = if (location.hasVdop()) Length.meters(location.vdop) else null - ) - } - - if (locationPoints.isNotEmpty()) { - LOG.info("Adding GPS route with ${locationPoints.size} location points for workout on device '$deviceName'.") - ExerciseRoute(locationPoints) - } else { - null - } + buildSanitisedRoute(activityPoints, workoutStartInstant, workoutEndInstant, deviceName) } else { null } @@ -362,6 +338,80 @@ internal object RecordedWorkoutSyncer { } } + internal fun buildSanitisedRoute( + activityPoints: List, + workoutStartInstant: Instant, + workoutEndInstant: Instant, + deviceName: String + ): ExerciseRoute? { + return try { + val locations = ArrayList() + val seenInstants = HashSet() + var droppedOutOfWindow = 0 + var droppedInvalidCoords = 0 + var droppedDuplicateTime = 0 + + for (point in activityPoints) { + val time = point.time ?: continue + val location = point.location ?: continue + val pointInstant = time.toInstant() + if (pointInstant.isBefore(workoutStartInstant) || pointInstant.isAfter(workoutEndInstant)) { + droppedOutOfWindow++ + continue + } + val lat = location.latitude + val lng = location.longitude + if (!lat.isFinite() || !lng.isFinite() || lat !in -90.0..90.0 || lng !in -180.0..180.0) { + droppedInvalidCoords++ + continue + } + if (!seenInstants.add(pointInstant)) { + droppedDuplicateTime++ + continue + } + val altitude = if (location.hasAltitude() && location.altitude.isFinite()) { + Length.meters(location.altitude) + } else null + val hdop = if (location.hasHdop() && location.hdop.isFinite() && location.hdop >= 0) { + Length.meters(location.hdop) + } else null + val vdop = if (location.hasVdop() && location.vdop.isFinite() && location.vdop >= 0) { + Length.meters(location.vdop) + } else null + locations.add( + ExerciseRoute.Location( + time = pointInstant, + latitude = lat, + longitude = lng, + altitude = altitude, + horizontalAccuracy = hdop, + verticalAccuracy = vdop + ) + ) + } + + if (droppedOutOfWindow > 0 || droppedInvalidCoords > 0 || droppedDuplicateTime > 0) { + LOG.info( + "[HC_SYNC] GPS route sanitisation for device '{}': dropped {} out-of-window, {} invalid coords, {} duplicate timestamps; kept {}.", + deviceName, droppedOutOfWindow, droppedInvalidCoords, droppedDuplicateTime, locations.size + ) + } + + if (locations.size < 2) { + if (locations.isNotEmpty()) { + LOG.info("[HC_SYNC] GPS route for device '{}' has only {} usable point(s); skipping route.", deviceName, locations.size) + } + return null + } + + LOG.info("Adding GPS route with ${locations.size} location points for workout on device '$deviceName'.") + ExerciseRoute(locations) + } catch (e: IllegalArgumentException) { + LOG.error("[HC_SYNC] Failed to build GPS route for device '{}': {}. Skipping route.", deviceName, e.message) + null + } + } + private fun processAggregateWorkout( gbDevice: GBDevice, workout: BaseActivitySummary, diff --git a/app/src/test/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/RecordedWorkoutSyncerRouteTest.kt b/app/src/test/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/RecordedWorkoutSyncerRouteTest.kt new file mode 100644 index 0000000000..582c93a6ac --- /dev/null +++ b/app/src/test/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/RecordedWorkoutSyncerRouteTest.kt @@ -0,0 +1,147 @@ +package nodomain.freeyourgadget.gadgetbridge.util.healthconnect.syncers + +import nodomain.freeyourgadget.gadgetbridge.model.ActivityPoint +import nodomain.freeyourgadget.gadgetbridge.model.GPSCoordinate +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Test +import java.time.Instant +import java.util.Date + +class RecordedWorkoutSyncerRouteTest { + + private val start: Instant = Instant.parse("2026-05-26T21:10:31Z") + private val end: Instant = Instant.parse("2026-05-26T21:19:36Z") + private val device = "test-device" + + private fun point(secondsFromStart: Long, lat: Double = 52.5, lng: Double = 13.4): ActivityPoint { + val p = ActivityPoint(Date.from(start.plusSeconds(secondsFromStart))) + p.location = GPSCoordinate(lng, lat, GPSCoordinate.UNKNOWN_ALTITUDE) + return p + } + + @Test + fun emptyInput_returnsNull() { + val route = RecordedWorkoutSyncer.buildSanitisedRoute(emptyList(), start, end, device) + assertNull(route) + } + + @Test + fun singlePoint_returnsNull() { + val route = RecordedWorkoutSyncer.buildSanitisedRoute(listOf(point(0)), start, end, device) + assertNull(route) + } + + @Test + fun twoUniqueTimestamps_returnsRoute() { + val route = RecordedWorkoutSyncer.buildSanitisedRoute( + listOf(point(0), point(1)), start, end, device + ) + assertNotNull(route) + assertEquals(2, route!!.route.size) + } + + @Test + fun duplicateTimestamps_areDeduped() { + val route = RecordedWorkoutSyncer.buildSanitisedRoute( + listOf(point(0), point(0, lat = 52.6), point(1)), start, end, device + ) + assertNotNull(route) + assertEquals(2, route!!.route.size) + } + + @Test + fun allSameTimestamp_returnsNull() { + val route = RecordedWorkoutSyncer.buildSanitisedRoute( + listOf(point(5), point(5), point(5)), start, end, device + ) + assertNull(route) + } + + @Test + fun reverseOrderTimestamps_routeBuilt() { + val route = RecordedWorkoutSyncer.buildSanitisedRoute( + listOf(point(10), point(5), point(0)), start, end, device + ) + assertNotNull(route) + assertEquals(3, route!!.route.size) + } + + @Test + fun outOfWindow_dropped() { + val before = ActivityPoint(Date.from(start.minusSeconds(5))).apply { + location = GPSCoordinate(13.4, 52.5, GPSCoordinate.UNKNOWN_ALTITUDE) + } + val after = ActivityPoint(Date.from(end.plusSeconds(5))).apply { + location = GPSCoordinate(13.4, 52.5, GPSCoordinate.UNKNOWN_ALTITUDE) + } + val route = RecordedWorkoutSyncer.buildSanitisedRoute( + listOf(before, point(1), point(2), after), start, end, device + ) + assertNotNull(route) + assertEquals(2, route!!.route.size) + } + + @Test + fun nanLatitude_dropped() { + val route = RecordedWorkoutSyncer.buildSanitisedRoute( + listOf(point(0, lat = Double.NaN), point(1), point(2)), + start, end, device + ) + assertNotNull(route) + assertEquals(2, route!!.route.size) + } + + @Test + fun outOfRangeLatitude_dropped() { + val route = RecordedWorkoutSyncer.buildSanitisedRoute( + listOf(point(0, lat = 95.0), point(1), point(2)), + start, end, device + ) + assertNotNull(route) + assertEquals(2, route!!.route.size) + } + + @Test + fun outOfRangeLongitude_dropped() { + val route = RecordedWorkoutSyncer.buildSanitisedRoute( + listOf(point(0, lng = 200.0), point(1), point(2)), + start, end, device + ) + assertNotNull(route) + assertEquals(2, route!!.route.size) + } + + @Test + fun infiniteLongitude_dropped() { + val route = RecordedWorkoutSyncer.buildSanitisedRoute( + listOf(point(0, lng = Double.POSITIVE_INFINITY), point(1), point(2)), + start, end, device + ) + assertNotNull(route) + assertEquals(2, route!!.route.size) + } + + @Test + fun pointWithNullLocation_dropped() { + val noLoc = ActivityPoint(Date.from(start.plusSeconds(0))) + val route = RecordedWorkoutSyncer.buildSanitisedRoute( + listOf(noLoc, point(1), point(2)), start, end, device + ) + assertNotNull(route) + assertEquals(2, route!!.route.size) + } + + @Test + fun pointWithNullTime_dropped() { + val noTime = ActivityPoint().apply { + location = GPSCoordinate(13.4, 52.5, GPSCoordinate.UNKNOWN_ALTITUDE) + } + val route = RecordedWorkoutSyncer.buildSanitisedRoute( + listOf(noTime, point(1), point(2)), start, end, device + ) + assertNotNull(route) + assertEquals(2, route!!.route.size) + } +}