diff --git a/app/src/main/java/eu/darken/capod/reaction/core/conversation/ConversationReaction.kt b/app/src/main/java/eu/darken/capod/reaction/core/conversation/ConversationReaction.kt index 56c09cf7..0fff7b50 100644 --- a/app/src/main/java/eu/darken/capod/reaction/core/conversation/ConversationReaction.kt +++ b/app/src/main/java/eu/darken/capod/reaction/core/conversation/ConversationReaction.kt @@ -121,7 +121,7 @@ class ConversationReaction @Inject constructor( if (current != null && current.owner == address) { // Duplicate START for the same speaker — don't re-act, just keep the session alive. // A START also cancels a pending wind-down fuse: the wearer is speaking again. - keepAlive(current, STALE_TIMEOUT) + armDisengageTimer(current, STALE_TIMEOUT) log(TAG) { "START from $address — already active ($action), keep-alive" } return } @@ -180,7 +180,7 @@ class ConversationReaction @Inject constructor( private suspend fun onSpeakingResume(address: BluetoothAddress) = mutex.withLock { val current = active ?: return if (current.owner != address) return - keepAlive(current, STALE_TIMEOUT) + armDisengageTimer(current, STALE_TIMEOUT) log(TAG) { "RESUME from $address — speech resumed, keep-alive" } } @@ -193,7 +193,7 @@ class ConversationReaction @Inject constructor( private suspend fun onSpeakingHold(address: BluetoothAddress) = mutex.withLock { val current = active ?: return if (current.owner != address) return - keepAlive(current, WIND_DOWN_TIMEOUT) + armDisengageTimer(current, WIND_DOWN_TIMEOUT) } private suspend fun onSpeakingStop(address: BluetoothAddress) { @@ -205,12 +205,19 @@ class ConversationReaction @Inject constructor( return } clearActive() - disengage(current, primary, "STOP on $address") + // An explicit terminal frame is positive evidence CA ended now → resume regardless of how + // long ago we engaged. The age guard is only for inferred (stale-backstop) ends. + disengage(current, primary, "STOP on $address", applyAgeGuard = false) } } - /** Graceful disengage (STOP event or stale timeout). Must be called under [mutex]. */ - private suspend fun disengage(record: Active, primary: PodDevice?, reason: String) { + /** + * Graceful disengage (STOP event or timer expiry). Must be called under [mutex]. [applyAgeGuard] + * gates resume on [PAUSE_RESUME_WINDOW]: `true` only for the inferred stale-backstop end (we lost + * track and timed out — surprise-resuming long after is worse than a stranded pause); `false` for + * an explicit terminal or the wind-down fuse, where a recent/real end signal makes resume correct. + */ + private suspend fun disengage(record: Active, primary: PodDevice?, reason: String, applyAgeGuard: Boolean) { when (val kind = record.kind) { is Kind.Paused -> { // Resume the pause WE caused, regardless of the current action setting. Gating on @@ -219,7 +226,7 @@ class ConversationReaction @Inject constructor( // surprising behaviour. The remaining guards are about real device/playback state. val age = timeSource.elapsedRealtime() - record.at when { - age.milliseconds > PAUSE_RESUME_WINDOW -> + applyAgeGuard && age.milliseconds > PAUSE_RESUME_WINDOW -> log(TAG) { "$reason — resume skipped (stale, ${age}ms)" } primary?.address != record.owner -> log(TAG) { "$reason — resume skipped (primary switched)" } @@ -278,28 +285,17 @@ class ConversationReaction @Inject constructor( active = null } - /** - * Must be called under [mutex]. Keep-alive for the ongoing session [current]: refresh its - * activity timestamp and (re)arm the disengage timer. The timestamp refresh makes - * [PAUSE_RESUME_WINDOW] measure "time since the last frame" rather than "since engage" — without - * it a conversation longer than the window would fail the resume guard and strand media paused - * when its terminal finally arrives. A genuinely back-stopped session (no frames for the whole - * [STALE_TIMEOUT]) is unaffected: its timestamp is never refreshed, so it still won't auto-resume. - */ - private fun keepAlive(current: Active, timeout: Duration) { - val refreshed = current.copy(at = timeSource.elapsedRealtime()) - active = refreshed - armDisengageTimer(refreshed, timeout) - } - /** * Must be called under [mutex]. (Re)arms the disengage timer for [record] — every frame picks * the fuse matching its meaning: START / RESUME → [STALE_TIMEOUT] (active speech, frames cease * for its whole duration), HOLD → [WIND_DOWN_TIMEOUT] (wind-down begun, terminal imminent). On * expiry the session is force-ended. Identity-checked so a late timer can't disengage a newer - * session. + * session. The age guard ([PAUSE_RESUME_WINDOW]) is applied only when the long [STALE_TIMEOUT] + * fires — that is the purely-inferred "we lost track" end; the short wind-down fuse is driven by a + * recent HOLD and should resume even after a long conversation (dropped-terminal #608 recovery). */ private fun armDisengageTimer(record: Active, timeout: Duration) { + val applyAgeGuard = timeout == STALE_TIMEOUT staleJob?.cancel() staleJob = appScope.launch { delay(timeout) @@ -308,7 +304,7 @@ class ConversationReaction @Inject constructor( if (active?.id == record.id) { val current = active!! clearActive() - disengage(current, primary, "no terminal frame within $timeout") + disengage(current, primary, "no terminal frame within $timeout", applyAgeGuard = applyAgeGuard) } } } @@ -340,7 +336,12 @@ class ConversationReaction @Inject constructor( */ private val WIND_DOWN_TIMEOUT = 6.seconds - /** A pause older than this no longer auto-resumes — unexpected late playback is worse. */ + /** + * For an *inferred* end only (the [STALE_TIMEOUT] backstop fired — we lost track of the + * conversation), a pause older than this no longer auto-resumes: surprise playback long after + * is worse than a stranded pause. An explicit terminal frame, or the short wind-down fuse, + * resumes regardless of age — those carry real/recent evidence the conversation just ended. + */ private val PAUSE_RESUME_WINDOW = 2.minutes } } diff --git a/app/src/test/java/eu/darken/capod/reaction/core/conversation/ConversationReactionTest.kt b/app/src/test/java/eu/darken/capod/reaction/core/conversation/ConversationReactionTest.kt index b6b11704..1cda15f7 100644 --- a/app/src/test/java/eu/darken/capod/reaction/core/conversation/ConversationReactionTest.kt +++ b/app/src/test/java/eu/darken/capod/reaction/core/conversation/ConversationReactionTest.kt @@ -451,10 +451,10 @@ class ConversationReactionTest : BaseTest() { @Test fun `PAUSE resumes on the terminal even after a conversation longer than the resume window`() = runTest(UnconfinedTestDispatcher()) { - // Each keep-alive frame refreshes the activity timestamp, so PAUSE_RESUME_WINDOW is - // measured from the last frame — not from engage. Without that refresh a 3-minute - // conversation would fail the age guard and strand media paused on its real terminal. - // Advance BOTH clocks so the window is genuinely exercised (the TestTimeSource drives age). + // A bursty conversation that runs past PAUSE_RESUME_WINDOW: the explicit STOP must still + // resume. The age guard applies only to the inferred stale backstop, not to a real + // terminal. Advance BOTH clocks so the window is genuinely exercised (TestTimeSource + // drives `age`). devicesFlow.value = listOf(mockPodDevice(primaryAddress, ConversationAction.PAUSE)) val job = launchReaction() @@ -465,15 +465,56 @@ class ConversationReactionTest : BaseTest() { timeSource.advanceBy(java.time.Duration.ofSeconds(60)) advanceTimeBy(60_000) // < STALE_TIMEOUT, so the backstop never fires runCurrent() - emit(primaryAddress, ConversationAwarenessEvent.RESUME) // keep-alive, refreshes `at` + emit(primaryAddress, ConversationAwarenessEvent.RESUME) } coVerify(exactly = 0) { mediaControl.sendPlay() } // 3 min in, still paused - emit(primaryAddress, ConversationAwarenessEvent.STOP) // terminal right after last activity + emit(primaryAddress, ConversationAwarenessEvent.STOP) // explicit terminal coVerify(exactly = 1) { mediaControl.sendPlay() } job.cancel() } + @Test + fun `PAUSE resumes on a direct STOP after long frame silence`() = runTest(UnconfinedTestDispatcher()) { + // Continuous speech sends no frames; then a direct terminal (no preceding wind-down) arrives + // past PAUSE_RESUME_WINDOW but before STALE_TIMEOUT. An explicit STOP is positive evidence CA + // ended now, so it must resume regardless of engage-age. (Fails if the age guard is applied to + // explicit terminals.) + devicesFlow.value = listOf(mockPodDevice(primaryAddress, ConversationAction.PAUSE)) + val job = launchReaction() + + emit(primaryAddress, ConversationAwarenessEvent.START) + coVerify(exactly = 1) { mediaControl.sendPause(false) } + + timeSource.advanceBy(java.time.Duration.ofSeconds(150)) // > 2-min window, < 5-min backstop + advanceTimeBy(150_000) + runCurrent() + coVerify(exactly = 0) { mediaControl.sendPlay() } // still paused, no frames yet + + emit(primaryAddress, ConversationAwarenessEvent.STOP) + coVerify(exactly = 1) { mediaControl.sendPlay() } + job.cancel() + } + + @Test + fun `PAUSE does not resume when the stale backstop fires past the resume window`() = + runTest(UnconfinedTestDispatcher()) { + // The inferred end (5-min backstop, no terminal ever) keeps the age guard: a long-stale + // pause must NOT surprise-resume. + devicesFlow.value = listOf(mockPodDevice(primaryAddress, ConversationAction.PAUSE)) + val job = launchReaction() + + emit(primaryAddress, ConversationAwarenessEvent.START) + coVerify(exactly = 1) { mediaControl.sendPause(false) } + + timeSource.advanceBy(java.time.Duration.ofMinutes(5)) + advanceTimeBy(5L * 60 * 1000 + 500) // STALE_TIMEOUT fires + runCurrent() + + coVerify(exactly = 0) { mediaControl.sendPlay() } // inferred stale end → not resumed + job.cancel() + } + @Test fun `RESUME without a prior start is a no-op`() = runTest(UnconfinedTestDispatcher()) { devicesFlow.value = listOf(mockPodDevice(primaryAddress, ConversationAction.PAUSE))