Skip to content

Commit aadf2e6

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! #89
1 parent 9fddcef commit aadf2e6

File tree

1 file changed

+90
-19
lines changed

1 file changed

+90
-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)