-
-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix/playlist_handling #122
Conversation
fix position of "now playing" handling fix PluginStream to MediaEntry extraction fix Playlist of PluginStream
WalkthroughThe recent changes involve deprecating the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Player
participant MediaEntry
participant PluginStream
User->>Player: set_now_playing(track)
Player->>MediaEntry: extract_stream()
MediaEntry-->>Player: return media info
Player->>PluginStream: update playlist
Player->>User: now playing updated
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
compat with utils 0.0.38
3932ae9
to
9f84118
Compare
@coderabbitai review |
Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
ovos_plugin_common_play/ocp/media.py (1)
197-197
: Deprecation notice approved.The
extract_stream
method is marked as deprecated. Consider adding a warning log to inform users when this method is used.def extract_stream(self): """ DEPRECATED: Get metadata from ocp_plugins and add it to this MediaEntry """ + LOG.warning("The `extract_stream` method is deprecated and will be removed in a future release.") uri = self.uri
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- ovos_plugin_common_play/ocp/media.py (1 hunks)
- ovos_plugin_common_play/ocp/player.py (2 hunks)
- requirements/requirements.txt (1 hunks)
- test/unittests/test_audio_backends.py (1 hunks)
- test/unittests/test_external_ocp.py (1 hunks)
- test/unittests/test_ocp_player.py (5 hunks)
Files skipped from review due to trivial changes (2)
- test/unittests/test_audio_backends.py
- test/unittests/test_external_ocp.py
Additional comments not posted (10)
requirements/requirements.txt (1)
4-4
: Dependency version update approved.The version of
ovos-workshop
has been updated to>=0.0.16a48, < 0.1.0
. Ensure compatibility with the rest of the codebase.ovos_plugin_common_play/ocp/player.py (6)
253-255
: Handling dictionary track approved.The handling of dictionary tracks has been refined, with clearer logging and a default URI assignment when syncing from MPRIS.
260-263
: Playlist index handling approved.The logic for finding the entry in the playlist has been improved.
265-269
: PluginStream extraction approved.The extraction of media entries from
PluginStream
has been handled correctly, with appropriate logging.
273-284
: Playlist position update approved.The logic for updating the playlist position and adding new tracks has been improved.
291-301
: New playlist handling approved.The handling of new playlists and updating the
now_playing
state has been improved.
327-330
: Stream validation simplification approved.The error handling for extracting the stream from
now_playing
has been removed, simplifying the flow.test/unittests/test_ocp_player.py (3)
Line range hint
207-209
:
Ensure proper handling of invalid strings inset_now_playing
.The test case correctly asserts that
self.player.set_now_playing
raises aValueError
for invalid strings. This ensures robustness against invalid inputs.
417-418
: Verify inclusion of "mpv" in preferred audio backends.The inclusion of "mpv" in the list of preferred audio backends broadens the scope of valid audio backends. Ensure this change is intentional and aligns with the supported backends.
Line range hint
210-212
:
Verify removal ofreset
method call.The removal of the
reset
method call indicates a change in how the state ofnow_playing
is managed. Ensure this change is intentional and does not affect other parts of the code.
* refactor/utils_0.1.0 (#108) * refactor/utils_0.1.0 * opm imports * no create .desktop file * no create .desktop file * requirements * requirements --------- Co-authored-by: JarbasAi <[email protected]> * Increment Version to 0.0.7a1 * Update Changelog * refactor/dry_ocp_imports (#109) * refactor/dry_ocp_imports import Playlist and MediaEntry objects from utils 0.1.0 * tests * tests * fix/ocp_playlist allow initing Playlist object as a regular list --------- Co-authored-by: JarbasAi <[email protected]> * Increment Version to 0.0.7a2 * Update Changelog * feat/experimental_ocp_pipeline (#112) allow using Classic OCP with the new pipeline companion to OpenVoiceOS/ovos-core#456 * Increment Version to 0.0.7a3 * Update Changelog * fix/coexistence_with_pipeline (#115) * fix/coexistence_with_pipeline intents were being registered even if pipeline enabled, only the media matchers were taking into account the new flag this commit moves the check to init instead * port/SEI report event report plugins to ovos-core like ovos-media does to help in transition * fix stop * Increment Version to 0.0.7a4 * Update Changelog * fix/playlist handling (#116) * fix deprecation warnings * fix type checks * better log * support older ovos-utils * type checking fixes * typing * yeah * Increment Version to 0.0.7a5 * Update Changelog * fix/playlist_again (#117) * Increment Version to 0.0.7a6 * Update Changelog * default to OCP pipeline + support for utils 0.0.38 (#119) * remove dead code since OpenVoiceOS/ovos-core#491 and OpenVoiceOS/ovos-config#96 this is effectively dead code all NLP matching is dropped in this PR * rm dead code * drop old tests * Increment Version to 0.0.7a7 * Update Changelog * fix/native_sources (#120) * fix/native_sources allow usage with voice satellites, without triggering playback on master side similar to what classic audio service does, but with a decorator to make it more usable * handle message=None * fix backend check * kwarg * add helper property * message None * default val * missing kwarg * drop old classifier tests * drop old classifier tests * keep your crappy test * keep your crappy test * a relevant test now * Increment Version to 0.0.7a8 * Update Changelog * fix/play_from_gui (#121) * fix/play_from_gui when selecting entries from featured media PluginStream was not being recognized if audio restarts with GUI opening, playback would fail due to GUI sending malformed data, this commit also works around that issue * more fixes * more fixes * update tests * restore tst * keep old OCP bugs around 👍 * fixy fixes * Increment Version to 0.0.7a9 * Update Changelog * fix/playlist_handling (#122) * fix/playlist_handling fix position of "now playing" handling fix PluginStream to MediaEntry extraction fix Playlist of PluginStream * deprecate extract_stream * check bounds of idx * remove test for old ovos-audio version compat with utils 0.0.38 * fix tests * Increment Version to 0.0.7a10 * Update Changelog * Update README.md * logs/end_of_track (#124) make it easier to debug issues like #123 * Increment Version to 0.0.7a11 * Update Changelog * Increment Version to 0.0.7 * Update Changelog --------- Co-authored-by: NeonJarbas <[email protected]> Co-authored-by: JarbasAi <[email protected]> Co-authored-by: JarbasAl <[email protected]> Co-authored-by: JarbasAI <[email protected]>
fix position of "now playing" handling
fix PluginStream to MediaEntry extraction
fix Playlist of PluginStream
Summary by CodeRabbit
New Features
Deprecation Notices
extract_stream
method to inform users of its phasing out.Bug Fixes
validate_stream
method.Tests