[FR] overhaul of the weather intent, with detailed report #2704
Conversation
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
|
My bad, I was so excited I forgot the tests. 😊 Working on it as we speak. |
|
Tests (force) pushed. I'm open about adjusting the sentences. |
| - sentences: | ||
| - "(<quelest>|<donnemoi>) la vitesse du vent [<aujourdhui>]" | ||
| - "(À|A) <quel> vitesse le vent souffle [<aujourdhui>]" | ||
| - "(<quelest>|<donnemoi>) la météo du vent [<aujourdhui>]" | ||
| - "(<quelest>|<donnemoi>) le détail du vent [<aujourdhui>]" | ||
| response: wind | ||
| - sentences: | ||
| - "(<quelest>|<donnemoi>) la vitesse du vent [<aujourdhui>] à [<le>]{name}" | ||
| - "(À|A) <quel> vitesse le vent souffle [<aujourdhui>] à [<le>]{name}" | ||
| - "(<quelest>|<donnemoi>) la météo du vent [<aujourdhui>] (pour|à) [<le>]{name}" | ||
| - "(<quelest>|<donnemoi>) le détail du vent (pour|à) [<le>]{name}" | ||
| requires_context: | ||
| domain: weather | ||
| response: wind |
There was a problem hiding this comment.
We have a specific device class under the sensor domain called wind_speed.
I do not think it's a problem we are removing most (if not all) sentences targeting specific device classes under the sensor domain.
@piitaya thoughts?
There was a problem hiding this comment.
If we only target by name, it shouldn't be a issue, right?
|
Awesome work ❤️ Review done, some comments added |
|
I find myself duplicating too much code, and it's getting worse. But it doesn't seem possible to have macros shared by all of the response templates. |
|
And forgot to ask, is there a way to tell if a response is meant to be read or spoken? I'm influenced by the fact my work is destined to my voice assistant. Nothing wrong about displaying "km/h" on the dashboard. |
|
The more I test it, the more I doubt my own choices. Like not trying to describe the wind, but just say "Vent de force 3", and you can ask details about the wind speed. The same with atmospheric pressure, that may be too much information. Its value is in the trend up or down to predict the weather. I could move it to another intent, to begin with. |
|
Updated with the changes I thought about overnight, like just saying the wind force in the detailed report. Instead I moved the air pressure in a separate intent. I was thinking one way to reduce clutter was having the wind attribute itself provide the Beaufort value. Is that possible through calling the weather integration itself, or some Jinja filter? It looks like the Beaufort scale is used worldwide to justify its integration into HA itself. I also allowed myself to use smaller sentences like "bulletin météo" or "météo du vent" which are convenient with the text assistant. Again, I'm open to suggestions on sentences and responses, I'd like something to please everyone, or as many people as possible. |
|
Updated and rebased with "il y a du vent aujourd'hui". Also simplified the detailed report to just mention wind gusts if any. |
|
Heavily simplified Beaufort scale computation, now I found about the conversion API in HA core, and borrowed their formula (and rebased). Edit: ceil rounding is important 😊 |
bors-ltd
force-pushed
the
fr_weather
branch
2 times, most recently
from
4eab281 to
cdda814
Compare
|
Thanks, I'll also rebase my branch, I see lots of work have happened in the French section, and hit that "Ready for review" button for a final merge. I was also thinking about posting on some forum to give people they keys to set up a weather report of their liking. And I'm still around to maintain and evolve this weather intent. |
bors-ltd
force-pushed
the
fr_weather
branch
2 times, most recently
from
7e45761 to
c4b38b5
Compare
The current one would never give the weather condition, as the state was never "clear" or "sunny", but in French already. The wind force is using the Beaufort scale. Try to keep the detailed report both informative and concise. Heavily inspired by the Hungarian template, kudos to RViktor!
The current one would never give the weather condition, as the state was never "clear" or "sunny", but in French already.
The wind description is using the Beaufort scale.
Heavily inspired by the Hungarian template, kudos to RViktor!
I'd like to push it even further, with sentences like "va-t-il pleuvoir/neiger/geler ?", but it requires the Météo-France integration, which not everyone has. I stick to the built-in Met.no integration. Unless there is a way to ship intents along with the Météo-France integration itself.
A more reasonable next step would be "quelle est la météo pour demain ?".