core icon indicating copy to clipboard operation
core copied to clipboard

Add support for multiple devices linked to a Viessmann account

Open CFenner opened this issue 11 months ago • 14 comments

Breaking change

Proposed change

This change enables the integration to display all heating devices from the configured Viessmann account.

Verified with two heatings (Vitodens 300-W) connected via two Vitoconnect 100 OPTO1 on one site.

Type of change

  • [ ] Dependency upgrade
  • [ ] 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 #103009
  • 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 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:

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 Jul 06 '23 23:07 CFenner

~~⚠️ This change leads to a naming clash as all entities are prefixed with ViCare instead of the device uid, shall this be fixed in here or in a separate pr?~~

edit: The sensors now make use of _attr_has_entity_name. In addition the device name has been extended by the serial number to avoid name clashes when the same model is used (which is not the case in my setup).

CFenner avatar Jul 07 '23 00:07 CFenner

Is anything more required from my side?

CFenner avatar Jul 26 '23 06:07 CFenner

I ran into API rate limit issues yesterday (end of month), which probably happened because of the additional requests for the second device. I guess we would need to have an ability to adjust the api call rate.

CFenner avatar Jul 27 '23 07:07 CFenner

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks :+1:

Learn more about our pull request process.

home-assistant[bot] avatar Sep 27 '23 15:09 home-assistant[bot]

@edenhaus I did a rebase in GH ui, no real world test.

CFenner avatar Sep 27 '23 16:09 CFenner

Drafting it as the above conversation resulted in requested changes.

Please mark this PR as "Ready for review" after implementing the requested changes.

edenhaus avatar Oct 02 '23 12:10 edenhaus

Is someone able to assist with the mypy findings? I'm not that use to python code, that I could fix the findings.

CFenner avatar Oct 06 '23 09:10 CFenner

I created #102254 to finish a lot of typing

joostlek avatar Oct 18 '23 17:10 joostlek

Actually there is no longer a breaking change.

CFenner avatar Oct 23 '23 19:10 CFenner

hey @CFenner I tested your PR today twice. Doesn't work.

Starting state: State yesterday evening = received API data from Viessmann with 1 component (Vitodens).

1st try: What I did : Uploaded PR##96044 on running HA. Everything runs smooth. Restart HA.

Result: Integration doesn't show any data from API.

2nd try: What I did :

  1. Deleted ViCare integration. Also config files in HA.
  2. Created a new API key in Viessmann Dev Portal.
  3. Installed ViCare integration again. API selection type "auto"

Result: Receives data from Viessman = Starting state

Then: Uploaded PR#96044 on running HA. Everything runs smooth. Restart HA.

Result: Integration doesn't show any data from API.

See debug file attached.

BTW: I am sorry that I can only provide input. I can't program :( Just have some code / network knowledge from university.

home-assistant_vicare_2023-10-30T17-43-26.214Z.log

Mnovu avatar Oct 30 '23 17:10 Mnovu

@Mnovu thanks for testing. The PR is based on the dev branch which includes a device feature that is not yet included in your release. You need to go into the file vicare/entity.py and remove the line starting with serial_number=...

CFenner avatar Oct 30 '23 18:10 CFenner

@CFenner works! Have to check this in detail at the weekend. Good night! Thanks so far, let's see how we can move this forward! Bildschirmfoto 2023-10-30 um 21 38 13 Bildschirmfoto 2023-10-30 um 21 38 21 Bildschirmfoto 2023-10-30 um 21 38 27

Mnovu avatar Oct 30 '23 20:10 Mnovu

@RikyUnreal would you be able to test this multi device PR as you stated here that you also own multiple devices?

CFenner avatar Nov 02 '23 09:11 CFenner

@RikyUnreal would you be able to test this multi device PR as you stated here that you also own multiple devices?

Hi CFenner. I have been running for months this HACS custom component https://github.com/oischinger/ha_vicare/releases/tag/1.0.0-beta.2 and it works like a charm with my two devices (I own an Vitodens and a Vitocal).

I'm sorry but I would prefer not to install another component because I don't want to risk losing all the sensors and statistics I've collected over the past few months.

After days of going crazy trying to get the Viesammn integration to work, the system is now pretty stable and I really don't want to start all over again.

RikyUnreal avatar Nov 02 '23 10:11 RikyUnreal

@CFenner CI is not passing, please fix the issues then mark the PR as ready for review

emontnemery avatar Feb 14 '24 13:02 emontnemery

I currently cannot commit anything due to KeyError: 'pylint-per-file-ignores'.

CFenner avatar Feb 14 '24 23:02 CFenner

I currently cannot commit anything due to KeyError: 'pylint-per-file-ignores'.

Do you mean you can't commit from your development environment? You probably need to update your environment. If you can't get it to work, you can bypass pre-commit by doing commit -n

emontnemery avatar Feb 15 '24 07:02 emontnemery

@CFenner Does the documentation https://www.home-assistant.io/integrations/vicare/ need to be updated, or was the limitation not mentioned there?

emontnemery avatar Feb 15 '24 11:02 emontnemery

Limitation was not mentioned. I think the docs are fine.

CFenner avatar Feb 15 '24 11:02 CFenner