Skip to content
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

Update for ovos-utils 0.0.X compat. #224

Merged
merged 6 commits into from
May 9, 2024
Merged

Conversation

NeonDaniel
Copy link
Member

@NeonDaniel NeonDaniel commented May 8, 2024

@NeonDaniel NeonDaniel requested a review from a team May 8, 2024 21:28
@NeonDaniel NeonDaniel marked this pull request as ready for review May 8, 2024 21:28
@NeonDaniel NeonDaniel requested a review from JarbasAl May 8, 2024 21:47
ovos_plugin_manager/templates/audio.py Outdated Show resolved Hide resolved
@NeonDaniel NeonDaniel marked this pull request as draft May 8, 2024 22:17
@JarbasAl
Copy link
Member

JarbasAl commented May 8, 2024

just for completeness, known affected plugins are

https://github.com/OpenVoiceOS/ovos-vlc-plugin

https://github.com/OpenVoiceOS/ovos-audio-plugin-simple

I'd also like some (human) tests with ovos-audio + these plugins

@NeonDaniel
Copy link
Member Author

just for completeness, known affected plugins are

https://github.com/OpenVoiceOS/ovos-vlc-plugin

https://github.com/OpenVoiceOS/ovos-audio-plugin-simple

I'd also like some (human) tests with ovos-audio + these plugins

Would it be easier then to just revert to the try/except imports rather than adding these new changes? This seems like a lot of refactoring/testing for a deprecated module

@JarbasAl
Copy link
Member

JarbasAl commented May 8, 2024

just for completeness, known affected plugins are
OpenVoiceOS/ovos-vlc-plugin
OpenVoiceOS/ovos-audio-plugin-simple
I'd also like some (human) tests with ovos-audio + these plugins

Would it be easier then to just revert to the try/except imports rather than adding these new changes? This seems like a lot of refactoring/testing for a deprecated module

looking at the plugins they are importing from https://github.com/OpenVoiceOS/ovos-ocp-audio-plugin/blob/dev/ovos_plugin_common_play/ocp/base.py , so i guess this is mostly irrelevant, as those are re-implemented there

up to you, as long as manually you test ovos-audio works with those 2 plugins + this branch it's all good

looking at the code in this package in isolation, these changes seem the correct thing to do that covers more use cases, but in practice i dont expect any impact at all looking at all components as a whole

dont think any plugins in the wild exist using this class directly at all

@NeonDaniel NeonDaniel force-pushed the FIX_OvosUtils0.0.xCompat branch from fab759f to 7c1f7b1 Compare May 8, 2024 23:25
@NeonDaniel NeonDaniel requested a review from JarbasAl May 9, 2024 17:48
@NeonDaniel NeonDaniel marked this pull request as ready for review May 9, 2024 17:48
@NeonDaniel
Copy link
Member Author

Validated by stopping the GUI and allowing OCP to select the Audio Service for playback

@JarbasAl JarbasAl added the packaging dependency updates or packaging changes label May 9, 2024
@JarbasAl JarbasAl merged commit eff6f90 into dev May 9, 2024
8 checks passed
@JarbasAl JarbasAl deleted the FIX_OvosUtils0.0.xCompat branch May 9, 2024 18:17
JarbasAl pushed a commit to OpenVoiceOS/ovos-audio that referenced this pull request May 9, 2024
* Add test case to validate backend plugin load

* Update dependency to test OpenVoiceOS/ovos-plugin-manager#224

* Update OPM dependency to pip spec

---------

Co-authored-by: Daniel McKnight <[email protected]>
@github-actions github-actions bot mentioned this pull request Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packaging dependency updates or packaging changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants