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

Pass entity untranslated state to sentences #1848

Closed
wants to merge 12 commits into from
Closed

Conversation

tetele
Copy link
Contributor

@tetele tetele commented Jan 3, 2024

The current implementation of HassGetWeather is that the weather entity state is passed to the response generator in order for it to add a translated weather condition. However, states are translated automatically when added to responses.

As such, original (i.e. untranslated) weather conditions or person state (home, not_home) are lost in translation. We need to have the original state, which is addressed in home-assistant/core#105524 but we also need to fix the sentences to match, which is addressed in this PR.

Fixes #1845

@andrejs2
Copy link
Contributor

andrejs2 commented Jan 3, 2024

@tetele I've fixed the states in cf6b3ca since I couldn't resolve the conflicts here for rulanguage. Can you check cf6b3ca?
Thanks!

@tetele
Copy link
Contributor Author

tetele commented Jan 3, 2024

@andrejs2 you're not supposed to fix conflicts for other languages. But regarding Slovenian, maybe you can rebase #1849 on https://github.com/home-assistant/intents/tree/untranslated-state instead of the main branch.

@andrejs2
Copy link
Contributor

andrejs2 commented Jan 3, 2024

@andrejs2 you're not supposed to fix conflicts for other languages. But regarding Slovenian, maybe you can rebase #1849 on https://github.com/home-assistant/intents/tree/untranslated-state instead of the main branch.

Yes, I'm aware, maybe I just didn't put it the correct form in my reply. I checked only sl.

Thanks, I'll rebase. I hope I don't screw up like the last time.

@TheFes
Copy link
Contributor

TheFes commented Jan 3, 2024

Hmm, intents are already included in 2024.1.0b6 release now. So I guess this will not make the 2024.1 cut?

@tetele
Copy link
Contributor Author

tetele commented Jan 3, 2024

It won't make any release until home-assistant/core#105524 is merged.

@TheFes
Copy link
Contributor

TheFes commented Jan 3, 2024

Ah, we're also still waiting for that (I'm on mobile, so navigating on GitHub is a bit of a hassle)

@HepoH3
Copy link
Member

HepoH3 commented Jan 3, 2024

@andrejs2 you're not supposed to fix conflicts for other languages. But regarding Slovenian, maybe you can rebase #1849 on https://github.com/home-assistant/intents/tree/untranslated-state instead of the main branch.

RU lang has conflicts, because they really are. Main branch has new changes placed close by changes made in this one. I can fix them manually, but as far as I understand, I should not do this until the merge itself. The only way to uncoflict this branch without manual resolving is to rebase this branch onto the main, but it will introduce a lot more hassle in here.

@tetele
Copy link
Contributor Author

tetele commented Jan 3, 2024

I can fix them manually, but as far as I understand, I should not do this until the merge itself.

Actually, you should do that to allow the merge to happen @HepoH3. Please merge the changes from main here when you have the time.

@HepoH3
Copy link
Member

HepoH3 commented Jan 3, 2024

I can fix them manually, but as far as I understand, I should not do this until the merge itself.

Actually, you should do that to allow the merge to happen @HepoH3. Please merge the changes from main here when you have the time.

I see. Sure, I can do this in a 24h.

@HepoH3
Copy link
Member

HepoH3 commented Jan 3, 2024

@tetele, sorry to disturb, but I'm not sure that I've got you correctly.

Even if I cherry-pick the conflicting commit (or even squash and merge entire main branch) with following conflict resolve in this branch, it will not remove conflict between this branch and main branch, because both branches will have different changes of the same file.

The only way I know is to either rebase this branch on top of main branch or merge main branch into this branch with the following conflict resolve. But common merge ability (not squash and merge) is disabled for this repository, so we have no way to make conflicting commit with the same SHA appear in this branch.

I believe #1851 has the same root of problem.

Maybe I miss something, will be glad if you guide me through this.

@tetele
Copy link
Contributor Author

tetele commented Jan 3, 2024

@HepoH3 ugh, you're right. Then just leave it as it is and me and/or @synesthesiam will try to fix the merge when the time comes.

@HepoH3
Copy link
Member

HepoH3 commented Jan 3, 2024

@HepoH3 ugh, you're right. Then just leave it as it is and me and/or @synesthesiam will try to fix the merge when the time comes.

I can tell how it should be resolved:
responses/ru/HassGetState.yaml should contain both changes:

      where: |
        {{ slots.name | capitalize }} находится в зоне {{ state.state }}

      sensor_value: |
        Текущее значение: {{ state.state_with_unit }}

      sensor_value_temperature: |
        Температура {{ state.state_with_unit }}

While sentences/ru/_common.yaml should contain this combination of changes:

  turn_on: "(запусти|включи)[ть]"
  turn_off: "(выключи|отключи)[ть]"
  set: "(сдела(й|ть)|установи[ть]|постав(ь|ить)|поменя(й|ть)|измени[ть]|выстав(ь|ить)|переключи[ть]|переве(сти|ди)|увелич(ь|ить)|уменьш(и|ить)|подн(ими|ять)|опусти[ть]|прибав(ь|ить)|отн(ими|ять)|повыс(ь|ить)|пониз(ь|ить))"
  close: "(закр(ой|ыть)|прикр(ой|ыть)|опусти[ть])"
  open: "(откр(ой|ыть)|подн(ими|ять))"
  what_is: "какая|како(е|й)|как(ую|ие)|каков[о|a|ы]|[на]сколько|когда|как"
  what_is_the_class_of_name: "<what_is> [показани(е|я)|значени(е|я)|уровен(и|ь)|концентраци(я|ю)] <class> [показывает[ся]|отображает[ся]] [у|на|в] [датчик(а|е)|сенсор(а|е)] <name> [<area>]"
  where_is: "где[ сейчас][ находится]"

… state (#1886)

* [HU - untranslated state] Hungarian version prepared for untranslated state

Hungarian version prepared for untranslated state

* [HU - untranslated state] v2
@frenck
Copy link
Member

frenck commented Oct 26, 2024

Is there anything we can do with this PR to get this pull request in an end-state?

There is quite a list of merge conflicts, and the linked upstream Core PR have been closed. Is this PR still viable?

../Frenck

@tetele
Copy link
Contributor Author

tetele commented Oct 26, 2024

I don't know. The approach was rejected by the HA team at the time according to @synesthesiam, who said he will be looking at alternatives. 9-10 months have passed since 🤷🏻‍♂️

This PR is probably no longer viable, but i would like to thank each and every language leader who have contributed very quickly to what i believed at the time to be a working solution.

@tetele tetele closed this Oct 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DE] getweather is missing weather condition