-
-
Notifications
You must be signed in to change notification settings - Fork 35.8k
Add counter to ZHA entity names when a device has multiple of the same type #151434
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
base: dev
Are you sure you want to change the base?
Add counter to ZHA entity names when a device has multiple of the same type #151434
Conversation
|
Hey there @dmulcahey, @Adminiuga, @puddly, @TheJulianJES, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
|
Let's move this feature to https://github.com/zigpy/zha instead |
|
Just to clarify, due to the usage of translation keys, we'll still require changes to Home Assistant Core itself. I'm in favor of adding something like a "postfix" attribute to ZHA entities, which we can then access from HA Core here. But ZHA will then decide if and what postfix number should be used. We can likely set that just once after entity discovery is done. |
e79eb53 to
a01baf5
Compare
|
Hey, thanks for the feedback. I'll work on implementing the logic on the zha library side. I'll also update this PR with the changes required for it to work with the new version of zha once I have that PR in. I'll try implementing it like @TheJulianJES's suggested. |
a01baf5 to
5309850
Compare
|
I've now created a PR in the zha repository zigpy/zha#525 which implements the same feature on the zha library side. It implements the I've also pushed the required changes to this PR. I have assumed that my changes would be in |
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.
Nice work! Two things regarding the HA part of this implementation:
-
Let's remove the dependency bump from this PR.
Since the dependency bump doesn't need this feature to be included, we should do the bump separately. Then merge this afterwards. -
Ideally, we would have at least one test or snapshot (for entities) checking the postfix implementation. We have that on the ZHA side, but not on the HA side.
We might be able to just add that to existing sensor tests in
test_sensoror possibly even generate a snapshot (similar to how it's done intest_diagnostics.py) for a mocked device with two identical clusters (and thus identical sensors) on different endpoints, and for one with no identical clusters, so no postfix.If you wanna have a go at that, feel free to do so. Otherwise, I should be able to do this soon.
|
Thanks for the feedback! I might be a bit low on free time in the coming days but I will leave a comment here if I find the time to do it. If I don't comment, feel free to assume I haven't started work and go ahead and modify the PR. I agree on the feedback given and the suggested changes / additions seem good to me. |
5309850 to
8eb08aa
Compare
|
@TheJulianJES I went ahead and implemented your feedback. I also had a go at creating a unit test that checks that the postfix is in the name of the entity. If there are more than two identical clusters, there should be one, and if there is only one cluster, there should be no postfix. I implemented it int he |
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.
Putting this in draft until the ZHA PR is merged. I'll come back to this before then though.
I'll also try to get to your ZHA PR as soon as possible and check if there are any real devices that would cause the issues epenet mentioned there, or if we can maybe internally count using the cluster id first, but just replace all digits to start from 1, potentially skipping one digit in the case epenet mentioned.
EDIT: Described that with an example in the ZHA PR: zigpy/zha#525 (comment)
This is definitely something we want included soon, so thanks for the time you spent on this already!
Proposed change
This PR implements a feature from the ZHA backlog. It improves the readability of entities on ZHA devices which have multiple entities of the same type by adding a counter to the names of repeated entities.
Type of change
Additional information
platform_entitiesattribute. This attribute, located in thezha.zigbee.device.Deviceclass, matches the order in which entities appear in the UI.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: