From 5a4903a2ff7458fa93443bc2bb12d9193900fa37 Mon Sep 17 00:00:00 2001 From: "D.R.racer" Date: Fri, 21 May 2021 07:46:18 +0200 Subject: [PATCH] Cleanup circular_buffer implementation unit tests will follow --- src/hal/avr/usart.cpp | 6 +-- src/hal/circle_buffer.hpp | 98 --------------------------------------- src/hal/circular_buffer.h | 63 +++++++++++++++++++++++++ src/hal/usart.h | 19 ++++---- 4 files changed, 76 insertions(+), 110 deletions(-) delete mode 100644 src/hal/circle_buffer.hpp create mode 100644 src/hal/circular_buffer.h diff --git a/src/hal/avr/usart.cpp b/src/hal/avr/usart.cpp index 966441e..5e6d205 100644 --- a/src/hal/avr/usart.cpp +++ b/src/hal/avr/usart.cpp @@ -8,7 +8,7 @@ USART usart1(USART1); uint8_t USART::Read() { uint8_t c = 0; - rx_buf.ConsumeFirst(c); + rx_buf.pop(c); return c; } @@ -18,7 +18,7 @@ void USART::Write(uint8_t c) { // to the data register and be done. This shortcut helps // significantly improve the effective datarate at high (> // 500kbit/s) bitrates, where interrupt overhead becomes a slowdown. - if (tx_buf.IsEmpty() && (husart->UCSRxA & (1 << 5))) { + if (tx_buf.empty() && (husart->UCSRxA & (1 << 5))) { husart->UDRx = c; husart->UCSRxA |= (1 << 6); return; @@ -26,7 +26,7 @@ void USART::Write(uint8_t c) { // If the output buffer is full, there's nothing for it other than to // wait for the interrupt handler to empty it a bit - while (!tx_buf.push_back_DontRewrite(c)) { + while (!tx_buf.push(c)) { if (bit_is_clear(SREG, SREG_I)) { // Interrupts are disabled, so we'll have to poll the data // register empty flag ourselves. If it is set, pretend an diff --git a/src/hal/circle_buffer.hpp b/src/hal/circle_buffer.hpp deleted file mode 100644 index e6b77cf..0000000 --- a/src/hal/circle_buffer.hpp +++ /dev/null @@ -1,98 +0,0 @@ -// circle_buffer.hpp -#pragma once - -#include -#include -/*****************************************************************************/ -// general circular buffer -// you can never use entire size -// because write position (end) cannot be equal to begin -// because begin == end == empty -template -class CircleBuffer { -public: - using Elem = T; - -protected: - T data[SIZE]; - volatile size_t begin; // position of first element - volatile size_t end; // position behind last element == write position - volatile size_t pushed; - static void incrementIndex(volatile size_t &index) { index = (index + 1) % SIZE; } - static void decrementIndex(volatile size_t &index) { index = (index + SIZE - 1) % SIZE; } - -public: - CircleBuffer() - : begin(0) - , end(0) - , pushed(0) {} - - void push_back(T elem); - bool push_back_DontRewrite(T elem); - size_t Count() const { return (end + SIZE - begin) % SIZE; } - bool IsEmpty() const { return begin == end; } - bool CanPush() const { - size_t index = begin; - incrementIndex(index); - return (index != end); - } - size_t PushedCount() const { return pushed; } - - constexpr size_t Size() const { return SIZE; } - - bool ConsumeFirst(T &elem); // data must be processed before next push_back - bool ConsumeLast(T &elem); // data must be processed before next push_back - const T &GetFirstIfAble() const; // data must be processed before next push_back, must not be empty - const T &GetLastIfAble() const; // data must be processed before next push_back, must not be empty -}; - -template -void CircleBuffer::push_back(T elem) { - data[end] = elem; - incrementIndex(end); - if (begin == end) { //begin just was erased, set new begin - incrementIndex(begin); - } - ++pushed; -} - -template -bool CircleBuffer::push_back_DontRewrite(T elem) { - size_t index = begin; - incrementIndex(index); - if (index != end) { - push_back(elem); - return true; - } - return false; -} - -template -bool CircleBuffer::ConsumeFirst(T &elem) { - if (IsEmpty()) - return false; - elem = GetFirstIfAble(); - incrementIndex(begin); - return true; -} - -template -bool CircleBuffer::ConsumeLast(T &elem) { - if (IsEmpty()) - return false; - elem = GetLastIfAble(); - decrementIndex(end); - return true; -} - -template -const T &CircleBuffer::GetFirstIfAble() const { - return data[begin]; -} - -template -const T &CircleBuffer::GetLastIfAble() const { - size_t index = end; - decrementIndex(index); - return data[index]; -} diff --git a/src/hal/circular_buffer.h b/src/hal/circular_buffer.h new file mode 100644 index 0000000..6ef1c6d --- /dev/null +++ b/src/hal/circular_buffer.h @@ -0,0 +1,63 @@ +#pragma once + +#include +#include + +/// A generic circular buffer class +/// Can hold up to (size-1) elements +/// @param T data type of stored elements +/// @param index_t data type of indices into array of elements +/// (recommended to keep uint8_fast8_t as single byte operations are atomical on the AVR) +/// @param size number of elements to store + 1. +/// It is recommended to keep a power of 2 to allow for optimal code generation on the AVR (there is no HW modulo instruction) +template +class CircularBuffer { +public: + constexpr inline CircularBuffer() + : tail(0) + , head(0) {} + + inline bool empty() const { + return tail == head; + } + + bool full() const { + return next(head) == tail; + } + + /// Insert an element into the buffer. + /// Checks for empty spot for the element and does not change the buffer content + /// in case the buffer is full. + /// @returns true if the insertion was successful (i.e. there was an empty spot for the element) + bool push(T elem) { + if (full()) + return false; + data[head] = elem; + head = next(head); + return true; + } + + /// @returns peeks the current element to extract from the buffer, however the element is left in the buffer + /// Does not perform any range checks for performance reasons, should be preceeded by if(!empty()) in the user code + inline T front() const { + return data[tail]; + } + + /// Extracts the current element from the buffer + /// @returns true in case there was an element for extraction (i.e. the buffer was not empty) + bool pop(T &elem) { + if (empty()) + return false; + elem = front(); + tail = next(tail); + return true; + } + +protected: + T data[size]; ///< array of stored elements + index_t tail; ///< index of element to read (pop/extract) from the buffer + index_t head; ///< index of an empty spot or element insertion (write) + + /// @returns next index wrapped past the end of the array of elements + static index_t next(index_t index) { return (index + 1) % size; } +}; diff --git a/src/hal/usart.h b/src/hal/usart.h index b7d478d..6f204dc 100644 --- a/src/hal/usart.h +++ b/src/hal/usart.h @@ -2,7 +2,8 @@ #include #include #include "gpio.h" -#include "circle_buffer.hpp" +#include "circular_buffer.h" + /// USART interface /// @@TODO decide, if this class will behave like a singleton, or there will be multiple classes /// for >1 USART interfaces @@ -33,11 +34,11 @@ public: /// @returns current character from the UART without extracting it from the read buffer uint8_t Peek() const { - return rx_buf.GetFirstIfAble(); + return rx_buf.front(); } /// @returns true if there are no bytes to be read bool ReadEmpty() const { - return rx_buf.IsEmpty(); + return rx_buf.empty(); } /// @returns current character from the UART and extracts it from the read buffer uint8_t Read(); @@ -48,7 +49,7 @@ public: void puts(const char *str); /// @returns true if there is at least one byte free in the TX buffer (i.e. some space to add a character to be sent) bool CanWrite() const { - return tx_buf.CanPush(); + return !tx_buf.full(); } /// blocks until the TX buffer was successfully transmitted void Flush(); @@ -69,13 +70,13 @@ public: if (husart->UCSRxA & (1 << 4)) { (void)husart->UDRx; } else { - rx_buf.push_back_DontRewrite(husart->UDRx); + rx_buf.push((uint8_t)husart->UDRx); } } /// implementation of the transmit ISR's body __attribute__((always_inline)) inline void ISR_UDRE() { uint8_t c = 0; - tx_buf.ConsumeFirst(c); + tx_buf.pop(c); husart->UDRx = c; // clear the TXC bit -- "can be cleared by writing a one to its bit @@ -83,7 +84,7 @@ public: // actually got written husart->UCSRxA |= (1 << 6); - if (tx_buf.IsEmpty()) + if (tx_buf.empty()) husart->UCSRxB &= ~(1 << 5); // disable UDRE interrupt } @@ -94,8 +95,8 @@ private: USART_TypeDef *husart; bool _written; - CircleBuffer tx_buf; - CircleBuffer rx_buf; + CircularBuffer tx_buf; + CircularBuffer rx_buf; }; /// beware - normally we'd make a singleton, but avr-gcc generates suboptimal code for them, therefore we only keep this extern variable