-
Notifications
You must be signed in to change notification settings - Fork 37
"Bad response for 8 -> 17" bug related to use of serial #89
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
Comments
Does it replicate in https://armmbed.github.io/dapjs/examples/daplink-serial/web.html ? |
If you get the device in the bad state in our editor then that example will fail to connect, but you can't trigger the bad state it in that example. Interestingly the example disconnects before starting serial read (which, based on the flickering USB icon in the tab, probably reconnects repeatedly). Also confirmed that restarting Chrome doesn't fix the bad state. |
And un/replugging the micro:bit but independent of doing something to the browser does? |
Confusingly, the error we see here comes from an error handler, though I think it's error handling a quite similar scenario. The original error message is "Bad response for 17 -> 8". Sequence of events so far as I can tell:
|
With the knock-on error code modified to swallow that error, I've also seen 17 -> 131 as the initial error. 131 is the serial read command. I'm pretty convinced nothing serial-related is happening on the browser end as that's all after the connect. Using the DAPLink serial sample I can't reproduce it even if I remain connected to daplink throughout. I'll try a combination of stripping out code down and building that example up to try to figure out what makes the difference. |
OK, that didn't take long. It reproduces in the demo if you set the delay passed to startSerialRead to 1 (as per V2 & V3) rather than the default of 100. If I set the delay to 10 then I can only reproduce it sporadically. So perhaps it's related to whether a serial read is in progress when the device is disconnected, the odds of which are significantly higher with 1ms delay. |
Potentially related discussion about the interval. It's not really clear from issues how happy folks were with the net result of the changes. ARMmbed/dapjs#51 We can't really guarantee shutting down serial before the user closes the tab (we can try in beforeunload, and it does sometimes seem to run in time, but not often enough to be worth it). Ideally we'd find some way to reset whatever's left in a bad state on the device that we could run as part of the initial connection. |
It reproduces on V1 but only with 0254:
Testing was done with the V3 app and 5 cycles of the reproduction. I've redone 0253 and 0254 with the modified serial sample and significantly more iterations and I'm now convinced that 0254 has introduces this issue. Serial sample modified code (contents of
|
It looks like this can be worked around with:
where serialPromise is the promise returned from startSerialRead (so far tested with a V1 device using 0254). From quick testing this seems reliable in Chrome though I'm not sure there's any standards-basis to expect it to work. My earlier (more rushed) testing of this didn't seem so positive, but that was in the app not the serial sample. I'll retry in the app. |
I can get away with calling "stopSerialInternal" but not "disconnect" (these are V3 app device.ts methods not DAPjs). Throttling the CPU doesn't seem to stop stopSerialInternal working so perhaps it's related to how many times the event loop is allowed to run before/during unload. Unfortunately there's no event when a user decided to stay on the page so not clear when to restart serial if it was in progress. Some timeout after the page gets focus again? Nasty. |
Confirmed the same bug affects MakeCode (though I was using a Python script sending over serial). |
- Stop serial for hidden tabs to reduce chance of disconnect during serial which triggers bug. - Best effort to stop serial before unload. No errors yet with this on MacOS Chrome but not tested elsewhere. Pretty nasty though! #89
- Stop serial for hidden tabs to reduce chance of disconnect during serial which triggers bug. - Best effort to stop serial before unload. No errors yet with this on MacOS Chrome but not tested elsewhere. Pretty nasty though! #89
Confirmed the issue also exists on Windows 10 with Edge and Chrome. This workaround seems to address the issue for me on MacOS and Windows. There's no real guarantee that it'll stay working but I don't see that there's much choice but to work around it or abandon plans to make more use of serial than in V2. I guess the most likely risk is browsers further restricting access to the beforeunload hook but could also be implementation specific changes relating to how quickly the page closes. I'll split it out from my other semi-related webusb testing work and create a PR. |
- Stop serial for hidden tabs to reduce chance of disconnect during serial which triggers bug. - Best effort to stop serial before unload. No errors yet with this on MacOS Chrome but not tested elsewhere. Pretty nasty though! #89
- Stop serial for hidden tabs to reduce chance of disconnect during serial which triggers bug. - Best effort to stop serial before unload. No errors yet with this on MacOS Chrome but not tested elsewhere. Pretty nasty though! Ensure we wait for the serial polling loop in normal operation too. #89
- Stop serial for hidden tabs to reduce chance of disconnect during serial which triggers bug. - Best effort to stop serial before unload. No errors yet with this on MacOS Chrome but not tested elsewhere. Pretty nasty though! Ensure we wait for the serial polling loop in normal operation too. #89
- Stop serial for hidden tabs to reduce chance of disconnect during serial which triggers bug. - Best effort to stop serial before unload. No errors yet with this on MacOS Chrome or Windows 10 Chrome/Edge. Clearly a hack though. Ensure we wait for the serial polling loop in normal operation too. #89
- Stop serial for hidden tabs to reduce chance of disconnect during serial which triggers bug. - Best effort to stop serial before unload. No errors yet with this on MacOS Chrome or Windows 10 Chrome/Edge. Clearly a hack though. Ensure we wait for the serial polling loop in normal operation too. #89
Somewhat speculative fix as I can't get the visibilitychange event to trigger during flash anymore. It seems to occur at the end of flash. But given there's async work in the flash it makes sense that it can occur. I've added logging for the visibility change work. I think we'd like to drop all this if we could fix the underlying cause of #89. Fixes #223
Is the workaround here what causes the microbit program and serial console to constantly reset when switching tabs? This behavior is quite annoying! |
or for V2
Only physically reconnecting the device seems to fix it.
If you disconnect (or in V2 close serial) before reloading the page then it works fine.
The text was updated successfully, but these errors were encountered: