Fix Shelly entity removal #147665
Fix Shelly entity removal #147665
Conversation
thecode
requested review from
balloob,
bdraco,
bieniu and
chemelli74
as code owners
|
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 |
| ): | ||
| 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.
| 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.
|
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: