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 20b849f..d9ff0a9 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 java.time.ZoneId import kotlin.time.Duration.Companion.minutes import kotlin.time.Instant import javax.inject.Inject @@ -188,38 +187,23 @@ class AlarmEngine @Inject constructor( val zone = zones.current() val byId = states.associateBy { it.alarmId } alarms - .map { alarm -> preview(alarm, byId[alarm.id] ?: AlarmRingState.initial(alarm.id), now, zone) } + .map { alarm -> + // The resolver's state is deliberately discarded: a read must + // not write, however much it resolved on the way past. + val resolution = AlarmResolver.resolve( + alarm, + byId[alarm.id] ?: AlarmRingState.initial(alarm.id), + now, + zone, + ) + UpcomingAlarm(alarm, resolution.nextFire, resolution.source) + } .sortedWith( compareBy(nullsLast()) { it.nextFire } .thenBy { it.alarm.id }, ) } - /** - * One alarm's [upcoming] entry, resolved against [now]. Read-only like - * [upcoming]; null for an id no alarm has. A caller that measures a - * countdown passes the `now` it measures from, so both ends of it are the - * same instant. - * - * It writes nothing, but it does take [lock]: the alarm and its ring state - * are two reads, and a fire landing between them would pair an alarm with - * a state that is not its own. `upcoming()` gets the same consistency from - * combining the two flows. - */ - suspend fun upcomingFor(alarmId: Long, now: Instant = wallClock.now()): UpcomingAlarm? = lock.withLock { - val alarm = alarms.find(alarmId) ?: return@withLock null - preview(alarm, states.state(alarmId), now, zones.current()) - } - - /** - * The resolver in preview mode. Its state is deliberately discarded: a read - * must not write, however much it resolved on the way past. - */ - private fun preview(alarm: Alarm, state: AlarmRingState, now: Instant, zone: ZoneId): UpcomingAlarm { - val resolution = AlarmResolver.resolve(alarm, state, now, zone) - return UpcomingAlarm(alarm, resolution.nextFire, resolution.source) - } - private suspend fun rescheduleLocked(): ScheduledFire? { val now = wallClock.now() val zone = zones.current() diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt index fbdf471..1020152 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmSetSnackbar.kt @@ -18,28 +18,29 @@ import de.jeanlucmakiola.clockula.domain.time.ElapsedRealtimeClock import de.jeanlucmakiola.clockula.domain.time.WallClock import de.jeanlucmakiola.floret.di.ApplicationScope import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.Job import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.collectLatest -import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.flatMapLatest +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch -import kotlinx.coroutines.sync.Mutex -import kotlinx.coroutines.sync.withLock import javax.inject.Inject import javax.inject.Singleton import kotlin.time.Duration import kotlin.time.Duration.Companion.seconds -/** "Alarm set for … from now", about one alarm, until a screen has shown it (#19). */ +/** "Alarm set for … from now", about one alarm, as a screen should show it (#19). */ data class AlarmSetNotice( val alarmId: Long, val countdown: NextFireLabel, - /** On the monotonic clock: a wall clock set back must not make an old notice fresh. */ - val announcedAt: Duration, + /** Which announcement this is, so [AlarmSetAnnouncer.consumed] takes back only that one. */ + val sequence: Long, ) /** @@ -50,12 +51,17 @@ data class AlarmSetNotice( * `AlarmClock` intents never announces itself: those already get the * status-bar icon. * - * App-wide, and a **state** rather than an event: the notice stays until a - * screen has shown it to the end ([consumed]). The usual way to set an alarm is - * pick → OK → back, and back disposes the editor's snackbar a moment after it - * appeared; the list underneath picks the same notice up. Announcing runs on - * the application scope for the same reason — leaving the editor while its - * repeat days settle must not lose them. + * It records only *which* alarm was set, and when. What to say is derived from + * the engine while a screen shows it, so it is never a stale or superseded + * value, and an alarm switched off or deleted by any path — the row's switch, + * the ring screen, a notification, an intent, a restore — says nothing. + * + * App-wide, and a **state** rather than an event: the announcement stands + * until a screen has shown it to the end ([consumed]). The usual way to set an + * alarm is pick → OK → back, and back disposes the editor's snackbar a moment + * after it appeared; the list underneath picks the same announcement up. The + * settle runs on the application scope for the same reason — leaving the + * editor while its repeat days settle must not lose them. */ @Singleton internal class AlarmSetAnnouncer @Inject constructor( @@ -66,99 +72,79 @@ internal class AlarmSetAnnouncer @Inject constructor( // annotation on a constructor `val` to the property instead. @param:ApplicationScope private val appScope: CoroutineScope, ) { - private val current = MutableStateFlow(null) + /** One user action's announcement. [at] is monotonic: a wall clock set back must not refresh it. */ + private data class Announcement(val alarmId: Long, val at: Duration, val sequence: Long) + + private val current = MutableStateFlow(null) + + /** The request still settling, and the alarm it is about. Guarded by `this`. */ + private var pending: Job? = null + private var pendingFor: Long? = null + private var sequence = 0L /** - * Serialises the read-then-publish: two switch taps in quick succession - * must not let the earlier read land last — "set for 9 hours" about an - * alarm that is off. - */ - private val lock = Mutex() - - /** - * Requests still settling, one per alarm: a request about one alarm must - * not cancel another's — switching B off in the list would otherwise - * swallow the repeat days just set on A. Guarded by `this`. - */ - private val pending = mutableMapOf() - - /** - * The notice to show, or null. Past [FRESH_FOR] it is dropped unshown: a - * countdown that surfaced when the user came back to the app a while later - * would be wrong, and about something they did long ago. + * The notice to show, or null. * - * Live against the engine: every alarm write re-reads it, so an alarm - * switched off or deleted by any path — the ring screen, a notification, - * an intent, a restore — takes a snackbar down even while it is showing, - * and one whose ring moved (a skip armed from the notification) says the - * new countdown rather than the one it was announced with. + * Freshness is judged once, when a screen picks the announcement up: past + * [FRESH_FOR] it is dropped unshown — a countdown surfacing when the user + * came back to the app a while later would be about something they did + * long ago — but one already showing is not cut off half-way. The engine + * is observed only while an announcement stands, and a new announcement + * re-subscribes, so its first read is from after the write that asked. */ - val notice: Flow = combine(current, engine.upcoming()) { notice, upcoming -> - if (notice == null || !isFresh(notice)) return@combine null - val entry = upcoming.firstOrNull { it.alarm.id == notice.alarmId } - AlarmSetCountdown.of(entry, wall.now())?.let { notice.copy(countdown = it) } - }.distinctUntilChanged() + @OptIn(ExperimentalCoroutinesApi::class) + val notice: Flow = current + .flatMapLatest { announcement -> + if (announcement == null || elapsed.elapsedRealtime() - announcement.at >= FRESH_FOR) { + flowOf(null) + } else { + engine.upcoming().map { upcoming -> + val entry = upcoming.firstOrNull { it.alarm.id == announcement.alarmId } + AlarmSetCountdown.of(entry, wall.now()) + ?.let { AlarmSetNotice(announcement.alarmId, it, announcement.sequence) } + } + } + } + .distinctUntilChanged() /** * Call once the write is stored. Announces [alarmId] after [settle], unless - * a newer request replaces this one first — latest wins, so a burst of - * changes is one snackbar. It reads the alarm as it is then, so it is never - * about a superseded value; an alarm that will not ring takes back a notice - * still standing about it, and otherwise says nothing. + * a newer request replaces this one first. Latest wins, across alarms too: + * the snackbar names no alarm, so a settling announcement about one must + * never land on top of the user's newer action on another and read as + * that one's countdown. */ fun announce(alarmId: Long, settle: Duration = Duration.ZERO) { synchronized(this) { - pending.remove(alarmId)?.cancel() - val job = appScope.launch { + pending?.cancel() + pendingFor = alarmId + pending = appScope.launch { delay(settle) - publish(alarmId) + synchronized(this@AlarmSetAnnouncer) { + current.value = Announcement(alarmId, elapsed.elapsedRealtime(), ++sequence) + } } - pending[alarmId] = job - job.invokeOnCompletion { synchronized(this) { pending.remove(alarmId, job) } } } } /** - * Takes back anything said or about to be said about [alarmId]: it was - * deleted or dismissed. Under the same lock as [publish], so a publish - * already past its read cannot put the notice back afterwards. + * Takes back anything said or about to be said about [alarmId]. For a + * change of state the engine would read as "still rings" — a dismissal + * that skips the occurrence being counted down to. */ - suspend fun retract(alarmId: Long) { - synchronized(this) { pending.remove(alarmId)?.cancel() } - lock.withLock { - current.update { notice -> notice?.takeUnless { it.alarmId == alarmId } } + fun retract(alarmId: Long) { + synchronized(this) { + if (pendingFor == alarmId) { + pending?.cancel() + pendingFor = null + } + current.update { standing -> standing?.takeUnless { it.alarmId == alarmId } } } } - /** - * [notice] was shown until it timed out or was swiped away. Matched on the - * announcement, not the words: the countdown shown may be a re-read one. - */ + /** [notice] was shown until it timed out or was swiped away. */ fun consumed(notice: AlarmSetNotice) { - current.update { standing -> - standing?.takeUnless { it.alarmId == notice.alarmId && it.announcedAt == notice.announcedAt } - } - } - - private suspend fun publish(alarmId: Long) = lock.withLock { - val countdown = countdownOf(alarmId) - current.update { standing -> - when { - countdown != null -> AlarmSetNotice(alarmId, countdown, elapsed.elapsedRealtime()) - standing?.alarmId == alarmId -> null - else -> standing - } - } - } - - private fun isFresh(notice: AlarmSetNotice): Boolean = - elapsed.elapsedRealtime() - notice.announcedAt < FRESH_FOR - - private suspend fun countdownOf(alarmId: Long): NextFireLabel? { - // One `now` for both ends of the countdown, or a resolve that lands a - // hair past the occurrence would read as a day and an hour. - val now = wall.now() - return AlarmSetCountdown.of(engine.upcomingFor(alarmId, now), now) + current.update { standing -> standing?.takeUnless { it.sequence == notice.sequence } } } private companion object { 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 2b38175..a0ba1a2 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 @@ -83,14 +83,11 @@ class AlarmsViewModel @Inject internal constructor( // One engine verb, so the contract's dismissal and the user's are the // same code: it closes a ring, arms a repeating alarm's skip, or // disables a one-shot — each of which moves state only the engine owns. - viewModelScope.launch { - engine.dismissUpcoming(alarmId) - // Whatever the dismissal did — disabled it, or skipped the very - // occurrence a notice counts down to — "set for" no longer holds. - // Not re-announced: this is the `AlarmClock` contract's request - // (M9 D13), which the snackbar stays out of (#19). - announcer.retract(alarmId) - } + // First, so the snackbar never re-reads the skip and counts down to the + // occurrence after it. Not re-announced: this is the `AlarmClock` + // contract's request (M9 D13), which the snackbar stays out of (#19). + announcer.retract(alarmId) + viewModelScope.launch { engine.dismissUpcoming(alarmId) } } /** Dismisses a ring first when switching off (M5 D11), then re-resolves. */ @@ -99,9 +96,10 @@ class AlarmsViewModel @Inject internal constructor( if (!enabled) stopRingIfRinging(alarmId) alarms.setEnabled(alarmId, enabled) engine.reschedule() - // Switching on says when it will ring; switching off takes back a - // "set for" still on screen about this alarm (#19). - announcer.announce(alarmId) + // Switching on says when it will ring (#19). Switching off needs + // nothing: the notice is read from the engine, so one about this + // alarm falls silent by itself, and one about another stands. + if (enabled) announcer.announce(alarmId) } } diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index cfa5939..9e7a5c8 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1116,25 +1116,22 @@ its own writes. `reschedule()` is called explicitly: from `ClockulaApp`, from the receivers, and from the alarm editor. `upcoming()` runs the same resolver in preview mode and *discards* the state it returns; a read must not write, and a test asserts the store recorded nothing across a full collection. -`upcomingFor(alarmId)` is the same read for one alarm, on demand: the list and -the editor ask it right after a user's change to say how long until that alarm -rings. `AlarmSetCountdown` (pure, in `domain/alarm/`) decides what that says — -only a natural occurrence of an enabled alarm, rounded as the row's countdown -is, never a snooze or a ring. The `AlarmClock` intents never reach it, since -they already get the status-bar icon (#19). The editor announces a time pick -at once, but waits for repeat-day taps to settle for a second, so Mon–Fri is -one snackbar rather than five. `AlarmSetAnnouncer` is an app-wide singleton -holding the notice as *state* until a screen has shown it to the end: pick → -OK → back disposes the editor's snackbar a moment after it appears, and the -list underneath picks the same notice up. The editor's fire-time-moving -writes, their reschedule and the per-alarm settle all run on the application -scope, so leaving the editor at once neither loses the announcement nor leaves -a stored time unscheduled. A screen shows the notice only while resumed, so a -cross-fade never shows it twice; the editor shows only notices about its own -alarm; a notice is re-checked against the engine before it is shown, so an -alarm switched off or deleted by any path is never announced as set. A notice -older than ten seconds on the monotonic clock is dropped unshown rather than -shown with a stale countdown. +The "Alarm set for … from now" snackbar (#19) is built on `upcoming()` too. +`AlarmSetAnnouncer`, an app-wide singleton, records only *which* alarm a user +action in the list or the editor just set; while a resumed screen shows it, +what it says is derived from `upcoming()`, through `AlarmSetCountdown` (pure, +in `domain/alarm/`): only a natural occurrence of an enabled alarm, rounded as +the row's countdown is, never a snooze or a ring. So an alarm switched off or +deleted by any path is never announced as set, and the `AlarmClock` intents, +which already get the status-bar icon, never reach it. The announcement is +*state*, standing until a screen has shown it to the end: pick → OK → back +disposes the editor's snackbar a moment after it appears, and the list +underneath picks it up. The editor announces a time pick at once and waits a +second for repeat-day taps to settle, so Mon–Fri is one snackbar; the settle, +and the editor's fire-time-moving writes with their reschedule, run in tap +order on the application scope, so leaving the editor at once neither loses +the announcement nor leaves a stored time unscheduled. An announcement older +than ten seconds on the monotonic clock is dropped unshown. `upcoming(refresh: Flow)` takes its **re-read cadence as a parameter**. The resolver needs a `now`, and `upcoming` reads one inside a `combine` of the