From 2d5c1d7895fd25e13288ca8409be54f2c72b2d50 Mon Sep 17 00:00:00 2001 From: Daniel McKnight <34697904+NeonDaniel@users.noreply.github.com> Date: Mon, 10 Jul 2023 11:09:45 -0700 Subject: [PATCH] Skills module tests, docstrings, and annotations (#108) --- .github/workflows/unit_tests.yml | 5 +- ovos_workshop/skills/active.py | 5 +- ovos_workshop/skills/auto_translatable.py | 97 +- ovos_workshop/skills/base.py | 1324 ++++++++++------- ovos_workshop/skills/decorators/__init__.py | 2 + ovos_workshop/skills/decorators/converse.py | 2 + .../skills/decorators/fallback_handler.py | 2 + ovos_workshop/skills/decorators/killable.py | 2 + ovos_workshop/skills/decorators/layers.py | 2 + ovos_workshop/skills/decorators/ocp.py | 2 + ovos_workshop/skills/fallback.py | 307 ++-- ovos_workshop/skills/idle_display_skill.py | 119 +- ovos_workshop/skills/intent_provider.py | 9 +- ovos_workshop/skills/layers.py | 2 + ovos_workshop/skills/mycroft_skill.py | 185 ++- ovos_workshop/skills/ovos.py | 208 ++- requirements/test.txt | 5 + .../skills/gui/ui/test.qml => __init__.py} | 0 .../vocab/en-us/test.intent => __init__.py} | 0 test/unittests/skills/test_active.py | 28 + .../skills/test_auto_translatable.py | 45 + test/unittests/skills/test_base.py | 407 +++++ test/unittests/skills/test_fallback_skill.py | 235 ++- .../gui/ui/test.qml} | 0 .../skills/test_idle_display_skill.py | 14 + .../skills/test_mycroft_skill/__init__.py | 0 .../intent_file/vocab/en-us/test.intent | 0 .../intent_file/vocab/en-us/test_ent.entity | 0 .../locale/en-us/turn_off2_test.voc | 0 .../locale/en-us/turn_off_test.voc | 0 .../skills/{ => test_mycroft_skill}/mocks.py | 0 .../test_mycroft_skill.py | 8 +- .../test_mycroft_skill_get_response.py | 0 .../test_skill/__init__.py | 0 .../dialog/en-us/what do you want.dialog | 0 .../in-dialog/dialog/en-us/good_things.list | 0 .../in-dialog/dialog/en-us/named_things.value | 0 .../in-dialog/dialog/en-us/test.template | 0 .../in-locale/locale/de-de/good_things.list | 0 .../in-locale/locale/de-de/named_things.value | 0 .../in-locale/locale/de-de/test.template | 0 .../in-locale/locale/en-us/good_things.list | 0 .../in-locale/locale/en-us/named_things.value | 0 .../in-locale/locale/en-us/not_in_german.list | 0 .../in-locale/locale/en-us/test.template | 0 .../test_ovos.py} | 144 +- test/unittests/test_abstract_app.py | 14 + test/unittests/test_skill.py | 45 +- test/unittests/test_skill_launcher.py | 6 +- 49 files changed, 2132 insertions(+), 1092 deletions(-) create mode 100644 requirements/test.txt rename test/{unittests/skills/gui/ui/test.qml => __init__.py} (100%) rename test/unittests/{skills/intent_file/vocab/en-us/test.intent => __init__.py} (100%) create mode 100644 test/unittests/skills/test_active.py create mode 100644 test/unittests/skills/test_auto_translatable.py create mode 100644 test/unittests/skills/test_base.py rename test/unittests/skills/{intent_file/vocab/en-us/test_ent.entity => test_gui/gui/ui/test.qml} (100%) create mode 100644 test/unittests/skills/test_idle_display_skill.py create mode 100644 test/unittests/skills/test_mycroft_skill/__init__.py create mode 100644 test/unittests/skills/test_mycroft_skill/intent_file/vocab/en-us/test.intent create mode 100644 test/unittests/skills/test_mycroft_skill/intent_file/vocab/en-us/test_ent.entity rename test/unittests/skills/{ => test_mycroft_skill}/locale/en-us/turn_off2_test.voc (100%) rename test/unittests/skills/{ => test_mycroft_skill}/locale/en-us/turn_off_test.voc (100%) rename test/unittests/skills/{ => test_mycroft_skill}/mocks.py (100%) rename test/unittests/skills/{ => test_mycroft_skill}/test_mycroft_skill.py (99%) rename test/unittests/skills/{ => test_mycroft_skill}/test_mycroft_skill_get_response.py (100%) rename test/unittests/skills/{ => test_mycroft_skill}/test_skill/__init__.py (100%) rename test/unittests/skills/{ => test_mycroft_skill}/test_skill/dialog/en-us/what do you want.dialog (100%) rename test/unittests/skills/{ => test_mycroft_skill}/translate/in-dialog/dialog/en-us/good_things.list (100%) rename test/unittests/skills/{ => test_mycroft_skill}/translate/in-dialog/dialog/en-us/named_things.value (100%) rename test/unittests/skills/{ => test_mycroft_skill}/translate/in-dialog/dialog/en-us/test.template (100%) rename test/unittests/skills/{ => test_mycroft_skill}/translate/in-locale/locale/de-de/good_things.list (100%) rename test/unittests/skills/{ => test_mycroft_skill}/translate/in-locale/locale/de-de/named_things.value (100%) rename test/unittests/skills/{ => test_mycroft_skill}/translate/in-locale/locale/de-de/test.template (100%) rename test/unittests/skills/{ => test_mycroft_skill}/translate/in-locale/locale/en-us/good_things.list (100%) rename test/unittests/skills/{ => test_mycroft_skill}/translate/in-locale/locale/en-us/named_things.value (100%) rename test/unittests/skills/{ => test_mycroft_skill}/translate/in-locale/locale/en-us/not_in_german.list (100%) rename test/unittests/skills/{ => test_mycroft_skill}/translate/in-locale/locale/en-us/test.template (100%) rename test/unittests/{test_skill_classes.py => skills/test_ovos.py} (50%) diff --git a/.github/workflows/unit_tests.yml b/.github/workflows/unit_tests.yml index 91600b7d..22c6eac0 100644 --- a/.github/workflows/unit_tests.yml +++ b/.github/workflows/unit_tests.yml @@ -49,12 +49,11 @@ jobs: python -m pip install build wheel - name: Install ovos workshop run: | - pip install . + pip install -e . - name: Install test dependencies run: | sudo apt install libssl-dev libfann-dev portaudio19-dev libpulse-dev - pip install ovos-core>=0.0.6a17 - pip install pytest pytest-timeout pytest-cov adapt-parser~=0.5 + pip install -r requirements/test.txt - name: Run unittests run: | pytest --cov=ovos_workshop --cov-report xml test/unittests diff --git a/ovos_workshop/skills/active.py b/ovos_workshop/skills/active.py index fd5b132e..325cd22c 100644 --- a/ovos_workshop/skills/active.py +++ b/ovos_workshop/skills/active.py @@ -9,8 +9,9 @@ def bind(self, bus): self.make_active() def handle_skill_deactivated(self, message=None): - """ skill is always in active skill list, - ie, converse is always called """ + """ + skill is always in active skill list, ie, converse is always called + """ self.make_active() diff --git a/ovos_workshop/skills/auto_translatable.py b/ovos_workshop/skills/auto_translatable.py index 0e8b0c05..0f29c39a 100644 --- a/ovos_workshop/skills/auto_translatable.py +++ b/ovos_workshop/skills/auto_translatable.py @@ -1,3 +1,5 @@ +from abc import ABC + from ovos_config import Configuration from ovos_plugin_manager.language import OVOSLangDetectionFactory, OVOSLangTranslationFactory from ovos_utils import get_handler_name @@ -5,53 +7,64 @@ from ovos_workshop.resource_files import SkillResources from ovos_workshop.skills.common_query_skill import CommonQuerySkill -from ovos_workshop.skills.ovos import OVOSSkill, OVOSFallbackSkill +from ovos_workshop.skills.ovos import OVOSSkill +from ovos_workshop.skills.fallback import FallbackSkillV2 class UniversalSkill(OVOSSkill): - ''' Skill that auto translates input/output from any language + """ + Skill that auto translates input/output from any language intent handlers are ensured to receive utterances in self.internal_language intent handlers are expected to produce utterances in self.internal_language - self.speak will always translate utterances from self.internal_lang to self.lang + self.speak will always translate utterances from + self.internal_lang to self.lang NOTE: self.lang reflects the original query language but received utterances are always in self.internal_language - ''' + """ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self.lang_detector = OVOSLangDetectionFactory.create() self.translator = OVOSLangTranslationFactory.create() - self.internal_language = None # the skill internally only works in this language - self.translate_tags = True # __tags__ private value will be translated (adapt entities) - self.translate_keys = ["utterance", "utterances"] # keys added here will have values translated in message.data + # the skill internally only works in this language + self.internal_language = None + # __tags__ private value will be translated (adapt entities) + self.translate_tags = True + # keys added here will have values translated in message.data + self.translate_keys = ["utterance", "utterances"] - # autodetect will detect the lang of the utterance regardless of what has been reported - # to test just type in the cli in another language and watch answers still coming + # autodetect will detect the lang of the utterance regardless of what + # has been reported to test just type in the cli in another language + # and watch answers still coming self.autodetect = False # TODO from mycroft.conf if self.internal_language is None: lang = Configuration().get("lang", "en-us") - LOG.warning(f"UniversalSkill are expected to specify their internal_language, casting to {lang}") + LOG.warning(f"UniversalSkill are expected to specify their " + f"internal_language, casting to {lang}") self.internal_language = lang def _load_lang(self, root_directory=None, lang=None): - """unlike base skill class all resources are in self.internal_language by default - instead of self.lang (which comes from message) + """ + unlike base skill class all resources are in self.internal_language by + default instead of self.lang (which comes from message) this ensures things like self.dialog_render reflect self.internal_lang """ lang = lang or self.internal_language # self.lang in base class root_directory = root_directory or self.res_dir if lang not in self._lang_resources: - self._lang_resources[lang] = SkillResources(root_directory, lang, skill_id=self.skill_id) + self._lang_resources[lang] = SkillResources(root_directory, lang, + skill_id=self.skill_id) return self._lang_resources[lang] def detect_language(self, utterance): try: return self.lang_detector.detect(utterance) - except: + except Exception as e: + LOG.error(e) # self.lang to account for lang defined in message return self.lang.split("-")[0] @@ -61,7 +74,8 @@ def translate_utterance(self, text, target_lang, sauce_lang=None): else: sauce_lang = sauce_lang or self.detect_language(text) if sauce_lang.split("-")[0] != target_lang: - translated = self.translator.translate(text, source=sauce_lang, target=target_lang) + translated = self.translator.translate(text, source=sauce_lang, + target=target_lang) LOG.info("translated " + text + " to " + translated) return translated return text @@ -76,11 +90,13 @@ def _translate_message(self, message): return message translation_data = {"original": {}, "translated": {}, - "source_lang": sauce_lang, "internal_lang": self.internal_language} + "source_lang": sauce_lang, + "internal_lang": self.internal_language} def _do_tx(thing): if isinstance(thing, str): - thing = self.translate_utterance(thing, target_lang=out_lang, sauce_lang=sauce_lang) + thing = self.translate_utterance(thing, target_lang=out_lang, + sauce_lang=sauce_lang) elif isinstance(thing, list): thing = [_do_tx(t) for t in thing] elif isinstance(thing, dict): @@ -90,16 +106,19 @@ def _do_tx(thing): for key in self.translate_keys: if key in message.data: translation_data["original"][key] = message.data[key] - translation_data["translated"][key] = message.data[key] = _do_tx(message.data[key]) + translation_data["translated"][key] = message.data[key] = \ + _do_tx(message.data[key]) # special case if self.translate_tags: translation_data["original"]["__tags__"] = message.data["__tags__"] for idx, token in enumerate(message.data["__tags__"]): - message.data["__tags__"][idx] = self.translate_utterance(token.get("key", ""), - target_lang=out_lang, - sauce_lang=sauce_lang) - translation_data["translated"]["__tags__"] = message.data["__tags__"] + message.data["__tags__"][idx] = \ + self.translate_utterance(token.get("key", ""), + target_lang=out_lang, + sauce_lang=sauce_lang) + translation_data["translated"]["__tags__"] = \ + message.data["__tags__"] message.context["translation_data"] = translation_data return message @@ -138,18 +157,20 @@ def speak(self, utterance, *args, **kwargs): super().speak(utterance, *args, **kwargs) -class UniversalFallback(UniversalSkill, OVOSFallbackSkill): - ''' Fallback Skill that auto translates input/output from any language +class UniversalFallback(UniversalSkill, FallbackSkillV2): + """ + Fallback Skill that auto translates input/output from any language - fallback handlers are ensured to receive utterances in self.internal_language - fallback handlers are expected to produce utterances in self.internal_language + fallback handlers are ensured to receive utterances and expected to produce + responses in self.internal_language - self.speak will always translate utterances from self.internal_lang to self.lang + self.speak will always translate utterances from + self.internal_lang to self.lang NOTE: self.lang reflects the original query language but received utterances are always in self.internal_language - ''' + """ def create_universal_fallback_handler(self, handler): def universal_fallback_handler(message): @@ -165,22 +186,25 @@ def universal_fallback_handler(message): def register_fallback(self, handler, priority): handler = self.create_universal_fallback_handler(handler) - super().register_fallback(handler, priority) + FallbackSkillV2.register_fallback(self, handler, priority) -class UniversalCommonQuerySkill(UniversalSkill, CommonQuerySkill): - ''' CommonQuerySkill that auto translates input/output from any language +class UniversalCommonQuerySkill(UniversalSkill, CommonQuerySkill, ABC): + """ + CommonQuerySkill that auto translates input/output from any language - CQS_match_query_phrase and CQS_action are ensured to received phrase in self.internal_language + CQS_match_query_phrase and CQS_action are ensured to received phrase in + self.internal_language - CQS_match_query_phrase is assumed to return a response in self.internal_lang - it will be translated back before speaking + CQS_match_query_phrase is assumed to return a response in self.internal_lang + it will be translated back before speaking - self.speak will always translate utterances from self.internal_lang to self.lang + self.speak will always translate utterances from + self.internal_lang to self.lang NOTE: self.lang reflects the original query language but received utterances are always in self.internal_language - ''' + """ def __handle_query_action(self, message): """Message handler for question:action. @@ -220,4 +244,3 @@ def __get_cq(self, search_phrase): def remove_noise(self, phrase, lang=None): """remove noise to produce essence of question""" return super().remove_noise(phrase, self.internal_language) - diff --git a/ovos_workshop/skills/base.py b/ovos_workshop/skills/base.py index 7dae3460..91d0079e 100644 --- a/ovos_workshop/skills/base.py +++ b/ovos_workshop/skills/base.py @@ -13,6 +13,7 @@ # limitations under the License. # """Common functionality relating to the implementation of mycroft skills.""" +import datetime import re import sys import time @@ -21,10 +22,11 @@ from hashlib import md5 from inspect import signature from itertools import chain -from os.path import join, abspath, dirname, basename, isfile, isdir +from os.path import join, abspath, dirname, basename, isfile from threading import Event -from typing import List +from typing import List, Optional, Dict, Callable, Union +from ovos_bus_client import MessageBusClient from ovos_bus_client.session import SessionManager from json_database import JsonStorage from lingua_franca.format import pronounce_number, join_list @@ -34,17 +36,19 @@ from ovos_config.config import Configuration from ovos_config.locations import get_xdg_config_save_path from ovos_utils import camel_case_split -from ovos_utils.dialog import get_dialog +from ovos_utils.dialog import get_dialog, MustacheDialogRenderer from ovos_utils.enclosure.api import EnclosureAPI -from ovos_utils.events import EventSchedulerInterface +from ovos_utils.events import EventContainer, EventSchedulerInterface from ovos_utils.file_utils import FileWatcher from ovos_utils.gui import GUIInterface, get_ui_directories from ovos_utils.intents import ConverseTracker from ovos_utils.intents import Intent, IntentBuilder -from ovos_utils.intents.intent_service_interface import munge_regex, munge_intent_parser, IntentServiceInterface +from ovos_utils.intents.intent_service_interface import munge_regex, \ + munge_intent_parser, IntentServiceInterface from ovos_utils.json_helper import merge_dict -from ovos_utils.log import LOG, deprecated -from ovos_utils.messagebus import get_handler_name, create_wrapper, EventContainer, get_message_lang +from ovos_utils.log import LOG, deprecated, log_deprecation +from ovos_utils.messagebus import get_handler_name, create_wrapper, \ + get_message_lang from ovos_utils.parse import match_one from ovos_utils.process_utils import RuntimeRequirements from ovos_utils.skills import get_non_properties @@ -63,31 +67,34 @@ # backwards compat alias class SkillNetworkRequirements(RuntimeRequirements): def __init__(self, *args, **kwargs): - LOG.warning("SkillNetworkRequirements has been renamed to RuntimeRequirements\n" - "from ovos_utils.process_utils import RuntimeRequirements") + log_deprecation("Replace with " + "`ovos_utils.process_utils.RuntimeRequirements`", + "0.1.0") super().__init__(*args, **kwargs) -def is_classic_core(): - """ Check if the current core is the classic mycroft-core """ +def is_classic_core() -> bool: + """ + Check if the current core is the classic mycroft-core + """ try: from mycroft.version import OVOS_VERSION_STR return False # ovos-core except ImportError: try: + log_deprecation("Support for `mycroft_core` will be deprecated", + "0.1.0") from mycroft.version import CORE_VERSION_STR return True # mycroft-core except ImportError: return False # standalone -def simple_trace(stack_trace): - """Generate a simplified traceback. - - Args: - stack_trace: Stack trace to simplify - - Returns: (str) Simplified stack trace. +def simple_trace(stack_trace: List[str]) -> str: + """ + Generate a simplified traceback. + @param stack_trace: Formatted stack trace (each string ends with \n) + @return: Stack trace with any empty lines removed and last line removed """ stack_trace = stack_trace[:-1] tb = 'Traceback:\n' @@ -131,50 +138,64 @@ class BaseSkill: bus (MycroftWebsocketClient): Optional bus connection """ - def __init__(self, name=None, bus=None, resources_dir=None, - settings: JsonStorage = None, - gui=None, enable_settings_manager=True, - skill_id=""): + def __init__(self, name: Optional[str] = None, + bus: Optional[MessageBusClient] = None, + resources_dir: Optional[str] = None, + settings: Optional[JsonStorage] = None, + gui: Optional[GUIInterface] = None, + enable_settings_manager: bool = True, + skill_id: str = ""): + """ + Create an OVOSSkill object. + @param name: DEPRECATED skill_name + @param bus: MessageBusClient to bind to skill + @param resources_dir: optional root resource directory (else defaults to + skill `root_dir` + @param settings: Optional settings object, else defined in skill config + path + @param gui: Optional SkillGUI, else one is initialized + @param enable_settings_manager: if True, enables a SettingsManager for + this skill to manage default settings and backend sync + @param skill_id: Unique ID for this skill + """ self.log = LOG # a dedicated namespace will be assigned in _startup self._enable_settings_manager = enable_settings_manager self._init_event = Event() self.name = name or self.__class__.__name__ self.resting_name = None - self.skill_id = skill_id # will be set by SkillLoader, guaranteed unique + self.skill_id = skill_id # set by SkillLoader, guaranteed unique self._settings_meta = None # DEPRECATED - backwards compat only self.settings_manager = None - # Get directory of skill - #: Member variable containing the absolute path of the skill's root - #: directory. E.g. $XDG_DATA_HOME/mycroft/skills/my-skill.me/ + # Get directory of skill source (__init__.py) self.root_dir = dirname(abspath(sys.modules[self.__module__].__file__)) self.res_dir = resources_dir or self.root_dir self.gui = gui - self._bus = bus self._enclosure = EnclosureAPI() - #: Mycroft global configuration. (dict) - self.config_core = Configuration() + # Core configuration + self.config_core: Configuration = Configuration() self._settings = None self._initial_settings = settings or dict() self._settings_watchdog = None - #: Set to register a callback method that will be called every time - #: the skills settings are updated. The referenced method should - #: include any logic needed to handle the updated settings. + # Override to register a callback method that will be called every time + # the skill's settings are updated. The referenced method should + # include any logic needed to handle the updated settings. self.settings_change_callback = None # fully initialized when self.skill_id is set self._file_system = None - self.log = LOG - self.reload_skill = True #: allow reloading (default True) + self.reload_skill = True # allow reloading (default True) self.events = EventContainer(bus) + + # Cached voc file contents self._voc_cache = {} # loaded lang file resources @@ -185,7 +206,7 @@ def __init__(self, name=None, bus=None, resources_dir=None, self.intent_service = IntentServiceInterface() # Skill Public API - self.public_api = {} + self.public_api: Dict[str, dict] = {} self.__original_converse = self.converse @@ -193,49 +214,51 @@ def __init__(self, name=None, bus=None, resources_dir=None, if self.skill_id and self.bus: self._startup(self.bus, self.skill_id) - # classproperty not present in mycroft-core @classproperty - def runtime_requirements(self): - """ skill developers should override this if they do not require connectivity - - some examples: - - IOT skill that controls skills via LAN could return: - scans_on_init = True - RuntimeRequirements(internet_before_load=False, - network_before_load=scans_on_init, - requires_internet=False, - requires_network=True, - no_internet_fallback=True, - no_network_fallback=False) - - online search skill with a local cache: - has_cache = False - RuntimeRequirements(internet_before_load=not has_cache, - network_before_load=not has_cache, - requires_internet=True, - requires_network=True, - no_internet_fallback=True, - no_network_fallback=True) - - a fully offline skill: - RuntimeRequirements(internet_before_load=False, - network_before_load=False, - requires_internet=False, - requires_network=False, - no_internet_fallback=True, - no_network_fallback=True) + def runtime_requirements(self) -> RuntimeRequirements: + """ + Override to specify what a skill expects to be available at init and at + runtime. Default will assume network and internet are required and GUI + is not required for backwards-compat. + + some examples: + + IOT skill that controls skills via LAN could return: + scans_on_init = True + RuntimeRequirements(internet_before_load=False, + network_before_load=scans_on_init, + requires_internet=False, + requires_network=True, + no_internet_fallback=True, + no_network_fallback=False) + + online search skill with a local cache: + has_cache = False + RuntimeRequirements(internet_before_load=not has_cache, + network_before_load=not has_cache, + requires_internet=True, + requires_network=True, + no_internet_fallback=True, + no_network_fallback=True) + + a fully offline skill: + RuntimeRequirements(internet_before_load=False, + network_before_load=False, + requires_internet=False, + requires_network=False, + no_internet_fallback=True, + no_network_fallback=True) """ return RuntimeRequirements() @classproperty - def network_requirements(self): + def network_requirements(self) -> RuntimeRequirements: LOG.warning("network_requirements renamed to runtime_requirements, " "will be removed in ovos-core 0.0.8") return self.runtime_requirements @property - def voc_match_cache(self): + def voc_match_cache(self) -> Dict[str, List[str]]: """ Backwards-compatible accessor method for vocab cache @return: dict vocab resources to parsed resources @@ -249,166 +272,48 @@ def voc_match_cache(self, val): if isinstance(val, dict): self._voc_cache = val - # property not present in mycroft-core + # not a property in mycroft-core @property - def _is_fully_initialized(self): - """Determines if the skill has been fully loaded and setup. - When True all data has been loaded and all internal state and events setup""" - return self._init_event.is_set() - - # method not present in mycroft-core - def _handle_first_run(self): - """The very first time a skill is run, speak the intro.""" - intro = self.get_intro_message() - if intro: - # supports .dialog files for easy localization - # when .dialog does not exist, the text is spoken - # it is backwards compatible - self.speak_dialog(intro) - - # method not present in mycroft-core - def _check_for_first_run(self): - """Determine if its the very first time a skill is run.""" - first_run = self.settings.get("__mycroft_skill_firstrun", True) - if first_run: - self.log.info("First run of " + self.skill_id) - self._handle_first_run() - self.settings["__mycroft_skill_firstrun"] = False - self.settings.store() - - # method not present in mycroft-core - def _startup(self, bus, skill_id=""): - """Startup the skill. - - This connects the skill to the messagebus, loads vocabularies and - data files and in the end calls the skill creator's "intialize" code. - - Arguments: - bus: Mycroft Messagebus connection object. - skill_id (str): need to be unique, by default is set from skill path - but skill loader can override this + def _is_fully_initialized(self) -> bool: """ - if self._is_fully_initialized: - self.log.warning(f"Tried to initialize {self.skill_id} multiple times, ignoring") - return - - # NOTE: this method is called by SkillLoader - # it is private to make it clear to skill devs they should not touch it - try: - # set the skill_id - self.skill_id = skill_id or basename(self.root_dir) - self.intent_service.set_id(self.skill_id) - self.event_scheduler.set_id(self.skill_id) - self.enclosure.set_id(self.skill_id) - - # initialize anything that depends on skill_id - self.log = LOG.create_logger(self.skill_id) - self._init_settings() - - # initialize anything that depends on the messagebus - self.bind(bus) - if not self.gui: - self._init_skill_gui() - if self._enable_settings_manager: - self._init_settings_manager() - self.load_data_files() - self._register_decorated() - self.register_resting_screen() - - # run skill developer initialization code - self.initialize() - self._check_for_first_run() - self._init_event.set() - except Exception as e: - self.log.exception('Skill initialization failed') - # If an exception occurs, attempt to clean up the skill - try: - self.default_shutdown() - except Exception as e2: - pass - raise e - - def _init_settings(self): - """Setup skill settings.""" - self.log.debug(f"initializing skill settings for {self.skill_id}") - - # NOTE: lock is disabled due to usage of deepcopy and to allow json serialization - self._settings = JsonStorage(self._settings_path, disable_lock=True) - if self._initial_settings and not self._is_fully_initialized: - self.log.warning("Copying default settings values defined in __init__ \n" - f"to correct this add kwargs __init__(bus=None, skill_id='') " - f"to skill class {self.__class__.__name__}") - for k, v in self._initial_settings.items(): - if k not in self._settings: - self._settings[k] = v - self._initial_settings = copy(self.settings) - - self._start_filewatcher() - - # method not in mycroft-core - def _init_skill_gui(self): - try: - self.gui = SkillGUI(self) - self.gui.setup_default_handlers() - except ImportError: - self.gui = GUIInterface(self.skill_id) - if self.bus: - self.gui.set_bus(self.bus) - - # method not in mycroft-core - def _init_settings_manager(self): - self.settings_manager = SkillSettingsManager(self) - - # method not present in mycroft-core - def _start_filewatcher(self): - if self._settings_watchdog is None and isfile(self._settings.path): - self._settings_watchdog = FileWatcher([self._settings.path], - callback=self._handle_settings_file_change, - ignore_creation=True) - - # method not present in mycroft-core - def _upload_settings(self): - if self.settings_manager and self.config_core.get("skills", {}).get("sync2way"): - # upload new settings to backend - generate = self.config_core.get("skills", {}).get("autogen_meta", True) - self.settings_manager.upload(generate) # this will check global sync flag - if generate: - # update settingsmeta file on disk - self.settings_manager.save_meta() - - # method not present in mycroft-core - def _handle_settings_file_change(self, path): - if self._settings: - self._settings.reload() - if self.settings_change_callback: - try: - self.settings_change_callback() - except: - self.log.exception("settings change callback failed, " - "file changes not handled!") - self._upload_settings() + Determines if the skill has been fully loaded and setup. + When True, all data has been loaded and all internal state + and events set up. + """ + return self._init_event.is_set() # not a property in mycroft-core @property - def _settings_path(self): - return join(get_xdg_config_save_path(), 'skills', self.skill_id, 'settings.json') + def _settings_path(self) -> str: + """ + Absolute file path of this skill's `settings.json` (file may not exist) + """ + return join(get_xdg_config_save_path(), 'skills', self.skill_id, + 'settings.json') # not a property in mycroft-core @property - def settings(self): + def settings(self) -> JsonStorage: + """ + Get settings specific to this skill + """ if self._settings is not None: return self._settings else: - self.log.warning('Skill not fully initialized. ' - 'Only default values can be set, no settings can be read or changed.' - f"to correct this add kwargs __init__(bus=None, skill_id='') " + self.log.warning('Skill not fully initialized. Only default values ' + 'can be set, no settings can be read or changed.' + f"to correct this add kwargs " + f"__init__(bus=None, skill_id='') " f"to skill class {self.__class__.__name__}") self.log.error(simple_trace(traceback.format_stack())) return self._initial_settings # not a property in mycroft-core @settings.setter - def settings(self, val): + def settings(self, val: dict): + """ + Update settings specific to this skill + """ assert isinstance(val, dict) # init method if self._settings is None: @@ -420,26 +325,34 @@ def settings(self, val): # not a property in mycroft-core @property - def dialog_renderer(self): + def dialog_renderer(self) -> Optional[MustacheDialogRenderer]: + """ + Get a dialog renderer for this skill. Language will be determined by + message history to match the language associated with the current + session or else from Configuration. + """ return self._resources.dialog_renderer @property - def enclosure(self): + def enclosure(self) -> EnclosureAPI: + """ + Get an EnclosureAPI object to interact with hardware + """ if self._enclosure: return self._enclosure else: self.log.warning('Skill not fully initialized.' - f"to correct this add kwargs __init__(bus=None, skill_id='') " + f"to correct this add kwargs " + f"__init__(bus=None, skill_id='') " f"to skill class {self.__class__.__name__}") self.log.error(simple_trace(traceback.format_stack())) raise Exception('Accessed MycroftSkill.enclosure in __init__') # not a property in mycroft-core @property - def file_system(self): - """ Filesystem access to skill specific folder. - - See mycroft.filesystem for details. + def file_system(self) -> FileSystemAccess: + """ + Get an object that provides managed access to a local Filesystem. """ if not self._file_system and self.skill_id: self._file_system = FileSystemAccess(join('skills', self.skill_id)) @@ -453,24 +366,40 @@ def file_system(self): raise Exception('Accessed MycroftSkill.file_system in __init__') @file_system.setter - def file_system(self, fs): - """Provided mainly for backwards compatibility with derivative MycroftSkill classes - Skills are advised against redefining the file system directory""" + def file_system(self, fs: FileSystemAccess): + """ + Provided mainly for backwards compatibility with derivative + MycroftSkill classes. Skills are advised against redefining the file + system directory. + @param fs: new FileSystemAccess object to use + """ + self.log.warning(f"Skill manually overriding file_system path to: " + f"{fs.path}") self._file_system = fs @property - def bus(self): + def bus(self) -> MessageBusClient: + """ + Get the MessageBusClient bound to this skill + """ if self._bus: return self._bus else: self.log.warning('Skill not fully initialized.' - f"to correct this add kwargs __init__(bus=None, skill_id='') " + f"to correct this add kwargs " + f"__init__(bus=None, skill_id='') " f"to skill class {self.__class__.__name__}") self.log.error(simple_trace(traceback.format_stack())) raise Exception('Accessed MycroftSkill.bus in __init__') @bus.setter - def bus(self, value): + def bus(self, value: MessageBusClient): + """ + Set the MessageBusClient bound to this skill. Note that setting this + after init may have unintended consequences as expected events might + not be registered. Call `bind` to connect a new MessageBusClient. + @param value: new MessageBusClient object + """ from ovos_bus_client import MessageBusClient from ovos_utils.messagebus import FakeBus if isinstance(value, (MessageBusClient, FakeBus)): @@ -479,31 +408,40 @@ def bus(self, value): raise TypeError(f"Expected a MessageBusClient, got: {type(value)}") @property - def location(self): - """Get the JSON data struction holding location information.""" + def location(self) -> dict: + """ + Get the JSON data struction holding location information. + """ # TODO: Allow Enclosure to override this for devices that - # contain a GPS. + # contain a GPS. return self.config_core.get('location') @property - def location_pretty(self): - """Get a more 'human' version of the location as a string.""" + def location_pretty(self) -> Optional[str]: + """ + Get a speakable city from the location config if available + """ loc = self.location if type(loc) is dict and loc['city']: return loc['city']['name'] return None @property - def location_timezone(self): - """Get the timezone code, such as 'America/Los_Angeles'""" + def location_timezone(self) -> Optional[str]: + """ + Get the timezone code, such as 'America/Los_Angeles' + """ loc = self.location if type(loc) is dict and loc['timezone']: return loc['timezone']['code'] return None @property - def lang(self): - """Get the current language.""" + def lang(self) -> str: + """ + Get the current language as a BCP-47 language code. This will consider + current session data if available, else Configuration. + """ lang = self._core_lang message = dig_for_message() if message: @@ -512,82 +450,266 @@ def lang(self): # property not present in mycroft-core @property - def _core_lang(self): - """Get the configured default language. - NOTE: this should be public, but since if a skill uses this it wont - work in regular mycroft-core it was made private!""" + def _core_lang(self) -> str: + """ + Get the configured default language as a BCP-47 language code. + + NOTE: this should be public, but since if a skill uses this it won't + work in regular mycroft-core it was made private! + """ return self.config_core.get("lang", "en-us").lower() # property not present in mycroft-core @property - def _secondary_langs(self): - """Get the configured secondary languages, mycroft is not - considered to be in these languages, but will load its resource - files. This provides initial support for multilingual input. A skill - may override this method to specify which languages intents are - registered in. - NOTE: this should be public, but since if a skill uses this it wont - work in regular mycroft-core it was made private!""" - return [l.lower() for l in self.config_core.get('secondary_langs', []) - if l != self._core_lang] + def _secondary_langs(self) -> List[str]: + """ + Get the configured secondary languages; resources will be loaded for + these languages to provide support for multilingual input, in addition + to `core_lang`. A skill may override this method to specify which + languages intents are registered in. + + NOTE: this should be public, but since if a skill uses this it won't + work in regular mycroft-core it was made private! + """ + return [lang.lower() for lang in self.config_core.get( + 'secondary_langs', []) if lang != self._core_lang] # property not present in mycroft-core @property - def _native_langs(self): - """Languages natively supported by core - ie, resource files available and explicitly supported - NOTE: this should be public, but since if a skill uses this it wont + def _native_langs(self) -> List[str]: + """ + Languages natively supported by this skill (ie, resource files available + and explicitly supported). This is equivalent to normalized + secondary_langs + core_lang. + + NOTE: this should be public, but since if a skill uses this it won't work in regular mycroft-core it was made private! """ - valid = set([l.lower() for l in self._secondary_langs - if '-' in l and l != self._core_lang] + [self._core_lang]) + valid = set([lang.lower() for lang in self._secondary_langs if '-' in + lang and lang != self._core_lang] + [self._core_lang]) return list(valid) # property not present in mycroft-core @property - def _alphanumeric_skill_id(self): - """skill id converted to only alphanumeric characters - Non alpha-numeric characters are converted to "_" + def _alphanumeric_skill_id(self) -> str: + """ + Skill id converted to only alphanumeric characters and "_". + Non alphanumeric characters are converted to "_" - NOTE: this should be public, but since if a skill uses this it wont + NOTE: this should be public, but since if a skill uses this it won't work in regular mycroft-core it was made private! - - Returns: - (str) String of letters """ return ''.join(c if c.isalnum() else '_' for c in str(self.skill_id)) # property not present in mycroft-core @property - def _resources(self): - """Instantiates a ResourceFileLocator instance when needed. - a new instance is always created to ensure self.lang - reflects the active language and not the default core language - NOTE: this should be public, but since if a skill uses this it wont + def _resources(self) -> SkillResources: + """ + Get a SkillResources object for the current language. Objects are + initialized for the current language as needed. + + NOTE: this should be public, but since if a skill uses this it won't work in regular mycroft-core it was made private! """ return self._load_lang(self.res_dir, self.lang) + # property not present in mycroft-core + @property + def _stop_is_implemented(self) -> bool: + """ + True if this skill implements a `stop` method + """ + return self.__class__.stop is not BaseSkill.stop + + # property not present in mycroft-core + @property + def _converse_is_implemented(self) -> bool: + """ + True if this skill implements a `converse` method + """ + return self.__class__.converse is not BaseSkill.converse or \ + self.__original_converse != self.converse + + # method not present in mycroft-core + def _handle_first_run(self): + """ + The very first time a skill is run, speak a provided intro_message. + """ + intro = self.get_intro_message() + if intro: + # supports .dialog files for easy localization + # when .dialog does not exist, the text is spoken + # it is backwards compatible + self.speak_dialog(intro) + + # method not present in mycroft-core + def _check_for_first_run(self): + """ + Determine if this is the very first time a skill is run by looking for + `__mycroft_skill_firstrun` in skill settings. + """ + first_run = self.settings.get("__mycroft_skill_firstrun", True) + if first_run: + self.log.info("First run of " + self.skill_id) + self._handle_first_run() + self.settings["__mycroft_skill_firstrun"] = False + self.settings.store() + + def _startup(self, bus: MessageBusClient, skill_id: str = ""): + """ + Startup the skill. Connects the skill to the messagebus, loads resources + and finally calls the skill's "intialize" method. + @param bus: MessageBusClient to bind to skill + @param skill_id: Unique skill identifier, defaults to skill path for + legacy skills and python entrypoints for modern skills + """ + if self._is_fully_initialized: + self.log.warning(f"Tried to initialize {self.skill_id} multiple " + f"times, ignoring") + return + + # NOTE: this method is called by SkillLoader + # it is private to make it clear to skill devs they should not touch it + try: + # set the skill_id + self.skill_id = skill_id or basename(self.root_dir) + self.intent_service.set_id(self.skill_id) + self.event_scheduler.set_id(self.skill_id) + self.enclosure.set_id(self.skill_id) + + # initialize anything that depends on skill_id + self.log = LOG.create_logger(self.skill_id) + self._init_settings() + + # initialize anything that depends on the messagebus + self.bind(bus) + if not self.gui: + self._init_skill_gui() + if self._enable_settings_manager: + self._init_settings_manager() + self.load_data_files() + self._register_decorated() + self.register_resting_screen() + + # run skill developer initialization code + self.initialize() + self._check_for_first_run() + self._init_event.set() + except Exception as e: + self.log.exception('Skill initialization failed') + # If an exception occurs, attempt to clean up the skill + try: + self.default_shutdown() + except Exception as e2: + LOG.debug(e2) + raise e + + def _init_settings(self): + """ + Set up skill settings. Defines settings in the specified file path, + handles any settings passed to skill init, and starts watching the + settings file for changes. + """ + self.log.debug(f"initializing skill settings for {self.skill_id}") + + # NOTE: lock is disabled due to usage of deepcopy and to allow json + # serialization + self._settings = JsonStorage(self._settings_path, disable_lock=True) + if self._initial_settings and not self._is_fully_initialized: + self.log.warning("Copying default settings values defined in " + "__init__ \nto correct this add kwargs " + "__init__(bus=None, skill_id='') " + f"to skill class {self.__class__.__name__}") + for k, v in self._initial_settings.items(): + if k not in self._settings: + self._settings[k] = v + self._initial_settings = copy(self.settings) + + self._start_filewatcher() + + def _init_skill_gui(self): + """ + Set up the SkillGUI for this skill and connect relevant bus events. + """ + self.gui = SkillGUI(self) + self.gui.setup_default_handlers() + + def _init_settings_manager(self): + """ + Set up the SkillSettingsManager for this skill. + """ + self.settings_manager = SkillSettingsManager(self) + + def _start_filewatcher(self): + """ + Start watching settings for file changes if settings file exists and + there isn't already a FileWatcher watching it + """ + if self._settings_watchdog is None and isfile(self._settings.path): + self._settings_watchdog = \ + FileWatcher([self._settings.path], + callback=self._handle_settings_file_change, + ignore_creation=True) + + # method not present in mycroft-core + def _upload_settings(self): + """ + Upload settings to a remote backend if configured. + """ + if self.settings_manager and self.config_core.get("skills", + {}).get("sync2way"): + # upload new settings to backend + generate = self.config_core.get("skills", {}).get("autogen_meta", + True) + # this will check global sync flag + self.settings_manager.upload(generate) + if generate: + # update settingsmeta file on disk + self.settings_manager.save_meta() + # method not present in mycroft-core - def _load_lang(self, root_directory=None, lang=None): - """Instantiates a ResourceFileLocator instance when needed. - a new instance is always created to ensure lang - reflects the active language and not the default core language - NOTE: this should be public, but since if a skill uses this it wont + def _handle_settings_file_change(self, path: str): + """ + Handle a FileWatcher notification that a file was changed. Reload + settings, call `self.settings_change_callback` if defined, and upload + changes if a backend is configured. + @param path: Modified file path + """ + if self._settings: + self._settings.reload() + if self.settings_change_callback: + try: + self.settings_change_callback() + except Exception as e: + self.log.exception("settings change callback failed, " + f"file changes not handled!: {e}") + self._upload_settings() + + # method not present in mycroft-core + def _load_lang(self, root_directory: Optional[str] = None, + lang: Optional[str] = None) -> SkillResources: + """ + Get a SkillResources object for this skill in the requested `lang` for + resource files in the requested `root_directory`. + @param root_directory: root path to find resources (default res_dir) + @param lang: language to get resources for (default self.lang) + @return: SkillResources object + + NOTE: this should be public, but since if a skill uses this it won't work in regular mycroft-core it was made private! """ lang = lang or self.lang root_directory = root_directory or self.res_dir if lang not in self._lang_resources: - self._lang_resources[lang] = SkillResources(root_directory, lang, skill_id=self.skill_id) + self._lang_resources[lang] = SkillResources(root_directory, lang, + skill_id=self.skill_id) return self._lang_resources[lang] - def bind(self, bus): - """Register messagebus emitter with skill. - - Args: - bus: Mycroft messagebus connection + def bind(self, bus: MessageBusClient): + """ + Register MessageBusClient with skill. + @param bus: MessageBusClient to bind to skill and internal objects """ if bus: self._bus = bus @@ -599,27 +721,28 @@ def bind(self, bus): self._register_public_api() if is_classic_core(): - # inject ovos exclusive features in vanila mycroft-core if possible - ## limited support for missing skill deactivated event + log_deprecation("Support for mycroft-core is deprecated", + "0.1.0") + # inject ovos exclusive features in vanilla mycroft-core + # if possible + # limited support for missing skill deactivated event # TODO - update ConverseTracker ConverseTracker.connect_bus(self.bus) # pull/1468 self.add_event("converse.skill.deactivated", - self._handle_skill_deactivated, speak_errors=False) + self._handle_skill_deactivated, + speak_errors=False) def _register_public_api(self): - """ Find and register api methods. - Api methods has been tagged with the api_method member, for each - method where this is found the method a message bus handler is - registered. - Finally create a handler for fetching the api info from any requesting - skill. + """ + Find and register API methods decorated with `@api_method` and create a + messagebus handler for fetching the api info if any handlers exist. """ - def wrap_method(func): - """Boiler plate for returning the response to the sender.""" + def wrap_method(fn): + """Boilerplate for returning the response to the sender.""" def wrapper(message): - result = func(*message.data['args'], **message.data['kwargs']) + result = fn(*message.data['args'], **message.data['kwargs']) message.context["skill_id"] = self.skill_id self.bus.emit(message.response(data={'result': result})) @@ -642,7 +765,8 @@ def wrapper(message): for key in self.public_api: if ('type' in self.public_api[key] and 'func' in self.public_api[key]): - self.log.debug(f"Adding api method: {self.public_api[key]['type']}") + self.log.debug(f"Adding api method: " + f"{self.public_api[key]['type']}") # remove the function member since it shouldn't be # reused and can't be sent over the messagebus @@ -654,44 +778,46 @@ def wrapper(message): self.add_event(f'{self.skill_id}.public_api', self._send_public_api, speak_errors=False) - # property not present in mycroft-core - @property - def _stop_is_implemented(self): - return self.__class__.stop is not BaseSkill.stop - - # property not present in mycroft-core - @property - def _converse_is_implemented(self): - return self.__class__.converse is not BaseSkill.converse or \ - self.__original_converse != self.converse - def _register_system_event_handlers(self): - """Add all events allowing the standard interaction with the Mycroft - system. + """ + Register default messagebus event handlers """ # Only register stop if it's been implemented if self._stop_is_implemented: - self.add_event('mycroft.stop', self.__handle_stop, speak_errors=False) - self.add_event('skill.converse.ping', self._handle_converse_ack, speak_errors=False) - self.add_event('skill.converse.request', self._handle_converse_request, speak_errors=False) - self.add_event(f"{self.skill_id}.activate", self.handle_activate, speak_errors=False) - self.add_event(f"{self.skill_id}.deactivate", self.handle_deactivate, speak_errors=False) - self.add_event("intent.service.skills.deactivated", self._handle_skill_deactivated, speak_errors=False) - self.add_event("intent.service.skills.activated", self._handle_skill_activated, speak_errors=False) - self.add_event('mycroft.skill.enable_intent', self.handle_enable_intent, speak_errors=False) - self.add_event('mycroft.skill.disable_intent', self.handle_disable_intent, speak_errors=False) - self.add_event('mycroft.skill.set_cross_context', self.handle_set_cross_context, speak_errors=False) - self.add_event('mycroft.skill.remove_cross_context', self.handle_remove_cross_context, speak_errors=False) - self.add_event('mycroft.skills.settings.changed', self.handle_settings_change, speak_errors=False) - - def handle_settings_change(self, message): - """Update settings if the remote settings changes apply to this skill. + self.add_event('mycroft.stop', self.__handle_stop, + speak_errors=False) + self.add_event('skill.converse.ping', self._handle_converse_ack, + speak_errors=False) + self.add_event('skill.converse.request', self._handle_converse_request, + speak_errors=False) + self.add_event(f"{self.skill_id}.activate", self.handle_activate, + speak_errors=False) + self.add_event(f"{self.skill_id}.deactivate", self.handle_deactivate, + speak_errors=False) + self.add_event("intent.service.skills.deactivated", + self._handle_skill_deactivated, speak_errors=False) + self.add_event("intent.service.skills.activated", + self._handle_skill_activated, speak_errors=False) + self.add_event('mycroft.skill.enable_intent', self.handle_enable_intent, + speak_errors=False) + self.add_event('mycroft.skill.disable_intent', + self.handle_disable_intent, speak_errors=False) + self.add_event('mycroft.skill.set_cross_context', + self.handle_set_cross_context, speak_errors=False) + self.add_event('mycroft.skill.remove_cross_context', + self.handle_remove_cross_context, speak_errors=False) + self.add_event('mycroft.skills.settings.changed', + self.handle_settings_change, speak_errors=False) + + def handle_settings_change(self, message: Message): + """ + Update settings if a remote settings changes apply to this skill. The skill settings downloader uses a single API call to retrieve the - settings for all skills. This is done to limit the number API calls. + settings for all skills to limit the number API calls. A "mycroft.skills.settings.changed" event is emitted for each skill - that had their settings changed. Only update this skill's settings - if its remote settings were among those changed + with settings changes. Only update this skill's settings if its remote + settings were among those changed. """ remote_settings = message.data.get(self.skill_id) if remote_settings is not None: @@ -701,68 +827,84 @@ def handle_settings_change(self, message): if self.settings_change_callback is not None: try: self.settings_change_callback() - except: + except Exception as e: self.log.exception("settings change callback failed, " - "remote changes not handled!") + f"remote changes not handled!: {e}") self._start_filewatcher() def detach(self): + """ + Detach all intents for this skill from the intent_service. + """ for (name, _) in self.intent_service: name = f'{self.skill_id}:{name}' self.intent_service.detach_intent(name) def initialize(self): - """Perform any final setup needed for the skill. - - Invoked after the skill is fully constructed and registered with the - system. + """ + Legacy method overridden by skills to perform extra init after __init__. + Skills should now move any code in this method to `__init__`, after a + call to `super().__init__`. """ pass - def _send_public_api(self, message): - """Respond with the skill's public api.""" + def _send_public_api(self, message: Message): + """ + Respond with the skill's public api. + @param message: `{self.skill_id}.public_api` Message + """ message.context["skill_id"] = self.skill_id self.bus.emit(message.response(data=self.public_api)) - def get_intro_message(self): - """Get a message to speak on first load of the skill. - - Useful for post-install setup instructions. - - Returns: - str: message that will be spoken to the user + def get_intro_message(self) -> str: """ - return None + Override to return a string to speak on first run. i.e. for post-install + setup instructions. + """ + return "" # method not present in mycroft-core - def _handle_skill_activated(self, message): - """ intent service activated a skill - if it was this skill fire the skill activation event""" + def _handle_skill_activated(self, message: Message): + """ + Intent service activated a skill. If it was this skill, + emit a skill activation message. + @param message: `intent.service.skills.activated` Message + """ if message.data.get("skill_id") == self.skill_id: self.bus.emit(message.forward(f"{self.skill_id}.activate")) # method not present in mycroft-core - def handle_activate(self, message): - """ skill is now considered active by the intent service - converse method will be called, skills might want to prepare/resume + def handle_activate(self, message: Message): + """ + Called when this skill is considered active by the intent service; + converse method will be called with every utterance. + Override this method to do any optional preparation. + @param message: `{self.skill_id}.activate` Message """ # method not present in mycroft-core def _handle_skill_deactivated(self, message): - """ intent service deactivated a skill - if it was this skill fire the skill deactivation event""" + """ + Intent service deactivated a skill. If it was this skill, + emit a skill deactivation message. + @param message: `intent.service.skills.deactivated` Message + """ if message.data.get("skill_id") == self.skill_id: self.bus.emit(message.forward(f"{self.skill_id}.deactivate")) # method not present in mycroft-core def handle_deactivate(self, message): - """ skill is no longer considered active by the intent service - converse method will not be called, skills might want to reset state here + """ + Called when this skill is no longer considered active by the intent + service; converse method will not be called until skill is active again. + Override this method to do any optional cleanup. + @param message: `{self.skill_id}.deactivate` Message """ # named make_active in mycroft-core def _activate(self): - """Bump skill to active_skill list in intent_service. + """ + Mark this skill as active and push to the top of the active skills list. This enables converse method to be called even without skill being used in last 5 minutes. """ @@ -770,18 +912,21 @@ def _activate(self): if "skill_id" not in msg.context: msg.context["skill_id"] = self.skill_id - m1 = msg.forward("intent.service.skills.activate", data={"skill_id": self.skill_id}) + m1 = msg.forward("intent.service.skills.activate", + data={"skill_id": self.skill_id}) self.bus.emit(m1) # backwards compat with mycroft-core # TODO - remove soon - m2 = msg.forward("active_skill_request", data={"skill_id": self.skill_id}) + m2 = msg.forward("active_skill_request", + data={"skill_id": self.skill_id}) self.bus.emit(m2) # method not present in mycroft-core def _deactivate(self): - """remove skill from active_skill list in intent_service. - This stops converse method from being called + """ + Mark this skill as inactive and remove from the active skills list. + This stops converse method from being called. """ msg = dig_for_message() or Message("") if "skill_id" not in msg.context: @@ -789,10 +934,14 @@ def _deactivate(self): self.bus.emit(msg.forward(f"intent.service.skills.deactivate", data={"skill_id": self.skill_id})) - # method not present in mycroft-core - def _handle_converse_ack(self, message): - """Inform skills service if we want to handle converse. - individual skills may override the property self.converse_is_implemented""" + def _handle_converse_ack(self, message: Message): + """ + Inform skills service if we want to handle converse. Individual skills + may override the property self.converse_is_implemented to enable or + disable converse support. Note that this does not affect a skill's + `active` status. + @param message: `skill.converse.ping` Message + """ self.bus.emit(message.reply( "skill.converse.pong", data={"skill_id": self.skill_id, @@ -800,9 +949,11 @@ def _handle_converse_ack(self, message): context={"skill_id": self.skill_id})) # method not present in mycroft-core - def _handle_converse_request(self, message): - """Check if the targeted skill id can handle conversation - If supported, the conversation is invoked. + def _handle_converse_request(self, message: Message): + """ + If this skill is requested and supports converse, handle the user input + with `converse`. + @param message: `skill.converse.request` Message """ skill_id = message.data['skill_id'] if skill_id == self.skill_id: @@ -817,33 +968,27 @@ def _handle_converse_request(self, message): self.bus.emit(message.reply('skill.converse.response', {"skill_id": self.skill_id, "result": result})) - except Exception: + except Exception as e: + LOG.error(e) self.bus.emit(message.reply('skill.converse.response', {"skill_id": self.skill_id, "result": False})) - def converse(self, message=None): - """Handle conversation. - - This method gets a peek at utterances before the normal intent - handling process after a skill has been invoked once. - - To use, override the converse() method and return True to - indicate that the utterance has been handled. - - utterances and lang are depreciated - - Args: - message: a message object containing a message type with an - optional JSON data packet - - Returns: - bool: True if an utterance was handled, otherwise False + def converse(self, message: Optional[Message] = None) -> bool: + """ + Override to handle an utterance before intent parsing while this skill + is active. Active skills are called in order of most recently used to + least recently used until one handles the converse request. If no skill + handles an utterance in `converse`, then the utterance will continue to + normal intent parsing. + @param message: Message containing user utterances to optionally handle + @return: True if the utterance was handled, else False """ return False def __get_response(self): - """Helper to get a response from the user + """ + Helper to get a response from the user NOTE: There is a race condition here. There is a small amount of time between the end of the device speaking and the converse method @@ -857,7 +1002,7 @@ def __get_response(self): Returns: str: user's response or None on a timeout """ - + # TODO: Support `message` signature like default? def converse(utterances, lang=None): converse.response = utterances[0] if utterances else None converse.finished = True @@ -874,6 +1019,7 @@ def converse(utterances, lang=None): # AbortEvent exception to kill the thread start = time.time() while time.time() - start <= 15 and not converse.finished: + # TODO: Refactor to event-based handling time.sleep(0.1) if self.__response is not False: if self.__response is None: @@ -884,40 +1030,27 @@ def converse(utterances, lang=None): self.converse = self.__original_converse return converse.response - def get_response(self, dialog='', data=None, validator=None, - on_fail=None, num_retries=-1): - """Get response from user. - - If a dialog is supplied it is spoken, followed immediately by listening - for a user response. If the dialog is omitted listening is started - directly. - - The response can optionally be validated before returning. - - Example:: - - color = self.get_response('ask.favorite.color') - - Args: - dialog (str): Optional dialog to speak to the user - data (dict): Data used to render the dialog - validator (any): Function with following signature:: - - def validator(utterance): - return utterance != "red" - - on_fail (any): - Dialog or function returning literal string to speak on - invalid input. For example:: - - def on_fail(utterance): - return "nobody likes the color red, pick another" - - num_retries (int): Times to ask user for input, -1 for infinite - NOTE: User can not respond and timeout or say "cancel" to stop - - Returns: - str: User's reply or None if timed out or canceled + def get_response(self, dialog: str = '', data: Optional[dict] = None, + validator: Optional[Callable[[str], bool]] = None, + on_fail: Optional[Union[str, Callable[[str], str]]] = None, + num_retries: int = -1) -> Optional[str]: + """ + Get a response from the user. If a dialog is supplied it is spoken, + followed immediately by listening for a user response. If the dialog is + omitted, listening is started directly. The response may optionally be + validated before returning. + @param dialog: Optional dialog resource or string to speak + @param data: Optional data to render dialog with + @param validator: Optional method to validate user input with. Accepts + the user's utterance as an arg and returns True if it is valid. + @param on_fail: Optional string or method that accepts a failing + utterance and returns a string to be spoken when validation fails. + @param num_retries: Number of times to retry getting a user response; + -1 will retry infinitely. + * If the user asks to "cancel", this method will exit + * If the user doesn't respond and this is `-1` this will only retry + once. + @return: String user response (None if no valid response is given) """ data = data or {} @@ -949,37 +1082,42 @@ def validator_default(utterance): else: msg = dig_for_message() msg = msg.reply('mycroft.mic.listen') if msg else \ - Message('mycroft.mic.listen', context={"skill_id": self.skill_id}) + Message('mycroft.mic.listen', + context={"skill_id": self.skill_id}) self.bus.emit(msg) return self._wait_response(is_cancel, validator, on_fail_fn, num_retries) - def _wait_response(self, is_cancel, validator, on_fail, num_retries): - """Loop until a valid response is received from the user or the retry + def _wait_response(self, is_cancel: callable, validator: callable, + on_fail: callable, num_retries: int) -> Optional[str]: + """ + Loop until a valid response is received from the user or the retry limit is reached. - - Arguments: - is_cancel (callable): function checking cancel criteria - validator (callbale): function checking for a valid response - on_fail (callable): function handling retries - + @param is_cancel: Function that returns `True` if user asked to cancel + @param validator: Function that returns `True` if user input is valid + @param on_fail: Function to call if validator returns `False` + @param num_retries: Number of times to retry getting a response + @returns: User response if validated, else None """ self.__response = False self._real_wait_response(is_cancel, validator, on_fail, num_retries) while self.__response is False: + # TODO: Refactor to Event time.sleep(0.1) - return self.__response + return self.__response or None - # method not present in mycroft-core def _handle_killed_wait_response(self): + """ + Handle "stop" request when getting a response. + """ self.__response = None self.converse = self.__original_converse - # method not present in mycroft-core @killable_event("mycroft.skills.abort_question", exc=AbortQuestion, callback=_handle_killed_wait_response, react_to_stop=True) def _real_wait_response(self, is_cancel, validator, on_fail, num_retries): - """Loop until a valid response is received from the user or the retry + """ + Loop until a valid response is received from the user or the retry limit is reached. Arguments: @@ -1032,18 +1170,15 @@ def _real_wait_response(self, is_cancel, validator, on_fail, num_retries): else: self.bus.emit(msg) - def ask_yesno(self, prompt, data=None): - """Read prompt and wait for a yes/no answer - - This automatically deals with translation and common variants, - such as 'yeah', 'sure', etc. - - Args: - prompt (str): a dialog id or string to read - data (dict): response data - Returns: - string: 'yes', 'no' or whatever the user response if not - one of those, including None + def ask_yesno(self, prompt: str, + data: Optional[dict] = None) -> Optional[str]: + """ + Read prompt and wait for a yes/no answer. This automatically deals with + translation and common variants, such as 'yeah', 'sure', etc. + @param prompt: a dialog id or string to read + @param data: optional data to render dialog with + @return: 'yes', 'no' or the user response if not matched to 'yes' or + 'no', including a response of None. """ resp = self.get_response(dialog=prompt, data=data) answer = yes_or_no(resp, lang=self.lang) if resp else resp @@ -1054,9 +1189,11 @@ def ask_yesno(self, prompt, data=None): else: return resp - def ask_selection(self, options, dialog='', - data=None, min_conf=0.65, numeric=False): - """Read options, ask dialog question and wait for an answer. + def ask_selection(self, options: List[str], dialog: str = '', + data: Optional[dict] = None, min_conf: float = 0.65, + numeric: bool = False): + """ + Read options, ask dialog question and wait for an answer. This automatically deals with fuzzy matching and selection by number e.g. @@ -1108,8 +1245,15 @@ def ask_selection(self, options, dialog='', return resp # method not present in mycroft-core - def _voc_list(self, voc_filename, lang=None) -> List[str]: - + def _voc_list(self, voc_filename: str, + lang: Optional[str] = None) -> List[str]: + """ + Get list of vocab options for the requested resource and cache the + results for future references. + @param voc_filename: Name of vocab resource to get options for + @param lang: language to get vocab for (default self.lang) + @return: list of string vocab options + """ lang = lang or self.lang cache_key = lang + voc_filename @@ -1121,8 +1265,10 @@ def _voc_list(self, voc_filename, lang=None) -> List[str]: return self._voc_cache.get(cache_key) or [] - def voc_match(self, utt, voc_filename, lang=None, exact=False): - """Determine if the given utterance contains the vocabulary provided. + def voc_match(self, utt: str, voc_filename: str, lang: Optional[str] = None, + exact: bool = False): + """ + Determine if the given utterance contains the vocabulary provided. By default the method checks if the utterance contains the given vocab thereby allowing the user to say things like "yes, please" and still @@ -1158,8 +1304,9 @@ def voc_match(self, utt, voc_filename, lang=None, exact=False): return match - def report_metric(self, name, data): - """Report a skill metric to the Mycroft servers. + def report_metric(self, name: str, data: dict): + """ + Report a skill metric to the Mycroft servers. Args: name (str): Name of metric. Must use only letters and hyphens @@ -1171,8 +1318,9 @@ def report_metric(self, name, data): except Exception as e: self.log.error(f'Metric couldn\'t be uploaded, due to a network error ({e})') - def send_email(self, title, body): - """Send an email to the registered user's email. + def send_email(self, title: str, body: str): + """ + Send an email to the registered user's email. Args: title (str): Title of email @@ -1181,10 +1329,11 @@ def send_email(self, title, body): """ EmailApi().send_email(title, body, self.skill_id) - def _handle_collect_resting(self, message=None): - """Handler for collect resting screen messages. + def _handle_collect_resting(self, message: Optional[Message] = None): + """ + Handler for collect resting screen messages. - Sends info on how to trigger this skills resting page. + Sends info on how to trigger this skill's resting page. """ self.log.info('Registering resting screen') msg = message or Message("") @@ -1196,7 +1345,8 @@ def _handle_collect_resting(self, message=None): self.bus.emit(message) def register_resting_screen(self): - """Registers resting screen from the resting_screen_handler decorator. + """ + Registers resting screen from the resting_screen_handler decorator. This only allows one screen and if two is registered only one will be used. @@ -1219,12 +1369,13 @@ def register_resting_screen(self): break def _register_decorated(self): - """Register all intent handlers that are decorated with an intent. + """ + Register all intent handlers that are decorated with an intent. Looks for all functions that have been marked by a decorator and read the intent data from them. The intent handlers aren't the only decorators used. Skip properties as calling getattr on them - executes the code which may have unintended side-effects + executes the code which may have unintended side effects """ for attr_name in get_non_properties(self): method = getattr(self, attr_name) @@ -1236,8 +1387,10 @@ def _register_decorated(self): for intent_file in getattr(method, 'intent_files'): self.register_intent_file(intent_file, method) - def find_resource(self, res_name, res_dirname=None, lang=None): - """Find a resource file. + def find_resource(self, res_name: str, res_dirname: Optional[str] = None, + lang: Optional[str] = None): + """ + Find a resource file. Searches for the given filename using this scheme: 1. Search the resource lang directory: @@ -1268,8 +1421,11 @@ def find_resource(self, res_name, res_dirname=None, lang=None): f"'{lang}' not found in skill") # method not present in mycroft-core - def _on_event_start(self, message, handler_info, skill_data): - """Indicate that the skill handler is starting.""" + def _on_event_start(self, message: Message, handler_info: str, + skill_data: dict): + """ + Indicate that the skill handler is starting. + """ if handler_info: # Indicate that the skill handler is starting if requested msg_type = handler_info + '.start' @@ -1277,8 +1433,11 @@ def _on_event_start(self, message, handler_info, skill_data): self.bus.emit(message.forward(msg_type, skill_data)) # method not present in mycroft-core - def _on_event_end(self, message, handler_info, skill_data): - """Store settings and indicate that the skill handler has completed + def _on_event_end(self, message: Message, handler_info: str, + skill_data: dict): + """ + Store settings (if changed) and indicate that the skill handler has + completed. """ if self.settings != self._initial_settings: self.settings.store() @@ -1289,7 +1448,8 @@ def _on_event_end(self, message, handler_info, skill_data): self.bus.emit(message.forward(msg_type, skill_data)) # method not present in mycroft-core - def _on_event_error(self, error, message, handler_info, skill_data, speak_errors): + def _on_event_error(self, error: str, message: Message, handler_info: str, + skill_data: dict, speak_errors: bool): """Speak and log the error.""" # Convert "MyFancySkill" to "My Fancy Skill" for speaking handler_name = camel_case_split(self.name) @@ -1307,8 +1467,11 @@ def _on_event_error(self, error, message, handler_info, skill_data, speak_errors message.context["skill_id"] = self.skill_id self.bus.emit(message.forward(msg_type, skill_data)) - def add_event(self, name, handler, handler_info=None, once=False, speak_errors=True): - """Create event handler for executing intent or other event. + def add_event(self, name: str, handler: callable, + handler_info: Optional[str] = None, once: bool = False, + speak_errors: bool = True): + """ + Create event handler for executing intent or other event. Args: name (string): IntentParser name @@ -1328,7 +1491,8 @@ def on_error(error, message): self.log.info("Skill execution aborted") self._on_event_end(message, handler_info, skill_data) return - self._on_event_error(error, message, handler_info, skill_data, speak_errors) + self._on_event_error(error, message, handler_info, skill_data, + speak_errors) def on_start(message): self._on_event_start(message, handler_info, skill_data) @@ -1340,8 +1504,9 @@ def on_end(message): on_error) return self.events.add(name, wrapper, once) - def remove_event(self, name): - """Removes an event from bus emitter and events list. + def remove_event(self, name: str) -> bool: + """ + Removes an event from bus emitter and events list. Args: name (string): Name of Intent or Scheduler Event @@ -1350,8 +1515,11 @@ def remove_event(self, name): """ return self.events.remove(name) - def _register_adapt_intent(self, intent_parser, handler): - """Register an adapt intent. + def _register_adapt_intent(self, + intent_parser: Union[IntentBuilder, Intent, str], + handler: callable): + """ + Register an adapt intent. Args: intent_parser: Intent object to parse utterance for the handler. @@ -1377,8 +1545,10 @@ def _register_adapt_intent(self, intent_parser, handler): self.add_event(intent_parser.name, handler, 'mycroft.skill.handler') - def register_intent(self, intent_parser, handler): - """Register an Intent with the intent service. + def register_intent(self, intent_parser: Union[IntentBuilder, Intent, str], + handler: callable): + """ + Register an Intent with the intent service. Args: intent_parser: Intent, IntentBuilder object or padatious intent @@ -1395,7 +1565,7 @@ def register_intent(self, intent_parser, handler): return self._register_adapt_intent(intent_parser, handler) - def register_intent_file(self, intent_file, handler): + def register_intent_file(self, intent_file: str, handler: callable): """Register an Intent file with the intent service. For example: @@ -1422,7 +1592,8 @@ def register_intent_file(self, intent_file, handler): """ for lang in self._native_langs: name = f'{self.skill_id}:{intent_file}' - resource_file = ResourceFile(self._resources.types.intent, intent_file) + resource_file = ResourceFile(self._resources.types.intent, + intent_file) if resource_file.file_path is None: self.log.error(f'Unable to find "{intent_file}"') continue @@ -1431,8 +1602,9 @@ def register_intent_file(self, intent_file, handler): if handler: self.add_event(name, handler, 'mycroft.skill.handler') - def register_entity_file(self, entity_file): - """Register an Entity file with the intent service. + def register_entity_file(self, entity_file: str): + """ + Register an Entity file with the intent service. An Entity file lists the exact values that an entity can hold. For example: @@ -1457,24 +1629,29 @@ def register_entity_file(self, entity_file): name = f"{self.skill_id}:{basename(entity_file)}_{md5(entity_file.encode('utf-8')).hexdigest()}" self.intent_service.register_padatious_entity(name, filename, lang) - def handle_enable_intent(self, message): - """Listener to enable a registered intent if it belongs to this skill. + def handle_enable_intent(self, message: Message): + """ + Listener to enable a registered intent if it belongs to this skill. + @param message: `mycroft.skill.enable_intent` Message """ intent_name = message.data['intent_name'] for (name, _) in self.intent_service.detached_intents: if name == intent_name: return self.enable_intent(intent_name) - def handle_disable_intent(self, message): - """Listener to disable a registered intent if it belongs to this skill. + def handle_disable_intent(self, message: Message): + """ + Listener to disable a registered intent if it belongs to this skill. + @param message: `mycroft.skill.disable_intent` Message """ intent_name = message.data['intent_name'] for (name, _) in self.intent_service.registered_intents: if name == intent_name: return self.disable_intent(intent_name) - def disable_intent(self, intent_name): - """Disable a registered intent if it belongs to this skill. + def disable_intent(self, intent_name: str) -> bool: + """ + Disable a registered intent if it belongs to this skill. Args: intent_name (string): name of the intent to be disabled @@ -1496,8 +1673,9 @@ def disable_intent(self, intent_name): self.log.error(f'Could not disable {intent_name}, it hasn\'t been registered.') return False - def enable_intent(self, intent_name): - """(Re)Enable a registered intent if it belongs to this skill. + def enable_intent(self, intent_name: str) -> bool: + """ + (Re)Enable a registered intent if it belongs to this skill. Args: intent_name: name of the intent to be enabled @@ -1518,8 +1696,9 @@ def enable_intent(self, intent_name): self.log.error(f'Could not enable {intent_name}, it hasn\'t been registered.') return False - def set_context(self, context, word='', origin=''): - """Add context to intent service + def set_context(self, context: str, word: str = '', origin: str = ''): + """ + Add context to intent service Args: context: Keyword @@ -1534,21 +1713,37 @@ def set_context(self, context, word='', origin=''): context = self._alphanumeric_skill_id + context self.intent_service.set_adapt_context(context, word, origin) - def handle_set_cross_context(self, message): - """Add global context to intent service.""" + def remove_context(self, context: str): + """ + Remove a keyword from the context manager. + """ + if not isinstance(context, str): + raise ValueError('context should be a string') + context = self._alphanumeric_skill_id + context + self.intent_service.remove_adapt_context(context) + + def handle_set_cross_context(self, message: Message): + """ + Add global context to the intent service. + @param message: `mycroft.skill.set_cross_context` Message + """ context = message.data.get('context') word = message.data.get('word') origin = message.data.get('origin') self.set_context(context, word, origin) - def handle_remove_cross_context(self, message): - """Remove global context from intent service.""" + def handle_remove_cross_context(self, message: Message): + """ + Remove global context from the intent service. + @param message: `mycroft.skill.remove_cross_context` Message + """ context = message.data.get('context') self.remove_context(context) - def set_cross_skill_context(self, context, word=''): - """Tell all skills to add a context to intent service + def set_cross_skill_context(self, context: str, word: str = ''): + """ + Tell all skills to add a context to the intent service Args: context: Keyword @@ -1561,8 +1756,10 @@ def set_cross_skill_context(self, context, word=''): {'context': context, 'word': word, 'origin': self.skill_id})) - def remove_cross_skill_context(self, context): - """Tell all skills to remove a keyword from the context manager.""" + def remove_cross_skill_context(self, context: str): + """ + Tell all skills to remove a keyword from the context manager. + """ if not isinstance(context, str): raise ValueError('context should be a string') msg = dig_for_message() or Message("") @@ -1571,35 +1768,32 @@ def remove_cross_skill_context(self, context): self.bus.emit(msg.forward('mycroft.skill.remove_cross_context', {'context': context})) - def remove_context(self, context): - """Remove a keyword from the context manager.""" - if not isinstance(context, str): - raise ValueError('context should be a string') - context = self._alphanumeric_skill_id + context - self.intent_service.remove_adapt_context(context) - - def register_vocabulary(self, entity, entity_type, lang=None): - """ Register a word to a keyword - - Args: - entity: word to register - entity_type: Intent handler entity to tie the word to + def register_vocabulary(self, entity: str, entity_type: str, + lang: Optional[str] = None): + """ + Register a word to a keyword + @param entity: word to register + @param entity_type: Intent handler entity name to associate entity to + @param lang: language of `entity` (default self.lang) """ keyword_type = self._alphanumeric_skill_id + entity_type lang = lang or self.lang - self.intent_service.register_adapt_keyword(keyword_type, entity, lang=lang) + self.intent_service.register_adapt_keyword(keyword_type, entity, + lang=lang) - def register_regex(self, regex_str, lang=None): - """Register a new regex. - Args: - regex_str: Regex string + def register_regex(self, regex_str: str, lang: Optional[str] = None): + """ + Register a new regex. + @param regex_str: Regex string to add + @param lang: language of regex_str (default self.lang) """ self.log.debug('registering regex string: ' + regex_str) regex = munge_regex(regex_str, self.skill_id) re.compile(regex) # validate regex self.intent_service.register_adapt_regex(regex, lang=lang or self.lang) - def speak(self, utterance, expect_response=False, wait=False, meta=None): + def speak(self, utterance: str, expect_response: bool = False, + wait: bool = False, meta: Optional[dict] = None): """Speak a sentence. Args: @@ -1645,10 +1839,13 @@ def handle_output_end(msg): self.bus.on("recognizer_loop:audio_output_end", handle_output_end) event.wait(timeout=15) - self.bus.remove("recognizer_loop:audio_output_end", handle_output_end) + self.bus.remove("recognizer_loop:audio_output_end", + handle_output_end) - def speak_dialog(self, key, data=None, expect_response=False, wait=False): - """ Speak a random sentence from a dialog file. + def speak_dialog(self, key: str, data: Optional[dict] = None, + expect_response: bool = False, wait: bool = False): + """ + Speak a random sentence from a dialog file. Args: key (str): dialog file key (e.g. "hello" to speak from the file @@ -1672,8 +1869,10 @@ def speak_dialog(self, key, data=None, expect_response=False, wait=False): ) self.speak(key, expect_response, wait, {}) - def acknowledge(self): - """Acknowledge a successful request. + @staticmethod + def acknowledge(): + """ + Acknowledge a successful request. This method plays a sound to acknowledge a request that does not require a verbal response. This is intended to provide simple feedback @@ -1682,17 +1881,23 @@ def acknowledge(self): return play_acknowledge_sound() # method named init_dialog in mycroft-core - def load_dialog_files(self, root_directory=None): + def load_dialog_files(self, root_directory: Optional[str] = None): + """ + Load dialog files for all configured languages + @param root_directory: Directory to locate resources in + (default self.res_dir) + """ root_directory = root_directory or self.res_dir - # If "/dialog/" exists, load from there. Otherwise + # If "/dialog/" exists, load from there. Otherwise, # load dialog from "/locale/" for lang in self._native_langs: resources = self._load_lang(root_directory, lang) if resources.types.dialog.base_directory is None: self.log.debug(f'No dialog loaded for {lang}') - def load_data_files(self, root_directory=None): - """Called by the skill loader to load intents, dialogs, etc. + def load_data_files(self, root_directory: Optional[str] = None): + """ + Called by the skill loader to load intents, dialogs, etc. Args: root_directory (str): root folder to use when loading files. @@ -1702,7 +1907,7 @@ def load_data_files(self, root_directory=None): self.load_vocab_files(root_directory) self.load_regex_files(root_directory) - def load_vocab_files(self, root_directory=None): + def load_vocab_files(self, root_directory: Optional[str] = None): """ Load vocab files found under skill's root directory.""" root_directory = root_directory or self.res_dir for lang in self._native_langs: @@ -1743,23 +1948,28 @@ def __handle_stop(self, message): {"by": "skill:" + self.skill_id}, {"skill_id": self.skill_id})) except Exception as e: - self.log.exception(f'Failed to stop skill: {self.skill_id}') + self.log.exception(f'Failed to stop skill: {self.skill_id}: {e}') def stop(self): - """Optional method implemented by subclass.""" + """ + Optional method implemented by subclass. Called when system or user + requests `stop` to cancel current execution. + """ pass def shutdown(self): - """Optional shutdown procedure implemented by subclass. + """ + Optional shutdown procedure implemented by subclass. This method is intended to be called during the skill process - termination. The skill implementation must shutdown all processes and + termination. The skill implementation must shut down all processes and operations in execution. """ pass def default_shutdown(self): - """Parent function called internally to shut down everything. + """ + Parent function called internally to shut down everything. Shuts down known entities and calls skill specific shutdown method. """ @@ -1784,21 +1994,25 @@ def default_shutdown(self): try: self.stop() - except Exception: - self.log.error(f'Failed to stop skill: {self.skill_id}', exc_info=True) - + except Exception as e: + self.log.error(f'Failed to stop skill: {self.skill_id}: {e}', + exc_info=True) try: self.shutdown() except Exception as e: - self.log.error(f'Skill specific shutdown function encountered an error: {e}') + self.log.error(f'Skill specific shutdown function encountered an ' + f'error: {e}') self.bus.emit( Message('detach_skill', {'skill_id': str(self.skill_id) + ':'}, {"skill_id": self.skill_id})) - def schedule_event(self, handler, when, data=None, name=None, - context=None): - """Schedule a single-shot event. + def schedule_event(self, handler: callable, + when: Union[int, float, datetime.datetime], + data: Optional[dict] = None, name: Optional[str] = None, + context: Optional[dict] = None): + """ + Schedule a single-shot event. Args: handler: method to be called @@ -1820,9 +2034,14 @@ def schedule_event(self, handler, when, data=None, name=None, return self.event_scheduler.schedule_event(handler, when, data, name, context=context) - def schedule_repeating_event(self, handler, when, frequency, - data=None, name=None, context=None): - """Schedule a repeating event. + def schedule_repeating_event(self, handler: callable, + when: Union[int, float, datetime.datetime], + frequency: Union[int, float], + data: Optional[dict] = None, + name: Optional[str] = None, + context: Optional[dict] = None): + """ + Schedule a repeating event. Args: handler: method to be called @@ -1840,34 +2059,31 @@ def schedule_repeating_event(self, handler, when, frequency, message = dig_for_message() context = context or message.context if message else {} context["skill_id"] = self.skill_id - return self.event_scheduler.schedule_repeating_event( - handler, - when, - frequency, - data, - name, - context=context - ) + self.event_scheduler.schedule_repeating_event(handler, when, frequency, + data, name, + context=context) - def update_scheduled_event(self, name, data=None): - """Change data of event. + def update_scheduled_event(self, name: str, data: Optional[dict] = None): + """ + Change data of event. Args: name (str): reference name of event (from original scheduling) data (dict): event data """ - return self.event_scheduler.update_scheduled_event(name, data) + self.event_scheduler.update_scheduled_event(name, data) - def cancel_scheduled_event(self, name): - """Cancel a pending event. The event will no longer be scheduled + def cancel_scheduled_event(self, name: str): + """ + Cancel a pending event. The event will no longer be scheduled to be executed Args: name (str): reference name of event (from original scheduling) """ - return self.event_scheduler.cancel_scheduled_event(name) + self.event_scheduler.cancel_scheduled_event(name) - def get_scheduled_event_status(self, name): + def get_scheduled_event_status(self, name: str) -> int: """Get scheduled event data and return the amount of time left Args: @@ -1882,8 +2098,10 @@ def get_scheduled_event_status(self, name): return self.event_scheduler.get_scheduled_event_status(name) def cancel_all_repeating_events(self): - """Cancel any repeating events started by the skill.""" - return self.event_scheduler.cancel_all_repeating_events() + """ + Cancel any repeating events started by the skill. + """ + self.event_scheduler.cancel_all_repeating_events() class SkillGUI(GUIInterface): diff --git a/ovos_workshop/skills/decorators/__init__.py b/ovos_workshop/skills/decorators/__init__.py index dc89c06d..18588273 100644 --- a/ovos_workshop/skills/decorators/__init__.py +++ b/ovos_workshop/skills/decorators/__init__.py @@ -1,2 +1,4 @@ from ovos_workshop.decorators import * # backwards compat import +from ovos_utils.log import log_deprecation +log_deprecation("Import from `ovos_workshop.decorators", "0.1.0") diff --git a/ovos_workshop/skills/decorators/converse.py b/ovos_workshop/skills/decorators/converse.py index 1c994fe9..b75b6224 100644 --- a/ovos_workshop/skills/decorators/converse.py +++ b/ovos_workshop/skills/decorators/converse.py @@ -1,2 +1,4 @@ from ovos_workshop.decorators.converse import * # backwards compat import +from ovos_utils.log import log_deprecation +log_deprecation("Import from `ovos_workshop.decorators", "0.1.0") diff --git a/ovos_workshop/skills/decorators/fallback_handler.py b/ovos_workshop/skills/decorators/fallback_handler.py index acfcefdc..003e994f 100644 --- a/ovos_workshop/skills/decorators/fallback_handler.py +++ b/ovos_workshop/skills/decorators/fallback_handler.py @@ -1,2 +1,4 @@ from ovos_workshop.decorators.fallback_handler import * # backwards compat import +from ovos_utils.log import log_deprecation +log_deprecation("Import from `ovos_workshop.decorators", "0.1.0") diff --git a/ovos_workshop/skills/decorators/killable.py b/ovos_workshop/skills/decorators/killable.py index e25241f8..d7de18d7 100644 --- a/ovos_workshop/skills/decorators/killable.py +++ b/ovos_workshop/skills/decorators/killable.py @@ -1,2 +1,4 @@ from ovos_workshop.decorators.killable import * # backwards compat import +from ovos_utils.log import log_deprecation +log_deprecation("Import from `ovos_workshop.decorators", "0.1.0") diff --git a/ovos_workshop/skills/decorators/layers.py b/ovos_workshop/skills/decorators/layers.py index 690a9368..2497168f 100644 --- a/ovos_workshop/skills/decorators/layers.py +++ b/ovos_workshop/skills/decorators/layers.py @@ -1,2 +1,4 @@ from ovos_workshop.decorators.layers import * # backwards compat import +from ovos_utils.log import log_deprecation +log_deprecation("Import from `ovos_workshop.decorators", "0.1.0") diff --git a/ovos_workshop/skills/decorators/ocp.py b/ovos_workshop/skills/decorators/ocp.py index f20850fd..32a563c5 100644 --- a/ovos_workshop/skills/decorators/ocp.py +++ b/ovos_workshop/skills/decorators/ocp.py @@ -1,2 +1,4 @@ from ovos_workshop.decorators.ocp import * # backwards compat import +from ovos_utils.log import log_deprecation +log_deprecation("Import from `ovos_workshop.decorators", "0.1.0") diff --git a/ovos_workshop/skills/fallback.py b/ovos_workshop/skills/fallback.py index 5eab2be2..87af5619 100644 --- a/ovos_workshop/skills/fallback.py +++ b/ovos_workshop/skills/fallback.py @@ -11,12 +11,11 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -# -"""The fallback skill implements a special type of skill handling -utterances not handled by the intent system. -""" + import operator +from typing import Optional, List, Callable, Tuple +from ovos_bus_client import MessageBusClient from ovos_utils.log import LOG from ovos_utils.messagebus import get_handler_name, Message from ovos_utils.metrics import Stopwatch @@ -40,62 +39,65 @@ class _MetaFB(OVOSSkill): class FallbackSkill(_MetaFB, metaclass=_MutableFallback): + """ + Fallbacks come into play when no skill matches an Adapt or closely with + a Padatious intent. All Fallback skills work together to give them a + view of the user's utterance. Fallback handlers are called in an order + determined the priority provided when the handler is registered. + + ======== =========================================================== + Priority Purpose + ======== =========================================================== + 0-4 High-priority fallbacks before medium-confidence Padatious + 5-89 Medium-priority fallbacks between medium and low Padatious + 90-100 Low-priority fallbacks after all other intent matches + + Handlers with the numerically lowest priority are invoked first. + Multiple fallbacks can exist at the same priority, but no order is + guaranteed. + + A Fallback can either observe or consume an utterance. A consumed + utterance will not be seen by any other Fallback handlers. + """ def __new__(cls, *args, **kwargs): if cls is FallbackSkill: - # direct instantiation of class, dynamic wizardry or unittests going on... + # direct instantiation of class, dynamic wizardry or unittests # return V2 as expected, V1 will eventually be dropped return FallbackSkillV2(*args, **kwargs) is_old = is_classic_core() if not is_old: try: - from mycroft.version import OVOS_VERSION_MAJOR, OVOS_VERSION_MINOR, OVOS_VERSION_BUILD, OVOS_VERSION_ALPHA - if OVOS_VERSION_MAJOR == 0 and OVOS_VERSION_MINOR == 0 and OVOS_VERSION_BUILD < 8: + from mycroft.version import OVOS_VERSION_MAJOR, \ + OVOS_VERSION_MINOR, OVOS_VERSION_BUILD, OVOS_VERSION_ALPHA + if OVOS_VERSION_MAJOR == 0 and OVOS_VERSION_MINOR == 0 and \ + OVOS_VERSION_BUILD < 8: is_old = True - elif OVOS_VERSION_MAJOR == 0 and OVOS_VERSION_MINOR == 0 and OVOS_VERSION_BUILD == 8 \ - and 0 < OVOS_VERSION_ALPHA < 5: + elif OVOS_VERSION_MAJOR == 0 and OVOS_VERSION_MINOR == 0 and \ + OVOS_VERSION_BUILD == 8 and 0 < OVOS_VERSION_ALPHA < 5: is_old = True except ImportError: pass if is_old: + LOG.debug("Using V1 Fallback") cls.__bases__ = (FallbackSkillV1, FallbackSkill, _MetaFB) else: + LOG.debug("Using V2 Fallback") cls.__bases__ = (FallbackSkillV2, FallbackSkill, _MetaFB) return super().__new__(cls, *args, **kwargs) @classmethod - def make_intent_failure_handler(cls, bus): - """backwards compat, old version of ovos-core call this method to bind the bus to old class""" + def make_intent_failure_handler(cls, bus: MessageBusClient): + """ + backwards compat, old version of ovos-core call this method to bind + the bus to old class + """ return FallbackSkillV1.make_intent_failure_handler(bus) - class FallbackSkillV1(_MetaFB, metaclass=_MutableFallback): - """Fallbacks come into play when no skill matches an Adapt or closely with - a Padatious intent. All Fallback skills work together to give them a - view of the user's utterance. Fallback handlers are called in an order - determined the priority provided when the the handler is registered. - - ======== ======== ================================================ - Priority Who? Purpose - ======== ======== ================================================ - 1-4 RESERVED Unused for now, slot for pre-Padatious if needed - 5 MYCROFT Padatious near match (conf > 0.8) - 6-88 USER General - 89 MYCROFT Padatious loose match (conf > 0.5) - 90-99 USER Uncaught intents - 100+ MYCROFT Fallback Unknown or other future use - ======== ======== ================================================ - - Handlers with the numerically lowest priority are invoked first. - Multiple fallbacks can exist at the same priority, but no order is - guaranteed. - - A Fallback can either observe or consume an utterance. A consumed - utterance will not be see by any other Fallback handlers. - """ fallback_handlers = {} - wrapper_map = [] # Map containing (handler, wrapper) tuples + wrapper_map: List[Tuple[callable, callable]] = [] # [(handler, wrapper)] def __init__(self, name=None, bus=None, use_settings=True): super().__init__(name, bus, use_settings) @@ -106,8 +108,10 @@ def __init__(self, name=None, bus=None, use_settings=True): self.fallback_config = self.config_core["skills"].get("fallbacks", {}) @classmethod - def make_intent_failure_handler(cls, bus): - """Goes through all fallback handlers until one returns True""" + def make_intent_failure_handler(cls, bus: MessageBusClient): + """ + Goes through all fallback handlers until one returns True + """ def handler(message): # No hard limit to 100, while not officially supported @@ -159,15 +163,16 @@ def handler(message): return handler @staticmethod - def _report_timing(ident, system, timing, additional_data=None): - """Create standardized message for reporting timing. - - Args: - ident (str): identifier of user interaction - system (str): system the that's generated the report - timing (stopwatch): Stopwatch object with recorded timing - additional_data (dict): dictionary with related data + def _report_timing(ident: str, system: str, timing: Stopwatch, + additional_data: Optional[dict] = None): + """ + Create standardized message for reporting timing. + @param ident: identifier for user interaction + @param system: identifier for system being timed + @param timing: Stopwatch object with recorded timing + @param additional_data: Optional dict data to include with metric """ + # TODO: Move to an imported function and deprecate this try: from mycroft.metrics import report_timing report_timing(ident, system, timing, additional_data) @@ -175,19 +180,13 @@ def _report_timing(ident, system, timing, additional_data=None): pass @classmethod - def _register_fallback(cls, handler, wrapper, priority): - """Register a function to be called as a general info fallback - Fallback should receive message and return - a boolean (True if succeeded or False if failed) - - Lower priority gets run first - 0 for high priority 100 for low priority - - Args: - handler (callable): original handler, used as a reference when - removing - wrapper (callable): wrapped version of handler - priority (int): fallback priority + def _register_fallback(cls, handler: callable, wrapper: callable, + priority: int): + """ + Add a fallback handler to the class + @param handler: original handler method used for reference + @param wrapper: wrapped handler used to handle fallback requests + @param priority: fallback priority """ while priority in cls.fallback_handlers: priority += 1 @@ -195,9 +194,14 @@ def _register_fallback(cls, handler, wrapper, priority): cls.fallback_handlers[priority] = wrapper cls.wrapper_map.append((handler, wrapper)) - def register_fallback(self, handler, priority): - """Register a fallback with the list of fallback handlers and with the - list of handlers registered by this instance + def register_fallback(self, handler: Callable[[Message], None], + priority: int): + """ + Register a fallback handler method with a given priority. This will + account for configuration overrides of fallback priority, as well as + configured fallback skill whitelist/blacklist. + @param handler: fallback handler method that accepts a `Message` arg + @param priority: fallback priority """ opmode = self.fallback_config.get("fallback_mode", FallbackMode.ACCEPT_ALL) @@ -225,14 +229,11 @@ def wrapper(*args, **kwargs): self._register_fallback(handler, wrapper, priority) @classmethod - def _remove_registered_handler(cls, wrapper_to_del): - """Remove a registered wrapper. - - Args: - wrapper_to_del (callable): wrapped handler to be removed - - Returns: - (bool) True if one or more handlers were removed, otherwise False. + def _remove_registered_handler(cls, wrapper_to_del: callable) -> bool: + """ + Remove a registered wrapper. + @param wrapper_to_del: wrapped handler to be removed + @return: True if one or more handlers were removed, otherwise False. """ found_handler = False for priority, handler in list(cls.fallback_handlers.items()): @@ -245,23 +246,22 @@ def _remove_registered_handler(cls, wrapper_to_del): return found_handler @classmethod - def remove_fallback(cls, handler_to_del): - """Remove a fallback handler. - - Args: - handler_to_del: reference to handler - Returns: - (bool) True if at least one handler was removed, otherwise False + def remove_fallback(cls, handler_to_del: callable) -> bool: + """ + Remove a fallback handler. + @param handler_to_del: registered callback handler (or wrapped handler) + @return: True if at least one handler was removed, otherwise False """ # Find wrapper from handler or wrapper wrapper_to_del = None for h, w in cls.wrapper_map: if handler_to_del in (h, w): + handler_to_del = h wrapper_to_del = w break if wrapper_to_del: - cls.wrapper_map.remove((h, w)) + cls.wrapper_map.remove((handler_to_del, wrapper_to_del)) remove_ok = cls._remove_registered_handler(wrapper_to_del) else: LOG.warning('Could not find matching fallback handler') @@ -269,24 +269,29 @@ def remove_fallback(cls, handler_to_del): return remove_ok def remove_instance_handlers(self): - """Remove all fallback handlers registered by the fallback skill.""" - self.log.info('Removing all handlers...') + """ + Remove all fallback handlers registered by the fallback skill. + """ + LOG.info('Removing all handlers...') while len(self.instance_fallback_handlers): handler = self.instance_fallback_handlers.pop() self.remove_fallback(handler) def default_shutdown(self): - """Remove all registered handlers and perform skill shutdown.""" + """ + Remove all registered handlers and perform skill shutdown. + """ self.remove_instance_handlers() super().default_shutdown() def _register_decorated(self): - """Register all intent handlers that are decorated with an intent. + """ + Register all decorated fallback handlers. Looks for all functions that have been marked by a decorator - and read the intent data from them. The intent handlers aren't the - only decorators used. Skip properties as calling getattr on them - executes the code which may have unintended side-effects + and read the fallback priority from them. The handlers aren't the + only decorators used. Skip properties as calling getattr on them + executes the code which may have unintended side effects. """ super()._register_decorated() for attr_name in get_non_properties(self): @@ -296,44 +301,15 @@ def _register_decorated(self): class FallbackSkillV2(_MetaFB, metaclass=_MutableFallback): - """ - Fallbacks come into play when no skill matches an intent. - - Fallback handlers are called in an order determined the - priority provided when the skill is registered. - - ======== ======== ================================================ - Priority Who? Purpose - ======== ======== ================================================ - 1-4 RESERVED Unused for now, slot for pre-Padatious if needed - 5 MYCROFT Padatious near match (conf > 0.8) - 6-88 USER General - 89 MYCROFT Padatious loose match (conf > 0.5) - 90-99 USER Uncaught intents - 100+ MYCROFT Fallback Unknown or other future use - ======== ======== ================================================ - - Handlers with the numerically lowest priority are invoked first. - Multiple fallbacks can exist at the same priority, but no order is - guaranteed. - - A Fallback can either observe or consume an utterance. A consumed - utterance will not be see by any other Fallback handlers. - - A skill might register several handlers, the lowest priority will be reported to core - If a skill is selected by core then all handlers are checked by - their priority until one can handle the utterance - - A skill may return False in the can_answer method to request - that core does not execute it's fallback handlers - """ - # "skill_id": priority (int) overrides fallback_config = Configuration().get("skills", {}).get("fallbacks", {}) @classmethod - def make_intent_failure_handler(cls, bus): - """backwards compat, old version of ovos-core call this method to bind the bus to old class""" + def make_intent_failure_handler(cls, bus: MessageBusClient): + """ + backwards compat, old version of ovos-core call this method to bind + the bus to old class + """ return FallbackSkillV1.make_intent_failure_handler(bus) def __init__(self, bus=None, skill_id=""): @@ -341,7 +317,13 @@ def __init__(self, bus=None, skill_id=""): super().__init__(bus=bus, skill_id=skill_id) @property - def priority(self): + def priority(self) -> int: + """ + Get this skill's minimum priority. Priority is determined as: + 1) Configured fallback skill priority + 2) Highest fallback handler priority + 3) Default `101` (no fallback handlers are registered) + """ priority_overrides = self.fallback_config.get("fallback_priorities", {}) if self.skill_id in priority_overrides: return priority_overrides.get(self.skill_id) @@ -349,30 +331,35 @@ def priority(self): return min([p[0] for p in self._fallback_handlers]) return 101 - def can_answer(self, utterances, lang): - """Check if the skill can answer the particular question. - - - Arguments: - utterances (list): list of possible transcriptions to parse - lang (str) : lang code - Returns: - (bool) True if skill can handle the query + def can_answer(self, utterances: List[str], lang: str) -> bool: + """ + Check if the skill can answer the particular question. Override this + method to validate whether a query can possibly be handled. By default, + assumes a skill can answer if it has any registered handlers + @param utterances: list of possible transcriptions to parse + @param lang: BCP-47 language code associated with utterances + @return: True if skill can handle the query """ return len(self._fallback_handlers) > 0 def _register_system_event_handlers(self): - """Add all events allowing the standard interaction with the Mycroft - system. + """ + Register messagebus event handlers and emit a message to register this + fallback skill. """ super()._register_system_event_handlers() - self.add_event('ovos.skills.fallback.ping', self._handle_fallback_ack, speak_errors=False) - self.add_event(f"ovos.skills.fallback.{self.skill_id}.request", self._handle_fallback_request, speak_errors=False) + self.add_event('ovos.skills.fallback.ping', self._handle_fallback_ack, + speak_errors=False) + self.add_event(f"ovos.skills.fallback.{self.skill_id}.request", + self._handle_fallback_request, speak_errors=False) self.bus.emit(Message("ovos.skills.fallback.register", - {"skill_id": self.skill_id, "priority": self.priority})) + {"skill_id": self.skill_id, + "priority": self.priority})) - def _handle_fallback_ack(self, message): - """Inform skills service we can handle fallbacks.""" + def _handle_fallback_ack(self, message: Message): + """ + Inform skills service we can handle fallbacks. + """ utts = message.data.get("utterances", []) lang = message.data.get("lang") self.bus.emit(message.reply( @@ -381,14 +368,22 @@ def _handle_fallback_ack(self, message): "can_handle": self.can_answer(utts, lang)}, context={"skill_id": self.skill_id})) - def _handle_fallback_request(self, message): + def _handle_fallback_request(self, message: Message): + """ + Handle a fallback request, calling any registered handlers in priority + order until one is successful. emits a response indicating whether the + request was handled. + @param message: `ovos.skills.fallback..request` message + """ # indicate fallback handling start - self.bus.emit(message.forward(f"ovos.skills.fallback.{self.skill_id}.start")) + self.bus.emit(message.forward( + f"ovos.skills.fallback.{self.skill_id}.start")) handler_name = None # each skill can register multiple handlers with different priorities - sorted_handlers = sorted(self._fallback_handlers, key=operator.itemgetter(0)) + sorted_handlers = sorted(self._fallback_handlers, + key=operator.itemgetter(0)) for prio, handler in sorted_handlers: try: if handler(message): @@ -401,16 +396,20 @@ def _handle_fallback_request(self, message): else: status = False - self.bus.emit(message.forward(f"ovos.skills.fallback.{self.skill_id}.response", - data={"result": status, - "fallback_handler": handler_name})) + self.bus.emit(message.forward( + f"ovos.skills.fallback.{self.skill_id}.response", + data={"result": status, "fallback_handler": handler_name})) - def register_fallback(self, handler, priority): - """Register a fallback with the list of fallback handlers and with the - list of handlers registered by this instance + def register_fallback(self, handler: callable, priority: int): + """ + Register a fallback handler and add a messagebus handler to call it on + any fallback request. + @param handler: Fallback handler + @param priority: priority of the registered handler """ - LOG.info(f"registering fallback handler -> ovos.skills.fallback.{self.skill_id}") + LOG.info(f"registering fallback handler -> " + f"ovos.skills.fallback.{self.skill_id}") def wrapper(*args, **kwargs): if handler(*args, **kwargs): @@ -422,18 +421,22 @@ def wrapper(*args, **kwargs): self.bus.on(f"ovos.skills.fallback.{self.skill_id}", wrapper) def default_shutdown(self): - """Remove all registered handlers and perform skill shutdown.""" - self.bus.emit(Message("ovos.skills.fallback.deregister", {"skill_id": self.skill_id})) + """ + Remove all registered handlers and perform skill shutdown. + """ + self.bus.emit(Message("ovos.skills.fallback.deregister", + {"skill_id": self.skill_id})) self.bus.remove_all_listeners(f"ovos.skills.fallback.{self.skill_id}") super().default_shutdown() def _register_decorated(self): - """Register all intent handlers that are decorated with an intent. + """ + Register all decorated fallback handlers. Looks for all functions that have been marked by a decorator - and read the intent data from them. The intent handlers aren't the - only decorators used. Skip properties as calling getattr on them - executes the code which may have unintended side-effects + and read the fallback priority from them. The handlers aren't the + only decorators used. Skip properties as calling getattr on them + executes the code which may have unintended side effects. """ super()._register_decorated() for attr_name in get_non_properties(self): diff --git a/ovos_workshop/skills/idle_display_skill.py b/ovos_workshop/skills/idle_display_skill.py index d239659c..7f680482 100644 --- a/ovos_workshop/skills/idle_display_skill.py +++ b/ovos_workshop/skills/idle_display_skill.py @@ -11,68 +11,76 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -"""Provide a common API for skills that define idle screen functionality. - -The idle display should show when no other skill is using the display. Some -skills use the display for a defined period of time before returning to the -idle display (e.g. Weather Skill). Some skills take control of the display -indefinitely (e.g. Timer Skill). - -The display could be a touch screen (such as on the Mark II), or an -Arduino LED array (such as on the Mark I), or any other type of display. This -base class is meant to be agnostic to the type of display, with the -implementation details defined within the skill that uses this as a base class. -""" -from ovos_utils.log import LOG + +from ovos_utils.log import LOG, log_deprecation from ovos_utils.messagebus import Message -from ovos_workshop.skills.mycroft_skill import MycroftSkill +from ovos_workshop.skills.base import BaseSkill -class IdleDisplaySkill(MycroftSkill): - """Base class for skills that define an idle display. +class IdleDisplaySkill(BaseSkill): + """ + Base class for skills that define an idle display. An idle display is what shows on a device's screen when it is not in use - by other skills. For example, Mycroft's Home Screen Skill. + by other skills. i.e. a Home Screen skill. + + The idle display should show when no other skill is using the display. Some + skills use the display for a defined period of time before returning to the + idle display (e.g. Weather Skill). Some skills take control of the display + indefinitely (e.g. Timer Skill). + + The display could be a touch screen (such as on the Mark II), or an + Arduino LED array (such as on the Mark I), or any other type of display. + This base class is meant to be agnostic to the type of display. """ def __init__(self, *args, **kwargs): - super(IdleDisplaySkill, self).__init__(*args, **kwargs) + BaseSkill.__init__(self, *args, **kwargs) self._homescreen_entry = None - def bind(self, bus): - """Tasks to complete during skill load but after bus initialization.""" - if bus: - super().bind(bus) - self._define_message_bus_handlers() - self._build_homescreen_entry() - def handle_idle(self): - """Override this method to display the idle screen.""" + """ + Override this method to display the idle screen. + """ raise NotImplementedError( - "Subclass must override the handle_idle method" - ) + "Subclass must override the handle_idle method") - def _define_message_bus_handlers(self): - """Defines the bus events handled in this skill and their handlers.""" + def _register_system_event_handlers(self): + """ + Defines the bus events handled in this skill and their handlers. + """ + BaseSkill._register_system_event_handlers(self) self.add_event("mycroft.ready", self._handle_mycroft_ready) - self.add_event("homescreen.manager.activate.display", self._display_homescreen_requested) - self.add_event("homescreen.manager.reload.list", self._reload_homescreen_entry) - self.add_event("mycroft.skills.shutdown", self._remove_homescreen_on_shutdown) + self.add_event("homescreen.manager.activate.display", + self._display_homescreen_requested) + self.add_event("homescreen.manager.reload.list", + self._reload_homescreen_entry) + self.add_event("mycroft.skills.shutdown", + self._remove_homescreen_on_shutdown) + self._build_homescreen_entry() def _handle_mycroft_ready(self, message): - """Shows idle screen when device is ready for use.""" + """ + Shows idle screen when device is ready for use. + """ self._show_idle_screen() self.bus.emit(message.reply("skill.idle.displayed")) LOG.debug("Homescreen ready") def _show_idle_screen(self): - """Method for compat with mycroft-core mark2/qa branch equivalent class - Skills made for mk2 will override this private method instead of the public handle_idle """ + Backwards-compat method for pre-Dinkum Mycroft Mark2 skills + """ + log_deprecation("Call `self.handle_idle()` directly", "0.1.0") self.handle_idle() - def _build_homescreen_entry(self, message=None): + def _build_homescreen_entry(self, message: Message = None): + """ + Update the internal _homescreen_entry object + for this skill and send it to the Home Screen Manager. + @param message: optional Message associated with request + """ # get the super class this inherits from super_class_name = "IdleDisplaySkill" super_class_object = self.__class__.__name__ @@ -81,27 +89,46 @@ def _build_homescreen_entry(self, message=None): "id": self.skill_id} self._add_available_homescreen(message) - def _add_available_homescreen(self, message=None): + def _add_available_homescreen(self, message: Message = None): + """ + Add this skill's homescreen_entry to the Home Screen Manager. + @param message: optional Message associated with request + """ message = message or Message("homescreen.manager.reload.list") LOG.debug(f"Registering Homescreen {self._homescreen_entry}") msg = message.forward("homescreen.manager.add", self._homescreen_entry) self.bus.emit(msg) - def _remove_homescreen(self, message): + def _remove_homescreen(self, message: Message): + """ + Remove this skill's homescreen_entry from the Home Screen Manager + @param message: `mycroft.skills.shutdown` message + """ LOG.debug(f"Requesting removal of {self._homescreen_entry}") msg = message.forward("homescreen.manager.remove", self._homescreen_entry) self.bus.emit(msg) - def _reload_homescreen_entry(self, message): + def _reload_homescreen_entry(self, message: Message): + """ + Reload this skill's homescreen_entry and send it to the + Home Screen Manager. + @param message: `homescreen.manager.reload.list` message + """ self._build_homescreen_entry(message) - def _remove_homescreen_on_shutdown(self, message): - shutdown_for_id = message.data["id"] - if shutdown_for_id == self.skill_id: + def _remove_homescreen_on_shutdown(self, message: Message): + """ + Remove this homescreen from the Home Screen Manager if requested + @param message: `mycroft.skills.shutdown` message + """ + if message.data["id"] == self.skill_id: self._remove_homescreen(message) - def _display_homescreen_requested(self, message): - request_for_id = message.data["homescreen_id"] - if request_for_id == self.skill_id: + def _display_homescreen_requested(self, message: Message): + """ + Display this home screen if requested by the Home Screen Manager + @param message: `homescreen.manager.activate.display` message + """ + if message.data["homescreen_id"] == self.skill_id: self._show_idle_screen() self.bus.emit(message.reply("skill.idle.displayed")) diff --git a/ovos_workshop/skills/intent_provider.py b/ovos_workshop/skills/intent_provider.py index 5a230669..f85a389e 100644 --- a/ovos_workshop/skills/intent_provider.py +++ b/ovos_workshop/skills/intent_provider.py @@ -1,14 +1,14 @@ from threading import Event from time import time as get_time, sleep -from ovos_utils.log import LOG +from ovos_utils.log import LOG, log_deprecation from ovos_utils.messagebus import Message -from ovos_workshop.skills.ovos import OVOSFallbackSkill +from ovos_workshop.skills.fallback import FallbackSkill from ovos_config.config import read_mycroft_config, update_mycroft_config class BaseIntentEngine: - # TODO move to OPM def __init__(self, name, config=None): + log_deprecation("This base class is not supported", "0.1.0") self.name = name.lower() config = config or read_mycroft_config() self.config = config.get(self.name, {}) @@ -49,8 +49,9 @@ def calc_intent(self, query): return data -class IntentEngineSkill(OVOSFallbackSkill): +class IntentEngineSkill(FallbackSkill): def __init__(self, *args, **kwargs): + log_deprecation("This base class is not supported", "0.1.0") super().__init__(*args, **kwargs) self.engine = None self.config = {} diff --git a/ovos_workshop/skills/layers.py b/ovos_workshop/skills/layers.py index 32a9f766..dcedc619 100644 --- a/ovos_workshop/skills/layers.py +++ b/ovos_workshop/skills/layers.py @@ -1 +1,3 @@ from ovos_workshop.decorators.layers import IntentLayers +from ovos_utils.log import log_deprecation +log_deprecation("Import from `ovos_workshop.decorators.layers`", "0.1.0") diff --git a/ovos_workshop/skills/mycroft_skill.py b/ovos_workshop/skills/mycroft_skill.py index 92d5dd23..4ea70744 100644 --- a/ovos_workshop/skills/mycroft_skill.py +++ b/ovos_workshop/skills/mycroft_skill.py @@ -11,26 +11,28 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -# -"""Common functionality relating to the implementation of mycroft skills.""" -import inspect import shutil -from abc import ABCMeta -from os.path import join, exists, dirname -from ovos_utils.log import LOG +from abc import ABCMeta +from os.path import join, exists +from typing import Optional +from ovos_bus_client import MessageBusClient, Message +from ovos_utils.log import LOG, log_deprecation from ovos_workshop.skills.base import BaseSkill, is_classic_core class _SkillMetaclass(ABCMeta): """ - this metaclass ensures we can load skills like regular python objects - mycroft-core required a skill loader helper class, which created the skill and then finished object init - this means skill_id and bus are not available in init method, mycroft introduced a method named initialize meant for this + This metaclass ensures we can load skills like regular python objects. + mycroft-core required a skill loader helper class, which created the skill + and then finished object init. This meant skill_id and bus were not + available in init method, so mycroft introduced a method named `initialize` + that was called after `skill_id` and `bus` were defined. - to make skills pythonic and standalone, this metaclass is used to auto init old skills and help in migrating to new standards + To make skills pythonic and standalone, this metaclass is used to auto init + old skills and help in migrating to new standards. To override isinstance checks we also need to use a metaclass @@ -47,11 +49,13 @@ def __call__(cls, *args, **kwargs): for a in args: if isinstance(a, MessageBusClient) or isinstance(a, FakeBus): bus = a - LOG.warning(f"bus should be a kwarg, guessing {a} is the bus") + LOG.warning( + f"bus should be a kwarg, guessing {a} is the bus") break else: - LOG.warning("skill initialized without bus!! this is legacy behaviour and" - " requires you to call skill.bind(bus) or skill._startup(skill_id, bus)\n" + LOG.warning("skill initialized without bus!! this is legacy " + "behaviour and requires you to call skill.bind(bus)" + " or skill._startup(skill_id, bus)\n" "bus will be required starting on ovos-core 0.1.0") return super().__call__(*args, **kwargs) @@ -60,37 +64,45 @@ def __call__(cls, *args, **kwargs): if "bus" in kwargs: bus = kwargs.pop("bus") if not skill_id: - LOG.warning(f"skill_id should be a kwarg, please update {cls.__name__}") + LOG.warning(f"skill_id should be a kwarg, please update " + f"{cls.__name__}") if args and isinstance(args[0], str): a = args[0] - if a[0].isupper(): # in mycroft name is CamelCase by convention, not skill_id - LOG.debug(f"ambiguous skill_id, ignoring {a} as it appears to be a CamelCase name") + if a[0].isupper(): + # in mycroft name is CamelCase by convention, not skill_id + LOG.debug(f"ambiguous skill_id, ignoring {a} as it appears " + f"to be a CamelCase name") else: - LOG.warning(f"ambiguous skill_id, assuming positional argument: {a}") + LOG.warning(f"ambiguous skill_id, assuming positional " + f"argument: {a}") skill_id = a if not skill_id: - LOG.warning("skill initialized without skill_id!! this is legacy behaviour and" - " requires you to call skill._startup(skill_id, bus)\n" - "skill_id will be required starting on ovos-core 0.1.0") + LOG.warning("skill initialized without bus!! this is legacy " + "behaviour and requires you to call skill.bind(bus)" + " or skill._startup(skill_id, bus)\n" + "bus will be required starting on ovos-core 0.1.0") return super().__call__(*args, **kwargs) # by convention skill_id is the folder name # usually repo.author # TODO - uncomment once above is deprecated - #skill_id = dirname(inspect.getfile(cls)).split("/")[-1] - #LOG.warning(f"missing skill_id, assuming folder name convention: {skill_id}") + # skill_id = dirname(inspect.getfile(cls)).split("/")[-1] + # LOG.warning(f"missing skill_id, assuming folder name " + # f"convention: {skill_id}") try: - # skill follows latest best practices, accepts kwargs and does its own init + # skill follows latest best practices, + # accepts kwargs and does its own init return super().__call__(skill_id=skill_id, bus=bus, **kwargs) except TypeError: - LOG.warning("legacy skill signature detected, attempting to init skill manually, " - f"self.bus and self.skill_id will only be available in self.initialize.\n" + - f"__init__ method needs to accept `skill_id` and `bus` to resolve this.") + LOG.warning("legacy skill signature detected, attempting to init " + "skill manually, self.bus and self.skill_id will only " + "be available in self.initialize.\n__init__ method " + "needs to accept `skill_id` and `bus` to resolve this.") - # skill did not update its init method, let's do some magic to init it manually - # NOTE: no try: except because all skills must accept this initialization and we want exception + # skill did not update its init method, init it manually + # NOTE: no try/except because all skills must accept this initialization # this is what skill loader does internally skill = super().__call__(*args, **kwargs) skill._startup(bus, skill_id) @@ -107,22 +119,20 @@ def __instancecheck__(self, instance): class MycroftSkill(BaseSkill, metaclass=_SkillMetaclass): - """Base class for mycroft skills providing common behaviour and parameters - to all Skill implementations. - - For information on how to get started with creating mycroft skills see - https://mycroft.ai/documentation/skills/introduction-developing-skills/ - - New methods added here are always private, public apis for Skill class are added in OVOSSkill - This is done to ensure no syntax errors when a MycroftSkill object comes from mycroft-core - - Args: - name (str): skill name - bus (MycroftWebsocketClient): Optional bus connection - use_settings (bool): Set to false to not use skill settings at all (DEPRECATED) + """ + Base class for mycroft skills providing common behaviour and parameters + to all Skill implementations. This class is kept for backwards-compat. It is + recommended to implement `OVOSSkill` to properly implement new methods. """ - def __init__(self, name=None, bus=None, use_settings=True, *args, **kwargs): + def __init__(self, name: str = None, bus: MessageBusClient = None, + use_settings: bool = True, *args, **kwargs): + """ + Create a MycroftSkill object. + @param name: DEPRECATED skill_name + @param bus: MessageBusClient to bind to skill + @param use_settings: DEPRECATED option to disable settings sync + """ super().__init__(name=name, bus=bus, *args, **kwargs) self._initial_settings = {} @@ -131,20 +141,24 @@ def __init__(self, name=None, bus=None, use_settings=True, *args, **kwargs): # old kludge from fallback skills, unused according to grep if use_settings is False: - LOG.warning("use_settings has been deprecated! skill settings are always enabled") + log_deprecation("use_settings has been deprecated! " + "skill settings are always enabled", "0.1.0") if is_classic_core(): self.settings_write_path = self.root_dir def _init_settings_manager(self): super()._init_settings_manager() - # backwards compat - self.settings_meta has been deprecated in favor of settings manager + # backwards compat - self.settings_meta has been deprecated + # in favor of settings manager if is_classic_core(): from mycroft.skills.settings import SettingsMetaUploader else: try: # ovos-core compat layer - from mycroft.deprecated.skills.settings import SettingsMetaUploader - self._settings_meta = SettingsMetaUploader(self.root_dir, self.skill_id) + from mycroft.deprecated.skills.settings import \ + SettingsMetaUploader + self._settings_meta = SettingsMetaUploader(self.root_dir, + self.skill_id) except ImportError: pass # standalone skill, skip backwards compat property @@ -152,32 +166,42 @@ def _init_settings(self): """Setup skill settings.""" if is_classic_core(): # migrate settings if needed - if not exists(self._settings_path) and exists(self._old_settings_path): - LOG.warning("Found skill settings at pre-xdg location, migrating!") + if not exists(self._settings_path) and \ + exists(self._old_settings_path): + LOG.warning("Found skill settings at pre-xdg location, " + "migrating!") shutil.copy(self._old_settings_path, self._settings_path) - LOG.info(f"{self._old_settings_path} moved to {self._settings_path}") + LOG.info(f"{self._old_settings_path} moved to " + f"{self._settings_path}") super()._init_settings() # renamed in base class for naming consistency - def init_dialog(self, root_directory=None): - """ DEPRECATED: use load_dialog_files instead """ + def init_dialog(self, root_directory: Optional[str] = None): + """ + DEPRECATED: use load_dialog_files instead + """ + log_deprecation("Use `load_dialog_files`", "0.1.0") self.load_dialog_files(root_directory) # renamed in base class for naming consistency def make_active(self): - """Bump skill to active_skill list in intent_service. + """ + Bump skill to active_skill list in intent_service. This enables converse method to be called even without skill being used in last 5 minutes. - deprecated: use self.activate() instead + deprecated: use self._activate() instead """ + log_deprecation("Use `_activate`", "0.1.0") self._activate() # patched due to functional (internal) differences under mycroft-core - def _on_event_end(self, message, handler_info, skill_data): - """Store settings and indicate that the skill handler has completed + def _on_event_end(self, message: Message, handler_info: str, + skill_data: dict): + """ + Store settings and indicate that the skill handler has completed """ if not is_classic_core(): return super()._on_event_end(message, handler_info, skill_data) @@ -189,7 +213,7 @@ def _on_event_end(self, message, handler_info, skill_data): save_settings(self.settings_write_path, self.settings) self._initial_settings = dict(self.settings) except Exception as e: - LOG.exception("Failed to save skill settings") + LOG.exception(f"Failed to save skill settings: {e}") if handler_info: msg_type = handler_info + '.complete' message.context["skill_id"] = self.skill_id @@ -197,47 +221,61 @@ def _on_event_end(self, message, handler_info, skill_data): # renamed in base class for naming consistency # refactored to use new resource utils - def translate(self, text, data=None): - """Deprecated method for translating a dialog file. - use self._resources.render_dialog(text, data) instead""" + def translate(self, text: str, data: Optional[dict] = None): + """ + Deprecated method for translating a dialog file. + use self._resources.render_dialog(text, data) instead + """ + log_deprecation("Use `_resources.render_dialog`", "0.1.0") return self._resources.render_dialog(text, data) # renamed in base class for naming consistency # refactored to use new resource utils - def translate_namedvalues(self, name, delim=','): - """Deprecated method for translating a name/value file. - use elf._resources.load_named_value_filetext, data) instead""" + def translate_namedvalues(self, name: str, delim: str = ','): + """ + Deprecated method for translating a name/value file. + use self._resources.load_named_value_filetext, data) instead + """ + log_deprecation("Use `_resources.load_named_value_file`", "0.1.0") return self._resources.load_named_value_file(name, delim) # renamed in base class for naming consistency # refactored to use new resource utils - def translate_list(self, list_name, data=None): - """Deprecated method for translating a list. - use delf._resources.load_list_file(text, data) instead""" + def translate_list(self, list_name: str, data: Optional[dict] = None): + """ + Deprecated method for translating a list. + use delf._resources.load_list_file(text, data) instead + """ + log_deprecation("Use `_resources.load_list_file`", "0.1.0") return self._resources.load_list_file(list_name, data) # renamed in base class for naming consistency # refactored to use new resource utils - def translate_template(self, template_name, data=None): - """Deprecated method for translating a template file - use delf._resources.template_file(text, data) instead""" + def translate_template(self, template_name: str, + data: Optional[dict] = None): + """ + Deprecated method for translating a template file + use delf._resources.template_file(text, data) instead + """ + log_deprecation("Use `_resources.template_file`", "0.1.0") return self._resources.load_template_file(template_name, data) # refactored - backwards compat + log warnings @property def settings_meta(self): - LOG.warning("self.settings_meta has been deprecated! please use self.settings_manager instead") + log_deprecation("Use `self.settings_manager`", "0.1.0") return self._settings_meta # refactored - backwards compat + log warnings @settings_meta.setter def settings_meta(self, val): - LOG.warning("self.settings_meta has been deprecated! please use self.settings_manager instead") + log_deprecation("Use `self.settings_manager`", "0.1.0") self._settings_meta = val # internal - deprecated under ovos-core @property def _old_settings_path(self): + log_deprecation("This path is no longer used", "0.1.0") old_dir = self.config_core.get("data_dir") or "/opt/mycroft" old_folder = self.config_core.get("skills", {}).get("msm", {}) \ .get("directory") or "skills" @@ -247,8 +285,9 @@ def _old_settings_path(self): @property def _settings_path(self): if is_classic_core(): - if self.settings_write_path and self.settings_write_path != self.root_dir: - LOG.warning("self.settings_write_path has been deprecated! " - "Support will be dropped in a future release") + if self.settings_write_path and \ + self.settings_write_path != self.root_dir: + log_deprecation("`self.settings_write_path` is no longer used", + "0.1.0") return join(self.settings_write_path, 'settings.json') return super()._settings_path diff --git a/ovos_workshop/skills/ovos.py b/ovos_workshop/skills/ovos.py index 8cee649e..65707178 100644 --- a/ovos_workshop/skills/ovos.py +++ b/ovos_workshop/skills/ovos.py @@ -1,17 +1,18 @@ import re -import time -from typing import List +from threading import Event +from typing import List, Optional, Union + +from ovos_bus_client import MessageBusClient +from ovos_bus_client.message import Message, dig_for_message from ovos_utils.intents import IntentBuilder, Intent -from ovos_utils.log import LOG -from ovos_utils.messagebus import Message, dig_for_message +from ovos_utils.log import LOG, log_deprecation from ovos_utils.skills import get_non_properties from ovos_utils.skills.audioservice import OCPInterface from ovos_utils.skills.settings import PrivateSettings from ovos_utils.sound import play_audio +from ovos_workshop.resource_files import SkillResources -from ovos_workshop.decorators.killable import killable_event, \ - AbortQuestion from ovos_workshop.skills.layers import IntentLayers from ovos_workshop.skills.mycroft_skill import MycroftSkill, is_classic_core @@ -34,7 +35,7 @@ def __init__(self, *args, **kwargs): self.audio_service = None super(OVOSSkill, self).__init__(*args, **kwargs) - def bind(self, bus): + def bind(self, bus: MessageBusClient): super().bind(bus) if bus: # here to ensure self.skill_id is populated @@ -44,117 +45,153 @@ def bind(self, bus): # new public api, these are not available in MycroftSkill @property - def is_fully_initialized(self): - """Determines if the skill has been fully loaded and setup. - When True all data has been loaded and all internal state and events setup""" + def is_fully_initialized(self) -> bool: + """ + Determines if the skill has been fully loaded and setup. + When True, all data has been loaded and all internal state + and events set up. + """ return self._is_fully_initialized @property - def stop_is_implemented(self): + def stop_is_implemented(self) -> bool: + """ + True if this skill implements a `stop` method + """ return self._stop_is_implemented @property - def converse_is_implemented(self): + def converse_is_implemented(self) -> bool: + """ + True if this skill implements a `converse` method + """ return self._converse_is_implemented + @property + def core_lang(self) -> str: + """ + Get the configured default language as a BCP-47 language code. + """ + return self._core_lang + + @property + def secondary_langs(self) -> List[str]: + """ + Get the configured secondary languages; resources will be loaded for + these languages to provide support for multilingual input, in addition + to `core_lang`. A skill may override this method to specify which + languages intents are registered in. + """ + return self._secondary_langs + + @property + def native_langs(self) -> List[str]: + """ + Languages natively supported by this skill (ie, resource files available + and explicitly supported). This is equivalent to normalized + secondary_langs + core_lang. + """ + return self._native_langs + + @property + def alphanumeric_skill_id(self) -> str: + """ + Skill id converted to only alphanumeric characters and "_". + Non alphanumeric characters are converted to "_" + """ + return self._alphanumeric_skill_id + + @property + def resources(self) -> SkillResources: + """ + Get a SkillResources object for the current language. Objects are + initialized for the current language as needed. + """ + return self._resources + def activate(self): - """Bump skill to active_skill list in intent_service. + """ + Mark this skill as active and push to the top of the active skills list. This enables converse method to be called even without skill being used in last 5 minutes. """ self._activate() def deactivate(self): - """remove skill from active_skill list in intent_service. - This stops converse method from being called + """ + Mark this skill as inactive and remove from the active skills list. + This stops converse method from being called. """ self._deactivate() - def play_audio(self, filename): + def play_audio(self, filename: str): + """ + Queue and audio file for playback + @param filename: File to play + """ core_supported = False if not is_classic_core(): try: - from mycroft.version import OVOS_VERSION_BUILD, OVOS_VERSION_MINOR, OVOS_VERSION_MAJOR + from mycroft.version import OVOS_VERSION_BUILD, \ + OVOS_VERSION_MINOR, OVOS_VERSION_MAJOR if OVOS_VERSION_MAJOR >= 1 or \ OVOS_VERSION_MINOR > 0 or \ OVOS_VERSION_BUILD >= 4: core_supported = True # min version of ovos-core - except ImportError: # skills don't require core anymore, running standalone + except ImportError: + # skills don't require core anymore, running standalone core_supported = True if core_supported: message = dig_for_message() or Message("") - self.bus.emit(message.forward("mycroft.audio.queue", {"filename": filename})) + self.bus.emit(message.forward("mycroft.audio.queue", + {"filename": filename})) else: - LOG.warning("self.play_audio requires ovos-core >= 0.0.4a45, falling back to local skill playback") + LOG.warning("self.play_audio requires ovos-core >= 0.0.4a45, " + "falling back to local skill playback") play_audio(filename).wait() - @property - def core_lang(self): - """Get the configured default language.""" - return self._core_lang - - @property - def secondary_langs(self): - """Get the configured secondary languages, mycroft is not - considered to be in these languages but i will load it's resource - files. This provides initial support for multilingual input""" - return self._secondary_langs - - @property - def native_langs(self): - """Languages natively supported by core - ie, resource files available and explicitly supported + def load_lang(self, root_directory: Optional[str] = None, + lang: Optional[str] = None): """ - return self._native_langs - - @property - def alphanumeric_skill_id(self): - """skill id converted to only alphanumeric characters - Non alpha-numeric characters are converted to "_" - - Returns: - (str) String of letters + Get a SkillResources object for this skill in the requested `lang` for + resource files in the requested `root_directory`. + @param root_directory: root path to find resources (default res_dir) + @param lang: language to get resources for (default self.lang) + @return: SkillResources object """ - return self._alphanumeric_skill_id + return self._load_lang(root_directory, lang) - @property - def resources(self): - """Instantiates a ResourceFileLocator instance when needed. - a new instance is always created to ensure self.lang - reflects the active language and not the default core language + def voc_match(self, *args, **kwargs) -> Union[str, bool]: """ - return self._resources - - def load_lang(self, root_directory=None, lang=None): - """Instantiates a ResourceFileLocator instance when needed. - a new instance is always created to ensure lang - reflects the active language and not the default core language + Wraps the default `voc_match` method, but returns `False` instead of + raising FileNotFoundError when a resource can't be resolved """ - return self._load_lang(root_directory, lang) - - def voc_match(self, *args, **kwargs): try: return super().voc_match(*args, **kwargs) except FileNotFoundError: return False - def voc_list(self, voc_filename, lang=None) -> List[str]: + def voc_list(self, voc_filename: str, + lang: Optional[str] = None) -> List[str]: """ - Get vocabulary list and cache the results - - Args: - voc_filename (str): Name of vocabulary file (e.g. 'yes' for - 'res/text/en-us/yes.voc') - lang (str): Language code, defaults to self.lang - - Returns: - list: List of vocabulary found in voc_filename + Get list of vocab options for the requested resource and cache the + results for future references. + @param voc_filename: Name of vocab resource to get options for + @param lang: language to get vocab for (default self.lang) + @return: list of string vocab options """ return self._voc_list(voc_filename, lang) - def remove_voc(self, utt, voc_filename, lang=None): - """ removes any entry in .voc file from the utterance """ + def remove_voc(self, utt: str, voc_filename: str, + lang: Optional[str] = None) -> str: + """ + Removes any vocab match from the utterance. + @param utt: Utterance to evaluate + @param voc_filename: vocab resource to remove from utt + @param lang: Optional language associated with vocab and utterance + @return: string with vocab removed + """ if utt: # Check for matches against complete words for i in self.voc_list(voc_filename, lang): @@ -164,7 +201,8 @@ def remove_voc(self, utt, voc_filename, lang=None): return utt def _register_decorated(self): - """Register all intent handlers that are decorated with an intent. + """ + Register all intent handlers that are decorated with an intent. Looks for all functions that have been marked by a decorator and read the intent data from them. The intent handlers aren't the @@ -183,7 +221,14 @@ def _register_decorated(self): if hasattr(method, 'converse'): self.converse = method - def register_intent_layer(self, layer_name, intent_list): + def register_intent_layer(self, layer_name: str, + intent_list: List[Union[IntentBuilder, Intent, + str]]): + """ + Register a named intent layer. + @param layer_name: Name of intent layer to add + @param intent_list: List of intents associated with the intent layer + """ for intent_file in intent_list: if IntentBuilder is not None and isinstance(intent_file, IntentBuilder): intent = intent_file.build() @@ -195,7 +240,12 @@ def register_intent_layer(self, layer_name, intent_list): self.intent_layers.update_layer(layer_name, [name]) # killable_events support - def send_stop_signal(self, stop_event=None): + def send_stop_signal(self, stop_event: Optional[str] = None): + """ + Notify services to stop current execution + @param stop_event: optional `stop` event name to forward + """ + waiter = Event() msg = dig_for_message() or Message("mycroft.stop") # stop event execution if stop_event: @@ -212,15 +262,19 @@ def send_stop_signal(self, stop_event=None): # NOTE: mycroft does not have an event to stop recording # this attempts to force a stop by sending silence to end STT step self.bus.emit(Message('mycroft.mic.mute')) - time.sleep(1.5) # the silence from muting should make STT stop recording + waiter.wait(1.5) # the silence from muting should make STT stop recording self.bus.emit(Message('mycroft.mic.unmute')) - time.sleep(0.5) # if TTS had not yet started + # TODO: register TTS events to track state instead of guessing + waiter.wait(0.5) # if TTS had not yet started self.bus.emit(msg.forward("mycroft.audio.speech.stop")) # backwards compat alias, no functional difference class OVOSFallbackSkill(OVOSSkill): def __new__(cls, *args, **kwargs): + log_deprecation("Implement " + "`ovos_workshop.skills.fallback.FallbackSkill`", + "0.1.0") from ovos_workshop.skills.fallback import FallbackSkill return FallbackSkill(*args, **kwargs) diff --git a/requirements/test.txt b/requirements/test.txt new file mode 100644 index 00000000..b8b902ed --- /dev/null +++ b/requirements/test.txt @@ -0,0 +1,5 @@ +ovos-core~=0.0.7 +neon-lang-plugin-libretranslate~=0.2 +adapt-parser~=0.5 +pytest +pytest-cov diff --git a/test/unittests/skills/gui/ui/test.qml b/test/__init__.py similarity index 100% rename from test/unittests/skills/gui/ui/test.qml rename to test/__init__.py diff --git a/test/unittests/skills/intent_file/vocab/en-us/test.intent b/test/unittests/__init__.py similarity index 100% rename from test/unittests/skills/intent_file/vocab/en-us/test.intent rename to test/unittests/__init__.py diff --git a/test/unittests/skills/test_active.py b/test/unittests/skills/test_active.py new file mode 100644 index 00000000..0dc1b1e3 --- /dev/null +++ b/test/unittests/skills/test_active.py @@ -0,0 +1,28 @@ +import unittest +from unittest.mock import Mock + +from ovos_utils.messagebus import FakeBus +from ovos_workshop.skills.active import ActiveSkill +from ovos_workshop.skills.base import BaseSkill + + +class ActiveSkillExample(ActiveSkill): + active = Mock() + + def make_active(self): + self.active() + ActiveSkill.make_active(self) + + +class TestActiveSkill(unittest.TestCase): + def test_skill(self): + skill = ActiveSkillExample() + self.assertIsInstance(skill, BaseSkill) + skill.bind(FakeBus()) + skill.active.assert_called_once() + self.assertTrue(skill.active) + skill.deactivate() + self.assertTrue(skill.active) + skill.handle_skill_deactivated() + self.assertTrue(skill.active) + self.assertEqual(skill.active.call_count, 2) diff --git a/test/unittests/skills/test_auto_translatable.py b/test/unittests/skills/test_auto_translatable.py new file mode 100644 index 00000000..6b0271cc --- /dev/null +++ b/test/unittests/skills/test_auto_translatable.py @@ -0,0 +1,45 @@ +import unittest + +from ovos_workshop.skills.common_query_skill import CommonQuerySkill +from ovos_workshop.skills.fallback import FallbackSkill +from ovos_workshop.skills.base import BaseSkill + + +class TestUniversalSkill(unittest.TestCase): + from ovos_workshop.skills.auto_translatable import UniversalSkill + test_skill = UniversalSkill() + + def test_00_init(self): + self.assertIsInstance(self.test_skill, self.UniversalSkill) + self.assertIsInstance(self.test_skill, BaseSkill) + + # TODO: Test other class methods + + +class TestUniversalFallbackSkill(unittest.TestCase): + from ovos_workshop.skills.auto_translatable import UniversalFallback + test_skill = UniversalFallback() + + def test_00_init(self): + self.assertIsInstance(self.test_skill, self.UniversalFallback) + self.assertIsInstance(self.test_skill, BaseSkill) + self.assertIsInstance(self.test_skill, FallbackSkill) + + # TODO: Test other class methods + + +class TestUniversalCommonQuerySkill(unittest.TestCase): + from ovos_workshop.skills.auto_translatable import UniversalCommonQuerySkill + + class UniveralCommonQueryExample(UniversalCommonQuerySkill): + def CQS_match_query_phrase(self, phrase): + pass + + test_skill = UniveralCommonQueryExample() + + def test_00_init(self): + self.assertIsInstance(self.test_skill, self.UniversalCommonQuerySkill) + self.assertIsInstance(self.test_skill, BaseSkill) + self.assertIsInstance(self.test_skill, CommonQuerySkill) + + # TODO: Test other class methods diff --git a/test/unittests/skills/test_base.py b/test/unittests/skills/test_base.py new file mode 100644 index 00000000..38f19d64 --- /dev/null +++ b/test/unittests/skills/test_base.py @@ -0,0 +1,407 @@ +import unittest + +from logging import Logger +from unittest.mock import Mock, patch +from os.path import join, dirname, isdir + +from ovos_utils.messagebus import FakeBus + + +class TestBase(unittest.TestCase): + def test_is_classic_core(self): + from ovos_workshop.skills.base import is_classic_core + self.assertIsInstance(is_classic_core(), bool) + + def test_simple_trace(self): + from ovos_workshop.skills.base import simple_trace + trace = ["line_1\n", " line_2 \n", " \n", "line_3 \n"] + self.assertEqual(simple_trace(trace), "Traceback:\nline_1\n line_2 \n") + + +class TestBaseSkill(unittest.TestCase): + from ovos_workshop.skills.base import BaseSkill + bus = FakeBus() + skill_id = "test_base_skill" + skill = BaseSkill(bus=bus, skill_id=skill_id) + + def test_00_skill_init(self): + from ovos_workshop.settings import SkillSettingsManager + from ovos_workshop.skills.base import SkillGUI + from ovos_utils.events import EventContainer, EventSchedulerInterface + from ovos_utils.intents import IntentServiceInterface + from ovos_utils.process_utils import RuntimeRequirements + from ovos_utils.enclosure.api import EnclosureAPI + from ovos_workshop.filesystem import FileSystemAccess + from ovos_workshop.resource_files import SkillResources + + self.assertIsInstance(self.skill.log, Logger) + self.assertIsInstance(self.skill._enable_settings_manager, bool) + self.assertEqual(self.skill.name, self.skill.__class__.__name__) + self.assertEqual(self.skill.skill_id, self.skill_id) + self.assertIsInstance(self.skill.settings_manager, SkillSettingsManager) + self.assertTrue(isdir(self.skill.root_dir)) + self.assertEqual(self.skill.res_dir, self.skill.root_dir) + self.assertIsInstance(self.skill.gui, SkillGUI) + self.assertIsInstance(self.skill.config_core, dict) + self.assertIsNone(self.skill.settings_change_callback) + self.assertTrue(self.skill.reload_skill) + self.assertIsInstance(self.skill.events, EventContainer) + self.assertEqual(self.skill.events.bus, self.bus) + self.assertIsInstance(self.skill.event_scheduler, + EventSchedulerInterface) + self.assertIsInstance(self.skill.intent_service, IntentServiceInterface) + + self.assertIsInstance(self.skill.runtime_requirements, + RuntimeRequirements) + self.assertIsInstance(self.skill.voc_match_cache, dict) + self.assertTrue(self.skill._is_fully_initialized) + self.assertTrue(isdir(dirname(self.skill._settings_path))) + self.assertIsInstance(self.skill.settings, dict) + self.assertIsNone(self.skill.dialog_renderer) + self.assertIsInstance(self.skill.enclosure, EnclosureAPI) + self.assertIsInstance(self.skill.file_system, FileSystemAccess) + self.assertTrue(isdir(self.skill.file_system.path)) + self.assertEqual(self.skill.bus, self.bus) + self.assertIsInstance(self.skill.location, dict) + self.assertIsInstance(self.skill.location_pretty, str) + self.assertIsInstance(self.skill.location_timezone, str) + self.assertIsInstance(self.skill.lang, str) + self.assertEqual(len(self.skill.lang.split('-')), 2) + self.assertEqual(self.skill._core_lang, self.skill.lang) + self.assertIsInstance(self.skill._secondary_langs, list) + self.assertIsInstance(self.skill._native_langs, list) + self.assertIn(self.skill._core_lang, self.skill._native_langs) + self.assertIsInstance(self.skill._alphanumeric_skill_id, str) + self.assertIsInstance(self.skill._resources, SkillResources) + self.assertEqual(self.skill._resources.language, self.skill.lang) + self.assertFalse(self.skill._stop_is_implemented) + self.assertFalse(self.skill._converse_is_implemented) + + def test_handle_first_run(self): + # TODO + pass + + def test_check_for_first_run(self): + # TODO + pass + + def test_startup(self): + # TODO + pass + + def test_init_settings(self): + # TODO + pass + + def test_init_skill_gui(self): + # TODO + pass + + def test_init_settings_manager(self): + # TODO + pass + + def test_start_filewatcher(self): + # TODO + pass + + def test_upload_settings(self): + # TODO + pass + + def test_handle_settings_file_change(self): + # TODO + pass + + def test_load_lang(self): + # TODO + pass + + def test_bind(self): + # TODO + pass + + def test_register_public_api(self): + # TODO + pass + + def test_register_system_event_handlers(self): + # TODO + pass + + def test_handle_settings_change(self): + # TODO + pass + + def test_detach(self): + # TODO + pass + + def test_send_public_api(self): + # TODO + pass + + def test_get_intro_message(self): + self.assertIsInstance(self.skill.get_intro_message(), str) + self.assertFalse(self.skill.get_intro_message()) + + def test_handle_skill_activated(self): + # TODO + pass + + def test_handle_skill_deactivated(self): + # TODO + pass + + def test_activate(self): + # TODO + pass + + def test_deactivate(self): + # TODO + pass + + def test_handle_converse_ack(self): + # TODO + pass + + def test_handle_converse_request(self): + # TODO + pass + + def test_converse(self): + # TODO + self.assertFalse(self.skill.converse()) + + # TODO port get_response methods per #69 + + def test_ask_yesno(self): + # TODO + pass + + def test_ask_selection(self): + # TODO + pass + + def test_voc_list(self): + # TODO + pass + + def test_voc_match(self): + # TODO + pass + + def test_report_metric(self): + # TODO + pass + + def test_send_email(self): + # TODO + pass + + def test_handle_collect_resting(self): + # TODO + pass + + def test_register_resting_screen(self): + # TODO + pass + + def test_register_decorated(self): + # TODO + pass + + def test_find_resource(self): + # TODO + pass + + def test_on_event_start(self): + # TODO + pass + + def test_on_event_end(self): + # TODO + pass + + def test_on_event_error(self): + # TODO + pass + + def test_add_event(self): + # TODO + pass + + def test_remove_event(self): + # TODO + pass + + def test_register_adapt_intent(self): + # TODO + pass + + def test_register_intent(self): + # TODO + pass + + def test_register_intent_file(self): + # TODO + pass + + def test_register_entity_file(self): + # TODO + pass + + def test_handle_enable_intent(self): + # TODO + pass + + def test_handle_disable_intent(self): + # TODO + pass + + def test_disable_intent(self): + # TODO + pass + + def test_enable_intent(self): + # TODO + pass + + def test_set_context(self): + # TODO + pass + + def test_remove_context(self): + # TODO + pass + + def test_handle_set_cross_context(self): + # TODO + pass + + def test_handle_remove_cross_context(self): + # TODO + pass + + def test_set_cross_skill_contest(self): + # TODO + pass + + def test_remove_cross_skill_context(self): + # TODO + pass + + def test_register_vocabulary(self): + # TODO + pass + + def test_register_regex(self): + # TODO + pass + + def test_speak(self): + # TODO + pass + + def test_speak_dialog(self): + # TODO + pass + + def test_acknowledge(self): + # TODO + pass + + def test_load_dialog_files(self): + # TODO + pass + + def test_load_data_files(self): + # TODO + pass + + def test_load_vocab_files(self): + # TODO + pass + + def test_load_regex_files(self): + # TODO + pass + + def test_handle_stop(self): + # TODO + pass + + def test_stop(self): + self.skill.stop() + + def test_shutdown(self): + self.skill.shutdown() + + def test_default_shutdown(self): + # TODO + pass + + def test_schedule_event(self): + # TODO + pass + + def test_schedule_repeating_event(self): + # TODO + pass + + def test_update_scheduled_event(self): + # TODO + pass + + def test_cancel_scheduled_event(self): + # TODO + pass + + def test_get_scheduled_event_status(self): + # TODO + pass + + def test_cancel_all_repeating_events(self): + # TODO + pass + + +class TestSkillGui(unittest.TestCase): + class LegacySkill(Mock): + skill_id = "old_skill" + bus = FakeBus() + config_core = {"gui": {"test": True, + "legacy": True}} + root_dir = join(dirname(__file__), "test_gui/gui") + + class GuiSkill(Mock): + skill_id = "new_skill" + bus = FakeBus() + config_core = {"gui": {"test": True, + "legacy": False}} + root_dir = join(dirname(__file__), "test_gui") + + @patch("ovos_workshop.skills.base.GUIInterface.__init__") + def test_skill_gui(self, interface_init): + from ovos_utils.gui import GUIInterface + from ovos_workshop.skills.base import SkillGUI + + # Old skill with `ui` directory in root + old_skill = self.LegacySkill() + old_gui = SkillGUI(old_skill) + self.assertEqual(old_gui.skill, old_skill) + self.assertIsInstance(old_gui, GUIInterface) + interface_init.assert_called_once_with( + old_gui, skill_id=old_skill.skill_id, bus=old_skill.bus, + config=old_skill.config_core['gui'], + ui_directories={"qt5": join(old_skill.root_dir, "ui")}) + + # New skill with `gui` directory in root + new_skill = self.GuiSkill() + new_gui = SkillGUI(new_skill) + self.assertEqual(new_gui.skill, new_skill) + self.assertIsInstance(new_gui, GUIInterface) + interface_init.assert_called_with( + new_gui, skill_id=new_skill.skill_id, bus=new_skill.bus, + config=new_skill.config_core['gui'], + ui_directories={"all": join(new_skill.root_dir, "gui")}) diff --git a/test/unittests/skills/test_fallback_skill.py b/test/unittests/skills/test_fallback_skill.py index ffa7450e..0188a82b 100644 --- a/test/unittests/skills/test_fallback_skill.py +++ b/test/unittests/skills/test_fallback_skill.py @@ -1,53 +1,234 @@ -from unittest import TestCase, mock +from unittest import TestCase +from unittest.mock import patch -from ovos_workshop.skills.fallback import FallbackSkillV1 as FallbackSkill +from ovos_utils.messagebus import FakeBus +from ovos_workshop.decorators import fallback_handler +from ovos_workshop.skills.base import BaseSkill +from ovos_workshop.skills.fallback import FallbackSkillV1, FallbackSkillV2, \ + FallbackSkill -def setup_fallback(fb_class): - fb_skill = fb_class() - fb_skill.bind(mock.Mock(name='bus')) - fb_skill.initialize() - return fb_skill +class SimpleFallback(FallbackSkillV1): + """Simple fallback skill used for test.""" + def initialize(self): + self.register_fallback(self.fallback_handler, 42) + + def fallback_handler(self, _): + pass + + +class V2FallbackSkill(FallbackSkillV2): + def __init__(self): + FallbackSkillV2.__init__(self, FakeBus(), "fallback_v2") + + @fallback_handler + def handle_fallback(self, message): + pass + + @fallback_handler(10) + def high_prio_fallback(self, message): + pass class TestFallbackSkill(TestCase): - def test_life_cycle(self): - """Test startup and shutdown of a fallback skill. + # TODO: Test `__new__` logic + pass + + def test_class_inheritance(self): + from ovos_workshop.skills.ovos import OVOSSkill + from ovos_workshop.skills.mycroft_skill import MycroftSkill + fallback = FallbackSkill("test") + self.assertIsInstance(fallback, BaseSkill) + self.assertIsInstance(fallback, OVOSSkill) + self.assertIsInstance(fallback, MycroftSkill) + self.assertIsInstance(fallback, FallbackSkillV1) + self.assertIsInstance(fallback, FallbackSkillV2) + self.assertIsInstance(fallback, FallbackSkill) + + +class TestFallbackSkillV1(TestCase): + @staticmethod + def setup_fallback(fb_class): + fb_skill = fb_class() + fb_skill.bind(FakeBus()) + fb_skill.initialize() + return fb_skill + + def test_inheritance(self): + from ovos_workshop.skills.ovos import OVOSSkill + from ovos_workshop.skills.mycroft_skill import MycroftSkill + fallback = FallbackSkillV1("test") + self.assertIsInstance(fallback, BaseSkill) + self.assertIsInstance(fallback, OVOSSkill) + self.assertIsInstance(fallback, MycroftSkill) + self.assertIsInstance(fallback, FallbackSkillV1) + self.assertIsInstance(fallback, FallbackSkillV2) + self.assertIsInstance(fallback, FallbackSkill) + + def test_make_intent_failure_handler(self): + # TODO + pass + + def test_report_timing(self): + # TODO + pass + + def test__register_fallback(self): + # TODO + pass + + def test_register_fallback(self): + # TODO + pass + + def test_remove_registered_handler(self): + # TODO + pass + + @patch("ovos_workshop.skills.fallback.FallbackSkillV1." + "_remove_registered_handler") + def test_remove_fallback(self, remove_handler): + def wrapper(handler): + def wrapped(): + if handler(): + return True + return False + return wrapped + + def _mock_1(): + pass + + def _mock_2(): + pass + + FallbackSkillV1.wrapper_map.append((_mock_1, wrapper(_mock_1))) + self.assertEqual(len(FallbackSkillV1.wrapper_map), 1) + + FallbackSkillV1.wrapper_map.append((_mock_2, wrapper(_mock_2))) + self.assertEqual(len(FallbackSkillV1.wrapper_map), 2) + + # Successful remove existing wrapper + remove_handler.return_value = True + self.assertTrue(FallbackSkillV1.remove_fallback(_mock_1)) + self.assertEqual(len(FallbackSkillV1.wrapper_map), 1) + self.assertFalse(FallbackSkillV1.remove_fallback(_mock_1)) + self.assertEqual(len(FallbackSkillV1.wrapper_map), 1) + # Failed remove existing wrapper + remove_handler.return_value = False + self.assertFalse(FallbackSkillV1.remove_fallback( + FallbackSkillV1.wrapper_map[0][1])) + self.assertEqual(FallbackSkillV1.wrapper_map, []) + + def test_remove_instance_handlers(self): + # TODO + pass + + def test_default_shutdown(self): + # TODO + pass + + def test_register_decorated(self): + # TODO + pass + + def test_life_cycle(self): + """ + Test startup and shutdown of a fallback skill. Ensure that an added handler is removed as part of default shutdown. """ - self.assertEqual(len(FallbackSkill.fallback_handlers), 0) - fb_skill = setup_fallback(SimpleFallback) - self.assertEqual(len(FallbackSkill.fallback_handlers), 1) - self.assertEqual(FallbackSkill.wrapper_map[0][0], + self.assertEqual(len(FallbackSkillV1.fallback_handlers), 0) + fb_skill = self.setup_fallback(SimpleFallback) + self.assertEqual(len(FallbackSkillV1.fallback_handlers), 1) + self.assertEqual(FallbackSkillV1.wrapper_map[0][0], fb_skill.fallback_handler) - self.assertEqual(len(FallbackSkill.wrapper_map), 1) + self.assertEqual(len(FallbackSkillV1.wrapper_map), 1) fb_skill.default_shutdown() - self.assertEqual(len(FallbackSkill.fallback_handlers), 0) - self.assertEqual(len(FallbackSkill.wrapper_map), 0) + self.assertEqual(len(FallbackSkillV1.fallback_handlers), 0) + self.assertEqual(len(FallbackSkillV1.wrapper_map), 0) def test_manual_removal(self): - """Test that the call to remove_fallback() removes the handler""" - self.assertEqual(len(FallbackSkill.fallback_handlers), 0) + """ + Test that the call to remove_fallback() removes the handler + """ + self.assertEqual(len(FallbackSkillV1.fallback_handlers), 0) # Create skill adding a single handler - fb_skill = setup_fallback(SimpleFallback) - self.assertEqual(len(FallbackSkill.fallback_handlers), 1) + fb_skill = self.setup_fallback(SimpleFallback) + self.assertEqual(len(FallbackSkillV1.fallback_handlers), 1) self.assertTrue(fb_skill.remove_fallback(fb_skill.fallback_handler)) # Both internal trackers of handlers should be cleared now - self.assertEqual(len(FallbackSkill.fallback_handlers), 0) - self.assertEqual(len(FallbackSkill.wrapper_map), 0) + self.assertEqual(len(FallbackSkillV1.fallback_handlers), 0) + self.assertEqual(len(FallbackSkillV1.wrapper_map), 0) # Removing after it's already been removed should fail self.assertFalse(fb_skill.remove_fallback(fb_skill.fallback_handler)) -class SimpleFallback(FallbackSkill): - """Simple fallback skill used for test.""" - def initialize(self): - self.register_fallback(self.fallback_handler, 42) +class TestFallbackSkillV2(TestCase): + fallback_skill = FallbackSkillV2(FakeBus(), "test_fallback_v2") + + def test_class_inheritance(self): + from ovos_workshop.skills.ovos import OVOSSkill + from ovos_workshop.skills.mycroft_skill import MycroftSkill + self.assertIsInstance(self.fallback_skill, BaseSkill) + self.assertIsInstance(self.fallback_skill, OVOSSkill) + self.assertIsInstance(self.fallback_skill, MycroftSkill) + self.assertIsInstance(self.fallback_skill, FallbackSkillV1) + self.assertIsInstance(self.fallback_skill, FallbackSkillV2) + self.assertIsInstance(self.fallback_skill, FallbackSkill) + + def test_00_init(self): + self.assertIsInstance(self.fallback_skill, FallbackSkillV2) + self.assertIsInstance(self.fallback_skill, FallbackSkill) + self.assertIsInstance(self.fallback_skill, BaseSkill) + + def test_priority(self): + FallbackSkillV2.fallback_config = {} + + # No config or handlers + self.assertEqual(self.fallback_skill.priority, 101) + # Config override + FallbackSkillV2.fallback_config = \ + {"fallback_priorities": {"test_fallback_v2": 10}} + self.assertEqual(self.fallback_skill.priority, 10, + self.fallback_skill.fallback_config) + + fallback_skill = V2FallbackSkill() + + # Minimum handler + self.assertEqual(fallback_skill.priority, 10) + # Config override + FallbackSkillV2.fallback_config['fallback_priorities'][ + fallback_skill.skill_id] = 80 + self.assertEqual(fallback_skill.priority, 80) + + def test_can_answer(self): + self.assertFalse(self.fallback_skill.can_answer([""], "en-us")) + # TODO + + def test_register_system_event_handlers(self): + # TODO + pass + + def test_handle_fallback_ack(self): + # TODO + pass + + def test_handle_fallback_request(self): + # TODO + pass + + def test_register_fallback(self): + # TODO + pass + + def test_default_shutdown(self): + # TODO + pass - def fallback_handler(self): + def test_register_decorated(self): + # TODO pass diff --git a/test/unittests/skills/intent_file/vocab/en-us/test_ent.entity b/test/unittests/skills/test_gui/gui/ui/test.qml similarity index 100% rename from test/unittests/skills/intent_file/vocab/en-us/test_ent.entity rename to test/unittests/skills/test_gui/gui/ui/test.qml diff --git a/test/unittests/skills/test_idle_display_skill.py b/test/unittests/skills/test_idle_display_skill.py new file mode 100644 index 00000000..93b6026e --- /dev/null +++ b/test/unittests/skills/test_idle_display_skill.py @@ -0,0 +1,14 @@ +import unittest + +from ovos_utils.messagebus import FakeBus +from ovos_workshop.skills.base import BaseSkill +from ovos_workshop.skills.idle_display_skill import IdleDisplaySkill + + +class TestIdleDisplaySkill(unittest.TestCase): + skill = IdleDisplaySkill(bus=FakeBus(), skill_id="test_idle_skill") + + def test_00_skill_init(self): + self.assertIsInstance(self.skill, BaseSkill) + self.assertIsInstance(self.skill, IdleDisplaySkill) + # TODO: Implement more tests \ No newline at end of file diff --git a/test/unittests/skills/test_mycroft_skill/__init__.py b/test/unittests/skills/test_mycroft_skill/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/test/unittests/skills/test_mycroft_skill/intent_file/vocab/en-us/test.intent b/test/unittests/skills/test_mycroft_skill/intent_file/vocab/en-us/test.intent new file mode 100644 index 00000000..e69de29b diff --git a/test/unittests/skills/test_mycroft_skill/intent_file/vocab/en-us/test_ent.entity b/test/unittests/skills/test_mycroft_skill/intent_file/vocab/en-us/test_ent.entity new file mode 100644 index 00000000..e69de29b diff --git a/test/unittests/skills/locale/en-us/turn_off2_test.voc b/test/unittests/skills/test_mycroft_skill/locale/en-us/turn_off2_test.voc similarity index 100% rename from test/unittests/skills/locale/en-us/turn_off2_test.voc rename to test/unittests/skills/test_mycroft_skill/locale/en-us/turn_off2_test.voc diff --git a/test/unittests/skills/locale/en-us/turn_off_test.voc b/test/unittests/skills/test_mycroft_skill/locale/en-us/turn_off_test.voc similarity index 100% rename from test/unittests/skills/locale/en-us/turn_off_test.voc rename to test/unittests/skills/test_mycroft_skill/locale/en-us/turn_off_test.voc diff --git a/test/unittests/skills/mocks.py b/test/unittests/skills/test_mycroft_skill/mocks.py similarity index 100% rename from test/unittests/skills/mocks.py rename to test/unittests/skills/test_mycroft_skill/mocks.py diff --git a/test/unittests/skills/test_mycroft_skill.py b/test/unittests/skills/test_mycroft_skill/test_mycroft_skill.py similarity index 99% rename from test/unittests/skills/test_mycroft_skill.py rename to test/unittests/skills/test_mycroft_skill/test_mycroft_skill.py index 32d87fd6..c5f1a639 100644 --- a/test/unittests/skills/test_mycroft_skill.py +++ b/test/unittests/skills/test_mycroft_skill/test_mycroft_skill.py @@ -20,12 +20,12 @@ from os.path import join, dirname, abspath from unittest.mock import MagicMock, patch -#import pytest from ovos_utils.intents import IntentBuilder from ovos_bus_client import Message from ovos_config.config import Configuration -from ovos_workshop.decorators import intent_handler, resting_screen_handler, intent_file_handler +from ovos_workshop.decorators import intent_handler, resting_screen_handler, \ + intent_file_handler from ovos_workshop.skills.mycroft_skill import MycroftSkill from .mocks import base_config @@ -247,7 +247,6 @@ def check_register_decorators(self, result_list): sorted(result_list, key=lambda d: sorted(d.items()))) self.emitter.reset() - #@pytest.mark.skip def test_register_decorators(self): """ Test decorated intents """ path_orig = sys.path @@ -266,7 +265,8 @@ def test_register_decorators(self): 'vocab', 'en-us', 'test.intent'), 'lang': 'en-us', 'samples': [], - 'name': str(s.skill_id) + ':test.intent'}] + 'name': str(s.skill_id) + ':test.intent'} + ] self.check_register_decorators(expected) # Restore sys.path diff --git a/test/unittests/skills/test_mycroft_skill_get_response.py b/test/unittests/skills/test_mycroft_skill/test_mycroft_skill_get_response.py similarity index 100% rename from test/unittests/skills/test_mycroft_skill_get_response.py rename to test/unittests/skills/test_mycroft_skill/test_mycroft_skill_get_response.py diff --git a/test/unittests/skills/test_skill/__init__.py b/test/unittests/skills/test_mycroft_skill/test_skill/__init__.py similarity index 100% rename from test/unittests/skills/test_skill/__init__.py rename to test/unittests/skills/test_mycroft_skill/test_skill/__init__.py diff --git a/test/unittests/skills/test_skill/dialog/en-us/what do you want.dialog b/test/unittests/skills/test_mycroft_skill/test_skill/dialog/en-us/what do you want.dialog similarity index 100% rename from test/unittests/skills/test_skill/dialog/en-us/what do you want.dialog rename to test/unittests/skills/test_mycroft_skill/test_skill/dialog/en-us/what do you want.dialog diff --git a/test/unittests/skills/translate/in-dialog/dialog/en-us/good_things.list b/test/unittests/skills/test_mycroft_skill/translate/in-dialog/dialog/en-us/good_things.list similarity index 100% rename from test/unittests/skills/translate/in-dialog/dialog/en-us/good_things.list rename to test/unittests/skills/test_mycroft_skill/translate/in-dialog/dialog/en-us/good_things.list diff --git a/test/unittests/skills/translate/in-dialog/dialog/en-us/named_things.value b/test/unittests/skills/test_mycroft_skill/translate/in-dialog/dialog/en-us/named_things.value similarity index 100% rename from test/unittests/skills/translate/in-dialog/dialog/en-us/named_things.value rename to test/unittests/skills/test_mycroft_skill/translate/in-dialog/dialog/en-us/named_things.value diff --git a/test/unittests/skills/translate/in-dialog/dialog/en-us/test.template b/test/unittests/skills/test_mycroft_skill/translate/in-dialog/dialog/en-us/test.template similarity index 100% rename from test/unittests/skills/translate/in-dialog/dialog/en-us/test.template rename to test/unittests/skills/test_mycroft_skill/translate/in-dialog/dialog/en-us/test.template diff --git a/test/unittests/skills/translate/in-locale/locale/de-de/good_things.list b/test/unittests/skills/test_mycroft_skill/translate/in-locale/locale/de-de/good_things.list similarity index 100% rename from test/unittests/skills/translate/in-locale/locale/de-de/good_things.list rename to test/unittests/skills/test_mycroft_skill/translate/in-locale/locale/de-de/good_things.list diff --git a/test/unittests/skills/translate/in-locale/locale/de-de/named_things.value b/test/unittests/skills/test_mycroft_skill/translate/in-locale/locale/de-de/named_things.value similarity index 100% rename from test/unittests/skills/translate/in-locale/locale/de-de/named_things.value rename to test/unittests/skills/test_mycroft_skill/translate/in-locale/locale/de-de/named_things.value diff --git a/test/unittests/skills/translate/in-locale/locale/de-de/test.template b/test/unittests/skills/test_mycroft_skill/translate/in-locale/locale/de-de/test.template similarity index 100% rename from test/unittests/skills/translate/in-locale/locale/de-de/test.template rename to test/unittests/skills/test_mycroft_skill/translate/in-locale/locale/de-de/test.template diff --git a/test/unittests/skills/translate/in-locale/locale/en-us/good_things.list b/test/unittests/skills/test_mycroft_skill/translate/in-locale/locale/en-us/good_things.list similarity index 100% rename from test/unittests/skills/translate/in-locale/locale/en-us/good_things.list rename to test/unittests/skills/test_mycroft_skill/translate/in-locale/locale/en-us/good_things.list diff --git a/test/unittests/skills/translate/in-locale/locale/en-us/named_things.value b/test/unittests/skills/test_mycroft_skill/translate/in-locale/locale/en-us/named_things.value similarity index 100% rename from test/unittests/skills/translate/in-locale/locale/en-us/named_things.value rename to test/unittests/skills/test_mycroft_skill/translate/in-locale/locale/en-us/named_things.value diff --git a/test/unittests/skills/translate/in-locale/locale/en-us/not_in_german.list b/test/unittests/skills/test_mycroft_skill/translate/in-locale/locale/en-us/not_in_german.list similarity index 100% rename from test/unittests/skills/translate/in-locale/locale/en-us/not_in_german.list rename to test/unittests/skills/test_mycroft_skill/translate/in-locale/locale/en-us/not_in_german.list diff --git a/test/unittests/skills/translate/in-locale/locale/en-us/test.template b/test/unittests/skills/test_mycroft_skill/translate/in-locale/locale/en-us/test.template similarity index 100% rename from test/unittests/skills/translate/in-locale/locale/en-us/test.template rename to test/unittests/skills/test_mycroft_skill/translate/in-locale/locale/en-us/test.template diff --git a/test/unittests/test_skill_classes.py b/test/unittests/skills/test_ovos.py similarity index 50% rename from test/unittests/test_skill_classes.py rename to test/unittests/skills/test_ovos.py index 9492d7ff..d43ab520 100644 --- a/test/unittests/test_skill_classes.py +++ b/test/unittests/skills/test_ovos.py @@ -1,13 +1,13 @@ import unittest -from unittest.mock import Mock -from ovos_workshop import OVOSAbstractApplication -from ovos_workshop.decorators import classproperty -from ovos_workshop.skills.ovos import OVOSSkill from ovos_utils.process_utils import RuntimeRequirements -from ovos_workshop.skills.mycroft_skill import is_classic_core from ovos_utils.messagebus import FakeBus +from ovos_utils import classproperty +from ovos_workshop import IntentLayers +from ovos_workshop.resource_files import SkillResources + from ovos_workshop.settings import SkillSettingsManager +from ovos_workshop.skills.ovos import OVOSSkill class OfflineSkill(OVOSSkill): @@ -33,39 +33,87 @@ def runtime_requirements(self): no_network_fallback=False) -class TestSkill(OVOSSkill): +class MockSkill(OVOSSkill): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) -class TestApplication(OVOSAbstractApplication): - def __init__(self, *args, **kwargs): - super().__init__(skill_id="Test Application", *args, **kwargs) - - -class TestSkills(unittest.TestCase): +class TestOVOSSkill(unittest.TestCase): + bus = FakeBus() + skill = OVOSSkill(bus=bus, skill_id="test_ovos_skill") + + def test_00_skill_init(self): + from ovos_utils.skills.audioservice import AudioServiceInterface + self.assertIsInstance(self.skill.private_settings, dict) + self.assertIsInstance(self.skill._threads, list) + self.assertIsNotNone(self.skill._original_converse) + self.assertIsInstance(self.skill.intent_layers, IntentLayers) + self.assertIsInstance(self.skill.audio_service, AudioServiceInterface) + self.assertTrue(self.skill.is_fully_initialized) + self.assertFalse(self.skill.stop_is_implemented) + self.assertFalse(self.skill.converse_is_implemented) + self.assertIsInstance(self.skill.core_lang, str) + self.assertIsInstance(self.skill.secondary_langs, list) + self.assertIsInstance(self.skill.native_langs, list) + self.assertIsInstance(self.skill.alphanumeric_skill_id, str) + self.assertIsInstance(self.skill.resources, SkillResources) + + def test_activate(self): + # TODO + pass + + def test_deactivate(self): + # TODO + pass + + def test_play_audio(self): + # TODO + pass + + def test_load_lang(self): + # TODO + pass + + def test_voc_match(self): + # TODO + pass + + def test_voc_list(self): + # TODO + pass + + def test_remove_voc(self): + # TODO + pass + + def test_register_decorated(self): + # TODO + pass + + def test_register_intent_layer(self): + # TODO + pass + + def test_send_stop_signal(self): + # TODO + pass def test_settings_manager_init(self): bus = FakeBus() - skill_default = TestSkill(bus=bus) + skill_default = MockSkill(bus=bus) skill_default._startup(bus) - # This doesn't apply to `mycroft-core`, only `ovos-core` - if not is_classic_core(): - self.assertIsInstance(skill_default.settings_manager, SkillSettingsManager) - skill_disabled_settings = TestSkill(bus=bus, - enable_settings_manager=False) - skill_disabled_settings._startup(bus) - self.assertIsNone(skill_disabled_settings.settings_manager) + self.assertIsInstance(skill_default.settings_manager, + SkillSettingsManager) - plugin = TestApplication(bus=bus) - plugin._startup(bus) - self.assertIsNone(plugin.settings_manager) + skill_disabled_settings = MockSkill(bus=bus, + enable_settings_manager=False) + skill_disabled_settings._startup(bus) + self.assertIsNone(skill_disabled_settings.settings_manager) def test_bus_setter(self): - from ovos_utils.messagebus import FakeBus bus = FakeBus() - skill = TestSkill() + skill = MockSkill() skill._startup(bus) self.assertEqual(skill.bus, bus) new_bus = FakeBus() @@ -74,7 +122,7 @@ def test_bus_setter(self): with self.assertRaises(TypeError): skill.bus = None - def test_class_property(self): + def test_runtime_requirements(self): self.assertEqual(OfflineSkill.runtime_requirements, RuntimeRequirements(internet_before_load=False, network_before_load=False, @@ -92,57 +140,17 @@ def test_class_property(self): no_network_fallback=False) ) self.assertEqual(OVOSSkill.runtime_requirements, - RuntimeRequirements() - ) + RuntimeRequirements()) def test_class_inheritance(self): from ovos_workshop.skills.base import BaseSkill from ovos_workshop.skills.ovos import OVOSSkill from ovos_workshop.skills.mycroft_skill import MycroftSkill - from ovos_workshop.skills.fallback import FallbackSkill, FallbackSkillV2, FallbackSkillV1 from ovos_workshop.app import OVOSAbstractApplication - skill = TestSkill() + skill = MockSkill() self.assertIsInstance(skill, BaseSkill) self.assertIsInstance(skill, OVOSSkill) self.assertIsInstance(skill, MycroftSkill) self.assertNotIsInstance(skill, OVOSAbstractApplication) - app = TestApplication() - self.assertIsInstance(app, BaseSkill) - self.assertIsInstance(app, OVOSSkill) - self.assertIsInstance(app, MycroftSkill) - self.assertIsInstance(app, OVOSAbstractApplication) - - mycroft_skill = MycroftSkill() - self.assertIsInstance(mycroft_skill, BaseSkill) - self.assertIsInstance(mycroft_skill, MycroftSkill) - self.assertNotIsInstance(mycroft_skill, OVOSSkill) - self.assertNotIsInstance(mycroft_skill, OVOSAbstractApplication) - - fallback = FallbackSkill("test") - self.assertIsInstance(fallback, BaseSkill) - self.assertIsInstance(fallback, OVOSSkill) - self.assertIsInstance(fallback, MycroftSkill) - self.assertIsInstance(fallback, FallbackSkillV1) - self.assertIsInstance(fallback, FallbackSkillV2) - self.assertIsInstance(fallback, FallbackSkill) - self.assertNotIsInstance(fallback, OVOSAbstractApplication) - - fallback = FallbackSkillV1("test") - self.assertIsInstance(fallback, BaseSkill) - self.assertIsInstance(fallback, OVOSSkill) - self.assertIsInstance(fallback, MycroftSkill) - self.assertIsInstance(fallback, FallbackSkillV1) - self.assertIsInstance(fallback, FallbackSkillV2) - self.assertIsInstance(fallback, FallbackSkill) - self.assertNotIsInstance(fallback, OVOSAbstractApplication) - - fallback = FallbackSkillV2("test") - self.assertIsInstance(fallback, BaseSkill) - self.assertIsInstance(fallback, OVOSSkill) - self.assertIsInstance(fallback, MycroftSkill) - self.assertIsInstance(fallback, FallbackSkillV1) - self.assertIsInstance(fallback, FallbackSkillV2) - self.assertIsInstance(fallback, FallbackSkill) - self.assertNotIsInstance(fallback, OVOSAbstractApplication) diff --git a/test/unittests/test_abstract_app.py b/test/unittests/test_abstract_app.py index 17773288..d743ce2b 100644 --- a/test/unittests/test_abstract_app.py +++ b/test/unittests/test_abstract_app.py @@ -31,6 +31,9 @@ def setUpClass(cls) -> None: settings=cls.settings_obj, gui=cls.gui) cls.app._startup(cls.bus) + def test_settings_manager_init(self): + self.assertIsNone(self.app.settings_manager) + def test_settings_init(self): self.assertNotEqual(self.app.settings, self.settings_obj) self.assertFalse(self.app.settings['__mycroft_skill_firstrun']) @@ -80,3 +83,14 @@ def test_settings_path(self): # Cleanup test files remove(test_app._settings_path) remove(test_skill._settings_path) + + def test_class_inheritance(self): + from ovos_workshop.skills.base import BaseSkill + from ovos_workshop.skills.ovos import OVOSSkill + from ovos_workshop.skills.mycroft_skill import MycroftSkill + from ovos_workshop.app import OVOSAbstractApplication + + self.assertIsInstance(self.app, BaseSkill) + self.assertIsInstance(self.app, OVOSSkill) + self.assertIsInstance(self.app, MycroftSkill) + self.assertIsInstance(self.app, OVOSAbstractApplication) diff --git a/test/unittests/test_skill.py b/test/unittests/test_skill.py index 7e15151a..c1f25234 100644 --- a/test/unittests/test_skill.py +++ b/test/unittests/test_skill.py @@ -1,6 +1,6 @@ import json import unittest -from unittest.mock import Mock, patch +from unittest.mock import Mock from ovos_bus_client import Message @@ -8,7 +8,7 @@ from ovos_workshop.skills.mycroft_skill import MycroftSkill, is_classic_core from mycroft.skills import MycroftSkill as CoreSkill from ovos_utils.messagebus import FakeBus -from os.path import dirname, join +from os.path import dirname from ovos_workshop.skill_launcher import SkillLoader @@ -207,44 +207,3 @@ def test_load(self): self.assertEqual(args.skill_id, "args") self.assertEqual(args.bus, bus) self.assertEqual(args.gui, gui) - - -class TestSkillGui(unittest.TestCase): - class LegacySkill(Mock): - skill_id = "old_skill" - bus = FakeBus() - config_core = {"gui": {"test": True, - "legacy": True}} - root_dir = join(dirname(__file__), "skills", "gui") - - class GuiSkill(Mock): - skill_id = "new_skill" - bus = FakeBus() - config_core = {"gui": {"test": True, - "legacy": False}} - root_dir = join(dirname(__file__), "skills") - - @patch("ovos_workshop.skills.base.GUIInterface.__init__") - def test_skill_gui(self, interface_init): - from ovos_utils.gui import GUIInterface - from ovos_workshop.skills.base import SkillGUI - - # Old skill with `ui` directory in root - old_skill = self.LegacySkill() - old_gui = SkillGUI(old_skill) - self.assertEqual(old_gui.skill, old_skill) - self.assertIsInstance(old_gui, GUIInterface) - interface_init.assert_called_once_with( - old_gui, skill_id=old_skill.skill_id, bus=old_skill.bus, - config=old_skill.config_core['gui'], - ui_directories={"qt5": join(old_skill.root_dir, "ui")}) - - # New skill with `gui` directory in root - new_skill = self.GuiSkill() - new_gui = SkillGUI(new_skill) - self.assertEqual(new_gui.skill, new_skill) - self.assertIsInstance(new_gui, GUIInterface) - interface_init.assert_called_with( - new_gui, skill_id=new_skill.skill_id, bus=new_skill.bus, - config=new_skill.config_core['gui'], - ui_directories={"all": join(new_skill.root_dir, "gui")}) diff --git a/test/unittests/test_skill_launcher.py b/test/unittests/test_skill_launcher.py index d865e416..187d113c 100644 --- a/test/unittests/test_skill_launcher.py +++ b/test/unittests/test_skill_launcher.py @@ -58,7 +58,7 @@ def test_remove_submodule_refs(self): def test_load_skill_module(self): from ovos_workshop.skill_launcher import load_skill_module - test_path = join(dirname(__file__), "skills", "test_skill", + test_path = join(dirname(__file__), "ovos_tskill_abort", "__init__.py") skill_id = "test_skill.test" module = load_skill_module(test_path, skill_id) @@ -70,7 +70,7 @@ def test_get_skill_class(self): from ovos_workshop.skill_launcher import get_skill_class, \ load_skill_module from ovos_workshop.skills.mycroft_skill import _SkillMetaclass - test_path = join(dirname(__file__), "skills", "test_skill", + test_path = join(dirname(__file__), "ovos_tskill_abort", "__init__.py") skill_id = "test_skill.test" module = load_skill_module(test_path, skill_id) @@ -85,7 +85,7 @@ def test_get_skill_class(self): def test_get_create_skill_function(self): from ovos_workshop.skill_launcher import get_create_skill_function, \ load_skill_module - test_path = join(dirname(__file__), "skills", "test_skill", + test_path = join(dirname(__file__), "ovos_tskill_abort", "__init__.py") skill_id = "test_skill.test" module = load_skill_module(test_path, skill_id)