From d4c36e2e363fadea2b91c5cef2e2f4334ed9e612 Mon Sep 17 00:00:00 2001 From: Gideon Zenz Date: Sat, 13 Jun 2026 11:13:37 +0200 Subject: [PATCH] Health Connect: Exclude bad-measurement sentinels from HR and SpO2 sync PR #6195 widened the syncer guards to match Health Connect's enforced bounds, but widened too much: it overlooked that some of Gadgetbridge's in-band sentinel values fall inside HC's accepted range, so placeholder readings leaked into Health Connect and showed up as spurious data points. - HeartRate / RestingHeartRate: 255 is GB's "illegal value" marker for a bad measurement (ActivitySample.getHeartRate), and it sits inside HC's 1..300 / 0..300 range. Exclude it explicitly while keeping HC's bounds. - SpO2: 0 means "not measured" (sample providers return null on 0 and all SpO2 charts filter getSpo2() > 0). Raise the lower bound to 1. Add HeartRateSyncerTest covering the HeartRateSyncer.sync() path (sentinel 255 and 0 are not inserted; valid range passes) and extend SyncerRangeValidationTest with the new boundaries. --- .../healthconnect/syncers/HeartRateSync.kt | 7 +- .../syncers/RestingHeartRateSyncer.kt | 5 +- .../util/healthconnect/syncers/Spo2Syncer.kt | 5 +- .../syncers/HeartRateSyncerTest.kt | 119 ++++++++++++++++++ .../syncers/SyncerRangeValidationTest.kt | 12 +- 5 files changed, 140 insertions(+), 8 deletions(-) create mode 100644 app/src/test/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/HeartRateSyncerTest.kt diff --git a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/HeartRateSync.kt b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/HeartRateSync.kt index 8f12b208fc..e01d3002ec 100644 --- a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/HeartRateSync.kt +++ b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/HeartRateSync.kt @@ -52,11 +52,12 @@ internal object HeartRateSyncer : ActivitySampleSyncer { return SyncerStatistics(recordType = "HeartRate") } - // 2. Relevant Input Data Check (HC enforces 1..300 bpm) + // 2. Relevant Input Data Check. HC enforces 1..300 bpm; 255 is GB's documented + // "illegal value" sentinel (ActivitySample.getHeartRate) and lands inside that range. var droppedOutOfRange = 0 val validHRSamples = deviceSamples .filter { - val inRange = it.heartRate in 1..300 + val inRange = it.heartRate in 1..300 && it.heartRate != 255 // 0 means "not measured" - common, don't count as out-of-range if (!inRange && it.heartRate != 0) { droppedOutOfRange++ @@ -66,7 +67,7 @@ internal object HeartRateSyncer : ActivitySampleSyncer { .sortedBy { it.timestamp } if (droppedOutOfRange > 0) { LOG.info( - "${HealthConnectUtils.HC_SYNC_TAG} Dropped {} out-of-range HeartRate sample(s) for device '{}' in slice {} to {} (HC requires 1..300 bpm).", + "${HealthConnectUtils.HC_SYNC_TAG} Dropped {} out-of-range HeartRate sample(s) for device '{}' in slice {} to {} (HC requires 1..300 bpm, 255 excluded as bad-measurement sentinel).", droppedOutOfRange, deviceName, sliceStartBoundary, sliceEndBoundary ) } diff --git a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/RestingHeartRateSyncer.kt b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/RestingHeartRateSyncer.kt index e02a4e3267..211a7a901f 100644 --- a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/RestingHeartRateSyncer.kt +++ b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/RestingHeartRateSyncer.kt @@ -45,8 +45,9 @@ internal object RestingHeartRateSyncer : AbstractTimeSampleSyncer 0); exclude it. + if (spo2AsDouble !in 1.0..100.0 || !spo2AsDouble.isFinite()) { + logger.skipOutOfRange(deviceName, "SpO2", spo2AsDouble, "1..100 %") return null } diff --git a/app/src/test/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/HeartRateSyncerTest.kt b/app/src/test/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/HeartRateSyncerTest.kt new file mode 100644 index 0000000000..f84ddeca1f --- /dev/null +++ b/app/src/test/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/HeartRateSyncerTest.kt @@ -0,0 +1,119 @@ +package nodomain.freeyourgadget.gadgetbridge.util.healthconnect.syncers + +import androidx.health.connect.client.HealthConnectClient +import androidx.health.connect.client.PermissionController +import androidx.health.connect.client.aggregate.AggregationResult +import androidx.health.connect.client.aggregate.AggregationResultGroupedByDuration +import androidx.health.connect.client.aggregate.AggregationResultGroupedByPeriod +import androidx.health.connect.client.permission.HealthPermission +import androidx.health.connect.client.records.HeartRateRecord +import androidx.health.connect.client.records.Record +import androidx.health.connect.client.records.metadata.Device +import androidx.health.connect.client.records.metadata.Metadata +import androidx.health.connect.client.request.AggregateGroupByDurationRequest +import androidx.health.connect.client.request.AggregateGroupByPeriodRequest +import androidx.health.connect.client.request.AggregateRequest +import androidx.health.connect.client.request.ChangesTokenRequest +import androidx.health.connect.client.request.ReadRecordsRequest +import androidx.health.connect.client.response.ChangesResponse +import androidx.health.connect.client.response.InsertRecordsResponse +import androidx.health.connect.client.response.ReadRecordResponse +import androidx.health.connect.client.response.ReadRecordsResponse +import androidx.health.connect.client.time.TimeRangeFilter +import kotlinx.coroutines.runBlocking +import nodomain.freeyourgadget.gadgetbridge.impl.GBDevice +import nodomain.freeyourgadget.gadgetbridge.model.ActivityKind +import nodomain.freeyourgadget.gadgetbridge.model.ActivitySample +import nodomain.freeyourgadget.gadgetbridge.model.DeviceType +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test +import java.time.Instant +import java.time.ZoneId +import kotlin.reflect.KClass + +class HeartRateSyncerTest { + + private val zoneId: ZoneId = ZoneId.of("UTC") + private val gbDevice = GBDevice("00:11:22:33:44:55", "Testie", "Testie Alias", "Test Folder", DeviceType.TEST) + private val metadata: Metadata = Metadata.unknownRecordingMethod( + Device(type = Device.TYPE_WATCH, manufacturer = "test", model = "test") + ) + private val grantedPermissions = setOf(HealthPermission.getWritePermission(HeartRateRecord::class)) + + private fun hrSample(ts: Int, bpm: Int): ActivitySample = + object : ActivitySample { + override fun getTimestamp(): Int = ts + override fun getProvider(): nodomain.freeyourgadget.gadgetbridge.devices.SampleProvider<*>? = null + override fun getRawKind(): Int = ActivityKind.UNKNOWN.code + override fun getKind(): ActivityKind = ActivityKind.UNKNOWN + override fun getRawIntensity(): Int = 0 + override fun getIntensity(): Float = 0f + override fun getSteps(): Int = 0 + override fun getDistanceCm(): Int = 0 + override fun getActiveCalories(): Int = 0 + override fun getHeartRate(): Int = bpm + override fun setHeartRate(value: Int) {} + } + + private fun syncBpm(values: List): List { + val baseTs = 1_700_000_000 + val samples = values.mapIndexed { i, bpm -> hrSample(baseTs + i * 60, bpm) } + val start = Instant.ofEpochSecond(baseTs.toLong()) + val end = Instant.ofEpochSecond(baseTs.toLong() + values.size * 60L) + val client = CapturingClient() + runBlocking { + HeartRateSyncer.sync(client, gbDevice, metadata, zoneId, start, end, grantedPermissions, samples) + } + return client.inserted + .filterIsInstance() + .flatMap { it.samples } + .map { it.beatsPerMinute } + } + + @Test + fun sentinel255_isNotSynced() { + val bpm = syncBpm(listOf(60, 255, 62)) + assertTrue("255 sentinel must not reach Health Connect", 255L !in bpm) + assertEquals(listOf(60L, 62L), bpm) + } + + @Test + fun zero_isNotSynced() { + val bpm = syncBpm(listOf(60, 0, 62)) + assertEquals(listOf(60L, 62L), bpm) + } + + @Test + fun validRange_isSynced() { + val bpm = syncBpm(listOf(1, 300)) + assertEquals(listOf(1L, 300L), bpm) + } + + @Test + fun aboveHcLimit_isNotSynced() { + val bpm = syncBpm(listOf(60, 301, 62)) + assertEquals(listOf(60L, 62L), bpm) + } + + private class CapturingClient : HealthConnectClient { + val inserted = mutableListOf() + + override suspend fun insertRecords(records: List): InsertRecordsResponse { + inserted.addAll(records) + return InsertRecordsResponse(records.map { "id" }) + } + + override val permissionController: PermissionController get() = throw NotImplementedError() + override suspend fun updateRecords(records: List) = throw NotImplementedError() + override suspend fun deleteRecords(recordType: KClass, recordIdsList: List, clientRecordIdsList: List) = throw NotImplementedError() + override suspend fun deleteRecords(recordType: KClass, timeRangeFilter: TimeRangeFilter) = throw NotImplementedError() + override suspend fun readRecord(recordType: KClass, recordId: String): ReadRecordResponse = throw NotImplementedError() + override suspend fun readRecords(request: ReadRecordsRequest): ReadRecordsResponse = throw NotImplementedError() + override suspend fun aggregate(request: AggregateRequest): AggregationResult = throw NotImplementedError() + override suspend fun aggregateGroupByDuration(request: AggregateGroupByDurationRequest): List = throw NotImplementedError() + override suspend fun aggregateGroupByPeriod(request: AggregateGroupByPeriodRequest): List = throw NotImplementedError() + override suspend fun getChangesToken(request: ChangesTokenRequest): String = throw NotImplementedError() + override suspend fun getChanges(changesToken: String): ChangesResponse = throw NotImplementedError() + } +} diff --git a/app/src/test/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/SyncerRangeValidationTest.kt b/app/src/test/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/SyncerRangeValidationTest.kt index 8a30477f8b..1294a7948f 100644 --- a/app/src/test/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/SyncerRangeValidationTest.kt +++ b/app/src/test/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/SyncerRangeValidationTest.kt @@ -158,7 +158,12 @@ class SyncerRangeValidationTest { @Test fun spo2_lowerBoundary_accepted() { - assertNotNull(Spo2Syncer.convertSample(spo2Sample(0), offset, metadata, device)) + assertNotNull(Spo2Syncer.convertSample(spo2Sample(1), offset, metadata, device)) + } + + @Test + fun spo2_zero_dropped() { + assertNull(Spo2Syncer.convertSample(spo2Sample(0), offset, metadata, device)) } @Test @@ -199,6 +204,11 @@ class SyncerRangeValidationTest { assertNull(RestingHeartRateSyncer.convertSample(hrSample(301), offset, metadata, device)) } + @Test + fun restingHr_sentinel255_dropped() { + assertNull(RestingHeartRateSyncer.convertSample(hrSample(255), offset, metadata, device)) + } + @Test fun restingHr_negative_dropped() { assertNull(RestingHeartRateSyncer.convertSample(hrSample(-1), offset, metadata, device))