[ws2812] resumable write and dma example

[hshose]

Tested in hardware

[salkinium]

The driver was explicitly written as blocking, because any pause in the SPI transfers will break the timing protocol of the LEDs. See the caveats.
This should be implemented using DMA SPI ideally.

[chris-durand]

This should be implemented using DMA SPI ideally.

That seems to be the intention of this change. If you look at the example you'll see DMA SPI is used. It won't work without DMA anymore, though.

[salkinium]

Ah, you're right, I only looked at the implementation. 🙈
If we could if constexpr on the DMA version of the SPI, then we can implement both versions, ie. non-blocking for DMA on STM32, and blocking for non-DMA on everything else?

[rleh]

I just don't see a nice way to determine in a driver if the SPI master uses DMA, so I would suggest adding a (constexpr) function bool usesDma() to all SPI master implementations and then we should be able to use something like the following code:

template<typename T = void>
std::enable_if_t<SpiMaster::usedDma(), modm::ResumableResult<void>>
write()
{
	return SpiMaster::transfer(data, nullptr, length+1);
}

template<typename T = void>
std::enable_if_t<! SpiMaster::usedDma(), void>
write()
{
	for (const auto value : data) {
		while (not SpiMaster::Hal::isTransmitRegisterEmpty()) ;
		SpiMaster::Hal::write(value);
	}
}

if constexpr would read nicer, but the return type of the function has to change...

[salkinium]

I think we should keep the function signature as RF for both implementations, otherwise you cannot write platform-independent code here.

[chris-durand]

@rleh In C++20 you could write that code without std::enable_if:

modm::ResumableResult<void>
write() requires SpiMaster::usesDma
{}

or something similar.

I agree with @salkinium, that we should keep the resumable function signature for both versions to allow platform independent code.

[rleh]

In C++20 you could write that code without std::enable_if: [...] or something similar.

Something similar is actually:

template<typename T = void>
static void
write()
requires (SpiMaster::usesDma())
{}

See details in compiler explorer.
I wanted to solve this with concepts but I couldn't figure it out earlier...

---

I think we should keep the function signature as RF for both implementations, otherwise you cannot write platform-independent code here.

Agree 👍🏽
(And then if constexpr works fine.)

[chris-durand]

You don't need the template<typename T = void> and in case usesDma is a variable parentheses are also not required. The code works exactly as proposed, see Compiler Explorer 😉.

[hshose]

Code works with DMA and non-DMA SPI (see both examples). Not sure, if you want to keep the copy of the F411/ws2812 for the F469 board. I used this for testing the non-dma version because I don't have an F411 board around...

[hshose]

LOL I think I don't get @chris-durand suggestion to actually use the DMA version...

modm::ResumableResult<void>
write() requires (SpiMaster::usesDma())
{}

The parenthesis seem to be neccessary?

[chris-durand]

The parenthesis seem to be neccessary?

If you defined a variable static constexpr bool usesDma{true}; instead of a function it would work. You can leave it as is if you prefer the function.