From 097af804cd52152d23af71fab621ec8f27bc6889 Mon Sep 17 00:00:00 2001 From: Scott Shawcroft Date: Fri, 7 Apr 2023 09:49:51 -0700 Subject: [PATCH] Fix ticks In #7497 port_background_task was renamed to port_background_tick but the actual call site wasn't changed. This meant that it was no longer called! Rename more functions from task to tick to make it clearer which is which. --- ports/atmel-samd/background.c | 8 ++++---- ports/broadcom/background.c | 4 ++-- ports/cxd56/background.c | 4 ++-- ports/espressif/background.c | 4 ++-- ports/litex/background.c | 4 ++-- ports/mimxrt10xx/background.c | 4 ++-- ports/nrf/background.c | 4 ++-- ports/raspberrypi/background.c | 4 ++-- ports/stm/background.c | 4 ++-- py/circuitpy_mpconfig.h | 4 ++-- supervisor/background_callback.h | 11 +++++++++-- supervisor/port.h | 11 ++++++----- supervisor/shared/tick.c | 17 ++++++----------- supervisor/shared/tick.h | 9 +-------- 14 files changed, 44 insertions(+), 48 deletions(-) diff --git a/ports/atmel-samd/background.c b/ports/atmel-samd/background.c index 6e1dc71d85..2f8357d07b 100644 --- a/ports/atmel-samd/background.c +++ b/ports/atmel-samd/background.c @@ -42,18 +42,18 @@ // PB03 is physical pin "SCL" on the Metro M4 express // so you can't use this code AND an i2c peripheral // at the same time unless you change this -void port_start_background_task(void) { +void port_start_background_tick(void) { REG_PORT_DIRSET1 = (1 << 3); REG_PORT_OUTSET1 = (1 << 3); } -void port_finish_background_task(void) { +void port_finish_background_tick(void) { REG_PORT_OUTCLR1 = (1 << 3); } #else -void port_start_background_task(void) { +void port_start_background_tick(void) { } -void port_finish_background_task(void) { +void port_finish_background_tick(void) { } #endif diff --git a/ports/broadcom/background.c b/ports/broadcom/background.c index 5d92f1b8bf..9074a80400 100644 --- a/ports/broadcom/background.c +++ b/ports/broadcom/background.c @@ -28,9 +28,9 @@ #include "py/runtime.h" #include "supervisor/port.h" -void port_start_background_task(void) { +void port_start_background_tick(void) { } -void port_finish_background_task(void) { +void port_finish_background_tick(void) { } void port_background_tick(void) { diff --git a/ports/cxd56/background.c b/ports/cxd56/background.c index da172cf5d7..8425f8cfa2 100644 --- a/ports/cxd56/background.c +++ b/ports/cxd56/background.c @@ -34,7 +34,7 @@ void port_background_tick(void) { } void port_background_task(void) { } -void port_start_background_task(void) { +void port_start_background_tick(void) { } -void port_finish_background_task(void) { +void port_finish_background_tick(void) { } diff --git a/ports/espressif/background.c b/ports/espressif/background.c index 3fac768f3f..f9069e98a4 100644 --- a/ports/espressif/background.c +++ b/ports/espressif/background.c @@ -51,8 +51,8 @@ void port_background_tick(void) { void port_background_task(void) { } -void port_start_background_task(void) { +void port_start_background_tick(void) { } -void port_finish_background_task(void) { +void port_finish_background_tick(void) { } diff --git a/ports/litex/background.c b/ports/litex/background.c index 1329d5fd83..e641daf53b 100644 --- a/ports/litex/background.c +++ b/ports/litex/background.c @@ -34,7 +34,7 @@ void port_background_task(void) { } void port_background_tick(void) { } -void port_start_background_task(void) { +void port_start_background_tick(void) { } -void port_finish_background_task(void) { +void port_finish_background_tick(void) { } diff --git a/ports/mimxrt10xx/background.c b/ports/mimxrt10xx/background.c index faa2bda3f8..04de4c1c1c 100644 --- a/ports/mimxrt10xx/background.c +++ b/ports/mimxrt10xx/background.c @@ -37,8 +37,8 @@ void PLACE_IN_ITCM(port_background_task)(void) { void port_background_tick(void) { } -void port_start_background_task(void) { +void port_start_background_tick(void) { } -void port_finish_background_task(void) { +void port_finish_background_tick(void) { } diff --git a/ports/nrf/background.c b/ports/nrf/background.c index b8d4df6324..f2df4db0e0 100644 --- a/ports/nrf/background.c +++ b/ports/nrf/background.c @@ -44,10 +44,10 @@ #include "common-hal/audiopwmio/PWMAudioOut.h" #endif -void port_start_background_task(void) { +void port_start_background_tick(void) { } -void port_finish_background_task(void) { +void port_finish_background_tick(void) { } void port_background_tick(void) { diff --git a/ports/raspberrypi/background.c b/ports/raspberrypi/background.c index 8e5e3fcd91..1024ff7b38 100644 --- a/ports/raspberrypi/background.c +++ b/ports/raspberrypi/background.c @@ -28,10 +28,10 @@ #include "py/runtime.h" #include "supervisor/port.h" -void port_start_background_task(void) { +void port_start_background_tick(void) { } -void port_finish_background_task(void) { +void port_finish_background_tick(void) { } void port_background_tick(void) { diff --git a/ports/stm/background.c b/ports/stm/background.c index 68703a5233..7dc617f49f 100644 --- a/ports/stm/background.c +++ b/ports/stm/background.c @@ -37,7 +37,7 @@ void port_background_task(void) { } void port_background_tick(void) { } -void port_start_background_task(void) { +void port_start_background_tick(void) { } -void port_finish_background_task(void) { +void port_finish_background_tick(void) { } diff --git a/py/circuitpy_mpconfig.h b/py/circuitpy_mpconfig.h index 5f140413bb..cc7886cb37 100644 --- a/py/circuitpy_mpconfig.h +++ b/py/circuitpy_mpconfig.h @@ -439,8 +439,8 @@ struct _supervisor_allocation_node; const char *readline_hist[8]; \ struct _supervisor_allocation_node *first_embedded_allocation; \ -void supervisor_run_background_tasks_if_tick(void); -#define RUN_BACKGROUND_TASKS (supervisor_run_background_tasks_if_tick()) +void background_callback_run_all(void); +#define RUN_BACKGROUND_TASKS (background_callback_run_all()) #define MICROPY_VM_HOOK_LOOP RUN_BACKGROUND_TASKS; #define MICROPY_VM_HOOK_RETURN RUN_BACKGROUND_TASKS; diff --git a/supervisor/background_callback.h b/supervisor/background_callback.h index 651ac020a6..36b0017f0c 100644 --- a/supervisor/background_callback.h +++ b/supervisor/background_callback.h @@ -37,8 +37,9 @@ * To schedule the work, use background_callback_add, with fun as the * function to call and data pointing to the object itself. * - * Next time run_background_tasks_if_tick is called, the callback will - * be run and removed from the linked list. + * Next time background_callback_run_all() is called, the callback will + * be run and removed from the linked list. Use `RUN_BACKGROUND_TASKS;` instead + * of calling background_callback_run_all() directly. * * Queueing a task that is already queued does nothing. Unconditionally * re-queueing it from its own background task will cause it to run during the @@ -46,6 +47,12 @@ * don't do that. * * background_callback_add can be called from interrupt context. + * + * If your work isn't triggered by an event, then it may be better implemented + * using ticks, which runs tasks every millisecond or so. Ticks are enabled with + * supervisor_enable_tick() and disabled with supervisor_disable_tick(). When + * enabled, a timer will schedule a callback to supervisor_background_tick(), + * which includes port_background_tick(), every millisecond. */ typedef void (*background_callback_fun)(void *data); typedef struct background_callback { diff --git a/supervisor/port.h b/supervisor/port.h index b50179e4de..7b48a3f13b 100644 --- a/supervisor/port.h +++ b/supervisor/port.h @@ -94,14 +94,15 @@ void port_idle_until_interrupt(void); void port_background_tick(void); // Execute port specific actions during background tasks. This is before the -// background callback system. +// background callback system and happens *very* often. Use +// port_background_tick() when possible. void port_background_task(void); -// Take port specific actions at the beginning and end of background tasks. +// Take port specific actions at the beginning and end of background ticks. // This is used e.g., to set a monitoring pin for debug purposes. "Actual -// work" should be done in port_background_task() instead. -void port_start_background_task(void); -void port_finish_background_task(void); +// work" should be done in port_background_tick() instead. +void port_start_background_tick(void); +void port_finish_background_tick(void); // Some ports need special handling to wake the main task from another task. The // port must implement the necessary code in this function. A default weak diff --git a/supervisor/shared/tick.c b/supervisor/shared/tick.c index 429ca35d60..b91257c8c8 100644 --- a/supervisor/shared/tick.c +++ b/supervisor/shared/tick.c @@ -65,8 +65,8 @@ static volatile uint64_t last_finished_tick = 0; static volatile size_t tick_enable_count = 0; -static void supervisor_background_tasks(void *unused) { - port_start_background_task(); +static void supervisor_background_tick(void *unused) { + port_start_background_tick(); assert_heap_ok(); @@ -80,16 +80,16 @@ static void supervisor_background_tasks(void *unused) { filesystem_background(); - port_background_task(); + port_background_tick(); assert_heap_ok(); last_finished_tick = port_get_raw_ticks(NULL); - port_finish_background_task(); + port_finish_background_tick(); } -bool supervisor_background_tasks_ok(void) { +bool supervisor_background_ticks_ok(void) { return port_get_raw_ticks(NULL) - last_finished_tick < 1024; } @@ -103,7 +103,7 @@ void supervisor_tick(void) { keypad_tick(); #endif - background_callback_add(&tick_callback, supervisor_background_tasks, NULL); + background_callback_add(&tick_callback, supervisor_background_tick, NULL); } uint64_t supervisor_ticks_ms64() { @@ -117,11 +117,6 @@ uint32_t supervisor_ticks_ms32() { return supervisor_ticks_ms64(); } - -void PLACE_IN_ITCM(supervisor_run_background_tasks_if_tick)() { - background_callback_run_all(); -} - void mp_hal_delay_ms(mp_uint_t delay_ms) { uint64_t start_tick = port_get_raw_ticks(NULL); // Adjust the delay to ticks vs ms. diff --git a/supervisor/shared/tick.h b/supervisor/shared/tick.h index d805aeb099..5f537ad2ed 100644 --- a/supervisor/shared/tick.h +++ b/supervisor/shared/tick.h @@ -54,13 +54,6 @@ extern uint32_t supervisor_ticks_ms32(void); */ extern uint64_t supervisor_ticks_ms64(void); -/** @brief Run background ticks, but only about every millisecond. - * - * Normally, this is not called directly. Instead use the RUN_BACKGROUND_TASKS - * macro. - */ -extern void supervisor_run_background_if_tick(void); - extern void supervisor_enable_tick(void); extern void supervisor_disable_tick(void); @@ -70,6 +63,6 @@ extern void supervisor_disable_tick(void); * Note that when ticks are not enabled, this function can return false; this is * intended. */ -extern bool supervisor_background_tasks_ok(void); +extern bool supervisor_background_ticks_ok(void); #endif