8000 ringbuf cleanup by dhalbert · Pull Request #6915 · adafruit/circuitpython · GitHub
[go: up one dir, main page]

Skip to content

ringbuf cleanup #6915

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 2 commits into from
Sep 25, 2022
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
1 change: 1 addition & 0 deletions devices/ble_hci/common-hal/_bleio/CharacteristicBuffer.c
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ bool common_hal_bleio_characteristic_buffer_deinited(bleio_characteristic_buffer
void common_hal_bleio_characteristic_buffer_deinit(bleio_characteristic_buffer_obj_t *self) {
if (!common_hal_bleio_characteristic_buffer_deinited(self)) {
bleio_characteristic_clear_observer(self->characteristic);
ringbuf_deinit(&self->ringbuf);
}
}

Expand Down
5 changes: 3 additions & 2 deletions devices/ble_hci/common-hal/_bleio/PacketBuffer.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,13 @@
#include "supervisor/shared/tick.h"

STATIC void write_to_ringbuf(bleio_packet_buffer_obj_t *self, uint8_t *data, uint16_t len) {
if (len + sizeof(uint16_t) > ringbuf_capacity(&self->ringbuf)) {
if (len + sizeof(uint16_t) > ringbuf_size(&self->ringbuf)) {
// This shouldn't happen.
return;
}
// Push all the data onto the ring buffer.
// Make room for the new value by dropping the oldest packets first.
while (ringbuf_capacity(&self->ringbuf) - ringbuf_num_filled(&self->ringbuf) < len + sizeof(uint16_t)) {
while (ringbuf_size(&self->ringbuf) - ringbuf_num_filled(&self->ringbuf) < len + sizeof(uint16_t)) {
uint16_t packet_length;
ringbuf_get_n(&self->ringbuf, (uint8_t *)&packet_length, sizeof(uint16_t));
for (uint16_t i = 0; i < packet_length; i++) {
Expand Down Expand Up @@ -264,5 +264,6 @@ bool common_hal_bleio_packet_buffer_deinited(bleio_packet_buffer_obj_t *self) {
void common_hal_bleio_packet_buffer_deinit(bleio_packet_buffer_obj_t *self) {
if (!common_hal_bleio_packet_buffer_deinited(self)) {
bleio_characteristic_clear_observer(self->characteristic);
ringbuf_deinit(&self->ringbuf);
}
}
6 changes: 3 additions & 3 deletions ports/broadcom/common-hal/busio/UART.c
Original file line number Diff line number Diff line change
Expand Up @@ -208,16 +208,16 @@ void common_hal_busio_uart_construct(busio_uart_obj_t *self,
self->sigint_enabled = sigint_enabled;

if (rx != NULL) {
// Use the provided buffer when given.
if (receiver_buffer != NULL) {
self->ringbuf = (ringbuf_t) { receiver_buffer, receiver_buffer_size };
ringbuf_init(&self->ringbuf, receiver_buffer, receiver_buffer_size);
} else {
// Initially allocate the UART's buffer in the long-lived part of the
// heap. UARTs are generally long-lived objects, but the "make long-
// lived" machinery is incapable of moving internal pointers like
// self->buffer, so do it manually. (However, as long as internal
// pointers like this are NOT moved, allocating the buffer
// in the long-lived pool is not strictly necessary)
// (This is a macro.)
if (!ringbuf_alloc(&self->ringbuf, receiver_buffer_size, true)) {
m_malloc_fail(receiver_buffer_size);
}
Expand Down Expand Up @@ -337,7 +337,7 @@ void common_hal_busio_uart_deinit(busio_uart_obj_t *self) {
pl011->CR = 0;
}
active_uart[self->uart_id] = NULL;
ringbuf_free(&self->ringbuf);
ringbuf_deinit(&self->ringbuf);
uart_status[self->uart_id] = STATUS_FREE;
common_hal_reset_pin(self->tx_pin);
common_hal_reset_pin(self->rx_pin);
Expand Down
4 changes: 2 additions & 2 deletions ports/broadcom/common-hal/busio/UART.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,11 @@ typedef struct {
const mcu_pin_obj_t *rx_pin;
const mcu_pin_obj_t *cts_pin;
const mcu_pin_obj_t *rts_pin;
uint8_t uart_id;
uint32_t baudrate;
uint32_t timeout_ms;
bool sigint_enabled;
ringbuf_t ringbuf;
bool sigint_enabled;
uint8_t uart_id;
} busio_uart_obj_t;

extern void reset_uart(void);
Expand Down
7 changes: 2 additions & 5 deletions ports/espressif/common-hal/_bleio/CharacteristicBuffer.c
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,7 @@ void _common_hal_bleio_characteristic_buffer_construct(bleio_characteristic_buff
void *static_handler_entry) {
self->characteristic = characteristic;
self->timeout_ms = timeout * 1000;

self->ringbuf.buf = (uint8_t *)buffer;
self->ringbuf.size = buffer_size;
self->ringbuf.iget = 0;
self->ringbuf.iput = 0;
ringbuf_init(&self->ringbuf, buffer, buffer_size);

if (static_handler_entry != NULL) {
ble_event_add_handler_entry((ble_event_handler_entry_t *)static_handler_entry, characteristic_buffer_on_ble_evt, self);
Expand Down Expand Up @@ -131,6 +127,7 @@ void common_hal_bleio_characteristic_buffer_deinit(bleio_characteristic_buffer_o
if (!common_hal_bleio_characteristic_buffer_deinited(self)) {
ble_event_remove_handler(characteristic_buffer_on_ble_evt, self);
self->characteristic = NULL;
ringbuf_deinit(&self->ringbuf);
}
}

Expand Down
13 changes: 5 additions & 8 deletions ports/espressif/common-hal/_bleio/PacketBuffer.c
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,13 @@

STATIC void write_to_ringbuf(bleio_packet_buffer_obj_t *self, const struct os_mbuf *mbuf) {
size_t len = OS_MBUF_PKTLEN(mbuf);
if (len + sizeof(uint16_t) > ringbuf_capacity(&self->ringbuf)) {
if (len + sizeof(uint16_t) > ringbuf_size(&self->ringbuf)) {
// This shouldn't happen but can if our buffer size was much smaller than
// the writes the client actually makes.
return;
}
// Make room for the new value by dropping the oldest packets first.
while (ringbuf_capacity(&self->ringbuf) - ringbuf_num_filled(&self->ringbuf) < len + sizeof(uint16_t)) {
while (ringbuf_size(&self->ringbuf) - ringbuf_num_filled(&self->ringbuf) < len + sizeof(uint16_t)) {
uint16_t packet_length;
ringbuf_get_n(&self->ringbuf, (uint8_t *)&packet_length, sizeof(uint16_t));
for (uint16_t i = 0; i < packet_length; i++) {
Expand Down Expand Up @@ -164,10 +164,7 @@ void _common_hal_bleio_packet_buffer_construct(
}

if (incoming) {
self->ringbuf.buf = (uint8_t *)incoming_buffer;
self->ringbuf.size = incoming_buffer_size;
self->ringbuf.iget = 0;
self->ringbuf.iput = 0;
ringbuf_init(&self->ringbuf, (uint8_t *)incoming_buffer, incoming_buffer_size);
}

self->packet_queued = false;
Expand Down Expand Up @@ -219,8 +216,7 @@ void common_hal_bleio_packet_buffer_construct(
size_t incoming_buffer_size = 0;
uint32_t *incoming_buffer = NULL;
if (incoming) {
incoming_buffer_size = buffer_size * (sizeof(uint16_t) + max_packet_size);
incoming_buffer = m_malloc(incoming_buffer_size, false);
ringbuf_init(&self->ringbuf, (uint8_t *)incoming_buffer, incoming_buffer_size);
}

uint32_t *outgoing1 = NULL;
Expand Down Expand Up @@ -414,5 +410,6 @@ bool common_hal_bleio_packet_buffer_deinited(bleio_packet_buffer_obj_t *self) {
void common_hal_bleio_packet_buffer_deinit(bleio_packet_buffer_obj_t *self) {
if (!common_hal_bleio_packet_buffer_deinited(self)) {
ble_event_remove_handler(packet_buffer_on_ble_client_evt, self);
ringbuf_deinit(&self->ringbuf);
}
}
6 changes: 2 additions & 4 deletions ports/nrf/common-hal/_bleio/CharacteristicBuffer.c
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,7 @@ void _common_hal_bleio_characteristic_buffer_construct(bleio_characteristic_buff
self->characteristic = characteristic;
self->timeout_ms = timeout * 1000;

self->ringbuf.buf = (uint8_t *)buffer;
self->ringbuf.size = buffer_size;
self->ringbuf.iget = 0;
self->ringbuf.iput = 0;
ringbuf_init(&self->ringbuf, buffer, buffer_size);

if (static_handler_entry != NULL) {
ble_drv_add_event_handler_entry((ble_drv_evt_handler_entry_t *)static_handler_entry, characteristic_buffer_on_ble_evt, self);
Expand Down Expand Up @@ -159,6 +156,7 @@ void common_hal_bleio_characteristic_buffer_deinit(bleio_characteristic_buffer_o
if (!common_hal_bleio_characteristic_buffer_deinited(self)) {
ble_drv_remove_event_handler(characteristic_buffer_on_ble_evt, self);
self->characteristic = NULL;
ringbuf_deinit(&self->ringbuf);
}
}

Expand Down
11 changes: 5 additions & 6 deletions ports/nrf/common-hal/_bleio/PacketBuffer.c
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
#include "supervisor/shared/bluetooth/serial.h"

STATIC void write_to_ringbuf(bleio_packet_buffer_obj_t *self, uint8_t *data, uint16_t len) {
if (len + sizeof(uint16_t) > ringbuf_capacity(&self->ringbuf)) {
if (len + sizeof(uint16_t) > ringbuf_size(&self->ringbuf)) {
// This shouldn't happen but can if our buffer size was much smaller than
// the writes the client actually makes.
return;
Expand All @@ -52,7 +52,7 @@ STATIC void write_to_ringbuf(bleio_packet_buffer_obj_t *self, uint8_t *data, uin
uint8_t is_nested_critical_region;
sd_nvic_critical_region_enter(&is_nested_critical_region);
// Make room for the new value by dropping the oldest packets first.
while (ringbuf_capacity(&self->ringbuf) - ringbuf_num_filled(&self->ringbuf) < len + sizeof(uint16_t)) {
while (ringbuf_size(&self->ringbuf) - ringbuf_num_filled(&self->ringbuf) < len + sizeof(uint16_t)) {
uint16_t packet_length;
ringbuf_get_n(&self->ringbuf, (uint8_t *)&packet_length, sizeof(uint16_t));
for (uint16_t i = 0; i < packet_length; i++) {
Expand Down Expand Up @@ -233,10 +233,7 @@ void _common_hal_bleio_packet_buffer_construct(
}

if (incoming) {
self->ringbuf.buf = (uint8_t *)incoming_buffer;
self->ringbuf.size = incoming_buffer_size;
self->ringbuf.iget = 0;
self->ringbuf.iput = 0;
ringbuf_init(&self->ringbuf, (uint8_t *)incoming_buffer, incoming_buffer_size);
}

self->packet_queued = false;
Expand Down Expand Up @@ -502,7 +499,9 @@ bool common_hal_bleio_packet_buffer_deinited(bleio_packet_buffer_obj_t *self) {
}

void common_hal_bleio_packet_buffer_deinit(bleio_packet_buffer_obj_t *self) {

if (!common_hal_bleio_packet_buffer_deinited(self)) {
ble_drv_remove_event_handler(packet_buffer_on_ble_client_evt, self);
ringbuf_deinit(&self->ringbuf);
}
}
22 changes: 8 additions & 14 deletions ports/nrf/common-hal/busio/UART.c
Original file line number Diff line number Diff line change
Expand Up @@ -213,24 +213,20 @@ void common_hal_busio_uart_construct(busio_uart_obj_t *self,

// Init buffer for rx
if (rx != NULL) {
self->allocated_ringbuf = true;
// Use the provided buffer when given.
if (receiver_buffer != NULL) {
self->ringbuf.buf = receiver_buffer;
self->ringbuf.size = receiver_buffer_size - 1;
self->ringbuf.iput = 0;
self->ringbuf.iget = 0;
self->allocated_ringbuf = false;
ringbuf_init(&self->ringbuf, receiver_buffer, receiver_buffer_size);
} else {
// Initially allocate the UART's buffer in the long-lived part of the
// heap. UARTs are generally long-lived objects, but the "make long-
// lived" machinery is incapable of moving internal pointers like
// self->buffer, so do it manually. (However, as long as internal
// pointers like this are NOT moved, allocating the buffer
// in the long-lived pool is not strictly necessary)
// (This is a macro.)
} else if (!ringbuf_alloc(&self->ringbuf, receiver_buffer_size, true)) {
nrfx_uarte_uninit(self->uarte);
m_malloc_fail(receiver_buffer_size);
if (!ringbuf_alloc(&self->ringbuf, receiver_buffer_size, true)) {
nrfx_uarte_uninit(self->uarte);
m_malloc_fail(receiver_buffer_size);
}
}

self->rx_pin_number = rx->number;
Expand Down Expand Up @@ -282,9 +278,7 @@ void common_hal_busio_uart_deinit(busio_uart_obj_t *self) {
self->rx_pin_number = NO_PIN;
self->rts_pin_number = NO_PIN;
self->cts_pin_number = NO_PIN;
if (self->allocated_ringbuf) {
ringbuf_free(&self->ringbuf);
}
ringbuf_deinit(&self->ringbuf);

for (size_t i = 0; i < MP_ARRAY_SIZE(nrfx_uartes); i++) {
if (self->uarte == &nrfx_uartes[i]) {
Expand All @@ -305,7 +299,7 @@ size_t common_hal_busio_uart_read(busio_uart_obj_t *self, uint8_t *data, size_t

// check removed to reduce code size
/*
if (len > ringbuf_capacity(&self->ringbuf)) {
if (len > ringbuf_size(&self->ringbuf)) {
mp_raise_ValueError(translate("Reading >receiver_buffer_size bytes is not supported"));
}
*/
Expand Down
1 change: 0 additions & 1 deletion ports/nrf/common-hal/busio/UART.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ typedef struct {
ringbuf_t ringbuf;
uint8_t rx_char; // EasyDMA buf
bool rx_paused; // set by irq if no space in rbuf
bool allocated_ringbuf;

uint8_t tx_pin_number;
uint8_t rx_pin_number;
Expand Down
44 changes: 25 additions & 19 deletions ports/raspberrypi/common-hal/busio/UART.c
Original file line number Diff line number Diff line change
Expand Up @@ -155,27 +155,33 @@ void common_hal_busio_uart_construct(busio_uart_obj_t *self,
uart_set_hw_flow(self->uart, (cts != NULL), (rts != NULL));

if (rx != NULL) {
// Initially allocate the UART's buffer in the long-lived part of the
// heap. UARTs are generally long-lived objects, but the "make long-
// lived" machinery is incapable of moving internal pointers like
// self->buffer, so do it manually. (However, as long as internal
// pointers like this are NOT moved, allocating the buffer
// in the long-lived pool is not strictly necessary)
// (This is a macro.)
if (!ringbuf_alloc(&self->ringbuf, receiver_buffer_size, true)) {
m_malloc_fail(receiver_buffer_size);
}
active_uarts[uart_id] = self;
if (uart_id == 1) {
self->uart_irq_id = UART1_IRQ;
irq_set_exclusive_handler(self->uart_irq_id, uart1_callback);
// Use the provided buffer when given.
if (receiver_buffer != NULL) {
ringbuf_init(&self->ringbuf, receiver_buffer, receiver_buffer_size);
} else {
self->uart_irq_id = UART0_IRQ;
irq_set_exclusive_handler(self->uart_irq_id, uart0_callback);
// Initially allocate the UART's buffer in the long-lived part of the
// heap. UARTs are generally long-lived objects, but the "make long-
// lived" machinery is incapable of moving internal pointers like
// self->buffer, so do it manually. (However, as long as internal
// pointers like this are NOT moved, allocating the buffer
// in the long-lived pool is not strictly necessary)
if (!ringbuf_alloc(&self->ringbuf, receiver_buffer_size, true)) {
uart_deinit(self->uart);
m_malloc_fail(receiver_buffer_size);
}
}
irq_set_enabled(self->uart_irq_id, true);
uart_set_irq_enables(self->uart, true /* rx has data */, false /* tx needs data */);
}

active_uarts[uart_id] = self;
if (uart_id == 1) {
self->uart_irq_id = UART1_IRQ;
irq_set_exclusive_handler(self->uart_irq_id, uart1_callback);
} else {
self->uart_irq_id = UART0_IRQ;
irq_set_exclusive_handler(self->uart_irq_id, uart0_callback);
}
irq_set_enabled B41A (self->uart_irq_id, true);
uart_set_irq_enables(self->uart, true /* rx has data */, false /* tx needs data */);
}

bool common_hal_busio_uart_deinited(busio_uart_obj_t *self) {
Expand All @@ -187,7 +193,7 @@ void common_hal_busio_uart_deinit(busio_uart_obj_t *self) {
return;
}
uart_deinit(self->uart);
ringbuf_free(&self->ringbuf);
ringbuf_deinit(&self->ringbuf);
active_uarts[self->uart_id] = NULL;
uart_status[self->uart_id] = STATUS_FREE;
reset_pin_number(self->tx_pin);
Expand Down
11 changes: 9 additions & 2 deletions ports/stm/common-hal/busio/UART.c
Original file line number Diff line number Diff line change
Expand Up @@ -214,9 +214,16 @@ void common_hal_busio_uart_construct(busio_uart_obj_t *self,

// Init buffer for rx and claim pins
if (self->rx != NULL) {
// Use the provided buffer when given.
if (receiver_buffer != NULL) {
self->ringbuf = (ringbuf_t) { receiver_buffer, receiver_buffer_size };
ringbuf_init(&self->ringbuf, receiver_buffer, receiver_buffer_size);
} else {
// Initially allocate the UART's buffer in the long-lived part of the
// heap. UARTs are generally long-lived objects, but the "make long-
// lived" machinery is incapable of moving internal pointers like
// self->buffer, so do it manually. (However, as long as internal
// pointers like this are NOT moved, allocating the buffer
// in the long-lived pool is not strictly necessary)
if (!ringbuf_alloc(&self->ringbuf, receiver_buffer_size, true)) {
m_malloc_fail(receiver_buffer_size);
}
Expand Down Expand Up @@ -281,7 +288,7 @@ void common_hal_busio_uart_deinit(busio_uart_obj_t *self) {
self->rx = NULL;
}

ringbuf_free(&self->ringbuf);
ringbuf_deinit(&self->ringbuf);
}

size_t common_hal_busio_uart_read(busio_uart_obj_t *self, uint8_t *data, size_t len, int *errcode) {
Expand Down
Loading
0