diff --git a/esphome/components/ld2420/ld2420.cpp b/esphome/components/ld2420/ld2420.cpp index 1117b43b81..ea251b1193 100644 --- a/esphome/components/ld2420/ld2420.cpp +++ b/esphome/components/ld2420/ld2420.cpp @@ -72,6 +72,7 @@ static constexpr uint8_t CMD_MAX_RETRIES = 3; // timeout and command retry count are shared with the blocking command engine. static constexpr uint32_t STARTUP_LISTEN_TIMEOUT_MS = 10000; static constexpr uint32_t STARTUP_RETRY_LISTEN_MS = 3000; +static constexpr uint32_t STARTUP_LISTEN_SETTLE_MS = 500; static constexpr uint32_t CMD_ACK_TIMEOUT_MS = 1000; static constexpr uint8_t CMD_MAX_RETRIES = 3; static constexpr uint8_t STARTUP_SEQUENCE_MAX_RETRIES = 3; @@ -251,37 +252,47 @@ void LD2420Component::drain_rx_() { this->buffer_pos_ = 0; } -// Builds the command frame for the current startup state -void LD2420Component::build_startup_frame_(CmdFrameT &frame) { +// Builds the command frame for the current startup state; returns false when +// the state has no associated command +bool LD2420Component::build_startup_frame_(CmdFrameT &frame) { switch (this->startup_state_) { case StartupState::STARTUP_STATE_ENTER_CONFIG: this->build_config_mode_frame_(frame, true); - break; + return true; case StartupState::STARTUP_STATE_READ_LIMITS: this->build_min_max_timeout_frame_(frame); - break; + return true; case StartupState::STARTUP_STATE_READ_VERSION: this->build_version_frame_(frame); - break; + return true; case StartupState::STARTUP_STATE_READ_GATES: this->build_gate_threshold_frame_(frame, this->startup_gate_); - break; + return true; case StartupState::STARTUP_STATE_SET_MODE: this->build_system_mode_frame_(frame, this->system_mode_); - break; + return true; case StartupState::STARTUP_STATE_EXIT_CONFIG: this->build_config_mode_frame_(frame, false); - break; + return true; default: - break; + return false; } } void LD2420Component::send_startup_cmd_() { CmdFrameT frame; - this->build_startup_frame_(frame); + if (!this->build_startup_frame_(frame)) { + return; + } + // Discard anything still buffered (including a late reply to a previous + // send of the same command) so a stale ack cannot be matched to this one. + // READ_LIMITS and all gate reads share the same command byte, so a late + // reply accepted for the wrong request would shift every following gate's + // thresholds by one. + this->drain_rx_(); this->startup_cmd_ = (uint8_t) frame.command; this->cmd_reply_.ack = false; + this->cmd_reply_.error = 0; this->write_cmd_frame_(frame); this->phase_start_ms_ = millis(); } @@ -299,6 +310,12 @@ bool LD2420Component::startup_ack_check_() { if (this->cmd_reply_.ack && this->cmd_reply_.command == this->startup_cmd_) { return true; } + if (this->cmd_reply_.error > 0) { + // The module explicitly rejected the command; log why instead of letting + // it look like silence. The normal retry cadence still applies. + this->handle_cmd_error(this->cmd_reply_.error); + this->cmd_reply_.error = 0; + } if (millis() - this->phase_start_ms_ <= CMD_ACK_TIMEOUT_MS) { return false; } @@ -319,6 +336,12 @@ bool LD2420Component::startup_ack_check_() { // Give up on configuration but keep parsing the stream; a module that is // still streaming keeps publishing sensor data even without a config read. ESP_LOGE(TAG, ESP_LOG_MSG_COMM_FAIL); + if (ld2420::get_firmware_int(this->firmware_ver_) == 0) { + // Old firmware streams text frames that are only parsed in simple mode; + // without a version read the mode was never negotiated, so such a module + // will not publish sensor data either. + ESP_LOGE(TAG, "Firmware version and operating mode were never read"); + } this->status_set_warning(ESP_LOG_MSG_COMM_FAIL); this->startup_state_ = StartupState::STARTUP_STATE_RUNNING; return false; @@ -328,10 +351,9 @@ void LD2420Component::abort_startup_cmd_() { // If the module already acknowledged config mode it stops streaming until // config mode is exited, so send the exit command blind before abandoning // the sequence; otherwise the stream never resumes and neither passive - // parsing nor the next listen phase would ever see data. - if (this->startup_state_ == StartupState::STARTUP_STATE_ENTER_CONFIG) { - return; // Config mode was never acknowledged; the module is still streaming - } + // parsing nor the next listen phase would ever see data. This is also sent + // when config mode was never acknowledged: the ack may merely have been + // lost, and the frame is harmless to a module that is not in config mode. CmdFrameT frame; this->build_config_mode_frame_(frame, false); this->write_cmd_frame_(frame); @@ -340,6 +362,21 @@ void LD2420Component::abort_startup_cmd_() { void LD2420Component::loop_startup_() { switch (this->startup_state_) { case StartupState::STARTUP_STATE_LISTEN: { + const uint32_t elapsed = millis() - this->phase_start_ms_; + // Bytes can already be in flight when the listen phase starts: the tail + // of a frame the module was transmitting when it was told to restart, + // stale data buffered before setup, or the ack to the blind config mode + // exit. Ignore reception during a short settle window (clearing the rx + // flag and the frame parser) so only data the module sends afterwards + // counts as proof that it is up and streaming. (A full-frame check + // cannot serve as that proof here: old-firmware text frames are only + // recognized once the operating mode is known, which requires the very + // handshake this phase gates.) + if (elapsed < STARTUP_LISTEN_SETTLE_MS) { + this->drain_rx_(); + this->rx_seen_ = false; + return; + } // The module locks up until power cycled if it receives data before it // has sent its first frame after powering on, so wait until it has // provably transmitted before sending anything. A module stuck in some @@ -347,7 +384,7 @@ void LD2420Component::loop_startup_() { if (!this->rx_seen_) { const uint32_t listen_timeout_ms = this->startup_sequence_retries_ == 0 ? STARTUP_LISTEN_TIMEOUT_MS : STARTUP_RETRY_LISTEN_MS; - if (millis() - this->phase_start_ms_ < listen_timeout_ms) { + if (elapsed < listen_timeout_ms) { return; } ESP_LOGW(TAG, "No data received from the module; attempting configuration anyway"); @@ -471,6 +508,7 @@ void LD2420Component::apply_config_action() { this->set_system_mode(this->system_mode_); this->set_config_mode(false); // Disable config mode to save new values in LD2420 nvm this->set_operating_mode(OP_NORMAL_MODE_STRING); + this->status_clear_warning(); } void LD2420Component::factory_reset_action() { @@ -503,6 +541,7 @@ void LD2420Component::factory_reset_action() { this->init_gate_config_numbers(); this->refresh_gate_config_numbers(); #endif + this->status_clear_warning(); } void LD2420Component::restart_module_action() { diff --git a/esphome/components/ld2420/ld2420.h b/esphome/components/ld2420/ld2420.h index 6321de1701..b32b2e4a3a 100644 --- a/esphome/components/ld2420/ld2420.h +++ b/esphome/components/ld2420/ld2420.h @@ -176,7 +176,7 @@ class LD2420Component final : public Component, public uart::UARTDevice { bool startup_ack_check_(); void drain_rx_(); void write_cmd_frame_(const CmdFrameT &frame); - void build_startup_frame_(CmdFrameT &frame); + bool build_startup_frame_(CmdFrameT &frame); void build_config_mode_frame_(CmdFrameT &frame, bool enable); void build_min_max_timeout_frame_(CmdFrameT &frame); void build_gate_threshold_frame_(CmdFrameT &frame, uint8_t gate); diff --git a/tests/integration/fixtures/uart_mock_ld2420.yaml b/tests/integration/fixtures/uart_mock_ld2420.yaml index c10885db8c..78dfe43d4e 100644 --- a/tests/integration/fixtures/uart_mock_ld2420.yaml +++ b/tests/integration/fixtures/uart_mock_ld2420.yaml @@ -111,7 +111,10 @@ uart_mock: ] injections: - # Phase 1 (t=100ms): Valid LD2420 energy mode data frame - happy path + # Phase 1 (t=700ms): Valid LD2420 energy mode data frame - happy path + # Delay=700ms keeps it outside the component's 500ms listen settle window, + # which is measured from boot and ignores earlier reception; this frame is + # both the happy path data and the wake-up that starts the setup handshake. # Buffer is clean (buffer_pos_=0). This frame should parse correctly. # Presence: 1 (target detected), Distance: 100cm, Gate energies: all 0 # @@ -122,7 +125,7 @@ uart_mock: # [7-8] 64 00 = distance 100 (uint16_t LE) # [9-40] 00 00 x16 = 16 gate energies (uint16_t LE each) # [41-44] F8 F7 F6 F5 = energy frame footer - - delay: 100ms + - delay: 700ms inject_rx: [ 0xF4, 0xF3, 0xF2, 0xF1, @@ -136,15 +139,15 @@ uart_mock: 0xF8, 0xF7, 0xF6, 0xF5, ] - # Phase 2 (t=800ms): Garbage bytes + # Phase 2 (t=1600ms): Garbage bytes # LD2420's readline_ does NOT check frame headers at position 0 (unlike LD2412), # so these bytes accumulate in the buffer. buffer_pos_ goes from 0 to 7. - # Delay=700ms leaves time for the setup handshake (triggered by Phase 1, + # Delay=900ms leaves time for the setup handshake (triggered by Phase 1, # the first data seen from the module) to finish first. - - delay: 700ms + - delay: 900ms inject_rx: [0xDE, 0xAD, 0xBE, 0xEF, 0x00, 0x11, 0x22] - # Phase 3 (t=900ms): Truncated energy frame WITH footer (13 bytes) + # Phase 3 (t=1700ms): Truncated energy frame WITH footer (13 bytes) # This tests PR #14458 bug #3: missing length validation in handle_energy_mode_. # The 7 garbage bytes from Phase 2 are still in the buffer (buffer_pos_=7). # These 13 bytes are appended at positions 7-19 (buffer_pos_=20). @@ -166,7 +169,7 @@ uart_mock: 0xF8, 0xF7, 0xF6, 0xF5, ] - # Phase 4 (t=1100ms): Overflow - inject 50 bytes of 0xFF (MAX_LINE_LENGTH=50) + # Phase 4 (t=1900ms): Overflow - inject 50 bytes of 0xFF (MAX_LINE_LENGTH=50) # After Phase 3, buffer_pos_=0 (reset after energy footer detection). # 49 bytes fill positions 0-48 (buffer_pos_=49), 50th byte triggers overflow. # Logs "Max command length exceeded; ignoring", buffer_pos_=0. @@ -180,7 +183,7 @@ uart_mock: 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, ] - # Phase 5 (t=1500ms): Valid frame after overflow - recovery test + # Phase 5 (t=2300ms): Valid frame after overflow - recovery test # Buffer was reset by overflow. This valid frame should parse correctly. # Presence: 1 (target), Distance: 50cm # Delay=400ms ensures >1000ms gap from Phase 1 for REFRESH_RATE_MS throttle. diff --git a/tests/integration/fixtures/uart_mock_ld2420_simple.yaml b/tests/integration/fixtures/uart_mock_ld2420_simple.yaml index 7195222cc1..9ead9f8d53 100644 --- a/tests/integration/fixtures/uart_mock_ld2420_simple.yaml +++ b/tests/integration/fixtures/uart_mock_ld2420_simple.yaml @@ -86,22 +86,12 @@ uart_mock: ] injections: - # Phase 0 (t=100ms): Wake-up frame. The component listens for data from the + # Phase 0 (t=700ms): Wake-up frame. The component listens for data from the # module before transmitting anything, so this frame starts the setup # handshake. It is not parsed as simple mode because the component's system # mode is only switched to simple after the firmware version is read. - - delay: 100ms - inject_rx: - [ - 0x4F, 0x4E, 0x20, 0x52, 0x61, 0x6E, 0x67, 0x65, 0x20, - 0x30, 0x31, 0x30, 0x30, - 0x0D, 0x0A, - ] - - # Phase 1 (t=800ms): Valid simple mode text frame - happy path - # "ON Range 0100\r\n" → presence=true, distance=100 - # Simple mode frames end with \r\n (0x0D 0x0A), triggering handle_simple_mode_. - # Delay=700ms leaves time for the setup handshake to finish first. + # Delay=700ms keeps it outside the component's 500ms listen settle window, + # which is measured from boot and ignores earlier reception. - delay: 700ms inject_rx: [ @@ -110,12 +100,24 @@ uart_mock: 0x0D, 0x0A, ] - # Phase 2 (t=1000ms): Garbage bytes + # Phase 1 (t=1600ms): Valid simple mode text frame - happy path + # "ON Range 0100\r\n" → presence=true, distance=100 + # Simple mode frames end with \r\n (0x0D 0x0A), triggering handle_simple_mode_. + # Delay=900ms leaves time for the setup handshake to finish first. + - delay: 900ms + inject_rx: + [ + 0x4F, 0x4E, 0x20, 0x52, 0x61, 0x6E, 0x67, 0x65, 0x20, + 0x30, 0x31, 0x30, 0x30, + 0x0D, 0x0A, + ] + + # Phase 2 (t=1800ms): Garbage bytes # LD2420's readline_ stores all bytes regardless of header. buffer_pos_ = 7. - delay: 200ms inject_rx: [0xDE, 0xAD, 0xBE, 0xEF, 0x00, 0x11, 0x22] - # Phase 3 (t=1200ms): Overflow - inject 50 bytes of 0xFF (MAX_LINE_LENGTH=50) + # Phase 3 (t=2000ms): Overflow - inject 50 bytes of 0xFF (MAX_LINE_LENGTH=50) # buffer_pos_ starts at 7 (from Phase 2 garbage). # Positions 7-48 fill (42 bytes), byte 43 triggers overflow (buffer_pos_=49). # After overflow: buffer_pos_=0, remaining 7 bytes fill positions 0-6. @@ -130,7 +132,7 @@ uart_mock: 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, ] - # Phase 4 (t=1900ms): Recovery after overflow + # Phase 4 (t=2700ms): Recovery after overflow # buffer_pos_ = 7 (from overflow remainder). These 15 bytes fill positions 7-21. # At position 21 (0x0A), \r\n detected → handle_simple_mode_(buffer, 22). # Parser skips 0xFF bytes at positions 0-6, finds "ON" at positions 7-8, @@ -144,7 +146,7 @@ uart_mock: 0x0D, 0x0A, ] - # Phase 5 (t=3000ms): 16-digit distance - tests PR #14458 bug #1 + # Phase 5 (t=3800ms): 16-digit distance - tests PR #14458 bug #1 # "ON Range 0000000000000000\r\n" has 16 digit characters. # handle_simple_mode_ outbuf is 16 bytes, can hold 15 digits (index 0-14). # @@ -163,7 +165,7 @@ uart_mock: 0x0D, 0x0A, ] - # Phase 6 (t=4200ms): Post-bug-trigger recovery + # Phase 6 (t=5000ms): Post-bug-trigger recovery # If Phase 5 didn't hang, this frame should parse correctly. # "ON Range 0025\r\n" → distance=25 # Delay=1200ms ensures >1000ms gap from Phase 5 for throttle.