smart-leds-rs / ws2812-spi-rs

Use ws2812 on rust with embedded-hal spi
Apache License 2.0
64 stars 23 forks source link

RGB order vs. GRB order #7

Open d6y opened 4 years ago

d6y commented 4 years ago

Hello - thanks for this driver. We've been using it, and it's working really well, but for the ws2811 IC we're using we've noticed the byte order is RGB (as opposed to the GRB order).

What we've done is add a feature called rgb_order, and if set, it sends the bytes in RGB order. Otherwise, the default is GRB.

Here's the change:

https://github.com/BrightonRust/ws2812-spi-rs/compare/master...rgb-byte-order

Any ideas for better ways to do this? Or are you interested in the above as a PR?

Thanks!

david-sawatzke commented 4 years ago

I haven't seen any mention of a different byte order before, so I'm hoping that this variant isn't that widespread. For that reason I also don't want to merge it (yet).

If other people are affected, please comment and I'll probably add this.

As a workaround, you could also wrap the iterator and switch the bytes, e.g.:

pub struct RbgToGrb<I> {
    iter: I,
}

impl<'a, I> Iterator for BrgToGrb<I>
where
    I: Iterator<Item = RGB8>,
{
    type Item = RGB8;

    fn next(&mut self) -> Option<RGB8> {
        self.iter.next().map(|a| RGB8 {
            r: a.g,
            g: a.r,
            b: a.b
        })
    }
}

and it'll probably work.

Thank for the typo fixes.

linuxtim commented 4 years ago

FYI, Worldsemi make both the WS2811 and the WS2812.

The WS2811 datasheet can be found here (WS) or here (Adafruit).

For comparison the WS2812 here and here.

The data ordering is on page 4 of both datasheets (which are otherwise very similar).

The WS2811 is a standalone IC (no integrated LEDs), and seems to most frequently be used with higher power setups (e.g. using a 12v supply and driving 3 strings of 3 series LEDs from each chip). I don't know why they decided to to change the colour ordering between parts, presumably it was some manufacturing convenience.

The c++ fastleds library which supports many different driver chips has chosen to make the colour ordering a parameter https://github.com/FastLED/FastLED/blob/6fd236684e31c7703dfba697e86b2b8e9028b75f/examples/Blink/Blink.ino#L15

https://github.com/FastLED/FastLED/blob/13a9e0cc0999025b7ffdf92932bc6efb2199f8ea/keywords.txt#L344

linuxtim commented 4 years ago

On closer inspection, the two WS2811 datasheet links differ in their timing specs (and slightly in their pinout!) - I'd guess this is due to a newer chip revision. They also differ a little from the WS2812.

Oh well, at least the newer datasheet has a changelog.

To get the WS2811 working reliably, I adapted the existing code to encode one WS2811 bit as four bits of SPI data (this is in-spec at SPI speeds of between 3MHz and 3.44MHz - assuming the rise and fall times are instantaneous :-) ).

https://github.com/BrightonRust/ws2811-spi-rs/blob/master/src/lib.rs

(just an initial proof-of-concept, it could do with some cleanup).

After I'd done that, I saw the "nibble_bits" branch on this repo...

If you think you'll make the nibble_bits approach the default, then I could add ws2811 support to this via a feature if you like? Any thoughts?

david-sawatzke commented 4 years ago

The nibble_bits was an open pr (#8), until i had some time to test it.

The features-based approach seems a bit limiting for different devices, so I went with a PhantomData field in #9, you're welcome to add support for the ws2811 using a similar approach.

FYI: Don't take the timing requirements of the data sheets too seriously, they're usually not very accurate.

jwillikers commented 2 years ago

@david-sawatzke I recently got a NeoPixel Triple-Ring which says in the datasheet it is using the WS2812B driver, but for some reason appears to take color data using GRB ordering instead of RGB. I submitted a question to the Adafruit Technical Support forum here. The first reply indicates that's why Adafruit's NeoPixel libraries allow selecting different ordering of the color data. Not sure if that makes it more worthwhile for supporting this feature, or perhaps even just adding a helper iterator to convert a certain number of LED's to the different color ordering. I'm imagining chaining together RGB and GRB NeoPixels, where something like that could really be useful. Thanks.

laenzlinger commented 1 year ago

I have migrated my hardware to these LEDs: https://www.digikey.ch/de/products/detail/sparkfun-electronics/COM-12986/5673799

This driver works perfectly but the byte order is grb. I can work around it on my firmware. but maybe just interesting to see that there are other byte orders around.

BlinkyStitt commented 8 months ago

+1 for having some LEDs that are GRB instead of RGB. I prefer to use the more expensive 4 wire ones, but these work well enough that I don't want to throw them out.

I'll use the workaround for now. But the rgb_order feature does what I want. I'd prefer a generic over a feature though.

okhsunrog commented 1 month ago

I also had to change color order in my fork: https://github.com/okhsunrog/ws2812-spi-rs/commit/809a17a9862c6aeb7f823f85c25e69c26744d2a7 Can we add a feature to switch between RGB and GRB color order? That would be very nice, so people don't need to use forks of this lib for correct colors. about the solution with wrapping the iterator - doesn't is add extra overhead?

okhsunrog commented 1 month ago

@david-sawatzke what do you think?