Else clause for automations #378
Comments
|
I think step 1 should be to add else logic to scripts, since scripts and the action part of automations are executed using the same code. Once that is in place, we might think differently about automations :) A discussion is currently ongoing here home-assistant/core#35133 but I think that should be moved to arch. I am 👍 on adding |
|
OK, adding this to scripts enables a lot of use cases, perhaps even more than my proposal. I wanted to start small and intuitive - a single else clause per automation, but if we're ok taking a bolder approach I'm all for it. |
|
If you don't mind, I think continuing the discussion here would make sense. The PR linked above adds an - condition: ...
else:
- condition failed
- condition passedrather than The pros of this method is that it simply replaces the default behavior of a failed condition - to stop execution. |
|
The perfect solution I think would be to have multiple script:
example_script:
sequence:
- condition: state
entity_id: device_tracker.paulus
state: 'home'
- service: notify.notify
data:
message: 'Turned on the ceiling light!'
sequence:
- condition: state
entity_id: device_tracker.paulus
state: 'away'
- service: notify.notify
data:
message: 'Turned on the ceiling light!'EDIT: |
|
Yes, I agree that |
|
Yeah, I like the idea from @thomasloven, it allow not too complex deeps as my variant, but it work out of the box everywhere and is fine for yaml. |
|
@pvizeli my main concern with the multiple sequences approach is that the conditions are evaluated multiple times, which can potentially lead to confusing results. In your example, the device tracker can change from |
|
To rephrase - I think there needs to be an atomic way to evaluate a condition and then branch out accordingly. |
|
Maybe a new type |
|
The nice thing about a And we keep the condition code clean. |
|
I like it. That's a very good point with the common continuation after any branch too. - branch:
- condition: state
entity_id: input_binary.guest_mode
state: 'on'
sequence:
- service: system_log.write
data:
message: There are guest
- condition: state
entity_id: input_binary.guest_mode
state: 'off'
sequence:
- service: system_log.write
data:
message: Social distancing
- sequence:
# No condition -> Always evaluates to true, i.e. an else-clause
- service: system_log.write
data:
message: Something went horribly wrong! |
|
I would prefer if we would not extend condition schemas but instead have condition and sequence live in a dictionary. This will make validation easier as we don't have to make copies of the dict to pass it to existing validations. - branch:
- condition:
condition: state
entity_id: input_binary.guest_mode
state: 'on'
sequence:
- service: system_log.write
data:
message: There are guest |
|
@balloob would the state machine be frozen between evaluation of the different condition clauses? Otherwise it wouldn't be possible to achieve atomic branching and ensure that one and only one branch is true. |
|
@OnFreund As long as there is no async await in the code path it will be frozen. |
|
I really like suggestion made by @balloob, it can be also extended to incorporate an "else" branch suggested by @thomasloven: - branch:
- condition:
condition: state
entity_id: input_select.some_value
state: state_1
sequence:
- service: system_log.write
data:
message: State is one
- condition:
condition: state
entity_id: input_select.some_value
state: state_2
sequence:
- service: system_log.write
data:
message: State is two
default_sequence:
- service: system_log.write
data:
message: State has unexpected valueIt would be something similar to switch statement and pattern matching known from programming languages. |
|
@elupus cool, that should take care of conditions (I don't believe conditions can have side effects), but we need to ensure that the sequences don't run between the conditions, because those would most likely have awaits in them. @PiotrMachowski I really like the |
I think that it would be better to run only a sequence for a first passed condition (like mentioned above by @balloob) |
|
Oh, if only the first true conditi
8000
on takes then you're right. (assuming conditions can indeed have no side effects). |
|
Would this support nesting? Contrived example for syntax of what I mean. - branch:
- condition:
condition: state
entity_id: input_binary.guest_mode
state: 'on'
sequence:
- branch:
- condition:
condition: state
entity_id: binary_sensor.is_house_cold
state: 'on'
sequence:
- service: system_log.write
data:
message: There are guests and it is cold |
|
Most likely. |
|
The |
|
I believe |
|
@pvizeli Could it be possible to make |
|
if we're going to execute first branch to match, we could just execute first branch that has no condition specified. And yes, these can be nested. A sequence doesn't know what it's executing, so it can be yet another branch. It won't be fun to keep track of which branch you're editing though 🤷 |
|
OK, sounds like we're converging to a good outcome. - branch:
- condition:
condition: state
entity_id: input_select.some_value
state: state_1
sequence:
- service: system_log.write
data:
message: State is one
- condition:
condition: state
entity_id: input_select.some_value
state: state_2
sequence:
- service: system_log.write
data:
message: State is two
- branch:
- condition:
condition: state
entity_id: binary_sensor.some_value
state: 'on'
sequence:
- service: system_log.write
data:
message: Nested conditions work but are not encouraged
- sequence: # no condition acts as a fallback - it's always true
- service: system_log.write
data:
message: State has unexpected value |
|
This would also pretty much require some way to stop a script execution, so you can stop after some branches, but continue after others. Simplest way would be to add a but that's not very user friendly. |
|
My pr has been updated to work with the new syntax: home-assistant/core#35133 |
|
@thomasloven what if conditions break out of the current sequence, but don't stop script execution altogether? I'm not entirely sure about current behavior, but if the following two statements are true, then it's a backwards compatible solution:
|
|
I'm talking about the case: Without the exit statement, the common action would need to be repeated in the A and B branches. |
|
I think exit is a nice-to-have, but not strictly required. The "do something common" part can be achieved with repetition, like you mentioned. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@gjbadros this is getting off topic. Maybe open a new arch issue? |
|
dropping it; my other arch issues are more important to me to get feedback on. |
|
Coincidentally I started thinking about what to do next with scripts and was toying with some variations of an if construct (and others for looping) and ran across this conversation. I kind of prefer this over the more traditional if, especially since it fits better with YAML. I'd probably use - choose:
- condition:
# One or more conditions that are AND'd
sequence:
# Sequence that executes if associated condition is first to evaluate to true
- # Repeat the above as many times as desired
default: # Optional
# Sequence that executes if none of the conditions evaluate to trueI know it's a bit off topic, but I've started to work on the implementation of the following new features for looping: - while:
# One or more conditions that are AND'd
sequence:
# Sequence that executes repeatedly while above conditions evaluate to trueand: - repeat: COUNT # template accepted here as well, rendered once each time script reaches this step
sequence:
# Sequence that executes repeatedly the number of times determined by repeat value aboveAlthough one can do looping now with a single, |
|
RE: taking over As mentioned in that PR, I also had some thoughts on looping and on script local variables. |
Each of the three I suggested above are perfectly valid. E.g., The "choose" step is a dictionary with two keys: I definitely looked at what you did in 35133 and it was a very good start. In fact, I had originally discarded the idea of "sub-blocks", thinking the implementation didn't lend itself to that. But I obviously had blinders on! 😄 In my "repeat" implementation I'm using that idea -- i.e., creating another I'll review your PR again in more detail to better understand what you were thinking re: looping & local variables. Regarding continuing and/or starting a new architecture topic, I leave that to others. I think this was a very good discussion, but it seems to have reached its natural conclusion. And for repeat & while, those seem straight forward enough that I think they can be handled via PRs. But that's just my opinion. |
|
@pnbruckner thanks for taking this on, that's awesome!
|
- choose: SEQUENCE
sequence:
abc:
# Sequence
def:
# Sequence
default:
# Sequencewhere
|
|
|
There certainly is an advantage with the generally agreed upon approach. However, there's also an advantage to a single template. Yes, it will likely be relatively large, but all the "logic" would be in one place, and hence possibly easier to understand and maintain, rather than having that "logic" scattered across many, many lines, with each list of conditions possible fairly far apart from each other. Not necessarily the best for understand-ability/maintenance. |
|
IDK, it feels like replacing switch statements in a general purpose programming language with a large regular expression and branching on its output :) |
|
In fact, it would be very similar to a switch statement -- one expression decides which case to run. Not sure why that would be a bad thing. It's definitely atomic, keeps logic in one place, makes it a bit more consistent with my "repeat" & "while" constructs (i.e., the second part is But, I'm only suggesting this as an alternative if the previously agreed on "choose" construct doesn't work out for some reason. |
|
Yeah, I get the one expression part, what I meant is that complex logic within a template would be akin to complex logic within a regular expression. Templates are great for simple stuff, but quickly become hard to follow when more logic is added. Anyway, thanks again for picking this up - really appreciated. |
|
Again, I know it's a bit off topic, but I believe I have the implementation of the repeat loop completed. I, of course, still have to add tests and update the docs, but if anyone is interested see https://github.com/home-assistant/core/tree/script-repeat-loop. |
- choose:
- if:
- condition: state
entity_id: device_tracker.me
state: office
- condition: numeric_state
entity_id: sensor.n_guests
above: 0
sequence:
- service: switch.turn_on
entity_id: switch.inside_lights
- if:
- condition: not
conditions:
- condition: state
entity_id: device_tracker.me
state: home
- condition: numeric_state
entity_id: sensor.luminance
below: 50
sequence:
- service: switch.turn_on
entity_id: switch.front_porch_lightto: - choose: >
{% if not is_state('device_tracker.me', 'home') and states('sensor.n_guests')|int > 0 %}
not_home
{% elif is_state('device_tracker.me', 'office') and states('sensor.luminance')|float < 50 %}
office
{% endif %}
sequence:
not_home:
- service: switch.turn_on
entity_id: switch.inside_lights
office:
- service: switch.turn_on
entity_id: switch.front_porch_lightI can tell you which I think is easier to understand and maintain, and the decision making is atomic. Yes, I understand the the first one could be changed to templates as well, and be more compact. The points are 1) whether the decision logic is in one place as opposed to scattered throughout the statement (which gets worse as there are more choices), and EDIT: BTW, I'm assuming rendering a template is an atomic operation in HA. I guess I don't know that is true. |
|
Can it be done atomically of all of the "when" condition clauses are of
type 'template'? It seems like we could document that the only way to have
the conditions evaluated against a stable global state is to limit to
conditions of a small number of types (template-only?) and then it'd be the
same as executing the latter that you give but with, IMO, a much
easier-to-read syntax that more closely mirrors other languages like 'cond'
in scheme/lisp and choose/when in XSLT (and other parts of hass).
There are some places where a user might care about the ability to use
different condition types without caring about atomicity especially given a
fall-through default case that could retrigger or otherwise handle the
obscure case of an expectation of a partitioning of cases not being met.
In particular, a when/default where the default is really just the else
clause would still work fine regardless of the kind of condition being used.
…On Wed, Jul 1, 2020 at 2:54 PM Phil Bruckner ***@***.***> wrote:
@OnFreund <https://github.com/OnFreund>
The implementation by @thomasloven <https://github.com/thomasloven>
managed to achieve atomicity
That's actually not true. At least it doesn't appear so to me. The code
that evaluates the conditions, although it does them all in a single loop,
contains await statements, so I don't think it can be said to be
guaranteed to be atomic.
If atomicity is important, then I think maybe my suggestion for using a
single template in a switch style statement may be the way to go. And I
don't agree with your assessment that that is less clear. E.g., compare:
- choose:
- if:
- condition: state
entity_id: device_tracker.me
state: office
- condition: numeric_state
entity_id: sensor.n_guests
above: 0
sequence:
- service: switch.turn_on
entity_id: switch.inside_lights
- if:
- condition: not
conditions:
- condition: state
entity_id: device_tracker.me
state: home
- condition: numeric_state
entity_id: sensor.luminance
below: 50
sequence:
- service: switch.turn_on
entity_id: switch.front_porch_light
to:
- choose: > {% if not is_state('device_tracker.me', 'home') and states('sensor.n_guests')|int > 0 %} not_home {% elif is_state('device_tracker.me', 'office') and states('sensor.luminance')|float < 50 %} office {% endif %} sequence:
not_home:
- service: switch.turn_on
entity_id: switch.inside_lights
office:
- service: switch.turn_on
entity_id: switch.front_porch_light
I can tell you which I think is easier to understand and maintain, and the
decision making is atomic.
Yes, I understand the the first one could be changed to templates as well,
and be more compact. The points are 1) whether the decision logic is in one
place as opposed to scattered throughout the statement (which gets worse as
there are more choices), and 2) whether atomicity can be achieved in the
decision logic.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#378 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AALOHTP2FHHKRQLKOVA3WBTRZOWARANCNFSM4MYIYI3Q>
.
|
If I'm reading this correctly, the As for the template approach, don't get me wrong - for me, as an advanced user, it's great, and most of my use cases are indeed about a single expression and multiple potential values to compare against. However, I think the barrier to entry with it might be too high, especially when complex logic is required. |
You are right, of course. Sorry. RE: atomicity, there are two problems. The first case probably can't be solved, and is also a "problem" for the normal Of course, an alternative approach could be to freeze a copy of the state machine at the beginning of |
Can you elaborate on what remains non-atomic in this implementation? |
|
No, you are right. My mind was stuck in pre-emptive threaded execution for some reason. |
|
Are you referring to this bit of code: conditions = [(
await self._get_condition_config(b["if"]) if b.get("if") else None,
b["sequence"]
) for b in branches]
I personally wouldn't expect the state of the system to be the same between the condition evaluation and the branch execution. That's asking a bit much. I wouldn't worry about that.
If there's one thing I've learned in over 40 years of embedded programming, if something can happen, it will happen. And this is exactly the sort of thing to make a system misbehave in mysterious ways that are next to impossible to debug/fix. I'm still not necessarily saying I think it has to be a single template. I do agree that raises the bar for the average user.
|
|
@pnbruckner that piece of code doesn't evaluate the conditions, but rather just gathers them. The evaluation happens here and is atomic (in the "between condition evaluations" sense, not "covering both conditions and sequences", which I agree isn't needed). |
|
Ah, ok, now I see. Thanks! I wasn't reading the code very closely. My bad! Yep, that makes perfect sense now. |
|
So just to make sure we're still all on the same page, the choose statement will look like this: - choose:
- if:
- condition: ...
sequence:
- ...
- if:
- condition: ...
sequence:
- ...
# "Default" case leaves out optional if part:
- sequence:
- ...Is that right? |
|
Perfect! |
|
Initial implementation complete: https://github.com/home-assistant/core/tree/script-choose-action. Still need to add test and update docs. |
Context
There are a lot of use cases that require one sequence of actions if a condition is true and different one if it's false. For example:
The existing methods of performing this have their limitations. The easiest way is creating two automations with the same trigger and opposite conditions (this is now easier thanks to home-assistant/core#34624). This means repeating the conditions, which is both not DRY and, more importantly, could also in some cases lead to both automations running (or not running) since the conditions are evaluated twice.
One can avoid repeating the conditions by use of templates and
input_*, but that could be cumbersome, requires redundant entities, and is still subject to potential timing issues.Other approaches could include a service template to run two different scripts, but that means the conditions now need to run as actions instead of conditions. I'm sure there are other approaches as well, each with its limitation.
Keeping in mind that we don't want to turn automations into a general purpose programming language, I wanted to propose a relatively simple mechanism, that is able to achieve quite a lot.
Proposal
I propose adding an else clause to automation, which will be defined in a similar way to the existing actions, but will run when the condition is false, rather than when it's true.
This covers simple use cases easily, and more advanced cases can be handled through firing events and triggering multiple automations.
For example:
Consequences
It's easier and less error prone to create automations that have if/else logic. Since the else clause is optional, existing automations aren't affected.
The text was updated successfully, but these errors were encountered: