mirror of
https://github.com/avinal/nikki.git
synced 2026-07-03 21:40:09 +05:30
Fix 5 notification reliability issues
1. Remove println debug statement from ReminderScheduler 2. Drop scheduledIds tracking — AlarmManager deduplicates via FLAG_UPDATE_CURRENT, and the tracking prevented rescheduling when a task's due time was edited 3. TaskCheckWorker uses live app DB via liveMemosProvider when available, falls back to opening its own DB only when the app process isn't running (boot receiver, background check) 4. Default notify time synced from DataStore to SharedPreferences via syncNotifyTime() — both DirectAlarmScheduler and TaskCheckWorker now read from the same source via readDefaultNotifyTime() helper 5. TaskReminderReceiver triggers runTaskCheckNow() after each alarm fires, so the next alarm is scheduled immediately instead of waiting up to 15 min for WorkManager Signed-off-by: Avinal Kumar <avinal.xlvii@gmail.com> Co-Authored-By: Claude Opus 4.6 (1M context)
This commit is contained in:
@@ -4,6 +4,7 @@ import android.content.BroadcastReceiver
|
|||||||
import android.content.Context
|
import android.content.Context
|
||||||
import android.content.Intent
|
import android.content.Intent
|
||||||
import com.avinal.memos.notifications.TaskNotificationManager
|
import com.avinal.memos.notifications.TaskNotificationManager
|
||||||
|
import com.avinal.memos.notifications.runTaskCheckNow
|
||||||
|
|
||||||
class TaskReminderReceiver : BroadcastReceiver() {
|
class TaskReminderReceiver : BroadcastReceiver() {
|
||||||
override fun onReceive(context: Context, intent: Intent) {
|
override fun onReceive(context: Context, intent: Intent) {
|
||||||
@@ -19,5 +20,6 @@ class TaskReminderReceiver : BroadcastReceiver() {
|
|||||||
dueLabel = dueLabel,
|
dueLabel = dueLabel,
|
||||||
priority = priority,
|
priority = priority,
|
||||||
)
|
)
|
||||||
|
runTaskCheckNow(context)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
+2
-6
@@ -7,7 +7,6 @@ import android.content.Intent
|
|||||||
import com.avinal.memos.domain.Memo
|
import com.avinal.memos.domain.Memo
|
||||||
import com.avinal.memos.parser.TaskParser
|
import com.avinal.memos.parser.TaskParser
|
||||||
import kotlin.time.Clock
|
import kotlin.time.Clock
|
||||||
import kotlinx.datetime.LocalTime
|
|
||||||
import kotlinx.datetime.TimeZone
|
import kotlinx.datetime.TimeZone
|
||||||
|
|
||||||
object DirectAlarmScheduler {
|
object DirectAlarmScheduler {
|
||||||
@@ -17,12 +16,9 @@ object DirectAlarmScheduler {
|
|||||||
val nowMillis = Clock.System.now().toEpochMilliseconds()
|
val nowMillis = Clock.System.now().toEpochMilliseconds()
|
||||||
val tz = TimeZone.currentSystemDefault()
|
val tz = TimeZone.currentSystemDefault()
|
||||||
|
|
||||||
val prefs = context.getSharedPreferences("memos_prefs", Context.MODE_PRIVATE)
|
val defaultTime = readDefaultNotifyTime(context)
|
||||||
val defaultTimeStr = prefs.getString("default_notify_time", "20:00") ?: "20:00"
|
|
||||||
val parts = defaultTimeStr.split(":")
|
|
||||||
val defaultTime = try { LocalTime(parts[0].toInt(), parts.getOrElse(1) { "0" }.toInt()) } catch (_: Exception) { LocalTime(20, 0) }
|
|
||||||
|
|
||||||
val alarms = ReminderScheduler.computeAlarms(allTasks, nowMillis, tz, emptySet(), defaultTime)
|
val alarms = ReminderScheduler.computeAlarms(allTasks, nowMillis, tz, defaultTime)
|
||||||
val alarmManager = context.getSystemService(Context.ALARM_SERVICE) as AlarmManager
|
val alarmManager = context.getSystemService(Context.ALARM_SERVICE) as AlarmManager
|
||||||
|
|
||||||
alarms.forEach { alarm ->
|
alarms.forEach { alarm ->
|
||||||
|
|||||||
@@ -0,0 +1,20 @@
|
|||||||
|
package com.avinal.memos.notifications
|
||||||
|
|
||||||
|
import android.content.Context
|
||||||
|
import kotlinx.datetime.LocalTime
|
||||||
|
|
||||||
|
fun readDefaultNotifyTime(context: Context): LocalTime {
|
||||||
|
val prefs = context.getSharedPreferences("nikki_notify", Context.MODE_PRIVATE)
|
||||||
|
val timeStr = prefs.getString("default_notify_time", "20:00") ?: "20:00"
|
||||||
|
val parts = timeStr.split(":")
|
||||||
|
return try {
|
||||||
|
LocalTime(parts[0].toInt(), parts.getOrElse(1) { "0" }.toInt())
|
||||||
|
} catch (_: Exception) {
|
||||||
|
LocalTime(20, 0)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fun writeDefaultNotifyTime(context: Context, time: String) {
|
||||||
|
context.getSharedPreferences("nikki_notify", Context.MODE_PRIVATE)
|
||||||
|
.edit().putString("default_notify_time", time).apply()
|
||||||
|
}
|
||||||
+18
-35
@@ -21,20 +21,26 @@ class TaskCheckWorker(
|
|||||||
) : CoroutineWorker(appContext, params) {
|
) : CoroutineWorker(appContext, params) {
|
||||||
|
|
||||||
override suspend fun doWork(): Result {
|
override suspend fun doWork(): Result {
|
||||||
val prefs = appContext.getSharedPreferences("task_notifications", Context.MODE_PRIVATE)
|
|
||||||
val scheduledIds = prefs.getStringSet("scheduled_ids", emptySet()) ?: emptySet()
|
|
||||||
val nowMillis = Clock.System.now().toEpochMilliseconds()
|
val nowMillis = Clock.System.now().toEpochMilliseconds()
|
||||||
|
val defaultTime = readDefaultNotifyTime(appContext)
|
||||||
|
|
||||||
// Read default notify time from DataStore file (shared prefs fallback)
|
val memos = com.avinal.memos.util.liveMemosProvider?.invoke()
|
||||||
val notifyPrefs = appContext.getSharedPreferences("memos_prefs", Context.MODE_PRIVATE)
|
?: readMemosFromDb()
|
||||||
val defaultTimeStr = notifyPrefs.getString("default_notify_time", "20:00") ?: "20:00"
|
|
||||||
val defaultTimeParts = defaultTimeStr.split(":")
|
val allTasks = memos.flatMap { memo -> TaskParser.extractTasks(memo.id, memo.content, memo.tags) }
|
||||||
val defaultTime = try {
|
val alarmManager = appContext.getSystemService(Context.ALARM_SERVICE) as AlarmManager
|
||||||
kotlinx.datetime.LocalTime(defaultTimeParts[0].toInt(), defaultTimeParts.getOrElse(1) { "0" }.toInt())
|
val tz = TimeZone.currentSystemDefault()
|
||||||
} catch (_: Exception) {
|
|
||||||
kotlinx.datetime.LocalTime(20, 0)
|
val alarms = ReminderScheduler.computeAlarms(allTasks, nowMillis, tz, defaultTime)
|
||||||
|
|
||||||
|
alarms.forEach { alarm ->
|
||||||
|
scheduleAlarm(alarmManager, alarm.taskId, alarm.taskText, alarm.label, alarm.triggerAtMillis, alarm.priority)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
return Result.success()
|
||||||
|
}
|
||||||
|
|
||||||
|
private suspend fun readMemosFromDb(): List<com.avinal.memos.domain.Memo> {
|
||||||
val db = Room.databaseBuilder<MemosDatabase>(
|
val db = Room.databaseBuilder<MemosDatabase>(
|
||||||
context = appContext,
|
context = appContext,
|
||||||
name = appContext.getDatabasePath("memos.db").absolutePath,
|
name = appContext.getDatabasePath("memos.db").absolutePath,
|
||||||
@@ -44,33 +50,11 @@ class TaskCheckWorker(
|
|||||||
.setQueryCoroutineContext(Dispatchers.IO)
|
.setQueryCoroutineContext(Dispatchers.IO)
|
||||||
.build()
|
.build()
|
||||||
|
|
||||||
try {
|
return try {
|
||||||
val memos = db.memoDao().getAll().map { it.toDomain() }
|
db.memoDao().getAll().map { it.toDomain() }
|
||||||
val allTasks = memos.flatMap { memo -> TaskParser.extractTasks(memo.id, memo.content, memo.tags) }
|
|
||||||
val alarmManager = appContext.getSystemService(Context.ALARM_SERVICE) as AlarmManager
|
|
||||||
val tz = TimeZone.currentSystemDefault()
|
|
||||||
|
|
||||||
val alarms = ReminderScheduler.computeAlarms(allTasks, nowMillis, tz, scheduledIds, defaultTime)
|
|
||||||
|
|
||||||
alarms.forEach { alarm ->
|
|
||||||
scheduleAlarm(alarmManager, alarm.taskId, alarm.taskText, alarm.label, alarm.triggerAtMillis, alarm.priority)
|
|
||||||
}
|
|
||||||
|
|
||||||
val newScheduledIds = scheduledIds.toMutableSet()
|
|
||||||
alarms.forEach { newScheduledIds.add(it.taskId) }
|
|
||||||
|
|
||||||
val activeTaskIds = allTasks.filter { !it.isCompleted }.map { it.id }.toSet()
|
|
||||||
val cleaned = newScheduledIds.filter { id ->
|
|
||||||
val baseId = id.removeSuffix("_am").removeSuffix("_pm").removeSuffix("_remind")
|
|
||||||
baseId in activeTaskIds
|
|
||||||
}.toSet()
|
|
||||||
|
|
||||||
prefs.edit().putStringSet("scheduled_ids", cleaned).apply()
|
|
||||||
} finally {
|
} finally {
|
||||||
db.close()
|
db.close()
|
||||||
}
|
}
|
||||||
|
|
||||||
return Result.success()
|
|
||||||
}
|
}
|
||||||
|
|
||||||
private fun scheduleAlarm(
|
private fun scheduleAlarm(
|
||||||
@@ -103,7 +87,6 @@ class TaskCheckWorker(
|
|||||||
try {
|
try {
|
||||||
alarmManager.setExactAndAllowWhileIdle(AlarmManager.RTC_WAKEUP, triggerAtMillis, pendingIntent)
|
alarmManager.setExactAndAllowWhileIdle(AlarmManager.RTC_WAKEUP, triggerAtMillis, pendingIntent)
|
||||||
} catch (_: SecurityException) {
|
} catch (_: SecurityException) {
|
||||||
// Fallback if exact alarm permission not granted
|
|
||||||
alarmManager.set(AlarmManager.RTC_WAKEUP, triggerAtMillis, pendingIntent)
|
alarmManager.set(AlarmManager.RTC_WAKEUP, triggerAtMillis, pendingIntent)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
+7
-1
@@ -6,12 +6,18 @@ import kotlinx.coroutines.CoroutineScope
|
|||||||
import kotlinx.coroutines.Dispatchers
|
import kotlinx.coroutines.Dispatchers
|
||||||
import kotlinx.coroutines.launch
|
import kotlinx.coroutines.launch
|
||||||
|
|
||||||
private var liveMemosProvider: (() -> List<com.avinal.memos.domain.Memo>)? = null
|
var liveMemosProvider: (() -> List<com.avinal.memos.domain.Memo>)? = null
|
||||||
|
private set
|
||||||
|
|
||||||
actual fun setLiveMemosProvider(provider: () -> List<com.avinal.memos.domain.Memo>) {
|
actual fun setLiveMemosProvider(provider: () -> List<com.avinal.memos.domain.Memo>) {
|
||||||
liveMemosProvider = provider
|
liveMemosProvider = provider
|
||||||
}
|
}
|
||||||
|
|
||||||
|
actual fun syncNotifyTime(time: String) {
|
||||||
|
val ctx = appContext ?: return
|
||||||
|
com.avinal.memos.notifications.writeDefaultNotifyTime(ctx, time)
|
||||||
|
}
|
||||||
|
|
||||||
actual fun triggerReminderCheck() {
|
actual fun triggerReminderCheck() {
|
||||||
val ctx = appContext ?: return
|
val ctx = appContext ?: return
|
||||||
val memos = liveMemosProvider?.invoke()
|
val memos = liveMemosProvider?.invoke()
|
||||||
|
|||||||
@@ -14,6 +14,7 @@ import kotlinx.coroutines.IO
|
|||||||
import kotlinx.coroutines.launch
|
import kotlinx.coroutines.launch
|
||||||
import kotlinx.coroutines.runBlocking
|
import kotlinx.coroutines.runBlocking
|
||||||
import com.avinal.memos.db.entity.toDomain
|
import com.avinal.memos.db.entity.toDomain
|
||||||
|
import com.avinal.memos.util.syncNotifyTime
|
||||||
|
|
||||||
class AppDependencies(
|
class AppDependencies(
|
||||||
dataStorePath: String,
|
dataStorePath: String,
|
||||||
@@ -55,6 +56,7 @@ class AppDependencies(
|
|||||||
launch { tokenStore.accessToken.collect { cachedToken = it } }
|
launch { tokenStore.accessToken.collect { cachedToken = it } }
|
||||||
launch { tokenStore.serverUrl.collect { cachedServerUrl = it } }
|
launch { tokenStore.serverUrl.collect { cachedServerUrl = it } }
|
||||||
launch { tokenStore.syncInterval.collect { memoRepository.syncIntervalMinutes = it } }
|
launch { tokenStore.syncInterval.collect { memoRepository.syncIntervalMinutes = it } }
|
||||||
|
launch { tokenStore.defaultNotifyTime.collect { syncNotifyTime(it) } }
|
||||||
launch { initializeLiveMemosProvider() }
|
launch { initializeLiveMemosProvider() }
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -34,7 +34,6 @@ object ReminderScheduler {
|
|||||||
tasks: List<Task>,
|
tasks: List<Task>,
|
||||||
nowMillis: Long,
|
nowMillis: Long,
|
||||||
timeZone: TimeZone,
|
timeZone: TimeZone,
|
||||||
alreadyScheduledIds: Set<String> = emptySet(),
|
|
||||||
defaultTime: LocalTime = LocalTime(20, 0),
|
defaultTime: LocalTime = LocalTime(20, 0),
|
||||||
): List<ScheduledAlarm> {
|
): List<ScheduledAlarm> {
|
||||||
val alarms = mutableListOf<ScheduledAlarm>()
|
val alarms = mutableListOf<ScheduledAlarm>()
|
||||||
@@ -48,8 +47,6 @@ object ReminderScheduler {
|
|||||||
|
|
||||||
val dueMs = effectiveDate.atTime(effectiveTime).toInstant(timeZone).toEpochMilliseconds()
|
val dueMs = effectiveDate.atTime(effectiveTime).toInstant(timeZone).toEpochMilliseconds()
|
||||||
|
|
||||||
println("ReminderScheduler: task=${task.text} effectiveDate=$effectiveDate effectiveTime=$effectiveTime dueMs=$dueMs nowMillis=$nowMillis future=${dueMs > nowMillis}")
|
|
||||||
|
|
||||||
// Explicit reminder: fire at dueDateTime - duration
|
// Explicit reminder: fire at dueDateTime - duration
|
||||||
if (task.reminder != null) {
|
if (task.reminder != null) {
|
||||||
val offsetMs = durationToMillis(task.reminder)
|
val offsetMs = durationToMillis(task.reminder)
|
||||||
|
|||||||
@@ -2,3 +2,4 @@ package com.avinal.memos.util
|
|||||||
|
|
||||||
expect fun triggerReminderCheck()
|
expect fun triggerReminderCheck()
|
||||||
expect fun setLiveMemosProvider(provider: () -> List<com.avinal.memos.domain.Memo>)
|
expect fun setLiveMemosProvider(provider: () -> List<com.avinal.memos.domain.Memo>)
|
||||||
|
expect fun syncNotifyTime(time: String)
|
||||||
|
|||||||
@@ -29,7 +29,7 @@ class ReminderSchedulerTest {
|
|||||||
reminder = reminder, priority = priority)
|
reminder = reminder, priority = priority)
|
||||||
|
|
||||||
private fun compute(tasks: List<Task>, now: Long = nowMillis) =
|
private fun compute(tasks: List<Task>, now: Long = nowMillis) =
|
||||||
ReminderScheduler.computeAlarms(tasks, now, tz, emptySet(), defaultTime)
|
ReminderScheduler.computeAlarms(tasks, now, tz, defaultTime)
|
||||||
|
|
||||||
@Test fun completedNoAlarms() { assertTrue(compute(listOf(task(completed = true))).isEmpty()) }
|
@Test fun completedNoAlarms() { assertTrue(compute(listOf(task(completed = true))).isEmpty()) }
|
||||||
@Test fun noDateNoTimeNoAlarms() { assertTrue(compute(listOf(task(date = null, time = null))).isEmpty()) }
|
@Test fun noDateNoTimeNoAlarms() { assertTrue(compute(listOf(task(date = null, time = null))).isEmpty()) }
|
||||||
@@ -128,7 +128,7 @@ class ReminderSchedulerTest {
|
|||||||
|
|
||||||
@Test fun customDefaultTime() {
|
@Test fun customDefaultTime() {
|
||||||
val alarms = ReminderScheduler.computeAlarms(
|
val alarms = ReminderScheduler.computeAlarms(
|
||||||
listOf(task()), nowMillis, tz, emptySet(), LocalTime(9, 0)
|
listOf(task()), nowMillis, tz, LocalTime(9, 0)
|
||||||
)
|
)
|
||||||
val expected = dueDate.atTime(LocalTime(9, 0)).toInstant(tz).toEpochMilliseconds()
|
val expected = dueDate.atTime(LocalTime(9, 0)).toInstant(tz).toEpochMilliseconds()
|
||||||
assertEquals(expected, alarms[0].triggerAtMillis)
|
assertEquals(expected, alarms[0].triggerAtMillis)
|
||||||
|
|||||||
@@ -2,3 +2,4 @@ package com.avinal.memos.util
|
|||||||
|
|
||||||
actual fun triggerReminderCheck() {}
|
actual fun triggerReminderCheck() {}
|
||||||
actual fun setLiveMemosProvider(provider: () -> List<com.avinal.memos.domain.Memo>) {}
|
actual fun setLiveMemosProvider(provider: () -> List<com.avinal.memos.domain.Memo>) {}
|
||||||
|
actual fun syncNotifyTime(time: String) {}
|
||||||
|
|||||||
Reference in New Issue
Block a user