Use critical section, not lock, in CharacteristicBuffer; use a root pointer for ble_drv list

This commit is contained in:
Dan Halbert 2019-02-21 00:19:31 -05:00
parent 4e75aaecd0
commit 99da3b9646
6 changed files with 44 additions and 39 deletions

View File

@ -79,11 +79,10 @@
#include "peripherals/samd/dma.h" #include "peripherals/samd/dma.h"
#define MICROPY_PORT_ROOT_POINTERS \
CIRCUITPY_COMMON_ROOT_POINTERS \
mp_obj_t playing_audio[AUDIO_DMA_CHANNEL_COUNT] \
;
#include "py/circuitpy_mpconfig.h" #include "py/circuitpy_mpconfig.h"
#define MICROPY_PORT_ROOT_POINTERS \
CIRCUITPY_COMMON_ROOT_POINTERS \
mp_obj_t playing_audio[AUDIO_DMA_CHANNEL_COUNT];
#endif // __INCLUDED_MPCONFIGPORT_H #endif // __INCLUDED_MPCONFIGPORT_H

View File

@ -35,27 +35,20 @@
#include "nrf_soc.h" #include "nrf_soc.h"
#include "nrfx_power.h" #include "nrfx_power.h"
#include "py/misc.h" #include "py/misc.h"
#include "py/mpstate.h"
nrf_nvic_state_t nrf_nvic_state = { 0 }; nrf_nvic_state_t nrf_nvic_state = { 0 };
__attribute__((aligned(4))) __attribute__((aligned(4)))
static uint8_t m_ble_evt_buf[sizeof(ble_evt_t) + (BLE_GATT_ATT_MTU_DEFAULT)]; static uint8_t m_ble_evt_buf[sizeof(ble_evt_t) + (BLE_GATT_ATT_MTU_DEFAULT)];
typedef struct event_handler {
struct event_handler *next;
void *param;
ble_drv_evt_handler_t func;
} event_handler_t;
static event_handler_t *m_event_handlers = NULL;
void ble_drv_reset() { void ble_drv_reset() {
// Linked list items will be gc'd. // Linked list items will be gc'd.
m_event_handlers = NULL; MP_STATE_VM(ble_drv_evt_handler_entries) = NULL;
} }
void ble_drv_add_event_handler(ble_drv_evt_handler_t func, void *param) { void ble_drv_add_event_handler(ble_drv_evt_handler_t func, void *param) {
event_handler_t *it = m_event_handlers; ble_drv_evt_handler_entry_t *it = MP_STATE_VM(ble_drv_evt_handler_entries);
while (it != NULL) { while (it != NULL) {
// If event handler and its corresponding param are already on the list, don't add again. // If event handler and its corresponding param are already on the list, don't add again.
if ((it->func == func) && (it->param == param)) { if ((it->func == func) && (it->param == param)) {
@ -65,17 +58,17 @@ void ble_drv_add_event_handler(ble_drv_evt_handler_t func, void *param) {
} }
// Add a new handler to the front of the list // Add a new handler to the front of the list
event_handler_t *handler = m_new_ll(event_handler_t, 1); ble_drv_evt_handler_entry_t *handler = m_new_ll(ble_drv_evt_handler_entry_t, 1);
handler->next = m_event_handlers; handler->next = MP_STATE_VM(ble_drv_evt_handler_entries);
handler->param = param; handler->param = param;
handler->func = func; handler->func = func;
m_event_handlers = handler; MP_STATE_VM(ble_drv_evt_handler_entries) = handler;
} }
void ble_drv_remove_event_handler(ble_drv_evt_handler_t func, void *param) { void ble_drv_remove_event_handler(ble_drv_evt_handler_t func, void *param) {
event_handler_t *it = m_event_handlers; ble_drv_evt_handler_entry_t *it = MP_STATE_VM(ble_drv_evt_handler_entries);
event_handler_t **prev = &m_event_handlers; ble_drv_evt_handler_entry_t **prev = &MP_STATE_VM(ble_drv_evt_handler_entries);
while (it != NULL) { while (it != NULL) {
if ((it->func == func) && (it->param == param)) { if ((it->func == func) && (it->param == param)) {
// Splice out the matching handler. // Splice out the matching handler.
@ -122,7 +115,7 @@ void SD_EVT_IRQHandler(void) {
break; break;
} }
event_handler_t *it = m_event_handlers; ble_drv_evt_handler_entry_t *it = MP_STATE_VM(ble_drv_evt_handler_entries);
while (it != NULL) { while (it != NULL) {
it->func((ble_evt_t *)m_ble_evt_buf, it->param); it->func((ble_evt_t *)m_ble_evt_buf, it->param);
it = it->next; it = it->next;

View File

@ -50,6 +50,12 @@
typedef void (*ble_drv_evt_handler_t)(ble_evt_t*, void*); typedef void (*ble_drv_evt_handler_t)(ble_evt_t*, void*);
typedef struct ble_drv_evt_handler_entry {
struct ble_drv_evt_handler_entry *next;
void *param;
ble_drv_evt_handler_t func;
} ble_drv_evt_handler_entry_t;
void ble_drv_reset(void); void ble_drv_reset(void);
void ble_drv_add_event_handler(ble_drv_evt_handler_t func, void *param); void ble_drv_add_event_handler(ble_drv_evt_handler_t func, void *param);
void ble_drv_remove_event_handler(ble_drv_evt_handler_t func, void *param); void ble_drv_remove_event_handler(ble_drv_evt_handler_t func, void *param);

View File

@ -29,7 +29,7 @@
#include "ble_drv.h" #include "ble_drv.h"
#include "ble_gatts.h" #include "ble_gatts.h"
#include "sd_mutex.h" #include "nrf_nvic.h"
#include "lib/utils/interrupt_char.h" #include "lib/utils/interrupt_char.h"
#include "py/runtime.h" #include "py/runtime.h"
@ -47,14 +47,14 @@ STATIC void characteristic_buffer_on_ble_evt(ble_evt_t *ble_evt, void *param) {
ble_gatts_evt_write_t *evt_write = &ble_evt->evt.gatts_evt.params.write; ble_gatts_evt_write_t *evt_write = &ble_evt->evt.gatts_evt.params.write;
// Event handle must match the handle for my characteristic. // Event handle must match the handle for my characteristic.
if (evt_write->handle == self->characteristic->handle) { if (evt_write->handle == self->characteristic->handle) {
// Push all the data onto the ring buffer, but wait for any reads to finish. // Push all the data onto the ring buffer.
sd_mutex_acquire_wait_no_vm(&self->ringbuf_mutex); uint8_t is_nested_critical_region;
sd_nvic_critical_region_enter(&is_nested_critical_region);
for (size_t i = 0; i < evt_write->len; i++) { for (size_t i = 0; i < evt_write->len; i++) {
ringbuf_put(&self->ringbuf, evt_write->data[i]); ringbuf_put(&self->ringbuf, evt_write->data[i]);
} }
// Don't check for errors: we're in an event handler. sd_nvic_critical_region_exit(is_nested_critical_region);
sd_mutex_release(&self->ringbuf_mutex); break;
break;
} }
} }
} }
@ -72,7 +72,6 @@ void common_hal_bleio_characteristic_buffer_construct(bleio_characteristic_buffe
// This is a macro. // This is a macro.
// true means long-lived, so it won't be moved. // true means long-lived, so it won't be moved.
ringbuf_alloc(&self->ringbuf, buffer_size, true); ringbuf_alloc(&self->ringbuf, buffer_size, true);
sd_mutex_new(&self->ringbuf_mutex);
ble_drv_add_event_handler(characteristic_buffer_on_ble_evt, self); ble_drv_add_event_handler(characteristic_buffer_on_ble_evt, self);
@ -92,8 +91,9 @@ int common_hal_bleio_characteristic_buffer_read(bleio_characteristic_buffer_obj_
#endif #endif
} }
// Copy received data. Lock out writes while copying. // Copy received data. Lock out write interrupt handler while copying.
sd_mutex_acquire_wait(&self->ringbuf_mutex); uint8_t is_nested_critical_region;
sd_nvic_critical_region_enter(&is_nested_critical_region);
size_t rx_bytes = MIN(ringbuf_count(&self->ringbuf), len); size_t rx_bytes = MIN(ringbuf_count(&self->ringbuf), len);
for ( size_t i = 0; i < rx_bytes; i++ ) { for ( size_t i = 0; i < rx_bytes; i++ ) {
@ -101,20 +101,25 @@ int common_hal_bleio_characteristic_buffer_read(bleio_characteristic_buffer_obj_
} }
// Writes now OK. // Writes now OK.
sd_mutex_release_check(&self->ringbuf_mutex); sd_nvic_critical_region_exit(is_nested_critical_region);
return rx_bytes; return rx_bytes;
} }
uint32_t common_hal_bleio_characteristic_buffer_rx_characters_available(bleio_characteristic_buffer_obj_t *self) { uint32_t common_hal_bleio_characteristic_buffer_rx_characters_available(bleio_characteristic_buffer_obj_t *self) {
return ringbuf_count(&self->ringbuf); uint8_t is_nested_critical_region;
sd_nvic_critical_region_enter(&is_nested_critical_region);
uint16_t count = ringbuf_count(&self->ringbuf);
sd_nvic_critical_region_exit(is_nested_critical_region);
return count;
} }
void common_hal_bleio_characteristic_buffer_clear_rx_buffer(bleio_characteristic_buffer_obj_t *self) { void common_hal_bleio_characteristic_buffer_clear_rx_buffer(bleio_characteristic_buffer_obj_t *self) {
// prevent conflict with uart irq // prevent conflict with uart irq
sd_mutex_acquire_wait(&self->ringbuf_mutex); uint8_t is_nested_critical_region;
sd_nvic_critical_region_enter(&is_nested_critical_region);
ringbuf_clear(&self->ringbuf); ringbuf_clear(&self->ringbuf);
sd_mutex_release_check(&self->ringbuf_mutex); sd_nvic_critical_region_exit(is_nested_critical_region);
} }
bool common_hal_bleio_characteristic_buffer_deinited(bleio_characteristic_buffer_obj_t *self) { bool common_hal_bleio_characteristic_buffer_deinited(bleio_characteristic_buffer_obj_t *self) {

View File

@ -38,7 +38,6 @@ typedef struct {
uint32_t timeout_ms; uint32_t timeout_ms;
// Ring buffer storing consecutive incoming values. // Ring buffer storing consecutive incoming values.
ringbuf_t ringbuf; ringbuf_t ringbuf;
nrf_mutex_t ringbuf_mutex;
} bleio_characteristic_buffer_obj_t; } bleio_characteristic_buffer_obj_t;
#endif // MICROPY_INCLUDED_COMMON_HAL_BLEIO_CHARACTERISTICBUFFER_H #endif // MICROPY_INCLUDED_COMMON_HAL_BLEIO_CHARACTERISTICBUFFER_H

View File

@ -28,6 +28,8 @@
#ifndef NRF5_MPCONFIGPORT_H__ #ifndef NRF5_MPCONFIGPORT_H__
#define NRF5_MPCONFIGPORT_H__ #define NRF5_MPCONFIGPORT_H__
#include "ble_drv.h"
#define MICROPY_CPYTHON_COMPAT (1) #define MICROPY_CPYTHON_COMPAT (1)
//#define MICROPY_MODULE_BUILTIN_INIT (1) // TODO check this //#define MICROPY_MODULE_BUILTIN_INIT (1) // TODO check this
//#define MICROPY_MODULE_WEAK_LINKS (1) // TODO check this //#define MICROPY_MODULE_WEAK_LINKS (1) // TODO check this
@ -52,10 +54,11 @@
// 24kiB stack // 24kiB stack
#define CIRCUITPY_DEFAULT_STACK_SIZE 0x6000 #define CIRCUITPY_DEFAULT_STACK_SIZE 0x6000
#define MICROPY_PORT_ROOT_POINTERS \
CIRCUITPY_COMMON_ROOT_POINTERS \
;
#include "py/circuitpy_mpconfig.h" #include "py/circuitpy_mpconfig.h"
#define MICROPY_PORT_ROOT_POINTERS \
CIRCUITPY_COMMON_ROOT_POINTERS \
ble_drv_evt_handler_entry_t* ble_drv_evt_handler_entries; \
#endif // NRF5_MPCONFIGPORT_H__ #endif // NRF5_MPCONFIGPORT_H__