From 61499dbdd829102f46937bf7a11d2bfa1dd43598 Mon Sep 17 00:00:00 2001 From: Kevin Ahrendt Date: Wed, 18 Dec 2024 21:07:07 -0500 Subject: [PATCH] [core] Bugfix: Implement ring buffer with xRingbuffer (#7973) --- esphome/core/ring_buffer.cpp | 90 +++++++++++++++++++++++++++++------- esphome/core/ring_buffer.h | 13 ++++-- 2 files changed, 82 insertions(+), 21 deletions(-) diff --git a/esphome/core/ring_buffer.cpp b/esphome/core/ring_buffer.cpp index 6152ada314..f779531263 100644 --- a/esphome/core/ring_buffer.cpp +++ b/esphome/core/ring_buffer.cpp @@ -13,8 +13,8 @@ static const char *const TAG = "ring_buffer"; RingBuffer::~RingBuffer() { if (this->handle_ != nullptr) { - vStreamBufferDelete(this->handle_); - ExternalRAMAllocator allocator(ExternalRAMAllocator::ALLOW_FAILURE); + vRingbufferDelete(this->handle_); + RAMAllocator allocator(RAMAllocator::ALLOW_FAILURE); allocator.deallocate(this->storage_, this->size_); } } @@ -22,26 +22,49 @@ RingBuffer::~RingBuffer() { std::unique_ptr RingBuffer::create(size_t len) { std::unique_ptr rb = make_unique(); - rb->size_ = len + 1; + rb->size_ = len; - ExternalRAMAllocator allocator(ExternalRAMAllocator::ALLOW_FAILURE); + RAMAllocator allocator(RAMAllocator::ALLOW_FAILURE); rb->storage_ = allocator.allocate(rb->size_); if (rb->storage_ == nullptr) { return nullptr; } - rb->handle_ = xStreamBufferCreateStatic(rb->size_, 1, rb->storage_, &rb->structure_); + rb->handle_ = xRingbufferCreateStatic(rb->size_, RINGBUF_TYPE_BYTEBUF, rb->storage_, &rb->structure_); ESP_LOGD(TAG, "Created ring buffer with size %u", len); + return rb; } size_t RingBuffer::read(void *data, size_t len, TickType_t ticks_to_wait) { - if (ticks_to_wait > 0) - xStreamBufferSetTriggerLevel(this->handle_, len); + size_t bytes_read = 0; - size_t bytes_read = xStreamBufferReceive(this->handle_, data, len, ticks_to_wait); + void *buffer_data = xRingbufferReceiveUpTo(this->handle_, &bytes_read, ticks_to_wait, len); - xStreamBufferSetTriggerLevel(this->handle_, 1); + if (buffer_data == nullptr) { + return 0; + } + + std::memcpy(data, buffer_data, bytes_read); + + vRingbufferReturnItem(this->handle_, buffer_data); + + if (bytes_read < len) { + // Data may have wrapped around, so read a second time to receive the remainder + size_t follow_up_bytes_read = 0; + size_t bytes_remaining = len - bytes_read; + + buffer_data = xRingbufferReceiveUpTo(this->handle_, &follow_up_bytes_read, 0, bytes_remaining); + + if (buffer_data == nullptr) { + return bytes_read; + } + + std::memcpy((void *) ((uint8_t *) (data) + bytes_read), buffer_data, follow_up_bytes_read); + + vRingbufferReturnItem(this->handle_, buffer_data); + bytes_read += follow_up_bytes_read; + } return bytes_read; } @@ -49,22 +72,55 @@ size_t RingBuffer::read(void *data, size_t len, TickType_t ticks_to_wait) { size_t RingBuffer::write(const void *data, size_t len) { size_t free = this->free(); if (free < len) { - size_t needed = len - free; - uint8_t discard[needed]; - xStreamBufferReceive(this->handle_, discard, needed, 0); + // Free enough space in the ring buffer to fit the new data + this->discard_bytes_(len - free); } - return xStreamBufferSend(this->handle_, data, len, 0); + return this->write_without_replacement(data, len, 0); } size_t RingBuffer::write_without_replacement(const void *data, size_t len, TickType_t ticks_to_wait) { - return xStreamBufferSend(this->handle_, data, len, ticks_to_wait); + if (!xRingbufferSend(this->handle_, data, len, ticks_to_wait)) { + // Couldn't fit all the data, so only write what will fit + size_t free = std::min(this->free(), len); + if (xRingbufferSend(this->handle_, data, free, 0)) { + return free; + } + return 0; + } + return len; } -size_t RingBuffer::available() const { return xStreamBufferBytesAvailable(this->handle_); } +size_t RingBuffer::available() const { + UBaseType_t ux_items_waiting = 0; + vRingbufferGetInfo(this->handle_, nullptr, nullptr, nullptr, nullptr, &ux_items_waiting); + return ux_items_waiting; +} -size_t RingBuffer::free() const { return xStreamBufferSpacesAvailable(this->handle_); } +size_t RingBuffer::free() const { return xRingbufferGetCurFreeSize(this->handle_); } -BaseType_t RingBuffer::reset() { return xStreamBufferReset(this->handle_); } +BaseType_t RingBuffer::reset() { + // Discards all the available data + return this->discard_bytes_(this->available()); +} + +bool RingBuffer::discard_bytes_(size_t discard_bytes) { + size_t bytes_read = 0; + + void *buffer_data = xRingbufferReceiveUpTo(this->handle_, &bytes_read, 0, discard_bytes); + if (buffer_data != nullptr) + vRingbufferReturnItem(this->handle_, buffer_data); + + if (bytes_read < discard_bytes) { + size_t wrapped_bytes_read = 0; + buffer_data = xRingbufferReceiveUpTo(this->handle_, &wrapped_bytes_read, 0, discard_bytes - bytes_read); + if (buffer_data != nullptr) { + vRingbufferReturnItem(this->handle_, buffer_data); + bytes_read += wrapped_bytes_read; + } + } + + return (bytes_read == discard_bytes); +} } // namespace esphome diff --git a/esphome/core/ring_buffer.h b/esphome/core/ring_buffer.h index aade1b5f49..bad96d3181 100644 --- a/esphome/core/ring_buffer.h +++ b/esphome/core/ring_buffer.h @@ -3,7 +3,7 @@ #ifdef USE_ESP32 #include -#include +#include #include #include @@ -82,9 +82,14 @@ class RingBuffer { static std::unique_ptr create(size_t len); protected: - StreamBufferHandle_t handle_; - StaticStreamBuffer_t structure_; - uint8_t *storage_; + /// @brief Discards data from the ring buffer. + /// @param discard_bytes amount of bytes to discard + /// @return True if all bytes were successfully discarded, false otherwise + bool discard_bytes_(size_t discard_bytes); + + RingbufHandle_t handle_{nullptr}; + StaticRingbuffer_t structure_; + uint8_t *storage_{nullptr}; size_t size_{0}; };