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

[SK] implement cancel & cancelAll timer intents #2846

Merged
merged 4 commits into from
Jan 10, 2025

Conversation

mustul
Copy link
Contributor

@mustul mustul commented Jan 7, 2025

Hi,

I'm continuing with timer intents implementation for slovak language. Now I implemented cancel and cancelAll intents.

I found a small issue during the implementation with timer intent testing. There is an issue with resolving area aliases when working with timers. Specifically when I tried to cancel all timers in a specific area the area slot was not correctly resolved if area alias was used. The issue is caused in function get_matched_timers where timers are filtered by
timers = [t for t in timers if t.area == areaSlot] where:

  • t.area is unresolved name of the area "kuchy(ňa|ni|ne|ňu|nsk(ý|é))" for slovak fixtures file
  • areaSlot is raw value of the slot

so final comparison is something like "kuchy(ňa|ni|ne|ňu|nsk(ý|é))" == "kuchyni" which is obviously always false so no timer is ever canceled. I'm not sure about actual implementation or slot resolving in HA itself as I'm not familiar with the codebase yet, but from what I was able to figure out very similar code is used in HA implementation itself.
Adjusting this code to accomodate aliases would break at least few other language tests as the timer would be correctly found and cancelled so responses would change from 0 cancelled timers to at least one.
I'm not sure how to proceed with this issue or if we will just ignore it.

@LubosKadasi
Copy link
Contributor

Hi,

I'm continuing with timer intents implementation for slovak language. Now I implemented cancel and cancelAll intents.

I found a small issue during the implementation with timer intent testing.

Hi, you are right. The tests passed, we can ignore it for now. Anyway, if you have a specific solution in mind, feel free to create a PR and tag @synesthesiam

@mustul mustul requested a review from LubosKadasi January 9, 2025 18:34
- sentences:
- "<timer_cancel> <all> <timer_type>"
- sentences:
- "<timer_cancel> <all> <timer_type> <in> <area>"
Copy link
Contributor

Choose a reason for hiding this comment

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

<area> už má optional in area: "[<in> ]{area}" ale to je ok

@LubosKadasi LubosKadasi merged commit 2d9be19 into home-assistant:main Jan 10, 2025
2 checks passed
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