From 014d31f9b9a6b5d136cf4d3465ea3cfdb0a2fc20 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Fri, 13 Nov 2026 17:56:11 +0100 Subject: [PATCH] fix(#171): review round 3 - re-check settings in the ring, follow calls, anchored timeout, close the screen on every stop --- .../data/reminders/FullScreenAlerts.kt | 3 + .../data/reminders/ReminderNotifier.kt | 11 +- .../data/reminders/ReminderRingService.kt | 101 +++++++++++++----- .../ui/reminders/ReminderAlertActivity.kt | 3 +- .../ui/settings/SettingsViewModel.kt | 10 +- docs/ARCHITECTURE.md | 7 +- 6 files changed, 103 insertions(+), 32 deletions(-) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/FullScreenAlerts.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/FullScreenAlerts.kt index 71947ae8..9c2fac71 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/FullScreenAlerts.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/FullScreenAlerts.kt @@ -40,6 +40,9 @@ class FullScreenAlerts @Inject constructor() { } } +/** What becomes of a ring once its settings are re-read. */ +enum class RingVerdict { Ring, HandBack, Drop } + /** Which ring a stop is for. */ sealed interface RingMatch { fun matches(alert: ReminderAlert): Boolean diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderNotifier.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderNotifier.kt index 2e446bfc..17692ff6 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderNotifier.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderNotifier.kt @@ -96,6 +96,7 @@ class ReminderNotifier @Inject constructor( return } // Android refused the ring: still full-screen, sounding once on the calendar's channel. + stopRing(RingMatch.Occurrence(alert.occurrenceKey)) val fallback = channelFor(alert, ReminderAlertStyle.FullScreen) ?: return notify(alert, build(alert, fallback, title, text, ReminderAlertStyle.FullScreen, quiet)) return @@ -105,6 +106,14 @@ class ReminderNotifier @Inject constructor( notify(alert, build(alert, channelId, title, text, style, quiet)) } + /** Whether a ring for [alert] should go on, now that settings may have changed since it started. */ + suspend fun ringVerdict(alert: ReminderAlert): RingVerdict = when { + !settingsPrefs.remindersEnabled.first() || isSilenced(alert.calendarId) -> RingVerdict.Drop + settingsPrefs.reminderAlertStyles.first().alertStyleFor(alert.calendarId) != ReminderAlertStyle.FullScreenRing || + !channels.canRing(alert.calendarId) -> RingVerdict.HandBack + else -> RingVerdict.Ring + } + private fun channelFor(alert: ReminderAlert, style: ReminderAlertStyle): String? = try { channels.channelFor(alert.calendarId, style) } catch (e: Exception) { @@ -214,7 +223,7 @@ class ReminderNotifier @Inject constructor( /** Stop the ring if it is for a reminder [match]es; [handBack] keeps it in the shade, quietly. */ fun stopRing(match: RingMatch, handBack: Boolean = false) { - if (alerts.mayBeRinging) ReminderRingService.stop(context, match, handBack) + if (alerts.mayBeRinging) ReminderRingService.stop(context, alerts, match, handBack) } /** Whether a different reminder for [alert]'s occurrence is on screen now. */ diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderRingService.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderRingService.kt index 8a167daf..af5eb259 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderRingService.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderRingService.kt @@ -25,6 +25,7 @@ import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.cancel import kotlinx.coroutines.delay import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext import javax.inject.Inject /** @@ -58,20 +59,26 @@ class ReminderRingService : Service() { private var notificationId = RING_NOTIFICATION_ID private var lastStartId: Int? = null + /** Whether the ring's players were ever built, so a stop-only service builds none. */ + private var rang = false + override fun onBind(intent: Intent?): IBinder? = null override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int { + val previousStart = lastStartId + lastStartId = startId if (intent?.action == ACTION_STOP) { - stop(intent, startId) + stop(intent) return START_NOT_STICKY } - if (intent?.action == ACTION_RING) { + // A redelivered start was never counted by this process. + if (intent?.action == ACTION_RING && flags and START_FLAG_REDELIVERY == 0) { alerts.pendingStarts.updateAndGet { (it - 1).coerceAtLeast(0) } } val alert = intent?.let(ReminderActionReceiver::alertFrom) val notification = intent?.let { IntentCompat.getParcelableExtra(it, EXTRA_NOTIFICATION, Notification::class.java) } if (alert == null || notification == null) { - if (alerts.ringing == null) stopSelfResult(startId) + stopIfIdle() return START_NOT_STICKY } val previous = alerts.ringing @@ -87,49 +94,83 @@ class ReminderRingService : Service() { Log.w(TAG, "Could not ring for event ${alert.eventId}", foreground.exceptionOrNull()) notificationId = shown handBack(alert, quiet = false) - if (alerts.ringing == null) stopSelfResult(startId) + stopIfIdle() return START_NOT_STICKY } if (replacing) NotificationManagerCompat.from(this).cancel(shown) // Only the newest start is redelivered after a process death; the older // ones are done, so a handled reminder never rings again. - lastStartId?.let { stopSelfResult(it) } - lastStartId = startId - if (previous?.key == alert.key && timeout?.isActive == true) return START_REDELIVER_INTENT + previousStart?.let { stopSelfResult(it) } + if (previous?.key == alert.key) return START_REDELIVER_INTENT alerts.ringing = alert - if (previous != null && previous.occurrenceKey != alert.occurrenceKey) handBack(previous) - ring() + if (previous != null) { + // Its screen closes either way; another occurrence goes back to the shade. + if (previous.occurrenceKey != alert.occurrenceKey) handBack(previous) else alerts.markHandled(previous) + } + val since = intent.getLongExtra(EXTRA_SINCE, System.currentTimeMillis()) + ring(alert, remaining = since + RING_TIMEOUT_MILLIS - System.currentTimeMillis()) return START_REDELIVER_INTENT } override fun onDestroy() { - releaseRing() + if (rang) releaseRing() alerts.ringing = null scope.cancel() super.onDestroy() } - private fun ring() { + /** Rings for what is left of the ring's time, so a ring restarted after a process death does not begin again. */ + private fun ring(alert: ReminderAlert, remaining: Long) { releaseRing() + rang = true wakeLock.acquire() // Vibration first: the sound may take a moment to open. vibrator.start() - // No alarm tone over a phone call; the vibration says enough. - if (!isInCall()) ringing = scope.launch { tones.play(scope, ringSources()) } + ringing = scope.launch { followCalls() } timeout = scope.launch { - delay(RING_TIMEOUT_MILLIS) + delay(remaining.coerceAtLeast(0L)) giveUp() } + // Settings changed while this ring was on its way apply to it too. + scope.launch { + val verdict = try { + withContext(Dispatchers.IO) { notifier.ringVerdict(alert) } + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + Log.w(TAG, "Could not re-check the ring for event ${alert.eventId}", e) + RingVerdict.Ring + } + if (alerts.ringing?.key != alert.key || verdict == RingVerdict.Ring) return@launch + if (verdict == RingVerdict.HandBack) handBack(alert) + end() + stopIfIdle() + } } - private fun stop(intent: Intent, startId: Int) { + /** The tone, but never over a phone call: it stops for one and comes back after. */ + private suspend fun followCalls() { + var playing = false + while (true) { + val inCall = isInCall() + if (inCall && playing) { + tones.stop() + playing = false + } else if (!inCall && !playing) { + tones.play(scope, ringSources()) + playing = true + } + delay(CALL_CHECK_MILLIS) + } + } + + private fun stop(intent: Intent) { val current = alerts.ringing if (current != null && matchFrom(intent).matches(current)) { if (intent.getBooleanExtra(EXTRA_HAND_BACK, false)) handBack(current) end() } - // Declines while a newer start is still on its way. - if (alerts.ringing == null) stopSelfResult(startId) + stopIfIdle() } /** Rung long enough: back to the shade quietly. */ @@ -137,24 +178,32 @@ class ReminderRingService : Service() { timeout = null alerts.ringing?.let(::handBack) end() - lastStartId?.let { stopSelfResult(it) } + stopIfIdle() } + /** Ends the ring and closes its screen. */ private fun end() { + alerts.ringing?.let(alerts::markHandled) releaseRing() alerts.ringing = null stopForeground(STOP_FOREGROUND_REMOVE) } + /** Declines while a newer command is still on its way. */ + private fun stopIfIdle() { + if (alerts.ringing == null) lastStartId?.let { stopSelfResult(it) } + } + /** * [alert] back in the shade, in the app's scope so a service stopped - * meanwhile cannot cancel it, and its alert screen closed. + * meanwhile cannot cancel it, and its alert screen closed. A newer reminder + * for the occurrence that took its place meanwhile is left alone. */ private fun handBack(alert: ReminderAlert, quiet: Boolean = true) { alerts.markHandled(alert) appScope.launch { try { - notifier.post(alert, quiet = quiet, ring = false) + if (!notifier.isSuperseded(alert)) notifier.post(alert, quiet = quiet, ring = false) } catch (e: CancellationException) { throw e } catch (e: Exception) { @@ -173,11 +222,10 @@ class ReminderRingService : Service() { wakeLock.release() } - /** The device's alarm sound, then the built-in ones; none opening leaves the vibration. */ + /** The device's alarm sound; "None", or none opening, leaves the vibration. */ private fun ringSources() = listOfNotNull( RingtoneManager.getActualDefaultRingtoneUri(this, RingtoneManager.TYPE_ALARM), RingtoneManager.getDefaultUri(RingtoneManager.TYPE_ALARM), - RingtoneManager.getDefaultUri(RingtoneManager.TYPE_NOTIFICATION), ) companion object { @@ -185,6 +233,8 @@ class ReminderRingService : Service() { private const val ACTION_RING = "de.jeanlucmakiola.calendula.reminders.RING" private const val ACTION_STOP = "de.jeanlucmakiola.calendula.reminders.RING_STOP" private const val EXTRA_NOTIFICATION = "notification" + private const val EXTRA_SINCE = "since" + private const val CALL_CHECK_MILLIS = 1_000L private const val EXTRA_MATCH_KIND = "match_kind" private const val EXTRA_MATCH_VALUE = "match_value" private const val EXTRA_HAND_BACK = "hand_back" @@ -206,6 +256,7 @@ class ReminderRingService : Service() { .setAction(ACTION_RING) .putAlert(alert) .putExtra(EXTRA_NOTIFICATION, notification) + .putExtra(EXTRA_SINCE, System.currentTimeMillis()) alerts.pendingStarts.incrementAndGet() return try { RingWakeLock.handOff(context, WAKE_LOCK_TAG) @@ -225,7 +276,7 @@ class ReminderRingService : Service() { * still on its way; sent while the ring's foreground service keeps the * app allowed to. */ - fun stop(context: Context, match: RingMatch, handBack: Boolean = false) { + fun stop(context: Context, alerts: FullScreenAlerts, match: RingMatch, handBack: Boolean = false) { val (kind, value) = when (match) { is RingMatch.Key -> 0 to match.key is RingMatch.Occurrence -> 1 to match.occurrenceKey @@ -241,8 +292,10 @@ class ReminderRingService : Service() { context.startService(intent) } catch (e: Exception) { Log.w(TAG, "Could not stop the ring", e) - // Refused from the background: nothing must ring on regardless. + // Refused from the background: nothing must ring on regardless, + // and the starts this drops will never arrive. context.stopService(Intent(context, ReminderRingService::class.java)) + alerts.pendingStarts.set(0) } } diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/reminders/ReminderAlertActivity.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/reminders/ReminderAlertActivity.kt index 5398e598..e132b4d3 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/reminders/ReminderAlertActivity.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/reminders/ReminderAlertActivity.kt @@ -127,7 +127,8 @@ class ReminderAlertActivity : ComponentActivity() { return } alert = next - val ringing = alerts.ringing?.key == next.key + // A ring still going, this one's or another's, keeps the screen up as long as it rings. + val ringing = alerts.ringing != null keepScreenOn(if (ringing) ReminderRingService.RING_TIMEOUT_MILLIS else ONCE_SCREEN_ON_MILLIS) loading?.cancel() state = ReminderAlertUiState.Loading diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/SettingsViewModel.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/SettingsViewModel.kt index 763d5326..265a22df 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/SettingsViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/SettingsViewModel.kt @@ -378,10 +378,12 @@ class SettingsViewModel @Inject constructor( } fun setCalendarAlertStyle(calendarId: Long, style: ReminderAlertStyle) { - save { setCalendarAlertStyle(calendarId, style) } - // A ring for it goes quiet but stays in the shade. - if (style != ReminderAlertStyle.FullScreenRing) { - reminderNotifier.stopRing(RingMatch.Calendar(calendarId), handBack = true) + viewModelScope.launch { + prefs.setCalendarAlertStyle(calendarId, style) + // A ring for it goes quiet but stays in the shade. + if (style != ReminderAlertStyle.FullScreenRing) { + reminderNotifier.stopRing(RingMatch.Calendar(calendarId), handBack = true) + } } } diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 45ac53f0..ceb3ae50 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -425,8 +425,11 @@ from the screen, the notification or a swipe, all go through `ReminderActionReceiver`, so the ring stops and the screen closes (`FullScreenAlerts`) however the reminder was handled. Starts and stops reach the service as commands, so a dismiss sent while a newer ring is on its way -applies in order rather than killing it. A blocked calendar, account group or -ring channel never rings, and during a call the ring only vibrates. When Android 14+ +applies in order rather than killing it, and the service re-reads the settings +once a ring arrives, so a style change or mute that raced a scan still wins. A +blocked calendar, account group or ring channel never rings, the tone pauses +for a phone call, and a ring restarted after a process death only rings out +what was left of its ten minutes. When Android 14+ withholds `USE_FULL_SCREEN_INTENT`, the intent is left off and the alert is a heads-up; Settings shows a row leading to the system switch. When Android refuses the ring service, the reminder falls back to sounding once.