From 08c365eff5325d684dcfe9783fe6d5314403a4fe Mon Sep 17 00:00:00 2001 From: Matt Hillsdon Date: Wed, 2 Nov 2022 09:23:27 +0000 Subject: [PATCH 1/2] Revert "Revert "Fix for reset/panic overriding the user's stop (#88)"" This reverts commit 9d7c83494529c2cc6cc3fcd3091047a17bc26d7e. --- src/board/index.ts | 112 +++++++++++++++++++++++++++++++++++---------- 1 file changed, 87 insertions(+), 25 deletions(-) diff --git a/src/board/index.ts b/src/board/index.ts index 9cf57b4c..9f662c61 100644 --- a/src/board/index.ts +++ b/src/board/index.ts @@ -35,6 +35,32 @@ import { Radio } from "./radio"; import { RangeSensor, State } from "./state"; import { ModuleWrapper } from "./wasm"; +enum StopKind { + /** + * The main Wasm function returned control to us in a normal way. + */ + Default = "default", + /** + * The program called panic. + */ + Panic = "panic", + /** + * The program requested a reset. + */ + Reset = "reset", + /** + * An internal mode where we do not display the stop state UI as we plan to immediately reset. + * Used for user-requested flash or reset. + */ + BriefStop = "brief", + /** + * The user requested the program be interrupted. + * + * Note the program could finish for other reasons, but should always count as a user stop. + */ + UserStop = "user", +} + export class PanicError extends Error { constructor(public code: number) { super("panic"); @@ -74,8 +100,6 @@ export class Board { radio: Radio; dataLogging: DataLogging; - private panicTimeout: any; - public serialInputBuffer: number[] = []; private stoppedOverlay: HTMLDivElement; @@ -108,10 +132,19 @@ export class Board { */ private module: ModuleWrapper | undefined; /** - * If undefined, then when main finishes we stay stopped. - * Otherwise we perform the action then clear this field. + * Controls the action after the user program completes. + * + * Determined by a combination of user actions (stop, reset etc) and program actions. */ - private afterStopped: (() => void) | undefined; + private stopKind: StopKind = StopKind.Default; + /** + * Timeout for a pending start call due to StopKind.Reset. + */ + private pendingRestartTimeout: any; + /** + * Timeout for the next frame of the panic animation. + */ + private panicTimeout: any; constructor( private notifications: Notifications, @@ -356,6 +389,8 @@ export class Board { if (this.modulePromise || this.module) { throw new Error("Module already exists!"); } + clearTimeout(this.pendingRestartTimeout); + this.pendingRestartTimeout = null; this.modulePromise = this.createModule(); const module = await this.modulePromise; @@ -365,11 +400,17 @@ export class Board { this.displayRunningState(); await module.start(); } catch (e: any) { + // Take care not to overwrite another kind of stop just because the program + // called restart or panic. if (e instanceof PanicError) { - panicCode = e.code; + if (this.stopKind === StopKind.Default) { + this.stopKind = StopKind.Panic; + panicCode = e.code; + } } else if (e instanceof ResetError) { - const noChangeRestart = () => {}; - this.afterStopped = noChangeRestart; + if (this.stopKind === StopKind.Default) { + this.stopKind = StopKind.Reset; + } } else { this.notifications.onInternalError(e); } @@ -386,30 +427,52 @@ export class Board { this.modulePromise = undefined; this.module = undefined; - if (panicCode !== undefined) { - this.displayPanic(panicCode); - } else { - if (this.afterStopped) { - this.afterStopped(); - this.afterStopped = undefined; - setTimeout(() => this.start(), 0); - } else { + switch (this.stopKind) { + case StopKind.Panic: { + if (panicCode === undefined) { + throw new Error("Must be set"); + } + this.displayPanic(panicCode); + break; + } + case StopKind.Reset: { + this.pendingRestartTimeout = setTimeout(() => this.start(), 0); + break; + } + case StopKind.BriefStop: { + // Skip the stopped state. + break; + } + case StopKind.UserStop: /* Fall through */ + case StopKind.Default: { this.displayStoppedState(); + break; + } + default: { + throw new Error("Unknown stop kind: " + this.stopKind); } } + this.stopKind = StopKind.Default; } - async stop( - afterStopped: (() => void) | undefined = undefined - ): Promise { - this.afterStopped = afterStopped; + async stop(brief: boolean = false): Promise { if (this.panicTimeout) { clearTimeout(this.panicTimeout); this.panicTimeout = null; this.display.clear(); - this.displayStoppedState(); + if (!brief) { + this.displayStoppedState(); + } + } + if (this.pendingRestartTimeout) { + clearTimeout(this.pendingRestartTimeout); + this.pendingRestartTimeout = null; + if (!brief) { + this.displayStoppedState(); + } } if (this.modulePromise) { + this.stopKind = brief ? StopKind.BriefStop : StopKind.UserStop; // Avoid this.module as we might still be creating it (async). const module = await this.modulePromise; module.requestStop(); @@ -422,11 +485,10 @@ export class Board { /** * An external reset. - * reset() in MicroPython code throws ResetError. */ async reset(): Promise { - const noChangeRestart = () => {}; - this.stop(noChangeRestart); + await this.stop(true); + return this.start(); } async flash(filesystem: Record): Promise { @@ -440,7 +502,7 @@ export class Board { }; if (this.modulePromise) { // If it's running then we need to stop before flash. - return this.stop(flashFileSystem); + await this.stop(true); } flashFileSystem(); return this.start(); From 556c80b81a22feaea863169fb61c7f7043109e4d Mon Sep 17 00:00:00 2001 From: Matt Hillsdon Date: Wed, 2 Nov 2022 09:48:13 +0000 Subject: [PATCH 2/2] Make `stop` actually stop the simulator. It's behaviour now matches how it's used for restart and flash. --- src/board/index.ts | 40 ++++++++++++++++++++++++++++++++-------- 1 file changed, 32 insertions(+), 8 deletions(-) diff --git a/src/board/index.ts b/src/board/index.ts index 9f662c61..ed53b6b3 100644 --- a/src/board/index.ts +++ b/src/board/index.ts @@ -123,12 +123,16 @@ export class Board { return result ?? id; }; + /** + * Defined for the duration of start(). + */ + private runningPromise: Promise | undefined; /** * Defined during start(). */ private modulePromise: Promise | undefined; /** - * Defined by start but async. + * Defined during start(). */ private module: ModuleWrapper | undefined; /** @@ -385,7 +389,19 @@ export class Board { this.stoppedOverlay.style.display = "flex"; } - private async start() { + /** + * Start the simulator. + * + * @returns a promise that resolves when the simulator has stopped. + */ + private start(): void { + if (this.runningPromise) { + throw new Error("Already running!"); + } + this.runningPromise = this.createRunningPromise(); + } + + private async createRunningPromise() { if (this.modulePromise || this.module) { throw new Error("Module already exists!"); } @@ -453,8 +469,17 @@ export class Board { } } this.stopKind = StopKind.Default; + this.runningPromise = undefined; } + /** + * Stop the simulator. + * + * This cancels any pending restart or panic requested by the program. + * + * @param brief If true the stopped UI is not shown. + * @returns A promise that resolves when the simulator is stopped. + */ async stop(brief: boolean = false): Promise { if (this.panicTimeout) { clearTimeout(this.panicTimeout); @@ -481,6 +506,7 @@ export class Board { // Ctrl-C, Ctrl-D to interrupt the main loop. this.writeSerialInput("\x03\x04"); } + return this.runningPromise; } /** @@ -488,7 +514,7 @@ export class Board { */ async reset(): Promise { await this.stop(true); - return this.start(); + this.start(); } async flash(filesystem: Record): Promise { @@ -500,10 +526,8 @@ export class Board { }); this.dataLogging.delete(); }; - if (this.modulePromise) { - // If it's running then we need to stop before flash. - await this.stop(true); - } + // Ensure it's stopped before flash. + await this.stop(true); flashFileSystem(); return this.start(); } @@ -648,7 +672,7 @@ export class Board { writeRadioRxBuffer(packet: Uint8Array): number { if (!this.module) { - throw new Error("Must be running"); + throw new Error("Must be running as called via HAL"); } return this.module.writeRadioRxBuffer(packet); }