fix(alarms): measure every ring against one window rule
Second round of review fixes for the silence-after setting. A fire now clears any stale ringingSince before it re-resolves the slot. Before, the resolver's new one-shot close could switch off, or delete, the very alarm that was about to ring. A one-shot alarm displaced by another alarm's fire is switched off, as on every other close of an undismissed cycle, instead of being armed for the same time tomorrow. onAutoSilence, onSilenceAfterChanged and the reboot resume now share one rule. A ring still inside the window has its backstop re-armed, measured from when it started, or from now if the clock has moved back behind that start, so a clock change cannot stretch a ring past one window. Refs #17
This commit is contained in:
1 parent
1c323d5b4c
commit
fb875ca71c
4 files changed
+81
-28
No files matched your search
@@ -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.
|
||||
|
||||
@@ -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. */
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in new issue
Block a user