From 8be39c99ea761c4960cbbdf3a6e9aad33763bbb1 Mon Sep 17 00:00:00 2001 From: Gideon Zenz Date: Fri, 5 Jun 2026 23:19:45 +0200 Subject: [PATCH] Health Connect: Advance sync cursor for temperature records TemperatureSyncer is the only syncer implementing HealthConnectSyncer directly instead of inheriting from AbstractTimeSampleSyncer or AbstractActivitySampleSyncer, both of which populate latestRecordTimestamp in their base sync(). Its hand-rolled sync() never set the field, so the orchestrator had nothing to advance the persisted cursor with and the TEMPERATURE cursor stayed pinned at its starting value. Every sync then re-read from that frozen point to now and re-inserted the whole span; the per-record clientRecordId dedup hid the duplicates in Health Connect, so it was silent but re-inserted weeks of records on each run. Return the furthest-forward edge of the inserted records (body record time, skin record endTime) as latestRecordTimestamp on the success path. The no-op early returns keep it null, which correctly holds the cursor, matching every other syncer's "nothing synced" behaviour. The cursor self-heals on the next successful sync; no reset needed. --- .../syncers/TemperatureSyncer.kt | 22 ++++- .../syncers/TemperatureSyncerCursorTest.kt | 85 +++++++++++++++++++ 2 files changed, 106 insertions(+), 1 deletion(-) create mode 100644 app/src/test/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/TemperatureSyncerCursorTest.kt diff --git a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/TemperatureSyncer.kt b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/TemperatureSyncer.kt index 54314928ba..729ca0fd90 100755 --- a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/TemperatureSyncer.kt +++ b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/TemperatureSyncer.kt @@ -207,7 +207,27 @@ internal object TemperatureSyncer : HealthConnectSyncer { LOG.info("Successfully inserted TemperatureRecord(s) (Body/Skin) for '$deviceName' for slice $sliceStartBoundary to $sliceEndBoundary.") LOG.info("Temperature sync completed for device '$deviceName' for slice $sliceStartBoundary to $sliceEndBoundary. Total synced: $totalRecordsSynced") - return SyncerStatistics(recordsSynced = totalRecordsSynced, recordsSkipped = totalRecordsSkipped, recordType = "Temperature") + return buildStatistics(recordsToInsert, totalRecordsSkipped) + } + + // Builds the slice result, including latestRecordTimestamp. The orchestrator only advances the + // persisted sync cursor from that field; omitting it freezes the cursor and re-inserts the whole + // span every run. Body records are point-in-time, skin records are intervals, so the cursor takes + // the furthest forward edge of either kind. + internal fun buildStatistics(insertedRecords: List, recordsSkipped: Int): SyncerStatistics { + val latest = insertedRecords.mapNotNull { + when (it) { + is BodyTemperatureRecord -> it.time + is SkinTemperatureRecord -> it.endTime + else -> null + } + }.maxOrNull() + return SyncerStatistics( + recordsSynced = insertedRecords.size, + recordsSkipped = recordsSkipped, + recordType = "Temperature", + latestRecordTimestamp = latest + ) } // Helper class to manage dynamic baseline calculation per device diff --git a/app/src/test/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/TemperatureSyncerCursorTest.kt b/app/src/test/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/TemperatureSyncerCursorTest.kt new file mode 100644 index 0000000000..1a3186ced7 --- /dev/null +++ b/app/src/test/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/TemperatureSyncerCursorTest.kt @@ -0,0 +1,85 @@ +package nodomain.freeyourgadget.gadgetbridge.util.healthconnect.syncers + +import androidx.health.connect.client.records.BodyTemperatureRecord +import androidx.health.connect.client.records.Record +import androidx.health.connect.client.records.SkinTemperatureRecord +import androidx.health.connect.client.records.metadata.Device +import androidx.health.connect.client.records.metadata.Metadata +import androidx.health.connect.client.units.Temperature +import androidx.health.connect.client.units.TemperatureDelta +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test +import java.time.Instant +import java.time.ZoneOffset + +// Guards the cursor-advance contract the orchestrator depends on: when temperature records are +// inserted, buildStatistics must carry the furthest-forward record timestamp, or the sync cursor +// freezes and re-inserts the whole span every run. Asserting on the returned SyncerStatistics keeps +// the test failing if the field is ever dropped from buildStatistics. +// +// Boundary: this covers buildStatistics, not sync()'s one-line delegation to it. Re-inlining sync() +// to build SyncerStatistics directly and bypass buildStatistics would reintroduce the bug without +// failing here; that wire is guarded by review, since exercising sync() needs static GBApplication/ +// HealthConnectClient mocks this suite does not carry. +class TemperatureSyncerCursorTest { + + private val base: Instant = Instant.ofEpochSecond(1_700_000_000L) + private val metadata: Metadata = Metadata.unknownRecordingMethod( + Device(type = Device.TYPE_WATCH, manufacturer = "test", model = "test") + ) + + private fun body(at: Instant) = BodyTemperatureRecord( + time = at, + zoneOffset = ZoneOffset.UTC, + temperature = Temperature.celsius(36.5), + metadata = metadata + ) + + private fun skin(start: Instant, end: Instant) = SkinTemperatureRecord( + startTime = start, + startZoneOffset = ZoneOffset.UTC, + endTime = end, + endZoneOffset = ZoneOffset.UTC, + deltas = listOf(SkinTemperatureRecord.Delta(start, TemperatureDelta.celsius(0.1))), + baseline = Temperature.celsius(33.0), + metadata = metadata + ) + + @Test + fun noRecords_holdsCursor() { + val stats = TemperatureSyncer.buildStatistics(emptyList(), recordsSkipped = 0) + assertNull(stats.latestRecordTimestamp) + assertEquals(0, stats.recordsSynced) + } + + @Test + fun bodyRecords_advanceToLatestTime() { + val records: List = listOf( + body(base.plusSeconds(60)), + body(base.plusSeconds(3600)), + body(base.plusSeconds(120)) + ) + val stats = TemperatureSyncer.buildStatistics(records, recordsSkipped = 0) + assertEquals(base.plusSeconds(3600), stats.latestRecordTimestamp) + assertEquals(3, stats.recordsSynced) + assertEquals("Temperature", stats.recordType) + } + + @Test + fun skinRecord_advancesToEndTime() { + val records: List = listOf(skin(base.plusSeconds(100), base.plusSeconds(900))) + val stats = TemperatureSyncer.buildStatistics(records, recordsSkipped = 0) + assertEquals(base.plusSeconds(900), stats.latestRecordTimestamp) + } + + @Test + fun mixedBodyAndSkin_advanceToFurthestForwardEdge() { + val records: List = listOf( + body(base.plusSeconds(500)), + skin(base.plusSeconds(100), base.plusSeconds(7200)) + ) + val stats = TemperatureSyncer.buildStatistics(records, recordsSkipped = 0) + assertEquals(base.plusSeconds(7200), stats.latestRecordTimestamp) + } +}