smart-leds-rs / ws2812-spi-rs

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

Alternative pre-rendered version using blocking::spi::Transfer #30

Closed embediver closed 2 months ago

embediver commented 1 year ago

This PR adds a second version of the pre-rendered variant utilizing the Transfer trait offered by the blocking spi hal.

The Transfer trait is implemented by default for all SPI's that implement the FullDuplex trait, but by providing a custom implementation one can easily alter the functionality (for example: implement a DMA version to mitigate timing issues).

david-sawatzke commented 1 year ago

Is there a reason you use Transfer and not the blocking Write?

I also think using combining the idle and data transfer calls makes more sense to avoiding timing issues (otherwise there might be spurious high levels while setting up the next DMA transfer.

embediver commented 1 year ago

Thanks for your time reviewing the PR.

No idea why I didn't use the Write trait, probably because I ended up using it while experimenting how to get DMA to work. I will change this.

When combining the idle and data write calls, we will have to pass one contiguous slice, which will either result in the user provided buffer needing to be bigger or a copy of the whole buffer before sending. The latter further has the problem that the user can't control the initialization of the buffer which is accessed by the hardware (which might cause problems when e.g. the buffer has to be 'static ).

Also I noticed that, when using the Write trait, it results in exactly the same behaviour as the prerendered variant when the default implementation for FullDuplex is used. Altering the prerendered variant to use the Write trait should thus be no problem.

david-sawatzke commented 2 months ago

This isn't a distinction offered by e-h 1.0.0 anymore