From 84d31fe7f846fc0ee3a962fdd6579d0a980483a6 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Wed, 21 Oct 2026 19:06:50 +0200 Subject: [PATCH] fix(timers): settle timer vibration outside the store's init and resume a ramp The DataStore migration ran its write inside the store's initialisation, so a failed write (a full disk) would have failed every preference read, the alarm path's included. The timer-vibrate key is now nullable instead: unsettled reads as the alarm default, which is what timers did before, and ClockulaApp settles it once at the head of its startup pass, off only for a never-updated package with an empty store. A failed settle leaves the fallback in place. A timer ring restarted mid-session (handed to a second expired timer, back after an alarm held it, a recreated service) now picks its ramp up from the session's elapsed time instead of either ramping again from a whisper or jumping straight to full volume. RingAudioPlayer takes the offset; alarms pass none. The Vibrate row's switch is display-only, so a screen reader stops on the row once. Refs #22 --- .../de/jeanlucmakiola/clockula/ClockulaApp.kt | 28 ++++ .../clockula/data/di/DataModule.kt | 16 --- .../clockula/data/prefs/ClockPrefs.kt | 12 +- .../clockula/data/prefs/SettingsPrefs.kt | 24 +++- .../data/prefs/TimerVibrateMigration.kt | 42 ------ .../clockula/domain/timer/TimerRing.kt | 10 ++ .../clockula/domain/timer/TimerSettings.kt | 8 -- .../clockula/ring/RingAudioPlayer.kt | 39 ++++-- .../clockula/timer/service/TimerService.kt | 11 +- .../ui/settings/TimerDefaultsScreen.kt | 7 +- .../clockula/data/di/DataModuleTest.kt | 1 - .../clockula/data/prefs/ClockPrefsTest.kt | 7 + .../clockula/data/prefs/SettingsPrefsTest.kt | 89 ++++++++++++ .../data/prefs/TimerVibrateMigrationTest.kt | 132 ------------------ .../domain/timer/TimerRingPolicyTest.kt | 13 +- docs/ARCHITECTURE.md | 20 ++- 16 files changed, 223 insertions(+), 236 deletions(-) delete mode 100644 app/src/main/java/de/jeanlucmakiola/clockula/data/prefs/TimerVibrateMigration.kt delete mode 100644 app/src/test/java/de/jeanlucmakiola/clockula/data/prefs/TimerVibrateMigrationTest.kt diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ClockulaApp.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ClockulaApp.kt index 3a6c66e..9953f26 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ClockulaApp.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ClockulaApp.kt @@ -4,6 +4,7 @@ import android.app.Application import dagger.hilt.android.HiltAndroidApp import de.jeanlucmakiola.clockula.alarm.AlarmEngine import de.jeanlucmakiola.clockula.alarm.ring.RingNotifications +import de.jeanlucmakiola.clockula.data.prefs.SettingsPrefs import de.jeanlucmakiola.clockula.stopwatch.StopwatchEngine import de.jeanlucmakiola.clockula.stopwatch.service.StopwatchNotifications import de.jeanlucmakiola.clockula.timer.TimerEngine @@ -12,6 +13,7 @@ import de.jeanlucmakiola.clockula.widget.WidgetSync import de.jeanlucmakiola.floret.crash.CrashConfig import de.jeanlucmakiola.floret.crash.CrashReporter import de.jeanlucmakiola.floret.di.ApplicationScope +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch import javax.inject.Inject @@ -41,6 +43,9 @@ class ClockulaApp : Application() { @Inject lateinit var widgetSync: WidgetSync + @Inject + lateinit var settings: SettingsPrefs + @Inject @ApplicationScope lateinit var scope: CoroutineScope @@ -67,6 +72,7 @@ class ClockulaApp : Application() { // a cold start caused by the fire broadcast must not interleave its // read-modify-write over `alarm_states` with the fire's own. scope.launch { + settleTimerVibrate() engine.onBootCompleted() // After the alarm engine's pass, and idempotent within a boot: the // boot-id gate makes the second repair a no-op, so neither engine @@ -86,4 +92,26 @@ class ClockulaApp : Application() { // for as long as the process lives (M10, docs/PLAN.md §9). scope.launch { widgetSync.run() } } + + /** + * Ahead of the engines' pass, because the boot gate writes the store and + * "nothing written yet" is half of how a fresh install is told apart. Never + * in the way of that pass: a failure here leaves the setting unsettled, + * which reads as the old behaviour (#22). + */ + private suspend fun settleTimerVibrate() { + try { + settings.settleTimerVibrate(freshInstall = isFreshInstall()) + } catch (cancelled: CancellationException) { + throw cancelled + } catch (_: Exception) { + // Unsettled is safe; the next launch tries again. + } + } + + /** Never updated since it was installed. Unknown reads as not fresh: the safe side. */ + private fun isFreshInstall(): Boolean = runCatching { + val info = packageManager.getPackageInfo(packageName, 0) + info.firstInstallTime == info.lastUpdateTime + }.getOrDefault(false) } 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 2fb730b..dcbd3c6 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 @@ -12,7 +12,6 @@ import dagger.Provides import dagger.hilt.InstallIn import dagger.hilt.android.qualifiers.ApplicationContext import dagger.hilt.components.SingletonComponent -import de.jeanlucmakiola.clockula.data.prefs.TimerVibrateMigration import de.jeanlucmakiola.floret.di.IoDispatcher import de.jeanlucmakiola.floret.prefs.PrefStore import kotlinx.coroutines.CoroutineDispatcher @@ -46,18 +45,8 @@ 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) @@ -73,17 +62,12 @@ object DataModule { * theme catches it. Replacing an unreadable file with empty preferences * costs the user their two appearance choices and keeps the app startable; * 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(isFreshInstall)), scope = scope, produceFile = produceFile, ) diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/data/prefs/ClockPrefs.kt b/app/src/main/java/de/jeanlucmakiola/clockula/data/prefs/ClockPrefs.kt index 8587aaf..118a782 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/data/prefs/ClockPrefs.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/data/prefs/ClockPrefs.kt @@ -29,11 +29,15 @@ object ClockPrefs { val timerRingtoneUri: Pref = nullableStringPref("default_timer_ringtone_uri").blankAsAbsent() /** - * Off on a fresh install. An install from before the setting existed has - * it written once by [TimerVibrateMigration], from [vibrate] — which is - * what its timers vibrated by until then. + * Null — absent, or not a boolean — means "never settled", and reads as + * [vibrate]: what every timer vibrated by before the setting existed (#22). + * `SettingsPrefs.settleTimerVibrate` writes it once at startup, off on a + * fresh install. Stored as text because the kit has no nullable boolean. */ - val timerVibrate: Pref = booleanPref("default_timer_vibrate", false) + val timerVibrate: Pref = nullableStringPref("default_timer_vibrate").map( + decode = { it?.toBooleanStrictOrNull() }, + encode = { it?.toString() }, + ) val timerVolumeRampSeconds: Pref = intPref("default_timer_volume_ramp_seconds", 0).clampedTo(ClockLimits.VOLUME_RAMP_SECONDS) diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/data/prefs/SettingsPrefs.kt b/app/src/main/java/de/jeanlucmakiola/clockula/data/prefs/SettingsPrefs.kt index 1d13e6e..977db31 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/data/prefs/SettingsPrefs.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/data/prefs/SettingsPrefs.kt @@ -75,7 +75,8 @@ class SettingsPrefs @Inject constructor(private val store: PrefStore) { partial.copy( alarmRingtoneUri = alarmRingtoneUri, timerRingtoneUri = timerRingtoneUri, - timerVibrate = timerVibrate, + // Unsettled follows the alarm default, which is what it did before #22. + timerVibrate = timerVibrate ?: partial.vibrate, timerVolumeRampSeconds = timerVolumeRampSeconds, homeZoneId = homeZoneId, ) @@ -120,6 +121,27 @@ class SettingsPrefs @Inject constructor(private val store: PrefStore) { suspend fun setTimerVibrate(enabled: Boolean) = store.set(ClockPrefs.timerVibrate, enabled) + /** + * Gives the timers' vibration a value of its own, once (#22). Until then a + * timer vibrated exactly when the alarm default said so, and an install + * from before keeps that: the alarm default is copied, so changing it + * later no longer drags the timers along. Only a fresh install — a + * [freshInstall] package *and* a store nothing has written yet — starts + * with it off. Either signal alone mistakes something for fresh (an Auto + * Backup restore; a store replaced after corruption), and that mistake + * would silently stop someone's timers buzzing. + * + * Best-effort by design: until it lands, and if it never does, the read + * side's fallback keeps the old behaviour, so a failed write costs nothing. + */ + suspend fun settleTimerVibrate(freshInstall: Boolean) { + val untouched = store.data.first().asMap().isEmpty() + val alarmVibrate = store.get(ClockPrefs.vibrate) + store.update(ClockPrefs.timerVibrate) { stored -> + stored ?: if (untouched && freshInstall) false else alarmVibrate + } + } + suspend fun setTimerVolumeRampSeconds(seconds: Int) = store.set(ClockPrefs.timerVolumeRampSeconds, seconds) suspend fun setDismissChallenge(challenge: DismissChallenge) = 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 deleted file mode 100644 index 0c82ba8..0000000 --- a/app/src/main/java/de/jeanlucmakiola/clockula/data/prefs/TimerVibrateMigration.kt +++ /dev/null @@ -1,42 +0,0 @@ -package de.jeanlucmakiola.clockula.data.prefs - -import androidx.datastore.core.DataMigration -import androidx.datastore.preferences.core.Preferences -import androidx.datastore.preferences.core.booleanPreferencesKey -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. Only a fresh install gets - * the setting's own default, off. - * - * 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 class TimerVibrateMigration( - private val isFreshInstall: () -> Boolean, -) : DataMigration { - - private val timerVibrateKey = booleanPreferencesKey(ClockPrefs.timerVibrate.name) - private val alarmVibrateKey = booleanPreferencesKey(ClockPrefs.vibrate.name) - - override suspend fun shouldMigrate(currentData: Preferences): Boolean = timerVibrateKey !in currentData - - override suspend fun migrate(currentData: Preferences): Preferences { - val inherited = if (currentData.asMap().isEmpty() && isFreshInstall()) { - ClockPrefs.timerVibrate.default - } else { - currentData[alarmVibrateKey] ?: ClockPrefs.vibrate.default - } - return currentData.toMutablePreferences().apply { this[ClockPrefs.timerVibrate] = inherited } - } - - override suspend fun cleanUp() = Unit -} diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/domain/timer/TimerRing.kt b/app/src/main/java/de/jeanlucmakiola/clockula/domain/timer/TimerRing.kt index e0c905d..f0402cd 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/domain/timer/TimerRing.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/domain/timer/TimerRing.kt @@ -22,4 +22,14 @@ object TimerRing { /** The floor under every `delay` the service takes, so a rounding error cannot spin it. */ val WAKE_FLOOR: Duration = 100.milliseconds + + /** + * How far into its ramp a ring is, from what is left of its window + * ([AUTO_SILENCE_AFTER] long, opened when the session first sounded). A ring + * restarted mid-session — handed to another expired timer, back after an + * alarm held it, or a recreated service — carries on from there rather than + * ramping again from a whisper. Zero for a session that has not sounded. + */ + fun rampElapsed(silenceIn: Duration?): Duration = + silenceIn?.let { (AUTO_SILENCE_AFTER - it).coerceAtLeast(Duration.ZERO) } ?: Duration.ZERO } 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 43cd22d..3c63b17 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 @@ -21,11 +21,3 @@ fun Timer.resolveSettings(defaults: ClockDefaults): TimerSettings = TimerSetting 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/ring/RingAudioPlayer.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ring/RingAudioPlayer.kt index a631089..a096520 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ring/RingAudioPlayer.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ring/RingAudioPlayer.kt @@ -72,19 +72,26 @@ class RingAudioPlayer(private val context: Context) { * * [onSilenced] is called, at most once, when that chain found no sound at * all; never after [stop]. + * + * [rampFrom] picks the ramp up partway, for a ring that is restarted rather + * than new — a timer ring handed from one timer to another, or back after an + * alarm held it — so the volume carries on rather than dropping back to a + * whisper. Past the ramp's end it starts at full volume. */ fun start( scope: CoroutineScope, ringtoneUri: String?, rampSeconds: Int, onSilenced: () -> Unit, + rampFrom: Duration = Duration.ZERO, ) { stop() val ring = generation val rampFor = rampSeconds.coerceAtLeast(0).seconds + val rampStart = rampFrom.coerceAtLeast(Duration.ZERO) scope.launch { - val audible = startMedia(ring, scope, ringtoneUri, rampFor, onSilenced) || - startPlayer(ring, scope, ringtoneUri, rampFor, onSilenced) + val audible = startMedia(ring, scope, ringtoneUri, rampFor, rampStart, onSilenced) || + startPlayer(ring, scope, ringtoneUri, rampFor, rampStart, onSilenced) if (!audible && ring == generation) onSilenced() } } @@ -108,6 +115,7 @@ class RingAudioPlayer(private val context: Context) { scope: CoroutineScope, ringtoneUri: String?, rampFor: Duration, + rampStart: Duration, onSilenced: () -> Unit, ): Boolean { val ref = MediaSources.decode(ringtoneUri) ?: return false @@ -126,7 +134,7 @@ class RingAudioPlayer(private val context: Context) { false } if (started && ring == generation) { - watchMedia(ring, scope, media, ringtoneUri, rampFor, onSilenced) + watchMedia(ring, scope, media, ringtoneUri, rampFor, rampStart, onSilenced) return true } media.stop() @@ -148,6 +156,7 @@ class RingAudioPlayer(private val context: Context) { scope: CoroutineScope, ringtoneUri: String?, rampFor: Duration, + rampStart: Duration, onSilenced: () -> Unit, from: Int = 0, ): Boolean { @@ -161,7 +170,7 @@ class RingAudioPlayer(private val context: Context) { return false } requestFocus() - val initial = VolumeRamp.levelAt(Duration.ZERO, rampFor) + val initial = VolumeRamp.levelAt(rampStart, rampFor) val started = runCatching { prepared.setVolume(initial, initial) prepared.start() @@ -180,13 +189,13 @@ class RingAudioPlayer(private val context: Context) { release(failed) abandonFocus() scope.launch { - val next = startPlayer(ring, scope, ringtoneUri, Duration.ZERO, onSilenced, index + 1) + val next = startPlayer(ring, scope, ringtoneUri, Duration.ZERO, Duration.ZERO, onSilenced, index + 1) if (!next && ring == generation) onSilenced() } } true } - ramp = launchRamp(scope, rampFor) { level -> runCatching { player?.setVolume(level, level) } } + ramp = launchRamp(scope, rampFor, rampStart) { level -> runCatching { player?.setVolume(level, level) } } return true } return false @@ -205,10 +214,11 @@ class RingAudioPlayer(private val context: Context) { media: MediaAppRing, ringtoneUri: String?, rampFor: Duration, + rampStart: Duration, onSilenced: () -> Unit, ) { - media.setLevel(VolumeRamp.levelAt(Duration.ZERO, rampFor)) - ramp = launchRamp(scope, rampFor, media::setLevel) + media.setLevel(VolumeRamp.levelAt(rampStart, rampFor)) + ramp = launchRamp(scope, rampFor, rampStart, media::setLevel) watchdog = scope.launch { var stall = MediaRingPolicy.Stall() while (isActive && ring == generation) { @@ -221,14 +231,19 @@ class RingAudioPlayer(private val context: Context) { ramp = null media.stop() mediaRing = null - if (!startPlayer(ring, scope, ringtoneUri, Duration.ZERO, onSilenced) && ring == generation) onSilenced() + if (!startPlayer(ring, scope, ringtoneUri, Duration.ZERO, Duration.ZERO, onSilenced) && ring == generation) onSilenced() } } - /** Steps [setLevel] along [VolumeRamp] every [RingAudio.RAMP_TICK] until [rampFor] has passed. */ - private fun launchRamp(scope: CoroutineScope, rampFor: Duration, setLevel: (Float) -> Unit): Job = + /** Steps [setLevel] along [VolumeRamp] every [RingAudio.RAMP_TICK] from [from] until [rampFor] has passed. */ + private fun launchRamp( + scope: CoroutineScope, + rampFor: Duration, + from: Duration, + setLevel: (Float) -> Unit, + ): Job = scope.launch { - var elapsed = Duration.ZERO + var elapsed = from while (isActive && elapsed < rampFor) { delay(RingAudio.RAMP_TICK) elapsed += RingAudio.RAMP_TICK 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 ea103e6..e4226bf 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 @@ -16,8 +16,8 @@ import de.jeanlucmakiola.clockula.data.timers.TimerRepository 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.TimerRing 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 @@ -35,6 +35,7 @@ import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.merge import kotlinx.coroutines.launch +import kotlin.time.Duration import kotlin.time.Instant import javax.inject.Inject @@ -190,7 +191,7 @@ class TimerService : Service() { return } - applyRing(state.ring) + applyRing(state.ring, state.silenceIn) post(notification, sounding = state.ring is TimerRingDecision.Sound) scheduleDelays(state) progressRefresh?.cancel() @@ -223,13 +224,12 @@ class TimerService : Service() { return engine.serviceState(alreadyAlerted) } - private fun applyRing(decision: TimerRingDecision) { + private fun applyRing(decision: TimerRingDecision, silenceIn: Duration?) { when (decision) { is TimerRingDecision.Sound -> { // 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() @@ -242,7 +242,8 @@ class TimerService : Service() { audio.start( scope = scope, ringtoneUri = decision.settings.ringtoneUri, - rampSeconds = decision.settings.rampSecondsFor(handover), + rampSeconds = decision.settings.rampSeconds, + rampFrom = TimerRing.rampElapsed(silenceIn), // 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 9130302..be6aca0 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 @@ -56,10 +56,9 @@ internal fun TimerDefaultsScreen( }, position = Position.Middle, trailing = { - Switch( - checked = defaults.timerVibrate, - onCheckedChange = viewModel::onTimerVibrateChanged, - ) + // Display only: the row is the one control, so a screen reader + // stops on it once rather than on the row and then the switch. + Switch(checked = defaults.timerVibrate, onCheckedChange = null) }, onClick = { viewModel.onTimerVibrateChanged(!defaults.timerVibrate) }, ) 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 df5d4a9..46a4cdc 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,7 +25,6 @@ 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/ClockPrefsTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/data/prefs/ClockPrefsTest.kt index 869cca3..8401734 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/data/prefs/ClockPrefsTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/data/prefs/ClockPrefsTest.kt @@ -37,6 +37,8 @@ class ClockPrefsTest { @Test fun `a fresh install reads the documented defaults`(@TempDir tempDir: Path) = runTest { val prefs = SettingsPrefs(PrefStore(newDataStore(tempDir))) + // As ClockulaApp's startup pass does on a fresh install. + prefs.settleTimerVibrate(freshInstall = true) assertThat(prefs.defaults.first()).isEqualTo(ClockDefaults()) } @@ -51,6 +53,8 @@ class ClockPrefsTest { prefs.setVolumeRampSeconds(0) prefs.setAlarmRingtoneUri("content://a") prefs.setTimerRingtoneUri("content://t") + prefs.setTimerVibrate(true) + prefs.setTimerVolumeRampSeconds(25) prefs.setDismissChallenge(DismissChallenge.HOLD) prefs.setHomeZoneId("Europe/Berlin") @@ -62,6 +66,8 @@ class ClockPrefsTest { volumeRampSeconds = 0, alarmRingtoneUri = "content://a", timerRingtoneUri = "content://t", + timerVibrate = true, + timerVolumeRampSeconds = 25, dismissChallenge = DismissChallenge.HOLD, homeZoneId = "Europe/Berlin", ), @@ -71,6 +77,7 @@ class ClockPrefsTest { @Test fun `the defaults do not re-emit when the same value is written again`(@TempDir tempDir: Path) = runTest { val prefs = SettingsPrefs(PrefStore(newDataStore(tempDir))) + prefs.settleTimerVibrate(freshInstall = true) prefs.defaults.test { assertThat(awaitItem()).isEqualTo(ClockDefaults()) diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/data/prefs/SettingsPrefsTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/data/prefs/SettingsPrefsTest.kt index 1e0a018..bdffca6 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/data/prefs/SettingsPrefsTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/data/prefs/SettingsPrefsTest.kt @@ -169,6 +169,95 @@ class SettingsPrefsTest { assertThat(defaults.timerVolumeRampSeconds).isEqualTo(10) } + @Test + fun `an unsettled timer vibration follows the alarm default, as timers did before`( + @TempDir tempDir: Path, + ) = runTest { + val prefs = SettingsPrefs(PrefStore(newDataStore(tempDir))) + + prefs.setVibrate(false) + assertThat(prefs.currentDefaults().timerVibrate).isFalse() + prefs.setVibrate(true) + assertThat(prefs.currentDefaults().timerVibrate).isTrue() + } + + @Test + fun `a fresh install settles timer vibration off`(@TempDir tempDir: Path) = runTest { + val prefs = SettingsPrefs(PrefStore(newDataStore(tempDir))) + + prefs.settleTimerVibrate(freshInstall = true) + + assertThat(prefs.currentDefaults().timerVibrate).isFalse() + assertThat(prefs.currentDefaults().vibrate).isTrue() + } + + @Test + fun `an older install settles timer vibration from the alarm default`(@TempDir tempDir: Path) = runTest { + val prefs = SettingsPrefs(PrefStore(newDataStore(tempDir))) + prefs.setVibrate(false) + + prefs.settleTimerVibrate(freshInstall = false) + // Settled, so the two now move apart. + prefs.setVibrate(true) + + assertThat(prefs.currentDefaults().timerVibrate).isFalse() + } + + @Test + fun `an older install that never touched alarm vibration keeps vibrating its timers`( + @TempDir tempDir: Path, + ) = runTest { + val prefs = SettingsPrefs(PrefStore(newDataStore(tempDir))) + prefs.setSnoozeMinutes(7) + + prefs.settleTimerVibrate(freshInstall = false) + prefs.setVibrate(false) + + assertThat(prefs.currentDefaults().timerVibrate).isTrue() + } + + @Test + fun `an updated install with an empty store is not taken for fresh`(@TempDir tempDir: Path) = runTest { + // A store replaced after corruption, or never written: empty, but not new. + val prefs = SettingsPrefs(PrefStore(newDataStore(tempDir))) + + prefs.settleTimerVibrate(freshInstall = false) + + assertThat(prefs.currentDefaults().timerVibrate).isTrue() + } + + @Test + fun `a new package with a restored store is not taken for fresh`(@TempDir tempDir: Path) = runTest { + // Auto Backup restored the old device's store before the first launch. + val prefs = SettingsPrefs(PrefStore(newDataStore(tempDir))) + prefs.setSnoozeMinutes(7) + + prefs.settleTimerVibrate(freshInstall = true) + + assertThat(prefs.currentDefaults().timerVibrate).isTrue() + } + + @Test + fun `settling never overwrites a choice`(@TempDir tempDir: Path) = runTest { + val prefs = SettingsPrefs(PrefStore(newDataStore(tempDir))) + prefs.setTimerVibrate(true) + + prefs.settleTimerVibrate(freshInstall = true) + + assertThat(prefs.currentDefaults().timerVibrate).isTrue() + } + + @Test + fun `a garbled timer vibration reads as unsettled`(@TempDir tempDir: Path) = runTest { + val dataStore = newDataStore(tempDir) + val prefs = SettingsPrefs(PrefStore(dataStore)) + dataStore.updateData { stored -> + stored.toMutablePreferences().apply { this[stringPreferencesKey("default_timer_vibrate")] = "maybe" } + } + + assertThat(prefs.currentDefaults().timerVibrate).isTrue() + } + @Test fun `the timer ramp is clamped like the alarm ramp`(@TempDir tempDir: Path) = runTest { val prefs = SettingsPrefs(PrefStore(newDataStore(tempDir))) 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 deleted file mode 100644 index dd55e95..0000000 --- a/app/src/test/java/de/jeanlucmakiola/clockula/data/prefs/TimerVibrateMigrationTest.kt +++ /dev/null @@ -1,132 +0,0 @@ -package de.jeanlucmakiola.clockula.data.prefs - -import androidx.datastore.preferences.core.PreferenceDataStoreFactory -import androidx.datastore.preferences.core.booleanPreferencesKey -import androidx.datastore.preferences.core.edit -import androidx.datastore.preferences.core.stringPreferencesKey -import com.google.common.truth.Truth.assertThat -import de.jeanlucmakiola.clockula.data.di.createDataStore -import de.jeanlucmakiola.floret.prefs.PrefStore -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Job -import kotlinx.coroutines.cancelAndJoin -import kotlinx.coroutines.job -import kotlinx.coroutines.test.TestScope -import kotlinx.coroutines.test.UnconfinedTestDispatcher -import kotlinx.coroutines.test.runTest -import org.junit.jupiter.api.Test -import org.junit.jupiter.api.io.TempDir -import java.io.File -import java.nio.file.Path - -/** - * #22's upgrade path, through the store `DataModule` builds: an install from - * before timers had their own vibration keeps vibrating its timers exactly - * when it did, and a fresh one starts with it off. - */ -class TimerVibrateMigrationTest { - - private fun TestScope.newScope(): CoroutineScope = - CoroutineScope(UnconfinedTestDispatcher(testScheduler) + Job()) - - private fun file(tempDir: Path): File = tempDir.resolve("clockula_prefs.preferences_pb").toFile() - - /** The file as a version before #22 left it: no migration, only the keys it knew. */ - private suspend fun TestScope.writeOldInstall(tempDir: Path, alarmVibrate: Boolean?) { - val scope = newScope() - PreferenceDataStoreFactory.create(scope = scope, produceFile = { file(tempDir) }).edit { prefs -> - prefs[stringPreferencesKey("last_boot_id")] = "boot" - if (alarmVibrate != null) prefs[booleanPreferencesKey("default_vibrate")] = alarmVibrate - } - // DataStore refuses a second store over a file a live one holds. - scope.coroutineContext.job.cancelAndJoin() - } - - /** [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, freshInstall = true) - - assertThat(prefs.currentDefaults().timerVibrate).isFalse() - assertThat(prefs.currentDefaults().vibrate).isTrue() - } - - @Test - fun `a fresh install stays off on the next launch, once the store is no longer empty`( - @TempDir tempDir: Path, - ) = runTest { - val firstLaunch = newScope() - openPrefs(tempDir, freshInstall = true, scope = firstLaunch).setSnoozeMinutes(7) - firstLaunch.coroutineContext.job.cancelAndJoin() - - // Updated since, too: by now the key is there, and nothing asks again. - val prefs = openPrefs(tempDir, freshInstall = false) - - assertThat(prefs.currentDefaults().timerVibrate).isFalse() - } - - @Test - fun `an older install with alarm vibration on keeps vibrating its timers`(@TempDir tempDir: Path) = runTest { - writeOldInstall(tempDir, alarmVibrate = true) - - assertThat(openPrefs(tempDir).currentDefaults().timerVibrate).isTrue() - } - - @Test - fun `an older install that never touched alarm vibration keeps vibrating its timers`( - @TempDir tempDir: Path, - ) = runTest { - writeOldInstall(tempDir, alarmVibrate = null) - - assertThat(openPrefs(tempDir).currentDefaults().timerVibrate).isTrue() - } - - @Test - fun `an older install with alarm vibration off keeps its timers still`(@TempDir tempDir: Path) = runTest { - writeOldInstall(tempDir, alarmVibrate = false) - - 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) - val prefs = openPrefs(tempDir) - - prefs.setVibrate(false) - - assertThat(prefs.currentDefaults().timerVibrate).isTrue() - } - - @Test - fun `a choice the user made is never overwritten`(@TempDir tempDir: Path) = runTest { - val firstLaunch = newScope() - 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 d033b1e..76a579d 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 @@ -103,11 +103,14 @@ class TimerRingPolicyTest { } @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) + fun `a ring restarted mid-session picks its ramp up where the window says it is`() { + assertThat(TimerRing.rampElapsed(silenceIn = null)).isEqualTo(Duration.ZERO) + assertThat(TimerRing.rampElapsed(silenceIn = TimerRing.AUTO_SILENCE_AFTER)).isEqualTo(Duration.ZERO) + assertThat(TimerRing.rampElapsed(silenceIn = TimerRing.AUTO_SILENCE_AFTER - 12.seconds)) + .isEqualTo(12.seconds) + // A window longer than the constant (a clock change) never ramps backwards. + assertThat(TimerRing.rampElapsed(silenceIn = TimerRing.AUTO_SILENCE_AFTER + 5.seconds)) + .isEqualTo(Duration.ZERO) } /** §5.4 #8 */ diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index c2ec184..f87ee1d 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1910,12 +1910,20 @@ Vibration is `ClockDefaults.timerVibrate`, never the alarms' `vibrate`: an 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. 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: +and keeps that. The key is nullable: absent reads as the alarm default, and +`SettingsPrefs.settleTimerVibrate` writes a value of its own once, at the head +of `ClockulaApp`'s startup pass (before the boot gate writes the store). Only a +package never updated since install *and* an empty store count as fresh and +get `false`; anything else (an update, an Auto Backup restore, a store replaced +after corruption) gets the alarm default copied. It is deliberately not a +DataStore migration: a migration's write runs inside the store's +initialisation, and a failed one would fail every read, the alarm path's +included. A failed settle only leaves the fallback in place. + +A ring restarted mid-session (handed to a second expired timer, back after an +alarm held it, a recreated service) picks its ramp up where the session is, +`TimerRing.rampElapsed` from the window's remaining time, rather than ramping +again from a whisper. 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.