From 7712b1838850d22f0676dc4c8d01965e22335022 Mon Sep 17 00:00:00 2001 From: darken Date: Mon, 31 Aug 2026 12:24:45 +0200 Subject: [PATCH] fix(monitor): Re-arm the teardown when a start request is short-circuited MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In AUTOMATIC mode with nothing connected the session is torn down 15 seconds after the state that said so, and only a new state emission cancels that. A start request arriving while a session is live is acknowledged and returns without touching the pending teardown, and the connection state that would abort it lags the Bluetooth event that caused the start by roughly 0.75s. A request landing in the tail of the window was therefore answered with "keeping current session" and the session went down anyway, taking every reaction with it — in one recording the popup reaction was down for 34 seconds spanning an entire lid cycle. The short-circuit now bumps a start signal that the mode pipeline combines in, so the bump cancels the pending inner flow through the existing cancellation topology and arms a fresh window. The countdown also re-reads the signal after its delay, which closes the case where the bump lands while an expired countdown is already unwinding. A re-armed window runs 15 seconds from the start request, so the total dwell can exceed 15 seconds: the request is fresh evidence of activity. The decision segment moves to a top-level internal function so it can be driven directly in tests, following MonitorModeState and buildMonitorModeState which are top-level for the same reason. --- .../monitor/core/worker/MonitorService.kt | 99 +++++++++----- .../core/worker/MonitorModeFlowTest.kt | 125 ++++++++++++++++++ .../monitor/core/worker/MonitorServiceTest.kt | 19 +++ 3 files changed, 211 insertions(+), 32 deletions(-) create mode 100644 app/src/test/java/eu/darken/capod/monitor/core/worker/MonitorModeFlowTest.kt diff --git a/app/src/main/java/eu/darken/capod/monitor/core/worker/MonitorService.kt b/app/src/main/java/eu/darken/capod/monitor/core/worker/MonitorService.kt index 4319578d..2a2f4570 100644 --- a/app/src/main/java/eu/darken/capod/monitor/core/worker/MonitorService.kt +++ b/app/src/main/java/eu/darken/capod/monitor/core/worker/MonitorService.kt @@ -55,6 +55,9 @@ import kotlinx.coroutines.Job import kotlinx.coroutines.cancel import kotlinx.coroutines.cancelChildren import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.catch import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.distinctUntilChanged @@ -95,6 +98,7 @@ class MonitorService : Service() { private val monitorScope = MonitorCoroutineScope() private var monitoringJob: Job? = null + private val startSignal = MutableStateFlow(0L) @Volatile private var monitorGeneration = 0 private var foregroundStartFailed = false private var injectionComplete = false @@ -212,6 +216,8 @@ class MonitorService : Service() { if (monitoringJob?.isActive == true && !forceStart) { log(TAG) { "Already monitoring and forceStart=false, keeping current session." } + // Fresh evidence of activity: re-arm any pending teardown countdown. + startSignal.value++ return START_STICKY } @@ -314,7 +320,7 @@ class MonitorService : Service() { } .launchIn(monitorScope) - permissionTool.missingScanPermissions + val modeStates = permissionTool.missingScanPermissions .flatMapLatest { missingPermsFlow -> if (missingPermsFlow.isNotEmpty()) { log(TAG, WARN) { "Aborting, scan permissions are missing: $missingPermsFlow" } @@ -333,38 +339,13 @@ class MonitorService : Service() { } .distinctUntilChanged() .setupCommonEventHandlers(TAG) { "MonitorMode" } - .flatMapLatest { state -> - log(TAG) { "Monitor mode: ${state.mode}" } - log(TAG) { "connectedAddresses: ${state.connectedAddresses}" } - log(TAG) { "knownAddresses: ${state.knownAddresses}" } - when (state.mode) { - MonitorMode.MANUAL -> flow { - monitorScope.coroutineContext.cancelChildren() - } - - MonitorMode.ALWAYS -> emptyFlow() - MonitorMode.AUTOMATIC -> flow { - when { - !state.hasProfiles && state.connectedAddresses.isNotEmpty() -> { - log(TAG, WARN) { "Main device address not set, staying alive while any is connected" } - } - - state.knownAddresses.any { it in state.connectedAddresses } || state.hasAapSession -> { - log(TAG) { "A device is connected, aborting any timeout." } - } - - else -> { - log(TAG) { "No known Pods are connected, stopping service soon." } - delay(15 * 1000) - log(TAG) { "Stopping service now, still no Pods connected." } - - monitorScope.coroutineContext.cancelChildren() - } - } - } - } - } + monitorModeFlow( + tag = TAG, + modeStates = modeStates, + startSignal = startSignal, + onTeardown = { monitorScope.coroutineContext.cancelChildren() }, + ) .catch { log(TAG, WARN) { "MonitorMode Flow failed:\n${it.asLog()}" } } @@ -492,6 +473,60 @@ internal data class MonitorModeState( val hasAapSession: Boolean, ) +/** + * Decides whether a monitor session may keep running. In [MonitorMode.AUTOMATIC] with nothing + * connected the teardown runs [timeoutMillis] after the state was seen. + * + * [startSignal] is bumped by a start request that found a live session and short-circuited. The + * bump restarts this flow, arming a fresh window, and the pre-teardown re-check covers the case + * where the bump lands while the expired countdown is already unwinding. + */ +internal fun monitorModeFlow( + tag: String, + modeStates: Flow, + startSignal: StateFlow, + timeoutMillis: Long = 15 * 1000, + onTeardown: () -> Unit, +): Flow = combine(modeStates, startSignal) { state, signal -> + state to signal +}.flatMapLatest { (state, armedSignal) -> + log(tag) { "Monitor mode: ${state.mode}" } + log(tag) { "connectedAddresses: ${state.connectedAddresses}" } + log(tag) { "knownAddresses: ${state.knownAddresses}" } + + when (state.mode) { + MonitorMode.MANUAL -> flow { + onTeardown() + } + + MonitorMode.ALWAYS -> emptyFlow() + MonitorMode.AUTOMATIC -> flow { + when { + !state.hasProfiles && state.connectedAddresses.isNotEmpty() -> { + log(tag, WARN) { "Main device address not set, staying alive while any is connected" } + } + + state.knownAddresses.any { it in state.connectedAddresses } || state.hasAapSession -> { + log(tag) { "A device is connected, aborting any timeout." } + } + + else -> { + log(tag) { "No known Pods are connected, stopping service soon." } + delay(timeoutMillis) + + if (startSignal.value != armedSignal) { + log(tag) { "A start request arrived during the timeout, staying alive." } + return@flow + } + log(tag) { "Stopping service now, still no Pods connected." } + + onTeardown() + } + } + } + } +} + internal fun buildMonitorModeState( mode: MonitorMode, profiles: List, diff --git a/app/src/test/java/eu/darken/capod/monitor/core/worker/MonitorModeFlowTest.kt b/app/src/test/java/eu/darken/capod/monitor/core/worker/MonitorModeFlowTest.kt new file mode 100644 index 00000000..0b52185e --- /dev/null +++ b/app/src/test/java/eu/darken/capod/monitor/core/worker/MonitorModeFlowTest.kt @@ -0,0 +1,125 @@ +package eu.darken.capod.monitor.core.worker + +import eu.darken.capod.common.bluetooth.BluetoothAddress +import eu.darken.capod.main.core.MonitorMode +import io.kotest.matchers.shouldBe +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.test.advanceTimeBy +import kotlinx.coroutines.test.runCurrent +import kotlinx.coroutines.test.runTest +import org.junit.jupiter.api.Test +import testhelpers.BaseTest + +/** + * The AUTOMATIC teardown is only cancelled by a new state emission, but a start request that finds + * a live session short-circuits without producing one — the connection state that would abort the + * countdown lags the Bluetooth event by roughly a second. A request landing in the tail of the + * window used to be acknowledged and the session torn down anyway. + */ +class MonitorModeFlowTest : BaseTest() { + + private fun state( + mode: MonitorMode = MonitorMode.AUTOMATIC, + hasProfiles: Boolean = true, + knownAddresses: Set = setOf(KNOWN_ADDRESS), + connectedAddresses: Set = emptySet(), + hasAapSession: Boolean = false, + ) = MonitorModeState( + mode = mode, + hasProfiles = hasProfiles, + knownAddresses = knownAddresses, + connectedAddresses = connectedAddresses, + hasAapSession = hasAapSession, + ) + + @Test + fun `nothing connected tears the session down after the timeout`() = runTest { + var teardowns = 0 + val job = monitorModeFlow( + tag = TAG, + modeStates = flowOf(state()), + startSignal = MutableStateFlow(0L), + onTeardown = { teardowns++ }, + ).launchIn(this) + + advanceTimeBy(TIMEOUT) + teardowns shouldBe 0 + + runCurrent() + teardowns shouldBe 1 + + job.cancel() + } + + @Test + fun `a start request during the window re-arms the countdown`() = runTest { + var teardowns = 0 + val startSignal = MutableStateFlow(0L) + val job = monitorModeFlow( + tag = TAG, + modeStates = flowOf(state()), + startSignal = startSignal, + onTeardown = { teardowns++ }, + ).launchIn(this) + + val bumpedAt = 14_750L + advanceTimeBy(bumpedAt) + runCurrent() + startSignal.value++ + + // The original window would have expired here. + advanceTimeBy(TIMEOUT - bumpedAt) + runCurrent() + teardowns shouldBe 0 + + // The re-armed window runs from the start request, not from the state emission. + advanceTimeBy(bumpedAt) + teardowns shouldBe 0 + + runCurrent() + teardowns shouldBe 1 + + job.cancel() + } + + @Test + fun `a connected device aborts the countdown`() = runTest { + var teardowns = 0 + val job = monitorModeFlow( + tag = TAG, + modeStates = flowOf(state(connectedAddresses = setOf(KNOWN_ADDRESS))), + startSignal = MutableStateFlow(0L), + onTeardown = { teardowns++ }, + ).launchIn(this) + + advanceTimeBy(10 * TIMEOUT) + runCurrent() + teardowns shouldBe 0 + + job.cancel() + } + + @Test + fun `manual mode tears the session down immediately`() = runTest { + var teardowns = 0 + val job = monitorModeFlow( + tag = TAG, + modeStates = flowOf(state(mode = MonitorMode.MANUAL)), + startSignal = MutableStateFlow(0L), + onTeardown = { teardowns++ }, + ).launchIn(this) + + runCurrent() + teardowns shouldBe 1 + + job.cancel() + } + + companion object { + private const val TAG = "MonitorModeFlowTest" + private const val TIMEOUT = 15 * 1000L + private const val KNOWN_ADDRESS = "AA:BB:CC:DD:EE:FF" + } +} diff --git a/app/src/test/java/eu/darken/capod/monitor/core/worker/MonitorServiceTest.kt b/app/src/test/java/eu/darken/capod/monitor/core/worker/MonitorServiceTest.kt index 1b7975d5..ec9d7521 100644 --- a/app/src/test/java/eu/darken/capod/monitor/core/worker/MonitorServiceTest.kt +++ b/app/src/test/java/eu/darken/capod/monitor/core/worker/MonitorServiceTest.kt @@ -13,6 +13,7 @@ import io.kotest.matchers.types.shouldNotBeSameInstanceAs import io.mockk.mockk import io.mockk.verify import kotlinx.coroutines.Job +import kotlinx.coroutines.flow.StateFlow import org.junit.Test import org.junit.runner.RunWith import org.robolectric.Robolectric @@ -42,6 +43,8 @@ class MonitorServiceTest { private fun MonitorService.getField(name: String): Any? = MonitorService::class.java.getDeclaredField(name).apply { isAccessible = true }.get(this) + private fun MonitorService.startSignal(): Long = (getField("startSignal") as StateFlow<*>).value as Long + private fun notification(title: String): Notification = NotificationCompat.Builder(context, MonitorNotifications.NOTIFICATION_CHANNEL_ID) .setContentTitle(title) @@ -204,6 +207,22 @@ class MonitorServiceTest { service.getField("lastNotification").shouldBeNull() } + /** + * A start request that finds a live session is acknowledged without touching it — including a + * teardown countdown that is already running. Bumping the signal is what re-arms that window. + */ + @Test + fun `a short-circuited start bumps the start signal`() { + val service = createService() + service.readyForMonitoring() + + val before = service.startSignal() + + service.onStartCommand(MonitorService.intent(context), 0, 1) shouldBe Service.START_STICKY + + service.startSignal() shouldBe before + 1 + } + @Test fun `onDestroy skips notification cleanup when injection never completed`() { val service = createService()