core icon indicating copy to clipboard operation
core copied to clipboard

Add ability to select/change the configured device in ViCare integration

Open CFenner opened this issue 5 months ago • 1 comments

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 (https://github.com/home-assistant/core/issues/107847 https://github.com/home-assistant/core/issues/107940).

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

Type of change

  • [ ] Dependency upgrade
  • [x] Bugfix (non-breaking change which fixes an issue)
  • [ ] New integration (thank you!)
  • [x] 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

  • This PR fixes or closes issue: fixes https://github.com/home-assistant/core/issues/107847 https://github.com/home-assistant/core/issues/107940
  • This PR is related to issue:
  • Link to documentation pull request:

Checklist

  • [x] The code change is tested and works locally.
  • [x] Local tests pass. Your PR cannot be merged unless tests pass
  • [x] There is no commented out code in this PR.
  • [x] I have followed the development checklist
  • [x] I have followed the perfect PR recommendations
  • [x] 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 avatar Jan 12 '24 21:01 CFenner

Review & merge https://github.com/home-assistant/core/pull/107994 first to ease the review.

CFenner avatar Jan 14 '24 09:01 CFenner

@CFenner regarding your suggestion from comment https://github.com/home-assistant/core/issues/107847#issuecomment-1892856918 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

sswierczyna avatar Jan 16 '24 11:01 sswierczyna

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.

CFenner avatar Jan 16 '24 12:01 CFenner

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 avatar Jan 16 '24 13:01 sswierczyna

Wow @CFenner I have just re-applied changes from this PR (with the commit https://github.com/home-assistant/core/pull/107906/commits/41a36666cc75faa23798233faab58ff4ae6fd577)

And so far everything seems to be perfectly fine: image

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

sswierczyna avatar Jan 16 '24 13:01 sswierczyna

Maybe this PR is not necessary, but https://github.com/home-assistant/core/pull/106477 will also fix the issues a lot of people have with accessing their heatings. Especially when we go for multi device support with https://github.com/home-assistant/core/pull/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 avatar Jan 21 '24 14:01 CFenner

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 avatar Jan 23 '24 11:01 CFenner