Remove LIFX devices with no entities #65964
Conversation
|
|
||
| async def remove_empty_devices(self): | ||
| """Remove devices with no entities.""" | ||
| entity_reg = await er.async_get_registry(self.hass) |
There was a problem hiding this comment.
We prefer using er.async_get. The other is legacy.
| device_entry.id, | ||
| include_disabled_entities=True, | ||
| ): | ||
| device_reg.async_remove_device(device_entry.id) |
There was a problem hiding this comment.
Maybe only remove the config entry id from this device? If it's the only config entry, the device will be removed.
There was a problem hiding this comment.
I am not sure how to do that. Anyway, for LIFX I think it does not matter since there is 1 config entry, N devices and 1 entity per device?
There was a problem hiding this comment.
It could matter if another integration has the device mapped via the mac address, but that probably isn't a problem here since LiFX is the main integration of these devices.
There was a problem hiding this comment.
I see. Can you give me a hint on how to implement your proposal?
There was a problem hiding this comment.
Pass the config entry_id that you want to remove from the device entry to async_update_device.
core/homeassistant/helpers/device_registry.py
Line 414 in c93d389
| self.async_add_entities = async_add_entities | ||
| self.effects_conductor = aiolifx_effects().Conductor(hass.loop) | ||
| self.discoveries = [] | ||
| self.cleanup_unsub = self.hass.bus.async_listen( | ||
| EVENT_HOMEASSISTANT_STOP, self.cleanup | ||
| ) | ||
| self.entity_registry_updated_unsub = self.hass.bus.async_listen( |
There was a problem hiding this comment.
We can use config_entry.async_on_unload to handle the clean up.
There was a problem hiding this comment.
I want to clean up as each entity is deleted, I don't think async_on_unload would do that?
There was a problem hiding this comment.
I'm referring to the unsubscribe callback that we call when cleaning up the manager. Instead of storing that in entity_registry_updated_unsub we can pass it to config_entry.async_on_unload.
There was a problem hiding this comment.
We could also make this a bit more efficient by using async_track_entity_registry_updated_event and updating the listener when entities are added/removed since entity registry updates can be a bit of a fire hose on large systems
There was a problem hiding this comment.
@MartinHjelmare I looked into this and it seems that async_on_unload is only for deleting a config entry, it is not called on shutdown? I realize that cleanup of this listener is probably not strictly required on shutdown but I would find it odd to have two different cleanup paths.
There was a problem hiding this comment.
Also, 27k+ views say that users would like a UI button to delete a device which I think could remove the guesswork. https://community.home-assistant.io/t/delete-individual-device-and-entity-from-integration/157428
Edit: Actually around 100k views when linked posts are included.
There was a problem hiding this comment.
I don't understand. Users can still remove auto-restored entities and they will – but are then left with lingering, empty devices.
@amelchio yes, you're correct.
There was a problem hiding this comment.
That's great @bdraco.
Since it looks like there is some core movement on this feature I will not implement the post-merge comments for now.
Proposed change
Delete the matching device when a LIFX entity is removed.
We have to search through all our devices because retired lights will be restored entities that the running integration does not know about.
Type of change
Additional information
Checklist
black --fast homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all..coveragerc.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: