refactor!:drop OCP #115
refactor!:drop OCP #115
Conversation
completely remove OCP integration, leave only the legacy audio system around
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request introduces significant changes to the audio handling components in the OVOS project. The modifications primarily focus on simplifying the Open Common Play (OCP) functionality by removing the Changes
Possibly related PRs
Suggested labels
Poem
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
breaking
![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 (3)
ovos_audio/audio.py (1)
353-353: Correct the spelling of “prefered_service”
Suggest renaming the parameter to “preferred_service” for clarity.- def play(self, tracks: Union[List[str], List[Tuple[str, str]]], - prefered_service: Optional[str], repeat: bool = False): + def play(self, tracks: Union[List[str], List[Tuple[str, str]]], + preferred_service: Optional[str], repeat: bool = False):ovos_audio/playback.py (1)
123-133: Add docstring to the newputmethod
The method is clear, but adding a docstring can improve maintainability and clarify usage, e.g.:+ def put(self, wav: str, + visemes: Optional[List[str]]=None, + listen: bool = False, + tts_id: Optional[str] = None, + message: Optional[Message] = None): + """ + Enqueue an audio file for future playback. + :param wav: Path to the WAV file for playback + :param visemes: Sequence of visemes for lip-sync (optional) + :param listen: Set to True if the system should listen subsequently + :param tts_id: Identifier for the TTS engine + :param message: Original message context + """ message = message or Message("") ...ovos_audio/service.py (1)
106-106:enable_old_audioserviceremains set to True
Since OCP is being dropped, consider defaulting this to False if legacy audio is truly optional.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
ovos_audio/audio.py(6 hunks)ovos_audio/playback.py(2 hunks)ovos_audio/service.py(2 hunks)requirements/extras.txt(0 hunks)requirements/requirements.txt(1 hunks)
💤 Files with no reviewable changes (1)
- requirements/extras.txt
🔇 Additional comments (11)
requirements/requirements.txt (1)
2-4: LGTM: Unchanged dependencies maintain stability
The version constraints for other core dependencies remain unchanged, which helps maintain stability during this significant refactor.
ovos_audio/audio.py (7)
25-25: Consider removing the MediaState import if OCP is fully dropped
MediaState is still referenced at line 316 for INVALID_MEDIA, but if the broader goal is to remove all OCP features, ensure this import remains strictly necessary.
28-28: Confirmed usage of require_native_source
The decorator is used across multiple handlers (pause, resume, queue, etc.) and appears necessary.
39-39: Removal of disable_ocp parameter aligns well with PR objectives
Streamlines the constructor and removes unnecessary logic linked to OCP.
49-49: No functional change at line 49
No issues observed here; it appears to be spacing or a minor structural adjustment.
55-55: Minor reorganization of fields
Declaring volume_is_speaking here is fine; no issues or conflicts detected.
222-227: Improved error handling on stopping current track
Wrapping self.current.stop() in a try-except block and setting stopped = False upon failure avoids unhandled exceptions and unexpected state. Good defensive coding.
238-242: Robust approach to restoring volume
Using try-except for self.current.restore_volume() prevents crashes if volume restoration fails.
ovos_audio/playback.py (1)
4-4: New type annotations
Adding List to the imports is consistent and supports the added parameter typing.
ovos_audio/service.py (2)
57-57: Removal of disable_ocp parameter
This change removes outdated OCP logic, making the constructor more concise.
110-110: Constructor usage clarifies OCP removal
This instantiation without disable_ocp aligns with the simplified audio service.
completely remove OCP integration, leave only the legacy audio system around
Summary by CodeRabbit
Release Notes
New Features
puttoPlaybackThreadfor queuing audio playbackImprovements
Changes
disable_ocpparameter from audio service constructorsDependency Updates
ovos-utilspackage version requirementovos_plugin_common_play[extractors]dependency