From ae65366d10a97f1b9e7a21761dcb93f657aa6442 Mon Sep 17 00:00:00 2001 From: Matt Hillsdon Date: Fri, 9 Sep 2022 20:48:58 +0100 Subject: [PATCH] Add filesystem limit. - Limit approximately matches the device and prevents running out of space. - Allow flash to exceed limit to avoid new error scenario. This would be good to revisit in future. - Fix broken listdir due to missing Module prefix. - Align flash and reset behaviour. Closes https://github.com/microbit-foundation/python-editor-next/issues/950 --- src/board/fs.ts | 18 +++++++++++++++--- src/board/index.ts | 39 +++++++++++++++++++++++++-------------- src/demo.html | 8 ++++++++ src/jshal.h | 2 +- src/jshal.js | 2 +- src/microbitfs.c | 7 ++++++- 6 files changed, 56 insertions(+), 20 deletions(-) diff --git a/src/board/fs.ts b/src/board/fs.ts index 2ce75b90..c88a5663 100644 --- a/src/board/fs.ts +++ b/src/board/fs.ts @@ -1,7 +1,11 @@ +// Size as per C implementation. +const maxSize = 31.5 * 1024; + export class FileSystem { // Each entry is an FsFile object. The indexes are used as identifiers. // When a file is deleted the entry becomes ['', null] and can be reused. private _content: Array = []; + private _size = 0; create(name: string) { let free_idx = -1; @@ -49,7 +53,11 @@ export class FileSystem { } remove(idx: number) { - this._content[idx] = null; + const file = this._content[idx]; + if (file) { + this._size -= file.size(); + this._content[idx] = null; + } } readbyte(idx: number, offset: number) { @@ -57,13 +65,17 @@ export class FileSystem { return file ? file.readbyte(offset) : -1; } - write(idx: number, data: Uint8Array) { + write(idx: number, data: Uint8Array, force: boolean = false): boolean { const file = this._content[idx]; if (!file) { throw new Error("File must exist"); } + if (!force && this._size + data.length > maxSize) { + return false; + } + this._size += data.length; file.append(data); - return data.length; + return true; } clear() { diff --git a/src/board/index.ts b/src/board/index.ts index 894cb142..8f398987 100644 --- a/src/board/index.ts +++ b/src/board/index.ts @@ -82,9 +82,10 @@ export class Board { */ private modulePromise: Promise | undefined; /** - * Flag to trigger a reset after start finishes. + * If undefined, then when main finishes we stay stopped. + * Otherwise we perform the action then clear this field. */ - private resetWhenDone: boolean = false; + private afterStopped: (() => void) | undefined; constructor( private notifications: Notifications, @@ -339,8 +340,9 @@ export class Board { if (panicCode !== undefined) { this.displayPanic(panicCode); } else { - if (this.resetWhenDone) { - this.resetWhenDone = false; + if (this.afterStopped) { + this.afterStopped(); + this.afterStopped = undefined; setTimeout(() => this.start(), 0); } else { this.displayStoppedState(); @@ -348,8 +350,10 @@ export class Board { } } - async stop(reset: boolean = false): Promise { - this.resetWhenDone = reset; + async stop( + afterStopped: (() => void) | undefined = undefined + ): Promise { + this.afterStopped = afterStopped; if (this.panicTimeout) { clearTimeout(this.panicTimeout); this.panicTimeout = null; @@ -366,17 +370,24 @@ export class Board { } async reset(): Promise { - this.stop(true); + const noChangeRestart = () => {}; + this.stop(noChangeRestart); } async flash(filesystem: Record): Promise { - await this.stop(); - this.fs.clear(); - Object.entries(filesystem).forEach(([name, value]) => { - const idx = this.fs.create(name); - this.fs.write(idx, value); - }); - this.dataLogging.delete(); + const flashFileSystem = () => { + this.fs.clear(); + Object.entries(filesystem).forEach(([name, value]) => { + const idx = this.fs.create(name); + this.fs.write(idx, value, true); + }); + this.dataLogging.delete(); + }; + if (this.modulePromise) { + // If it's running then we need to stop before flash. + return this.stop(flashFileSystem); + } + flashFileSystem(); return this.start(); } diff --git a/src/demo.html b/src/demo.html index ab8c4e68..438d421a 100644 --- a/src/demo.html +++ b/src/demo.html @@ -197,6 +197,14 @@

MicroPython-micro:bit simulator example embedding

); break; } + case "internal_error": { + console.error("Simulator internal error:"); + console.error(e.data.error); + break; + } + default: { + // Ignore unknown message + } } } }); diff --git a/src/jshal.h b/src/jshal.h index f88f7454..29811063 100644 --- a/src/jshal.h +++ b/src/jshal.h @@ -37,7 +37,7 @@ int mp_js_hal_filesystem_name(int idx, char *buf); int mp_js_hal_filesystem_size(int idx); void mp_js_hal_filesystem_remove(int idx); int mp_js_hal_filesystem_readbyte(int idx, size_t offset); -int mp_js_hal_filesystem_write(int idx, const char *buf, size_t len); +bool mp_js_hal_filesystem_write(int idx, const char *buf, size_t len); void mp_js_hal_panic(int code); void mp_js_hal_reset(void); diff --git a/src/jshal.js b/src/jshal.js index 6c3caf2c..87fa585e 100644 --- a/src/jshal.js +++ b/src/jshal.js @@ -56,7 +56,7 @@ mergeInto(LibraryManager.library, { }, mp_js_hal_filesystem_name: function (idx, buf) { - const name = fs.name(idx); + const name = Module.fs.name(idx); if (name === undefined) { return -1; } diff --git a/src/microbitfs.c b/src/microbitfs.c index 0f1c3047..fc685e57 100644 --- a/src/microbitfs.c +++ b/src/microbitfs.c @@ -209,7 +209,12 @@ STATIC mp_uint_t microbit_file_write(mp_obj_t self_in, const void *buf, mp_uint_ *errcode = MP_EBADF; return MP_STREAM_ERROR; } - return mp_js_hal_filesystem_write(self->idx, buf, size); + bool success = mp_js_hal_filesystem_write(self->idx, buf, size); + if (!success) { + *errcode = MP_ENOSPC; + return MP_STREAM_ERROR; + } + return size; } STATIC mp_uint_t microbit_file_ioctl(mp_obj_t self_in, mp_uint_t request, uintptr_t arg, int *errcode) {