home-assistant / core

:house_with_garden: Open source home automation that puts local control and privacy first.
https://www.home-assistant.io
Apache License 2.0
73.58k stars 30.75k forks source link

"Wait for trigger" with defined but zero timeout timesout immediately #109586

Open 20BBrown14 opened 9 months ago

20BBrown14 commented 9 months ago

The problem

"Wait for trigger", wait_for_trigger actions with a defined but zero timeout fails to wait for one of the triggers and instead times out immediately.

timeout:
  hours: 0
  minutes: 0
  seconds: 0
  milliseconds: 0

What version of Home Assistant Core has the issue?

core-2024.1.2

What was the last working version of Home Assistant Core?

No response

What type of installation are you running?

Home Assistant Container

Integration causing the issue

Automations

Link to integration documentation on our website

https://www.home-assistant.io/docs/automation/

Diagnostics information

No response

Example YAML snippet

wait_for_trigger:
  - platform: device
    type: turned_on
    device_id: <device_id>
    entity_id: <entity_id>
    domain: switch
timeout:
  hours: 0
  minutes: 0
  seconds: 0
  milliseconds: 0

Anything in the logs that might be useful for us?

No response

Additional information

No response

TheJulianJES commented 9 months ago

Hmm, what would you expect to happen in that case? Not time out at all?

20BBrown14 commented 9 months ago

If the timeout is zero I would except it be treated as a null timeout. That makes sense to me at least.

On Mon, Feb 5, 2024, 11:22 PM TheJulianJES @.***> wrote:

Hmm, what would you expect to happen in that case? Not time out at all?

— Reply to this email directly, view it on GitHub https://github.com/home-assistant/core/issues/109586#issuecomment-1928806372, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHBJCWJIXTUP5QXBECCRMGDYSG4ZHAVCNFSM6AAAAABCYRYLGKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMRYHAYDMMZXGI . You are receiving this because you authored the thread.Message ID: @.***>

TheJulianJES commented 9 months ago

You can just use continue_on_timeout: false in that case (instead of providing a timeout).

20BBrown14 commented 9 months ago

Hm. Not sure how that solves the problem I think is present.

If I had provided a timeout previously and then switched it to a zero second timeout, I would like the automation to wait for the triggers indefinitely.

As is, if I set a timeout of zero and false for continue on timeout, the automation would immediately cancel not waiting for any of the triggers AND not continuing the script.

If the timeout is zero I would simply except it to be treated as null timeout and wait indefinitely. With a timeout of zero, the continue on timeout flag shouldn't change any behavior in my head.

On Mon, Feb 5, 2024, 11:57 PM TheJulianJES @.***> wrote:

You can just use continue_on_timeout: false in that case (instead of providing a timeout).

— Reply to this email directly, view it on GitHub https://github.com/home-assistant/core/issues/109586#issuecomment-1928836917, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHBJCWI23KU2VMOGJOKBPGTYSHA4DAVCNFSM6AAAAABCYRYLGKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMRYHAZTMOJRG4 . You are receiving this because you authored the thread.Message ID: @.***>

TheJulianJES commented 9 months ago

But the behavior of waiting indefinitely for the triggers is the default one when you don't provide a timeout at all.

20BBrown14 commented 9 months ago

Right. I guess this ends up being a question of whether a timeout of zero seconds is the same as no timeout. Which it seems you're saying it's not and that a timeout of zero seconds is exactly that. Just doesn't seem very intuitive.

Can we think of an actual use case where a timeout of zero seconds would be intended?

On Tue, Feb 6, 2024, 1:02 PM TheJulianJES @.***> wrote:

But the behavior of waiting indefinitely for the triggers is the default one when you don't provide a timeout at all.

— Reply to this email directly, view it on GitHub https://github.com/home-assistant/core/issues/109586#issuecomment-1930575017, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHBJCWMZOVKNIYO6A6WWEOTYSJ423AVCNFSM6AAAAABCYRYLGKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMZQGU3TKMBRG4 . You are receiving this because you authored the thread.Message ID: @.***>

vlacsap commented 8 months ago

I have also got probleme with this automation with the UI interface.

1 - when you create the automation without touching this timeout value, it's works fine. So zero ==> no time out.

2 - but il you put a value (ie : 1 min) and return at zéro, the automation works completely differently.

If buton continue_on_timeout = true
its continue imediatly. So zero ==> time out infinitely short at zero second. Same thing on the UI but different operation. it's very disturbing.

