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

[FEATURE REQUEST] Custom color order for SK6812 compatible strips #540

Open
pawelsky opened this issue May 12, 2024 · 9 comments
Open

[FEATURE REQUEST] Custom color order for SK6812 compatible strips #540

pawelsky opened this issue May 12, 2024 · 9 comments

Comments

@pawelsky
Copy link

pawelsky commented May 12, 2024

I recently came across a WS2814/SK6812 compatible strip with (unusual?) WRGB color order, which I unfortunately cannot configure properly to use with this great library. The closest I can get is SK6812_STRIP_BRGW but the Blue and White are swapped.

The problem I'm facing is that currently the library assumes that W is always last, and and detects presence of W by checking only the highest nibble of the strip type

// If our shift mask includes the highest nibble, then we have 4 LEDs, RBGW.
if (channel->strip_type & SK6812_SHIFT_WMASK)
{
    array_size = 4;
}

where SK6812_SHIFT_WMASK is defined as

#define SK6812_SHIFT_WMASK 0xf0000000

The way I've temporarily solved it for myself is to check the highest bit of each byte of the strip type with the mask defined as follows:

#define SK6812_SHIFT_WMASK 0x80808080

and modified the strip types to the highest bit of the W byte enabled here

// 4 color R, G, B and W ordering
#define SK6812_STRIP_RGBW                        0x98100800
#define SK6812_STRIP_RBGW                        0x98100008
#define SK6812_STRIP_GRBW                        0x98081000
#define SK6812_STRIP_GBRW                        0x98080010
#define SK6812_STRIP_BRGW                        0x98001008
#define SK6812_STRIP_BGRW                        0x98000810

In addition to that I've modified setting the color shift values here and here to

channel->wshift = (channel->strip_type >> 24) & 0x7f;
channel->rshift = (channel->strip_type >> 16) & 0x7f;
channel->gshift = (channel->strip_type >> 8)  & 0x7f;
channel->bshift = (channel->strip_type >> 0)  & 0x7f;

Do you think such modification could be added to the library?

P.S. Not sure how popular my type of the strip is, but just in case you would find it worth adding it, here is the strip type #define for it

#define SK6812_STRIP_WRGB 0x00981008

@mattfox12
Copy link

Thanks so much for this, I was running into the exact same problem.

I wonder if this would be a better fix, mostly because the Python library was not liking those changes. If we just change the comparison:

// If our shift mask includes the highest nibble, then we have 4 LEDs, RBGW.
if (channel->strip_type & SK6812_SHIFT_WMASK)
{
    array_size = 4;
}

To:

// If any shift includes 0x18, then we have 4 LEDs, RBGW.
if (channel->strip_type >> 24 == 0x18 || ((channel->strip_type >> 16) & 0xff) == 0x18  || ((channel->strip_type >> 8) & 0xff) == 0x18 || (channel->strip_type & 0xff) == 0x18)
{
    array_size = 4;
}

It correctly adjusts to 4 colors, and the Python library likes it better

#define SK6812_STRIP_WRGB                        0x00181008

@pawelsky
Copy link
Author

Python library was not liking those changes

What was the problem?

@mattfox12
Copy link

I didn't save the exact error, but it threw type conversion fits if the strip started with '98', i.e. 0x98100800

@mattfox12
Copy link

Went back to get it, here is the Python error output:
File "/usr/local/lib/python3.9/dist-packages/rpi_ws281x/rpi_ws281x.py", line 99, in init
ws.ws2811_channel_t_strip_type_set(self._channel, strip_type)
OverflowError: in method 'ws2811_channel_t_strip_type_set', argument 2 of type 'int'
swig/python detected a memory leak of type 'ws2811_t *', no destructor found.

@pawelsky
Copy link
Author

What hardware are you using this library on? 64 or 32 bit?

@mattfox12
Copy link

mattfox12 commented Aug 22, 2024

32bit

@pawelsky
Copy link
Author

I'm not a Python expert but that could explain why you see the error. I believe on 32bit systems int is limited to 2,147,483,647, hence the overflow.

To avoid this issue you can try using not the highest, but the second highest bit to signal the W component, which this means you'd need to do the following changes:

#define SK6812_SHIFT_WMASK 0x40404040

// 4 color R, G, B and W ordering
#define SK6812_STRIP_RGBW                        0x58100800
#define SK6812_STRIP_RBGW                        0x58100008
#define SK6812_STRIP_GRBW                        0x58081000
#define SK6812_STRIP_GBRW                        0x58080010
#define SK6812_STRIP_BRGW                        0x58001008
#define SK6812_STRIP_BGRW                        0x58000810
channel->wshift = (channel->strip_type >> 24) & 0x3f;
channel->rshift = (channel->strip_type >> 16) & 0x3f;
channel->gshift = (channel->strip_type >> 8)  & 0x3f;
channel->bshift = (channel->strip_type >> 0)  & 0x3f;

#define SK6812_STRIP_WRGB 0x00581008

@mattfox12
Copy link

Thanks for the change, I'll try it out.
Will the performance difference be noticeable?

@pawelsky
Copy link
Author

pawelsky commented Aug 22, 2024

Will the performance difference be noticeable?

You won't notice any performance difference, but it will be faster than your solution ;)

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

No branches or pull requests

2 participants