Hypothetical patterns for asynchronous IO reads

dtex commented 2 years ago

This is a continuation of conversations had in:

On the call I offered to model some hypothetical API calls to a currently non-existent ECMA-419 asynchronous read API. To be clear, these are examples of I2C reads from the user's perspective. The inner workings of the read implementation are left to the implementor. We're using callbacks since not all target platforms will support promises, and error first call backs can easily be "promisified" in other host environments.

Existing ECMA-419 implementation

Synchronous

read(option[, stop])	Param.	Required	Range	Default
option	yes*	positive integer, ArrayBuffer, TypedArray
stop	no	true or false	true

The number of readable bytes is undefined so option is required

let result = myI2C.read(64);

This pattern makes perfect sense for embedded systems, or anywhere reads are fast enough not to be a problem while blocking.

Possible async patterns

Callback param

readAsync(option[, stop], cb)	Param.	Required
option	yes	positive integer (number of bytes)
*sto	no	true or false.
cb	yes	err first callback

myI2C.readAsync(64, (err, data) => {
  // Do things with the data
});

This seems like the most practical option. By adding a distinct, optional method for asynchronous reads the absence of the method should throw in an unambiguous way. All of the other options listed below could result in implementations that do not throw an error but also do not behave as expected.

Variadic read

read(option[, stop][, cb])	Param.	Required	Range
option	yes	positive integer (number of bytes)
stop	no	true or false	true
cb	no	err first callback*	null

myI2C.read(64, (err, data) => {
  // Do things with the data
});

I believe that variadic functions make implementation, documentation, and support more difficult. I do not think we should use them here.

Leveraging onReadable

read(option[, stop])	Param.	Required	Range	Default
option	yes	positive integer (number of bytes)
stop	no	true or false	true

First read returns, when read is complete onReadable fires. This requires user code to manage state:

Does each read trigger a read or process the results of a read?
What register address was last read and which read request does this onReadable correspond to?

const myI2C = new device.I2C({
            ...device.I2C.default,
            onReadable: () => {
     let data = myI2C.read(64);
     if data === null return;
     // do something with the data
            }
 });

let data = myI2C.read(64); // expets null

Because of the burden put on the user, I do not think this is a good pattern.

Adding an onread property

read(option[, stop])	Param.	Required	Range	Default
option	yes	positive integer (number of bytes)
stop	no	true or false	true

Read only triggers, when read is complete onReadable fires. This requires user code to manage state:

Does each read trigger a read or process the results of a read?
What register address was last read and which read request does this onReadable correspond to?

const myI2C = new device.I2C({
            ...device.I2C.default,
            onReadable: () => {
     this.read(64);
   },
   onRead: (data) => {
     if data === null return;
     // do something with the data
            }
 });

let data = myI2C.read(64); // expets null

Because of the burden put on the user, I do not think this is a good pattern

phoddie commented 2 years ago

@dtex – thanks for putting together these excellent notes. It is helpful to be able to see different options side-by-side.

I completely agree with you that the onReadable and onRead approach is difficult to use. I implemented the onReadable approach for Fermata and, while it worked, there's no question that it was confusing to use.

That leaves the callback pattern and variadic read. They are mostly the same, if I understand correctly, apart from the method name.

On the call Wednesday, we talked a bit about read versus readAsync. The read in the I/O Class Pattern is already both synchronous and asynchronous, depending on the I/O. Because of that, while I agree with your point about the potential for confusion, I'm not sure we have another option.

I think the potential for confusion is reduced because on the call we also agreed that a given I²C instance must be either synchronous or asynchronous.

error first call backs can easily be "promisified" in other host environments

I don't have experience that gives me an intuition of how the order of the arguments in the callbacks simplifies "promisifying" of an API. It is a small point here, but I'd like to understand. Would you mind explaining?

As we talked about Wednesday, the design space here is bigger than read. In addition to asynchronous read, we need to consider asynchronous write. At a minimum, that would seem to require a callback to indicate success or failure of the call. More substantial, since SMBus is defined as an extension of I²C, we need consider how SMBus operates asynchronously as well.

I put together some notes to try to address the full space (thank you to @andycarle for reviewing and providing feedback). It is more-or-less an expansion of your variadic read. The constructor is explicitly passed a flag to enable asynchronous operation. That's necessary for compatibility, but it also has the benefit of making explicit that the instance is asynchronous (or synchronous) which may help a bit with the confusion concern.

phoddie commented 2 years ago

We had a very good discussion about this during the monthly TC53 call yesterday. I've updated the notes to reflect the outcome of that conversation. It feels like we're near a solution to integrate into the 2nd Edition draft and begin implementation work around.

EcmaTC53 / spec