WebUSB: Always reset the serial terminal when closed #312
Conversation
Same code was in 3 different places. Now when the serial button is used to close the terminal the terminal itself is reset as well, as it can be deceiving to have the old content when the micro:bit serial is reset on reconnects.
…eners when closed.
|
@microbit-sam could you review when you find some time after everything else? This is very low priority at the moment. |
|
|
||
| var daplink = usePartialFlashing ? window.dapwrapper.daplink : window.daplink; | ||
| daplink.stopSerialRead(); | ||
| daplink.removeAllListeners(DAPjs.DAPLink.EVENT_SERIAL_DATA); |
There was a problem hiding this comment.
I think this is the main reason sometimes we were seeing double characters on the terminal.
I don't quite have a way to replicate the original issue reported by some users, @microbit-mark do you have any additional info on the problem? I couldn't find an issue in this repo.
There was a problem hiding this comment.
It's mentioned in #253 and originally from
https://support.microbit.org/a/tickets/24489 However the user says that they are unsure whether they accidentally typed the extra characters or not, so it maybe hard to reproduce.
There was a problem hiding this comment.
Oh right, this is not the same then.
This issue would have caused all typed characters to appear twice. Before this PR I only saw this type of double characters very sporadically, but this refactor somehow exuberated the issue and this line fixed it, I was hoping it was the same, but it isn't.
| } | ||
| if (usePartialFlashing && window.dapwrapper) { | ||
| console.log('Disconnecting: Using Quick Flash'); | ||
| p = p.then(function() { return window.dapwrapper.disconnectAsync() }); |
There was a problem hiding this comment.
@microbit-sam I think these two were meant to return the disconnect promise?
I assume this was originally an arrow function .then(() => window.dapwrapper.disconnectAsync()); that returned the promise from disconnectAsync(), but then when converting to ES5 the return was accidentally left out?
On the other hand stopSerialRead() does not return a promise, so the original line removed in this PR (p.then(function() { window.dapwrapper.daplink.stopSerialRead() } )) is correct in not using return in the function.
It also captures any possible error during WebUSB disconnection.
|
|
||
| // Device disconnect listener | ||
| // Clears dapwrapper | ||
| navigator.usb.addEventListener('disconnect', clearDapWrapper); |
There was a problem hiding this comment.
Removed from here since the disconnect event already executes showDisconnectError().
showDisconnectError() -> webusbErrorHandler() -> clearDapWrapper()
| function clearDapWrapper(event) { | ||
| if(window.dapwrapper || window.previousDapWrapper) { | ||
| window.dapwrapper = null; | ||
| window.previousDapWrapper = null; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Function removed from here, but code still present in webusbErrorHandler():
| } | ||
|
|
||
| // Disconnect from the microbit | ||
| doDisconnect(); |
There was a problem hiding this comment.
Before this PR, at this point of we had already set to null the window.dapwrapper and window.previousDapWrapper variables above before executing doDisconnect().
So doDisconnect() never attempted to execute disconnectAsync() since we had if (window.dapwrapper) window.dapwrapper.disconnectAsync(); and dapwrapper was already falsy.
It does this by refactoring the "serial close" code from multiple places into a single function.
By leaving the content of the terminal when closing the serial it might look like the REPL is still running, which it isn't after a serial reconnection.
In this GIF you can see me opening the REPL, execute a command, and then trying to go back and press the UP key in the keyboard to try to repeat the last command.
Every time the UP key is pressed we can see an error in the console (different issue), and it isn't until Ctrl+C is pressed that the REPL is running again.
I think this also possibly fixes the issue that was reported by some users where typing in the terminal would print on the screen the same character multiple times.