skill-weather
skill-weather copied to clipboard
fix condition/.. requests with location information
To reproduce:
issue a fog / sunrise report for a specific location ("is it foggy in New York")
Problems:
-
Several intents (condition/storm/rain/...) have
.optionally("Location")in their constructor while others go with.optionally("location"). This resulting in the location getting ignored. -
If you issue a sunrise request at a specific location
now = now_local(tz=self.intent_data.geolocation["timezone"])throws an exception
Traceback (most recent call last):
File "/home/sweng/mycroft-core/mycroft/skills/mycroft_skill/event_container.py", line 73, in wrapper
handler(message)
File "/opt/mycroft/skills/skill-weather.emphasize/__init__.py", line 565, in handle_sunset
dialog.build_sunset_dialog()
File "/opt/mycroft/skills/skill-weather.emphasize/skill/dialog.py", line 170, in build_sunset_dialog
now = now_local(tz=self.intent_data.geolocation["timezone"])
File "/home/sweng/mycroft-core/mycroft/util/time.py", line 68, in now_local
return datetime.now(tz)
TypeError: tzinfo argument must be None or of a tzinfo subclass, not type 'str'
- Typo in dialog string constructor for sunrise/sunset
self.name += ".sunset.future"(should be"-sunset-future")
What this PR does
- all location adapt intent construction strings are ~~lower case~~ capitalized
- dialog.py imports
get_tz_infofrom skill.util to convert the timezone string - Fixes the sunset/sunrise dialog construction
Type of PR
-
[x] Bugfix
-
[x] CLA
I would think that this should be Location with capital L since the regex match group has the capitalization in the location.rx
There is something strange here since the vocab file for location (location.voc) has small letters so this won't be used in addition to the regex. If so it could maybe be removed?
@forslund Then self.location = message.data.get("location") has to be changed too. #. Was about to change that (and this one worked on my test rig), as i saw that the constructor using both Loc../loc... Did't take regex into account.
Late changes > bad
I wondered about the location.voc though as it would restrict the locations to choose from,
basically leaving location None if not one of those is hit in the intent. I took it as a residue. Overlooking that it now would do exactly that.
Will revert to uppercase in the constructors and message key get.
Hey emphasize, thanks heaps for all the detective work and fixes on these Skills!
I was just doing some testing on a blank Skill and with only a location.rx file, both the capital and lowercase work in the Adapt intent builder. But they should be consistent as the capitalization will definitely be a problem when accessing it on the message.data object.
There are two more hanging around around lines 422 and 433.
The potential conflict of location.rx and location.voc is an interesting one too. So far in my testing the regex takes priority over the vocab.
Given this is @chrisveilleux's refactor I'll let him make the final call on when this is ready.