Poll now-playing metadata for radio stations#151
Merged
Conversation
Fetch stream_title from the API every 15 seconds while a radio station is playing. Updates both the in-app stream title and the OS media session (lock screen / Dynamic Island) — when a stream title is available, it becomes the MediaItem title and the station name moves to artist. Polling stops on station stop/switch/dispose. API failures are logged silently without affecting playback. Stale responses are discarded if the station changed between request and response.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdded a 15-second periodic polling mechanism to fetch "now playing" metadata while radio playback is active; polling updates the provider's Changes
Sequence DiagramsequenceDiagram
participant User
participant Provider as RadioPlayerProvider
participant Timer as Timer/Scheduler
participant API as Radio API
participant Handler as AudioHandler
User->>Provider: play(station)
activate Provider
Provider->>Provider: start playback succeeds
Provider->>Timer: _startNowPlayingPolling(station)
activate Timer
Timer->>API: _fetchNowPlaying(station) [immediate]
activate API
API-->>Provider: { stream_title }
deactivate API
Provider->>Handler: update mediaItem (title/artist)
Provider->>Provider: notifyListeners()
Note over Timer: wait 15s
Timer->>API: _fetchNowPlaying(station) [periodic]
activate API
API-->>Provider: { stream_title }
deactivate API
Provider->>Handler: update mediaItem (if changed)
Provider->>Provider: notifyListeners()
User->>Provider: stop()
Provider->>Timer: _stopNowPlayingPolling()
deactivate Timer
deactivate Provider
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
test/ui/screens/radio_stations_test.dart (1)
138-186: Consider adding tests for polling lifecycle behavior.The current tests cover
mediaItemForStationwell, but there's no test coverage for:
- Polling starts after
play()completes- Polling stops on
stop()or station switch- Stale responses are discarded when station changes mid-request
These behaviors are mentioned in the PR objectives but aren't validated by tests. This would require mocking the API and possibly using fake timers.
Would you like me to help draft integration tests for the polling lifecycle? This could use
FakeAsyncto control timer advancement and mock HTTP responses.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/ui/screens/radio_stations_test.dart` around lines 138 - 186, Add tests in radio_stations_test.dart that validate the polling lifecycle: verify polling begins only after RadioPlayerProvider.play() completes, verify polling stops on RadioPlayerProvider.stop() and when switching stations, and verify stale HTTP responses are ignored if the station changes mid-request. Implement these using FakeAsync to control timers and a mocked HTTP client (or mock API) to return controllable responses; advance timers to trigger polling and assert that the mocked client is called (or not) as expected, and assert that state updates correspond to the latest station (i.e., stale responses do not overwrite current station metadata). Target tests to exercise RadioPlayerProvider.play(), RadioPlayerProvider.stop(), and whatever internal polling method (e.g., _poll or poll loop) by observing side-effects such as calls to the mocked API, media item updates, and polling cancellation when switching stations.lib/providers/radio_player_provider.dart (1)
149-168: Consider whether paused state should continue polling.The polling continues during pause (only
stop()cancels it). This matches the PR description but means metadata updates occur even when paused. If this is intentional, it's fine—just confirming the design choice.Additionally, the stale response guard at line 155 correctly discards responses when the station changed mid-request.
One minor observation:
togglePlayPause()(lines 128-134) doesn't affect polling. If the user pauses a radio station, polling continues. Consider whether pausing should suspend polling to reduce unnecessary network requests:💡 Optional: Pause polling when playback is paused
Future<void> togglePlayPause() async { if (_playing) { + _stopNowPlayingPolling(); await _player.pause(); } else if (_currentStation != null) { await _player.play(); + _startNowPlayingPolling(_currentStation!); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/providers/radio_player_provider.dart` around lines 149 - 168, Polling keeps running when playback is paused because togglePlayPause() doesn't change the polling state; update togglePlayPause() to suspend metadata polling when pausing and resume when playing by either toggling the existing active flag the _fetchNowPlaying() guard uses or cancelling/creating the polling Timer (the inverse of stop()). Specifically, in togglePlayPause() set active = false (or cancel _pollingTimer) when pausing and set active = true (or restart the poller) when resuming so _fetchNowPlaying() will early-return for paused playback; ensure stop() still fully cancels the poller.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@lib/providers/radio_player_provider.dart`:
- Around line 149-168: Polling keeps running when playback is paused because
togglePlayPause() doesn't change the polling state; update togglePlayPause() to
suspend metadata polling when pausing and resume when playing by either toggling
the existing active flag the _fetchNowPlaying() guard uses or
cancelling/creating the polling Timer (the inverse of stop()). Specifically, in
togglePlayPause() set active = false (or cancel _pollingTimer) when pausing and
set active = true (or restart the poller) when resuming so _fetchNowPlaying()
will early-return for paused playback; ensure stop() still fully cancels the
poller.
In `@test/ui/screens/radio_stations_test.dart`:
- Around line 138-186: Add tests in radio_stations_test.dart that validate the
polling lifecycle: verify polling begins only after RadioPlayerProvider.play()
completes, verify polling stops on RadioPlayerProvider.stop() and when switching
stations, and verify stale HTTP responses are ignored if the station changes
mid-request. Implement these using FakeAsync to control timers and a mocked HTTP
client (or mock API) to return controllable responses; advance timers to trigger
polling and assert that the mocked client is called (or not) as expected, and
assert that state updates correspond to the latest station (i.e., stale
responses do not overwrite current station metadata). Target tests to exercise
RadioPlayerProvider.play(), RadioPlayerProvider.stop(), and whatever internal
polling method (e.g., _poll or poll loop) by observing side-effects such as
calls to the mocked API, media item updates, and polling cancellation when
switching stations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3673002e-7f9a-4266-98fa-357355693787
📒 Files selected for processing (2)
lib/providers/radio_player_provider.darttest/ui/screens/radio_stations_test.dart
When a stream title is available, it displays as the main bold text with the station name underneath as subtitle. Falls back to station name as primary text when no stream title is available.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
GET radio/stations/{id}/now-playingevery 15s while a radio station is playing (matching the web client's behavior)streamTitle)debugPrintwithout affecting playbackTest plan
flutter testSummary by CodeRabbit