search

MycroftAI / skill-weather

Mycroft AI official Weather Skill, providing weather conditions and forecasts.
https://mycroft.ai/skills
Apache License 2.0
19 stars 59 forks source link

Fix handling of tonight #109

Closed forslund closed 4 years ago

forslund commented 4 years ago

Lingua-franca has improved the datetime parsing for tonight resulting in tonight resolving to 22:00. This adds a check for the forecast to only trigger on dates not today instead of times not equal to midnight today.

This seems to be working ok (and all tests passes) but I'm not 100 % sure of the ramifications...

krisgesling commented 4 years ago

I think we also need to remove lines 1275-1279.

BJ added a to_UTC to all calls of extract_time however I'm not sure it consistently gets switched back to local for reporting. I'm not confident on how it's handling time really. Want to minimize the need to switch timezones at all.

But I think removing these specific lines should do it for now.

forslund commented 4 years ago

Those lines does indeed look weird...but surely 1275 should be needed?

forslund commented 4 years ago

I couldn't get things to work without that line (or something similar). I'm pushing a change to it to make it look more reasonable to me. Let me know if I'm making some stupid mistake here...

If it's good, I can rebase and squash the commits.

krisgesling commented 4 years ago

Sorry I should have linked to the code, line numbers are always changing

I meant this manual editing of the time if 'tonight' is in the utterance

forslund commented 4 years ago

Ah I see now. I've removed that now as well.

krisgesling commented 4 years ago

Yep looks great, merging.