From 5ee777b196aa2a67a715022a9b875091c567ec68 Mon Sep 17 00:00:00 2001 From: darken Date: Sun, 31 May 2026 05:38:06 +0200 Subject: [PATCH] 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. --- .../core/apple/aap/protocol/AapSetting.kt | 8 +- .../protocol/ConversationAwarenessEvent.kt | 19 +++-- .../core/conversation/ConversationReaction.kt | 44 ++++++---- .../apple/aap/engine/AapSessionEngineTest.kt | 9 +- .../conversation/ConversationReactionTest.kt | 84 +++++++++++++++++-- 5 files changed, 126 insertions(+), 38 deletions(-) diff --git a/app/src/main/java/eu/darken/capod/pods/core/apple/aap/protocol/AapSetting.kt b/app/src/main/java/eu/darken/capod/pods/core/apple/aap/protocol/AapSetting.kt index c9296a6f..87bebb3d 100644 --- a/app/src/main/java/eu/darken/capod/pods/core/apple/aap/protocol/AapSetting.kt +++ b/app/src/main/java/eu/darken/capod/pods/core/apple/aap/protocol/AapSetting.kt @@ -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, diff --git a/app/src/main/java/eu/darken/capod/pods/core/apple/aap/protocol/ConversationAwarenessEvent.kt b/app/src/main/java/eu/darken/capod/pods/core/apple/aap/protocol/ConversationAwarenessEvent.kt index d5844a2b..3565bb99 100644 --- a/app/src/main/java/eu/darken/capod/pods/core/apple/aap/protocol/ConversationAwarenessEvent.kt +++ b/app/src/main/java/eu/darken/capod/pods/core/apple/aap/protocol/ConversationAwarenessEvent.kt @@ -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 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 4e60e9a9..6b6be184 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 @@ -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 } } diff --git a/app/src/test/java/eu/darken/capod/pods/core/apple/aap/engine/AapSessionEngineTest.kt b/app/src/test/java/eu/darken/capod/pods/core/apple/aap/engine/AapSessionEngineTest.kt index d015f6ba..279be8ac 100644 --- a/app/src/test/java/eu/darken/capod/pods/core/apple/aap/engine/AapSessionEngineTest.kt +++ b/app/src/test/java/eu/darken/capod/pods/core/apple/aap/engine/AapSessionEngineTest.kt @@ -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 } } 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 7bb8d0dd..6239a4d1 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 @@ -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> private lateinit var statesFlow: MutableStateFlow> @@ -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() + } }