Serial port list throttling and repl error with multiple boards #1456
Conversation
When listing available serial connections, a maximum number of 10 attempts every 400 milliseconds are allowed, then the `board.fail` method is called. When disconnecting on SIGINT, add a 1000 millisecond timeout before forcing the exit. This has been added because if one disconnects a succesfully connected board the process could never exit on SIGINT.
When connecting multiple boards, an exception is thrown when exiting the process because `state.board.io.pending` is not defined. This commit fixes the problem by checking the `length` property of `state.board` and looping through the board list to check the `board.io.pending` property. Futhermore, it add a timeout of 1000 milliseconds to force the process exit in case the `board.io.pending` property never becomes `false` (it has happened disconnetting a succesfully connected board).
Since `board.fail` is called when connection fails, it might be useful to echo the event in the Boards class.
| @@ -88,7 +91,14 @@ var Serial = { | |||
| Serial.attempts[Serial.used.length]++; | |||
|
|
|||
| // Retry Serial connection | |||
| Serial.detect.call(this, callback); | |||
| if (Serial.attempts[Serial.used.length] > maxAttempts) { | |||
| this.fail("Board", "No connected device found"); | |||
There was a problem hiding this comment.
Here I would have logged the id of the board but I haven't because when initializing multiple boards without specifing ids they all end up having the same generated uid. Is this how it is supposed to be?
|
I'm going to need a bit of time to test and fully understand these changes. I appreciate your patience. |
|
@rwaldron Absolutely, thanks for reviewing this PR. If you need anything (a basic code sample that triggers the issue or anything else) I am here to help. |
There was a problem hiding this comment.
This is not true anymore, list will report an error if there's an error. Also port.list() returns a promise if there isn't a callback.
All in all a good approach.
| // Max number of times listing serial conntections can fail | ||
| var maxAttempts = 10; | ||
| // Delay (ms) before trying again to list serial connections | ||
| var retryDelay = 400; |
There was a problem hiding this comment.
My gut says this is a fine timeout.
|
I've been in the middle of moving my office for about a month now :( All of my hardware is in boxes and I'm slowing unpacking things. I'd like to try this with real hardware before I land it—I appreciate your patience |
|
Is anybody here able to test with X-Bee's? It's probably fine, but that's a special case that may be affected. |
|
I've tested this against hardware and I'm satisfied with the changes—thanks and great work!!
|
|
@rwaldron Happy this PR was useful :-). I have one question if you don't mind. all the boards will have the same generated id. Is this how it is supposed to be |
Reading issue #1406 I started playing with Board.detect and indeed in case
of no connection the function gets called with no throttling.
I found a couple of other things too:
if a connected board(s) is(are) disconnected while running, when terminating the process the SIGINT event gets sometimes ignored (it has happened running a board with
repl: trueandrepl: false)if multiple boards are connected with
repl: true, when exiting the process an error is thrown becausestate.board.io.pendingis not definedIn this PR a simple solution to the first problem follows what @dtex suggests in the issue discussion, while for the other two I add a fail safe timer of 1 second to force exit and fix the code in the
repl.jsmodule to handle the case of multiple boards.This is not definitive and I don't know how much of this is needed, I would like to know what do you think (@rwaldron @dtex).