Merge pull request #1101 from Kijewski/issue-979-2

ringbuffer: versatility and optimizations

sys/include/ringbuffer.h

@@ -12,24 +12,114 @@
  * @{
  * @file   ringbuffer.h
  * @author Kaspar Schleiser <kaspar@schleiser.de>
+ * @author René Kijewski <rene.kijewski@fu-berlin.de>
  * @}
  */
 
 #ifndef __RINGBUFFER_H
 #define __RINGBUFFER_H
 
+/**
+ * @brief     Ringbuffer.
+ * @details   Non thread-safe FIFO ringbuffer implementation around a `char` array.
+ */
 typedef struct ringbuffer {
-    char *buf;
-    unsigned int        start;
-    unsigned int        end;
-    unsigned int        size;
-    unsigned int        avail;
+    char *buf;          /**< Buffer to operate on. */
+    unsigned int size;  /**< Size of buf. */
+    unsigned int start; /**< Current read position in the ring buffer. */
+    unsigned int avail; /**< Number of elements available for reading. */
 } ringbuffer_t;
 
-void ringbuffer_init(ringbuffer_t *rb, char *buffer, unsigned int bufsize);
-void ringbuffer_add_one(ringbuffer_t *rb, char c);
-void ringbuffer_add(ringbuffer_t *rb, char *buf, int n);
-int ringbuffer_get_one(ringbuffer_t *rb);
-int ringbuffer_get(ringbuffer_t *rb, char *buf, int n);
+/**
+ * @def          RINGBUFFER_INIT(BUF)
+ * @brief        Initialize a ringbuffer.
+ * @details      This macro is meant for static ringbuffers.
+ * @param[in]    BUF   Buffer to use for the ringbuffer. The size is deduced through `sizeof (BUF)`.
+ * @returns      The static initializer.
+ */
+#define RINGBUFFER_INIT(BUF) { (BUF), sizeof (BUF), 0, 0 }
+
+/**
+ * @brief        Initialize a ringbuffer.
+ * @param[out]   rb        Datum to initialize.
+ * @param[in]    buffer    Buffer to use by rb.
+ * @param[in]    bufsize   `sizeof (buffer)`
+ */
+void ringbuffer_init(ringbuffer_t *restrict rb, char *buffer, unsigned bufsize);
+
+/**
+ * @brief           Add an element to the ringbuffer.
+ * @details         If rb is full, then the oldest element gets overwritten.
+ *                  Test ringbuffer_full() first if overwriting is not intended.
+ * @param[in,out]   rb   Ringbuffer to operate on.
+ * @param[in]       c    Element to add.
+ * @returns         The element that was dropped, iff the buffer was full.
+ *                  -1 iff the buffer was not full.
+ */
+int ringbuffer_add_one(ringbuffer_t *restrict rb, char c);
+
+/**
+ * @brief           Add a number of elements to the ringbuffer.
+ * @details         Only so many elements are added as fit in the ringbuffer.
+ *                  No elements get overwritten.
+ *                  If this is not the intended behavior, then use ringbuffer_add_one() in a loop instead.
+ * @param[in,out]   rb    Ringbuffer to operate on.
+ * @param[in]       buf   Buffer to add elements from.
+ * @param[in]       n     Maximum number of elements to add.
+ * @returns         Number of elements actually added. 0 if rb is full.
+ */
+unsigned ringbuffer_add(ringbuffer_t *restrict rb, const char *buf, unsigned n);
+
+/**
+ * @brief           Peek and remove oldest element from the ringbuffer.
+ * @param[in,out]   rb   Ringbuffer to operate on.
+ * @returns         The oldest element that was added, or `-1` if rb is empty.
+ */
+int ringbuffer_get_one(ringbuffer_t *restrict rb);
+
+/**
+ * @brief           Read and remove a number of elements from the ringbuffer.
+ * @param[in,out]   rb    Ringbuffer to operate on.
+ * @param[out]      buf   Buffer to write into.
+ * @param[in]       n     Read at most n elements.
+ * @returns         Number of elements actually read.
+ */
+unsigned ringbuffer_get(ringbuffer_t *restrict rb, char *buf, unsigned n);
+
+/**
+ * @brief           Test if the ringbuffer is empty.
+ * @param[in,out]   rb    Ringbuffer to operate on.
+ * @returns         0 iff not empty
+ */
+static inline int ringbuffer_empty(const ringbuffer_t *restrict rb)
+{
+    return rb->avail == 0;
+}
+
+/**
+ * @brief           Test if the ringbuffer is full.
+ * @param[in,out]   rb    Ringbuffer to operate on.
+ * @returns         0 iff not full
+ */
+static inline int ringbuffer_full(const ringbuffer_t *restrict rb)
+{
+    return rb->avail == rb->size;
+}
+
+/**
+ * @brief           Read, but don't remove, the oldest element in the buffer.
+ * @param[in]       rb    Ringbuffer to operate on.
+ * @returns         Same as ringbuffer_get_one()
+ */
+int ringbuffer_peek_one(const ringbuffer_t *restrict rb);
+
+/**
+ * @brief           Read, but don't remove, the a number of element of the buffer.
+ * @param[in]       rb    Ringbuffer to operate on.
+ * @param[out]      buf   Buffer to write into.
+ * @param[in]       n     Read at most n elements.
+ * @returns         Same as ringbuffer_get()
+ */
+unsigned ringbuffer_peek(const ringbuffer_t *restrict rb, char *buf, unsigned n);
 
 #endif /* __RINGBUFFER_H */

