mirror of
https://github.com/d4rken-org/capod.git
synced 2026-09-14 18:26:11 -04:00
fix(monitor): Re-arm the teardown when a start request is short-circuited
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.
This commit is contained in:
@@ -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<Unit> {
|
||||
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<MonitorModeState>,
|
||||
startSignal: StateFlow<Long>,
|
||||
timeoutMillis: Long = 15 * 1000,
|
||||
onTeardown: () -> Unit,
|
||||
): Flow<Unit> = 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<Unit> {
|
||||
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<DeviceProfile>,
|
||||
|
||||
@@ -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<BluetoothAddress> = setOf(KNOWN_ADDRESS),
|
||||
connectedAddresses: Set<BluetoothAddress> = 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"
|
||||
}
|
||||
}
|
||||
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user