-
-
Notifications
You must be signed in to change notification settings - Fork 33.1k
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
Add "at_least" and "at_most" actions to the Timer integration #139049
base: dev
Are you sure you want to change the base?
Conversation
Thanks @StaleLoafOfBread. I've added a commit that renames the methods as you suggested. |
Following the conventions for actions in Home Assistants they should begin with a verb like all existing timer actions do. So you should name them as
The "Duration" fields of both actions should then also become
as calling them "duration" is quite misleading: The actual timer duration may not change at all with these actions if it's either above or below the value given by the respective action. This results in these action descriptions:
|
@NoRi2909: thanks for the feedback. I'm happy to change the name so that it starts with a verb, but I worry that "set minimum" would be misleading because there isn't a field named "minimum" that's getting set. What do you think of these suggestions instead:
|
Perhaps we find the best name for the actions themselves if you go through my suggestions from the bottom up. Then the fields "Minimum" and "Maximum" are given in the action (instead of the current "duration" in both) and their respective purpose is explained in the action descriptions. |
Hi there @kevinbackhouse 👋 Thanks for opening a pull request. I don't know if this is something I would introduce to this component. To be sure this is not just limited to my opinion, I've scheduled this pull request for discussion in the next Core architectural meeting. I will get back to you on the results of this in the upcoming week. I'm marking this PR as a draft in the meantime. ../Frenck |
@frenck: Thanks for taking a look! To give you an example of why I think this feature would be useful, my house has a short corridor with a door on each end:
If the door on the left opens, I want to switch on a light in the corridor for the next 5 minutes. And if the door on the right opens, I want to switch on the light for the next 2 minutes. Timers are a very useful intermediary for this kind of thing because they let me check how much time is left on the clock. But the logic for doing so in YAML is quite cumbersome:
And that snippet doesn't even include the logic for handling idle or paused timers. But besides the yaml solution being clunky, this feels to me like an operation that should be atomic. I'm not sure if home assistant is multi-threaded and able to run two automations in parallel, but in that scenario the yaml above would have a race condition. |
Breaking change
Proposed change
Add two new actions to the Timer integration: "at_least" and "at_most". These features were suggested here (not by me, but I also think they would be very useful).
Type of change
Additional information
timer.at_least
andtimer.at_most
. home-assistant.io#37624Checklist
ruff format homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
.To help with the load of incoming pull requests: