fix(ios): keep tick/chime cadence synchronized with timer#207
Conversation
Use a persistent warm AVAudioEngine with interruption and lifecycle resume handling so tick/chime playback timing tracks timer progression across start, pause/resume, and app state transitions. Made-with: Cursor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 3 minutes and 30 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds iOS audio warm-up functionality to synchronize audio cues with timer display progression. Changes include a persistent AVAudioEngine with lifecycle management, sample rate synchronization for audio callbacks, SessionViewModel integration of warm-up initialization, and .gitignore updates for build artifacts. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App Lifecycle
participant SVM as SessionViewModel
participant AE as AudioEngine
participant AVS as AVAudioSession
participant Engine as AVAudioEngine
App->>SVM: start()
SVM->>SVM: update startDate
SVM->>AE: warmUp()
AE->>AVS: configure session
AE->>Engine: ensureEngineRunning()
Engine->>Engine: start on serial queue
AE->>AE: register lifecycle observers<br/>(interruptions, background/foreground)
AE-->>SVM: warmUp() complete
SVM->>SVM: start timer/scheduling
Note over AE,Engine: Later: Audio Playback
SVM->>AE: playSynthesized()
AE->>Engine: query mixer sampleRate
AE->>AE: generator callback<br/>with active sampleRate
AE->>Engine: connect source node
Engine->>Engine: start (if needed)
Engine-->>AE: render audio frames
AE->>Engine: disconnect/detach source
App->>AE: background transition
AE->>Engine: pause conditionally
App->>AE: foreground transition
AE->>Engine: resume conditionally
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📋 Issue PlannerBuilt with CodeRabbit's Coding Plans for faster development and fewer bugs. View plan used: ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ios/StillPointApp/ViewModels/SessionViewModel.swift (1)
108-113: Skip warm-up when all session sounds are disabled.Line 111 spins up the shared audio engine even when
tick,chime, andcompletionare all off, which still pays the audio-session/lifecycle cost for a silent session. A small guard keeps the latency fix for audible sessions without doing unnecessary work here.Suggested change
isActive = true isPaused = false startDate = Date().addingTimeInterval(-pausedElapsed) - AudioEngine.shared.warmUp() + if soundPrefs.tick || soundPrefs.chime || soundPrefs.completion { + AudioEngine.shared.warmUp() + } startTimer() scheduleControlHide()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/StillPointApp/ViewModels/SessionViewModel.swift` around lines 108 - 113, Wrap the AudioEngine.shared.warmUp() call in a guard that only runs it when at least one of the session sound flags (tick, chime, completion) is enabled; e.g., check the booleans named tick, chime, and completion and call AudioEngine.shared.warmUp() only if tick || chime || completion is true, leaving isActive, isPaused, startDate, startTimer(), and scheduleControlHide() as-is.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ios/StillPointShared/Sources/StillPointShared/AudioEngine.swift`:
- Around line 247-259: The foreground restart must be gated on whether the
engine was running before backgrounding or an interruption indicated it should
resume: add a stored Bool (e.g., wasRunningBeforeBackground) and in
handleDidEnterBackground() set wasRunningBeforeBackground =
self.engine.isRunning before calling self.engine.pause(); then in
handleWillEnterForeground() only call self.ensureEngineRunning() if
wasRunningBeforeBackground || self.pendingResumeAfterConfigurationChange is true
(still call configureAudioSession() unconditionally), and clear/reset
wasRunningBeforeBackground appropriately after using it; update references to
self and weak captures as needed in handleDidEnterBackground and
handleWillEnterForeground to use this new flag.
---
Nitpick comments:
In `@ios/StillPointApp/ViewModels/SessionViewModel.swift`:
- Around line 108-113: Wrap the AudioEngine.shared.warmUp() call in a guard that
only runs it when at least one of the session sound flags (tick, chime,
completion) is enabled; e.g., check the booleans named tick, chime, and
completion and call AudioEngine.shared.warmUp() only if tick || chime ||
completion is true, leaving isActive, isPaused, startDate, startTimer(), and
scheduleControlHide() as-is.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d1d0d14a-c021-42a8-a93d-74605dcc7d9a
📒 Files selected for processing (3)
.gitignoreios/StillPointApp/ViewModels/SessionViewModel.swiftios/StillPointShared/Sources/StillPointShared/AudioEngine.swift
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Only warm the audio engine when session sounds are enabled, and only restart playback on foreground if the engine was active before background or has a pending interruption resume. Made-with: Cursor
Mirror generated samples across every channel buffer from AVAudioSourceNode so tick/chime playback remains audible and consistent on stereo output formats. Made-with: Cursor
|
@cursor review |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit ecf2b17. Configure here.
| } | ||
| self.wasRunningBeforeBackground = false | ||
| } | ||
| } |
There was a problem hiding this comment.
Stale flag not cleared after foreground engine restart
Low Severity
handleWillEnterForeground uses pendingResumeAfterConfigurationChange to decide whether to restart the engine, but unlike resumeAfterInterruptionIfNeeded, it never clears the flag after a successful restart. This leaves the flag permanently stale, causing handleEngineConfigurationChange to unnecessarily call configureAudioSession() and ensureEngineRunning() on every future configuration-change notification.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit ecf2b17. Configure here.
1 participant
Summary
AudioEngineto keep a persistent warmAVAudioEnginealive instead of creating a new engine for each tick/chime playbackios/StillPointShared/.build/artifacts to keep local SwiftPM outputs out of review/PR noiseCloses #201
Test plan
swift test --package-path ios/StillPointSharedcoderabbit review --prompt-only(clean pass)coderabbit review --prompt-only(second clean pass)Made with Cursor
Summary by CodeRabbit
Bug Fixes
Chores