These changes are meant to improve the flashing problems we're seeing, and address #1060, #1078, and possibly others too.
Without these changes, I am able to reproduce #1078 with 100% reliability on Linux (but not on Windows or macOS). With the patches, I can't reproduce it on either.
I've also reproduced - reliably - multiple problems on Windows: one where we failed to reconnect after flash, and another where we succeeded, but failed to reconnect after the eeprom wipe. This patchset appears to fix those too.
I have tested the changes on all three platforms, with and without chunked writes on all three of them, and they worked every case, multiple times in a row.
On the other hand, it doesn't feel great that we're still finnicky when it comes to timing. The reason these issues were present - as far as I can explain, at least - is that we tried to reconnect when the keyboard wasn't fully ready yet, while it was still booting up: serial port up, but not fully booted yet, or something along those lines? Or VID/PID present, but no port yet, in at least one case. I'm not sure how plausible these explanations are, but this is the best idea I have to explain what happens, and why a few delays seem to help.
The patchset also decouples new SerialPort from opening the device, and we do the open separately. We do this so we can await on the open, and trap it in a try/catch block. We still throw an error, but hopefully we'll have more context this way. I've made this change to try and fix / workaround a case where we received an uncaught exception during open, originating from a Promise within SerialPort. We can't catch that during object construction time, because it's in a promise, and we can't await on the new SerialPort. Hence, separating the open, which we can await on. At least it won't be uncaught in a promise this time. This seems to have absolutely no effect on robustness, all the tests I made worked fine with only the delays. But I feel it's a useful debugging aid at the worst, and thus, made sense to include it in this PR.
These changes are meant to improve the flashing problems we're seeing, and address #1060, #1078, and possibly others too.
Without these changes, I am able to reproduce #1078 with 100% reliability on Linux (but not on Windows or macOS). With the patches, I can't reproduce it on either.
I've also reproduced - reliably - multiple problems on Windows: one where we failed to reconnect after flash, and another where we succeeded, but failed to reconnect after the eeprom wipe. This patchset appears to fix those too.
I have tested the changes on all three platforms, with and without chunked writes on all three of them, and they worked every case, multiple times in a row.
On the other hand, it doesn't feel great that we're still finnicky when it comes to timing. The reason these issues were present - as far as I can explain, at least - is that we tried to reconnect when the keyboard wasn't fully ready yet, while it was still booting up: serial port up, but not fully booted yet, or something along those lines? Or VID/PID present, but no port yet, in at least one case. I'm not sure how plausible these explanations are, but this is the best idea I have to explain what happens, and why a few delays seem to help.
The patchset also decouples
new SerialPortfrom opening the device, and we do the open separately. We do this so we canawaiton the open, and trap it in atry/catchblock. We still throw an error, but hopefully we'll have more context this way. I've made this change to try and fix / workaround a case where we received an uncaught exception during open, originating from a Promise within SerialPort. We can't catch that during object construction time, because it's in a promise, and we can't await on thenew SerialPort. Hence, separating the open, which we can await on. At least it won't be uncaught in a promise this time. This seems to have absolutely no effect on robustness, all the tests I made worked fine with only the delays. But I feel it's a useful debugging aid at the worst, and thus, made sense to include it in this PR.