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 ZHA startup creating entities with non-unique IDs #99679
Fix ZHA startup creating entities with non-unique IDs #99679
Conversation
Hey there @dmulcahey, @Adminiuga, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
9177624
to
2eedfbd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
@@ -134,7 +134,9 @@ async def async_setup_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> b | |||
else: | |||
_LOGGER.debug("ZHA storage file does not exist or was already removed") | |||
|
|||
zha_gateway = ZHAGateway(hass, config, config_entry) | |||
# Re-use the gateway object between ZHA reloads | |||
if (zha_gateway := zha_data.get(DATA_ZHA_GATEWAY)) is None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not for this PR, but zha_data should be a dataclass.
self.ha_device_registry = dr.async_get(self._hass) | ||
self.ha_entity_registry = er.async_get(self._hass) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also not for this PR, but this looks like a remnant from the olden days when getting the registries required await
-ing. I'd suggest to just remove it.
@@ -134,7 +134,9 @@ async def async_setup_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> b | |||
else: | |||
_LOGGER.debug("ZHA storage file does not exist or was already removed") | |||
|
|||
zha_gateway = ZHAGateway(hass, config, config_entry) | |||
# Re-use the gateway object between ZHA reloads | |||
if (zha_gateway := zha_data.get(DATA_ZHA_GATEWAY)) is None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reusing the gateway instance requires special care in the gateway to manage the instance state and life cycle. Have we reviewed all the instance attributes of the gateway and documented that all instance attributes and future changes need to take this into consideration?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about this instead? #99694
* Make the ZHAGateway initialization restartable so entities are unique * Add a unit test
Proposed change
#98082 introduced a regression where startup retries caused entities to be re-created multiple times with the same IDs, triggering errors. I've reorganized startup to only create entities only once.
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
.To help with the load of incoming pull requests: