-
Notifications
You must be signed in to change notification settings - Fork 663
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
Add support for innr RB 285 C #91
Conversation
And add innr devices to the README
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 does the quirk do that ZHA doesn't handle on its own? I think I see it... device type reassignment?
'manufacturer': 'innr', | ||
'model': 'RB 285 C', | ||
'profile_id': zha.PROFILE_ID, | ||
'device_type': 269, |
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.
Please use a constant for this.
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.
Sorry, should've read the contributer documentation about this, just copied the existing innr quirk.
I found out that this device type is an 'Extended Colour Light', so probably we need to add support to home assistant/zigpy for these types so we don't need quirks. What do you think?
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.
The device types are defined in Zigpy here: https://github.com/zigpy/zigpy/blob/a607f6284cd9dffbe7030ba265f9c7d3a02d95d1/zigpy/profiles/zha.py#L7 and here: https://github.com/zigpy/zigpy/blob/a607f6284cd9dffbe7030ba265f9c7d3a02d95d1/zigpy/profiles/zll.py#L7
and then that info is linked to domains in ZHA/HA here:
https://github.com/home-assistant/home-assistant/blob/b71baef7c8effa036c17e4955bdf829580b1401a/homeassistant/components/zha/core/registries.py#L112
so you could PR the changes there instead and avoid the quirks all together.
# output_clusters=[33]> | ||
242: { | ||
'profile_id': 41440, | ||
'device_type': 97, |
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.
ditto
# input_clusters=[] | ||
# output_clusters=[33]> | ||
242: { | ||
'profile_id': 41440, |
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.
ditto
], | ||
}, | ||
242: { | ||
'profile_id': 41440, |
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.
ditto
}, | ||
242: { | ||
'profile_id': 41440, | ||
'device_type': 97, |
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.
ditto
Thanks for the pointers on how to fix this @dmulcahey! zigpy/zigpy#176 has been merged, and I just opened home-assistant/core#24429. When both are merged I think the quirks for the innr RS 228 T can be removed from this repository, as that bulb also does device type reassignment, but then for the newly added |
sounds good. Thanks for the contributions!!! |
:) Will close this PR since this doesn't make sense anymore. |
So the color is adjustable and the light is dimmable.
And add innr devices to the README