Add vicare config flow #56691

oischinger · 2021-09-26T18:15:55Z

Proposed change

Adds Config flow / YAML deprecation for Vicare

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New integration (thank you!)
New feature (which adds functionality to an existing integration)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Additional information

Config flow / YAML deprecation

Support discovery via MAC address
Support import of YAML config
Switch to ConfigEntry, get rid of platform setup
This PR fixes or closes issue:
This PR is related to issue:
Link to documentation pull request:

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
The code has been formatted using Black (black --fast homeassistant tests)
Tests have been added to verify that the new code works.

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

Documentation added/updated for www.home-assistant.io

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

The manifest file has all fields filled out correctly.
Updated and included derived files by running: python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt.
Updated by running python3 -m script.gen_requirements_all.
For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

No score or internal
🥈 Silver
🥇 Gold
🏆 Platinum

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

emontnemery · 2021-09-27T11:36:41Z

We require tests for config flows, please add that.

oischinger · 2021-10-02T20:34:47Z

@emontnemery I added some config flow tests which lead me to also handle authentication erros ;)

Sorry for the force-push but the original commit was only modified to be rebased.

Also another update to latest PyVicare was necessary due to some REST API changes.

emontnemery · 2021-10-05T14:47:16Z

Do the breaking changes in PyViCare have any impact on the Home Assistant vicare integration?

emontnemery · 2021-10-05T14:47:46Z

There are still failing tests, please fix them.

oischinger · 2021-10-05T15:35:56Z

Do the breaking changes in PyViCare have any impact on the Home Assistant vicare integration?

Yes, instead of having a single "Device" the API now provides a device with multiple circuits and burners.

This allowed the removal of the CONF_CIRCUIT_ID parameter since the integration can now iterate over the data provided by all circuits and create all possible sensors.

This could have been done in a separate PR but it would then involve future changes to the configuration schema. I guess we should avoid that.

oischinger · 2021-10-06T20:05:37Z

There are still failing tests, please fix them.

Fixed. Sorry for the delay.

oischinger · 2021-10-10T18:08:05Z

Seems Viessmann is changing things on their API and we won't be able to fix that without the latest PyViCare release (#57304)

oischinger · 2021-10-12T19:44:33Z

@emontnemery How should we move on. Is this PR ok for a review or should we split it up into a library update PR and a config flow PR?
The problem with the latter is that the config flow PR would still need to include the changes for multiple circuits/burners. So it might not be much smaller.

emontnemery · 2021-10-13T06:44:59Z

@oischinger let's split the PR in two, one adjusting for the new library and one for config flow + multiple burners

oischinger · 2021-10-14T19:32:28Z

PR for the library update created: #57700

With PyViCare 2.8.1 a breaking change was introduced which required changes on sensor and binary_sensor platforms. See Home Assistant Core PR home-assistant/core#57700 As an intermediate step the circuit ID config option is still present but ignored. Now all circuits are supported at the same time. As a 2nd step the option will be removed completely since the integration will introduce setup via the UI (see this PR: home-assistant/core#56691 )

oischinger · 2021-10-17T16:35:16Z

@emontnemery
I've split out the library PR into #57700
How should I proceed with this PR: It depends on #57700 , should I

close it and reopen it once the other PR is merged?
just rebase it on top of Update PyVicare to 2.13.0 #57700

I'm wondering because I doubt that it's good to have these sort of "stale" PRs around that cannot really be reviewed before the other PR isn't merged.

emontnemery · 2021-10-18T07:18:30Z

You can mark this PR as "draft" and add a note in the PR-description that it will be updated once #57700 is merged 👍

oischinger · 2021-10-25T18:30:38Z

un-drafting this PR.
I force-pushed the branch to rebase and get rid of all unrelated changes

oischinger · 2021-10-25T20:37:20Z

Had to convert to draft again. Needs some more work

oischinger · 2021-10-27T10:26:58Z

Finally I would say it's ready for review ;)

homeassistant/components/vicare/__init__.py

emontnemery · 2021-10-27T15:26:22Z

homeassistant/components/vicare/__init__.py

+                    vol.Optional(
+                        CONF_SCAN_INTERVAL, default=DEFAULT_SCAN_INTERVAL
+                    ): vol.All(cv.time_period, lambda value: value.total_seconds()),


Why do we tweak the config schema when moving to a config flow?

Well this change is not really needed since we will delete this at a later point when yaml is officially removed from this integration.
But I still need the CONFIG_SCHEMA in init.py or are you saying I could get rid of the complete section now?

The CONFIG_SCHEMA is needed until the migration step is removed, I just mean there's no value added by modifying it since it's deprecated.

homeassistant/components/vicare/__init__.py

homeassistant/components/vicare/binary_sensor.py

homeassistant/components/vicare/config_flow.py

This includes the current HA Core dev branch plus the vicare_config_flow PR: home-assistant/core#56691

Config flow / YAML deprecation - Support discovery via MAC address - Support import of YAML config - Switch to ConfigEntry, get rid of platform setup

oischinger · 2021-11-21T07:17:54Z

@emontnemery
Could you please review again? Thanks alot!

emontnemery

LGTM, thanks @oischinger 👍

frenck · 2021-11-22T15:02:16Z

I think this PR is missing a documentation PR (and should therefore not have been merged).

@oischinger Can you make sure a documentation PR is created?

emontnemery · 2021-11-22T16:04:15Z

Oops, I missed that 🤦
@oischinger you need to update the "configuration" section; remove documentation of manual configuration and add the template for config flow configuration.

MartinHjelmare

Please address the comments in a new PR. Thanks!

homeassistant/components/vicare/binary_sensor.py

homeassistant/components/vicare/config_flow.py

tests/components/vicare/test_config_flow.py

homeassistant/components/vicare/__init__.py

homeassistant/components/vicare/config_flow.py

homeassistant added the cla-signed label Sep 26, 2021

probot-home-assistant bot added integration: vicare by-code-owner labels Sep 26, 2021

oischinger mentioned this pull request Sep 26, 2021

Add vicare config flow oischinger/ha_vicare#43

Merged

MartinHjelmare changed the title ~~Configuration via UI~~ Add vicare config flow Sep 26, 2021

MartinHjelmare added this to By Code Owner in Dev Sep 27, 2021

oischinger force-pushed the vicare_config_flow branch from c761534 to 1c25701 Compare October 2, 2021 20:31

oischinger force-pushed the vicare_config_flow branch from 1c25701 to 4cf8b03 Compare October 2, 2021 20:36

oischinger mentioned this pull request Oct 10, 2021

Vicare burner binary sensor unavailable #57304

Closed

oischinger mentioned this pull request Oct 14, 2021

Update PyVicare to 2.13.0 #57700

Merged

22 tasks

oischinger mentioned this pull request Oct 17, 2021

vicare: Remove circuit config option home-assistant/home-assistant.io#19814

Merged

8 tasks

oischinger marked this pull request as draft October 18, 2021 18:10

oischinger force-pushed the vicare_config_flow branch from 2f21ee3 to da06b6f Compare October 25, 2021 18:27

oischinger marked this pull request as ready for review October 25, 2021 18:29

oischinger marked this pull request as draft October 25, 2021 20:36

oischinger force-pushed the vicare_config_flow branch 3 times, most recently from 019cab7 to ba8e298 Compare October 26, 2021 18:28

oischinger marked this pull request as ready for review October 27, 2021 10:26

emontnemery reviewed Oct 27, 2021

View reviewed changes

oischinger added a commit to oischinger/ha_vicare that referenced this pull request Nov 1, 2021

Update to follow latest HA Core PRs

812dee3

This includes the current HA Core dev branch plus the vicare_config_flow PR: home-assistant/core#56691

oischinger added 2 commits November 13, 2021 21:55

Configuration via UI

1202710

Config flow / YAML deprecation - Support discovery via MAC address - Support import of YAML config - Switch to ConfigEntry, get rid of platform setup

Fix review comments

f05550f

oischinger force-pushed the vicare_config_flow branch from ba8e298 to f05550f Compare November 13, 2021 21:05

More tests for vicare yaml import

01ae176

emontnemery approved these changes Nov 22, 2021

View reviewed changes

Dev automation moved this from By Code Owner to Reviewer approved Nov 22, 2021

emontnemery merged commit 38b976e into home-assistant:dev Nov 22, 2021

Dev automation moved this from Reviewer approved to Done Nov 22, 2021

oischinger deleted the vicare_config_flow branch November 22, 2021 16:59

oischinger mentioned this pull request Nov 22, 2021

Adds Config flow / YAML deprecation for Vicare home-assistant/home-assistant.io#20403

Merged

8 tasks

github-actions bot locked and limited conversation to collaborators Nov 23, 2021

MartinHjelmare reviewed Dec 2, 2021

View reviewed changes

home-assistant unlocked this conversation Dec 2, 2021

frenck added the breaking-change label Dec 3, 2021

github-actions bot locked and limited conversation to collaborators Dec 4, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add vicare config flow #56691

Add vicare config flow #56691

oischinger commented Sep 26, 2021 •

edited

emontnemery commented Sep 27, 2021

oischinger commented Oct 2, 2021

emontnemery commented Oct 5, 2021

emontnemery commented Oct 5, 2021

oischinger commented Oct 5, 2021

oischinger commented Oct 6, 2021

oischinger commented Oct 10, 2021

oischinger commented Oct 12, 2021

emontnemery commented Oct 13, 2021

oischinger commented Oct 14, 2021

oischinger commented Oct 17, 2021

emontnemery commented Oct 18, 2021

oischinger commented Oct 25, 2021

oischinger commented Oct 25, 2021

oischinger commented Oct 27, 2021

emontnemery Oct 27, 2021

oischinger Oct 27, 2021

emontnemery Oct 28, 2021

oischinger commented Nov 21, 2021

emontnemery left a comment

frenck commented Nov 22, 2021

emontnemery commented Nov 22, 2021 •

edited

MartinHjelmare left a comment

Add vicare config flow #56691

Add vicare config flow #56691

Conversation

oischinger commented Sep 26, 2021 • edited

Proposed change

Type of change

Additional information

Checklist

emontnemery commented Sep 27, 2021

oischinger commented Oct 2, 2021

emontnemery commented Oct 5, 2021

emontnemery commented Oct 5, 2021

oischinger commented Oct 5, 2021

oischinger commented Oct 6, 2021

oischinger commented Oct 10, 2021

oischinger commented Oct 12, 2021

emontnemery commented Oct 13, 2021

oischinger commented Oct 14, 2021

oischinger commented Oct 17, 2021

emontnemery commented Oct 18, 2021

oischinger commented Oct 25, 2021

oischinger commented Oct 25, 2021

oischinger commented Oct 27, 2021

emontnemery Oct 27, 2021

Choose a reason for hiding this comment

oischinger Oct 27, 2021

Choose a reason for hiding this comment

emontnemery Oct 28, 2021

Choose a reason for hiding this comment

oischinger commented Nov 21, 2021

emontnemery left a comment

Choose a reason for hiding this comment

frenck commented Nov 22, 2021

emontnemery commented Nov 22, 2021 • edited

MartinHjelmare left a comment

Choose a reason for hiding this comment

oischinger commented Sep 26, 2021 •

edited

emontnemery commented Nov 22, 2021 •

edited