From 46fcf5a4cb186c1544dafab08ecdb46b37016f11 Mon Sep 17 00:00:00 2001 From: darken Date: Mon, 13 Apr 2026 17:53:44 +0200 Subject: [PATCH] fix(popup): use fresh broadcast timing for connection popup --- .../reaction/core/popup/PopUpReaction.kt | 16 ++++++++++----- .../core/popup/PopUpReactionLogicTest.kt | 20 +++++++++++++------ 2 files changed, 25 insertions(+), 11 deletions(-) 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 27af0e00..58e82f78 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 @@ -166,16 +166,18 @@ class PopUpReaction @Inject constructor( return@mapNotNull null } - val deviceSeenFirst = currentBroadcasted.seenFirstAt ?: return@mapNotNull null + // Use the most recent BLE advertisement, not the first time this device was ever seen. + // Connection popups are meant to verify that a connection coincides with a fresh broadcast. + val broadcastSeenLast = currentBroadcasted.ble?.seenLastAt ?: return@mapNotNull null val now = timeSource.now() - val deviceAge = Duration.between(deviceSeenFirst, now) + val broadcastAge = Duration.between(broadcastSeenLast, now) val connectionAge = Duration.between(currentConnected.seenFirstAt, now) val decision = evaluateConnectionPopUp( hasConnectedDevice = true, hasPodDevice = true, hasAlreadyShown = connectionCoolDowns.containsKey(currentConnected.address), - deviceAge = deviceAge, + broadcastAge = broadcastAge, connectionAge = connectionAge, ) @@ -264,8 +266,9 @@ class PopUpReaction @Inject constructor( hasConnectedDevice: Boolean, hasPodDevice: Boolean, hasAlreadyShown: Boolean, - deviceAge: Duration, + broadcastAge: Duration, connectionAge: Duration, + maxConnectionAge: Duration = Duration.ofSeconds(30), maxAgeDiff: Duration = Duration.ofSeconds(30), ): ConnectionPopUpDecision { if (!hasConnectedDevice) { @@ -274,7 +277,10 @@ class PopUpReaction @Inject constructor( if (!hasPodDevice) { return ConnectionPopUpDecision(false, "No pod device found") } - if (deviceAge.abs() > (connectionAge.abs() + maxAgeDiff)) { + if (connectionAge.abs() > maxConnectionAge) { + return ConnectionPopUpDecision(false, "Connection is no longer new") + } + if (broadcastAge.abs() > (connectionAge.abs() + maxAgeDiff)) { return ConnectionPopUpDecision(false, "Broadcast too old, likely false positive") } if (hasAlreadyShown) { 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 19724cdd..1fa5d0de 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 @@ -117,13 +117,13 @@ class PopUpReactionLogicTest : BaseTest() { hasConnectedDevice: Boolean = true, hasPodDevice: Boolean = true, hasAlreadyShown: Boolean = false, - deviceAge: Duration = Duration.ofSeconds(5), + broadcastAge: Duration = Duration.ofSeconds(5), connectionAge: Duration = Duration.ofSeconds(5), ) = popUpReaction.evaluateConnectionPopUp( hasConnectedDevice = hasConnectedDevice, hasPodDevice = hasPodDevice, hasAlreadyShown = hasAlreadyShown, - deviceAge = deviceAge, + broadcastAge = broadcastAge, connectionAge = connectionAge, ) @@ -142,6 +142,14 @@ class PopUpReactionLogicTest : BaseTest() { evaluate(hasPodDevice = false).shouldShow shouldBe false } + @Test + fun `connected long ago - should NOT show`() { + evaluate( + broadcastAge = Duration.ofSeconds(5), + connectionAge = Duration.ofSeconds(60), + ).shouldShow shouldBe false + } + @Test fun `already shown for this connection - should NOT show`() { evaluate(hasAlreadyShown = true).shouldShow shouldBe false @@ -150,7 +158,7 @@ class PopUpReactionLogicTest : BaseTest() { @Test fun `broadcast device age much older than connection (false positive) - should NOT show`() { evaluate( - deviceAge = Duration.ofSeconds(60), + broadcastAge = Duration.ofSeconds(60), connectionAge = Duration.ofSeconds(5), ).shouldShow shouldBe false } @@ -158,7 +166,7 @@ class PopUpReactionLogicTest : BaseTest() { @Test fun `broadcast device age within threshold of connection - should show`() { evaluate( - deviceAge = Duration.ofSeconds(20), + broadcastAge = Duration.ofSeconds(20), connectionAge = Duration.ofSeconds(5), ).shouldShow shouldBe true } @@ -166,7 +174,7 @@ class PopUpReactionLogicTest : BaseTest() { @Test fun `broadcast device age exactly at threshold - should NOT show`() { evaluate( - deviceAge = Duration.ofSeconds(36), + broadcastAge = Duration.ofSeconds(36), connectionAge = Duration.ofSeconds(5), ).shouldShow shouldBe false } @@ -176,7 +184,7 @@ class PopUpReactionLogicTest : BaseTest() { // Duration.between(now, pastTimestamp) produces negative durations. // The false-positive filter must work regardless of sign. evaluate( - deviceAge = Duration.ofSeconds(-60), + broadcastAge = Duration.ofSeconds(-60), connectionAge = Duration.ofSeconds(-5), ).shouldShow shouldBe false }