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

Add docs on SerialPort.connected and Bluetooth port event support #37002

Merged

Conversation

chrisdavidmills
Copy link
Contributor

Description

Chrome 130 supports the SerialPort.connected attribute, a boolean that reports whether a serial port is logically connected.

Another part of this work item is that the SerialPort connect and disconnect events were previously only fired by physical/wired serial ports. With this change, wireless Bluetooth RFCOMM serial ports also fire these events when they connect and disconnect.

This PR updates the docs to add a connected reference page and update the event pages to cover the Bluetooth support.

Motivation

Additional details

Related issues and pull requests

See https://chromestatus.com/feature/5118102654418944.

@chrisdavidmills chrisdavidmills requested a review from a team as a code owner November 27, 2024 13:11
@chrisdavidmills chrisdavidmills requested review from wbamberg and removed request for a team November 27, 2024 13:11
@github-actions github-actions bot added Content:WebAPI Web API docs size/m [PR only] 51-500 LoC changed labels Nov 27, 2024
Copy link
Contributor

github-actions bot commented Nov 27, 2024

Copy link
Contributor

@beaufortfrancois beaufortfrancois left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with nits

The following snippet invokes {{domxref("Serial.requestPort()")}} when the user presses a {{htmlelement("button")}}, prompting them to choose a serial port to connect to, then logs a message to the console reporting the connection status:

```js
requestPortButton.onclick = async () => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call; I've changed them to use addEventListener().


```js
navigator.serial.onconnect = ({ target: port }) => {
console.log(`onconnect event fired. Connected: ${port.connected}`);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
console.log(`onconnect event fired. Connected: ${port.connected}`);
console.log(`connect event fired. Connected: ${port.connected}`);

...etc

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated both instances that were dubious.

Comment on lines 48 to 50
When a wireless device goes out of range of the host, any wireless serial port opened by a web app automatically closes, even though it stays logically connected. In such cases, the web app could attempt to reopen the port using {{domxref("SerialPort.open()")}}.

However, if the wireless device was intentionally disconnected (for example, if the user chose to disconnect it using the operating system control panel), the web app should refrain from reopening the port to prevent reconnecting to the wireless device.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really the key message about this PR AIUI. It would be good to have this in the description of "logically connected" instead of buried in an example down the page.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that makes sense. I've moved this example and description up to a "Description" section up-top.

Comment on lines 58 to 59
// The port is logically connected
// automatically try to connect to the Bluetooth device
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are connected, why are we trying to connect?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, that should have been "automatically try to reopen the port" — updated. It should make more sense now.

Copy link
Collaborator

@wbamberg wbamberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Thanks Chris, I'm happy with this. I'm leaving this open in case @beaufortfrancois has more comments but happy for you to merge when you're ready.

Copy link
Contributor

@beaufortfrancois beaufortfrancois left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@chrisdavidmills chrisdavidmills merged commit 861d367 into mdn:main Dec 2, 2024
8 checks passed
@chrisdavidmills chrisdavidmills deleted the web-serial-connected-property branch December 2, 2024 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:WebAPI Web API docs size/m [PR only] 51-500 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants