fix(reaction): Stop case popup flicker when one AirPod is out of case

When one pod is out of the case, the out-of-case pod broadcasts bit4-only frames whose lid byte is stale and decodes to a phantom OPEN even while the case is shut, interleaved ~1:1 with the correct in-case-pod (bit6) frames. The derived lid state flapped OPEN<->CLOSED, so the case-open popup re-popped ~0.5s after closing and sometimes lingered. Verified on AirPods Pro 1 (issue log) and Pro 3 (live BLE capture). - Trust the lid bit only from in-case-pod (bit6) or both-in-case (bit2) frames; bit4-only frames now decode to UNKNOWN, and the real state is recovered from a recent in-case broadcast (matches LibrePods). - getLatestCaseLidState recovers from history within a 2s age window instead of a fixed frame count, so a missed CLOSED can't keep a stale OPEN. - Don't refresh the show-cooldown on a non-CLOSED hide, so a transient UNKNOWN can't suppress a genuine re-open. - Add a freshness backstop: dismiss the popup if a fresh OPEN broadcast stops arriving (device left BLE range while open). - Key popup/auto-connect de-duplication on the derived lid state, not just raw advertisement bytes, since the effective lid can change while bytes don't. Closes #598
2026-09-14 18:26:11 -04:00 · 2026-06-08 08:05:50 +02:00
parent 81e2bc1dba
commit 4bab7c61a9
6 changed files with 270 additions and 20 deletions
@@ -4,6 +4,14 @@ import eu.darken.capod.common.bluetooth.BleScanResult
 import eu.darken.capod.pods.core.apple.ble.history.KnownDevice
 import eu.darken.capod.pods.core.apple.ble.protocol.ProximityMessage
 import eu.darken.capod.pods.core.apple.ble.protocol.ProximityPayload
