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

Add availability_template option to all template platforms #25891

Closed
wants to merge 12 commits into from
Closed

Add availability_template option to all template platforms #25891

wants to merge 12 commits into from

Conversation

grillp
Copy link
Contributor

@grillp grillp commented Aug 12, 2019

Breaking Change:

Not a Breaking change

Description:

This PR adds an availability_template option to each component in the Template Platform.

By defining the availability_template the availability of a template component can be controlled using a template.

As Template Components rely on other component states, (i.e. they do not have state themselves) it would be useful to also be able to control when a component is unavailable. e.g. when a component that the template component relies on to function becomes 'unavailable it would be useful to be able to mark the template component as 'unavailable also.

If the availability_template is not configured, the device will always be 'available' (default behavior)

If the availability_template is configured, the device will be 'available' if the template returns true or 'unavailable' if the template returns any other value.

home-assistant/home-assistant.io#10123

Example entry for configuration.yaml (if applicable):

For a Cover Template component that requires a 'switch.garage_clicker' to be available to operate:

- platform: template
  covers:
    garage:
      device_class: garage
      friendly_name: "Garage Door"
      open_cover:
        service: script.garage_click
      close_cover:
        service: script.garage_click
      stop_cover:
        service: script.garage_click
      availability_template: >-
        {%- if not is_state("switch.garage_clicker", "unavailable") %}
          true
        {%- endif %}

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • [N/A] The manifest file has all fields filled out correctly. Update and include derived files by running python3 -m script.hassfest.
  • [N/A] New or updated dependencies have been added to requirements_all.txt by running python3 -m script.gen_requirements_all.
  • [N/A] Untested files have been added to .coveragerc.

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

@grillp grillp requested a review from a team as a code owner August 12, 2019 11:41
@homeassistant
Copy link
Contributor

Hi @grillp,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

@probot-home-assistant
Copy link

Hey there @PhracturedBlue, mind taking a look at this pull request as its been labeled with a integration (template) you are listed as a codeowner for? Thanks!

@frenck
Copy link
Member

frenck commented Aug 12, 2019

⚠️ I was unable to find a documentation PR (matching this code change) in our documentation repository.

Please, update the documentation and open a PR for it (be sure to create a documentation PR against the next branch) and link 🔗 this PR and the new documentation PR by mentioning the link to the PR's in both openings posts/descriptions.

🏷 I am adding the docs-missing label until this has been resolved.

@grillp
Copy link
Contributor Author

grillp commented Aug 12, 2019

home-assistant.io document PR added (Sorry. chicken/egg problem ;-) )

@MartinHjelmare MartinHjelmare changed the title Added `availability_template' configuration option to all Tempalte platform components. Add availability_template option to all template platforms Aug 13, 2019
@Limych
Copy link
Contributor

Limych commented Aug 25, 2019

Please note that in your code, the state of the sensors is considered available until the first update. This may cause a malfunction. It’s more correct to consider the state unavailable until the first update, because it is not yet defined.

It will also be more correct to always give None for state when the state is unavailable. You have not done this.

@grillp
Copy link
Contributor Author

grillp commented Aug 26, 2019

Hey @Limych,

Please note that in your code, the state of the sensors is considered available until the first update. This may cause a malfunction. It’s more correct to consider the state unavailable until the first update, because it is not yet defined.

Do you mean that the available() function should return false until at least one status update_state() has been called? If so, I can change that. It makes sense.

It will also be more correct to always give None for state when the state is unavailable. You have not done this.

Not sure I agree. By just having available() return false, I allow the underlying HASS behaviour to return/set a state of unavailable, rather than forcing it to None, which I assume is the correct hass behavior? Your thoughts?

I also am not sure about my implementation where I check for a string result of True from the Availability Template. I think I should either:

  • allow the user to configure the template return value which is considered available e.g. payload_available which defaults to True
    or
  • Use the same method you used in Add available_template option to template sensor #25918 where the template returns a boolean: e.g. {{ is_state('switch.track_sun', 'on') }}, where true is available and anything else is unavailable

Thoughts?

@MartinHjelmare
Copy link
Member

There are merge conflicts.

@balloob
Copy link
Member

balloob commented Sep 6, 2019

This PR should be broken up in 1 PR per platform. it is too big as is.

@MartinHjelmare MartinHjelmare moved this from Needs review to Incoming in Dev Sep 7, 2019
@grillp
Copy link
Contributor Author

grillp commented Sep 8, 2019

There are merge conflicts.

That's what happens when a PR stays open for 20+ days!

Fixing now.

@grillp
Copy link
Contributor Author

grillp commented Sep 8, 2019

This PR should be broken up in 1 PR per platform. it is too big as is.

@balloob , It is the same feature 'availability_template' feature added to all template platform components, so I think they all belong together. And each change to a component source is discreet/isolated and has an associated test file change.

Also all require the same change to homeassistant/const.py so would I do 1, wait for it to be accepted and then do the rest? Chicken and Egg?

Cheers

G./

@MartinHjelmare
Copy link
Member

Please break it up and start with one platform.

@grillp
Copy link
Contributor Author

grillp commented Sep 8, 2019

I have split this into 8 pull requests:
#26509
#26510
#26511
#26512
#26513
#26514
#26516
#26517

@grillp grillp closed this Sep 8, 2019
Dev automation moved this from Incoming to Cancelled Sep 8, 2019
@lock lock bot locked and limited conversation to collaborators Sep 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Dev
  
Cancelled
Development

Successfully merging this pull request may close these issues.

None yet

7 participants