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

Auto-reconnecting guidance needed #156

Open
2 of 3 tasks
dattasaurabh82 opened this issue Jan 6, 2022 · 13 comments
Open
2 of 3 tasks

Auto-reconnecting guidance needed #156

dattasaurabh82 opened this issue Jan 6, 2022 · 13 comments

Comments

@dattasaurabh82
Copy link

dattasaurabh82 commented Jan 6, 2022

As one might have guessed, it is not a bug report and an issue.

Also thanks a lot for putting such a good and useful API available to us.   🙏🏽

So I was looking into:

// -- Handle Events for physical re-connection of serial port by users
navigator.serial.addEventListener("connect", (event) => {
    // TODO: Automatically open event.target or warn user a port is available.
});

navigator.serial.addEventListener("disconnect", (event) => {
    // TODO: Remove |event.target| from the UI.
    // If the serial port was opened, a stream error would be observed as well.
});

My intention:

  • User connects to a serial port by some click event
  • User on physically disconnecting, handle the "proper disconnection".
    1. what do you mean by Remove event.target...? I have just set my serial port object to null;
    2. What else is needed here to handle?
  • The user physically connects the HW device back and the serial port opens again.
    1. The intention is that the old serial port object shall now not remain null anymore.
    2. And should open again.

So this is what I was doing:

// -- Handle Events for physical re-connection of serial port by users
navigator.serial.addEventListener("connect", (event) => {
        // Old serial port object which was used to open the specific serial port re-assigned again, 
        // is now assigned as the event target. 
        port = await event.target || event.port;  

       try {
            await port.open({ baudRate: 9600});
            // after that do other things if needed
            // set up encode stream etc. 
       } catch (e) {
            console.log(e);
       }
});

But when I physically disconnect and reconnect back, I get:
DOMException: Failed to open serial port.

But the Serial port is not null (I checked).

I would highly appreciate if you guys can fill this section again in : https://web.dev/serial/ and the Explainer

Meanwhile it would be very kind and helpful, if someone can show me how this part should be properly handled.
Something to do with bubbles ?

Also these events are always firing twice !🤔

@beaufortfrancois
Copy link
Collaborator

https://github.com/GoogleChromeLabs/serial-terminal/blob/main/src/index.ts#L378 shows you how https://github.com/googlechromelabs/serial-terminal/ handles it.

Note that you don't need await in your code.

-        port = await event.target || event.port; 
+        port = event.target || event.port;  

Can I try out your code somewhere?

@dattasaurabh82
Copy link
Author

