NZXT 2023 RGB Controller #690
Conversation
|
(Sorry about the CI failure for MacOS, I reran it here just to test as it seems that Github changed something and it consistently fails) |
There was a problem hiding this comment.
I left a few minor notes.
[...] shipped e.g. with NZXT Kraken 240/360 Elite.
Can you explain in a bit more detail the relationship between this RGB-only controller and those coolers.
I don't see a reference of it being shipped with those coolers, and we already (supposedly) support them, so I'm a bit lost here.
| @@ -151,6 +151,7 @@ Device notes are sorted alphabetically, major (upper case) notes before minor | |||
| | Fan/LED controller | [NZXT RGB & Fan Controller (3+6 channels)](docs/nzxt-hue2-guide.md) | <sup>_hp_</sup> | 1.12.1 | | |||
| | Fan/LED controller | [NZXT Smart Device](docs/nzxt-smart-device-v1-guide.md) | <sup>_h_</sup> | 1.11.1 | | |||
| | Fan/LED controller | [NZXT Smart Device V2](docs/nzxt-hue2-guide.md) | <sup>_h_</sup> | 1.11.1 | | |||
| | FAN/LED controller | [NZXT 2023 RGB Controller](docs/nzxt-smart-device-v1-guide.md) | <sup>_p_</sup> | git | | |||
There was a problem hiding this comment.
| | FAN/LED controller | [NZXT 2023 RGB Controller](docs/nzxt-smart-device-v1-guide.md) | <sup>_p_</sup> | git | | |
| | LED controller | [NZXT 2023 RGB Controller](docs/nzxt-hue2-guide.md) | <sup>_p_</sup> | git | |
further down with the other LED-only controller.
On that note, Isn't this new controller closer to the HUE 2 family? It supports HUE 2 accessories, which the V1 family doesn't. Thus, I think it would be a better fit for nzxt-hue2-guide.md.
Or maybe it should get its own driver module and guide?
Finally, why the p (partial support) note?
| @@ -695,7 +694,170 @@ def parse_fan_info(msg): | |||
| return sorted(ret) | |||
|
|
|||
|
|
|||
| class RGBController(_BaseSmartDevice): | |||
There was a problem hiding this comment.
The class name is exposed as part of the API, so it would be better to have a minimally more descriptive name. How about NzxtRgbController?
Describe what the changes are meant to address.
Added driver support for the new NZXT 2023 RGB Controller, shipped e.g. with NZXT Kraken 240/360 Elite. I used the smart_device base class to keep the API. The control of the RGB lighting is similar to the smart devices and other controllers, the exact protocol is just a little bit different. I also added the changes @mthier to support RGB controllers with other IDs.
Fixes:
Closes: #550, #606
Related: (maybe) #541
Checklist:
docs/*guide.mddevice guides, with "new/changed in" notesliquidctl.8Linux/Unix/Mac OS man pagedocs/developer/protocolNew CLI flag?
extra/completions/New device?
extra/linux/71-liquidctl.rules(instructions in the file header)gitMRLVNew driver?
docs/developer/protocol/