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.
2026-07-31 07:44:24 +02:00 · 2026-05-27 10:20:07 +02:00
parent 35d926ce4e
commit 7832f8a4a8
2 changed files with 222 additions and 25 deletions
@@ -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<ActivityPoint>,
+        workoutStartInstant: Instant,
+        workoutEndInstant: Instant,
+        deviceName: String
+    ): ExerciseRoute? {
+        return try {
+            val locations = ArrayList<ExerciseRoute.Location>()
+            val seenInstants = HashSet<Instant>()
+            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,
@@ -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)
+    }
+}