Fix the sleep timer (#3112)

Fixes #3110

Author: PaulWoitaschek

core/sleeptimer/impl/src/main/kotlin/voice/core/sleeptimer/ShakeDetector.kt

@@ -1,32 +1,9 @@
 package voice.core.sleeptimer
 
-import android.content.Context
-import android.hardware.SensorManager
-import dev.zacsweers.metro.Inject
-import kotlinx.coroutines.suspendCancellableCoroutine
-import kotlin.coroutines.resume
-import com.squareup.seismic.ShakeDetector as SeismicShakeDetector
-
-@Inject
-class ShakeDetector(private val context: Context) {
+interface ShakeDetector {
 
   /**
    * This function returns once a shake was detected
    */
-  suspend fun detect() {
-    suspendCancellableCoroutine { cont ->
-      val sensorManager = context.getSystemService(Context.SENSOR_SERVICE) as SensorManager?
-        ?: return@suspendCancellableCoroutine
-      val listener = SeismicShakeDetector.Listener {
-        if (!cont.isCompleted) {
-          cont.resume(Unit)
-        }
-      }
-      val shakeDetector = SeismicShakeDetector(listener)
-      shakeDetector.start(sensorManager, SensorManager.SENSOR_DELAY_GAME)
-      cont.invokeOnCancellation {
-        shakeDetector.stop()
-      }
-    }
-  }
+  suspend fun detect()
 }

core/sleeptimer/impl/src/main/kotlin/voice/core/sleeptimer/ShakeDetectorImpl.kt

@@ -0,0 +1,32 @@
+package voice.core.sleeptimer
+
+import android.content.Context
+import android.hardware.SensorManager
+import dev.zacsweers.metro.AppScope
+import dev.zacsweers.metro.ContributesBinding
+import dev.zacsweers.metro.Inject
+import kotlinx.coroutines.suspendCancellableCoroutine
+import kotlin.coroutines.resume
+import com.squareup.seismic.ShakeDetector as SeismicShakeDetector
+
+@ContributesBinding(AppScope::class)
+@Inject
+class ShakeDetectorImpl(private val context: Context) : ShakeDetector {
+
+  override suspend fun detect() {
+    suspendCancellableCoroutine { cont ->
+      val sensorManager = context.getSystemService(Context.SENSOR_SERVICE) as SensorManager?
+        ?: return@suspendCancellableCoroutine
+      val listener = SeismicShakeDetector.Listener {
+        if (!cont.isCompleted) {
+          cont.resume(Unit)
+        }
+      }
+      val shakeDetector = SeismicShakeDetector(listener)
+      shakeDetector.start(sensorManager, SensorManager.SENSOR_DELAY_GAME)
+      cont.invokeOnCancellation {
+        shakeDetector.stop()
+      }
+    }
+  }
+}

core/sleeptimer/impl/src/main/kotlin/voice/core/sleeptimer/SleepTimerImpl.kt

@@ -71,7 +71,7 @@ class SleepTimerImpl(
     playerController.setVolume(1F)
   }
 
-  private suspend fun startCountdown(duration: Duration) {
+  private tailrec suspend fun startCountdown(duration: Duration) {
     Logger.d("startCountdown(duration=$duration)")
     var left = duration
     _state.value = SleepTimerState.Enabled.WithDuration(left)
@@ -95,15 +95,21 @@ class SleepTimerImpl(
 
     playerController.pauseWithRewind(fadeOutDuration)
 
-    val shakeToResetTime = 30.seconds
-    Logger.d("Waiting $shakeToResetTime for shake...")
-    withTimeoutOrNull(shakeToResetTime) {
-      shakeDetector.detect()
+    val shakeDetected = detectShakeWithTimeout()
+    playerController.setVolume(1F)
+    if (shakeDetected) {
       Logger.i("Shake detected, resetting timer")
       playerController.play()
       startCountdown(duration)
     }
-    playerController.setVolume(1F)
+  }
+
+  private suspend fun detectShakeWithTimeout(): Boolean {
+    Logger.d("Waiting $SHAKE_TO_RESET_TIME for shake...")
+    return withTimeoutOrNull(SHAKE_TO_RESET_TIME) {
+      shakeDetector.detect()
+      true
+    } ?: false
   }
 
   private fun updateVolume(
@@ -122,4 +128,8 @@ class SleepTimerImpl(
       Logger.i("Playback resumed.")
     }
   }
+
+  internal companion object {
+    val SHAKE_TO_RESET_TIME = 30.seconds
+  }
 }

core/sleeptimer/impl/src/test/kotlin/voice/core/sleeptimer/impl/SleepTimerImplTest.kt

@@ -6,16 +6,16 @@ import io.kotest.matchers.collections.shouldEndWith
 import io.kotest.matchers.collections.shouldNotBeEmpty
 import io.kotest.matchers.shouldBe
 import io.mockk.Runs
-import io.mockk.coEvery
 import io.mockk.coVerify
 import io.mockk.every
 import io.mockk.just
 import io.mockk.mockk
-import kotlinx.coroutines.delay
-import kotlinx.coroutines.suspendCancellableCoroutine
+import io.mockk.verify
+import kotlinx.coroutines.channels.Channel
 import kotlinx.coroutines.test.StandardTestDispatcher
 import kotlinx.coroutines.test.TestScope
 import kotlinx.coroutines.test.advanceTimeBy
+import kotlinx.coroutines.test.runCurrent
 import kotlinx.coroutines.test.runTest
 import kotlinx.coroutines.yield
 import org.junit.BeforeClass
@@ -33,24 +33,35 @@ import voice.core.sleeptimer.SleepTimerMode
 import voice.core.sleeptimer.SleepTimerState
 import kotlin.time.Duration.Companion.seconds
 
+private class TestShakeDetector : ShakeDetector {
+  private val shakes = Channel<Unit>(capacity = Channel.UNLIMITED)
+  override suspend fun detect() {
+    shakes.receive()
+  }
+
+  fun shake() {
+    shakes.trySend(Unit)
+  }
+}
+
 class SleepTimerImplTest {
 
   private val playStateManager = PlayStateManager().apply {
     playState = PlayStateManager.PlayState.Playing
   }
-  private val shakeDetector = mockk<ShakeDetector> {
-    coEvery { detect() } coAnswers {
-      delay(30.seconds)
-    }
-  }
-
+  private val shakeDetector = TestShakeDetector()
   private val sleepTimerPreferenceStore = MemoryDataStore(SleepTimerPreference.Default)
   private val setVolumeSlots = mutableListOf<Float>()
   private val playerController = mockk<PlayerController> {
     every { setVolume(capture(setVolumeSlots)) } just Runs
     every { pauseWithRewind(any()) } answers {
       playStateManager.playState = PlayStateManager.PlayState.Paused
     }
+    every {
+      play()
+    } answers {
+      playStateManager.playState = PlayStateManager.PlayState.Playing
+    }
   }
 
   private val fadeOutStore = MemoryDataStore(2.seconds)
@@ -78,10 +89,6 @@ class SleepTimerImplTest {
 
   @Test
   fun `enable with fixed duration eventually disables and pauses playback`() = testScope.runTest {
-    coEvery { shakeDetector.detect() } coAnswers {
-      suspendCancellableCoroutine { } // never completes
-    }
-
     sleepTimer.enable(SleepTimerMode.TimedWithDuration(1.seconds))
 
     advanceTimeBy(2.seconds)
@@ -130,6 +137,34 @@ class SleepTimerImplTest {
     setVolumeSlots.shouldEndWith(1f)
   }
 
+  @Test
+  fun shake_does_not_cancel_second_countdown_after_window() = testScope.runTest {
+    // Use a LONG duration so we can observe behavior across the 30s window
+    val longDuration = SleepTimerImpl.SHAKE_TO_RESET_TIME * 2
+
+    sleepTimer.enable(SleepTimerMode.TimedWithDuration(longDuration))
+
+    // 1) Let the first countdown finish and enter the shake window
+    advanceTimeBy(longDuration + 1.seconds)
+    runCurrent()
+    coVerify(exactly = 1) { playerController.pauseWithRewind(any()) }
+    sleepTimer.state.value shouldBe SleepTimerState.Disabled
+
+    // 2) Trigger the shake → a new countdown should start independently of the old timeout
+    shakeDetector.shake()
+    runCurrent()
+    verify(exactly = 1) { playerController.play() }
+    sleepTimer.state.value shouldBe SleepTimerState.Enabled.WithDuration(longDuration)
+
+    // 3) Advance past the original 30s shake window and allow the second countdown to finish
+    advanceTimeBy(SleepTimerImpl.SHAKE_TO_RESET_TIME + longDuration + 2.seconds)
+    runCurrent()
+
+    // The second countdown should complete normally
+    coVerify(exactly = 2) { playerController.pauseWithRewind(any()) }
+    sleepTimer.state.value shouldBe SleepTimerState.Disabled
+  }
+
   companion object {
 
     @BeforeClass