fix(reaction): Tighten auto-pause debounce after review findings

- Commit pending pause when a trusted source (AAP / BLE_IRK_MATCH) corroborates the not-worn condition mid-debounce, instead of dropping pending silently. - Scope debounceFreshness to not-worn samples only; identical both-in samples no longer pass distinctUntilChangedBy and can't accidentally trigger BLE-only auto-play confirmation. - Add resetTolerance to PendingPauseDebounce so a single corrupt count-up advert no longer kills a legitimate pending pause; reorder reset checks so rawDecision.shouldPlay resets immediately. - Drop bleKeyState from the INFO autoPause log; source already encodes trust without leaking key-configuration state to logcat. - Add flow-level MonitorFlowTests verifying the distinctUntilChangedBy interaction with seenLastAt freshness, plus the #557-direction test (AAP-worn vs corrupt-BLE-not-worn) and rebound-tolerance test. - Clarify in BLE_ANONYMOUS KDoc that the path is unreachable in production via DeviceMonitor.primaryDevice.
2026-09-15 02:36:12 -04:00 · 2026-04-29 18:14:51 +02:00
parent 67fead3589
commit 228d7d1f0c
2 changed files with 204 additions and 10 deletions
@@ -228,7 +228,6 @@ class PlayPause @Inject constructor(
                            "autoPause triggered: source=$source, " +
                                "wasWorn=${prevState.bothInEar}, isWorn=${currState.bothInEar}, " +
                                "podCount=${prevState.podCount}->${currState.podCount}, " +
-                                "bleKeyState=${current.bleKeyState}, " +
                                "aapEar=${current.aap?.aapEarDetection != null}, " +
                                "aapConn=${current.aap?.connectionState}, " +
                                "pauseSent=$pauseSent, reason=${decision.reason}"
@@ -427,8 +426,25 @@ class PlayPause @Inject constructor(
        }

        // Non-debounce-eligible trusted sources (AAP / BLE_IRK_MATCH) and disabled debounce:
-        // pass raw decision through and clear any active pending.
+        // pass raw decision through and clear any active pending. Special case: if a
+        // pending was active and the trusted source still shows the not-worn condition
+        // (currentState.podCount <= initialPodCount and no play decision), commit the
+        // pause now — the trusted source corroborates what BLE-debounce was waiting on.
        if (!needsDebounce || !autoPauseEnabled || profileId == null) {
+            if (activePending != null && autoPauseEnabled &&
+                currentState.podCount <= activePending.initialPodCount &&
+                !rawDecision.shouldPlay
+            ) {
+                return PauseDebounceResult(
+                    decision = PlayPauseDecision(
+                        shouldPlay = false,
+                        shouldPause = true,
+                        reason = "Pause confirmed by trusted source ($source) after BLE-debounce",
+                    ),
+                    pending = null,
+                    event = PauseDebounceEvent.COMMITTED,
+                )
+            }
            val event = if (activePending != null) PauseDebounceEvent.RESET else PauseDebounceEvent.NONE
            return PauseDebounceResult(decision = rawDecision, pending = null, event = event)
        }
@@ -456,8 +472,24 @@ class PlayPause @Inject constructor(
        }

        // Confirmation phase: pending exists.
-        // Reset cases: pod returned (count went up) or raw decision wants to play.
-        if (currentState.podCount > activePending.initialPodCount || rawDecision.shouldPlay) {
+        // Reset cases — checked in order of authority:
+        //   1. Raw decision wants to play → genuine play signal, reset immediately.
+        //   2. Pod count went up → tolerate one rebound sample (corrupt count-up
+        //      protection), reset only on the second consecutive count-up.
+        if (rawDecision.shouldPlay) {
+            return PauseDebounceResult(decision = rawDecision, pending = null, event = PauseDebounceEvent.RESET)
+        }
+        if (currentState.podCount > activePending.initialPodCount) {
+            if (activePending.resetTolerance > 0) {
+                return PauseDebounceResult(
+                    decision = rawDecision.copy(
+                        shouldPause = false,
+                        reason = "Debouncing pause (rebound tolerated)",
+                    ),
+                    pending = activePending.copy(resetTolerance = activePending.resetTolerance - 1),
+                    event = PauseDebounceEvent.ADVANCED,
+                )
+            }
            return PauseDebounceResult(decision = rawDecision, pending = null, event = PauseDebounceEvent.RESET)
        }

@@ -563,7 +595,11 @@ class PlayPause @Inject constructor(
     *   identity key. Identity-authenticated; debounce skipped.
     * - [BLE_PROFILE_FALLBACK]: BLE advertisement assigned to a profile via signal-quality
     *   fallback (no IRK match). Could be a stray advert from a nearby pair. Debounced.
-     * - [BLE_ANONYMOUS]: BLE advertisement with no profile match. Debounced.
+     * - [BLE_ANONYMOUS]: BLE advertisement with no profile match. Filtered out by
+     *   [primaryDevice] in production (devices without profileId are dropped before
+     *   reaching the reaction layer); kept here defensively in case the upstream filter
+     *   changes. Note: [PendingPauseDebounce] is profile-keyed, so a null profileId can
+     *   never sustain pending state even if this branch is hit.
     * - [NO_LIVE_BLE]: No live BLE snapshot at all (cache-only or empty). Not debounced —
     *   the cached state is not "fresh evidence" so it must not advance the debounce counter.
     *   Any active pending is cleared.
@@ -574,6 +610,11 @@ class PlayPause @Inject constructor(
        val profileId: String,
        val initialPodCount: Int,
        val confirmationsRemaining: Int,
+        // Tolerates one count-up rebound sample before the pending is reset. Mirrors the
+        // count-down debounce on the pause side: a single corrupt advert that briefly
+        // shows a pod returning shouldn't kill the pending, since the next sample may
+        // confirm the pods are still out.
+        val resetTolerance: Int = 1,
    )

    /** Discrete event produced by [applyPauseDebounce] for diagnostic logging. */
@@ -623,8 +664,13 @@ class PlayPause @Inject constructor(

    internal fun PodDevice.toPlayPauseMonitorKey(): PlayPauseMonitorKey {
        val source = earDetectionSource()
-        val needsFreshness = source == EarDetectionSource.BLE_PROFILE_FALLBACK ||
-            source == EarDetectionSource.BLE_ANONYMOUS
+        // Freshness is needed only on debounce-eligible NOT-WORN samples. Including
+        // worn samples would also break distinctUntilChangedBy for identical both-in
+        // samples, accidentally enabling BLE-only auto-play confirmation to fire on
+        // repeated unauthenticated adverts.
+        val needsFreshness = (source == EarDetectionSource.BLE_PROFILE_FALLBACK ||
+            source == EarDetectionSource.BLE_ANONYMOUS) &&
+            isBeingWorn != true
        return PlayPauseMonitorKey(
            profileId = profileId,
            autoPlay = reactions.autoPlay,
@@ -1,19 +1,33 @@
 package eu.darken.capod.reaction.core.playpause

+import eu.darken.capod.common.MediaControl
+import eu.darken.capod.common.bluetooth.BluetoothManager2
+import eu.darken.capod.monitor.core.DeviceMonitor
 import eu.darken.capod.monitor.core.PodDevice
+import eu.darken.capod.pods.core.apple.PodModel
 import eu.darken.capod.pods.core.apple.aap.AapPodState
 import eu.darken.capod.pods.core.apple.aap.protocol.AapSetting
 import eu.darken.capod.pods.core.apple.ble.DualBlePodSnapshot
+import eu.darken.capod.pods.core.apple.ble.devices.ApplePods
 import eu.darken.capod.pods.core.apple.ble.devices.DualApplePods
+import eu.darken.capod.profiles.core.ReactionConfig
 import eu.darken.capod.reaction.core.playpause.PlayPause.EarDetectionState
 import io.kotest.matchers.shouldBe
 import io.kotest.matchers.shouldNotBe
+import io.mockk.coEvery
+import io.mockk.coVerify
 import io.mockk.every
 import io.mockk.mockk
+import kotlinx.coroutines.flow.MutableStateFlow
+import kotlinx.coroutines.flow.flowOf
+import kotlinx.coroutines.launch
+import kotlinx.coroutines.test.advanceUntilIdle
+import kotlinx.coroutines.test.runTest
 import org.junit.jupiter.api.BeforeEach
 import org.junit.jupiter.api.Nested
 import org.junit.jupiter.api.Test
 import testhelpers.BaseTest
+import java.time.Instant

 class PlayPauseLogicTest : BaseTest() {

@@ -1132,14 +1146,43 @@ class PlayPauseLogicTest : BaseTest() {
        }

        @Test
-        fun `BLE_PROFILE_FALLBACK pod returns mid-debounce - pending cleared`() {
-            // First detection: both pods removed
+        fun `BLE_PROFILE_FALLBACK first count-up is tolerated as a rebound`() {
+            // First detection: both pods removed (initialPodCount=0). A single corrupt
+            // count-up sample (pod=1) should be tolerated — the helper holds pending
+            // and decrements resetTolerance.
            val pending = PlayPause.PendingPauseDebounce(
                profileId = "profile",
                initialPodCount = 0,
                confirmationsRemaining = 1,
+                resetTolerance = 1,
+            )
+            val current = EarDetectionState.fromDualPod(true, false)
+
+            val result = playPause.applyPauseDebounce(
+                pending = pending,
+                profileId = "profile",
+                source = PlayPause.EarDetectionSource.BLE_PROFILE_FALLBACK,
+                rawDecision = noopDecision,
+                currentState = current,
+                autoPauseEnabled = true,
+            )
+
+            result.decision.shouldPause shouldBe false
+            result.pending shouldNotBe null
+            result.pending!!.resetTolerance shouldBe 0
+            result.event shouldBe PlayPause.PauseDebounceEvent.ADVANCED
+        }
+
+        @Test
+        fun `BLE_PROFILE_FALLBACK second count-up resets pending (tolerance exhausted)`() {
+            // After the first rebound was tolerated, resetTolerance=0. A second count-up
+            // sample resets pending — this is a genuine pod-return signal.
+            val pending = PlayPause.PendingPauseDebounce(
+                profileId = "profile",
+                initialPodCount = 0,
+                confirmationsRemaining = 1,
+                resetTolerance = 0,
            )
-            // Pod returned: count went from 0 -> 1 (worn back)
            val current = EarDetectionState.fromDualPod(true, false)

            val result = playPause.applyPauseDebounce(
@@ -1153,6 +1196,7 @@ class PlayPauseLogicTest : BaseTest() {

            result.decision.shouldPause shouldBe false
            result.pending shouldBe null
+            result.event shouldBe PlayPause.PauseDebounceEvent.RESET
        }

        @Test
@@ -1326,6 +1370,43 @@ class PlayPauseLogicTest : BaseTest() {
            state.podCount shouldBe 0
        }

+        @Test
+        fun `AAP says worn while BLE per-side falsely says not-worn - returns worn (#557 scenario)`() {
+            // The motivating bug for the per-side gap fix: AAP authoritative state says
+            // pods worn, but BLE fallback bits say not-worn. AAP must win — this is the
+            // scenario users hit in #557 when high-RF interference flipped BLE bits.
+            val ble = mockk<DualApplePods>(relaxed = true) {
+                every { isLeftPodInEar } returns false   // BLE corrupt: says not-worn
+                every { isRightPodInEar } returns false
+                every { isBeingWorn } returns false
+                every { isEitherPodInEar } returns false
+                every { primaryPod } returns DualBlePodSnapshot.Pod.LEFT
+            }
+            val aapEarDetection = AapSetting.EarDetection(
+                primaryPod = AapSetting.EarDetection.PodPlacement.IN_EAR,
+                secondaryPod = AapSetting.EarDetection.PodPlacement.IN_EAR,
+            )
+            val aapPrimary = AapSetting.PrimaryPod(pod = AapSetting.PrimaryPod.Pod.LEFT)
+            val aap = AapPodState(
+                connectionState = AapPodState.ConnectionState.READY,
+                settings = mapOf(
+                    AapSetting.EarDetection::class to aapEarDetection,
+                    AapSetting.PrimaryPod::class to aapPrimary,
+                ),
+            )
+            val device = PodDevice(
+                profileId = "test",
+                ble = ble,
+                aap = aap,
+            )
+
+            val state = with(playPause) { device.toEarDetectionState() }
+
+            // AAP says worn → aggregate must reflect worn even though BLE bits say not-worn.
+            state.bothInEar shouldBe true
+            state.podCount shouldBe 2
+        }
+
        @Test
        fun `AAP absent - falls back to BLE per-side`() {
            val ble = mockk<DualApplePods>(relaxed = true) {
@@ -1450,4 +1531,71 @@ class PlayPauseLogicTest : BaseTest() {
            state.podCount shouldBe 2
        }
    }
+
+    @Nested
+    inner class MonitorFlowTests {
+
+        private fun buildBle(seenAt: Instant, leftWorn: Boolean, rightWorn: Boolean) =
+            mockk<DualApplePods>(relaxed = true) {
+                every { meta } returns ApplePods.AppleMeta(
+                    isIRKMatch = false,
+                    profile = mockk(relaxed = true),
+                )
+                every { seenLastAt } returns seenAt
+                every { isLeftPodInEar } returns leftWorn
+                every { isRightPodInEar } returns rightWorn
+                every { isBeingWorn } returns (leftWorn && rightWorn)
+                every { isEitherPodInEar } returns (leftWorn || rightWorn)
+                every { primaryPod } returns DualBlePodSnapshot.Pod.LEFT
+                every { model } returns PodModel.AIRPODS_PRO3
+            }
+
+        private fun buildDevice(seenAt: Instant, leftWorn: Boolean, rightWorn: Boolean) =
+            PodDevice(
+                profileId = "test-profile",
+                ble = buildBle(seenAt, leftWorn, rightWorn),
+                aap = null,
+                profileModel = PodModel.AIRPODS_PRO3,
+                reactions = ReactionConfig(autoPlay = true, autoPause = true),
+            )
+
+        @Test
+        fun `flow - 3 consecutive not-worn unauthenticated samples fire pause exactly once`() = runTest {
+            val deviceFlow = MutableStateFlow<List<PodDevice>>(emptyList())
+            val deviceMonitor: DeviceMonitor = mockk(relaxed = true) {
+                every { devices } returns deviceFlow
+            }
+            val bluetoothManager: BluetoothManager2 = mockk(relaxed = true) {
+                every { connectedDevices } returns flowOf(listOf(mockk(relaxed = true)))
+            }
+            val mediaControl: MediaControl = mockk(relaxed = true) {
+                every { isPlaying } returns true
+                every { wasRecentlyPausedByCap } returns false
+                coEvery { sendPause() } returns true
+            }
+            val flowPlayPause = PlayPause(deviceMonitor, bluetoothManager, mediaControl)
+
+            val now = Instant.parse("2026-01-01T00:00:00Z")
+            val job = launch { flowPlayPause.monitor().collect {} }
+
+            // T0: worn baseline
+            deviceFlow.value = listOf(buildDevice(now, leftWorn = true, rightWorn = true))
+            advanceUntilIdle()
+
+            // T1, T2, T3: identical not-worn samples with incrementing seenLastAt.
+            // Without the freshness fix, distinctUntilChangedBy would collapse T2 and T3
+            // and the debounce counter could never advance. With it, the helper sees
+            // 3 samples and commits the pause on the third.
+            deviceFlow.value = listOf(buildDevice(now.plusMillis(1000), leftWorn = false, rightWorn = false))
+            advanceUntilIdle()
+            deviceFlow.value = listOf(buildDevice(now.plusMillis(2000), leftWorn = false, rightWorn = false))
+            advanceUntilIdle()
+            deviceFlow.value = listOf(buildDevice(now.plusMillis(3000), leftWorn = false, rightWorn = false))
+            advanceUntilIdle()
+
+            coVerify(exactly = 1) { mediaControl.sendPause() }
+
+            job.cancel()
+        }
+    }
 }