From 553b95643e3b9c42ce75cefc9f83cd9b8d9814ba Mon Sep 17 00:00:00 2001 From: penrods Date: Tue, 5 Dec 2017 11:18:12 -0600 Subject: [PATCH 1/4] Add support for unnamed intents This allows skill writers to ignore naming intents. Combined with a forthcoming change to Adapt that creates a default of None for IntentBuilder() This allows the current: @intent_handler(IntentBuilder("CurrentWeatherIntent").require( "Weather").optionally("Location").build()) def handle_current_weather(self, message): ... To become: @intent_handler(IntentBuilder().require("Weather").optionally("Location")) def handle_current_weather(self, message): ... Which will automatically name the Intent "handle_current_weather". Also dropped the log message in the default initialize() method since it is common to not override it now. --- mycroft/skills/core.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/mycroft/skills/core.py b/mycroft/skills/core.py index af7325b4d4..2e6a1d9d65 100644 --- a/mycroft/skills/core.py +++ b/mycroft/skills/core.py @@ -295,11 +295,11 @@ class MycroftSkill(object): def initialize(self): """ - Initialization function to be implemented by all Skills. + Initialization function, run after fully constructed Usually used to create intents rules and register them. """ - LOG.debug("No initialize function implemented") + pass def get_intro_message(self): """ @@ -459,6 +459,10 @@ class MycroftSkill(object): elif type(intent_parser) != Intent: raise ValueError('intent_parser is not an Intent') + if not intent_parser.name: + # Default to the handler's function name if None or "" + intent_parser.name = handler.__name__ + name = intent_parser.name intent_parser.name = str(self.skill_id) + ':' + intent_parser.name self.emitter.emit(Message("register_intent", intent_parser.__dict__)) From 0ca157685c80c3319afb8b9646148a9a8786e0e3 Mon Sep 17 00:00:00 2001 From: penrods Date: Tue, 5 Dec 2017 17:57:28 -0600 Subject: [PATCH 2/4] Refined comments and made implementation more Pythonic --- mycroft/skills/core.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/mycroft/skills/core.py b/mycroft/skills/core.py index 2e6a1d9d65..cd756ebfc1 100644 --- a/mycroft/skills/core.py +++ b/mycroft/skills/core.py @@ -295,9 +295,8 @@ class MycroftSkill(object): def initialize(self): """ - Initialization function, run after fully constructed - - Usually used to create intents rules and register them. + Invoked after the skill is fully constructed and registered with the + system. Use to perform any final setup needed for the skill. """ pass @@ -459,11 +458,9 @@ class MycroftSkill(object): elif type(intent_parser) != Intent: raise ValueError('intent_parser is not an Intent') - if not intent_parser.name: - # Default to the handler's function name if None or "" - intent_parser.name = handler.__name__ + # Default to the handler's function name if none given + name = intent_parser.name or handler.__name__ - name = intent_parser.name intent_parser.name = str(self.skill_id) + ':' + intent_parser.name self.emitter.emit(Message("register_intent", intent_parser.__dict__)) self.registered_intents.append((name, intent_parser)) From 49062bfa59567624ec5ac5ad1bf5d40063b69c6b Mon Sep 17 00:00:00 2001 From: penrods Date: Wed, 6 Dec 2017 03:55:54 -0600 Subject: [PATCH 3/4] Fix based on review, improve error handling Corrected my refinement after a previous review. Also added support for splitting the name of a skill before running it through TTS, making "VolumeSkill" sound like "Volume Skill" and such. Plus a log message before raising some errors in the skill wrapper. --- mycroft/skills/core.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/mycroft/skills/core.py b/mycroft/skills/core.py index cd756ebfc1..f4030068c0 100644 --- a/mycroft/skills/core.py +++ b/mycroft/skills/core.py @@ -400,6 +400,8 @@ class MycroftSkill(object): except TypeError: handler(self) else: + LOG.error("Unexpected argument count:" + + str(len(getargspec(handler).args))) raise TypeError else: if len(getargspec(handler).args) == 2: @@ -407,13 +409,17 @@ class MycroftSkill(object): elif len(getargspec(handler).args) == 1: handler() else: + LOG.error("Unexpected argument count:" + + str(len(getargspec(handler).args))) raise TypeError self.settings.store() # Store settings if they've changed except Exception as e: + # Convert "MyFancySkill" to "My Fancy Skill" for speaking + name = re.sub("([a-z])([A-Z])","\g<1> \g<2>", self.name) # TODO: Localize self.speak( "An error occurred while processing a request in " + - self.name) + name) LOG.error( "An error occurred while processing a request in " + self.name, exc_info=True) @@ -460,8 +466,7 @@ class MycroftSkill(object): # Default to the handler's function name if none given name = intent_parser.name or handler.__name__ - - intent_parser.name = str(self.skill_id) + ':' + intent_parser.name + intent_parser.name = str(self.skill_id) + ':' + name self.emitter.emit(Message("register_intent", intent_parser.__dict__)) self.registered_intents.append((name, intent_parser)) self.add_event(intent_parser.name, handler, need_self) From 4555e3f6979046a46a9853efa9a54f52bb3d4358 Mon Sep 17 00:00:00 2001 From: penrods Date: Wed, 6 Dec 2017 03:57:56 -0600 Subject: [PATCH 4/4] And a PEP8 error... --- mycroft/skills/core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mycroft/skills/core.py b/mycroft/skills/core.py index f4030068c0..832226ccdb 100644 --- a/mycroft/skills/core.py +++ b/mycroft/skills/core.py @@ -415,7 +415,7 @@ class MycroftSkill(object): self.settings.store() # Store settings if they've changed except Exception as e: # Convert "MyFancySkill" to "My Fancy Skill" for speaking - name = re.sub("([a-z])([A-Z])","\g<1> \g<2>", self.name) + name = re.sub("([a-z])([A-Z])", "\g<1> \g<2>", self.name) # TODO: Localize self.speak( "An error occurred while processing a request in " +