diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt index ea5ce1d..f04f7ca 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt @@ -21,6 +21,7 @@ import de.jeanlucmakiola.clockula.domain.resolveSettings import de.jeanlucmakiola.clockula.domain.time.WallClock import de.jeanlucmakiola.clockula.domain.time.ZoneProvider import de.jeanlucmakiola.clockula.system.RebootRepair +import kotlinx.coroutines.NonCancellable import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.distinctUntilChanged @@ -29,6 +30,8 @@ import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.map import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock +import kotlinx.coroutines.withContext +import kotlin.time.Duration import kotlin.time.Duration.Companion.minutes import kotlin.time.Instant import javax.inject.Inject @@ -84,8 +87,11 @@ class AlarmEngine @Inject constructor( // A backstop armed for a shorter window than the one now chosen ("Silence // after" was raised mid-ring): the ring still has time, so the backstop - // moves rather than ending it early. - if (rearmIfInsideWindowLocked(alarmId, since)) return@withLock + // moves rather than ending it early. A negative age is a clock moved back + // behind the ring's start; the backstop was then armed a window from that + // moved clock, so the ring has had its window and closes. + val age = wallClock.now() - since + if (age >= Duration.ZERO && rearmIfInsideWindowLocked(alarmId, since)) return@withLock // Missed, not snoozed: the ring stops, the cycle closes, the next // occurrence is scheduled. There is no missed-alarm notification in v1. @@ -93,15 +99,25 @@ class AlarmEngine @Inject constructor( } /** - * "Silence after" changed. A ring in progress is measured against the new + * Writes the alarms' "Silence after" — the engine is its only writer outside + * a backup restore — and measures a ring in progress against the new * window at once: one already past it closes as missed, exactly as the * backstop would close it, and one still inside it has its backstop moved * to the new end. Measured from when the ring started, so changing the * setting back and forth cannot buy a ring a fresh window. */ - suspend fun onSilenceAfterChanged(): Unit = lock.withLock { - val state = states.ringing() ?: return@withLock - val since = state.ringingSince ?: return@withLock + suspend fun setAlarmSilenceAfter(minutes: Int): Unit = withContext(NonCancellable) { + // Not cancellable: a write that lands without the re-measure would leave + // a ring in progress on the old window. + lock.withLock { + settings.setAlarmSilenceAfterMinutes(minutes) + remeasureRingLocked() + } + } + + private suspend fun remeasureRingLocked() { + val state = states.ringing() ?: return + val since = state.ringingSince ?: return if (!rearmIfInsideWindowLocked(state.alarmId, since)) closeCycleLocked(state.alarmId, state) } @@ -155,7 +171,15 @@ class AlarmEngine @Inject constructor( rescheduleLocked() } - suspend fun onSystemTimeOrZoneChanged(): ScheduledFire? = reschedule() + /** + * The backstop is a wall-clock registration, so a clock change moves it. A + * ring in progress is re-measured first: a clock moved forward past its + * window closes it, one moved back re-arms it a window from now at most. + */ + suspend fun onSystemTimeOrZoneChanged(): ScheduledFire? = lock.withLock { + remeasureRingLocked() + rescheduleLocked() + } suspend fun onPackageReplaced(): ScheduledFire? = reschedule() @@ -369,6 +393,9 @@ class AlarmEngine @Inject constructor( */ private suspend fun persist(alarm: Alarm, before: AlarmRingState, resolution: AlarmResolution) { if (resolution.state != before) states.save(resolution.state) + // A snooze the resolver dropped (it never fired, or the alarm was + // switched off) takes its "Snoozed until" notification with it. + if (before.snoozedUntil != null && resolution.state.snoozedUntil == null) notifier.clearSnoozed(alarm.id) // The resolver closed a ring: its window ran out before the backstop // fired ("Silence after" was shortened mid-ring, or the clock jumped), // or the alarm was switched off while ringing. Storage now says it is diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmRing.kt b/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmRing.kt index 6d11d99..a062c0e 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmRing.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmRing.kt @@ -1,5 +1,6 @@ package de.jeanlucmakiola.clockula.domain.alarm +import de.jeanlucmakiola.clockula.domain.ring.SilenceAfter import kotlin.time.Duration import kotlin.time.Duration.Companion.minutes @@ -23,7 +24,7 @@ object AlarmRing { * window or this, whichever is longer, so a short window never makes a * promised snooze easier to lose. */ - val SNOOZE_GRACE: Duration = 10.minutes + val SNOOZE_GRACE: Duration = SilenceAfter.DEFAULT_MINUTES.minutes /** How late a snooze may come due and still ring, given the ring window. */ fun snoozeGrace(silenceAfter: Duration): Duration = maxOf(silenceAfter, SNOOZE_GRACE) diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/OverridePickers.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/OverridePickers.kt index 6046147..4ba9867 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/OverridePickers.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/OverridePickers.kt @@ -5,7 +5,6 @@ import androidx.compose.ui.res.pluralStringResource import androidx.compose.ui.res.stringResource import de.jeanlucmakiola.clockula.R import de.jeanlucmakiola.clockula.domain.DismissChallenge -import de.jeanlucmakiola.clockula.domain.ring.SilenceAfter import de.jeanlucmakiola.floret.components.OptionPicker /** @@ -68,37 +67,6 @@ internal fun challengeText(challenge: DismissChallenge): String = stringResource }, ) -/** [SilenceAfter.NEVER] is "Never" in words, never "0 minutes". */ -@Composable -internal fun silenceAfterText(minutes: Int): String = if (minutes == SilenceAfter.NEVER) { - stringResource(R.string.silence_after_never) -} else { - pluralStringResource(R.plurals.silence_after_minutes, minutes, minutes) -} - -/** - * "Silence after" is app-wide only (#17): no alarm or timer overrides it, so - * this picker has no "App default" rung. A stored value no choice matches (a - * backup can carry any minute up to the hard stop) is offered too, in its - * place, so the picker always shows what is selected and can put it back. - */ -@Composable -internal fun SilenceAfterPicker( - selected: Int, - onSelect: (Int) -> Unit, - onDismiss: () -> Unit, -) { - val finite = (SilenceAfter.CHOICES + selected).filter { it != SilenceAfter.NEVER }.distinct().sorted() - OptionPicker( - title = stringResource(R.string.settings_silence_after), - options = finite + SilenceAfter.NEVER, - selected = selected, - label = { silenceAfterText(it) }, - onSelect = onSelect, - onDismiss = onDismiss, - ) -} - @Composable internal fun VibratePicker( stored: Boolean?, diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/AlarmDefaultsScreen.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/AlarmDefaultsScreen.kt index 4942a8e..0a56420 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/AlarmDefaultsScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/AlarmDefaultsScreen.kt @@ -12,13 +12,11 @@ import androidx.compose.ui.res.stringResource import androidx.compose.ui.unit.dp import de.jeanlucmakiola.clockula.R import de.jeanlucmakiola.clockula.ui.alarms.DismissChallengePicker -import de.jeanlucmakiola.clockula.ui.alarms.SilenceAfterPicker import de.jeanlucmakiola.clockula.ui.alarms.SnoozeLengthPicker import de.jeanlucmakiola.clockula.ui.alarms.SnoozeLimitPicker import de.jeanlucmakiola.clockula.ui.alarms.VibratePicker import de.jeanlucmakiola.clockula.ui.alarms.VolumeRampPicker import de.jeanlucmakiola.clockula.ui.alarms.challengeText -import de.jeanlucmakiola.clockula.ui.alarms.silenceAfterText import de.jeanlucmakiola.clockula.ui.alarms.snoozeLimitText import de.jeanlucmakiola.clockula.ui.alarms.snoozeMinutesText import de.jeanlucmakiola.clockula.ui.alarms.vibrateText diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/SettingsViewModel.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/SettingsViewModel.kt index 213ac9c..f8697fa 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/SettingsViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/SettingsViewModel.kt @@ -146,8 +146,7 @@ class SettingsViewModel @Inject internal constructor( /** The engine re-measures a ring in progress against the new window at once. */ fun onAlarmSilenceAfterChanged(minutes: Int) = viewModelScope.launch { - settings.setAlarmSilenceAfterMinutes(minutes) - alarmEngine.onSilenceAfterChanged() + alarmEngine.setAlarmSilenceAfter(minutes) } fun onTimerSilenceAfterChanged(minutes: Int) = diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/SilenceAfterPicker.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/SilenceAfterPicker.kt new file mode 100644 index 0000000..e926f70 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/settings/SilenceAfterPicker.kt @@ -0,0 +1,39 @@ +package de.jeanlucmakiola.clockula.ui.settings + +import androidx.compose.runtime.Composable +import androidx.compose.ui.res.pluralStringResource +import androidx.compose.ui.res.stringResource +import de.jeanlucmakiola.clockula.R +import de.jeanlucmakiola.clockula.domain.ring.SilenceAfter +import de.jeanlucmakiola.floret.components.OptionPicker + +/** [SilenceAfter.NEVER] is "Never" in words, never "0 minutes". */ +@Composable +internal fun silenceAfterText(minutes: Int): String = if (minutes == SilenceAfter.NEVER) { + stringResource(R.string.silence_after_never) +} else { + pluralStringResource(R.plurals.silence_after_minutes, minutes, minutes) +} + +/** + * "Silence after" is app-wide only (#17): no alarm or timer overrides it, so + * this picker has no "App default" rung. A stored value no choice matches (a + * backup can carry any minute up to the hard stop) is offered too, in its + * place, so the picker always shows what is selected and can put it back. + */ +@Composable +internal fun SilenceAfterPicker( + selected: Int, + onSelect: (Int) -> Unit, + onDismiss: () -> Unit, +) { + val finite = (SilenceAfter.CHOICES + selected).filter { it != SilenceAfter.NEVER }.distinct().sorted() + OptionPicker( + title = stringResource(R.string.settings_silence_after), + options = finite + SilenceAfter.NEVER, + selected = selected, + label = { silenceAfterText(it) }, + onSelect = onSelect, + onDismiss = onDismiss, + ) +} 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 90ffbb2..a1396ed 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 @@ -11,8 +11,6 @@ import androidx.compose.ui.Modifier import androidx.compose.ui.res.stringResource import androidx.compose.ui.unit.dp import de.jeanlucmakiola.clockula.R -import de.jeanlucmakiola.clockula.ui.alarms.SilenceAfterPicker -import de.jeanlucmakiola.clockula.ui.alarms.silenceAfterText import de.jeanlucmakiola.floret.components.CollapsingScaffold import de.jeanlucmakiola.floret.components.GroupedRow import de.jeanlucmakiola.floret.components.Position diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineTest.kt index b8540c4..b469608 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineTest.kt @@ -50,6 +50,7 @@ import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.Test import org.junit.jupiter.api.io.TempDir import java.nio.file.Path +import kotlin.time.Duration.Companion.hours import kotlin.time.Duration.Companion.minutes import kotlin.time.Duration.Companion.seconds import kotlin.time.Instant @@ -937,8 +938,7 @@ class AlarmEngineTest { engine.onFire(alarmId = 1L, scheduledFor = sevenToday) wallClock.instant = sevenToday + 3.minutes - settings.setAlarmSilenceAfterMinutes(SilenceAfter.NEVER) - engine.onSilenceAfterChanged() + engine.setAlarmSilenceAfter(SilenceAfter.NEVER) assertThat(ring.ringing.value).isEqualTo(1L) assertThat(scheduler.autoSilence).isEqualTo(Registration(sevenToday + SilenceAfter.HARD_STOP, 1L)) @@ -953,8 +953,7 @@ class AlarmEngineTest { engine.onFire(alarmId = 1L, scheduledFor = sevenToday) wallClock.instant = sevenToday + 5.minutes - settings.setAlarmSilenceAfterMinutes(1) - engine.onSilenceAfterChanged() + engine.setAlarmSilenceAfter(1) assertThat(ring.ringing.value).isNull() assertThat(states.state(1L).ringingSince).isNull() @@ -969,8 +968,7 @@ class AlarmEngineTest { val settings = settings(dir) val engine = engineOver(settings) - settings.setAlarmSilenceAfterMinutes(1) - engine.onSilenceAfterChanged() + engine.setAlarmSilenceAfter(1) assertThat(ring.events).isEmpty() assertThat(scheduler.autoSilence).isNull() @@ -1030,12 +1028,70 @@ class AlarmEngineTest { engine.onFire(alarmId = 1L, scheduledFor = sevenToday) wallClock.instant = sevenToday - 30.minutes - settings.setAlarmSilenceAfterMinutes(5) - engine.onSilenceAfterChanged() + engine.setAlarmSilenceAfter(5) assertThat(scheduler.autoSilence).isEqualTo(Registration(sevenToday - 30.minutes + 5.minutes, 1L)) } + @Test + fun `a backstop reached with the clock still behind the ring's start closes it`(@TempDir dir: Path) = + runTest { + givenAlarm(repeatDays = RepeatDays.EVERY_DAY) + wallClock.instant = sevenToday + val engine = engine(dir) + engine.onFire(alarmId = 1L, scheduledFor = sevenToday) + wallClock.instant = sevenToday - 20.minutes + + engine.onAutoSilence(1L) + + assertThat(ring.ringing.value).isNull() + assertThat(states.state(1L).ringingSince).isNull() + } + + @Test + fun `a clock moved back mid-ring re-arms the backstop a window from now`(@TempDir dir: Path) = runTest { + givenAlarm(repeatDays = RepeatDays.EVERY_DAY) + wallClock.instant = sevenToday + val engine = engine(dir) + engine.onFire(alarmId = 1L, scheduledFor = sevenToday) + wallClock.instant = sevenToday - 1.hours + + engine.onSystemTimeOrZoneChanged() + + assertThat(ring.ringing.value).isEqualTo(1L) + assertThat(scheduler.autoSilence) + .isEqualTo(Registration(sevenToday - 1.hours + ClockDefaults().alarmSilenceAfter, 1L)) + } + + @Test + fun `a clock moved forward past the window closes the ring`(@TempDir dir: Path) = runTest { + givenAlarm(repeatDays = RepeatDays.EVERY_DAY) + wallClock.instant = sevenToday + val engine = engine(dir) + engine.onFire(alarmId = 1L, scheduledFor = sevenToday) + wallClock.instant = sevenToday + 1.hours + + engine.onSystemTimeOrZoneChanged() + + assertThat(ring.ringing.value).isNull() + assertThat(states.state(1L).ringingSince).isNull() + assertThat(scheduler.autoSilence).isNull() + } + + @Test + fun `a snooze the resolver drops takes its notification with it`(@TempDir dir: Path) = runTest { + givenAlarm(repeatDays = RepeatDays.EVERY_DAY) + states.save( + ringState(snoozedUntil = sevenToday + 9.minutes, snoozeCount = 1, handledOccurrence = sevenToday), + ) + wallClock.instant = sevenToday + 2.hours + val engine = engine(dir) + + engine.reschedule() + + assertThat(notifier.cleared).contains(1L) + } + // --- upcoming() --- @Test diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 4344b71..9beeda0 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1179,13 +1179,17 @@ go through `SilenceAfter.sanitise` on read, on write and from a backup: over 60 is 60, so no value can make a ring outlast the hard stop, and a negative one is the default rather than the clamp's floor, which would be "Never". -Changing the setting mid-ring applies at once. The settings screen calls -`AlarmEngine.onSilenceAfterChanged()`, which measures the ring in progress -against the new window from when it *started*: past it, the cycle closes as -missed through `closeCycleLocked`; inside it, the backstop moves to the new -end. As a second line, `onAutoSilence` re-arms rather than closes a ring that -is still inside the current window, so a backstop armed for a shorter one can -never end a ring early. +Changing the setting mid-ring applies at once. The settings screen writes it +through `AlarmEngine.setAlarmSilenceAfter()`, which stores it and, under the +same lock and uncancellably, measures the ring in progress against the new +window from when it *started*: past it, the cycle closes as missed through +`closeCycleLocked`; inside it, the backstop moves to the new end. As a second +line, `onAutoSilence` re-arms rather than closes a ring that is still inside +the current window, so a backstop armed for a shorter one can never end a ring +early. A clock change re-measures the ring the same way before rescheduling: +the backstop is a wall-clock registration, so a clock moved back re-arms it at +most one window from the moved clock (and the backstop then closes the ring, +its age still negative), and one moved forward past the window closes it. Whenever the resolver closes a ring (a clock jumped forward, a ringing alarm switched off, a reboot outlasting the window), `persist` stops the ring service