MycroftAI / skill-homeassistant

Mycroft Skill/Integration for Homeassistant
GNU Lesser General Public License v3.0
114 stars 62 forks source link

Add warning when there isn't a url #5

Closed LinusSkucas closed 5 years ago

LinusSkucas commented 5 years ago

Description:

Fixes the issue where the skill fails to report back the error (ironically) because a url is not set.

Checklist:

krisgesling commented 5 years ago

Hey Linus, thanks again for jumping on this so quickly.

It looks like e.request doesn't exist so this still fails when trying to evaluate e.request.url

How about:

if e.request is None or e.request.url is None:
    # There is no url configured
    self.speak_dialog('homeassistant.error.needurl')
else:
    self.speak_dialog('homeassistant.error.invalidurl', data={
    'url': e.request.url})

Also worth noting that with acronyms like URL, Mimic2 has had some great tweaks recently to improve how these are spoken, so just putting URL is probably better than "u r l" now.

LinusSkucas commented 5 years ago

Okay @krisgesling Should be all good now.

krisgesling commented 5 years ago

Thanks this works for me!

With the "url" dialog, if you have it lowercase it is pronounced like the name "Earl" where as Mimic treats uppercase "URL" as an acronym and pronounces it like "ewe - are - el".

I'll let you guys decide how you prefer that one