refactor(alarms): derive the alarm-set notice from the engine alone
The announcer now records only which alarm a user action set, and when. What the snackbar says is derived from upcoming() while a screen shows it, re-subscribed per announcement so the first read is from after the write. That removes the second resolve path through the engine lock (upcomingFor is gone), the publish lock, and the flash of a pre-write countdown. The engine is observed only while an announcement stands. Freshness is judged when a screen picks the announcement up, so one already showing is not cut off half-way. Consuming matches a sequence number. Latest action wins across alarms again: the sentence names no alarm, so a settling announcement must never land on top of a newer action on another one. Switching an alarm off announces nothing, and a dismissal retracts before it skips. Refs #19
This commit is contained in:
1 parent
ff49667d39
commit
9516dc1198
4 files changed
+106
-141
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 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<UpcomingAlarm, Instant?>(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()
|
||||
|
||||
@@ -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<AlarmSetNotice?>(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<Announcement?>(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<Long, Job>()
|
||||
|
||||
/**
|
||||
* 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<AlarmSetNotice?> = 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<AlarmSetNotice?> = 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 {
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+16
-19
@@ -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<Unit>)` takes its **re-read cadence as a parameter**.
|
||||
The resolver needs a `now`, and `upcoming` reads one inside a `combine` of the
|
||||
|
||||
Reference in new issue
Block a user