-
Notifications
You must be signed in to change notification settings - Fork 3k
Add mutiple push and pop for circular buffer #13563
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 @@ class CircularBuffer { | |
{ | ||
} | ||
|
||
/** 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; | ||
} | ||
|
||
_buffer[_head] = data; | ||
|
||
_head = incrementCounter(_head); | ||
|
||
if (_full) { | ||
_tail = _head; | ||
} else if (_head == _tail) { | ||
_full = true; | ||
} | ||
_pool[_head++] = data; | ||
if (_head == BufferSize) { | ||
|
||
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. | ||
* @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; | ||
} | ||
if (_head == _tail) { | ||
_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(); | ||
} | ||
|
||
/** Pop the transaction from the buffer | ||
/** 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 Data to be popped from the buffer | ||
* @return True if the buffer is not empty and data contains a transaction, false otherwise | ||
* @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; | ||
|
||
if (!non_critical_empty()) { | ||
data_popped = true; | ||
|
||
data = _buffer[_tail]; | ||
_tail = incrementCounter(_tail); | ||
_full = false; | ||
} | ||
|
||
core_util_critical_section_exit(); | ||
|
||
return data_popped; | ||
} | ||
|
||
/** | ||
* Pop multiple elements from the buffer. | ||
* | ||
* @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; | ||
data_popped = true; | ||
} | ||
|
||
core_util_critical_section_exit(); | ||
|
||
return data_popped; | ||
} | ||
|
||
/** Check if the buffer is empty | ||
/** | ||
* 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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a random note that as of current C++ version you can do Don't know what our style view on that is. I'm conflicted. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's great to push good C++ coding standard. https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md uses There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you have a more specific reference? That's quite a big page, and can't see anything specifically discussing this case. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They don't discuss, they use the construct in code examples for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Then it's not really a specific style judgment on how explicit or not to be, which is what I'd like to see. And there's something of a difference - a pair is a pair, so it's "just" a tuple-type container. You're effectively filling in an aggregate from a list, which is the oldest-school use of Here, you're invoking a constructor. I always feel a bit icky when using Anyway, I'm broadly in favour of the more concise style; I don't like excess clutter. I think the case for it is even stronger in function parameter context, where the span could just be one of several arguments. Whereas on (C++17 will change the situation again, because you no longer have to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the detailed explanations :) (your comments are always great learning material for me) One side question (also asked here, what's the difference between There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just seen Vincent's answer - this partly repeats it...
Now that C++20 has been finalised, and following other work done in this style pending full The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you both (again) for the detailed answers :) I was not aware of the |
||
} | ||
|
||
/** Check if the buffer is empty. | ||
* | ||
* @return True if the buffer is empty, false if not | ||
* @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 @@ class CircularBuffer { | |
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 @@ class CircularBuffer { | |
} 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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is a private function so we can guarantee we will never exceed buffersize even after increment so we can simplify calculation |
||
|
||
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; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the same, just changed for readability