Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions ports/atmel-samd/mpconfigport.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,10 @@

#include "peripherals/samd/dma.h"

#include "py/circuitpy_mpconfig.h"

#define MICROPY_PORT_ROOT_POINTERS \
CIRCUITPY_COMMON_ROOT_POINTERS \
mp_obj_t playing_audio[AUDIO_DMA_CHANNEL_COUNT] \
;

#include "py/circuitpy_mpconfig.h"
mp_obj_t playing_audio[AUDIO_DMA_CHANNEL_COUNT];

#endif // __INCLUDED_MPCONFIGPORT_H
25 changes: 9 additions & 16 deletions ports/nrf/bluetooth/ble_drv.c
Original file line number Diff line number Diff line change
Expand Up @@ -35,27 +35,20 @@
#include "nrf_soc.h"
#include "nrfx_power.h"
#include "py/misc.h"
#include "py/mpstate.h"

nrf_nvic_state_t nrf_nvic_state = { 0 };

__attribute__((aligned(4)))
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() {
// 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) {
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) {
// If event handler and its corresponding param are already on the list, don't add again.
if ((it->func == func) && (it->param == param)) {
Expand All @@ -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
event_handler_t *handler = m_new_ll(event_handler_t, 1);
handler->next = m_event_handlers;
ble_drv_evt_handler_entry_t *handler = m_new_ll(ble_drv_evt_handler_entry_t, 1);
handler->next = MP_STATE_VM(ble_drv_evt_handler_entries);
handler->param = param;
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) {
event_handler_t *it = m_event_handlers;
event_handler_t **prev = &m_event_handlers;
ble_drv_evt_handler_entry_t *it = MP_STATE_VM(ble_drv_evt_handler_entries);
ble_drv_evt_handler_entry_t **prev = &MP_STATE_VM(ble_drv_evt_handler_entries);
while (it != NULL) {
if ((it->func == func) && (it->param == param)) {
// Splice out the matching handler.
Expand Down Expand Up @@ -122,7 +115,7 @@ void SD_EVT_IRQHandler(void) {
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) {
it->func((ble_evt_t *)m_ble_evt_buf, it->param);
it = it->next;
Expand Down
6 changes: 6 additions & 0 deletions ports/nrf/bluetooth/ble_drv.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,12 @@

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_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);
Expand Down
31 changes: 18 additions & 13 deletions ports/nrf/common-hal/bleio/CharacteristicBuffer.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@

#include "ble_drv.h"
#include "ble_gatts.h"
#include "sd_mutex.h"
#include "nrf_nvic.h"

#include "lib/utils/interrupt_char.h"
#include "py/runtime.h"
Expand All @@ -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;
// Event handle must match the handle for my characteristic.
if (evt_write->handle == self->characteristic->handle) {
// Push all the data onto the ring buffer, but wait for any reads to finish.
sd_mutex_acquire_wait_no_vm(&self->ringbuf_mutex);
// Push all the data onto the ring buffer.
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++) {
ringbuf_put(&self->ringbuf, evt_write->data[i]);
}
// Don't check for errors: we're in an event handler.
sd_mutex_release(&self->ringbuf_mutex);
break;
sd_nvic_critical_region_exit(is_nested_critical_region);
break;
}
}
}
Expand All @@ -72,7 +72,6 @@ void common_hal_bleio_characteristic_buffer_construct(bleio_characteristic_buffe
// This is a macro.
// true means long-lived, so it won't be moved.
ringbuf_alloc(&self->ringbuf, buffer_size, true);
sd_mutex_new(&self->ringbuf_mutex);

ble_drv_add_event_handler(characteristic_buffer_on_ble_evt, self);

Expand All @@ -92,29 +91,35 @@ int common_hal_bleio_characteristic_buffer_read(bleio_characteristic_buffer_obj_
#endif
}

// Copy received data. Lock out writes while copying.
sd_mutex_acquire_wait(&self->ringbuf_mutex);
// Copy received data. Lock out write interrupt handler while copying.
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);
for ( size_t i = 0; i < rx_bytes; i++ ) {
data[i] = ringbuf_get(&self->ringbuf);
}

// Writes now OK.
sd_mutex_release_check(&self->ringbuf_mutex);
sd_nvic_critical_region_exit(is_nested_critical_region);

return rx_bytes;
}

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) {
// 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);
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) {
Expand Down
1 change: 0 additions & 1 deletion ports/nrf/common-hal/bleio/CharacteristicBuffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ typedef struct {
uint32_t timeout_ms;
// Ring buffer storing consecutive incoming values.
ringbuf_t ringbuf;
nrf_mutex_t ringbuf_mutex;
} bleio_characteristic_buffer_obj_t;

#endif // MICROPY_INCLUDED_COMMON_HAL_BLEIO_CHARACTERISTICBUFFER_H
7 changes: 5 additions & 2 deletions ports/nrf/mpconfigport.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
#ifndef NRF5_MPCONFIGPORT_H__
#define NRF5_MPCONFIGPORT_H__

#include "ble_drv.h"

#define MICROPY_CPYTHON_COMPAT (1)
//#define MICROPY_MODULE_BUILTIN_INIT (1) // TODO check this
//#define MICROPY_MODULE_WEAK_LINKS (1) // TODO check this
Expand All @@ -52,10 +54,11 @@
// 24kiB stack
#define CIRCUITPY_DEFAULT_STACK_SIZE 0x6000

#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; \

#include "py/circuitpy_mpconfig.h"

#endif // NRF5_MPCONFIGPORT_H__