diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/data/di/DataModule.kt b/app/src/main/java/de/jeanlucmakiola/clockula/data/di/DataModule.kt index 707addc..2fb730b 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/data/di/DataModule.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/data/di/DataModule.kt @@ -46,8 +46,18 @@ object DataModule { ): DataStore = 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): 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 = PreferenceDataStoreFactory.create( corruptionHandler = ReplaceFileCorruptionHandler { emptyPreferences() }, - migrations = listOf(TimerVibrateMigration), + migrations = listOf(TimerVibrateMigration(isFreshInstall)), scope = scope, produceFile = produceFile, ) diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/data/prefs/TimerVibrateMigration.kt b/app/src/main/java/de/jeanlucmakiola/clockula/data/prefs/TimerVibrateMigration.kt index b31291c..0c82ba8 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/data/prefs/TimerVibrateMigration.kt +++ b/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 { +internal class TimerVibrateMigration( + private val isFreshInstall: () -> Boolean, +) : DataMigration { private val timerVibrateKey = booleanPreferencesKey(ClockPrefs.timerVibrate.name) private val alarmVibrateKey = booleanPreferencesKey(ClockPrefs.vibrate.name) @@ -24,7 +30,7 @@ internal object TimerVibrateMigration : DataMigration { 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 diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/domain/timer/TimerSettings.kt b/app/src/main/java/de/jeanlucmakiola/clockula/domain/timer/TimerSettings.kt index 91833b2..43cd22d 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/domain/timer/TimerSettings.kt +++ b/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 diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/timer/service/TimerService.kt b/app/src/main/java/de/jeanlucmakiola/clockula/timer/service/TimerService.kt index 5010fbf..ea103e6 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/timer/service/TimerService.kt +++ b/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, diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/TimerDefaultsScreen.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/TimerDefaultsScreen.kt index b3f25d9..9130302 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/TimerDefaultsScreen.kt +++ b/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( diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index c9117ae..f1c70a3 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -300,7 +300,7 @@ Timers Sound, vibration and gradual volume How timers ring. A timer can still pick its own sound in its editor. - A timer set to silent vibrates either way + Off. A timer set to Silent still vibrates Clock Home time zone World clocks show how far each city is from your home time zone. diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/data/di/DataModuleTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/data/di/DataModuleTest.kt index 46a4cdc..df5d4a9 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/data/di/DataModuleTest.kt +++ b/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 diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/data/prefs/TimerVibrateMigrationTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/data/prefs/TimerVibrateMigrationTest.kt index ba2832e..dd55e95 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/data/prefs/TimerVibrateMigrationTest.kt +++ b/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() diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/domain/timer/TimerRingPolicyTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/domain/timer/TimerRingPolicyTest.kt index 43ae2ab..d033b1e 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/domain/timer/TimerRingPolicyTest.kt +++ b/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`() { diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 0366fa3..c2ec184 100644 --- a/docs/ARCHITECTURE.md +++ b/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.