vmakeev / huawei_mesh_router

Huawei mesh router component for Home Assistant
MIT License
53 stars 7 forks source link

Devices does not get detached from integration after removing their entities #31

Closed felipecrs closed 1 year ago

felipecrs commented 1 year ago

I disabled all device specific entities in the integration configuration, and then removed all entities from Home Assistant. But the integration still thinks it contributes to all my devices:

https://user-images.githubusercontent.com/29582865/210156339-822cc555-bf79-494c-8519-db57747b41e9.mp4

felipecrs commented 1 year ago

And there's no way to get rid of this if I want to keep using the integration, because, if I remove the integration configuration, then, when adding it back, it automatically creates these links again since device tracker entities comes enabled by default. It would also be nice to have a chance to configure the integration entities types during the initial configuration.

vmakeev commented 1 year ago

Thanks for the feedback, the component really had a problem with unloading the device_tracker platform after changing the settings.

Now, when the Devices tracking option is disabled, the component will immediately stop providing data about entities of the device_tracker type. To unlink the extra devices from the huawei_mesh_router integration, you should restart the Home Assistant so that the device registry is correctly rebuilt.

As for the appearance of "devices": this is the logic of the Home Assistant core. Entities of the type device_tracker always try to join some device by matching the MAC address. I can disable this behavior for the huawei_mesh_router component, but it seems to me that this would not be in the spirit of Home Assistant. However, disabling the Devices tracking option and restarting the Home Assistant should solve this problem in your case. Or maybe you can suggest some workaround?

It would also be nice to have a chance to configure the integration entities types during the initial configuration.

I fully agree with you on this. Unfortunately, Home Assistant strictly separates the "configuration" and "options" of integration (config flow and options flow). If I add some options to the initial integration setup, then access to change them via CONFIGURE will be lost. However, this is now bypassed by disabling unnecessary options and restarting HA.

Please try to install version v0.8.4.1-rc1, and let me know if you managed to achieve the desired results with it. If not, then I will think further how to get out of this situation.

P.S. Happy New Year!

felipecrs commented 1 year ago

Hi, thanks a lot for the reply. Happy new year for you too!

In fact I did not say, but your integration is awesome. It's going to help a lot, since I can't change the firmware of my AX3 Quad-core routers to OpenWRT, as it's not supported.

So, thank you for it.

About the issue, I tried with v0.8.4.1-rc1. Previously, the process to disable device tracker was:

  1. Setup integration
  2. It's going to create device tracker entities for everything
  3. Configure integration to disable device tracker
  4. Reload integration, so the device tracker entities can be deleted
  5. Select all device tracker entities and delete them manually
  6. The list of devices keeps populated even though the integration isn't contributing any data or entity to these devices any longer.

image

I think the difference with v0.8.4.1-rc1 is that step 4 isn't necessary anymore. The rest seems to remain the same.

To unlink the extra devices from the huawei_mesh_router integration, you should restart the Home Assistant so that the device registry is correctly rebuilt.

Nothing changes for me when I restart Home Assistant.

As for the appearance of "devices": this is the logic of the Home Assistant core. Entities of the type device_tracker always try to join some device by matching the MAC address. I can disable this behavior for the huawei_mesh_router component, but it seems to me that this would not be in the spirit of Home Assistant.

This is fine. I don't think it should be changed. For people that actually uses the device tracker feature, this is probably important.

However, disabling the Devices tracking option and restarting the Home Assistant should solve this problem in your case. Or maybe you can suggest some workaround?

Restarting Home Assistant doesn't work as mentioned. But I do have a suggested workaround. How about having Device tracking option disabled by default when setting up the integration?

image

I suppose this will prevent the integration from linking to devices, unless I go to Configure and explicitly enable it.

vmakeev commented 1 year ago

Nothing changes for me when I restart Home Assistant.

Strange behavior, usually devices lose link with integration when all related entities are deleted or inactive

How about having Device tracking option disabled by default when setting up the integration?

This is not an easy decision :) Device tracking is most often used in such integrations, disabling this option by default may mislead users.

On the other hand, the mikrotik_router integration, which I was initially inspired by, uses exactly this approach: all additional options are not enabled by default, including device tracking.

I believe that in the case of my integration, this should also not create serious problems, especially if everything is documented (although who reads the manuals? :))

In version v0.8.4.1-rc2, all additional options are disabled by default, can you try to upgrade?

felipecrs commented 1 year ago

I totally understand you, and I agree that some common options should indeed be enabled. The only reason not to do it would be to work around an issue like this one.

There doesn't seem to have any change between the two versions...

https://github.com/vmakeev/huawei_mesh_router/compare/v0.8.4.1-rc1...v0.8.4.1-rc2

I have installed it here but it behaves just like before.

vmakeev commented 1 year ago

I'm sorry, my mistake. I made a release not from the right branch, but from master. v0.8.4.1-rc3

felipecrs commented 1 year ago

Great! Everything comes disabled by default, and now devices aren't linked to the integration anymore.

chrome_OczXgBBtKH

It would be nice if we could fix the root cause of the issue, but this is already good enough as I don't plan to use any device specific feature of the integration.

One suggestion I have would be to add a text/hint in the integration initial setup flow that users can enable additional features after, by using the Configure button. This should help alleviate the fact that these options comes disabled by default now.

felipecrs commented 1 year ago

Thank you!

vmakeev commented 1 year ago

You are welcome!