From 1a891948d9921fe5ecb55df72360bef01be6dba5 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Mon, 19 Jan 2026 15:49:31 -0600 Subject: [PATCH 01/11] fix(led_strip): Update LedStrip to support DMA --- components/led_strip/CMakeLists.txt | 2 +- .../example/main/led_strip_example.cpp | 10 +- components/led_strip/include/led_strip.hpp | 122 ++++++++++++++++-- 3 files changed, 118 insertions(+), 16 deletions(-) diff --git a/components/led_strip/CMakeLists.txt b/components/led_strip/CMakeLists.txt index 9faa14ad2..31b3bf78b 100644 --- a/components/led_strip/CMakeLists.txt +++ b/components/led_strip/CMakeLists.txt @@ -1,5 +1,5 @@ idf_component_register( INCLUDE_DIRS "include" SRC_DIRS "src" - REQUIRES "base_component" "color" + REQUIRES "heap" "base_component" "color" ) diff --git a/components/led_strip/example/main/led_strip_example.cpp b/components/led_strip/example/main/led_strip_example.cpp index 0b7086422..19f1fec59 100644 --- a/components/led_strip/example/main/led_strip_example.cpp +++ b/components/led_strip/example/main/led_strip_example.cpp @@ -118,10 +118,14 @@ extern "C" void app_main(void) { }); //! [led strip ex1] - // create the rmt object + // create the rmt object with DMA enabled for better performance espp::Rmt rmt(espp::Rmt::Config{ .gpio_num = NEO_BFF_IO, + .clock_src = RMT_CLK_SRC_DEFAULT, + .dma_enabled = true, + .block_size = 1024, .resolution_hz = SK6805_FREQ_HZ, + .transaction_queue_depth = 2, .log_level = espp::Logger::Verbosity::INFO, }); @@ -138,7 +142,7 @@ extern "C" void app_main(void) { rmt.transmit(data, len); }; - // now create the LedStrip object + // now create the LedStrip object with DMA support espp::LedStrip led_strip(espp::LedStrip::Config{ .num_leds = NEO_BFF_NUM_LEDS, .write = neopixel_write, @@ -146,6 +150,8 @@ extern "C" void app_main(void) { .byte_order = espp::LedStrip::ByteOrder::GRB, .start_frame = {}, .end_frame = {}, + .use_dma = true, + .dma_allocation_flags = MALLOC_CAP_DMA, .log_level = espp::Logger::Verbosity::INFO, }); diff --git a/components/led_strip/include/led_strip.hpp b/components/led_strip/include/led_strip.hpp index c6abd324b..6bc2632ce 100644 --- a/components/led_strip/include/led_strip.hpp +++ b/components/led_strip/include/led_strip.hpp @@ -9,6 +9,7 @@ #include "base_component.hpp" #include "color.hpp" +#include "esp_heap_caps.h" namespace espp { /// \brief Class to control LED strips @@ -69,7 +70,10 @@ class LedStrip : public BaseComponent { ///< before the first LED if not empty. std::vector end_frame{}; ///< End frame for the strip. Optional - will be sent after ///< the last LED if not empty. - Logger::Verbosity log_level; ///< Log level for this class + bool use_dma{false}; ///< Whether to use DMA-capable memory allocation + uint32_t dma_allocation_flags{ + 0}; ///< DMA allocation flags (if use_dma is true). Defaults to MALLOC_CAP_DMA. + Logger::Verbosity log_level; ///< Log level for this class }; /// \brief Constructor @@ -79,19 +83,103 @@ class LedStrip : public BaseComponent { , num_leds_(config.num_leds) , send_brightness_(config.send_brightness) , byte_order_(config.byte_order) - , write_(config.write) { + , write_(config.write) + , use_dma_(config.use_dma) { // set the color data size pixel_size_ = send_brightness_ ? 4 : 3; - data_.resize(num_leds_ * pixel_size_ + config.start_frame.size() + config.end_frame.size()); + data_size_ = num_leds_ * pixel_size_ + config.start_frame.size() + config.end_frame.size(); + + // Allocate memory based on DMA preference + if (use_dma_) { + uint32_t dma_flags = config.dma_allocation_flags; + if (dma_flags == 0) { + dma_flags = MALLOC_CAP_DMA; + } + data_ = (uint8_t *)heap_caps_malloc(data_size_, dma_flags); + if (!data_) { + logger_.warn("Failed to allocate DMA memory, falling back to regular malloc"); + data_ = (uint8_t *)malloc(data_size_); + } + } else { + data_ = (uint8_t *)malloc(data_size_); + } + + if (!data_) { + logger_.error("Failed to allocate memory for LED strip data"); + return; + } + // copy the start frame - std::copy(config.start_frame.begin(), config.start_frame.end(), data_.begin()); + if (!config.start_frame.empty()) { + memcpy(data_, config.start_frame.data(), config.start_frame.size()); + } + // copy the end frame - std::copy(config.end_frame.begin(), config.end_frame.end(), - data_.end() - config.end_frame.size()); + if (!config.end_frame.empty()) { + memcpy(data_ + data_size_ - config.end_frame.size(), config.end_frame.data(), + config.end_frame.size()); + } + start_offset_ = config.start_frame.size(); end_offset_ = config.end_frame.size(); } + /// \brief Destructor + /// \details This function frees the memory allocated for the LED strip data. + ~LedStrip() { + if (data_) { + free(data_); + data_ = nullptr; + } + } + + /// \brief Copy constructor (deleted to prevent double-free) + LedStrip(const LedStrip &) = delete; + + /// \brief Assignment operator (deleted to prevent double-free) + LedStrip &operator=(const LedStrip &) = delete; + + /// \brief Move constructor + LedStrip(LedStrip &&other) noexcept + : BaseComponent(std::move(other)) + , num_leds_(other.num_leds_) + , send_brightness_(other.send_brightness_) + , byte_order_(other.byte_order_) + , pixel_size_(other.pixel_size_) + , start_offset_(other.start_offset_) + , end_offset_(other.end_offset_) + , data_size_(other.data_size_) + , data_(other.data_) + , write_(std::move(other.write_)) + , use_dma_(other.use_dma_) { + other.data_ = nullptr; + other.data_size_ = 0; + } + + /// \brief Move assignment operator + LedStrip &operator=(LedStrip &&other) noexcept { + if (this != &other) { + if (data_) { + free(data_); + } + + num_leds_ = other.num_leds_; + send_brightness_ = other.send_brightness_; + byte_order_ = other.byte_order_; + pixel_size_ = other.pixel_size_; + start_offset_ = other.start_offset_; + end_offset_ = other.end_offset_; + data_size_ = other.data_size_; + data_ = other.data_; + write_ = std::move(other.write_); + use_dma_ = other.use_dma_; + + other.data_ = nullptr; + other.data_size_ = 0; + } + return *this; + } + /// \brief Get the number of LEDs in the strip /// \return Number of LEDs in the strip size_t num_leds() const { return num_leds_; } @@ -112,8 +200,8 @@ class LedStrip : public BaseComponent { } if (shift_by < 0) shift_by += num_leds_; - std::rotate(data_.begin() + start_offset_, - data_.begin() + start_offset_ + pixel_size_ * shift_by, data_.end() + end_offset_); + std::rotate(data_ + start_offset_, data_ + start_offset_ + pixel_size_ * shift_by, + data_ + data_size_ - end_offset_); } /// \brief Shift the LEDs to the right @@ -128,8 +216,10 @@ class LedStrip : public BaseComponent { } if (shift_by < 0) shift_by += num_leds_; - std::rotate(data_.rbegin() + end_offset_, data_.rbegin() + end_offset_ + pixel_size_ * shift_by, - data_.rend() + start_offset_); + std::rotate(std::reverse_iterator(data_ + data_size_ - end_offset_), + std::reverse_iterator(data_ + data_size_ - end_offset_) + + pixel_size_ * shift_by, + std::reverse_iterator(data_ + start_offset_)); } /// \brief Set the color of a single LED @@ -243,8 +333,12 @@ class LedStrip : public BaseComponent { /// \sa set_pixel /// \sa set_all void show() { - logger_.debug("writing data {::02x}", data_); - write_(&data_[0], data_.size()); + if (!data_) { + logger_.error("No data allocated for LED strip"); + return; + } + logger_.debug("writing data {::02x}", std::span(data_, data_ + data_size_)); + write_(data_, data_size_); } protected: @@ -254,7 +348,9 @@ class LedStrip : public BaseComponent { size_t pixel_size_{3}; size_t start_offset_{0}; size_t end_offset_{0}; - std::vector data_; + size_t data_size_{0}; + uint8_t *data_{nullptr}; write_fn write_; + bool use_dma_{false}; }; } // namespace espp From dbd05939dc37eaba84cb7dcb9e68f70665c6d585 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Mon, 19 Jan 2026 16:58:41 -0600 Subject: [PATCH 02/11] Update components/led_strip/include/led_strip.hpp Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- components/led_strip/include/led_strip.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/led_strip/include/led_strip.hpp b/components/led_strip/include/led_strip.hpp index 6bc2632ce..783931ed8 100644 --- a/components/led_strip/include/led_strip.hpp +++ b/components/led_strip/include/led_strip.hpp @@ -72,7 +72,7 @@ class LedStrip : public BaseComponent { ///< the last LED if not empty. bool use_dma{false}; ///< Whether to use DMA-capable memory allocation uint32_t dma_allocation_flags{ - 0}; ///< DMA allocation flags (if use_dma is true). Defaults to MALLOC_CAP_DMA. + MALLOC_CAP_DMA}; ///< DMA allocation flags (if use_dma is true). Defaults to MALLOC_CAP_DMA. Logger::Verbosity log_level; ///< Log level for this class }; From ffb740955fdf562767afd77fe5e1d99fbc9f4415 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Mon, 19 Jan 2026 16:59:12 -0600 Subject: [PATCH 03/11] Update components/led_strip/include/led_strip.hpp Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- components/led_strip/include/led_strip.hpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/components/led_strip/include/led_strip.hpp b/components/led_strip/include/led_strip.hpp index 783931ed8..48a1d03f0 100644 --- a/components/led_strip/include/led_strip.hpp +++ b/components/led_strip/include/led_strip.hpp @@ -95,13 +95,13 @@ class LedStrip : public BaseComponent { if (dma_flags == 0) { dma_flags = MALLOC_CAP_DMA; } - data_ = (uint8_t *)heap_caps_malloc(data_size_, dma_flags); + data_ = static_cast(heap_caps_malloc(data_size_, dma_flags)); if (!data_) { logger_.warn("Failed to allocate DMA memory, falling back to regular malloc"); - data_ = (uint8_t *)malloc(data_size_); + data_ = static_cast(malloc(data_size_)); } } else { - data_ = (uint8_t *)malloc(data_size_); + data_ = static_cast(malloc(data_size_)); } if (!data_) { From 2c1991b43f12254f39f3aebbb3ef83d8443e6db4 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Mon, 19 Jan 2026 16:59:26 -0600 Subject: [PATCH 04/11] Update components/led_strip/include/led_strip.hpp Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- components/led_strip/include/led_strip.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/led_strip/include/led_strip.hpp b/components/led_strip/include/led_strip.hpp index 48a1d03f0..f28508fcb 100644 --- a/components/led_strip/include/led_strip.hpp +++ b/components/led_strip/include/led_strip.hpp @@ -9,7 +9,7 @@ #include "base_component.hpp" #include "color.hpp" -#include "esp_heap_caps.h" +#include namespace espp { /// \brief Class to control LED strips From cfaf12cb0b206e3462c8998aa2ec9fce9585256d Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Mon, 19 Jan 2026 22:53:09 -0600 Subject: [PATCH 05/11] improve led strip some more (dependent on logger change) --- components/led_strip/include/led_strip.hpp | 90 ++++++++++++++++------ 1 file changed, 67 insertions(+), 23 deletions(-) diff --git a/components/led_strip/include/led_strip.hpp b/components/led_strip/include/led_strip.hpp index 6bc2632ce..5f6628a41 100644 --- a/components/led_strip/include/led_strip.hpp +++ b/components/led_strip/include/led_strip.hpp @@ -66,11 +66,11 @@ class LedStrip : public BaseComponent { write_fn write; ///< Function to write data to the strip bool send_brightness{true}; ///< Whether to use the brightness value for the LEDs ByteOrder byte_order{ByteOrder::RGB}; ///< Byte order for the LEDs - std::vector start_frame{}; ///< Start frame for the strip. Optional - will be sent + std::span start_frame{}; ///< Start frame for the strip. Optional - will be sent ///< before the first LED if not empty. - std::vector end_frame{}; ///< End frame for the strip. Optional - will be sent after - ///< the last LED if not empty. - bool use_dma{false}; ///< Whether to use DMA-capable memory allocation + std::span end_frame{}; ///< End frame for the strip. Optional - will be sent after + ///< the last LED if not empty. + bool use_dma{false}; ///< Whether to use DMA-capable memory allocation uint32_t dma_allocation_flags{ 0}; ///< DMA allocation flags (if use_dma is true). Defaults to MALLOC_CAP_DMA. Logger::Verbosity log_level; ///< Log level for this class @@ -85,6 +85,11 @@ class LedStrip : public BaseComponent { , byte_order_(config.byte_order) , write_(config.write) , use_dma_(config.use_dma) { + if (num_leds_ == 0) { + logger_.warn("Initialized LedStrip with 0 LEDs"); + return; + } + // set the color data size pixel_size_ = send_brightness_ ? 4 : 3; data_size_ = num_leds_ * pixel_size_ + config.start_frame.size() + config.end_frame.size(); @@ -140,6 +145,7 @@ class LedStrip : public BaseComponent { LedStrip &operator=(const LedStrip &) = delete; /// \brief Move constructor + /// \param other The other LedStrip to move from LedStrip(LedStrip &&other) noexcept : BaseComponent(std::move(other)) , num_leds_(other.num_leds_) @@ -157,12 +163,17 @@ class LedStrip : public BaseComponent { } /// \brief Move assignment operator + /// \param other The other LedStrip to move from + /// \return Reference to this LedStrip LedStrip &operator=(LedStrip &&other) noexcept { if (this != &other) { + // Free existing data if (data_) { free(data_); } + // Move members from other + logger_ = std::move(other.logger_); num_leds_ = other.num_leds_; send_brightness_ = other.send_brightness_; byte_order_ = other.byte_order_; @@ -174,6 +185,7 @@ class LedStrip : public BaseComponent { write_ = std::move(other.write_); use_dma_ = other.use_dma_; + // Nullify other's pointers other.data_ = nullptr; other.data_size_ = 0; } @@ -190,29 +202,40 @@ class LedStrip : public BaseComponent { /// \brief Shift the LEDs to the left /// \param shift_by Number of LEDs to shift by + /// \return true if successful, false if data is invalid or shift_by is out of range /// \note A negative value for shift_by will shift the LEDs to the right - void shift_left(int shift_by = 1) { + bool shift_left(int shift_by = 1) { + if (!data_) { + logger_.error("No data allocated for LED strip"); + return false; + } if (shift_by == 0) - return; + return true; if (shift_by >= num_leds_) { logger_.error("Shift by {} is greater than the number of LEDs ({})", shift_by, num_leds_); - return; + return false; } if (shift_by < 0) shift_by += num_leds_; std::rotate(data_ + start_offset_, data_ + start_offset_ + pixel_size_ * shift_by, data_ + data_size_ - end_offset_); + return true; } /// \brief Shift the LEDs to the right /// \param shift_by Number of LEDs to shift by + /// \return true if successful, false if data is invalid or shift_by is out of range /// \note A negative value for shift_by will shift the LEDs to the left - void shift_right(int shift_by = 1) { + bool shift_right(int shift_by = 1) { + if (!data_) { + logger_.error("No data allocated for LED strip"); + return false; + } if (shift_by == 0) - return; + return true; if (shift_by >= num_leds_) { logger_.error("Shift by {} is greater than the number of LEDs ({})", shift_by, num_leds_); - return; + return false; } if (shift_by < 0) shift_by += num_leds_; @@ -220,33 +243,36 @@ class LedStrip : public BaseComponent { std::reverse_iterator(data_ + data_size_ - end_offset_) + pixel_size_ * shift_by, std::reverse_iterator(data_ + start_offset_)); + return true; } /// \brief Set the color of a single LED /// \param index Index of the LED to set /// \param hsv Color to set the LED to /// \param brightness Brightness of the LED + /// \return true if successful, false if data is invalid or index is out of range /// \note The index is zero-based. /// \sa Hsv for more information on the HSV color space /// \sa show - void set_pixel(int index, const Hsv &hsv, float brightness = 1.0f) { - set_pixel(index, hsv.rgb(), brightness); + bool set_pixel(int index, const Hsv &hsv, float brightness = 1.0f) { + return set_pixel(index, hsv.rgb(), brightness); } /// \brief Set the color of a single LED /// \param index Index of the LED to set /// \param rgb Color to set the LED to /// \param brightness Brightness of the LED + /// \return true if successful, false if data is invalid or index is out of range /// \note The index is zero-based. /// \sa Rgb for more information on the RGB color space /// \sa show - void set_pixel(int index, const Rgb &rgb, float brightness = 1.0f) { + bool set_pixel(int index, const Rgb &rgb, float brightness = 1.0f) { uint8_t brightness_byte = std::clamp(brightness * 31.0f, 0, 31); uint8_t r, g, b; r = std::clamp(rgb.r * 255.0f, 0, 255); g = std::clamp(rgb.g * 255.0f, 0, 255); b = std::clamp(rgb.b * 255.0f, 0, 255); - set_pixel(index, r, g, b, brightness_byte); + return set_pixel(index, r, g, b, brightness_byte); } /// \brief Set the color of a single LED @@ -255,12 +281,17 @@ class LedStrip : public BaseComponent { /// \param g Green component of the color to set the LED to [0-255] /// \param b Blue component of the color to set the LED to [0-255] /// \param brightness Brightness of the LED [0-31] + /// \return true if successful, false if data is invalid or index is out of range /// \note The index is zero-based. /// \sa show - void set_pixel(int index, uint8_t r, uint8_t g, uint8_t b, uint8_t brightness = 0b11111) { + bool set_pixel(int index, uint8_t r, uint8_t g, uint8_t b, uint8_t brightness = 0b11111) { + if (!data_) { + logger_.error("No data allocated for LED strip"); + return false; + } if (index < 0 || index >= num_leds_) { logger_.error("set_pixel: index out of range: %d", index); - return; + return false; } int offset = start_offset_ + index * pixel_size_; @@ -291,24 +322,27 @@ class LedStrip : public BaseComponent { data_[offset++] = r; data_[offset++] = g; data_[offset++] = b; + return true; } /// \brief Set the color of all the LEDs /// \param hsv Color to set the LEDs to /// \param brightness Brightness of the LEDs + /// \return true if successful, false if data is invalid /// \note The index is zero-based. /// \sa set_pixel /// \sa show - void set_all(const Hsv &hsv, float brightness = 1.0f) { set_all(hsv.rgb(), brightness); } + bool set_all(const Hsv &hsv, float brightness = 1.0f) { return set_all(hsv.rgb(), brightness); } /// \brief Set the color of all the LEDs /// \param rgb Color to set the LEDs to /// \param brightness Brightness of the LEDs + /// \return true if successful, false if data is invalid /// \sa set_pixel /// \sa show - void set_all(const Rgb &rgb, float brightness = 1.0f) { + bool set_all(const Rgb &rgb, float brightness = 1.0f) { uint8_t brightness_byte = std::clamp(brightness * 255.0f, 0, 255); - set_all(rgb.r, rgb.g, rgb.b, brightness_byte); + return set_all(rgb.r, rgb.g, rgb.b, brightness_byte); } /// \brief Set the color of all the LEDs @@ -316,29 +350,39 @@ class LedStrip : public BaseComponent { /// \param g Green component of the color to set the LEDs to /// \param b Blue component of the color to set the LEDs to /// \param brightness Brightness of the LEDs + /// \return true if successful, false if data is invalid /// \sa set_pixel /// \sa show - void set_all(uint8_t r, uint8_t g, uint8_t b, uint8_t brightness = 0xff) { + bool set_all(uint8_t r, uint8_t g, uint8_t b, uint8_t brightness = 0xff) { + if (!data_) { + logger_.error("No data allocated for LED strip"); + return false; + } // fill out all the color data for (int i = 0; i < num_leds_; i++) { - set_pixel(i, r, g, b, brightness); + if (!set_pixel(i, r, g, b, brightness)) { + return false; + } } + return true; } /// \brief Show the colors on the strip /// \details This function writes the colors to the strip. It /// should be called after setting the colors of the LEDs. + /// \return true if successful, false if data is invalid /// \note This function blocks until the colors have been written /// to the strip. /// \sa set_pixel /// \sa set_all - void show() { + bool show() { if (!data_) { logger_.error("No data allocated for LED strip"); - return; + return false; } logger_.debug("writing data {::02x}", std::span(data_, data_ + data_size_)); write_(data_, data_size_); + return true; } protected: From bd9e2b1ab1650662c3ebdad8edda3cbea4526e74 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Wed, 21 Jan 2026 21:57:44 -0600 Subject: [PATCH 06/11] fix sa? --- components/led_strip/include/led_strip.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/led_strip/include/led_strip.hpp b/components/led_strip/include/led_strip.hpp index 0731cd3a5..4b2714d33 100644 --- a/components/led_strip/include/led_strip.hpp +++ b/components/led_strip/include/led_strip.hpp @@ -147,7 +147,7 @@ class LedStrip : public BaseComponent { /// \brief Move constructor /// \param other The other LedStrip to move from LedStrip(LedStrip &&other) noexcept - : BaseComponent(std::move(other)) + : BaseComponent(std::move(other)) // cppcheck-suppress accessMoved , num_leds_(other.num_leds_) , send_brightness_(other.send_brightness_) , byte_order_(other.byte_order_) @@ -173,7 +173,7 @@ class LedStrip : public BaseComponent { } // Move members from other - logger_ = std::move(other.logger_); + logger_ = std::move(other.logger_); // cppcheck-suppress accessMoved num_leds_ = other.num_leds_; send_brightness_ = other.send_brightness_; byte_order_ = other.byte_order_; From be05d4106b805ce9d8479af7d85ec5389514951d Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Wed, 21 Jan 2026 21:58:47 -0600 Subject: [PATCH 07/11] remove unneeded suppression --- components/led_strip/include/led_strip.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/led_strip/include/led_strip.hpp b/components/led_strip/include/led_strip.hpp index 4b2714d33..79660da49 100644 --- a/components/led_strip/include/led_strip.hpp +++ b/components/led_strip/include/led_strip.hpp @@ -173,7 +173,7 @@ class LedStrip : public BaseComponent { } // Move members from other - logger_ = std::move(other.logger_); // cppcheck-suppress accessMoved + logger_ = std::move(other.logger_); num_leds_ = other.num_leds_; send_brightness_ = other.send_brightness_; byte_order_ = other.byte_order_; From d5b800a84d8f6ff2959da991e08a216951b59d87 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Wed, 21 Jan 2026 22:05:24 -0600 Subject: [PATCH 08/11] address comments --- components/led_strip/include/led_strip.hpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/components/led_strip/include/led_strip.hpp b/components/led_strip/include/led_strip.hpp index 79660da49..a61400eb5 100644 --- a/components/led_strip/include/led_strip.hpp +++ b/components/led_strip/include/led_strip.hpp @@ -114,6 +114,9 @@ class LedStrip : public BaseComponent { return; } + // zero out the data + memset(data_, 0, data_size_); + // copy the start frame if (!config.start_frame.empty()) { memcpy(data_, config.start_frame.data(), config.start_frame.size()); @@ -172,8 +175,10 @@ class LedStrip : public BaseComponent { free(data_); } + // move-assign base class + BaseComponent::operator=(std::move(other)); // cppcheck-suppress accessMoved + // Move members from other - logger_ = std::move(other.logger_); num_leds_ = other.num_leds_; send_brightness_ = other.send_brightness_; byte_order_ = other.byte_order_; From d8bccc4f43ab3c4cacfcdb1ce1c9e762769eb75f Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Wed, 21 Jan 2026 22:24:06 -0600 Subject: [PATCH 09/11] fix sa --- components/led_strip/include/led_strip.hpp | 12 +++++++++--- components/logger/include/logger.hpp | 6 +++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/components/led_strip/include/led_strip.hpp b/components/led_strip/include/led_strip.hpp index a61400eb5..95fc4c720 100644 --- a/components/led_strip/include/led_strip.hpp +++ b/components/led_strip/include/led_strip.hpp @@ -150,17 +150,19 @@ class LedStrip : public BaseComponent { /// \brief Move constructor /// \param other The other LedStrip to move from LedStrip(LedStrip &&other) noexcept - : BaseComponent(std::move(other)) // cppcheck-suppress accessMoved + // cppcheck-suppress-begin accessMoved + : BaseComponent(std::move(other)) , num_leds_(other.num_leds_) , send_brightness_(other.send_brightness_) , byte_order_(other.byte_order_) , pixel_size_(other.pixel_size_) - , start_offset_(other.start_offset_) + , start_offset_(other.start_offset_) , end_offset_(other.end_offset_) , data_size_(other.data_size_) , data_(other.data_) , write_(std::move(other.write_)) , use_dma_(other.use_dma_) { + // cppcheck-suppress-end accessMoved other.data_ = nullptr; other.data_size_ = 0; } @@ -176,8 +178,10 @@ class LedStrip : public BaseComponent { } // move-assign base class - BaseComponent::operator=(std::move(other)); // cppcheck-suppress accessMoved + BaseComponent::operator=(std::move(other)); + // cppcheck-suppress-begin accessMoved + // Move members from other num_leds_ = other.num_leds_; send_brightness_ = other.send_brightness_; @@ -190,6 +194,8 @@ class LedStrip : public BaseComponent { write_ = std::move(other.write_); use_dma_ = other.use_dma_; + // cppcheck-suppress-end accessMoved + // Nullify other's pointers other.data_ = nullptr; other.data_size_ = 0; diff --git a/components/logger/include/logger.hpp b/components/logger/include/logger.hpp index 04318d9b4..f0394aa5f 100644 --- a/components/logger/include/logger.hpp +++ b/components/logger/include/logger.hpp @@ -130,7 +130,7 @@ rate limit. @note Only calls that have _rate_limited suffixed will be rate limit * @brief Move constructor * @param other The other logger to move from. */ - Logger(Logger &&other) noexcept + Logger(Logger &&other) noexcept // cppcheck-suppress missingMemberCopy : tag_([&other] { std::scoped_lock lock(other.tag_mutex_); return std::move(other.tag_); @@ -138,7 +138,7 @@ rate limit. @note Only calls that have _rate_limited suffixed will be rate limit , rate_limit_(std::move(other.rate_limit_)) , last_print_(std::move(other.last_print_)) , include_time_(other.include_time_.load()) - , level_(other.level_.load()) {} + , level_(other.level_.load()) { } /** * @brief Copy assignment operator @@ -147,7 +147,7 @@ rate limit. @note Only calls that have _rate_limited suffixed will be rate limit * @note This will NOT copy the last_print_ time, as this is not meaningful * for the new logger. */ - Logger &operator=(const Logger &other) { // cppcheck-suppress missingMemberCopy + Logger &operator=(const Logger &other) { // cppcheck-suppress operatorEqVarError if (this != &other) { std::scoped_lock lock(tag_mutex_, other.tag_mutex_); tag_ = other.tag_; From dffa6b80111643c2bfb37a9410c21235870f9ec4 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Thu, 22 Jan 2026 08:42:30 -0600 Subject: [PATCH 10/11] fix sa --- components/led_strip/include/led_strip.hpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/components/led_strip/include/led_strip.hpp b/components/led_strip/include/led_strip.hpp index 95fc4c720..c3e79936c 100644 --- a/components/led_strip/include/led_strip.hpp +++ b/components/led_strip/include/led_strip.hpp @@ -162,9 +162,9 @@ class LedStrip : public BaseComponent { , data_(other.data_) , write_(std::move(other.write_)) , use_dma_(other.use_dma_) { - // cppcheck-suppress-end accessMoved other.data_ = nullptr; other.data_size_ = 0; + // cppcheck-suppress-end accessMoved } /// \brief Move assignment operator @@ -194,11 +194,11 @@ class LedStrip : public BaseComponent { write_ = std::move(other.write_); use_dma_ = other.use_dma_; - // cppcheck-suppress-end accessMoved - // Nullify other's pointers other.data_ = nullptr; other.data_size_ = 0; + + // cppcheck-suppress-end accessMoved } return *this; } From 11fee8a8db3345e601b4eb9677304f298dc546ce Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Thu, 22 Jan 2026 09:04:18 -0600 Subject: [PATCH 11/11] clean up refactor to use span instead of raw pointer+size --- components/led_strip/README.md | 10 +++ components/led_strip/include/led_strip.hpp | 84 +++++++++++----------- components/led_strip/src/led_strip.cpp | 1 + components/t-dongle-s3/src/t-dongle-s3.cpp | 4 +- 4 files changed, 57 insertions(+), 42 deletions(-) mode change 100644 => 100755 components/led_strip/README.md mode change 100644 => 100755 components/led_strip/include/led_strip.hpp diff --git a/components/led_strip/README.md b/components/led_strip/README.md old mode 100644 new mode 100755 index 3cbe878fe..d7605a5d2 --- a/components/led_strip/README.md +++ b/components/led_strip/README.md @@ -8,6 +8,16 @@ You can use it directly with an SPI driver to talk to APA102 LED strips, or you can use it with a RMT driver (such as the `Rmt` component) to talk to WS2812, WS2811, WS2813, SK6812, etc. LED strips. +## Usage + +The Config struct accepts `std::span` for start and end frames, +allowing efficient use of memory. For backwards compatibility, you can: + +- Use empty frames: `.start_frame = {}` +- Use the predefined `LedStrip::APA102_START_FRAME` vector (auto-converts to span) +- Use `std::array` with CTAD: `.start_frame = std::array{0x00, 0x00, 0x00, 0x00}` +- Pass any existing vector or array that will be converted to a span + ## Example The [example](./example) shows the use of the `LedStrip` class to control a 5x5 diff --git a/components/led_strip/include/led_strip.hpp b/components/led_strip/include/led_strip.hpp old mode 100644 new mode 100755 index c3e79936c..dd0e6c4ff --- a/components/led_strip/include/led_strip.hpp +++ b/components/led_strip/include/led_strip.hpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include @@ -59,6 +60,8 @@ class LedStrip : public BaseComponent { /// \brief Start frame for the APA102 protocol static const std::vector APA102_START_FRAME; + /// \brief End frame for the APA102 protocol + static const std::vector APA102_END_FRAME; /// \brief Configuration for the LedStrip class struct Config { @@ -66,11 +69,13 @@ class LedStrip : public BaseComponent { write_fn write; ///< Function to write data to the strip bool send_brightness{true}; ///< Whether to use the brightness value for the LEDs ByteOrder byte_order{ByteOrder::RGB}; ///< Byte order for the LEDs - std::span start_frame{}; ///< Start frame for the strip. Optional - will be sent - ///< before the first LED if not empty. - std::span end_frame{}; ///< End frame for the strip. Optional - will be sent after - ///< the last LED if not empty. - bool use_dma{false}; ///< Whether to use DMA-capable memory allocation + std::span start_frame{}; ///< Start frame for the strip. Optional - will be sent + ///< before the first LED if not empty. Can be initialized + ///< with std::array{0x00, 0x00, ...} for compatibility. + std::span end_frame{}; ///< End frame for the strip. Optional - will be sent after + ///< the last LED if not empty. Can be initialized + ///< with std::array{0x00, 0x00, ...} for compatibility. + bool use_dma{false}; ///< Whether to use DMA-capable memory allocation uint32_t dma_allocation_flags{ MALLOC_CAP_DMA}; ///< DMA allocation flags (if use_dma is true). Defaults to MALLOC_CAP_DMA. Logger::Verbosity log_level; ///< Log level for this class @@ -92,39 +97,43 @@ class LedStrip : public BaseComponent { // set the color data size pixel_size_ = send_brightness_ ? 4 : 3; - data_size_ = num_leds_ * pixel_size_ + config.start_frame.size() + config.end_frame.size(); + size_t data_size = num_leds_ * pixel_size_ + config.start_frame.size() + config.end_frame.size(); // Allocate memory based on DMA preference + uint8_t *raw_data = nullptr; if (use_dma_) { uint32_t dma_flags = config.dma_allocation_flags; if (dma_flags == 0) { dma_flags = MALLOC_CAP_DMA; } - data_ = static_cast(heap_caps_malloc(data_size_, dma_flags)); - if (!data_) { + raw_data = static_cast(heap_caps_malloc(data_size, dma_flags)); + if (!raw_data) { logger_.warn("Failed to allocate DMA memory, falling back to regular malloc"); - data_ = static_cast(malloc(data_size_)); + raw_data = static_cast(malloc(data_size)); } } else { - data_ = static_cast(malloc(data_size_)); + raw_data = static_cast(malloc(data_size)); } - if (!data_) { + if (!raw_data) { logger_.error("Failed to allocate memory for LED strip data"); return; } + // create span from raw pointer + data_ = std::span(raw_data, data_size); + // zero out the data - memset(data_, 0, data_size_); + memset(data_.data(), 0, data_.size()); // copy the start frame if (!config.start_frame.empty()) { - memcpy(data_, config.start_frame.data(), config.start_frame.size()); + memcpy(data_.data(), config.start_frame.data(), config.start_frame.size()); } // copy the end frame if (!config.end_frame.empty()) { - memcpy(data_ + data_size_ - config.end_frame.size(), config.end_frame.data(), + memcpy(data_.data() + data_.size() - config.end_frame.size(), config.end_frame.data(), config.end_frame.size()); } @@ -135,9 +144,9 @@ class LedStrip : public BaseComponent { /// \brief Destructor /// \details This function frees the memory allocated for the LED strip data. ~LedStrip() { - if (data_) { - free(data_); - data_ = nullptr; + if (!data_.empty()) { + free(data_.data()); + data_ = std::span(); } } @@ -158,12 +167,10 @@ class LedStrip : public BaseComponent { , pixel_size_(other.pixel_size_) , start_offset_(other.start_offset_) , end_offset_(other.end_offset_) - , data_size_(other.data_size_) , data_(other.data_) , write_(std::move(other.write_)) , use_dma_(other.use_dma_) { - other.data_ = nullptr; - other.data_size_ = 0; + other.data_ = std::span(); // cppcheck-suppress-end accessMoved } @@ -173,8 +180,8 @@ class LedStrip : public BaseComponent { LedStrip &operator=(LedStrip &&other) noexcept { if (this != &other) { // Free existing data - if (data_) { - free(data_); + if (!data_.empty()) { + free(data_.data()); } // move-assign base class @@ -189,14 +196,12 @@ class LedStrip : public BaseComponent { pixel_size_ = other.pixel_size_; start_offset_ = other.start_offset_; end_offset_ = other.end_offset_; - data_size_ = other.data_size_; data_ = other.data_; write_ = std::move(other.write_); use_dma_ = other.use_dma_; - // Nullify other's pointers - other.data_ = nullptr; - other.data_size_ = 0; + // Nullify other's span + other.data_ = std::span(); // cppcheck-suppress-end accessMoved } @@ -216,7 +221,7 @@ class LedStrip : public BaseComponent { /// \return true if successful, false if data is invalid or shift_by is out of range /// \note A negative value for shift_by will shift the LEDs to the right bool shift_left(int shift_by = 1) { - if (!data_) { + if (data_.empty()) { logger_.error("No data allocated for LED strip"); return false; } @@ -228,8 +233,8 @@ class LedStrip : public BaseComponent { } if (shift_by < 0) shift_by += num_leds_; - std::rotate(data_ + start_offset_, data_ + start_offset_ + pixel_size_ * shift_by, - data_ + data_size_ - end_offset_); + std::rotate(data_.data() + start_offset_, data_.data() + start_offset_ + pixel_size_ * shift_by, + data_.data() + data_.size() - end_offset_); return true; } @@ -238,7 +243,7 @@ class LedStrip : public BaseComponent { /// \return true if successful, false if data is invalid or shift_by is out of range /// \note A negative value for shift_by will shift the LEDs to the left bool shift_right(int shift_by = 1) { - if (!data_) { + if (data_.empty()) { logger_.error("No data allocated for LED strip"); return false; } @@ -250,10 +255,10 @@ class LedStrip : public BaseComponent { } if (shift_by < 0) shift_by += num_leds_; - std::rotate(std::reverse_iterator(data_ + data_size_ - end_offset_), - std::reverse_iterator(data_ + data_size_ - end_offset_) + + std::rotate(std::reverse_iterator(data_.data() + data_.size() - end_offset_), + std::reverse_iterator(data_.data() + data_.size() - end_offset_) + pixel_size_ * shift_by, - std::reverse_iterator(data_ + start_offset_)); + std::reverse_iterator(data_.data() + start_offset_)); return true; } @@ -296,7 +301,7 @@ class LedStrip : public BaseComponent { /// \note The index is zero-based. /// \sa show bool set_pixel(int index, uint8_t r, uint8_t g, uint8_t b, uint8_t brightness = 0b11111) { - if (!data_) { + if (data_.empty()) { logger_.error("No data allocated for LED strip"); return false; } @@ -365,7 +370,7 @@ class LedStrip : public BaseComponent { /// \sa set_pixel /// \sa show bool set_all(uint8_t r, uint8_t g, uint8_t b, uint8_t brightness = 0xff) { - if (!data_) { + if (data_.empty()) { logger_.error("No data allocated for LED strip"); return false; } @@ -387,12 +392,12 @@ class LedStrip : public BaseComponent { /// \sa set_pixel /// \sa set_all bool show() { - if (!data_) { + if (data_.empty()) { logger_.error("No data allocated for LED strip"); return false; } - logger_.debug("writing data {::02x}", std::span(data_, data_ + data_size_)); - write_(data_, data_size_); + logger_.debug("writing data {::02x}", data_); + write_(data_.data(), data_.size()); return true; } @@ -403,8 +408,7 @@ class LedStrip : public BaseComponent { size_t pixel_size_{3}; size_t start_offset_{0}; size_t end_offset_{0}; - size_t data_size_{0}; - uint8_t *data_{nullptr}; + std::span data_; write_fn write_; bool use_dma_{false}; }; diff --git a/components/led_strip/src/led_strip.cpp b/components/led_strip/src/led_strip.cpp index 798a5aa05..45c9e1467 100644 --- a/components/led_strip/src/led_strip.cpp +++ b/components/led_strip/src/led_strip.cpp @@ -1,3 +1,4 @@ #include "led_strip.hpp" const std::vector espp::LedStrip::APA102_START_FRAME{0x00, 0x00, 0x00, 0x00}; +const std::vector espp::LedStrip::APA102_END_FRAME{0xFF, 0xFF, 0xFF, 0xFF}; diff --git a/components/t-dongle-s3/src/t-dongle-s3.cpp b/components/t-dongle-s3/src/t-dongle-s3.cpp index a7b8ddfb5..0b01f2b4b 100644 --- a/components/t-dongle-s3/src/t-dongle-s3.cpp +++ b/components/t-dongle-s3/src/t-dongle-s3.cpp @@ -82,8 +82,8 @@ bool TDongleS3::initialize_led() { .write = std::bind(&TDongleS3::led_write, this, std::placeholders::_1, std::placeholders::_2), .send_brightness = true, .byte_order = espp::LedStrip::ByteOrder::BGR, - .start_frame = {0x00, 0x00, 0x00, 0x00}, // APA102 start frame - .end_frame = {0xFF, 0xFF, 0xFF, 0xFF}, // APA102 end frame + .start_frame = std::array{0x00, 0x00, 0x00, 0x00}, // APA102 start frame + .end_frame = std::array{0xFF, 0xFF, 0xFF, 0xFF}, // APA102 end frame .log_level = espp::Logger::Verbosity::INFO}); led_->set_all(espp::Rgb(0, 0, 0)); led_->show();