From 829f9b7c316280c28d77f3ac06d58dae86f4de74 Mon Sep 17 00:00:00 2001 From: Gideon Zenz Date: Tue, 2 Jun 2026 17:51:59 +0200 Subject: [PATCH] Health Connect: Recover late-arriving step samples The ACTIVITY sync cursor is a single (deviceId, dataType) timestamp shared by the steps, active-calories and distance syncers. The cursor advances to the latest record produced by any of them. When a device delivers calorie/distance detail for the most recent minutes before the matching step detail, the cursor moves past those minutes on the calorie front; the step minutes that arrive on the next sync are then clipped by the slice start boundary and lost forever. Resetting the HC sync state recovered them, confirming the data was present in the database but skipped. Give the per-minute steps, calories and distance records a deterministic clientRecordId (gb-{type}-{manufacturer}-{model}-{endEpochSecond}) so that re-emitting a record upserts instead of duplicating, mirroring SleepSyncer. With records now idempotent, extend the per-syncer slice lower bound backwards by a one-hour lookback so the trailing window is re-emitted on the next sync and the late minutes are recovered. Heart rate is a grouped series keyed on its start time, cannot be deduped this way, and does not extend the shared base, so it keeps strict boundaries and never re-emits. --- .../util/healthconnect/HealthConnectUtils.kt | 2 + .../syncers/AbstractActivitySampleSyncer.kt | 20 ++++- .../syncers/ActiveCaloriesSyncer.kt | 2 +- .../healthconnect/syncers/DistanceSyncer.kt | 2 +- .../syncers/HealthConnectSyncer.kt | 17 +++++ .../util/healthconnect/syncers/StepsSyncer.kt | 2 +- .../ActivitySampleSliceBoundaryTest.kt | 75 +++++++++++++++++++ 7 files changed, 116 insertions(+), 4 deletions(-) create mode 100644 app/src/test/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/ActivitySampleSliceBoundaryTest.kt diff --git a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/HealthConnectUtils.kt b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/HealthConnectUtils.kt index 0996df08e2..eb16791032 100755 --- a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/HealthConnectUtils.kt +++ b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/HealthConnectUtils.kt @@ -50,6 +50,7 @@ import java.time.Instant import java.time.ZoneId import java.time.ZonedDateTime import java.time.format.DateTimeFormatter +import java.time.temporal.ChronoUnit import java.util.* import java.util.function.BiConsumer import kotlin.math.pow @@ -410,6 +411,7 @@ class HealthConnectUtils { val sliceTotalSynced = sliceStats.sumOf { it.recordsSynced } val latestRecordTs = sliceStats.mapNotNull { it.latestRecordTimestamp }.maxOrNull() + if (latestRecordTs != null && latestRecordTs.isAfter(timestampToPersistForThisDataType)) { timestampToPersistForThisDataType = latestRecordTs } else if (sliceTotalSynced == 0) { diff --git a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/AbstractActivitySampleSyncer.kt b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/AbstractActivitySampleSyncer.kt index 670f28c7a4..92e9f59e09 100644 --- a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/AbstractActivitySampleSyncer.kt +++ b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/AbstractActivitySampleSyncer.kt @@ -24,6 +24,7 @@ import nodomain.freeyourgadget.gadgetbridge.impl.GBDevice import nodomain.freeyourgadget.gadgetbridge.model.ActivitySample import nodomain.freeyourgadget.gadgetbridge.util.healthconnect.HealthConnectUtils import org.slf4j.Logger +import java.time.Duration import java.time.Instant import java.time.ZoneId import java.time.ZoneOffset @@ -34,6 +35,23 @@ internal abstract class AbstractActivitySampleSyncer : Activit protected abstract val logger: Logger protected abstract val recordClass: KClass + // Re-emit samples this far before the slice start. The ACTIVITY cursor is shared across + // steps/calories/distance and advances to the furthest delivered; when step or distance detail + // trails the calorie summary, those minutes would be clipped and lost. The per-minute + // clientRecordId makes the re-emitted overlap an upsert. Heart rate is a series keyed on its + // start time, does not extend this base, and keeps strict boundaries. + protected open val lateSampleLookback: Duration = Duration.ofHours(1) + + internal fun isWithinSlice( + endTs: Instant, + startTs: Instant, + sliceStartBoundary: Instant, + sliceEndBoundary: Instant + ): Boolean { + val effectiveStart = sliceStartBoundary.minus(lateSampleLookback) + return !endTs.isBefore(effectiveStart) && !startTs.isAfter(sliceEndBoundary) + } + internal abstract fun convertSample( sample: ActivitySample, offset: ZoneOffset, @@ -76,7 +94,7 @@ internal abstract class AbstractActivitySampleSyncer : Activit val endTs = Instant.ofEpochSecond(currentSample.timestamp.toLong()) val startTs = endTs.minus(1, ChronoUnit.MINUTES) - if (endTs.isBefore(sliceStartBoundary) || startTs.isAfter(sliceEndBoundary)) { + if (!isWithinSlice(endTs, startTs, sliceStartBoundary, sliceEndBoundary)) { logger.trace( "Skipping {} for device '{}' for sample at {} (interval {} to {}) as its interval is outside the slice {} - {}.", recordTypeName, diff --git a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/ActiveCaloriesSyncer.kt b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/ActiveCaloriesSyncer.kt index 3a7f5ab2b6..0eb468e51a 100644 --- a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/ActiveCaloriesSyncer.kt +++ b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/ActiveCaloriesSyncer.kt @@ -56,7 +56,7 @@ internal object ActiveCaloriesSyncer : AbstractActivitySampleSyncer() endTime = endTs, endZoneOffset = offset, distance = Length.meters(distanceCm / 100.0), - metadata = metadata + metadata = clientRecordMetadata(metadata, "distance", endTs.epochSecond, distanceCm.toLong()) ) } } diff --git a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/HealthConnectSyncer.kt b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/HealthConnectSyncer.kt index 1ea2f76293..75fabc079d 100644 --- a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/HealthConnectSyncer.kt +++ b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/HealthConnectSyncer.kt @@ -24,6 +24,23 @@ import nodomain.freeyourgadget.gadgetbridge.model.ActivitySample import java.time.Instant import java.time.ZoneId +// Deterministic clientRecordId so re-emitting an overlapping window upserts instead of duplicating. +// version is the clientRecordVersion; HC keeps the highest on conflict, so a later, more complete +// value wins. +internal fun clientRecordMetadata( + base: Metadata, + type: String, + idKey: Long, + version: Long +): Metadata { + val device = base.device ?: return base + val id = "gb-$type-${device.manufacturer ?: "unknown"}-${device.model ?: "unknown"}-$idKey" + return when (base.recordingMethod) { + Metadata.RECORDING_METHOD_ACTIVELY_RECORDED -> Metadata.activelyRecorded(device, id, version) + else -> Metadata.autoRecorded(device, id, version) + } +} + /** * Statistics returned by a syncer after processing a slice. */ diff --git a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/StepsSyncer.kt b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/StepsSyncer.kt index 09f0327136..f224c26bff 100644 --- a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/StepsSyncer.kt +++ b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/StepsSyncer.kt @@ -55,7 +55,7 @@ internal object StepsSyncer : AbstractActivitySampleSyncer() { endTs, offset, stepsInMinute, - metadata + clientRecordMetadata(metadata, "steps", endTs.epochSecond, stepsInMinute) ) } } diff --git a/app/src/test/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/ActivitySampleSliceBoundaryTest.kt b/app/src/test/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/ActivitySampleSliceBoundaryTest.kt new file mode 100644 index 0000000000..d895f30066 --- /dev/null +++ b/app/src/test/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/ActivitySampleSliceBoundaryTest.kt @@ -0,0 +1,75 @@ +package nodomain.freeyourgadget.gadgetbridge.util.healthconnect.syncers + +import androidx.health.connect.client.records.StepsRecord +import androidx.health.connect.client.records.metadata.Metadata +import nodomain.freeyourgadget.gadgetbridge.model.ActivitySample +import org.junit.Assert.* +import org.junit.Test +import java.time.Instant +import java.time.ZoneOffset +import kotlin.reflect.KClass +import org.slf4j.Logger +import org.slf4j.LoggerFactory + +class ActivitySampleSliceBoundaryTest { + + private val sliceStart: Instant = Instant.ofEpochSecond(1_700_000_000L) + private val sliceEnd: Instant = sliceStart.plusSeconds(14 * 60) + + // Concrete syncer with the default 1h lookback (mirrors Steps/Calories/Distance). + private object IdempotentSyncer : AbstractActivitySampleSyncer() { + override val logger: Logger = LoggerFactory.getLogger("test") + override val recordClass: KClass = StepsRecord::class + override fun convertSample(sample: ActivitySample, offset: ZoneOffset, metadata: Metadata, deviceName: String): StepsRecord? = null + } + + // Syncer that opts out of the lookback (mirrors a non-idempotent record type). + private object StrictSyncer : AbstractActivitySampleSyncer() { + override val logger: Logger = LoggerFactory.getLogger("test") + override val recordClass: KClass = StepsRecord::class + override val lateSampleLookback = java.time.Duration.ZERO + override fun convertSample(sample: ActivitySample, offset: ZoneOffset, metadata: Metadata, deviceName: String): StepsRecord? = null + } + + private fun within(syncer: AbstractActivitySampleSyncer<*>, endTs: Instant): Boolean = + syncer.isWithinSlice(endTs, endTs.minusSeconds(60), sliceStart, sliceEnd) + + @Test + fun sampleInsideSlice_emitted() { + assertTrue(within(IdempotentSyncer, sliceStart.plusSeconds(300))) + } + + @Test + fun lateSampleWithinLookback_recoveredByIdempotentSyncer() { + // 30 min before the slice start: clipped without lookback, recovered with it. + val lateTs = sliceStart.minusSeconds(30 * 60) + assertTrue(within(IdempotentSyncer, lateTs)) + } + + @Test + fun lateSampleWithinLookback_clippedByStrictSyncer() { + val lateTs = sliceStart.minusSeconds(30 * 60) + assertFalse(within(StrictSyncer, lateTs)) + } + + @Test + fun sampleOlderThanLookback_clipped() { + // 90 min before start is beyond the 1h window. + val tooOld = sliceStart.minusSeconds(90 * 60) + assertFalse(within(IdempotentSyncer, tooOld)) + } + + @Test + fun sampleAfterSliceEnd_clippedRegardlessOfLookback() { + // Upper bound is never relaxed: a sample whose interval starts after slice end is dropped. + val afterEnd = sliceEnd.plusSeconds(120) + assertFalse(within(IdempotentSyncer, afterEnd)) + } + + @Test + fun sampleStraddlingSliceEnd_emitted() { + // endTs just past sliceEnd but startTs (endTs-60s) still within: kept. + val straddle = sliceEnd.plusSeconds(30) + assertTrue(within(IdempotentSyncer, straddle)) + } +}