From bf7e4441e92246e6fd4f7c7c14f3d7196cdca41c Mon Sep 17 00:00:00 2001 From: NeonJarbas <59943014+NeonJarbas@users.noreply.github.com> Date: Thu, 2 Jun 2022 23:28:44 +0100 Subject: [PATCH] feat/common_query service (#127) * feat/feat/common_query moves common query from a skill to a first class intent service instead of being the first fallback skill it is now called before fallback stage the common query skill only listened and emited events, it doesnt do "skill things", fits better as it's own intent matcher * fix * fix * cleanup * bump ovos_utils * ovos_utils - get_message_lang * fix unittests * bump ovos_utils * fix tests * stable ovos_utils Co-authored-by: jarbasai --- mycroft/skills/common_query_skill.py | 14 +- mycroft/skills/intent_service.py | 56 +++--- mycroft/skills/intent_services/__init__.py | 1 + .../intent_services/commonqa_service.py | 175 ++++++++++++++++++ mycroft/skills/mycroft_skill/mycroft_skill.py | 3 +- requirements/minimal.txt | 2 +- requirements/requirements.txt | 2 +- .../skills/test_common_query_skill.py | 26 +-- test/unittests/skills/test_intent_service.py | 19 +- 9 files changed, 215 insertions(+), 83 deletions(-) create mode 100644 mycroft/skills/intent_services/commonqa_service.py diff --git a/mycroft/skills/common_query_skill.py b/mycroft/skills/common_query_skill.py index cb9f897a7258..34ebef68d71f 100644 --- a/mycroft/skills/common_query_skill.py +++ b/mycroft/skills/common_query_skill.py @@ -70,15 +70,13 @@ class CommonQuerySkill(MycroftSkill, ABC): def __init__(self, name=None, bus=None): super().__init__(name, bus) - noise_words_filepath = "text/%s/noise_words.list" % (self.lang,) + noise_words_filepath = f"text/{self.lang}/noise_words.list" noise_words_filename = resolve_resource_file(noise_words_filepath) self.translated_noise_words = [] - try: + if noise_words_filename: with open(noise_words_filename) as f: self.translated_noise_words = f.read().strip() self.translated_noise_words = self.translated_noise_words.split() - except FileNotFoundError: - self.log.warning("Missing noise_words.list file in res/text/lang") # these should probably be configurable self.level_confidence = { @@ -148,12 +146,6 @@ def __calc_confidence(self, match, phrase, level, answer): # bonus for more sentences num_sentences = float(float(len(answer.split("."))) / float(10)) - # Add bonus if match has visuals and the device supports them. - platform = self.config_core.get("enclosure", {}).get("platform") - bonus = 0.0 - if is_CQSVisualMatchLevel(level) and handles_visuals(platform): - bonus = 0.1 - # extract topic topic = self.remove_noise(match) @@ -177,7 +169,7 @@ def __calc_confidence(self, match, phrase, level, answer): wc_mod = float(float(answer_size) / float(WORD_COUNT_DIVISOR)) * 2 confidence = self.level_confidence[level] + \ - consumed_pct + bonus + num_sentences + relevance + wc_mod + consumed_pct + num_sentences + relevance + wc_mod return confidence diff --git a/mycroft/skills/intent_service.py b/mycroft/skills/intent_service.py index 2865056839c4..158c9ea8dfe0 100644 --- a/mycroft/skills/intent_service.py +++ b/mycroft/skills/intent_service.py @@ -20,28 +20,13 @@ from mycroft.skills.intent_services import ( AdaptService, FallbackService, PadatiousService, PadatiousMatcher, - ConverseService, IntentMatch + ConverseService, CommonQAService, + IntentMatch ) from mycroft.skills.permissions import ConverseMode, ConverseActivationMode from mycroft.util.log import LOG from mycroft.util.parse import normalize - - -def _get_message_lang(message=None): - """Get the language from the message or the default language. - - Args: - message: message to check for language code. - - Returns: - The language code from the message or the default language. - """ - message = message or dig_for_message() - # TODO read active locale from LF instead - default_lang = Configuration.get().get('lang', 'en-us') - if not message: - return default_lang - return message.data.get('lang', default_lang).lower() +from ovos_utils.messagebus import get_message_lang def _normalize_all_utterances(utterances): @@ -71,7 +56,7 @@ def _normalize_all_utterances(utterances): else: combined.append((utt, norm)) - LOG.debug("Utterances: {}".format(combined)) + LOG.debug(f"Utterances: {combined}") return combined @@ -95,6 +80,7 @@ def __init__(self, bus): LOG.exception(f'Failed to create padatious handlers ({err})') self.fallback = FallbackService(bus) self.converse = ConverseService(bus) + self.common_qa = CommonQAService(bus) self.bus.on('register_vocab', self.handle_register_vocab) self.bus.on('register_intent', self.handle_register_intent) @@ -137,7 +123,7 @@ def __init__(self, bus): @property def registered_intents(self): - lang = _get_message_lang() + lang = get_message_lang() return [parser.__dict__ for parser in self.adapt_service.engines[lang].intent_parsers] @@ -181,7 +167,7 @@ def handle_deactivate_skill_request(self, message): def reset_converse(self, message): """Let skills know there was a problem with speech recognition""" - lang = _get_message_lang(message) + lang = get_message_lang(message) try: setup_locale(lang) # restore default lang except Exception as e: @@ -254,6 +240,8 @@ def send_metrics(self, intent, context, stopwatch): intent_type = f'{intent.skill_id}:converse' elif intent and intent.intent_service == 'Fallback': intent_type = 'fallback' + elif intent and intent.intent_service == 'CommonQuery': + intent_type = 'common_qa' elif intent: # Handled by an other intent parser # Recreate skill name from skill id parts = intent.intent_type.split(':') @@ -277,11 +265,12 @@ def handle_utterance(self, message): 1) Active skills attempt to handle using converse() 2) Padatious high match intents (conf > 0.95) 3) Adapt intent handlers - 5) High Priority Fallbacks - 6) Padatious near match intents (conf > 0.8) - 7) General Fallbacks - 8) Padatious loose match intents (conf > 0.5) - 9) Catch all fallbacks including Unknown intent handler + 5) CommonQuery Skills + 6) High Priority Fallbacks + 7) Padatious near match intents (conf > 0.8) + 8) General Fallbacks + 9) Padatious loose match intents (conf > 0.5) + 10) Catch all fallbacks including Unknown intent handler If all these fail the complete_intent_failure message will be sent and a generic info of the failure will be spoken. @@ -290,7 +279,7 @@ def handle_utterance(self, message): message (Message): The messagebus data """ try: - lang = _get_message_lang(message) + lang = get_message_lang(message) try: setup_locale(lang) except Exception as e: @@ -308,9 +297,10 @@ def handle_utterance(self, message): # These are listed in priority order. match_funcs = [ self.converse.converse_with_skills, padatious_matcher.match_high, - self.adapt_service.match_intent, self.fallback.high_prio, - padatious_matcher.match_medium, self.fallback.medium_prio, - padatious_matcher.match_low, self.fallback.low_prio + self.adapt_service.match_intent, self.common_qa.match, + self.fallback.high_prio, padatious_matcher.match_medium, + self.fallback.medium_prio, padatious_matcher.match_low, + self.fallback.low_prio ] match = None @@ -367,7 +357,7 @@ def handle_register_vocab(self, message): entity_type = message.data.get('entity_type') regex_str = message.data.get('regex') alias_of = message.data.get('alias_of') - lang = _get_message_lang(message) + lang = get_message_lang(message) self.adapt_service.register_vocabulary(entity_value, entity_type, alias_of, regex_str, lang) self.registered_vocab.append(message.data) @@ -441,7 +431,7 @@ def handle_get_intent(self, message): message (Message): message containing utterance """ utterance = message.data["utterance"] - lang = _get_message_lang(message) + lang = get_message_lang(message) combined = _normalize_all_utterances([utterance]) # Create matchers @@ -503,7 +493,7 @@ def handle_get_adapt(self, message): message (Message): message containing utterance """ utterance = message.data["utterance"] - lang = _get_message_lang(message) + lang = get_message_lang(message) combined = _normalize_all_utterances([utterance]) intent = self.adapt_service.match_intent(combined, lang) intent_data = intent.intent_data if intent else None diff --git a/mycroft/skills/intent_services/__init__.py b/mycroft/skills/intent_services/__init__.py index b9ca6224a84a..ffbec4675653 100644 --- a/mycroft/skills/intent_services/__init__.py +++ b/mycroft/skills/intent_services/__init__.py @@ -5,3 +5,4 @@ from mycroft.skills.intent_services.padatious_service \ import PadatiousService, PadatiousMatcher from mycroft.skills.intent_services.converse_service import ConverseService +from mycroft.skills.intent_services.commonqa_service import CommonQAService diff --git a/mycroft/skills/intent_services/commonqa_service.py b/mycroft/skills/intent_services/commonqa_service.py new file mode 100644 index 000000000000..134442762cd9 --- /dev/null +++ b/mycroft/skills/intent_services/commonqa_service.py @@ -0,0 +1,175 @@ +from mycroft.skills.intent_services.base import IntentMatch +from ovos_utils.log import LOG +from ovos_utils.enclosure.api import EnclosureAPI +from mycroft_bus_client.message import Message, dig_for_message +from ovos_utils.messagebus import get_message_lang +from threading import Lock, Event +import time + +EXTENSION_TIME = 10 + + +class CommonQAService: + """Intent Service handling common query skills. + All common query skills answer and the best answer is selected + This is in contrast to triggering best intent directly. + """ + + def __init__(self, bus): + self.bus = bus + self.skill_id = "common_query.openvoiceos" # fake skill + self.query_replies = {} # cache of received replies + self.query_extensions = {} # maintains query timeout extensions + self.lock = Lock() + self.searching = Event() + self.waiting = True + self.answered = False + self.enclosure = EnclosureAPI(self.bus, self.skill_id) + self.bus.on('question:query.response', self.handle_query_response) + + def match(self, utterances, lang, message): + """Send common query request and select best response + + Args: + utterances (list): List of tuples, + utterances and normalized version + lang (str): Language code + message: Message for session context + Returns: + IntentMatch or None + """ + message.data["lang"] = lang # only used for speak + message.data["utterance"] = utterances[0][0] + answered = self.handle_question(message) + if answered: + ret = IntentMatch('CommonQuery', None, {}, None) + else: + ret = None + return ret + + def handle_question(self, message): + """ Send the phrase to the CommonQuerySkills and prepare for handling + the replies. + """ + self.searching.set() + self.waiting = True + self.answered = False + utt = message.data.get('utterance') + self.enclosure.mouth_think() + + self.query_replies[utt] = [] + self.query_extensions[utt] = [] + LOG.info(f'Searching for {utt}') + # Send the query to anyone listening for them + msg = message.reply('question:query', data={'phrase': utt}) + if "skill_id" not in msg.context: + msg.context["skill_id"] = self.skill_id + self.bus.emit(msg) + + self.timeout_time = time.time() + 1 + while self.searching.is_set(): + if not self.waiting or time.time() > self.timeout_time + 1: + break + time.sleep(0.2) + + # forcefully timeout if search is still going + self._query_timeout(message) + return self.answered + + def handle_query_response(self, message): + search_phrase = message.data['phrase'] + skill_id = message.data['skill_id'] + searching = message.data.get('searching') + answer = message.data.get('answer') + + # Manage requests for time to complete searches + if searching: + # extend the timeout by 5 seconds + self.timeout_time = time.time() + EXTENSION_TIME + # TODO: Perhaps block multiple extensions? + if (search_phrase in self.query_extensions and + skill_id not in self.query_extensions[search_phrase]): + self.query_extensions[search_phrase].append(skill_id) + elif search_phrase in self.query_extensions: + # Search complete, don't wait on this skill any longer + if answer and search_phrase in self.query_replies: + LOG.info(f'Answer from {skill_id}') + self.query_replies[search_phrase].append(message.data) + + # Remove the skill from list of timeout extensions + if skill_id in self.query_extensions[search_phrase]: + self.query_extensions[search_phrase].remove(skill_id) + + # not waiting for any more skills + if not self.query_extensions[search_phrase]: + self._query_timeout(message) + else: + LOG.warning(f'{skill_id} Answered too slowly, will be ignored.') + + def _query_timeout(self, message): + if not self.searching.is_set(): + return # not searching, ignore timeout event + self.searching.clear() + + # Prevent any late-comers from retriggering this query handler + with self.lock: + LOG.info('Timeout occured check responses') + search_phrase = message.data.get('phrase', "") + if search_phrase in self.query_extensions: + self.query_extensions[search_phrase] = [] + self.enclosure.mouth_reset() + + # Look at any replies that arrived before the timeout + # Find response(s) with the highest confidence + best = None + ties = [] + if search_phrase in self.query_replies: + for handler in self.query_replies[search_phrase]: + if not best or handler['conf'] > best['conf']: + best = handler + ties = [] + elif handler['conf'] == best['conf']: + ties.append(handler) + + if best: + if ties: + # TODO: Ask user to pick between ties or do it automagically + pass + + # invoke best match + self.speak(best['answer']) + LOG.info('Handling with: ' + str(best['skill_id'])) + cb = best.get('callback_data') or {} + self.bus.emit(message.forward('question:action', + data={'skill_id': best['skill_id'], + 'phrase': search_phrase, + 'callback_data': cb})) + self.answered = True + else: + self.answered = False + self.waiting = False + if search_phrase in self.query_replies: + del self.query_replies[search_phrase] + if search_phrase in self.query_extensions: + del self.query_extensions[search_phrase] + + def speak(self, utterance, message=None): + """Speak a sentence. + + Args: + utterance (str): sentence mycroft should speak + """ + # registers the skill as being active + self.enclosure.register(self.skill_id) + + message = message or dig_for_message() + lang = get_message_lang(message) + data = {'utterance': utterance, + 'expect_response': False, + 'meta': {"skill": self.skill_id}, + 'lang': lang} + + m = message.forward("speak", data) if message \ + else Message("speak", data) + m.context["skill_id"] = self.skill_id + self.bus.emit(m) diff --git a/mycroft/skills/mycroft_skill/mycroft_skill.py b/mycroft/skills/mycroft_skill/mycroft_skill.py index a29eda9443d6..dad9e0da83e8 100644 --- a/mycroft/skills/mycroft_skill/mycroft_skill.py +++ b/mycroft/skills/mycroft_skill/mycroft_skill.py @@ -62,6 +62,7 @@ from ovos_utils.configuration import get_xdg_base, get_xdg_data_save_path, get_xdg_config_save_path from ovos_utils.enclosure.api import EnclosureAPI from ovos_utils.file_utils import get_temp_path +from ovos_utils.messagebus import get_message_lang import shutil @@ -357,7 +358,7 @@ def lang(self): lang = self._core_lang message = dig_for_message() if message: - lang = message.data.get("lang") or lang + lang = get_message_lang(message) return lang.lower() @property diff --git a/requirements/minimal.txt b/requirements/minimal.txt index a8ec20965842..eca0f04bae49 100644 --- a/requirements/minimal.txt +++ b/requirements/minimal.txt @@ -1,6 +1,6 @@ requests~=2.26 mycroft-messagebus-client~=0.9,!=0.9.2,!=0.9.3 combo-lock~=0.2 -ovos-utils~=0.0, >=0.0.21a4 +ovos-utils~=0.0, >=0.0.22 ovos-plugin-manager~=0.0, >=0.0.17 python-dateutil~=2.6 \ No newline at end of file diff --git a/requirements/requirements.txt b/requirements/requirements.txt index 6174e7262ed2..3c027a5bf313 100644 --- a/requirements/requirements.txt +++ b/requirements/requirements.txt @@ -7,7 +7,7 @@ python-dateutil~=2.6 combo-lock~=0.2 PyYAML~=5.4 -ovos-utils~=0.0, >=0.0.21a4 +ovos-utils~=0.0, >=0.0.22 ovos-plugin-manager~=0.0, >=0.0.17 ovos-stt-plugin-server~=0.0, >=0.0.2 ovos-tts-plugin-mimic~=0.2, >=0.2.6 diff --git a/test/unittests/skills/test_common_query_skill.py b/test/unittests/skills/test_common_query_skill.py index 21a8bd3d7588..7b31dcea4042 100644 --- a/test/unittests/skills/test_common_query_skill.py +++ b/test/unittests/skills/test_common_query_skill.py @@ -96,31 +96,7 @@ def test_successful_match_query_phrase(self): 'What\'s the meaning of life') self.assertEqual(response.data['skill_id'], self.skill.skill_id) self.assertEqual(response.data['answer'], '42') - self.assertEqual(response.data['conf'], 1.12) - - 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 = ( - 'What\'s the meaning of life', CQSVisualMatchLevel.EXACT, '42') - - query_phrase(Message('question:query', - data={'phrase': 'What\'s the meaning of life'})) - - # Check that the skill replied that it was searching - extension = self.bus.emit.call_args_list[-2][0][0] - self.assertEqual(extension.data['phrase'], - 'What\'s the meaning of life') - self.assertEqual(extension.data['skill_id'], self.skill.skill_id) - self.assertEqual(extension.data['searching'], True) - - # Assert that the skill responds with answer and confidence level - response = self.bus.emit.call_args_list[-1][0][0] - self.assertEqual(response.data['phrase'], - 'What\'s the meaning of life') - self.assertEqual(response.data['skill_id'], self.skill.skill_id) - self.assertEqual(response.data['answer'], '42') - self.assertEqual(response.data['conf'], 1.2200000000000002) + self.assertTrue(response.data['conf'] >= 1.0) class CQSTest(CommonQuerySkill): diff --git a/test/unittests/skills/test_intent_service.py b/test/unittests/skills/test_intent_service.py index 9dab6eebd06d..5d9e7a6ba59a 100644 --- a/test/unittests/skills/test_intent_service.py +++ b/test/unittests/skills/test_intent_service.py @@ -18,7 +18,8 @@ from mycroft.configuration.locale import setup_locale from mycroft.configuration import Configuration from mycroft.messagebus import Message -from mycroft.skills.intent_service import IntentService, _get_message_lang +from mycroft.skills.intent_service import IntentService +from ovos_utils.messagebus import get_message_lang from mycroft.skills.intent_services.adapt_service import (ContextManager, AdaptIntent) @@ -202,23 +203,19 @@ def response(message, return_msg_type): class TestLanguageExtraction(TestCase): @mock.patch.dict(Configuration._Configuration__config, BASE_CONF) def test_no_lang_in_message(self): - """No lang in message should result in lang from config.""" + """No lang in message should result in lang from active locale.""" + setup_locale("it-it") msg = Message('test msg', data={}) - self.assertEqual(_get_message_lang(msg), 'it-it') - - @mock.patch.dict(Configuration._Configuration__config, NO_LANG_CONF) - def test_no_lang_at_all(self): - """Not in message and not in config, should result in en-us.""" - msg = Message('test msg', data={}) - self.assertEqual(_get_message_lang(msg), 'en-us') + self.assertEqual(get_message_lang(msg), 'it-it') + setup_locale("en-us") @mock.patch.dict(Configuration._Configuration__config, BASE_CONF) def test_lang_exists(self): """Message has a lang code in data, it should be used.""" msg = Message('test msg', data={'lang': 'de-de'}) - self.assertEqual(_get_message_lang(msg), 'de-de') + self.assertEqual(get_message_lang(msg), 'de-de') msg = Message('test msg', data={'lang': 'sv-se'}) - self.assertEqual(_get_message_lang(msg), 'sv-se') + self.assertEqual(get_message_lang(msg), 'sv-se') def create_old_style_vocab_msg(keyword, value):