sys/lib/ringbuffer.c

@@ -12,137 +12,104 @@
  * @{
  * @file   ringbuffer.c
  * @author Kaspar Schleiser <kaspar@schleiser.de>
+ * @author René Kijewski <rene.kijewski@fu-berlin.de>
  * @}
  */
 
-#include <stdio.h>
-#include <stdint.h>
-#include <string.h>
-
-#if (defined(__MACH__) || defined(__FreeBSD__))
-#include <stdlib.h>
-#else
-#include "malloc.h"
-#endif
-
-#define ENABLE_DEBUG (0)
-#include "debug.h"
-
 #include "ringbuffer.h"
 
-void ringbuffer_init(ringbuffer_t *rb, char *buffer, unsigned int bufsize)
+void ringbuffer_init(ringbuffer_t *restrict rb, char *buffer, unsigned bufsize)
 {
     rb->buf = buffer;
-    rb->start = 0;
-    rb->end = 0;
     rb->size = bufsize;
+    rb->start = 0;
     rb->avail = 0;
 }
 
-void ringbuffer_add(ringbuffer_t *rb, char *buf, int n)
+/**
+ * @brief           Add an element to the end of the ringbuffer.
+ * @details         This helper function does not check the pre-requirements for adding,
+ *                  i.e. the caller has to ensure that ringbuffer_full() is false.
+ * @param[in,out]   rb   Ringbuffer to operate on.
+ * @param[in]       c    Element to add.
+ */
+static void add_tail(ringbuffer_t *restrict rb, char c)
 {
-    for (int i = 0; i < n; i++) {
-        ringbuffer_add_one(rb, buf[i]);
+    unsigned pos = rb->start + rb->avail++;
+    if (pos >= rb->size) {
+        pos -= rb->size;
     }
+    rb->buf[pos] = c;
 }
 
-void ringbuffer_add_one(ringbuffer_t *rb, char c)
+/**
+ * @brief           Remove an element from the start of the ringbuffer.
+ * @details         This helper function does not check the pre-requirements for reading,
+ *                  i.e. the caller has to ensure that ringbuffer_empty() is false.
+ * @param[in,out]   rb   Ringbuffer to operate on.
+ * @returns         The removed element.
+ */
+static char get_head(ringbuffer_t *restrict rb)
 {
-    if (rb->avail == rb->size) {
-        ringbuffer_get_one(rb);
-    }
-
-    rb->buf[rb->end++] = c;
-
-    if (rb->end >= rb->size) {
-        rb->end = 0;
+    char result = rb->buf[rb->start];
+    if ((--rb->avail == 0) || (++rb->start == rb->size)) {
+        rb->start = 0;
     }
-
-    rb->avail++;
+    return result;
 }
 
-int ringbuffer_get_one(ringbuffer_t *rb)
+unsigned ringbuffer_add(ringbuffer_t *restrict rb, const char *buf, unsigned n)
 {
-    if (rb->avail == 0) {
-        return -1;
+    unsigned i;
+    for (i = 0; i < n; i++) {
+        if (ringbuffer_full(rb)) {
+            break;
+        }
+        add_tail(rb, buf[i]);
     }
+    return i;
+}
 
-    rb->avail--;
-
-    int c = (char)rb->buf[rb->start++];
-
-    if (rb->start >= rb->size) {
-        rb->start = 0;
+int ringbuffer_add_one(ringbuffer_t *restrict rb, char c)
+{
+    int result = -1;
+    if (ringbuffer_full(rb)) {
+        result = (unsigned char) get_head(rb);
     }
-
-    return c;
+    add_tail(rb, c);
+    return result;
 }
 
-int ringbuffer_get(ringbuffer_t *rb, char *buf, int n)
+int ringbuffer_get_one(ringbuffer_t *restrict rb)
 {
-    int count = 0;
-
-    while (rb->avail && (count < n)) {
-        buf[count++] = ringbuffer_get_one(rb);
+    if (!ringbuffer_empty(rb)) {
+        return (unsigned char) get_head(rb);
+    }
+    else {
+        return -1;
     }
-
-    return count;
 }
 
-/*
-int main(int argc, char *argv[] ){
-    ringbuffer r;
-    char buffer[5];
-    ringbuffer_init(&r, buffer, sizeof(buffer));
-
-    ringbuffer_add_one(&r, 1);
-    ringbuffer_add_one(&r, 2);
-    ringbuffer_add_one(&r, 3);
-    ringbuffer_add_one(&r, 4);
-    ringbuffer_add_one(&r, 5);
-    ringbuffer_add_one(&r, 6);
-    ringbuffer_add_one(&r, 7);
-    ringbuffer_add_one(&r, 8);
-    ringbuffer_add_one(&r, 9);
-    ringbuffer_add_one(&r, 10);
-
-    int c;
-    while ( r.avail ) {
-        c = ringbuffer_get_one(&r);
-        if (c == -1) break;
-        printf("c=%i\n", (int)c);
+unsigned ringbuffer_get(ringbuffer_t *restrict rb, char *buf, unsigned n)
+{
+    if (n > rb->avail) {
+        n = rb->avail;
     }
 
-    ringbuffer_add_one(&r, 1);
-    ringbuffer_add_one(&r, 2);
-    ringbuffer_add_one(&r, 3);
-    ringbuffer_add_one(&r, 4);
-    ringbuffer_add_one(&r, 5);
-
-    char buffer2[10];
-
-    int n = ringbuffer_get(&r, buffer2, sizeof(buffer2));
-
-    for (int i = 0; i < n; i++) {
-        printf("%i\n", buffer2[i]);
+    for (unsigned i = 0; i < n; ++i) {
+        buf[i] = get_head(rb);
     }
+    return n;
+}
 
-    ringbuffer_add_one(&r, 1);
-    ringbuffer_add_one(&r, 2);
-    ringbuffer_add_one(&r, 3);
-    ringbuffer_add_one(&r, 4);
-    ringbuffer_add_one(&r, 5);
-    ringbuffer_add_one(&r, 6);
-    ringbuffer_add_one(&r, 7);
-    ringbuffer_add_one(&r, 8);
-    ringbuffer_add_one(&r, 9);
-    ringbuffer_add_one(&r, 10);
-
-    while ( r.avail ) {
-        c = ringbuffer_get_one(&r);
-        if (c == -1) break;
-        printf("c=%i\n", (int)c);
-    }
+int ringbuffer_peek_one(const ringbuffer_t *restrict rb_)
+{
+    ringbuffer_t rb = *rb_;
+    return ringbuffer_get_one(&rb);
+}
 
-    return 0;
-}*/
+unsigned ringbuffer_peek(const ringbuffer_t *restrict rb_, char *buf, unsigned n)
+{
+    ringbuffer_t rb = *rb_;
+    return ringbuffer_get(&rb, buf, n);
+}

tests/unittests/tests-lib/Makefile

+include $(RIOTBASE)/Makefile.base

tests/unittests/tests-lib/Makefile.include

+USEMODULE += lib

tests/unittests/tests-lib/tests-lib-ringbuffer.c

+/*
+ * Copyright (C) 2014  René Kijewski  <rene.kijewski@fu-berlin.de>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#include "thread.h"
+#include "flags.h"
+#include "kernel.h"
+#include "ringbuffer.h"
+#include "mutex.h"
+
+#include "tests-lib.h"
+
+/* (ITERATIONS * (BUF_SIZE + 1)) needs to be <= 127! Otherwise `char` overflows. */
+#define ITERATIONS 15
+#define BUF_SIZE 7
+
+static char stack_get[KERNEL_CONF_STACKSIZE_DEFAULT];
+
+static char rb_buf[BUF_SIZE];
+static ringbuffer_t rb = RINGBUFFER_INIT(rb_buf);
+static mutex_t mutex;
+static unsigned pid_add, pid_get;
+
+static void assert_avail(unsigned assumed)
+{
+    TEST_ASSERT_EQUAL_INT(assumed, rb.avail);
+}
+
+static void assert_add_one(char to_add, int assumed_result)
+{
+    int actual_result = ringbuffer_add_one(&rb, to_add);
+    TEST_ASSERT_EQUAL_INT(assumed_result, actual_result);
+}
+
+static void assert_get_one(int assumed_result)
+{
+    int actual_result = ringbuffer_get_one(&rb);
+    TEST_ASSERT_EQUAL_INT(assumed_result, actual_result);
+}
+
+static void run_add(void)
+{
+    char next = 0;
+    for (unsigned iteration = 0; iteration < ITERATIONS; ++iteration) {
+        mutex_lock(&mutex);
+
+        for (unsigned i = 0; i < BUF_SIZE; ++i) {
+            assert_avail(i);
+            assert_add_one(next, -1);
+            assert_avail(i + 1);
+            ++next;
+        }
+
+        /* Overwrite oldest element. It should be returned to us. */
+        assert_avail(BUF_SIZE);
+        assert_add_one(next, next - BUF_SIZE);
+        assert_avail(BUF_SIZE);
+        ++next;
+
+        thread_wakeup(pid_get);
+        mutex_unlock_and_sleep(&mutex);
+    }
+
+    thread_wakeup(pid_get);
+}
+
+static void *run_get(void *arg)
+{
+    (void) arg;
+
+    char next = 0;
+    for (unsigned iteration = 0; iteration < ITERATIONS; ++iteration) {
+        ++next; /* the first element of a stride is always overwritten */
+
+        mutex_lock(&mutex);
+
+        for (unsigned i = BUF_SIZE; i > 0; --i) {
+            assert_avail(i);
+            assert_get_one(next);
+            assert_avail(i - 1);
+            ++next;
+        }
+
+        assert_avail(0);
+        assert_get_one(-1);
+        assert_avail(0);
+
+        thread_wakeup(pid_add);
+        mutex_unlock_and_sleep(&mutex);
+    }
+
+    return NULL;
+}
+
+static void tests_lib_ringbuffer(void)
+{
+    pid_add = sched_active_pid;
+    pid_get = thread_create(stack_get, sizeof (stack_get),
+                            PRIORITY_MAIN, CREATE_SLEEPING | CREATE_STACKTEST,
+                            run_get, NULL, "get");
+    run_add();
+}
+
+Test *tests_lib_ringbuffer_tests(void)
+{
+    EMB_UNIT_TESTFIXTURES(fixtures) {
+        new_TestFixture(tests_lib_ringbuffer),
+    };
+
+    EMB_UNIT_TESTCALLER(ringbuffer_tests, NULL, NULL, fixtures);
+
+    return (Test *)&ringbuffer_tests;
+}

tests/unittests/tests-lib/tests-lib.c

+/*
+ * Copyright (C) 2014 René Kijewski
+ *
+ * This file is subject to the terms and conditions of the GNU Lesser General
+ * Public License. See the file LICENSE in the top level directory for more
+ * details.
+ */
+
+#include "tests-lib.h"
+
+void tests_lib(void)
+{
+    TESTS_RUN(tests_lib_ringbuffer_tests());
+}

tests/unittests/tests-lib/tests-lib.h

+/*
+ * Copyright (C) 2014 René Kijewski
+ *
+ * This file is subject to the terms and conditions of the GNU Lesser General
+ * Public License. See the file LICENSE in the top level directory for more
+ * details.
+ */
+
+/**
+ * @addtogroup  unittests
+ * @{
+ *
+ * @file        tests-lib.h
+ * @brief       Unittests for the ``lib`` sysmodule
+ *
+ * @author      Freie Universität Berlin, Computer Systems & Telematics
+ * @author      René Kijewski <rene.kijewski@fu-berlin.de>
+ */
+#ifndef __TESTS_CORE_H_
+#define __TESTS_CORE_H_
+
+#include "../unittests.h"
+
+/**
+ * @brief   The entry point of this test suite.
+ */
+void tests_lib(void);
+
+/**
+ * @brief   Generates tests ringbuffer.h
+ *
+ * @return  embUnit tests if successful, NULL if not.
+ */
+Test *tests_lib_ringbuffer_tests(void);
+
+#endif /* __TESTS_CORE_H_ */
+/** @} */