smart-leds-rs / ws2812-spi-rs

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

Blocking read from SPI never returnes when using RPPAL #26

Open BrunoWallner opened 2 years ago

BrunoWallner commented 2 years ago

I get access to SPI on the Raspberrypi-4B using rppal like this:

use rppal::spi::{Bus, Mode, SlaveSelect, Spi};

let spi = Spi::new(Bus::Spi0, SlaveSelect::Ss0, 3_000_000, Mode::Mode0)?;

Because the library automatically implements the embedded_hal traits, I can easily initialize the ws2812 driver and send data to it like this:

let mut ws = Ws2812::new(spi);

let c = RGB8 {
    r: 255,
    g: 0,
    b: 127,
};
let colors: [RGB8; 3] = [c, c, c];
ws.write(colors.iter().cloned())?;

Which works just fine and 3 Leds start to light up in the correct Color.

But the ws.write() function never returns, because it tries to read data from SPI in a blocking way. I simply tried to comment out the parts where it reads but unsurprisingly everything started to break.

What do I have to do to send data multiple times in a loop for example?

full code:

use rppal::spi::{Bus, Mode, SlaveSelect, Spi};

use ws2812_spi::Ws2812;
use smart_leds::SmartLedsWrite;
use smart_leds::RGB8;

use std::error::Error;

fn main() -> Result<(), Box<dyn Error>> {
    let spi = Spi::new(Bus::Spi0, SlaveSelect::Ss0, 3_000_000, Mode::Mode0)?;
    println!("got spi device");
    let mut ws = Ws2812::new(spi);
    println!("initialized ws2812 driver");

let c = RGB8 {
    r: 255,
    g: 0,
    b: 127,
};
let colors: [RGB8; 3] = [c, c, c];
ws.write(colors.iter().cloned())?;
    println!("sent data to strip"); // never gets executed;

    Ok(())
}
david-sawatzke commented 2 years ago

The reason for the read at the end is that for every byte that is sent via SPI, one is received.

One byte is always kept in transit, and this is resolved with the last read, where the amount of read calls matches the write calls.

I'm really surprised this works at all on the rpi, I would've thought that the individual calls would be too slow for linux.

The "proper" solution will likely be to write something special for the rpi, where a buffer matching the length of the attached strip is allocated and sent with a single call, very similar to the prerendered implementation.

BrunoWallner commented 2 years ago

I tried to rewrite the driver to buffer all of it's data that gets sent to Spi and then send that buffer with a single call.

Code ```rs use crate::spi::Spi; #[derive(Copy, Clone, Debug, PartialEq)] pub struct Rgb8 { pub r: u8, pub g: u8, pub b: u8 } pub struct Ws2812 { spi: Spi, data: Vec } impl Ws2812 { pub fn new(spi: Spi) -> Self { Self { spi, data: Vec::new() } } pub fn write_byte(&mut self, mut byte: u8) { let patterns = [0b1000_1000, 0b1000_1110, 0b11101000, 0b11101110]; for _ in 0..4 { let bits = (byte & 0b1100_0000) >> 6; self.data.push(patterns[bits as usize]); byte <<= 2; } } pub fn write_colors(&mut self, colors: &[Rgb8]) { self.data.push(0); for color in colors { self.write_byte(color.g); self.write_byte(color.r); self.write_byte(color.b); } self.flush(); // resolve the offset introduced with push(0) // todo()! } pub fn flush(&mut self) { // 300µs at 3.8Mhz low bits self.data.append(&mut vec![0_u8; 140]); } pub fn flush_spi(&mut self) { let data = &self.data[..]; self.spi.write(data).unwrap(); self.data.clear(); } } ```

And again it works but only the first time and after flushing using flush_spi() once and then sending data to the strip, the LED's turn white.

I think this is happening because I do not wait to read the data from the Spi for every byte that it sends, because I believe it is not possible with this buffering method.

Expecially troublesome is this at the write_colors function because the original driver introduces an offset in the fifo and then resolves this offset when all data is sent and flushed with another read call. In this implementation it only introduces this offset with self.data.push(0) but it does not resolve it.

Is there something I can do about it?

david-sawatzke commented 2 years ago

I haven't take a closer look yet, but you might be interested in https://github.com/smart-leds-rs/smart-leds-samples/pull/13

david-sawatzke commented 2 years ago

I've implemented something very similar in #27 and it seems to work fine on my RPi 1. I don't see any obvious reason why yours didn't work, but could you try it?

agalakhov commented 2 years ago

I observe the same issue on STM32G030J6M using the example code from stm32g0xx-hal (with i/o pins changed). The block!(read()) goes into infinite loop. The SWD debugger shows that SPI1_SR is 0x02 (transmit buffer empty, receive buffer empty). Removing block! and leaving just read() (that is, read only if there is something to read right now, don't wait) works for me.

BTW, looks like the flush() and mosi_idle_high logic is based on misunderstood datasheet of WS2812. It is really badly written at this place. In fact, WS2812 does not need idle period (called "Reset" in the datasheet) at the end of transmission. It is only needed at the beginning in order to let the first LED in the chain know where the start of the transmission is. So flush() (better rename it to start()) is needed before writing bytes, not after, even if mosi_idle_high is not set.

reitermarkus commented 10 months ago

And again it works but only the first time and after flushing using flush_spi() once and then sending data to the strip, the LED's turn white.

I had the same problem initially. You need to set core_freq=250 in /boot/config.txt to have a stable SPI frequency.