If buton continue_on_timeout: fals everything else is blocked. I don't understant

Nota : if I look at the Yaml, on the 1 (when i create) there is not time out defined. But on the 2 (when I write zero) the timeout is defined at zero.

Sorry for my english. I hope that is understandable.

20BBrown14 commented 8 months ago

That's exactly the issue that happened to me that caused me to open this issue in the first place. If it's decided that the timeout should still operate as is, then the UI should be updated such that if you input 0 timeout it goes back to the default of no timeout

On Wed, Mar 6, 2024, 11:32 AM vlacsap @.***> wrote:

I have also got probleme with this automation with the UI interface.

1 - when you create the automation without touching this timeout value, it's works fine. So zero ==> no time out.

2 - but il you put a value (ie : 1 min) and return at zéro, the automation works completely differently.

If buton continue_on_timeout = true its continue imediatly. So zero ==> time out infinitely short at zero second. Same thing on the UI but different operation. it's very disturbing.

If buton continue_on_timeout: fals everything else is blocked. I don't understant

Nota : if I look at the Yaml, on the 1 (when i create) there is not time out defined. But on the 2 (when I write zero) the timeout is defined at zero.

Sorry for my english. I hope that is understandable.

— Reply to this email directly, view it on GitHub https://github.com/home-assistant/core/issues/109586#issuecomment-1981421744, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHBJCWMC5VNLZ4AIDSGQPVLYW5HMBAVCNFSM6AAAAABCYRYLGKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBRGQZDCNZUGQ . You are receiving this because you authored the thread.Message ID: @.***>

danka621 commented 7 months ago

But the behavior of waiting indefinitely for the triggers is the default one when you don't provide a timeout at all.

In the GUI "Wait for timeout" is described as optional, and by default the trigger does not timeout. As soon as you change the value, the section is added in the yaml code. If you change it back to zero, there is no way to see that you have added the zero timeout if you only look in the GUI.

The GUI makes no difference between a timeout of 0 s and no timeout at all. So it looks the same but behaves radically different.

GUI-wise this would need some kind of checkbox.

TheJulianJES commented 6 months ago

Are there any changes in behavior regarding this with Home Assistant Core 2024.4.x?

Surgikill commented 6 months ago

@bdraco I cannot comment on https://github.com/home-assistant/core/pull/115830. To me, having a 0 timeout makes absolutely no sense. A 0 timeout should be treated as 'no timeout' not '0s timeout'. If a user wants a '0s timeout', they have the option of a '1ms timeout'. Currently you MUST have a timeout, unless you re-create the 'wait for trigger' and then never touch the timeout. Now I must go through and re-do a bunch of 'wait for trigger' automations, because I tried a timeout, didn't like it, and now my automatons are broken because the timeout thinks it is 0 seconds instead of 'no timeout'.

srescio commented 6 months ago

Related to https://github.com/home-assistant/frontend/issues/19360#issuecomment-2078744325

vhough commented 3 months ago

Is this issue resolved? As I still have this problem and stumbled upon this issue :)

Executed: 21 July 2024 at 20:34:38 Error: TimeoutError Result: wait: remaining: 0 trigger: null timeout: true

issue-triage-workflows[bot] commented 3 weeks ago

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍 This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

marschallphilipp commented 3 weeks ago

Same here. Sorry, but this should be fixed ASAP in my opinion!

image

karwosts commented 6 days ago

After frontend #22614 the timeout/duration field is now clearable.

I don't have a position on what 0 a timeout should mean for the automation editor, but we do now offer a way for users to remove the timeout if it had been previously set. I'm not sure if that could be considered a resolution of this issue or not.

danka621 commented 5 days ago

After frontend #22614 the timeout/duration field is now clearable.

I don't have a position on what 0 a timeout should mean for the automation editor, but we do now offer a way for users to remove the timeout if it had been previously set. I'm not sure if that could be considered a resolution of this issue or not.

22614 is in the core part and closed 5 years ago.. Are you sure it is related to this?

karwosts commented 5 days ago

After frontend #22614 the timeout/duration field is now clearable. I don't have a position on what 0 a timeout should mean for the automation editor, but we do now offer a way for users to remove the timeout if it had been previously set. I'm not sure if that could be considered a resolution of this issue or not.

22614 is in the core part and closed 5 years ago.. Are you sure it is related to this?

Sorry it's 22614 in the frontend repo, they have a separate counter. I have fixed the bad link, thanks!