Update CommonPlaySkill and CommonQuerySkill to pass messages

mycroft/skills/common_play_skill.py

@@ -15,9 +15,12 @@
 import re
 from enum import Enum, IntEnum
 from abc import ABC, abstractmethod
+from inspect import signature
+
 from mycroft.messagebus.message import Message
 from .mycroft_skill import MycroftSkill
 from .audioservice import AudioService
+from mycroft.util import LOG
 
 
 class CPSMatchLevel(Enum):
@@ -93,7 +96,12 @@ def __handle_play_query(self, message):
                                         "searching": True}))
 
         # Now invoke the CPS handler to let the skill perform its search
-        result = self.CPS_match_query_phrase(search_phrase)
+        if 'phrase' in signature(self.CPS_match_query_phrase).parameters:
+            LOG.warning(f"Depreciated CPS method in {self.skill_id}!"
+                        f" Message should be referenced")
+            result = self.CPS_match_query_phrase(search_phrase)
+        else:
+            result = self.CPS_match_query_phrase(message=message)
 
         if result:
             match = result[0]
@@ -127,7 +135,7 @@ def __calc_confidence(self, match, phrase, level):
         """
         consumed_pct = len(match.split()) / len(phrase.split())
         if consumed_pct > 1.0:
-            consumed_pct = 1.0 / consumed_pct  # deal with over/under-matching
+            consumed_pct = 1.0 / consumed_pct  # deal with over/under-match
 
         # We'll use this to modify the level, but don't want it to allow a
         # match to jump to the next match level.  So bonus is 0 - 0.05 (1/20)
@@ -166,7 +174,12 @@ def __handle_play_start(self, message):
         self.play_service_string = phrase
 
         # Invoke derived class to provide playback data
-        self.CPS_start(phrase, data)
+        if 'phrase' in signature(self.CPS_start).parameters:
+            LOG.warning(f"Depreciated CPS method in {self.skill_id}!"
+                        f" Message should be referenced")
+            self.CPS_start(phrase, data)
+        else:
+            self.CPS_start(message=message, callback_data=data)
 
     def CPS_play(self, *args, **kwargs):
         """Begin playback of a media file or stream
@@ -200,11 +213,11 @@ def stop(self):
     # All of the following must be implemented by a skill that wants to
     # act as a CommonPlay Skill
     @abstractmethod
-    def CPS_match_query_phrase(self, phrase):
+    def CPS_match_query_phrase(self, message):
         """Analyze phrase to see if it is a play-able phrase with this skill.
 
         Arguments:
-            phrase (str): User phrase uttered after "Play", e.g. "some music"
+            message (Message): Message associated with play request
 
         Returns:
             (match, CPSMatchLevel[, callback_data]) or None: Tuple containing
@@ -213,7 +226,7 @@ def CPS_match_query_phrase(self, phrase):
                  match is selected.
         """
         # Derived classes must implement this, e.g.
-        #
+        # phrase = message.data.get("phrase")
         # if phrase in ["Zoosh"]:
         #     return ("Zoosh", CPSMatchLevel.Generic, {"hint": "music"})
         # or:
@@ -230,12 +243,13 @@ def CPS_match_query_phrase(self, phrase):
         return None
 
     @abstractmethod
-    def CPS_start(self, phrase, data):
+    def CPS_start(self, message, callback_data):
         """Begin playing whatever is specified in 'phrase'
 
         Arguments:
-            phrase (str): User phrase uttered after "Play", e.g. "some music"
-            data (dict): Callback data specified in match_query_phrase()
+            message (Message): Message associated with playback request
+            callback_data (dict): Callback data specified in
+                match_query_phrase()
         """
         # Derived classes must implement this, e.g.
         # self.CPS_play("http://zoosh.com/stream_music")

mycroft/skills/common_query_skill.py

@@ -14,7 +14,9 @@
 
 from enum import IntEnum
 from abc import ABC, abstractmethod
+from inspect import signature
 from .mycroft_skill import MycroftSkill
+from mycroft.util import LOG
 
 
 class CQSMatchLevel(IntEnum):
@@ -73,7 +75,12 @@ def __handle_question_query(self, message):
                                         "searching": True}))
 
         # Now invoke the CQS handler to let the skill perform its search
-        result = self.CQS_match_query_phrase(search_phrase)
+        if 'phrase' in signature(self.CQS_match_query_phrase).parameters:
+            LOG.warning(f"Depreciated CQS method in {self.skill_id}!"
+                        f" Message should be referenced")
+            result = self.CQS_match_query_phrase(search_phrase)
+        else:
+            result = self.CQS_match_query_phrase(message=message)
 
         if result:
             match = result[0]
@@ -126,16 +133,21 @@ def __handle_query_action(self, message):
         phrase = message.data["phrase"]
         data = message.data.get("callback_data")
         # Invoke derived class to provide playback data
-        self.CQS_action(phrase, data)
+        if 'phrase' in signature(self.CQS_action).parameters:
+            LOG.warning(f"Depreciated CQS method in {self.skill_id}!"
+                        f" Message should be referenced")
+            self.CQS_action(phrase, data)
+        else:
+            self.CQS_action(message=message, callback_data=data)
 
     @abstractmethod
-    def CQS_match_query_phrase(self, phrase):
+    def CQS_match_query_phrase(self, message):
         """Analyze phrase to see if it is a play-able phrase with this skill.
 
         Needs to be implemented by the skill.
 
         Arguments:
-            phrase (str): User phrase, "What is an aardwark"
+            message (Message): User phrase, "What is an aardwark"
 
         Returns:
             (match, CQSMatchLevel[, callback_data]) or None: Tuple containing
@@ -144,18 +156,32 @@ def CQS_match_query_phrase(self, phrase):
                  match is selected.
         """
         # Derived classes must implement this, e.g.
+        # query = message.data.get("phrase")
+        # answer = None
+        # for noun in self.question_words:
+        #     for verb in self.question_verbs:
+        #         for article in [i + ' ' for i in self.articles] + ['']:
+        #             test = noun + verb + ' ' + article
+        #             if query[:len(test)] == test:
+        #                 answer = self.respond(message, query[len(test):])
+        #                 break
+        # if answer:
+        #     return query, CQSMatchLevel.CATEGORY, answer
+        # else:
+        #     return None
         return None
 
-    def CQS_action(self, phrase, data):
+    def CQS_action(self, message, callback_data):
         """Take additional action IF the skill is selected.
 
         The speech is handled by the common query but if the chosen skill
         wants to display media, set a context or prepare for sending
         information info over e-mail this can be implemented here.
 
         Args:
-            phrase (str): User phrase uttered after "Play", e.g. "some music"
-            data (dict): Callback data specified in match_query_phrase()
+            message (Message): Message associated with query
+            callback_data (dict): Callback data specified in
+                match_query_phrase()
         """
         # Derived classes may implement this if they use additional media
         # or wish to set context after being called.

test/unittests/skills/test_common_query_skill.py

@@ -34,11 +34,11 @@ def test_common_test_skill_action(self):
         query_action(Message('query:action', data={
             'phrase': 'What\'s the meaning of life',
             'skill_id': 'asdf'}))
-        self.skill.CQS_action.assert_not_called()
+        self.skill.CQS_action_mock.assert_not_called()
         query_action(Message('query:action', data={
             'phrase': 'What\'s the meaning of life',
             'skill_id': 'CQSTest'}))
-        self.skill.CQS_action.assert_called_once_with(
+        self.skill.CQS_action_mock.assert_called_once_with(
             'What\'s the meaning of life', None)
 
 
@@ -53,7 +53,7 @@ def setUp(self):
         self.query_phrase = self.bus.on.call_args_list[-2][0][1]
 
     def test_failing_match_query_phrase(self):
-        self.skill.CQS_match_query_phrase.return_value = None
+        self.skill.CQS_match_mock.return_value = None
         self.query_phrase(Message('question:query',
                                   data={
                                       'phrase': 'What\'s the meaning of life'
@@ -75,7 +75,7 @@ def test_failing_match_query_phrase(self):
         self.assertEqual(response.data['searching'], False)
 
     def test_successful_match_query_phrase(self):
-        self.skill.CQS_match_query_phrase.return_value = (
+        self.skill.CQS_match_mock.return_value = (
             'What\'s the meaning of life', CQSMatchLevel.EXACT, '42')
 
         self.query_phrase(Message('question:query',
@@ -101,7 +101,7 @@ def test_successful_match_query_phrase(self):
     def test_successful_visual_match_query_phrase(self):
         self.skill.config_core['enclosure']['platform'] = 'mycroft_mark_2'
         query_phrase = self.bus.on.call_args_list[-2][0][1]
-        self.skill.CQS_match_query_phrase.return_value = (
+        self.skill.CQS_match_mock.return_value = (
             'What\'s the meaning of life', CQSVisualMatchLevel.EXACT, '42')
 
         query_phrase(Message('question:query',
@@ -127,12 +127,12 @@ class CQSTest(CommonQuerySkill):
     """Simple skill for testing the CommonQuerySkill"""
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
-        self.CQS_match_query_phrase = mock.Mock(name='match_phrase')
-        self.CQS_action = mock.Mock(name='selected_action')
+        self.CQS_match_mock = mock.Mock(name='match_phrase')
+        self.CQS_action_mock = mock.Mock(name='selected_action')
         self.skill_id = 'CQSTest'
 
     def CQS_match_query_phrase(self, phrase):
-        pass
+        return self.CQS_match_mock(phrase)
 
     def CQS_action(self, phrase, data):
-        pass
+        return self.CQS_action_mock(phrase, data)