-
Notifications
You must be signed in to change notification settings - Fork 79
docs(lp32): Elaborate README #3044
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
erights
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
|
|
||
| For example, a 5-byte message `hello` is transmitted as: | ||
| ``` | ||
| [0x05, 0x00, 0x00, 0x00] [h, e, l, l, o] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this example is specific to a particular host endianness, then you should give an example of each right here and say which is which. For most programmer, including me, our ability to think about endianness has gone stale.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW, I find it incredibly bizarre that a communications format would use host-dependent endianness. How would communication between hosts of different endiannesses work?
(At least we can be relieved that the world no longer needs to worry about VAX endianness any longer.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the designers of this protocol intended it to be specifically used only for same-host communication between a web browser and a child process, so discounted the upside of network order (cross-host portability) in favor of the upside of host order (no translation). For such a length prefix, I would probably have gone another way. In any case, we have this in anticipation of needing it to connect a web extension to the daemon, which we’ve prototyped but have no concrete plan for.
| let drained = false; | ||
| while (!drained && length >= 4) { | ||
| const messageLength = data.getUint32(0, littleEndian); | ||
| const messageLength = data.getUint32(0, hostIsLittleEndian); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right here. What happens if the length we read was written on a host with a different endianness?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would fail in a strange way. Probably would block for a very long time, waiting for sufficient data to fill a 4MB buffer, then produce a protocol error. Likely never.
As such, this is not a suitable protocol for cross-host comms and only intended for browser to app comms.
| }); | ||
|
|
||
| const encoder = new TextEncoder(); | ||
| await writer.next(encoder.encode('hello')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, using the iterator protocol directly, we don’t suffer the parity mismatch of generators. This comes up in the draft exo-stream change as well.
|
|
||
| const reader = makeLp32Reader(byteStream, { | ||
| name: '<my-stream>', // optional, for error messages | ||
| maxMessageLength: 1024 * 1024, // optional, defaults to 1MB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if the max is exceeded?
| // Producer | ||
| await writer.next(encoder.encode('message 1')); | ||
| await writer.next(encoder.encode('message 2')); | ||
| await writer.return(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these awaits for flow control? I notice there is an initialCapacity and a maxMessageLength but no maxCapacity. So what does the flow control wait for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The awaits are for flow control. The reader must call next for the producer to advance. There’s a facility in @endo/stream for pre-acking so that these pipeline within pre-arranged bounds, but the default is that they’re synchronized.
This change documents the lp32 package. It also addresses some minor inconsistencies in the implementation. The byte order for native messages is not configurable.