From 712b606779e748336803fc95cba6581c01503fb1 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Thu, 22 Oct 2026 08:21:33 +0200 Subject: [PATCH] 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