From fd3c1fad45147fd8a6ab2dc798b0752df166f4ce Mon Sep 17 00:00:00 2001 From: Igor Stadnyk <33496176+fallenmi@users.noreply.github.com> Date: Mon, 19 Oct 2026 20:23:07 +0200 Subject: [PATCH] [ld2410s] Fix parameter writes after long uptime (#21121) --- esphome/components/ld2410s/ld2410s.cpp | 5 +- .../ld2410s/ld2410s_parameter_write.cpp | 179 ++++++++++++++++++ tests/components/ld2410s/number/__init__.py | 16 ++ 3 files changed, 199 insertions(+), 1 deletion(-) create mode 100644 tests/components/ld2410s/ld2410s_parameter_write.cpp create mode 100644 tests/components/ld2410s/number/__init__.py diff --git a/esphome/components/ld2410s/ld2410s.cpp b/esphome/components/ld2410s/ld2410s.cpp index 3f71c77a62b..740604e4bc9 100644 --- a/esphome/components/ld2410s/ld2410s.cpp +++ b/esphome/components/ld2410s/ld2410s.cpp @@ -116,7 +116,10 @@ void LD2410SComponent::loop() { #ifdef USE_LD2410S_PARAMETERS } else if (this->parameters_dirty_) { this->parameters_dirty_ = false; - this->start_sequence_(WRITE_SEQUENCE, WRITE_SEQUENCE_LENGTH, this->next_send_at_); + // Keep the pause after the last acknowledgement; a start time further ahead than that is long past, + // and after 2^31 ms the run_sequence_() check would read it as the future + const bool in_gap = this->next_send_at_ - now <= COMMAND_GAP_MS; + this->start_sequence_(WRITE_SEQUENCE, WRITE_SEQUENCE_LENGTH, in_gap ? this->next_send_at_ : now); #endif } } diff --git a/tests/components/ld2410s/ld2410s_parameter_write.cpp b/tests/components/ld2410s/ld2410s_parameter_write.cpp new file mode 100644 index 00000000000..2f32aa47ae8 --- /dev/null +++ b/tests/components/ld2410s/ld2410s_parameter_write.cpp @@ -0,0 +1,179 @@ +#include +#include +#include +#include +#include "esphome/components/ld2410s/ld2410s.h" +#include "esphome/components/uart/uart_component.h" +#include "esphome/core/application.h" + +#if !defined(USE_LD2410S_PARAMETERS) || !defined(USE_LD2410S_VERSION) +#error "tests/components/ld2410s/number/__init__.py must define the LD2410S features" +#endif + +namespace esphome::ld2410s::testing { + +// Vectors, so EXPECT_EQ prints both command lists on a mismatch +static const std::vector INIT_COMMANDS = {0x00FF, 0x0000, 0x0071, 0x007A, 0x00FE}; +static const std::vector WRITE_COMMANDS = {0x00FF, 0x0070, 0x00FE}; +static const std::vector CONFIG_MODE_ON = {0x00FF}; +static constexpr uint32_t STEP_MS = 300; +// 0x0070 frame: 4 header + 2 length + 2 command + 6 * (2 key + 4 value) + 4 footer +static constexpr size_t WRITE_FRAME_SIZE = 48; +static constexpr size_t MAX_DISTANCE_VALUE_POS = 10; +static constexpr size_t NO_DELAY_VALUE_POS = 22; +// Init far from the wrap, across it, and ending just before it: nine 300 ms passes put the last +// acknowledgement at 0xFFFFFF00, so the gap itself crosses the wrap +static constexpr uint32_t START_FAR_FROM_WRAP = 1000; +static constexpr uint32_t START_ACROSS_WRAP = 0xFFFFFC00; +static constexpr uint32_t START_GAP_ACROSS_WRAP = 0xFFFFF474; + +// Acknowledges every command in the layout ld2410s.cpp parses; a fixture, not a hardware claim +class FakeUART : public uart::UARTComponent { + public: + void write_array(const uint8_t *data, size_t len) override { + const uint16_t command = static_cast(data[6] | (data[7] << 8)); + this->commands.push_back(command); + this->frames.emplace_back(data, data + len); + std::vector payload = {data[6], static_cast(data[7] | 0x01), 0x00, 0x00}; + if (command == 0x0071) // six 32-bit values, each within a byte + payload.insert(payload.end(), {8, 0, 0, 0, 1, 0, 0, 0, 10, 0, 0, 0, 20, 0, 0, 0, 20, 0, 0, 0, 5, 0, 0, 0}); + if (command == 0x0000) // device type and three version words + payload.insert(payload.end(), size_t{10}, uint8_t{0}); + std::vector frame = {0xFD, 0xFC, 0xFB, 0xFA, static_cast(payload.size()), 0x00}; + frame.insert(frame.end(), payload.begin(), payload.end()); + frame.insert(frame.end(), {0x04, 0x03, 0x02, 0x01}); + this->queue(frame); + } + void queue(const std::vector &bytes) { this->rx_.insert(this->rx_.end(), bytes.begin(), bytes.end()); } + bool read_array(uint8_t *data, size_t len) override { + if (this->rx_.size() - this->pos_ < len) + return false; + std::copy_n(this->rx_.begin() + this->pos_, len, data); + this->pos_ += len; + return true; + } + size_t available() override { return this->rx_.size() - this->pos_; } + bool peek_byte(uint8_t *data) override { return false; } + uart::UARTFlushResult flush() override { return uart::UARTFlushResult::UART_FLUSH_RESULT_SUCCESS; } + void check_logger_conflict() override {} +#if defined(USE_ESP8266) || defined(USE_ESP32) + void load_settings(bool dump_config) override {} +#endif + + std::vector commands; + std::vector> frames; + + protected: + std::vector rx_; + size_t pos_{0}; +}; + +class LD2410SWriteTest : public ::testing::Test { + public: + LD2410SWriteTest() { this->radar_.set_uart_parent(&this->uart_); } + + protected: + void loop_at_(uint32_t now) { + LoopBlockingGuard guard{nullptr, nullptr, now}; + this->radar_.loop(); + } + // Runs init until the configuration-mode-off acknowledgement is taken, then queues one change + void finish_init_(uint32_t now) { + for (int pass = 0; pass < 20; pass++, now += STEP_MS) { + this->loop_at_(now); + if (!this->uart_.commands.empty() && this->uart_.commands.back() == 0x00FE && this->uart_.available() == 0) + break; + } + ASSERT_EQ(this->uart_.commands, INIT_COMMANDS); + ASSERT_EQ(this->uart_.available(), 0u); + ASSERT_TRUE(this->radar_.set_parameter(Parameter::PARAMETER_NO_DELAY, 30)); + this->acked_ = now; + this->sent_ = this->uart_.commands.size(); + } + // Loops every 300 ms with a short data frame before each pass, so the link check never restarts init + std::vector write_from_(uint32_t now, int passes) { + for (int pass = 0; pass < passes; pass++, now += STEP_MS) { + this->uart_.queue({0x6E, 0x02, 0x64, 0x00, 0x62}); + this->loop_at_(now); + } + return {this->uart_.commands.begin() + this->sent_, this->uart_.commands.end()}; + } + void check_waits_for_gap_(uint32_t start, uint32_t elapsed) { + this->finish_init_(start); + if (::testing::Test::HasFatalFailure()) + return; + EXPECT_TRUE(this->write_from_(this->acked_ + elapsed, 1).empty()); + EXPECT_EQ(this->write_from_(this->acked_ + STEP_MS, 1), CONFIG_MODE_ON); + EXPECT_EQ(this->write_from_(this->acked_ + 2 * STEP_MS, 5), WRITE_COMMANDS); + } + void check_starts_on_first_pass_(uint32_t start, uint32_t elapsed) { + this->finish_init_(start); + if (::testing::Test::HasFatalFailure()) + return; + const uint32_t now = this->acked_ + elapsed; + EXPECT_EQ(this->write_from_(now, 1), CONFIG_MODE_ON); + // The write, not a re-init (which would send 0000 next) + ASSERT_EQ(this->write_from_(now + STEP_MS, 5), WRITE_COMMANDS); + const auto &frame = this->uart_.frames[this->sent_ + 1]; + ASSERT_EQ(frame.size(), WRITE_FRAME_SIZE); + EXPECT_EQ(frame[NO_DELAY_VALUE_POS], 30); + EXPECT_EQ(frame[MAX_DISTANCE_VALUE_POS], 8); // the module's own value, read during init + } + + FakeUART uart_; + LD2410SComponent radar_; + uint32_t acked_{0}; + size_t sent_{0}; +}; + +// Time since the last acknowledgement is inside the 300 ms gap: nothing is sent until it ends +class LD2410SGapTest : public LD2410SWriteTest {}; +// Time since the last acknowledgement is at or past the gap end, up to the full 32-bit range +class LD2410SStartTest : public LD2410SWriteTest {}; + +TEST_F(LD2410SGapTest, FarFromWrapElapsed0) { this->check_waits_for_gap_(START_FAR_FROM_WRAP, 0); } +TEST_F(LD2410SGapTest, FarFromWrapElapsed100) { this->check_waits_for_gap_(START_FAR_FROM_WRAP, 100); } +TEST_F(LD2410SGapTest, FarFromWrapElapsed299) { this->check_waits_for_gap_(START_FAR_FROM_WRAP, 299); } +TEST_F(LD2410SGapTest, AcrossWrapElapsed0) { this->check_waits_for_gap_(START_ACROSS_WRAP, 0); } +TEST_F(LD2410SGapTest, AcrossWrapElapsed100) { this->check_waits_for_gap_(START_ACROSS_WRAP, 100); } +TEST_F(LD2410SGapTest, AcrossWrapElapsed299) { this->check_waits_for_gap_(START_ACROSS_WRAP, 299); } +TEST_F(LD2410SGapTest, GapAcrossWrapElapsed0) { this->check_waits_for_gap_(START_GAP_ACROSS_WRAP, 0); } +TEST_F(LD2410SGapTest, GapAcrossWrapElapsed100) { this->check_waits_for_gap_(START_GAP_ACROSS_WRAP, 100); } +TEST_F(LD2410SGapTest, GapAcrossWrapElapsed299) { this->check_waits_for_gap_(START_GAP_ACROSS_WRAP, 299); } + +TEST_F(LD2410SStartTest, FarFromWrapGapEnd) { this->check_starts_on_first_pass_(START_FAR_FROM_WRAP, 300); } +TEST_F(LD2410SStartTest, FarFromWrapGapEndPlus1) { this->check_starts_on_first_pass_(START_FAR_FROM_WRAP, 301); } +TEST_F(LD2410SStartTest, FarFromWrapHalfRangeMinus1) { + this->check_starts_on_first_pass_(START_FAR_FROM_WRAP, 0x8000012B); +} +TEST_F(LD2410SStartTest, FarFromWrapHalfRange) { this->check_starts_on_first_pass_(START_FAR_FROM_WRAP, 0x8000012C); } +TEST_F(LD2410SStartTest, FarFromWrapThreeQuarterRange) { + this->check_starts_on_first_pass_(START_FAR_FROM_WRAP, 0xC0000000); +} +TEST_F(LD2410SStartTest, FarFromWrapFullRange) { this->check_starts_on_first_pass_(START_FAR_FROM_WRAP, 0xFFFFFFFF); } +TEST_F(LD2410SStartTest, AcrossWrapGapEnd) { this->check_starts_on_first_pass_(START_ACROSS_WRAP, 300); } +TEST_F(LD2410SStartTest, AcrossWrapGapEndPlus1) { this->check_starts_on_first_pass_(START_ACROSS_WRAP, 301); } +TEST_F(LD2410SStartTest, AcrossWrapHalfRangeMinus1) { + this->check_starts_on_first_pass_(START_ACROSS_WRAP, 0x8000012B); +} +TEST_F(LD2410SStartTest, AcrossWrapHalfRange) { this->check_starts_on_first_pass_(START_ACROSS_WRAP, 0x8000012C); } +TEST_F(LD2410SStartTest, AcrossWrapThreeQuarterRange) { + this->check_starts_on_first_pass_(START_ACROSS_WRAP, 0xC0000000); +} +TEST_F(LD2410SStartTest, AcrossWrapFullRange) { this->check_starts_on_first_pass_(START_ACROSS_WRAP, 0xFFFFFFFF); } +TEST_F(LD2410SStartTest, GapAcrossWrapGapEnd) { this->check_starts_on_first_pass_(START_GAP_ACROSS_WRAP, 300); } +TEST_F(LD2410SStartTest, GapAcrossWrapGapEndPlus1) { this->check_starts_on_first_pass_(START_GAP_ACROSS_WRAP, 301); } +TEST_F(LD2410SStartTest, GapAcrossWrapHalfRangeMinus1) { + this->check_starts_on_first_pass_(START_GAP_ACROSS_WRAP, 0x8000012B); +} +TEST_F(LD2410SStartTest, GapAcrossWrapHalfRange) { + this->check_starts_on_first_pass_(START_GAP_ACROSS_WRAP, 0x8000012C); +} +TEST_F(LD2410SStartTest, GapAcrossWrapThreeQuarterRange) { + this->check_starts_on_first_pass_(START_GAP_ACROSS_WRAP, 0xC0000000); +} +TEST_F(LD2410SStartTest, GapAcrossWrapFullRange) { + this->check_starts_on_first_pass_(START_GAP_ACROSS_WRAP, 0xFFFFFFFF); +} + +} // namespace esphome::ld2410s::testing diff --git a/tests/components/ld2410s/number/__init__.py b/tests/components/ld2410s/number/__init__.py new file mode 100644 index 00000000000..31446f8d549 --- /dev/null +++ b/tests/components/ld2410s/number/__init__.py @@ -0,0 +1,16 @@ +import esphome.codegen as cg +from esphome.core import CORE +from esphome.types import ConfigType +from tests.testing_helpers import ComponentManifestOverride + + +def override_manifest(manifest: ComponentManifestOverride) -> None: + # The hub's to_code emits no feature defines; this platform stub turns on the parameter and + # version code in the real ld2410s.cpp for the host test build + async def to_code_testing(config: ConfigType) -> None: + CORE.testing_ensure_platform_registered("text_sensor") + cg.add_define("USE_LD2410S_PARAMETERS") + cg.add_define("USE_LD2410S_VERSION") + + manifest.to_code = to_code_testing + manifest.dependencies = manifest.dependencies + ["text_sensor"]