Feedback on a POSIX style wrapper for Web Serial API

[marciot]

This is sort of a follow up on a question that started in thread #123.

I am working on porting some 3D printing and serial flashing code that was originally written in Python, eventually was ported to Electron and now I am rewriting using the Web Serial API.

I've run into a few issues stemming from the fact that the Web Serial API presents a Stream interface rather than a more traditional POSIX interface.

The code I am porting has several functions that look like this:

    async readByte(address) {
        const cmd = "o" + Samba.hex8(address) + ",1#\n";
        await this.serial.write(cmd);
        await this.serial.flush();
        const bytes = await this.serial.read(1);
        return bytes[0];
    }

    async readWord(address) {
        const cmd = "w" + Samba.hex8(address) + ",4#\n";
        await this.serial.write(cmd);
        await this.serial.flush();
        const bytes = await this.serial.read(4);
        return Samba.littleEndian(bytes);
    }

The very nature of this code lends itself to a Serial interface with the following methods:

class Serial {
   async open(baudRate, timeout) // Open the serial port (timeout in ms is optional)
   async read(len)  // Read this many bytes from serial port, throw exception after timeout
   async readLine() // Read a line of text (terminated by '\n') from the serial port
   async write(data)  // Write string, list of bytes or Uint8Array to the serial port
   async flush()  // Flush any output data to the stream
   async close()  // Close the serial port
   async wait(ms) // Wait a certain number of milliseconds
}

Right now, it turns out it is possible to write such an interface using the Web Serial API, but it's not a trivial exercise. Here is the code I am using right now (I am not 100% sure it is correct, but it seems to work pretty well):

    class SequentialSerial {

        constructor(port) {
            this.serial = port;
        }

        async open(baud, timeout = 5) {
            this.decoder = new TextDecoder();
            this.encoder = new TextEncoder();
            this.timeout = timeout;
            await this.serial.open({baudRate: baud);
            this.writer = this.serial.writable.getWriter();
            this.reader = this.serial.readable.getReader();
            this.readBytes = [];
            this.readIndex = 0;
        }

        async close() {
            if(this.reader) {
                await this.writer.releaseLock();
                await this.reader.releaseLock();
                await this.serial.close();
                this.reader = null;
                this.writer = null;
            }
        }

        async flush() {
            if(this.reader) {
                await this.writer.ready;
                await this.writer.close();
                this.writer = this.serial.writable.getWriter();
            }
        }

        async discardBuffers() {
            this.readBytes = [];
            this.readIndex = 0;
            await this.reader.close();
            this.reader = this.serial.readable.getReader();
        }

        toUint8Array(data) {
            let type = Array.isArray(data) ? "array" : typeof data;
            switch(type) {
                case "array":  return Uint8Array.from(data);
                case "string": return this.encoder.encode(data);
                case "object": if(data instanceof Uint8Array) return data;
            }
            console.error("Tried to write unknown type to serial port:", typeof data, data);
        }

        async write(data) {
            const data = this.toUint8Array(data);
            return this.writer.write(data);
        }

        getTimeoutPromise() {
            if(this.timeout) {
                return new Promise(
                    (resolve, reject) =>
                        this.timeoutId = setTimeout(
                            () => reject(new SerialTimeout("Timeout expired while waiting for data")),
                            this.timeout * 1000
                        )
                );
            }
        }

        async read(len) {
            let timeoutId;
            let timeoutPromise = this.getTimeoutPromise();
            const dst = new Uint8Array(len);
            for(let i = 0; i < len;) {
                if(this.readIndex == this.readBytes.length) {
                    if(!this.readPromise) {
                        this.readPromise = this.reader.read();
                    }
                    const bothPromise = timeoutPromise ? Promise.race([this.readPromise, timeoutPromise]) : this.readPromise;
                    const { value, done } = await bothPromise;
                    this.readPromise = null;
                    if (done) {
                        // Allow the serial port to be closed later.
                        clearTimeout(this.timeoutId);
                        this.reader.releaseLock();
                        throw new SerialDisconnected("Serial port closed while waiting for data");
                    }
                    this.readBytes = value;
                    this.readIndex = 0;
                }
                dst[i++] = this.readBytes[this.readIndex++];
            }
            clearTimeout(this.timeoutId);
            return dst;
        }

        async readline() {
            let line = "";
            while(true) {
                let c = this.decoder.decode(await this.read(1));
                switch(c) {
                    case '\r': break;
                    case '\n': return this.encoder.encode(line);
                    default:   line += c;
                }
            }
        }

        wait(ms) {
            return new Promise((resolve, reject) => setTimeout(resolve,ms));
        }
    }

I hardly think the kind of code I am trying to port is uncommon; the code looks very much as what you would see in any serial related C++, Python, or Arduino code.

While it is possible to write a wrapper class around the existing Web Serial API, just as I have done, I see potential drawbacks to this:

• The read() routine is somewhat inefficient because:
  • It has to copy bytes from pre-existing ArrayBuffers into new ArrayBuffers.
  • It creates several promises and a timer, which must be garbage collected.
• Calling the "flush()" routine requires:
  • Creating a new Writer object and garbage collecting the old one.

I feel that this style of serial port is common enough and useful enough that the standard should provide for it. At the very least, this would prevent the creation of hundreds of separate implementations which pretty much do the same thing.

Perhaps the standard it could provide a polyfill, as I have done, but having a common implementation would allow implementers to optimize it by implementing it directly via calls to the host OS, which most likely already has a POSIX-style serial port under the hood anyway.

@reillyeon, @domenic: Thought and comments?

[reillyeon]

Supporting a BYOB reader for on the ReadableStream will avoid the buffer allocation issues as you'll be able to append bytes directly from the underlying source to an existing buffer.

The bulkRead() method proposed in whatwg/streams#1003 would also allow you to specify a timeout but that has the same questions about semantics as passing an AbortSignal to read() being discussed in whatwg/streams#1103.

[marciot]

@reillyeon: Cool. Thanks. I'll investigate the BYOB reader.

This post is more along the lines of whether it would be beneficial to have an alternative interface that doesn't involve streams for folks who did not want to get into these kind of details. It seems like such an interface can be provided as a third-party library, so if that's the idea, I understand.

[reillyeon]

I just filed #127 because right now the ReadableStream we create does not support a BYOB reader because it doesn't mark itself as a byte stream.

[reillyeon]

This post is more along the lines of whether it would be beneficial to have an alternative interface that doesn't involve streams for folks who did not want to get into these kind of details. It seems like such an interface can be provided as a third-party library, so if that's the idea, I understand.

I think the first order of business is to make sure that the current API supports all the necessary primitives to implement that kind of library. If it doesn't then there is a behavior that is missing that we'd have to specify before it could be supported natively or through a polyfill. I think we're in good shape on that front, though like I said BYOB readers and abortable reads would simplify things for the library.

[marciot]

@reillyeon: Sounds fair. BTW, thank you so much for your help! I now have both firmware flashing and 3D printing working great! It's working just as well as it did under Electron!

[reillyeon]

When you have the site published please let us know so we can reference it as an example.

[marciot]

@reillyeon: It’s currently here:

https://syndaverco.github.io/firmware/

Source Code:

https://github.com/SynDaverCO/symple-slicer

[reillyeon]

Closing this issue as the feature requests discussed are tracked elsewhere.