Skip to content

Commit 2d7784b

Browse files
Workarounds for #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
1 parent 53c5a11 commit 2d7784b

File tree

3 files changed

+75813
-19
lines changed

3 files changed

+75813
-19
lines changed

Diff for: src/device/device.ts

+90-19
Original file line numberDiff line numberDiff line change
@@ -133,10 +133,50 @@ export class MicrobitWebUSBConnection extends EventEmitter {
133133
*/
134134
private connection: DAPWrapper | undefined;
135135

136+
/**
137+
* DAPLink gives us a promise that lasts as long as we're serial reading.
138+
* When stopping serial we await it to be sure we're done.
139+
*/
140+
private serialReadInProgress: Promise<void> | undefined;
141+
136142
private serialListener = (data: string) => {
137143
this.emit(EVENT_SERIAL_DATA, data);
138144
};
139145

146+
private visibilityChangeListener = () => {
147+
console.log("Visibility change " + document.visibilityState);
148+
if (document.visibilityState === "visible") {
149+
// We could reconnect here if we'd previously disconnected.
150+
} else {
151+
if (!this.unloading) {
152+
this.disconnect();
153+
}
154+
}
155+
};
156+
157+
private unloading = false;
158+
159+
private beforeUnloadListener = () => {
160+
// Workaround https://github.com/microbit-foundation/python-editor-next/issues/89
161+
this.unloading = true;
162+
this.stopSerialInternal();
163+
// The user might stay on the page if they have unsaved changes and there's
164+
// another beforeunload listener. It's hard to tell that's what's happened!
165+
window.addEventListener(
166+
"focus",
167+
() => {
168+
const assumePageIsStayingOpenDelay = 1000;
169+
setTimeout(() => {
170+
if (this.status === ConnectionStatus.CONNECTED) {
171+
this.unloading = false;
172+
this.startSerialInternal();
173+
}
174+
}, assumePageIsStayingOpenDelay);
175+
},
176+
{ once: true }
177+
);
178+
};
179+
140180
private logging: Logging;
141181

142182
constructor(
@@ -154,6 +194,15 @@ export class MicrobitWebUSBConnection extends EventEmitter {
154194
if (navigator.usb) {
155195
navigator.usb.addEventListener("disconnect", this.handleDisconnect);
156196
}
197+
if (window) {
198+
window.addEventListener("beforeunload", this.beforeUnloadListener);
199+
}
200+
if (window && window.document) {
201+
window.document.addEventListener(
202+
"visibilitychange",
203+
this.visibilityChangeListener
204+
);
205+
}
157206
}
158207

159208
/**
@@ -164,6 +213,15 @@ export class MicrobitWebUSBConnection extends EventEmitter {
164213
if (navigator.usb) {
165214
navigator.usb.removeEventListener("disconnect", this.handleDisconnect);
166215
}
216+
if (window) {
217+
window.removeEventListener("beforeunload", this.beforeUnloadListener);
218+
}
219+
if (window && window.document) {
220+
window.document.removeEventListener(
221+
"visibilitychange",
222+
this.visibilityChangeListener
223+
);
224+
}
167225
}
168226

169227
/**
@@ -192,13 +250,6 @@ export class MicrobitWebUSBConnection extends EventEmitter {
192250
);
193251
}
194252

195-
private async stopSerial() {
196-
if (this.connection) {
197-
this.connection.stopSerial(this.serialListener);
198-
}
199-
this.emit(EVENT_SERIAL_RESET, {});
200-
}
201-
202253
private async flashInternal(
203254
dataSource: FlashDataSource,
204255
options: {
@@ -210,7 +261,7 @@ export class MicrobitWebUSBConnection extends EventEmitter {
210261
await this.connectInternal(false);
211262
} else {
212263
this.log("Stopping serial before flash");
213-
this.stopSerial();
264+
await this.stopSerialInternal();
214265
this.log("Reconnecting before flash");
215266
await this.connection.reconnectAsync();
216267
}
@@ -233,26 +284,46 @@ export class MicrobitWebUSBConnection extends EventEmitter {
233284

234285
this.log("Reinstating serial after flash");
235286
await this.connection.daplink.connect();
236-
237-
// This is async but won't return until we've finished serial.
238-
// TODO: consider error handling here, via an event?
239-
this.connection
240-
.startSerial(this.serialListener)
241-
.then(() => this.log("Finished listening for serial data"))
242-
.catch((e) => this.emit(EVENT_SERIAL_ERROR, e));
287+
this.startSerialInternal();
288+
await this.connection.disconnectAsync();
243289
} finally {
244290
progress(undefined);
245291
}
246292
}
247293

294+
private async startSerialInternal() {
295+
// This is async but won't return until we stop serial so we error handle with an event.
296+
if (!this.connection) {
297+
throw new Error("Must be connected now");
298+
}
299+
if (this.serialReadInProgress) {
300+
await this.stopSerialInternal();
301+
}
302+
this.serialReadInProgress = this.connection
303+
.startSerial(this.serialListener)
304+
.then(() => this.log("Finished listening for serial data"))
305+
.catch((e) => {
306+
this.emit(EVENT_SERIAL_ERROR, e);
307+
});
308+
}
309+
310+
private async stopSerialInternal() {
311+
if (this.connection && this.serialReadInProgress) {
312+
this.connection.stopSerial(this.serialListener);
313+
await this.serialReadInProgress;
314+
this.serialReadInProgress = undefined;
315+
}
316+
this.emit(EVENT_SERIAL_RESET, {});
317+
}
318+
248319
/**
249320
* Disconnect from the device.
250321
*/
251322
async disconnect(): Promise<void> {
252323
try {
253324
if (this.connection) {
254-
this.stopSerial();
255-
this.connection.disconnectAsync();
325+
await this.stopSerialInternal();
326+
await this.connection.disconnectAsync();
256327
}
257328
} catch (e) {
258329
this.log("Error during disconnection:\r\n" + e);
@@ -291,7 +362,7 @@ export class MicrobitWebUSBConnection extends EventEmitter {
291362
serialWrite(data: string): Promise<void> {
292363
return this.withEnrichedErrors(async () => {
293364
if (this.connection) {
294-
this.connection.daplink.serialWrite(data);
365+
return this.connection.daplink.serialWrite(data);
295366
}
296367
});
297368
}
@@ -311,7 +382,7 @@ export class MicrobitWebUSBConnection extends EventEmitter {
311382
}
312383
await this.connection.reconnectAsync();
313384
if (serial) {
314-
this.connection.startSerial(this.serialListener);
385+
this.startSerialInternal();
315386
}
316387
this.setStatus(ConnectionStatus.CONNECTED);
317388
}

0 commit comments

Comments
 (0)