forked from trent_larson/crowd-funder-for-time-pwa
105 lines
6.0 KiB
Markdown
105 lines
6.0 KiB
Markdown
# Plugin feedback: Android rollover interval – two bugs (logcat evidence)
|
||
|
||
**Date:** 2026-03-04
|
||
**Target:** daily-notification-plugin (Android)
|
||
**Feature:** `rolloverIntervalMinutes` (e.g. 10 minutes for testing)
|
||
**Result:** Two bugs prevent rollover notifications from firing every 10 minutes when the user does not open the app.
|
||
|
||
---
|
||
|
||
## Test setup
|
||
|
||
- Schedule set with **rolloverIntervalMinutes=10** (e.g. first run 20:05).
|
||
- Expected: notification at 20:05, 20:15, 20:25, 20:35, 20:40 (after user edit), 20:50, 21:00, 21:10, 21:20, …
|
||
- User did **not** open the app between 20:25 and 20:36, or between 21:10 and 21:20.
|
||
|
||
---
|
||
|
||
## Bug 1: Rollover interval not applied when the firing run is a rollover schedule
|
||
|
||
### Observed
|
||
|
||
- **20:25** – Notification fired (room content; work id UUID, scheduleId `daily_rollover_1772540701872`).
|
||
- **20:35** – **No notification.**
|
||
|
||
### Logcat evidence (20:25 fire)
|
||
|
||
There is **no** `DN|ROLLOVER_INTERVAL` or `DN|ROLLOVER_NEXT using_interval_minutes=10` in this block. Next run is set to **next day** at 20:25, not today 20:35:
|
||
|
||
```
|
||
03-03 20:25:01.844 D/DailyNotificationWorker: DN|RESCHEDULE_START id=29e1e984-d8b2-49ea-bb69-68b923fe4428
|
||
03-03 20:25:01.874 D/DailyNotificationWorker: DN|ROLLOVER next=1772627100000 scheduleId=daily_rollover_1772540701872 static=false
|
||
03-03 20:25:01.928 I/DNP-SCHEDULE: Scheduling next daily alarm: id=daily_rollover_1772540701872, nextRun=2026-03-04 20:25:00, source=ROLLOVER_ON_FIRE
|
||
```
|
||
|
||
Compare with a fire that **does** use the interval (e.g. 20:15):
|
||
|
||
```
|
||
03-03 20:15:01.860 D/DailyNotificationWorker: DN|ROLLOVER_INTERVAL scheduleId=daily_timesafari_reminder minutes=10
|
||
03-03 20:15:01.862 D/DailyNotificationWorker: DN|ROLLOVER_NEXT using_interval_minutes=10 next=1772540700870
|
||
03-03 20:15:01.870 D/DailyNotificationWorker: DN|ROLLOVER next=1772540700870 scheduleId=daily_timesafari_reminder static=false
|
||
```
|
||
|
||
### Root cause
|
||
|
||
When the notification that just fired was scheduled from a **previous** rollover (i.e. work id is UUID / scheduleId is `daily_rollover_*`), the rollover path appears to use **+24 hours** and never reads or applies the stored `rolloverIntervalMinutes`. The interval is only applied when the firing schedule is the main/canonical one (e.g. `daily_timesafari_reminder`).
|
||
|
||
### Required fix
|
||
|
||
When scheduling the next run after a notification fires (rollover path), **always** resolve the **logical** schedule (e.g. map `daily_rollover_*` back to the main schedule id) and read the stored `rolloverIntervalMinutes` for that reminder. If present and > 0, set next trigger = current trigger + that many minutes (using the same logic as the path that already logs `ROLLOVER_INTERVAL` / `ROLLOVER_NEXT`). Only use +24 hours when the interval is absent or 0.
|
||
|
||
---
|
||
|
||
## Bug 2: ROLLOVER_ON_FIRE reschedule skipped as “duplicate” so next alarm is never set
|
||
|
||
### Observed
|
||
|
||
- **21:10** – Notification fired; worker correctly computes next = 21:20 (epoch 1772544000862).
|
||
- **21:20** – **No notification.**
|
||
|
||
### Logcat evidence (21:10 fire)
|
||
|
||
Worker applies interval and requests next at 21:20; schedule layer skips and does **not** set the alarm:
|
||
|
||
```
|
||
03-03 21:10:01.281 D/DailyNotificationWorker: DN|ROLLOVER_INTERVAL scheduleId=daily_timesafari_reminder minutes=10
|
||
03-03 21:10:01.284 D/DailyNotificationWorker: DN|ROLLOVER_NEXT using_interval_minutes=10 next=1772544000862
|
||
03-03 21:10:01.294 D/DailyNotificationWorker: DN|ROLLOVER next=1772544000862 scheduleId=daily_timesafari_reminder static=false
|
||
03-03 21:10:01.313 W/DNP-SCHEDULE: Skipping duplicate schedule: id=daily_timesafari_reminder, nextRun=2026-03-03 21:20:00, source=ROLLOVER_ON_FIRE
|
||
03-03 21:10:01.314 W/DNP-SCHEDULE: Existing PendingIntent found for requestCode=53438 - alarm already scheduled
|
||
03-03 21:10:01.332 I/DailyNotificationWorker: DN|RESCHEDULE_OK ...
|
||
```
|
||
|
||
So the worker reports RESCHEDULE_OK, but the scheduler did **not** call through to set the OS alarm for 21:20. The “existing” PendingIntent was for the alarm that **just fired** (21:10). Idempotence is preventing the **update** to the new trigger time.
|
||
|
||
### Root cause
|
||
|
||
Duplicate/idempotence logic (e.g. “Existing PendingIntent found for requestCode=53438”) is applied in a way that skips scheduling when the same schedule id is used with a **new** trigger time. For `source=ROLLOVER_ON_FIRE`, the same schedule id is **supposed** to be updated to a new trigger time every time a rollover fires. Skipping when only the trigger time changes breaks the rollover chain.
|
||
|
||
### Required fix
|
||
|
||
For `source=ROLLOVER_ON_FIRE`, do **not** skip scheduling when the only “match” is the same schedule id with a **different** `nextRun`/trigger time. Either:
|
||
|
||
- Treat “same schedule id, different trigger time” as an **update**: cancel the existing alarm (or PendingIntent) for that schedule and set the new one for the new trigger time, or
|
||
- In the idempotence check, require that the **existing** alarm’s trigger time equals the **requested** trigger time before skipping; if the requested time is different, proceed with cancel + set.
|
||
|
||
After the fix, when the 21:10 alarm fires and the worker requests next at 21:20, the schedule layer should cancel the 21:10 alarm and set a new alarm for 21:20 (same schedule id, new trigger).
|
||
|
||
---
|
||
|
||
## Desired behavior (for reference)
|
||
|
||
Once both bugs are fixed:
|
||
|
||
- Rollover notifications should keep being scheduled every `rolloverIntervalMinutes` (e.g. 10 minutes) **without the user opening the app** between fires.
|
||
- Flow: alarm fires → Receiver → Worker (display + reschedule) → schedule layer sets next alarm. All of this runs when the alarm fires; no app launch required.
|
||
|
||
---
|
||
|
||
## Summary table
|
||
|
||
| Time | Expected | Actual | Bug |
|
||
|--------|------------------------|---------------|-----|
|
||
| 20:35 | Rollover notification | No notification | **Bug 1:** Rollover from `daily_rollover_*` path uses +24h instead of `rolloverIntervalMinutes`. |
|
||
| 21:20 | Rollover notification | No notification | **Bug 2:** Schedule layer skips with “Skipping duplicate schedule” / “Existing PendingIntent found”; 21:20 alarm never set. |
|