Fix 'wings' mode in SmartDevice #769
Conversation
It would fail with an IndexError otherwise
There was a problem hiding this comment.
Can you please check if the two changes I suggested still work on the real hardware?
If they do, could you also please try to add a minimal test case so that we don't break this mode again in the future?
Oh, and please ignore the style check error on 3379ac6. That's from an unrelated change in the formatter, and I already fixed the offending docstring in main.
| @@ -643,7 +643,7 @@ def _write_colors(self, cid, mode, colors, sval, direction='forward',): | |||
| for [g, r, b] in colors: | |||
| self._write([0x22, 0x10, cid]) # clear out all independent LEDs | |||
| self._write([0x22, 0x11, cid]) # clear out all independent LEDs | |||
| color_lists = [] * 3 | |||
| color_lists = [0] * 3 | |||
There was a problem hiding this comment.
Looking back into how the broken line of coded ended up here (mea culpa), it seems that the intention was to create a list of four empty lists:
| color_lists = [0] * 3 | |
| color_lists = [[]] * 4 |
The first three lists would be filled in, but the forth would be left empty.
| @@ -653,7 +653,7 @@ def _write_colors(self, cid, mode, colors, sval, direction='forward',): | |||
| 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x06, | |||
| 0x05, 0x85, 0x05, 0x85, 0x05, 0x85, 0x00, 0x00, | |||
| 0x00, 0x00, 0x00, 0x00]) | |||
| self._write(msg + color_lists[i % 4]) | |||
| self._write(msg + color_lists[i % 3]) | |||
There was a problem hiding this comment.
For reasons unknown to me (don't have this particular device), it seems that the modulo 4 indexing was intentional, and that some writes should send an empty color list.
| self._write(msg + color_lists[i % 3]) | |
| self._write(msg + color_lists[i % 4]) |
Using
wingsmode currently fails with anIndexError. I didn't run the automated tests since it's just two lines and it's broken in its current state anyway.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/