rp-rs / ws2812-pio-rs Goto Github PK

I want to use this crate, but can't with my SK6812RGBW LED's. Would it be possible to add support for this like is available in the smart-leds-rs/ws2812-spi-rs crate?

https://crates.io/crates/ws2812-pio/ is still at 0.1.0 and I would prefer not to use git dependencies where I can :)

Why does the nop have a delay of T2 - 2 instead of T2 - 1?

https://github.com/ithinuel/ws2812-pio-rs/blob/37ade6dd937c62c2b9c0b0fe212c291fe5f33317/src/lib.rs#L107

Usually it's T - 1 because the instruction itself uses a cycle and then you want to wait T - 1 more cycles.

The offical raspberry pi pio ws2812 example also uses T2 - 1.

https://github.com/raspberrypi/pico-examples/blob/afd1d2008f3fb3fa7a837dd1bdf17a6fecbc57fe/pio/ws2812/ws2812.pio#L25

Hello,
i'm trying to connect to a ws2812e led strip with my pi_pico board.
I'm having a rather terrible level shifter on my setup which leads to messy signals, the level shifters struggles to push the voltage to 5v5 on the output side which leads to most of the pwm signal beeing closer to 3v3 for most of the high phases of the signal.
That's my problem really but just for the explanation.
The issue is i tested this setup with the ws2812 micropython library and had no issues with my led strip.
Once i switched to rust and your library, i started to notice issues with the signal and incorrect outputs on my led strips.
After diving into both code bases pio state machine setup i noticed a difference in calculating the clock divider value.

this is how the c implementation of the micropython code calculates the divider values.

// Frequency given in Hz, compute clkdiv from it.
uint64_t div = (uint64_t)clock_get_hz(clk_sys) * 256ULL / (uint64_t)args[ARG_freq].u_int;
if (!(div >= 1 * 256 && div <= 65536 * 256)) {
  mp_raise_ValueError(MP_ERROR_TEXT("freq out of range"));
}
clkdiv_int = div / 256;
clkdiv_frac = div & 0xff;

and this is your solution.

// Configure the PIO state machine.
let bit_freq = FREQ * CYCLES_PER_BIT;
let mut int = clock_freq / bit_freq;
let rem = clock_freq - (int * bit_freq);
let frac = (rem * 256) / bit_freq;
assert!(
    (1..=65536).contains(&int) && (int != 65536 || frac == 0),
    "(System Clock / {}) must be within [1.0, 65536.0].",
    bit_freq.to_kHz()
);

// 65536.0 is represented as 0 in the pio's clock divider
if int == 65536 {
    int = 0;
}
// Using lossy conversion because range have been checked
let int: u16 = int as u16;
let frac: u8 = frac as u8;

the problem with your solution might be that dividing the clock freq and after that multiplying it by 256 is that the calculation increases the interger maths error.
By expanding the calculation to 64 bits as in the python base solution and then multiplying it by 256 and dividing it by 256 again after the divisions is that the incluence of calcualtion errors goes down.

After i have to confess rather clumsily trying to port the python code to rust i came up with this solution and experience no further errors in my led output.

let bit_freq = FREQ * CYCLES_PER_BIT;
let div: u64 = ((clock_freq.to_Hz() as u64)  * 256u64) / (bit_freq.to_Hz() as u64);
       
assert!(div >= 1 * 256 && div <= 65536 * 256,
    "(System Clock / {}) must be within [1.0, 65536.0].",
    bit_freq.to_kHz()
);

// Using lossy conversion because range have been checked
let int: u16 = (div / 256u64) as u16;
let frac: u8 = div as u8;

Can we switch to pio 0.1.0? It's likely to block https://github.com/rp-rs/rp-hal/runs/4493715165?check_suite_focus=true.

In Cargo.toml you have

rp2040-hal = {git="https://github.com/rp-rs/rp-hal.git"}

By not specifying a branch this will fail on older toolchains that will default for master.

I'm curious how the reset signal duration value of 60us was calculated?

The timing seems very tight to the tolerances of the chip (>50us) and I can't find any code which accommodates any flush of the 8-word FIFO buffer. Ws2812::write adds 60us onto Ws2812Direct::write but that only blocks until the write to the FIFO buffer is complete, not until the PIO has processed it. In the worst case I think this lag could be -

8 (word tx only FIFO buffer) x  3 (colours) x 8 (bits) x 1.25e-6 (800kHz) = 2.4e-4 = 2.4ms

The original example from the RP Pico datasheet uses a value of 10ms. This seems large enough to cover any required flush and the reset signal itself. However, 60us does not.

Please note I haven't taken a probe to the actual output or observed any real-world problems. I'm just trying to understand it from the specifications and the code written. A perfectly reasonable answer would be that someone attached a scope to the pin and observed the real-world timings (assuming sufficient data to fill the buffer)!