Skip to content

Commit

Permalink
Fix for reset/panic overriding the user's stop (#88)
Browse files Browse the repository at this point in the history
Switch to an enum to make the possible states easier to follow and so we
can check we're in the default state before committing to a reset or
panic.

Closes #86
  • Loading branch information
microbit-matt-hillsdon authored Oct 31, 2022
1 parent 365bad7 commit 7c5b2e6
Showing 1 changed file with 87 additions and 25 deletions.
112 changes: 87 additions & 25 deletions src/board/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -74,8 +100,6 @@ export class Board {
radio: Radio;
dataLogging: DataLogging;

private panicTimeout: any;

public serialInputBuffer: number[] = [];

private stoppedOverlay: HTMLDivElement;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand All @@ -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);
}
Expand All @@ -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<void> {
this.afterStopped = afterStopped;
async stop(brief: boolean = false): Promise<void> {
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();
Expand All @@ -422,11 +485,10 @@ export class Board {

/**
* An external reset.
* reset() in MicroPython code throws ResetError.
*/
async reset(): Promise<void> {
const noChangeRestart = () => {};
this.stop(noChangeRestart);
await this.stop(true);
return this.start();
}

async flash(filesystem: Record<string, Uint8Array>): Promise<void> {
Expand All @@ -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();
Expand Down

0 comments on commit 7c5b2e6

Please sign in to comment.