From afbc45bf32fe8146183d0d44fc16213cdebddbdb Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 19 Feb 2026 20:35:42 -0600 Subject: [PATCH 1/5] [e131] Drain all queued packets per loop iteration (#14133) --- esphome/components/e131/e131.cpp | 26 +++++--------------------- esphome/components/e131/e131.h | 9 +++++++++ 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/esphome/components/e131/e131.cpp b/esphome/components/e131/e131.cpp index 941927122c..a7a695c167 100644 --- a/esphome/components/e131/e131.cpp +++ b/esphome/components/e131/e131.cpp @@ -70,27 +70,12 @@ void E131Component::loop() { E131Packet packet; int universe = 0; uint8_t buf[1460]; + ssize_t len; -#if defined(USE_SOCKET_IMPL_BSD_SOCKETS) || defined(USE_SOCKET_IMPL_LWIP_SOCKETS) - ssize_t len = this->socket_->read(buf, sizeof(buf)); - if (len == -1) { - return; - } - - if (!this->packet_(buf, (size_t) len, universe, packet)) { - ESP_LOGV(TAG, "Invalid packet received of size %d.", (int) len); - return; - } - - if (!this->process_(universe, packet)) { - ESP_LOGV(TAG, "Ignored packet for %d universe of size %d.", universe, packet.count); - } -#elif defined(USE_SOCKET_IMPL_LWIP_TCP) - while (auto packet_size = this->udp_.parsePacket()) { - auto len = this->udp_.read(buf, sizeof(buf)); - if (len <= 0) - continue; - + // Drain all queued packets so multi-universe frames are applied + // atomically before the light writes. Without this, each universe + // packet would trigger a separate full-strip write causing tearing. + while ((len = this->read_(buf, sizeof(buf))) > 0) { if (!this->packet_(buf, (size_t) len, universe, packet)) { ESP_LOGV(TAG, "Invalid packet received of size %d.", (int) len); continue; @@ -100,7 +85,6 @@ void E131Component::loop() { ESP_LOGV(TAG, "Ignored packet for %d universe of size %d.", universe, packet.count); } } -#endif } void E131Component::add_effect(E131AddressableLightEffect *light_effect) { diff --git a/esphome/components/e131/e131.h b/esphome/components/e131/e131.h index fee447b678..8f0b808946 100644 --- a/esphome/components/e131/e131.h +++ b/esphome/components/e131/e131.h @@ -46,6 +46,15 @@ class E131Component : public esphome::Component { void set_method(E131ListenMethod listen_method) { this->listen_method_ = listen_method; } protected: + inline ssize_t read_(uint8_t *buf, size_t len) { +#if defined(USE_SOCKET_IMPL_BSD_SOCKETS) || defined(USE_SOCKET_IMPL_LWIP_SOCKETS) + return this->socket_->read(buf, len); +#elif defined(USE_SOCKET_IMPL_LWIP_TCP) + if (!this->udp_.parsePacket()) + return -1; + return this->udp_.read(buf, len); +#endif + } bool packet_(const uint8_t *data, size_t len, int &universe, E131Packet &packet); bool process_(int universe, const E131Packet &packet); bool join_igmp_groups_(); From 7a2a149061c3b9480a3d1900f3e8cad1f1ad75ac Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Thu, 19 Feb 2026 21:43:29 -0500 Subject: [PATCH 2/5] [esp32] Bump ESP-IDF to 5.5.3 (#14122) Co-authored-by: Claude Opus 4.6 --- .clang-tidy.hash | 2 +- esphome/components/esp32/__init__.py | 17 +++++++++++++---- esphome/components/esp32_ble/__init__.py | 15 +++++++++------ platformio.ini | 4 ++-- 4 files changed, 25 insertions(+), 13 deletions(-) diff --git a/.clang-tidy.hash b/.clang-tidy.hash index d6d401ee66..df584fa716 100644 --- a/.clang-tidy.hash +++ b/.clang-tidy.hash @@ -1 +1 @@ -ce05c28e9dc0b12c4f6e7454986ffea5123ac974a949da841be698c535f2083e +3258307fa645ba77307e502075c02c4d710e92c48250839db3526d36a9655444 diff --git a/esphome/components/esp32/__init__.py b/esphome/components/esp32/__init__.py index b1b3f0dc16..cdde1c4ed5 100644 --- a/esphome/components/esp32/__init__.py +++ b/esphome/components/esp32/__init__.py @@ -643,7 +643,7 @@ ARDUINO_PLATFORM_VERSION_LOOKUP = { # These versions correspond to pioarduino/esp-idf releases # See: https://github.com/pioarduino/esp-idf/releases ARDUINO_IDF_VERSION_LOOKUP = { - cv.Version(3, 3, 7): cv.Version(5, 5, 2), + cv.Version(3, 3, 7): cv.Version(5, 5, 3), cv.Version(3, 3, 6): cv.Version(5, 5, 2), cv.Version(3, 3, 5): cv.Version(5, 5, 2), cv.Version(3, 3, 4): cv.Version(5, 5, 1), @@ -662,11 +662,12 @@ ARDUINO_IDF_VERSION_LOOKUP = { # The default/recommended esp-idf framework version # - https://github.com/espressif/esp-idf/releases ESP_IDF_FRAMEWORK_VERSION_LOOKUP = { - "recommended": cv.Version(5, 5, 2), - "latest": cv.Version(5, 5, 2), - "dev": cv.Version(5, 5, 2), + "recommended": cv.Version(5, 5, 3), + "latest": cv.Version(5, 5, 3), + "dev": cv.Version(5, 5, 3), } ESP_IDF_PLATFORM_VERSION_LOOKUP = { + cv.Version(5, 5, 3): cv.Version(55, 3, 37), cv.Version(5, 5, 2): cv.Version(55, 3, 37), cv.Version(5, 5, 1): cv.Version(55, 3, 31, "2"), cv.Version(5, 5, 0): cv.Version(55, 3, 31, "2"), @@ -1471,6 +1472,14 @@ async def to_code(config): f"CONFIG_ESPTOOLPY_FLASHSIZE_{config[CONF_FLASH_SIZE]}", True ) + # ESP32-P4: ESP-IDF 5.5.3 changed the default of ESP32P4_SELECTS_REV_LESS_V3 + # from y to n. PlatformIO uses sections.ld.in (for rev <3) or + # sections.rev3.ld.in (for rev >=3) based on board definition. + # Set the sdkconfig option to match the board's revision. + if variant == VARIANT_ESP32P4: + is_rev3 = "_r3" in config[CONF_BOARD] + add_idf_sdkconfig_option("CONFIG_ESP32P4_SELECTS_REV_LESS_V3", not is_rev3) + # Set minimum chip revision for ESP32 variant # Setting this to 3.0 or higher reduces flash size by excluding workaround code, # and for PSRAM users saves significant IRAM by keeping C library functions in ROM. diff --git a/esphome/components/esp32_ble/__init__.py b/esphome/components/esp32_ble/__init__.py index d2020ada22..80fcf051b9 100644 --- a/esphome/components/esp32_ble/__init__.py +++ b/esphome/components/esp32_ble/__init__.py @@ -413,18 +413,21 @@ def final_validation(config): add_idf_sdkconfig_option("CONFIG_ESP_HOSTED_ENABLE_BT_BLUEDROID", True) add_idf_sdkconfig_option("CONFIG_ESP_HOSTED_BLUEDROID_HCI_VHCI", True) - # Check if BLE Server is needed - has_ble_server = "esp32_ble_server" in full_config - # Check if BLE Client is needed (via esp32_ble_tracker or esp32_ble_client) has_ble_client = ( "esp32_ble_tracker" in full_config or "esp32_ble_client" in full_config ) - # ESP-IDF BLE stack requires GATT Server to be enabled when GATT Client is enabled - # This is an internal dependency in the Bluedroid stack (tested ESP-IDF 5.4.2-5.5.1) + # Always enable GATTS: ESP-IDF 5.5.2.260206 has a bug in gatt_main.c where a + # GATT_TRACE_DEBUG references 'msg_len' outside the GATTS_INCLUDED/GATTC_INCLUDED + # guard, causing a compile error when both are disabled. + # Additionally, when GATT Client is enabled, GATT Server must also be enabled + # as an internal dependency in the Bluedroid stack. # See: https://github.com/espressif/esp-idf/issues/17724 - add_idf_sdkconfig_option("CONFIG_BT_GATTS_ENABLE", has_ble_server or has_ble_client) + # TODO: Revert to conditional once the gatt_main.c bug is fixed upstream: + # has_ble_server = "esp32_ble_server" in full_config + # add_idf_sdkconfig_option("CONFIG_BT_GATTS_ENABLE", has_ble_server or has_ble_client) + add_idf_sdkconfig_option("CONFIG_BT_GATTS_ENABLE", True) add_idf_sdkconfig_option("CONFIG_BT_GATTC_ENABLE", has_ble_client) # Handle max_connections: check for deprecated location in esp32_ble_tracker diff --git a/platformio.ini b/platformio.ini index 09b3d8722d..fdd6a36428 100644 --- a/platformio.ini +++ b/platformio.ini @@ -136,7 +136,7 @@ extends = common:arduino platform = https://github.com/pioarduino/platform-espressif32/releases/download/55.03.37/platform-espressif32.zip platform_packages = pioarduino/framework-arduinoespressif32@https://github.com/espressif/arduino-esp32/releases/download/3.3.7/esp32-core-3.3.7.tar.xz - pioarduino/framework-espidf@https://github.com/pioarduino/esp-idf/releases/download/v5.5.2/esp-idf-v5.5.2.tar.xz + pioarduino/framework-espidf@https://github.com/pioarduino/esp-idf/releases/download/v5.5.3/esp-idf-v5.5.3.tar.xz framework = arduino, espidf ; Arduino as an ESP-IDF component lib_deps = @@ -171,7 +171,7 @@ extra_scripts = post:esphome/components/esp32/post_build.py.script extends = common:idf platform = https://github.com/pioarduino/platform-espressif32/releases/download/55.03.37/platform-espressif32.zip platform_packages = - pioarduino/framework-espidf@https://github.com/pioarduino/esp-idf/releases/download/v5.5.2/esp-idf-v5.5.2.tar.xz + pioarduino/framework-espidf@https://github.com/pioarduino/esp-idf/releases/download/v5.5.3/esp-idf-v5.5.3.tar.xz framework = espidf lib_deps = From b67b2cc3ab86ebbc7cfb3d0a893bf96bd50ef9a6 Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Thu, 19 Feb 2026 21:56:20 -0500 Subject: [PATCH 3/5] [ld2450] Add frame header synchronization to fix initialization regression (#14135) Co-authored-by: Claude Opus 4.6 Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> --- esphome/components/ld2450/ld2450.cpp | 22 ++- esphome/components/ld2450/ld2450.h | 1 + tests/components/ld2450/common.h | 61 +++++++ tests/components/ld2450/ld2450_readline.cpp | 181 ++++++++++++++++++++ 4 files changed, 263 insertions(+), 2 deletions(-) create mode 100644 tests/components/ld2450/common.h create mode 100644 tests/components/ld2450/ld2450_readline.cpp diff --git a/esphome/components/ld2450/ld2450.cpp b/esphome/components/ld2450/ld2450.cpp index 1ea5c18271..2af45235a3 100644 --- a/esphome/components/ld2450/ld2450.cpp +++ b/esphome/components/ld2450/ld2450.cpp @@ -769,15 +769,33 @@ void LD2450Component::readline_(int readch) { return; // No data available } + // Frame header synchronization: verify first 4 bytes match a known frame header. + // This prevents the parser from accumulating mid-frame data after losing sync + // (e.g. after module restart or UART noise). + if (this->buffer_pos_ < HEADER_FOOTER_SIZE) { + const uint8_t byte = static_cast(readch); + // Verify header bytes match the frame type established by byte 0 + if (this->buffer_pos_ > 0) { + const uint8_t *expected = (this->buffer_data_[0] == DATA_FRAME_HEADER[0]) ? DATA_FRAME_HEADER : CMD_FRAME_HEADER; + if (byte != expected[this->buffer_pos_]) { + this->buffer_pos_ = 0; // Reset and fall through to check if this byte starts a new frame + } + } + // First byte must match start of a data or command frame header + if (this->buffer_pos_ == 0 && byte != DATA_FRAME_HEADER[0] && byte != CMD_FRAME_HEADER[0]) { + return; + } + } + if (this->buffer_pos_ < MAX_LINE_LENGTH - 1) { this->buffer_data_[this->buffer_pos_++] = readch; this->buffer_data_[this->buffer_pos_] = 0; } else { - // We should never get here, but just in case... ESP_LOGW(TAG, "Max command length exceeded; ignoring"); this->buffer_pos_ = 0; + return; } - if (this->buffer_pos_ < 4) { + if (this->buffer_pos_ < HEADER_FOOTER_SIZE) { return; // Not enough data to process yet } if (this->buffer_data_[this->buffer_pos_ - 2] == DATA_FRAME_FOOTER[0] && diff --git a/esphome/components/ld2450/ld2450.h b/esphome/components/ld2450/ld2450.h index fe69cd81d0..44e5912b2a 100644 --- a/esphome/components/ld2450/ld2450.h +++ b/esphome/components/ld2450/ld2450.h @@ -1,5 +1,6 @@ #pragma once +#include "esphome/core/automation.h" #include "esphome/core/defines.h" #include "esphome/core/component.h" #ifdef USE_SENSOR diff --git a/tests/components/ld2450/common.h b/tests/components/ld2450/common.h new file mode 100644 index 0000000000..d5ffbe1295 --- /dev/null +++ b/tests/components/ld2450/common.h @@ -0,0 +1,61 @@ +#pragma once +#include +#include +#include +#include +#include +#include "esphome/components/ld2450/ld2450.h" +#include "esphome/components/uart/uart_component.h" + +namespace esphome::ld2450::testing { + +// Mock UART component to satisfy UARTDevice parent requirement. +class MockUARTComponent : public uart::UARTComponent { + public: + void write_array(const uint8_t *data, size_t len) override {} + MOCK_METHOD(bool, read_array, (uint8_t * data, size_t len), (override)); + MOCK_METHOD(bool, peek_byte, (uint8_t * data), (override)); + MOCK_METHOD(size_t, available, (), (override)); + MOCK_METHOD(void, flush, (), (override)); + MOCK_METHOD(void, check_logger_conflict, (), (override)); +}; + +// Expose protected members for testing. +class TestableLD2450 : public LD2450Component { + public: + using LD2450Component::buffer_data_; + using LD2450Component::buffer_pos_; + using LD2450Component::readline_; + + void feed(const std::vector &data) { + for (uint8_t byte : data) { + this->readline_(byte); + } + } +}; + +// LD2450 periodic data frame: header (4) + 3 targets * 8 bytes + footer (2) = 30 bytes +// All-zero targets means no presence detected. +inline std::vector make_periodic_frame(uint8_t fill = 0x00) { + std::vector frame = {0xAA, 0xFF, 0x03, 0x00}; // DATA_FRAME_HEADER + for (int i = 0; i < 24; i++) { + frame.push_back(fill); // 3 targets * 8 bytes + } + frame.push_back(0x55); // DATA_FRAME_FOOTER + frame.push_back(0xCC); + return frame; +} + +// LD2450 command ACK frame for CMD_ENABLE_CONF (0xFF), successful. +// header (4) + length (2) + command (2) + result (2) + footer (4) = 14 bytes +inline std::vector make_ack_frame() { + return { + 0xFD, 0xFC, 0xFB, 0xFA, // CMD_FRAME_HEADER + 0x04, 0x00, // length = 4 + 0xFF, 0x01, // command = enable_conf, status = success + 0x00, 0x00, // result = ok + 0x04, 0x03, 0x02, 0x01 // CMD_FRAME_FOOTER + }; +} + +} // namespace esphome::ld2450::testing diff --git a/tests/components/ld2450/ld2450_readline.cpp b/tests/components/ld2450/ld2450_readline.cpp new file mode 100644 index 0000000000..68b1dd6881 --- /dev/null +++ b/tests/components/ld2450/ld2450_readline.cpp @@ -0,0 +1,181 @@ +#include "common.h" + +namespace esphome::ld2450::testing { + +class LD2450ReadlineTest : public ::testing::Test { + protected: + void SetUp() override { + this->ld2450_.set_uart_parent(&this->mock_uart_); + // Ensure clean state + ASSERT_EQ(this->ld2450_.buffer_pos_, 0); + } + + MockUARTComponent mock_uart_; + TestableLD2450 ld2450_; +}; + +// --- Good data tests --- + +TEST_F(LD2450ReadlineTest, ValidPeriodicFrame) { + auto frame = make_periodic_frame(); + this->ld2450_.feed(frame); + // After a complete valid frame, buffer should be reset + EXPECT_EQ(this->ld2450_.buffer_pos_, 0); +} + +TEST_F(LD2450ReadlineTest, ValidCommandAckFrame) { + auto frame = make_ack_frame(); + this->ld2450_.feed(frame); + EXPECT_EQ(this->ld2450_.buffer_pos_, 0); +} + +TEST_F(LD2450ReadlineTest, BackToBackPeriodicFrames) { + auto frame = make_periodic_frame(); + for (int i = 0; i < 5; i++) { + this->ld2450_.feed(frame); + EXPECT_EQ(this->ld2450_.buffer_pos_, 0) << "Frame " << i << " not processed"; + } +} + +TEST_F(LD2450ReadlineTest, BackToBackMixedFrames) { + auto periodic = make_periodic_frame(); + auto ack = make_ack_frame(); + this->ld2450_.feed(periodic); + EXPECT_EQ(this->ld2450_.buffer_pos_, 0); + this->ld2450_.feed(ack); + EXPECT_EQ(this->ld2450_.buffer_pos_, 0); + this->ld2450_.feed(periodic); + EXPECT_EQ(this->ld2450_.buffer_pos_, 0); +} + +// --- Garbage rejection tests --- + +TEST_F(LD2450ReadlineTest, GarbageDiscarded) { + // Feed bytes that don't match any header start byte + std::vector garbage = {0x01, 0x02, 0x03, 0x42, 0x99, 0x00, 0xFF, 0x7F}; + this->ld2450_.feed(garbage); + // Header sync should discard all of these + EXPECT_EQ(this->ld2450_.buffer_pos_, 0); +} + +TEST_F(LD2450ReadlineTest, GarbageThenValidFrame) { + std::vector garbage = {0x01, 0x02, 0x03, 0x42, 0x99}; + this->ld2450_.feed(garbage); + EXPECT_EQ(this->ld2450_.buffer_pos_, 0); + + auto frame = make_periodic_frame(); + this->ld2450_.feed(frame); + EXPECT_EQ(this->ld2450_.buffer_pos_, 0); +} + +// --- Header synchronization tests --- + +TEST_F(LD2450ReadlineTest, PartialDataHeaderThenMismatch) { + // Start of a data frame header, then invalid byte + this->ld2450_.feed({0xAA, 0xFF, 0x42}); // 0x42 doesn't match DATA_FRAME_HEADER[2] (0x03) + // Parser should have reset + EXPECT_EQ(this->ld2450_.buffer_pos_, 0); +} + +TEST_F(LD2450ReadlineTest, PartialCmdHeaderThenMismatch) { + // Start of a command frame header, then invalid byte + this->ld2450_.feed({0xFD, 0xFC, 0xFB, 0x42}); // 0x42 doesn't match CMD_FRAME_HEADER[3] (0xFA) + EXPECT_EQ(this->ld2450_.buffer_pos_, 0); +} + +TEST_F(LD2450ReadlineTest, PartialHeaderThenValidFrame) { + // Partial header that fails, then a complete valid frame + this->ld2450_.feed({0xAA, 0xFF, 0x42}); // Fails at byte 3 + EXPECT_EQ(this->ld2450_.buffer_pos_, 0); + + auto frame = make_periodic_frame(); + this->ld2450_.feed(frame); + EXPECT_EQ(this->ld2450_.buffer_pos_, 0); +} + +TEST_F(LD2450ReadlineTest, HeaderMismatchRecoveryOnNewHeaderByte) { + // Start data header, mismatch at byte 2, but mismatch byte is start of command header + this->ld2450_.feed({0xAA, 0xFF}); + EXPECT_EQ(this->ld2450_.buffer_pos_, 2); // Accumulating header + + this->ld2450_.feed({0xFD}); // Doesn't match DATA_FRAME_HEADER[2]=0x03, but IS CMD_FRAME_HEADER[0] + // Parser should reset and start new frame with 0xFD + EXPECT_EQ(this->ld2450_.buffer_pos_, 1); + EXPECT_EQ(this->ld2450_.buffer_data_[0], 0xFD); +} + +// --- Mid-frame / overflow recovery tests --- + +TEST_F(LD2450ReadlineTest, MidFrameDataRecovery) { + // Simulate starting mid-frame: feed the tail end of a periodic frame (no valid header) + // These bytes would be part of target data in a real frame + std::vector mid_frame = {0x10, 0x20, 0x30, 0x40, 0x55, 0xCC}; + this->ld2450_.feed(mid_frame); + // All discarded (none match header start bytes) + EXPECT_EQ(this->ld2450_.buffer_pos_, 0); + + // Now feed a valid frame + auto frame = make_periodic_frame(); + this->ld2450_.feed(frame); + EXPECT_EQ(this->ld2450_.buffer_pos_, 0); +} + +TEST_F(LD2450ReadlineTest, OverflowRecovery) { + // Feed a valid data frame header followed by enough filler to cause overflow. + // Header (4) + 36 filler = 40 bytes in buffer. The 41st byte triggers overflow. + std::vector overflow_data = {0xAA, 0xFF, 0x03, 0x00}; // Valid header + for (int i = 0; i < 37; i++) { + overflow_data.push_back(0x11); // Filler that won't match any footer + } + // 41 bytes total: 40 stored, 41st triggers overflow and resets buffer_pos_ to 0 + this->ld2450_.feed(overflow_data); + EXPECT_EQ(this->ld2450_.buffer_pos_, 0); + + // Feed a valid frame and verify recovery + auto frame = make_periodic_frame(); + this->ld2450_.feed(frame); + EXPECT_EQ(this->ld2450_.buffer_pos_, 0); +} + +TEST_F(LD2450ReadlineTest, RepeatedOverflowDoesNotLoop) { + // Simulate the bug scenario: repeated overflows should not prevent recovery. + // Feed 3 rounds of overflow-inducing data. + for (int round = 0; round < 3; round++) { + std::vector overflow_data = {0xAA, 0xFF, 0x03, 0x00}; + for (int i = 0; i < 37; i++) { + overflow_data.push_back(0x22); + } + this->ld2450_.feed(overflow_data); + EXPECT_EQ(this->ld2450_.buffer_pos_, 0) << "Overflow round " << round; + } + + // Parser should still recover and process a valid frame + auto frame = make_periodic_frame(); + this->ld2450_.feed(frame); + EXPECT_EQ(this->ld2450_.buffer_pos_, 0); +} + +TEST_F(LD2450ReadlineTest, SimulatedRestartGarbageThenFrames) { + // Simulate LD2450 restart: burst of garbage bytes (partial frames, noise) + // followed by normal periodic data. + // Partial periodic frame (as if we started reading mid-frame), a stale footer, and more garbage + std::vector restart_noise = { + 0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x38, 0x39, 0x3A, 0x3B, 0x3C, 0x3D, 0x3E, // mid-frame data + 0x55, 0xCC, // stale footer bytes + 0x80, 0x81, 0x82, 0x83, 0x84, 0x85, 0x86, 0x87, 0x88, 0x89, // more garbage + }; + + this->ld2450_.feed(restart_noise); + // All garbage should be discarded + EXPECT_EQ(this->ld2450_.buffer_pos_, 0); + + // Now the LD2450 starts sending valid frames + auto frame = make_periodic_frame(); + this->ld2450_.feed(frame); + EXPECT_EQ(this->ld2450_.buffer_pos_, 0); + + this->ld2450_.feed(frame); + EXPECT_EQ(this->ld2450_.buffer_pos_, 0); +} + +} // namespace esphome::ld2450::testing From a2f0607c1e860d1173c80e0a763fda99e71e69f6 Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Thu, 19 Feb 2026 22:04:38 -0500 Subject: [PATCH 4/5] [ld2410] Add frame header synchronization to readline_() (#14136) Co-authored-by: Claude Opus 4.6 --- esphome/components/ld2410/ld2410.cpp | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/esphome/components/ld2410/ld2410.cpp b/esphome/components/ld2410/ld2410.cpp index 95a04f768a..a3c2193d67 100644 --- a/esphome/components/ld2410/ld2410.cpp +++ b/esphome/components/ld2410/ld2410.cpp @@ -601,15 +601,33 @@ void LD2410Component::readline_(int readch) { return; // No data available } + // Frame header synchronization: verify first 4 bytes match a known frame header. + // This prevents the parser from getting stuck in an overflow loop after losing sync + // (e.g. after module restart or UART noise). + if (this->buffer_pos_ < HEADER_FOOTER_SIZE) { + const uint8_t byte = static_cast(readch); + // Verify header bytes match the frame type established by byte 0 + if (this->buffer_pos_ > 0) { + const uint8_t *expected = (this->buffer_data_[0] == DATA_FRAME_HEADER[0]) ? DATA_FRAME_HEADER : CMD_FRAME_HEADER; + if (byte != expected[this->buffer_pos_]) { + this->buffer_pos_ = 0; // Reset and fall through to check if this byte starts a new frame + } + } + // First byte must match start of a data or command frame header + if (this->buffer_pos_ == 0 && byte != DATA_FRAME_HEADER[0] && byte != CMD_FRAME_HEADER[0]) { + return; + } + } + if (this->buffer_pos_ < MAX_LINE_LENGTH - 1) { this->buffer_data_[this->buffer_pos_++] = readch; this->buffer_data_[this->buffer_pos_] = 0; } else { - // We should never get here, but just in case... ESP_LOGW(TAG, "Max command length exceeded; ignoring"); this->buffer_pos_ = 0; + return; } - if (this->buffer_pos_ < 4) { + if (this->buffer_pos_ < HEADER_FOOTER_SIZE) { return; // Not enough data to process yet } if (ld2410::validate_header_footer(DATA_FRAME_FOOTER, &this->buffer_data_[this->buffer_pos_ - 4])) { From dbe3dfb2657709918f332a79e65eabad28b3e28f Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Thu, 19 Feb 2026 22:38:33 -0500 Subject: [PATCH 5/5] [ld2410/ld2450] Replace header sync with buffer size increase for frame resync Reverts the frame header synchronization added in #14135 and #14136 in favor of a simpler fix: increasing MAX_LINE_LENGTH so that the existing footer-based resynchronization can recover after losing sync. Both components already check for frame footers at every byte position, which naturally resyncs the parser. The problem was that the buffers were sized exactly to fit the largest frame, so a desynced parser's footer could land at the overflow boundary and get discarded. Increasing the buffer by 4 bytes (footer size) ensures the footer always lands inside the buffer. - ld2450: 41 -> 45 (zone query response = 40 bytes + 1 null + 4 footer) - ld2410: 46 -> 50 (engineering data frame = 45 bytes + 1 null + 4 footer) Co-Authored-By: Claude Opus 4.6 --- esphome/components/ld2410/ld2410.cpp | 19 +-- esphome/components/ld2410/ld2410.h | 6 +- esphome/components/ld2450/ld2450.cpp | 19 +-- esphome/components/ld2450/ld2450.h | 8 +- tests/components/ld2450/ld2450_readline.cpp | 166 +++++++++----------- 5 files changed, 81 insertions(+), 137 deletions(-) diff --git a/esphome/components/ld2410/ld2410.cpp b/esphome/components/ld2410/ld2410.cpp index a3c2193d67..f8f782f804 100644 --- a/esphome/components/ld2410/ld2410.cpp +++ b/esphome/components/ld2410/ld2410.cpp @@ -601,28 +601,11 @@ void LD2410Component::readline_(int readch) { return; // No data available } - // Frame header synchronization: verify first 4 bytes match a known frame header. - // This prevents the parser from getting stuck in an overflow loop after losing sync - // (e.g. after module restart or UART noise). - if (this->buffer_pos_ < HEADER_FOOTER_SIZE) { - const uint8_t byte = static_cast(readch); - // Verify header bytes match the frame type established by byte 0 - if (this->buffer_pos_ > 0) { - const uint8_t *expected = (this->buffer_data_[0] == DATA_FRAME_HEADER[0]) ? DATA_FRAME_HEADER : CMD_FRAME_HEADER; - if (byte != expected[this->buffer_pos_]) { - this->buffer_pos_ = 0; // Reset and fall through to check if this byte starts a new frame - } - } - // First byte must match start of a data or command frame header - if (this->buffer_pos_ == 0 && byte != DATA_FRAME_HEADER[0] && byte != CMD_FRAME_HEADER[0]) { - return; - } - } - if (this->buffer_pos_ < MAX_LINE_LENGTH - 1) { this->buffer_data_[this->buffer_pos_++] = readch; this->buffer_data_[this->buffer_pos_] = 0; } else { + // We should never get here, but just in case... ESP_LOGW(TAG, "Max command length exceeded; ignoring"); this->buffer_pos_ = 0; return; diff --git a/esphome/components/ld2410/ld2410.h b/esphome/components/ld2410/ld2410.h index efe585fb76..687ed21d1d 100644 --- a/esphome/components/ld2410/ld2410.h +++ b/esphome/components/ld2410/ld2410.h @@ -33,8 +33,10 @@ namespace esphome::ld2410 { using namespace ld24xx; -static constexpr uint8_t MAX_LINE_LENGTH = 46; // Max characters for serial buffer -static constexpr uint8_t TOTAL_GATES = 9; // Total number of gates supported by the LD2410 +// Engineering data frame is 45 bytes; +1 for null terminator, +4 so that a frame footer always +// lands inside the buffer during footer-based resynchronization after losing sync. +static constexpr uint8_t MAX_LINE_LENGTH = 50; +static constexpr uint8_t TOTAL_GATES = 9; // Total number of gates supported by the LD2410 class LD2410Component : public Component, public uart::UARTDevice { #ifdef USE_BINARY_SENSOR diff --git a/esphome/components/ld2450/ld2450.cpp b/esphome/components/ld2450/ld2450.cpp index 2af45235a3..d30c164769 100644 --- a/esphome/components/ld2450/ld2450.cpp +++ b/esphome/components/ld2450/ld2450.cpp @@ -769,28 +769,11 @@ void LD2450Component::readline_(int readch) { return; // No data available } - // Frame header synchronization: verify first 4 bytes match a known frame header. - // This prevents the parser from accumulating mid-frame data after losing sync - // (e.g. after module restart or UART noise). - if (this->buffer_pos_ < HEADER_FOOTER_SIZE) { - const uint8_t byte = static_cast(readch); - // Verify header bytes match the frame type established by byte 0 - if (this->buffer_pos_ > 0) { - const uint8_t *expected = (this->buffer_data_[0] == DATA_FRAME_HEADER[0]) ? DATA_FRAME_HEADER : CMD_FRAME_HEADER; - if (byte != expected[this->buffer_pos_]) { - this->buffer_pos_ = 0; // Reset and fall through to check if this byte starts a new frame - } - } - // First byte must match start of a data or command frame header - if (this->buffer_pos_ == 0 && byte != DATA_FRAME_HEADER[0] && byte != CMD_FRAME_HEADER[0]) { - return; - } - } - if (this->buffer_pos_ < MAX_LINE_LENGTH - 1) { this->buffer_data_[this->buffer_pos_++] = readch; this->buffer_data_[this->buffer_pos_] = 0; } else { + // We should never get here, but just in case... ESP_LOGW(TAG, "Max command length exceeded; ignoring"); this->buffer_pos_ = 0; return; diff --git a/esphome/components/ld2450/ld2450.h b/esphome/components/ld2450/ld2450.h index 44e5912b2a..30f96c0a9c 100644 --- a/esphome/components/ld2450/ld2450.h +++ b/esphome/components/ld2450/ld2450.h @@ -38,9 +38,11 @@ using namespace ld24xx; // Constants static constexpr uint8_t DEFAULT_PRESENCE_TIMEOUT = 5; // Timeout to reset presense status 5 sec. -static constexpr uint8_t MAX_LINE_LENGTH = 41; // Max characters for serial buffer -static constexpr uint8_t MAX_TARGETS = 3; // Max 3 Targets in LD2450 -static constexpr uint8_t MAX_ZONES = 3; // Max 3 Zones in LD2450 +// Zone query response is 40 bytes; +1 for null terminator, +4 so that a frame footer always +// lands inside the buffer during footer-based resynchronization after losing sync. +static constexpr uint8_t MAX_LINE_LENGTH = 45; +static constexpr uint8_t MAX_TARGETS = 3; // Max 3 Targets in LD2450 +static constexpr uint8_t MAX_ZONES = 3; // Max 3 Zones in LD2450 enum Direction : uint8_t { DIRECTION_APPROACHING = 0, diff --git a/tests/components/ld2450/ld2450_readline.cpp b/tests/components/ld2450/ld2450_readline.cpp index 68b1dd6881..66dec7a36b 100644 --- a/tests/components/ld2450/ld2450_readline.cpp +++ b/tests/components/ld2450/ld2450_readline.cpp @@ -48,19 +48,39 @@ TEST_F(LD2450ReadlineTest, BackToBackMixedFrames) { EXPECT_EQ(this->ld2450_.buffer_pos_, 0); } -// --- Garbage rejection tests --- - -TEST_F(LD2450ReadlineTest, GarbageDiscarded) { - // Feed bytes that don't match any header start byte - std::vector garbage = {0x01, 0x02, 0x03, 0x42, 0x99, 0x00, 0xFF, 0x7F}; - this->ld2450_.feed(garbage); - // Header sync should discard all of these - EXPECT_EQ(this->ld2450_.buffer_pos_, 0); -} +// --- Garbage then valid frame tests --- TEST_F(LD2450ReadlineTest, GarbageThenValidFrame) { + // Garbage bytes accumulate in the buffer but don't match any footer. + // A valid frame follows; its footer resets the buffer and resyncs. std::vector garbage = {0x01, 0x02, 0x03, 0x42, 0x99}; this->ld2450_.feed(garbage); + EXPECT_GT(this->ld2450_.buffer_pos_, 0); // Garbage accumulated + + auto frame = make_periodic_frame(); + this->ld2450_.feed(frame); + // Footer from the valid frame resyncs the parser + EXPECT_EQ(this->ld2450_.buffer_pos_, 0); +} + +// --- Footer-based resynchronization tests --- + +TEST_F(LD2450ReadlineTest, FooterInGarbageResyncs) { + // Garbage containing a periodic frame footer (0x55 0xCC) triggers + // a buffer reset, allowing the next frame to be parsed cleanly. + std::vector garbage_with_footer = {0x01, 0x02, 0x03, 0x04, 0x55, 0xCC}; + this->ld2450_.feed(garbage_with_footer); + EXPECT_EQ(this->ld2450_.buffer_pos_, 0); // Footer reset the buffer + + auto frame = make_periodic_frame(); + this->ld2450_.feed(frame); + EXPECT_EQ(this->ld2450_.buffer_pos_, 0); +} + +TEST_F(LD2450ReadlineTest, CmdFooterInGarbageResyncs) { + // Garbage containing a command frame footer (04 03 02 01) also resyncs. + std::vector garbage_with_footer = {0x10, 0x20, 0x30, 0x40, 0x04, 0x03, 0x02, 0x01}; + this->ld2450_.feed(garbage_with_footer); EXPECT_EQ(this->ld2450_.buffer_pos_, 0); auto frame = make_periodic_frame(); @@ -68,112 +88,66 @@ TEST_F(LD2450ReadlineTest, GarbageThenValidFrame) { EXPECT_EQ(this->ld2450_.buffer_pos_, 0); } -// --- Header synchronization tests --- +// --- Overflow recovery tests --- -TEST_F(LD2450ReadlineTest, PartialDataHeaderThenMismatch) { - // Start of a data frame header, then invalid byte - this->ld2450_.feed({0xAA, 0xFF, 0x42}); // 0x42 doesn't match DATA_FRAME_HEADER[2] (0x03) - // Parser should have reset - EXPECT_EQ(this->ld2450_.buffer_pos_, 0); -} - -TEST_F(LD2450ReadlineTest, PartialCmdHeaderThenMismatch) { - // Start of a command frame header, then invalid byte - this->ld2450_.feed({0xFD, 0xFC, 0xFB, 0x42}); // 0x42 doesn't match CMD_FRAME_HEADER[3] (0xFA) - EXPECT_EQ(this->ld2450_.buffer_pos_, 0); -} - -TEST_F(LD2450ReadlineTest, PartialHeaderThenValidFrame) { - // Partial header that fails, then a complete valid frame - this->ld2450_.feed({0xAA, 0xFF, 0x42}); // Fails at byte 3 - EXPECT_EQ(this->ld2450_.buffer_pos_, 0); - - auto frame = make_periodic_frame(); - this->ld2450_.feed(frame); - EXPECT_EQ(this->ld2450_.buffer_pos_, 0); -} - -TEST_F(LD2450ReadlineTest, HeaderMismatchRecoveryOnNewHeaderByte) { - // Start data header, mismatch at byte 2, but mismatch byte is start of command header - this->ld2450_.feed({0xAA, 0xFF}); - EXPECT_EQ(this->ld2450_.buffer_pos_, 2); // Accumulating header - - this->ld2450_.feed({0xFD}); // Doesn't match DATA_FRAME_HEADER[2]=0x03, but IS CMD_FRAME_HEADER[0] - // Parser should reset and start new frame with 0xFD - EXPECT_EQ(this->ld2450_.buffer_pos_, 1); - EXPECT_EQ(this->ld2450_.buffer_data_[0], 0xFD); -} - -// --- Mid-frame / overflow recovery tests --- - -TEST_F(LD2450ReadlineTest, MidFrameDataRecovery) { - // Simulate starting mid-frame: feed the tail end of a periodic frame (no valid header) - // These bytes would be part of target data in a real frame - std::vector mid_frame = {0x10, 0x20, 0x30, 0x40, 0x55, 0xCC}; - this->ld2450_.feed(mid_frame); - // All discarded (none match header start bytes) - EXPECT_EQ(this->ld2450_.buffer_pos_, 0); - - // Now feed a valid frame - auto frame = make_periodic_frame(); - this->ld2450_.feed(frame); - EXPECT_EQ(this->ld2450_.buffer_pos_, 0); -} - -TEST_F(LD2450ReadlineTest, OverflowRecovery) { - // Feed a valid data frame header followed by enough filler to cause overflow. - // Header (4) + 36 filler = 40 bytes in buffer. The 41st byte triggers overflow. - std::vector overflow_data = {0xAA, 0xFF, 0x03, 0x00}; // Valid header - for (int i = 0; i < 37; i++) { +TEST_F(LD2450ReadlineTest, OverflowResetsBuffer) { + // Fill the buffer to capacity with filler that won't match any footer. + // MAX_LINE_LENGTH is 45, usable is 44. The 45th byte triggers overflow. + std::vector overflow_data; + for (int i = 0; i < MAX_LINE_LENGTH; i++) { overflow_data.push_back(0x11); // Filler that won't match any footer } - // 41 bytes total: 40 stored, 41st triggers overflow and resets buffer_pos_ to 0 this->ld2450_.feed(overflow_data); - EXPECT_EQ(this->ld2450_.buffer_pos_, 0); - - // Feed a valid frame and verify recovery - auto frame = make_periodic_frame(); - this->ld2450_.feed(frame); - EXPECT_EQ(this->ld2450_.buffer_pos_, 0); + // After overflow, buffer_pos_ resets to 0 (via the < 4 early return path) + EXPECT_LT(this->ld2450_.buffer_pos_, 4); } -TEST_F(LD2450ReadlineTest, RepeatedOverflowDoesNotLoop) { - // Simulate the bug scenario: repeated overflows should not prevent recovery. - // Feed 3 rounds of overflow-inducing data. - for (int round = 0; round < 3; round++) { - std::vector overflow_data = {0xAA, 0xFF, 0x03, 0x00}; - for (int i = 0; i < 37; i++) { - overflow_data.push_back(0x22); - } - this->ld2450_.feed(overflow_data); - EXPECT_EQ(this->ld2450_.buffer_pos_, 0) << "Overflow round " << round; +TEST_F(LD2450ReadlineTest, OverflowThenValidFrame) { + // Overflow, then a valid frame should be processed. + std::vector overflow_data; + for (int i = 0; i < MAX_LINE_LENGTH; i++) { + overflow_data.push_back(0x11); } + this->ld2450_.feed(overflow_data); - // Parser should still recover and process a valid frame auto frame = make_periodic_frame(); this->ld2450_.feed(frame); EXPECT_EQ(this->ld2450_.buffer_pos_, 0); } -TEST_F(LD2450ReadlineTest, SimulatedRestartGarbageThenFrames) { - // Simulate LD2450 restart: burst of garbage bytes (partial frames, noise) - // followed by normal periodic data. - // Partial periodic frame (as if we started reading mid-frame), a stale footer, and more garbage +TEST_F(LD2450ReadlineTest, BufferLargeEnoughForDesyncedFooter) { + // The key fix: the buffer (45) is large enough that a desynced periodic frame's + // footer (at most 30 bytes into the stream) will land inside the buffer before overflow. + // Simulate starting 10 bytes into a periodic frame, then a full frame follows. + std::vector mid_frame; + // 10 bytes of "mid-frame" data (no footer match) + for (int i = 0; i < 10; i++) { + mid_frame.push_back(0x30 + i); + } + // Then a complete periodic frame whose footer will land at position 40 (10 + 30), + // well within the buffer size of 45. + auto frame = make_periodic_frame(); + mid_frame.insert(mid_frame.end(), frame.begin(), frame.end()); + + this->ld2450_.feed(mid_frame); + // The footer from the frame should have triggered a reset + EXPECT_EQ(this->ld2450_.buffer_pos_, 0); +} + +TEST_F(LD2450ReadlineTest, SimulatedRestartThenFrames) { + // Simulate LD2450 restart: burst of garbage followed by valid periodic frames. + // The garbage + first frame should fit in the buffer so the footer resyncs. std::vector restart_noise = { - 0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x38, 0x39, 0x3A, 0x3B, 0x3C, 0x3D, 0x3E, // mid-frame data - 0x55, 0xCC, // stale footer bytes - 0x80, 0x81, 0x82, 0x83, 0x84, 0x85, 0x86, 0x87, 0x88, 0x89, // more garbage + 0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, // 8 bytes of mid-frame data }; + auto frame = make_periodic_frame(); + // 8 garbage + 30 frame = 38 bytes, well within buffer of 45 + restart_noise.insert(restart_noise.end(), frame.begin(), frame.end()); this->ld2450_.feed(restart_noise); - // All garbage should be discarded - EXPECT_EQ(this->ld2450_.buffer_pos_, 0); - - // Now the LD2450 starts sending valid frames - auto frame = make_periodic_frame(); - this->ld2450_.feed(frame); EXPECT_EQ(this->ld2450_.buffer_pos_, 0); + // Subsequent frames should work normally this->ld2450_.feed(frame); EXPECT_EQ(this->ld2450_.buffer_pos_, 0); }