fix/mycroft_backwards_compat

ovos_workshop/skills/mycroft_skill.py

@@ -12,12 +12,16 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import inspect
 import shutil
-from os.path import join, exists
+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
 
 
@@ -93,18 +97,12 @@ def __call__(cls, *args, **kwargs):
                     skill_id = a
 
             if not skill_id:
-                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)
-
+                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
-                # TODO - uncomment once above is deprecated
-                # skill_id = dirname(inspect.getfile(cls)).split("/")[-1]
-                # LOG.warning(f"missing skill_id, assuming folder name "
-                #             f"convention: {skill_id}")
+                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,
@@ -169,6 +167,56 @@ def __init__(self, name: str = None, bus: MessageBusClient = None,
         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
@@ -206,7 +254,7 @@ def _init_settings(self):
 
     # patched due to functional (internal) differences under mycroft-core
     def __on_end_classic(self, message: Message, handler_info: str,
-                         skill_data: dict, is_intent=False):
+                         skill_data: dict, is_intent: bool = False):
         # mycroft-core style settings
         if self.settings != self._initial_settings:
             try:
@@ -222,11 +270,11 @@ def __on_end_classic(self, message: Message, handler_info: str,
 
     @backwards_compat(classic_core=__on_end_classic)
     def _on_event_end(self, message: Message, handler_info: str,
-                      skill_data: dict, is_intent=False):
+                      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)
+        return super()._on_event_end(message, handler_info, skill_data, is_intent)
 
     # refactored - backwards compat + log warnings
     @property

ovos_workshop/skills/ovos.py

@@ -428,7 +428,8 @@ def settings(self) -> JsonStorage:
                              '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__}")
+                             f"to skill class {self.__class__.__name__} "
+                             "You can only use self.settings after the call to 'super()'")
             self.log.error(simple_trace(traceback.format_stack()))
             return self._initial_settings
 
@@ -437,6 +438,7 @@ def settings(self, val: dict):
         """
         Update settings specific to this skill
         """
+        LOG.warning("Skills are not supposed to override self.settings, expect breakage! Set individual dict keys instead")
         assert isinstance(val, dict)
         # init method
         if self._settings is None:
@@ -458,9 +460,10 @@ def enclosure(self) -> EnclosureAPI:
             self.log.warning('Skill not fully initialized.'
                              f"to correct this add kwargs "
                              f"__init__(bus=None, skill_id='') "
-                             f"to skill class {self.__class__.__name__}")
+                             f"to skill class {self.__class__.__name__}." 
+                             "You can only use self.enclosure after the call to 'super()'")
             self.log.error(simple_trace(traceback.format_stack()))
-            raise Exception('Accessed MycroftSkill.enclosure in __init__')
+            raise Exception('Accessed OVOSSkill.enclosure in __init__')
 
     @property
     def file_system(self) -> FileSystemAccess:
@@ -474,9 +477,10 @@ def file_system(self) -> FileSystemAccess:
         else:
             self.log.warning('Skill not fully initialized.'
                              f"to correct this add kwargs __init__(bus=None, skill_id='') "
-                             f"to skill class {self.__class__.__name__}")
+                             f"to skill class {self.__class__.__name__} " 
+                             "You can only use self.file_system after the call to 'super()'")
             self.log.error(simple_trace(traceback.format_stack()))
-            raise Exception('Accessed MycroftSkill.file_system in __init__')
+            raise Exception('Accessed OVOSSkill.file_system in __init__')
 
     @file_system.setter
     def file_system(self, fs: FileSystemAccess):
@@ -486,8 +490,7 @@ def file_system(self, fs: FileSystemAccess):
         system directory.
         @param fs: new FileSystemAccess object to use
         """
-        self.log.warning(f"Skill manually overriding file_system path to: "
-                         f"{fs.path}")
+        LOG.warning(f"Skill manually overriding file_system path to: {fs.path}")
         self._file_system = fs
 
     @property
@@ -501,9 +504,10 @@ def bus(self) -> MessageBusClient:
             self.log.warning('Skill not fully initialized.'
                              f"to correct this add kwargs "
                              f"__init__(bus=None, skill_id='') "
-                             f"to skill class {self.__class__.__name__}")
+                             f"to skill class {self.__class__.__name__} "
+                             "You can only use self.bus after the call to 'super()'")
             self.log.error(simple_trace(traceback.format_stack()))
-            raise Exception('Accessed MycroftSkill.bus in __init__')
+            raise Exception('Accessed OVOSSkill.bus in __init__')
 
     @bus.setter
     def bus(self, value: MessageBusClient):

test/unittests/test_skill.py

@@ -178,7 +178,7 @@ def test_legacy(self):
         # a legacy skill not accepting args at all
         with self.assertRaises(Exception) as ctxt:
             BadLegacySkill()  # accesses self.bus in __init__
-        self.assertTrue("Accessed MycroftSkill.bus in __init__" in str(ctxt.exception))
+        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)