Add configurable timer ring duration and repeat reminders - #865
Marcus2626 wants to merge 8 commits into
Conversation
this pic was useless
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe built-in timer now supports configurable repeating alerts with persisted ring-duration and repeat-interval settings. TimerManager coordinates alert phases and sound playback across timer ticks, lifecycle changes, and sleep or wake. A regression executable validates alert timing and behavior, and CI runs it. ChangesBuilt-in timer alerts
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant TimerManager
participant TimerAlertController
participant SoundPlayer
TimerManager->>TimerAlertController: Start alert when timer reaches zero
loop On timer ticks
TimerManager->>TimerAlertController: Update alert phase with configured duration and interval
TimerAlertController-->>TimerManager: Return phase change or no change
TimerManager->>SoundPlayer: Play or stop sound for the phase
end
Merge Risk: ⚪ Minimal · up to The built-in timer gains configurable repeating alerts. No concrete merge-blocking risk was identified in the supplied context, so the change looks ready to merge after normal checks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change is confined to the built-in timer and its existing audio playback. Bounded timing settings, session cleanup, and rejection of obsolete callbacks limit its effects. No new security issue was identified in the reviewed paths, although complete caller and runtime coverage was not established. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
Function-level implementation notesThis PR changes Atoll's built-in timer. On expiry, it rings for the configured duration, remains silent for the configured interval, and repeats until dismissed. Overtime remains visible during silent periods. The defaults are 30 seconds of ringing and 5 minutes of silence. TimerAlertController.swift
TimerManager.swift
Settings and validation
|
Closes #859
Summary
Atoll's built-in timer currently rings continuously after expiry until manually stopped. This change adds a configurable ring duration and silent interval, allowing reminders to repeat while the timer remains in overtime.
For example, with a 30-second ring duration and a 5-minute interval, the timer rings for 30 seconds, stays silent for 5 minutes, then rings again until dismissed.
Changes
TimerManager's one-second countdown tick to advance reminders.TimerAlertControlleronly stores ringing/silent phase state; audio playback remains inTimerManager. The timer uses the common RunLoop mode so updates continue during UI tracking.Timerinstance. Keep cleanup in the existing session and pause methods.The existing public timer method signatures and macOS Clock integration remain unchanged. These settings apply to Atoll's built-in timer, not the Clock app's alarms. Alert transitions are checked once per second and may be delayed if the main thread is busy.
Validation
bash tests/run_timer_alert_regression.shpassed: phase boundaries, live setting changes, replacement, cancellation, simulated sleep/wake, delayed updates, and invalid persisted-value bounds.python3 -m unittest discover -s tests -vpassed: 7 tests, including the existing timer lifecycle regression.git diff --checkpassed for the committed changes.Settings preview
Previously captured settings preview; the same controls are retained in this implementation:
Summary by CodeRabbit