fix: Add tests and fix RingBuffer implementation

src/midi_RingBuffer.h

@@ -34,13 +34,16 @@ BEGIN_MIDI_NAMESPACE
 template<typename DataType, int Size>
 class RingBuffer
 {
+private:
+    static const int sMask = Size - 1;
+
 public:
      RingBuffer();
     ~RingBuffer();
 
 public:
-    int getLength() const;
+    inline int getLength() const;
-    bool isEmpty() const;
+    inline bool isEmpty() const;
 
 public:
     void write(DataType inData);
@@ -53,8 +56,9 @@ public:
 
 private:
     DataType mData[Size];
-    DataType* mWriteHead;
+    int mLength;
-    DataType* mReadHead;
+    int mWriteHead;
+    int mReadHead;
 };
 
 END_MIDI_NAMESPACE

src/midi_RingBuffer.hpp

@@ -27,13 +27,27 @@
 
 #pragma once
 
+BEGIN_UNNAMED_NAMESPACE
+
+template<int N>
+struct isPowerOfTwo
+{
+    static const bool value = N && !(N & (N - 1));
+};
+
+END_UNNAMED_NAMESPACE
+
+// --
+
 BEGIN_MIDI_NAMESPACE
 
 template<typename DataType, int Size>
 RingBuffer<DataType, Size>::RingBuffer()
-    : mWriteHead(mData)
+    : mLength(0)
-    , mReadHead(mData)
+    , mWriteHead(0)
+    , mReadHead(0)
 {
+    static_assert(isPowerOfTwo<Size>::value, "Size must be a power of two.");
     memset(mData, DataType(0), Size * sizeof(DataType));
 }
 
@@ -45,26 +59,15 @@ RingBuffer<DataType, Size>::~RingBuffer()
 // -----------------------------------------------------------------------------
 
 template<typename DataType, int Size>
-int RingBuffer<DataType, Size>::getLength() const
+inline int RingBuffer<DataType, Size>::getLength() const
 {
-    if (mReadHead == mWriteHead)
+    return mLength;
-    {
-        return 0;
-    }
-    else if (mWriteHead > mReadHead)
-    {
-        return int(mWriteHead - mReadHead);
-    }
-    else
-    {
-        return int(mWriteHead - mData) + Size - int(mReadHead - mData);
-    }
 }
 
 template<typename DataType, int Size>
-bool RingBuffer<DataType, Size>::isEmpty() const
+inline bool RingBuffer<DataType, Size>::isEmpty() const
 {
-    return mReadHead == mWriteHead;
+    return mLength == 0;
 }
 
 // -----------------------------------------------------------------------------
