From e43339c386545e20453c0bc07a08f03adb98cba4 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Wed, 21 Oct 2026 20:01:21 +0200 Subject: [PATCH 01/10] feat(alarms): notify before an alarm rings, with Dismiss Two hours before a natural occurrence (AOSP DeskClock's lead), a silent "Upcoming alarm" notification offers Dismiss, and the alarm's row shows the same Dismiss as a chip. Dismissing is dismissUpcoming's own branches: a repeating alarm skips that occurrence, a one-shot is switched off. The notices are synced at the end of every reschedule, after the real registration, and a third AlarmManager slot on its own request code wakes when the next window opens. The sync is wrapped so a notification failure can never stop a ring. Dismiss names the occurrence and is refused once the alarm is ringing or has moved, so it can't get round the dismiss challenge. No challenge is asked before the ring, as in Google Clock. Fixes #16 --- CHANGELOG.md | 5 + app/src/main/AndroidManifest.xml | 2 +- .../de/jeanlucmakiola/clockula/ClockulaApp.kt | 2 + .../clockula/alarm/AlarmEngine.kt | 87 +++++++-- .../clockula/alarm/AlarmIntents.kt | 12 ++ .../clockula/alarm/AlarmNotifier.kt | 9 +- .../clockula/alarm/AlarmScheduler.kt | 11 +- .../alarm/android/AndroidAlarmNotifier.kt | 52 +++++- .../alarm/android/AndroidAlarmScheduler.kt | 36 +++- .../alarm/receiver/AlarmActionReceiver.kt | 7 +- .../alarm/receiver/AlarmFireReceiver.kt | 4 +- .../alarm/ring/UpcomingNotifications.kt | 116 ++++++++++++ .../clockula/domain/alarm/UpcomingAlarm.kt | 6 + .../clockula/domain/alarm/UpcomingNotices.kt | 60 +++++++ .../clockula/ui/alarms/AlarmRow.kt | 26 +++ .../clockula/ui/alarms/AlarmRowState.kt | 8 + .../clockula/ui/alarms/AlarmsScreen.kt | 3 + .../clockula/ui/alarms/AlarmsViewModel.kt | 10 ++ app/src/main/res/values/strings.xml | 5 + .../clockula/alarm/AlarmEngineUpcomingTest.kt | 167 ++++++++++++++++++ .../clockula/alarm/AlarmIntentsTest.kt | 5 + .../domain/alarm/UpcomingNoticesTest.kt | 91 ++++++++++ .../clockula/testing/FakeAlarmSeams.kt | 27 ++- .../clockula/ui/alarms/AlarmListRowsTest.kt | 17 ++ docs/ARCHITECTURE.md | 50 +++++- 25 files changed, 785 insertions(+), 33 deletions(-) create mode 100644 app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/UpcomingNotifications.kt create mode 100644 app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNotices.kt create mode 100644 app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt create mode 100644 app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNoticesTest.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index 76d4d0d..b48fdc6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,11 @@ The first release of Clockula: alarms, timers, a stopwatch and a world clock. Changes apply as you make them, with no Save button. - Repeat days that start on your locale's first day of the week, and a "skip next alarm" switch so you can sleep in once without turning the alarm off. +- An "Upcoming alarm" notification two hours before an alarm rings, with a + Dismiss button, so you can turn off today's alarm when you're already awake. + A repeating alarm skips just that once; a one-time alarm is switched off. + The same Dismiss shows on the alarm in the list. The notification is silent, + and turning it off in the system settings doesn't affect the alarm. - Per-alarm sound, vibration, gradual volume, snooze length and limit, and a dismiss challenge (a quick sum or a two-second hold). Each one can follow the app default, so changing a default later still reaches it. diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index cfe76b4..fd9442c 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -203,7 +203,7 @@ android:exported="false" android:foregroundServiceType="systemExempted" /> - ? = null + val resolved = mutableListOf() for (alarm in alarms.alarms().first()) { val state = states.state(alarm.id) val resolution = AlarmResolver.resolve(alarm, state, now, zone) persist(alarm, state, resolution) + resolved += UpcomingAlarm(alarm, resolution.nextFire, resolution.source) val fireAt = resolution.nextFire ?: continue val best = earliest @@ -222,13 +240,52 @@ class AlarmEngine @Inject constructor( if (isEarlier) earliest = alarm to resolution } - val winner = earliest ?: run { + val winner = earliest + val scheduled = if (winner == null) { scheduler.cancelNext() - return null + null + } else { + val fireAt = winner.second.nextFire!! + val mode = scheduler.scheduleNext(fireAt, winner.first.id) + ScheduledFire(winner.first.id, fireAt, winner.second.source, mode) } - val fireAt = winner.second.nextFire!! - val mode = scheduler.scheduleNext(fireAt, winner.first.id) - return ScheduledFire(winner.first.id, fireAt, winner.second.source, mode) + // After the real registration, never before it. + syncUpcomingLocked(resolved, now) + return scheduled + } + + /** + * The upcoming-alarm notices and the wake-up for the next window (#16). + * Every path that moves an alarm — an edit, a delete, a switch, a skip, a + * snooze, a fire, a boot, a clock or zone change — already ends in + * [rescheduleLocked], so this is the one place they are kept true. + * + * Nothing here may cost an alarm: [onFireLocked] reschedules *before* it + * starts the ring, so a failure on the way is swallowed rather than thrown. + */ + private fun syncUpcomingLocked(resolved: List, now: Instant) { + runCatching { + val plan = UpcomingNotices.plan(resolved, now) + notifier.showUpcoming(plan.notices) + val next = plan.nextCheckAt + if (next == null) scheduler.cancelUpcomingCheck() else scheduler.scheduleUpcomingCheck(next) + } + } + + /** + * "Dismissing a repeating alarm only prevents the upcoming instance from + * ringing"; dismissing a single occurrence "effectively disables" it. + */ + private suspend fun dismissComingLocked(alarm: Alarm): DismissOutcome { + if (alarm.repeatDays.isRepeating) { + alarms.setSkipNextOccurrence(alarm.id, true) + rescheduleLocked() + return DismissOutcome.SkippedNext(alarm.id) + } + + disableOrDeleteLocked(alarm) + rescheduleLocked() + return DismissOutcome.Disabled(alarm.id) } private suspend fun onFireLocked(alarmId: Long, scheduledFor: Instant): FireOutcome { diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmIntents.kt b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmIntents.kt index a5876be..5b79f4c 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmIntents.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmIntents.kt @@ -14,6 +14,12 @@ object AlarmIntents { const val ACTION_SNOOZE: String = PREFIX + "action.ALARM_SNOOZE" const val ACTION_DISMISS: String = PREFIX + "action.ALARM_DISMISS" + /** The upcoming-alarm window opening: re-resolve and post the notices (#16). */ + const val ACTION_UPCOMING_CHECK: String = PREFIX + "action.ALARM_UPCOMING_CHECK" + + /** The upcoming notification's Dismiss, which carries [EXTRA_FIRE_AT] (#16). */ + const val ACTION_DISMISS_UPCOMING: String = PREFIX + "action.ALARM_DISMISS_UPCOMING" + /** Internal: "show me the alarms tab", the transport for `SHOW_ALARMS` and the next-alarm show intent. */ const val ACTION_SHOW_ALARMS: String = PREFIX + "action.SHOW_ALARMS" @@ -26,6 +32,9 @@ object AlarmIntents { const val REQUEST_SNOOZE: Int = 4 const val REQUEST_DISMISS: Int = 5 const val REQUEST_REPOST: Int = 6 + const val REQUEST_UPCOMING_CHECK: Int = 7 + const val REQUEST_DISMISS_UPCOMING: Int = 8 + const val REQUEST_SHOW_UPCOMING: Int = 9 val ALL_REQUEST_CODES: List = listOf( REQUEST_FIRE, @@ -34,5 +43,8 @@ object AlarmIntents { REQUEST_SNOOZE, REQUEST_DISMISS, REQUEST_REPOST, + REQUEST_UPCOMING_CHECK, + REQUEST_DISMISS_UPCOMING, + REQUEST_SHOW_UPCOMING, ) } diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmNotifier.kt b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmNotifier.kt index 2aeda4b..b91fbc2 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmNotifier.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmNotifier.kt @@ -1,11 +1,18 @@ package de.jeanlucmakiola.clockula.alarm import de.jeanlucmakiola.clockula.domain.Alarm +import de.jeanlucmakiola.clockula.domain.alarm.UpcomingNotice import kotlin.time.Instant -/** The snoozed notification — the one surface the engine posts directly. */ +/** The snoozed and upcoming notifications — the surfaces the engine posts directly. */ interface AlarmNotifier { fun showSnoozed(alarm: Alarm, until: Instant) fun clearSnoozed(alarmId: Long) + + /** + * Makes the upcoming-alarm notifications exactly [notices]: posts the new + * ones and removes any that are no longer due (#16). Never throws. + */ + fun showUpcoming(notices: List) } diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmScheduler.kt b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmScheduler.kt index e18f065..a83b245 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmScheduler.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmScheduler.kt @@ -3,7 +3,7 @@ package de.jeanlucmakiola.clockula.alarm import de.jeanlucmakiola.clockula.domain.alarm.ScheduleMode import kotlin.time.Instant -/** The AlarmManager seam. Two slots, deliberately separate. */ +/** The AlarmManager seam. Three slots, deliberately separate. */ interface AlarmScheduler { /** Replaces the single next-alarm registration. Returns the mode actually used. */ fun scheduleNext(fireAt: Instant, alarmId: Long): ScheduleMode @@ -14,6 +14,15 @@ interface AlarmScheduler { fun cancelAutoSilence() + /** + * When the next upcoming-alarm window opens (#16). Its own PendingIntent and + * request code, never `setAlarmClock`: a housekeeping wake-up must not + * replace what the system shows as the user's next alarm. + */ + fun scheduleUpcomingCheck(at: Instant) + + fun cancelUpcomingCheck() + /** `AlarmManager.getNextAlarmClock()`, as the *system* sees it. */ fun systemNextAlarm(): Instant? diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/android/AndroidAlarmNotifier.kt b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/android/AndroidAlarmNotifier.kt index 541d4f4..113440d 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/android/AndroidAlarmNotifier.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/android/AndroidAlarmNotifier.kt @@ -1,6 +1,7 @@ package de.jeanlucmakiola.clockula.alarm.android import android.Manifest +import android.app.NotificationManager import android.content.Context import android.content.pm.PackageManager import android.os.Build @@ -9,15 +10,18 @@ import androidx.core.content.ContextCompat import dagger.hilt.android.qualifiers.ApplicationContext import de.jeanlucmakiola.clockula.alarm.AlarmNotifier import de.jeanlucmakiola.clockula.alarm.ring.RingNotifications +import de.jeanlucmakiola.clockula.alarm.ring.UpcomingNotifications import de.jeanlucmakiola.clockula.domain.Alarm +import de.jeanlucmakiola.clockula.domain.alarm.UpcomingNotice import kotlin.time.Instant import javax.inject.Inject import javax.inject.Singleton /** - * The snoozed notification — the one surface the engine posts directly. A denied - * notification permission costs the user this line and nothing else: the snooze - * itself is a row in `alarm_states` and an AlarmManager registration. + * The snoozed and upcoming notifications — the surfaces the engine posts + * directly. A denied notification permission costs the user these lines and + * nothing else: the snooze itself is a row in `alarm_states` and an + * AlarmManager registration, and the upcoming Dismiss is also on the alarm row. */ @Singleton class AndroidAlarmNotifier @Inject constructor( @@ -47,4 +51,46 @@ class AndroidAlarmNotifier @Inject constructor( override fun clearSnoozed(alarmId: Long) { notifications.cancel(RingNotifications.snoozedNotificationId(alarmId)) } + + /** + * What this process last posted, per alarm. The engine syncs on every + * reschedule — each app start, each edit — and re-posting an unchanged + * notice would bring back one the user swiped away. Only the engine calls + * this, under its lock, so the map needs no lock of its own. + */ + private val postedUpcoming = mutableMapOf() + + override fun showUpcoming(notices: List) { + runCatching { + val wanted = notices.associateBy { it.alarm.id } + // What is on screen, including what an earlier process posted and + // this one has no memory of. + val onScreen = context.getSystemService(NotificationManager::class.java) + .activeNotifications + .filter { it.tag == UpcomingNotifications.TAG } + .map { it.id } + val wantedIds = wanted.keys.map(UpcomingNotifications::notificationId).toSet() + (onScreen + postedUpcoming.keys.map(UpcomingNotifications::notificationId)) + .filterNot { it in wantedIds } + .forEach { notifications.cancel(UpcomingNotifications.TAG, it) } + postedUpcoming.keys.retainAll(wanted.keys) + + // Denied, nothing is recorded, so a later grant posts on the next sync. + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU && + ContextCompat.checkSelfPermission(context, Manifest.permission.POST_NOTIFICATIONS) != + PackageManager.PERMISSION_GRANTED + ) { + return@runCatching + } + for (notice in notices) { + if (postedUpcoming[notice.alarm.id] == notice) continue + notifications.notify( + UpcomingNotifications.TAG, + UpcomingNotifications.notificationId(notice.alarm.id), + UpcomingNotifications.build(context, notice), + ) + postedUpcoming[notice.alarm.id] = notice + } + } + } } diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/android/AndroidAlarmScheduler.kt b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/android/AndroidAlarmScheduler.kt index 98ace1e..2beaabb 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/android/AndroidAlarmScheduler.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/android/AndroidAlarmScheduler.kt @@ -17,11 +17,13 @@ import javax.inject.Inject import javax.inject.Singleton /** - * Two AlarmManager slots, deliberately separate. The user's next alarm goes + * Three AlarmManager slots, deliberately separate. The user's next alarm goes * through `setAlarmClock`, which is the only variant that populates * `getNextAlarmClock()` — the status-bar icon and the lockscreen line. The * auto-silence backstop goes through its own PendingIntent, so an internal - * housekeeping time can never overwrite what the system shows the user. + * housekeeping time can never overwrite what the system shows the user. The + * upcoming-alarm check (#16) is the third, on its own PendingIntent for the + * same reason. * * Translation only: which mode to use is [SchedulingPolicy]'s decision. */ @@ -64,6 +66,27 @@ class AndroidAlarmScheduler @Inject constructor( override fun cancelAutoSilence() = manager.cancel(autoSilencePendingIntent(alarmId = 0L)) + /** + * Exact where allowed, so the notice is up when its window opens; inexact + * otherwise, since a late notice costs nothing an alarm needs. Never + * `setAlarmClock`, and wrapped whole: nothing about the notice may throw + * into the engine's reschedule (#16). + */ + override fun scheduleUpcomingCheck(at: Instant) { + val pendingIntent = upcomingCheckPendingIntent() + val millis = at.toEpochMilliseconds() + runCatching { + val exact = runCatching { + manager.setExactAndAllowWhileIdle(AlarmManager.RTC_WAKEUP, millis, pendingIntent) + }.isSuccess + if (!exact) manager.setAndAllowWhileIdle(AlarmManager.RTC_WAKEUP, millis, pendingIntent) + } + } + + override fun cancelUpcomingCheck() { + runCatching { manager.cancel(upcomingCheckPendingIntent()) } + } + override fun systemNextAlarm(): Instant? = manager.nextAlarmClock?.let { Instant.fromEpochMilliseconds(it.triggerTime) } @@ -94,6 +117,15 @@ class AndroidAlarmScheduler @Inject constructor( PENDING_INTENT_FLAGS, ) + private fun upcomingCheckPendingIntent(): PendingIntent = PendingIntent.getBroadcast( + context, + AlarmIntents.REQUEST_UPCOMING_CHECK, + Intent(context, AlarmFireReceiver::class.java).apply { + action = AlarmIntents.ACTION_UPCOMING_CHECK + }, + PENDING_INTENT_FLAGS, + ) + /** * What the system opens when the user taps the status-bar alarm icon or the * lockscreen's next-alarm line: the **alarms list**, which is what "show me diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/receiver/AlarmActionReceiver.kt b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/receiver/AlarmActionReceiver.kt index f7b9b99..687e239 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/receiver/AlarmActionReceiver.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/receiver/AlarmActionReceiver.kt @@ -8,9 +8,10 @@ import de.jeanlucmakiola.clockula.alarm.AlarmIntents import de.jeanlucmakiola.floret.di.ApplicationScope import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch +import kotlin.time.Instant import javax.inject.Inject -/** The notification's Snooze and Dismiss buttons. */ +/** The notifications' Snooze and Dismiss buttons, the upcoming notice's included. */ @AndroidEntryPoint class AlarmActionReceiver : HiltBroadcastReceiver() { @@ -32,6 +33,10 @@ class AlarmActionReceiver : HiltBroadcastReceiver() { when (action) { AlarmIntents.ACTION_SNOOZE -> engine.snooze(alarmId) AlarmIntents.ACTION_DISMISS -> engine.dismiss(alarmId) + AlarmIntents.ACTION_DISMISS_UPCOMING -> engine.dismissUpcomingOccurrence( + alarmId = alarmId, + fireAt = Instant.fromEpochMilliseconds(intent.getLongExtra(AlarmIntents.EXTRA_FIRE_AT, 0L)), + ) } } finally { pendingResult.finish() diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/receiver/AlarmFireReceiver.kt b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/receiver/AlarmFireReceiver.kt index 176d0c6..637e583 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/receiver/AlarmFireReceiver.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/receiver/AlarmFireReceiver.kt @@ -13,7 +13,7 @@ import kotlin.time.Instant import javax.inject.Inject /** - * The two AlarmManager slots arriving. Thin by design: every decision the engine + * The AlarmManager slots arriving: the fire, the backstop and the upcoming check. Thin by design: every decision the engine * could make is the engine's, so the receiver only unpacks and dispatches. */ @AndroidEntryPoint @@ -48,6 +48,8 @@ class AlarmFireReceiver : HiltBroadcastReceiver() { ) AlarmIntents.ACTION_AUTO_SILENCE -> engine.onAutoSilence(alarmId) + + AlarmIntents.ACTION_UPCOMING_CHECK -> engine.onUpcomingWindow() } } finally { pendingResult.finish() diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/UpcomingNotifications.kt b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/UpcomingNotifications.kt new file mode 100644 index 0000000..981a842 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/UpcomingNotifications.kt @@ -0,0 +1,116 @@ +package de.jeanlucmakiola.clockula.alarm.ring + +import android.app.Notification +import android.app.NotificationChannel +import android.app.NotificationManager +import android.app.PendingIntent +import android.content.Context +import android.content.Intent +import androidx.core.app.NotificationCompat +import de.jeanlucmakiola.clockula.MainActivity +import de.jeanlucmakiola.clockula.R +import de.jeanlucmakiola.clockula.alarm.AlarmIntents +import de.jeanlucmakiola.clockula.alarm.receiver.AlarmActionReceiver +import de.jeanlucmakiola.clockula.domain.alarm.UpcomingNotice +import java.time.Instant as JavaInstant +import java.time.ZoneId +import java.time.format.DateTimeFormatter +import java.time.format.FormatStyle + +/** + * "Upcoming alarm" with a Dismiss, posted while an alarm is inside its window + * (#16). Silent and low-importance: it is a way out for someone already awake, + * not a second alarm. Its Dismiss is the engine's `dismissUpcoming` for the one + * occurrence it names, so it can never end a ring that has already started. + */ +object UpcomingNotifications { + + const val CHANNEL_ID: String = "alarm_upcoming" + + /** Tagged, so the per-alarm ids cannot collide with any other notification's. */ + const val TAG: String = "alarm_upcoming" + + /** Idempotent; called from the app class on every start. */ + fun createChannel(context: Context) { + val channel = NotificationChannel( + CHANNEL_ID, + context.getString(R.string.channel_alarm_upcoming_name), + NotificationManager.IMPORTANCE_LOW, + ).apply { + description = context.getString(R.string.channel_alarm_upcoming_description) + } + context.getSystemService(NotificationManager::class.java).createNotificationChannel(channel) + } + + fun notificationId(alarmId: Long): Int = alarmId.toInt() + + fun build(context: Context, notice: UpcomingNotice): Notification { + val alarm = notice.alarm + val time = formatInstant(context, notice.fireAt) + val text = if (alarm.label.isBlank()) { + time + } else { + context.getString(R.string.alarm_upcoming_text_labelled, time, alarm.label) + } + return NotificationCompat.Builder(context, CHANNEL_ID) + .setSmallIcon(R.drawable.ic_notification) + .setContentTitle(context.getString(R.string.alarm_upcoming_title)) + .setContentText(text) + .setCategory(NotificationCompat.CATEGORY_REMINDER) + .setPriority(NotificationCompat.PRIORITY_LOW) + .setVisibility(NotificationCompat.VISIBILITY_PUBLIC) + .setOnlyAlertOnce(true) + .setSilent(true) + .setShowWhen(false) + // Several alarms in the window read in the order they ring. + .setSortKey(notice.fireAt.toEpochMilliseconds().toString().padStart(SORT_KEY_WIDTH, '0')) + .setContentIntent(showAlarmsPendingIntent(context)) + .addAction( + R.drawable.ic_notification, + context.getString(R.string.alarm_action_dismiss), + dismissPendingIntent(context, notice), + ) + .build() + } + + /** The alarms list, the same door the status-bar alarm icon opens. */ + private fun showAlarmsPendingIntent(context: Context): PendingIntent = PendingIntent.getActivity( + context, + AlarmIntents.REQUEST_SHOW_UPCOMING, + Intent(context, MainActivity::class.java).apply { + action = AlarmIntents.ACTION_SHOW_ALARMS + flags = Intent.FLAG_ACTIVITY_NEW_TASK + }, + PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE, + ) + + private fun dismissPendingIntent(context: Context, notice: UpcomingNotice): PendingIntent = + PendingIntent.getBroadcast( + context, + // Per alarm, so two notices' Dismiss buttons keep their own extras. + AlarmIntents.REQUEST_DISMISS_UPCOMING * REQUEST_CODE_STRIDE + + (notice.alarm.id % REQUEST_CODE_STRIDE).toInt(), + Intent(context, AlarmActionReceiver::class.java).apply { + action = AlarmIntents.ACTION_DISMISS_UPCOMING + putExtra(AlarmIntents.EXTRA_ALARM_ID, notice.alarm.id) + putExtra(AlarmIntents.EXTRA_FIRE_AT, notice.fireAt.toEpochMilliseconds()) + }, + PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE, + ) + + /** The device's 12/24-hour choice, then the locale's own short form. */ + private fun formatInstant(context: Context, instant: kotlin.time.Instant): String { + val time = JavaInstant.ofEpochMilli(instant.toEpochMilliseconds()).atZone(ZoneId.systemDefault()).toLocalTime() + val pattern = if (android.text.format.DateFormat.is24HourFormat(context)) { + DateTimeFormatter.ofPattern("HH:mm") + } else { + DateTimeFormatter.ofLocalizedTime(FormatStyle.SHORT) + } + return pattern.format(time) + } + + /** Matches `RingNotifications`: keeps a derived request code clear of the small ones. */ + private const val REQUEST_CODE_STRIDE = 100_000 + + private const val SORT_KEY_WIDTH = 20 +} diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingAlarm.kt b/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingAlarm.kt index 581ee36..b0c7e32 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingAlarm.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingAlarm.kt @@ -43,6 +43,12 @@ sealed interface DismissOutcome { /** Unknown id, or an alarm with no upcoming instance. Nothing was written. */ data object NotFound : DismissOutcome + + /** + * The occurrence a notice or row named is no longer the one to come: it + * started ringing, or the alarm moved. Nothing was dismissed (#16). + */ + data object Stale : DismissOutcome } sealed interface SnoozeOutcome { diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNotices.kt b/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNotices.kt new file mode 100644 index 0000000..1b366d6 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNotices.kt @@ -0,0 +1,60 @@ +package de.jeanlucmakiola.clockula.domain.alarm + +import de.jeanlucmakiola.clockula.domain.Alarm +import kotlin.time.Duration +import kotlin.time.Duration.Companion.hours +import kotlin.time.Instant + +/** One alarm about to ring, offered for dismissal before it does (#16). */ +data class UpcomingNotice( + val alarm: Alarm, + /** The occurrence the notice is about; a dismissal names it, so a stale tap cannot reach a later one. */ + val fireAt: Instant, +) + +/** + * What the upcoming-alarm surface should show now, and when it next has to + * look again. [nextCheckAt] is null when no alarm's window is still to open. + */ +data class UpcomingNoticePlan( + val notices: List, + val nextCheckAt: Instant?, +) + +/** + * The "upcoming alarm" window: the stretch before an alarm rings in which it + * can be dismissed without opening the app (#16). Pure; the engine and the + * alarm list both ask it, so the notification and the row's chip agree. + */ +object UpcomingNotices { + /** + * How long before the ring the notice appears. AOSP DeskClock posts its + * "Upcoming alarm" notification two hours ahead, which is also what Google + * Clock is reported to do. Long enough to cover waking early; the notice + * is silent, so being early costs nothing. A value, not a setting: the + * notification channel is the off switch. + */ + val LEAD: Duration = 2.hours + + /** + * Only a natural occurrence still to come. A snoozed alarm already has its + * own notification with a Dismiss, and a ringing one is the ring screen's, + * where the dismiss challenge is. + */ + fun isInWindow(nextFire: Instant?, source: FireSource, now: Instant, lead: Duration = LEAD): Boolean = + source == FireSource.OCCURRENCE && nextFire != null && nextFire > now && nextFire - now <= lead + + /** Notices ordered by when they ring, then id; the next check is the earliest window still to open. */ + fun plan(upcoming: List, now: Instant, lead: Duration = LEAD): UpcomingNoticePlan { + val notices = upcoming + .filter { isInWindow(it.nextFire, it.source, now, lead) } + .map { UpcomingNotice(it.alarm, it.nextFire!!) } + .sortedWith(compareBy({ it.fireAt }, { it.alarm.id })) + val nextCheckAt = upcoming + .filter { it.source == FireSource.OCCURRENCE } + .mapNotNull { entry -> entry.nextFire?.let { it - lead } } + .filter { it > now } + .minOrNull() + return UpcomingNoticePlan(notices, nextCheckAt) + } +} diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmRow.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmRow.kt index c47eb5a..c0cc71c 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmRow.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmRow.kt @@ -5,6 +5,12 @@ import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.size +import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.rounded.AlarmOff +import androidx.compose.material3.AssistChip +import androidx.compose.material3.AssistChipDefaults +import androidx.compose.material3.Icon import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Switch import androidx.compose.material3.Text @@ -48,6 +54,7 @@ internal fun AlarmRow( onClick: () -> Unit, onToggle: (Boolean) -> Unit, modifier: Modifier = Modifier, + onDismissComing: () -> Unit = {}, ) { val formatTime = rememberAlarmTimeFormatter() val time = formatTime(row.time) @@ -113,6 +120,25 @@ internal fun AlarmRow( style = MaterialTheme.typography.bodyMedium, color = supportingColor, ) + // Inside the upcoming notification's window, its Dismiss is here + // too, for whoever opens the app instead (#16). + if (row.enabled && row.dismissibleFireAt != null) { + val dismissDescription = stringResource(R.string.alarm_row_dismiss_upcoming, time) + AssistChip( + onClick = onDismissComing, + label = { Text(stringResource(R.string.alarm_action_dismiss)) }, + leadingIcon = { + Icon( + imageVector = Icons.Rounded.AlarmOff, + contentDescription = null, + modifier = Modifier.size(AssistChipDefaults.IconSize), + ) + }, + // "Dismiss the alarm at 07:00": the bare word names neither + // the row nor that it is the coming ring, not this one. + modifier = Modifier.semantics { contentDescription = dismissDescription }, + ) + } } } } diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmRowState.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmRowState.kt index bfa9310..28ff9cd 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmRowState.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmRowState.kt @@ -4,6 +4,7 @@ import de.jeanlucmakiola.clockula.domain.RepeatDays import de.jeanlucmakiola.clockula.domain.TimeOfDay import de.jeanlucmakiola.clockula.domain.alarm.FireSource import de.jeanlucmakiola.clockula.domain.alarm.UpcomingAlarm +import de.jeanlucmakiola.clockula.domain.alarm.UpcomingNotices import de.jeanlucmakiola.clockula.domain.format.NextFireFormat import de.jeanlucmakiola.clockula.domain.format.NextFireLabel import kotlin.time.Instant @@ -22,6 +23,11 @@ data class AlarmRowState( val skippingNext: Boolean, /** The earliest non-null next fire. Never a ringing alarm. */ val isNext: Boolean, + /** + * The occurrence the row's Dismiss chip would dismiss, while the alarm is + * inside the same window as the upcoming notification; null outside it (#16). + */ + val dismissibleFireAt: Instant? = null, ) sealed interface AlarmsUiState { @@ -58,6 +64,8 @@ internal object AlarmListRows { // only a repeating alarm can be "skipping next" (M5 D14). skippingNext = alarm.skipNextOccurrence && alarm.repeatDays.isRepeating, isNext = !ringing && next?.alarm?.id == alarm.id, + dismissibleFireAt = entry.nextFire + ?.takeIf { UpcomingNotices.isInWindow(it, entry.source, now) }, ) } } diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsScreen.kt b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsScreen.kt index bdb6c1d..ed3a195 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmsScreen.kt @@ -116,6 +116,9 @@ fun AlarmsScreen( } }, onToggle = { enabled -> viewModel.onToggleEnabled(row.id, enabled) }, + onDismissComing = { + row.dismissibleFireAt?.let { viewModel.onDismissComing(row.id, it) } + }, // An alarm whose time changed slides to its new place // instead of teleporting. modifier = animateItemMotion(), 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..3df75c9 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 @@ -18,6 +18,7 @@ import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.receiveAsFlow import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch +import kotlin.time.Instant import javax.inject.Inject /** The alarm list: what it shows, the "+" that creates a row, and the row's switch. */ @@ -76,6 +77,15 @@ class AlarmsViewModel @Inject internal constructor( viewModelScope.launch { engine.dismissUpcoming(alarmId) } } + /** + * The row's Dismiss chip (#16): the upcoming notification's own verb, so + * a chip tapped as the alarm starts ringing dismisses nothing rather than + * getting round the ring screen's challenge. + */ + fun onDismissComing(alarmId: Long, fireAt: Instant) { + viewModelScope.launch { engine.dismissUpcomingOccurrence(alarmId, fireAt) } + } + /** Dismisses a ring first when switching off (M5 D11), then re-resolves. */ fun onToggleEnabled(alarmId: Long, enabled: Boolean) { viewModelScope.launch { diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index ad0a66c..6ca0762 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -10,6 +10,10 @@ The full-screen alarm while it is ringing. Turning this off does not silence an alarm. Snoozed alarms A quiet reminder that an alarm is snoozed, and when it comes back. + Upcoming alarms + A silent note up to two hours before an alarm rings, so you can dismiss it if you are already up. Turning this off does not affect the alarm. + Upcoming alarm + %1$s · %2$s Alarm Snooze Dismiss @@ -102,6 +106,7 @@ Add alarm Tap + to set your first alarm Alarm at %1$s + Dismiss the alarm at %1$s in less than a minute in %1$d min diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt new file mode 100644 index 0000000..82e3bc8 --- /dev/null +++ b/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt @@ -0,0 +1,167 @@ +package de.jeanlucmakiola.clockula.alarm + +import com.google.common.truth.Truth.assertThat +import de.jeanlucmakiola.clockula.domain.RepeatDays +import de.jeanlucmakiola.clockula.domain.alarm.DismissOutcome +import de.jeanlucmakiola.clockula.domain.alarm.FireOutcome +import de.jeanlucmakiola.clockula.domain.alarm.UpcomingNotices +import de.jeanlucmakiola.clockula.testing.RingEvent +import de.jeanlucmakiola.clockula.testing.alarmEngineHarness +import de.jeanlucmakiola.clockula.testing.offsetInstant +import kotlinx.coroutines.test.runTest +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.io.TempDir +import java.nio.file.Path + +/** + * The upcoming-alarm notification (#16). The harness starts at 05:00 Berlin, so + * a 06:30 alarm is inside the two-hour window and an 08:00 one is not yet. + */ +class AlarmEngineUpcomingTest { + + private val sixThirty = offsetInstant("2026-06-10T06:30+02:00") + private val eight = offsetInstant("2026-06-10T08:00+02:00") + private val sixThirtyTomorrow = offsetInstant("2026-06-11T06:30+02:00") + + @Test + fun `a reschedule notices the alarm in its window and wakes for the next window`(@TempDir tempDir: Path) = + runTest { + val harness = alarmEngineHarness(tempDir) + harness.givenAlarm(id = 1L, hour = 6, minute = 30) + harness.givenAlarm(id = 2L, hour = 8) + + harness.engine.reschedule() + + assertThat(harness.notifier.upcoming.map { it.alarm.id to it.fireAt }).containsExactly(1L to sixThirty) + assertThat(harness.scheduler.upcomingCheck).isEqualTo(eight - UpcomingNotices.LEAD) + // The real slot is the earliest alarm, as it always was. + assertThat(harness.scheduler.next?.fireAt).isEqualTo(sixThirty) + } + + @Test + fun `the window opening posts the later alarm's notice too`(@TempDir tempDir: Path) = runTest { + val harness = alarmEngineHarness(tempDir) + harness.givenAlarm(id = 1L, hour = 6, minute = 30) + harness.givenAlarm(id = 2L, hour = 8) + harness.engine.reschedule() + + harness.wallClock.instant = eight - UpcomingNotices.LEAD + harness.engine.onUpcomingWindow() + + assertThat(harness.notifier.upcoming.map { it.alarm.id }).containsExactly(1L, 2L).inOrder() + assertThat(harness.scheduler.upcomingCheck).isNull() + } + + @Test + fun `dismissing a repeating alarm's notice skips today and leaves it on`(@TempDir tempDir: Path) = + runTest { + val harness = alarmEngineHarness(tempDir) + harness.givenAlarm(id = 1L, hour = 6, minute = 30, repeatDays = RepeatDays.EVERY_DAY) + harness.engine.reschedule() + + val outcome = harness.engine.dismissUpcomingOccurrence(1L, sixThirty) + + val alarm = harness.alarms.find(1L)!! + assertThat(outcome).isEqualTo(DismissOutcome.SkippedNext(1L)) + assertThat(alarm.enabled).isTrue() + assertThat(alarm.skipNextOccurrence).isTrue() + assertThat(harness.scheduler.next?.fireAt).isEqualTo(sixThirtyTomorrow) + assertThat(harness.notifier.upcoming).isEmpty() + assertThat(harness.scheduler.upcomingCheck).isEqualTo(sixThirtyTomorrow - UpcomingNotices.LEAD) + } + + @Test + fun `dismissing a one-shot alarm's notice switches it off`(@TempDir tempDir: Path) = runTest { + val harness = alarmEngineHarness(tempDir) + harness.givenAlarm(id = 1L, hour = 6, minute = 30) + harness.engine.reschedule() + + val outcome = harness.engine.dismissUpcomingOccurrence(1L, sixThirty) + + assertThat(outcome).isEqualTo(DismissOutcome.Disabled(1L)) + assertThat(harness.alarms.find(1L)!!.enabled).isFalse() + assertThat(harness.scheduler.next).isNull() + assertThat(harness.notifier.upcoming).isEmpty() + assertThat(harness.scheduler.upcomingCheck).isNull() + } + + @Test + fun `a Dismiss that lands once the alarm is ringing dismisses nothing`(@TempDir tempDir: Path) = + runTest { + val harness = alarmEngineHarness(tempDir) + harness.givenAlarm(id = 1L, hour = 6, minute = 30, repeatDays = RepeatDays.EVERY_DAY) + harness.engine.reschedule() + harness.givenRinging(alarmId = 1L, at = sixThirty) + + val outcome = harness.engine.dismissUpcomingOccurrence(1L, sixThirty) + + // The ring screen and its challenge own a ringing alarm. + assertThat(outcome).isEqualTo(DismissOutcome.Stale) + assertThat(harness.states.state(1L).ringingSince).isEqualTo(sixThirty) + assertThat(harness.ring.events.last()).isEqualTo(RingEvent.Started(1L)) + assertThat(harness.alarms.find(1L)!!.skipNextOccurrence).isFalse() + } + + @Test + fun `a Dismiss for an occurrence the alarm no longer has dismisses nothing`(@TempDir tempDir: Path) = + runTest { + val harness = alarmEngineHarness(tempDir) + harness.givenAlarm(id = 1L, hour = 6, minute = 30) + harness.engine.reschedule() + + val outcome = harness.engine.dismissUpcomingOccurrence(1L, eight) + + assertThat(outcome).isEqualTo(DismissOutcome.Stale) + assertThat(harness.alarms.find(1L)!!.enabled).isTrue() + assertThat(harness.notifier.upcoming.map { it.alarm.id }).containsExactly(1L) + } + + @Test + fun `an unknown alarm is not found`(@TempDir tempDir: Path) = runTest { + val harness = alarmEngineHarness(tempDir) + + assertThat(harness.engine.dismissUpcomingOccurrence(9L, sixThirty)).isEqualTo(DismissOutcome.NotFound) + } + + @Test + fun `the notice goes when the alarm fires, and a snooze does not bring it back`(@TempDir tempDir: Path) = + runTest { + val harness = alarmEngineHarness(tempDir) + harness.givenAlarm(id = 1L, hour = 6, minute = 30) + harness.engine.reschedule() + + harness.givenRinging(alarmId = 1L, at = sixThirty) + assertThat(harness.notifier.upcoming).isEmpty() + + harness.engine.snooze(1L) + assertThat(harness.notifier.upcoming).isEmpty() + } + + @Test + fun `switching the alarm off takes its notice away`(@TempDir tempDir: Path) = runTest { + val harness = alarmEngineHarness(tempDir) + harness.givenAlarm(id = 1L, hour = 6, minute = 30) + harness.engine.reschedule() + + harness.alarms.setEnabled(1L, false) + harness.engine.reschedule() + + assertThat(harness.notifier.upcoming).isEmpty() + assertThat(harness.scheduler.upcomingCheck).isNull() + } + + @Test + fun `a failing notification never costs the alarm its ring`(@TempDir tempDir: Path) = runTest { + val harness = alarmEngineHarness(tempDir) + harness.givenAlarm(id = 1L, hour = 6, minute = 30, repeatDays = RepeatDays.EVERY_DAY) + harness.notifier.failUpcoming = true + + harness.engine.reschedule() + harness.wallClock.instant = sixThirty + val outcome = harness.engine.onFire(1L, sixThirty) + + assertThat(outcome).isEqualTo(FireOutcome.Ringing(1L)) + assertThat(harness.ring.events).containsExactly(RingEvent.Started(1L)) + assertThat(harness.scheduler.next?.fireAt).isEqualTo(sixThirtyTomorrow) + } +} diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmIntentsTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmIntentsTest.kt index 95e67ff..d2e96de 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmIntentsTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmIntentsTest.kt @@ -15,6 +15,8 @@ class AlarmIntentsTest { AlarmIntents.ACTION_AUTO_SILENCE, AlarmIntents.ACTION_SNOOZE, AlarmIntents.ACTION_DISMISS, + AlarmIntents.ACTION_UPCOMING_CHECK, + AlarmIntents.ACTION_DISMISS_UPCOMING, ) @Test @@ -28,6 +30,9 @@ class AlarmIntentsTest { AlarmIntents.REQUEST_SHOW, AlarmIntents.REQUEST_SNOOZE, AlarmIntents.REQUEST_DISMISS, + AlarmIntents.REQUEST_UPCOMING_CHECK, + AlarmIntents.REQUEST_DISMISS_UPCOMING, + AlarmIntents.REQUEST_SHOW_UPCOMING, ) } diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNoticesTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNoticesTest.kt new file mode 100644 index 0000000..f13e755 --- /dev/null +++ b/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNoticesTest.kt @@ -0,0 +1,91 @@ +package de.jeanlucmakiola.clockula.domain.alarm + +import com.google.common.truth.Truth.assertThat +import de.jeanlucmakiola.clockula.testing.offsetInstant +import de.jeanlucmakiola.clockula.testing.upcomingAlarm +import org.junit.jupiter.api.Test +import kotlin.time.Duration.Companion.hours +import kotlin.time.Duration.Companion.milliseconds +import kotlin.time.Duration.Companion.minutes + +/** + * The upcoming-alarm window (#16): which alarms get a notice, and when the + * engine next has to look. The engine and the alarm row both read it, so this + * is where "the notification and the chip agree" is decided. + */ +class UpcomingNoticesTest { + + private val now = offsetInstant("2026-06-10T05:30+02:00") + + @Test + fun `the lead is two hours, as in AOSP's clock`() { + assertThat(UpcomingNotices.LEAD).isEqualTo(2.hours) + } + + @Test + fun `an occurrence inside the window gets a notice naming it`() { + val fireAt = now + 90.minutes + val plan = UpcomingNotices.plan(listOf(upcomingAlarm(id = 1L, nextFire = fireAt)), now) + + assertThat(plan.notices.map { it.alarm.id to it.fireAt }).containsExactly(1L to fireAt) + } + + @Test + fun `the window includes its opening instant and excludes the ring itself`() { + assertThat(UpcomingNotices.isInWindow(now + 2.hours, FireSource.OCCURRENCE, now)).isTrue() + assertThat(UpcomingNotices.isInWindow(now + 2.hours + 1.milliseconds, FireSource.OCCURRENCE, now)) + .isFalse() + assertThat(UpcomingNotices.isInWindow(now, FireSource.OCCURRENCE, now)).isFalse() + assertThat(UpcomingNotices.isInWindow(now - 1.minutes, FireSource.OCCURRENCE, now)).isFalse() + } + + @Test + fun `a snooze, a ring and a switched-off alarm get no notice`() { + val soon = now + 5.minutes + listOf(FireSource.SNOOZE, FireSource.RESUMED_RING, FireSource.NONE).forEach { source -> + assertThat(UpcomingNotices.isInWindow(soon, source, now)).isFalse() + } + assertThat(UpcomingNotices.isInWindow(null, FireSource.OCCURRENCE, now)).isFalse() + } + + @Test + fun `several alarms in the window are each noticed, in the order they ring`() { + val plan = UpcomingNotices.plan( + listOf( + upcomingAlarm(id = 3L, nextFire = now + 60.minutes), + upcomingAlarm(id = 1L, nextFire = now + 100.minutes), + upcomingAlarm(id = 2L, nextFire = now + 60.minutes), + ), + now, + ) + + assertThat(plan.notices.map { it.alarm.id }).containsExactly(2L, 3L, 1L).inOrder() + } + + @Test + fun `the next check is when the earliest window still to open opens`() { + val plan = UpcomingNotices.plan( + listOf( + // Already open: noticed now, needs no check. + upcomingAlarm(id = 1L, nextFire = now + 30.minutes), + upcomingAlarm(id = 2L, nextFire = now + 5.hours), + upcomingAlarm(id = 3L, nextFire = now + 3.hours), + // A snooze's own notification covers it; no window to wait for. + upcomingAlarm(id = 4L, nextFire = now + 150.minutes, source = FireSource.SNOOZE), + ), + now, + ) + + assertThat(plan.nextCheckAt).isEqualTo(now + 1.hours) + } + + @Test + fun `nothing to come means no notice and no check`() { + val plan = UpcomingNotices.plan( + listOf(upcomingAlarm(id = 1L, enabled = false, nextFire = null, source = FireSource.NONE)), + now, + ) + + assertThat(plan).isEqualTo(UpcomingNoticePlan(notices = emptyList(), nextCheckAt = null)) + } +} diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/testing/FakeAlarmSeams.kt b/app/src/test/java/de/jeanlucmakiola/clockula/testing/FakeAlarmSeams.kt index 591ec13..856aaa0 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/testing/FakeAlarmSeams.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/testing/FakeAlarmSeams.kt @@ -18,6 +18,7 @@ import de.jeanlucmakiola.clockula.domain.Timer import de.jeanlucmakiola.clockula.domain.TimerDraft import de.jeanlucmakiola.clockula.domain.alarm.CapabilitySnapshot import de.jeanlucmakiola.clockula.domain.alarm.ScheduleMode +import de.jeanlucmakiola.clockula.domain.alarm.UpcomingNotice import de.jeanlucmakiola.clockula.domain.selfcheck.DeviceState import de.jeanlucmakiola.clockula.domain.time.BootId import de.jeanlucmakiola.clockula.domain.time.BootIdProvider @@ -32,7 +33,7 @@ import java.time.ZoneId import kotlin.time.Duration import kotlin.time.Instant -/** One registration in one of the two AlarmManager slots. */ +/** One registration in one of the AlarmManager slots. */ data class Registration(val fireAt: Instant, val alarmId: Long) /** @@ -80,6 +81,18 @@ class FakeAlarmScheduler( autoSilence = null } + /** The upcoming-alarm check (#16), held as the instant it would wake at. */ + var upcomingCheck: Instant? = null + private set + + override fun scheduleUpcomingCheck(at: Instant) { + upcomingCheck = at + } + + override fun cancelUpcomingCheck() { + upcomingCheck = null + } + override fun systemNextAlarm(): Instant? = next?.fireAt var nextOwnerPackage: String? = null @@ -131,6 +144,18 @@ class FakeAlarmNotifier : AlarmNotifier { override fun clearSnoozed(alarmId: Long) { cleared += alarmId } + + /** The upcoming notices as the shade would show them now (#16). */ + var upcoming: List = emptyList() + private set + + /** Set to make the next sync throw, as a broken notification service might. */ + var failUpcoming: Boolean = false + + override fun showUpcoming(notices: List) { + check(!failUpcoming) { "notification service unavailable" } + upcoming = notices + } } class FakeAlarmCapabilities( diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmListRowsTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmListRowsTest.kt index eb3aaf0..f66e03b 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmListRowsTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/ui/alarms/AlarmListRowsTest.kt @@ -186,4 +186,21 @@ class AlarmListRowsTest { .containsExactly(NextFireLabel.InMinutes(30), NextFireLabel.InHoursMinutes(1, 30)) .inOrder() } + + @Test + fun `a row offers its Dismiss only inside the upcoming window`() { + val inWindow = now + 90.minutes + val upcoming = listOf( + upcomingAlarm(id = 1L, hour = 21, minute = 30, nextFire = inWindow), + upcomingAlarm(id = 2L, hour = 7, nextFire = sevenTomorrow), + upcomingAlarm(id = 3L, hour = 20, minute = 30, nextFire = now + 30.minutes, source = FireSource.SNOOZE), + ) + + val rows = AlarmListRows.from(upcoming, now).associateBy { it.id } + + assertThat(rows.getValue(1L).dismissibleFireAt).isEqualTo(inWindow) + assertThat(rows.getValue(2L).dismissibleFireAt).isNull() + // A snoozed alarm is dismissed from its own notification, not as "upcoming". + assertThat(rows.getValue(3L).dismissibleFireAt).isNull() + } } diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 97ef4e8..e9f8c17 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -111,7 +111,7 @@ kit module: the kit's roadmap keeps schedulers app-local. | `data/di/` | `DataModule`, `DatabaseModule`, `RepositoryModule`, `TimeModule`, `RingtoneModule`, `ZoneModule` (M8; one `@Binds`, `ZoneNames → IcuZoneNames`), `BackupModule` (M10) | | `data/backup/` | the backup's data-layer half (M10): `BackupDto`/`BackupCodec` (the pure JSON wire format, `ignoreUnknownKeys`, strict on known fields), `BackupDao` (one `@Transaction` replace-all over alarms/timers/world clocks; no schema or version change), `BackupRepository(+Impl)`, mapping through the existing `AlarmMapper`/`TimerMapper`/`WorldClockMapper`, never a second copy | | `alarm/` | `AlarmEngine` and the four seams it talks to (`AlarmScheduler`, `AlarmCapabilities`, `RingCoordinator`, `AlarmNotifier`), plus `AlarmIntents` | -| `alarm/android/` | the seams' Android implementations: AlarmManager, the capability reads, the service handle, the snoozed notification | +| `alarm/android/` | the seams' Android implementations: AlarmManager, the capability reads, the service handle, the snoozed and upcoming notifications | | `alarm/receiver/` | `AlarmFireReceiver`, `AlarmActionReceiver`, `SystemEventReceiver` | | `alarm/ring/` | the alarm's ringing foreground service and its notifications. Its audio player and vibrator moved to `ring/` in M6, shared with the timer's ring | | `alarm/di/` | `AlarmModule`: `@Binds` for the four seams | @@ -729,7 +729,7 @@ and a TalkBack-shaped accessibility click completing the hold challenge. throws, ring state for an alarm that is not there fails the foreign key, and an explicit id moves the row-id allocator past it as SQLite's would. - **Six fakes for the engine's seams.** `FakeAlarmScheduler` holds each of the - two AlarmManager slots as the value it currently holds, so a test asserts on + AlarmManager slots as the value it currently holds, so a test asserts on what the system would be showing rather than on a call log; `FakeRingCoordinator` records start/stop *in order*, because the order is the take-over contract; `FakeAlarmNotifier`, `FakeAlarmCapabilities`, @@ -964,7 +964,7 @@ declared because a specific path would otherwise fail, silently, at 07:00. | `SCHEDULE_EXACT_ALARM` `android:maxSdkVersion="32"` | covers 31–32, where it is pre-granted. Bounded at 32 so it never overlaps the one above; that is the documented pattern and it keeps the store listing's story clean. Below 31 an exact alarm needs no permission at all | | `RECEIVE_BOOT_COMPLETED` | AlarmManager keeps nothing across a reboot | | `USE_FULL_SCREEN_INTENT` | the ring screen over the lock screen | -| `POST_NOTIFICATIONS` | the ring and snoozed notifications | +| `POST_NOTIFICATIONS` | the ring, snoozed and upcoming-alarm notifications | | `FOREGROUND_SERVICE`, `FOREGROUND_SERVICE_SYSTEM_EXEMPTED` | the two ringing services: the alarm's, and (since M6) the timers' | | `WAKE_LOCK` | keep the CPU up for the length of a ring | | `VIBRATE` | an alarm and a timer both vibrate, and vibrate alone when no audio source opens | @@ -1172,7 +1172,8 @@ user later makes repeating is safe by construction (§17). The engine's verbs, for a reader looking for the one to call: `reschedule`, `onFire`, `onAutoSilence`, `snooze(id, minutesOverride = null)`, `dismiss`, -`dismissUpcoming`, `onBootCompleted`, `resumeRingingIfAny`, `ringSession` and +`dismissUpcoming`, `dismissUpcomingOccurrence` (#16), `onUpcomingWindow`, +`onBootCompleted`, `resumeRingingIfAny`, `ringSession` and the read-only `upcoming`. M9 added the last of the dismissals and the snooze's parameter: `dismissUpcoming(id)` is the `AlarmClock` contract's dismissal *and* the chooser dialog's, so the user's dismissal and an assistant's are the same @@ -1200,17 +1201,18 @@ back the moment the slot is re-resolved. unlimited, and "0 means infinite" is a trap. A refused snooze *dismisses*; it never leaves the user with a ringing alarm they cannot silence. -### Two AlarmManager slots, deliberately separate +### Three AlarmManager slots, deliberately separate | Slot | Registered with | Purpose | |---|---|---| | next alarm | `setAlarmClock(AlarmClockInfo(fireAt, showIntent), …)` | the user's next alarm. Doze-exempt, and the only variant that populates `getNextAlarmClock()` | | auto-silence backstop | `setExactAndAllowWhileIdle` | stops a ring nobody dismissed | +| upcoming check (#16) | `setExactAndAllowWhileIdle`, else `setAndAllowWhileIdle` | wakes when the next upcoming-alarm window opens; see below | They are separate because `getNextAlarmClock()` is what draws the status-bar icon and the lockscreen line: a backstop sharing that slot would overwrite the user's -07:00 with an internal 07:10. Two `PendingIntent`s, two request codes, two -independent cancels. A test asserts no two request codes collide, because +07:00 with an internal 07:10. One `PendingIntent`, one request code and one +independent cancel per slot. A test asserts no two request codes collide, because that collision is exactly how a backstop eats a user's alarm. A **snooze does** go through the next-alarm slot: a snoozed alarm *is* the next alarm. @@ -1218,6 +1220,40 @@ If exact alarms are unavailable the alarm is still registered, with `setAndAllowWhileIdle`. There is no third branch: late is survivable, silent is not. +### The upcoming-alarm notification (#16) + +For someone already awake, an alarm about to ring can be dismissed without +opening the app: a silent "Upcoming alarm" notification with a **Dismiss**, +posted while the alarm is inside its window, and the same Dismiss as a chip on +the alarm's row for whoever opens the app instead. + +- **The window** is `UpcomingNotices.LEAD` = **two hours** before a natural + occurrence, AOSP DeskClock's own lead for its upcoming notification and what + Google Clock is reported to use. The notice is silent and low-importance, so + being early costs nothing. It is a value, not a setting: the `alarm_upcoming` + channel is the off switch. Only `FireSource.OCCURRENCE` qualifies: a snoozed + alarm has its own notification with a Dismiss, and a ringing one belongs to + the ring screen. +- **Kept true in one place.** Every path that moves an alarm already ends in + `rescheduleLocked`, so that is where the notices are synced and the third + slot is armed for the earliest window still to open, *after* the real + registration. The sync is wrapped: `onFire` reschedules before it starts the + ring, and a notification failure must not cost the ring. The check's + receiver just calls `onUpcomingWindow()`, which is `reschedule()`. +- **Dismiss is `dismissUpcoming`, pinned to one occurrence.** + `dismissUpcomingOccurrence(id, fireAt)` re-resolves under the lock and acts + only if the alarm's next fire is still that natural occurrence; otherwise it + is `DismissOutcome.Stale` and writes nothing. Without the pin a tap landing + as the alarm starts would close the ring and get round its challenge. When it + matches, it takes `dismissUpcoming`'s own branches: a repeating alarm skips + that occurrence, a one-shot is switched off (or deleted, if transient). +- **No dismiss challenge before the ring**, as in Google Clock. The challenge + proves you are awake while an alarm sounds; before it, skipping is already one + unchallenged switch away in the editor. +- **Denied notifications** cost the notice and nothing else; the row's chip + still works. The notifier remembers what it posted in this process, so a + reschedule does not bring back a notice the user swiped away. + **The `showIntent` was wrong until M9.** It is what the system opens when the user taps the status-bar alarm icon or the lockscreen's next-alarm line, and it pointed at the ring screen with `FLAG_ACTIVITY_CLEAR_TASK`, so tapping "my next From 5b35ef65d4b464b83b001c0c54d478c03944357c Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Wed, 21 Oct 2026 20:09:47 +0200 Subject: [PATCH 02/10] fix(alarms): keep the upcoming Dismiss away from the fire A fire broadcast already in flight when a skip was armed on its occurrence is now ignored, and a Dismiss that lands once the occurrence is due is stale. The notices are synced after the ring starts on the fire path, and a swiped-away notice stays gone across a process restart. A switched-off alarm answers NotFound. Fixes #16 --- .../clockula/alarm/AlarmEngine.kt | 39 +++++++++---- .../alarm/android/AndroidAlarmNotifier.kt | 58 ++++++++++++------- .../clockula/alarm/ring/RingNotifications.kt | 3 +- .../alarm/ring/UpcomingNotifications.kt | 17 +----- .../clockula/alarm/AlarmEngineUpcomingTest.kt | 43 ++++++++++++++ docs/ARCHITECTURE.md | 26 +++++---- 6 files changed, 126 insertions(+), 60 deletions(-) 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 a81440f..044b2af 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt @@ -128,11 +128,14 @@ class AlarmEngine @Inject constructor( * already one switch away in the editor, unchallenged. */ suspend fun dismissUpcomingOccurrence(alarmId: Long, fireAt: Instant): DismissOutcome = lock.withLock { - val alarm = alarms.find(alarmId) ?: return@withLock DismissOutcome.NotFound + val alarm = alarms.find(alarmId)?.takeIf { it.enabled } ?: return@withLock DismissOutcome.NotFound + val now = wallClock.now() // Read-only: the resolver's state is discarded, and the reschedule below - // persists whatever it should. - val resolution = AlarmResolver.resolve(alarm, states.state(alarmId), wallClock.now(), zones.current()) - if (resolution.source != FireSource.OCCURRENCE || resolution.nextFire != fireAt) { + // persists whatever it should. `fireAt > now` because the resolver keeps + // a just-passed occurrence through the fire grace: one already due is + // the fire's, not the notice's. + val resolution = AlarmResolver.resolve(alarm, states.state(alarmId), now, zones.current()) + if (resolution.source != FireSource.OCCURRENCE || resolution.nextFire != fireAt || fireAt <= now) { // Whatever posted the stale notice is put right on the way out. rescheduleLocked() return@withLock DismissOutcome.Stale @@ -220,7 +223,12 @@ class AlarmEngine @Inject constructor( ) } - private suspend fun rescheduleLocked(): ScheduledFire? { + /** + * [deferUpcomingTo] collects what the upcoming notices would be synced from + * instead of syncing them, for a caller with something more urgent to do + * first (the fire path). + */ + private suspend fun rescheduleLocked(deferUpcomingTo: MutableList? = null): ScheduledFire? { val now = wallClock.now() val zone = zones.current() @@ -250,7 +258,7 @@ class AlarmEngine @Inject constructor( ScheduledFire(winner.first.id, fireAt, winner.second.source, mode) } // After the real registration, never before it. - syncUpcomingLocked(resolved, now) + if (deferUpcomingTo == null) syncUpcomingLocked(resolved, now) else deferUpcomingTo += resolved return scheduled } @@ -260,8 +268,8 @@ class AlarmEngine @Inject constructor( * snooze, a fire, a boot, a clock or zone change — already ends in * [rescheduleLocked], so this is the one place they are kept true. * - * Nothing here may cost an alarm: [onFireLocked] reschedules *before* it - * starts the ring, so a failure on the way is swallowed rather than thrown. + * Nothing here may cost an alarm, so a failure is swallowed rather than + * thrown into whichever entry point is rescheduling. */ private fun syncUpcomingLocked(resolved: List, now: Instant) { runCatching { @@ -297,9 +305,12 @@ class AlarmEngine @Inject constructor( // 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) AlarmRing.AUTO_SILENCE_AFTER 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) { + // A skip armed on this very occurrence after its fire was already on its + // way: the user dismissed it in advance (#16), so it must not ring. + val skipped = !isSnoozeFire && state.skippedOccurrence == scheduledFor + // Deleted, switched off, skipped, or a PendingIntent so stale the moment + // has gone: re-resolve the slot and stay quiet. + if (alarm == null || !alarm.enabled || skipped || now - scheduledFor > lateBy) { rescheduleLocked() return FireOutcome.Ignored } @@ -323,11 +334,15 @@ class AlarmEngine @Inject constructor( // it holds this alarm's *next* occurrence rather than nothing at all. states.save(opened) notifier.clearSnoozed(alarmId) - rescheduleLocked() + // The upcoming notices wait until the ring has started: their binder + // calls must not stand between the fire and the sound. + val upcoming = mutableListOf() + rescheduleLocked(deferUpcomingTo = upcoming) states.save(states.state(alarmId).copy(ringingSince = now)) ring.startRinging(alarmId) scheduler.scheduleAutoSilence(now + AlarmRing.AUTO_SILENCE_AFTER, alarmId) + syncUpcomingLocked(upcoming, now) return FireOutcome.Ringing(alarmId) } diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/android/AndroidAlarmNotifier.kt b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/android/AndroidAlarmNotifier.kt index 113440d..865aee9 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/android/AndroidAlarmNotifier.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/android/AndroidAlarmNotifier.kt @@ -53,44 +53,60 @@ class AndroidAlarmNotifier @Inject constructor( } /** - * What this process last posted, per alarm. The engine syncs on every - * reschedule — each app start, each edit — and re-posting an unchanged - * notice would bring back one the user swiped away. Only the engine calls - * this, under its lock, so the map needs no lock of its own. + * The notices already posted for the occurrences still wanted, as + * `id@fireAt` keys. Persisted, because every process start reschedules: a + * notice the user swiped away must stay gone across a restart, and only a + * notice this code cancelled itself (the alarm left its window) may come + * back. The engine calls this under its lock, so it needs none of its own. */ - private val postedUpcoming = mutableMapOf() + private val record by lazy { context.getSharedPreferences(RECORD_FILE, Context.MODE_PRIVATE) } + + /** What this process last posted, per alarm, so an unchanged notice is not re-sent. */ + private val lastPosted = mutableMapOf() override fun showUpcoming(notices: List) { runCatching { - val wanted = notices.associateBy { it.alarm.id } - // What is on screen, including what an earlier process posted and - // this one has no memory of. + val wantedIds = notices.map { UpcomingNotifications.notificationId(it.alarm.id) }.toSet() + // What is on screen, including what an earlier process posted. val onScreen = context.getSystemService(NotificationManager::class.java) .activeNotifications .filter { it.tag == UpcomingNotifications.TAG } .map { it.id } - val wantedIds = wanted.keys.map(UpcomingNotifications::notificationId).toSet() - (onScreen + postedUpcoming.keys.map(UpcomingNotifications::notificationId)) - .filterNot { it in wantedIds } - .forEach { notifications.cancel(UpcomingNotifications.TAG, it) } - postedUpcoming.keys.retainAll(wanted.keys) + .toSet() + onScreen.filterNot { it in wantedIds }.forEach { notifications.cancel(UpcomingNotifications.TAG, it) } + lastPosted.keys.retainAll(notices.map { it.alarm.id }.toSet()) - // Denied, nothing is recorded, so a later grant posts on the next sync. + val wantedKeys = notices.map(::recordKey).toSet() + val posted = record.getStringSet(RECORD_KEY, emptySet()).orEmpty().intersect(wantedKeys).toMutableSet() + + // Denied, nothing new is recorded, so a later grant posts on the next sync. if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU && ContextCompat.checkSelfPermission(context, Manifest.permission.POST_NOTIFICATIONS) != PackageManager.PERMISSION_GRANTED ) { + record.edit().putStringSet(RECORD_KEY, posted).apply() return@runCatching } for (notice in notices) { - if (postedUpcoming[notice.alarm.id] == notice) continue - notifications.notify( - UpcomingNotifications.TAG, - UpcomingNotifications.notificationId(notice.alarm.id), - UpcomingNotifications.build(context, notice), - ) - postedUpcoming[notice.alarm.id] = notice + val id = UpcomingNotifications.notificationId(notice.alarm.id) + val shown = id in onScreen + // Posted before and no longer on screen: the user swiped it away. + if (!shown && recordKey(notice) in posted) continue + // On screen and unchanged: nothing to send. + if (shown && lastPosted[notice.alarm.id] == notice) continue + notifications.notify(UpcomingNotifications.TAG, id, UpcomingNotifications.build(context, notice)) + lastPosted[notice.alarm.id] = notice + posted += recordKey(notice) } + record.edit().putStringSet(RECORD_KEY, posted).apply() } } + + private fun recordKey(notice: UpcomingNotice): String = + "${notice.alarm.id}@${notice.fireAt.toEpochMilliseconds()}" + + private companion object { + const val RECORD_FILE = "alarm_upcoming_notices" + const val RECORD_KEY = "posted" + } } diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/RingNotifications.kt b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/RingNotifications.kt index dbe1bc9..17a22d1 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/RingNotifications.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/RingNotifications.kt @@ -216,7 +216,8 @@ object RingNotifications { private fun formatTime(context: Context, hour: Int, minute: Int): String = format(context, LocalTime.of(hour, minute)) - private fun formatInstant(context: Context, instant: Instant): String = format( + /** Also the upcoming notice's (#16): one formatter for every alarm notification. */ + internal fun formatInstant(context: Context, instant: Instant): String = format( context, JavaInstant.ofEpochMilli(instant.toEpochMilliseconds()).atZone(ZoneId.systemDefault()).toLocalTime(), ) diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/UpcomingNotifications.kt b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/UpcomingNotifications.kt index 981a842..ae3b027 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/UpcomingNotifications.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/UpcomingNotifications.kt @@ -12,10 +12,6 @@ import de.jeanlucmakiola.clockula.R import de.jeanlucmakiola.clockula.alarm.AlarmIntents import de.jeanlucmakiola.clockula.alarm.receiver.AlarmActionReceiver import de.jeanlucmakiola.clockula.domain.alarm.UpcomingNotice -import java.time.Instant as JavaInstant -import java.time.ZoneId -import java.time.format.DateTimeFormatter -import java.time.format.FormatStyle /** * "Upcoming alarm" with a Dismiss, posted while an alarm is inside its window @@ -46,7 +42,7 @@ object UpcomingNotifications { fun build(context: Context, notice: UpcomingNotice): Notification { val alarm = notice.alarm - val time = formatInstant(context, notice.fireAt) + val time = RingNotifications.formatInstant(context, notice.fireAt) val text = if (alarm.label.isBlank()) { time } else { @@ -98,17 +94,6 @@ object UpcomingNotifications { PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE, ) - /** The device's 12/24-hour choice, then the locale's own short form. */ - private fun formatInstant(context: Context, instant: kotlin.time.Instant): String { - val time = JavaInstant.ofEpochMilli(instant.toEpochMilliseconds()).atZone(ZoneId.systemDefault()).toLocalTime() - val pattern = if (android.text.format.DateFormat.is24HourFormat(context)) { - DateTimeFormatter.ofPattern("HH:mm") - } else { - DateTimeFormatter.ofLocalizedTime(FormatStyle.SHORT) - } - return pattern.format(time) - } - /** Matches `RingNotifications`: keeps a derived request code clear of the small ones. */ private const val REQUEST_CODE_STRIDE = 100_000 diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt index 82e3bc8..718a488 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt @@ -12,6 +12,7 @@ import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.Test import org.junit.jupiter.api.io.TempDir import java.nio.file.Path +import kotlin.time.Duration.Companion.minutes /** * The upcoming-alarm notification (#16). The harness starts at 05:00 Berlin, so @@ -164,4 +165,46 @@ class AlarmEngineUpcomingTest { assertThat(harness.ring.events).containsExactly(RingEvent.Started(1L)) assertThat(harness.scheduler.next?.fireAt).isEqualTo(sixThirtyTomorrow) } + + @Test + fun `a fire already on its way when the notice was dismissed does not ring`(@TempDir tempDir: Path) = + runTest { + val harness = alarmEngineHarness(tempDir) + harness.givenAlarm(id = 1L, hour = 6, minute = 30, repeatDays = RepeatDays.EVERY_DAY) + harness.engine.reschedule() + harness.engine.dismissUpcomingOccurrence(1L, sixThirty) + + // The broadcast for 06:30 was sent before the skip was armed. + harness.wallClock.instant = sixThirty + val outcome = harness.engine.onFire(1L, sixThirty) + + assertThat(outcome).isEqualTo(FireOutcome.Ignored) + assertThat(harness.ring.events).isEmpty() + assertThat(harness.scheduler.next?.fireAt).isEqualTo(sixThirtyTomorrow) + // The skip is spent on the one occurrence, not on tomorrow's. + assertThat(harness.alarms.find(1L)!!.skipNextOccurrence).isFalse() + } + + @Test + fun `a Dismiss once the occurrence is due belongs to the fire, not the notice`(@TempDir tempDir: Path) = + runTest { + val harness = alarmEngineHarness(tempDir) + harness.givenAlarm(id = 1L, hour = 6, minute = 30, repeatDays = RepeatDays.EVERY_DAY) + harness.engine.reschedule() + + // Inside the fire grace, with the fire itself running late. + harness.wallClock.instant = sixThirty + 1.minutes + val outcome = harness.engine.dismissUpcomingOccurrence(1L, sixThirty) + + assertThat(outcome).isEqualTo(DismissOutcome.Stale) + assertThat(harness.alarms.find(1L)!!.skipNextOccurrence).isFalse() + } + + @Test + fun `a switched-off alarm has nothing to dismiss`(@TempDir tempDir: Path) = runTest { + val harness = alarmEngineHarness(tempDir) + harness.givenAlarm(id = 1L, hour = 6, minute = 30, enabled = false) + + assertThat(harness.engine.dismissUpcomingOccurrence(1L, sixThirty)).isEqualTo(DismissOutcome.NotFound) + } } diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index e9f8c17..8fa4e3f 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1237,22 +1237,28 @@ the alarm's row for whoever opens the app instead. - **Kept true in one place.** Every path that moves an alarm already ends in `rescheduleLocked`, so that is where the notices are synced and the third slot is armed for the earliest window still to open, *after* the real - registration. The sync is wrapped: `onFire` reschedules before it starts the - ring, and a notification failure must not cost the ring. The check's - receiver just calls `onUpcomingWindow()`, which is `reschedule()`. + registration. The sync is wrapped so a notification failure costs nothing, + and on the fire path it is deferred until the ring has started, so its binder + calls never stand between the fire and the sound. The check's receiver just + calls `onUpcomingWindow()`, which is `reschedule()`. - **Dismiss is `dismissUpcoming`, pinned to one occurrence.** `dismissUpcomingOccurrence(id, fireAt)` re-resolves under the lock and acts - only if the alarm's next fire is still that natural occurrence; otherwise it - is `DismissOutcome.Stale` and writes nothing. Without the pin a tap landing - as the alarm starts would close the ring and get round its challenge. When it - matches, it takes `dismissUpcoming`'s own branches: a repeating alarm skips - that occurrence, a one-shot is switched off (or deleted, if transient). + only if the alarm's next fire is still that natural occurrence *and still in + the future*; otherwise it is `DismissOutcome.Stale` and dismisses nothing. + Without the pin a tap landing as the alarm starts would close the ring and get + round its challenge. When it matches, it takes `dismissUpcoming`'s own + branches: a repeating alarm skips that occurrence, a one-shot is switched off + (or deleted, if transient). A fire broadcast already in flight when a skip + was armed is `Ignored` by `onFire`, which checks the skip watermark against + the instant it carries. - **No dismiss challenge before the ring**, as in Google Clock. The challenge proves you are awake while an alarm sounds; before it, skipping is already one unchallenged switch away in the editor. - **Denied notifications** cost the notice and nothing else; the row's chip - still works. The notifier remembers what it posted in this process, so a - reschedule does not bring back a notice the user swiped away. + still works. The notifier keeps a small SharedPreferences record of the + `id@fireAt` notices it has posted, so a reschedule, even in a new process, + does not bring back a notice the user swiped away; one the app cancelled + itself (the alarm left its window) is dropped from the record and may return. **The `showIntent` was wrong until M9.** It is what the system opens when the user taps the status-bar alarm icon or the lockscreen's next-alarm line, and it From dedcbd5dfdac34f6bfae51b44239dba16d92118b Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Wed, 21 Oct 2026 20:18:05 +0200 Subject: [PATCH 03/10] fix(alarms): a consumed skip stays handled, and only a swipe hides a notice A skip spent on its occurrence now becomes the handled-occurrence watermark, so a second resolution inside the fire grace cannot ring the skipped alarm after all. The upcoming notice records a swipe through its delete intent rather than inferring one, so a reboot or a blocked channel no longer hides it for good; its Dismiss asks for an unlock, and a Dismiss for a switched-off alarm clears its notice. Fixes #16 --- .../clockula/alarm/AlarmEngine.kt | 6 +- .../clockula/alarm/AlarmIntents.kt | 5 ++ .../alarm/android/AndroidAlarmNotifier.kt | 39 ++++--------- .../alarm/receiver/AlarmActionReceiver.kt | 6 ++ .../alarm/ring/UpcomingNotifications.kt | 57 +++++++++++++++++-- .../clockula/domain/alarm/AlarmResolver.kt | 12 +++- .../clockula/alarm/AlarmIntentsTest.kt | 2 + .../domain/alarm/AlarmResolverTest.kt | 17 ++++++ docs/ARCHITECTURE.md | 20 ++++--- 9 files changed, 122 insertions(+), 42 deletions(-) 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 044b2af..5e3a23c 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt @@ -128,7 +128,11 @@ class AlarmEngine @Inject constructor( * already one switch away in the editor, unchallenged. */ suspend fun dismissUpcomingOccurrence(alarmId: Long, fireAt: Instant): DismissOutcome = lock.withLock { - val alarm = alarms.find(alarmId)?.takeIf { it.enabled } ?: return@withLock DismissOutcome.NotFound + val alarm = alarms.find(alarmId)?.takeIf { it.enabled } ?: run { + // Its notice, if one is still up, is put right on the way out. + rescheduleLocked() + return@withLock DismissOutcome.NotFound + } val now = wallClock.now() // Read-only: the resolver's state is discarded, and the reschedule below // persists whatever it should. `fireAt > now` because the resolver keeps diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmIntents.kt b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmIntents.kt index 5b79f4c..7f9f630 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmIntents.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmIntents.kt @@ -20,6 +20,9 @@ object AlarmIntents { /** The upcoming notification's Dismiss, which carries [EXTRA_FIRE_AT] (#16). */ const val ACTION_DISMISS_UPCOMING: String = PREFIX + "action.ALARM_DISMISS_UPCOMING" + /** The user swiped the upcoming notification away; it carries [EXTRA_FIRE_AT] (#16). */ + const val ACTION_UPCOMING_SWIPED: String = PREFIX + "action.ALARM_UPCOMING_SWIPED" + /** Internal: "show me the alarms tab", the transport for `SHOW_ALARMS` and the next-alarm show intent. */ const val ACTION_SHOW_ALARMS: String = PREFIX + "action.SHOW_ALARMS" @@ -35,6 +38,7 @@ object AlarmIntents { const val REQUEST_UPCOMING_CHECK: Int = 7 const val REQUEST_DISMISS_UPCOMING: Int = 8 const val REQUEST_SHOW_UPCOMING: Int = 9 + const val REQUEST_UPCOMING_SWIPED: Int = 10 val ALL_REQUEST_CODES: List = listOf( REQUEST_FIRE, @@ -46,5 +50,6 @@ object AlarmIntents { REQUEST_UPCOMING_CHECK, REQUEST_DISMISS_UPCOMING, REQUEST_SHOW_UPCOMING, + REQUEST_UPCOMING_SWIPED, ) } diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/android/AndroidAlarmNotifier.kt b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/android/AndroidAlarmNotifier.kt index 865aee9..3c0ace1 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/android/AndroidAlarmNotifier.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/android/AndroidAlarmNotifier.kt @@ -53,15 +53,10 @@ class AndroidAlarmNotifier @Inject constructor( } /** - * The notices already posted for the occurrences still wanted, as - * `id@fireAt` keys. Persisted, because every process start reschedules: a - * notice the user swiped away must stay gone across a restart, and only a - * notice this code cancelled itself (the alarm left its window) may come - * back. The engine calls this under its lock, so it needs none of its own. + * What this process last posted, per alarm, so an unchanged notice on + * screen is not re-sent. The engine calls [showUpcoming] under its lock, so + * the map needs no lock of its own. */ - private val record by lazy { context.getSharedPreferences(RECORD_FILE, Context.MODE_PRIVATE) } - - /** What this process last posted, per alarm, so an unchanged notice is not re-sent. */ private val lastPosted = mutableMapOf() override fun showUpcoming(notices: List) { @@ -76,37 +71,27 @@ class AndroidAlarmNotifier @Inject constructor( onScreen.filterNot { it in wantedIds }.forEach { notifications.cancel(UpcomingNotifications.TAG, it) } lastPosted.keys.retainAll(notices.map { it.alarm.id }.toSet()) - val wantedKeys = notices.map(::recordKey).toSet() - val posted = record.getStringSet(RECORD_KEY, emptySet()).orEmpty().intersect(wantedKeys).toMutableSet() + // A swipe is the user's word for that occurrence; the record keeps + // only the occurrences still noticed. + val swiped = UpcomingNotifications.swipedKeys(context) + val stillSwiped = swiped.intersect(notices.map(UpcomingNotifications::key).toSet()) + if (stillSwiped != swiped) UpcomingNotifications.retainSwiped(context, stillSwiped) - // Denied, nothing new is recorded, so a later grant posts on the next sync. + // POST_NOTIFICATIONS is a runtime permission from API 33. Denied, the + // user loses the notice and keeps the alarm and the row's Dismiss. if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU && ContextCompat.checkSelfPermission(context, Manifest.permission.POST_NOTIFICATIONS) != PackageManager.PERMISSION_GRANTED ) { - record.edit().putStringSet(RECORD_KEY, posted).apply() return@runCatching } for (notice in notices) { + if (UpcomingNotifications.key(notice) in stillSwiped) continue val id = UpcomingNotifications.notificationId(notice.alarm.id) - val shown = id in onScreen - // Posted before and no longer on screen: the user swiped it away. - if (!shown && recordKey(notice) in posted) continue - // On screen and unchanged: nothing to send. - if (shown && lastPosted[notice.alarm.id] == notice) continue + if (id in onScreen && lastPosted[notice.alarm.id] == notice) continue notifications.notify(UpcomingNotifications.TAG, id, UpcomingNotifications.build(context, notice)) lastPosted[notice.alarm.id] = notice - posted += recordKey(notice) } - record.edit().putStringSet(RECORD_KEY, posted).apply() } } - - private fun recordKey(notice: UpcomingNotice): String = - "${notice.alarm.id}@${notice.fireAt.toEpochMilliseconds()}" - - private companion object { - const val RECORD_FILE = "alarm_upcoming_notices" - const val RECORD_KEY = "posted" - } } diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/receiver/AlarmActionReceiver.kt b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/receiver/AlarmActionReceiver.kt index 687e239..e9101a9 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/receiver/AlarmActionReceiver.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/receiver/AlarmActionReceiver.kt @@ -5,6 +5,7 @@ import android.content.Intent import dagger.hilt.android.AndroidEntryPoint import de.jeanlucmakiola.clockula.alarm.AlarmEngine import de.jeanlucmakiola.clockula.alarm.AlarmIntents +import de.jeanlucmakiola.clockula.alarm.ring.UpcomingNotifications import de.jeanlucmakiola.floret.di.ApplicationScope import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch @@ -27,6 +28,11 @@ class AlarmActionReceiver : HiltBroadcastReceiver() { val alarmId = intent.getLongExtra(AlarmIntents.EXTRA_ALARM_ID, 0L) val action = intent.action ?: return + if (action == AlarmIntents.ACTION_UPCOMING_SWIPED) { + // Bookkeeping only: the alarm itself is untouched. + UpcomingNotifications.recordSwiped(context, alarmId, intent.getLongExtra(AlarmIntents.EXTRA_FIRE_AT, 0L)) + return + } val pendingResult = goAsync() scope.launch { try { diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/UpcomingNotifications.kt b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/UpcomingNotifications.kt index ae3b027..c7d431e 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/UpcomingNotifications.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/UpcomingNotifications.kt @@ -16,8 +16,10 @@ import de.jeanlucmakiola.clockula.domain.alarm.UpcomingNotice /** * "Upcoming alarm" with a Dismiss, posted while an alarm is inside its window * (#16). Silent and low-importance: it is a way out for someone already awake, - * not a second alarm. Its Dismiss is the engine's `dismissUpcoming` for the one - * occurrence it names, so it can never end a ring that has already started. + * not a second alarm. Its Dismiss is the engine's `dismissUpcomingOccurrence` + * for the one occurrence it names, so it can never end a ring that has already + * started; it asks for an unlock first, so a locked phone cannot be talked out + * of its alarm by whoever picks it up. */ object UpcomingNotifications { @@ -61,14 +63,43 @@ object UpcomingNotifications { // Several alarms in the window read in the order they ring. .setSortKey(notice.fireAt.toEpochMilliseconds().toString().padStart(SORT_KEY_WIDTH, '0')) .setContentIntent(showAlarmsPendingIntent(context)) + .setDeleteIntent(swipedPendingIntent(context, notice)) .addAction( - R.drawable.ic_notification, - context.getString(R.string.alarm_action_dismiss), - dismissPendingIntent(context, notice), + NotificationCompat.Action.Builder( + R.drawable.ic_notification, + context.getString(R.string.alarm_action_dismiss), + dismissPendingIntent(context, notice), + ).setAuthenticationRequired(true).build(), ) .build() } + /** + * The `id@fireAt` notices the user swiped away. Recorded only from the + * delete intent, which fires for a swipe or "clear all" and never for the + * app's own cancel, a reboot or a blocked channel — so a reschedule, even in + * a new process, never brings back a notice the user got rid of, and a + * notice the system took away does come back. + */ + fun swipedKeys(context: Context): Set = + prefs(context).getStringSet(SWIPED_KEY, emptySet()).orEmpty() + + fun recordSwiped(context: Context, alarmId: Long, fireAtMillis: Long) { + val keys = swipedKeys(context) + key(alarmId, fireAtMillis) + prefs(context).edit().putStringSet(SWIPED_KEY, keys).apply() + } + + /** Drops the keys for occurrences no longer noticed, so the record stays small. */ + fun retainSwiped(context: Context, keys: Set) { + prefs(context).edit().putStringSet(SWIPED_KEY, keys).apply() + } + + fun key(notice: UpcomingNotice): String = key(notice.alarm.id, notice.fireAt.toEpochMilliseconds()) + + private fun key(alarmId: Long, fireAtMillis: Long): String = "$alarmId@$fireAtMillis" + + private fun prefs(context: Context) = context.getSharedPreferences(RECORD_FILE, Context.MODE_PRIVATE) + /** The alarms list, the same door the status-bar alarm icon opens. */ private fun showAlarmsPendingIntent(context: Context): PendingIntent = PendingIntent.getActivity( context, @@ -94,6 +125,22 @@ object UpcomingNotifications { PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE, ) + private fun swipedPendingIntent(context: Context, notice: UpcomingNotice): PendingIntent = + PendingIntent.getBroadcast( + context, + AlarmIntents.REQUEST_UPCOMING_SWIPED * REQUEST_CODE_STRIDE + + (notice.alarm.id % REQUEST_CODE_STRIDE).toInt(), + Intent(context, AlarmActionReceiver::class.java).apply { + action = AlarmIntents.ACTION_UPCOMING_SWIPED + putExtra(AlarmIntents.EXTRA_ALARM_ID, notice.alarm.id) + putExtra(AlarmIntents.EXTRA_FIRE_AT, notice.fireAt.toEpochMilliseconds()) + }, + PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE, + ) + + private const val RECORD_FILE = "alarm_upcoming_notices" + private const val SWIPED_KEY = "swiped" + /** Matches `RingNotifications`: keeps a derived request code clear of the small ones. */ private const val REQUEST_CODE_STRIDE = 100_000 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 ba6d548..4adf88b 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 @@ -120,7 +120,17 @@ object AlarmResolver { // cannot ring on its way out through the grace window. nextFire = candidates(alarm, working, now, zone, fireGrace).first { it > watermark }, source = FireSource.OCCURRENCE, - state = working.copy(skippedOccurrence = watermark.takeUnless { consumed }), + // A consumed skip becomes the handled-occurrence watermark: with both + // the flag and the skip watermark gone, nothing else would stop the + // next resolution inside the fire grace from ringing it after all. + state = if (consumed) { + working.copy( + skippedOccurrence = null, + handledOccurrence = maxOf(watermark, working.handledOccurrence ?: watermark), + ) + } else { + working.copy(skippedOccurrence = watermark) + }, clearSkipFlag = consumed, ) } diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmIntentsTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmIntentsTest.kt index d2e96de..26b23c9 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmIntentsTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmIntentsTest.kt @@ -17,6 +17,7 @@ class AlarmIntentsTest { AlarmIntents.ACTION_DISMISS, AlarmIntents.ACTION_UPCOMING_CHECK, AlarmIntents.ACTION_DISMISS_UPCOMING, + AlarmIntents.ACTION_UPCOMING_SWIPED, ) @Test @@ -33,6 +34,7 @@ class AlarmIntentsTest { AlarmIntents.REQUEST_UPCOMING_CHECK, AlarmIntents.REQUEST_DISMISS_UPCOMING, AlarmIntents.REQUEST_SHOW_UPCOMING, + AlarmIntents.REQUEST_UPCOMING_SWIPED, ) } 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 960cb2a..6d89e0e 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 @@ -257,6 +257,23 @@ class AlarmResolverTest { assertThat(resolution.nextFire).isEqualTo(offsetInstant("2026-06-11T07:00+02:00")) } + @Test + fun `a consumed skip cannot ring on the next pass through the fire grace`() { + val alarm = alarmAt(hour = 7, repeatDays = RepeatDays.EVERY_DAY, skipNextOccurrence = true) + val watermark = offsetInstant("2026-06-10T07:00+02:00") + val first = resolve(alarm, ringState(skippedOccurrence = watermark), now = watermark + 30.seconds) + + // The engine persists the first pass: the flag cleared, the state saved. + val second = resolve( + alarm.copy(skipNextOccurrence = false), + first.state, + now = watermark + 40.seconds, + ) + + assertThat(first.state.handledOccurrence).isEqualTo(watermark) + assertThat(second.nextFire).isEqualTo(offsetInstant("2026-06-11T07:00+02:00")) + } + @Test fun `skipping a one-shot alarm disables it`() { val alarm = alarmAt(hour = 7, skipNextOccurrence = true) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 8fa4e3f..7ab3848 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -250,9 +250,10 @@ so the skip eats a second alarm. With the watermark: one after; - watermark still in the future ⇒ fire the first candidate after it; write nothing; -- watermark at or before now ⇒ consumed: clear the flag and the watermark, and - *still* return the first candidate after it, so the skipped occurrence cannot - ring on its way out through the grace window. +- watermark at or before now ⇒ consumed: clear the flag and the watermark, + record the skipped occurrence as the handled one, and *still* return the + first candidate after it, so the skipped occurrence cannot ring on its way + out through the grace window, on this pass or the next (#16). Arming is deferred while a snooze is pending, and a **non-repeating** alarm with the flag set is *disabled* rather than skipped: a one-shot has exactly one @@ -1253,12 +1254,15 @@ the alarm's row for whoever opens the app instead. the instant it carries. - **No dismiss challenge before the ring**, as in Google Clock. The challenge proves you are awake while an alarm sounds; before it, skipping is already one - unchallenged switch away in the editor. + unchallenged switch away in the editor. The notification's Dismiss does ask + for an **unlock** (`setAuthenticationRequired`), so a locked phone cannot be + talked out of its alarm by whoever picks it up. - **Denied notifications** cost the notice and nothing else; the row's chip - still works. The notifier keeps a small SharedPreferences record of the - `id@fireAt` notices it has posted, so a reschedule, even in a new process, - does not bring back a notice the user swiped away; one the app cancelled - itself (the alarm left its window) is dropped from the record and may return. + still works. A swipe is recorded as an `id@fireAt` key in a small + SharedPreferences file through the notification's delete intent, which fires + for the user's swipe and never for the app's own cancel, a reboot or a + blocked channel. So a reschedule, even in a new process, does not bring back + a notice the user got rid of, and one the system took away does come back. **The `showIntent` was wrong until M9.** It is what the system opens when the user taps the status-bar alarm icon or the lockscreen's next-alarm line, and it From 1253c0fc556a4c52484602b9615f2f2eeeb27d88 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Wed, 21 Oct 2026 20:26:50 +0200 Subject: [PATCH 04/10] fix(alarms): no fire for an occurrence already accounted for A natural fire whose instant equals the skip or the handled-occurrence watermark is ignored, so a dismissal racing a reschedule cannot ring. Dismissing a repeating alarm persists its resolution before arming the skip, so an unspent old skip cannot absorb it. The deferred notice sync runs even if starting the ring throws, the swipe record is one synchronised read-modify-write, and it stays out of backups. Fixes #16 --- .../clockula/alarm/AlarmEngine.kt | 31 ++++++++++----- .../alarm/android/AndroidAlarmNotifier.kt | 7 ++-- .../alarm/receiver/AlarmFireReceiver.kt | 5 ++- .../alarm/ring/UpcomingNotifications.kt | 24 ++++++++---- app/src/main/res/xml/backup_rules.xml | 2 + .../main/res/xml/data_extraction_rules.xml | 5 ++- .../clockula/alarm/AlarmEngineUpcomingTest.kt | 38 +++++++++++++++++++ docs/ARCHITECTURE.md | 8 +++- 8 files changed, 95 insertions(+), 25 deletions(-) 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 5e3a23c..6c95c1e 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt @@ -290,6 +290,10 @@ class AlarmEngine @Inject constructor( */ private suspend fun dismissComingLocked(alarm: Alarm): DismissOutcome { if (alarm.repeatDays.isRepeating) { + // Persist the resolution first: an earlier skip that has gone by but + // not yet been spent in storage would otherwise absorb this one, + // and the occurrence just dismissed would ring. + rescheduleLocked() alarms.setSkipNextOccurrence(alarm.id, true) rescheduleLocked() return DismissOutcome.SkippedNext(alarm.id) @@ -309,12 +313,16 @@ class AlarmEngine @Inject constructor( // 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) AlarmRing.AUTO_SILENCE_AFTER else AlarmRing.FIRE_GRACE - // A skip armed on this very occurrence after its fire was already on its - // way: the user dismissed it in advance (#16), so it must not ring. - val skipped = !isSnoozeFire && state.skippedOccurrence == scheduledFor - // Deleted, switched off, skipped, or a PendingIntent so stale the moment - // has gone: re-resolve the slot and stay quiet. - if (alarm == null || !alarm.enabled || skipped || now - scheduledFor > lateBy) { + // An occurrence already accounted for: skipped in advance (#16) — armed or, + // once a reschedule spent the skip, recorded as handled — or a duplicate + // of one that already rang. A fire already on its way when that was + // written must not ring it. Equality only: a watermark ahead of a clock + // moved backwards must never silence an earlier occurrence. + val accountedFor = !isSnoozeFire && + (scheduledFor == state.skippedOccurrence || scheduledFor == state.handledOccurrence) + // Deleted, switched off, accounted for, or a PendingIntent so stale the + // moment has gone: re-resolve the slot and stay quiet. + if (alarm == null || !alarm.enabled || accountedFor || now - scheduledFor > lateBy) { rescheduleLocked() return FireOutcome.Ignored } @@ -343,10 +351,13 @@ class AlarmEngine @Inject constructor( val upcoming = mutableListOf() rescheduleLocked(deferUpcomingTo = upcoming) - states.save(states.state(alarmId).copy(ringingSince = now)) - ring.startRinging(alarmId) - scheduler.scheduleAutoSilence(now + AlarmRing.AUTO_SILENCE_AFTER, alarmId) - syncUpcomingLocked(upcoming, now) + try { + states.save(states.state(alarmId).copy(ringingSince = now)) + ring.startRinging(alarmId) + scheduler.scheduleAutoSilence(now + AlarmRing.AUTO_SILENCE_AFTER, alarmId) + } finally { + syncUpcomingLocked(upcoming, now) + } return FireOutcome.Ringing(alarmId) } diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/android/AndroidAlarmNotifier.kt b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/android/AndroidAlarmNotifier.kt index 3c0ace1..bc2703c 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/android/AndroidAlarmNotifier.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/android/AndroidAlarmNotifier.kt @@ -73,9 +73,10 @@ class AndroidAlarmNotifier @Inject constructor( // A swipe is the user's word for that occurrence; the record keeps // only the occurrences still noticed. - val swiped = UpcomingNotifications.swipedKeys(context) - val stillSwiped = swiped.intersect(notices.map(UpcomingNotifications::key).toSet()) - if (stillSwiped != swiped) UpcomingNotifications.retainSwiped(context, stillSwiped) + val stillSwiped = UpcomingNotifications.retainSwiped( + context, + notices.map(UpcomingNotifications::key).toSet(), + ) // POST_NOTIFICATIONS is a runtime permission from API 33. Denied, the // user loses the notice and keeps the alarm and the row's Dismiss. diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/receiver/AlarmFireReceiver.kt b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/receiver/AlarmFireReceiver.kt index 637e583..812e193 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/receiver/AlarmFireReceiver.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/receiver/AlarmFireReceiver.kt @@ -13,8 +13,9 @@ import kotlin.time.Instant import javax.inject.Inject /** - * The AlarmManager slots arriving: the fire, the backstop and the upcoming check. Thin by design: every decision the engine - * could make is the engine's, so the receiver only unpacks and dispatches. + * The AlarmManager slots arriving: the fire, the backstop and the upcoming + * check. Thin by design: every decision the engine could make is the + * engine's, so the receiver only unpacks and dispatches. */ @AndroidEntryPoint class AlarmFireReceiver : HiltBroadcastReceiver() { diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/UpcomingNotifications.kt b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/UpcomingNotifications.kt index c7d431e..c4f182b 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/UpcomingNotifications.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/UpcomingNotifications.kt @@ -81,19 +81,28 @@ object UpcomingNotifications { * a new process, never brings back a notice the user got rid of, and a * notice the system took away does come back. */ - fun swipedKeys(context: Context): Set = - prefs(context).getStringSet(SWIPED_KEY, emptySet()).orEmpty() - + @Synchronized fun recordSwiped(context: Context, alarmId: Long, fireAtMillis: Long) { - val keys = swipedKeys(context) + key(alarmId, fireAtMillis) + val keys = read(context) + key(alarmId, fireAtMillis) prefs(context).edit().putStringSet(SWIPED_KEY, keys).apply() } - /** Drops the keys for occurrences no longer noticed, so the record stays small. */ - fun retainSwiped(context: Context, keys: Set) { - prefs(context).edit().putStringSet(SWIPED_KEY, keys).apply() + /** + * Drops the keys for occurrences no longer in [noticed], so the record stays + * small, and returns what is left. One synchronised read-modify-write, so a + * swipe recorded from the receiver meanwhile is never overwritten. + */ + @Synchronized + fun retainSwiped(context: Context, noticed: Set): Set { + val swiped = read(context) + val kept = swiped.intersect(noticed) + if (kept != swiped) prefs(context).edit().putStringSet(SWIPED_KEY, kept).apply() + return kept } + private fun read(context: Context): Set = + prefs(context).getStringSet(SWIPED_KEY, emptySet()).orEmpty().toSet() + fun key(notice: UpcomingNotice): String = key(notice.alarm.id, notice.fireAt.toEpochMilliseconds()) private fun key(alarmId: Long, fireAtMillis: Long): String = "$alarmId@$fireAtMillis" @@ -138,6 +147,7 @@ object UpcomingNotifications { PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE, ) + /** Excluded from backup and device transfer: it describes this device's shade. */ private const val RECORD_FILE = "alarm_upcoming_notices" private const val SWIPED_KEY = "swiped" diff --git a/app/src/main/res/xml/backup_rules.xml b/app/src/main/res/xml/backup_rules.xml index a407ce6..7177e8b 100644 --- a/app/src/main/res/xml/backup_rules.xml +++ b/app/src/main/res/xml/backup_rules.xml @@ -5,4 +5,6 @@ otherwise prompt again after a restore. Keep in step with data_extraction_rules.xml. --> + + diff --git a/app/src/main/res/xml/data_extraction_rules.xml b/app/src/main/res/xml/data_extraction_rules.xml index a8a7cba..55a8382 100644 --- a/app/src/main/res/xml/data_extraction_rules.xml +++ b/app/src/main/res/xml/data_extraction_rules.xml @@ -1,10 +1,13 @@ - + + + diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt index 718a488..c6e4fba 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt @@ -8,6 +8,7 @@ import de.jeanlucmakiola.clockula.domain.alarm.UpcomingNotices import de.jeanlucmakiola.clockula.testing.RingEvent import de.jeanlucmakiola.clockula.testing.alarmEngineHarness import de.jeanlucmakiola.clockula.testing.offsetInstant +import de.jeanlucmakiola.clockula.testing.ringState import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.Test import org.junit.jupiter.api.io.TempDir @@ -207,4 +208,41 @@ class AlarmEngineUpcomingTest { assertThat(harness.engine.dismissUpcomingOccurrence(1L, sixThirty)).isEqualTo(DismissOutcome.NotFound) } + + @Test + fun `a fire that lands after the dismissed skip was spent still does not ring`(@TempDir tempDir: Path) = + runTest { + val harness = alarmEngineHarness(tempDir) + harness.givenAlarm(id = 1L, hour = 6, minute = 30, repeatDays = RepeatDays.EVERY_DAY) + harness.engine.reschedule() + harness.engine.dismissUpcomingOccurrence(1L, sixThirty) + + // Another entry point reschedules first and spends the skip. + harness.wallClock.instant = sixThirty + harness.engine.reschedule() + val outcome = harness.engine.onFire(1L, sixThirty) + + assertThat(outcome).isEqualTo(FireOutcome.Ignored) + assertThat(harness.ring.events).isEmpty() + } + + @Test + fun `an old skip gone by but not yet spent does not swallow the new dismissal`(@TempDir tempDir: Path) = + runTest { + val harness = alarmEngineHarness(tempDir) + harness.givenAlarm( + id = 1L, + hour = 6, + minute = 30, + repeatDays = RepeatDays.EVERY_DAY, + skipNextOccurrence = true, + ) + // Yesterday's skip, never spent because nothing rescheduled since. + harness.states.save(ringState(alarmId = 1L, skippedOccurrence = offsetInstant("2026-06-09T06:30+02:00"))) + + val outcome = harness.engine.dismissUpcomingOccurrence(1L, sixThirty) + + assertThat(outcome).isEqualTo(DismissOutcome.SkippedNext(1L)) + assertThat(harness.scheduler.next?.fireAt).isEqualTo(sixThirtyTomorrow) + } } diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 7ab3848..cfda3ee 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1250,8 +1250,12 @@ the alarm's row for whoever opens the app instead. round its challenge. When it matches, it takes `dismissUpcoming`'s own branches: a repeating alarm skips that occurrence, a one-shot is switched off (or deleted, if transient). A fire broadcast already in flight when a skip - was armed is `Ignored` by `onFire`, which checks the skip watermark against - the instant it carries. + was armed is `Ignored` by `onFire`: a natural fire whose instant *equals* + the skip or the handled-occurrence watermark is already accounted for. + Equality, not order, so a watermark left ahead of a clock moved backwards + never silences an earlier occurrence. A repeating alarm's resolution is + persisted before the skip is armed, so an old skip that has gone by but was + never spent cannot absorb the new one. - **No dismiss challenge before the ring**, as in Google Clock. The challenge proves you are awake while an alarm sounds; before it, skipping is already one unchallenged switch away in the editor. The notification's Dismiss does ask From f570bdefb738bafad6a6f7983508419411935c1d Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Thu, 22 Oct 2026 07:41:05 +0200 Subject: [PATCH 05/10] fix(alarms): keep the upcoming notice off a locked screen below Android 12 The Dismiss action's unlock requirement is honoured only from Android 12, so on 10 and 11 the notice is now secret on the secure lock screen rather than dismissible by whoever holds the phone. Fixes #16 --- .../clockula/alarm/ring/UpcomingNotifications.kt | 16 +++++++++++++--- docs/ARCHITECTURE.md | 3 ++- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/UpcomingNotifications.kt b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/UpcomingNotifications.kt index c4f182b..92e0c9b 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/UpcomingNotifications.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/UpcomingNotifications.kt @@ -6,6 +6,7 @@ import android.app.NotificationManager import android.app.PendingIntent import android.content.Context import android.content.Intent +import android.os.Build import androidx.core.app.NotificationCompat import de.jeanlucmakiola.clockula.MainActivity import de.jeanlucmakiola.clockula.R @@ -18,8 +19,9 @@ import de.jeanlucmakiola.clockula.domain.alarm.UpcomingNotice * (#16). Silent and low-importance: it is a way out for someone already awake, * not a second alarm. Its Dismiss is the engine's `dismissUpcomingOccurrence` * for the one occurrence it names, so it can never end a ring that has already - * started; it asks for an unlock first, so a locked phone cannot be talked out - * of its alarm by whoever picks it up. + * started; it asks for an unlock first (and below Android 12, where it cannot, + * the notice is kept off the secure lock screen), so a locked phone cannot be + * talked out of its alarm by whoever picks it up. */ object UpcomingNotifications { @@ -56,7 +58,15 @@ object UpcomingNotifications { .setContentText(text) .setCategory(NotificationCompat.CATEGORY_REMINDER) .setPriority(NotificationCompat.PRIORITY_LOW) - .setVisibility(NotificationCompat.VISIBILITY_PUBLIC) + // The Dismiss asks for an unlock from Android 12; below that the + // platform cannot, so there the notice stays off a locked screen. + .setVisibility( + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) { + NotificationCompat.VISIBILITY_PUBLIC + } else { + NotificationCompat.VISIBILITY_SECRET + }, + ) .setOnlyAlertOnce(true) .setSilent(true) .setShowWhen(false) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index cfda3ee..c3bd2a0 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1260,7 +1260,8 @@ the alarm's row for whoever opens the app instead. proves you are awake while an alarm sounds; before it, skipping is already one unchallenged switch away in the editor. The notification's Dismiss does ask for an **unlock** (`setAuthenticationRequired`), so a locked phone cannot be - talked out of its alarm by whoever picks it up. + talked out of its alarm by whoever picks it up. That flag needs Android 12; + below it the notice is `VISIBILITY_SECRET`, kept off the secure lock screen. - **Denied notifications** cost the notice and nothing else; the row's chip still works. A swipe is recorded as an `id@fireAt` key in a small SharedPreferences file through the notification's delete intent, which fires From b4af1648268c67b38242243a3a76bb909aca2d51 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Thu, 22 Oct 2026 07:51:57 +0200 Subject: [PATCH 06/10] fix(alarms): spend a dismissed skip on time, and ring a re-armed handled alarm The upcoming check now also wakes once a noticed occurrence's fire grace has run out, so a notice cannot outlive a late fire, and once an armed skip's has, so the list stops saying "skipping next" about an occurrence already gone. onFire is back to checking only the armed skip: the resolver deliberately re-arms a handled occurrence that a clock moved backwards put back in the future, and that one must ring. Fixes #16 --- .../clockula/alarm/AlarmEngine.kt | 44 +++++++++++-------- .../clockula/domain/alarm/UpcomingNotices.kt | 32 +++++++++++--- .../clockula/alarm/AlarmEngineUpcomingTest.kt | 37 +++++++++------- .../domain/alarm/UpcomingNoticesTest.kt | 31 ++++++++++++- docs/ARCHITECTURE.md | 16 ++++--- 5 files changed, 114 insertions(+), 46 deletions(-) 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 6c95c1e..2177204 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/AlarmEngine.kt @@ -11,7 +11,6 @@ import de.jeanlucmakiola.clockula.domain.alarm.AlarmRing import de.jeanlucmakiola.clockula.domain.alarm.AlarmRingState import de.jeanlucmakiola.clockula.domain.alarm.DismissOutcome import de.jeanlucmakiola.clockula.domain.alarm.FireOutcome -import de.jeanlucmakiola.clockula.domain.alarm.FireSource import de.jeanlucmakiola.clockula.domain.alarm.RingSession import de.jeanlucmakiola.clockula.domain.alarm.ScheduledFire import de.jeanlucmakiola.clockula.domain.alarm.SnoozeOutcome @@ -139,7 +138,7 @@ class AlarmEngine @Inject constructor( // a just-passed occurrence through the fire grace: one already due is // the fire's, not the notice's. val resolution = AlarmResolver.resolve(alarm, states.state(alarmId), now, zones.current()) - if (resolution.source != FireSource.OCCURRENCE || resolution.nextFire != fireAt || fireAt <= now) { + if (resolution.nextFire != fireAt || !UpcomingNotices.isInWindow(fireAt, resolution.source, now)) { // Whatever posted the stale notice is put right on the way out. rescheduleLocked() return@withLock DismissOutcome.Stale @@ -232,17 +231,19 @@ class AlarmEngine @Inject constructor( * instead of syncing them, for a caller with something more urgent to do * first (the fire path). */ - private suspend fun rescheduleLocked(deferUpcomingTo: MutableList? = null): ScheduledFire? { + private suspend fun rescheduleLocked(deferUpcomingTo: MutableList? = null): ScheduledFire? { val now = wallClock.now() val zone = zones.current() var earliest: Pair? = null val resolved = mutableListOf() + val pendingSkips = mutableListOf() for (alarm in alarms.alarms().first()) { val state = states.state(alarm.id) val resolution = AlarmResolver.resolve(alarm, state, now, zone) persist(alarm, state, resolution) resolved += UpcomingAlarm(alarm, resolution.nextFire, resolution.source) + resolution.state.skippedOccurrence?.let { pendingSkips += it } val fireAt = resolution.nextFire ?: continue val best = earliest @@ -262,7 +263,8 @@ class AlarmEngine @Inject constructor( ScheduledFire(winner.first.id, fireAt, winner.second.source, mode) } // After the real registration, never before it. - if (deferUpcomingTo == null) syncUpcomingLocked(resolved, now) else deferUpcomingTo += resolved + val upcoming = UpcomingSync(resolved, pendingSkips) + if (deferUpcomingTo == null) syncUpcomingLocked(upcoming, now) else deferUpcomingTo += upcoming return scheduled } @@ -275,9 +277,9 @@ class AlarmEngine @Inject constructor( * Nothing here may cost an alarm, so a failure is swallowed rather than * thrown into whichever entry point is rescheduling. */ - private fun syncUpcomingLocked(resolved: List, now: Instant) { + private fun syncUpcomingLocked(upcoming: UpcomingSync, now: Instant) { runCatching { - val plan = UpcomingNotices.plan(resolved, now) + val plan = UpcomingNotices.plan(upcoming.resolved, now, upcoming.pendingSkips) notifier.showUpcoming(plan.notices) val next = plan.nextCheckAt if (next == null) scheduler.cancelUpcomingCheck() else scheduler.scheduleUpcomingCheck(next) @@ -313,16 +315,15 @@ class AlarmEngine @Inject constructor( // 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) AlarmRing.AUTO_SILENCE_AFTER else AlarmRing.FIRE_GRACE - // An occurrence already accounted for: skipped in advance (#16) — armed or, - // once a reschedule spent the skip, recorded as handled — or a duplicate - // of one that already rang. A fire already on its way when that was - // written must not ring it. Equality only: a watermark ahead of a clock - // moved backwards must never silence an earlier occurrence. - val accountedFor = !isSnoozeFire && - (scheduledFor == state.skippedOccurrence || scheduledFor == state.handledOccurrence) - // Deleted, switched off, accounted for, or a PendingIntent so stale the - // moment has gone: re-resolve the slot and stay quiet. - if (alarm == null || !alarm.enabled || accountedFor || now - scheduledFor > lateBy) { + // A skip armed on this very occurrence after its fire was already on its + // way: the user dismissed it in advance (#16), so it must not ring. + // Only the armed skip, not the handled-occurrence watermark: the + // resolver deliberately re-arms a handled occurrence that a clock moved + // backwards has put back in the future, and that one must ring. + val skipped = !isSnoozeFire && scheduledFor == state.skippedOccurrence + // Deleted, switched off, skipped, or a PendingIntent so stale the moment + // has gone: re-resolve the slot and stay quiet. + if (alarm == null || !alarm.enabled || skipped || now - scheduledFor > lateBy) { rescheduleLocked() return FireOutcome.Ignored } @@ -348,7 +349,7 @@ class AlarmEngine @Inject constructor( notifier.clearSnoozed(alarmId) // The upcoming notices wait until the ring has started: their binder // calls must not stand between the fire and the sound. - val upcoming = mutableListOf() + val upcoming = mutableListOf() rescheduleLocked(deferUpcomingTo = upcoming) try { @@ -356,7 +357,7 @@ class AlarmEngine @Inject constructor( ring.startRinging(alarmId) scheduler.scheduleAutoSilence(now + AlarmRing.AUTO_SILENCE_AFTER, alarmId) } finally { - syncUpcomingLocked(upcoming, now) + upcoming.forEach { syncUpcomingLocked(it, now) } } return FireOutcome.Ringing(alarmId) } @@ -450,3 +451,10 @@ class AlarmEngine @Inject constructor( rescheduleLocked() } } + +/** What one reschedule pass hands the upcoming-alarm sync (#16). */ +private class UpcomingSync( + val resolved: List, + /** Armed skips' watermarks, so a check can spend each one on time. */ + val pendingSkips: List, +) diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNotices.kt b/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNotices.kt index 1b366d6..955c81b 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNotices.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNotices.kt @@ -14,7 +14,7 @@ data class UpcomingNotice( /** * What the upcoming-alarm surface should show now, and when it next has to - * look again. [nextCheckAt] is null when no alarm's window is still to open. + * look again. [nextCheckAt] is null when there is nothing left to look at. */ data class UpcomingNoticePlan( val notices: List, @@ -44,17 +44,37 @@ object UpcomingNotices { fun isInWindow(nextFire: Instant?, source: FireSource, now: Instant, lead: Duration = LEAD): Boolean = source == FireSource.OCCURRENCE && nextFire != null && nextFire > now && nextFire - now <= lead - /** Notices ordered by when they ring, then id; the next check is the earliest window still to open. */ - fun plan(upcoming: List, now: Instant, lead: Duration = LEAD): UpcomingNoticePlan { + /** + * Notices ordered by when they ring, then id. The next check is the + * earliest of three instants still to come: + * + * - a window opening, so its notice goes up; + * - a noticed occurrence's fire grace running out, so a notice whose fire + * came late (Doze, no exact alarms) does not outlive its alarm; + * - an armed skip's ([pendingSkips]) fire grace running out, so a skip + * dismissed in advance is spent on time and the list stops saying + * "skipping next" about an occurrence that has gone by. + * + * The last two wait out the grace rather than land on the alarm's own + * instant, so they never race its fire. + */ + fun plan( + upcoming: List, + now: Instant, + pendingSkips: List = emptyList(), + lead: Duration = LEAD, + fireGrace: Duration = AlarmRing.FIRE_GRACE, + ): UpcomingNoticePlan { val notices = upcoming .filter { isInWindow(it.nextFire, it.source, now, lead) } .map { UpcomingNotice(it.alarm, it.nextFire!!) } .sortedWith(compareBy({ it.fireAt }, { it.alarm.id })) - val nextCheckAt = upcoming + val openings = upcoming .filter { it.source == FireSource.OCCURRENCE } .mapNotNull { entry -> entry.nextFire?.let { it - lead } } - .filter { it > now } - .minOrNull() + val closings = notices.map { it.fireAt + fireGrace } + val skipsSpent = pendingSkips.map { it + fireGrace } + val nextCheckAt = (openings + closings + skipsSpent).filter { it > now }.minOrNull() return UpcomingNoticePlan(notices, nextCheckAt) } } diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt index c6e4fba..c2ae9a4 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt @@ -2,6 +2,7 @@ package de.jeanlucmakiola.clockula.alarm import com.google.common.truth.Truth.assertThat import de.jeanlucmakiola.clockula.domain.RepeatDays +import de.jeanlucmakiola.clockula.domain.alarm.AlarmRing import de.jeanlucmakiola.clockula.domain.alarm.DismissOutcome import de.jeanlucmakiola.clockula.domain.alarm.FireOutcome import de.jeanlucmakiola.clockula.domain.alarm.UpcomingNotices @@ -51,7 +52,8 @@ class AlarmEngineUpcomingTest { harness.engine.onUpcomingWindow() assertThat(harness.notifier.upcoming.map { it.alarm.id }).containsExactly(1L, 2L).inOrder() - assertThat(harness.scheduler.upcomingCheck).isNull() + // Next: the first notice's alarm, gone by. + assertThat(harness.scheduler.upcomingCheck).isEqualTo(sixThirty + AlarmRing.FIRE_GRACE) } @Test @@ -69,7 +71,12 @@ class AlarmEngineUpcomingTest { assertThat(alarm.skipNextOccurrence).isTrue() assertThat(harness.scheduler.next?.fireAt).isEqualTo(sixThirtyTomorrow) assertThat(harness.notifier.upcoming).isEmpty() - assertThat(harness.scheduler.upcomingCheck).isEqualTo(sixThirtyTomorrow - UpcomingNotices.LEAD) + // Back once the skipped occurrence has gone by, to spend the skip. + assertThat(harness.scheduler.upcomingCheck).isEqualTo(sixThirty + AlarmRing.FIRE_GRACE) + harness.wallClock.instant = sixThirty + AlarmRing.FIRE_GRACE + harness.engine.onUpcomingWindow() + assertThat(harness.alarms.find(1L)!!.skipNextOccurrence).isFalse() + assertThat(harness.scheduler.next?.fireAt).isEqualTo(sixThirtyTomorrow) } @Test @@ -210,21 +217,21 @@ class AlarmEngineUpcomingTest { } @Test - fun `a fire that lands after the dismissed skip was spent still does not ring`(@TempDir tempDir: Path) = - runTest { - val harness = alarmEngineHarness(tempDir) - harness.givenAlarm(id = 1L, hour = 6, minute = 30, repeatDays = RepeatDays.EVERY_DAY) - harness.engine.reschedule() - harness.engine.dismissUpcomingOccurrence(1L, sixThirty) + fun `a handled occurrence a clock moved backwards re-armed still rings`(@TempDir tempDir: Path) = runTest { + val harness = alarmEngineHarness(tempDir) + harness.givenAlarm(id = 1L, hour = 6, minute = 30, repeatDays = RepeatDays.EVERY_DAY) + harness.engine.reschedule() + harness.givenRinging(alarmId = 1L, at = sixThirty) + harness.engine.dismiss(1L) - // Another entry point reschedules first and spends the skip. - harness.wallClock.instant = sixThirty - harness.engine.reschedule() - val outcome = harness.engine.onFire(1L, sixThirty) + // The clock is set back under the dismissed alarm; the resolver re-arms it. + harness.wallClock.instant = sixThirty - 15.minutes + harness.engine.onSystemTimeOrZoneChanged() + assertThat(harness.scheduler.next?.fireAt).isEqualTo(sixThirty) - assertThat(outcome).isEqualTo(FireOutcome.Ignored) - assertThat(harness.ring.events).isEmpty() - } + harness.wallClock.instant = sixThirty + assertThat(harness.engine.onFire(1L, sixThirty)).isEqualTo(FireOutcome.Ringing(1L)) + } @Test fun `an old skip gone by but not yet spent does not swallow the new dismissal`(@TempDir tempDir: Path) = diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNoticesTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNoticesTest.kt index f13e755..b7135d1 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNoticesTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNoticesTest.kt @@ -4,6 +4,7 @@ import com.google.common.truth.Truth.assertThat import de.jeanlucmakiola.clockula.testing.offsetInstant import de.jeanlucmakiola.clockula.testing.upcomingAlarm import org.junit.jupiter.api.Test +import kotlin.time.Duration.Companion.days import kotlin.time.Duration.Companion.hours import kotlin.time.Duration.Companion.milliseconds import kotlin.time.Duration.Companion.minutes @@ -66,8 +67,6 @@ class UpcomingNoticesTest { fun `the next check is when the earliest window still to open opens`() { val plan = UpcomingNotices.plan( listOf( - // Already open: noticed now, needs no check. - upcomingAlarm(id = 1L, nextFire = now + 30.minutes), upcomingAlarm(id = 2L, nextFire = now + 5.hours), upcomingAlarm(id = 3L, nextFire = now + 3.hours), // A snooze's own notification covers it; no window to wait for. @@ -79,6 +78,34 @@ class UpcomingNoticesTest { assertThat(plan.nextCheckAt).isEqualTo(now + 1.hours) } + @Test + fun `a noticed occurrence is looked at again once its fire grace has run out`() { + val fireAt = now + 30.minutes + val plan = UpcomingNotices.plan( + listOf( + upcomingAlarm(id = 1L, nextFire = fireAt), + upcomingAlarm(id = 2L, nextFire = now + 5.hours), + ), + now, + ) + + // So a notice whose fire came late does not outlive its alarm. + assertThat(plan.nextCheckAt).isEqualTo(fireAt + AlarmRing.FIRE_GRACE) + } + + @Test + fun `an armed skip is looked at again once its occurrence has gone by`() { + val skipped = now + 40.minutes + val plan = UpcomingNotices.plan( + listOf(upcomingAlarm(id = 1L, nextFire = now + 1.days)), + now, + pendingSkips = listOf(skipped), + ) + + // So the skip is spent then, not at tomorrow's window. + assertThat(plan.nextCheckAt).isEqualTo(skipped + AlarmRing.FIRE_GRACE) + } + @Test fun `nothing to come means no notice and no check`() { val plan = UpcomingNotices.plan( diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index c3bd2a0..6ad2880 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1238,7 +1238,12 @@ the alarm's row for whoever opens the app instead. - **Kept true in one place.** Every path that moves an alarm already ends in `rescheduleLocked`, so that is where the notices are synced and the third slot is armed for the earliest window still to open, *after* the real - registration. The sync is wrapped so a notification failure costs nothing, + registration. The check also wakes once a noticed occurrence's fire grace has + run out (so a notice cannot outlive a fire that came late) and once an armed + skip's has (so a skip dismissed in advance is spent on time, and the list + stops saying "skipping next" about an occurrence already gone). Both wait out + the grace rather than land on the alarm's own instant, so they never race its + fire. The sync is wrapped so a notification failure costs nothing, and on the fire path it is deferred until the ring has started, so its binder calls never stand between the fire and the sound. The check's receiver just calls `onUpcomingWindow()`, which is `reschedule()`. @@ -1250,10 +1255,11 @@ the alarm's row for whoever opens the app instead. round its challenge. When it matches, it takes `dismissUpcoming`'s own branches: a repeating alarm skips that occurrence, a one-shot is switched off (or deleted, if transient). A fire broadcast already in flight when a skip - was armed is `Ignored` by `onFire`: a natural fire whose instant *equals* - the skip or the handled-occurrence watermark is already accounted for. - Equality, not order, so a watermark left ahead of a clock moved backwards - never silences an earlier occurrence. A repeating alarm's resolution is + was armed is `Ignored` by `onFire`, which checks the armed skip watermark + against the instant it carries. Only the skip: the handled-occurrence + watermark is left to the resolver, which deliberately re-arms a handled + occurrence that a clock moved backwards has put back in the future. A + repeating alarm's resolution is persisted before the skip is armed, so an old skip that has gone by but was never spent cannot absorb the new one. - **No dismiss challenge before the ring**, as in Google Clock. The challenge From a88e696d52a719aed5e358bc74ed5c80be9a750c Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Thu, 22 Oct 2026 08:00:38 +0200 Subject: [PATCH 07/10] fix(alarms): spend a skip only after its fire grace The resolver now keeps a skip armed until its occurrence is out of the fire grace too, so a reschedule in those two minutes cannot spend it early and let a fire already in flight ring the dismissed alarm; onFire refuses that fire by the armed watermark. The notifier also cancels notices it posted itself, which the platform may not list as active yet. Fixes #16 --- .../alarm/android/AndroidAlarmNotifier.kt | 6 ++++- .../clockula/domain/alarm/AlarmResolver.kt | 5 +++- .../clockula/alarm/AlarmEngineUpcomingTest.kt | 25 ++++++++++++++++- .../domain/alarm/AlarmResolverTest.kt | 27 ++++++++++++++++--- docs/ARCHITECTURE.md | 10 ++++--- 5 files changed, 63 insertions(+), 10 deletions(-) diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/android/AndroidAlarmNotifier.kt b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/android/AndroidAlarmNotifier.kt index bc2703c..2783ab7 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/android/AndroidAlarmNotifier.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/android/AndroidAlarmNotifier.kt @@ -68,7 +68,11 @@ class AndroidAlarmNotifier @Inject constructor( .filter { it.tag == UpcomingNotifications.TAG } .map { it.id } .toSet() - onScreen.filterNot { it in wantedIds }.forEach { notifications.cancel(UpcomingNotifications.TAG, it) } + // Also what this process posted: a notify is asynchronous, so one + // posted a moment ago may not be listed as active yet. + (onScreen + lastPosted.keys.map(UpcomingNotifications::notificationId)) + .filterNot { it in wantedIds } + .forEach { notifications.cancel(UpcomingNotifications.TAG, it) } lastPosted.keys.retainAll(notices.map { it.alarm.id }.toSet()) // A swipe is the user's word for that occurrence; the record keeps 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 4adf88b..a13bc60 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 @@ -113,7 +113,10 @@ object AlarmResolver { } val watermark = working.skippedOccurrence ?: candidates(alarm, working, now, zone, fireGrace).first() - val consumed = watermark <= now + // Spent only once the skipped occurrence is out of the fire grace too: + // a fire for it can still be accepted until then, and the engine refuses + // that fire by this very watermark (#16). + val consumed = watermark + fireGrace <= now return AlarmResolution( alarmId = alarm.id, // Always the candidate *after* the watermark, so a skipped occurrence diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt index c2ae9a4..65b062e 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt @@ -15,6 +15,7 @@ import org.junit.jupiter.api.Test import org.junit.jupiter.api.io.TempDir import java.nio.file.Path import kotlin.time.Duration.Companion.minutes +import kotlin.time.Duration.Companion.seconds /** * The upcoming-alarm notification (#16). The harness starts at 05:00 Berlin, so @@ -189,8 +190,12 @@ class AlarmEngineUpcomingTest { assertThat(outcome).isEqualTo(FireOutcome.Ignored) assertThat(harness.ring.events).isEmpty() assertThat(harness.scheduler.next?.fireAt).isEqualTo(sixThirtyTomorrow) - // The skip is spent on the one occurrence, not on tomorrow's. + // The skip is spent on the one occurrence once its grace is over, + // not on tomorrow's. + harness.wallClock.instant = sixThirty + AlarmRing.FIRE_GRACE + harness.engine.onUpcomingWindow() assertThat(harness.alarms.find(1L)!!.skipNextOccurrence).isFalse() + assertThat(harness.scheduler.next?.fireAt).isEqualTo(sixThirtyTomorrow) } @Test @@ -216,6 +221,24 @@ class AlarmEngineUpcomingTest { assertThat(harness.engine.dismissUpcomingOccurrence(1L, sixThirty)).isEqualTo(DismissOutcome.NotFound) } + @Test + fun `a fire for the dismissed occurrence is refused even after another reschedule`(@TempDir tempDir: Path) = + runTest { + val harness = alarmEngineHarness(tempDir) + harness.givenAlarm(id = 1L, hour = 6, minute = 30, repeatDays = RepeatDays.EVERY_DAY) + harness.engine.reschedule() + harness.engine.dismissUpcomingOccurrence(1L, sixThirty) + + // Another entry point reschedules inside the fire grace, before the + // fire already on its way is processed: the skip is not spent yet. + harness.wallClock.instant = sixThirty + 30.seconds + harness.engine.reschedule() + val outcome = harness.engine.onFire(1L, sixThirty) + + assertThat(outcome).isEqualTo(FireOutcome.Ignored) + assertThat(harness.ring.events).isEmpty() + } + @Test fun `a handled occurrence a clock moved backwards re-armed still rings`(@TempDir tempDir: Path) = runTest { val harness = alarmEngineHarness(tempDir) 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 6d89e0e..2d3d87b 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 @@ -250,7 +250,11 @@ class AlarmResolverTest { val alarm = alarmAt(hour = 7, repeatDays = RepeatDays.EVERY_DAY, skipNextOccurrence = true) val watermark = offsetInstant("2026-06-10T07:00+02:00") - val resolution = resolve(alarm, ringState(skippedOccurrence = watermark), now = watermark + 30.seconds) + val resolution = resolve( + alarm, + ringState(skippedOccurrence = watermark), + now = watermark + AlarmRing.FIRE_GRACE + 30.seconds, + ) assertThat(resolution.clearSkipFlag).isTrue() assertThat(resolution.state.skippedOccurrence).isNull() @@ -261,19 +265,36 @@ class AlarmResolverTest { fun `a consumed skip cannot ring on the next pass through the fire grace`() { val alarm = alarmAt(hour = 7, repeatDays = RepeatDays.EVERY_DAY, skipNextOccurrence = true) val watermark = offsetInstant("2026-06-10T07:00+02:00") - val first = resolve(alarm, ringState(skippedOccurrence = watermark), now = watermark + 30.seconds) + val first = resolve( + alarm, + ringState(skippedOccurrence = watermark), + now = watermark + AlarmRing.FIRE_GRACE + 30.seconds, + ) // The engine persists the first pass: the flag cleared, the state saved. val second = resolve( alarm.copy(skipNextOccurrence = false), first.state, - now = watermark + 40.seconds, + now = watermark + AlarmRing.FIRE_GRACE + 40.seconds, ) assertThat(first.state.handledOccurrence).isEqualTo(watermark) assertThat(second.nextFire).isEqualTo(offsetInstant("2026-06-11T07:00+02:00")) } + @Test + fun `a skip stays armed through its occurrence's fire grace`() { + val alarm = alarmAt(hour = 7, repeatDays = RepeatDays.EVERY_DAY, skipNextOccurrence = true) + val watermark = offsetInstant("2026-06-10T07:00+02:00") + + val resolution = resolve(alarm, ringState(skippedOccurrence = watermark), now = watermark + 30.seconds) + + // A fire for it can still arrive; the engine refuses it by this watermark. + assertThat(resolution.clearSkipFlag).isFalse() + assertThat(resolution.state.skippedOccurrence).isEqualTo(watermark) + assertThat(resolution.nextFire).isEqualTo(offsetInstant("2026-06-11T07:00+02:00")) + } + @Test fun `skipping a one-shot alarm disables it`() { val alarm = alarmAt(hour = 7, skipNextOccurrence = true) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 6ad2880..008afb1 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -250,10 +250,12 @@ so the skip eats a second alarm. With the watermark: one after; - watermark still in the future ⇒ fire the first candidate after it; write nothing; -- watermark at or before now ⇒ consumed: clear the flag and the watermark, - record the skipped occurrence as the handled one, and *still* return the - first candidate after it, so the skipped occurrence cannot ring on its way - out through the grace window, on this pass or the next (#16). +- watermark and its fire grace both behind now ⇒ consumed: clear the flag and + the watermark, record the skipped occurrence as the handled one, and *still* + return the first candidate after it, so the skipped occurrence cannot ring on + this pass or the next. Not spent during the grace itself (#16): a late fire + for the skipped occurrence can still be accepted until then, and `onFire` + refuses it by the armed watermark. Arming is deferred while a snooze is pending, and a **non-repeating** alarm with the flag set is *disabled* rather than skipped: a one-shot has exactly one From fdb725e64ceb6df77a5a368fdd705b6c447a98d0 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Thu, 22 Oct 2026 08:07:52 +0200 Subject: [PATCH 08/10] fix(alarms): spend a skip at its occurrence again Keeping a skip armed through its fire grace let the editor's switch or another dismissal inside those two minutes ring the skipped alarm late, or be absorbed. The resolver goes back to spending the skip at its occurrence and recording it as handled, and the upcoming check wakes at an armed skip's occurrence so the list stops saying "skipping next" on time. Fixes #16 --- .../clockula/domain/alarm/AlarmResolver.kt | 5 +--- .../clockula/domain/alarm/UpcomingNotices.kt | 13 ++++---- .../clockula/alarm/AlarmEngineUpcomingTest.kt | 30 +++---------------- .../domain/alarm/AlarmResolverTest.kt | 27 ++--------------- .../domain/alarm/UpcomingNoticesTest.kt | 2 +- docs/ARCHITECTURE.md | 20 ++++++------- 6 files changed, 25 insertions(+), 72 deletions(-) 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 a13bc60..4adf88b 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 @@ -113,10 +113,7 @@ object AlarmResolver { } val watermark = working.skippedOccurrence ?: candidates(alarm, working, now, zone, fireGrace).first() - // Spent only once the skipped occurrence is out of the fire grace too: - // a fire for it can still be accepted until then, and the engine refuses - // that fire by this very watermark (#16). - val consumed = watermark + fireGrace <= now + val consumed = watermark <= now return AlarmResolution( alarmId = alarm.id, // Always the candidate *after* the watermark, so a skipped occurrence diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNotices.kt b/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNotices.kt index 955c81b..1717b68 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNotices.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNotices.kt @@ -51,12 +51,13 @@ object UpcomingNotices { * - a window opening, so its notice goes up; * - a noticed occurrence's fire grace running out, so a notice whose fire * came late (Doze, no exact alarms) does not outlive its alarm; - * - an armed skip's ([pendingSkips]) fire grace running out, so a skip - * dismissed in advance is spent on time and the list stops saying - * "skipping next" about an occurrence that has gone by. + * - an armed skip's ([pendingSkips]) occurrence, so a skip dismissed in + * advance is spent on time and the list stops saying "skipping next" + * about an occurrence that has gone by. Nothing is registered to fire + * at a skipped occurrence, so there is no fire to race. * - * The last two wait out the grace rather than land on the alarm's own - * instant, so they never race its fire. + * The closing waits out the grace rather than land on the alarm's own + * instant, so it never races that alarm's fire. */ fun plan( upcoming: List, @@ -73,7 +74,7 @@ object UpcomingNotices { .filter { it.source == FireSource.OCCURRENCE } .mapNotNull { entry -> entry.nextFire?.let { it - lead } } val closings = notices.map { it.fireAt + fireGrace } - val skipsSpent = pendingSkips.map { it + fireGrace } + val skipsSpent = pendingSkips val nextCheckAt = (openings + closings + skipsSpent).filter { it > now }.minOrNull() return UpcomingNoticePlan(notices, nextCheckAt) } diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt index 65b062e..56de9e9 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt @@ -15,7 +15,6 @@ import org.junit.jupiter.api.Test import org.junit.jupiter.api.io.TempDir import java.nio.file.Path import kotlin.time.Duration.Companion.minutes -import kotlin.time.Duration.Companion.seconds /** * The upcoming-alarm notification (#16). The harness starts at 05:00 Berlin, so @@ -72,9 +71,9 @@ class AlarmEngineUpcomingTest { assertThat(alarm.skipNextOccurrence).isTrue() assertThat(harness.scheduler.next?.fireAt).isEqualTo(sixThirtyTomorrow) assertThat(harness.notifier.upcoming).isEmpty() - // Back once the skipped occurrence has gone by, to spend the skip. - assertThat(harness.scheduler.upcomingCheck).isEqualTo(sixThirty + AlarmRing.FIRE_GRACE) - harness.wallClock.instant = sixThirty + AlarmRing.FIRE_GRACE + // Back at the skipped occurrence, to spend the skip. + assertThat(harness.scheduler.upcomingCheck).isEqualTo(sixThirty) + harness.wallClock.instant = sixThirty harness.engine.onUpcomingWindow() assertThat(harness.alarms.find(1L)!!.skipNextOccurrence).isFalse() assertThat(harness.scheduler.next?.fireAt).isEqualTo(sixThirtyTomorrow) @@ -190,10 +189,7 @@ class AlarmEngineUpcomingTest { assertThat(outcome).isEqualTo(FireOutcome.Ignored) assertThat(harness.ring.events).isEmpty() assertThat(harness.scheduler.next?.fireAt).isEqualTo(sixThirtyTomorrow) - // The skip is spent on the one occurrence once its grace is over, - // not on tomorrow's. - harness.wallClock.instant = sixThirty + AlarmRing.FIRE_GRACE - harness.engine.onUpcomingWindow() + // The skip is spent on the one occurrence, not on tomorrow's. assertThat(harness.alarms.find(1L)!!.skipNextOccurrence).isFalse() assertThat(harness.scheduler.next?.fireAt).isEqualTo(sixThirtyTomorrow) } @@ -221,24 +217,6 @@ class AlarmEngineUpcomingTest { assertThat(harness.engine.dismissUpcomingOccurrence(1L, sixThirty)).isEqualTo(DismissOutcome.NotFound) } - @Test - fun `a fire for the dismissed occurrence is refused even after another reschedule`(@TempDir tempDir: Path) = - runTest { - val harness = alarmEngineHarness(tempDir) - harness.givenAlarm(id = 1L, hour = 6, minute = 30, repeatDays = RepeatDays.EVERY_DAY) - harness.engine.reschedule() - harness.engine.dismissUpcomingOccurrence(1L, sixThirty) - - // Another entry point reschedules inside the fire grace, before the - // fire already on its way is processed: the skip is not spent yet. - harness.wallClock.instant = sixThirty + 30.seconds - harness.engine.reschedule() - val outcome = harness.engine.onFire(1L, sixThirty) - - assertThat(outcome).isEqualTo(FireOutcome.Ignored) - assertThat(harness.ring.events).isEmpty() - } - @Test fun `a handled occurrence a clock moved backwards re-armed still rings`(@TempDir tempDir: Path) = runTest { val harness = alarmEngineHarness(tempDir) 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 2d3d87b..6d89e0e 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 @@ -250,11 +250,7 @@ class AlarmResolverTest { val alarm = alarmAt(hour = 7, repeatDays = RepeatDays.EVERY_DAY, skipNextOccurrence = true) val watermark = offsetInstant("2026-06-10T07:00+02:00") - val resolution = resolve( - alarm, - ringState(skippedOccurrence = watermark), - now = watermark + AlarmRing.FIRE_GRACE + 30.seconds, - ) + val resolution = resolve(alarm, ringState(skippedOccurrence = watermark), now = watermark + 30.seconds) assertThat(resolution.clearSkipFlag).isTrue() assertThat(resolution.state.skippedOccurrence).isNull() @@ -265,36 +261,19 @@ class AlarmResolverTest { fun `a consumed skip cannot ring on the next pass through the fire grace`() { val alarm = alarmAt(hour = 7, repeatDays = RepeatDays.EVERY_DAY, skipNextOccurrence = true) val watermark = offsetInstant("2026-06-10T07:00+02:00") - val first = resolve( - alarm, - ringState(skippedOccurrence = watermark), - now = watermark + AlarmRing.FIRE_GRACE + 30.seconds, - ) + val first = resolve(alarm, ringState(skippedOccurrence = watermark), now = watermark + 30.seconds) // The engine persists the first pass: the flag cleared, the state saved. val second = resolve( alarm.copy(skipNextOccurrence = false), first.state, - now = watermark + AlarmRing.FIRE_GRACE + 40.seconds, + now = watermark + 40.seconds, ) assertThat(first.state.handledOccurrence).isEqualTo(watermark) assertThat(second.nextFire).isEqualTo(offsetInstant("2026-06-11T07:00+02:00")) } - @Test - fun `a skip stays armed through its occurrence's fire grace`() { - val alarm = alarmAt(hour = 7, repeatDays = RepeatDays.EVERY_DAY, skipNextOccurrence = true) - val watermark = offsetInstant("2026-06-10T07:00+02:00") - - val resolution = resolve(alarm, ringState(skippedOccurrence = watermark), now = watermark + 30.seconds) - - // A fire for it can still arrive; the engine refuses it by this watermark. - assertThat(resolution.clearSkipFlag).isFalse() - assertThat(resolution.state.skippedOccurrence).isEqualTo(watermark) - assertThat(resolution.nextFire).isEqualTo(offsetInstant("2026-06-11T07:00+02:00")) - } - @Test fun `skipping a one-shot alarm disables it`() { val alarm = alarmAt(hour = 7, skipNextOccurrence = true) diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNoticesTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNoticesTest.kt index b7135d1..5bc3970 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNoticesTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNoticesTest.kt @@ -103,7 +103,7 @@ class UpcomingNoticesTest { ) // So the skip is spent then, not at tomorrow's window. - assertThat(plan.nextCheckAt).isEqualTo(skipped + AlarmRing.FIRE_GRACE) + assertThat(plan.nextCheckAt).isEqualTo(skipped) } @Test diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 008afb1..491b8c0 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -250,12 +250,10 @@ so the skip eats a second alarm. With the watermark: one after; - watermark still in the future ⇒ fire the first candidate after it; write nothing; -- watermark and its fire grace both behind now ⇒ consumed: clear the flag and - the watermark, record the skipped occurrence as the handled one, and *still* - return the first candidate after it, so the skipped occurrence cannot ring on - this pass or the next. Not spent during the grace itself (#16): a late fire - for the skipped occurrence can still be accepted until then, and `onFire` - refuses it by the armed watermark. +- watermark at or before now ⇒ consumed: clear the flag and the watermark, + record the skipped occurrence as the handled one, and *still* return the + first candidate after it, so the skipped occurrence cannot ring on its way + out through the grace window, on this pass or the next (#16). Arming is deferred while a snooze is pending, and a **non-repeating** alarm with the flag set is *disabled* rather than skipped: a one-shot has exactly one @@ -1241,11 +1239,11 @@ the alarm's row for whoever opens the app instead. `rescheduleLocked`, so that is where the notices are synced and the third slot is armed for the earliest window still to open, *after* the real registration. The check also wakes once a noticed occurrence's fire grace has - run out (so a notice cannot outlive a fire that came late) and once an armed - skip's has (so a skip dismissed in advance is spent on time, and the list - stops saying "skipping next" about an occurrence already gone). Both wait out - the grace rather than land on the alarm's own instant, so they never race its - fire. The sync is wrapped so a notification failure costs nothing, + run out (so a notice cannot outlive a fire that came late; waiting out the + grace keeps it from racing that fire) and at an armed skip's occurrence (so a + skip dismissed in advance is spent on time, and the list stops saying + "skipping next" about an occurrence already gone; nothing is registered to + fire then, so there is nothing to race). The sync is wrapped so a notification failure costs nothing, and on the fire path it is deferred until the ring has started, so its binder calls never stand between the fire and the sound. The check's receiver just calls `onUpcomingWindow()`, which is `reschedule()`. From 5ad9bd02d6be693942cc1f48407a4f9faa18b497 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Thu, 22 Oct 2026 08:13:31 +0200 Subject: [PATCH 09/10] fix(alarms): spend a pending skip only after its fire grace The upcoming check that spends a skip dismissed in advance now wakes once the skipped occurrence's fire grace is over rather than at the occurrence itself, so it can never get ahead of a fire still in flight for it. Fixes #16 --- .../clockula/domain/alarm/UpcomingNotices.kt | 15 +++++++-------- .../clockula/alarm/AlarmEngineUpcomingTest.kt | 6 +++--- .../clockula/domain/alarm/UpcomingNoticesTest.kt | 2 +- docs/ARCHITECTURE.md | 10 +++++----- 4 files changed, 16 insertions(+), 17 deletions(-) diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNotices.kt b/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNotices.kt index 1717b68..41d0ea0 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNotices.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNotices.kt @@ -51,13 +51,13 @@ object UpcomingNotices { * - a window opening, so its notice goes up; * - a noticed occurrence's fire grace running out, so a notice whose fire * came late (Doze, no exact alarms) does not outlive its alarm; - * - an armed skip's ([pendingSkips]) occurrence, so a skip dismissed in - * advance is spent on time and the list stops saying "skipping next" - * about an occurrence that has gone by. Nothing is registered to fire - * at a skipped occurrence, so there is no fire to race. + * - an armed skip's ([pendingSkips]) fire grace running out, so a skip + * dismissed in advance is spent on time and the list stops saying + * "skipping next" about an occurrence that has gone by. * - * The closing waits out the grace rather than land on the alarm's own - * instant, so it never races that alarm's fire. + * Both of the last two wait out the grace rather than land on the + * occurrence itself: a fire for it still in flight is refused as too late + * by then, so the check can never race it. */ fun plan( upcoming: List, @@ -74,8 +74,7 @@ object UpcomingNotices { .filter { it.source == FireSource.OCCURRENCE } .mapNotNull { entry -> entry.nextFire?.let { it - lead } } val closings = notices.map { it.fireAt + fireGrace } - val skipsSpent = pendingSkips - val nextCheckAt = (openings + closings + skipsSpent).filter { it > now }.minOrNull() + val nextCheckAt = (openings + closings + pendingSkips.map { it + fireGrace }).filter { it > now }.minOrNull() return UpcomingNoticePlan(notices, nextCheckAt) } } diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt index 56de9e9..8027d9c 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt @@ -71,9 +71,9 @@ class AlarmEngineUpcomingTest { assertThat(alarm.skipNextOccurrence).isTrue() assertThat(harness.scheduler.next?.fireAt).isEqualTo(sixThirtyTomorrow) assertThat(harness.notifier.upcoming).isEmpty() - // Back at the skipped occurrence, to spend the skip. - assertThat(harness.scheduler.upcomingCheck).isEqualTo(sixThirty) - harness.wallClock.instant = sixThirty + // Back once the skipped occurrence's grace is over, to spend the skip. + assertThat(harness.scheduler.upcomingCheck).isEqualTo(sixThirty + AlarmRing.FIRE_GRACE) + harness.wallClock.instant = sixThirty + AlarmRing.FIRE_GRACE harness.engine.onUpcomingWindow() assertThat(harness.alarms.find(1L)!!.skipNextOccurrence).isFalse() assertThat(harness.scheduler.next?.fireAt).isEqualTo(sixThirtyTomorrow) diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNoticesTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNoticesTest.kt index 5bc3970..b7135d1 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNoticesTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNoticesTest.kt @@ -103,7 +103,7 @@ class UpcomingNoticesTest { ) // So the skip is spent then, not at tomorrow's window. - assertThat(plan.nextCheckAt).isEqualTo(skipped) + assertThat(plan.nextCheckAt).isEqualTo(skipped + AlarmRing.FIRE_GRACE) } @Test diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 491b8c0..1c6340c 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1239,11 +1239,11 @@ the alarm's row for whoever opens the app instead. `rescheduleLocked`, so that is where the notices are synced and the third slot is armed for the earliest window still to open, *after* the real registration. The check also wakes once a noticed occurrence's fire grace has - run out (so a notice cannot outlive a fire that came late; waiting out the - grace keeps it from racing that fire) and at an armed skip's occurrence (so a - skip dismissed in advance is spent on time, and the list stops saying - "skipping next" about an occurrence already gone; nothing is registered to - fire then, so there is nothing to race). The sync is wrapped so a notification failure costs nothing, + run out (so a notice cannot outlive a fire that came late) and once an armed + skip's has (so a skip dismissed in advance is spent on time, and the list + stops saying "skipping next" about an occurrence already gone). Both wait out + the grace rather than land on the occurrence itself: a fire for it still in + flight is refused as too late by then, so the check never races it. The sync is wrapped so a notification failure costs nothing, and on the fire path it is deferred until the ring has started, so its binder calls never stand between the fire and the sound. The check's receiver just calls `onUpcomingWindow()`, which is `reschedule()`. From 712b606779e748336803fc95cba6581c01503fb1 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Thu, 22 Oct 2026 08:21:33 +0200 Subject: [PATCH 10/10] fix(alarms): keep the upcoming notice off watches, and check past the grace The notice is local-only, so its Dismiss is never bridged to a watch where the phone's unlock does not apply. The closing and skip-spending checks land a second past the fire grace, since onFire still accepts a fire at exactly the grace. Fixes #16 --- .../clockula/alarm/ring/UpcomingNotifications.kt | 2 ++ .../clockula/domain/alarm/UpcomingNotices.kt | 8 ++++++-- .../clockula/alarm/AlarmEngineUpcomingTest.kt | 7 ++++--- .../clockula/domain/alarm/UpcomingNoticesTest.kt | 5 +++-- docs/ARCHITECTURE.md | 10 +++++++++- 5 files changed, 24 insertions(+), 8 deletions(-) diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/UpcomingNotifications.kt b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/UpcomingNotifications.kt index 92e0c9b..797c2d1 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/UpcomingNotifications.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/alarm/ring/UpcomingNotifications.kt @@ -68,6 +68,8 @@ object UpcomingNotifications { }, ) .setOnlyAlertOnce(true) + // Not bridged to a watch, where its Dismiss would skip the unlock. + .setLocalOnly(true) .setSilent(true) .setShowWhen(false) // Several alarms in the window read in the order they ring. diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNotices.kt b/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNotices.kt index 41d0ea0..c95f4c4 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNotices.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNotices.kt @@ -3,6 +3,7 @@ package de.jeanlucmakiola.clockula.domain.alarm import de.jeanlucmakiola.clockula.domain.Alarm import kotlin.time.Duration import kotlin.time.Duration.Companion.hours +import kotlin.time.Duration.Companion.seconds import kotlin.time.Instant /** One alarm about to ring, offered for dismissal before it does (#16). */ @@ -73,8 +74,11 @@ object UpcomingNotices { val openings = upcoming .filter { it.source == FireSource.OCCURRENCE } .mapNotNull { entry -> entry.nextFire?.let { it - lead } } - val closings = notices.map { it.fireAt + fireGrace } - val nextCheckAt = (openings + closings + pendingSkips.map { it + fireGrace }).filter { it > now }.minOrNull() + // Strictly past the grace: `onFire` still accepts a fire at exactly it. + val afterGrace = fireGrace + 1.seconds + val closings = notices.map { it.fireAt + afterGrace } + val skipsSpent = pendingSkips.map { it + afterGrace } + val nextCheckAt = (openings + closings + skipsSpent).filter { it > now }.minOrNull() return UpcomingNoticePlan(notices, nextCheckAt) } } diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt index 8027d9c..7cf9924 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/alarm/AlarmEngineUpcomingTest.kt @@ -15,6 +15,7 @@ import org.junit.jupiter.api.Test import org.junit.jupiter.api.io.TempDir import java.nio.file.Path import kotlin.time.Duration.Companion.minutes +import kotlin.time.Duration.Companion.seconds /** * The upcoming-alarm notification (#16). The harness starts at 05:00 Berlin, so @@ -53,7 +54,7 @@ class AlarmEngineUpcomingTest { assertThat(harness.notifier.upcoming.map { it.alarm.id }).containsExactly(1L, 2L).inOrder() // Next: the first notice's alarm, gone by. - assertThat(harness.scheduler.upcomingCheck).isEqualTo(sixThirty + AlarmRing.FIRE_GRACE) + assertThat(harness.scheduler.upcomingCheck).isEqualTo(sixThirty + AlarmRing.FIRE_GRACE + 1.seconds) } @Test @@ -72,8 +73,8 @@ class AlarmEngineUpcomingTest { assertThat(harness.scheduler.next?.fireAt).isEqualTo(sixThirtyTomorrow) assertThat(harness.notifier.upcoming).isEmpty() // Back once the skipped occurrence's grace is over, to spend the skip. - assertThat(harness.scheduler.upcomingCheck).isEqualTo(sixThirty + AlarmRing.FIRE_GRACE) - harness.wallClock.instant = sixThirty + AlarmRing.FIRE_GRACE + assertThat(harness.scheduler.upcomingCheck).isEqualTo(sixThirty + AlarmRing.FIRE_GRACE + 1.seconds) + harness.wallClock.instant = sixThirty + AlarmRing.FIRE_GRACE + 1.seconds harness.engine.onUpcomingWindow() assertThat(harness.alarms.find(1L)!!.skipNextOccurrence).isFalse() assertThat(harness.scheduler.next?.fireAt).isEqualTo(sixThirtyTomorrow) diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNoticesTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNoticesTest.kt index b7135d1..65cc7ef 100644 --- a/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNoticesTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/clockula/domain/alarm/UpcomingNoticesTest.kt @@ -8,6 +8,7 @@ import kotlin.time.Duration.Companion.days import kotlin.time.Duration.Companion.hours import kotlin.time.Duration.Companion.milliseconds import kotlin.time.Duration.Companion.minutes +import kotlin.time.Duration.Companion.seconds /** * The upcoming-alarm window (#16): which alarms get a notice, and when the @@ -90,7 +91,7 @@ class UpcomingNoticesTest { ) // So a notice whose fire came late does not outlive its alarm. - assertThat(plan.nextCheckAt).isEqualTo(fireAt + AlarmRing.FIRE_GRACE) + assertThat(plan.nextCheckAt).isEqualTo(fireAt + AlarmRing.FIRE_GRACE + 1.seconds) } @Test @@ -103,7 +104,7 @@ class UpcomingNoticesTest { ) // So the skip is spent then, not at tomorrow's window. - assertThat(plan.nextCheckAt).isEqualTo(skipped + AlarmRing.FIRE_GRACE) + assertThat(plan.nextCheckAt).isEqualTo(skipped + AlarmRing.FIRE_GRACE + 1.seconds) } @Test diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 1c6340c..b92af35 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1261,13 +1261,21 @@ the alarm's row for whoever opens the app instead. occurrence that a clock moved backwards has put back in the future. A repeating alarm's resolution is persisted before the skip is armed, so an old skip that has gone by but was - never spent cannot absorb the new one. + never spent cannot absorb the new one. One window is left open on purpose: a Dismiss + whose handling straddles the occurrence by milliseconds, with another + reschedule spending the skip before the fire already in flight is handled, + can still let that fire ring. Closing it means keeping a skip tombstone apart + from the flag, and each attempt to do so opened a worse window elsewhere (a + skip switched off or re-armed inside the grace); a dismissed alarm ringing in + a millisecond race costs less than either. - **No dismiss challenge before the ring**, as in Google Clock. The challenge proves you are awake while an alarm sounds; before it, skipping is already one unchallenged switch away in the editor. The notification's Dismiss does ask for an **unlock** (`setAuthenticationRequired`), so a locked phone cannot be talked out of its alarm by whoever picks it up. That flag needs Android 12; below it the notice is `VISIBILITY_SECRET`, kept off the secure lock screen. + It is local-only, so it is never bridged to a watch, where the unlock would + not apply. - **Denied notifications** cost the notice and nothing else; the row's chip still works. A swipe is recorded as an `id@fireAt` key in a small SharedPreferences file through the notification's delete intent, which fires