Fix Shelly entity removal #147665
Fix Shelly entity removal #147665
Conversation
|
Hey there @balloob, @bieniu, @chemelli74, @bdraco, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the removal of Shelly entities when a device’s configuration switches from a switch to a light mode by injecting and processing an RPC config_changed event and waiting out a reload cooldown.
- Updated tests to simulate RPC events, advance time with
freezer, and verify entity removal and creation - Modified entity removal logic to derive
domainandunique_idvia the actual entity class
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/components/shelly/test_switch.py | Added RPC event injection, cooldown wait, and light entity assertion to test switch-to-light removal |
| homeassistant/components/shelly/entity.py | Changed removal logic to instantiate the entity class for unique_id and log removal via LOGGER.debug |
| @@ -192,8 +192,12 @@ def async_setup_rpc_attribute_entities( | |||
| if description.removal_condition and description.removal_condition( | |||
| coordinator.device.config, coordinator.device.status, key | |||
| ): | |||
| domain = sensor_class.__module__.split(".")[-1] | |||
| unique_id = f"{coordinator.mac}-{key}-{sensor_id}" | |||
| entity_class = get_entity_class(sensor_class, description) | |||
There was a problem hiding this comment.
Deriving domain by splitting the __module__ string is brittle; consider adding an explicit platform or domain attribute to the entity description or class for more robust domain determination.
Copilot uses AI. Check for mistakes.
| unique_id = f"{coordinator.mac}-{key}-{sensor_id}" | ||
| entity_class = get_entity_class(sensor_class, description) | ||
| domain = entity_class.__module__.split(".")[-1] | ||
| unique_id = entity_class( |
There was a problem hiding this comment.
[nitpick] Instantiating the full entity_class just to compute unique_id may incur unnecessary overhead; consider providing a static method or utility function to generate the unique ID without creating the entity instance.
Copilot uses AI. Check for mistakes.
|
Thanks 👍 |
Proposed change
Fix Shelly entity removal
Shelly uses conditions to remove entities when the device configuration change (for example the output type is changed from
SwitchtoLight. This logic was broken and the test just checked that Switch entity is not created in Light mode, but in fact the test didn't validate that the Switch entity is actually created to so it can be removed.Note: There is some cleanup that can be done around the changed code, I plan to do it in a following PR, I limited this PR to the minimal changes lines to fix the bug.
Type of change
Additional information
Checklist
ruff format 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.To help with the load of incoming pull requests: