further ringbuf cleanup

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

@@ -206,6 +206,8 @@ void common_hal_busio_uart_construct(busio_uart_obj_t *self,
         }
 
         LPUART_TransferCreateHandle(self->uart, &self->handle, LPUART_UserCallback, self);
+        // Pass actual allocated size; the LPUART routines are cognizant that
+        // the capacity is one less than the size.
         LPUART_TransferStartRingBuffer(self->uart, &self->handle, self->rbuf.buf, self->rbuf.size);
 
         claim_pin(self->rx_pin->pin);

ports/nrf/boards/circuitplayground_bluefruit/mpconfigboard.mk

@@ -11,7 +11,6 @@ EXTERNAL_FLASH_DEVICES = "GD25Q16C"
 
 # Allocate two, not just one I2C peripheral for CPB, so that we have both
 # on-board and off-board I2C available.
-# When SPIM3 becomes available we'll be able to have two I2C and two SPI peripherals.
 # We use a CFLAGS define here because there are include order issues
 # if we try to include "mpconfigport.h" into nrfx_config.h .
 CFLAGS += -DCIRCUITPY_NRF_NUM_I2C=2

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

@@ -42,13 +42,11 @@
 #include "supervisor/shared/tick.h"
 #include "common-hal/_bleio/CharacteristicBuffer.h"
 
+// Push all the data onto the ring buffer. When the buffer is full, new bytes will be dropped.
 STATIC void write_to_ringbuf(bleio_characteristic_buffer_obj_t *self, uint8_t *data, uint16_t len) {
-    // 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 < len; i++) {
-        ringbuf_put(&self->ringbuf, data[i]);
-    }
+    ringbuf_put_n(&self->ringbuf, data, len);
     sd_nvic_critical_region_exit(is_nested_critical_region);
 }
 
@@ -100,11 +98,11 @@ void common_hal_bleio_characteristic_buffer_construct(bleio_characteristic_buffe
 
 }
 
-int common_hal_bleio_characteristic_buffer_read(bleio_characteristic_buffer_obj_t *self, uint8_t *data, size_t len, int *errcode) {
+uint32_t common_hal_bleio_characteristic_buffer_read(bleio_characteristic_buffer_obj_t *self, uint8_t *data, size_t len, int *errcode) {
     uint64_t start_ticks = supervisor_ticks_ms64();
 
     // Wait for all bytes received or timeout
-    while ( (ringbuf_count(&self->ringbuf) < len) && (supervisor_ticks_ms64() - start_ticks < self->timeout_ms) ) {
+    while ( (ringbuf_avail(&self->ringbuf) < len) && (supervisor_ticks_ms64() - start_ticks < self->timeout_ms) ) {
         RUN_BACKGROUND_TASKS;
         // Allow user to break out of a timeout with a KeyboardInterrupt.
         if ( mp_hal_is_interrupted() ) {
@@ -116,21 +114,18 @@ int common_hal_bleio_characteristic_buffer_read(bleio_characteristic_buffer_obj_
     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);
-    }
+    uint32_t num_bytes_read = ringbuf_get_n(&self->ringbuf, data, len);
 
     // Writes now OK.
     sd_nvic_critical_region_exit(is_nested_critical_region);
 
-    return rx_bytes;
+    return num_bytes_read;
 }
 
 uint32_t common_hal_bleio_characteristic_buffer_rx_characters_available(bleio_characteristic_buffer_obj_t *self) {
     uint8_t is_nested_critical_region;
     sd_nvic_critical_region_enter(&is_nested_critical_region);
-    uint16_t count = ringbuf_count(&self->ringbuf);
+    uint16_t count = ringbuf_avail(&self->ringbuf);
     sd_nvic_critical_region_exit(is_nested_critical_region);
     return count;
 }

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

@@ -43,7 +43,7 @@
 #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) > self->ringbuf.size) {
+    if (len + sizeof(uint16_t) > ringbuf_capacity(&self->ringbuf)) {
         // This shouldn't happen.
         return;
     }
@@ -51,7 +51,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 (self->ringbuf.size - ringbuf_count(&self->ringbuf) < (int) (len + sizeof(uint16_t))) {
+    while (ringbuf_capacity(&self->ringbuf) - ringbuf_avail(&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++) {
@@ -250,27 +250,33 @@ void common_hal_bleio_packet_buffer_construct(
 }
 
 int common_hal_bleio_packet_buffer_readinto(bleio_packet_buffer_obj_t *self, uint8_t *data, size_t len) {
-    if (ringbuf_count(&self->ringbuf) < 2) {
+    if (ringbuf_avail(&self->ringbuf) < 2) {
         return 0;
     }
 
     uint16_t packet_length;
-    ringbuf_get_n(&self->ringbuf, (uint8_t*) &packet_length, sizeof(uint16_t));
+    int ret;
 
     // 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);
 
-    if (packet_length > len) {
-        return len - packet_length;
-    }
+    // Get packet length first.
+    ringbuf_get_n(&self->ringbuf, (uint8_t*) &packet_length, sizeof(uint16_t));
 
-    ringbuf_get_n(&self->ringbuf, data, packet_length);
+    if (packet_length > len) {
+        // Packet is longer than requested. Return negative of overrun value.
+        ret = len - packet_length;
+    } else {
+        // Read as much as possible, but might be shorter than len.
+        ringbuf_get_n(&self->ringbuf, data, packet_length);
+        ret = packet_length;
+    }
 
     // Writes now OK.
     sd_nvic_critical_region_exit(is_nested_critical_region);
 
-    return packet_length;
+    return ret;
 }
 
 void common_hal_bleio_packet_buffer_write(bleio_packet_buffer_obj_t *self, uint8_t *data, size_t len, uint8_t* header, size_t header_len) {

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

@@ -240,11 +240,10 @@ size_t common_hal_busio_uart_read(busio_uart_obj_t *self, uint8_t *data, size_t
         mp_raise_ValueError(translate("No RX pin"));
     }
 
-    size_t rx_bytes = 0;
     uint64_t start_ticks = supervisor_ticks_ms64();
 
     // Wait for all bytes received or timeout
-    while ( (ringbuf_count(&self->rbuf) < len) && (supervisor_ticks_ms64() - start_ticks < self->timeout_ms) ) {
+    while ( (ringbuf_avail(&self->rbuf) < len) && (supervisor_ticks_ms64() - start_ticks < self->timeout_ms) ) {
         RUN_BACKGROUND_TASKS;
         // Allow user to break out of a timeout with a KeyboardInterrupt.
         if ( mp_hal_is_interrupted() ) {
@@ -255,12 +254,8 @@ size_t common_hal_busio_uart_read(busio_uart_obj_t *self, uint8_t *data, size_t
     // prevent conflict with uart irq
     NVIC_DisableIRQ(nrfx_get_irq_number(self->uarte->p_reg));
 
-    // copy received data
-    rx_bytes = ringbuf_count(&self->rbuf);
-    rx_bytes = MIN(rx_bytes, len);
-    for ( uint16_t i = 0; i < rx_bytes; i++ ) {
-        data[i] = ringbuf_get(&self->rbuf);
-    }
+    // Copy as much received data as available, up to len bytes.
+    size_t rx_bytes = ringbuf_get_n(&self->rbuf, data, len);
 
     NVIC_EnableIRQ(nrfx_get_irq_number(self->uarte->p_reg));
 
@@ -317,7 +312,7 @@ void common_hal_busio_uart_set_timeout(busio_uart_obj_t *self, mp_float_t timeou
 }
 
 uint32_t common_hal_busio_uart_rx_characters_available(busio_uart_obj_t *self) {
-    return ringbuf_count(&self->rbuf);
+    return ringbuf_avail(&self->rbuf);
 }
 
 void common_hal_busio_uart_clear_rx_buffer(busio_uart_obj_t *self) {

ports/nrf/mpconfigport.mk

@@ -24,7 +24,7 @@ CIRCUITPY_AUDIOIO = 0
 
 # The ifndef's allow overriding in mpconfigboard.mk.
 
-ifndef
+ifndef CIRCUITPY_BLEIO
 CIRCUITPY_BLEIO = 1
 endif
 
@@ -51,8 +51,17 @@ endif
 # frequencyio not yet implemented
 CIRCUITPY_FREQUENCYIO = 0
 
+ifndef CIRCUITPY_PROTOMATTER
 CIRCUITPY_PROTOMATTER = 1
+endif
+
+ifndef CIRCUITPY_FRAMEBUFFERIO
 CIRCUITPY_FRAMEBUFFERIO = 1
+endif
+
+ifndef CIRCUITPY_ULAB
+CIRCUITPY_ULAB = 1
+endif
 
 # nRF52840-specific
 
@@ -72,5 +81,4 @@ NRF_DEFINES += -DNRF52840_XXAA -DNRF52840
 # Defined here because system_nrf52840.c doesn't #include any of our own include files.
 CFLAGS += -DCONFIG_NFCT_PINS_AS_GPIOS
 
-CIRCUITPY_ULAB = 1
 endif

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

@@ -262,7 +262,7 @@ size_t common_hal_busio_uart_read(busio_uart_obj_t *self, uint8_t *data, size_t
     uint64_t start_ticks = supervisor_ticks_ms64();
 
     // Wait for all bytes received or timeout, same as nrf
-    while ( (ringbuf_count(&self->rbuf) < len) && (supervisor_ticks_ms64() - start_ticks < self->timeout_ms) ) {
+    while ( (ringbuf_avail(&self->rbuf) < len) && (supervisor_ticks_ms64() - start_ticks < self->timeout_ms) ) {
         RUN_BACKGROUND_TASKS;
         //restart if it failed in the callback
         if (errflag != HAL_OK) {
@@ -276,12 +276,8 @@ size_t common_hal_busio_uart_read(busio_uart_obj_t *self, uint8_t *data, size_t
 
     // Halt reception
     HAL_NVIC_DisableIRQ(self->irq);
-    // copy received data
-    rx_bytes = ringbuf_count(&self->rbuf);
-    rx_bytes = MIN(rx_bytes, len);
-    for (uint16_t i = 0; i < rx_bytes; i++) {
-        data[i] = ringbuf_get(&self->rbuf);
-    }
+    // Copy as much received data as available, up to len bytes.
+    size_t rx_bytes = ringbuf_get_n(&self->rbuf, data, len);
     HAL_NVIC_EnableIRQ(self->irq);
 
     if (rx_bytes == 0) {
@@ -380,7 +376,7 @@ void common_hal_busio_uart_set_timeout(busio_uart_obj_t *self, mp_float_t timeou
 }
 
 uint32_t common_hal_busio_uart_rx_characters_available(busio_uart_obj_t *self) {
-    return ringbuf_count(&self->rbuf);
+    return ringbuf_avail(&self->rbuf);
 }
 
 void common_hal_busio_uart_clear_rx_buffer(busio_uart_obj_t *self) {

py/py.mk

@@ -257,6 +257,7 @@ PY_CORE_O_BASENAME = $(addprefix py/,\
 	repl.o \
 	smallint.o \
 	frozenmod.o \
+	ringbuf.o \
 	)
 
 PY_EXTMOD_O_BASENAME = \

py/ringbuf.c

@@ -0,0 +1,109 @@
+/*
+ * This file is part of the MicroPython project, http://micropython.org/
+ *
+ * The MIT License (MIT)
+ *
+ * Copyright (c) 2016 Paul Sokolovsky
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "ringbuf.h"
+
+// Dynamic initialization. This should be accessible from a root pointer.
+// capacity is the number of bytes the ring buffer can hold. The actual
+// size of the buffer is one greater than that, due to how the buffer
+// handles empty and full statuses.
+bool ringbuf_alloc(ringbuf_t *r, size_t capacity, bool long_lived) {
+    r->buf = gc_alloc(capacity + 1, false, long_lived);
+    r->size = capacity +  1;
+    r->iget = r->iput = 0;
+    return r->buf != NULL;
+}
+
+size_t ringbuf_capacity(ringbuf_t *r) {
+    return r->size - 1;
+}
+
+// Returns -1 if buffer is empty, else returns byte fetched.
+int ringbuf_get(ringbuf_t *r) {
+    if (r->iget == r->iput) {
+        return -1;
+    }
+    uint8_t v = r->buf[r->iget++];
+    if (r->iget >= r->size) {
+        r->iget = 0;
+    }
+    return v;
+}
+
+// Returns -1 if no room in buffer, else returns 0.
+int ringbuf_put(ringbuf_t *r, uint8_t v) {
+    uint32_t iput_new = r->iput + 1;
+    if (iput_new >= r->size) {
+        iput_new = 0;
+    }
+    if (iput_new == r->iget) {
+        return -1;
+    }
+    r->buf[r->iput] = v;
+    r->iput = iput_new;
+    return 0;
+}
+
+void ringbuf_clear(ringbuf_t *r) {
+    r->iput = r->iget = 0;
+}
+
+// Number of free slots that can be written.
+size_t ringbuf_free(ringbuf_t *r) {
+    return (r->size + r->iget - r->iput - 1) % r->size;
+}
+
+// Number of bytes available to read.
+size_t ringbuf_avail(ringbuf_t *r) {
+    return (r->size + r->iput - r->iget) % r->size;
+}
+
+// If the ring buffer fills up, not all bytes will be written.
+// Returns how many bytes were successfully written.
+size_t ringbuf_put_n(ringbuf_t* r, uint8_t* buf, size_t bufsize)
+{
+    for(size_t i=0; i < bufsize; i++) {
+        if ( ringbuf_put(r, buf[i]) < 0 ) {
+            // If ringbuf is full, give up and return how many bytes
+            // we wrote so far.
+            return i;
+        }
+    }
+    return bufsize;
+}
+
+// Returns how many bytes were fetched.
+size_t ringbuf_get_n(ringbuf_t* r, uint8_t* buf, size_t bufsize)
+{
+    for(size_t i=0; i < bufsize; i++) {
+        int b = ringbuf_get(r);
+        if (b < 0) {
+            return i;
+        }
+        buf[i] = b;
+    }
+    return bufsize;
+}

py/ringbuf.h

@@ -32,78 +32,26 @@
 
 typedef struct _ringbuf_t {
     uint8_t *buf;
-    uint16_t size;
-    uint16_t iget;
-    uint16_t iput;
+    // Allocated size; capacity is one less. Don't reference this directly.
+    uint32_t size;
+    uint32_t iget;
+    uint32_t iput;
 } ringbuf_t;
 
+// Note that the capacity of the buffer is N-1!
+
 // Static initialization:
 // byte buf_array[N];
 // ringbuf_t buf = {buf_array, sizeof(buf_array)};
 
-// Dynamic initialization. This creates root pointer!
-#define ringbuf_alloc(r, sz, long_lived)                   \
-{ \
-    (r)->buf = gc_alloc(sz, false, long_lived);   \
-    (r)->size = sz; \
-    (r)->iget = (r)->iput = 0; \
-}
+bool ringbuf_alloc(ringbuf_t *r, size_t capacity, bool long_lived);
+size_t ringbuf_capacity(ringbuf_t *r);
+int ringbuf_get(ringbuf_t *r);
+int ringbuf_put(ringbuf_t *r, uint8_t v);
+void ringbuf_clear(ringbuf_t *r);
+size_t ringbuf_free(ringbuf_t *r);
+size_t ringbuf_avail(ringbuf_t *r);
+size_t ringbuf_put_n(ringbuf_t* r, uint8_t* buf, size_t bufsize);
+size_t ringbuf_get_n(ringbuf_t* r, uint8_t* buf, size_t bufsize);
 
-static inline int ringbuf_get(ringbuf_t *r) {
-    if (r->iget == r->iput) {
-        return -1;
-    }
-    uint8_t v = r->buf[r->iget++];
-    if (r->iget >= r->size) {
-        r->iget = 0;
-    }
-    return v;
-}
-
-static inline int ringbuf_put(ringbuf_t *r, uint8_t v) {
-    uint32_t iput_new = r->iput + 1;
-    if (iput_new >= r->size) {
-        iput_new = 0;
-    }
-    if (iput_new == r->iget) {
-        return -1;
-    }
-    r->buf[r->iput] = v;
-    r->iput = iput_new;
-    return 0;
-}
-
-static inline uint16_t ringbuf_count(ringbuf_t *r)
-{
-    volatile int count = r->iput - r->iget;
-    if ( count < 0 ) {
-        count += r->size;
-    }
-
-    return (uint16_t) count;
-}
-
-static inline void ringbuf_clear(ringbuf_t *r)
-{
-    r->iput = r->iget = 0;
-}
-
-// will overwrite old data
-static inline void ringbuf_put_n(ringbuf_t* r, uint8_t* buf, uint8_t bufsize)
-{
-    for(uint8_t i=0; i < bufsize; i++) {
-        if ( ringbuf_put(r, buf[i]) < 0 ) {
-            // if full overwrite old data
-            (void) ringbuf_get(r);
-            ringbuf_put(r, buf[i]);
-        }
-    }
-}
-
-static inline void ringbuf_get_n(ringbuf_t* r, uint8_t* buf, uint8_t bufsize)
-{
-    for(uint8_t i=0; i < bufsize; i++) {
-        buf[i] = ringbuf_get(r);
-    }
-}
 #endif // MICROPY_INCLUDED_PY_RINGBUF_H

shared-bindings/_bleio/CharacteristicBuffer.h

@@ -32,7 +32,7 @@
 extern const mp_obj_type_t bleio_characteristic_buffer_type;
 
 extern void common_hal_bleio_characteristic_buffer_construct(bleio_characteristic_buffer_obj_t *self, bleio_characteristic_obj_t *characteristic, mp_float_t timeout, size_t buffer_size);
-int common_hal_bleio_characteristic_buffer_read(bleio_characteristic_buffer_obj_t *self, uint8_t *data, size_t len, int *errcode);
+uint32_t common_hal_bleio_characteristic_buffer_read(bleio_characteristic_buffer_obj_t *self, uint8_t *data, size_t len, int *errcode);
 uint32_t common_hal_bleio_characteristic_buffer_rx_characters_available(bleio_characteristic_buffer_obj_t *self);
 void common_hal_bleio_characteristic_buffer_clear_rx_buffer(bleio_characteristic_buffer_obj_t *self);
 bool common_hal_bleio_characteristic_buffer_deinited(bleio_characteristic_buffer_obj_t *self);

shared-module/_bleio/ScanResults.c

@@ -45,10 +45,10 @@ bleio_scanresults_obj_t* shared_module_bleio_new_scanresults(size_t buffer_size,
 }
 
 mp_obj_t common_hal_bleio_scanresults_next(bleio_scanresults_obj_t *self) {
-    while (ringbuf_count(&self->buf) == 0 && !self->done && !mp_hal_is_interrupted()) {
+    while (ringbuf_avail(&self->buf) == 0 && !self->done && !mp_hal_is_interrupted()) {
         RUN_BACKGROUND_TASKS;
     }
-    if (ringbuf_count(&self->buf) == 0 || mp_hal_is_interrupted()) {
+    if (ringbuf_avail(&self->buf) == 0 || mp_hal_is_interrupted()) {
         return mp_const_none;
     }
 
@@ -97,7 +97,7 @@ void shared_module_bleio_scanresults_append(bleio_scanresults_obj_t* self,
                                             uint16_t len) {
     int32_t packet_size = sizeof(uint8_t) + sizeof(ticks_ms) + sizeof(rssi) + NUM_BLEIO_ADDRESS_BYTES +
         sizeof(addr_type) + sizeof(len) + len;
-    int32_t empty_space = self->buf.size - ringbuf_count(&self->buf);
+    int32_t empty_space = self->buf.size - ringbuf_avail(&self->buf);
     if (packet_size >= empty_space) {
         // We can't fit the packet so skip it.
         return;