Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FR] Weather State not translated #2899

Open
jlpouffier opened this issue Jan 22, 2025 · 12 comments · May be fixed by #2910
Open

[FR] Weather State not translated #2899

jlpouffier opened this issue Jan 22, 2025 · 12 comments · May be fixed by #2910

Comments

@jlpouffier
Copy link
Contributor

@bors-ltd Hey 👋🏻

I am creating an issue to check if you want to fix this yourself, as you did a really great job with the weather intent in French (Credits where it's due ;) )

The weather state (On both our default and detailed sentences) does not seem to be translated.

Image

Image

A small mapping of all weather states and their corresponding French translation would be worth it.

The available states can be found here:
https://developers.home-assistant.io/docs/core/entity/weather/

Image

Important note:
Feel free to tell me if you do not have the time to fix this.
In any case, I will assign this ticket to myself in a few days and fix it in case you are unavailable.

Have a great day.
JLo

@bors-ltd
Copy link
Contributor

Hi, I can look into this but I haven't met the issue myself, and I ask Nabu the weather daily.

Probably did, but have you checked your voice assistant settings? And you're not running a development version in those examples?

@bors-ltd
Copy link
Contributor

I the meantime, I checked I had no custom intent, and commits to the weather integration or intents that could explain a behaviour change.

I could just as well add this mapping, but I want to understand. The initial mapping not working is what got me into rewriting the weather intent in the first place. In your example, Cloudy is produced by {{ state.state }} in the template, so the fact it's already uppercased tells me your weather component is in English. If it was the cloudy lowercased value, the mapping would make sense, but I don't want to duplicate the translation job.

If your weather improves and you get Partly cloudy instead of partlycloudy, that would be more obvious.

@jlpouffier
Copy link
Contributor Author

@bors-ltd Agreed. LEt me ping @synesthesiam before doing any extra work. Hold tight

@jlpouffier
Copy link
Contributor Author

@bors-ltd Translation of state is hacky today -
I would suggest you to try this way (The DE way)
https://github.com/home-assistant/intents/blob/main/responses/de/HassGetWeather.yaml

Basically creating a mapping and testing it against the "raw" state value.

@jlpouffier
Copy link
Contributor Author

Wait.. no I do not think this work either. state.state is not the raw state it's the translated state ...
God damn it 🙃

ok hold on again 😂

@bors-ltd
Copy link
Contributor

bors-ltd commented Jan 23, 2025

Speaking of translated state, I was thinking about using the state_translated() template function, but I can't test it as I can't reproduce the issue.

But it's more of a bypass than anything.

Edit: here's a example from using a custom file in the configuration directory (and assuming the behaviour is the same than shipped with the intents repo):

Template:

        test state {{ state.state }}
        test state_translated {{ state_translated(state.entity_id) }}

Result:

test state Pluvieux test state_translated Pluvieux et il fait 6,2 degrés.

And I have my own shenanigans with the weather domain, like not exposing the wind gusts anymore.

@bors-ltd
Copy link
Contributor

Another update, I finally took the time to read the developer documentation, and now I have a development environment up and running.

I set up a new HA instance, with the France/French settings, and set up a few forecast entities:

Image

As you can see, the cards are presented in English, although everything else is in French.

And indeed now I could reproduce your bug:

Image

And as I suspected, we get "Partly cloudy", and not the keyword "partlycloudy", and it would be maddening to re-translate the English translation of a keyword, when that job was already done for the French language.

So there's a deeper cause to the weather entities not translating... or a development environment is not 100% representative, and I'm following the wrong track.

If that's a real bug, I guess I'm lucky not to get it (yet?) on my own instance, and I use both the MET and Metéo France integrations.

@bors-ltd
Copy link
Contributor

Here's a funny one, could it be related? While working on my contribution to the holiday integration, I find out it's named "Holiday" when adding it in the UI, while it's named "Vacances" in my production instance. Could just as well be a coincidence or limitation of the dev env.

@jlpouffier
Copy link
Contributor Author

Bottom line

Core is not able to provide good translation because core has only one language. But many pipelines can have many different language in the same system.

So. We're axing the translation (or more like the attempt to translate)

home-assistant/core#136382

So we will be able to properly translate actual state value such as partly-cloudy freely ok our side.

@jlpouffier
Copy link
Contributor Author

I guess we can work on that soon. And make it ready for the next release. (And align in beta if we kissed anything)

I'll test every sentence after the beta cut. I suspect this change may affect other intents

@bors-ltd
Copy link
Contributor

Well, that sorts it out. 🤣

Bummer we have to duplicate the translation work, but I can also close a never exposed branch about accessing the raw condition value.

PR incoming.

bors-ltd added a commit to bors-ltd/intents that referenced this issue Jan 24, 2025
As it goes for every exposed entity of any domain from now on.

This concludes uncertainty about the content of "state.state", and will
have the nice side effect of putting the development environment on par
with the production behaviour.

Fixes home-assistant#2899
@bors-ltd
Copy link
Contributor

PR pushed! #2910

Took me longer than expected because I'm not happy with the code duplication. So I started a branch in core to have reusable templates and macros in intents. But it's taking too long, so I made it quick and dirty to match the release milestone.

I'm open about naming the weather conditions, we have to please everyone, or as many people as possible, in every French-speaking country.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants