diff --git a/esphome/components/e131/e131.cpp b/esphome/components/e131/e131.cpp index a1cab0be52..a7a695c167 100644 --- a/esphome/components/e131/e131.cpp +++ b/esphome/components/e131/e131.cpp @@ -72,6 +72,9 @@ void E131Component::loop() { uint8_t buf[1460]; ssize_t len; + // 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); 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.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 95c05d489c..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,118 +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); + // After overflow, buffer_pos_ resets to 0 (via the < 4 early return path) + EXPECT_LT(this->ld2450_.buffer_pos_, 4); +} + +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); - // 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. - std::vector restart_noise; - // Partial periodic frame (as if we started reading mid-frame) - for (int i = 0; i < 15; i++) { - restart_noise.push_back(0x30 + i); - } - // A footer that would have been part of the previous frame - restart_noise.push_back(0x55); - restart_noise.push_back(0xCC); - // 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++) { - restart_noise.push_back(0x80 + 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, // 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); }