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