From a4246bcfa3b3305fed60732e46451cb3ca1c41b3 Mon Sep 17 00:00:00 2001 From: Scott Shawcroft Date: Mon, 30 Aug 2021 18:05:14 -0700 Subject: [PATCH 1/3] Fix two watchdog crashes Fixes a crash from trying to raise an exception when trying to deinit a RESET wdt by not raising an exception. Fixes a crash when raise a wdt exception in the REPL when waiting for input. We now catch and print any exceptions raised. Fixes #5261 --- lib/utils/pyexec.c | 16 +++++++++++++++- locale/circuitpython.pot | 1 + ports/nrf/common-hal/watchdog/WatchDogTimer.c | 7 ++++++- supervisor/shared/bluetooth/file_transfer.c | 2 +- 4 files changed, 23 insertions(+), 3 deletions(-) diff --git a/lib/utils/pyexec.c b/lib/utils/pyexec.c index c1bb66aad1..a272eefbcb 100644 --- a/lib/utils/pyexec.c +++ b/lib/utils/pyexec.c @@ -650,7 +650,21 @@ friendly_repl_reset: } vstr_reset(&line); - int ret = readline(&line, ">>> "); + + nlr_buf_t nlr; + nlr.ret_val = NULL; + int ret = 0; + if (nlr_push(&nlr) == 0) { + ret = readline(&line, ">>> "); + } else { + // Uncaught exception + mp_handle_pending(false); // clear any pending exceptions (and run any callbacks) + + // Print exceptions but stay in the REPL. There are very few delayed + // exceptions. The WatchDogTimer can raise one though. + mp_hal_stdout_tx_str("\r\n"); + mp_obj_print_exception(&mp_plat_print, MP_OBJ_FROM_PTR(nlr.ret_val)); + } mp_parse_input_kind_t parse_input_kind = MP_PARSE_SINGLE_INPUT; if (ret == CHAR_CTRL_A) { diff --git a/locale/circuitpython.pot b/locale/circuitpython.pot index a614078beb..12b42cddf7 100644 --- a/locale/circuitpython.pot +++ b/locale/circuitpython.pot @@ -3939,6 +3939,7 @@ msgstr "" #: ports/esp32s2/boards/gravitech_cucumber_r/mpconfigboard.h #: ports/esp32s2/boards/gravitech_cucumber_rs/mpconfigboard.h #: ports/esp32s2/boards/lilygo_ttgo_t8_s2_st7789/mpconfigboard.h +#: ports/esp32s2/boards/lolin_s2_mini/mpconfigboard.h #: ports/esp32s2/boards/microdev_micro_s2/mpconfigboard.h #: ports/esp32s2/boards/morpheans_morphesp-240/mpconfigboard.h #: ports/esp32s2/boards/muselab_nanoesp32_s2_wroom/mpconfigboard.h diff --git a/ports/nrf/common-hal/watchdog/WatchDogTimer.c b/ports/nrf/common-hal/watchdog/WatchDogTimer.c index 3423d3466b..1798c52609 100644 --- a/ports/nrf/common-hal/watchdog/WatchDogTimer.c +++ b/ports/nrf/common-hal/watchdog/WatchDogTimer.c @@ -28,6 +28,7 @@ #include #include +#include "py/gc.h" #include "py/obj.h" #include "py/objproperty.h" #include "py/runtime.h" @@ -94,7 +95,11 @@ void common_hal_watchdog_feed(watchdog_watchdogtimer_obj_t *self) { void common_hal_watchdog_deinit(watchdog_watchdogtimer_obj_t *self) { if (self->mode == WATCHDOGMODE_RESET) { - mp_raise_NotImplementedError(translate("WatchDogTimer cannot be deinitialized once mode is set to RESET")); + if (gc_alloc_possible()) { + mp_raise_NotImplementedError(translate("WatchDogTimer cannot be deinitialized once mode is set to RESET")); + } + // Don't change anything because RESET cannot be undone. + return; } if (timer) { timer_free(); diff --git a/supervisor/shared/bluetooth/file_transfer.c b/supervisor/shared/bluetooth/file_transfer.c index d105892582..908cc958d6 100644 --- a/supervisor/shared/bluetooth/file_transfer.c +++ b/supervisor/shared/bluetooth/file_transfer.c @@ -404,7 +404,7 @@ STATIC uint8_t _process_delete(const uint8_t *raw_buf, size_t command_len) { FATFS *fs = &((fs_user_mount_t *)MP_STATE_VM(vfs_mount_table)->obj)->fatfs; char *path = (char *)((uint8_t *)command) + header_size; path[command->path_length] = '\0'; - FRESULT result; + FRESULT result = FR_OK; FILINFO file; if (f_stat(fs, path, &file) == FR_OK) { if ((file.fattrib & AM_DIR) != 0) { From 771b4c7464f0ca30fd5e225c758167a7d1b36dd2 Mon Sep 17 00:00:00 2001 From: Scott Shawcroft Date: Tue, 31 Aug 2021 13:02:34 -0700 Subject: [PATCH 2/3] Add two space saving knobs * Reduce the number of supported HID reports of IDs per descriptor. This saves ~200 bytes in the default HID objects. * (Not enabled) Compute QSTR attrs on init. This trades 1k RAM for flash. Flash is the default (1). --- ports/atmel-samd/mpconfigport.h | 3 +++ py/circuitpy_mpconfig.h | 17 +++++++++++++++++ py/qstr.c | 16 +++++++++++++++- shared-module/usb_hid/Device.c | 14 +++++++------- shared-module/usb_hid/Device.h | 15 +++++---------- 5 files changed, 47 insertions(+), 18 deletions(-) diff --git a/ports/atmel-samd/mpconfigport.h b/ports/atmel-samd/mpconfigport.h index 6332c9d3fc..e200ce67db 100644 --- a/ports/atmel-samd/mpconfigport.h +++ b/ports/atmel-samd/mpconfigport.h @@ -61,6 +61,9 @@ #define MICROPY_FATFS_EXFAT 0 +// Only support simpler HID descriptors on SAMD21. +#define CIRCUITPY_USB_HID_MAX_REPORT_IDS_PER_DESCRIPTOR (1) + #endif // SAMD21 //////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/py/circuitpy_mpconfig.h b/py/circuitpy_mpconfig.h index 91e71078fb..8a34c346e4 100644 --- a/py/circuitpy_mpconfig.h +++ b/py/circuitpy_mpconfig.h @@ -717,6 +717,13 @@ void supervisor_run_background_tasks_if_tick(void); #define CIRCUITPY_VERBOSE_BLE 0 +// This trades ~1k flash space (1) for that much in RAM plus the cost to compute +// the values once on init (0). Only turn it off, when you really need the flash +// space and are willing to trade the RAM. +#ifndef CIRCUITPY_PRECOMPUTE_QSTR_ATTR +#define CIRCUITPY_PRECOMPUTE_QSTR_ATTR (1) +#endif + // USB settings // If the port requires certain USB endpoint numbers, define these in mpconfigport.h. @@ -761,6 +768,16 @@ void supervisor_run_background_tasks_if_tick(void); #define USB_HID_EP_NUM_IN (0) #endif +// The most complicated device currently known of is the head and eye tracker, which requires 5 +// report ids. +// https://usb.org/sites/default/files/hutrr74_-_usage_page_for_head_and_eye_trackers_0.pdf +// The default descriptors only use 1, so that is the minimum. +#ifndef CIRCUITPY_USB_HID_MAX_REPORT_IDS_PER_DESCRIPTOR +#define CIRCUITPY_USB_HID_MAX_REPORT_IDS_PER_DESCRIPTOR (6) +#elif CIRCUITPY_USB_HID_MAX_REPORT_IDS_PER_DESCRIPTOR < 1 +#error "CIRCUITPY_USB_HID_MAX_REPORT_IDS_PER_DESCRIPTOR must be at least 1" +#endif + #ifndef USB_MIDI_EP_NUM_OUT #define USB_MIDI_EP_NUM_OUT (0) #endif diff --git a/py/qstr.c b/py/qstr.c index 230a230909..9ea7ab83db 100644 --- a/py/qstr.c +++ b/py/qstr.c @@ -77,7 +77,8 @@ mp_uint_t qstr_compute_hash(const byte *data, size_t len) { return hash; } -const qstr_attr_t mp_qstr_const_attr[] = { +#if CIRCUITPY_PRECOMPUTE_QSTR_ATTR +const qstr_attr_t mp_qstr_const_attr[MP_QSTRnumber_of] = { #ifndef NO_QSTR #define QDEF(id, hash, len, str) { hash, len }, #define TRANSLATION(id, length, compressed ...) @@ -86,6 +87,9 @@ const qstr_attr_t mp_qstr_const_attr[] = { #undef QDEF #endif }; +#else +qstr_attr_t mp_qstr_const_attr[MP_QSTRnumber_of]; +#endif const qstr_pool_t mp_qstr_const_pool = { NULL, // no previous pool @@ -115,6 +119,16 @@ void qstr_init(void) { MP_STATE_VM(last_pool) = (qstr_pool_t *)&CONST_POOL; // we won't modify the const_pool since it has no allocated room left MP_STATE_VM(qstr_last_chunk) = NULL; + #if CIRCUITPY_PRECOMPUTE_QSTR_ATTR == 0 + if (mp_qstr_const_attr[MP_QSTR_circuitpython].len == 0) { + for (size_t i = 0; i < mp_qstr_const_pool.len; i++) { + size_t len = strlen(mp_qstr_const_pool.qstrs[i]); + mp_qstr_const_attr[i].hash = qstr_compute_hash((const byte *)mp_qstr_const_pool.qstrs[i], len); + mp_qstr_const_attr[i].len = len; + } + } + #endif + #if MICROPY_PY_THREAD && !MICROPY_PY_THREAD_GIL mp_thread_mutex_init(&MP_STATE_VM(qstr_mutex)); #endif diff --git a/shared-module/usb_hid/Device.c b/shared-module/usb_hid/Device.c index e864b08873..71c6dea7af 100644 --- a/shared-module/usb_hid/Device.c +++ b/shared-module/usb_hid/Device.c @@ -81,7 +81,7 @@ const usb_hid_device_obj_t usb_hid_device_keyboard_obj = { .usage = 0x06, .num_report_ids = 1, .report_ids = { 0x01, }, - .in_report_lengths = { 8, 0, 0, 0, 0, 0, }, + .in_report_lengths = { 8, }, .out_report_lengths = { 1, }, }; @@ -131,7 +131,7 @@ const usb_hid_device_obj_t usb_hid_device_mouse_obj = { .usage = 0x02, .num_report_ids = 1, .report_ids = { 0x02, }, - .in_report_lengths = { 4, 0, 0, 0, 0, 0, }, + .in_report_lengths = { 4, }, .out_report_lengths = { 0, }, }; @@ -160,7 +160,7 @@ const usb_hid_device_obj_t usb_hid_device_consumer_control_obj = { .usage = 0x01, .num_report_ids = 1, .report_ids = { 0x03 }, - .in_report_lengths = { 2, 0, 0, 0, 0, 0, }, + .in_report_lengths = { 2, }, .out_report_lengths = { 0, }, }; @@ -170,7 +170,7 @@ STATIC size_t get_report_id_idx(usb_hid_device_obj_t *self, size_t report_id) { return i; } } - return MAX_REPORT_IDS_PER_DESCRIPTOR; + return CIRCUITPY_USB_HID_MAX_REPORT_IDS_PER_DESCRIPTOR; } // See if report_id is used by this device. If it is -1, then return the sole report id used by this device, @@ -180,16 +180,16 @@ uint8_t common_hal_usb_hid_device_validate_report_id(usb_hid_device_obj_t *self, return self->report_ids[0]; } if (!(report_id_arg >= 0 && - get_report_id_idx(self, (size_t)report_id_arg) < MAX_REPORT_IDS_PER_DESCRIPTOR)) { + get_report_id_idx(self, (size_t)report_id_arg) < CIRCUITPY_USB_HID_MAX_REPORT_IDS_PER_DESCRIPTOR)) { mp_raise_ValueError_varg(translate("Invalid %q"), MP_QSTR_report_id); } return (uint8_t)report_id_arg; } void common_hal_usb_hid_device_construct(usb_hid_device_obj_t *self, mp_obj_t report_descriptor, uint8_t usage_page, uint8_t usage, size_t num_report_ids, uint8_t *report_ids, uint8_t *in_report_lengths, uint8_t *out_report_lengths) { - if (num_report_ids > MAX_REPORT_IDS_PER_DESCRIPTOR) { + if (num_report_ids > CIRCUITPY_USB_HID_MAX_REPORT_IDS_PER_DESCRIPTOR) { mp_raise_ValueError_varg(translate("More than %d report ids not supported"), - MAX_REPORT_IDS_PER_DESCRIPTOR); + CIRCUITPY_USB_HID_MAX_REPORT_IDS_PER_DESCRIPTOR); } // report buffer pointers are NULL at start, and are created when USB is initialized. diff --git a/shared-module/usb_hid/Device.h b/shared-module/usb_hid/Device.h index a3c523acd5..5a09d19526 100644 --- a/shared-module/usb_hid/Device.h +++ b/shared-module/usb_hid/Device.h @@ -32,21 +32,16 @@ #include "py/obj.h" -// The most complicated device currently known of is the head and eye tracker, which requires 5 -// report ids. -// https://usb.org/sites/default/files/hutrr74_-_usage_page_for_head_and_eye_trackers_0.pdf -#define MAX_REPORT_IDS_PER_DESCRIPTOR (6) - typedef struct { mp_obj_base_t base; // Python buffer object whose contents are the descriptor. const uint8_t *report_descriptor; - uint8_t *in_report_buffers[MAX_REPORT_IDS_PER_DESCRIPTOR]; - uint8_t *out_report_buffers[MAX_REPORT_IDS_PER_DESCRIPTOR]; + uint8_t *in_report_buffers[CIRCUITPY_USB_HID_MAX_REPORT_IDS_PER_DESCRIPTOR]; + uint8_t *out_report_buffers[CIRCUITPY_USB_HID_MAX_REPORT_IDS_PER_DESCRIPTOR]; uint16_t report_descriptor_length; - uint8_t report_ids[MAX_REPORT_IDS_PER_DESCRIPTOR]; - uint8_t in_report_lengths[MAX_REPORT_IDS_PER_DESCRIPTOR]; - uint8_t out_report_lengths[MAX_REPORT_IDS_PER_DESCRIPTOR]; + uint8_t report_ids[CIRCUITPY_USB_HID_MAX_REPORT_IDS_PER_DESCRIPTOR]; + uint8_t in_report_lengths[CIRCUITPY_USB_HID_MAX_REPORT_IDS_PER_DESCRIPTOR]; + uint8_t out_report_lengths[CIRCUITPY_USB_HID_MAX_REPORT_IDS_PER_DESCRIPTOR]; uint8_t usage_page; uint8_t usage; uint8_t num_report_ids; From 92a43192f81c7bb36f122112c632c6b42eb3a311 Mon Sep 17 00:00:00 2001 From: Scott Shawcroft Date: Tue, 31 Aug 2021 13:38:37 -0700 Subject: [PATCH 3/3] Fix mpy-cross by providing default --- py/qstr.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/py/qstr.c b/py/qstr.c index 9ea7ab83db..d7b05930f7 100644 --- a/py/qstr.c +++ b/py/qstr.c @@ -76,8 +76,10 @@ mp_uint_t qstr_compute_hash(const byte *data, size_t len) { } return hash; } - -#if CIRCUITPY_PRECOMPUTE_QSTR_ATTR +#ifndef CIRCUITPY_PRECOMPUTE_QSTR_ATTR +#define CIRCUITPY_PRECOMPUTE_QSTR_ATTR (1) +#endif +#if CIRCUITPY_PRECOMPUTE_QSTR_ATTR == 1 const qstr_attr_t mp_qstr_const_attr[MP_QSTRnumber_of] = { #ifndef NO_QSTR #define QDEF(id, hash, len, str) { hash, len },