From b16394443737113678d63c047f67f05d50696eb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=85ke=20Forslund?= Date: Fri, 5 Oct 2018 14:50:41 +0200 Subject: [PATCH 1/5] Move trace formatting to separate function The new simple_trace() function can now generate a simplified stack trace instead of doing it inline in the MycroftSkill class --- mycroft/skills/core.py | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/mycroft/skills/core.py b/mycroft/skills/core.py index 4266eb2754b3..a4188221aeb6 100644 --- a/mycroft/skills/core.py +++ b/mycroft/skills/core.py @@ -48,6 +48,15 @@ MainModule = '__init__' +def simple_trace(stack_trace): + stack_trace = stack_trace[:-1] + tb = "Traceback:\n" + for line in stack_trace: + if line.strip(): + tb += line + return tb + + def dig_for_message(): """ Dig Through the stack for message. @@ -238,13 +247,9 @@ def enclosure(self): if self._enclosure: return self._enclosure else: - LOG.error("ERROR: Skill not fully initialized. Move code from " + - " __init__() to initialize() to correct this.") - tb = "Traceback:\n" - for line in traceback.format_stack()[:-1]: - if line.strip(): - tb += line - LOG.error(tb) + LOG.error("Skill not fully initialized. Move code " + + "from __init__() to initialize() to correct this.") + LOG.error(simple_trace(traceback.format_stack())) raise Exception("Accessed MycroftSkill.enclosure in __init__") @property @@ -252,13 +257,9 @@ def bus(self): if self._bus: return self._bus else: - LOG.error("ERROR: Skill not fully initialized. Move code from " + - " __init__() to initialize() to correct this.") - tb = "Traceback:\n" - for line in traceback.format_stack()[:-1]: - if line.strip(): - tb += line - LOG.error(tb) + LOG.error("Skill not fully initialized. Move code " + + "from __init__() to initialize() to correct this.") + LOG.error(simple_trace(traceback.format_stack())) raise Exception("Accessed MycroftSkill.bus in __init__") @property From edaa2cb0af677d97a6d0d1637d4b4b7094d13db3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=85ke=20Forslund?= Date: Fri, 5 Oct 2018 14:57:41 +0200 Subject: [PATCH 2/5] Make load_skill slightly more compact --- 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 a4188221aeb6..bbed7b73ad80 100644 --- a/mycroft/skills/core.py +++ b/mycroft/skills/core.py @@ -114,19 +114,16 @@ def load_skill(skill_descriptor, bus, skill_id, BLACKLISTED_SKILLS=None): BLACKLISTED_SKILLS = BLACKLISTED_SKILLS or [] path = skill_descriptor["path"] name = basename(path) - LOG.info("ATTEMPTING TO LOAD SKILL: {} with ID {}".format( - name, skill_id - )) + LOG.info("ATTEMPTING TO LOAD SKILL: {} with ID {}".format(name, skill_id)) if name in BLACKLISTED_SKILLS: LOG.info("SKILL IS BLACKLISTED " + name) return None main_file = join(path, MainModule + '.py') try: with open(main_file, 'rb') as fp: - skill_module = imp.load_module( - name.replace('.', '_'), fp, main_file, - ('.py', 'rb', imp.PY_SOURCE) - ) + skill_module = imp.load_module(name.replace('.', '_'), fp, + main_file, ('.py', 'rb', + imp.PY_SOURCE)) if (hasattr(skill_module, 'create_skill') and callable(skill_module.create_skill)): # v2 skills framework From bd76379e5c4185aaac3edf155dda6e648c7f3f36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=85ke=20Forslund?= Date: Fri, 5 Oct 2018 14:58:41 +0200 Subject: [PATCH 3/5] Improve readability of get_handler_name() --- mycroft/skills/core.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/mycroft/skills/core.py b/mycroft/skills/core.py index bbed7b73ad80..c0122cf8bdcc 100644 --- a/mycroft/skills/core.py +++ b/mycroft/skills/core.py @@ -173,11 +173,10 @@ def get_handler_name(handler): Returns: string: handler name as string """ - name = '' if '__self__' in dir(handler) and 'name' in dir(handler.__self__): - name += handler.__self__.name + '.' - name += handler.__name__ - return name + return handler.__self__.name + '.' + handler.__name__ + else: + return handler.__name__ def intent_handler(intent_parser): From c06a4711a5b0e2fa663becfbd99b6ebf39533784 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=85ke=20Forslund?= Date: Sat, 6 Oct 2018 09:55:39 +0200 Subject: [PATCH 4/5] Minor restructurings The most notable things: - Use the new wait flag when calling speak - Fix except without defined exception --- mycroft/skills/core.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/mycroft/skills/core.py b/mycroft/skills/core.py index c0122cf8bdcc..d07ced1ca716 100644 --- a/mycroft/skills/core.py +++ b/mycroft/skills/core.py @@ -430,8 +430,7 @@ def validator_default(utterance): validator = validator or validator_default on_fail_fn = on_fail if callable(on_fail) else on_fail_default - self.speak(get_announcement(), expect_response=True) - wait_while_speaking() + self.speak(get_announcement(), expect_response=True, wait=True) num_fails = 0 while True: response = self.__get_response() @@ -472,11 +471,10 @@ def ask_yesno(self, prompt, data=None): if self.voc_match(resp, 'yes'): return 'yes' - - if self.voc_match(resp, 'no'): + elif self.voc_match(resp, 'no'): return 'no' - - return resp + else: + return resp def voc_match(self, utt, voc_filename, lang=None): """ Determine if the given utterance contains the vocabulary provided @@ -1126,7 +1124,7 @@ def default_shutdown(self): Message("detach_skill", {"skill_id": str(self.skill_id) + ":"})) try: self.stop() - except: # noqa + except Exception: LOG.error("Failed to stop skill: {}".format(self.name), exc_info=True) From cce610b57e850cf58528667bb7c9ec0c09d800e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=85ke=20Forslund?= Date: Sat, 6 Oct 2018 09:58:29 +0200 Subject: [PATCH 5/5] use nonlocal in get_scheduled_event_status() --- mycroft/skills/core.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/mycroft/skills/core.py b/mycroft/skills/core.py index d07ced1ca716..d7f6a5e903e5 100644 --- a/mycroft/skills/core.py +++ b/mycroft/skills/core.py @@ -1255,27 +1255,29 @@ def get_scheduled_event_status(self, name): data = {'name': event_name} # making event_status an object so it's refrence can be changed - event_status = [None] - finished_callback = [False] + event_status = None + finished_callback = False def callback(message): + nonlocal event_status + nonlocal finished_callback if message.data is not None: event_time = int(message.data[0][0]) current_time = int(time.time()) time_left_in_seconds = event_time - current_time - event_status[0] = time_left_in_seconds - finished_callback[0] = True + event_status = time_left_in_seconds + finished_callback = True emitter_name = 'mycroft.event_status.callback.{}'.format(event_name) self.bus.once(emitter_name, callback) self.bus.emit(Message('mycroft.scheduler.get_event', data=data)) start_wait = time.time() - while finished_callback[0] is False and time.time() - start_wait < 3.0: + while finished_callback is False and time.time() - start_wait < 3.0: time.sleep(0.1) if time.time() - start_wait > 3.0: raise Exception("Event Status Messagebus Timeout") - return event_status[0] + return event_status def cancel_all_repeating_events(self): """ Cancel any repeating events started by the skill. """