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

Update automation.markdown #8778

Merged
merged 4 commits into from Mar 4, 2019
Merged

Update automation.markdown #8778

merged 4 commits into from Mar 4, 2019

Conversation

akasma74
Copy link
Contributor

Clarification of initial_state behaviour

Description:
It it now unclear that automation is disabled by default and one need to enable each and every one the one way or another, which creates a lot of discussions like that

Pull request in home-assistant (if applicable): home-assistant/home-assistant#

Checklist:

  • Branch: next is for changes and new documentation that will go public with the next home-assistant release. Fixes, changes and adjustments for the current release should be created against current.
  • The documentation follows the standards.

Clarification of initial_state behaviour
@ghost ghost added the to-do label Feb 28, 2019
@klaasnicolaas klaasnicolaas added enhancement ready-for-review This PR needs to be reviewed current This PR goes into the current branch and removed to-do labels Feb 28, 2019
@@ -52,6 +52,7 @@ Actions are all about calling services. To explore the available services open t
If you always want your automations to be enabled or disabled upon Home Assistant restart, then you have to set an initial state in your automations. Otherwise the previous state will be restored.

If an automation is disabled (turned off) then it will never trigger. Only automations that are enabled (turned on) will trigger.
To have your automation enabled you should either add `initial_state: true` to your automation description (it will always make it enabled on Home Assistant startup) or turn it on manually via UI/another automation/developer tools (that way the last stored state of the automation will be restored on the next Home Assistant startup).
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this covered already above (line 52)?

Copy link
Member

Choose a reason for hiding this comment

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

I agree with @rohankapoorcom on this @akasma74, it is already stated at line 52.

It would make more sense to extend the description of line 52 instead of adding it at this place (and kinda repeating things).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please have a look at the amended version.
The main reason of this PR is that the current text covers state's restoration , but says nothing about initial state of newly created automation without initial_state (that behaviour has changed recently to opposite), these 2 things are completely separate and should not be mixed up/omitted.
It also says nothing about HA startup failure.
Hope that makes more sense.

Copy link
Contributor Author

@akasma74 akasma74 left a comment

Choose a reason for hiding this comment

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

Added some clarification about restoring state and HA startup failure

come more adjustments and clarification
Copy link
Contributor Author

@akasma74 akasma74 left a comment

Choose a reason for hiding this comment

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

Some more clarification and adjustment

@ghost ghost assigned frenck Mar 4, 2019
@frenck
Copy link
Member

frenck commented Mar 4, 2019

Thanks, @akasma74!
👍

@frenck frenck merged commit 47f78da into home-assistant:current Mar 4, 2019
@ghost ghost removed the ready-for-review This PR needs to be reviewed label Mar 4, 2019
@tdejneka
Copy link
Contributor

tdejneka commented Jun 6, 2019

This PR should have never been merged because it added false statements to the documentation.

It states this:

