From dedcbd5dfdac34f6bfae51b44239dba16d92118b Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Wed, 21 Oct 2026 20:18:05 +0200 Subject: [PATCH] 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