[ld2420] Address second review round

Ignore reception during a settle window at the start of each listen
phase so in-flight bytes from a restarting module cannot count as
proof it is up, drain stale replies before each startup send so acks
cannot be matched to the wrong command, always send the blind config
mode exit when abandoning a sequence, log module error replies and an
unknown firmware version on the give-up path, skip transmitting for
states with no command frame, and clear the warning status when a
later apply or factory reset succeeds. Fixture timings adjusted for
the settle window.

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() {

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);

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.

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.