diff --git a/app/src/main/java/eu/darken/capod/pods/core/apple/ble/devices/ApplePodsFactory.kt b/app/src/main/java/eu/darken/capod/pods/core/apple/ble/devices/ApplePodsFactory.kt index 8a675f32..21da970f 100644 --- a/app/src/main/java/eu/darken/capod/pods/core/apple/ble/devices/ApplePodsFactory.kt +++ b/app/src/main/java/eu/darken/capod/pods/core/apple/ble/devices/ApplePodsFactory.kt @@ -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() + .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() + // Last resort: any non-UNKNOWN from recent history + return recentHistory .lastOrNull { it.caseLidState != DualApplePods.LidState.UNKNOWN } ?.caseLidState ?: DualApplePods.LidState.NOT_IN_CASE diff --git a/app/src/main/java/eu/darken/capod/pods/core/apple/ble/devices/DualApplePods.kt b/app/src/main/java/eu/darken/capod/pods/core/apple/ble/devices/DualApplePods.kt index 8d141150..07fd298c 100644 --- a/app/src/main/java/eu/darken/capod/pods/core/apple/ble/devices/DualApplePods.kt +++ b/app/src/main/java/eu/darken/capod/pods/core/apple/ble/devices/DualApplePods.kt @@ -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 diff --git a/app/src/main/java/eu/darken/capod/reaction/core/autoconnect/AutoConnect.kt b/app/src/main/java/eu/darken/capod/reaction/core/autoconnect/AutoConnect.kt index d8ce7c69..0703fed5 100644 --- a/app/src/main/java/eu/darken/capod/reaction/core/autoconnect/AutoConnect.kt +++ b/app/src/main/java/eu/darken/capod/reaction/core/autoconnect/AutoConnect.kt @@ -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 diff --git a/app/src/main/java/eu/darken/capod/reaction/core/popup/PopUpReaction.kt b/app/src/main/java/eu/darken/capod/reaction/core/popup/PopUpReaction.kt index 58e82f78..d07e4743 100644 --- a/app/src/main/java/eu/darken/capod/reaction/core/popup/PopUpReaction.kt +++ b/app/src/main/java/eu/darken/capod/reaction/core/popup/PopUpReaction.kt @@ -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 = 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 = 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 = 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 = flow { + while (true) { + emit(Unit) + delay(STALE_CHECK_INTERVAL.toMillis()) + } + } + + fun monitor(): Flow = 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) } } diff --git a/app/src/test/java/eu/darken/capod/pods/core/apple/ble/devices/DualApplePodsTest.kt b/app/src/test/java/eu/darken/capod/pods/core/apple/ble/devices/DualApplePodsTest.kt index a35abb92..da2452be 100644 --- a/app/src/test/java/eu/darken/capod/pods/core/apple/ble/devices/DualApplePodsTest.kt +++ b/app/src/test/java/eu/darken/capod/pods/core/apple/ble/devices/DualApplePodsTest.kt @@ -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( + 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 diff --git a/app/src/test/java/eu/darken/capod/reaction/core/popup/PopUpReactionLogicTest.kt b/app/src/test/java/eu/darken/capod/reaction/core/popup/PopUpReactionLogicTest.kt index 1fa5d0de..1ffe63bd 100644 --- a/app/src/test/java/eu/darken/capod/reaction/core/popup/PopUpReactionLogicTest.kt +++ b/app/src/test/java/eu/darken/capod/reaction/core/popup/PopUpReactionLogicTest.kt @@ -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() + + // 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() + + // ...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() + } + + @Test + fun `OPEN within cooldown is still throttled`() { + reaction.throttleCasePopUps(device(DualApplePods.LidState.OPEN)) + .shouldBeInstanceOf() + 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 + } }