fix/activation_session (#511)

* fix/activation_session

companion to OpenVoiceOS/OVOS-workshop#212

update session data so the skill already sees the skill as active, otherwise since it was async the Session was outdated in the intent handler

* no ocp double activation

* move dialog to the intent handler

* fix tests

* fix message.context

* requirements.txt

ovos_core/intent_services/__init__.py

@@ -295,8 +295,19 @@ def _emit_match_message(self, match: IntentMatch, message: Message):
             # keep all original message.data and update with intent match
             data = dict(message.data)
             data.update(match.intent_data)
+
             # NOTE: message.reply to ensure correct message destination
             reply = message.reply(match.intent_type, data)
+
+            # let's activate the skill BEFORE the intent is triggered
+            # to ensure an accurate Session
+            # NOTE: this was previously done async by the skill,
+            #   but then the skill was missing from Session.active_skills
+            sess = self.converse.activate_skill(message=reply,
+                                                skill_id=match.skill_id)
+            if sess:
+                reply.context["session"] = sess.serialize()
+
             self.bus.emit(reply)
 
     def send_cancel_event(self, message):

ovos_core/intent_services/converse_service.py

@@ -105,6 +105,7 @@ def activate_skill(self, skill_id, source_skill=None, message=None):
             self.bus.emit(message)
             # update activation counter
             self._consecutive_activations[skill_id] += 1
+            return session
 
     def _activate_allowed(self, skill_id, source_skill=None):
         """Checks if a skill_id is allowed to jump to the front of active skills list

ovos_core/intent_services/ocp_service.py

@@ -730,7 +730,6 @@ def match_high(self, utterances: List[str], lang: str, message: Message = None):
             else:
                 return None
 
-        self.activate()  # mark skill_id as active, this is a catch all for all OCP skills
         return ovos_core.intent_services.IntentMatch(intent_service="OCP_intents",
                                                      intent_type=f'ocp:{match["name"]}',
                                                      intent_data=match,
@@ -756,7 +755,6 @@ def match_medium(self, utterances: List[str], lang: str, message: Message = None
         # extract the query string
         query = self.remove_voc(utterance, "Play", lang).strip()
 
-        self.activate()  # mark skill_id as active, this is a catch all for all OCP skills
         return ovos_core.intent_services.IntentMatch(intent_service="OCP_media",
                                                      intent_type=f"ocp:play",
                                                      intent_data={"media_type": media_type,
@@ -783,7 +781,7 @@ def match_fallback(self, utterances: List[str], lang: str, message: Message = No
 
         # extract the query string
         query = self.remove_voc(utterance, "Play", lang).strip()
-        self.activate()  # mark skill_id as active, this is a catch all for all OCP skills
+
         return ovos_core.intent_services.IntentMatch(intent_service="OCP_fallback",
                                                      intent_type=f"ocp:play",
                                                      intent_data={"media_type": media_type,
@@ -796,8 +794,6 @@ def match_fallback(self, utterances: List[str], lang: str, message: Message = No
     def _process_play_query(self, utterance: str, lang: str, match: dict = None,
                             message: Optional[Message] = None):
 
-        self.activate()  # mark skill_id as active, this is a catch all for all OCP skills
-
         match = match or {}
         player = self.get_player(message)
         # if media is currently paused, empty string means "resume playback"
@@ -820,8 +816,6 @@ def _process_play_query(self, utterance: str, lang: str, match: dict = None,
                                                              skill_id=OCP_ID,
                                                              utterance=utterance)
 
-        self.speak_dialog("just.one.moment")
-
         sess = SessionManager.get(message)
         # if a skill was explicitly requested, search it first
         valid_skills = [
@@ -888,6 +882,9 @@ def handle_play_favorites_intent(self, message: Message):
 
     # intent handlers
     def handle_play_intent(self, message: Message):
+
+        self.speak_dialog("just.one.moment")
+
         lang = message.data["lang"]
         query = message.data["query"]
         media_type = message.data["media_type"]
@@ -1412,8 +1409,6 @@ def match_legacy(self, utterances: List[str], lang: str, message: Message = None
             return None
         if match["name"] == "play":
             LOG.info(f"Legacy Mycroft CommonPlay match: {match}")
-            # we dont call self.activate , the skill itself is activated on selection
-            # playback is happening outside of OCP
             utterance = match["entities"].pop("query")
             return ovos_core.intent_services.IntentMatch(intent_service="OCP_media",
                                                          intent_type=f"ocp:legacy_cps",

requirements/requirements.txt

@@ -12,7 +12,7 @@ ovos-plugin-manager<0.1.0, >=0.0.25
 ovos-config~=0.0,>=0.0.13a8
 ovos-lingua-franca>=0.4.7
 ovos-backend-client~=0.1.0
-ovos-workshop<0.1.0, >=0.0.16a35
+ovos-workshop<0.1.0, >=0.0.16a36
 # provides plugins and classic machine learning framework
 ovos-classifiers<0.1.0, >=0.0.0a53
 

test/end2end/routing/test_sched.py

@@ -47,14 +47,10 @@ def wait_for_n_messages(n):
         # confirm all expected messages are sent
         expected_messages = [
             "recognizer_loop:utterance",  # no session
-            f"{self.skill_id}:ScheduleIntent",  # intent trigger
-            "mycroft.skill.handler.start",  # intent code 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)
-
+            f"{self.skill_id}:ScheduleIntent",  # intent trigger
+            "mycroft.skill.handler.start",  # intent code start
             "enclosure.active_skill",
             "speak",
             "mycroft.scheduler.schedule_event",
@@ -72,16 +68,18 @@ def wait_for_n_messages(n):
 
         self.assertEqual(len(expected_messages), len(messages))
 
-        mtypes = [m.msg_type for m in messages]
-        for m in expected_messages:
-            self.assertTrue(m in mtypes)
+        for idx, m in enumerate(messages):
+            self.assertEqual(m.msg_type, expected_messages[idx])
 
         # verify that source and destination are swapped after intent trigger
-        self.assertEqual(messages[1].msg_type, f"{self.skill_id}:ScheduleIntent")
+        self.assertEqual(messages[3].msg_type, f"{self.skill_id}:ScheduleIntent")
         for m in messages:
-            if m.msg_type in ["recognizer_loop:utterance", "ovos.session.update_default"]:
+            # messages FOR ovos-core
+            if m.msg_type in ["recognizer_loop:utterance",
+                              "ovos.session.update_default"]:
                 self.assertEqual(messages[0].context["source"], "A")
                 self.assertEqual(messages[0].context["destination"], "B")
+            # messages FROM ovos-core
             else:
                 self.assertEqual(m.context["source"], "B")
                 self.assertEqual(m.context["destination"], "A")

test/end2end/routing/test_session.py

@@ -50,12 +50,10 @@ def wait_for_n_messages(n):
         # confirm all expected messages are sent
         expected_messages = [
             "recognizer_loop:utterance",  # no session
-            f"{self.skill_id}:HelloWorldIntent",
-            "mycroft.skill.handler.start",
-            "intent.service.skills.activate",
             "intent.service.skills.activated",
             f"{self.skill_id}.activate",
-            "ovos.session.update_default",
+            f"{self.skill_id}:HelloWorldIntent",
+            "mycroft.skill.handler.start",
             "enclosure.active_skill",
             "speak",
             "mycroft.skill.handler.complete",
@@ -66,17 +64,16 @@ def wait_for_n_messages(n):
 
         self.assertEqual(len(expected_messages), len(messages))
 
-        mtypes = [m.msg_type for m in messages]
-        for m in expected_messages:
-            self.assertTrue(m in mtypes)
+        for idx, m in enumerate(messages):
+            self.assertEqual(m.msg_type, expected_messages[idx])
 
         # verify that "session" is injected
         # (missing in utterance message) and kept in all messages
         for m in messages[1:]:
             self.assertEqual(m.context["session"]["session_id"], "default")
 
         # verify that source and destination are swapped after intent trigger
-        self.assertEqual(messages[1].msg_type, f"{self.skill_id}:HelloWorldIntent")
+        self.assertEqual(messages[3].msg_type, f"{self.skill_id}:HelloWorldIntent")
         for m in messages:
             if m.msg_type in ["recognizer_loop:utterance", "ovos.session.update_default"]:
                 self.assertEqual(messages[0].context["source"], "A")

test/end2end/session/test_blacklist.py

@@ -58,11 +58,10 @@ def wait_for_n_messages(n):
         expected_messages = [
             "recognizer_loop:utterance",
             # skill selected
-            f"{self.skill_id}:HelloWorldIntent",
-            "mycroft.skill.handler.start",
-            "intent.service.skills.activate",
             "intent.service.skills.activated",
             f"{self.skill_id}.activate",
+            f"{self.skill_id}:HelloWorldIntent",
+            "mycroft.skill.handler.start",
             # skill code executing
             "enclosure.active_skill",
             "speak",
@@ -76,7 +75,7 @@ def wait_for_n_messages(n):
             self.assertEqual(m.msg_type, expected_messages[idx])
 
         # sanity check correct intent triggered
-        self.assertEqual(messages[7].data["meta"]["dialog"], "hello.world")
+        self.assertEqual(messages[-3].data["meta"]["dialog"], "hello.world")
 
         ########################################
         # skill in blacklist
@@ -176,12 +175,11 @@ def wait_for_n_messages(n):
         expected_messages = [
             "recognizer_loop:utterance",
             "ovos.common_play.status",
-            "intent.service.skills.activate",
             "intent.service.skills.activated",
             "ovos.common_play.activate",
+            "ocp:play",
             "enclosure.active_skill",
             "speak",
-            "ocp:play",
             "ovos.common_play.search.start",
             "enclosure.mouth.think",
             "ovos.common_play.search.stop",  # any ongoing previous search
@@ -222,12 +220,11 @@ def wait_for_n_messages(n):
         expected_messages = [
             "recognizer_loop:utterance",
             "ovos.common_play.status",
-            "intent.service.skills.activate",
             "intent.service.skills.activated",
             "ovos.common_play.activate",
+            "ocp:play",
             "enclosure.active_skill",
             "speak",
-            "ocp:play",
             "ovos.common_play.search.start",
             "enclosure.mouth.think",
             "ovos.common_play.search.stop",  # any ongoing previous search
@@ -267,12 +264,11 @@ def wait_for_n_messages(n):
         expected_messages = [
             "recognizer_loop:utterance",
             "ovos.common_play.status",
-            "intent.service.skills.activate",
             "intent.service.skills.activated",
             "ovos.common_play.activate",
+            "ocp:play",
             "enclosure.active_skill",
             "speak",
-            "ocp:play",
             "ovos.common_play.search.start",
             "enclosure.mouth.think",
             "ovos.common_play.search.stop",  # any ongoing previous search