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 ability to select/change the configured device in ViCare integration #107906

Closed
wants to merge 24 commits into from

Conversation

CFenner
Copy link
Contributor

@CFenner CFenner commented Jan 12, 2024

Proposed change

This PR adds an options flow and adds the ability to select the device that should be used for this integration (instead of the first one). This is also possible, when the integration is setup from scratch.

This enabled users to fix a bug introduced by Viessmann that leads to the wrong device being used (#107847 #107940).

Bildschirmfoto 2024-01-14 um 01 38 12
  • config entry migration
  • strings.json

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)
  • Deprecation (breaking change to happen in the future)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

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
  • I have followed the perfect PR recommendations
  • The code has been formatted using Ruff (ruff format homeassistant tests)
  • Tests have been added to verify that the new code works.

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

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.

To help with the load of incoming pull requests:

@CFenner
Copy link
Contributor Author

CFenner commented Jan 14, 2024

Review & merge #107994 first to ease the review.

"flow_title": "{name} ({host})",
"step": {
"init": {
"description": "Select the serial number of the device to connect. The serial number can be found on the device or in the settings of the VICare app.",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
"description": "Select the serial number of the device to connect. The serial number can be found on the device or in the settings of the VICare app.",
"description": "Select the serial number of the device to connect. The serial number can be found on the device or in the settings of the ViCare app.",

@CFenner CFenner marked this pull request as ready for review January 15, 2024 12:13
return vicare_api
async def async_update_entry(hass: HomeAssistant, entry: ConfigEntry) -> None:
"""Update a given config entry."""
await hass.config_entries.async_reload(entry.entry_id)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This seems not to work, after options changed the integration needs to be reloaded.

@CFenner CFenner marked this pull request as draft January 15, 2024 13:59
@CFenner CFenner marked this pull request as ready for review January 16, 2024 07:46
@sswierczyna
Copy link

@CFenner regarding your suggestion from comment #107847 (comment) I have applied this PR using command

curl -o- -L https://gist.githubusercontent.com/bdraco/43f8043cb04b9838383fd71353e99b18/raw/core_integration_pr | bash /dev/stdin -d vicare -p 107906 

Then i was able to select one of two devices from my vicare
image
I have selected **CU401B_S** and restarted HA

But for some reason it seems to that I still have the data about Heatbox2_SRC.
Iam expecting data about CU401B_S

image

@CFenner
Copy link
Contributor Author

CFenner commented Jan 16, 2024

Nice that you tested!

So when I look at your first screenshot, I get the impression that you get displayed two times the same serial, am I correct? Can you share your diagnostics file? This is probably the case because you have a gateway that is not recognized as such.

I will add your gateway model name to the code to get around this. Run the curl command again or do the changes yourself if you feel comfortable.

@sswierczyna
Copy link

Thanks for quick reply @CFenner

So when I look at your first screenshot, I get the impression that you get displayed two times the same serial, am I correct?

Correct, they both have the same serial numbers

This is probably the case because you have a gateway that is not recognized as such.

Not sure about that. I have just HeatPump Vitocall 200-S with communication modue Vitoconnect OPTO2
Please find attached diagnostic file:
config_entry-vicare-a21e13b619277bd151b765854465e564.json (1).txt

@sswierczyna
Copy link

Wow @CFenner
I have just re-applied changes from this PR (with the commit 41a3666)

And so far everything seems to be perfectly fine:
image

I will report how this solve problem with api request quota.
Thanks Again!

@CFenner CFenner marked this pull request as draft January 21, 2024 14:45
@CFenner
Copy link
Contributor Author

CFenner commented Jan 21, 2024

Maybe this PR is not necessary, but #106477 will also fix the issues a lot of people have with accessing their heatings. Especially when we go for multi device support with #96044.

⚠️ For those who already used this PR via the script:

curl -o- -L https://gist.githubusercontent.com/bdraco/43f8043cb04b9838383fd71353e99b18/raw/core_integration_pr | bash /dev/stdin -d vicare -p 107906 

It will migrate your vicare config entry to a version 2 to store the device you want to connect to. This leads to issues when you want to migrate back to the original version or any other branch.
The simplest way to fix this is to remove the integration entry and reconfigure it again. Another way is to fix the config entry by reverting back the version to 1 in the config/.storage/core.config_entries file.

@CFenner
Copy link
Contributor Author

CFenner commented Jan 23, 2024

As discussed with @edenhaus we should strive to make all devices available with #96044 instead of giving the ability to select on. A user can always disable devices later on.

@CFenner CFenner closed this Jan 23, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jan 24, 2024
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.

No device or entities after reinstall of the integration
2 participants