From b5b0b585cc912678c04f547dbb71a523bfefca45 Mon Sep 17 00:00:00 2001 From: Gideon Zenz Date: Mon, 2 Mar 2026 20:17:31 +0100 Subject: [PATCH] Fix #5797: Advance HC sync cursor by last-written record timestamp The sync state cursor was advancing to the slice end timestamp unconditionally, even when no records were written. Late-arriving data (sleep, workouts, VO2Max, etc.) that arrives after the cursor has already advanced past it would be permanently skipped. Each syncer now reports the latest timestamp of records actually inserted into Health Connect. The sync cursor advances to max(currentPersisted, latestRecordTimestamp) instead of sliceEndTimestamp. If no records were written for a slice, the cursor stays put. This handles all late-arriving data generically without needing per-data-type special-casing. --- .../util/healthconnect/HealthConnectUtils.kt | 14 +- .../syncers/AbstractActivitySampleSyncer.kt | 9 +- .../syncers/AbstractTimeSampleSyncer.kt | 16 ++- .../syncers/HealthConnectSyncer.kt | 3 +- .../healthconnect/syncers/HeartRateSync.kt | 2 +- .../syncers/RecordedWorkoutSyncer.kt | 7 +- .../util/healthconnect/syncers/SleepSyncer.kt | 3 +- .../SyncTimestampAdvancementTest.kt | 125 ++++++++++++++++++ 8 files changed, 166 insertions(+), 13 deletions(-) create mode 100644 app/src/test/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/SyncTimestampAdvancementTest.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 b1352e3355..2a23698f5e 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 @@ -396,7 +396,19 @@ class HealthConnectUtils { recordsSyncedByType[key] = currentSynced + stat.recordsSynced } - timestampToPersistForThisDataType = currentSliceEndTs + 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) { + LOG.debug( + "$HC_SYNC_TAG Holding sync state for {}({}) at {} (no records synced in slice {} to {})", + gbDevice.aliasOrName, dataType.name, timestampToPersistForThisDataType, + currentSliceStartTs, currentSliceEndTs + ) + } + currentSliceStartTs = currentSliceEndTs } catch (e: SyncException) { LOG.warn( 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 91f43480d5..f486f01e94 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 @@ -70,12 +70,11 @@ internal abstract class AbstractActivitySampleSyncer : Activit val recordsToInsert = mutableListOf() var skippedCount = 0 + var latestSyncedTimestamp: Instant? = null relevantSamples.forEach { currentSample -> val endTs = Instant.ofEpochSecond(currentSample.timestamp.toLong()) val startTs = endTs.minus(1, ChronoUnit.MINUTES) - // Use inclusive boundaries [sliceStart, sliceEnd] for the slice - // Overlap check: interval overlaps if endTs > sliceStart AND startTs <= sliceEnd if (endTs.isBefore(sliceStartBoundary) || startTs.isAfter(sliceEndBoundary)) { logger.debug( "Skipping {} for device '{}' for sample at {} (interval {} to {}) as its interval is outside the slice {} - {}.", @@ -96,6 +95,9 @@ internal abstract class AbstractActivitySampleSyncer : Activit return@forEach } recordsToInsert.add(record) + if (latestSyncedTimestamp == null || endTs.isAfter(latestSyncedTimestamp)) { + latestSyncedTimestamp = endTs + } } // 3. No Valid Records to Insert @@ -114,7 +116,8 @@ internal abstract class AbstractActivitySampleSyncer : Activit return SyncerStatistics( recordsSynced = recordsToInsert.size, recordsSkipped = skippedCount, - recordType = recordTypeName + recordType = recordTypeName, + latestRecordTimestamp = latestSyncedTimestamp ) } } diff --git a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/AbstractTimeSampleSyncer.kt b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/AbstractTimeSampleSyncer.kt index 16715c45e5..c446a41dde 100644 --- a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/AbstractTimeSampleSyncer.kt +++ b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/AbstractTimeSampleSyncer.kt @@ -99,10 +99,10 @@ internal abstract class AbstractTimeSampleSyncer convertSample( - it, + sample, offset, metadata, deviceName - ) ?: run { + )?.also { + val ts = Instant.ofEpochMilli(sample.timestamp) + if (latestSyncedTimestamp == null || ts.isAfter(latestSyncedTimestamp)) { + latestSyncedTimestamp = ts + } + } ?: run { skippedCount++ null } @@ -138,7 +143,8 @@ internal abstract class AbstractTimeSampleSyncer() + var latestWorkoutEndTs: Instant? = null for (workout in workouts) { try { @@ -187,6 +188,10 @@ internal object RecordedWorkoutSyncer { HealthConnectUtils.insertRecords(recordsToInsert, healthConnectClient) LOG.info("Successfully inserted ${recordsToInsert.size} record(s) for workout (Type: ${activityKind}, Start: $workoutStartInstant) for device '$deviceName'.") workoutRecordList.addAll(recordsToInsert) + val currentEnd = workoutEndInstant + if (latestWorkoutEndTs == null || currentEnd.isAfter(latestWorkoutEndTs)) { + latestWorkoutEndTs = currentEnd + } } catch (e: Exception) { LOG.error("Error processing workout for device '$deviceName'", e) // Continue with next workout instead of failing entire sync @@ -205,7 +210,7 @@ internal object RecordedWorkoutSyncer { } LOG.info("Successfully inserted ${workoutRecordList.size} ExerciseSessionRecord(s) for device '$deviceName' for slice $sliceStartBoundary to $sliceEndBoundary.") - return SyncerStatistics(recordsSynced = workoutRecordList.size, recordType = "Workout") + return SyncerStatistics(recordsSynced = workoutRecordList.size, recordType = "Workout", latestRecordTimestamp = latestWorkoutEndTs) } diff --git a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/SleepSyncer.kt b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/SleepSyncer.kt index 83e1c1d6ba..992f82d2b0 100644 --- a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/SleepSyncer.kt +++ b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/SleepSyncer.kt @@ -101,7 +101,8 @@ internal object SleepSyncer : ContextualActivitySampleSyncer { LOG.info("Attempting to insert ${sleepSessionRecordList.size} SleepSessionRecord(s) for device '$deviceName' for slice $sliceStartBoundary to $sliceEndBoundary.") HealthConnectUtils.insertRecords(sleepSessionRecordList, healthConnectClient) LOG.info("Successfully inserted SleepSessionRecord(s) for device '$deviceName' for slice $sliceStartBoundary to $sliceEndBoundary.") - return SyncerStatistics(recordsSynced = sleepSessionRecordList.size, recordsSkipped = skippedCount, recordType = "Sleep") + val latestTs = sleepSessionRecordList.maxOfOrNull { it.endTime } + return SyncerStatistics(recordsSynced = sleepSessionRecordList.size, recordsSkipped = skippedCount, recordType = "Sleep", latestRecordTimestamp = latestTs) } /** diff --git a/app/src/test/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/SyncTimestampAdvancementTest.kt b/app/src/test/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/SyncTimestampAdvancementTest.kt new file mode 100644 index 0000000000..e7a8ea1a5a --- /dev/null +++ b/app/src/test/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/SyncTimestampAdvancementTest.kt @@ -0,0 +1,125 @@ +package nodomain.freeyourgadget.gadgetbridge.util.healthconnect + +import nodomain.freeyourgadget.gadgetbridge.util.healthconnect.syncers.SyncerStatistics +import org.junit.Assert.* +import org.junit.Test +import java.time.Instant + +class SyncTimestampAdvancementTest { + + private val baseEpoch = 1_700_000_000L + private val baseInstant: Instant = Instant.ofEpochSecond(baseEpoch) + + @Test + fun latestRecordTimestamp_advancesCursor_whenPresent() { + val stats = listOf( + SyncerStatistics(recordsSynced = 3, recordType = "Steps", latestRecordTimestamp = baseInstant.plusSeconds(3600)) + ) + val latestRecordTs = stats.mapNotNull { it.latestRecordTimestamp }.maxOrNull() + + assertNotNull(latestRecordTs) + assertTrue(latestRecordTs!!.isAfter(baseInstant)) + } + + @Test + fun cursorDoesNotAdvance_whenNoRecordsSynced() { + val stats = listOf( + SyncerStatistics(recordsSynced = 0, recordType = "Sleep") + ) + val latestRecordTs = stats.mapNotNull { it.latestRecordTimestamp }.maxOrNull() + + assertNull(latestRecordTs) + } + + @Test + fun multipleStats_usesMaxTimestamp() { + val earlier = baseInstant.plusSeconds(1000) + val later = baseInstant.plusSeconds(5000) + val stats = listOf( + SyncerStatistics(recordsSynced = 2, recordType = "Steps", latestRecordTimestamp = earlier), + SyncerStatistics(recordsSynced = 5, recordType = "HeartRate", latestRecordTimestamp = later), + SyncerStatistics(recordsSynced = 1, recordType = "Calories", latestRecordTimestamp = earlier) + ) + val latestRecordTs = stats.mapNotNull { it.latestRecordTimestamp }.maxOrNull() + + assertEquals(later, latestRecordTs) + } + + @Test + fun mixedNullAndNonNull_usesNonNull() { + val ts = baseInstant.plusSeconds(7200) + val stats = listOf( + SyncerStatistics(recordsSynced = 0, recordType = "Steps"), + SyncerStatistics(recordsSynced = 1, recordType = "HeartRate", latestRecordTimestamp = ts), + SyncerStatistics(recordsSynced = 0, recordType = "Calories") + ) + val latestRecordTs = stats.mapNotNull { it.latestRecordTimestamp }.maxOrNull() + + assertEquals(ts, latestRecordTs) + } + + @Test + fun allNullTimestamps_cursorStaysAtCurrentValue() { + val currentPersisted = baseInstant + val stats = listOf( + SyncerStatistics(recordsSynced = 0, recordType = "Sleep"), + SyncerStatistics(recordsSynced = 0, recordType = "Workout") + ) + val latestRecordTs = stats.mapNotNull { it.latestRecordTimestamp }.maxOrNull() + val newPersisted = if (latestRecordTs != null && latestRecordTs.isAfter(currentPersisted)) { + latestRecordTs + } else { + currentPersisted + } + + assertEquals(currentPersisted, newPersisted) + } + + @Test + fun cursorDoesNotGoBackward_whenLatestIsBeforeCurrent() { + val currentPersisted = baseInstant.plusSeconds(10000) + val olderTs = baseInstant.plusSeconds(5000) + val stats = listOf( + SyncerStatistics(recordsSynced = 1, recordType = "Steps", latestRecordTimestamp = olderTs) + ) + val latestRecordTs = stats.mapNotNull { it.latestRecordTimestamp }.maxOrNull() + val newPersisted = if (latestRecordTs != null && latestRecordTs.isAfter(currentPersisted)) { + latestRecordTs + } else { + currentPersisted + } + + assertEquals(currentPersisted, newPersisted) + } + + @Test + fun emptyStatsList_cursorStays() { + val currentPersisted = baseInstant + val stats = emptyList() + val latestRecordTs = stats.mapNotNull { it.latestRecordTimestamp }.maxOrNull() + val newPersisted = if (latestRecordTs != null && latestRecordTs.isAfter(currentPersisted)) { + latestRecordTs + } else { + currentPersisted + } + + assertEquals(currentPersisted, newPersisted) + } + + @Test + fun defaultSyncerStatistics_hasNullTimestamp() { + val stats = SyncerStatistics() + assertNull(stats.latestRecordTimestamp) + } + + @Test + fun syncerStatistics_preservesTimestampInCopy() { + val ts = Instant.ofEpochSecond(baseEpoch + 999) + val original = SyncerStatistics(recordsSynced = 1, recordType = "HRV", latestRecordTimestamp = ts) + val copy = original.copy(recordsSynced = 2) + + assertEquals(ts, copy.latestRecordTimestamp) + assertEquals(2, copy.recordsSynced) + } +} +