KsenijaS / krakenx

Python script to control NZXT cooler Kraken X52/X62 in Linux
GNU General Public License v2.0
181 stars 20 forks source link

Pad all writes with zeros #23

Closed jonasmalacofilho closed 6 years ago

jonasmalacofilho commented 6 years ago

TL;DR

The absence of padding made some of the messages sent to the device shorter than others. This made the cooler appear unresponsive after the second run (see #10).

To fix this, this patch repurposes _flatten (renaming it to _build_msg) to both flatten and pad the messages, and applies it where it was missing. The apparently magical value of 65 bytes is borrowed from my experience with liquidctl, and is the same for both Kraken and Smart Device.


The firmware on the device probably reads a fixed amount of bytes every time, just like we do. What happens when that overflows will depend on the specific kernel and driver in use, as well as the firmware version. Maybe in the past it implemented an internal buffer but was later simplified, because the developers felt the device did not need to handle these cases if CAM simply pad all messages.

As for the number of bytes to write, I specifically tested 65 and other (lower) values with the krakenx codebase: 32 (the maximum number of non-zero bytes sent), 33 (that plus one) and 64 (a more obvious choice), as well as null-terminated messages with no extra padding (which would not make sense, but could still be easily tested).

Partially for fun, I then ran the script bellow with an adapted _build_msg. The "output" was, indeed, 65.

import os
import subprocess
import time

env = os.environ.copy()
write_len = 70

while write_len >= 0:
    print('=> write_len={}'.format(write_len))
    env['WRITE_LENGTH'] = str(write_len)
    # use the ring leds to track whether this write_len worked or not:
    # run colctl twice, first re-rendering the previous write_len value, then
    # overwritting it with the current one to mark it as working
    for i in range(0, 2):
        args = ['colctl', '-m', 'solidall', '-fs', '80', '-ps', '80', '-cc', '8', '-c' '0,0,0']
        # "render" the last known good write_len on the ring, in binary;
        # not that easy to read (because of the infinity mirror), but works
        for j in range(0, 8):
            args += ['-c' + str(j), '255,0,64' if (write_len + 1 - i) & (1 << j) else '255,255,255']
        subprocess.run(args, env=env)
        time.sleep(1)
    write_len -= 1
  @classmethod
  def _build_msg(cls, *args):
    import os
    payload = list(itertools.chain(*args))
    return payload + [0]*(int(os.environ['WRITE_LENGTH']) - len(payload))
jneumaier commented 6 years ago

Thank you! Sorry for the late verification. I will try to invest some more time for the project on Sunday.