-
Notifications
You must be signed in to change notification settings - Fork 31
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
Allow device cluster entities overwrite from v2 quirks #328
base: dev
Are you sure you want to change the base?
Conversation
@@ -314,7 +314,7 @@ def discover_quirks_v2_entities(self, device: Device) -> None: | |||
endpoint.async_new_entity( | |||
platform=platform, | |||
entity_class=entity_class, | |||
unique_id=endpoint.unique_id, | |||
unique_id=f"{endpoint.unique_id}-{cluster.cluster_id}", |
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.
I think this will break all existing v2 entities and it will require a migration of the HA entity registry.
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.
Is that migration a post-install operation?
(self.PLATFORM, self.unique_id), | ||
self._device.platform_entities[(self.PLATFORM, self.unique_id)], | ||
self, |
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 is this doing?
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.
Intention was to offer a debugging message for those cases where it is not intended or expected to encounter duplicated entities. Can be removed or demoted to debug.
) | ||
self._device.platform_entities[(self.PLATFORM, self.unique_id)] = self |
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.
Same question as above
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.
It is allowing a newer entity instance to overwrite an existing one. In my specific case, an automatic zha created local temp calibration entity would be replaced by one provided by a quirk v2. At least, in my setup, the quirk v2 entity gets created last.
Entities created by a v2 quirk should be allowed to overwrite any default generated ones.
Current patch allows all overwrites and I'm not fully aware of the possible consequences outside the above mentioned use case.
Fixes home-assistant/core#128253.