mirror of
https://codeberg.org/Freeyourgadget/Gadgetbridge.git
synced 2026-07-31 07:44:24 +02:00
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.
This commit is contained in:
+4
-3
@@ -52,11 +52,12 @@ internal object HeartRateSyncer : ActivitySampleSyncer {
|
|||||||
return SyncerStatistics(recordType = "HeartRate")
|
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
|
var droppedOutOfRange = 0
|
||||||
val validHRSamples = deviceSamples
|
val validHRSamples = deviceSamples
|
||||||
.filter {
|
.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
|
// 0 means "not measured" - common, don't count as out-of-range
|
||||||
if (!inRange && it.heartRate != 0) {
|
if (!inRange && it.heartRate != 0) {
|
||||||
droppedOutOfRange++
|
droppedOutOfRange++
|
||||||
@@ -66,7 +67,7 @@ internal object HeartRateSyncer : ActivitySampleSyncer {
|
|||||||
.sortedBy { it.timestamp }
|
.sortedBy { it.timestamp }
|
||||||
if (droppedOutOfRange > 0) {
|
if (droppedOutOfRange > 0) {
|
||||||
LOG.info(
|
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
|
droppedOutOfRange, deviceName, sliceStartBoundary, sliceEndBoundary
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
+3
-2
@@ -45,8 +45,9 @@ internal object RestingHeartRateSyncer : AbstractTimeSampleSyncer<HeartRateSampl
|
|||||||
metadata: Metadata,
|
metadata: Metadata,
|
||||||
deviceName: String
|
deviceName: String
|
||||||
): RestingHeartRateRecord? {
|
): RestingHeartRateRecord? {
|
||||||
if (sample.heartRate !in 0..300) {
|
// HC enforces 0..300 bpm; exclude 255, the bad-measurement sentinel GB uses for HR values.
|
||||||
logger.skipOutOfRange(deviceName, "RestingHeartRate", sample.heartRate, "0..300 bpm")
|
if (sample.heartRate !in 0..300 || sample.heartRate == 255) {
|
||||||
|
logger.skipOutOfRange(deviceName, "RestingHeartRate", sample.heartRate, "0..300 bpm (255 excluded)")
|
||||||
return null
|
return null
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+3
-2
@@ -48,8 +48,9 @@ internal object Spo2Syncer : AbstractTimeSampleSyncer<Spo2Sample, OxygenSaturati
|
|||||||
): OxygenSaturationRecord? {
|
): OxygenSaturationRecord? {
|
||||||
val spo2AsDouble = sample.spo2.toDouble()
|
val spo2AsDouble = sample.spo2.toDouble()
|
||||||
|
|
||||||
if (spo2AsDouble !in 0.0..100.0 || !spo2AsDouble.isFinite()) {
|
// 0 means "not measured" in GB (charts filter getSpo2() > 0); exclude it.
|
||||||
logger.skipOutOfRange(deviceName, "SpO2", spo2AsDouble, "0..100 %")
|
if (spo2AsDouble !in 1.0..100.0 || !spo2AsDouble.isFinite()) {
|
||||||
|
logger.skipOutOfRange(deviceName, "SpO2", spo2AsDouble, "1..100 %")
|
||||||
return null
|
return null
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+119
@@ -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<Int>): List<Long> {
|
||||||
|
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<HeartRateRecord>()
|
||||||
|
.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<Record>()
|
||||||
|
|
||||||
|
override suspend fun insertRecords(records: List<Record>): InsertRecordsResponse {
|
||||||
|
inserted.addAll(records)
|
||||||
|
return InsertRecordsResponse(records.map { "id" })
|
||||||
|
}
|
||||||
|
|
||||||
|
override val permissionController: PermissionController get() = throw NotImplementedError()
|
||||||
|
override suspend fun updateRecords(records: List<Record>) = throw NotImplementedError()
|
||||||
|
override suspend fun deleteRecords(recordType: KClass<out Record>, recordIdsList: List<String>, clientRecordIdsList: List<String>) = throw NotImplementedError()
|
||||||
|
override suspend fun deleteRecords(recordType: KClass<out Record>, timeRangeFilter: TimeRangeFilter) = throw NotImplementedError()
|
||||||
|
override suspend fun <T : Record> readRecord(recordType: KClass<T>, recordId: String): ReadRecordResponse<T> = throw NotImplementedError()
|
||||||
|
override suspend fun <T : Record> readRecords(request: ReadRecordsRequest<T>): ReadRecordsResponse<T> = throw NotImplementedError()
|
||||||
|
override suspend fun aggregate(request: AggregateRequest): AggregationResult = throw NotImplementedError()
|
||||||
|
override suspend fun aggregateGroupByDuration(request: AggregateGroupByDurationRequest): List<AggregationResultGroupedByDuration> = throw NotImplementedError()
|
||||||
|
override suspend fun aggregateGroupByPeriod(request: AggregateGroupByPeriodRequest): List<AggregationResultGroupedByPeriod> = throw NotImplementedError()
|
||||||
|
override suspend fun getChangesToken(request: ChangesTokenRequest): String = throw NotImplementedError()
|
||||||
|
override suspend fun getChanges(changesToken: String): ChangesResponse = throw NotImplementedError()
|
||||||
|
}
|
||||||
|
}
|
||||||
+11
-1
@@ -158,7 +158,12 @@ class SyncerRangeValidationTest {
|
|||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun spo2_lowerBoundary_accepted() {
|
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
|
@Test
|
||||||
@@ -199,6 +204,11 @@ class SyncerRangeValidationTest {
|
|||||||
assertNull(RestingHeartRateSyncer.convertSample(hrSample(301), offset, metadata, device))
|
assertNull(RestingHeartRateSyncer.convertSample(hrSample(301), offset, metadata, device))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun restingHr_sentinel255_dropped() {
|
||||||
|
assertNull(RestingHeartRateSyncer.convertSample(hrSample(255), offset, metadata, device))
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun restingHr_negative_dropped() {
|
fun restingHr_negative_dropped() {
|
||||||
assertNull(RestingHeartRateSyncer.convertSample(hrSample(-1), offset, metadata, device))
|
assertNull(RestingHeartRateSyncer.convertSample(hrSample(-1), offset, metadata, device))
|
||||||
|
|||||||
Reference in New Issue
Block a user