Skip to content
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

fix(tc53): serial format defaults to number #41

Merged
merged 2 commits into from
Nov 26, 2023

Conversation

HipsterBrown
Copy link
Contributor

As I was reading through the spec and noticed the annex for the Serial IO class documented the default format to "buffer" when the main text documents:

The Serial class data format is either "number" for individual bytes or "buffer" for groups of bytes. The default data format is "number".

"number" appears to be the default in the Moddable implementation as well.

I'm not sure what the typical workflow is for corrections like this, so happy to make changes as needed.

@phoddie
Copy link
Collaborator

phoddie commented Nov 24, 2023

Interesting find. Either default could be OK. There is a goal to keep Serial and TCP as consistent as possible (they can be interchangeable for some uses). TCP uses buffer as the default consistently in the spec. As you note, the outlier is the main spec text for serial

The Moddable SDK has seven implementations of serial. Most default to "number" format but at least one (Windows) defaults to "buffer".

The Moddable SDK's serial echo example assumes the default is "number".

All that means that if we change the spec, it will be a breaking change. I don't think it breaks enough to be a major concern. I'm OK with making the change here. We should update the Moddable SDK at about the same time.

Alas, GitHub won't show me a useful diff of your change. Is it just changing "number" to "buffer"?

When working on 2nd Edition, we maintained a change-log at the top of the spec markdown document so that it was easy to know what changed. I suggest doing the same here.

@HipsterBrown
Copy link
Contributor Author

@phoddie thanks for reviewing this. I updated the Annex to match the main spec text, so "buffer" -> "number".

There is a goal to keep Serial and TCP as consistent as possible (they can be interchangeable for some uses). TCP uses buffer as the default consistently in the spec.

I'm not sure I've seen that use case but I trust your experience. 😄

The Moddable SDK has seven implementations of serial. Most default to "number" format but at least one (Windows) defaults to "buffer".

Is the Windows default another outlier or a specific requirement?


I'm fine changing the main text to "buffer" if it's more important for TCP interoperability. Then I'll update the change-log at the top of the spec.

@phoddie
Copy link
Collaborator

phoddie commented Nov 26, 2023

@HipsterBrown – We definitely could go either way with the default. I'm pretty confident that buffer is the better choice.

Regarding TCP, in fact you probably are familiar with a protocol that uses serial and TCP interchangeably: Firmata. Firmata is defined over both, and there's really no difference apart from the transport. Buy having serial & TCP both support buffer and number they can be used interchangeably. Checking the client Firmata implementation built on ECMA-419, unsurprisingly, it initializes the format for TCP to number but not for serial. This needless inconsistency in the scripts using these two IO methods (same with the echo example) points to the benefit of having the default be consistent. There's no way the default for TCP should be number, so the default for Serial should be buffer.

FWIW – WebSocket is sometimes used like a TCP socket on the web. If you ignore the message framing and stick to binary messages, it is just a bidirectional connection like TCP or Serial. I have a note in my local implementation that it could also support number format, to allow for that use. It wasn't essential for our work on 2nd Edition though.

Is the Windows default another outlier or a specific requirement?

There's no reason for it to be different .Just a different implementation that made a different choice.

If you want to go ahead and make the change the spec so that Serial defaults to buffer, I'll update the Moddable SDK implementations after that.

@HipsterBrown
Copy link
Contributor Author

Regarding TCP, in fact you probably are familiar with a protocol that uses serial and TCP interchangeably: Firmata. Firmata is defined over both, and there's really no difference apart from the transport.

That's a good reference! I've also used Firmata over Bluetooth in the past, so I'll keep that in mind if it comes up for the 3rd edition.

If you want to go ahead and make the change the spec so that Serial defaults to buffer, I'll update the Moddable SDK implementations after that.

Done. ✅

@phoddie phoddie merged commit 4163f4f into EcmaTC53:master Nov 26, 2023
@phoddie
Copy link
Collaborator

phoddie commented Nov 26, 2023

Merged!

I've also used Firmata over Bluetooth in the past

Bluetooth is often used as a serial connection too (as @BobFrankston has remarked on more than one occasion). BLE is really intended to be packet based, but it has a serial profile too. The BLE serial profile is outside the standard, authored by Nordic.

I believe Modbus also uses TCP as an alternative to its original UART/serial transport. It has been a while since I looked.

More or less, serial is so fundamental to device-to-device communication that it seems to re-emerge in many other transports.

I'll make the supporting Moddable SDK changes.

mkellner pushed a commit to Moddable-OpenSource/moddable that referenced this pull request Dec 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants