diff --git a/esphome/components/ld2420/ld2420.cpp b/esphome/components/ld2420/ld2420.cpp index ea251b1193..bb767d119b 100644 --- a/esphome/components/ld2420/ld2420.cpp +++ b/esphome/components/ld2420/ld2420.cpp @@ -197,10 +197,13 @@ static int32_t get_firmware_int(const char *version_string) { } void LD2420Component::dump_config() { + // Setup no longer blocks, so the config dump usually runs before the + // version is read; do not present the "v0.0.0" placeholder as real + const int32_t firmware = ld2420::get_firmware_int(this->firmware_ver_); ESP_LOGCONFIG(TAG, "LD2420:\n" " Firmware version: %7s", - this->firmware_ver_); + firmware > 0 ? this->firmware_ver_ : "unknown"); #ifdef USE_NUMBER ESP_LOGCONFIG(TAG, "Number:"); LOG_NUMBER(" ", "Gate Timeout:", this->gate_timeout_number_); @@ -222,7 +225,6 @@ void LD2420Component::dump_config() { ESP_LOGCONFIG(TAG, "Select:"); LOG_SELECT(" ", "Operating Mode", this->operating_selector_); #endif - const int32_t firmware = ld2420::get_firmware_int(this->firmware_ver_); if (firmware > 0 && firmware < CALIBRATE_VERSION_MIN) { ESP_LOGW(TAG, "Firmware version %s and older supports Simple Mode only", this->firmware_ver_); } @@ -240,6 +242,7 @@ void LD2420Component::begin_startup_() { void LD2420Component::begin_listen_() { this->rx_seen_ = false; + this->listen_drained_ = false; this->buffer_pos_ = 0; this->phase_start_ms_ = millis(); this->startup_state_ = StartupState::STARTUP_STATE_LISTEN; @@ -293,6 +296,9 @@ void LD2420Component::send_startup_cmd_() { this->startup_cmd_ = (uint8_t) frame.command; this->cmd_reply_.ack = false; this->cmd_reply_.error = 0; + // A short reply acks without filling every data word; zero them so stale + // values from the previous command cannot be stored as this command's data + memset(this->cmd_reply_.data, 0, sizeof(this->cmd_reply_.data)); this->write_cmd_frame_(frame); this->phase_start_ms_ = millis(); } @@ -342,6 +348,11 @@ bool LD2420Component::startup_ack_check_() { // will not publish sensor data either. ESP_LOGE(TAG, "Firmware version and operating mode were never read"); } +#ifdef USE_NUMBER + // Publish whatever was read before giving up so the number entities show + // values next to the warning status instead of staying unknown forever + this->init_gate_config_numbers(); +#endif this->status_set_warning(ESP_LOG_MSG_COMM_FAIL); this->startup_state_ = StartupState::STARTUP_STATE_RUNNING; return false; @@ -368,13 +379,17 @@ void LD2420Component::loop_startup_() { // 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 + // counts as proof that it is up and streaming. The listen_drained_ flag + // guarantees at least one such drain pass even when the main loop + // stalls past the whole window, so bytes that arrived before the listen + // phase can never be mistaken for fresh data. (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) { + if (elapsed < STARTUP_LISTEN_SETTLE_MS || !this->listen_drained_) { this->drain_rx_(); this->rx_seen_ = false; + this->listen_drained_ = true; return; } // The module locks up until power cycled if it receives data before it @@ -485,6 +500,12 @@ void LD2420Component::apply_config_action() { ESP_LOGW(TAG, "Module is still starting up; ignoring"); return; } + if (ld2420::get_firmware_int(this->firmware_ver_) == 0) { + // Setup gave up before the configuration was ever read; writing the + // unread config to the module's NVM would wipe its stored thresholds + ESP_LOGW(TAG, "Module configuration was never read; ignoring"); + return; + } const uint8_t checksum = calc_checksum(&this->new_config, sizeof(this->new_config)); if (checksum == calc_checksum(&this->current_config, sizeof(this->current_config))) { ESP_LOGD(TAG, "No configuration change detected"); @@ -506,9 +527,15 @@ void LD2420Component::apply_config_action() { this->init_gate_config_numbers(); #endif this->set_system_mode(this->system_mode_); - this->set_config_mode(false); // Disable config mode to save new values in LD2420 nvm + // Disable config mode to save new values in LD2420 nvm. The individual + // write commands do not report errors, so use the final ack as the best + // available signal before reporting the reconfiguration as healthy. + if (this->set_config_mode(false) == LD2420_ERROR_NONE) { + this->status_clear_warning(); + } else { + this->status_set_warning(ESP_LOG_MSG_COMM_FAIL); + } this->set_operating_mode(OP_NORMAL_MODE_STRING); - this->status_clear_warning(); } void LD2420Component::factory_reset_action() { @@ -516,6 +543,10 @@ void LD2420Component::factory_reset_action() { ESP_LOGW(TAG, "Module is still starting up; ignoring"); return; } + if (ld2420::get_firmware_int(this->firmware_ver_) == 0) { + ESP_LOGW(TAG, "Module configuration was never read; ignoring"); + return; + } ESP_LOGD(TAG, "Setting factory defaults"); if (this->set_config_mode(true) == LD2420_ERROR_TIMEOUT) { ESP_LOGE(TAG, ESP_LOG_MSG_COMM_FAIL); @@ -536,12 +567,15 @@ void LD2420Component::factory_reset_action() { } memcpy(&this->current_config, &this->new_config, sizeof(this->new_config)); this->set_system_mode(this->system_mode_); - this->set_config_mode(false); + if (this->set_config_mode(false) == LD2420_ERROR_NONE) { + this->status_clear_warning(); + } else { + this->status_set_warning(ESP_LOG_MSG_COMM_FAIL); + } #ifdef USE_NUMBER this->init_gate_config_numbers(); this->refresh_gate_config_numbers(); #endif - this->status_clear_warning(); } void LD2420Component::restart_module_action() { @@ -558,6 +592,10 @@ void LD2420Component::restart_module_action() { } void LD2420Component::revert_config_action() { + if (this->startup_state_ != StartupState::STARTUP_STATE_RUNNING) { + ESP_LOGW(TAG, "Module is still starting up; ignoring"); + return; + } memcpy(&this->new_config, &this->current_config, sizeof(this->current_config)); #ifdef USE_NUMBER this->init_gate_config_numbers(); diff --git a/esphome/components/ld2420/ld2420.h b/esphome/components/ld2420/ld2420.h index b32b2e4a3a..24c2f0b62e 100644 --- a/esphome/components/ld2420/ld2420.h +++ b/esphome/components/ld2420/ld2420.h @@ -51,8 +51,8 @@ class LD2420Component final : public Component, public uart::UARTDevice { }; struct RegConfigT { - uint32_t move_thresh[TOTAL_GATES]; - uint32_t still_thresh[TOTAL_GATES]; + uint32_t move_thresh[TOTAL_GATES]{}; + uint32_t still_thresh[TOTAL_GATES]{}; uint16_t min_gate{0}; uint16_t max_gate{0}; uint16_t timeout{0}; @@ -218,6 +218,7 @@ class LD2420Component final : public Component, public uart::UARTDevice { uint8_t startup_sequence_retries_{0}; uint8_t startup_gate_{0}; bool rx_seen_{false}; + bool listen_drained_{false}; uint8_t buffer_pos_{0}; // where to resume processing/populating buffer uint8_t buffer_data_[MAX_LINE_LENGTH]; char firmware_ver_[8]{"v0.0.0"}; diff --git a/tests/integration/fixtures/uart_mock_ld2420_cmd_retry.yaml b/tests/integration/fixtures/uart_mock_ld2420_cmd_retry.yaml new file mode 100644 index 0000000000..78a6a64b4c --- /dev/null +++ b/tests/integration/fixtures/uart_mock_ld2420_cmd_retry.yaml @@ -0,0 +1,131 @@ +esphome: + name: uart-mock-ld2420-retry-test + +host: +api: + batch_delay: 0ms # Disable batching to receive all state updates +logger: + level: VERBOSE + +external_components: + - source: + type: local + path: EXTERNAL_COMPONENT_PATH + +# Dummy uart entry to satisfy ld2420's DEPENDENCIES = ["uart"] +uart: + baud_rate: 115200 + port: /dev/null + +# Exercises the per-command retry path: the module ignores the first config +# mode enable command and only answers the resend, so the startup handshake +# must time out once, resend, and then complete normally. +uart_mock: + id: mock_uart + baud_rate: 115200 + auto_start: true + + injections: + # Wake-up frame (t=700ms): energy frame (presence=1, distance=100). + # Delay=700ms keeps it outside the component's 500ms listen settle window. + - delay: 700ms + inject_rx: + [ + 0xF4, 0xF3, 0xF2, 0xF1, + 0x23, 0x00, + 0x01, + 0x64, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0xF8, 0xF7, 0xF6, 0xF5, + ] + + # The config mode enable command is answered from the on_tx hook below so + # that the first attempt can be ignored; it must not have a responder here. + responses: + # Version response: returns "v2.0.0" → 200 >= 154 → energy mode + - expect_tx: + [0xFD, 0xFC, 0xFB, 0xFA, 0x02, 0x00, 0x00, 0x00, 0x04, 0x03, 0x02, 0x01] + inject_rx: + [ + 0xFD, 0xFC, 0xFB, 0xFA, + 0x0C, 0x00, + 0x00, 0x01, + 0x00, 0x00, + 0x06, 0x00, + 0x76, 0x32, 0x2E, 0x30, 0x2E, 0x30, + 0x04, 0x03, 0x02, 0x01, + ] + + # System mode write: CMD_WRITE_SYS_PARAM (0x0012), mode = energy (0x0004) + - expect_tx: + [0xFD, 0xFC, 0xFB, 0xFA, 0x08, 0x00, 0x12, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x04, 0x03, 0x02, 0x01] + inject_rx: + [ + 0xFD, 0xFC, 0xFB, 0xFA, + 0x04, 0x00, + 0x12, 0x01, + 0x00, 0x00, + 0x04, 0x03, 0x02, 0x01, + ] + + # Config mode disable: CMD_DISABLE_CONF (0x00FE) + - expect_tx: [0xFD, 0xFC, 0xFB, 0xFA, 0x02, 0x00, 0xFE, 0x00, 0x04, 0x03, 0x02, 0x01] + inject_rx: + [ + 0xFD, 0xFC, 0xFB, 0xFA, + 0x04, 0x00, + 0xFE, 0x01, + 0x00, 0x00, + 0x04, 0x03, 0x02, 0x01, + ] + + # Catch-all for the CMD_READ_ABD_PARAM (0x0008) reads: limits and the 16 + # gate threshold reads. Three zeroed uint32 data values. + - expect_tx: [0x04, 0x03, 0x02, 0x01] + inject_rx: + [ + 0xFD, 0xFC, 0xFB, 0xFA, + 0x10, 0x00, + 0x08, 0x01, + 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x04, 0x03, 0x02, 0x01, + ] + + # Ignore the first config mode enable command; ack every one after it + on_tx: + - lambda: |- + static int enable_count = 0; + if (data.size() == 14 && data[6] == 0xFF) { + enable_count++; + if (enable_count >= 2) { + id(mock_uart).inject_to_rx_buffer(std::vector{ + 0xFD, 0xFC, 0xFB, 0xFA, 0x04, 0x00, 0xFF, 0x01, 0x00, 0x00, 0x04, 0x03, 0x02, 0x01}); + } + } + +ld2420: + id: ld2420_dev + uart_id: mock_uart + +sensor: + - platform: ld2420 + ld2420_id: ld2420_dev + moving_distance: + name: "Moving Distance" + filters: + - timeout: + timeout: 50ms + value: last + - throttle_with_priority: 50ms + +binary_sensor: + - platform: ld2420 + ld2420_id: ld2420_dev + has_target: + name: "Has Target" + filters: + - settle: 50ms diff --git a/tests/integration/fixtures/uart_mock_ld2420_give_up.yaml b/tests/integration/fixtures/uart_mock_ld2420_give_up.yaml new file mode 100644 index 0000000000..8ae8f69105 --- /dev/null +++ b/tests/integration/fixtures/uart_mock_ld2420_give_up.yaml @@ -0,0 +1,128 @@ +esphome: + name: uart-mock-ld2420-giveup-test + +host: +api: + batch_delay: 0ms # Disable batching to receive all state updates +logger: + level: VERBOSE + +external_components: + - source: + type: local + path: EXTERNAL_COMPONENT_PATH + +# Dummy uart entry to satisfy ld2420's DEPENDENCIES = ["uart"] +uart: + baud_rate: 115200 + port: /dev/null + +# Exercises the sequence retry and give-up path: the module streams energy +# frames and answers every command except the firmware version read. The +# startup handshake must retry the whole sequence, eventually give up with a +# warning instead of marking the component failed, and keep parsing the +# stream afterwards. Runs for roughly 16 seconds of retry cadence. +uart_mock: + id: mock_uart + baud_rate: 115200 + auto_start: true + + # Module streams a valid energy frame (presence=1, distance=100) continuously + periodic_rx: + - interval: 250ms + data: + [ + 0xF4, 0xF3, 0xF2, 0xF1, + 0x23, 0x00, + 0x01, + 0x64, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0xF8, 0xF7, 0xF6, 0xF5, + ] + + injections: + # Post-give-up parser probe (t=22s): 50 bytes of 0xFF overflow the frame + # buffer, which the parser answers with a "Max command length exceeded" + # warning. The give-up happens around t=16s, so seeing that warning after + # the give-up proves the stream parser is still running in the degraded + # state. (A distinct sensor value cannot serve as the probe: the 1s + # publish throttle races the constant periodic stream, and the API + # deduplicates repeated identical states.) + - delay: 22000ms + inject_rx: + [ + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + ] + + responses: + # Version read: matched so the catch-all cannot answer it, but never + # replied to; this is the command the handshake gives up on + - expect_tx: + [0xFD, 0xFC, 0xFB, 0xFA, 0x02, 0x00, 0x00, 0x00, 0x04, 0x03, 0x02, 0x01] + inject_rx: [] + + # Config mode enable: CMD_ENABLE_CONF (0x00FF) + - expect_tx: + [0xFD, 0xFC, 0xFB, 0xFA, 0x04, 0x00, 0xFF, 0x00, 0x02, 0x00, 0x04, 0x03, 0x02, 0x01] + inject_rx: + [ + 0xFD, 0xFC, 0xFB, 0xFA, + 0x04, 0x00, + 0xFF, 0x01, + 0x00, 0x00, + 0x04, 0x03, 0x02, 0x01, + ] + + # Config mode disable: CMD_DISABLE_CONF (0x00FE), sent blind before each + # sequence retry and on the final give-up + - expect_tx: [0xFD, 0xFC, 0xFB, 0xFA, 0x02, 0x00, 0xFE, 0x00, 0x04, 0x03, 0x02, 0x01] + inject_rx: + [ + 0xFD, 0xFC, 0xFB, 0xFA, + 0x04, 0x00, + 0xFE, 0x01, + 0x00, 0x00, + 0x04, 0x03, 0x02, 0x01, + ] + + # Catch-all for the CMD_READ_ABD_PARAM (0x0008) reads + - expect_tx: [0x04, 0x03, 0x02, 0x01] + inject_rx: + [ + 0xFD, 0xFC, 0xFB, 0xFA, + 0x10, 0x00, + 0x08, 0x01, + 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x04, 0x03, 0x02, 0x01, + ] + +ld2420: + id: ld2420_dev + uart_id: mock_uart + +sensor: + - platform: ld2420 + ld2420_id: ld2420_dev + moving_distance: + name: "Moving Distance" + filters: + - timeout: + timeout: 50ms + value: last + - throttle_with_priority: 50ms + +binary_sensor: + - platform: ld2420 + ld2420_id: ld2420_dev + has_target: + name: "Has Target" + filters: + - settle: 50ms diff --git a/tests/integration/test_uart_mock_ld2420.py b/tests/integration/test_uart_mock_ld2420.py index 37e24fcaee..753032b67b 100644 --- a/tests/integration/test_uart_mock_ld2420.py +++ b/tests/integration/test_uart_mock_ld2420.py @@ -35,6 +35,16 @@ test_uart_mock_ld2420_restart_button (module restart action): boots. The component must not treat the tail bytes as proof the module is up and must only re-run its handshake after the module's first post-boot frame; transmitting into the boot window locks up real hardware. + +test_uart_mock_ld2420_cmd_retry (per-command resend): + The module ignores the first config mode enable command and only answers + the resend. The handshake must time out once, resend, and complete. + +test_uart_mock_ld2420_give_up (sequence retry and give-up): + The module streams and answers everything except the firmware version + read. The handshake must retry the whole sequence, eventually give up with + a warning instead of marking the component failed, and keep publishing + sensor data from the stream afterwards. """ from __future__ import annotations @@ -316,6 +326,173 @@ async def test_uart_mock_ld2420_delayed_boot( ) +@pytest.mark.asyncio +async def test_uart_mock_ld2420_cmd_retry( + yaml_config: str, + run_compiled: RunCompiledFunction, + api_client_connected: APIClientConnectedFactory, +) -> None: + """First config command gets no reply; the resend must recover.""" + loop = asyncio.get_running_loop() + + setup_complete = loop.create_future() + resend_seen = loop.create_future() + failure_lines: list[str] = [] + + def line_callback(line: str) -> None: + if "No reply to startup command" in line and not resend_seen.done(): + resend_seen.set_result(True) + if ( + "Module setup complete; firmware v2.0.0" in line + and not setup_complete.done() + ): + setup_complete.set_result(True) + if ( + "marked FAILED" in line + or "Communication failed" in line + or "Module setup attempt" in line + ): + failure_lines.append(line) + + collector = SensorStateCollector( + sensor_names=["moving_distance"], + binary_sensor_names=["has_target"], + ) + + async with ( + run_compiled(yaml_config, line_callback=line_callback), + api_client_connected() as client, + ): + entities, _ = await client.list_entities_services() + collector.build_key_mapping(entities) + + initial_state_helper = InitialStateHelper(entities) + client.subscribe_states( + initial_state_helper.on_state_wrapper(collector.on_state) + ) + + try: + await initial_state_helper.wait_for_initial_states() + except TimeoutError: + pytest.fail("Timeout waiting for initial states") + + # The first enable command is ignored, so a resend must happen + try: + await asyncio.wait_for(resend_seen, timeout=10.0) + except TimeoutError: + pytest.fail("Timeout waiting for the startup command resend log line") + + # The resend gets an ack and the handshake completes normally + try: + await asyncio.wait_for(setup_complete, timeout=10.0) + except TimeoutError: + pytest.fail("Timeout waiting for 'Module setup complete' after the resend") + + try: + await collector.wait_for_all(timeout=5.0) + except TimeoutError: + pytest.fail( + f"Timeout waiting for sensor data. Received:\n" + f" sensor_states: {collector.sensor_states}" + ) + + assert collector.sensor_states["moving_distance"][0] == pytest.approx(100.0) + + # A single command resend must not burn a whole sequence retry or + # produce any failure log line + assert not failure_lines, f"Unexpected failure log lines: {failure_lines}" + + +@pytest.mark.asyncio +async def test_uart_mock_ld2420_give_up( + yaml_config: str, + run_compiled: RunCompiledFunction, + api_client_connected: APIClientConnectedFactory, +) -> None: + """Version read never answers; retries then give-up, stream keeps working.""" + loop = asyncio.get_running_loop() + + sequence_retry_seen = loop.create_future() + give_up_seen = loop.create_future() + parser_alive_after_give_up = loop.create_future() + marked_failed_lines: list[str] = [] + + def line_callback(line: str) -> None: + if "Module setup attempt 1 failed; retrying" in line and ( + not sequence_retry_seen.done() + ): + sequence_retry_seen.set_result(True) + if "Firmware version and operating mode were never read" in line and ( + not give_up_seen.done() + ): + give_up_seen.set_result(True) + # The overflow probe injected at t=22s (after the give-up) makes the + # parser log this warning only if it is still running + if ( + "Max command length exceeded" in line + and give_up_seen.done() + and not parser_alive_after_give_up.done() + ): + parser_alive_after_give_up.set_result(True) + if "marked FAILED" in line: + marked_failed_lines.append(line) + + collector = SensorStateCollector( + sensor_names=["moving_distance"], + binary_sensor_names=["has_target"], + ) + + async with ( + run_compiled(yaml_config, line_callback=line_callback), + api_client_connected() as client, + ): + entities, _ = await client.list_entities_services() + collector.build_key_mapping(entities) + + initial_state_helper = InitialStateHelper(entities) + client.subscribe_states( + initial_state_helper.on_state_wrapper(collector.on_state) + ) + + try: + await initial_state_helper.wait_for_initial_states() + except TimeoutError: + pytest.fail("Timeout waiting for initial states") + + # The version read times out three times, then the sequence retries + try: + await asyncio.wait_for(sequence_retry_seen, timeout=15.0) + except TimeoutError: + pytest.fail("Timeout waiting for the sequence retry log line") + + # After all sequence retries the component gives up with a warning + try: + await asyncio.wait_for(give_up_seen, timeout=30.0) + except TimeoutError: + pytest.fail("Timeout waiting for the give-up log line") + + # The stream must still be parsed after giving up; the overflow probe + # injected at t=22s only produces its warning if the parser runs + try: + await asyncio.wait_for(parser_alive_after_give_up, timeout=20.0) + except TimeoutError: + pytest.fail( + "No parser activity after the give-up; the stream parser " + "must keep running in the degraded state" + ) + + # The stream published sensor data while the handshake was failing + assert pytest.approx(100.0) in collector.sensor_states["moving_distance"], ( + f"Expected the stream to publish distance=100, " + f"got: {collector.sensor_states['moving_distance']}" + ) + + # The whole point of the degraded state: the component keeps running + assert not marked_failed_lines, ( + f"Component was marked failed: {marked_failed_lines}" + ) + + @pytest.mark.asyncio async def test_uart_mock_ld2420_restart_button( yaml_config: str,