Skip to content

Updater component is always available and shows on/off depending on whether an update is available or not#25418

Merged
balloob merged 27 commits into
home-assistant:devfrom
Santobert:edit_updater_component
Aug 7, 2019
Merged

Updater component is always available and shows on/off depending on whether an update is available or not#25418
balloob merged 27 commits into
home-assistant:devfrom
Santobert:edit_updater_component

Conversation

@Santobert
Copy link
Copy Markdown
Member

@Santobert Santobert commented Jul 23, 2019

Breaking Change:

The updater component is now a binary sensor that is always available. The entity ID is binary_sensor.updater. The state is on/off depending on whether an update is available or not. The latest version as well as the release notes are attributes of this binary sensor.

Automations that are listening for the existence of updater.updater should now trigger when binary_sensor.updater changes to on.

This makes the component more transparent and understandable for the user. Additionally it is visible if there is an error or the source (https://updater.home-assistant.io/) is outdated.

Description:

See Breaking Change.

Furthermore the updater logs a warning if the current version is before the newest version. This is the case if the JSON from https://updater.home-assistant.io/ is outdated.

If you don't want the updater component to always be available, I suggest at least keeping the warning if the current version is greater than the newest version. Otherwise, people would wonder if the updater component is broken.

But personally, I think it would be more understandable if the updater component is always visible and displays the newest version. Automations can be triggered by the state and not by the existence of this component.

Related issue (if applicable): fixes #24385

Pull request with documentation for home-assistant.io (if applicable): home-assistant/home-assistant.io#9957

Example entry for configuration.yaml (if applicable):

No changes

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 does not interact with devices:

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

@Santobert Santobert requested a review from a team as a code owner July 23, 2019 08:44
@homeassistant
Copy link
Copy Markdown
Contributor

Hi @Santobert,

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!

@ghost
Copy link
Copy Markdown

ghost commented Jul 23, 2019

Hey there @home-assistant/core, mind taking a look at this pull request as its been labeled with a integration (updater) you are listed as a codeowner for? Thanks!

This is a automatic comment generated by codeowners-mention to help ensure issues and pull requests are seen by the right people.

@Santobert
Copy link
Copy Markdown
Member Author

Please give me feedback if this change is acceptable. If so, I will update the documentation as soon as possible.

@frenck
Copy link
Copy Markdown
Member

frenck commented Jul 23, 2019

Please don't hold off creating a documentation PR, it slows down all progress, since merging without docs is a no go to being with. Adding docs-missing label.

@Santobert
Copy link
Copy Markdown
Member Author

Santobert commented Jul 23, 2019

The documentation now is up to date: home-assistant/home-assistant.io#9957

Comment thread homeassistant/components/updater/__init__.py Outdated
@Santobert
Copy link
Copy Markdown
Member Author

The docs-missing label can be removed. I've added the docs here: home-assistant/home-assistant.io#9957

Comment thread homeassistant/components/updater/__init__.py Outdated
Comment thread homeassistant/components/updater/__init__.py Outdated
@Santobert
Copy link
Copy Markdown
Member Author

I guess there is something wrong with the CI? These errors are import errors and they have nothing to do with my changes.

Comment thread homeassistant/components/updater/binary_sensor.py Outdated
Comment thread homeassistant/components/updater/binary_sensor.py Outdated
Comment thread homeassistant/components/updater/binary_sensor.py Outdated
Comment thread homeassistant/components/updater/binary_sensor.py Outdated
Comment thread tests/components/updater/test_init.py Outdated
Comment thread tests/components/updater/test_init.py Outdated
Comment thread tests/components/updater/test_init.py Outdated
@Santobert
Copy link
Copy Markdown
Member Author

Santobert commented Aug 2, 2019

The platform is already tested in the current tests. Make sure all the new state attributes are tested. The only method not tested is probably the remove method that is called when the entity is removed. I'm not sure when that would happen here, since there's no config entry.

@MartinHjelmare I'm afraid we have to test the remove method since we miss the coverage target by 0.12% 😆 Can we call that method directly or do we have to trigger hass to remove the component?

Comment thread tests/components/updater/test_init.py
Comment thread tests/components/updater/test_init.py Outdated
Comment thread tests/components/updater/test_init.py Outdated
Comment thread tests/components/updater/test_init.py Outdated
@Santobert
Copy link
Copy Markdown
Member Author

Thank you @MartinHjelmare for all your help. I wouldn't have been able to do this without you.

Is it possible to merge this into the upcoming version 0.97.0 or do we have to wait for 0.97.1?

@MartinHjelmare
Copy link
Copy Markdown
Member

I think we should wait until 0.98 since it's a new feature and a breaking change.

@MartinHjelmare
Copy link
Copy Markdown
Member

Please update the breaking change paragraph in the PR description with what the user needs to do to cope with the breaking change. Ie how to handle the change of entity domain for the updater entity.

@Santobert
Copy link
Copy Markdown
Member Author

Please update the breaking change paragraph in the PR description with what the user needs to do to cope with the breaking change. Ie how to handle the change of entity domain for the updater entity.

Done

@MartinHjelmare
Copy link
Copy Markdown
Member

@balloob ok to merge?

@Santobert Santobert changed the title Updater is always available and shows on/off wether an update is available Updater component is always available and shows on/off depending on whether an update is available or not Aug 7, 2019
@balloob balloob merged commit c3455ef into home-assistant:dev Aug 7, 2019
@Santobert Santobert deleted the edit_updater_component branch August 7, 2019 22:29
@lock lock Bot locked and limited conversation to collaborators Aug 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Updater Component is not available

5 participants