Add Candeo LC20 smart LED controllers, RD1/RD1P rotary dimmers #4234
Conversation
…ry dimmer switches Adds support for the following Candeo devices: C-ZB-LC20-Dim / C-ZB-LC20-DIM C-ZB-LC20-CCT C-ZB-LC20-RGB C-ZB-LC20-RGBW C-ZB-LC20-RGBCCT C-ZB-RD1 C-ZB-RD1P-DIM C-ZB-RD1P-REM C-ZB-RD1P-DPM @TheJulianJES would you mind giving us some pointers should there be any need for unit tests for these?
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #4234 +/- ##
==========================================
+ Coverage 92.05% 92.16% +0.10%
==========================================
Files 350 360 +10
Lines 11602 11966 +364
==========================================
+ Hits 10680 11028 +348
- Misses 922 938 +16 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Fix pre-commit.ci failure
|
You can remove |
|
@puddly I thought direction was a required field, If I remove it then I get an error:
Apologies if I've misunderstood what you meant! |
|
Make sure you're using the latest version of zigpy and all of the other packages. |
Ah right, we're just on the latest HA release (2025.7.4), we'll switch to the beta channel and recheck. |
Removed direction= from all of the command definitions.
Removed unneeded import.
|
@puddly we've updated the code, please let us know if anything further is required. Thanks for your help! |
TheJulianJES
changed the title
TheJulianJES
added
needs review
Use actual data value for device automation triggers rather than enum. Using an enum throws an "unsupported schema data type" error in HA itself when creating an automation.
TheJulianJES
added
manufacturer
|
Hi @TheJulianJES - will this merge make into the next release? I see the smash label there for a couple of weeks - I'm just double checking that there's nothing more you need from us? Thanks |
There was a problem hiding this comment.
I'll come back to this shortly.
Except for the English strings in the device triggers for the dimmer switches, everything looks good.
Since the smart LED controller quirks are also in this PR, I can't just merge that part, unfortunately. It's always nicer to have separate PRs if possible.
| ) | ||
| .device_automation_triggers( | ||
| { | ||
| ("Pressed", "Rotary knob"): { |
There was a problem hiding this comment.
I'll come back to you on this, but I believe we should not have English strings in here. Instead, we should use translation keys (with the translations in ZHA).
There was a problem hiding this comment.
Hi @TheJulianJES - is this feedback confirmed?
Waiting for your feedback so we can do what's required (if anything). - Thanks
There was a problem hiding this comment.
Similar to other device triggers, use something like that here (e.g. see const.py and other quirks). We'll then use that string as the translation key in HA's ZHA integration with a translation there.
So, we'll need to add the trigger strings to HA's ZHA integration here:
https://github.com/home-assistant/core/blob/00955b8e6af26d5a73386a2773a69486e50154dd/homeassistant/components/zha/strings.json#L269-L294
I should be able to do that in the ZHA bump if you provide me with the translations we want to go with.
|
Thanks @TheJulianJES - what is the update on this please?
Yes - we'd like to do that, and we do still have several products to submit. We batched a few together this time, just due to the time it takes to get things through. If we can get this through as soon as possible, we'll look to do the PRs product-by-product going forwards. Thanks |
TheJulianJES
added
the
priority: medium
|
@TheJulianJES we've updated the code to replace the hard coded strings. We used existing consts where available, but think we'd need the following created on the HA's ZHA integration to match up with the actual device events: Hope that makes sense? Please let us know if there's anything else that we need to do. Thanks for your help! |
|
That looks good! |
| COMMAND_PRESS = "press" | ||
| COMMAND_DOUBLE_PRESS = "double_press" | ||
| COMMAND_HOLD = "hold" | ||
| COMMAND_RELEASE = "release" | ||
| COMMAND_STARTED_ROTATING = "started_rotating" | ||
| COMMAND_CONTINUED_ROTATING = "continued_rotating" | ||
| COMMAND_STOPPED_ROTATING = "stopped_rotating" | ||
|
|
||
| ROTARY_KNOB = "rotary_knob" | ||
| STARTED_ROTATING = "rotary_knob_started_rotating" | ||
| CONTINUED_ROTATING = "rotary_knob_continued_rotating" | ||
| STOPPED_ROTATING = "rotary_knob_stopped_rotating" |
There was a problem hiding this comment.
Can we just move all of them into the zhaquirks const.py file? We're gonna add translations for this in ZHA either way, so that way, other quirks can also use them.
There was a problem hiding this comment.
Yes, I don't see why not, as long as they don't conflict with or overwrite anything that's already defined?
4 participants
Proposed change
Adds support for the following Candeo devices:
C-ZB-LC20-Dim / C-ZB-LC20-DIM
C-ZB-LC20-CCT
C-ZB-LC20-RGB
C-ZB-LC20-RGBW
C-ZB-LC20-RGBCCT
C-ZB-RD1
C-ZB-RD1P-DIM
C-ZB-RD1P-REM
C-ZB-RD1P-DPM
Additional information
@TheJulianJES would you mind giving us some pointers should there be any need for unit tests for these?
Checklist
pre-commitchecks pass / the code has been formatted using Black