fix(timers): keep a ramp from restarting mid-ring and a migration from guessing

A timer ring that hands over from one expired timer to another now starts at full volume instead of ramping again from near silence.

The timer-vibrate migration only treats an install as fresh when the package was never updated and the store is empty. An updated install whose store was replaced after corruption, or an Auto Backup restore onto a new device, keeps the alarm default as before.

The Vibrate row now says whether it is on, and only when it is off adds that a silent timer still vibrates.

Refs #22

app/src/main/java/de/jeanlucmakiola/clockula/data/di/DataModule.kt

@@ -46,8 +46,18 @@ object DataModule {
     ): DataStore<Preferences> = createDataStore(
         scope = CoroutineScope(io + SupervisorJob()),
         produceFile = { context.preferencesDataStoreFile(PREFS_NAME) },
+        isFreshInstall = { isFreshInstall(context) },
     )
 
+    /**
+     * Never updated since it was installed. Unknown reads as not fresh, which
+     * is the migrations' safe side: it keeps what an install already did.
+     */
+    private fun isFreshInstall(context: Context): Boolean = runCatching {
+        val info = context.packageManager.getPackageInfo(context.packageName, 0)
+        info.firstInstallTime == info.lastUpdateTime
+    }.getOrDefault(false)
+
     @Provides
     @Singleton
     fun providePrefStore(dataStore: DataStore<Preferences>): PrefStore = PrefStore(dataStore)
@@ -65,13 +75,15 @@ object DataModule {
  * the alternative is an app that can only be fixed by clearing its data.
  *
  * Migrations run before the first read, so nothing ever sees a key they fill.
+ * [isFreshInstall] is asked only by them, and only once.
  */
 internal fun createDataStore(
     scope: CoroutineScope,
     produceFile: () -> File,
+    isFreshInstall: () -> Boolean,
 ): DataStore<Preferences> = PreferenceDataStoreFactory.create(
     corruptionHandler = ReplaceFileCorruptionHandler { emptyPreferences() },
-    migrations = listOf(TimerVibrateMigration),
+    migrations = listOf(TimerVibrateMigration(isFreshInstall)),
     scope = scope,
     produceFile = produceFile,
 )

app/src/main/java/de/jeanlucmakiola/clockula/data/prefs/TimerVibrateMigration.kt

@@ -8,15 +8,21 @@ import de.jeanlucmakiola.floret.prefs.set
 /**
  * Gives an install from before #22 a timer-vibrate setting that keeps its
  * timers doing what they did: until then a timer vibrated exactly when the
- * alarm default did, so that is the value written. A fresh install — an empty
- * store, since anything that has run once has at least written its boot id —
- * gets the setting's own default, off.
+ * alarm default did, so that is the value written. Only a fresh install gets
+ * the setting's own default, off.
  *
- * The value is always written, the default included, so this runs once: left
- * absent on a fresh install, the next launch would see a non-empty store and
- * copy the alarm default after all.
+ * Fresh means both: the package was never updated ([isFreshInstall], from the
+ * package manager) **and** the store is empty. The first alone would take an
+ * Auto Backup restore onto a new device for fresh; the second alone would take
+ * an updated install whose store was replaced after corruption, or never
+ * written, for fresh. Either mistake silently stops someone's timers buzzing,
+ * so when in doubt this keeps the old behaviour.
+ *
+ * The value is always written, the default included, so this runs once.
  */
-internal object TimerVibrateMigration : DataMigration<Preferences> {
+internal class TimerVibrateMigration(
+    private val isFreshInstall: () -> Boolean,
+) : DataMigration<Preferences> {
 
     private val timerVibrateKey = booleanPreferencesKey(ClockPrefs.timerVibrate.name)
     private val alarmVibrateKey = booleanPreferencesKey(ClockPrefs.vibrate.name)
@@ -24,7 +30,7 @@ internal object TimerVibrateMigration : DataMigration<Preferences> {
     override suspend fun shouldMigrate(currentData: Preferences): Boolean = timerVibrateKey !in currentData
 
     override suspend fun migrate(currentData: Preferences): Preferences {
-        val inherited = if (currentData.asMap().isEmpty()) {
+        val inherited = if (currentData.asMap().isEmpty() && isFreshInstall()) {
             ClockPrefs.timerVibrate.default
         } else {
             currentData[alarmVibrateKey] ?: ClockPrefs.vibrate.default

app/src/main/java/de/jeanlucmakiola/clockula/domain/timer/TimerSettings.kt

@@ -20,3 +20,12 @@ fun Timer.resolveSettings(defaults: ClockDefaults): TimerSettings = TimerSetting
     vibrate = defaults.timerVibrate,
     rampSeconds = defaults.timerVolumeRampSeconds,
 )
+
+/**
+ * The ramp a ring actually starts with. A [handover] — the sounding timer
+ * changes while the ring goes on, because a second one expired and sorts first
+ * or the first was stopped with another still expired — starts at full volume:
+ * the user has already heard the ring, and starting the ramp over would drop it
+ * to a whisper partway through.
+ */
+fun TimerSettings.rampSecondsFor(handover: Boolean): Int = if (handover) 0 else rampSeconds

app/src/main/java/de/jeanlucmakiola/clockula/timer/service/TimerService.kt

@@ -17,6 +17,7 @@ import de.jeanlucmakiola.clockula.domain.ring.RingFallbackPolicy
 import de.jeanlucmakiola.clockula.domain.timer.TimerNotificationState
 import de.jeanlucmakiola.clockula.domain.timer.TimerMode
 import de.jeanlucmakiola.clockula.domain.timer.TimerRingDecision
+import de.jeanlucmakiola.clockula.domain.timer.rampSecondsFor
 import de.jeanlucmakiola.clockula.ring.RingAudioPlayer
 import de.jeanlucmakiola.clockula.ring.RingVibrator
 import de.jeanlucmakiola.clockula.timer.AlarmRingStatus
@@ -228,6 +229,7 @@ class TimerService : Service() {
                 // An unchanged decision does nothing: restarting the player on
                 // every emission would stutter the ring.
                 if (sounding == decision.subjectId) return
+                val handover = sounding != null
                 releaseRing()
                 sounding = decision.subjectId
                 acquireWakeLock()
@@ -240,7 +242,7 @@ class TimerService : Service() {
                 audio.start(
                     scope = scope,
                     ringtoneUri = decision.settings.ringtoneUri,
-                    rampSeconds = decision.settings.rampSeconds,
+                    rampSeconds = decision.settings.rampSecondsFor(handover),
                     // Silence is not one of the outcomes: with no audio source at
                     // all the timer vibrates even when vibration is switched off.
                     onSilenced = vibrator::start,

app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/TimerDefaultsScreen.kt

@@ -13,6 +13,7 @@ import androidx.compose.ui.res.stringResource
 import androidx.compose.ui.unit.dp
 import de.jeanlucmakiola.clockula.R
 import de.jeanlucmakiola.clockula.ui.alarms.VolumeRampPicker
+import de.jeanlucmakiola.clockula.ui.alarms.vibrateText
 import de.jeanlucmakiola.clockula.ui.alarms.volumeRampText
 import de.jeanlucmakiola.floret.components.CollapsingScaffold
 import de.jeanlucmakiola.floret.components.GroupedRow
@@ -44,11 +45,15 @@ internal fun TimerDefaultsScreen(
             onClick = { viewModel.onOpenRingtonePicker(DefaultRingtoneKind.TIMER) },
         )
         // A switch, not the alarm screen's picker: there is no per-timer
-        // override for it to pick "app default" against. The summary says what
-        // the ring does with Silent and vibration both off — vibrates anyway.
+        // override for it to pick "app default" against. Off, the summary says
+        // what the ring does with Silent and vibration both off: vibrates anyway.
         GroupedRow(
             title = stringResource(R.string.editor_vibrate),
-            summary = stringResource(R.string.settings_timer_vibrate_hint),
+            summary = if (defaults.timerVibrate) {
+                vibrateText(on = true)
+            } else {
+                stringResource(R.string.settings_timer_vibrate_off)
+            },
             position = Position.Middle,
             trailing = {
                 Switch(

app/src/main/res/values/strings.xml

@@ -300,7 +300,7 @@
     <string name="settings_section_timers">Timers</string>
     <string name="settings_timers_subtitle">Sound, vibration and gradual volume</string>
     <string name="settings_timer_defaults_hint">How timers ring. A timer can still pick its own sound in its editor.</string>
-    <string name="settings_timer_vibrate_hint">A timer set to silent vibrates either way</string>
+    <string name="settings_timer_vibrate_off">Off. A timer set to Silent still vibrates</string>
     <string name="settings_section_clock">Clock</string>
     <string name="settings_clock_subtitle">Home time zone</string>
     <string name="settings_clock_hint">World clocks show how far each city is from your home time zone.</string>

app/src/test/java/de/jeanlucmakiola/clockula/data/di/DataModuleTest.kt

@@ -25,6 +25,7 @@ class DataModuleTest {
     private fun TestScope.newStore(file: File) = createDataStore(
         scope = CoroutineScope(UnconfinedTestDispatcher(testScheduler) + Job()),
         produceFile = { file },
+        isFreshInstall = { false },
     )
 
     @Test

app/src/test/java/de/jeanlucmakiola/clockula/data/prefs/TimerVibrateMigrationTest.kt

@@ -42,12 +42,18 @@ class TimerVibrateMigrationTest {
         scope.coroutineContext.job.cancelAndJoin()
     }
 
-    private suspend fun TestScope.openPrefs(tempDir: Path, scope: CoroutineScope = newScope()): SettingsPrefs =
-        SettingsPrefs(PrefStore(createDataStore(scope = scope, produceFile = { file(tempDir) })))
+    /** [freshInstall] is the package manager's half: never updated since installed. */
+    private fun TestScope.openPrefs(
+        tempDir: Path,
+        freshInstall: Boolean = false,
+        scope: CoroutineScope = newScope(),
+    ): SettingsPrefs = SettingsPrefs(
+        PrefStore(createDataStore(scope = scope, produceFile = { file(tempDir) }, isFreshInstall = { freshInstall })),
+    )
 
     @Test
     fun `a fresh install does not vibrate its timers`(@TempDir tempDir: Path) = runTest {
-        val prefs = openPrefs(tempDir)
+        val prefs = openPrefs(tempDir, freshInstall = true)
 
         assertThat(prefs.currentDefaults().timerVibrate).isFalse()
         assertThat(prefs.currentDefaults().vibrate).isTrue()
@@ -58,10 +64,11 @@ class TimerVibrateMigrationTest {
         @TempDir tempDir: Path,
     ) = runTest {
         val firstLaunch = newScope()
-        openPrefs(tempDir, firstLaunch).setSnoozeMinutes(7)
+        openPrefs(tempDir, freshInstall = true, scope = firstLaunch).setSnoozeMinutes(7)
         firstLaunch.coroutineContext.job.cancelAndJoin()
 
-        val prefs = openPrefs(tempDir)
+        // Updated since, too: by now the key is there, and nothing asks again.
+        val prefs = openPrefs(tempDir, freshInstall = false)
 
         assertThat(prefs.currentDefaults().timerVibrate).isFalse()
     }
@@ -89,6 +96,21 @@ class TimerVibrateMigrationTest {
         assertThat(openPrefs(tempDir).currentDefaults().timerVibrate).isFalse()
     }
 
+    @Test
+    fun `an updated install with an empty store keeps vibrating its timers`(@TempDir tempDir: Path) = runTest {
+        // A store replaced after corruption, or never written: empty, but not new.
+        val prefs = openPrefs(tempDir, freshInstall = false)
+
+        assertThat(prefs.currentDefaults().timerVibrate).isTrue()
+    }
+
+    @Test
+    fun `a new install restored from Auto Backup keeps what the old device did`(@TempDir tempDir: Path) = runTest {
+        writeOldInstall(tempDir, alarmVibrate = true)
+
+        assertThat(openPrefs(tempDir, freshInstall = true).currentDefaults().timerVibrate).isTrue()
+    }
+
     @Test
     fun `once migrated, the two settings move independently`(@TempDir tempDir: Path) = runTest {
         writeOldInstall(tempDir, alarmVibrate = true)
@@ -102,7 +124,7 @@ class TimerVibrateMigrationTest {
     @Test
     fun `a choice the user made is never overwritten`(@TempDir tempDir: Path) = runTest {
         val firstLaunch = newScope()
-        openPrefs(tempDir, firstLaunch).setTimerVibrate(true)
+        openPrefs(tempDir, freshInstall = true, scope = firstLaunch).setTimerVibrate(true)
         firstLaunch.coroutineContext.job.cancelAndJoin()
 
         assertThat(openPrefs(tempDir).currentDefaults().timerVibrate).isTrue()

app/src/test/java/de/jeanlucmakiola/clockula/domain/timer/TimerRingPolicyTest.kt

@@ -102,6 +102,14 @@ class TimerRingPolicyTest {
         assertThat(ramped.sound().settings.rampSeconds).isEqualTo(20)
     }
 
+    @Test
+    fun `a new ring ramps, a handover between timers mid-ring does not`() {
+        val settings = decide(defaults = defaults.copy(timerVolumeRampSeconds = 20)).sound().settings
+
+        assertThat(settings.rampSecondsFor(handover = false)).isEqualTo(20)
+        assertThat(settings.rampSecondsFor(handover = true)).isEqualTo(0)
+    }
+
     /** §5.4 #8 */
     @Test
     fun `a session already sounding is not restarted`() {

docs/ARCHITECTURE.md

@@ -1911,9 +1911,11 @@ alarm that should buzz the bed and a kitchen timer that should only beep are
 different questions. A fresh install has it **off**. An install from before
 the setting existed had its timers vibrate exactly when the alarm default did,
 so `TimerVibrateMigration` (a DataStore migration, run before the first read)
-writes that value once rather than silently changing what its timers do; it
-tells a fresh install by an empty store, since anything that has run has
-written its boot id. Silent with vibration off is still not silence:
+writes that value once rather than silently changing what its timers do. Only
+a package never updated since install *and* an empty store count as fresh;
+anything else (an update, an Auto Backup restore, a store replaced after
+corruption) keeps the old behaviour. A ring that hands over from one expired
+timer to another mid-ring does not start the ramp again. Silent with vibration off is still not silence:
 `AudioSourcePolicy` gives Silent no sources, and the player's `onSilenced`
 starts the vibrator regardless of the setting.