+import java.time.Duration
+
+/**
+ * How far back to look when recovering the lid state from a recent in-case-pod broadcast.
+ * Time-bounded rather than count-bounded: BLE scan batching means a fixed number of frames is not a
+ * stable time window, and an old reading must not be allowed to resurrect a stale OPEN/CLOSED.
+ */
+private val MAX_LID_RECOVERY_AGE: Duration = Duration.ofSeconds(2)

 interface ApplePodsFactory {
    fun isResponsible(message: ProximityMessage): Boolean
@@ -22,7 +30,8 @@ interface ApplePodsFactory {
    fun KnownDevice.getLatestCaseBattery(): Float? = this.lastCaseBattery

    fun KnownDevice.getLatestCaseLidState(basic: DualApplePods): DualApplePods.LidState? {
-        // A pod broadcasting from inside the case has authoritative case state
+        // A pod broadcasting from inside the case has authoritative case state. Out-of-case frames
+        // (one pod removed) report UNKNOWN via DualApplePods.caseLidState and are skipped here.
        if (basic.hasCaseContext && basic.caseLidState in setOf(
                DualApplePods.LidState.OPEN,
                DualApplePods.LidState.CLOSED,
@@ -31,11 +40,14 @@ interface ApplePodsFactory {
            return basic.caseLidState
        }

-        // Current pod lacks case context (e.g. pod on desk) or reports UNKNOWN.
-        // Check recent history for a sibling broadcast that has case context.
-        val fromCaseContext = history
-            .takeLast(4)
+        // Current pod lacks case context (e.g. pod on desk) or reports UNKNOWN (out-of-case pod's
+        // stale lid byte). Recover the last authoritative reading from a recent in-case broadcast,
+        // bounded by time so a missed CLOSED can't keep a stale OPEN alive across scan gaps.
+        val recentHistory = history
            .filterIsInstance<DualApplePods>()
+            .filter { Duration.between(it.seenLastAt, basic.seenLastAt).abs() <= MAX_LID_RECOVERY_AGE }
+
+        val fromCaseContext = recentHistory
            .lastOrNull { it.hasCaseContext && it.caseLidState != DualApplePods.LidState.UNKNOWN }
            ?.caseLidState

@@ -44,10 +56,8 @@ interface ApplePodsFactory {
        // No case-context broadcast in recent history — current value is best we have
        if (basic.caseLidState != DualApplePods.LidState.UNKNOWN) return basic.caseLidState

-        // Last resort: any non-UNKNOWN from history
-        return history
-            .takeLast(2)
-            .filterIsInstance<DualApplePods>()
+        // Last resort: any non-UNKNOWN from recent history
+        return recentHistory
            .lastOrNull { it.caseLidState != DualApplePods.LidState.UNKNOWN }
            ?.caseLidState
            ?: DualApplePods.LidState.NOT_IN_CASE
@@ -148,7 +148,14 @@ interface DualApplePods : ApplePods, HasChargeDetectionDual, DualBlePodSnapshot,
        get() = isThisPodInThecase || isOnePodInCase || areBothPodsInCase

    val caseLidState: LidState
-        get() = LidState.fromRaw(pubCaseLidState, hasCaseContext)
+        get() = LidState.fromRaw(
+            raw = pubCaseLidState,
+            hasCaseContext = hasCaseContext,
+            // The lid bit is only trustworthy from a pod broadcasting inside the case (bit 6), or
+            // while both pods are in the case (bit 2). A bit4-only frame comes from the out-of-case
+            // pod and carries a stale lid byte (see LidState.fromRaw).
+            lidReadingReliable = isThisPodInThecase || areBothPodsInCase,
+        )

    /**
     * TODO this is glitchy
@@ -165,8 +172,20 @@ interface DualApplePods : ApplePods, HasChargeDetectionDual, DualBlePodSnapshot,
        UNKNOWN;

        companion object {
-            fun fromRaw(raw: UByte, hasCaseContext: Boolean): LidState {
+            /**
+             * Derives the lid state from the raw lid byte.
+             *
+             * The open/closed bit is only meaningful when broadcast by a pod that is itself inside
+             * the case ([isThisPodInThecase]) or while both pods are in the case ([areBothPodsInCase]).
+             * When only one pod is in the case and the *other*, out-of-case pod is the one
+             * broadcasting (bit4-only), its lid byte is stale and decodes to a phantom OPEN even while
+             * the case is physically shut (verified on AirPods Pro 1 & Pro 3). Such frames must report
+             * [UNKNOWN] so they don't drive case-open reactions; consumers recover the real state from
+             * an in-case-pod broadcast instead.
+             */
+            fun fromRaw(raw: UByte, hasCaseContext: Boolean, lidReadingReliable: Boolean): LidState {
                if (!hasCaseContext) return NOT_IN_CASE
+                if (!lidReadingReliable) return UNKNOWN

                return when ((raw.toInt() shr 3) and 0x01) {
                    0 -> OPEN
@@ -39,7 +39,9 @@ class AutoConnect @Inject constructor(
                combine(
                    bluetoothManager.connectedDevices,
                    deviceMonitor.primaryDevice().filterNotNull().distinctUntilChangedBy {
-                        Triple(it.rawDataHex, it.reactions.autoConnectCondition, it.reactions.onePodMode)
+                        // Include caseLidState: for the CASE_OPEN condition it is derived from history
+                        // and can flip to OPEN while the selected frame's raw bytes are unchanged.
+                        listOf(it.rawDataHex, it.reactions.autoConnectCondition, it.reactions.onePodMode, it.caseLidState)
                    },
                ) { connectedDevices, mainDevice ->
                    connectedDevices to mainDevice
@@ -13,9 +13,12 @@ import eu.darken.capod.monitor.core.DeviceMonitor
 import eu.darken.capod.monitor.core.PodDevice
 import eu.darken.capod.monitor.core.primaryDevice
 import eu.darken.capod.pods.core.apple.ble.devices.DualApplePods
+import kotlinx.coroutines.delay
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.combine
+import kotlinx.coroutines.flow.distinctUntilChanged
 import kotlinx.coroutines.flow.distinctUntilChangedBy
+import kotlinx.coroutines.flow.flow
 import kotlinx.coroutines.flow.mapNotNull
 import kotlinx.coroutines.flow.merge
 import java.time.Duration
@@ -34,8 +37,12 @@ class PopUpReaction @Inject constructor(

    private fun monitorCase(): Flow<Event> = deviceMonitor.primaryDevice()
        .distinctUntilChangedBy {
-            // Re-emit on profile changes (eligibility) AND on raw BLE state changes (lid).
-            Triple(it?.profileId, it?.reactions?.showPopUpOnCaseOpen, it?.rawDataHex)
+            // Re-emit on profile changes (eligibility), raw BLE changes (content), AND the derived
+            // lid state. The latter is essential: caseLidState is recovered from history, so it can
+            // flip OPEN<->CLOSED while the *selected* frame's raw bytes stay identical (e.g. a steady
+            // out-of-case frame while a sibling in-case frame updates). Keying on rawDataHex alone
+            // would swallow that transition and the popup would miss its show/hide.
+            listOf(it?.profileId, it?.reactions?.showPopUpOnCaseOpen, it?.rawDataHex, it?.caseLidState)
        }
        .withPrevious()
        .setupCommonEventHandlers(TAG) { "popUpCase" }
@@ -71,7 +78,7 @@ class PopUpReaction @Inject constructor(
            throttleCasePopUps(current)
        }

-    private fun throttleCasePopUps(current: PodDevice): Event? {
+    internal fun throttleCasePopUps(current: PodDevice): Event? {
        val cooldownKey = current.profileId ?: current.identifier?.toString() ?: return null
        val now = timeSource.now()
        val lastShown = caseCoolDowns[cooldownKey]
@@ -95,9 +102,9 @@ class PopUpReaction @Inject constructor(
            }

            decision.shouldHide -> {
-                if (!decision.shouldResetCooldown) {
-                    caseCoolDowns[cooldownKey] = now
-                }
+                // Don't stamp the cooldown on a non-CLOSED hide (UNKNOWN/NOT_IN_CASE). Refreshing it
+                // here would let a transient UNKNOWN (e.g. a brief out-of-case frame) suppress a
+                // genuine OPEN for the whole cooldown window. CLOSED still resets it above.
                Event.PopupHide(now)
            }

@@ -192,7 +199,48 @@ class PopUpReaction @Inject constructor(
        }
        .setupCommonEventHandlers(TAG) { "popUpConnection" }

-    fun monitor(): Flow<Event> = merge(monitorCase(), monitorConnection())
+    /**
+     * Backstop for case-open popups that never receive a CLOSED frame because the device left BLE
+     * range while the lid was open — otherwise the overlay lingers until manually dismissed (one of
+     * the symptoms in #598). A ticker re-checks the primary device's freshness; once a previously
+     * fresh OPEN broadcast goes stale past [CASE_OPEN_STALE_TIMEOUT] (no newer advertisement, or the
+     * device dropped to cache-only), a single Hide is emitted. The lid-driven [monitorCase] still
+     * handles the normal close; a redundant Hide here is harmless ([PopUpWindow.close] is idempotent).
+     */
+    private fun monitorCaseStaleClose(): Flow<Event> = combine(
+        deviceMonitor.primaryDevice(),
+        staleCheckTicker(),
+    ) { device, _ -> isCaseOpenBroadcastFresh(device) }
+        .distinctUntilChanged()
+        .withPrevious()
+        .mapNotNull { (wasFresh, isFresh) ->
+            if (wasFresh == true && !isFresh) {
+                log(TAG) { "Case-open broadcast went stale, emitting Hide" }
+                Event.PopupHide(timeSource.now())
+            } else {
+                null
+            }
+        }
+        .setupCommonEventHandlers(TAG) { "popUpCaseStale" }
+
+    /** True while the primary device is eligible and currently advertising a fresh OPEN lid. */
+    internal fun isCaseOpenBroadcastFresh(device: PodDevice?): Boolean {
+        if (device?.reactions?.showPopUpOnCaseOpen != true) return false
+        if (device.caseLidState != DualApplePods.LidState.OPEN) return false
+        // Track BLE freshness specifically, not PodDevice.seenLastAt (which also counts AAP/cache):
+        // the lid is a BLE-only signal, so a live AAP socket must not keep a stale OPEN on screen.
+        val bleSeenLastAt = device.ble?.seenLastAt ?: return false
+        return Duration.between(bleSeenLastAt, timeSource.now()) <= CASE_OPEN_STALE_TIMEOUT
+    }
+
+    private fun staleCheckTicker(): Flow<Unit> = flow {
+        while (true) {
+            emit(Unit)
+            delay(STALE_CHECK_INTERVAL.toMillis())
+        }
+    }
+
+    fun monitor(): Flow<Event> = merge(monitorCase(), monitorConnection(), monitorCaseStaleClose())

    sealed class Event {
        data class PopupShow(
@@ -296,5 +344,11 @@ class PopUpReaction @Inject constructor(

    companion object {
        private val TAG = logTag("Reaction", "PopUp")
+
+        /** A case-open popup is force-dismissed once its OPEN broadcast hasn't refreshed for this long. */
+        private val CASE_OPEN_STALE_TIMEOUT: Duration = Duration.ofSeconds(4)
+
+        /** How often the stale-close backstop re-evaluates freshness. */
+        private val STALE_CHECK_INTERVAL: Duration = Duration.ofSeconds(2)
    }
 }
@@ -7,6 +7,7 @@ import eu.darken.capod.pods.core.apple.ble.devices.airpods.HasStateDetectionAirP
 import eu.darken.capod.pods.core.apple.ble.history.KnownDevice
 import eu.darken.capod.pods.core.apple.ble.protocol.ProximityPayload
 import io.kotest.matchers.shouldBe
+import io.kotest.matchers.shouldNotBe
 import kotlinx.coroutines.test.runTest
 import org.junit.jupiter.api.Test
 import java.time.Instant
@@ -250,13 +251,34 @@ class DualApplePodsTest : BaseBlePodsTest() {

    @Test
    fun `test AirPodDevice - case lid uses status derived case context`() {
-        directAirPodsPro(status = 0x10, rawCaseLidState = 0x51).caseLidState shouldBe DualApplePods.LidState.OPEN
+        // bit4 only = one pod in case, broadcast by the OUT-of-case pod. Its lid byte is stale and
+        // decodes to a phantom OPEN even when the case is shut, so it must report UNKNOWN (#598).
+        directAirPodsPro(status = 0x10, rawCaseLidState = 0x51).caseLidState shouldBe DualApplePods.LidState.UNKNOWN
+        // bit2 = both pods in case → lid byte is trustworthy.
        directAirPodsPro(status = 0x04, rawCaseLidState = 0x5A).caseLidState shouldBe DualApplePods.LidState.CLOSED
+        // bit6 = this (broadcasting) pod is in the case → lid byte is trustworthy.
        directAirPodsPro(status = 0x40, rawCaseLidState = 0x51).caseLidState shouldBe DualApplePods.LidState.OPEN
+        // No case context at all → NOT_IN_CASE.
        directAirPodsPro(status = 0x2B, rawCaseLidState = 0x11).caseLidState shouldBe DualApplePods.LidState.NOT_IN_CASE
        directAirPodsPro(status = 0x20, rawCaseLidState = 0x5A).caseLidState shouldBe DualApplePods.LidState.NOT_IN_CASE
    }

+    @Test
+    fun `case lid - out-of-case pod frame reports UNKNOWN, not a phantom OPEN (issue 598)`() {
+        // Real captures while the case is physically shut with one pod removed. The in-case pod
+        // reports CLOSED; the out-of-case (bit4-only) pod carries a stale lid byte that the old
+        // decoder turned into a phantom OPEN. It must now be UNKNOWN.
+        // AirPods Pro 3:
+        directAirPodsPro(status = 0x73, rawCaseLidState = 0x39).caseLidState shouldBe DualApplePods.LidState.CLOSED
+        directAirPodsPro(status = 0x13, rawCaseLidState = 0x11).caseLidState shouldBe DualApplePods.LidState.UNKNOWN
+        // AirPods Pro 1:
+        directAirPodsPro(status = 0x53, rawCaseLidState = 0x39).caseLidState shouldBe DualApplePods.LidState.CLOSED
+        directAirPodsPro(status = 0x33, rawCaseLidState = 0x02).caseLidState shouldBe DualApplePods.LidState.UNKNOWN
+        // Both pods in the case (bit2) stays trustworthy even without bit6 (Pro 3 0x15, Pro 1 0x04).
+        directAirPodsPro(status = 0x15, rawCaseLidState = 0x31).caseLidState shouldBe DualApplePods.LidState.OPEN
+        directAirPodsPro(status = 0x04, rawCaseLidState = 0x31).caseLidState shouldBe DualApplePods.LidState.OPEN
+    }
+
    private fun knownDeviceOf(vararg pods: AirPodsPro): KnownDevice {
        val id = pods.first().identifier
        return KnownDevice(
@@ -315,6 +337,47 @@ class DualApplePodsTest : BaseBlePodsTest() {
        result shouldBe DualApplePods.LidState.NOT_IN_CASE
    }

+    @Test
+    fun `getLatestCaseLidState - phantom out-of-case frame recovers CLOSED from in-case history (issue 598)`() {
+        val sharedId = BlePodSnapshot.Id()
+        // In-case pod reports the real, shut lid; out-of-case pod's bit4-only frame is UNKNOWN.
+        val inCaseClosed = directAirPodsPro(status = 0x73, rawCaseLidState = 0x39).copy(identifier = sharedId)
+        val outOfCasePhantom = directAirPodsPro(status = 0x13, rawCaseLidState = 0x11).copy(identifier = sharedId)
+
+        val known = knownDeviceOf(inCaseClosed, outOfCasePhantom)
+        val result = with(testFactory) { known.getLatestCaseLidState(outOfCasePhantom) }
+
+        // Must recover CLOSED from the in-case broadcast, not surface the phantom OPEN.
+        result shouldBe DualApplePods.LidState.CLOSED
+    }
+
+    @Test
+    fun `getLatestCaseLidState - only out-of-case frames never report a phantom OPEN (issue 598)`() {
+        val sharedId = BlePodSnapshot.Id()
+        val phantom1 = directAirPodsPro(status = 0x13, rawCaseLidState = 0x11).copy(identifier = sharedId)
+        val phantom2 = directAirPodsPro(status = 0x13, rawCaseLidState = 0x11).copy(identifier = sharedId)
+
+        val known = knownDeviceOf(phantom1, phantom2)
+        val result = with(testFactory) { known.getLatestCaseLidState(phantom2) }
+
+        // No authoritative reading anywhere → fall back to a coarse signal, never a guessed OPEN.
+        result shouldBe DualApplePods.LidState.NOT_IN_CASE
+    }
+
+    @Test
+    fun `case lid - first-seen out-of-case frame does not leak a phantom OPEN through the factory (issue 598)`() = runTest {
+        // Going through the real factory, a bit4-only out-of-case frame must never surface a phantom
+        // OPEN. The exact non-OPEN value depends on whether the device already has history
+        // (UNKNOWN when first-seen, NOT_IN_CASE once a single phantom frame is in history) — both are
+        // acceptable; what matters is that it is never OPEN.
+        create<DualApplePods>(
+            hex = "07 19 01 0E 20 13 AA B5 11 00 00 E0 0C A7 8A 60 4B D3 7D F4 60 4F 2C 73 E9 A7 F4",
+            address = "AA:BB:CC:DD:EE:01",
+        ) {
+            caseLidState shouldNotBe DualApplePods.LidState.OPEN
+        }
+    }
+
    @Test
    fun `test AirPodDevice - connection state`() = runTest {
        // Disconnected
@@ -1,7 +1,10 @@
 package eu.darken.capod.reaction.core.popup

+import eu.darken.capod.monitor.core.PodDevice
 import eu.darken.capod.pods.core.apple.ble.devices.DualApplePods
 import io.kotest.matchers.shouldBe
+import io.kotest.matchers.types.shouldBeInstanceOf
+import io.mockk.every
 import io.mockk.mockk
 import org.junit.jupiter.api.BeforeEach
 import org.junit.jupiter.api.Nested
@@ -189,4 +192,103 @@ class PopUpReactionLogicTest : BaseTest() {
            ).shouldShow shouldBe false
        }
    }
+
+    @Nested
+    inner class CooldownGuardTests {
+
+        private val timeSource = TestTimeSource(wallNow = Instant.parse("2026-01-01T00:00:00Z"))
+        private val reaction = PopUpReaction(
+            deviceMonitor = mockk(relaxed = true),
+            bluetoothManager = mockk(relaxed = true),
+            timeSource = timeSource,
+        )
+
+        @Test
+        fun `UNKNOWN hide does not refresh the show cooldown (issue 598)`() {
+            // Show on OPEN → cooldown stamped now.
+            reaction.throttleCasePopUps(device(DualApplePods.LidState.OPEN))
+                .shouldBeInstanceOf<PopUpReaction.Event.PopupShow>()
+
+            // Long past the 10s cooldown, a transient out-of-case frame hides the popup...
+            timeSource.advanceBy(Duration.ofSeconds(11))
+            reaction.throttleCasePopUps(device(DualApplePods.LidState.UNKNOWN))
+                .shouldBeInstanceOf<PopUpReaction.Event.PopupHide>()
+
+            // ...and must NOT have refreshed the cooldown: a genuine OPEN 1s later still shows.
+            timeSource.advanceBy(Duration.ofSeconds(1))
+            reaction.throttleCasePopUps(device(DualApplePods.LidState.OPEN))
+                .shouldBeInstanceOf<PopUpReaction.Event.PopupShow>()
+        }
+
+        @Test
+        fun `OPEN within cooldown is still throttled`() {
+            reaction.throttleCasePopUps(device(DualApplePods.LidState.OPEN))
+                .shouldBeInstanceOf<PopUpReaction.Event.PopupShow>()
+            timeSource.advanceBy(Duration.ofSeconds(5))
+            reaction.throttleCasePopUps(device(DualApplePods.LidState.OPEN)) shouldBe null
+        }
+
+        private fun device(lid: DualApplePods.LidState?) = mockDevice(lid = lid)
+    }
+
+    @Nested
+    inner class StaleCloseTests {
+
+        private val timeSource = TestTimeSource(wallNow = Instant.parse("2026-01-01T00:00:00Z"))
+        private val reaction = PopUpReaction(
+            deviceMonitor = mockk(relaxed = true),
+            bluetoothManager = mockk(relaxed = true),
+            timeSource = timeSource,
+        )
+
+        @Test
+        fun `fresh OPEN broadcast keeps the popup`() {
+            reaction.isCaseOpenBroadcastFresh(mockDevice(DualApplePods.LidState.OPEN, lastSeen = timeSource.now())) shouldBe true
+        }
+
+        @Test
+        fun `OPEN broadcast older than the timeout does not keep the popup`() {
+            val stale = timeSource.now().minus(Duration.ofSeconds(10))
+            reaction.isCaseOpenBroadcastFresh(mockDevice(DualApplePods.LidState.OPEN, lastSeen = stale)) shouldBe false
+        }
+
+        @Test
+        fun `CLOSED lid is not kept open`() {
+            reaction.isCaseOpenBroadcastFresh(mockDevice(DualApplePods.LidState.CLOSED, lastSeen = timeSource.now())) shouldBe false
+        }
+
+        @Test
+        fun `null lid (dropped to cache, out of range) is not kept open`() {
+            reaction.isCaseOpenBroadcastFresh(mockDevice(lid = null, lastSeen = timeSource.now())) shouldBe false
+        }
+
+        @Test
+        fun `ineligible device is not kept open`() {
+            reaction.isCaseOpenBroadcastFresh(
+                mockDevice(DualApplePods.LidState.OPEN, eligible = false, lastSeen = timeSource.now())
+            ) shouldBe false
+        }
+
+        @Test
+        fun `null device is not kept open`() {
+            reaction.isCaseOpenBroadcastFresh(null) shouldBe false
+        }
+    }
+
+    private fun mockDevice(
+        lid: DualApplePods.LidState?,
+        eligible: Boolean = true,
+        lastSeen: Instant = Instant.parse("2026-01-01T00:00:00Z"),
+        profile: String? = "profile-1",
+    ): PodDevice = mockk(relaxed = true) {
+        every { caseLidState } returns lid
+        every { reactions } returns mockk(relaxed = true) {
+            every { showPopUpOnCaseOpen } returns eligible
+        }
+        // isCaseOpenBroadcastFresh reads BLE freshness via device.ble?.seenLastAt.
+        every { ble } returns mockk(relaxed = true) {
+            every { seenLastAt } returns lastSeen
+        }
+        every { profileId } returns profile
+    }
 }