diff --git a/extmod/modbluetooth.h b/extmod/modbluetooth.h index 6ed086d553..52e3446ff3 100644 --- a/extmod/modbluetooth.h +++ b/extmod/modbluetooth.h @@ -50,6 +50,7 @@ #endif // This is used to protect the ringbuffer. +// A port may no-op this if MICROPY_PY_BLUETOOTH_USE_SYNC_EVENTS is enabled. #ifndef MICROPY_PY_BLUETOOTH_ENTER #define MICROPY_PY_BLUETOOTH_ENTER mp_uint_t atomic_state = MICROPY_BEGIN_ATOMIC_SECTION(); #define MICROPY_PY_BLUETOOTH_EXIT MICROPY_END_ATOMIC_SECTION(atomic_state); diff --git a/extmod/nimble/hal/hal_uart.c b/extmod/nimble/hal/hal_uart.c index c6d0850fea..230970b089 100644 --- a/extmod/nimble/hal/hal_uart.c +++ b/extmod/nimble/hal/hal_uart.c @@ -28,10 +28,13 @@ #include "py/mphal.h" #include "nimble/ble.h" #include "extmod/nimble/hal/hal_uart.h" +#include "extmod/nimble/nimble/nimble_npl_os.h" #include "extmod/mpbthci.h" #if MICROPY_PY_BLUETOOTH && MICROPY_BLUETOOTH_NIMBLE +#define HCI_TRACE (0) + static hal_uart_tx_cb_t hal_uart_tx_cb; static void *hal_uart_tx_arg; static hal_uart_rx_cb_t hal_uart_rx_cb; @@ -62,10 +65,10 @@ void hal_uart_start_tx(uint32_t port) { mp_bluetooth_hci_cmd_buf[len++] = data; } - #if 0 - printf("[% 8d] BTUTX: %02x", mp_hal_ticks_ms(), hci_cmd_buf[0]); - for (int i = 1; i < len; ++i) { - printf(":%02x", hci_cmd_buf[i]); + #if HCI_TRACE + printf("< [% 8d] %02x", mp_hal_ticks_ms(), mp_bluetooth_hci_cmd_buf[0]); + for (size_t i = 1; i < len; ++i) { + printf(":%02x", mp_bluetooth_hci_cmd_buf[i]); } printf("\n"); #endif @@ -77,13 +80,21 @@ int hal_uart_close(uint32_t port) { return 0; // success } -void mp_bluetooth_nimble_hci_uart_process(void) { +void mp_bluetooth_nimble_hci_uart_process(bool run_events) { bool host_wake = mp_bluetooth_hci_controller_woken(); int chr; while ((chr = mp_bluetooth_hci_uart_readchar()) >= 0) { - // printf("UART RX: %02x\n", data); + #if HCI_TRACE + printf("> %02x (%d)\n", chr); + #endif hal_uart_rx_cb(hal_uart_rx_arg, chr); + + // Incoming data may result in events being enqueued. If we're in + // scheduler context then we can run those events immediately. + if (run_events) { + mp_bluetooth_nimble_os_eventq_run_all(); + } } if (host_wake) { diff --git a/extmod/nimble/hal/hal_uart.h b/extmod/nimble/hal/hal_uart.h index 1ff1c17436..647e8ab477 100644 --- a/extmod/nimble/hal/hal_uart.h +++ b/extmod/nimble/hal/hal_uart.h @@ -43,6 +43,6 @@ void hal_uart_start_tx(uint32_t port); int hal_uart_close(uint32_t port); // --- Called by the MicroPython port when UART data is available ------------- -void mp_bluetooth_nimble_hci_uart_process(void); +void mp_bluetooth_nimble_hci_uart_process(bool run_events); #endif // MICROPY_INCLUDED_EXTMOD_NIMBLE_HAL_HAL_UART_H diff --git a/extmod/nimble/modbluetooth_nimble.c b/extmod/nimble/modbluetooth_nimble.c index 7ee6ae8677..c961aee326 100644 --- a/extmod/nimble/modbluetooth_nimble.c +++ b/extmod/nimble/modbluetooth_nimble.c @@ -382,6 +382,7 @@ int mp_bluetooth_init(void) { mp_bluetooth_nimble_ble_state = MP_BLUETOOTH_NIMBLE_BLE_STATE_WAITING_FOR_SYNC; // Initialise NimBLE memory and data structures. + DEBUG_printf("mp_bluetooth_init: nimble_port_init\n"); nimble_port_init(); // Make sure that the HCI UART and event handling task is running. @@ -402,6 +403,8 @@ int mp_bluetooth_init(void) { return MP_ETIMEDOUT; } + DEBUG_printf("mp_bluetooth_init: starting services\n"); + // By default, just register the default gap/gatt service. ble_svc_gap_init(); ble_svc_gatt_init(); @@ -417,7 +420,7 @@ int mp_bluetooth_init(void) { } void mp_bluetooth_deinit(void) { - DEBUG_printf("mp_bluetooth_deinit\n"); + DEBUG_printf("mp_bluetooth_deinit %d\n", mp_bluetooth_nimble_ble_state); if (mp_bluetooth_nimble_ble_state == MP_BLUETOOTH_NIMBLE_BLE_STATE_OFF) { return; } diff --git a/extmod/nimble/nimble.mk b/extmod/nimble/nimble.mk index fbd031b3e3..00a244d6ea 100644 --- a/extmod/nimble/nimble.mk +++ b/extmod/nimble/nimble.mk @@ -17,6 +17,13 @@ CFLAGS_MOD += -DMICROPY_BLUETOOTH_NIMBLE_BINDINGS_ONLY=$(MICROPY_BLUETOOTH_NIMBL ifeq ($(MICROPY_BLUETOOTH_NIMBLE_BINDINGS_ONLY),0) +# On all ports where we provide the full implementation (i.e. not just +# bindings like on ESP32), then we don't need to use the ringbuffer. In this +# case, all NimBLE events are run by the MicroPython scheduler. On Unix, the +# scheduler is also responsible for polling the UART, whereas on STM32 the +# UART is also polled by the RX IRQ. +CFLAGS_MOD += -DMICROPY_PY_BLUETOOTH_USE_SYNC_EVENTS=1 + NIMBLE_LIB_DIR = lib/mynewt-nimble LIB_SRC_C += $(addprefix $(NIMBLE_LIB_DIR)/, \ diff --git a/extmod/nimble/nimble/nimble_npl_os.c b/extmod/nimble/nimble/nimble_npl_os.c index 2ec012940f..b68957fabf 100644 --- a/extmod/nimble/nimble/nimble_npl_os.c +++ b/extmod/nimble/nimble/nimble_npl_os.c @@ -179,63 +179,100 @@ int nimble_sprintf(char *str, const char *fmt, ...) { struct ble_npl_eventq *global_eventq = NULL; +// This must not be called recursively or concurrently with the UART handler. void mp_bluetooth_nimble_os_eventq_run_all(void) { - for (struct ble_npl_eventq *evq = global_eventq; evq != NULL; evq = evq->nextq) { - int n = 0; - while (evq->head != NULL && mp_bluetooth_nimble_ble_state > MP_BLUETOOTH_NIMBLE_BLE_STATE_OFF) { - struct ble_npl_event *ev = evq->head; - evq->head = ev->next; - if (ev->next) { - ev->next->prev = NULL; - ev->next = NULL; - } - ev->prev = NULL; - DEBUG_EVENT_printf("event_run(%p)\n", ev); - ev->fn(ev); - DEBUG_EVENT_printf("event_run(%p) done\n", ev); + if (mp_bluetooth_nimble_ble_state == MP_BLUETOOTH_NIMBLE_BLE_STATE_OFF) { + return; + } - if (++n > 3) { - // Limit to running 3 tasks per queue. - // Some tasks (such as reset) can enqueue themselves - // making this an infinite loop (while in PENDSV). + // Keep running while there are pending events. + while (true) { + struct ble_npl_event *ev = NULL; + + os_sr_t sr; + OS_ENTER_CRITICAL(sr); + // Search all queues for an event. + for (struct ble_npl_eventq *evq = global_eventq; evq != NULL; evq = evq->nextq) { + ev = evq->head; + if (ev) { + // Remove this event from the queue. + evq->head = ev->next; + if (ev->next) { + ev->next->prev = NULL; + ev->next = NULL; + } + ev->prev = NULL; + + ev->pending = false; + + // Stop searching and execute this event. break; } } + OS_EXIT_CRITICAL(sr); + + if (!ev) { + break; + } + + // Run the event handler. + DEBUG_EVENT_printf("event_run(%p)\n", ev); + ev->fn(ev); + DEBUG_EVENT_printf("event_run(%p) done\n", ev); + + if (ev->pending) { + // If this event has been re-enqueued while it was running, then + // stop running further events. This prevents an infinite loop + // where the reset event re-enqueues itself on HCI timeout. + break; + } } } void ble_npl_eventq_init(struct ble_npl_eventq *evq) { DEBUG_EVENT_printf("ble_npl_eventq_init(%p)\n", evq); + os_sr_t sr; + OS_ENTER_CRITICAL(sr); evq->head = NULL; struct ble_npl_eventq **evq2; for (evq2 = &global_eventq; *evq2 != NULL; evq2 = &(*evq2)->nextq) { } *evq2 = evq; evq->nextq = NULL; + OS_EXIT_CRITICAL(sr); } void ble_npl_eventq_put(struct ble_npl_eventq *evq, struct ble_npl_event *ev) { DEBUG_EVENT_printf("ble_npl_eventq_put(%p, %p (%p, %p))\n", evq, ev, ev->fn, ev->arg); + os_sr_t sr; + OS_ENTER_CRITICAL(sr); ev->next = NULL; + ev->pending = true; if (evq->head == NULL) { + // Empty list, make this the first item. evq->head = ev; ev->prev = NULL; } else { - struct ble_npl_event *ev2 = evq->head; + // Find the tail of this list. + struct ble_npl_event *tail = evq->head; while (true) { - if (ev2 == ev) { + if (tail == ev) { DEBUG_EVENT_printf(" --> already in queue\n"); - return; - } - if (ev2->next == NULL) { + // Already in the list (e.g. a fragmented ACL will enqueue an + // event to process it for each fragment). break; } - DEBUG_EVENT_printf(" --> %p\n", ev2->next); - ev2 = ev2->next; + if (tail->next == NULL) { + // Found the end of the list, add this event as the tail. + tail->next = ev; + ev->prev = tail; + break; + } + DEBUG_EVENT_printf(" --> %p\n", tail->next); + tail = tail->next; } - ev2->next = ev; - ev->prev = ev2; } + OS_EXIT_CRITICAL(sr); } void ble_npl_event_init(struct ble_npl_event *ev, ble_npl_event_fn *fn, void *arg) { @@ -243,6 +280,7 @@ void ble_npl_event_init(struct ble_npl_event *ev, ble_npl_event_fn *fn, void *ar ev->fn = fn; ev->arg = arg; ev->next = NULL; + ev->pending = false; } void *ble_npl_event_get_arg(struct ble_npl_event *ev) { @@ -258,44 +296,17 @@ void ble_npl_event_set_arg(struct ble_npl_event *ev, void *arg) { /******************************************************************************/ // MUTEX -// This is what MICROPY_BEGIN_ATOMIC_SECTION returns on Unix (i.e. we don't -// need to preserve the atomic state to unlock). -#define ATOMIC_STATE_MUTEX_NOT_HELD 0xffffffff - ble_npl_error_t ble_npl_mutex_init(struct ble_npl_mutex *mu) { DEBUG_MUTEX_printf("ble_npl_mutex_init(%p)\n", mu); mu->locked = 0; - mu->atomic_state = ATOMIC_STATE_MUTEX_NOT_HELD; return BLE_NPL_OK; } ble_npl_error_t ble_npl_mutex_pend(struct ble_npl_mutex *mu, ble_npl_time_t timeout) { - DEBUG_MUTEX_printf("ble_npl_mutex_pend(%p, %u) locked=%u irq=%d\n", mu, (uint)timeout, (uint)mu->locked); + DEBUG_MUTEX_printf("ble_npl_mutex_pend(%p, %u) locked=%u\n", mu, (uint)timeout, (uint)mu->locked); - // This is a recursive mutex which we implement on top of the IRQ priority - // scheme. Unfortunately we have a single piece of global storage, where - // enter/exit critical needs an "atomic state". - - // There are two different acquirers, either running in a VM thread (i.e. - // a direct Python call into NimBLE), or in the NimBLE task (i.e. polling - // or UART RX). - - // On STM32 the NimBLE task runs in PENDSV, so cannot be interrupted by a VM thread. - // Therefore we only need to ensure that a VM thread that acquires a currently-unlocked mutex - // now raises the priority (thus preventing context switches to other VM threads and - // the PENDSV irq). If the mutex is already locked, then it must have been acquired - // by us. - - // On Unix, the critical section is completely recursive and doesn't require us to manage - // state so we just acquire and release every time. - - // TODO: The "volatile" on locked/atomic_state isn't enough to protect against memory re-ordering. - - // First acquirer of this mutex always enters the critical section, unless - // we're on Unix where it happens every time. - if (mu->atomic_state == ATOMIC_STATE_MUTEX_NOT_HELD) { - mu->atomic_state = mp_bluetooth_nimble_hci_uart_enter_critical(); - } + // All NimBLE code is executed by the scheduler (and is therefore + // implicitly mutexed) so this mutex implementation is a no-op. ++mu->locked; @@ -303,17 +314,11 @@ ble_npl_error_t ble_npl_mutex_pend(struct ble_npl_mutex *mu, ble_npl_time_t time } ble_npl_error_t ble_npl_mutex_release(struct ble_npl_mutex *mu) { - DEBUG_MUTEX_printf("ble_npl_mutex_release(%p) locked=%u irq=%d\n", mu, (uint)mu->locked); + DEBUG_MUTEX_printf("ble_npl_mutex_release(%p) locked=%u\n", mu, (uint)mu->locked); assert(mu->locked > 0); --mu->locked; - // Only exit the critical section for the final release, unless we're on Unix. - if (mu->locked == 0 || mu->atomic_state == ATOMIC_STATE_MUTEX_NOT_HELD) { - mp_bluetooth_nimble_hci_uart_exit_critical(mu->atomic_state); - mu->atomic_state = ATOMIC_STATE_MUTEX_NOT_HELD; - } - return BLE_NPL_OK; } @@ -329,30 +334,19 @@ ble_npl_error_t ble_npl_sem_init(struct ble_npl_sem *sem, uint16_t tokens) { ble_npl_error_t ble_npl_sem_pend(struct ble_npl_sem *sem, ble_npl_time_t timeout) { DEBUG_SEM_printf("ble_npl_sem_pend(%p, %u) count=%u\n", sem, (uint)timeout, (uint)sem->count); - // This is called by NimBLE to synchronously wait for an HCI ACK. The - // corresponding ble_npl_sem_release is called directly by the UART rx - // handler (i.e. hal_uart_rx_cb in extmod/nimble/hal/hal_uart.c). - - // So this implementation just polls the UART until either the semaphore - // is released, or the timeout occurs. + // This is only called by NimBLE in ble_hs_hci_cmd_tx to synchronously + // wait for an HCI ACK. The corresponding ble_npl_sem_release is called + // directly by the UART rx handler (i.e. hal_uart_rx_cb in + // extmod/nimble/hal/hal_uart.c). So this loop needs to run only the HCI + // UART processing but not run any events. if (sem->count == 0) { uint32_t t0 = mp_hal_ticks_ms(); while (sem->count == 0 && mp_hal_ticks_ms() - t0 < timeout) { - // This can be called either from code running in NimBLE's "task" - // (i.e. PENDSV) or directly by application code, so for the - // latter case, prevent the "task" from running while we poll the - // UART directly. - MICROPY_PY_BLUETOOTH_ENTER - mp_bluetooth_nimble_hci_uart_process(); - MICROPY_PY_BLUETOOTH_EXIT - if (sem->count != 0) { break; } - // Because we're polling, might as well wait for a UART IRQ indicating - // more data available. mp_bluetooth_nimble_hci_uart_wfi(); } @@ -384,6 +378,8 @@ uint16_t ble_npl_sem_get_count(struct ble_npl_sem *sem) { static struct ble_npl_callout *global_callout = NULL; void mp_bluetooth_nimble_os_callout_process(void) { + os_sr_t sr; + OS_ENTER_CRITICAL(sr); uint32_t tnow = mp_hal_ticks_ms(); for (struct ble_npl_callout *c = global_callout; c != NULL; c = c->nextc) { if (!c->active) { @@ -393,17 +389,24 @@ void mp_bluetooth_nimble_os_callout_process(void) { DEBUG_CALLOUT_printf("callout_run(%p) tnow=%u ticks=%u evq=%p\n", c, (uint)tnow, (uint)c->ticks, c->evq); c->active = false; if (c->evq) { + // Enqueue this callout for execution in the event queue. ble_npl_eventq_put(c->evq, &c->ev); } else { + // Execute this callout directly. + OS_EXIT_CRITICAL(sr); c->ev.fn(&c->ev); + OS_ENTER_CRITICAL(sr); } DEBUG_CALLOUT_printf("callout_run(%p) done\n", c); } } + OS_EXIT_CRITICAL(sr); } void ble_npl_callout_init(struct ble_npl_callout *c, struct ble_npl_eventq *evq, ble_npl_event_fn *ev_cb, void *ev_arg) { DEBUG_CALLOUT_printf("ble_npl_callout_init(%p, %p, %p, %p)\n", c, evq, ev_cb, ev_arg); + os_sr_t sr; + OS_ENTER_CRITICAL(sr); c->active = false; c->ticks = 0; c->evq = evq; @@ -413,17 +416,22 @@ void ble_npl_callout_init(struct ble_npl_callout *c, struct ble_npl_eventq *evq, for (c2 = &global_callout; *c2 != NULL; c2 = &(*c2)->nextc) { if (c == *c2) { // callout already in linked list so don't link it in again + OS_EXIT_CRITICAL(sr); return; } } *c2 = c; c->nextc = NULL; + OS_EXIT_CRITICAL(sr); } ble_npl_error_t ble_npl_callout_reset(struct ble_npl_callout *c, ble_npl_time_t ticks) { DEBUG_CALLOUT_printf("ble_npl_callout_reset(%p, %u) tnow=%u\n", c, (uint)ticks, (uint)mp_hal_ticks_ms()); + os_sr_t sr; + OS_ENTER_CRITICAL(sr); c->active = true; c->ticks = ble_npl_time_get() + ticks; + OS_EXIT_CRITICAL(sr); return BLE_NPL_OK; } @@ -493,23 +501,20 @@ void ble_npl_time_delay(ble_npl_time_t ticks) { // CRITICAL // This is used anywhere NimBLE modifies global data structures. -// We need to protect between: -// - A MicroPython VM thread. -// - The NimBLE "task" (e.g. PENDSV on STM32, pthread on Unix). -// On STM32, by disabling PENDSV, we ensure that either: -// - If we're in the NimBLE task, we're exclusive anyway. -// - If we're in a VM thread, we can't be interrupted by the NimBLE task, or switched to another thread. -// On Unix, there's a global mutex. -// TODO: Both ports currently use MICROPY_PY_BLUETOOTH_ENTER in their implementation, -// maybe this doesn't need to be port-specific? +// Currently all NimBLE code is invoked by the scheduler so there should be no +// races, so on STM32 MICROPY_PY_BLUETOOTH_ENTER/MICROPY_PY_BLUETOOTH_EXIT are +// no-ops. However, in the future we may wish to make HCI UART processing +// happen asynchronously (e.g. on RX IRQ), so the port can implement these +// macros accordingly. uint32_t ble_npl_hw_enter_critical(void) { DEBUG_CRIT_printf("ble_npl_hw_enter_critical()\n"); - return mp_bluetooth_nimble_hci_uart_enter_critical(); + MICROPY_PY_BLUETOOTH_ENTER + return atomic_state; } -void ble_npl_hw_exit_critical(uint32_t ctx) { - DEBUG_CRIT_printf("ble_npl_hw_exit_critical(%u)\n", (uint)ctx); - mp_bluetooth_nimble_hci_uart_exit_critical(ctx); +void ble_npl_hw_exit_critical(uint32_t atomic_state) { + MICROPY_PY_BLUETOOTH_EXIT + DEBUG_CRIT_printf("ble_npl_hw_exit_critical(%u)\n", (uint)atomic_state); } diff --git a/extmod/nimble/nimble/nimble_npl_os.h b/extmod/nimble/nimble/nimble_npl_os.h index 3ef07aa9cc..bfabe56e89 100644 --- a/extmod/nimble/nimble/nimble_npl_os.h +++ b/extmod/nimble/nimble/nimble_npl_os.h @@ -42,6 +42,7 @@ typedef int32_t ble_npl_stime_t; struct ble_npl_event { ble_npl_event_fn *fn; void *arg; + bool pending; struct ble_npl_event *prev; struct ble_npl_event *next; }; @@ -61,7 +62,6 @@ struct ble_npl_callout { struct ble_npl_mutex { volatile uint8_t locked; - volatile uint32_t atomic_state; }; struct ble_npl_sem { @@ -76,7 +76,5 @@ void mp_bluetooth_nimble_os_callout_process(void); // --- Must be provided by the MicroPython port ------------------------------- void mp_bluetooth_nimble_hci_uart_wfi(void); -uint32_t mp_bluetooth_nimble_hci_uart_enter_critical(void); -void mp_bluetooth_nimble_hci_uart_exit_critical(uint32_t atomic_state); #endif // MICROPY_INCLUDED_STM32_NIMBLE_NIMBLE_NPL_OS_H diff --git a/ports/stm32/main.c b/ports/stm32/main.c index db8222479b..d00c2ec713 100644 --- a/ports/stm32/main.c +++ b/ports/stm32/main.c @@ -433,8 +433,8 @@ void stm32_main(uint32_t reset_mode) { systick_enable_dispatch(SYSTICK_DISPATCH_LWIP, mod_network_lwip_poll_wrapper); #endif #if MICROPY_PY_BLUETOOTH - extern void mp_bluetooth_hci_poll_wrapper(uint32_t ticks_ms); - systick_enable_dispatch(SYSTICK_DISPATCH_BLUETOOTH_HCI, mp_bluetooth_hci_poll_wrapper); + extern void mp_bluetooth_hci_systick(uint32_t ticks_ms); + systick_enable_dispatch(SYSTICK_DISPATCH_BLUETOOTH_HCI, mp_bluetooth_hci_systick); #endif #if MICROPY_PY_NETWORK_CYW43 diff --git a/ports/stm32/mpbthciport.c b/ports/stm32/mpbthciport.c index a5977ff12c..ee9f4e31eb 100644 --- a/ports/stm32/mpbthciport.c +++ b/ports/stm32/mpbthciport.c @@ -27,6 +27,7 @@ #include "py/runtime.h" #include "py/mphal.h" #include "extmod/mpbthci.h" +#include "extmod/modbluetooth.h" #include "systick.h" #include "pendsv.h" #include "lib/utils/mpirq.h" @@ -35,23 +36,58 @@ #if MICROPY_PY_BLUETOOTH -#define DEBUG_printf(...) // printf(__VA_ARGS__) +#define DEBUG_printf(...) // printf("mpbthciport.c: " __VA_ARGS__) uint8_t mp_bluetooth_hci_cmd_buf[4 + 256]; // Must be provided by the stack bindings (e.g. mpnimbleport.c or mpbtstackport.c). +// Request new data from the uart and pass to the stack, and run pending events/callouts. extern void mp_bluetooth_hci_poll(void); // Hook for pendsv poller to run this periodically every 128ms #define BLUETOOTH_HCI_TICK(tick) (((tick) & ~(SYSTICK_DISPATCH_NUM_SLOTS - 1) & 0x7f) == 0) +#if MICROPY_PY_BLUETOOTH_USE_SYNC_EVENTS + +// For synchronous mode, we run all BLE stack code inside a scheduled task. +// This task is scheduled periodically (every 128ms) via SysTick, or +// immediately on HCI UART RXIDLE. + +// Prevent double-enqueuing of the scheduled task. +STATIC volatile bool events_task_is_scheduled = false; + +STATIC mp_obj_t run_events_scheduled_task(mp_obj_t none_in) { + (void)none_in; + events_task_is_scheduled = false; + // This will process all buffered HCI UART data, and run any callouts or events. + mp_bluetooth_hci_poll(); + return mp_const_none; +} +STATIC MP_DEFINE_CONST_FUN_OBJ_1(run_events_scheduled_task_obj, run_events_scheduled_task); + +// Called periodically (systick) or directly (e.g. UART RX IRQ) in order to +// request that processing happens ASAP in the scheduler. +void mp_bluetooth_hci_systick(uint32_t ticks_ms) { + if (events_task_is_scheduled) { + return; + } + + if (ticks_ms == 0 || BLUETOOTH_HCI_TICK(ticks_ms)) { + events_task_is_scheduled = mp_sched_schedule(MP_OBJ_FROM_PTR(&run_events_scheduled_task_obj), mp_const_none); + } +} + +#else // !MICROPY_PY_BLUETOOTH_USE_SYNC_EVENTS + // Called periodically (systick) or directly (e.g. uart irq). -void mp_bluetooth_hci_poll_wrapper(uint32_t ticks_ms) { +void mp_bluetooth_hci_systick(uint32_t ticks_ms) { if (ticks_ms == 0 || BLUETOOTH_HCI_TICK(ticks_ms)) { pendsv_schedule_dispatch(PENDSV_DISPATCH_BLUETOOTH_HCI, mp_bluetooth_hci_poll); } } +#endif + #if defined(STM32WB) /******************************************************************************/ @@ -67,13 +103,23 @@ STATIC uint8_t hci_uart_rx_buf_data[256]; int mp_bluetooth_hci_uart_init(uint32_t port, uint32_t baudrate) { (void)port; (void)baudrate; + + DEBUG_printf("mp_bluetooth_hci_uart_init (stm32 rfcore)\n"); + + #if MICROPY_PY_BLUETOOTH_USE_SYNC_EVENTS + events_task_is_scheduled = false; + #endif + rfcore_ble_init(); hci_uart_rx_buf_cur = 0; hci_uart_rx_buf_len = 0; + return 0; } int mp_bluetooth_hci_uart_deinit(void) { + DEBUG_printf("mp_bluetooth_hci_uart_deinit (stm32 rfcore)\n"); + return 0; } @@ -125,12 +171,12 @@ int mp_bluetooth_hci_uart_readchar(void) { pyb_uart_obj_t mp_bluetooth_hci_uart_obj; mp_irq_obj_t mp_bluetooth_hci_uart_irq_obj; -static uint8_t hci_uart_rxbuf[512]; +static uint8_t hci_uart_rxbuf[768]; mp_obj_t mp_uart_interrupt(mp_obj_t self_in) { - // DEBUG_printf("mp_uart_interrupt\n"); - // New HCI data, schedule mp_bluetooth_hci_poll via PENDSV to make the stack handle it. - mp_bluetooth_hci_poll_wrapper(0); + // Queue up the scheduler to run the HCI UART and event processing ASAP. + mp_bluetooth_hci_systick(0); + return mp_const_none; } MP_DEFINE_CONST_FUN_OBJ_1(mp_uart_interrupt_obj, mp_uart_interrupt); @@ -138,6 +184,10 @@ MP_DEFINE_CONST_FUN_OBJ_1(mp_uart_interrupt_obj, mp_uart_interrupt); int mp_bluetooth_hci_uart_init(uint32_t port, uint32_t baudrate) { DEBUG_printf("mp_bluetooth_hci_uart_init (stm32)\n"); + #if MICROPY_PY_BLUETOOTH_USE_SYNC_EVENTS + events_task_is_scheduled = false; + #endif + // bits (8), stop (1), parity (none) and flow (rts/cts) are assumed to match MYNEWT_VAL_BLE_HCI_UART_ constants in syscfg.h. mp_bluetooth_hci_uart_obj.base.type = &pyb_uart_type; mp_bluetooth_hci_uart_obj.uart_id = port; @@ -147,7 +197,7 @@ int mp_bluetooth_hci_uart_init(uint32_t port, uint32_t baudrate) { mp_bluetooth_hci_uart_obj.timeout_char = 200; MP_STATE_PORT(pyb_uart_obj_all)[mp_bluetooth_hci_uart_obj.uart_id - 1] = &mp_bluetooth_hci_uart_obj; - // This also initialises the UART. + // This also initialises the UART and adds the RXIDLE IRQ handler. mp_bluetooth_hci_uart_set_baudrate(baudrate); return 0; @@ -155,6 +205,7 @@ int mp_bluetooth_hci_uart_init(uint32_t port, uint32_t baudrate) { int mp_bluetooth_hci_uart_deinit(void) { DEBUG_printf("mp_bluetooth_hci_uart_deinit (stm32)\n"); + // TODO: deinit mp_bluetooth_hci_uart_obj return 0; diff --git a/ports/stm32/mpconfigport.h b/ports/stm32/mpconfigport.h index 5f8e7ec2de..b6906ef998 100644 --- a/ports/stm32/mpconfigport.h +++ b/ports/stm32/mpconfigport.h @@ -420,9 +420,16 @@ static inline mp_uint_t disable_irq(void) { #define MICROPY_PY_LWIP_REENTER MICROPY_PY_PENDSV_REENTER #define MICROPY_PY_LWIP_EXIT MICROPY_PY_PENDSV_EXIT -// Prevent the "Bluetooth task" from running (either NimBLE or btstack). +#if MICROPY_PY_BLUETOOTH_USE_SYNC_EVENTS +// Bluetooth code only runs in the scheduler, no locking/mutex required. +#define MICROPY_PY_BLUETOOTH_ENTER uint32_t atomic_state = 0; +#define MICROPY_PY_BLUETOOTH_EXIT (void)atomic_state; +#else +// When async events are enabled, need to prevent PendSV execution racing with +// scheduler execution. #define MICROPY_PY_BLUETOOTH_ENTER MICROPY_PY_PENDSV_ENTER #define MICROPY_PY_BLUETOOTH_EXIT MICROPY_PY_PENDSV_EXIT +#endif // We need an implementation of the log2 function which is not a macro #define MP_NEED_LOG2 (1) diff --git a/ports/stm32/mpnimbleport.c b/ports/stm32/mpnimbleport.c index 1d7c095139..0ba76fb277 100644 --- a/ports/stm32/mpnimbleport.c +++ b/ports/stm32/mpnimbleport.c @@ -31,24 +31,29 @@ #if MICROPY_PY_BLUETOOTH && MICROPY_BLUETOOTH_NIMBLE +#define DEBUG_printf(...) // printf("mpnimbleport.c: " __VA_ARGS__) + #include "host/ble_hs.h" #include "nimble/nimble_npl.h" #include "extmod/mpbthci.h" +#include "extmod/modbluetooth.h" #include "extmod/nimble/modbluetooth_nimble.h" #include "extmod/nimble/hal/hal_uart.h" -// This implements the Nimble "background task". It's called at PENDSV -// priority, either every 128ms or whenever there's UART data available. -// Because it's called via PENDSV, you can implicitly consider that it -// is surrounded by MICROPY_PY_BLUETOOTH_ENTER / MICROPY_PY_BLUETOOTH_EXIT. +// Get any pending data from the UART and send it to NimBLE's HCI buffers. +// Any further processing by NimBLE will be run via its event queue. void mp_bluetooth_hci_poll(void) { if (mp_bluetooth_nimble_ble_state >= MP_BLUETOOTH_NIMBLE_BLE_STATE_WAITING_FOR_SYNC) { - // Ask NimBLE to process UART data. - mp_bluetooth_nimble_hci_uart_process(); + // DEBUG_printf("mp_bluetooth_hci_poll_uart %d\n", mp_bluetooth_nimble_ble_state); - // Run pending background operations and events, but only after HCI sync. + // Run any timers. mp_bluetooth_nimble_os_callout_process(); + + // Process incoming UART data, and run events as they are generated. + mp_bluetooth_nimble_hci_uart_process(true); + + // Run any remaining events (e.g. if there was no UART data). mp_bluetooth_nimble_os_eventq_run_all(); } } @@ -57,15 +62,10 @@ void mp_bluetooth_hci_poll(void) { void mp_bluetooth_nimble_hci_uart_wfi(void) { __WFI(); -} -uint32_t mp_bluetooth_nimble_hci_uart_enter_critical(void) { - MICROPY_PY_BLUETOOTH_ENTER - return atomic_state; -} - -void mp_bluetooth_nimble_hci_uart_exit_critical(uint32_t atomic_state) { - MICROPY_PY_BLUETOOTH_EXIT + // This is called while NimBLE is waiting in ble_npl_sem_pend, i.e. waiting for an HCI ACK. + // Do not need to run events here (it must not invoke Python code), only processing incoming HCI data. + mp_bluetooth_nimble_hci_uart_process(false); } #endif // MICROPY_PY_BLUETOOTH && MICROPY_BLUETOOTH_NIMBLE diff --git a/ports/stm32/pendsv.h b/ports/stm32/pendsv.h index 585f81e8bd..aa8f90e3e4 100644 --- a/ports/stm32/pendsv.h +++ b/ports/stm32/pendsv.h @@ -34,7 +34,7 @@ enum { PENDSV_DISPATCH_CYW43, #endif #endif - #if MICROPY_PY_BLUETOOTH + #if MICROPY_PY_BLUETOOTH && !MICROPY_PY_BLUETOOTH_USE_SYNC_EVENTS PENDSV_DISPATCH_BLUETOOTH_HCI, #endif PENDSV_DISPATCH_MAX diff --git a/ports/stm32/rfcore.c b/ports/stm32/rfcore.c index 1fc0c9531d..08338fcdc2 100644 --- a/ports/stm32/rfcore.c +++ b/ports/stm32/rfcore.c @@ -634,9 +634,9 @@ void IPCC_C1_RX_IRQHandler(void) { LL_C1_IPCC_ClearFlag_CHx(IPCC, IPCC_CH_BLE); - // Schedule PENDSV to process incoming HCI payload. - extern void mp_bluetooth_hci_poll_wrapper(uint32_t ticks_ms); - mp_bluetooth_hci_poll_wrapper(0); + // Queue up the scheduler to process UART data and run events. + extern void mp_bluetooth_hci_systick(uint32_t ticks_ms); + mp_bluetooth_hci_systick(0); } IRQ_EXIT(IPCC_C1_RX_IRQn); diff --git a/ports/unix/mpbthciport.c b/ports/unix/mpbthciport.c index 316a8831fe..14afbebcd7 100644 --- a/ports/unix/mpbthciport.c +++ b/ports/unix/mpbthciport.c @@ -50,22 +50,67 @@ uint8_t mp_bluetooth_hci_cmd_buf[4 + 256]; +STATIC int uart_fd = -1; + // Must be provided by the stack bindings (e.g. mpnimbleport.c or mpbtstackport.c). extern bool mp_bluetooth_hci_poll(void); -STATIC const useconds_t UART_POLL_INTERVAL_US = 1000; +#if MICROPY_PY_BLUETOOTH_USE_SYNC_EVENTS -STATIC int uart_fd = -1; +// For synchronous mode, we run all BLE stack code inside a scheduled task. +// This task is scheduled periodically (every 1ms) by a background thread. + +// Allows the stack to tell us that we should stop trying to schedule. +extern bool mp_bluetooth_hci_active(void); + +// Prevent double-enqueuing of the scheduled task. +STATIC volatile bool events_task_is_scheduled = false; + +STATIC mp_obj_t run_events_scheduled_task(mp_obj_t none_in) { + (void)none_in; + MICROPY_PY_BLUETOOTH_ENTER + events_task_is_scheduled = false; + MICROPY_PY_BLUETOOTH_EXIT + mp_bluetooth_hci_poll(); + return mp_const_none; +} +STATIC MP_DEFINE_CONST_FUN_OBJ_1(run_events_scheduled_task_obj, run_events_scheduled_task); + +#endif // MICROPY_PY_BLUETOOTH_USE_SYNC_EVENTS + +STATIC const useconds_t UART_POLL_INTERVAL_US = 1000; STATIC pthread_t hci_poll_thread_id; STATIC void *hci_poll_thread(void *arg) { (void)arg; + DEBUG_printf("hci_poll_thread: starting\n"); + + #if MICROPY_PY_BLUETOOTH_USE_SYNC_EVENTS + + events_task_is_scheduled = false; + + while (mp_bluetooth_hci_active()) { + MICROPY_PY_BLUETOOTH_ENTER + if (!events_task_is_scheduled) { + events_task_is_scheduled = mp_sched_schedule(MP_OBJ_FROM_PTR(&run_events_scheduled_task_obj), mp_const_none); + } + MICROPY_PY_BLUETOOTH_EXIT + usleep(UART_POLL_INTERVAL_US); + } + + #else + + // In asynchronous (i.e. ringbuffer) mode, we run the BLE stack directly from the thread. // This will return false when the stack is shutdown. while (mp_bluetooth_hci_poll()) { usleep(UART_POLL_INTERVAL_US); } + #endif + + DEBUG_printf("hci_poll_thread: stopped\n"); + return NULL; } @@ -122,6 +167,11 @@ int mp_bluetooth_hci_uart_init(uint32_t port, uint32_t baudrate) { DEBUG_printf("mp_bluetooth_hci_uart_init (unix)\n"); + if (uart_fd != -1) { + DEBUG_printf("mp_bluetooth_hci_uart_init: already active\n"); + return 0; + } + char uart_device_name[256] = "/dev/ttyUSB0"; char *path = getenv("MICROPYBTUART"); diff --git a/ports/unix/mpnimbleport.c b/ports/unix/mpnimbleport.c index 8961910098..29f558f74d 100644 --- a/ports/unix/mpnimbleport.c +++ b/ports/unix/mpnimbleport.c @@ -47,38 +47,28 @@ bool mp_bluetooth_hci_poll(void) { } if (mp_bluetooth_nimble_ble_state >= MP_BLUETOOTH_NIMBLE_BLE_STATE_WAITING_FOR_SYNC) { - - // Pretend like we're running in IRQ context (i.e. other things can't be running at the same time). - mp_uint_t atomic_state = MICROPY_BEGIN_ATOMIC_SECTION(); - - // Ask NimBLE to process UART data. - mp_bluetooth_nimble_hci_uart_process(); - - // Run pending background operations and events, but only after HCI sync. + // Run any timers. mp_bluetooth_nimble_os_callout_process(); - mp_bluetooth_nimble_os_eventq_run_all(); - MICROPY_END_ATOMIC_SECTION(atomic_state); + // Process incoming UART data, and run events as they are generated. + mp_bluetooth_nimble_hci_uart_process(true); + + // Run any remaining events (e.g. if there was no UART data). + mp_bluetooth_nimble_os_eventq_run_all(); } return true; } +bool mp_bluetooth_hci_active(void) { + return mp_bluetooth_nimble_ble_state != MP_BLUETOOTH_NIMBLE_BLE_STATE_OFF; +} + // Extra port-specific helpers. void mp_bluetooth_nimble_hci_uart_wfi(void) { - // DEBUG_printf("mp_bluetooth_nimble_hci_uart_wfi\n"); - // TODO: this should do a select() on the uart_fd. -} - -uint32_t mp_bluetooth_nimble_hci_uart_enter_critical(void) { - // DEBUG_printf("mp_bluetooth_nimble_hci_uart_enter_critical\n"); - MICROPY_PY_BLUETOOTH_ENTER - return atomic_state; // Always 0xffffffff -} - -void mp_bluetooth_nimble_hci_uart_exit_critical(uint32_t atomic_state) { - MICROPY_PY_BLUETOOTH_EXIT - // DEBUG_printf("mp_bluetooth_nimble_hci_uart_exit_critical\n"); + // This is called while NimBLE is waiting in ble_npl_sem_pend, i.e. waiting for an HCI ACK. + // Do not need to run events here, only processing incoming HCI data. + mp_bluetooth_nimble_hci_uart_process(false); } #endif // MICROPY_PY_BLUETOOTH && MICROPY_BLUETOOTH_NIMBLE