Thanks for such a quick reply @beaufortfrancois
I was already actually looking into the src of https://googlechromelabs.github.io/serial-terminal/ 😅
But the auto re-opening of port upon reconnection of HW is not really implemented (or may be I'm missing something).
That's what I am looking for if possible.

Here is what I'm working on a demo: https://github.com/devATdbsutdio/webserial_time_resetter/blob/main/public/src/serialManager.js

Please note:

  1. Have not implemented Polyfill methods for mobile devices as it is not intended use-case. May be in near future.
  2. Warning: Dirty Vanilla JS code and bad css ahead !

Some insights would be really helpful 🙏🏽

@reillyeon
Copy link
Collaborator

As written your code should work. I'm thinking the issue might be that trying to open the port as soon as it is detected is causing a race with another application on the system that probes serial ports or the operating system isn't actually quite ready for an application to try opening the port even though it's already delivered the notification that the port is available. If you add a small delay does that fix the problem? What operating system are you using?

Note, checking event.port is unnecessary at this point. It was only part of the experimental version of the API and all browsers should be up-to-date by this point. Also it should be event.port || event.target since event.target is always truthy but prior to the final API shape was set to the Serial object instead of the SerialPort object. In modern browsers event.port is undefined.

@dattasaurabh82
Copy link
Author

@reillyeon Adding delays helped, so guessing it was the actual availability and race condition situation.

BTW how do we avoid 2 times firing, gracefully, of those connect and disconnect events?

@beaufortfrancois
Copy link
Collaborator

Delay(s) helped there but I wonder if we can make it better in Chromium.
Would it be possible to delay a bit firing the serial "connect" event or actually try opening internally the serial port a second time if the first time failed?

@dattasaurabh82
Copy link
Author

dattasaurabh82 commented Jan 7, 2022

@beaufortfrancois I was thinking the same (But didn't get time today to try). Will try it tomorrow.
Persistence sounds better over chance in theory.
So we can do a try catch DOM Exception ... (i.e try to catch the failure info I was getting earlier as a result of it being unable to open the port (for potential reasons as mentioned by @reillyeon)) and then try again till it succeeds.

Something like this may be 🤔 :

while (true){
    try {
        await port.open({ baudRate: baudrate});
        if (port) break;
    } catch(err){
        // log ...
    }
}

@reillyeon
Copy link
Collaborator

Can you check about://device-log to see if the error from the operating system is really "device busy"? It would also be good to understand what other application on the system might be trying to communicate with the device as that could cause other problems if it puts the device in a weird state. I'm hesitant to hide the problem with a workaround in Chrome.

@dattasaurabh82
Copy link
Author

So when I ran:

var a = performance.now();

while (true){
    try {
        await port.open({ baudRate: baudrate});
        if (port) break;
    } catch(err){
        // log ...
    }
}
var b = performance.now();

if (port.writable && port.readable) serialConnected = true;

console.log('Serial Port is available again and re-connected & it took:' + (b - a)/1000 + 's approximately');

It took: 1sec or 1009 ms

So. @reillyeon when I looked into the device log, on auto re-connect attempt, I saw the Error: FILE_ERROR_IN_USE.

Screenshot 2022-01-08 at 5 13 09 PM

Does it mean device busy err?

Also I saw an interesting Phenomenon in macOS (my machine) and my Serial device is FTDI based:

Screenshot 2022-01-08 at 5 32 55 PM

As we know, macOS already comes with FTDI drivers but I also have the VCP drivers from FTDI installed in my machine.

  1. During auto re-connect, FTDI drivers detect the port, tries to open fails.
  2. Then I guess there is a race condition between the apple driver and FTDI driver.
  3. and then, the apple default sys FTDI drivers wins to open the port.

@dattasaurabh82
Copy link
Author

dattasaurabh82 commented Jan 8, 2022

@reillyeon @beaufortfrancois would be nice to have an example in your guide that illustrates (with how and why):

1. Open selected port on click(user action) (you have it)
2. While port is open, if user physically removes the device, notify and destroy the port related pointers. 
3. When the device is available again (and if was removed accidentally), then only reconnect and open the same port. 
4. Disconnect the port on click(user action). (you have it)

@reillyeon
Copy link
Collaborator

FILE_ERROR_IN_USE means the device is busy.

The behavior between the VCP driver and built-in driver makes it sound like at some point you granted your site access to both versions of the device. If you click the lock icon, clear all the serial port permissions for the site and only grant it permission using one of the two drivers does that improve the behavior?

@dattasaurabh82
Copy link
Author

Hello @ALL
Sorry for vanishing.
I was busy with some other stuff.

I made a design choice in my software: It doesn't reconnect again automatically. The user has to manually reconnect the device if he/she disconnected the physical device accidentally.

So basically I'm using this call back below to remove the serial object:

navigator.serial.addEventListener("disconnect", (event) => {
    // TODO: Remove |event.target| from the UI.
    // If the serial port was opened, a stream error would be observed as well.
});

And nothing else.

@reillyeon
Copy link
Collaborator

Right now it's not actually possible to properly implement example 3 from your comment above because when a port is reconnected there's no identifier for recognizing it as the device that was connected recently rather than another device that the site has permission to access. This will be solved by #128.

@Septimus
Copy link

I'm having a similar on macOS (Apple Silicon). The disconnect event fires just fine, but the connection event doesn't fire at all for any serial connections. Disconnect removes the paired device from the list of authorized/paired devices. Anytime I disconnect, I need to request device access again.

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

No branches or pull requests

4 participants