@@ -72,10 +75,12 @@ bool RingBuffer<DataType, Size>::isEmpty() const
 template<typename DataType, int Size>
 void RingBuffer<DataType, Size>::write(DataType inData)
 {
-    *mWriteHead++ = inData;
+    mData[mWriteHead] = inData;
-    if (mWriteHead >= mData + Size)
+    mWriteHead = (mWriteHead + 1) & sMask;
-    {
+    mLength++;
-        mWriteHead = mData;
+    if (mLength > Size) {
+        mLength = Size;
+        mReadHead = (mReadHead + 1) & sMask;
     }
 }
 
@@ -92,8 +97,9 @@ template<typename DataType, int Size>
 void RingBuffer<DataType, Size>::clear()
 {
     memset(mData, DataType(0), Size * sizeof(DataType));
-    mReadHead  = mData;
+    mReadHead  = 0;
-    mWriteHead = mData;
+    mWriteHead = 0;
+    mLength = 0;
 }
 
 // -----------------------------------------------------------------------------
@@ -101,11 +107,13 @@ void RingBuffer<DataType, Size>::clear()
 template<typename DataType, int Size>
 DataType RingBuffer<DataType, Size>::read()
 {
-    const DataType data = *mReadHead++;
+    mLength--;
-    if (mReadHead >= mData + Size)
+    if (mLength < 0) {
-    {
+        mLength = 0;
-        mReadHead = mData;
+        return 0;
     }
+    const DataType data = mData[mReadHead];
+    mReadHead = (mReadHead + 1) & sMask;
     return data;
 }
 

test/unit-tests/CMakeLists.txt

@@ -18,6 +18,7 @@ add_executable(unit-tests
     tests/unit-tests_Settings.cpp
     tests/unit-tests_Settings.h
     tests/unit-tests_SysExCodec.cpp
+    tests/unit-tests_RingBuffer.cpp
     tests/unit-tests_SerialMock.cpp
     tests/unit-tests_MidiInput.cpp
     tests/unit-tests_MidiInputCallbacks.cpp

test/unit-tests/tests/unit-tests_RingBuffer.cpp

@@ -0,0 +1,174 @@
+#include "unit-tests.h"
+#include <src/midi_RingBuffer.h>
+
+BEGIN_UNNAMED_NAMESPACE
+using namespace testing;
+using Buffer = midi::RingBuffer<uint8_t, 8>;
+
+TEST(RingBuffer, writeScalar)
+{
+    Buffer buffer;
+    EXPECT_EQ(buffer.isEmpty(), true);
+    EXPECT_EQ(buffer.getLength(), 0);
+    buffer.write(42);
+    buffer.write(47);
+    EXPECT_EQ(buffer.isEmpty(), false);
+    EXPECT_EQ(buffer.getLength(), 2);
+}
+
+TEST(RingBuffer, readScalar)
+{
+    Buffer buffer;
+    buffer.write(42);
+    EXPECT_EQ(buffer.getLength(), 1);
+    buffer.write(47);
+    EXPECT_EQ(buffer.getLength(), 2);
+    EXPECT_EQ(buffer.read(), 42);
+    EXPECT_EQ(buffer.getLength(), 1);
+    EXPECT_EQ(buffer.read(), 47);
+    EXPECT_EQ(buffer.isEmpty(), true);
+    EXPECT_EQ(buffer.getLength(), 0);
+}
+
+TEST(RingBuffer, clear)
+{
+    Buffer buffer;
+    buffer.write(42);
+    buffer.write(47);
+    buffer.clear();
+    EXPECT_EQ(buffer.isEmpty(), true);
+    EXPECT_EQ(buffer.getLength(), 0);
+}
+
+TEST(RingBuffer, writeArray)
+{
+    Buffer buffer;
+    const uint8_t input[4] = {
+        1, 2, 3, 4
+    };
+    buffer.write(input, 4);
+    EXPECT_EQ(buffer.isEmpty(), false);
+    EXPECT_EQ(buffer.getLength(), 4);
+}
+
+TEST(RingBuffer, writeOverflow)
+{
+    Buffer buffer;
+
+    buffer.write(1);
+    EXPECT_EQ(buffer.getLength(), 1);
+    EXPECT_EQ(buffer.isEmpty(), false);
+    buffer.write(2);
+    EXPECT_EQ(buffer.getLength(), 2);
+    EXPECT_EQ(buffer.isEmpty(), false);
+    buffer.write(3);
+    EXPECT_EQ(buffer.getLength(), 3);
+    EXPECT_EQ(buffer.isEmpty(), false);
+    buffer.write(4);
+    EXPECT_EQ(buffer.getLength(), 4);
+    EXPECT_EQ(buffer.isEmpty(), false);
+    buffer.write(5);
+    EXPECT_EQ(buffer.getLength(), 5);
+    EXPECT_EQ(buffer.isEmpty(), false);
+    buffer.write(6);
+    EXPECT_EQ(buffer.getLength(), 6);
+    EXPECT_EQ(buffer.isEmpty(), false);
+    buffer.write(7);
+    EXPECT_EQ(buffer.getLength(), 7);
+    EXPECT_EQ(buffer.isEmpty(), false);
+    buffer.write(8);
+    EXPECT_EQ(buffer.getLength(), 8);
+    EXPECT_EQ(buffer.isEmpty(), false);
+    buffer.write(9);
+    EXPECT_EQ(buffer.getLength(), 8);
+    EXPECT_EQ(buffer.isEmpty(), false);
+    buffer.write(10);
+    EXPECT_EQ(buffer.getLength(), 8);
+    EXPECT_EQ(buffer.isEmpty(), false);
+    buffer.write(11);
+    EXPECT_EQ(buffer.getLength(), 8);
+    EXPECT_EQ(buffer.isEmpty(), false);
+    buffer.write(12);
+    EXPECT_EQ(buffer.getLength(), 8);
+    EXPECT_EQ(buffer.isEmpty(), false);
+}
+
+TEST(RingBuffer, readOverflow)
+{
+    Buffer buffer;
+
+    buffer.write(1);
+    buffer.write(2);
+    buffer.write(3);
+    buffer.write(4);
+    buffer.write(5);
+    buffer.write(6);
+    buffer.write(7);
+    buffer.write(8);
+    buffer.write(9);
+    buffer.write(10);
+    buffer.write(11);
+    buffer.write(12);
+    EXPECT_EQ(buffer.read(), 5);
+    EXPECT_EQ(buffer.getLength(), 7);
+    EXPECT_EQ(buffer.read(), 6);
+    EXPECT_EQ(buffer.getLength(), 6);
+    EXPECT_EQ(buffer.read(), 7);
+    EXPECT_EQ(buffer.getLength(), 5);
+    EXPECT_EQ(buffer.read(), 8);
+    EXPECT_EQ(buffer.getLength(), 4);
+    EXPECT_EQ(buffer.read(), 9);
+    EXPECT_EQ(buffer.getLength(), 3);
+    EXPECT_EQ(buffer.read(), 10);
+    EXPECT_EQ(buffer.getLength(), 2);
+    EXPECT_EQ(buffer.read(), 11);
+    EXPECT_EQ(buffer.getLength(), 1);
+    EXPECT_EQ(buffer.read(), 12);
+    EXPECT_EQ(buffer.getLength(), 0);
+    EXPECT_EQ(buffer.read(), 0);
+    EXPECT_EQ(buffer.getLength(), 0);
+}
+
+TEST(RingBuffer, writeArrayOverflow)
+{
+    Buffer buffer;
+    const uint8_t input[12] = {
+        1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12,
+    };
+    buffer.write(input, 12);
+    EXPECT_EQ(buffer.isEmpty(), false);
+    EXPECT_EQ(buffer.getLength(), 8); // Max size
+}
+
+TEST(RingBuffer, readArray)
+{
+    Buffer buffer;
+    const uint8_t input[4] = {
+        1, 2, 3, 4
+    };
+    uint8_t output[4] = { 0 };
+    buffer.write(input, 4);
+    buffer.read(output, 4);
+    EXPECT_THAT(output, ContainerEq(input));
+}
+
+TEST(RingBuffer, readArrayOverflow)
+{
+    Buffer buffer;
+    const uint8_t input[12] = {
+        1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12,
+    };
+    const uint8_t expected[8] = {
+        5, 6, 7, 8, 9, 10, 11, 12,
+    };
+    uint8_t output[8] = { 0 };
+
+    buffer.write(input, 12);
+    buffer.read(output, 8);
+    EXPECT_THAT(output, ContainerEq(expected));
+    EXPECT_EQ(buffer.isEmpty(), true);
+    EXPECT_EQ(buffer.getLength(), 0);
+}
+
+END_UNNAMED_NAMESPACE
+

test/unit-tests/tests/unit-tests_SerialMock.cpp

@@ -6,7 +6,7 @@ BEGIN_UNNAMED_NAMESPACE
 USING_NAMESPACE_TEST_MOCKS
 using namespace testing;
 
-TEST(RingBuffer, initialState)
+TEST(RingBufferMock, initialState)
 {
     typedef RingBuffer<uint8, 32> Buffer;
     Buffer buffer;
@@ -17,7 +17,7 @@ TEST(RingBuffer, initialState)
     EXPECT_EQ(buffer.isEmpty(),   true);
 }
 
-TEST(RingBuffer, uint8)
+TEST(RingBufferMock, uint8)
 {
     typedef RingBuffer<uint8, 8> Buffer;
     Buffer buffer;
@@ -48,7 +48,7 @@ TEST(RingBuffer, uint8)
     EXPECT_EQ(buffer.isEmpty(),   true);
 }
 
-TEST(RingBuffer, uint32)
+TEST(RingBufferMock, uint32)
 {
     typedef RingBuffer<uint32_t, 32> Buffer;
     Buffer buffer;