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 9acaedc..fc09bc8 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt @@ -29,6 +29,7 @@ import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.map import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock +import kotlin.time.Duration import kotlin.time.Duration.Companion.minutes import kotlin.time.Instant import javax.inject.Inject @@ -80,13 +81,42 @@ class AlarmEngine @Inject constructor( suspend fun onAutoSilence(alarmId: Long): Unit = lock.withLock { val state = states.state(alarmId) - if (state.ringingSince == null) return@withLock + val since = state.ringingSince ?: return@withLock + + // A backstop armed for a shorter window than the one now chosen ("Silence + // after" was raised mid-ring, and the re-arm below has not run yet): the + // ring still has time, so the backstop moves rather than firing early. A + // negative age is a clock moved backwards, and closes as it always did. + val age = wallClock.now() - since + val silenceAfter = settings.currentDefaults().alarmSilenceAfter + if (age >= Duration.ZERO && age < silenceAfter) { + scheduler.scheduleAutoSilence(since + silenceAfter, alarmId) + return@withLock + } // Missed, not snoozed: the ring stops, the cycle closes, the next // occurrence is scheduled. There is no missed-alarm notification in v1. closeCycleLocked(alarmId, state) } + /** + * "Silence after" changed. A ring in progress is measured 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 + val silenceAfter = settings.currentDefaults().alarmSilenceAfter + if (wallClock.now() - since >= silenceAfter) { + closeCycleLocked(state.alarmId, state) + } else { + scheduler.scheduleAutoSilence(since + silenceAfter, state.alarmId) + } + } + /** [minutesOverride] is a one-off, already clamped; it is never persisted anywhere (M9 D15). */ suspend fun snooze(alarmId: Long, minutesOverride: Int? = null): SnoozeOutcome = lock.withLock { snoozeLocked(alarmId, minutesOverride) } @@ -247,7 +277,7 @@ class AlarmEngine @Inject constructor( // A snooze is kept for as long as the resolver keeps it pending, so one // delivered late (after a reboot, or by Doze) still rings. Rejecting it // here would only re-register the same past instant and fire again. - val lateBy = if (isSnoozeFire) silenceAfter else AlarmRing.FIRE_GRACE + val lateBy = if (isSnoozeFire) AlarmRing.snoozeGrace(silenceAfter) else AlarmRing.FIRE_GRACE // Deleted, switched off, or a PendingIntent so stale the moment has gone: // re-resolve the slot and stay quiet. if (alarm == null || !alarm.enabled || now - scheduledFor > lateBy) { diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/AlarmRingService.kt b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/AlarmRingService.kt index 11d71a2..1b18bd8 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/AlarmRingService.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/AlarmRingService.kt @@ -26,7 +26,6 @@ import kotlinx.coroutines.Job import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.cancel import kotlinx.coroutines.launch -import kotlin.time.Duration.Companion.minutes import javax.inject.Inject /** @@ -200,11 +199,8 @@ class AlarmRingService : Service() { private const val ACTION_REPOST = AlarmIntents.ACTION_DISMISS + ".SERVICE_REPOST" private const val WAKE_LOCK_TAG = "clockula:alarm-ring" - /** - * A backstop on the backstop: past the longest ring window any "Silence - * after" choice allows, so a ring is never cut short by its own wake lock. - */ - private val WAKE_LOCK_TIMEOUT_MILLIS = (SilenceAfter.HARD_STOP + 5.minutes).inWholeMilliseconds + /** A backstop on the backstop, shared with the other ring service. */ + private val WAKE_LOCK_TIMEOUT_MILLIS = SilenceAfter.WAKE_LOCK_TIMEOUT.inWholeMilliseconds fun start(context: Context) { context.startForegroundService(Intent(context, AlarmRingService::class.java)) 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 a14e002..080a367 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 @@ -34,16 +34,17 @@ object ClockPrefs { /** * How long a ringing alarm, and a finished timer, keep going before they - * stop on their own (#17): minutes, or [SilenceAfter.NEVER]. An absent key - * reads as v1's fixed ten minutes, so an upgrade changes nothing. + * stop on their own (#17): minutes, or [SilenceAfter.NEVER], sanitised both + * ways through [SilenceAfter.sanitise]. An absent key reads as v1's fixed + * ten minutes, so an upgrade changes nothing. */ val alarmSilenceAfterMinutes: Pref = intPref("default_alarm_silence_after_minutes", SilenceAfter.DEFAULT_MINUTES) - .clampedTo(ClockLimits.SILENCE_AFTER_MINUTES) + .map(decode = SilenceAfter::sanitise, encode = SilenceAfter::sanitise) val timerSilenceAfterMinutes: Pref = intPref("default_timer_silence_after_minutes", SilenceAfter.DEFAULT_MINUTES) - .clampedTo(ClockLimits.SILENCE_AFTER_MINUTES) + .map(decode = SilenceAfter::sanitise, encode = SilenceAfter::sanitise) /** Normalised through [Zones] both ways, so a zone the device dropped reads as absent. */ val homeZoneId: Pref = nullableStringPref("home_zone_id") diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/domain/ClockDefaults.kt b/app/src/main/java/de/jeanlucmakiola/clockula/domain/ClockDefaults.kt index 20b2dd2..8f37066 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/domain/ClockDefaults.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/domain/ClockDefaults.kt @@ -39,5 +39,4 @@ object ClockLimits { val SNOOZE_MINUTES: IntRange = 1..60 val SNOOZE_LIMIT: IntRange = 0..10 val VOLUME_RAMP_SECONDS: IntRange = 0..60 - val SILENCE_AFTER_MINUTES: IntRange = SilenceAfter.MINUTES } diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmResolver.kt b/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmResolver.kt index 7109ba4..c188d16 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmResolver.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmResolver.kt @@ -72,8 +72,11 @@ object AlarmResolver { // 3. Snooze. A snooze promised before a reboot is kept, even if it came // due while the device was off; one older than the ring window is gone. + // Never sooner than SNOOZE_GRACE, so a short "Silence after" cannot make + // a snooze easier to lose than it was in v1. val snoozedUntil = working.snoozedUntil - val snoozePending = snoozedUntil != null && now - snoozedUntil < autoSilenceAfter + val snoozeGrace = AlarmRing.snoozeGrace(autoSilenceAfter) + val snoozePending = snoozedUntil != null && now - snoozedUntil < snoozeGrace if (snoozedUntil != null && !snoozePending) { working = working.copy(snoozedUntil = null, snoozeCount = 0) } @@ -93,6 +96,21 @@ object AlarmResolver { } } + // A cycle that has just closed as missed (a ring past its window, or a + // snooze that never fired) closes a one-shot alarm for good, exactly as + // the engine's own close does. Falling through would hand it the same + // time tomorrow. + val closedAsMissed = ringingSince != null || snoozedUntil != null + if (closedAsMissed && !alarm.repeatDays.isRepeating) { + return AlarmResolution( + alarmId = alarm.id, + nextFire = null, + source = FireSource.NONE, + state = working.copy(skippedOccurrence = null), + disableAlarm = true, + ) + } + // 4. The natural occurrence, with the skip watermark on top. if (!alarm.skipNextOccurrence) { return AlarmResolution( 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 061b57c..6d11d99 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 @@ -16,4 +16,15 @@ object AlarmRing { * boot or a `TIME_SET` nudge still rings; one missed by hours does not. */ val FIRE_GRACE: Duration = 2.minutes + + /** + * The least time a late snooze is still kept: v1's fixed ring window. A + * snooze delayed by a reboot or by Doze is kept for the "Silence after" + * window or this, whichever is longer, so a short window never makes a + * promised snooze easier to lose. + */ + val SNOOZE_GRACE: Duration = 10.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/domain/ring/SilenceAfter.kt b/app/src/main/java/de/jeanlucmakiola/clockula/domain/ring/SilenceAfter.kt index 5312037..2982b15 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/domain/ring/SilenceAfter.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/domain/ring/SilenceAfter.kt @@ -18,27 +18,45 @@ object SilenceAfter { /** The stored value for "Never". Zero, because no window can be zero minutes long. */ const val NEVER: Int = 0 - /** - * Everything a stored value is clamped into, on read, on write and from a - * backup. Any value in it means something: [NEVER], or that many minutes. - */ - val MINUTES: IntRange = NEVER..60 - - /** - * The longest a ring ever lasts, and what "Never" means in practice. The - * settings hints say "an hour" in words; change them with it. - */ - val HARD_STOP: Duration = MINUTES.last.minutes - /** The default, for alarms and timers alike: AOSP DeskClock's, and v1's fixed value. */ const val DEFAULT_MINUTES: Int = 10 + /** + * The longest a ring ever lasts, in minutes, and what "Never" means in + * practice. The settings hints say "an hour" in words; change them with it. + */ + const val HARD_STOP_MINUTES: Int = 60 + + val HARD_STOP: Duration = HARD_STOP_MINUTES.minutes + + /** + * How long the ring services hold their wake locks: a backstop on the + * backstop, past the longest window there is, so a ring is never cut + * short by its own wake lock. + */ + val WAKE_LOCK_TIMEOUT: Duration = HARD_STOP + 5.minutes + + /** Every value [sanitise] lets through: [NEVER], or that many minutes up to the hard stop. */ + val MINUTES: IntRange = NEVER..HARD_STOP_MINUTES + /** The choices the pickers offer, in order: Google Clock's, so nothing here surprises. */ val CHOICES: List = listOf(1, 5, 10, 15, 20, 25, NEVER) - /** The ring window [minutes] stands for, clamped first so the engines never see an unofferable value. */ + /** + * What a stored value is read and written as, from a picker, a hand-edited + * file or a backup. Too long is the hard stop, which is what it would ring + * for anyway. Negative is nothing anyone chose, so it is the default rather + * than the clamp's floor, which would be "Never". + */ + fun sanitise(minutes: Int): Int = when { + minutes < NEVER -> DEFAULT_MINUTES + minutes > HARD_STOP_MINUTES -> HARD_STOP_MINUTES + else -> minutes + } + + /** The ring window [minutes] stands for, sanitised first so the engines never see an unofferable value. */ fun window(minutes: Int): Duration { - val clamped = minutes.coerceIn(MINUTES) - return if (clamped == NEVER) HARD_STOP else clamped.minutes + val sane = sanitise(minutes) + return if (sane == NEVER) HARD_STOP else sane.minutes } } diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/domain/timer/TimerRingPolicy.kt b/app/src/main/java/de/jeanlucmakiola/clockula/domain/timer/TimerRingPolicy.kt index 478747d..f9bce4f 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/domain/timer/TimerRingPolicy.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/domain/timer/TimerRingPolicy.kt @@ -1,6 +1,7 @@ package de.jeanlucmakiola.clockula.domain.timer import de.jeanlucmakiola.clockula.domain.ClockDefaults +import de.jeanlucmakiola.clockula.domain.ring.SilenceAfter import kotlin.time.Instant sealed interface TimerRingDecision { @@ -18,6 +19,11 @@ sealed interface TimerRingDecision { val settings: TimerSettings, /** When the window closes: the session anchor + [ClockDefaults.timerSilenceAfter]. */ val silenceAt: Instant, + /** + * The session anchor: when its audio opened. The session's identity + * (D10), which [silenceAt] is not, since "Silence after" can move it. + */ + val openedAt: Instant, ) : TimerRingDecision } @@ -53,7 +59,10 @@ object TimerRingPolicy { if (soundingSince != null && now - soundingSince >= window) { return TimerRingOutcome( TimerRingDecision.AutoSilenced(subject.timer.id), - soundingSince = soundingSince, + // Stays run out whatever "Silence after" becomes: the anchor is + // moved back past the longest window there is, so raising the + // setting later cannot start a silenced timer ringing again. + soundingSince = minOf(soundingSince, now - SilenceAfter.HARD_STOP), ) } @@ -75,6 +84,7 @@ object TimerRingPolicy { subjectId = subject.timer.id, settings = subject.timer.resolveSettings(defaults), silenceAt = anchor + window, + openedAt = anchor, ), soundingSince = anchor, ) 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 0775b6a..6c666d1 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 @@ -35,7 +35,6 @@ import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.merge import kotlinx.coroutines.launch -import kotlin.time.Duration.Companion.minutes import kotlin.time.Instant import javax.inject.Inject @@ -97,11 +96,11 @@ class TimerService : Service() { private var alreadyAlerted: Boolean = false /** - * The auto-silence window the heads-up above was spent inside. A window is - * one ring session (D10), so a *new* session is a new event and gets its - * own one heads-up — even while an earlier timer stays EXPIRED. + * The anchor of the ring session the heads-up above was spent inside. A + * *new* session is a new event and gets its own one heads-up (D10) — even + * while an earlier timer stays EXPIRED. */ - private var alertedWindow: Instant? = null + private var alertedSession: Instant? = null /** * `startForeground` is for going foreground, not for posting: every engine @@ -206,8 +205,10 @@ class TimerService : Service() { } /** - * A ring session's window is its identity (D10), so when the window moves the - * flag that spends the session's one heads-up moves with it. The state is + * A ring session's anchor is its identity (D10), so when a new session opens + * the flag that spends the session's one heads-up moves with it. The anchor + * rather than the window's end, which a "Silence after" edit can move + * mid-session without making it a new one. The state is * asked for again, because it was computed from the answer that has just * changed — a second pass rather than a notification that says the right * thing one trigger too late. @@ -216,9 +217,9 @@ class TimerService : Service() { // Only a sounding decision carries a window; a suppressed or silenced one // must not clear the record, or an alarm ringing across a session would // buy it a second heads-up (D9). - val window = (state.ring as? TimerRingDecision.Sound)?.silenceAt ?: return state - val reopened = alreadyAlerted && window != alertedWindow - alertedWindow = window + val session = (state.ring as? TimerRingDecision.Sound)?.openedAt ?: return state + val reopened = alreadyAlerted && session != alertedSession + alertedSession = session if (!reopened) return state alreadyAlerted = false return engine.serviceState(alreadyAlerted) @@ -365,11 +366,8 @@ class TimerService : Service() { companion object { private const val WAKE_LOCK_TAG = "clockula:timer-ring" - /** - * A backstop on the backstop: past the longest ring window any "Silence - * after" choice allows, so a ring is never cut short by its own wake lock. - */ - private val WAKE_LOCK_TIMEOUT_MILLIS = (SilenceAfter.HARD_STOP + 5.minutes).inWholeMilliseconds + /** A backstop on the backstop, shared with the other ring service. */ + private val WAKE_LOCK_TIMEOUT_MILLIS = SilenceAfter.WAKE_LOCK_TIMEOUT.inWholeMilliseconds fun start(context: Context) { context.startForegroundService(Intent(context, TimerService::class.java)) diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModel.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModel.kt index d17e9fe..b0f608c 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsViewModel.kt @@ -86,11 +86,12 @@ class AlarmsViewModel @Inject internal constructor( } /** - * The ring service is a single global slot the resolver cannot reach, and - * its auto-silence backstop disarms itself once `ringingSince` is null — so - * switching off a *ringing* alarm has to go through the engine first, or it - * sounds until the wake lock expires. Conditional, because dismissing a - * merely snoozed alarm would drop a snooze it should keep (M5 D11). + * Switching off a *ringing* alarm goes through the engine first, so the ring + * closes as a dismissal: the snooze notification cleared and the cycle + * closed under the engine's lock. (Since #17 the engine would also stop the + * sound when the next resolution sees the alarm disabled, but that is a + * backstop, not the path.) Conditional, because dismissing a merely + * snoozed alarm would drop a snooze it should keep (M5 D11). */ private suspend fun stopRingIfRinging(alarmId: Long) { if (engine.ringSession()?.alarm?.id == alarmId) engine.dismiss(alarmId) 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 d93d58c..6046147 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 @@ -78,24 +78,26 @@ internal fun silenceAfterText(minutes: Int): String = if (minutes == SilenceAfte /** * "Silence after" is app-wide only (#17): no alarm or timer overrides it, so - * this picker never offers "App default". It shares the overrides' shape so - * the settings screens' pickers all look and behave alike. + * 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, -) = OverridePicker( - title = stringResource(R.string.settings_silence_after), - options = SilenceAfter.CHOICES, - stored = selected, - default = selected, - text = { silenceAfterText(it) }, - onSelect = { it?.let(onSelect) }, - onDismiss = onDismiss, - includeAppDefault = false, -) +) { + 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( 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 224d8b2..213ac9c 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 @@ -3,6 +3,7 @@ package de.jeanlucmakiola.clockula.ui.settings import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import dagger.hilt.android.lifecycle.HiltViewModel +import de.jeanlucmakiola.clockula.alarm.AlarmEngine import de.jeanlucmakiola.clockula.alarm.DeviceStateReader import de.jeanlucmakiola.clockula.data.appicon.LauncherIcon import de.jeanlucmakiola.clockula.data.appicon.LauncherIconManager @@ -48,6 +49,7 @@ import javax.inject.Inject @HiltViewModel class SettingsViewModel @Inject internal constructor( private val settings: SettingsPrefs, + private val alarmEngine: AlarmEngine, private val ringtones: RingtoneCatalog, private val preview: RingtonePreviewer, private val worldClocks: WorldClockSource, @@ -142,8 +144,11 @@ class SettingsViewModel @Inject internal constructor( fun onDismissChallengeChanged(value: DismissChallenge) = viewModelScope.launch { settings.setDismissChallenge(value) } - fun onAlarmSilenceAfterChanged(minutes: Int) = - viewModelScope.launch { settings.setAlarmSilenceAfterMinutes(minutes) } + /** 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() + } fun onTimerSilenceAfterChanged(minutes: Int) = viewModelScope.launch { settings.setTimerSilenceAfterMinutes(minutes) } 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 bb87cb9..50f13cd 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineTest.kt @@ -911,6 +911,85 @@ class AlarmEngineTest { assertThat(entry.source).isEqualTo(FireSource.RESUMED_RING) } + @Test + fun `a backstop armed for a shorter window moves rather than firing early`(@TempDir dir: Path) = runTest { + givenAlarm(repeatDays = RepeatDays.EVERY_DAY) + wallClock.instant = sevenToday + val settings = settings(dir) + val engine = engineOver(settings) + engine.onFire(alarmId = 1L, scheduledFor = sevenToday) + settings.setAlarmSilenceAfterMinutes(25) + wallClock.instant = sevenToday + 10.minutes + + engine.onAutoSilence(1L) + + assertThat(ring.ringing.value).isEqualTo(1L) + assertThat(states.state(1L).ringingSince).isEqualTo(sevenToday) + assertThat(scheduler.autoSilence).isEqualTo(Registration(sevenToday + 25.minutes, 1L)) + } + + @Test + fun `raising silence-after mid-ring moves the backstop at once`(@TempDir dir: Path) = runTest { + givenAlarm(repeatDays = RepeatDays.EVERY_DAY) + wallClock.instant = sevenToday + val settings = settings(dir) + val engine = engineOver(settings) + engine.onFire(alarmId = 1L, scheduledFor = sevenToday) + wallClock.instant = sevenToday + 3.minutes + + settings.setAlarmSilenceAfterMinutes(SilenceAfter.NEVER) + engine.onSilenceAfterChanged() + + assertThat(ring.ringing.value).isEqualTo(1L) + assertThat(scheduler.autoSilence).isEqualTo(Registration(sevenToday + SilenceAfter.HARD_STOP, 1L)) + } + + @Test + fun `lowering silence-after below a ring's age closes it as missed at once`(@TempDir dir: Path) = runTest { + givenAlarm() + wallClock.instant = sevenToday + val settings = settings(dir) + val engine = engineOver(settings) + engine.onFire(alarmId = 1L, scheduledFor = sevenToday) + wallClock.instant = sevenToday + 5.minutes + + settings.setAlarmSilenceAfterMinutes(1) + engine.onSilenceAfterChanged() + + assertThat(ring.ringing.value).isNull() + assertThat(states.state(1L).ringingSince).isNull() + assertThat(scheduler.autoSilence).isNull() + // A one-shot alarm's cycle has closed, so it is off, not armed for tomorrow. + assertThat(alarms.find(1L)?.enabled).isFalse() + } + + @Test + fun `changing silence-after with nothing ringing touches nothing`(@TempDir dir: Path) = runTest { + givenAlarm() + val settings = settings(dir) + val engine = engineOver(settings) + + settings.setAlarmSilenceAfterMinutes(1) + engine.onSilenceAfterChanged() + + assertThat(ring.events).isEmpty() + assertThat(scheduler.autoSilence).isNull() + } + + @Test + fun `a one-shot ring missed across a reboot is disabled, not re-armed for tomorrow`(@TempDir dir: Path) = + runTest { + givenAlarm() + wallClock.instant = sevenToday + 40.minutes + givenRingingAt(alarmId = 1L, occurrence = sevenToday, since = sevenToday) + val engine = engine(dir) + + engine.onBootCompleted() + + assertThat(alarms.find(1L)?.enabled).isFalse() + assertThat(scheduler.next).isNull() + } + // --- upcoming() --- @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 bdfe100..11817ca 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 @@ -162,7 +162,7 @@ class ClockPrefsTest { prefs.setAlarmSilenceAfterMinutes(999) prefs.setTimerSilenceAfterMinutes(-3) assertThat(prefs.defaults.first().alarmSilenceAfterMinutes).isEqualTo(60) - assertThat(prefs.defaults.first().timerSilenceAfterMinutes).isEqualTo(SilenceAfter.NEVER) + assertThat(prefs.defaults.first().timerSilenceAfterMinutes).isEqualTo(SilenceAfter.DEFAULT_MINUTES) dataStore.updateData { stored -> stored.toMutablePreferences() diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmResolverTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmResolverTest.kt index e963c99..fb67163 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmResolverTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/AlarmResolverTest.kt @@ -409,4 +409,39 @@ class AlarmResolverTest { assertThat(kept.nextFire to kept.source).isEqualTo(snoozedUntil to FireSource.SNOOZE) assertThat(dropped.state.snoozedUntil).isNull() } + + @Test + fun `a one-shot ring closed as missed disables the alarm rather than ringing tomorrow`() { + val alarm = alarmAt(hour = 7) + val state = ringState(ringingSince = morning - 2.minutes, handledOccurrence = morning - 2.minutes) + + val resolution = resolve(alarm, state, silenceAfter = SilenceAfter.window(1)) + + assertThat(resolution.disableAlarm).isTrue() + assertThat(resolution.nextFire).isNull() + assertThat(resolution.state.ringingSince).isNull() + } + + @Test + fun `a one-shot snooze that never fired disables the alarm rather than ringing tomorrow`() { + val alarm = alarmAt(hour = 7) + val state = ringState(snoozedUntil = morning - 11.minutes, snoozeCount = 1) + + val resolution = resolve(alarm, state) + + assertThat(resolution.disableAlarm).isTrue() + assertThat(resolution.nextFire).isNull() + assertThat(resolution.state.snoozedUntil).isNull() + } + + @Test + fun `a short window never makes a late snooze easier to lose than v1's ten minutes`() { + val alarm = alarmAt(hour = 7, repeatDays = RepeatDays.EVERY_DAY) + val snoozedUntil = morning - 8.minutes + val state = ringState(snoozedUntil = snoozedUntil, snoozeCount = 1) + + val resolution = resolve(alarm, state, silenceAfter = SilenceAfter.window(1)) + + assertThat(resolution.nextFire to resolution.source).isEqualTo(snoozedUntil to FireSource.SNOOZE) + } } diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/domain/ring/SilenceAfterTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/domain/ring/SilenceAfterTest.kt index e977bbb..a484e83 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/domain/ring/SilenceAfterTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/domain/ring/SilenceAfterTest.kt @@ -27,13 +27,18 @@ class SilenceAfterTest { fun `Never is the hard stop, an hour`() { assertThat(SilenceAfter.window(SilenceAfter.NEVER)).isEqualTo(SilenceAfter.HARD_STOP) assertThat(SilenceAfter.HARD_STOP).isEqualTo(1.hours) + assertThat(SilenceAfter.WAKE_LOCK_TIMEOUT).isGreaterThan(SilenceAfter.HARD_STOP) } @Test fun `no value reaches the engines as a window longer than the hard stop`() { assertThat(SilenceAfter.window(24 * 60)).isEqualTo(SilenceAfter.HARD_STOP) - // Negative is clamped to Never rather than read as an instant silence. - assertThat(SilenceAfter.window(-5)).isEqualTo(SilenceAfter.HARD_STOP) + } + + @Test + fun `a negative value is the default, not Never`() { + assertThat(SilenceAfter.sanitise(-1)).isEqualTo(SilenceAfter.DEFAULT_MINUTES) + assertThat(SilenceAfter.window(-5)).isEqualTo(10.minutes) } @Test 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 70f4c8f..5019802 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 @@ -156,6 +156,7 @@ class TimerRingPolicyTest { subjectId = 7L, settings = released.sound().settings, silenceAt = now + ClockDefaults().timerSilenceAfter, + openedAt = now, ), soundingSince = now, ), @@ -179,8 +180,10 @@ class TimerRingPolicyTest { val outcome = decide(soundingSince = opened) - assertThat(outcome) - .isEqualTo(TimerRingOutcome(TimerRingDecision.AutoSilenced(7L), soundingSince = opened)) + // The anchor moves back past the longest window, so the session stays run out (#17). + assertThat(outcome).isEqualTo( + TimerRingOutcome(TimerRingDecision.AutoSilenced(7L), soundingSince = now - SilenceAfter.HARD_STOP), + ) } /** §5.4 #15 */ @@ -196,12 +199,13 @@ class TimerRingPolicyTest { /** §5.4 #16 */ @Test fun `an auto-silenced session does not restart a minute later`() { - val opened = now - ClockDefaults().timerSilenceAfter + val silenced = decide(soundingSince = now - ClockDefaults().timerSilenceAfter) - val outcome = decide(soundingSince = opened, at = now + 1.minutes) + val outcome = decide(soundingSince = silenced.soundingSince, at = now + 1.minutes) - assertThat(outcome) - .isEqualTo(TimerRingOutcome(TimerRingDecision.AutoSilenced(7L), soundingSince = opened)) + assertThat(outcome).isEqualTo( + TimerRingOutcome(TimerRingDecision.AutoSilenced(7L), soundingSince = silenced.soundingSince), + ) } /** §5.4 #17 */ @@ -274,4 +278,28 @@ class TimerRingPolicyTest { assertThat(sounding.sound().silenceAt).isEqualTo(now - 59.minutes + SilenceAfter.HARD_STOP) assertThat(silenced.decision).isEqualTo(TimerRingDecision.AutoSilenced(7L)) } + + @Test + fun `raising silence-after does not restart a session that already went quiet`() { + val short = defaults.copy(timerSilenceAfterMinutes = 1) + val silenced = decide(defaults = short, soundingSince = now - 1.minutes) + + val raised = decide( + defaults = defaults.copy(timerSilenceAfterMinutes = SilenceAfter.NEVER), + soundingSince = silenced.soundingSince, + at = now + 4.minutes, + ) + + assertThat(raised.decision).isEqualTo(TimerRingDecision.AutoSilenced(7L)) + } + + @Test + fun `changing silence-after mid-session moves the window but not the session`() { + val opened = now - 2.minutes + + val sound = decide(defaults = defaults.copy(timerSilenceAfterMinutes = 25), soundingSince = opened).sound() + + assertThat(sound.openedAt).isEqualTo(opened) + assertThat(sound.silenceAt).isEqualTo(opened + 25.minutes) + } } diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/ui/settings/SettingsViewModelTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/ui/settings/SettingsViewModelTest.kt index 1af202e..f35a2b0 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/ui/settings/SettingsViewModelTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/ui/settings/SettingsViewModelTest.kt @@ -1,13 +1,30 @@ package de.jeanlucmakiola.clockula.ui.settings import com.google.common.truth.Truth.assertThat +import de.jeanlucmakiola.clockula.alarm.AlarmEngine +import de.jeanlucmakiola.clockula.data.alarms.AlarmMapper +import de.jeanlucmakiola.clockula.data.alarms.AlarmRepositoryImpl +import de.jeanlucmakiola.clockula.data.alarms.AlarmStateRepositoryImpl import de.jeanlucmakiola.clockula.data.appicon.LauncherIcon import de.jeanlucmakiola.clockula.domain.DismissChallenge +import de.jeanlucmakiola.clockula.domain.RepeatDays +import de.jeanlucmakiola.clockula.domain.ring.SilenceAfter +import de.jeanlucmakiola.clockula.testing.BERLIN +import de.jeanlucmakiola.clockula.testing.FakeAlarmDao +import de.jeanlucmakiola.clockula.testing.FakeAlarmNotifier +import de.jeanlucmakiola.clockula.testing.FakeAlarmScheduler +import de.jeanlucmakiola.clockula.testing.FakeAlarmStateDao import de.jeanlucmakiola.clockula.testing.FakeDeviceStateReader import de.jeanlucmakiola.clockula.testing.FakeLauncherIconManager import de.jeanlucmakiola.clockula.testing.FakeRingtoneCatalog +import de.jeanlucmakiola.clockula.testing.FakeRingCoordinator import de.jeanlucmakiola.clockula.testing.FakeRingtonePreviewer +import de.jeanlucmakiola.clockula.testing.FakeWallClock +import de.jeanlucmakiola.clockula.testing.FakeZoneProvider import de.jeanlucmakiola.clockula.testing.MainDispatcherExtension +import de.jeanlucmakiola.clockula.testing.alarmAt +import de.jeanlucmakiola.clockula.testing.RecordingRebootRepair +import de.jeanlucmakiola.clockula.testing.instantAt import de.jeanlucmakiola.clockula.testing.worldClockHarness import de.jeanlucmakiola.floret.prefs.ThemeMode import kotlinx.coroutines.Dispatchers @@ -21,6 +38,7 @@ import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.RegisterExtension import org.junit.jupiter.api.io.TempDir import java.nio.file.Path +import kotlin.time.Duration.Companion.minutes /** * The real [SettingsViewModel] over the real [de.jeanlucmakiola.clockula.data.prefs.SettingsPrefs] @@ -35,13 +53,33 @@ class SettingsViewModelTest { @RegisterExtension val main = MainDispatcherExtension() + // The alarm engine "Silence after" re-measures a ring through, over fakes. + private val alarmDao = FakeAlarmDao() + private val stateDao = FakeAlarmStateDao(alarms = alarmDao) + private val wallClock = FakeWallClock(instantAt(BERLIN, "2026-06-10T07:00")) + private val alarmScheduler = FakeAlarmScheduler() + private val ring = FakeRingCoordinator() + private lateinit var alarmEngine: AlarmEngine + private fun TestScope.newViewModel( tempDir: Path, launcherIcons: FakeLauncherIconManager = FakeLauncherIconManager(), ): SettingsViewModel { val harness = worldClockHarness(tempDir) + alarmEngine = AlarmEngine( + alarms = AlarmRepositoryImpl(alarmDao, wallClock), + states = AlarmStateRepositoryImpl(stateDao), + settings = harness.settings, + scheduler = alarmScheduler, + ring = ring, + notifier = FakeAlarmNotifier(), + rebootRepair = RecordingRebootRepair(), + wallClock = wallClock, + zones = FakeZoneProvider(BERLIN), + ) return SettingsViewModel( settings = harness.settings, + alarmEngine = alarmEngine, ringtones = FakeRingtoneCatalog(), preview = FakeRingtonePreviewer(), worldClocks = harness.source, @@ -119,6 +157,8 @@ class SettingsViewModelTest { viewModel.onSnoozeLimitChanged(2) viewModel.onVolumeRampChanged(30) viewModel.onDismissChallengeChanged(DismissChallenge.MATH) + viewModel.onAlarmSilenceAfterChanged(25) + viewModel.onTimerSilenceAfterChanged(SilenceAfter.NEVER) advanceUntilIdle() val defaults = (viewModel.state.value as SettingsUiState.Loaded).defaults @@ -127,8 +167,24 @@ class SettingsViewModelTest { assertThat(defaults.snoozeLimit).isEqualTo(2) assertThat(defaults.volumeRampSeconds).isEqualTo(30) assertThat(defaults.dismissChallenge).isEqualTo(DismissChallenge.MATH) + assertThat(defaults.alarmSilenceAfterMinutes).isEqualTo(25) + assertThat(defaults.timerSilenceAfterMinutes).isEqualTo(SilenceAfter.NEVER) } + @Test + fun `changing the alarm silence-after re-measures a ring in progress`(@TempDir tempDir: Path) = + runTest(main.dispatcher) { + val viewModel = collecting(newViewModel(tempDir)) + alarmDao.insert(AlarmMapper.toEntity(alarmAt(id = 1L, hour = 7, repeatDays = RepeatDays.EVERY_DAY))) + alarmEngine.onFire(alarmId = 1L, scheduledFor = wallClock.instant) + advanceUntilIdle() + + viewModel.onAlarmSilenceAfterChanged(25) + advanceUntilIdle() + + assertThat(alarmScheduler.autoSilence?.fireAt).isEqualTo(wallClock.instant + 25.minutes) + } + @Test fun `choosing a default alarm ringtone writes through and closes the picker`(@TempDir tempDir: Path) = runTest(main.dispatcher) { diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 31868fc..b035745 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1163,26 +1163,37 @@ Never), **10 minutes** by default, AOSP DeskClock's default and v1's fixed value, so an upgrade changes nothing. Snoozing keeps it open. The window is read from `ClockDefaults.alarmSilenceAfter` everywhere a ring is -measured: the resolver's ringing and late-snooze checks, the backstop `onFire` -arms, how late a snooze fire may arrive, and whether a ring interrupted by a -reboot is resumed. `AlarmResolver.resolve` takes it without a default, so no -caller can measure against a window the user did not choose. +measured: the resolver's ringing check, the backstop `onFire` arms, and whether +a ring interrupted by a reboot is resumed. `AlarmResolver.resolve` takes it +without a default, so no caller can measure against a window the user did not +choose. How late a snooze may come due and still ring is the window **or +`AlarmRing.SNOOZE_GRACE` (v1's ten minutes), whichever is longer**: a short +"Silence after" must not make a promised snooze easier to lose to a reboot. **"Never" is an hour.** The backstop is also what stops a phone left at home from ringing until its battery is flat, what bounds the ring services' wake locks, and what keeps a reboot hours later from resuming a ring nobody was there for. So "Never" maps to `SilenceAfter.HARD_STOP` (60 minutes), longer than any finite choice, and the settings hint says so in words. Stored values -are clamped to 0..60 on read, on write and from a backup, so no value can make -a ring outlast it. +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". -A ring started under one window keeps the backstop it was armed with. When the -window is shortened mid-ring, the next re-resolution closes the ring once it is -past the new window, and `persist` then stops the ring service and cancels the -backstop with it: storage says nothing rings, so nothing may. The same applies -to any ring the resolver closes (a clock jumped forward, a ringing alarm -switched off), which earlier only cleared `ringingSince` and left the sound to -the backstop. +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. + +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 +and cancels the backstop: storage says nothing rings, so nothing may. Earlier +that path only cleared `ringingSince` and left the sound to the backstop. And a +cycle the resolver closes as missed (a ring past its window, or a snooze that +never fired) **disables a one-shot alarm**, exactly as `closeCycleLocked` +does; falling through would have armed it for the same time tomorrow. A **non-repeating alarm is disabled when its cycle closes, never when it opens**. Disabling it at the start would clear its own snooze and the alarm would vanish @@ -1924,7 +1935,12 @@ The window is the timer's own **"Silence after"** setting default: the same choices as the alarm's and the same one-hour "Never", but a separate value, because coupling the two would make one number answer two questions. `TimerRingPolicy` reads it from the defaults it already takes, so a -change applies on the service's next pass. Unlike an alarm's, a timer's +change applies on the service's next pass. A session that has gone quiet stays +quiet: on auto-silence the policy moves the stored anchor back past +`SilenceAfter.HARD_STOP`, so raising the setting afterwards cannot start it +ringing again. The service's one heads-up is keyed to the session's anchor +(`Sound.openedAt`) rather than to the window's end, which an edit can move +without making a new session. Unlike an alarm's, a timer's **auto-silence stops the noise and leaves the row `EXPIRED`.** An auto-silenced alarm is a *missed* alarm and its cycle closes; an auto-silenced timer has still finished, and the user coming back must be able to see that and add a minute diff --git a/docs/BACKUP.md b/docs/BACKUP.md index 670499b..c5d7799 100644 --- a/docs/BACKUP.md +++ b/docs/BACKUP.md @@ -151,7 +151,8 @@ other unknown field. are how long a ringing alarm or a finished timer keeps going before it stops on its own: whole minutes, with `0` meaning "Never" (which still stops after an hour). A file without them, from before the setting existed, imports as `10`, -the value it was made with. Out-of-range values clamp to 0..60. +the value it was made with. A value over 60 reads as 60 (which is what Never +rings for anyway); a negative one reads as the default, 10. Every numeric/duration value is clamped on *apply* using the same ranges and encoding as the existing preference setters, never on parse.