fix(#171): review round 3 - re-check settings in the ring, follow calls, anchored timeout, close the screen on every stop

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

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. */

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)
             }
         }
 

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

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)
+            }
         }
     }
 

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.