jgarff / rpi_ws281x

Userspace Raspberry Pi PWM library for WS281X LEDs
BSD 2-Clause "Simplified" License
1.76k stars 615 forks source link

add ws2813 timings support #510

Open sir-go opened 1 year ago

sir-go commented 1 year ago

https://github.com/jgarff/rpi_ws281x/issues/509#issue-1475172107

Gadgetoid commented 1 year ago

I don't have any access to ws2813 LEDs to sign this off, so if anyone else has got some and willing to give it a shot it would be much appreciated.

Will need to regression test, too, since this changes some pretty fundamental stuff in the main thrust of the code that I only sometimes understand, on a good day.

Seems the main thrust is that a nibble instead of three bits gives you the extra timing resolution you need for ws2813, but does it maintain the ratios required for ws2812/ws2811?

Gadgetoid commented 1 year ago

Note also that this change conflicts #468, which I have just merged but I think you can probably accomplish pretty much the same thing starting from there.

It was on my TODO list to refactor or document that code to make it a little easier to understand, but I don't think it's too onerous.

I suspect we could probably rustle up a Python script to produce the relevant timing tables, except now with your 4 bits instead of the original 3 I think my proposal to pack as a uint32 makes a lot more sense.

My other concern for this change would be performance. If ws2811 and ws2812 pixels currently work with 3-bit symbols then adding a fourth bit makes them slower (in theory, in practise it might just mean that the hardware is clocked slightly higher to achieve the exact same timings but the timings do have some wiggle room IIRC) but this precomputed table update probably cancels that out and then some.

Anyway- thank you for taking the time to make and PR this change, and sorry I'm moving the goal posts!