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
This commit is contained in:
1 parent
5b35ef65d4
commit
dedcbd5dfd
9 files changed
+122
-42
No files matched your search
@@ -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
|
||||
|
||||
@@ -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<Int> = listOf(
|
||||
REQUEST_FIRE,
|
||||
@@ -46,5 +50,6 @@ object AlarmIntents {
|
||||
REQUEST_UPCOMING_CHECK,
|
||||
REQUEST_DISMISS_UPCOMING,
|
||||
REQUEST_SHOW_UPCOMING,
|
||||
REQUEST_UPCOMING_SWIPED,
|
||||
)
|
||||
}
|
||||
+12
-27
@@ -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<Long, UpcomingNotice>()
|
||||
|
||||
override fun showUpcoming(notices: List<UpcomingNotice>) {
|
||||
@@ -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"
|
||||
}
|
||||
}
|
||||
@@ -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 {
|
||||
|
||||
@@ -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<String> =
|
||||
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<String>) {
|
||||
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
|
||||
|
||||
|
||||
@@ -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,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
+12
-8
@@ -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
|
||||
|
||||
Reference in new issue
Block a user