virtio_buffer: manage buffer rollover

Author: ABOSTM

cores/arduino/VirtIOSerial.cpp

@@ -113,17 +113,15 @@ int VirtIOSerial::read(void)
 
 size_t VirtIOSerial::readBytes(char *buffer, size_t length)
 {
-  const size_t size = length;
-  _startMillis = millis();
-  while (length > 0 && (millis() - _startMillis < _timeout)) {
-    uint16_t prev_write_available = virtio_buffer_write_available(&ring);
-    length -= virtio_buffer_read(&ring, reinterpret_cast<uint8_t *>(buffer), length);
-    if (prev_write_available < RPMSG_BUFFER_SIZE
-        && virtio_buffer_write_available(&ring) >= RPMSG_BUFFER_SIZE) {
-      MAILBOX_Notify_Rx_Buf_Free();
-    }
+  uint16_t prev_write_available = virtio_buffer_write_available(&_VirtIOSerialObj.ring);
+  const size_t size = virtio_buffer_read(&_VirtIOSerialObj.ring, reinterpret_cast<uint8_t *>(buffer), length);
+
+  if (prev_write_available < RPMSG_BUFFER_SIZE
+      && virtio_buffer_write_available(&_VirtIOSerialObj.ring) >= RPMSG_BUFFER_SIZE) {
+    MAILBOX_Notify_Rx_Buf_Free();
   }
-  return size - length;
+
+  return size;
 }
 
 size_t VirtIOSerial::write(uint8_t ch)

cores/arduino/stm32/virtio/virtio_buffer.c

@@ -27,85 +27,102 @@
 #include <string.h>
 #include "wiring.h"
 
-#define BUFFER_END (VIRTIO_BUFFER_SIZE - 1)
-
 void virtio_buffer_init(virtio_buffer_t *ring)
 {
-  ring->write = 0;
-  ring->read = 0;
+  ring->write_index = 0;
+  ring->read_index = 0;
 }
 
 uint16_t virtio_buffer_read_available(virtio_buffer_t *ring)
 {
-  // This will make the function safe when write operations are done in interrupts
-  volatile uint16_t write = ring->write;
+  int32_t delta = ring->write_index - ring->read_index;
 
-  if (write < ring->read) {
-    return (BUFFER_END - ring->read) + (write + 1);
+  if (delta < 0) {
+    return (VIRTIO_BUFFER_SIZE + delta);
   }
-  return write - ring->read;
+  return delta;
 }
 
+/* WARNING no protection against race codition (on ring->read_index) when used in interruption */
 static uint16_t read(virtio_buffer_t *ring, uint8_t *dst, uint16_t size, bool peek)
 {
-  // This will make the function safe when write operations are done in interrupts
-  volatile uint16_t write = ring->write;
-  uint16_t end = (write >= ring->read) ? write : BUFFER_END + 1;
+  uint16_t read_index = ring->read_index;
+  int32_t delta = ring->write_index - read_index;
+  if (delta < 0) {
+    delta += VIRTIO_BUFFER_SIZE;
+  }
+
+  size = min(size, delta);
+
+  if ((size + read_index) > VIRTIO_BUFFER_SIZE) {
+    // Manage ring buffer rollover
+    // First, copy ring buffer from read index to end of buffer
+    memcpy(dst, ring->buffer + read_index, VIRTIO_BUFFER_SIZE - read_index);
+    // then, copy ring buffer from begining of buffer to end of read
+    memcpy(dst + VIRTIO_BUFFER_SIZE - read_index, ring->buffer, size - (VIRTIO_BUFFER_SIZE - read_index));
+  } else {
+    memcpy(dst, ring->buffer + read_index, size);
+  }
 
-  size = min(end - ring->read, size);
-  memcpy(dst, ring->buffer + ring->read, size);
+  // Update read index if not peeked
   if (!peek) {
-    ring->read += size;
+    ring->read_index += size;
 
-    if (ring->read > BUFFER_END) {
-      ring->read = 0;
+    // Manage ring buffer rollover
+    if (ring->read_index >= VIRTIO_BUFFER_SIZE) {
+      ring->read_index -= VIRTIO_BUFFER_SIZE;
     }
   }
   return size;
 }
 
 uint16_t virtio_buffer_read(virtio_buffer_t *ring, uint8_t *dst, uint16_t size)
 {
-  uint16_t recv_size = read(ring, dst, size, false);
-  return recv_size;
+  return read(ring, dst, size, false);
 }
 
-/**
- * WARNING: The size of read cannot be larger than virtio_buffer_read_available().
- */
+
 uint16_t virtio_buffer_peek(virtio_buffer_t *ring, uint8_t *dst, uint16_t size)
 {
-  size = min(size, virtio_buffer_read_available(ring));
-  uint16_t recv_size = 0;
-  while (recv_size < size) {
-    recv_size += read(ring, dst + recv_size, size - recv_size, true);
-  }
-  return recv_size;
+  return read(ring, dst, size, true);
 }
 
+
+
 uint16_t virtio_buffer_write_available(virtio_buffer_t *ring)
 {
-  // This will make the function safe when read operations are done in interrupts
-  volatile uint16_t read = ring->read;
+  int32_t delta = ring->read_index - ring->write_index - 1;
 
-  if (ring->write < read) {
-    return (read - 1) - ring->write;
+  if (delta < 0) {
+    return (VIRTIO_BUFFER_SIZE + delta);
   }
-  return read + (BUFFER_END - ring->write);
+  return delta;
 }
 
+/* WARNING no protection against race codition (on ring->write_index) when used in interruption */
 uint16_t virtio_buffer_write(virtio_buffer_t *ring, uint8_t *src, uint16_t size)
 {
-  // This will make the function safe when read operations are done in a interrupt
-  volatile uint16_t read = ring->read;
-  uint16_t end = (ring->write < read) ? read - 1
-                 : (read == 0) ? BUFFER_END : BUFFER_END + 1;
-
-  size = min(end - ring->write, size);
-  memcpy(ring->buffer + ring->write, src, size);
-  ring->write += size;
-  if (ring->write > BUFFER_END) {
-    ring->write = 0;
+  uint16_t write_index = ring->write_index;
+  int32_t delta = ring->read_index - write_index - 1;
+  if (delta < 0) {
+    delta += VIRTIO_BUFFER_SIZE;
+  }
+
+  size = min(size, delta);
+
+  if ((size + write_index) > VIRTIO_BUFFER_SIZE) {
+    // Manage ring buffer rollover
+    // First, write ring buffer from write index to end of buffer
+    memcpy(ring->buffer + write_index, src, VIRTIO_BUFFER_SIZE - write_index);
+    // then,  write ring buffer from beginning of buffer to end of read
+    memcpy(ring->buffer, src + VIRTIO_BUFFER_SIZE - write_index, size - (VIRTIO_BUFFER_SIZE - write_index));
+  } else {
+    memcpy(ring->buffer + write_index, src, size);
+  }
+
+  ring->write_index += size;
+  if (ring->write_index >= VIRTIO_BUFFER_SIZE) {
+    ring->write_index -= VIRTIO_BUFFER_SIZE;
   }
   return size;
 }

cores/arduino/stm32/virtio/virtio_buffer.h

@@ -34,8 +34,8 @@ extern "C" {
 
 typedef struct {
   uint8_t buffer[VIRTIO_BUFFER_SIZE];
-  volatile uint16_t write;
-  volatile uint16_t read;
+  volatile uint16_t write_index;
+  volatile uint16_t read_index;
 } virtio_buffer_t;
 
 void virtio_buffer_init(virtio_buffer_t *ring);