EcmaTC53 / spec

Ecma TC53 spec work
23 stars 9 forks source link

Asynchronous i2c.read #26

Closed nebrius closed 2 years ago

nebrius commented 2 years ago

Reading through the IO class docs, it states that read is synchronous and returns data that is available. Can we also make this take an optional callback to do an asynchronous read instead, and throw on platforms that can't support synchronous reads and a callback isn't supplied?

It's not really possible to implement the synchronous style of read as spec'ed on the Raspberry Pi. The mechanism for the Raspberry PI (https://www.npmjs.com/package/i2c-bus) supports two ways of reading:

nebrius commented 2 years ago

Update: I just found a note in the spec that says:

Note: The read and write methods may operate synchronously. Doing so does not violate the requirement that IO is non-blocking because these operations typically complete within a short period of time. Additionally, synchronous operation is required for microcontrollers which do not support asynchronous I²C IO.

This implies that these can be asynchronous? Does this mean that read returns a promise? Or takes a callback?

phoddie commented 2 years ago

I²C reads and writes are synchronous. There's no other practical option on some embedded targets (ESP8266, for example). The note you reference in the standard is an attempt to state that in more general terms.

While I²C can potentially block for a long time, it rarely does. In my experience, the main reason that happens is that I accidentally disconnected a wire to the sensor. To handle potential communication failures, many I²C native stacks have a timeout to avoid blocking for too long. The 1st edition standard reserves the timeout property in the constructor's options object to support this in the future. If timeout is an option on Raspberry Pi, now could be a good time to begin to explore that so we have it for 2nd edition.

In theory. there could be an asynchronous I²C read and write. I explored that in the Firmata provider implemented using ECMA-419 (blog post, technical docs, async I²C notes). It works. In that case, the IO Class Pattern onReadable callback is used to indicate when a response is available so there's no need to pass a callback to read. However, it is not generally useful because code is written to expect synchronous read and write and will fail until updated. So, for general compatibility, even if asynchronous I²C is provided, a host would generally want to support synchronous I²C.

nebrius commented 2 years ago

I²C reads and writes are synchronous. There's no other practical option on some embedded targets (ESP8266, for example).

I want to challenge you on this, because in Javascript any synchronous operation can be made asynchronous. There's no such thing as asynchronicity being impossible in JavaScript. Here's how you do it in a naive way using ES5, which all targets support:

function asyncRead(length, cb) {
  var bytes = [];
  function pump() {
    var byte= readByteSync()
    if (byte !== undefined) {
      bytes.push(byte);
      if (bytes.length === length) {
        cb(bytes);
        return;
      }
    }
    setTimeout(pump);
  }
  setTimeout(pump);
}

It's also possible to do effectively the same thing in C++ using function pointers combined with some sort of delay or sleep function.

However, it's not possible to convert from asynchronous to synchronous though in a generic and efficient way like it's possible to go from synchronous to asynchronous. There can be workarounds, such as what you described above, but I consider that workaround to be unacceptable because it blocks the event loop unnecessarily.

phoddie commented 2 years ago

I don't know that I would characterize the example above as asynchronous. It divides one large synchronous operation (reading length bytes) into a sequence of length synchronous operations (reading one byte each). When a call to readByteSync takes 30 seconds to return, all JavaScript execution in that virtual machine is blocked for 30 seconds.

I would like to clarify one point above. The statement that some embedded targets support only synchronous I²C describes the capabilities of native I²C API of the platform provider's SDK, not the JavaScript engine.

nebrius commented 2 years ago

I don't know that I would characterize the example above as asynchronous. It divides one large synchronous operation (reading length bytes) into a sequence of length synchronous operations (reading one byte each). When a call to readByteSync takes 30 seconds to return, all JavaScript execution in that virtual machine is blocked for 30 seconds.

It may not be asynchronous from the perspective of the under the hood implementation when compared to, say, how a file read is done in Node.js via libuv, sure, but it's absolutely asynchronous from the perspective of the API, which really is all that matters here since the spec defines an API, not an implementation.

I would like to clarify one point above. The statement that some embedded targets support only synchronous I²C describes the capabilities of native I²C API of the platform provider's SDK, not the JavaScript engine.

Honestly I'd argue that's a distinction without a difference, since again this spec defines an API, not an implementation.

nebrius commented 2 years ago

However, it is not generally useful because code is written to expect synchronous read and write and will fail until updated.

To put this another way, this is, in practice, a blocker for me implementing ECMA-419 support for the Rasperry Pi. Either we need to find a way to change the spec, or I don't think the spec is a good fit for the Raspberry Pi and I'm going to abandon my implementation.

dtex commented 2 years ago

This will be a recurring issue on platforms/architectures where I2C reads (or any IO reads) are slow enough that we cannot allow them to block, so I think it's worth addressing. If we don't address it, others will and there's no guarantee that it will be consistent.

Leveraging onReadable seems messy since the behavior would change depending on the platform.

Is there a place in an annex or non-normative part of the spec where we can define an asyncRead method for the IOCP that can be implemented by platforms where it is necessary?

jsiegelmsu commented 2 years ago

Hi folks, it's nice to see the I²C implementation getting attention -- and this is some very thorough and thoughtful discussion about how we might define (and build for/with/around) asynchronous and synchronous operations.

As I read through this thread, I see some great information sharing, some new ideas, and lots of potential for fruitful collaboration. I also see a few instances where the intended spirit and tone of a message may not necessarily translate well to plaintext.

As we continue discussion on this and other issues, I would encourage all contributors to be mindful that Ecma TC53 explicitly adopts TC39's Code of Conduct (https://tc39.es/code-of-conduct/) -- I say this not as a warning or threat of censorship, but rather as a reminder that we have all chosen to become members of a community that recognizes that we do our best work when we are able to work effectively together.

nebrius commented 2 years ago

Thank you for the reminder @jsiegelmsu. Rereading my comments, I was overly aggressive and for that I'm sorry, that was unnecessary and unproductive.

I've been reflecting on the API and my conversations here, and I've decided to halt my implementation. If anyone wants to continue the work I've started, feel free to fork my repo and continue working on it (subject to the MIT license stipulations of course).

I've come to the conclusion that the TC53 spec is a poor fit for any embedded device that runs a full host OS. That's ok too! You seem to have a focus on smaller embedded systems that are either bare metal or running an RTOS.

The API needs of a multi user host OS are going to be different because hardware is abstracted through some sort of kernel that implements a driver+HID system. These systems provide very different interfaces than bare metal/RTOS systems, so in retrospect it's not surprising that it's been so difficult trying to create an optimal implementation for the Raspberry Pi. Square peg in a round hole and all that.

I wish you all the best, and if you ever decide to create a new spec designed for full host OS based embedded systems, I'd love to be involved.

phoddie commented 2 years ago

@nebrius – thank you for your feedback. The committee intends to hold a call to discuss I²C during the week of June 27. You are welcome to join that discussion to share the technical details of your experience. If you are interested in participating, please complete the Doodle poll to help find a time that works for everyone.

dtex commented 2 years ago

I think we should also invite @fivdi as he's familiar with the issue (see #4), and @rwaldron because he is an excellent big-picture thinker who is familiar with the standards process, and understands the issue.

phoddie commented 2 years ago

@dtex – great idea to invite Brian and Rick. Their past contributions have been valuable and their expertise is always welcome.

nebrius commented 2 years ago

Thanks, I added my availability to the Doodle.

nebrius commented 2 years ago

I see we've picked tomorrow morning as our meeting time, but I don't see any details on how to join the call. Can someone get that information to me?

dtex commented 2 years ago

I'm in the same boat as @nebrius. Also, @rwaldron would like to join us so I need to relay the information to him.

phoddie commented 2 years ago

Details were sent to TC53 mailing list. I forwarded them yesterday to Bryan. I can resend to you in a bit.

phoddie commented 2 years ago

Email forwarded to you and Rick.