todbot / blink1-tool

Command-line tools and C library for blink(1) USB RGB LED
https://blink1.thingm.com/
Other
84 stars 15 forks source link

Inconsistent Handling of `dmillis` Value Conversion in CLI Tool #67

Closed vt128 closed 9 months ago

vt128 commented 11 months ago

Issue:

While reviewing the CLI and firmware code, I noticed a potential error in the dmillis definition of CLI.

Currently, using % 0xff in the CLI tool stored in this repo causes input values like 2550ms to be interpreted as 0ms. However, the firmware code handles the value correctly by treating it as a 16-bit unsigned integer using & 0xff. I suggest using & 0xff instead of % 0xff in the CLI tool for consistent behavior.

Related Code Samples:

From CLI Tool: blink1-lib.c

buf[5] = (dms >> 8);
buf[6] = dms % 0xff;

From Firmware: main.c

uint16_t dmillis = (msgbuf[5] << 8) | msgbuf[6];

Fix:

Please consider updating all the related code in this repo to use & 0xff for consistent handling of the CLI value conversion.

todbot commented 9 months ago

Oh my goodness how embarrassing. Fixing this shortly. Thank you.