OpenVoiceOS / OVOS-workshop

frameworks, templates and patches for the OpenVoiceOS universe
Apache License 2.0
4 stars 11 forks source link

refactor/skill_activation_before_events #198

Closed JarbasAl closed 5 months ago

JarbasAl commented 5 months ago

when using self.add_event we can now request that BEFORE the handler fires the skill is activated or deactivated

once this is adopted in core this line should be deleted https://github.com/OpenVoiceOS/ovos-core/blob/dev/ovos_core/intent_services/__init__.py#L296

and that will fix https://github.com/OpenVoiceOS/ovos-core/issues/450 (test need to be updated)

before either PR the event order is messy (extra messages omitted)

"recognizer_loop:utterance",   
f"{skill_id}:{intent_name}",  # intent trigger
"mycroft.skill.handler.start", # intent start
(...)  # intent code, speak messages etc
"mycroft.skill.handler.complete",  # intent end
"intent.service.skills.activated",  # skill activation (from core)
f"{self.skill_id}.activate",  # skill callback
"ovos.session.update_default", # session update (end of utterance default sync)

with BOTH PRs the new events order becomes

"recognizer_loop:utterance", 
f"{skill_id}:{intent_name}",  # intent trigger
"mycroft.skill.handler.start",  # intent start
"intent.service.skills.activate",  # request (from workshop)
"intent.service.skills.activated",  # response (from core)
f"{self.skill_id}.activate",  # skill callback
"ovos.session.update_default",  # session update (active skill list ync)
(...)  # intent code, speak messages etc
"mycroft.skill.handler.complete",  # intent end
"ovos.session.update_default",  # session update (end of utterance default sync)

and now we can use deactivate inside skills as documentation says

@intent_handler("my_intent.intent")
def handle_intent(self, message):
     self.deactivate()

def converse(self, message):
     if self.asked_to_stop:
        self.deactivate()
     return True    

this PR should be merged first, there is no consequence for older core versions, just means an extra activation event will be emitted, that should be harmless and only happen on partial updates (update workshop but not ovos-core)

codecov[bot] commented 5 months ago

Codecov Report

Attention: Patch coverage is 58.33333% with 5 lines in your changes are missing coverage. Please review.

:exclamation: No coverage uploaded for pull request base (dev@18f4e11). Click here to learn what that means.

Files Patch % Lines
ovos_workshop/skills/ovos.py 66.66% 3 Missing :warning:
ovos_workshop/skills/common_query_skill.py 0.00% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## dev #198 +/- ## ====================================== Coverage ? 53.53% ====================================== Files ? 36 Lines ? 3818 Branches ? 0 ====================================== Hits ? 2044 Misses ? 1774 Partials ? 0 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.