Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DOC] RGB Matrix - Add blurb and sample to get RGB struct in proper GRB/RGB/BGR orde #24354

Open
iamdanielv opened this issue Sep 1, 2024 · 2 comments

Comments

@iamdanielv
Copy link

Issue Description

I would like to add a blurb and sample code to the RGB Matrix Documentation

I was having an issue with the RGB struct in color.h:

typedef struct PACKED rgb_led_t {
#if (WS2812_BYTE_ORDER == WS2812_BYTE_ORDER_GRB)
    uint8_t g; uint8_t r; uint8_t b;
#elif (WS2812_BYTE_ORDER == WS2812_BYTE_ORDER_RGB)
    uint8_t r; uint8_t g; uint8_t b;
// removed some of the other defines that I am not using
#endif
} rgb_led_t;

typedef rgb_led_t RGB;

My controller is using WS2812_BYTE_ORDER_GRB.
When I was using the struct I was trying to do something like this:

(RGB){RGB_RED}
// which would get converted to something like:
(RGB){ g=0xFF, r=0x00, b=0x00 }

Problem is that it has the g and r values flipped, since the struct gets defined as:

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

After asking for help on the Disord channel, @elpekenin suggested using something like this:

static inline const rgb_led_t get_rgb_led(uint8_t _r, uint8_t _g, uint8_t _b) {
    return (rgb_led_t) {.r = _r, .g = _g, .b = _b};
}

This works and properly handles all combinations of the defined rgb_led_t struct.

As a user I can then call something like:

rgb_led_t my_indicator = get_rgb_led(RGB_RED);

and get a properly defined rgb_led_t regardless of which ordering the struct is using.

I propose to add the above sample code snippet to the documentation and a blurb on how to use it.

I can write the documentation and create a PR if the team feels this is a good idea.

Having something like this would have saved me a lot of time, since I thought there was something I was doing wrong or had wired something wrong where it was just a matter of the values being assigned wrong.

@fauxpark
Copy link
Member

fauxpark commented Sep 1, 2024

Really, rgb_led_t should not need to know anything about any specific LED driver in the first place.

I'm already working on refactoring the WS2812 driver API to match the other LED drivers and move the handling of byte ordering to an internal ws2812_led_t array:

void ws2812_init(void);
void ws2812_setleds(rgb_led_t *ledarray, uint16_t number_of_leds);

// Becomes

void ws2812_init(void);
void ws2812_set_color(int index, uint8_t red, uint8_t green, uint8_t blue);
void ws2812_set_color_all(uint8_t red, uint8_t green, uint8_t blue);
void ws2812_flush(void);

@iamdanielv
Copy link
Author

This issue will be fixed with #24364 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants