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 fc09bc8..ea5ce1d 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt @@ -29,7 +29,6 @@ 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 @@ -84,15 +83,9 @@ class AlarmEngine @Inject constructor( 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 - } + // 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 // Missed, not snoozed: the ring stops, the cycle closes, the next // occurrence is scheduled. There is no missed-alarm notification in v1. @@ -109,12 +102,7 @@ class AlarmEngine @Inject constructor( 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) - } + if (!rearmIfInsideWindowLocked(state.alarmId, since)) closeCycleLocked(state.alarmId, state) } /** [minutesOverride] is a one-off, already clamped; it is never persisted anywhere (M9 D15). */ @@ -288,16 +276,22 @@ class AlarmEngine @Inject constructor( // At most one alarm rings, and the newly-firing one takes over. The one // it displaces keeps its handled-occurrence watermark, so it does not // come back the moment the slot is re-resolved. + // Its cycle has closed undismissed, so a one-shot alarm is switched off + // as on every other such close, rather than armed for tomorrow. states.ringing()?.takeIf { it.alarmId != alarmId }?.let { other -> ring.stopRinging(other.alarmId) states.save(other.copy(ringingSince = null, snoozedUntil = null, snoozeCount = 0)) + alarms.find(other.alarmId)?.takeIf { !it.repeatDays.isRepeating }?.let { disableOrDeleteLocked(it) } } + // `ringingSince` is cleared too: this fire opens the ring afresh, and a + // stale value left behind would let the re-resolution below close (and + // switch off) the very alarm that is about to ring. val opened = if (isSnoozeFire) { // The cycle is the same one: the count and the watermark carry over. - state.copy(snoozedUntil = null) + state.copy(ringingSince = null, snoozedUntil = null) } else { - state.copy(snoozedUntil = null, snoozeCount = 0, handledOccurrence = scheduledFor) + state.copy(ringingSince = null, snoozedUntil = null, snoozeCount = 0, handledOccurrence = scheduledFor) } // The watermark is written before anything else can interrupt, and the // slot is re-resolved while the alarm is not yet marked as ringing — so @@ -345,18 +339,30 @@ class AlarmEngine @Inject constructor( private suspend fun resumeRingingIfAnyLocked(): Long? { val state = states.ringing() ?: return null val since = state.ringingSince ?: return null - val silenceAfter = settings.currentDefaults().alarmSilenceAfter - // Slept through it: leave it to the next resolution to close the cycle. - if (wallClock.now() - since >= silenceAfter) return null val alarm = alarms.find(state.alarmId)?.takeIf { it.enabled } ?: return null + // Slept through it: leave it to the next resolution to close the cycle. + if (!rearmIfInsideWindowLocked(alarm.id, since)) return null ring.startRinging(alarm.id) - // The backstop is measured from when the ring started, not from now, so - // a reboot does not buy the alarm a fresh window. - scheduler.scheduleAutoSilence(since + silenceAfter, alarm.id) return alarm.id } + /** + * The one rule for measuring a ring against "Silence after": still inside + * the window ⇒ the backstop is (re-)armed at its end and this returns true; + * run out ⇒ false, and the caller closes or drops it. Measured from when the + * ring started, so neither a reboot nor a setting change buys it a fresh + * window — or from now, if the clock has since moved back behind that + * start, so a clock change cannot stretch a ring past one window either. + */ + private suspend fun rearmIfInsideWindowLocked(alarmId: Long, since: Instant): Boolean { + val now = wallClock.now() + val silenceAfter = settings.currentDefaults().alarmSilenceAfter + if (now - since >= silenceAfter) return false + scheduler.scheduleAutoSilence(minOf(since, now) + silenceAfter, alarmId) + return true + } + /** * Everything the resolver asked to be written, and nothing it did not — * plus stopping the sound of a ring it closed, which storage alone cannot do. 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 2982b15..a61b9ab 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 @@ -49,9 +49,8 @@ object SilenceAfter { * 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 + minutes < MINUTES.first -> DEFAULT_MINUTES + else -> minutes.coerceAtMost(MINUTES.last) } /** The ring window [minutes] stands for, sanitised first so the engines never see an unofferable value. */ 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 50f13cd..b8540c4 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineTest.kt @@ -990,6 +990,52 @@ class AlarmEngineTest { assertThat(scheduler.next).isNull() } + @Test + fun `a one-shot alarm displaced by another's fire is switched off`(@TempDir dir: Path) = runTest { + givenAlarm(id = 1L, hour = 7) + val sevenFive = offsetInstant("2026-06-10T07:05+02:00") + givenAlarm(id = 2L, hour = 7, minute = 5, repeatDays = RepeatDays.EVERY_DAY) + givenRingingAt(alarmId = 1L, occurrence = sevenToday, since = sevenToday) + wallClock.instant = sevenFive + val engine = engine(dir) + + engine.onFire(alarmId = 2L, scheduledFor = sevenFive) + + assertThat(alarms.find(1L)?.enabled).isFalse() + assertThat(ring.ringing.value).isEqualTo(2L) + } + + @Test + fun `a stale ring left in storage cannot switch off the alarm that is firing`(@TempDir dir: Path) = + runTest { + givenAlarm() + val yesterday = offsetInstant("2026-06-09T07:00+02:00") + givenRingingAt(alarmId = 1L, occurrence = yesterday, since = yesterday) + wallClock.instant = sevenToday + val engine = engine(dir) + + val outcome = engine.onFire(alarmId = 1L, scheduledFor = sevenToday) + + assertThat(outcome).isEqualTo(FireOutcome.Ringing(1L)) + assertThat(alarms.find(1L)?.enabled).isTrue() + assertThat(states.state(1L).ringingSince).isEqualTo(sevenToday) + } + + @Test + fun `a clock moved back mid-ring cannot stretch it past one window`(@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 - 30.minutes + + settings.setAlarmSilenceAfterMinutes(5) + engine.onSilenceAfterChanged() + + assertThat(scheduler.autoSilence).isEqualTo(Registration(sevenToday - 30.minutes + 5.minutes, 1L)) + } + // --- upcoming() --- @Test diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index b035745..4344b71 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1229,7 +1229,9 @@ genuinely ringing alarm sounding with its backstop intact. **At most one alarm rings, and a newly-firing alarm takes over.** Two ringtones at once is not a feature, and queueing invents a state machine nobody can debug at 07:00. The alarm that is displaced keeps its watermark, so it does not come -back the moment the slot is re-resolved. +back the moment the slot is re-resolved. Its cycle has closed undismissed, so a +displaced one-shot alarm is switched off (or deleted, if transient), as on +every other close. `snoozeLimit == 0` means **snoozing is disabled**, not unlimited; v1 offers no unlimited, and "0 means infinite" is a trap. A refused snooze *dismisses*; it