diff --git a/mycroft/skills/intent_service.py b/mycroft/skills/intent_service.py index 9c0b5397ee..f92b6ca18a 100644 --- a/mycroft/skills/intent_service.py +++ b/mycroft/skills/intent_service.py @@ -19,7 +19,6 @@ from adapt.engine import IntentDeterminationEngine from adapt.intent import IntentBuilder from mycroft.configuration import Configuration -from mycroft.messagebus.message import Message from mycroft.util.lang import set_active_lang from mycroft.util.log import LOG from mycroft.util.parse import normalize @@ -124,7 +123,6 @@ class ContextManager: if entity['origin'] != last or entity['origin'] == '': depth += 1 last = entity['origin'] - print(depth) result = [] if len(missing_entities) > 0: @@ -198,14 +196,11 @@ class IntentService: self.handle_vocab_manifest) def update_skill_name_dict(self, message): - """ - Messagebus handler, updates dictionary of if to skill name - conversions. - """ + """Messagebus handler, updates dict of id to skill name conversions.""" self.skill_names[message.data['id']] = message.data['name'] def get_skill_name(self, skill_id): - """ Get skill name from skill ID. + """Get skill name from skill ID. Args: skill_id: a skill id as encoded in Intent handlers. @@ -236,6 +231,7 @@ class IntentService: return False def handle_converse_error(self, message): + LOG.error(message.data['error']) skill_id = message.data["skill_id"] if message.data["error"] == "skill id does not exist": self.remove_active_skill(skill_id) @@ -246,14 +242,26 @@ class IntentService: self.active_skills.remove(skill) def add_active_skill(self, skill_id): + """Add a skill or update the position of an active skill. + + The skill is added to the front of the list, if it's already in the + list it's removed so there is only a single entry of it. + + Arguments: + skill_id (str): identifier of skill to be added. + """ # search the list for an existing entry that already contains it # and remove that reference - self.remove_active_skill(skill_id) - # add skill with timestamp to start of skill_list - self.active_skills.insert(0, [skill_id, time.time()]) + if skill_id != '': + self.remove_active_skill(skill_id) + # add skill with timestamp to start of skill_list + self.active_skills.insert(0, [skill_id, time.time()]) + else: + LOG.warning('Skill ID was empty, won\'t add to list of ' + 'active skills.') def update_context(self, intent): - """ Updates context with keyword from the intent. + """Updates context with keyword from the intent. NOTE: This method currently won't handle one_of intent keywords since it's not using quite the same format as other intent @@ -272,8 +280,7 @@ class IntentService: self.context_manager.inject_context(context_entity) def send_metrics(self, intent, context, stopwatch): - """ - Send timing metrics to the backend. + """Send timing metrics to the backend. NOTE: This only applies to those with Opt In. """ @@ -291,7 +298,7 @@ class IntentService: {'intent_type': 'intent_failure'}) def handle_utterance(self, message): - """ Main entrypoint for handling user utterances with Mycroft skills + """Main entrypoint for handling user utterances with Mycroft skills Monitor the messagebus for 'recognizer_loop:utterance', typically generated by a spoken interaction but potentially also from a CLI @@ -386,7 +393,7 @@ class IntentService: LOG.exception(e) def _converse(self, utterances, lang, message): - """ Give active skills a chance at the utterance + """Give active skills a chance at the utterance Args: utterances (list): list of utterances @@ -403,7 +410,7 @@ class IntentService: 1] <= self.converse_timeout * 60] # check if any skill wants to handle utterance - for skill in self.active_skills: + for skill in copy(self.active_skills): if self.do_converse(utterances, skill[0], lang, message): # update timestamp, or there will be a timeout where # intent stops conversing whether its being used or not @@ -412,7 +419,7 @@ class IntentService: return False def _adapt_intent_match(self, raw_utt, norm_utt, lang): - """ Run the Adapt engine to search for an matching intent + """Run the Adapt engine to search for an matching intent Args: raw_utt (list): list of utterances @@ -484,7 +491,7 @@ class IntentService: self.engine.intent_parsers = new_parsers def handle_add_context(self, message): - """ Add context + """Add context Args: message: data contains the 'context' item to add @@ -505,7 +512,7 @@ class IntentService: self.context_manager.inject_context(entity) def handle_remove_context(self, message): - """ Remove specific context + """Remove specific context Args: message: data contains the 'context' item to remove @@ -515,7 +522,7 @@ class IntentService: self.context_manager.remove_context(context) def handle_clear_context(self, message): - """ Clears all keywords from context """ + """Clears all keywords from context """ self.context_manager.clear_context() def handle_get_adapt(self, message): diff --git a/mycroft/skills/padatious_service.py b/mycroft/skills/padatious_service.py index 04b18c5a75..5d0652228a 100644 --- a/mycroft/skills/padatious_service.py +++ b/mycroft/skills/padatious_service.py @@ -82,6 +82,10 @@ class PadatiousService(FallbackSkill): self.registered_intents = [] self.registered_entities = [] + def make_active(self): + """Override the make active since this is not a real fallback skill.""" + pass + def train(self, message=None): padatious_single_thread = Configuration.get()[ 'padatious']['single_thread'] diff --git a/test/unittests/skills/test_intent_service.py b/test/unittests/skills/test_intent_service.py index e2c3401083..2272a5bad0 100644 --- a/test/unittests/skills/test_intent_service.py +++ b/test/unittests/skills/test_intent_service.py @@ -112,6 +112,45 @@ class ConversationTest(TestCase): # Check that a skill responded that it could handle the message self.assertTrue(result) + def test_converse_error(self): + """Check that all skill IDs in the active_skills list are called. + even if there's an error. + """ + def response(message, return_msg_type): + c64 = Message(return_msg_type, {'skill_id': 'c64_skill', + 'result': False}) + amiga = Message(return_msg_type, + {'skill_id': 'amiga_skill', + 'error': 'skill id does not exist'}) + atari = Message(return_msg_type, {'skill_id': 'atari_skill', + 'result': False}) + msgs = {'c64_skill': c64, + 'atari_skill': atari, + 'amiga_skill': amiga} + + return msgs[message.data['skill_id']] + + self.intent_service.add_active_skill('amiga_skill') + self.intent_service.bus.wait_for_response.side_effect = response + + hello = ['hello old friend'] + utterance_msg = Message('recognizer_loop:utterance', + data={'lang': 'en-US', + 'utterances': hello}) + result = self.intent_service._converse(hello, 'en-US', utterance_msg) + + # Check that the active skill list was updated to set the responding + # Skill first. + + # Check that a skill responded that it couldn't handle the message + self.assertFalse(result) + + # Check that each skill in the list of active skills were called + call_args = self.intent_service.bus.wait_for_response.call_args_list + sent_skill_ids = [call[0][0].data['skill_id'] for call in call_args] + self.assertEqual(sent_skill_ids, + ['amiga_skill', 'c64_skill', 'atari_skill']) + def test_reset_converse(self): """Check that a blank stt sends the reset signal to the skills.""" def response(message, return_msg_type):