When you create a new automation, it will be disabled (and therefore won't trigger) unless you explicitly add initial_state: true to it or turn it on manually via UI/another automation/developer tools.

That is completely false. How did such an utterly invalid statement every pass the vetting process?

I've created dozens of automations (from version 0.84 when the restore feature changed to use a JSON store up to version 0.91) and they are all enabled by default. None of my automations employ initial_state: true and all have their state properly restored after a restart.

This PR should be reversed. The revised documentation it introduced is false and misleads users to believe initial_state: true is an obligatory option (it's definitely not).

The previous documentation was correct:

If you always want your automations to be enabled or disabled upon Home Assistant restart, then you have to set an initial state in your automations. Otherwise the previous state will be restored.

NOTE
I use VS Code to create my automations and not the built-in Automation Editor.

@akasma74
Copy link
Contributor Author

akasma74 commented Jun 6, 2019

@tdejneka Have you read my comments to this PR and the supporting discussions?
It wasn't my only experience, it was a conclusion of the discussion of HA users that experienced issues because the documentation did not reflect the state of things at the time.

You are referring to your personal experience. That's great, but not enough to prove the fact I'm afraid.
And using sentences like "How did such an utterly invalid statement every pass the vetting process?" is a bit offensive of be honest. Even if a person made a mistake (as we all do sometimes), it's wise be polite (unless you don't care that people won't contribute because of such a behaviour).

And after all, HA is evolving and if now it's different - feel free to make your own PR.

@tdejneka
Copy link
Contributor

tdejneka commented Jun 6, 2019

In the evidence you've provided (the linked community discussion) I was an active participant. The issue of initial_state: true is easily one of Home Assistant's most misunderstood options. That's largely due to a misunderstanding of its purpose and misinformation like this PR created has not helped to clarify it.

The discussion thread was created shortly after version 0.85 was released. Many users were jumping from pre-version 0.84 and first encountering the newly revised restore feature which, as mentioned, defaulted to setting all automations to off after the upgrade.

The fix was to simply turn them back on (using the Services page) and then the restore feature would take care of future restarts. However, the updated restore feature became conflated with the initial_state option. It became a fad to recommend adding initial_state: true to every automation as both a fix for the "all automatons are off after the upgrade" situation and to ensure the automation was always enabled after an interrupted restart.

This spectacularly bad advice omitted to mention that it overrides the functionality of the restore feature and causes the affected automations to have their last_triggered reset after a restart.

This PR only served to make a bad situation worse by stating all new automations are turned off by default (they are not) and that the corrective action is the obligatory inclusion of initial_state: true (it is not). These false statements are demonstrably false, not only for me and not only now but in every version since the restore feature was revised (circa 0.84).

You've chosen to focus on 'feelings' as opposed to the core issue that numerous users have been led astray by misinformation. I interpret your response as misdirection and a reluctance to correct the mistake. I'll take the initiative to do it myself.

@akasma74
Copy link
Contributor Author

akasma74 commented Jun 6, 2019

In the evidence you've provided (the linked community discussion) I was an active participant. The issue of initial_state: true is easily one of Home Assistant's most misunderstood options. That's largely due to a misunderstanding of its purpose and misinformation like this PR created has not helped to clarify it.

As far as I remember, this PR served 2 purposes, one of them - to clarify what happens when the restore feature fails. There was nothing about that in docs before this PR as the feature was rather new and added (apparently) without reflecting any details in docs.

The fix was to simply turn them back on (using the Services page) and then the restore feature would take care of future restarts. However, the updated restore feature became conflated with the initial_state option. It became a fad to recommend adding initial_state: true to every automation as both a fix for the "all automatons are off after the upgrade" situation and to ensure the automation was always enabled after an interrupted restart.

there is nothing bad in having some automations always enabled, especially those responsible for critical tasks. speaking about users who mindlessly copy stuff - well, every feature might be dangerous is used by not very competent person.

This spectacularly bad advice omitted to mention that it overrides the functionality of the restore feature and causes the affected automations to have their last_triggered reset after a restart.

perhaps because there was (and still is) no such information anywhere in the docs.
actually, this is the very first time I personally hear about it. haven't checked it yet though..
and not'sure that' THAT bad..

This PR only served to make a bad situation worse by stating all new automations are turned off by default (they are not) and that the corrective action is the obligatory inclusion of initial_state: true (it is not). These false statements are demonstrably false, not only for me and not only now but in every version since the restore feature was revised (circa 0.84).

Possibly. Sorry, I haven't got time to investigate it properly at the moment.
This PR has already merged. The only way to correct the mistake is to create a new PR.
You seem to be in a great position to put things right, honestly.

You've chosen to focus on 'feelings' as opposed to the core issue that numerous users have been led astray by misinformation. I interpret your response as misdirection and a reluctance to correct the mistake. I'll take the initiative to do it myself.

You mean my previous response was all about feelings and nothing else? Oh dear..

I'd love to see your/anyone else's PR that fixes this matter because all I want to achieve is better documentation as I deeply understand how annoying can it be to use incomplete/outdated/wrong information. I'm not looking for fame or any special status here ;)
Could you do me a favour and make sure I'm aware of your PR when you submit it?

@tdejneka
Copy link
Contributor

tdejneka commented Jun 7, 2019

You mean my previous response was all about feelings and nothing else? Oh dear..

You wrote more about "offense" and "politeness" than addressing the false statements you introduced into the documentation. New automations are not off by default and it's not required to use initial_state: true.

In fact, your only response to those erroneous statements has been:

Possibly. Sorry, I haven't got time to investigate it properly at the moment.

It takes less than minute to prove the documentation is wrong. The simple answer would've been to acknowledge it was a mistake and that it would be corrected ASAP.

FWIW, I've posted a draft version of the new documentation here for public commentary.

@frenck
Copy link
Member

frenck commented Jun 7, 2019

I'm going to lock this conversation for two reasons.

  1. Don't comment or discuss things in merged/closed commits. It should be a new issue.
  2. We are now aware of this and are discussing how to resolve this.

Above documentation is indeed incorrect and should not have been merged (my bad).
Nevertheless, all the unclear discussions around this clearly signal an issue with the part in general. We are looking into the options to make this more clear and avoid future confusion overall.

@home-assistant home-assistant locked as resolved and limited conversation to collaborators Jun 7, 2019
@akasma74 akasma74 deleted the patch-2 branch April 9, 2020 10:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
current This PR goes into the current branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants