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

Fix device conflict with the official daikin integration #253

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

andrein
Copy link

@andrein andrein commented Jul 5, 2024

This will make the devices discovered by the daikin_onecta integration not match the devices discovered by the daikin integration, such that HA will create two independent entities. This way, users who wish to use both integrations can disable the duplicate devices easily.

I had to remove the old devices from .storage/core.device_registry and restart home assistant to recreate the devices.

I don't believe most users will be impacted by this change, the integration works fine with the old device data.
I am open for suggestions on how to handle this entity migration automatically.

Fixes #240

@andrein andrein force-pushed the fix-daikin-device-conflict branch 2 times, most recently from a2ab283 to 81e57a1 Compare July 5, 2024 22:52
@andrein andrein force-pushed the fix-daikin-device-conflict branch from 81e57a1 to 2f47d5e Compare July 5, 2024 23:26
Copy link

codecov bot commented Jul 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.33%. Comparing base (9d0e56c) to head (2f47d5e).
Report is 4 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #253   +/-   ##
=======================================
  Coverage   95.33%   95.33%           
=======================================
  Files          14       14           
  Lines        1630     1630           
=======================================
  Hits         1554     1554           
  Misses         76       76           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jwillemsen
Copy link
Owner

I don't like this proposed change, also for example the unify integration shows some entities under the altherma device as it also uses the mac address

@andrein
Copy link
Author

andrein commented Jul 14, 2024

I did a very broad review of the official HA components and I couldn't find any cloud integration that exposed MAC addresses in the DeviceInfo class.

I understand that no longer merging the entities provided by the unify integration could be annoying to some, but the entities will still be available, it won't affect dashboards.

On the other hand, people using both the official daikin integration and this one are stuck with duplicate entities they can't easily get rid of. This PR started after your suggestion in #240 (comment), which would work if this PR is merged.

What's your final stance on this? Do you see a better solution? I've been running this patch for ~1 week now and it fixes the issue of duplicates for me. I don't mind using my own fork for the foreseeable future , but would rather get the changes upstream if possible.

@jwillemsen
Copy link
Owner

Just back after a week of vacation, need to look more into this and test a few things. The concept that a single device is exposed by multiple components seems to match the HA concepts (see https://developers.home-assistant.io/docs/device_registry_index/).

@andrein
Copy link
Author

andrein commented Jul 15, 2024

I totally agree with the fact that a device's entities might be exposed by different components, the example you gave with the UniFi integration is a good one, they complement each other. But the daikin / dainkin_onecta interaction is pretty weird right now since they duplicate almost all the entities, so it makes it look like each AC unit has two thermostats.

Maybe I went about it the wrong way. At first I tried to add a "skip devices" entry in the config flow. I could select the devices created by the daikin integration. I got their device IDs, but I'm not sure how to match those with what the Onecta API returns. Do you think this is worth pursuing further?

@jwillemsen
Copy link
Owner

I need a little bit more time to think about this. In my setup I don't have AC units with local control, but I do have a somfy tahoma box, that exposes some of the sensors of the Daikin AC units also, but only the temperature ones readonly, no control. I don't have added those to my dashboard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Issue]: Add ability to exclude devices
2 participants