Merge pull request #13563 from paul-szczepanek-arm/circ-buf

Add mutiple push and pop for circular buffer

platform/include/platform/CircularBuffer.h

@@ -20,6 +20,8 @@
 #include <stdint.h>
 #include "platform/mbed_critical.h"
 #include "platform/mbed_assert.h"
+#include "platform/Span.h"
+#include "platform/mbed_atomic.h"
 
 namespace mbed {
 
@@ -60,8 +62,8 @@ struct is_unsigned<unsigned long long> {
 
 /** Templated Circular buffer class
  *
- *  @note Synchronization level: Interrupt safe
- *  @note CounterType must be unsigned and consistent with BufferSize
+ *  @note Synchronization level: Interrupt safe.
+ *  @note CounterType must be unsigned and consistent with BufferSize.
  */
 template<typename T, uint32_t BufferSize, typename CounterType = uint32_t>
 class CircularBuffer {
@@ -84,77 +86,197 @@ public:
     {
     }
 
-    /** Push the transaction to the buffer. This overwrites the buffer if it's
-     *  full
+    /** Push the transaction to the buffer. This overwrites the buffer if it's full.
      *
-     * @param data Data to be pushed to the buffer
+     * @param data Data to be pushed to the buffer.
      */
     void push(const T &data)
     {
         core_util_critical_section_enter();
-        if (full()) {
-            _tail++;
-            if (_tail == BufferSize) {
-                _tail = 0;
-            }
-        }
-        _pool[_head++] = data;
-        if (_head == BufferSize) {
-            _head = 0;
-        }
-        if (_head == _tail) {
+
+        _buffer[_head] = data;
+
+        _head = incrementCounter(_head);
+
+        if (_full) {
+            _tail = _head;
+        } else if (_head == _tail) {
             _full = true;
         }
+
         core_util_critical_section_exit();
     }
 
-    /** Pop the transaction from the buffer
+    /** Push the transaction to the buffer. This overwrites the buffer if it's full.
      *
-     * @param data Data to be popped from the buffer
-     * @return True if the buffer is not empty and data contains a transaction, false otherwise
+     * @param src Data to be pushed to the buffer.
+     * @param len Number of items to be pushed to the buffer.
+     */
+    void push(const T *src, CounterType len)
+    {
+        MBED_ASSERT(len > 0);
+
+        core_util_critical_section_enter();
+
+        /* if we try to write more bytes than the buffer can hold we only bother writing the last bytes */
+        if (len > BufferSize) {
+            _tail = 0;
+            _head = 0;
+            _full = true;
+            std::copy(src + len - BufferSize, src + len, _buffer);
+        } else {
+            /* we need to adjust the tail at the end if we're filling the buffer of overflowing */
+            bool adjust_tail = ((BufferSize - non_critical_size()) <= len);
+
+            CounterType written = len;
+
+            /* on first pass we write as much as we can to the right of head */
+            if ((_head + written) > BufferSize) {
+                written = BufferSize - _head;
+            }
+
+            std::copy(src, src + written, _buffer + _head);
+            _head = incrementCounter(_head, written);
+
+            CounterType left_to_write = len - written;
+
+            /* we might need to continue to write from the start of the buffer */
+            if (left_to_write) {
+                std::copy(src + written, src + written + left_to_write, _buffer);
+                _head = left_to_write;
+            }
+
+            if (adjust_tail) {
+                _tail = _head;
+                _full = true;
+            }
+        }
+
+        core_util_critical_section_exit();
+    }
+
+    /** Push the transaction to the buffer. This overwrites the buffer if it's full.
+     *
+     * @param src Data to be pushed to the buffer.
+     */
+    void push(mbed::Span<const T> src)
+    {
+        push(src.data(), src.size());
+    }
+
+    /** Pop from the buffer.
+     *
+     * @param data Container to store the data to be popped from the buffer.
+     * @return True if data popped.
      */
     bool pop(T &data)
     {
         bool data_popped = false;
+
         core_util_critical_section_enter();
-        if (!empty()) {
-            data = _pool[_tail++];
-            if (_tail == BufferSize) {
-                _tail = 0;
-            }
-            _full = false;
+
+        if (!non_critical_empty()) {
             data_popped = true;
+
+            data = _buffer[_tail];
+            _tail = incrementCounter(_tail);
+            _full = false;
         }
+
         core_util_critical_section_exit();
+
         return data_popped;
     }
 
-    /** Check if the buffer is empty
+    /**
+     * Pop multiple elements from the buffer.
      *
-     * @return True if the buffer is empty, false if not
+     * @param dest The array which will receive the elements.
+     * @param len The number of elements to pop.
+     *
+     * @return The number of elements popped.
+     */
+    CounterType pop(T *dest, CounterType len)
+    {
+        MBED_ASSERT(len > 0);
+
+        if (len == 0) {
+            return 0;
+        }
+
+        CounterType data_popped = 0;
+
+        core_util_critical_section_enter();
+
+        if (!non_critical_empty()) {
+            /* make sure we only try to read as much as we have items present */
+            if (len > non_critical_size()) {
+                len = non_critical_size();
+            }
+            data_popped = len;
+
+            /* items may be split by overlap, take only the number we have to the right of tail */
+            if ((_tail + data_popped) > BufferSize) {
+                data_popped = BufferSize - _tail;
+            }
+
+            std::copy(_buffer + _tail, _buffer + _tail + data_popped, dest);
+            _tail = incrementCounter(_tail, data_popped);
+
+            /* if we looped over the end we may need to pop again */
+            CounterType left_to_pop = len - data_popped;
+
+            if (left_to_pop) {
+                std::copy(_buffer, _buffer + left_to_pop, dest + data_popped);
+                _tail = left_to_pop;
+
+                data_popped += left_to_pop;
+            }
+
+            _full = false;
+        }
+
+        core_util_critical_section_exit();
+
+        return data_popped;
+    }
+
+    /**
+     * Pop multiple elements from the buffer.
+     *
+     * @param dest The span that contains the buffer that will be used to store the elements.
+     *
+     * @return The span with the size set to number of elements popped using the buffer passed in as the parameter.
+     */
+    mbed::Span<T> pop(mbed::Span<T> dest)
+    {
+        CounterType popped = pop(dest.data(), dest.size());
+        return mbed::make_Span(dest.data(), popped);
+    }
+
+    /** Check if the buffer is empty.
+     *
+     * @return True if the buffer is empty, false if not.
      */
     bool empty() const
     {
         core_util_critical_section_enter();
-        bool is_empty = (_head == _tail) && !_full;
+        bool is_empty = non_critical_empty();
         core_util_critical_section_exit();
         return is_empty;
     }
 
-    /** Check if the buffer is full
+    /** Check if the buffer is full.
      *
      * @return True if the buffer is full, false if not
      */
     bool full() const
     {
-        core_util_critical_section_enter();
-        bool full = _full;
-        core_util_critical_section_exit();
-        return full;
+        return core_util_atomic_load_bool(&_full);
     }
 
-    /** Reset the buffer
-     *
+    /**
+     * Reset the buffer.
      */
     void reset()
     {
@@ -165,10 +287,43 @@ public:
         core_util_critical_section_exit();
     }
 
-    /** Get the number of elements currently stored in the circular_buffer */
+    /**
+     * Get the number of elements currently stored in the circular_buffer.
+     */
     CounterType size() const
     {
         core_util_critical_section_enter();
+        CounterType elements = non_critical_size();
+        core_util_critical_section_exit();
+        return elements;
+    }
+
+    /** Peek into circular buffer without popping.
+     *
+     * @param data Data to be peeked from the buffer.
+     * @return True if the buffer is not empty and data contains a transaction, false otherwise.
+     */
+    bool peek(T &data) const
+    {
+        bool data_updated = false;
+        core_util_critical_section_enter();
+        if (!empty()) {
+            data = _buffer[_tail];
+            data_updated = true;
+        }
+        core_util_critical_section_exit();
+        return data_updated;
+    }
+
+private:
+    bool non_critical_empty() const
+    {
+        bool is_empty = (_head == _tail) && !_full;
+        return is_empty;
+    }
+
+    CounterType non_critical_size() const
+    {
         CounterType elements;
         if (!_full) {
             if (_head < _tail) {
@@ -179,29 +334,30 @@ public:
         } else {
             elements = BufferSize;
         }
-        core_util_critical_section_exit();
         return elements;
     }
 
-    /** Peek into circular buffer without popping
+    /** Used to increment _tail or _head by a given value.
      *
-     * @param data Data to be peeked from the buffer
-     * @return True if the buffer is not empty and data contains a transaction, false otherwise
+     * @param val The value of the counter to be incremented.
+     * @param increment The amount to be added, the value after this incremented must not exceed BufferSize.
+     * @return The new value of the counter.
      */
-    bool peek(T &data) const
+    CounterType incrementCounter(CounterType val, CounterType increment = 1)
     {
-        bool data_updated = false;
-        core_util_critical_section_enter();
-        if (!empty()) {
-            data = _pool[_tail];
-            data_updated = true;
+        val += increment;
+
+        MBED_ASSERT(val <= BufferSize);
+
+        if (val == BufferSize) {
+            val = 0;
         }
-        core_util_critical_section_exit();
-        return data_updated;
+
+        return val;
     }
 
 private:
-    T _pool[BufferSize];
+    T _buffer[BufferSize];
     CounterType _head;
     CounterType _tail;
     bool _full;

platform/tests/UNITTESTS/CircularBuffer/test_CircularBuffer.cpp

@@ -18,13 +18,15 @@
 #include "gtest/gtest.h"
 #include "platform/CircularBuffer.h"
 
+#define TEST_BUFFER_SIZE (10)
+
 class TestCircularBuffer : public testing::Test {
 protected:
-    mbed::CircularBuffer<int, 10> *buf;
+    mbed::CircularBuffer<int, TEST_BUFFER_SIZE> *buf;
 
     virtual void SetUp()
     {
-        buf = new mbed::CircularBuffer<int, 10>;
+        buf = new mbed::CircularBuffer<int, TEST_BUFFER_SIZE>;
     }
 
     virtual void TearDown()
@@ -37,3 +39,93 @@ TEST_F(TestCircularBuffer, constructor)
 {
     EXPECT_TRUE(buf);
 }
+
+TEST_F(TestCircularBuffer, push_pop)
+{
+    int item = 0;
+    buf->push(1);
+    bool ret = buf->pop(item);
+    EXPECT_TRUE(ret);
+    EXPECT_EQ(item, 1);
+}
+
+TEST_F(TestCircularBuffer, reset)
+{
+    buf->push(1);
+    EXPECT_EQ(buf->size(), 1);
+    buf->reset();
+    EXPECT_EQ(buf->size(), 0);
+}
+
+TEST_F(TestCircularBuffer, pop_empty)
+{
+    int item = 0;
+    bool ret = buf->pop(item);
+    EXPECT_FALSE(ret);
+}
+
+TEST_F(TestCircularBuffer, push_pop_multiple)
+{
+    const int test_numbers[TEST_BUFFER_SIZE] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };
+
+    /* this will check pushing across the buffer end */
+    for (int i = 0; i < TEST_BUFFER_SIZE; i++) {
+        int test_numbers_popped[TEST_BUFFER_SIZE] = { 0 };
+        buf->push(test_numbers, i);
+        EXPECT_EQ(buf->size(), i);
+        int number_of_items = buf->pop(test_numbers_popped, i);
+        EXPECT_EQ(buf->size(), 0);
+        EXPECT_EQ(number_of_items, i);
+        EXPECT_TRUE(0 == memcmp(test_numbers, test_numbers_popped, i));
+    }
+}
+
+TEST_F(TestCircularBuffer, overflow)
+{
+    const int test_numbers[TEST_BUFFER_SIZE] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };
+    int test_numbers_popped[TEST_BUFFER_SIZE] = { 0 };
+
+    buf->push(-1);
+
+    /* there is now not enough space for all the elements, old ones should be overwritten */
+
+    buf->push(test_numbers, TEST_BUFFER_SIZE);
+
+    int number_of_items = buf->pop(test_numbers_popped, TEST_BUFFER_SIZE);
+    EXPECT_EQ(number_of_items, TEST_BUFFER_SIZE);
+    EXPECT_TRUE(0 == memcmp(test_numbers, test_numbers_popped, TEST_BUFFER_SIZE));
+
+    /* there is a difference where the overflow is caused by a smaller write
+     * and the buffer should retain part of old values */
+
+    buf->push(-1);
+    buf->push(-2);
+    buf->push(test_numbers, TEST_BUFFER_SIZE-1); /* -1 is overwritten but -2 is kept */
+
+    int popped_number;
+    buf->pop(popped_number);
+    EXPECT_EQ(popped_number, -2);
+
+    buf->pop(test_numbers_popped, TEST_BUFFER_SIZE - 1);
+    EXPECT_TRUE(0 == memcmp(test_numbers, test_numbers_popped, TEST_BUFFER_SIZE - 1));
+}
+
+TEST_F(TestCircularBuffer, writing_over_max_capacity)
+{
+    const int test_numbers[TEST_BUFFER_SIZE + 1] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11 };
+    int test_numbers_popped[TEST_BUFFER_SIZE] = { 0 };
+
+    /* the loop creates different amounts of existing elements prior to write over capacity */
+    for (int i = 0; i < TEST_BUFFER_SIZE; i++) {
+        for (int j = 0; j < i; j++) {
+            buf->push(-1);
+        }
+        /* first element should be dropped */
+        buf->push(test_numbers, TEST_BUFFER_SIZE + 1);
+
+        int number_of_items = buf->pop(test_numbers_popped, TEST_BUFFER_SIZE + 1);
+        EXPECT_EQ(number_of_items, TEST_BUFFER_SIZE);
+        EXPECT_EQ(buf->size(), 0);
+        EXPECT_TRUE(0 == memcmp(test_numbers + 1, test_numbers_popped, TEST_BUFFER_SIZE));
+    }
+}

platform/tests/UNITTESTS/CircularBuffer/unittest.cmake

@@ -8,4 +8,6 @@ set(unittest-sources
 
 set(unittest-test-sources
   ../platform/tests/UNITTESTS/CircularBuffer/test_CircularBuffer.cpp
+  stubs/mbed_critical_stub.c
+  stubs/mbed_assert_stub.cpp
 )