fix(reaction): Don't resume media mid-talk on Conversational Awareness

The pod only signals CA start and end, not continuous keep-alives. On fw 6861 it held CA engaged for 21s with zero 0x4B frames while the wearer kept talking, so the 12s stale-timeout fired mid-speech and resumed media; the pod never re-sent a start frame, so it didn't re-pause. Disengage now waits for the explicit not-speaking frame (status 5 added as a terminal STOP, since that firmware winds down 3->5 and never reaches 6/8/9). Transitional/unknown statuses stay engaged. The stale timer is demoted to a long 5min backstop for a fully-dropped terminal frame; constants moved to Kotlin Durations.
2026-09-15 02:36:12 -04:00 · 2026-05-31 06:39:52 +02:00
parent dd827af3cd
commit 5ee777b196
5 changed files with 126 additions and 38 deletions
@@ -102,10 +102,10 @@ sealed class AapSetting {
    /**
     * Push-only from device — reports speaking detection state (command 0x4B).
     *
-     * [rawValue] is the first payload byte, preserved so consumers can distinguish the known
-     * speaking-start (0x01) and speaking-stop (0x04) markers from other values (e.g. 0x00, seen
-     * in captures with unclear meaning). [speaking] collapses everything non-0x01 to false for
-     * storage/UI; reaction logic must gate on [rawValue] to avoid acting on unknown values.
+     * [rawValue] is the status byte: the last byte of the 4-byte `02 00 01 XX` form (or the single
+     * byte of the legacy form), preserved so consumers can classify it. [speaking] is `true` only
+     * for the speaking-onset statuses (`1`, `2`); every other value (`0`, `3`, `4`, `5`, `0x0B`, …)
+     * is `false`. START/STOP/HOLD classification for the reaction lives in [ConversationAwarenessEvent].
     */
    data class ConversationalAwarenessState(
        val speaking: Boolean,
@@ -3,14 +3,19 @@ package eu.darken.capod.pods.core.apple.aap.protocol
 /**
 * Classified Conversational Awareness signal derived from the status byte of a `0x4B` frame.
 *
- * Status-byte mapping (confirmed against a live AirPods Pro 3 capture and the librepods project):
+ * Status-byte mapping (from live AirPods Pro 3 captures + the librepods project):
 * - `1`, `2` → [START] (wearer started / is speaking → engage the reaction)
- * - `6`, `8`, `9` → [STOP] (wearer stopped → disengage)
- * - any other value (`3`, `4`, `0x0B`, …) → [HOLD] (intermediate "still in session" frame; the pod
- *   streams these while speaking — they act as a keep-alive and must NOT disengage the reaction)
+ * - `5`, `6`, `8`, `9` → [STOP] (wearer stopped → disengage). `5` is the terminal value on fw `…6861`,
+ *   which winds down `3`→`5` and never reaches `6/8/9`; `6/8/9` are the terminal values on fw `…6503`.
+ * - any other value (`0`, `3`, `4`, `0x0B`, … and anything unrecognised) → [HOLD]: a transitional or
+ *   unknown frame. It must NOT disengage the reaction — only an explicit terminal [STOP] does that.
 *
- * The pod emits no `0x4B` frames at all during silence, so [HOLD] frames ceasing is itself a
- * reliable "speaking ended" signal (used as a stale-timeout fallback for a missed [STOP]).
+ * Frame cadence is firmware-dependent and the pod does NOT reliably stream keep-alives while you
+ * talk: fw `…6861` sent only an onset (`1`,`2`) then NO `0x4B` frames for 21s of continuous speech
+ * (proven still-speaking — it held its own CA/ANC-transparency engaged the whole time), then the
+ * wind-down `3`,`5`. So frame-silence must NOT be read as "speaking ended"; disengage is driven by
+ * the explicit terminal [STOP] frame. [ConversationReaction]'s stale timeout is only a long backstop
+ * for a fully-dropped terminal frame, not the normal disengage path.
 */
 enum class ConversationAwarenessEvent {
    START,
@@ -20,7 +25,7 @@ enum class ConversationAwarenessEvent {

    companion object {
        val SPEAKING_STATUSES = setOf(1, 2)
-        val STOPPED_STATUSES = setOf(6, 8, 9)
+        val STOPPED_STATUSES = setOf(5, 6, 8, 9)

        fun fromStatus(status: Int): ConversationAwarenessEvent = when (status) {
            in SPEAKING_STATUSES -> START
@@ -30,17 +30,22 @@ import kotlinx.coroutines.sync.withLock
 import kotlinx.coroutines.withContext
 import javax.inject.Inject
 import javax.inject.Singleton
+import kotlin.time.Duration.Companion.milliseconds
+import kotlin.time.Duration.Companion.minutes

 /**
 * Reacts to Conversational Awareness speaking transitions (AAP `0x4B`) by either lowering media
 * volume or pausing, per the primary device's [ReactionConfig.conversationAction], and reverts when
 * speaking stops. On Android the pod firmware does not duck audio itself, so CAPod performs it.
 *
- * The pod streams classified frames while you talk ([ConversationAwarenessEvent.START] at onset,
- * then [ConversationAwarenessEvent.HOLD] keep-alives) and an explicit [ConversationAwarenessEvent.STOP]
- * when you finish; no frames at all during silence. Disengage happens on STOP, or — if a STOP is
- * dropped — via a stale timeout that fires once frames stop arriving. Each frame (START or HOLD)
- * resets that timer, so a long conversation stays engaged.
+ * Disengage is driven by the pod's explicit end-of-speech frame ([ConversationAwarenessEvent.STOP]).
+ * The pod does NOT reliably stream keep-alive frames while you talk — on some firmware it sends an
+ * onset ([ConversationAwarenessEvent.START]) then nothing for many seconds while speech continues
+ * (observed: CA held engaged 21s with zero `0x4B` frames). So frame-silence must NOT be read as
+ * "speaking ended". [STALE_TIMEOUT] is only a long backstop for the rare case where every terminal
+ * frame is lost while the link stays up; a link drop is handled by the owner-disconnect revert.
+ * [ConversationAwarenessEvent.HOLD] frames (transitional/unknown statuses) keep the reaction engaged
+ * and refresh the backstop.
 *
 * State is a single global slot (media volume / playback is system-wide, not per-device) guarded by
 * a [Mutex] — events, AAP-state-removal, the stale timer, and monitor completion all mutate it.
@@ -182,7 +187,7 @@ class ConversationReaction @Inject constructor(
                // surprising behaviour. The remaining guards are about real device/playback state.
                val age = timeSource.elapsedRealtime() - record.at
                when {
-                    age > PAUSE_RESUME_WINDOW_MS ->
+                    age.milliseconds > PAUSE_RESUME_WINDOW ->
                        log(TAG) { "$reason — resume skipped (stale, ${age}ms)" }
                    primary?.address != record.owner ->
                        log(TAG) { "$reason — resume skipped (primary switched)" }
@@ -242,15 +247,15 @@ class ConversationReaction @Inject constructor(
    }

    /**
-     * Must be called under [mutex]. (Re)starts the stale timer for [record]. Reset on every frame
-     * (START/HOLD); if no frame arrives for [STALE_TIMEOUT_MS] the speaking session is treated as
-     * ended (recovers from a dropped STOP). Identity-checked so a late timer can't disengage a newer
-     * session.
+     * Must be called under [mutex]. (Re)starts the backstop timer for [record], reset on every frame
+     * (START/HOLD). If no frame arrives for [STALE_TIMEOUT] the session is force-ended — a last
+     * resort for a fully-dropped terminal frame, not the normal disengage. Identity-checked so a
+     * late timer can't disengage a newer session.
     */
    private fun restartStaleTimer(record: Active) {
        staleJob?.cancel()
        staleJob = appScope.launch {
-            delay(STALE_TIMEOUT_MS)
+            delay(STALE_TIMEOUT)
            val primary = deviceMonitor.primaryDevice().first()
            mutex.withLock {
                if (active?.id == record.id) {
@@ -268,14 +273,17 @@ class ConversationReaction @Inject constructor(
        private val TAG = logTag("Reaction", "Conversation")

        /**
-         * Disengage if no `0x4B` frame arrives for this long while engaged. The pod streams frames
-         * (~1/s) throughout active speech and none during silence, so this both recovers a dropped
-         * STOP and bounds how long a duck can linger. Long enough not to disengage mid-conversation
-         * between frames.
+         * Pure backstop: disengage if no `0x4B` frame arrives for this long while engaged. This is
+         * NOT the normal disengage path — the pod does not stream keep-alives during speech, so a
+         * short timeout would fire mid-conversation (the original 12s value did exactly that).
+         * Normal disengage is the explicit terminal [ConversationAwarenessEvent.STOP] frame; this
+         * only recovers a fully-dropped terminal frame while the link stays up. Kept longer than
+         * [PAUSE_RESUME_WINDOW] so a back-stopped pause is never auto-resumed (only a duck is
+         * restored on stale — a stranded low volume is the worse failure).
         */
-        private const val STALE_TIMEOUT_MS = 12L * 1000L
+        private val STALE_TIMEOUT = 5.minutes

-        /** A disengage older than this no longer auto-resumes a pause — unexpected late playback is worse. */
-        private const val PAUSE_RESUME_WINDOW_MS = 2L * 60L * 1000L
+        /** A pause older than this no longer auto-resumes — unexpected late playback is worse. */
+        private val PAUSE_RESUME_WINDOW = 2.minutes
    }
 }
@@ -529,16 +529,21 @@ class AapSessionEngineTest : BaseTest() {
        }

        @Test
-        fun `status 6, 8, 9 emit STOP`() = runTest(UnconfinedTestDispatcher()) {
+        fun `terminal statuses 5, 6, 8, 9 emit STOP`() = runTest(UnconfinedTestDispatcher()) {
+            // 5 is the terminal wind-down value on fw …6861 (never reaches 6/8/9); 6/8/9 on fw …6503.
+            firstEventFor(5) shouldBe ConversationAwarenessEvent.STOP
            firstEventFor(6) shouldBe ConversationAwarenessEvent.STOP
            firstEventFor(8) shouldBe ConversationAwarenessEvent.STOP
            firstEventFor(9) shouldBe ConversationAwarenessEvent.STOP
        }

        @Test
-        fun `intermediate status emits HOLD (keep-alive)`() = runTest(UnconfinedTestDispatcher()) {
+        fun `transitional and unknown statuses emit HOLD (stay engaged)`() = runTest(UnconfinedTestDispatcher()) {
+            // Must never disengage on these — only an explicit terminal STOP does.
            firstEventFor(3) shouldBe ConversationAwarenessEvent.HOLD
+            firstEventFor(4) shouldBe ConversationAwarenessEvent.HOLD
            firstEventFor(0x0B) shouldBe ConversationAwarenessEvent.HOLD
+            firstEventFor(7) shouldBe ConversationAwarenessEvent.HOLD
        }
    }

@@ -31,8 +31,9 @@ class ConversationReactionTest : BaseTest() {
    private val primaryAddress: BluetoothAddress = "AA:BB:CC:DD:EE:FF"
    private val otherAddress: BluetoothAddress = "11:22:33:44:55:66"

-    // Mirror of ConversationReaction.STALE_TIMEOUT_MS (private there).
-    private val staleTimeoutMs = 12_000L
+    // Mirror of ConversationReaction.STALE_TIMEOUT (private there) — a long backstop, not the
+    // normal disengage path (which is the explicit STOP frame).
+    private val staleTimeoutMs = 5L * 60 * 1000

    private lateinit var eventsFlow: MutableSharedFlow<Pair<BluetoothAddress, ConversationAwarenessEvent>>
    private lateinit var statesFlow: MutableStateFlow<Map<BluetoothAddress, AapPodState>>
@@ -148,18 +149,18 @@ class ConversationReactionTest : BaseTest() {
    }

    @Test
-    fun `HOLD keep-alive resets the stale timer`() = runTest(UnconfinedTestDispatcher()) {
+    fun `HOLD keep-alive resets the stale backstop`() = runTest(UnconfinedTestDispatcher()) {
        val job = launchReaction()

        emit(primaryAddress, ConversationAwarenessEvent.START)
-        advanceTimeBy(8_000)
+        advanceTimeBy(staleTimeoutMs * 7 / 10)
        runCurrent()
-        emit(primaryAddress, ConversationAwarenessEvent.HOLD) // resets the timer
-        advanceTimeBy(8_000) // 8s since the HOLD — still within the window
+        emit(primaryAddress, ConversationAwarenessEvent.HOLD) // resets the backstop
+        advanceTimeBy(staleTimeoutMs * 7 / 10) // <1 backstop since the HOLD — still engaged
        runCurrent()
        verify(exactly = 0) { mediaControl.restoreMusicVolume(any()) }

-        advanceTimeBy(5_000) // now >12s since the last frame
+        advanceTimeBy(staleTimeoutMs / 2) // now >1 backstop since the last frame
        runCurrent()
        verify(exactly = 1) { mediaControl.restoreMusicVolume(10) }
        job.cancel()
@@ -272,4 +273,73 @@ class ConversationReactionTest : BaseTest() {
        verify(exactly = 1) { mediaControl.duckMusicVolume(any()) }
        job.cancel()
    }
+
+    @Test
+    fun `PAUSE stays paused through frame silence, resumes only on explicit STOP, then re-engages`() =
+        runTest(UnconfinedTestDispatcher()) {
+            // Regression for the fw …6861 bug: the pod sends an onset, then NO frames for ~20s while
+            // the wearer keeps talking, then a terminal STOP. The old 12s stale timeout resumed media
+            // mid-speech; the backstop must not, and a fresh talk must re-arm.
+            devicesFlow.value = listOf(mockPodDevice(primaryAddress, ConversationAction.PAUSE))
+            val job = launchReaction()
+
+            emit(primaryAddress, ConversationAwarenessEvent.START)
+            emit(primaryAddress, ConversationAwarenessEvent.START) // status 1 then 2
+            coVerify(exactly = 1) { mediaControl.sendPause(false) }
+
+            advanceTimeBy(20_000) // 20s of silence — well under the backstop
+            runCurrent()
+            coVerify(exactly = 0) { mediaControl.sendPlay() } // NOT resumed mid-speech
+
+            emit(primaryAddress, ConversationAwarenessEvent.STOP) // wearer stopped → pod's terminal frame
+            coVerify(exactly = 1) { mediaControl.sendPlay() }
+
+            emit(primaryAddress, ConversationAwarenessEvent.START) // a fresh talk re-arms
+            coVerify(exactly = 2) { mediaControl.sendPause(false) }
+            job.cancel()
+        }
+
+    @Test
+    fun `HOLD frames keep media paused, only terminal STOP resumes`() = runTest(UnconfinedTestDispatcher()) {
+        // fw …6503-style wind-down 1,2,3,0xB,4,8,9: transitional frames must not resume.
+        devicesFlow.value = listOf(mockPodDevice(primaryAddress, ConversationAction.PAUSE))
+        val job = launchReaction()
+
+        emit(primaryAddress, ConversationAwarenessEvent.START)
+        emit(primaryAddress, ConversationAwarenessEvent.HOLD) // 3
+        emit(primaryAddress, ConversationAwarenessEvent.HOLD) // 0x0B
+        emit(primaryAddress, ConversationAwarenessEvent.HOLD) // 4
+        coVerify(exactly = 0) { mediaControl.sendPlay() }
+
+        emit(primaryAddress, ConversationAwarenessEvent.STOP) // 8
+        coVerify(exactly = 1) { mediaControl.sendPlay() }
+        job.cancel()
+    }
+
+    @Test
+    fun `STOP from a non-owner does not disengage the active owner`() = runTest(UnconfinedTestDispatcher()) {
+        devicesFlow.value = listOf(mockPodDevice(primaryAddress, ConversationAction.PAUSE))
+        val job = launchReaction()
+
+        emit(primaryAddress, ConversationAwarenessEvent.START)
+        emit(otherAddress, ConversationAwarenessEvent.STOP) // a different device's STOP
+
+        coVerify(exactly = 0) { mediaControl.sendPlay() }
+        job.cancel()
+    }
+
+    @Test
+    fun `explicit STOP then stale backstop does not double-resume`() = runTest(UnconfinedTestDispatcher()) {
+        devicesFlow.value = listOf(mockPodDevice(primaryAddress, ConversationAction.PAUSE))
+        val job = launchReaction()
+
+        emit(primaryAddress, ConversationAwarenessEvent.START)
+        emit(primaryAddress, ConversationAwarenessEvent.STOP)
+        coVerify(exactly = 1) { mediaControl.sendPlay() }
+
+        advanceTimeBy(staleTimeoutMs + 500) // backstop would fire if STOP hadn't cancelled it
+        runCurrent()
+        coVerify(exactly = 1) { mediaControl.sendPlay() } // still only once
+        job.cancel()
+    }
 }