From d1a48f68dbcb6def8cdf5a27f519d874133bc5ed Mon Sep 17 00:00:00 2001 From: miro Date: Wed, 18 Sep 2024 21:00:22 +0100 Subject: [PATCH] refactor!:drop mycroft --- ovos_workshop/decorators/compat.py | 44 --- ovos_workshop/decorators/ocp.py | 2 +- ovos_workshop/settings.py | 4 +- ovos_workshop/skill_launcher.py | 14 +- ovos_workshop/skills/__init__.py | 5 - ovos_workshop/skills/base.py | 17 - ovos_workshop/skills/common_play.py | 2 +- ovos_workshop/skills/common_query_skill.py | 16 - ovos_workshop/skills/decorators/__init__.py | 4 - ovos_workshop/skills/decorators/converse.py | 4 - .../skills/decorators/fallback_handler.py | 4 - ovos_workshop/skills/decorators/killable.py | 4 - ovos_workshop/skills/decorators/layers.py | 4 - ovos_workshop/skills/decorators/ocp.py | 4 - ovos_workshop/skills/fallback.py | 245 +------------- ovos_workshop/skills/mycroft_skill.py | 312 ------------------ ovos_workshop/skills/ovos.py | 274 +-------------- test/unittests/skills/test_active.py | 4 +- .../skills/test_auto_translatable.py | 8 +- test/unittests/skills/test_base.py | 18 +- .../skills/test_common_query_skill.py | 4 - test/unittests/skills/test_fallback_skill.py | 13 +- .../skills/test_idle_display_skill.py | 4 +- test/unittests/skills/test_ovos.py | 4 - test/unittests/test_abstract_app.py | 11 - test/unittests/test_imports.py | 2 - test/unittests/test_skill.py | 62 ---- 27 files changed, 44 insertions(+), 1045 deletions(-) delete mode 100644 ovos_workshop/decorators/compat.py delete mode 100644 ovos_workshop/skills/base.py delete mode 100644 ovos_workshop/skills/decorators/__init__.py delete mode 100644 ovos_workshop/skills/decorators/converse.py delete mode 100644 ovos_workshop/skills/decorators/fallback_handler.py delete mode 100644 ovos_workshop/skills/decorators/killable.py delete mode 100644 ovos_workshop/skills/decorators/layers.py delete mode 100644 ovos_workshop/skills/decorators/ocp.py delete mode 100644 ovos_workshop/skills/mycroft_skill.py diff --git a/ovos_workshop/decorators/compat.py b/ovos_workshop/decorators/compat.py deleted file mode 100644 index a9952b91..00000000 --- a/ovos_workshop/decorators/compat.py +++ /dev/null @@ -1,44 +0,0 @@ -from functools import wraps - - -def backwards_compat(classic_core=None, pre_008=None, no_core=None): - """ - Decorator to run a different method if specific ovos-core versions are detected - """ - - def backwards_compat_decorator(func): - is_classic = False - is_old = False - is_standalone = True - try: - from mycroft.version import CORE_VERSION_STR # all classic mycroft and ovos versions - is_classic = True - is_standalone = False - - try: - from ovos_core.version import OVOS_VERSION_MINOR # ovos-core >= 0.0.8 - is_classic = False - except ImportError: - is_old = True - try: - from mycroft.version import OVOS_VERSION_MINOR # ovos-core <= 0.0.7 - is_classic = False - except: - is_standalone = True - - except: - is_standalone = True - - @wraps(func) - def func_wrapper(*args, **kwargs): - if is_classic and callable(classic_core): - return classic_core(*args, **kwargs) - if is_old and callable(pre_008): - return pre_008(*args, **kwargs) - if is_standalone and callable(no_core): - return no_core(*args, **kwargs) - return func(*args, **kwargs) - - return func_wrapper - - return backwards_compat_decorator diff --git a/ovos_workshop/decorators/ocp.py b/ovos_workshop/decorators/ocp.py index 45999382..a8e60f05 100644 --- a/ovos_workshop/decorators/ocp.py +++ b/ovos_workshop/decorators/ocp.py @@ -1,4 +1,4 @@ -from ovos_workshop.backwards_compat import MediaType, PlayerState, MediaState, MatchConfidence, \ +from ovos_utils.ocp import MediaType, PlayerState, MediaState, MatchConfidence, \ PlaybackType, PlaybackMode, LoopState, TrackState diff --git a/ovos_workshop/settings.py b/ovos_workshop/settings.py index 52988452..5f3ea82c 100644 --- a/ovos_workshop/settings.py +++ b/ovos_workshop/settings.py @@ -6,6 +6,7 @@ import yaml from json_database import JsonStorageXDG +from ovos_workshop.skills.ovos import OVOSSkill from ovos_backend_client.api import DeviceApi from ovos_backend_client.pairing import is_paired, requires_backend from ovos_backend_client.settings import RemoteSkillSettings, get_display_name @@ -16,9 +17,8 @@ class SkillSettingsManager: def __init__(self, skill): - from ovos_workshop.skills.base import BaseSkill self.download_timer: Optional[Timer] = None - self.skill: BaseSkill = skill + self.skill: OVOSSkill = skill self.api = DeviceApi() self.remote_settings = \ RemoteSkillSettings(self.skill_id, diff --git a/ovos_workshop/skill_launcher.py b/ovos_workshop/skill_launcher.py index e67277b6..0d55fd0d 100644 --- a/ovos_workshop/skill_launcher.py +++ b/ovos_workshop/skill_launcher.py @@ -18,23 +18,19 @@ from ovos_workshop.skills.active import ActiveSkill from ovos_workshop.skills.auto_translatable import UniversalSkill, UniversalFallback -from ovos_workshop.skills.base import BaseSkill from ovos_workshop.skills.common_play import OVOSCommonPlaybackSkill from ovos_workshop.skills.common_query_skill import CommonQuerySkill from ovos_workshop.skills.fallback import FallbackSkill -from ovos_workshop.skills.mycroft_skill import MycroftSkill -from ovos_workshop.skills.ovos import OVOSSkill, OVOSFallbackSkill +from ovos_workshop.skills.ovos import OVOSSkill SKILL_BASE_CLASSES = [ - BaseSkill, MycroftSkill, OVOSSkill, OVOSFallbackSkill, - OVOSCommonPlaybackSkill, OVOSFallbackSkill, CommonQuerySkill, ActiveSkill, + OVOSSkill, OVOSCommonPlaybackSkill, CommonQuerySkill, ActiveSkill, FallbackSkill, UniversalSkill, UniversalFallback ] SKILL_MAIN_MODULE = '__init__.py' - def remove_submodule_refs(module_name: str): """ Ensure submodules are reloaded by removing the refs from sys.modules. @@ -85,13 +81,13 @@ def load_skill_module(path: str, skill_id: str) -> ModuleType: def get_skill_class(skill_module: ModuleType) -> Optional[callable]: """ - Find MycroftSkill based class in skill module. + Find OVOSSkill based class in skill module. Arguments: skill_module (module): module to search for Skill class Returns: - (MycroftSkill): Found subclass of MycroftSkill or None. + (OVOSSkill): Found subclass of OVOSSkill or None. """ if not skill_module: raise ValueError("Expected module and got None") @@ -156,7 +152,7 @@ def __init__(self, bus: MessageBusClient, self._loaded = None self.load_attempted = False self.last_loaded = 0 - self.instance: Optional[BaseSkill] = None + self.instance: Optional[OVOSSkill] = None self.active = True self._watchdog = None self.config = Configuration() diff --git a/ovos_workshop/skills/__init__.py b/ovos_workshop/skills/__init__.py index eaacb75c..e69de29b 100644 --- a/ovos_workshop/skills/__init__.py +++ b/ovos_workshop/skills/__init__.py @@ -1,5 +0,0 @@ -from ovos_workshop.decorators.layers import IntentLayers -from ovos_workshop.skills.ovos import OVOSSkill, OVOSFallbackSkill -from ovos_workshop.skills.idle_display_skill import IdleDisplaySkill -from ovos_workshop.skills.mycroft_skill import MycroftSkill - diff --git a/ovos_workshop/skills/base.py b/ovos_workshop/skills/base.py deleted file mode 100644 index 4a8195c4..00000000 --- a/ovos_workshop/skills/base.py +++ /dev/null @@ -1,17 +0,0 @@ -# DEPRECATED - merged into OVOSSkill, imports for compat onlu -from ovos_workshop.skills.ovos import OVOSSkill, simple_trace, is_classic_core, SkillGUI -from ovos_utils.log import log_deprecation -from ovos_utils.process_utils import RuntimeRequirements - - -# backwards compat alias -class SkillNetworkRequirements(RuntimeRequirements): - def __init__(self, *args, **kwargs): - log_deprecation("Replace with " - "`ovos_utils.process_utils.RuntimeRequirements`", - "0.1.0") - super().__init__(*args, **kwargs) - - -BaseSkill = OVOSSkill # backwards compat - diff --git a/ovos_workshop/skills/common_play.py b/ovos_workshop/skills/common_play.py index c299b41f..0e41a4da 100644 --- a/ovos_workshop/skills/common_play.py +++ b/ovos_workshop/skills/common_play.py @@ -13,7 +13,7 @@ # backwards compat imports, do not delete, skills import from here from ovos_workshop.decorators.ocp import ocp_play, ocp_next, ocp_pause, ocp_resume, ocp_search, \ ocp_previous, ocp_featured_media -from ovos_workshop.backwards_compat import MediaType, MediaState, MatchConfidence, \ +from ovos_utils.ocp import MediaType, MediaState, MatchConfidence, \ PlaybackType, PlaybackMode, PlayerState, LoopState, TrackState, Playlist, PluginStream, MediaEntry diff --git a/ovos_workshop/skills/common_query_skill.py b/ovos_workshop/skills/common_query_skill.py index fb77fc50..f7659d6e 100644 --- a/ovos_workshop/skills/common_query_skill.py +++ b/ovos_workshop/skills/common_query_skill.py @@ -19,7 +19,6 @@ from ovos_utils.file_utils import resolve_resource_file from ovos_utils.log import LOG, log_deprecation -from ovos_workshop.decorators.compat import backwards_compat from ovos_workshop.skills.ovos import OVOSSkill @@ -233,21 +232,6 @@ def __calc_confidence(self, match: str, phrase: str, level: CQSMatchLevel, return confidence - def __handle_query_classic(self, message): - """ - does not perform self.speak, < 0.0.8 this is done by core itself - """ - if message.data["skill_id"] != self.skill_id: - # Not for this skill! - return - self.activate() - phrase = message.data["phrase"] - data = message.data.get("callback_data") or {} - # Invoke derived class to provide playback data - self.CQS_action(phrase, data) - - @backwards_compat(classic_core=__handle_query_classic, - pre_008=__handle_query_classic) def __handle_query_action(self, message: Message): """ If this skill's response was spoken to the user, this method is called. diff --git a/ovos_workshop/skills/decorators/__init__.py b/ovos_workshop/skills/decorators/__init__.py deleted file mode 100644 index 18588273..00000000 --- a/ovos_workshop/skills/decorators/__init__.py +++ /dev/null @@ -1,4 +0,0 @@ -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 deleted file mode 100644 index b75b6224..00000000 --- a/ovos_workshop/skills/decorators/converse.py +++ /dev/null @@ -1,4 +0,0 @@ -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 deleted file mode 100644 index 003e994f..00000000 --- a/ovos_workshop/skills/decorators/fallback_handler.py +++ /dev/null @@ -1,4 +0,0 @@ -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 deleted file mode 100644 index d7de18d7..00000000 --- a/ovos_workshop/skills/decorators/killable.py +++ /dev/null @@ -1,4 +0,0 @@ -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 deleted file mode 100644 index 2497168f..00000000 --- a/ovos_workshop/skills/decorators/layers.py +++ /dev/null @@ -1,4 +0,0 @@ -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 deleted file mode 100644 index 32a563c5..00000000 --- a/ovos_workshop/skills/decorators/ocp.py +++ /dev/null @@ -1,4 +0,0 @@ -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 8001984b..67a7adae 100644 --- a/ovos_workshop/skills/fallback.py +++ b/ovos_workshop/skills/fallback.py @@ -13,7 +13,7 @@ # limitations under the License. import operator -from typing import Optional, List, Callable, Tuple +from typing import Optional, List from ovos_bus_client import MessageBusClient from ovos_bus_client.message import Message, dig_for_message @@ -24,25 +24,10 @@ from ovos_utils.skills import get_non_properties from ovos_workshop.decorators.killable import AbortEvent, killable_event -from ovos_workshop.decorators.compat import backwards_compat -from ovos_workshop.permissions import FallbackMode from ovos_workshop.skills.ovos import OVOSSkill -class _MutableFallback(type(OVOSSkill)): - """ To override isinstance checks we need to use a metaclass """ - - def __instancecheck__(self, instance): - if isinstance(instance, _MetaFB): - return True - return super().__instancecheck__(instance) - - -class _MetaFB(OVOSSkill): - pass - - -class FallbackSkill(_MetaFB, metaclass=_MutableFallback): +class FallbackSkill(OVOSSkill): """ 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 @@ -63,45 +48,8 @@ class FallbackSkill(_MetaFB, metaclass=_MutableFallback): A Fallback can either observe or consume an utterance. A consumed utterance will not be seen by any other Fallback handlers. """ - - def __new__classic__(cls, *args, **kwargs): - if cls is FallbackSkill: - # direct instantiation of class, dynamic wizardry for unittests - # return V2 as expected, V1 will eventually be dropped - return FallbackSkillV2(*args, **kwargs) - cls.__bases__ = (FallbackSkillV1, FallbackSkill, _MetaFB) - return super().__new__(cls) - - @backwards_compat(classic_core=__new__classic__, - pre_008=__new__classic__) - def __new__(cls, *args, **kwargs): - if cls is FallbackSkill: - # direct instantiation of class, dynamic wizardry for unittests - # return V2 as expected, V1 will eventually be dropped - return FallbackSkillV2(*args, **kwargs) - cls.__bases__ = (FallbackSkillV2, FallbackSkill, _MetaFB) - return super().__new__(cls) - - @classmethod - 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): - fallback_handlers = {} - wrapper_map: List[Tuple[callable, callable]] = [] # [(handler, wrapper)] - - def __init__(self, name=None, bus=None, use_settings=True, **kwargs): - # list of fallback handlers registered by this instance - self.instance_fallback_handlers = [] - # "skill_id": priority (int) overrides - self.fallback_config = Configuration()["skills"].get("fallbacks", {}) - - super().__init__(name=name, bus=bus, **kwargs) + # "skill_id": priority (int) overrides + fallback_config = Configuration().get("skills", {}).get("fallbacks", {}) @classmethod def make_intent_failure_handler(cls, bus: MessageBusClient): @@ -158,152 +106,6 @@ def handler(message): return handler - @staticmethod - 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) - except ImportError: - pass - - @classmethod - 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 - - cls.fallback_handlers[priority] = wrapper - cls.wrapper_map.append((handler, wrapper)) - - def register_fallback(self, handler: Callable[[Message], None], - priority: int): - """ - core >= 0.8.0 makes skill active - """ - opmode = self.fallback_config.get("fallback_mode", - FallbackMode.ACCEPT_ALL) - priority_overrides = self.fallback_config.get("fallback_priorities", {}) - fallback_blacklist = self.fallback_config.get("fallback_blacklist", []) - fallback_whitelist = self.fallback_config.get("fallback_whitelist", []) - - if opmode == FallbackMode.BLACKLIST and \ - self.skill_id in fallback_blacklist: - return - if opmode == FallbackMode.WHITELIST and \ - self.skill_id not in fallback_whitelist: - return - - # check if .conf is overriding the priority for this skill - priority = priority_overrides.get(self.skill_id, priority) - - def wrapper(*args, **kwargs): - if handler(*args, **kwargs): - self.activate() - return True - return False - - self.instance_fallback_handlers.append(handler) - self._register_fallback(handler, wrapper, priority) - - @classmethod - 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()): - if handler == wrapper_to_del: - found_handler = True - del cls.fallback_handlers[priority] - - if not found_handler: - LOG.warning('No fallback matching {}'.format(wrapper_to_del)) - return found_handler - - @classmethod - 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((handler_to_del, wrapper_to_del)) - remove_ok = cls._remove_registered_handler(wrapper_to_del) - else: - LOG.warning('Could not find matching fallback handler') - remove_ok = False - return remove_ok - - def remove_instance_handlers(self): - """ - 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. - """ - self.remove_instance_handlers() - super().default_shutdown() - - def _register_decorated(self): - """ - Register all decorated fallback handlers. - - Looks for all functions that have been marked by a decorator - 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): - method = getattr(self, attr_name) - if hasattr(method, 'fallback_priority'): - self.register_fallback(method, method.fallback_priority) - - -class FallbackSkillV2(_MetaFB, metaclass=_MutableFallback): - # "skill_id": priority (int) overrides - fallback_config = Configuration().get("skills", {}).get("fallbacks", {}) - - @classmethod - 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="", **kwargs): self._fallback_handlers = [] super().__init__(bus=bus, skill_id=skill_id, **kwargs) @@ -340,10 +142,9 @@ def _register_system_event_handlers(self): fallback skill. """ super()._register_system_event_handlers() - self.add_event('ovos.skills.fallback.ping', self._handle_fallback_ack, + 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(f"ovos.skills.fallback.{self.skill_id}.request", - self._handle_fallback_request, speak_errors=False) def _handle_fallback_ack(self, message: Message): """ @@ -351,18 +152,16 @@ def _handle_fallback_ack(self, message: Message): """ utts = message.data.get("utterances", []) lang = message.data.get("lang") - self.bus.emit(message.reply( - "ovos.skills.fallback.pong", - data={"skill_id": self.skill_id, - "can_handle": self.can_answer(utts, lang)}, - context={"skill_id": self.skill_id})) + self.bus.emit(message.reply("ovos.skills.fallback.pong", + data={"skill_id": self.skill_id, + "can_handle": self.can_answer(utts, lang)}, + context={"skill_id": self.skill_id})) def _on_timeout(self): """_handle_fallback_request timed out and was forcefully killed by ovos-core""" message = dig_for_message() - self.bus.emit(message.forward( - f"ovos.skills.fallback.{self.skill_id}.killed", - data={"error": "timed out"})) + self.bus.emit(message.forward(f"ovos.skills.fallback.{self.skill_id}.killed", + data={"error": "timed out"})) @killable_event("ovos.skills.fallback.force_timeout", callback=_on_timeout, check_skill_id=True) @@ -404,26 +203,6 @@ def _handle_fallback_request(self, message: Message): self.bus.emit(message.forward("ovos.utterance.handled", {"handler": handler_name})) - def _old_register_fallback(self, handler: callable, priority: int): - """ core < 0.0.8 """ - - LOG.info(f"registering fallback handler -> " - f"ovos.skills.fallback.{self.skill_id}") - - def wrapper(*args, **kwargs): - if handler(*args, **kwargs): - self.activate() - return True - return False - - self._fallback_handlers.append((priority, wrapper)) - self.bus.on(f"ovos.skills.fallback.{self.skill_id}", wrapper) - # register with fallback service - self.bus.emit(Message("ovos.skills.fallback.register", - {"skill_id": self.skill_id, - "priority": self.priority})) - - @backwards_compat(classic_core=_old_register_fallback, pre_008=_old_register_fallback) def register_fallback(self, handler: callable, priority: int): """ Register a fallback handler and add a messagebus handler to call it on diff --git a/ovos_workshop/skills/mycroft_skill.py b/ovos_workshop/skills/mycroft_skill.py deleted file mode 100644 index 17cdb133..00000000 --- a/ovos_workshop/skills/mycroft_skill.py +++ /dev/null @@ -1,312 +0,0 @@ -# Copyright 2019 Mycroft AI Inc. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# 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. - -import inspect -import shutil -from os.path import join, exists, dirname - -from json_database import JsonStorage -from ovos_bus_client import MessageBusClient, Message -from ovos_utils.log import LOG, log_deprecation, deprecated - -from ovos_workshop.decorators.compat import backwards_compat -from ovos_workshop.filesystem import FileSystemAccess -from ovos_workshop.skills.ovos import OVOSSkill, is_classic_core, _OVOSSkillMetaclass - - -class _SkillMetaclass(_OVOSSkillMetaclass): - """ - 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 override isinstance checks we also need to use a metaclass - - TODO: remove compat ovos-core 0.2.0 at the latest, including MycroftSkill class - """ - - def __instancecheck_classic__(self, instance): - # instance imported from vanilla mycroft - from mycroft.skills import MycroftSkill as _CoreSkill - from ovos_workshop.app import OVOSAbstractApplication - if issubclass(instance.__class__, _CoreSkill): - return True - return issubclass(instance.__class__, OVOSSkill) and \ - not issubclass(instance.__class__, OVOSAbstractApplication) - - @backwards_compat(classic_core=__instancecheck_classic__) - def __instancecheck__(self, instance): - from ovos_workshop.app import OVOSAbstractApplication - if issubclass(instance.__class__, OVOSAbstractApplication): - return False - return super().__instancecheck__(instance) or \ - issubclass(instance.__class__, OVOSSkill) - - def __call__(cls, *args, **kwargs): - from ovos_bus_client import MessageBusClient - from ovos_utils.messagebus import FakeBus - bus = None - skill_id = None - - if "bus" not in 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") - 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" - "bus will be required starting on ovos-core 0.1.0") - return super().__call__(*args, **kwargs) - - if "skill_id" in kwargs: - skill_id = kwargs.pop("skill_id") - if "bus" in kwargs: - bus = kwargs.pop("bus") - if not skill_id: - 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 " - f"to be a CamelCase name") - else: - LOG.warning(f"ambiguous skill_id, assuming positional " - f"argument: {a}") - skill_id = a - - if not skill_id: - LOG.error("skill initialized without skill_id!! this is legacy " - "behaviour. skill_id will be required starting on ovos-core 0.1.0") - # by convention skill_id is the folder name - # usually repo.author - skill_id = dirname(inspect.getfile(cls)).split("/")[-1] - LOG.warning(f"missing skill_id, assuming folder name convention: {skill_id}") - - try: - # 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(f"Legacy skill signature detected for {skill_id};" - f" attempting to init skill manually, self.bus and " - f"self.skill_id will only be available in " - f"self.initialize. `__init__` method needs to accept " - f"`skill_id` and `bus` to resolve this.") - - # 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) - return skill - - -class MycroftSkill(OVOSSkill, metaclass=_SkillMetaclass): - """ - 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. - """ - - @deprecated("mycroft-core has been deprecated, please move to ovos-core", "0.1.0") - def __classic_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 = {} - self.settings_write_path = None - self.settings_manager = None - - # old kludge from fallback skills, unused according to grep - if use_settings is False: - log_deprecation("use_settings has been deprecated! " - "skill settings are always enabled", "0.1.0") - - self.settings_write_path = self.root_dir - - @backwards_compat(classic_core=__classic_init__) - @deprecated("MycroftSkill class has been deprecated, please subclass from OVOSSkill", "0.1.0") - 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 = {} - self.settings_write_path = None - self.settings_manager = None - - @property - def file_system(self) -> FileSystemAccess: - """ - Get an object that provides managed access to a local Filesystem. - """ - if not self._file_system: - self._file_system = FileSystemAccess(join('skills', self.name)) - LOG.warning(f"with MycroftSkill self.file_system does not respect self.skill_id, path: {self._file_system.path}") - return self._file_system - - @file_system.setter - def file_system(self, val): - LOG.warning("you are not supposed to override self.file_system, expect breakage!") - self._file_system = val - - @property - def settings(self) -> dict: - """ - Get settings specific to this skill - """ - if self._settings is None: - self._settings = JsonStorage(self.settings_write_path, - disable_lock=True) - LOG.warning(f"with MycroftSkill self.settings may not respect self.skill_id, path: {self._settings.path}") - return self._settings - - @settings.setter - def settings(self, val): - LOG.warning("Skills are not supposed to override self.settings, expect breakage! Set individual dict keys instead") - self._settings = val - - 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}") - - if self._settings is None: - self._settings = JsonStorage(self.settings_write_path, - disable_lock=True) - - # starting on ovos-core 0.0.8 a bus event is emitted - # all settings.json files are monitored for changes in ovos-core - self.add_event("ovos.skills.settings_changed", self._handle_settings_changed) - - if self._monitor_own_settings: - self._start_filewatcher() - - def __init_settings_manager_classic(self): - super()._init_settings_manager() - from mycroft.skills.settings import SettingsMetaUploader - self._settings_meta = SettingsMetaUploader(self.root_dir, - self.skill_id) - - def __init_settings_manager_standalone(self): - super()._init_settings_manager() - - @backwards_compat(classic_core=__init_settings_manager_classic, - no_core=__init_settings_manager_standalone) - def _init_settings_manager(self): - super()._init_settings_manager() - # backwards compat - self.settings_meta has been deprecated - # in favor of settings manager - from mycroft.deprecated.skills.settings import SettingsMetaUploader - self._settings_meta = SettingsMetaUploader(self.root_dir, - self.skill_id) - - def __init_settings_classic(self): - # 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!") - shutil.copy(self._old_settings_path, self.settings_path) - LOG.info(f"{self._old_settings_path} moved to " - f"{self.settings_path}") - super()._init_settings() - - @backwards_compat(classic_core=__init_settings_classic) - def _init_settings(self): - """Setup skill settings.""" - super()._init_settings() - - # patched due to functional (internal) differences under mycroft-core - def __on_end_classic(self, message: Message, handler_info: str, - skill_data: dict, is_intent: bool = False): - # mycroft-core style settings - if self.settings != self._initial_settings: - try: - from mycroft.skills.settings import save_settings - save_settings(self.settings_write_path, self.settings) - self._initial_settings = dict(self.settings) - except Exception as e: - LOG.exception(f"Failed to save skill settings: {e}") - if handler_info: - msg_type = handler_info + '.complete' - message.context["skill_id"] = self.skill_id - self.bus.emit(message.forward(msg_type, skill_data)) - - @backwards_compat(classic_core=__on_end_classic) - def _on_event_end(self, message: Message, handler_info: str, - skill_data: dict, is_intent: bool = False): - """ - Store settings and indicate that the skill handler has completed - """ - return super()._on_event_end(message, handler_info, skill_data, is_intent) - - # refactored - backwards compat + log warnings - @property - def settings_meta(self): - 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_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" - return join(old_dir, old_folder, self.skill_id, 'settings.json') - - # patched due to functional (internal) differences under mycroft-core - def __get_settings_pclassic(self): - 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 - - @property - @backwards_compat(classic_core=__get_settings_pclassic) - def settings_path(self): - return super().settings_path diff --git a/ovos_workshop/skills/ovos.py b/ovos_workshop/skills/ovos.py index 61117452..b6d92d9f 100644 --- a/ovos_workshop/skills/ovos.py +++ b/ovos_workshop/skills/ovos.py @@ -5,7 +5,6 @@ import sys import time import traceback -from abc import ABCMeta from copy import copy from hashlib import md5 from inspect import signature @@ -17,7 +16,6 @@ from json_database import JsonStorage from lingua_franca.format import pronounce_number, join_list from lingua_franca.parse import yes_or_no, extract_number -from ovos_backend_client.api import EmailApi, MetricsApi from ovos_bus_client import MessageBusClient from ovos_bus_client.apis.enclosure import EnclosureAPI from ovos_bus_client.apis.gui import GUIInterface @@ -32,17 +30,15 @@ from ovos_utils.dialog import get_dialog, MustacheDialogRenderer from ovos_utils.events import EventContainer, EventSchedulerInterface from ovos_utils.events import get_handler_name, create_wrapper -from ovos_utils.file_utils import FileWatcher, resolve_resource_file +from ovos_utils.file_utils import FileWatcher from ovos_utils.gui import get_ui_directories from ovos_utils.json_helper import merge_dict -from ovos_utils.log import LOG, log_deprecation, deprecated +from ovos_utils.log import LOG from ovos_utils.parse import match_one from ovos_utils.process_utils import RuntimeRequirements from ovos_utils.skills import get_non_properties -from ovos_utils.sound import play_audio from padacioso import IntentContainer -from ovos_workshop.decorators.compat import backwards_compat from ovos_workshop.decorators.killable import AbortEvent, killable_event, \ AbortQuestion from ovos_workshop.decorators.layers import IntentLayers @@ -55,11 +51,6 @@ from ovos_workshop.settings import SkillSettingsManager -@deprecated("OVOS no longer supports running under classic mycroft-core! this function always returns False", "1.0.0") -def is_classic_core(): - return False - - def simple_trace(stack_trace: List[str]) -> str: """ Generate a simplified traceback. @@ -74,24 +65,7 @@ def simple_trace(stack_trace: List[str]) -> str: return tb -class _OVOSSkillMetaclass(ABCMeta): - """ - To override isinstance checks - """ - - def __instancecheck_classic__(self, instance): - # instance imported from vanilla mycroft - from mycroft.skills import MycroftSkill as _CoreSkill - if issubclass(instance.__class__, _CoreSkill): - return True - return issubclass(instance.__class__, OVOSSkill) - - @backwards_compat(classic_core=__instancecheck_classic__) - def __instancecheck__(self, instance): - return super().__instancecheck__(instance) - - -class OVOSSkill(metaclass=_OVOSSkillMetaclass): +class OVOSSkill: """ Base class for OpenVoiceOS skills providing common behaviour and parameters to all Skill implementations. @@ -838,8 +812,7 @@ def _init_settings(self): # all settings.json files are monitored for changes in ovos-core self.add_event("ovos.skills.settings_changed", self._handle_settings_changed) - if self._monitor_own_settings: - self._start_filewatcher() + self._start_filewatcher() @property def _monitor_own_settings(self): @@ -970,32 +943,6 @@ def _upload_settings(self): # update settingsmeta file on disk self.settings_manager.save_meta() - def __bind_classic(self, bus): - self._bus = bus - self.events.set_bus(bus) - self.intent_service.set_bus(bus) - self.event_scheduler.set_bus(bus) - self._enclosure.set_bus(bus) - self._register_system_event_handlers() - self._register_public_api() - self.intent_layers.bind(self) - self.audio_service = OCPInterface(self.bus) - self.private_settings = PrivateSettings(self.skill_id) - - 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 - try: - from ovos_utils.intents.converse import ConverseTracker - ConverseTracker.connect_bus(self.bus) # pull/1468 - except ImportError: - pass # deprecated in utils 0.1.0 - self.add_event("converse.skill.deactivated", - self._handle_skill_deactivated, - speak_errors=False) - - @backwards_compat(classic_core=__bind_classic) def bind(self, bus: MessageBusClient): """ Register MessageBusClient with skill. @@ -1701,33 +1648,6 @@ def speak_dialog(self, key: str, data: Optional[dict] = None, ) self.speak(key, expect_response, wait, {}) - def _play_audio_old(self, filename: str, instant: bool = False, - wait: Union[bool, int] = False): - """ compat for ovos-core <= 0.0.7 """ - if instant: - LOG.warning("self.play_audio instant flag requires ovos-core >= 0.0.8, " - "falling back to local skill playback") - play_audio(filename).wait() - else: - message = dig_for_message() or Message("") - self.bus.emit(message.forward("mycroft.audio.queue", - {"filename": filename, # TODO - deprecate filename in ovos-audio - "uri": filename # new namespace - })) - if wait: - timeout = 30 if isinstance(wait, bool) else wait - sess = SessionManager.get(message) - sess.is_speaking = True - SessionManager.wait_while_speaking(timeout, sess) - - def _play_audio_classic(self, filename: str, instant: bool = False, - wait: Union[bool, int] = False): - """ compat for classic mycroft-core """ - LOG.warning("self.play_audio requires ovos-core >= 0.0.4, " - "falling back to local skill playback") - play_audio(filename).wait() - - @backwards_compat(pre_008=_play_audio_old, classic_core=_play_audio_classic) def play_audio(self, filename: str, instant: bool = False, wait: Union[bool, int] = False): """ @@ -1765,53 +1685,6 @@ def play_audio(self, filename: str, instant: bool = False, sess.is_speaking = True SessionManager.wait_while_speaking(timeout, sess) - def __get_response_v1(self, session=None): - """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 - being overridden in this method. If an utterance is injected during - this time, the wrong converse method is executed. The condition is - hidden during normal use due to the amount of time it takes a user - to speak a response. The condition is revealed when an automated - process injects an utterance quicker than this method can flip the - converse methods. - - Returns: - str: user's response or None on a timeout - """ - session = session or SessionManager.get() - - def converse(utterances, lang=None): - self.__responses[session.session_id] = utterances - converse.response = utterances[0] if utterances else None - converse.finished = True - return True - - # install a temporary conversation handler - self.activate() - converse.finished = False - converse.response = None - self.converse = converse - - # 10 for listener, 5 for SST, then timeout - ans = [] - # NOTE: a threading.Event is not used otherwise we can't raise the - # AbortEvent exception to kill the thread - # this is for compat with killable_intents decorators - start = time.time() - while time.time() - start <= 15 and not ans: - ans = self.__responses[session.session_id] - time.sleep(0.1) - if ans is None: - # aborted externally (if None) - self.log.debug("get_response aborted") - converse.finished = True - break - - self.converse = self._original_converse - return ans - def __handle_get_response(self, message): """ Handle the response message to a previous get_response / speak call @@ -1828,7 +1701,6 @@ def __handle_get_response(self, message): # received get_response self.__responses[sess2.session_id] = utterances - @backwards_compat(classic_core=__get_response_v1, pre_008=__get_response_v1) def __get_response(self, session: Session): """Helper to get a response from the user @@ -2089,21 +1961,6 @@ def _real_wait_response(self, is_cancel, validator, on_fail, num_retries, else: self.bus.emit(message.reply('mycroft.mic.listen')) - def __acknowledge_classic(self): - """ - 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 - to the user that their request was handled successfully. - """ - audio_file = self.config_core.get('sounds', {}).get('acknowledge', - 'snd/acknowledge.mp3') - audio_file = resolve_resource_file(audio_file) - if audio_file: - return play_audio(audio_file) - - @backwards_compat(classic_core=__acknowledge_classic) def acknowledge(self): """ Acknowledge a successful request. @@ -2554,34 +2411,6 @@ def remove_cross_skill_context(self, context: str): {'context': context})) # killable_events support - def __send_stop_signal_classic(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: - self.bus.emit(msg.forward(stop_event)) - - # stop TTS - self.bus.emit(msg.forward("mycroft.audio.speech.stop")) - - # Tell ovos-core to stop recording (not in mycroft-core) - self.bus.emit(msg.forward('recognizer_loop:record_stop')) - - # 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')) - waiter.wait(1.5) # the silence from muting should make STT stop recording - self.bus.emit(Message('mycroft.mic.unmute')) - - # 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(classic_core=__send_stop_signal_classic) def send_stop_signal(self, stop_event: Optional[str] = None): """ Notify services to stop current execution @@ -2603,34 +2432,6 @@ def send_stop_signal(self, stop_event: Optional[str] = None): waiter.wait(0.5) # if TTS had not yet started self.bus.emit(msg.forward("mycroft.audio.speech.stop")) - # below deprecated and marked for removal - @deprecated("use MetricsApi().report_metric", "0.1.0") - 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 - data (dict): JSON dictionary to report. Must be valid JSON - """ - try: - if Configuration().get('opt_in', False): - MetricsApi().report_metric(name, data) - except Exception as e: - self.log.error(f'Metric couldn\'t be uploaded, due to a network error ({e})') - - @deprecated("use EmailApi().send_email", "0.1.0") - def send_email(self, title: str, body: str): - """ - Send an email to the registered user's email. - - Args: - title (str): Title of email - body (str): HTML body of email. This supports - simple HTML like bold and italics - """ - EmailApi().send_email(title, body, self.skill_id) - @classproperty def network_requirements(self) -> RuntimeRequirements: LOG.warning("network_requirements renamed to runtime_requirements, " @@ -2652,59 +2453,6 @@ def voc_match_cache(self, val): if isinstance(val, dict): self._voc_cache = val - # below only for api compat with MycroftSkill class - @deprecated("Use `self.resources.render_dialog`", "0.1.0") - def translate(self, text: str, data: Optional[dict] = None): - """ - Deprecated method for translating a dialog file. - use self.resources.render_dialog(text, data) instead - """ - return self.resources.render_dialog(text, data) - - @deprecated("Use `self.resources.load_named_value_file`", "0.1.0") - 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 - """ - return self.resources.load_named_value_file(name, delim) - - @deprecated("Use `self.resources.load_list_file`", "0.1.0") - def translate_list(self, list_name: str, data: Optional[dict] = None): - """ - Deprecated method for translating a list. - use self.resources.load_list_file(text, data) instead - """ - return self.resources.load_list_file(list_name, data) - - @deprecated("Use `self.resources.load_template_file`", "0.1.0") - def translate_template(self, template_name: str, - data: Optional[dict] = None): - """ - Deprecated method for translating a template file - use self.resources.template_file(text, data) instead - """ - return self.resources.load_template_file(template_name, data) - - @deprecated("Use `self.resources.load_dialog_files`", "0.1.0") - def init_dialog(self, root_directory: Optional[str] = None): - """ - DEPRECATED: use load_dialog_files instead - """ - self.load_dialog_files(root_directory) - - @deprecated("Use `activate`", "0.1.0") - def make_active(self): - """ - 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 - """ - self.activate() - class SkillGUI(GUIInterface): def __init__(self, skill: OVOSSkill): @@ -2719,17 +2467,3 @@ def __init__(self, skill: OVOSSkill): GUIInterface.__init__(self, skill_id=skill_id, bus=bus, config=config, ui_directories=ui_directories) - @property - @deprecated("`skill` should not be referenced directly", "0.1.0") - def skill(self): - return self._skill - - -# 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/test/unittests/skills/test_active.py b/test/unittests/skills/test_active.py index 9811a0f3..0f12dcae 100644 --- a/test/unittests/skills/test_active.py +++ b/test/unittests/skills/test_active.py @@ -3,7 +3,7 @@ from ovos_utils.messagebus import FakeBus from ovos_workshop.skills.active import ActiveSkill -from ovos_workshop.skills.base import BaseSkill +from ovos_workshop.skills.ovos import OVOSSkill class ActiveSkillExample(ActiveSkill): @@ -17,7 +17,7 @@ def make_active(self): class TestActiveSkill(unittest.TestCase): def test_skill(self): skill = ActiveSkillExample() - self.assertIsInstance(skill, BaseSkill) + self.assertIsInstance(skill, OVOSSkill) skill.bind(FakeBus()) skill.active.assert_called_once() self.assertTrue(skill.active) diff --git a/test/unittests/skills/test_auto_translatable.py b/test/unittests/skills/test_auto_translatable.py index 6b0271cc..c1dd9fed 100644 --- a/test/unittests/skills/test_auto_translatable.py +++ b/test/unittests/skills/test_auto_translatable.py @@ -2,7 +2,7 @@ from ovos_workshop.skills.common_query_skill import CommonQuerySkill from ovos_workshop.skills.fallback import FallbackSkill -from ovos_workshop.skills.base import BaseSkill +from ovos_workshop.skills.ovos import OVOSSkill class TestUniversalSkill(unittest.TestCase): @@ -11,7 +11,7 @@ class TestUniversalSkill(unittest.TestCase): def test_00_init(self): self.assertIsInstance(self.test_skill, self.UniversalSkill) - self.assertIsInstance(self.test_skill, BaseSkill) + self.assertIsInstance(self.test_skill, OVOSSkill) # TODO: Test other class methods @@ -22,7 +22,7 @@ class TestUniversalFallbackSkill(unittest.TestCase): def test_00_init(self): self.assertIsInstance(self.test_skill, self.UniversalFallback) - self.assertIsInstance(self.test_skill, BaseSkill) + self.assertIsInstance(self.test_skill, OVOSSkill) self.assertIsInstance(self.test_skill, FallbackSkill) # TODO: Test other class methods @@ -39,7 +39,7 @@ def CQS_match_query_phrase(self, phrase): def test_00_init(self): self.assertIsInstance(self.test_skill, self.UniversalCommonQuerySkill) - self.assertIsInstance(self.test_skill, BaseSkill) + self.assertIsInstance(self.test_skill, OVOSSkill) 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 index 33fbf804..9cbe51c7 100644 --- a/test/unittests/skills/test_base.py +++ b/test/unittests/skills/test_base.py @@ -23,13 +23,13 @@ def test_simple_trace(self): self.assertEqual(simple_trace(trace), "Traceback:\nline_1\n line_2 \n") -class TestBaseSkill(unittest.TestCase): +class TestOVOSSkill(unittest.TestCase): test_config_path = join(dirname(__file__), "temp_config") os.environ["XDG_CONFIG_HOME"] = test_config_path - from ovos_workshop.skills.base import BaseSkill + from ovos_workshop.skills.ovos import OVOSSkill bus = FakeBus() skill_id = "test_base_skill" - skill = BaseSkill(bus=bus, skill_id=skill_id) + skill = OVOSSkill(bus=bus, skill_id=skill_id) @classmethod def tearDownClass(cls) -> None: @@ -161,7 +161,7 @@ def test_init_settings_manager(self): def test_start_filewatcher(self): test_skill_id = "test_settingschanged.skill" - test_skill = self.BaseSkill(bus=self.bus, skill_id=test_skill_id) + test_skill = self.OVOSSkill(bus=self.bus, skill_id=test_skill_id) settings_changed = Event() on_file_change = Mock(side_effect=lambda x: settings_changed.set()) test_skill._handle_settings_file_change = on_file_change @@ -335,8 +335,8 @@ def test_register_intent(self): pass def test_register_intent_file(self): - from ovos_workshop.skills.base import BaseSkill - skill = BaseSkill(bus=self.bus, skill_id=self.skill_id) + from ovos_workshop.skills.ovos import OVOSSkill + skill = OVOSSkill(bus=self.bus, skill_id=self.skill_id) skill._lang_resources = dict() skill.intent_service = Mock() skill.res_dir = join(dirname(__file__), "test_locale") @@ -362,8 +362,8 @@ def test_register_intent_file(self): f"{skill.skill_id}:time.intent", uk_intent_file, "uk-ua") def test_register_entity_file(self): - from ovos_workshop.skills.base import BaseSkill - skill = BaseSkill(bus=self.bus, skill_id=self.skill_id) + from ovos_workshop.skills.ovos import OVOSSkill + skill = OVOSSkill(bus=self.bus, skill_id=self.skill_id) skill._lang_resources = dict() skill.intent_service = Mock() skill.res_dir = join(dirname(__file__), "test_locale") @@ -479,7 +479,7 @@ def test_shutdown(self): def test_default_shutdown(self): test_skill_id = "test_shutdown.skill" - test_skill = self.BaseSkill(bus=self.bus, skill_id=test_skill_id) + test_skill = self.OVOSSkill(bus=self.bus, skill_id=test_skill_id) test_skill.settings["changed"] = True test_skill.stop = Mock() test_skill.shutdown = Mock() diff --git a/test/unittests/skills/test_common_query_skill.py b/test/unittests/skills/test_common_query_skill.py index ef182e86..7c1a822c 100644 --- a/test/unittests/skills/test_common_query_skill.py +++ b/test/unittests/skills/test_common_query_skill.py @@ -1,7 +1,6 @@ from unittest import TestCase from ovos_utils.messagebus import FakeBus -from ovos_workshop.skills.base import BaseSkill from ovos_workshop.skills.common_query_skill import CommonQuerySkill, CQSMatchLevel @@ -18,10 +17,7 @@ class TestCommonQuerySkill(TestCase): def test_class_inheritance(self): from ovos_workshop.skills.ovos import OVOSSkill - from ovos_workshop.skills.mycroft_skill import MycroftSkill - self.assertIsInstance(self.skill, BaseSkill) self.assertIsInstance(self.skill, OVOSSkill) - self.assertIsInstance(self.skill, MycroftSkill) self.assertIsInstance(self.skill, CommonQuerySkill) def test_00_skill_init(self): diff --git a/test/unittests/skills/test_fallback_skill.py b/test/unittests/skills/test_fallback_skill.py index 656ab4da..22dd0f9a 100644 --- a/test/unittests/skills/test_fallback_skill.py +++ b/test/unittests/skills/test_fallback_skill.py @@ -6,7 +6,6 @@ from ovos_utils.messagebus import FakeBus from ovos_bus_client.message import Message from ovos_workshop.decorators import fallback_handler -from ovos_workshop.skills.base import BaseSkill from ovos_workshop.skills.fallback import FallbackSkillV1, FallbackSkillV2, \ FallbackSkill @@ -39,11 +38,8 @@ class TestFallbackSkill(TestCase): 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) @@ -59,11 +55,8 @@ def setup_fallback(fb_class): 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) @@ -175,18 +168,16 @@ class TestFallbackSkillV2(TestCase): 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): + from ovos_workshop.skills.ovos import OVOSSkill self.assertIsInstance(self.fallback_skill, FallbackSkillV2) self.assertIsInstance(self.fallback_skill, FallbackSkill) - self.assertIsInstance(self.fallback_skill, BaseSkill) + self.assertIsInstance(self.fallback_skill, OVOSSkill) def test_priority(self): FallbackSkillV2.fallback_config = {} diff --git a/test/unittests/skills/test_idle_display_skill.py b/test/unittests/skills/test_idle_display_skill.py index a145db78..e187bd6d 100644 --- a/test/unittests/skills/test_idle_display_skill.py +++ b/test/unittests/skills/test_idle_display_skill.py @@ -1,7 +1,7 @@ import unittest from ovos_utils.messagebus import FakeBus -from ovos_workshop.skills.base import BaseSkill +from ovos_workshop.skills.ovos import OVOSSkill from ovos_workshop.skills.idle_display_skill import IdleDisplaySkill @@ -15,6 +15,6 @@ class TestIdleDisplaySkill(unittest.TestCase): skill = TestSkill(bus=FakeBus(), skill_id="test_idle_skill") def test_00_skill_init(self): - self.assertIsInstance(self.skill, BaseSkill) + self.assertIsInstance(self.skill, OVOSSkill) self.assertIsInstance(self.skill, IdleDisplaySkill) # TODO: Implement more tests diff --git a/test/unittests/skills/test_ovos.py b/test/unittests/skills/test_ovos.py index fbdcf8d9..775a3bbd 100644 --- a/test/unittests/skills/test_ovos.py +++ b/test/unittests/skills/test_ovos.py @@ -143,14 +143,10 @@ def test_runtime_requirements(self): 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.app import OVOSAbstractApplication skill = MockSkill() - self.assertIsInstance(skill, BaseSkill) self.assertIsInstance(skill, OVOSSkill) - self.assertIsInstance(skill, MycroftSkill) self.assertNotIsInstance(skill, OVOSAbstractApplication) diff --git a/test/unittests/test_abstract_app.py b/test/unittests/test_abstract_app.py index 2b630f92..8af1ca52 100644 --- a/test/unittests/test_abstract_app.py +++ b/test/unittests/test_abstract_app.py @@ -55,18 +55,11 @@ def test_settings_path(self): # Test settings path conflicts test_app = OVOSAbstractApplication(skill_id="test", bus=self.bus) from ovos_workshop.skills import OVOSSkill - from ovos_workshop.skills.mycroft_skill import MycroftSkill test_skill = OVOSSkill(skill_id="test", bus=self.bus) - mycroft_skill = MycroftSkill(skill_id="test", bus=self.bus) # Test app vs skill base directories self.assertIn("/apps/", test_app.settings_path) self.assertIn("/skills/", test_skill.settings_path) - self.assertEqual(test_skill.settings_path, - mycroft_skill.settings_path) - self.assertEqual(test_skill.settings.path, - mycroft_skill.settings.path) - self.assertEqual(test_skill.settings, mycroft_skill.settings) # Test settings changes test_skill.settings['is_skill'] = True @@ -101,12 +94,8 @@ def test_clear_intents(self): pass 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.assertNotIsInstance(self.app, MycroftSkill) self.assertIsInstance(self.app, OVOSAbstractApplication) diff --git a/test/unittests/test_imports.py b/test/unittests/test_imports.py index 8158af10..52283c09 100644 --- a/test/unittests/test_imports.py +++ b/test/unittests/test_imports.py @@ -7,6 +7,4 @@ class TestImports(unittest.TestCase): """ def test_skills(self): import ovos_workshop.skills - self.assertIsNotNone(ovos_workshop.skills.MycroftSkill) self.assertIsNotNone(ovos_workshop.skills.OVOSSkill) - self.assertIsNotNone(ovos_workshop.skills.OVOSFallbackSkill) diff --git a/test/unittests/test_skill.py b/test/unittests/test_skill.py index 18d9621a..571d038b 100644 --- a/test/unittests/test_skill.py +++ b/test/unittests/test_skill.py @@ -5,42 +5,11 @@ from ovos_bus_client import Message from ovos_workshop.skills.ovos import OVOSSkill -from ovos_workshop.skills.mycroft_skill import MycroftSkill -from ovos_workshop.skills import MycroftSkill as CoreSkill from ovos_utils.messagebus import FakeBus from os.path import dirname from ovos_workshop.skill_launcher import SkillLoader -class LegacySkill(CoreSkill): - def __init__(self, skill_name="LegacySkill", bus=None, **kwargs): - self.inited = True - self.initialized = False - self.startup_called = False - super().__init__(skill_name, bus, **kwargs) - # __new__ calls `_startup` so this should be defined in __init__ - assert self.skill_id is not None - - def initialize(self): - self.initialized = True - - def _startup(self, bus, skill_id=""): - self.startup_called = True - self.initialize() - - -class BadLegacySkill(LegacySkill): - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - print(self.bus) # not set, exception in property - - -class GoodLegacySkill(CoreSkill): - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - print(self.bus) # maybe not set, exception in property - - class SpecificArgsSkill(OVOSSkill): def __init__(self, skill_id="SpecificArgsSkill", bus=None, **kwargs): self.inited = True @@ -91,14 +60,9 @@ def get_msg(msg): def test_skill_id(self): self.assertTrue(isinstance(self.skill.instance, OVOSSkill)) - self.assertTrue(isinstance(self.skill.instance, MycroftSkill)) self.assertEqual(self.skill.skill_id, "abort.test") - # the metaclass ensures this returns True under ovos-core - # but we have no control over mycroft-core so can not patch isinstance checks there - self.assertTrue(isinstance(self.skill.instance, CoreSkill)) - # if running in ovos-core every message will have the skill_id in context for msg in self.bus.emitted_msgs: if msg["type"] == 'mycroft.skills.loaded': # emitted by SkillLoader, not by skill @@ -133,7 +97,6 @@ def test_registered_events(self): for event in default_skill: self.assertTrue(event in registered_events) - # base skill class events exclusive to ovos-core default_ovos = [f"{self.skill.skill_id}.converse.ping", f"{self.skill.skill_id}.converse.request", "intent.service.skills.activated", @@ -162,31 +125,6 @@ def tearDown(self) -> None: class TestSkillNew(unittest.TestCase): - def test_legacy(self): - bus = FakeBus() - - # a legacy skill accepts wrong args, but accepts kwargs - legacy = LegacySkill("LegacyName", bus, skill_id="legacy.mycroft") - self.assertTrue(legacy.inited) - self.assertTrue(legacy.initialized) - self.assertTrue(legacy.startup_called) - self.assertIsNotNone(legacy.skill_id) - self.assertEqual(legacy.bus, bus) - - # a legacy skill not accepting args at all - with self.assertRaises(Exception) as ctxt: - BadLegacySkill() # accesses self.bus in __init__ - self.assertTrue("Accessed OVOSSkill.bus in __init__" in str(ctxt.exception)) - - legacynoargs = LegacySkill() # no exception this time because bus is not used in init - self.assertTrue(legacynoargs.inited) - self.assertFalse(legacynoargs.initialized) - self.assertFalse(legacynoargs.startup_called) - - # a legacy skill fully inited at once - legacy = GoodLegacySkill(skill_id="legacy.mycroft", bus=bus) # accesses self.bus in __init__ - self.assertEqual(legacy.skill_id, "legacy.mycroft") - self.assertEqual(legacy.bus, bus) def test_load(self): bus = FakeBus()