Merge branch 'dev' into ld6002b-static-ring-buffer

esphome/components/ld6002b/ld6002b.cpp

@@ -1066,20 +1066,19 @@ bool LD6002BComponent::queue_command_(uint16_t type, const uint8_t *data, uint8_
     ESP_LOGW(TAG, "Command data too large: %u", len);
     return false;
   }
-  if (this->cmd_count_ >= CMD_QUEUE_SIZE) {
+  if (this->cmd_queue_.size() >= CMD_QUEUE_SIZE) {
     ESP_LOGW(TAG, "Command queue full, dropping command 0x%04X", type);
     return false;
   }
 
-  PendingCommand &cmd = this->cmd_queue_[this->cmd_tail_];
+  PendingCommand cmd;
   cmd.type = type;
   cmd.len = len;
   if (len > 0 && data != nullptr) {
     std::memcpy(cmd.data.data(), data, len);
   }
+  this->cmd_queue_.push(cmd);
 
-  this->cmd_tail_ = (this->cmd_tail_ + 1) % CMD_QUEUE_SIZE;
-  this->cmd_count_++;
   this->process_command_queue_();
   return true;
 }
@@ -1146,12 +1145,11 @@ void LD6002BComponent::process_command_queue_() {
     return;
   }
 
-  if (this->cmd_count_ == 0)
+  if (this->cmd_queue_.empty())
     return;
 
-  this->active_command_ = this->cmd_queue_[this->cmd_head_];
-  this->cmd_head_ = (this->cmd_head_ + 1) % CMD_QUEUE_SIZE;
-  this->cmd_count_--;
+  this->active_command_ = this->cmd_queue_.front();
+  this->cmd_queue_.pop();
 
   this->send_generation_++;
   this->retries_left_ = CMD_MAX_RETRIES;

esphome/components/ld6002b/ld6002b.h

