fix/no_media_skills #517
Merged
fix/no_media_skills #517
Conversation
This file contains 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
do not search OCP when no OCP skills are installed
|
Really like this small making it better type of things. Nice catch! |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #517 +/- ##
==========================================
- Coverage 75.99% 75.57% -0.42%
==========================================
Files 15 15
Lines 3020 3042 +22
==========================================
+ Hits 2295 2299 +4
- Misses 725 743 +18
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
LGTM |
goldyfruit
approved these changes
Jun 26, 2024
to test this you would need to ask "play XXXX" with no OCP skills installed, in your logs seems you asked "what are you" so it isnt checking this code path |
|
With media skill: |
|
With no media skill: |
|
this has news skill installed
|
Erf, is there a way to identify the "media" skills? |
15 seconds is pretty long, any way to reduce this? |
any OCP skill, if it's supposed to handle "play" queries it's a media skill
try now (but also try without news skill as that was the original fix in this PR) |
I re-installed the news skill and still takes 15 seconds when it can't play what is required. |
|
asking for a search before this PR could take up to 30+ seconds (media type search + generic search) i didnt see any logs without any OCP skill yet, those should speak the new dialog and take < 1 second asking for a media type without skills for that media type (eg, ask radio with only news installed) with this PR can take up to 15 seconds, but not the 30+ it originally could further reducing those 15 seconds will need a PR in ovos-workshop, but that max timeout value can also be set in config |
JarbasAl
added a commit
to OpenVoiceOS/OVOS-workshop
that referenced
this pull request
Jun 29, 2024
inform ovos-core this skill ignored a media type query, otherwise the OCPQuery will block for 15 seconds waiting for an answer that never comes companion to OpenVoiceOS/ovos-core#517
JarbasAl
added a commit
to OpenVoiceOS/OVOS-workshop
that referenced
this pull request
Jun 29, 2024
inform ovos-core this skill ignored a media type query, otherwise the OCPQuery will block for 15 seconds waiting for an answer that never comes companion to OpenVoiceOS/ovos-core#517
|
Tested on Mac and it works as expected, especially with the ovos-workshop PR mentioned. Once tests are passing I'm happy to approve the PR |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
do not search OCP when no OCP skills are installed
based on chat feedback