From 99858d0608baf40ec3865698ec89452056c3c5e6 Mon Sep 17 00:00:00 2001 From: miro Date: Thu, 9 Jan 2025 16:43:49 +0000 Subject: [PATCH 01/11] fix:better_stop --- ovos_core/intent_services/__init__.py | 7 ++- ovos_core/intent_services/converse_service.py | 1 + ovos_core/intent_services/stop_service.py | 48 +++++++++++++------ 3 files changed, 40 insertions(+), 16 deletions(-) diff --git a/ovos_core/intent_services/__init__.py b/ovos_core/intent_services/__init__.py index 298e907b3076..e812a94009d0 100644 --- a/ovos_core/intent_services/__init__.py +++ b/ovos_core/intent_services/__init__.py @@ -276,7 +276,7 @@ def _emit_match_message(self, match: Union[IntentHandlerMatch, PipelineMatch], m message (Message): The messagebus data. """ reply = None - sess = SessionManager.get(message) + sess = match.updated_session or SessionManager.get(message) # utterance fully handled by pipeline matcher if isinstance(match, PipelineMatch): @@ -303,10 +303,13 @@ def _emit_match_message(self, match: Union[IntentHandlerMatch, PipelineMatch], m was_deactivated = match.skill_id in self._deactivations[sess.session_id] if not was_deactivated: sess.activate_skill(match.skill_id) - reply.context["session"] = sess.serialize() # emit event for skills callback -> self.handle_activate self.bus.emit(reply.forward(f"{match.skill_id}.activate")) + # update Session if modified by pipeline + reply.context["session"] = sess.serialize() + + # finally emit reply message self.bus.emit(reply) def send_cancel_event(self, message): diff --git a/ovos_core/intent_services/converse_service.py b/ovos_core/intent_services/converse_service.py index 2c229e39eea3..dfda3e88eccc 100644 --- a/ovos_core/intent_services/converse_service.py +++ b/ovos_core/intent_services/converse_service.py @@ -342,6 +342,7 @@ def converse_with_skills(self, utterances: List[str], lang: str, message: Messag # handled == True -> emit "ovos.utterance.handled" match_data={}, skill_id=skill_id, + updated_session=session, utterance=utterances[0]) return None diff --git a/ovos_core/intent_services/stop_service.py b/ovos_core/intent_services/stop_service.py index 651597136aa3..5590b8a83a1d 100644 --- a/ovos_core/intent_services/stop_service.py +++ b/ovos_core/intent_services/stop_service.py @@ -52,11 +52,13 @@ def get_active_skills(message: Optional[Message] = None) -> List[str]: def _collect_stop_skills(self, message: Message) -> List[str]: """use the messagebus api to determine which skills can stop This includes all skills and external applications""" + sess = SessionManager.get(message) want_stop = [] skill_ids = [] - active_skills = self.get_active_skills(message) + active_skills = [s for s in self.get_active_skills(message) + if s not in sess.blacklisted_skills] if not active_skills: return want_stop @@ -64,7 +66,7 @@ def _collect_stop_skills(self, message: Message) -> List[str]: event = Event() def handle_ack(msg): - nonlocal event + nonlocal event, skill_ids skill_id = msg.data["skill_id"] # validate the stop pong @@ -111,7 +113,24 @@ def stop_skill(self, skill_id: str, message: Message) -> bool: LOG.error(f"{skill_id}: {error_msg}") return False elif result is not None: - return result.data.get('result', False) + stopped = result.data.get('result', False) + else: + stopped = False + + if stopped: + sess = SessionManager.get(message) + state = sess.utterance_states.get(skill_id, "intent") + LOG.debug(f"skill response status: {state}") + if state == "response": # TODO this is never happening and it should... + LOG.debug(f"stopping {skill_id} in middle of get_response!") + + # force-kill any ongoing get_response/converse/TTS - see @killable_event decorator + self.bus.emit(message.forward("mycroft.skills.abort_question", {"skill_id": skill_id})) + self.bus.emit(message.forward("ovos.skills.converse.force_timeout", {"skill_id": skill_id})) + # TODO - track if speech is coming from this skill! not currently tracked + self.bus.emit(message.reply("mycroft.audio.speech.stop",{"skill_id": skill_id})) + + return stopped def match_stop_high(self, utterances: List[str], lang: str, message: Message) -> Optional[PipelineMatch]: """If utterance is an exact match for "stop" , run before intent stage @@ -140,6 +159,7 @@ def match_stop_high(self, utterances: List[str], lang: str, message: Message) -> conf = 1.0 if is_global_stop: + LOG.info(f"Emitting global stop, {len(self.get_active_skills(message))} active skills") # emit a global stop, full stop anything OVOS is doing self.bus.emit(message.reply("mycroft.stop", {})) return PipelineMatch(handled=True, @@ -150,15 +170,15 @@ def match_stop_high(self, utterances: List[str], lang: str, message: Message) -> if is_stop: # check if any skill can stop for skill_id in self._collect_stop_skills(message): - if skill_id in sess.blacklisted_skills: - LOG.debug(f"ignoring match, skill_id '{skill_id}' blacklisted by Session '{sess.session_id}'") - continue - + LOG.debug(f"Checking if skill wants to stop: {skill_id}") if self.stop_skill(skill_id, message): + LOG.info(f"Skill stopped: {skill_id}") + sess.disable_response_mode(skill_id) return PipelineMatch(handled=True, match_data={"conf": conf}, skill_id=skill_id, - utterance=utterance) + utterance=utterance, + updated_session=sess) return None def match_stop_medium(self, utterances: List[str], lang: str, message: Message) -> Optional[PipelineMatch]: @@ -229,17 +249,17 @@ def match_stop_low(self, utterances: List[str], lang: str, message: Message) -> # check if any skill can stop for skill_id in self._collect_stop_skills(message): - if skill_id in sess.blacklisted_skills: - LOG.debug(f"ignoring match, skill_id '{skill_id}' blacklisted by Session '{sess.session_id}'") - continue - + LOG.debug(f"Checking if skill wants to stop: {skill_id}") if self.stop_skill(skill_id, message): + sess.disable_response_mode(skill_id) return PipelineMatch(handled=True, - # emit instead of intent message match_data={"conf": conf}, - skill_id=skill_id, utterance=utterance) + skill_id=skill_id, + utterance=utterance, + updated_session=sess) # emit a global stop, full stop anything OVOS is doing + LOG.debug(f"Emitting global stop signal, {len(self.get_active_skills(message))} active skills") self.bus.emit(message.reply("mycroft.stop", {})) return PipelineMatch(handled=True, # emit instead of intent message {"conf": conf}, From 4623705c3cc09393ba2df5ee85086b2eb1dae108 Mon Sep 17 00:00:00 2001 From: miro Date: Thu, 9 Jan 2025 16:59:52 +0000 Subject: [PATCH 02/11] requirements.txt --- requirements/requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements/requirements.txt b/requirements/requirements.txt index 6e71f4033f5d..701e664afad8 100644 --- a/requirements/requirements.txt +++ b/requirements/requirements.txt @@ -10,6 +10,6 @@ ovos-common-query-pipeline-plugin>=1.0.5, <2.0.0 ovos-utils[extras]>=0.6.0,<1.0.0 ovos_bus_client>=0.1.4,<2.0.0 -ovos-plugin-manager>=0.5.6,<1.0.0 +ovos-plugin-manager>=0.8.0,<1.0.0 ovos-config>=0.0.13,<2.0.0 ovos-workshop>=3.1.2,<4.0.0 From 8cca76419c40065ab00ae7e0b4258edd45aea773 Mon Sep 17 00:00:00 2001 From: miro Date: Thu, 9 Jan 2025 18:56:06 +0000 Subject: [PATCH 03/11] python versions --- .github/workflows/build_tests.yml | 2 +- .github/workflows/coverage.yml | 2 +- .github/workflows/pipaudit.yml | 2 +- .github/workflows/sync_tx.yml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/build_tests.yml b/.github/workflows/build_tests.yml index ffb1cc8dab7d..f32375b81f62 100644 --- a/.github/workflows/build_tests.yml +++ b/.github/workflows/build_tests.yml @@ -16,7 +16,7 @@ jobs: strategy: max-parallel: 2 matrix: - python-version: [3.8, 3.9, "3.10", "3.11"] + python-version: ["3.10", "3.11"] runs-on: ubuntu-latest steps: - uses: actions/checkout@v2 diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 81ff52dd464e..6187b3dc95ba 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -15,7 +15,7 @@ jobs: - name: Setup Python uses: actions/setup-python@master with: - python-version: 3.9 + python-version: "3.10" - name: Install System Dependencies run: | sudo apt-get update diff --git a/.github/workflows/pipaudit.yml b/.github/workflows/pipaudit.yml index de640a5e322d..00623be5707f 100644 --- a/.github/workflows/pipaudit.yml +++ b/.github/workflows/pipaudit.yml @@ -11,7 +11,7 @@ jobs: strategy: max-parallel: 2 matrix: - python-version: [ 3.7, 3.8, 3.9, "3.10", "3.11" ] + python-version: [3.9, "3.10", "3.11"] runs-on: ubuntu-latest steps: - uses: actions/checkout@v2 diff --git a/.github/workflows/sync_tx.yml b/.github/workflows/sync_tx.yml index 2fd378e8ec58..ff67209f9df9 100644 --- a/.github/workflows/sync_tx.yml +++ b/.github/workflows/sync_tx.yml @@ -18,7 +18,7 @@ jobs: - name: Setup Python uses: actions/setup-python@v1 with: - python-version: 3.9 + python-version: "3.10" - name: Run script if merged by gitlocalize-app[bot] if: github.event_name == 'push' && github.event.head_commit.author.username == 'gitlocalize-app[bot]' From 4fa2355bf6c632c36d78df1cefa6ed3846301554 Mon Sep 17 00:00:00 2001 From: miro Date: Thu, 9 Jan 2025 19:31:14 +0000 Subject: [PATCH 04/11] requirements.txt --- requirements/requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements/requirements.txt b/requirements/requirements.txt index 701e664afad8..4b2eeba17ce4 100644 --- a/requirements/requirements.txt +++ b/requirements/requirements.txt @@ -12,4 +12,4 @@ ovos-utils[extras]>=0.6.0,<1.0.0 ovos_bus_client>=0.1.4,<2.0.0 ovos-plugin-manager>=0.8.0,<1.0.0 ovos-config>=0.0.13,<2.0.0 -ovos-workshop>=3.1.2,<4.0.0 +ovos-workshop>=3.3.3a1,<4.0.0 From e2a1b08af3efa0e5051db22e54556a652a4fc774 Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Thu, 9 Jan 2025 19:47:47 +0000 Subject: [PATCH 05/11] =?UTF-8?q?=F0=9F=93=9D=20Add=20docstrings=20to=20`s?= =?UTF-8?q?top`=20(#644)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Docstrings generation was requested by @JarbasAl. * https://github.com/OpenVoiceOS/ovos-core/pull/643#issuecomment-2580788217 The following files were modified: * `ovos_core/intent_services/__init__.py` * `ovos_core/intent_services/converse_service.py` * `ovos_core/intent_services/stop_service.py` Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- ovos_core/intent_services/__init__.py | 47 +++++- ovos_core/intent_services/converse_service.py | 28 +++- ovos_core/intent_services/stop_service.py | 147 ++++++++++++++---- 3 files changed, 176 insertions(+), 46 deletions(-) diff --git a/ovos_core/intent_services/__init__.py b/ovos_core/intent_services/__init__.py index e812a94009d0..009935a668d1 100644 --- a/ovos_core/intent_services/__init__.py +++ b/ovos_core/intent_services/__init__.py @@ -268,12 +268,31 @@ def _handle_deactivate(self, message): self._deactivations[sess.session_id].append(skill_id) def _emit_match_message(self, match: Union[IntentHandlerMatch, PipelineMatch], message: Message): - """Update the message data with the matched utterance information and - activate the corresponding skill if available. - + """ + Emit a reply message for a matched intent, updating session and skill activation. + + This method processes matched intents from either a pipeline matcher or an intent handler, + creating a reply message with matched intent details and managing skill activation. + Args: - match (IntentHandlerMatch): The matched utterance object. - message (Message): The messagebus data. + match (Union[IntentHandlerMatch, PipelineMatch]): The matched intent object containing + utterance and matching information. + message (Message): The original messagebus message that triggered the intent match. + + Details: + - Handles two types of matches: PipelineMatch and IntentHandlerMatch + - Creates a reply message with matched intent data + - Activates the corresponding skill if not previously deactivated + - Updates session information + - Emits the reply message on the messagebus + + Side Effects: + - Modifies session state + - Emits a messagebus event + - Can trigger skill activation events + + Returns: + None """ reply = None sess = match.updated_session or SessionManager.get(message) @@ -313,6 +332,24 @@ def _emit_match_message(self, match: Union[IntentHandlerMatch, PipelineMatch], m self.bus.emit(reply) def send_cancel_event(self, message): + """ + Emit events and play a sound when an utterance is canceled. + + Logs the cancellation with the specific cancel word, plays a predefined cancel sound, + and emits multiple events to signal the utterance cancellation. + + Parameters: + message (Message): The original message that triggered the cancellation. + + Events Emitted: + - 'mycroft.audio.play_sound': Plays a cancel sound from configuration + - 'ovos.utterance.cancelled': Signals that the utterance was canceled + - 'ovos.utterance.handled': Indicates the utterance processing is complete + + Notes: + - Uses the default cancel sound path 'snd/cancel.mp3' if not specified in configuration + - Ensures events are sent as replies to the original message + """ LOG.info("utterance canceled, cancel_word:" + message.context.get("cancel_word")) # play dedicated cancel sound sound = Configuration().get('sounds', {}).get('cancel', "snd/cancel.mp3") diff --git a/ovos_core/intent_services/converse_service.py b/ovos_core/intent_services/converse_service.py index dfda3e88eccc..4d6421f97ed3 100644 --- a/ovos_core/intent_services/converse_service.py +++ b/ovos_core/intent_services/converse_service.py @@ -313,15 +313,29 @@ def converse(self, utterances: List[str], skill_id: str, lang: str, message: Mes return False def converse_with_skills(self, utterances: List[str], lang: str, message: Message) -> Optional[PipelineMatch]: - """Give active skills a chance at the utterance - + """ + Attempt to converse with active skills for a given set of utterances. + + Iterates through active skills to find one that can handle the utterance. Filters skills based on timeout and blacklist status. + Args: - utterances (list): list of utterances - lang (string): 4 letter ISO language code - message (Message): message to use to generate reply - + utterances (List[str]): List of utterance strings to process + lang (str): 4-letter ISO language code for the utterances + message (Message): Message context for generating a reply + Returns: - IntentMatch if handled otherwise None. + PipelineMatch: Match details if a skill successfully handles the utterance, otherwise None + - handled (bool): Whether the utterance was fully handled + - match_data (dict): Additional match metadata + - skill_id (str): ID of the skill that handled the utterance + - updated_session (Session): Current session state after skill interaction + - utterance (str): The original utterance processed + + Notes: + - Standardizes language tag + - Filters out blacklisted skills + - Checks for skill conversation timeouts + - Attempts conversation with each eligible skill """ lang = standardize_lang_tag(lang) session = SessionManager.get(message) diff --git a/ovos_core/intent_services/stop_service.py b/ovos_core/intent_services/stop_service.py index 5590b8a83a1d..3d0c82651ffe 100644 --- a/ovos_core/intent_services/stop_service.py +++ b/ovos_core/intent_services/stop_service.py @@ -50,8 +50,25 @@ def get_active_skills(message: Optional[Message] = None) -> List[str]: return [skill[0] for skill in session.active_skills] def _collect_stop_skills(self, message: Message) -> List[str]: - """use the messagebus api to determine which skills can stop - This includes all skills and external applications""" + """ + Collect skills that can be stopped based on a ping-pong mechanism. + + This method determines which active skills can handle a stop request by sending + a stop ping to each active skill and waiting for their acknowledgment. + + Parameters: + message (Message): The original message triggering the stop request. + + Returns: + List[str]: A list of skill IDs that can be stopped. If no skills explicitly + indicate they can stop, returns all active skills. + + Notes: + - Excludes skills that are blacklisted in the current session + - Uses a non-blocking event mechanism to collect skill responses + - Waits up to 0.5 seconds for skills to respond + - Falls back to all active skills if no explicit stop confirmation is received + """ sess = SessionManager.get(message) want_stop = [] @@ -66,6 +83,23 @@ def _collect_stop_skills(self, message: Message) -> List[str]: event = Event() def handle_ack(msg): + """ + Handle acknowledgment from skills during the stop process. + + This method is a nested function used in skill stopping negotiation. It validates and tracks skill responses to a stop request. + + Parameters: + msg (Message): Message containing skill acknowledgment details. + + Side Effects: + - Modifies the `want_stop` list with skills that can handle stopping + - Updates the `skill_ids` list to track which skills have responded + - Sets the threading event when all active skills have responded + + Notes: + - Checks if a skill can handle stopping based on multiple conditions + - Ensures all active skills provide a response before proceeding + """ nonlocal event, skill_ids skill_id = msg.data["skill_id"] @@ -96,15 +130,28 @@ def handle_ack(msg): return want_stop or active_skills def stop_skill(self, skill_id: str, message: Message) -> bool: - """Tell a skill to stop anything it's doing, - taking into account the message Session - + """ + Stop a skill's ongoing activities and manage its session state. + + Sends a stop command to a specific skill and handles its response, ensuring + that any active interactions or processes are terminated. The method checks + for errors, verifies the skill's stopped status, and emits additional signals + to forcibly abort ongoing actions like conversations, questions, or speech. + Args: - skill_id: skill to query. - message (Message): message containing interaction info. - + skill_id (str): Unique identifier of the skill to be stopped. + message (Message): The original message context containing interaction details. + Returns: - handled (bool): True if handled otherwise False. + bool: True if the skill was successfully stopped, False otherwise. + + Raises: + Logs error if skill stop request encounters an issue. + + Notes: + - Emits multiple bus messages to ensure complete skill termination + - Checks and handles different skill interaction states + - Supports force-stopping of conversations, questions, and speech """ stop_msg = message.reply(f"{skill_id}.stop") result = self.bus.wait_for_response(stop_msg, f"{skill_id}.stop.response") @@ -133,15 +180,28 @@ def stop_skill(self, skill_id: str, message: Message) -> bool: return stopped def match_stop_high(self, utterances: List[str], lang: str, message: Message) -> Optional[PipelineMatch]: - """If utterance is an exact match for "stop" , run before intent stage - - Args: - utterances (list): list of utterances - lang (string): 4 letter ISO language code - message (Message): message to use to generate reply - + """ + Handles high-confidence stop requests by matching exact stop vocabulary and managing skill stopping. + + Attempts to stop skills when an exact "stop" or "global_stop" command is detected. Performs the following actions: + - Identifies the closest language match for vocabulary + - Checks for global stop command when no active skills exist + - Emits a global stop message if applicable + - Attempts to stop individual skills if a stop command is detected + - Disables response mode for stopped skills + + Parameters: + utterances (List[str]): List of user utterances to match against stop vocabulary + lang (str): Four-letter ISO language code for language-specific matching + message (Message): Message context for generating appropriate responses + Returns: - PipelineMatch if handled otherwise None. + Optional[PipelineMatch]: Match result indicating whether stop was handled, with optional skill and session information + - Returns None if no stop action could be performed + - Returns PipelineMatch with handled=True for successful global or skill-specific stop + + Raises: + No explicit exceptions raised, but may log debug/info messages during processing """ lang = self._get_closest_lang(lang) if lang is None: # no vocs registered for this lang @@ -182,16 +242,26 @@ def match_stop_high(self, utterances: List[str], lang: str, message: Message) -> return None def match_stop_medium(self, utterances: List[str], lang: str, message: Message) -> Optional[PipelineMatch]: - """ if "stop" intent is in the utterance, - but it contains additional words not in .intent files - - Args: - utterances (list): list of utterances - lang (string): 4 letter ISO language code - message (Message): message to use to generate reply - + """ + Handle stop intent with additional context beyond simple stop commands. + + This method processes utterances that contain "stop" or global stop vocabulary but may include + additional words not explicitly defined in intent files. It performs a medium-confidence + intent matching for stop requests. + + Parameters: + utterances (List[str]): List of input utterances to analyze + lang (str): Four-letter ISO language code for localization + message (Message): Message context for generating appropriate responses + Returns: - PipelineMatch if handled otherwise None. + Optional[PipelineMatch]: A pipeline match if the stop intent is successfully processed, + otherwise None if no stop intent is detected + + Notes: + - Attempts to match stop vocabulary with fuzzy matching + - Falls back to low-confidence matching if medium-confidence match is inconclusive + - Handles global stop scenarios when no active skills are present """ lang = self._get_closest_lang(lang) if lang is None: # no vocs registered for this lang @@ -222,15 +292,24 @@ def _get_closest_lang(self, lang: str) -> Optional[str]: return None def match_stop_low(self, utterances: List[str], lang: str, message: Message) -> Optional[PipelineMatch]: - """ before fallback_low , fuzzy match stop intent - - Args: - utterances (list): list of utterances - lang (string): 4 letter ISO language code - message (Message): message to use to generate reply - + """ + Perform a low-confidence fuzzy match for stop intent before fallback processing. + + This method attempts to match stop-related vocabulary with low confidence and handle stopping of active skills. + + Parameters: + utterances (List[str]): List of input utterances to match against stop vocabulary + lang (str): Four-letter ISO language code for vocabulary matching + message (Message): Message context used for generating replies and managing session + Returns: - PipelineMatch if handled otherwise None. + Optional[PipelineMatch]: A pipeline match object if a stop action is handled, otherwise None + + Notes: + - Increases confidence if active skills are present + - Attempts to stop individual skills before emitting a global stop signal + - Handles language-specific vocabulary matching + - Configurable minimum confidence threshold for stop intent """ lang = self._get_closest_lang(lang) if lang is None: # no vocs registered for this lang From 040042fc45a96292c0d4915f80f3c506960c0c9a Mon Sep 17 00:00:00 2001 From: miro Date: Thu, 9 Jan 2025 22:05:49 +0000 Subject: [PATCH 06/11] fix: stop DRY no need for 2 handlers, decrease complexity the only difference is an emitted event that is not listened too anywhere --- test/end2end/session/test_stop.py | 74 ++++++++++++------------------- 1 file changed, 29 insertions(+), 45 deletions(-) diff --git a/test/end2end/session/test_stop.py b/test/end2end/session/test_stop.py index 6ccde84ea081..a30bfc1cf84e 100644 --- a/test/end2end/session/test_stop.py +++ b/test/end2end/session/test_stop.py @@ -67,22 +67,12 @@ def wait_for_n_messages(n): "recognizer_loop:utterance", # global stop trigger "mycroft.stop", - # intent pipelines - "common_query.openvoiceos.stop", - "common_query.openvoiceos.stop.response", # reporting nothing to stop - "ovos.common_play.stop", - "ovos.common_play.stop.response", # reporting nothing to stop - - # skill reporting - f"{self.skill_id}.stop", # internal, @killable_events - f"{self.skill_id}.stop.response", # skill reporting nothing to stop - + "common_query.openvoiceos.stop.response", + "ovos.common_play.stop.response", + f"{self.skill_id}.stop.response", # sanity check in test skill that method was indeed called "speak", # "utterance":"old stop called" - - # NOTE: messages below might show up before enclosure.active_skill - f"{self.new_skill_id}.stop", # internal, @killable_events - f"{self.new_skill_id}.stop.response", # skill reporting nothing to stop + f"{self.new_skill_id}.stop.response", # nothing to stop "ovos.utterance.handled" ] @@ -151,30 +141,17 @@ def wait_for_n_messages(n): f"{self.skill_id}.stop.ping", # check if active skill wants to stop "skill.stop.pong", # "can_handle":true f"{self.skill_id}.stop", # skill specific stop trigger - f"{self.skill_id}.stop.response", # skill fails to stop (old style) + "speak", # "old stop called" in the test skill stop method + f"{self.skill_id}.stop.response", # skill stops and reports back - # stop medium - f"{self.skill_id}.stop.ping", - "skill.stop.pong", - f"{self.skill_id}.stop", # skill specific stop trigger - f"{self.skill_id}.stop.response", # skill fails to stop (old style) + # skill reports it stopped, so core ensures any threaded activity is also killed + "mycroft.skills.abort_question", # core kills any ongoing get_response + "ovos.skills.converse.force_timeout", # core kills any ongoing converse + "mycroft.audio.speech.stop", # core kills any ongoing TTS - # stop fallback - "mycroft.stop", # global stop for backwards compat - f"{self.skill_id}.stop", - f"{self.skill_id}.stop.response", # apparently fails to stop (old style) - - # test in skill that global stop was called - "speak", # "utterance":"stop" - - # report old-style stop handled event - "mycroft.stop.handled", # {"by":"skill:skill-old-stop.openvoiceos"} + f"{self.skill_id}.activate", # update of skill last usage timestamp + "ovos.utterance.handled" - # old style unwanted side effects (global stop is global) - f"{self.new_skill_id}.stop", - f"{self.new_skill_id}.stop.response", - # other test skill also speaks - "speak" # "utterance":"old stop called" ] wait_for_n_messages(len(expected_messages)) @@ -246,14 +223,15 @@ def wait_for_n_messages(n): "recognizer_loop:utterance", # global stop trigger "mycroft.stop", - f"{self.skill_id}.stop", # internal, @killable_events + "common_query.openvoiceos.stop.response", # common_query framework reporting nothing to stop + "ovos.common_play.stop.response", # OCP framework reporting nothing to stop f"{self.skill_id}.stop.response", # skill reporting nothing to stop - f"{self.new_skill_id}.stop", # internal, @killable_events - f"{self.new_skill_id}.stop.response", # skill reporting nothing to stop # sanity check in test skill that method was indeed called "speak", # "utterance":"old stop called" + f"{self.new_skill_id}.stop.response", # skill reporting it stopped + "ovos.utterance.handled", ] @@ -331,8 +309,11 @@ def wait_for_n_messages(n): # test session specific stop was called "speak", # "utterance":"stop 123" - f"{self.new_skill_id}.stop.response", # skill reports it stopped (new style), + + "mycroft.skills.abort_question", # core kills any ongoing get_response + "ovos.skills.converse.force_timeout", # core kills any ongoing converse + "mycroft.audio.speech.stop", # core kills any ongoing TTS f"{self.new_skill_id}.activate", # update timestamp of last interaction with skill "ovos.utterance.handled" # handle_utterance returned (intent service) ] @@ -350,7 +331,7 @@ def wait_for_n_messages(n): self.assertEqual(m.data["utterance"], "stop 123") # confirm "skill-new-stop" was the one that reported success - handler = messages[-3] + handler = messages[-6] self.assertEqual(handler.msg_type, f"{self.new_skill_id}.stop.response") self.assertEqual(handler.data["result"], True) @@ -373,6 +354,7 @@ def wait_for_n_messages(n): f"{self.new_skill_id}.stop.ping", # check if active skill wants to stop "skill.stop.pong", # "can_handle":true f"{self.new_skill_id}.stop", # skill specific stop trigger + "speak", # it's in the stop method even if it returns False! f"{self.new_skill_id}.stop.response", # dont want to stop (new style) # rest of pipeline @@ -380,17 +362,19 @@ def wait_for_n_messages(n): f"{self.new_skill_id}.stop.ping", "skill.stop.pong", f"{self.new_skill_id}.stop", # skill specific stop trigger + "speak", # it's in the stop method even if it returns False! f"{self.new_skill_id}.stop.response", # dont want to stop (new style) # global stop fallback "mycroft.stop", - f"{self.skill_id}.stop", # skill specific stop trigger + "common_query.openvoiceos.stop.response", # dont want to stop + "ovos.common_play.stop.response", # dont want to stop + f"{self.skill_id}.stop.response", # old style, never stops - f"{self.new_skill_id}.stop", # skill specific stop trigger - f"{self.skill_id}.stop.response", # dont want to stop (new style) + "speak", # it's in the stop method even if it returns False! + f"{self.new_skill_id}.stop.response", # dont want to stop (new style) - # check the global stop handlers are called - "speak", # "utterance":"old stop called" + "ovos.utterance.handled" ] wait_for_n_messages(len(expected_messages)) From f30ff533e2e48b33cd41d881f47ce93836788a83 Mon Sep 17 00:00:00 2001 From: miro Date: Thu, 9 Jan 2025 22:28:59 +0000 Subject: [PATCH 07/11] requirements.txt --- requirements/requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements/requirements.txt b/requirements/requirements.txt index 4b2eeba17ce4..7ed907286ce6 100644 --- a/requirements/requirements.txt +++ b/requirements/requirements.txt @@ -12,4 +12,4 @@ ovos-utils[extras]>=0.6.0,<1.0.0 ovos_bus_client>=0.1.4,<2.0.0 ovos-plugin-manager>=0.8.0,<1.0.0 ovos-config>=0.0.13,<2.0.0 -ovos-workshop>=3.3.3a1,<4.0.0 +ovos-workshop>=3.3.4a1,<4.0.0 From 3c5de472e98752c8fba4634f625d97834ae7d074 Mon Sep 17 00:00:00 2001 From: miro Date: Fri, 10 Jan 2025 01:20:31 +0000 Subject: [PATCH 08/11] tests --- test/end2end/session/test_sched.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/end2end/session/test_sched.py b/test/end2end/session/test_sched.py index e95c5e6b7ef1..a9378e3b9e73 100644 --- a/test/end2end/session/test_sched.py +++ b/test/end2end/session/test_sched.py @@ -110,7 +110,8 @@ def wait_for_n_messages(n): self.assertEqual(messages[8].data["session_data"]["active_skills"][0][0], self.skill_id) # ensure context in triggered event is the same from message that triggered the intent - intent_context = messages[1].context # when skill added to active list (last context change) + intent_context = messages[-3].context # when skill added to active list (last context change) + intent_context["skill_id"] = 'skill-ovos-schedule.openvoiceos' # for tests below, skill_id is injected self.assertEqual(messages[-2].msg_type, "skill-ovos-schedule.openvoiceos:my_event") self.assertEqual(messages[-2].context, intent_context) From bd7abf780d8f50e6a70c1f46ff118aefc1b3961c Mon Sep 17 00:00:00 2001 From: miro Date: Fri, 10 Jan 2025 01:54:11 +0000 Subject: [PATCH 09/11] tests --- test/end2end/session/test_ocp.py | 18 +++++++++++++++--- test/end2end/skill-fake-fm/__init__.py | 2 +- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/test/end2end/session/test_ocp.py b/test/end2end/session/test_ocp.py index d18ba0c3eedf..651395fc3daf 100644 --- a/test/end2end/session/test_ocp.py +++ b/test/end2end/session/test_ocp.py @@ -78,7 +78,11 @@ def wait_for_n_messages(n): "ovos.common_play.search.end", # no good results "ovos.common_play.reset", - + + # TODO - this is now matching ?? + # "add_context", + # "ovos.common_play.play", + # "ovos.common_play.search.populate", "speak", # error, "ovos.utterance.handled" # handle_utterance returned (intent service) ] @@ -146,7 +150,9 @@ def wait_for_n_messages(n): "ovos.common_play.SEI.get", # request info again, cause player didnt answer before "ovos.common_play.search.end", "ovos.common_play.reset", - + + # TODO - it is matching here now? + "speak", # nothing to play "ovos.utterance.handled" # handle_utterance returned (intent service) ] @@ -351,11 +357,17 @@ def wait_for_n_messages(n): # skill searching (generic) "ovos.common_play.skill.search_start", "ovos.common_play.query.response", + "ovos.common_play.query.response", + "ovos.common_play.query.response", + "ovos.common_play.query.response", + "ovos.common_play.query.response", "ovos.common_play.skill.search_end", "ovos.common_play.search.end", # no good results "ovos.common_play.reset", - + + # TODO - it is matching here now? + "speak", # error "ovos.utterance.handled" # handle_utterance returned (intent service) ] diff --git a/test/end2end/skill-fake-fm/__init__.py b/test/end2end/skill-fake-fm/__init__.py index 880b9205941a..79eb070d0907 100644 --- a/test/end2end/skill-fake-fm/__init__.py +++ b/test/end2end/skill-fake-fm/__init__.py @@ -15,7 +15,7 @@ def __init__(self, *args, **kwargs): @ocp_search() def search_fakefm(self, phrase, media_type): - score = 50 + score = 30 if "fake" in phrase: score += 35 if media_type == MediaType.RADIO: From 56635be6065487b59050dc72a34dddbb9e9e4d64 Mon Sep 17 00:00:00 2001 From: miro Date: Fri, 10 Jan 2025 01:55:02 +0000 Subject: [PATCH 10/11] tests --- test/end2end/session/test_ocp.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/test/end2end/session/test_ocp.py b/test/end2end/session/test_ocp.py index 651395fc3daf..5bdb9d934fcc 100644 --- a/test/end2end/session/test_ocp.py +++ b/test/end2end/session/test_ocp.py @@ -78,11 +78,6 @@ def wait_for_n_messages(n): "ovos.common_play.search.end", # no good results "ovos.common_play.reset", - - # TODO - this is now matching ?? - # "add_context", - # "ovos.common_play.play", - # "ovos.common_play.search.populate", "speak", # error, "ovos.utterance.handled" # handle_utterance returned (intent service) ] @@ -151,8 +146,6 @@ def wait_for_n_messages(n): "ovos.common_play.search.end", "ovos.common_play.reset", - # TODO - it is matching here now? - "speak", # nothing to play "ovos.utterance.handled" # handle_utterance returned (intent service) ] @@ -366,8 +359,6 @@ def wait_for_n_messages(n): # no good results "ovos.common_play.reset", - # TODO - it is matching here now? - "speak", # error "ovos.utterance.handled" # handle_utterance returned (intent service) ] From cf5e1a9838dc1099b25e645fd4d924b48e5f717e Mon Sep 17 00:00:00 2001 From: miro Date: Fri, 10 Jan 2025 02:25:40 +0000 Subject: [PATCH 11/11] tests --- .github/workflows/license_tests.yml | 2 +- requirements/requirements.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/license_tests.yml b/.github/workflows/license_tests.yml index 94e0dec3674a..39faae79f794 100644 --- a/.github/workflows/license_tests.yml +++ b/.github/workflows/license_tests.yml @@ -37,7 +37,7 @@ jobs: requirements: 'requirements-all.txt' fail: 'Copyleft,Other,Error' fails-only: true - exclude: '^(precise-runner|fann2|ovos-adapt-parser|ovos-padatious|tqdm|bs4|sonopy|caldav|recurring-ical-events|x-wr-timezone|zeroconf|mutagen).*' + exclude: '^(precise-runner|fann2|ovos-adapt-parser|ovos-padatious|tqdm|bs4|sonopy|caldav|recurring-ical-events|x-wr-timezone|zeroconf|mutagen|attrs).*' exclude-license: '^(Mozilla).*$' - name: Print report if: ${{ always() }} diff --git a/requirements/requirements.txt b/requirements/requirements.txt index 7ed907286ce6..f2170d3750db 100644 --- a/requirements/requirements.txt +++ b/requirements/requirements.txt @@ -12,4 +12,4 @@ ovos-utils[extras]>=0.6.0,<1.0.0 ovos_bus_client>=0.1.4,<2.0.0 ovos-plugin-manager>=0.8.0,<1.0.0 ovos-config>=0.0.13,<2.0.0 -ovos-workshop>=3.3.4a1,<4.0.0 +ovos-workshop>=3.3.4,<4.0.0