rust-embedded / nb

Minimal and reusable non-blocking I/O layer
Apache License 2.0
86 stars 15 forks source link

Permissible side-effects of calls that return WouldBlock #16

Open chrysn opened 5 years ago

chrysn commented 5 years ago

There seem to be a few problems that are caused by side-effects of calls that return WouldBlock:

The Core Idea section of the docs says that WouldBlock means that an operation would need to block in order to complete; my impression from the well-working use cases is that moreover, WouldBlock also indicates that an operation would need to block in order to start: Given the callee should not hold state about the arguments between calls, starting operations silently should not be desirable pattern.

I'd propose to-be-evaluated guidelines like this:

The WouldBlock error variant signals that the operation can't be executed right now and would need to block to complete, and that no action has been taken. WouldBlock is a special error in the sense that's not fatal; the operation was not started, and can still be executed by retrying again later.

A non-WouldBlock result indicates that the operation has been executed and completed – or at least been taken up for execution, and the result is already predictable and can be returned. Operations in which both the start and the execution may block can be split up into two parts (like embedded_hal::SPI::FullDuplex::{send,read}).

That's obviously not a change that can be just-so made (and invalidate existing use cases like embedded_hal::adc::OneShot), but more of something to keep around as a usage pattern, to try implementations against, and to see whether it can evolve into something that can at some point supersede the current description.

HarkonenBade commented 5 years ago

I strongly disagree that a WouldBlock should indicate that the action has not even been started. As how are long running tasks which must be synchronously triggered meant to fit within this? As if you can start it without blocking, but can't retrieve the result without blocking, it would seem fatal to be unable to return WouldBlock in that instance.

I'm primarily interested in this in the case of software triggered ADC reads, given they take at minimum a number of micro seconds, but also must be actively triggered, so just waiting won't make them happen. It would seem appropriate that in such a situation an implementation should setup the ADC and trigger a conversion, before returning WouldBlock, subsequent calls will also return WouldBlock until the sampling and conversion has been completed, at which point the first subsequent call will return the data.

chrysn commented 5 years ago

On Fri, Feb 01, 2019 at 04:58:17PM -0800, Tom wrote:

As how are long running tasks which must be synchronously triggered meant to fit within this?

By having a .start() and a .read() method (or .start().read())).

If it's a long-running task (and not just an action that may or may not take place right now), there is some per-task state involved, and I think it's part of the nb idea that there should not be such an implicit state held by callee. (Cf. https://github.com/japaric/nb/issues/13)

I'm primarily interested in this in the case of software triggered ADC reads

They need the rather awkward .cancel() as long as they work by allowing a "start the process" side-effect.

IMO any function that receives a &mut adc would need to .cancel() on it first, for it can't know from its type state that there is certainly no old value pending in there that might be fetched when reading.