@@ -419,10 +419,7 @@ class LD6002BComponent : public Component, public uart::UARTDevice {
   // A reply cannot trail the frame that earned it for longer than this; the field worst case is ~726ms.
   static constexpr uint32_t STALE_ACK_MAX_AGE_MS = 1000;
 
-  std::array<PendingCommand, CMD_QUEUE_SIZE> cmd_queue_{};
-  uint8_t cmd_head_{0};
-  uint8_t cmd_tail_{0};
-  uint8_t cmd_count_{0};
+  StaticRingBuffer<PendingCommand, CMD_QUEUE_SIZE> cmd_queue_;
   bool command_active_{false};
   bool command_sent_{false};
   PendingCommand active_command_{};

esphome/components/usb_uart/usb_uart.cpp

@@ -108,6 +108,35 @@ std::vector<CdcEps> USBUartTypeCdcAcm::parse_descriptors(usb_device_handle_t dev
   return cdc_devs;
 }
 
+void RingBuffer::push(uint8_t item) {
+  if (this->get_free_space() == 0)
+    return;
+  this->buffer_[this->insert_pos_] = item;
+  this->insert_pos_ = (this->insert_pos_ + 1) % this->buffer_size_;
+}
+void RingBuffer::push(const uint8_t *data, size_t len) {
+  size_t free = this->get_free_space();
+  if (len > free)
+    len = free;
+  for (size_t i = 0; i != len; i++) {
+    this->buffer_[this->insert_pos_] = *data++;
+    this->insert_pos_ = (this->insert_pos_ + 1) % this->buffer_size_;
+  }
+}
+
+uint8_t RingBuffer::pop() {
+  uint8_t item = this->buffer_[this->read_pos_];
+  this->read_pos_ = (this->read_pos_ + 1) % this->buffer_size_;
+  return item;
+}
+size_t RingBuffer::pop(uint8_t *data, size_t len) {
+  len = std::min(len, this->get_available());
+  for (size_t i = 0; i != len; i++) {
+    *data++ = this->buffer_[this->read_pos_];
+    this->read_pos_ = (this->read_pos_ + 1) % this->buffer_size_;
+  }
+  return len;
+}
 void USBUartChannelBase::write_array(const uint8_t *data, size_t len) {
   if (!this->initialised_.load()) {
     ESP_LOGD(TAG, "Channel not initialised - write ignored");
@@ -159,10 +188,10 @@ uart::UARTFlushResult USBUartChannelBase::flush() {
 }
 
 bool USBUartChannelBase::peek_byte(uint8_t *data) {
-  if (this->input_buffer_.empty()) {
+  if (this->input_buffer_.is_empty()) {
     return false;
   }
-  *data = this->input_buffer_.front();
+  *data = this->input_buffer_.peek();
   return true;
 }
 bool USBUartChannelBase::read_array(uint8_t *data, size_t len) {
@@ -170,12 +199,18 @@ bool USBUartChannelBase::read_array(uint8_t *data, size_t len) {
     ESP_LOGV(TAG, "Channel not initialised - read ignored");
     return false;
   }
-  const size_t got = this->input_buffer_.pop(data, len);
-  if (got < len) {
-    ESP_LOGV(TAG, "underflow: requested %zu but returned %zu bytes", len, got);
+  auto available = this->available();
+  bool status = true;
+  if (len > available) {
+    ESP_LOGV(TAG, "underflow: requested %zu but returned %d, bytes", len, available);
+    len = available;
+    status = false;
+  }
+  for (size_t i = 0; i != len; i++) {
+    *data++ = this->input_buffer_.pop();
   }
   this->parent_->start_input(this);
-  return got == len;
+  return status;
 }
 void USBUartComponent::setup() { USBClient::setup(); }
 void USBUartComponent::loop() {
@@ -198,7 +233,7 @@ void USBUartComponent::loop() {
 
     // If there is not enough space for the full chunk, let the device subclass
     // handle it (e.g. FTDI clears the buffer to prevent mid-telegram corruption).
-    if (channel->input_buffer_.free_space() < chunk->length) {
+    if (channel->input_buffer_.get_free_space() < chunk->length) {
       this->on_rx_overflow(channel);
     }
     // Push data to ring buffer (now safe in main loop)

esphome/components/usb_uart/usb_uart.h

@@ -112,6 +112,28 @@ enum class ConfigMode : uint8_t {
   CONFIG_MODE_MODEM,   ///< Apply the DTR/RTS state of one already-open channel
 };
 
+class RingBuffer {
+ public:
+  RingBuffer(uint16_t buffer_size) : buffer_size_(buffer_size), buffer_(new uint8_t[buffer_size]) {}
+  bool is_empty() const { return this->read_pos_ == this->insert_pos_; }
+  size_t get_available() const {
+    return (this->insert_pos_ + this->buffer_size_ - this->read_pos_) % this->buffer_size_;
+  };
+  size_t get_free_space() const { return this->buffer_size_ - 1 - this->get_available(); }
+  uint8_t peek() const { return this->buffer_[this->read_pos_]; }
+  void push(uint8_t item);
+  void push(const uint8_t *data, size_t len);
+  uint8_t pop();
+  size_t pop(uint8_t *data, size_t len);
+  void clear() { this->read_pos_ = this->insert_pos_ = 0; }
+
+ protected:
+  uint16_t insert_pos_ = 0;
+  uint16_t read_pos_ = 0;
+  uint16_t buffer_size_;
+  uint8_t *buffer_;
+};
+
 // Structure for queuing received USB data chunks
 struct UsbDataChunk {
   uint8_t data[usb_host::USB_MAX_PACKET_SIZE];
@@ -152,7 +174,7 @@ class USBUartChannelBase : public uart::UARTComponent, public Parented<USBUartCo
   void write_array(const uint8_t *data, size_t len) override;
   bool peek_byte(uint8_t *data) override;
   bool read_array(uint8_t *data, size_t len) override;
-  size_t available() override { return this->input_buffer_.size(); }
+  size_t available() override { return this->input_buffer_.get_available(); }
   bool is_connected() override { return this->initialised_.load(); }
   uart::UARTFlushResult flush() override;
   // Re-apply the current line settings (baud, parity, etc) to this already-open channel.
@@ -193,13 +215,12 @@ class USBUartChannelBase : public uart::UARTComponent, public Parented<USBUartCo
 
  protected:
   // Not directly instantiable; construct a concrete channel type instead.
-  USBUartChannelBase(uint8_t index, uint16_t buffer_size) : index_(index) { this->input_buffer_.init(buffer_size); }
+  USBUartChannelBase(uint8_t index, uint16_t buffer_size) : input_buffer_(RingBuffer(buffer_size)), index_(index) {}
   void check_logger_conflict() override {}
   LineStopBits line_stop_bits_() const;
   LineParity line_parity_() const;
   // Larger structures first (8+ bytes)
-  // Only the main loop touches it; the USB task hands data over through usb_data_queue_
-  FixedRingBuffer<uint8_t> input_buffer_;
+  RingBuffer input_buffer_;
   LockFreeQueue<UsbOutputChunk, USB_OUTPUT_CHUNK_COUNT> output_queue_;
   // Pool sized to queue capacity (SIZE-1) because LockFreeQueue<T,N> is a ring
   // buffer that holds N-1 elements. This guarantees allocate() returns nullptr

esphome/core/helpers.h

@@ -475,7 +475,7 @@ template<typename T, size_t MAX_CAPACITY = std::numeric_limits<uint16_t>::max()>
   class Iterator {
    public:
     Iterator(FixedRingBuffer *buf, index_type pos) : buf_(buf), pos_(pos) {}
-    T &operator*() { return buf_->data_[buf_->wrap_(static_cast<size_t>(buf_->head_) + pos_)]; }
+    T &operator*() { return buf_->data_[(buf_->head_ + pos_) % buf_->capacity_]; }
     Iterator &operator++() {
       ++pos_;
       return *this;
@@ -490,7 +490,7 @@ template<typename T, size_t MAX_CAPACITY = std::numeric_limits<uint16_t>::max()>
   class ConstIterator {
    public:
     ConstIterator(const FixedRingBuffer *buf, index_type pos) : buf_(buf), pos_(pos) {}
-    const T &operator*() const { return buf_->data_[buf_->wrap_(static_cast<size_t>(buf_->head_) + pos_)]; }
+    const T &operator*() const { return buf_->data_[(buf_->head_ + pos_) % buf_->capacity_]; }
     ConstIterator &operator++() {
       ++pos_;
       return *this;
@@ -532,7 +532,7 @@ template<typename T, size_t MAX_CAPACITY = std::numeric_limits<uint16_t>::max()>
     if (this->count_ >= this->capacity_)
       return false;
     this->data_[this->tail_] = value;
-    this->tail_ = this->wrap_(static_cast<size_t>(this->tail_) + 1);
+    this->tail_ = (this->tail_ + 1) % this->capacity_;
     ++this->count_;
     return true;
   }
@@ -540,7 +540,7 @@ template<typename T, size_t MAX_CAPACITY = std::numeric_limits<uint16_t>::max()>
   /// Push a value, overwriting the oldest if full.
   void push_overwrite(const T &value) {
     this->data_[this->tail_] = value;
-    this->tail_ = this->wrap_(static_cast<size_t>(this->tail_) + 1);
+    this->tail_ = (this->tail_ + 1) % this->capacity_;
     if (this->count_ >= this->capacity_) {
       // Buffer full - advance head to drop oldest, count stays at capacity
       this->head_ = this->tail_;
@@ -549,48 +549,19 @@ template<typename T, size_t MAX_CAPACITY = std::numeric_limits<uint16_t>::max()>
     }
   }
 
-  /// Push up to `len` values without overwriting. Returns how many fit.
-  size_t push(const T *data, size_t len) {
-    static_assert(std::is_trivially_copyable<T>::value, "bulk push copies with memcpy");
-    len = std::min<size_t>(len, this->free_space());
-    if (len == 0)
-      return 0;  // data_ may be null, and memcpy with a null pointer is undefined even for 0 bytes
-    const size_t first = std::min<size_t>(len, this->capacity_ - this->tail_);
-    std::memcpy(this->data_ + this->tail_, data, first * sizeof(T));
-    std::memcpy(this->data_, data + first, (len - first) * sizeof(T));
-    this->tail_ = this->wrap_(static_cast<size_t>(this->tail_) + len);
-    this->count_ += len;
-    return len;
-  }
-
   /// Remove the oldest element.
   void pop() {
     if (this->count_ > 0) {
-      this->head_ = this->wrap_(static_cast<size_t>(this->head_) + 1);
+      this->head_ = (this->head_ + 1) % this->capacity_;
       --this->count_;
     }
   }
 
-  /// Move up to `len` of the oldest values into `out`. Returns how many were copied.
-  size_t pop(T *out, size_t len) {
-    static_assert(std::is_trivially_copyable<T>::value, "bulk pop copies with memcpy");
-    len = std::min<size_t>(len, this->count_);
-    if (len == 0)
-      return 0;
-    const size_t first = std::min<size_t>(len, this->capacity_ - this->head_);
-    std::memcpy(out, this->data_ + this->head_, first * sizeof(T));
-    std::memcpy(out + first, this->data_, (len - first) * sizeof(T));
-    this->head_ = this->wrap_(static_cast<size_t>(this->head_) + len);
-    this->count_ -= len;
-    return len;
-  }
-
   T &front() { return this->data_[this->head_]; }
   const T &front() const { return this->data_[this->head_]; }
   index_type size() const { return this->count_; }
   bool empty() const { return this->count_ == 0; }
   index_type capacity() const { return this->capacity_; }
-  index_type free_space() const { return this->capacity_ - this->count_; }
   bool full() const { return this->count_ == this->capacity_; }
 
   /// Clear all elements (reset to empty, keep capacity)
@@ -606,12 +577,14 @@ template<typename T, size_t MAX_CAPACITY = std::numeric_limits<uint16_t>::max()>
   ConstIterator end() const { return ConstIterator(this, this->count_); }
 
   /// The element `pos` places after the oldest one; the sum is widened so a small index type cannot truncate it
-  const T &operator[](index_type pos) const { return this->data_[this->wrap_(static_cast<size_t>(this->head_) + pos)]; }
+  const T &operator[](index_type pos) const {
+    size_t index = static_cast<size_t>(this->head_) + pos;
+    if (index >= this->capacity_)
+      index -= this->capacity_;
+    return this->data_[index];
+  }
 
  protected:
-  /// Fold an index below 2 * capacity back into the buffer
-  index_type wrap_(size_t index) const { return index >= this->capacity_ ? index - this->capacity_ : index; }
-
   T *data_{nullptr};
   index_type head_{0};
   index_type tail_{0};

tests/components/core/test_fixed_ring_buffer.cpp

@@ -1,137 +0,0 @@
-#include <gtest/gtest.h>
-
-#include <array>
-#include <cstdint>
-
-#include "esphome/core/helpers.h"
-
-namespace esphome::core::testing {
-
-TEST(FixedRingBufferBulk, PushAndPopInOrder) {
-  FixedRingBuffer<uint8_t, 255> buf;
-  buf.init(8);
-  const uint8_t in[] = {1, 2, 3, 4, 5};
-  EXPECT_EQ(buf.push(in, sizeof(in)), 5u);
-  EXPECT_EQ(buf.size(), 5);
-  EXPECT_EQ(buf.front(), 1);
-
-  std::array<uint8_t, 5> out{};
-  EXPECT_EQ(buf.pop(out.data(), out.size()), 5u);
-  EXPECT_EQ(out, (std::array<uint8_t, 5>{1, 2, 3, 4, 5}));
-  EXPECT_TRUE(buf.empty());
-}
-
-TEST(FixedRingBufferBulk, PushStopsWhenFullWithoutOverwriting) {
-  FixedRingBuffer<uint8_t, 255> buf;
-  buf.init(4);
-  const uint8_t first[] = {1, 2, 3};
-  const uint8_t second[] = {4, 5, 6};
-  EXPECT_EQ(buf.push(first, sizeof(first)), 3u);
-  EXPECT_EQ(buf.free_space(), 1);
-  EXPECT_EQ(buf.push(second, sizeof(second)), 1u);
-  EXPECT_TRUE(buf.full());
-  EXPECT_EQ(buf.free_space(), 0);
-  EXPECT_EQ(buf.push(second, sizeof(second)), 0u);
-
-  std::array<uint8_t, 4> out{};
-  EXPECT_EQ(buf.pop(out.data(), out.size()), 4u);
-  EXPECT_EQ(out, (std::array<uint8_t, 4>{1, 2, 3, 4}));
-}
-
-TEST(FixedRingBufferBulk, PopFromEmptyReturnsZero) {
-  FixedRingBuffer<uint8_t, 255> buf;
-  buf.init(4);
-  uint8_t out[4] = {9, 9, 9, 9};
-  EXPECT_EQ(buf.pop(out, sizeof(out)), 0u);
-  EXPECT_EQ(out[0], 9);
-}
-
-TEST(FixedRingBufferBulk, PartialPopLeavesTheRest) {
-  FixedRingBuffer<uint8_t, 255> buf;
-  buf.init(6);
-  const uint8_t in[] = {1, 2, 3, 4, 5};
-  buf.push(in, sizeof(in));
-  uint8_t out[2];
-  EXPECT_EQ(buf.pop(out, sizeof(out)), 2u);
-  EXPECT_EQ(out[0], 1);
-  EXPECT_EQ(out[1], 2);
-  EXPECT_EQ(buf.size(), 3);
-  EXPECT_EQ(buf.front(), 3);
-}
-
-TEST(FixedRingBufferBulk, WrapsAroundTheEnd) {
-  FixedRingBuffer<uint8_t, 255> buf;
-  buf.init(5);
-  const uint8_t lead[] = {10, 11, 12, 13};
-  buf.push(lead, sizeof(lead));
-  uint8_t drop[3];
-  buf.pop(drop, sizeof(drop));  // head 3, tail 4, one element
-
-  // Fills the last slot, then wraps to the start
-  const uint8_t in[] = {20, 21, 22, 23};
-  EXPECT_EQ(buf.push(in, sizeof(in)), 4u);
-  EXPECT_TRUE(buf.full());
-  EXPECT_EQ(buf[0], 13);
-  EXPECT_EQ(buf[4], 23);
-
-  // Pop crosses the end in two runs
-  std::array<uint8_t, 5> out{};
-  EXPECT_EQ(buf.pop(out.data(), out.size()), 5u);
-  EXPECT_EQ(out, (std::array<uint8_t, 5>{13, 20, 21, 22, 23}));
-  EXPECT_TRUE(buf.empty());
-
-  // Indices stay consistent after wrapping, including an exact landing on the end
-  EXPECT_EQ(buf.push(in, 3), 3u);
-  EXPECT_EQ(buf.pop(out.data(), 3), 3u);
-  EXPECT_EQ(out[0], 20);
-  EXPECT_EQ(out[2], 22);
-  EXPECT_EQ(buf.push(lead, sizeof(lead)), 4u);
-  buf.push(lead[0]);
-  EXPECT_TRUE(buf.full());
-  EXPECT_EQ(buf.pop(out.data(), out.size()), 5u);
-  EXPECT_EQ(out, (std::array<uint8_t, 5>{10, 11, 12, 13, 10}));
-}
-
-TEST(FixedRingBufferBulk, MixesWithSingleElementCalls) {
-  FixedRingBuffer<uint16_t, 1000> buf;
-  buf.init(3);
-  buf.push(uint16_t{1});
-  const uint16_t in[] = {2, 3};
-  EXPECT_EQ(buf.push(in, 2), 2u);
-  buf.pop();
-  EXPECT_TRUE(buf.push(uint16_t{4}));
-  uint16_t out[3];
-  EXPECT_EQ(buf.pop(out, 3), 3u);
-  EXPECT_EQ(out[0], 2);
-  EXPECT_EQ(out[1], 3);
-  EXPECT_EQ(out[2], 4);
-}
-
-TEST(FixedRingBufferBulk, UninitialisedHoldsNothing) {
-  FixedRingBuffer<uint8_t, 255> buf;
-  const uint8_t in[] = {1};
-  uint8_t out[1];
-  EXPECT_EQ(buf.push(in, 1), 0u);
-  EXPECT_EQ(buf.pop(out, 1), 0u);
-}
-
-// A uint8_t index at its limit: head and tail plus a full-buffer length go past 255
-TEST(FixedRingBufferBulk, WrapsAtTheIndexTypeLimit) {
-  FixedRingBuffer<uint8_t, 255> buf;
-  buf.init(255);
-  std::array<uint8_t, 255> data{};
-  ASSERT_EQ(buf.push(data.data(), 250), 250u);
-  ASSERT_EQ(buf.pop(data.data(), 250), 250u);  // head and tail now at 250
-  for (size_t i = 0; i < data.size(); i++)
-    data[i] = static_cast<uint8_t>(i);
-  EXPECT_EQ(buf.push(data.data(), data.size()), 255u);
-  EXPECT_TRUE(buf.full());
-  EXPECT_EQ(buf[0], 0);
-  EXPECT_EQ(buf[254], 254);
-  std::array<uint8_t, 255> out{};
-  EXPECT_EQ(buf.pop(out.data(), out.size()), 255u);
-  EXPECT_EQ(out, data);
-  EXPECT_TRUE(buf.empty());
-}
-
-}  // namespace esphome::core::testing