From 4529b107dda2482a0f2d5532252529c4917d9a99 Mon Sep 17 00:00:00 2001 From: Matthias Urhahn Date: Thu, 7 May 2026 13:48:34 +0200 Subject: [PATCH] fix(reaction): Only auto-resume after ear-removal pause, not stem or sleep Matches Apple's iOS/macOS behavior: stem-press pauses and sleep-detection pauses are explicit user intent, not eligible for auto-resume on next pod-in. sendPause gains a rememberForResume parameter (default false); only PlayPause's auto-pause branch passes true. New sendStop wrapper clears the auto-resume flag for stem-mapped MEDIA_STOP. --- .../eu/darken/capod/common/MediaControl.kt | 42 +++++--- .../monitor/core/aap/StemPressReaction.kt | 2 +- .../reaction/core/playpause/PlayPause.kt | 2 +- .../darken/capod/common/MediaControlTest.kt | 97 ++++++++++++++++--- .../monitor/core/aap/StemPressReactionTest.kt | 4 +- .../core/playpause/PlayPauseLogicTest.kt | 16 +-- 6 files changed, 127 insertions(+), 36 deletions(-) diff --git a/app/src/main/java/eu/darken/capod/common/MediaControl.kt b/app/src/main/java/eu/darken/capod/common/MediaControl.kt index 24a8893a..46310600 100644 --- a/app/src/main/java/eu/darken/capod/common/MediaControl.kt +++ b/app/src/main/java/eu/darken/capod/common/MediaControl.kt @@ -66,15 +66,24 @@ class MediaControl @Inject constructor( /** * Dispatches a MEDIA_PAUSE key event if music is currently playing. * - * Returns `true` when a key event was actually dispatched (and the [wasRecentlyPausedByCap] - * flag was set), `false` when the call was a no-op because nothing was playing. Callers that - * need to distinguish "we actually paused" from "there was nothing to pause" — e.g. the sleep - * reaction, which gates its notification and cooldown on a real pause — should branch on the - * return value rather than checking [isPlaying] themselves to avoid a check-then-act race - * with the audio system. + * @param rememberForResume When `true`, arms [wasRecentlyPausedByCap] so a subsequent + * pod-in transition can auto-resume — set this only from the auto-pause / ear-detection + * flow. When `false` (default), the dispatched pause clears any pending auto-resume — + * this is the path for stem-press play/pause, sleep detection, and anywhere else CAPod + * is relaying an explicit user choice to stop playback. Matches Apple's iOS/macOS + * behavior where only ear-removal auto-pauses are eligible for auto-resume. + * + * Returns `true` when a key event was actually dispatched, `false` when the call was a + * no-op because nothing was playing. A no-op leaves the existing flag state untouched + * (so a sleep-reaction firing while music is already paused doesn't accidentally cancel + * a pending auto-resume from a recent ear-removal pause). Callers that need to + * distinguish "we actually paused" from "there was nothing to pause" — e.g. the sleep + * reaction, which gates its notification and cooldown on a real pause — should branch + * on the return value rather than checking [isPlaying] themselves to avoid a + * check-then-act race with the audio system. */ - suspend fun sendPause(): Boolean { - log(TAG, INFO) { "sendPause()" } + suspend fun sendPause(rememberForResume: Boolean = false): Boolean { + log(TAG, INFO) { "sendPause(rememberForResume=$rememberForResume)" } if (!audioManager.isMusicActive) { log(TAG, INFO) { "Music is not playing, not sending pause" } return false @@ -82,13 +91,24 @@ class MediaControl @Inject constructor( // Set BEFORE the suspending sendKey() call. If we set after, an inactive→active // playback callback that fires during the dispatch (e.g. a fast user resume on the // phone, or another app grabbing audio focus and immediately starting) could clear - // capPaused, and then we'd overwrite it back to true on a stale pause — leaving the - // sticky flag set while music is genuinely playing. - capPaused = true + // capPaused mid-dispatch and we'd then overwrite it back to true on a stale pause. + // This single explicit assignment also covers the contract that an explicit user + // pause cancels a pending auto-resume. + capPaused = rememberForResume sendKey(KeyEvent.KEYCODE_MEDIA_PAUSE) return true } + /** + * Dispatches MEDIA_STOP and clears any pending auto-resume — Stop is an explicit user + * "stay stopped" action, so a later pod-in must not auto-resume from a prior auto-pause. + */ + suspend fun sendStop() { + log(TAG, INFO) { "sendStop()" } + capPaused = false + sendKey(KeyEvent.KEYCODE_MEDIA_STOP) + } + suspend fun sendPlayPause() { log(TAG) { "sendPlayPause()" } if (capPaused) { diff --git a/app/src/main/java/eu/darken/capod/monitor/core/aap/StemPressReaction.kt b/app/src/main/java/eu/darken/capod/monitor/core/aap/StemPressReaction.kt index 468380ce..947d600f 100644 --- a/app/src/main/java/eu/darken/capod/monitor/core/aap/StemPressReaction.kt +++ b/app/src/main/java/eu/darken/capod/monitor/core/aap/StemPressReaction.kt @@ -66,7 +66,7 @@ class StemPressReaction @Inject constructor( is StemAction.PreviousTrack -> mediaControl.sendKey(KeyEvent.KEYCODE_MEDIA_PREVIOUS) is StemAction.VolumeUp -> mediaControl.adjustVolumeUp() is StemAction.VolumeDown -> mediaControl.adjustVolumeDown() - is StemAction.Stop -> mediaControl.sendKey(KeyEvent.KEYCODE_MEDIA_STOP) + is StemAction.Stop -> mediaControl.sendStop() is StemAction.FastForward -> mediaControl.sendKey(KeyEvent.KEYCODE_MEDIA_FAST_FORWARD) is StemAction.Rewind -> mediaControl.sendKey(KeyEvent.KEYCODE_MEDIA_REWIND) is StemAction.MuteToggle -> mediaControl.toggleMuteMusic() diff --git a/app/src/main/java/eu/darken/capod/reaction/core/playpause/PlayPause.kt b/app/src/main/java/eu/darken/capod/reaction/core/playpause/PlayPause.kt index 8334c5e2..4819cef8 100644 --- a/app/src/main/java/eu/darken/capod/reaction/core/playpause/PlayPause.kt +++ b/app/src/main/java/eu/darken/capod/reaction/core/playpause/PlayPause.kt @@ -265,7 +265,7 @@ class PlayPause @Inject constructor( } decision.shouldPause && reactions.autoPause -> { - val pauseSent = mediaControl.sendPause() + val pauseSent = mediaControl.sendPause(rememberForResume = true) log(TAG, INFO) { "autoPause triggered: source=$source, " + "wasWorn=${prevState.bothInEar}, isWorn=${currState.bothInEar}, " + diff --git a/app/src/test/java/eu/darken/capod/common/MediaControlTest.kt b/app/src/test/java/eu/darken/capod/common/MediaControlTest.kt index a36f93f8..3dfe4d41 100644 --- a/app/src/test/java/eu/darken/capod/common/MediaControlTest.kt +++ b/app/src/test/java/eu/darken/capod/common/MediaControlTest.kt @@ -43,10 +43,10 @@ class MediaControlTest : BaseTest() { } @Test - fun `sendPlay ignores stale active state after cap pause`() = runTest { + fun `sendPlay ignores stale active state after auto-pause`() = runTest { every { audioManager.isMusicActive } returns true - mediaControl.sendPause() + mediaControl.sendPause(rememberForResume = true) assertTrue(mediaControl.wasRecentlyPausedByCap) clearMocks(audioManager, answers = false, recordedCalls = true) @@ -57,10 +57,10 @@ class MediaControlTest : BaseTest() { } @Test - fun `sendPlayPause resumes after cap pause even when audio manager still reports active`() = runTest { + fun `sendPlayPause resumes after auto-pause even when audio manager still reports active`() = runTest { every { audioManager.isMusicActive } returns true - mediaControl.sendPause() + mediaControl.sendPause(rememberForResume = true) assertTrue(mediaControl.wasRecentlyPausedByCap) clearMocks(audioManager, answers = false, recordedCalls = true) @@ -71,16 +71,45 @@ class MediaControlTest : BaseTest() { } @Test - fun `sendPause returns true and dispatches when music is active`() = runTest { + fun `default sendPause dispatches but does NOT arm auto-resume`() = runTest { every { audioManager.isMusicActive } returns true val dispatched = mediaControl.sendPause() + assertTrue(dispatched) + // Critical: a user-initiated pause (stem, sleep, etc.) must not arm the auto-resume + // flag. Only `rememberForResume = true` (auto-pause from ear removal) does that. + assertFalse(mediaControl.wasRecentlyPausedByCap) + verify(exactly = 2) { audioManager.dispatchMediaKeyEvent(any()) } + } + + @Test + fun `sendPause with rememberForResume dispatches and arms auto-resume`() = runTest { + every { audioManager.isMusicActive } returns true + + val dispatched = mediaControl.sendPause(rememberForResume = true) + assertTrue(dispatched) assertTrue(mediaControl.wasRecentlyPausedByCap) verify(exactly = 2) { audioManager.dispatchMediaKeyEvent(any()) } } + @Test + fun `default sendPause clears any prior auto-resume flag when actually dispatching`() = runTest { + // Prime: ear-removal auto-pause armed the flag. + every { audioManager.isMusicActive } returns true + mediaControl.sendPause(rememberForResume = true) + assertTrue(mediaControl.wasRecentlyPausedByCap) + + // Music is somehow playing again (manual resume). Then a stem-press or sleep-pause + // fires while music is active — explicit user pause must SUPERSEDE the prior + // auto-pause memory. + every { audioManager.isMusicActive } returns true + mediaControl.sendPause() + + assertFalse(mediaControl.wasRecentlyPausedByCap) + } + @Test fun `sendPause returns false and is a no-op when no music is active`() = runTest { every { audioManager.isMusicActive } returns false @@ -88,17 +117,59 @@ class MediaControlTest : BaseTest() { val dispatched = mediaControl.sendPause() assertFalse(dispatched) - // Critical: the cap-pause flag must NOT be set for a no-op pause, otherwise an - // unrelated sendPlay would treat it as "we just paused, resume from it". assertFalse(mediaControl.wasRecentlyPausedByCap) verify(exactly = 0) { audioManager.dispatchMediaKeyEvent(any()) } } + @Test + fun `no-op sendPause does NOT clear an existing auto-resume flag`() = runTest { + // Prime: auto-pause armed the flag and music is now inactive. + every { audioManager.isMusicActive } returns true + mediaControl.sendPause(rememberForResume = true) + assertTrue(mediaControl.wasRecentlyPausedByCap) + + // Music has gone inactive (the auto-pause took effect). A sleep reaction now fires + // while music is already paused — sendPause is a no-op (returns false) and must NOT + // wipe the pending auto-resume intent from the prior ear-removal pause. + every { audioManager.isMusicActive } returns false + val dispatched = mediaControl.sendPause() + + assertFalse(dispatched) + assertTrue(mediaControl.wasRecentlyPausedByCap) + } + + @Test + fun `sendPause with rememberForResume returns false and does not arm the flag when no music is active`() = runTest { + every { audioManager.isMusicActive } returns false + + val dispatched = mediaControl.sendPause(rememberForResume = true) + + assertFalse(dispatched) + // Critical: arming the flag for a no-op pause would later make sendPlay treat it as + // "we just paused" and force a resume. + assertFalse(mediaControl.wasRecentlyPausedByCap) + verify(exactly = 0) { audioManager.dispatchMediaKeyEvent(any()) } + } + + @Test + fun `sendStop dispatches MEDIA_STOP and clears auto-resume`() = runTest { + // Prime auto-resume. + every { audioManager.isMusicActive } returns true + mediaControl.sendPause(rememberForResume = true) + assertTrue(mediaControl.wasRecentlyPausedByCap) + clearMocks(audioManager, answers = false, recordedCalls = true) + + mediaControl.sendStop() + + assertFalse(mediaControl.wasRecentlyPausedByCap) + verify(exactly = 2) { audioManager.dispatchMediaKeyEvent(any()) } + } + @Test fun `wasRecentlyPausedByCap is sticky and does not expire on its own`() = runTest { every { audioManager.isMusicActive } returns true - mediaControl.sendPause() + mediaControl.sendPause(rememberForResume = true) assertTrue(mediaControl.wasRecentlyPausedByCap) // Wait an arbitrarily long time. With the previous timer-based design this would have @@ -111,11 +182,9 @@ class MediaControlTest : BaseTest() { @Test fun `wasRecentlyPausedByCap clears when music transitions inactive to active from any source`() = runTest { every { audioManager.isMusicActive } returns true + fireCallback() // seed lastKnownMusicActive=true - // First the seed transition active→active so lastKnownMusicActive is true. - fireCallback() - - mediaControl.sendPause() + mediaControl.sendPause(rememberForResume = true) assertTrue(mediaControl.wasRecentlyPausedByCap) // Music goes inactive (CAP's pause took effect). @@ -131,7 +200,7 @@ class MediaControlTest : BaseTest() { } @Test - fun `sendPause sets capPaused before dispatching so a racing inactive-active callback cannot leave a stale true`() = runTest { + fun `auto-pause sets capPaused before dispatching so a racing inactive-active callback cannot leave a stale true`() = runTest { // Repro for a race where the playback config callback fires during sendKey()'s // suspension. If capPaused were set after dispatch, an interleaved inactive→active // callback would clear it, then sendPause's post-dispatch line would put it back to @@ -151,7 +220,7 @@ class MediaControlTest : BaseTest() { fireCallback() } - mediaControl.sendPause() + mediaControl.sendPause(rememberForResume = true) // After the suspended dispatch returns, capPaused should be in a coherent state with // the live callback observations. Music is currently active (per the racing callback) diff --git a/app/src/test/java/eu/darken/capod/monitor/core/aap/StemPressReactionTest.kt b/app/src/test/java/eu/darken/capod/monitor/core/aap/StemPressReactionTest.kt index 8d9ed35b..a1a81328 100644 --- a/app/src/test/java/eu/darken/capod/monitor/core/aap/StemPressReactionTest.kt +++ b/app/src/test/java/eu/darken/capod/monitor/core/aap/StemPressReactionTest.kt @@ -156,7 +156,9 @@ class StemPressReactionTest : BaseTest() { events.emit(addressA to StemPressEvent(StemPressEvent.PressType.LONG, StemPressEvent.Bud.LEFT)) advanceUntilIdle() - coVerify(exactly = 1) { mediaControl.sendKey(KeyEvent.KEYCODE_MEDIA_STOP) } + // Stop now goes through `sendStop()` (which clears auto-resume + dispatches MEDIA_STOP) + // instead of dispatching the raw key — explicit user "stay stopped" intent. + coVerify(exactly = 1) { mediaControl.sendStop() } coVerify(exactly = 1) { mediaControl.sendKey(KeyEvent.KEYCODE_MEDIA_FAST_FORWARD) } coVerify(exactly = 1) { mediaControl.sendKey(KeyEvent.KEYCODE_MEDIA_REWIND) } coVerify(exactly = 1) { mediaControl.toggleMuteMusic() } diff --git a/app/src/test/java/eu/darken/capod/reaction/core/playpause/PlayPauseLogicTest.kt b/app/src/test/java/eu/darken/capod/reaction/core/playpause/PlayPauseLogicTest.kt index a26d419d..661ad4ca 100644 --- a/app/src/test/java/eu/darken/capod/reaction/core/playpause/PlayPauseLogicTest.kt +++ b/app/src/test/java/eu/darken/capod/reaction/core/playpause/PlayPauseLogicTest.kt @@ -1755,7 +1755,7 @@ class PlayPauseLogicTest : BaseTest() { val mediaControl: MediaControl = mockk(relaxed = true) { every { isPlaying } returns true every { wasRecentlyPausedByCap } returns false - coEvery { sendPause() } returns true + coEvery { sendPause(rememberForResume = true) } returns true } val flowPlayPause = PlayPause(deviceMonitor, bluetoothManager, mediaControl) @@ -1777,7 +1777,7 @@ class PlayPauseLogicTest : BaseTest() { deviceFlow.value = listOf(buildDevice(now.plusMillis(3000), leftWorn = false, rightWorn = false)) advanceUntilIdle() - coVerify(exactly = 1) { mediaControl.sendPause() } + coVerify(exactly = 1) { mediaControl.sendPause(rememberForResume = true) } job.cancel() } @@ -1794,7 +1794,7 @@ class PlayPauseLogicTest : BaseTest() { val mediaControl: MediaControl = mockk(relaxed = true) { every { isPlaying } returns true every { wasRecentlyPausedByCap } returns false - coEvery { sendPause() } returns true + coEvery { sendPause(rememberForResume = true) } returns true } val flowPlayPause = PlayPause(deviceMonitor, bluetoothManager, mediaControl) @@ -1823,13 +1823,13 @@ class PlayPauseLogicTest : BaseTest() { deviceFlow.value = listOf(buildDevice(now.plusMillis(5000), leftWorn = false, rightWorn = false)) advanceUntilIdle() - coVerify(exactly = 0) { mediaControl.sendPause() } + coVerify(exactly = 0) { mediaControl.sendPause(rememberForResume = true) } // T6: the new removal sequence reaches three consecutive not-worn samples. deviceFlow.value = listOf(buildDevice(now.plusMillis(6000), leftWorn = false, rightWorn = false)) advanceUntilIdle() - coVerify(exactly = 1) { mediaControl.sendPause() } + coVerify(exactly = 1) { mediaControl.sendPause(rememberForResume = true) } job.cancel() } @@ -1978,7 +1978,7 @@ class PlayPauseLogicTest : BaseTest() { val mediaControl: MediaControl = mockk(relaxed = true) { every { isPlaying } returns true every { wasRecentlyPausedByCap } returns false - coEvery { sendPause() } returns true + coEvery { sendPause(rememberForResume = true) } returns true } val flowPlayPause = PlayPause(deviceMonitor, bluetoothManager, mediaControl) @@ -1998,7 +1998,7 @@ class PlayPauseLogicTest : BaseTest() { deviceFlow.value = listOf(buildIrkMatchedDevice(now.plusMillis(2000), leftWorn = true, rightWorn = true)) advanceUntilIdle() - coVerify(exactly = 0) { mediaControl.sendPause() } + coVerify(exactly = 0) { mediaControl.sendPause(rememberForResume = true) } coVerify(exactly = 0) { mediaControl.sendPlay() } job.cancel() @@ -2120,7 +2120,7 @@ class PlayPauseLogicTest : BaseTest() { ) advanceUntilIdle() - coVerify(exactly = 0) { mediaControl.sendPause() } + coVerify(exactly = 0) { mediaControl.sendPause(rememberForResume = true) } // T2: pod reinserted. Auto-play must NOT fire — the user paused, not CAP, and the // user hasn't opted into cold-wear.