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

[DE] Add timer length and name to responses #2513

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

andreasbrett
Copy link
Contributor

@andreasbrett andreasbrett commented Nov 12, 2024

DE version of #2494 and #2521

As Missy mentioned on GitHub, currently the timer length is not included in the response. This can lead incorrectly set timers of which the user will be unware.

This PR adds the timer length and timer name to the response, so the user is aware of what has been set. The set name can be used to ask about the status of the timer.

also adds the duration added or removed from a timer when the respective intents are used

Copy link
Contributor

@mib1185 mib1185 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @andreasbrett
many thanks for adopting this ❤️
But it would be nice, if we could have further test sentences to cover these singular and plural variations (Stunde vs Stunden, same for minutes and seconds) and these ['halbe', '1/2'] variations.
thx 👍

@home-assistant home-assistant bot marked this pull request as draft November 14, 2024 20:22
@home-assistant
Copy link

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

@andreasbrett
Copy link
Contributor Author

Hi @andreasbrett many thanks for adopting this ❤️ But it would be nice, if we could have further test sentences to cover these singular and plural variations (Stunde vs Stunden, same for minutes and seconds) and these ['halbe', '1/2'] variations. thx 👍

Thanks for being a pain with the tests :-) Especially with those mere EN->DE migrations I sometimes forget how much more thorough some of the DE tests are compared to EN. Which is a good thing. Tests added.

The 'halbe', '1/2' variations were leftovers from EN that we don't have in the DE sentences. Again well spotted. I removed them, since they're outside the scope of this PR. Let someone else or future-me handle them.

@andreasbrett andreasbrett marked this pull request as ready for review November 14, 2024 23:21
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.

2 participants