Add non-blocking spi::Send trait

[david-sawatzke]

Also add a default implementation for FullDuplex, that only needs an additional read function. Implies some constraints resulting from #120.

I also want to do an spi::Read trait, but the api seems less clear cut, as you have to start an spi transaction and read it at a later time, so two distinct things.

[therealprof]

I think the flush() is not only a mistake API-wise but also superfluous: You can only read() data when you have send() data before, it's the job of the HAL impl to make sure that read() only returns a word if the hardware actually has read a word by checking appropriate status flags for "data received" which should be reset at the beginning of a send() operation (for the most simplistic I/O implementation). A "flush" operation would only make sure that the data was sent out, not that the data from the MISO was fully read in (which should ideally be at the same time but one shalt not make wild assumptions about all possible implementations) so one cannot rely on the status flag for "data transmitted" such an implementation would use.

[david-sawatzke]

flush() is not for checking if read() would succeed, it's for checking if it's safe to disable slave select. As such, just caring about the data being transmitted is the intended result, without caring about the read data. Implementation details may necessitate waiting until the data is fully read, but it's not required.

If an implementation only provides the Send trait (e.g. because the miso line isn't connected), there's no other way to determine if the write transfer is done, so we have to provide at least a similar method.

[therealprof]

If an implementation only provides the Send trait (e.g. because the miso line isn't connected), there's no other way to determine if the write transfer is done, so we have to provide at least a similar method.

I have not seen a SPI peripheral which actually cared whether you have anything connected to the SPI (physically or logically). Clocking out and clocking in are supposed to happen at the same so you might as well do a dummy read to make sure you're not deasserting the CS/SS before communication is really done.

[david-sawatzke]

Yeah, but if you don't have an miso line and can thus only implement Send, the implementation shouldn't provide a read function, as there's no real data to read here. That means the driver can't use the read function to check and something else has to be provided.

[therealprof]

Yeah, but if you don't have an miso line and can thus only implement Send, the implementation shouldn't provide a read function, as there's no real data to read here. That means the driver can't use the read function to check and something else has to be provided.

So you're actually planning to introduce a HalfDuplex trait for that? 🤔

[david-sawatzke]

HalfDuplex? No, I'm not reading anything, only writing.

If we had a separate Read trait, then it'd be able to represent a half duplex line by implement Write and Read, but not FullDuplex.

[therealprof]

HalfDuplex? No, I'm not reading anything, only writing.

If we had a separate Read trait, then it'd be able to represent a half duplex line by implement Write and Read, but not FullDuplex.

You speak in riddles. Anyway, my point was we don't have separate Read/Write traits, only FullDuplex.

[david-sawatzke]

Ok, I'm just going to do a brain dump:

There are a couple of common spi configuration as i understand them, these are a subset :

1. FullDuplex, writing and sending at the same time
2. Write only, not having a MISO line, useful for lcds or some shift registers
3. Read only, not having a MOSI line, useful for some shift registers, no idea if there's other peripherals
4. HalfDuplex, having one common data line that switches between being slave driven or master driven

(2) & (3) are both (usually) a subset of (1).

Currently the non-blocking traits can only represent the first one. That means drivers can't declare that they're only going to Read or Write. Which means that if you have a driver that only Reads/Writes, you can't easily just use a configuration like (2) or (3). You'd need to inspect the driver code you use and repeat that with every update. In the worst case, you have made a board using (2) or (3) and the driver switches to also using the other data line. Now you can't update the driver.

This could happen with LCDs, as they commonly have a MOSI & MISO line, but the MISO line is often unused by drivers.

Having a Send trait allows drivers to say in their api that they only need (2) and changing that would be an api change, thus needing a semver upgrade. It also allows hal implementations to accurately represent an SPI Peripheral that only has SCK & MOSI connected (or implemented in software/hardware).

This is also the case for a potential future Read trait.

I think an implementation that only implements Send & Read traits could accurately model a HalfDuplex spi line, but I haven't thought too much about that.

If we have a Send trait, it can't provide a read method, as there's nothing to read (otherwise you'd use FullDuplex). That means we have to provide some way of making sure that all data is clocked out, otherwise determining when to release slave select is going to be hard/impossible. That is the purpose of the flush method.

Anyway, my point was we don't have separate Read/Write traits, only FullDuplex.

That's what i want to change with this PR.

[therealprof]

(2) & (3) are both (usually) a subset of (1).

"Officially" (2) and (3) do not exist so they're not a subset but a special case.

Which means that if you have a driver that only Reads/Writes, you can't easily just use a configuration like (2) or (3).

Every SPI master on this planet can do real 4-wire SPI, that is exactly how implementations work today. I have not idea why someone would add separate implementations to handle simplex SPI communication when you simply use a regular SPI communication and not (physically and/or logically) connect a GPIO to the peripheral.

If those traits are implemented "correctly" they can't be used to compose a FullDuplex trait because e.g. for STM32 this would mean switching into Rx-only/Tx-only mode which does not compose (apart from the fact the Rx-only mode has totally weird semantics with the SPI clock signal being constantly on).

Creating these traits would mean that drivers might actually use them which would bring the HALs not implementing them (for whatever reasons) into the trouble to not being able to support the driver.

I think an implementation that only implements Send & Read traits could accurately model a HalfDuplex spi line, but I haven't thought too much about that.

Don't know, using SISO instead of MISO/MOSI is somewhat special; it can be bitbanged but I don't think all SPI hardware can directly support it.

That is the purpose of the flush method.

It is still a misnomer. Flush has a different meaning in computer science.

[david-sawatzke]

Creating these traits would mean that drivers might actually use them which would bring the HALs not implementing them (for whatever reasons) into the trouble to not being able to support the driver.

In the simplest case, you could just implement flush as self.read().map(_: ()). This would still guarantee that it's all clocked out. No need to go into a special Rx/Tx only mode, especially if it doesn't work for the intended purpose.

(I think) We could also implement the Send trait for the FullDuplex trait, then we wouldn't have to count on the hals to implement this trait.

and not (physically and/or logically) connect a GPIO to the peripheral.

Yeah, but then you have the issue that you have to verify all the code you depend on for every single update to make sure it doesn't utilize it. There also exists a widely used tx only software implementation (shiftOut).

(Probably) One of the Reasons why serial has a separate Read x & Write trait even though I can count the (hardware) tx only implementations i know on one hand (esp8266). Otherwise you'd give a driver a (potential) FullDuplex serial trait and hope that they never do the non-intended read/write, since almost all Serial ports can do that. It's makes very little sense to split it up otherwise (for the drivers), since the likelihood that two drivers and their accompanying peripherals use the same baudrate is pretty low.

Flush has a different meaning in computer science.

But it's used with the exact same meaning in the serial::Write trait, so I'd rather stay consistent.

"Officially" (2) and (3) do not exist so they're not a subset but a special case.

A very common special case, probably used more often than the FullDuplex case in the 'Maker' scene (if you look at the amount of shift registers & lcds).

You currently have some driver crates that do their own software tx-only spi implementation (https://github.com/kellerkindt/pcd8544), which can't be what we want, if they could utilize a non-blocking Send. (They should first start using the blocking Write trait, but it's better to do more)

Why would we even have Write this wasn't an intended use case? It could just be a part of the transfer trait if it was only intended to be used with FullDuplex spi.

[therealprof]

Yeah, but then you have the issue that you have to verify all the code you depend on for every single update to make sure it doesn't utilize it.

How come?

(Probably) One of the Reasons why serial has a separate Read x & Write trait even though I can count the (hardware) tx only implementations i know on one hand (esp8266). Otherwise you'd give a driver a (potential) FullDuplex serial trait and hope that they never do the non-intended read/write, since almost all Serial ports can do that. It's makes very little sense to split it up otherwise (for the drivers), since the likelihood that two drivers and their accompanying peripherals use the same baudrate is pretty low.

That is a completely different situation. Serial communication is modelled after a regular I/O model because that's how it operates, you can have a receiving channel, you can have a sending channel, both are independent from one another and everything you communicate with are just self-contained Words at a certain speed with a certain signalling. In theory the communication parameters don't even need to be the same although for practical reasons they usually are.

But it's used with the exact same meaning in the serial::Write trait, so I'd rather stay consistent.

It's also inaccurate there. No reason to repeat mistakes.

A very common special case, probably used more often than the FullDuplex case in the 'Maker' scene (if you look at the amount of shift registers & lcds).

So? (BTW: LCDs are quite often FullDuplex, some have built-in memory, many allow configuration read-back)

You currently have some driver crates that do their own software tx-only spi implementation (https://github.com/kellerkindt/pcd8544), which can't be what we want, if they could utilize a non-blocking Send. (They should first start using the blocking Write trait, but it's better to do more)

I don't follow. Whatever the reason may be to not use the SPI implementation and instead to bitbang it, I don't see what it has to do with the lack of a Simplex trait. Maybe it was the lack of SPI peripherals in a particular MCU, maybe it wasn't deemed necessary due to the low resolution, maybe the implementation requires some additional synchronisation of signals to the SPI clock which is not easily possible with our traits...

[david-sawatzke]

So? (BTW: LCDs are quite often FullDuplex, some have built-in memory, many allow configuration read-back)

They can be, but it's often used in a tx-only way

How come?

You do a board design without a MISO pin, so you don't have to route it and you have one more pin to use otherwise, an often rare resource. You (of course) have an already existing peripheral, like an lcd, that can be used without miso. But you (or the hal implementation) has to provide a FullDuplex spi instance, since there's nothing less. Now, one day your driver updates to also use the read method. Everything's broken. You have to now find out that this happened. Alternatively, the driver only requires a Send trait, which you can happily provide. If the driver ever needs to read as well, it's an api change and you won't update without thinking because of a breaking-change and even if you would, it wouldn't compile because the rust type system protected you.

I don't follow

(This is probably also because of a lack of a bit-banging lib, but) We want to provide interfaces for various usages that can be implemented in multiple ways (bit-banging, native peripheral, something in between) and not force people to bit-bang, because their needed interface isn't there (a simple nb spi::Write). The example was just given to illustrate the point, while it seems like a pretty normal send-only spi implementation, I can't reason about what the author thought.

It's also inaccurate there. No reason to repeat mistakes.

Ok, changed it.

Ultimately I think it's better to provide a nb equivalent to a concept currently only existing in blocking form (the Write trait) than not to do so.

[david-sawatzke]

I've also implemented Write for FullDuplex. This limits the freedom of implementations somewhat (afaik), but it results in hal's not needing to adopt this trait if not needed for some special purpose (like the examples given).

[Rahix]

Hmm, if you want to leave the freedom of implementation you can use a pattern similar to the one used with eg. the ToggleableOutputPin

[david-sawatzke]

The default implementation would still need participation on part of the hal authors. Not sure, but default is probably more consistent with the rest of the traits and a bit nicer.

I'm also not sure if the blocking default write implementation should be changed to be based on the nb Write. It would be the nicer solution and probably force the hal authors a bit, but also a breaking change.

[Disasm]

@david-sawatzke in general you can't have a write-only SPI. Imagine that you have some weird SPI controller that generates an interrupt on every RX FIFO overflow. You still need to handle this RX data somehow or you will have an interrupt each time you send a byte. The most portable way is to handle RX every time you do a TX, even if you have MISO pin floating. Note that HAL is not only about portability across external device libraries, it's also about portability across main device controllers.
If you need to check whether you can de-assert your CS pin, you can just wait for RX to complete discarding any data that came in.

[mathk]

I suspect an XY issue

[david-sawatzke]

The most portable way is to handle RX every time you do a TX, even if you have MISO pin floating.

If you have a device that needs this, you could empty the rx fifo in the send call.

I suspect an XY issue

I want to attach shift-registers, motor-drivers, lcd screens etc. to a micro. They don't need a miso pin, since they only receive data. To be able to use that unconnected miso pin otherwise, my hal would implement an spi, that can only send. This can already be done, needing a FullDuplex implementation that errors out or returns 0 with the read call. This may break when the used device library upgrades (without needing even an minor version bump) so that it uses this function, without warning. It also seems pretty hacky. (This specific use case is also "solved" this way in the mbed lib).

This PR would introduce a way to solve this, by introducing an interface for this not particularly uncommon use case.

[Disasm]

If you have a device that needs this, you could empty the rx fifo in the send call.

And loose all the data if read for the previous byte has not called yet. Nope.

Implementing properly this as async Write will require some state, leading to non-zero overhead. You can still use embedded_hal::blocking::spi::Write for your modules which has zero cost.

[david-sawatzke]

which has zero cost.

Except the whole blocking thing (which makes any possible implementation cost negligible).

And loose all the data if read for the previous byte has not called yet.

Maybe? You could also loose it just because your device works that way. If you depend on that, the code is device-specific anyway and you don't need this trait.