Rewrite HeartRateListener to better handle watches that don't regularely send heart rate events.

Make ActionHeartRate and HeartRate consistent.

app-wear-shared/shared/src/main/java/info/nightscout/rx/weardata/EventData.kt

@@ -3,8 +3,8 @@ package info.nightscout.rx.weardata
 import info.nightscout.rx.events.Event
 import kotlinx.serialization.Serializable
 import kotlinx.serialization.json.Json
-import java.util.*
+import org.joda.time.DateTime
-import kotlin.collections.ArrayList
+import java.util.Objects
 
 @Serializable
 sealed class EventData : Event() {
@@ -93,11 +93,12 @@ sealed class EventData : Event() {
 
     @Serializable
     data class ActionHeartRate(
-        val samplingStartMillis: Long,
+        val duration: Long,
-        val samplingEndMillis: Long,
+        val timestamp: Long,
-        val beatsPerMinute: Int,
+        val beatsPerMinute: Double,
         val device: String): EventData() {
-        override fun toString() = "HR $beatsPerMinute [${Date(samplingStartMillis)}..${Date(samplingEndMillis)}] $device"
+        override fun toString() =
+            "HR ${beatsPerMinute.toInt()} at ${DateTime(timestamp)} for ${duration / 1000.0}sec $device"
     }
 
     @Serializable

plugins/main/src/main/java/info/nightscout/plugins/general/wear/wearintegration/DataHandlerMobile.kt

@@ -1241,10 +1241,10 @@ class DataHandlerMobile @Inject constructor(
     private fun handleHeartRate(actionHeartRate: EventData.ActionHeartRate) {
         aapsLogger.debug(LTag.WEAR, "Heart rate received $actionHeartRate from ${actionHeartRate.sourceNodeId}")
         val hr = HeartRate(
-            duration = actionHeartRate.samplingEndMillis - actionHeartRate.samplingStartMillis,
+            duration = actionHeartRate.duration,
-            timestamp = actionHeartRate.samplingEndMillis,
+            timestamp = actionHeartRate.timestamp,
-            beatsPerMinute = actionHeartRate.beatsPerMinute.toDouble(),
+            beatsPerMinute = actionHeartRate.beatsPerMinute,
             device = actionHeartRate.device)
-        repository.runTransaction(InsertOrUpdateHeartRateTransaction(hr))
+        repository.runTransaction(InsertOrUpdateHeartRateTransaction(hr)).blockingAwait()
     }
 }

wear/src/main/java/info/nightscout/androidaps/heartrate/HeartRateListener.kt

@@ -9,22 +9,54 @@ import android.hardware.SensorManager
 import android.os.Build
 import androidx.annotation.VisibleForTesting
 import info.nightscout.androidaps.comm.IntentWearToMobile
+import info.nightscout.rx.AapsSchedulers
 import info.nightscout.rx.logging.AAPSLogger
 import info.nightscout.rx.logging.LTag
 import info.nightscout.rx.weardata.EventData
-import java.lang.Long.max
+import io.reactivex.rxjava3.disposables.Disposable
-import java.lang.Long.min
+import java.util.concurrent.TimeUnit
+import java.util.concurrent.locks.ReentrantLock
+import kotlin.concurrent.withLock
+import kotlin.math.roundToInt
 
 /**
- * Gets heart rate readings from watch and sends them once per minute to the phone.
+ * Gets heart rate readings from watch and sends them to the phone.
+ *
+ * The Android API doesn't define how often heart rate events are sent do the
+ * listener, it could be once per second or only when the heart rate changes.
+ *
+ * Heart rate is not a point in time measurement but is always sampled over a
+ * certain time, i.e. you count the the number of heart beats and divide by the
+ * minutes that have passed. Therefore, the provided value has to be for the past.
+ * However, we ignore this here.
+ *
+ * We don't need very exact values, but rather values that are easy to consume
+ * and don't produce too much data that would cause much battery consumption.
+ * Therefore, this class averages the heart rate over a minute ([samplingIntervalMillis])
+ * and sends this value to the phone.
+ *
+ * We will not always get valid values, e.g. if the watch is taken of. The listener
+ * ignores such time unless we don't get good values for more than 90% of time. Since
+ * heart rate doesn't change so fast this should be good enough.
  */
 class HeartRateListener(
     private val ctx: Context,
-    private val aapsLogger: AAPSLogger
+    private val aapsLogger: AAPSLogger,
-) :  SensorEventListener {
+    aapsSchedulers: AapsSchedulers,
+    now: Long = System.currentTimeMillis(),
+) :  SensorEventListener, Disposable {
 
+    /** How often we send values to the phone. */
     private val samplingIntervalMillis = 60_000L
-    private var sampler: Sampler? = null
+    private val sampler = Sampler(now)
+    private var schedule: Disposable? = null
+
+    /** We only use values with these accuracies and ignore NO_CONTACT and UNRELIABLE. */
+    private val goodAccuracies = arrayOf(
+        SensorManager.SENSOR_STATUS_ACCURACY_LOW,
+        SensorManager.SENSOR_STATUS_ACCURACY_MEDIUM,
+        SensorManager.SENSOR_STATUS_ACCURACY_HIGH,
+    )
 
     init {
         aapsLogger.info(LTag.WEAR, "Create ${javaClass.simpleName}")
@@ -39,71 +71,116 @@ class HeartRateListener(
                 sensorManager.registerListener(this, heartRateSensor, SensorManager.SENSOR_DELAY_NORMAL)
             }
         }
+        schedule = aapsSchedulers.io.schedulePeriodicallyDirect(
+            ::send, samplingIntervalMillis, samplingIntervalMillis, TimeUnit.MILLISECONDS)
     }
 
+    /**
+     * Gets the most recent heart rate reading and null if there is no valid
+     * value at the moment.
+     */
+    val currentHeartRateBpm get() = sampler.currentBpm?.roundToInt()
+
     @VisibleForTesting
     var sendHeartRate: (EventData.ActionHeartRate)->Unit = { hr -> ctx.startService(IntentWearToMobile(ctx, hr)) }
 
-    fun onDestroy() {
+    override fun isDisposed() = schedule == null
-        aapsLogger.info(LTag.WEAR, "Destroy ${javaClass.simpleName}")
+
+    override fun dispose() {
+        aapsLogger.info(LTag.WEAR, "Dispose ${javaClass.simpleName}")
+        schedule?.dispose()
         (ctx.getSystemService(SENSOR_SERVICE) as SensorManager?)?.unregisterListener(this)
     }
 
-    override fun onAccuracyChanged(sensor: Sensor, accuracy: Int) {
+    /** Sends currently sampled value to the phone. Executed every [samplingIntervalMillis]. */
+    private fun send() {
+        send(System.currentTimeMillis())
     }
 
-    private fun send(sampler: Sampler) {
+    @VisibleForTesting
-        sampler.heartRate.let { hr ->
+    fun send(timestampMillis: Long) {
+        sampler.getAndReset(timestampMillis)?.let { hr ->
             aapsLogger.info(LTag.WEAR, "Send heart rate $hr")
             sendHeartRate(hr)
         }
     }
 
+    override fun onAccuracyChanged(sensor: Sensor, accuracy: Int) {
+        onAccuracyChanged(sensor.type, accuracy, System.currentTimeMillis())
+    }
+
+    @VisibleForTesting
+    fun onAccuracyChanged(sensorType: Int, accuracy: Int, timestampMillis: Long) {
+        if (sensorType != Sensor.TYPE_HEART_RATE) {
+            aapsLogger.error(LTag.WEAR, "Invalid SensorEvent $sensorType $accuracy")
+            return
+        }
+        if (accuracy !in goodAccuracies) sampler.setHeartRate(timestampMillis, null)
+    }
+
     override fun onSensorChanged(event: SensorEvent) {
         onSensorChanged(event.sensor?.type, event.accuracy, System.currentTimeMillis(), event.values)
     }
 
     @VisibleForTesting
-    fun onSensorChanged(sensorType: Int?, accuracy: Int, timestamp: Long, values: FloatArray) {
+    fun onSensorChanged(sensorType: Int?, accuracy: Int, timestampMillis: Long, values: FloatArray) {
         if (sensorType == null || sensorType != Sensor.TYPE_HEART_RATE || values.isEmpty()) {
-            aapsLogger.error(LTag.WEAR, "Invalid SensorEvent $sensorType $accuracy $timestamp ${values.joinToString()}")
+            aapsLogger.error(LTag.WEAR, "Invalid SensorEvent $sensorType $accuracy $timestampMillis ${values.joinToString()}")
             return
         }
-        when (accuracy) {
+        val heartRate = values[0].toDouble().takeIf { accuracy in goodAccuracies }
-            SensorManager.SENSOR_STATUS_NO_CONTACT -> return
+        sampler.setHeartRate(timestampMillis, heartRate)
-            SensorManager.SENSOR_STATUS_UNRELIABLE -> return
-        }
-        val heartRate = values[0].toInt()
-        sampler = sampler.let { s ->
-            if (s == null || s.age(timestamp) !in 0..120_000 ) {
-                Sampler(timestamp, heartRate)
-            } else if (s.age(timestamp) >= samplingIntervalMillis) {
-                send(s)
-                Sampler(timestamp, heartRate)
-            } else {
-                s.addHeartRate(timestamp, heartRate)
-                s
-            }
-        }
     }
 
-    private class Sampler(timestampMillis: Long, heartRate: Int) {
+    private class Sampler(timestampMillis: Long) {
         private var startMillis: Long = timestampMillis
-        private var endMillis: Long = timestampMillis
+        private var lastEventMillis: Long = timestampMillis
-        private var valueCount: Int = 1
+        /** Number of heart beats sampled so far. */
-        private var valueSum: Int = heartRate
+        private var beats: Double = 0.0
+        /** Time we could sample valid values during the current sampling interval. */
+        private var activeMillis: Long = 0
         private val device = (Build.MANUFACTURER ?: "unknown") + " " + (Build.MODEL ?: "unknown")
+        private val lock = ReentrantLock()
 
-        val beatsPerMinute get() = valueSum / valueCount
+        var currentBpm: Double? = null
-        val heartRate get() = EventData.ActionHeartRate(startMillis, endMillis, beatsPerMinute, device)
+            get() = field
+            private set(value) { field = value }
 
-        fun age(timestampMillis: Long) = timestampMillis - startMillis
+        private fun Long.toMinute(): Double = this / 60_000.0
 
-        fun addHeartRate(timestampMillis: Long, heartRate: Int) {
+        private fun fix(timestampMillis: Long) {
-            startMillis = min(startMillis, timestampMillis)
+            currentBpm?.let { bpm ->
-            endMillis = max(endMillis, timestampMillis)
+                val elapsed = timestampMillis - lastEventMillis
-            valueCount++
+                beats += elapsed.toMinute() * bpm
-            valueSum += heartRate
+                activeMillis += elapsed
+            }
+            lastEventMillis = timestampMillis
+        }
+
+        /** Gets the current sampled value and resets the samplers clock to the given timestamp. */
+        fun getAndReset(timestampMillis: Long): EventData.ActionHeartRate? {
+            lock.withLock {
+                fix(timestampMillis)
+                return if (10 * activeMillis > lastEventMillis - startMillis) {
+                    val bpm = beats / activeMillis.toMinute()
+                    EventData.ActionHeartRate(timestampMillis - startMillis, timestampMillis, bpm, device)
+                } else {
+                    null
+                }.also {
+                    startMillis = timestampMillis
+                    lastEventMillis = timestampMillis
+                    beats = 0.0
+                    activeMillis = 0
+                }
+            }
+        }
+
+        fun setHeartRate(timestampMillis: Long, heartRate: Double?) {
+            lock.withLock {
+                if (timestampMillis < lastEventMillis) return
+                fix(timestampMillis)
+                currentBpm = heartRate
+            }
         }
     }
 }

wear/src/main/java/info/nightscout/androidaps/interaction/ConfigurationActivity.kt

@@ -1,6 +1,5 @@
 package info.nightscout.androidaps.interaction
 
-import preference.WearPreferenceActivity
 import android.os.Bundle
 import android.view.View
 import android.view.ViewGroup
@@ -9,7 +8,7 @@ import dagger.android.AndroidInjection
 import info.nightscout.androidaps.R
 import info.nightscout.rx.logging.AAPSLogger
 import info.nightscout.rx.logging.LTag
-
+import preference.WearPreferenceActivity
 import javax.inject.Inject
 
 class ConfigurationActivity : WearPreferenceActivity() {

wear/src/main/java/info/nightscout/androidaps/watchfaces/utils/BaseWatchFace.kt

@@ -152,12 +152,17 @@ abstract class BaseWatchFace : WatchFace() {
 
     private fun updateHeartRateListener() {
         if (sp.getBoolean(R.string.key_heart_rate_sampling, false)) {
-            heartRateListener = heartRateListener ?: HeartRateListener(this, aapsLogger)
+            if (heartRateListener == null) {
+                heartRateListener = HeartRateListener(
+                    this, aapsLogger, aapsSchedulers).also { hrl -> disposable += hrl }
+            }
         } else {
-            heartRateListener?.onDestroy()
+            heartRateListener?.let { hrl ->
+                disposable.remove(hrl)
                 heartRateListener = null
             }
         }
+    }
 
     override fun onTapCommand(tapType: Int, x: Int, y: Int, eventTime: Long) {
         binding.chart?.let { chart ->
@@ -240,7 +245,6 @@ abstract class BaseWatchFace : WatchFace() {
 
     override fun onDestroy() {
         disposable.clear()
-        heartRateListener?.onDestroy()
         simpleUi.onDestroy()
         super.onDestroy()
     }

wear/src/test/java/info/nightscout/androidaps/heartrate/HeartRateListenerTest.kt

@@ -3,21 +3,43 @@ package info.nightscout.androidaps.heartrate
 import android.content.Context
 import android.hardware.Sensor
 import android.hardware.SensorManager
+import info.nightscout.rx.AapsSchedulers
 import info.nightscout.rx.logging.AAPSLoggerTest
 import info.nightscout.rx.weardata.EventData.ActionHeartRate
+import io.reactivex.rxjava3.core.Scheduler
+import io.reactivex.rxjava3.disposables.Disposable
+import org.junit.jupiter.api.AfterEach
 import org.junit.jupiter.api.Assertions.assertEquals
+import org.junit.jupiter.api.Assertions.assertNull
 import org.junit.jupiter.api.BeforeEach
 import org.junit.jupiter.api.Test
+import org.mockito.ArgumentMatchers.any
+import org.mockito.ArgumentMatchers.eq
+import org.mockito.Mockito
 import org.mockito.Mockito.mock
+import org.mockito.Mockito.verify
+import org.mockito.Mockito.`when`
+import java.util.concurrent.TimeUnit
 
 internal class HeartRateListenerTest {
     private val aapsLogger = AAPSLoggerTest()
+    private val aapsSchedulers = object: AapsSchedulers {
+        override val main: Scheduler = mock(Scheduler::class.java)
+        override val io: Scheduler = mock(Scheduler::class.java)
+        override val cpu: Scheduler = mock(Scheduler::class.java)
+        override val newThread: Scheduler = mock(Scheduler::class.java)
+    }
+    private val schedule = mock(Disposable::class.java)
     private val heartRates = mutableListOf<ActionHeartRate>()
     private val device = "unknown unknown"
 
-    private fun create(): HeartRateListener {
+    private fun create(timestampMillis: Long): HeartRateListener {
         val ctx = mock(Context::class.java)
-        val listener = HeartRateListener(ctx, aapsLogger)
+        `when`(aapsSchedulers.io.schedulePeriodicallyDirect(
+            any(), eq(60_000L), eq(60_000L), eq(TimeUnit.MILLISECONDS))).thenReturn(schedule)
+        val listener = HeartRateListener(ctx, aapsLogger, aapsSchedulers, timestampMillis)
+        verify(aapsSchedulers.io).schedulePeriodicallyDirect(
+            any(), eq(60_000L), eq(60_000L), eq(TimeUnit.MILLISECONDS))
         listener.sendHeartRate = { hr -> heartRates.add(hr) }
         return listener
     }
@@ -32,73 +54,111 @@ internal class HeartRateListenerTest {
     }
     
     @BeforeEach
-    fun init() {
+    fun before() {
         heartRates.clear()
     }
 
+    @AfterEach
+    fun cleanup() {
+        Mockito.verifyNoInteractions(aapsSchedulers.main)
+        Mockito.verifyNoMoreInteractions(aapsSchedulers.io)
+        Mockito.verifyNoInteractions(aapsSchedulers.cpu)
+        Mockito.verifyNoInteractions(aapsSchedulers.newThread)
+        verify(schedule).dispose()
+    }
+    
     @Test
     fun onSensorChanged() {
-        val listener = create()
         val start = System.currentTimeMillis()
-        sendSensorEvent(listener, start, 80)
+        val d1 = 10_000L
+        val d2 = 20_000L
+        val listener = create(start)
+
+        assertNull(listener.currentHeartRateBpm)
+        sendSensorEvent(listener, start + d1, 80)
         assertEquals(0, heartRates.size)
-        sendSensorEvent(listener, start + 60_001L,180)
+        assertEquals(80, listener.currentHeartRateBpm)
+
+        listener.send(start + d2)
         assertEquals(1, heartRates.size)
-        assertEquals(ActionHeartRate(start, start, 80, device), heartRates.first())
+        assertEquals(ActionHeartRate(d2, start + d2, 80.0, device), heartRates.first())
+        listener.dispose()
     }
 
     @Test
     fun onSensorChanged2() {
-        val listener = create()
         val start = System.currentTimeMillis()
-        sendSensorEvent(listener, start, 80)
+        val d1 = 10_000L
-        assertEquals(0, heartRates.size)
+        val d2 = 40_000L
-        sendSensorEvent(listener, start + 1000L,100)
+        val listener = create(start)
-        assertEquals(0, heartRates.size)
-        sendSensorEvent(listener, start + 60_001L,180)
-        assertEquals(1, heartRates.size)
-        assertEquals(ActionHeartRate(start, start + 1000L, 90, device), heartRates.first())
-    }
 
-    @Test
-    fun onSensorChangedOldValue() {
-        val listener = create()
-        val start = System.currentTimeMillis()
         sendSensorEvent(listener, start, 80)
         assertEquals(0, heartRates.size)
-        val start2 = start + 120_001L
+        assertEquals(80, listener.currentHeartRateBpm)
-        sendSensorEvent(listener, start2, 100)
+        sendSensorEvent(listener, start + d1,100)
         assertEquals(0, heartRates.size)
-        sendSensorEvent(listener, start + 180_001L, 180)
+        assertEquals(100, listener.currentHeartRateBpm)
+
+
+        listener.send(start + d2)
         assertEquals(1, heartRates.size)
-        assertEquals(ActionHeartRate(start2, start2, 100, device), heartRates.first())
+        assertEquals(ActionHeartRate(d2, start + d2, 95.0, device), heartRates.first())
+        listener.dispose()
     }
 
     @Test
     fun onSensorChangedMultiple() {
-        val listener = create()
         val start = System.currentTimeMillis()
+        val d1 = 10_000L
+        val d2 = 40_000L
+        val listener = create(start)
+
         sendSensorEvent(listener, start, 80)
-        assertEquals(0, heartRates.size)
+        listener.send(start + d1)
-        val start2 = start + 60_000L
-        sendSensorEvent(listener, start2,100)
         assertEquals(1, heartRates.size)
-        sendSensorEvent(listener, start2 + 60_000L,180)
+
+        sendSensorEvent(listener, start + d1,100)
+        assertEquals(1, heartRates.size)
+        listener.send(start + d2)
         assertEquals(2, heartRates.size)
-        assertEquals(ActionHeartRate(start, start, 80, device), heartRates[0])
+
-        assertEquals(ActionHeartRate(start2, start2, 100, device), heartRates[1])
+        assertEquals(ActionHeartRate(d1, start + d1, 80.0, device), heartRates[0])
+        assertEquals(ActionHeartRate(d2 - d1, start + d2, 100.0, device), heartRates[1])
+        listener.dispose()
     }
 
     @Test
     fun onSensorChangedNoContact() {
-        val listener = create()
         val start = System.currentTimeMillis()
+        val d1 = 10_000L
+        val d2 = 40_000L
+        val listener = create(start)
+
         sendSensorEvent(listener, start, 80)
-        assertEquals(0, heartRates.size)
+        sendSensorEvent(listener, start + d1, 100, accuracy = SensorManager.SENSOR_STATUS_NO_CONTACT)
-        sendSensorEvent(listener, start + 1000L, 100, accuracy = SensorManager.SENSOR_STATUS_NO_CONTACT)
+        assertNull(listener.currentHeartRateBpm)
-        assertEquals(0, heartRates.size)
+        listener.send(start + d2)
-        sendSensorEvent(listener, start + 60_001L, 180)
+
         assertEquals(1, heartRates.size)
-        assertEquals(ActionHeartRate(start, start, 80, device), heartRates.first())
+        assertEquals(ActionHeartRate(d2, start + d2, 80.0, device), heartRates.first())
+        listener.dispose()
+    }
+
+    @Test
+    fun onAccuracyChanged() {
+        val start = System.currentTimeMillis()
+        val d1 = 10_000L
+        val d2 = 40_000L
+        val d3 = 70_000L
+        val listener = create(start)
+
+        sendSensorEvent(listener, start, 80)
+        listener.onAccuracyChanged(Sensor.TYPE_HEART_RATE, SensorManager.SENSOR_STATUS_UNRELIABLE, start + d1)
+        sendSensorEvent(listener, start + d2, 100)
+        listener.send(start + d3)
+
+        assertEquals(1, heartRates.size)
+        assertEquals(ActionHeartRate(d3, start + d3, 95.0, device), heartRates.first())
+        listener.dispose()
     }
 }