8000 ringbuf cleanup · adafruit/circuitpython@ddfbaee · GitHub
[go: up one dir, main page]

Skip to content

Commit ddfbaee

Browse files
committed
ringbuf cleanup
1 parent 32d8dd4 commit ddfbaee

File tree

14 files changed

+130
-157
lines changed

14 files changed

+130
-157
lines changed

devices/ble_hci/common-hal/_bleio/PacketBuffer.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,13 +37,13 @@
3737
#include "supervisor/shared/tick.h"
3838

3939
STATIC void write_to_ringbuf(bleio_packet_buffer_obj_t *self, uint8_t *data, uint16_t len) {
40-
if (len + sizeof(uint16_t) > ringbuf_capacity(&self->ringbuf)) {
40+
if (len + sizeof(uint16_t) > ringbuf_size(&self->ringbuf)) {
4141
// This shouldn't happen.
4242
return;
4343
}
4444
// Push all the data onto the ring buffer.
4545
// Make room for the new value by dropping the oldest packets first.
46-
while (ringbuf_capacity(&self->ringbuf) - ringbuf_num_filled(&self->ringbuf) < len + sizeof(uint16_t)) {
46+
while (ringbuf_size(&self->ringbuf) - ringbuf_num_filled(&self->ringbuf) < len + sizeof(uint16_t)) {
4747
uint16_t packet_length;
4848
ringbuf_get_n(&self->ringbuf, (uint8_t *)&packet_length, sizeof(uint16_t));
4949
for (uint16_t i = 0; i < packet_length; i++) {

ports/broadcom/common-hal/busio/UART.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,9 @@ void common_hal_busio_uart_construct(busio_uart_obj_t *self,
208208
self->sigint_enabled = sigint_enabled;
209209

210210
if (rx != NULL) {
211+
// Use the provided buffer when given.
211212
if (receiver_buffer != NULL) {
213+
ringbuf_init(&self->ringbuf, receiver_buffer, receiver_buffer_size);
212214
self->ringbuf = (ringbuf_t) { receiver_buffer, receiver_buffer_size };
213215
} else {
214216
// Initially allocate the UART's buffer in the long-lived part of the
@@ -217,7 +219,6 @@ void common_hal_busio_uart_construct(busio_uart_obj_t *self,
217219
// self->buffer, so do it manually. (However, as long as internal
218220
// pointers like this are NOT moved, allocating the buffer
219221
// in the long-lived pool is not strictly necessary)
220-
// (This is a macro.)
221222
if (!ringbuf_alloc(&self->ringbuf, receiver_buffer_size, true)) {
222223
m_malloc_fail(receiver_buffer_size);
223224
}
@@ -337,7 +338,7 @@ void common_hal_busio_uart_deinit(busio_uart_obj_t *self) {
337338
pl011->CR = 0;
338339
}
339340
active_uart[self->uart_id] = NULL;
340-
ringbuf_free(&self->ringbuf);
341+
ringbuf_deinit(&self->ringbuf);
341342
uart_status[self->uart_id] = STATUS_FREE;
342343
common_hal_reset_pin(self->tx_pin);
343344
common_hal_reset_pin(self->rx_pin);

ports/broadcom/common-hal/busio/UART.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,11 @@ typedef struct {
3636
const mcu_pin_obj_t *rx_pin;
3737
const mcu_pin_obj_t *cts_pin;
3838
const mcu_pin_obj_t *rts_pin;
39-
uint8_t uart_id;
4039
uint32_t baudrate;
4140
uint32_t timeout_ms;
42-
bool sigint_enabled;
4341
ringbuf_t ringbuf;
42+
bool sigint_enabled;
43+
uint8_t uart_id;
4444
} busio_uart_obj_t;
4545

4646
extern void reset_uart(void);

ports/espressif/common-hal/_bleio/CharacteristicBuffer.c

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -72,11 +72,7 @@ void _common_hal_bleio_characteristic_buffer_construct(bleio_characteristic_buff
7272
void *static_handler_entry) {
7373
self->characteristic = characteristic;
7474
self->timeout_ms = timeout * 1000;
75-
76-
self->ringbuf.buf = (uint8_t *)buffer;
77-
self->ringbuf.size = buffer_size;
78-
self->ringbuf.iget = 0;
79-
self->ringbuf.iput = 0;
75+
ringbuf_init(&self->ringbuf, buffer, buffer_size);
8076

8177
if (static_handler_entry != NULL) {
8278
ble_event_add_handler_entry((ble_event_handler_entry_t *)static_handler_entry, characteristic_buffer_on_ble_evt, self);
@@ -131,6 +127,7 @@ void common_hal_bleio_characteristic_buffer_deinit(bleio_characteristic_buffer_o
131127
if (!common_hal_bleio_characteristic_buffer_deinited(self)) {
132128
ble_event_remove_handler(characteristic_buffer_on_ble_evt, self);
133129
self->characteristic = NULL;
130+
ringbuf_deinit(self->ringbuf);
134131
}
135132
}
136133

ports/espressif/common-hal/_bleio/PacketBuffer.c

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,13 @@
4242

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

166166
if (incoming) {
167-
self->ringbuf.buf = (uint8_t *)incoming_buffer;
168-
self->ringbuf.size = incoming_buffer_size;
169-
self->ringbuf.iget = 0;
170-
self->ringbuf.iput = 0;
167+
ringbuf_init(&self->ringbuf, (uint8_t *)incoming_buffer, incoming_buffer_size);
171168
}
172169

173170
self->packet_queued = false;
@@ -219,8 +216,7 @@ void common_hal_bleio_packet_buffer_construct(
219216
size_t incoming_buffer_size = 0;
220217
uint32_t *incoming_buffer = NULL;
221218
if (incoming) {
222-
incoming_buffer_size = buffer_size * (sizeof(uint16_t) + max_packet_size);
223-
incoming_buffer = m_malloc(incoming_buffer_size, false);
219+
ringbuf_init(&self->ringbuf, (uint8_t *)incoming_buffer, incoming_buffer_size);
224220
}
225221

226222
uint32_t *outgoing1 = NULL;
@@ -414,5 +410,6 @@ bool common_hal_bleio_packet_buffer_deinited(bleio_packet_buffer_obj_t *self) {
414410
void common_hal_bleio_packet_buffer_deinit(bleio_packet_buffer_obj_t *self) {
415411
if (!common_hal_bleio_packet_buffer_deinited(self)) {
416412
ble_event_remove_handler(packet_buffer_on_ble_client_evt, self);
413+
ringbuf_deinit(&self->ringbuf);
417414
}
418415
}

ports/nrf/common-hal/_bleio/CharacteristicBuffer.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,10 +90,7 @@ void _common_hal_bleio_characteristic_buffer_construct(bleio_characteristic_buff
9090
self->characteristic = characteristic;
9191
self->timeout_ms = timeout * 1000;
9292

93-
self->ringbuf.buf = (uint8_t *)buffer;
94-
self->ringbuf.size = buffer_size;
95-
self->ringbuf.iget = 0;
96-
self->ringbuf.iput = 0;
93+
ringbuf_init(&self->ringbuf, buffer, buffer_size);
9794

9895
if (static_handler_entry != NULL) {
9996
ble_drv_add_event_handler_entry((ble_drv_evt_handler_entry_t *)static_handler_entry, characteristic_buffer_on_ble_evt, self);
@@ -159,6 +156,7 @@ void common_hal_bleio_characteristic_buffer_deinit(bleio_characteristic_buffer_o
159156
if (!common_hal_bleio_characteristic_buffer_deinited(self)) {
160157
ble_drv_remove_event_handler(characteristic_buffer_on_ble_evt, self);
161158
self->characteristic = NULL;
159+
ringbuf_deinit(&self->ringbuf);
162160
}
163161
}
164162

ports/nrf/common-hal/_bleio/PacketBuffer.c

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@
4343
#include "supervisor/shared/bluetooth/serial.h"
4444

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

235235
if (incoming) {
236-
self->ringbuf.buf = (uint8_t *)incoming_buffer;
237-
self->ringbuf.size = incoming_buffer_size;
238-
self->ringbuf.iget = 0;
239-
self->ringbuf.iput = 0;
236+
ringbuf_init(&self->ringbuf, (uint8_t *)incoming_buffer, incoming_buffer_size);
240237
}
241238

242239
self->packet_queued = false;
@@ -502,7 +499,9 @@ bool common_hal_bleio_packet_buffer_deinited(bleio_packet_buffer_obj_t *self) {
502499
}
503500

504501
void common_hal_bleio_packet_buffer_deinit(bleio_packet_buffer_obj_t *self) {
502+
505503
if (!common_hal_bleio_packet_buffer_deinited(self)) {
506504
ble_drv_remove_event_handler(packet_buffer_on_ble_client_evt, self);
505+
ringbuf_deinit(&self->ringbuf);
507506
}
508507
}

ports/nrf/common-hal/busio/UART.c

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -213,24 +213,20 @@ void common_hal_busio_uart_construct(busio_uart_obj_t *self,
213213

214214
// Init buffer for rx
215215
if (rx != NULL) {
216-
self->allocated_ringbuf = true;
217216
// Use the provided buffer when given.
218217
if (receiver_buffer != NULL) {
219-
self->ringbuf.buf = receiver_buffer;
220-
self->ringbuf.size = receiver_buffer_size - 1;
221-
self->ringbuf.iput = 0;
222-
self->ringbuf.iget = 0;
223-
self->allocated_ringbuf = false;
218+
ringbuf_init(&self->ringbuf, receiver_buffer, receiver_buffer_size);
219+
} else {
224220
// Initially allocate the UART's buffer in the long-lived part of the
225221
// heap. UARTs are generally long-lived objects, but the "make long-
226222
// lived" machinery is incapable of moving internal pointers like
227223
// self->buffer, so do it manually. (However, as long as internal
228224
// pointers like this are NOT moved, allocating the buffer
229225
// in the long-lived pool is not strictly necessary)
230-
// (This is a macro.)
231-
} else if (!ringbuf_alloc(&self->ringbuf, receiver_buffer_size, true)) {
232-
nrfx_uarte_uninit(self->uarte);
233-
m_malloc_fail(receiver_buffer_size);
226+
if (!ringbuf_alloc(&self->ringbuf, receiver_buffer_size, true)) {
227+
nrfx_uarte_uninit(self->uarte);
228+
m_malloc_fail(receiver_buffer_size);
229+
}
234230
}
235231

236232
self->rx_pin_number = rx->number;
@@ -282,9 +278,7 @@ void common_hal_busio_uart_deinit(busio_uart_obj_t *self) {
282278
self->rx_pin_number = NO_PIN;
283279
self->rts_pin_number = NO_PIN;
284280
self->cts_pin_number = NO_PIN;
285-
if (self->allocated_ringbuf) {
286-
ringbuf_free(&self->ringbuf);
287-
}
281+
ringbuf_deinit(&self->ringbuf);
288282

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

306300
// check removed to reduce code size
307301
/*
308-
if (len > ringbuf_capacity(&self->ringbuf)) {
302+
if (len > ringbuf_size(&self->ringbuf)) {
309303
mp_raise_ValueError(translate("Reading >receiver_buffer_size bytes is not supported"));
310304
}
311305
*/

ports/nrf/common-hal/busio/UART.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ typedef struct {
4444
ringbuf_t ringbuf;
4545
uint8_t rx_char; // EasyDMA buf
4646
bool rx_paused; // set by irq if no space in rbuf
47-
bool allocated_ringbuf;
4847

4948
uint8_t tx_pin_number;
5049
uint8_t rx_pin_number;

ports/raspberrypi/common-hal/busio/UART.c

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -155,27 +155,33 @@ void common_hal_busio_uart_construct(busio_uart_obj_t *self,
155155
uart_set_hw_flow(self->uart, (cts != NULL), (rts != NULL));
156156

157157
if (rx != NULL) {
158-
// Initially allocate the UART's buffer in the long-lived part of the
159-
// heap. UARTs are generally long-lived objects, but the "make long-
160-
// lived" machinery is incapable of moving internal pointers like
161-
// self->buffer, so do it manually. (However, as long as internal
162-
// pointers like this are NOT moved, allocating the buffer
163-
// in the long-lived pool is not strictly necessary)
164-
// (This is a macro.)
165-
if (!ringbuf_alloc(&self->ringbuf, receiver_buffer_size, true)) {
166-
m_malloc_fail(receiver_buffer_size);
167-
}
168-
active_uarts[uart_id] = self;
169-
if (uart_id == 1) {
170-
self->uart_irq_id = UART1_IRQ;
171-
irq_set_exclusive_handler(self->uart_irq_id, uart1_callback);
158+
// Use the provided buffer when given.
159+
if (receiver_buffer != NULL) {
160+
ringbuf_init(&self->ringbuf, receiver_buffer, receiver_buffer_size);
172161
} else {
173-
self->uart_irq_id = UART0_IRQ;
174-
irq_set_exclusive_handler(self->uart_irq_id, uart0_callback);
162+
// Initially allocate the UART's buffer in the long-lived part of the
163+
// heap. UARTs are generally long-lived objects, but the "make long-
164+
// lived" machinery is incapable of moving internal pointers like
165+
// self->buffer, so do it manually. (However, as long as internal
166+
// pointers like this are NOT moved, allocating the buffer
167+
// in the long-lived pool is not strictly necessary)
168+
if (!ringbuf_alloc(&self->ringbuf, receiver_buffer_size, true)) {
169+
uart_deinit(self->uart);
170+
m_malloc_fail(receiver_buffer_size);
171+
}
175172
}
176-
irq_set_enabled(self->uart_irq_id, true);
177-
uart_set_irq_enables(self->uart, true /* rx has data */, false /* tx needs data */);
178173
}
174+
175+
active_uarts[uart_id] = self;
176+
if (uart_id == 1) {
177+
self->uart_irq_id = UART1_IRQ;
178+
irq_set_exclusive_handler(self->uart_irq_id, uart1_callback);
179+
} else {
180+
self->uart_irq_id = UART0_IRQ;
181+
irq_set_exclusive_handler(self->uart_irq_id, uart0_callback);
182+
}
183+
irq_set_enabled(self->uart_irq_id, true);
184+
uart_set_irq_enables(self->uart, true /* rx has data */, false /* tx needs data */);
179185
}
180186

181187
bool common_hal_busio_uart_deinited(busio_uart_obj_t *self) {
@@ -187,7 +193,7 @@ void common_hal_busio_uart_deinit(busio_uart_obj_t *self) {
187193
return;
188194
}
189195
uart_deinit(self->uart);
190-
ringbuf_free(&self->ringbuf);
196+
ringbuf_deinit(&self->ringbuf);
191197
active_uarts[self->uart_id] = NULL;
192198
uart_status[self->uart_id] = STATUS_FREE;
193199
reset_pin_number(self->tx_pin);

ports/stm/common-hal/busio/UART.c

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -214,9 +214,16 @@ void common_hal_busio_uart_construct(busio_uart_obj_t *self,
214214

215215
// Init buffer for rx and claim pins
216216
if (self->rx != NULL) {
217+
// Use the provided buffer when given.
217218
if (receiver_buffer != NULL) {
218-
self->ringbuf = (ringbuf_t) { receiver_buffer, receiver_buffer_size };
219+
ringbuf_init(&self->ringbuf, receiver_buffer, receiver_buffer_size);
219220
} else {
221+
// Initially allocate the UART's buffer in the long-lived part of the
222+
// heap. UARTs are generally long-lived objects, but the "make long-
223+
// lived" machinery is incapable of moving internal pointers like
224+
// self->buffer, so do it manually. (However, as long as internal
225+
// pointers like this are NOT moved, allocating the buffer
226+
// in the long-lived pool is not strictly necessary)
220227
if (!ringbuf_alloc(&self->ringbuf, receiver_buffer_size, true)) {
221228
m_malloc_fail(receiver_buffer_size);
222229
}
@@ -281,7 +288,7 @@ void common_hal_busio_uart_deinit(busio_uart_obj_t *self) {
281288
self->rx = NULL;
282289
}
283290

284-
ringbuf_free(&self->ringbuf);
291+
ringbuf_deinit(&self->ringbuf);
285292
}
286293

287294
size_t common_hal_busio_uart_read(busio_uart_obj_t *self, uint8_t *data, size_t len, int *errcode) {

ports/unix/coverage.c

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -526,7 +526,9 @@ STATIC mp_obj_t extra_coverage(void) {
526526

527527
// ringbuf
528528
{
529-
byte buf[100];
529+
#define RINGBUF_SIZE 99
530+
531+
byte buf[RINGBUF_SIZE];
530532
ringbuf_t ringbuf;
531533
ringbuf_init(&ringbuf, &buf[0], sizeof(buf));
532534

@@ -546,7 +548,7 @@ STATIC mp_obj_t extra_coverage(void) {
546548
mp_printf(&mp_plat_print, "%d %d\n", ringbuf_num_empty(&ringbuf), ringbuf_num_filled(&ringbuf));
547549

548550
// Two-byte put with full ringbuf.
549-
for (int i = 0; i < 99; ++i) {
551+
for (int i = 0; i < RINGBUF_SIZE; ++i) {
550552
ringbuf_put(&ringbuf, i);
551553
}
552554
mp_printf(&mp_plat_print, "%d %d\n", ringbuf_num_empty(&ringbuf), ringbuf_num_filled(&ringbuf));
@@ -558,40 +560,36 @@ STATIC mp_obj_t extra_coverage(void) {
558560
ringbuf_get(&ringbuf);
559561
mp_printf(&mp_plat_print, "%d %d\n", ringbuf_num_empty(&ringbuf), ringbuf_num_filled(&ringbuf));
560562
mp_printf(&mp_plat_print, "%d\n", ringbuf_put16(&ringbuf, 0xcc99));
561-
for (int i = 0; i < 97; ++i) {
563+
for (int i = 0; i < RINGBUF_SIZE - 2; ++i) {
562564
ringbuf_get(&ringbuf);
563565
}
564566
mp_printf(&mp_plat_print, "%04x\n", ringbuf_get16(&ringbuf));
565567
mp_printf(&mp_plat_print, "%d %d\n", ringbuf_num_empty(&ringbuf), ringbuf_num_filled(&ringbuf));
566568

567569
// Two-byte put with wrap around on first byte:
568-
ringbuf.iput = 0;
569-
ringbuf.iget = 0;
570-
for (int i = 0; i < 99; ++i) {
570+
ringbuf_clear(&ringbuf);
571+
for (int i = 0; i < RINGBUF_SIZE; ++i) {
571572
ringbuf_put(&ringbuf, i);
572573
ringbuf_get(&ringbuf);
573574
}
574575
mp_printf(&mp_plat_print, "%d\n", ringbuf_put16(&ringbuf, 0x11bb));
575576
mp_printf(&mp_plat_print, "%04x\n", ringbuf_get16(&ringbuf));
576577

577578
// Two-byte put with wrap around on second byte:
578-
ringbuf.iput = 0;
579-
ringbuf.iget = 0;
580-
for (int i = 0; i < 98; ++i) {
579+
ringbuf_clear(&ringbuf);
580+
for (int i = 0; i < RINGBUF_SIZE - 1; ++i) {
581581
ringbuf_put(&ringbuf, i);
582582
ringbuf_get(&ringbuf);
583583
}
584584
mp_printf(&mp_plat_print, "%d\n", ringbuf_put16(&ringbuf, 0x22ff));
585585
mp_printf(&mp_plat_print, "%04x\n", ringbuf_get16(&ringbuf));
586586

587587
// Two-byte get from empty ringbuf.
588-
ringbuf.iput = 0;
589-
ringbuf.iget = 0;
588+
ringbuf_clear(&ringbuf);
590589
mp_printf(&mp_plat_print, "%d\n", ringbuf_get16(&ringbuf));
591590

592591
// Two-byte get from ringbuf with one byte available.
593-
ringbuf.iput = 0;
594-
ringbuf.iget = 0;
592+
ringbuf_clear(&ringbuf);
595593
ringbuf_put(&ringbuf, 0xaa);
596594
mp_printf(&mp_plat_print, "%d\n", ringbuf_get16(&ringbuf));
597595
}

0 commit comments

Comments
 (0)
0