[Bug] WS2812_BYTE_ORDER may cancel itself out and not behave as expected

[YosefBayoude]

Describe the Bug

Setting WS2812_BYTE_ORDER under specific conditions will cancel itself out. What I mean by this, is when setting the directive WS2812_BYTE_ORDER for the WS2812 driver, the color order can be inverted at two separate locations and canceling itself out.

Example : For example, setting WS2812_BYTE_ORDER to GRB, will invert the color in the color.h file, as expected, we get therefore the following result :

typedef struct PACKED rgb_led_t {
    uint8_t g;
    uint8_t r;
    uint8_t b;
} rgb_led_t;

Meaning rgb_led_t myColor = {255, 0, 0} should output green, since red and green locations have been switched.

However, setting WS2812_BYTE_ORDER will also invert the order of red and green in some driver functions, such as this one.

Therefore, calling set_rgb_matrix_color(1, myColor), will invert the colors red and green again in the function, effectively canceling WS2812_BYTE_ORDER out.

This is an issue, because I would expect set_rgb_matrix_color(1, RGB_GREEN) to light up the led in green, but the way it currently works, is that this function (set_rgb_matrix_color(0, RGB_GREEN)) will always output red and more importantly, it will always have the same effect regardless of what the WS2812_BYTE_ORDER directive is. I do understand the reasoning behind it however, because set_rgb_matrix_color(0, 255, 0, 0) does light up green as expected, since it has to take into account the WS2812_BYTE_ORDER when color is passed in as separate values.

I would make a PR (removing the order set color.h) , but I'm not familiar enough with the code, and i feel that this is more an issue about code structure and intent rather than something technical.

Keyboard Used

keebio/iris/rev8

Link to product page (if applicable)

https://keeb.io/products/iris-keyboard-split-ergonomic-keyboard

Operating System

Mac OS

qmk doctor Output

No response

Is AutoHotKey / Karabiner installed

• [ ] AutoHotKey (Windows)
• [ ] Karabiner (macOS)

Other keyboard-related software installed

No response

Additional Context

No response

[sigprof]

The order of elements in rgb_led_t is defined to match WS2812_BYTE_ORDER to make the AVR bitbang driver work (unlike ChibiOS drivers, the AVR bitbang driver is really dumb and processes the rgb_led_t array as a byte sequence). So you are not supposed to write code that depends on the order of elements in rgb_led_t. There are several ways to do it:

1. Use wrapper functions with separate uint8_t r, uint8_t g, uint8_t b arguments (like rgb_matrix_set_color()); this way you can also use the defines like RGB_GREEN.
2. Use designated initializers: rgb_led_t myColor = { .r = 255, .g = 0, .b = 0}; they will do the right thing even if the order of fields in the type definition is actually different. (However, there is also an optional uint8_t w field for RGBW LEDs, which will end up initalized as 0 by the code like that.)
3. Use HSV to specify colors, then convert those colors to RGB (e.g., using hsv_to_rgb()); most of the RGB matrix code actually does that to make various color manipulations easier.

Also I'm not sure what is the set_rgb_matrix_color() function that you mention. The core function is rgb_matrix_set_color(), and it is supposed to receive the color parameters in the fixed r, g, b order, so rgb_matrix_set_color(0, RGB_GREEN) or rgb_matrix_set_color(0, 0, 255, 0) should make the LED glow green if the LED driver is configured properly.

[YosefBayoude]

Yes, my mistake, I was indeed referring to rgb_matrix_set_color(). So essentially what you are saying is that it comes down to how these drivers were implemented. In that case this issue can be closed, and perhaps left for reference (for people with the same issue).