Merge branch 'dev' into ld2412_out_pin_level

esphome/components/ld2412/ld2412.cpp

@@ -154,7 +154,6 @@ template<size_t N> const char *find_str(const Uint8ToString (&arr)[N], uint8_t v
   return "";  // Not found
 }
 
-static constexpr uint8_t DEFAULT_PRESENCE_TIMEOUT = 5;  // Default used when number component is not defined
 // Commands
 static constexpr uint8_t CMD_ENABLE_CONF = 0xFF;
 static constexpr uint8_t CMD_DISABLE_CONF = 0xFE;
@@ -474,8 +473,10 @@ static inline bool has_value(number::Number *n) { return n != nullptr && n->has_
 
 // Timeout ids 0 (still) and 1 (motion) for a write waiting on a re-query
 static constexpr uint32_t GATE_QUERY_TIMEOUT_ID = 0;
-static constexpr uint32_t GATE_QUERY_TIMEOUT_MS = 2000;
 #endif
+// Timeout id for a basic configuration write waiting on a re-query, and how long any such write waits
+static constexpr uint32_t BASIC_CONF_TIMEOUT_ID = 2;
+static constexpr uint32_t QUERY_TIMEOUT_MS = 2000;
 
 bool LD2412Component::handle_ack_data_() {
   ESP_LOGV(TAG, "Handling ACK DATA for COMMAND %02X", this->buffer_data_[COMMAND]);
@@ -643,37 +644,42 @@ bool LD2412Component::handle_ack_data_() {
       break;
     }
 
-    case CMD_QUERY_BASIC_CONF:  // Query parameters response
-    {
-#ifdef USE_NUMBER
+    case CMD_QUERY_BASIC_CONF: {  // Query parameters response
       if (this->buffer_pos_ < 15) {
         return false;
       }
-      /*
-        Moving distance range: 9th byte
-        Still distance range: 10th byte
-      */
-      set_number_value(this->min_distance_gate_number_, this->buffer_data_[10]);
-      set_number_value(this->max_distance_gate_number_, this->buffer_data_[11] - 1);
-      ESP_LOGV(TAG, "min_distance_gate_number_: %u, max_distance_gate_number_ %u", this->buffer_data_[10],
-               this->buffer_data_[11]);
-      /*
-        None Duration: 11~12th bytes
-      */
-      set_number_value(this->timeout_number_, encode_uint16(this->buffer_data_[13], this->buffer_data_[12]));
-      ESP_LOGV(TAG, "timeout_number_: %u", encode_uint16(this->buffer_data_[13], this->buffer_data_[12]));
-      /*
-        Output pin configuration: 13th bytes
-      */
+      // True only while a held-back write waits for this answer
+      const bool pending = this->cancel_timeout(BASIC_CONF_TIMEOUT_ID);
+      this->basic_conf_read_ = true;
+      // Min gate: 9th byte, max gate: 10th byte, timeout: 11~12th bytes, output pin level: 13th byte
+      this->min_gate_ = this->buffer_data_[10];
+      this->max_gate_ = this->buffer_data_[11];
+      this->timeout_ = encode_uint16(this->buffer_data_[13], this->buffer_data_[12]);
       this->out_pin_level_ = this->buffer_data_[14];
-#ifdef USE_SELECT
-      const auto *out_pin_level_str = find_str(OUT_PIN_LEVELS_BY_UINT, this->out_pin_level_);
-      if (this->out_pin_level_select_ != nullptr) {
-        this->out_pin_level_select_->publish_state(out_pin_level_str);
+      ESP_LOGV(TAG, "min_distance_gate_number_: %u, max_distance_gate_number_ %u", this->min_gate_, this->max_gate_);
+      ESP_LOGV(TAG, "timeout_number_: %u", this->timeout_);
+      // The pending write's user values must survive the answer
+#ifdef USE_NUMBER
+      if (!pending || !has_value(this->min_distance_gate_number_)) {
+        set_number_value(this->min_distance_gate_number_, this->min_gate_);
+      }
+      if (!pending || !has_value(this->max_distance_gate_number_)) {
+        set_number_value(this->max_distance_gate_number_, this->max_gate_ - 1);
+      }
+      if (!pending || !has_value(this->timeout_number_)) {
+        set_number_value(this->timeout_number_, this->timeout_);
       }
 #endif
+#ifdef USE_SELECT
+      if (this->out_pin_level_select_ != nullptr && !(pending && this->out_pin_level_select_->has_state())) {
+        this->out_pin_level_select_->publish_state(find_str(OUT_PIN_LEVELS_BY_UINT, this->out_pin_level_));
+      }
 #endif
-    } break;
+      if (pending) {
+        this->set_basic_config();
+      }
+      break;
+    }
     default:
       break;
   }
@@ -771,8 +777,9 @@ void LD2412Component::set_engineering_mode(bool enable) {
 void LD2412Component::factory_reset() {
   this->set_config_mode_(true);
   this->send_command_(CMD_FACTORY_RESET, nullptr, 0);
+  // Reset discards the settings; the next write must wait for the re-read
+  this->basic_conf_read_ = false;
 #ifdef USE_NUMBER
-  // Reset discards the thresholds; the next write must wait for the re-read
   this->gate_move_thresholds_.read = false;
   this->gate_still_thresholds_.read = false;
 #endif
@@ -795,10 +802,22 @@ void LD2412Component::get_distance_resolution_() { this->send_command_(CMD_QUERY
 void LD2412Component::query_light_control_() { this->send_command_(CMD_QUERY_LIGHT_CONTROL, nullptr, 0); }
 
 void LD2412Component::set_basic_config() {
-  uint8_t min_gate = 1;
-  uint8_t max_gate = TOTAL_GATES;
-  uint16_t timeout = DEFAULT_PRESENCE_TIMEOUT;
-  uint8_t out_pin_level = 0x01;
+  // Entities that are not configured keep what the module last reported
+  uint8_t min_gate = this->min_gate_;
+  uint8_t max_gate = this->max_gate_;
+  uint16_t timeout = this->timeout_;
+  uint8_t out_pin_level = this->out_pin_level_;
+  bool from_module = false;
+
+#ifdef USE_NUMBER
+  from_module = this->min_distance_gate_number_ == nullptr || this->max_distance_gate_number_ == nullptr ||
+                this->timeout_number_ == nullptr;
+#else
+  from_module = true;
+#endif
+#ifndef USE_SELECT
+  from_module = true;
+#endif
 
 #ifdef USE_NUMBER
   if (this->min_distance_gate_number_ != nullptr) {
@@ -822,8 +841,19 @@ void LD2412Component::set_basic_config() {
     if (!this->out_pin_level_select_->has_state())
       return;
     out_pin_level = find_uint8(OUT_PIN_LEVELS_BY_STR, this->out_pin_level_select_->current_option().c_str());
+  } else {
+    from_module = true;
   }
 #endif
+  if (from_module && !this->basic_conf_read_) {
+    // The module never reported, so the values on hand are the built in defaults
+    ESP_LOGW(TAG, "Command %02X held back until the module reports its settings", CMD_BASIC_CONF);
+    this->set_config_mode_(true);
+    this->query_parameters_();
+    this->set_config_mode_(false);
+    this->set_timeout(BASIC_CONF_TIMEOUT_ID, QUERY_TIMEOUT_MS, [this]() { this->basic_conf_timeout_(); });
+    return;
+  }
 
   uint8_t value[5] = {
       lowbyte(min_gate), lowbyte(max_gate), lowbyte(timeout), highbyte(timeout), out_pin_level,
@@ -833,6 +863,24 @@ void LD2412Component::set_basic_config() {
   this->set_config_mode_(false);
 }
 
+void LD2412Component::basic_conf_timeout_() {
+  ESP_LOGE(TAG, "Command %02X not sent, the module did not report its settings", CMD_BASIC_CONF);
+#ifdef USE_NUMBER
+  // The values on hand never reached the module, so forgetting them loses nothing
+  for (number::Number *n : {this->min_distance_gate_number_, this->max_distance_gate_number_, this->timeout_number_}) {
+    if (n != nullptr) {
+      n->publish_state(NAN);
+    }
+  }
+#endif
+#ifdef USE_SELECT
+  // Show the last known or default level until the module reports one
+  if (this->out_pin_level_select_ != nullptr) {
+    this->out_pin_level_select_->publish_state(find_str(OUT_PIN_LEVELS_BY_UINT, this->out_pin_level_));
+  }
+#endif
+}
+
 #ifdef USE_NUMBER
 void LD2412Component::send_gate_thresholds_(bool motion) {
   GateThresholds &group = this->gate_thresholds_(motion);
@@ -850,9 +898,9 @@ void LD2412Component::send_gate_thresholds_(bool motion) {
   }
   if (configured < TOTAL_GATES && !group.read) {
     // Unread gates would go out as zero, which is maximum sensitivity
-    ESP_LOGW(TAG, "Command %02X held back until the module reports its gate thresholds", command);
+    ESP_LOGW(TAG, "Command %02X held back until the module reports its settings", command);
     this->send_command_(motion ? CMD_QUERY_MOTION_GATE_SENS : CMD_QUERY_STATIC_GATE_SENS, nullptr, 0);
-    this->set_timeout(GATE_QUERY_TIMEOUT_ID + motion, GATE_QUERY_TIMEOUT_MS,
+    this->set_timeout(GATE_QUERY_TIMEOUT_ID + motion, QUERY_TIMEOUT_MS,
                       [this, motion]() { this->gate_query_timeout_(motion); });
     return;
   }
@@ -860,7 +908,7 @@ void LD2412Component::send_gate_thresholds_(bool motion) {
 }
 
 void LD2412Component::gate_query_timeout_(bool motion) {
-  ESP_LOGE(TAG, "Command %02X not sent, the module did not report its gate thresholds",
+  ESP_LOGE(TAG, "Command %02X not sent, the module did not report its settings",
            motion ? CMD_MOTION_GATE_SENS : CMD_STATIC_GATE_SENS);
   // The values on hand never reached the module or are stale, so forgetting them loses nothing
   for (number::Number *n : this->gate_thresholds_(motion).numbers) {

esphome/components/ld2412/ld2412.h

@@ -35,6 +35,7 @@ using namespace ld24xx;
 
 static constexpr uint8_t MAX_LINE_LENGTH = 54;  // Max characters for serial buffer
 static constexpr uint8_t TOTAL_GATES = 14;      // Total number of gates supported by the LD2412
+static constexpr uint16_t DEFAULT_PRESENCE_TIMEOUT = 5;
 
 class LD2412Component final : public Component, public uart::UARTDevice {
 #ifdef USE_BINARY_SENSOR
@@ -112,6 +113,7 @@ class LD2412Component final : public Component, public uart::UARTDevice {
   void set_config_mode_(bool enable);
   void handle_periodic_data_();
   bool handle_ack_data_();
+  void basic_conf_timeout_();
   void readline_(int readch);
   void query_parameters_();
   void get_version_();
@@ -136,7 +138,12 @@ class LD2412Component final : public Component, public uart::UARTDevice {
 
   uint8_t light_function_ = 0;
   uint8_t light_threshold_ = 0;
-  uint8_t out_pin_level_ = 0;
+  // Basic configuration as last reported by the module; resent for entities that are not configured
+  bool basic_conf_read_{false};  // answered since boot or the last factory reset
+  uint16_t timeout_ = DEFAULT_PRESENCE_TIMEOUT;
+  uint8_t min_gate_ = 1;
+  uint8_t max_gate_ = TOTAL_GATES;
+  uint8_t out_pin_level_ = 1;
   uint8_t buffer_pos_ = 0;  // where to resume processing/populating buffer
   uint8_t buffer_data_[MAX_LINE_LENGTH];
   uint8_t mac_address_[MAC_ADDRESS_SIZE] = {0, 0, 0, 0, 0, 0};

esphome/components/ld2412/select/__init__.py

@@ -42,7 +42,7 @@ CONFIG_SCHEMA = {
         icon=ICON_LIGHTBULB,
     ),
     cv.Optional(CONF_OUT_PIN_LEVEL): select.select_schema(
-        LightOutControlSelect,
+        select.ApplySelect,
         entity_category=ENTITY_CATEGORY_CONFIG,
         icon=ICON_SCALE,
     ),
@@ -76,8 +76,11 @@ async def to_code(config: ConfigType) -> None:
         hub.set_light_function_select,
         options=["off", "below", "above"],
     )
+    # The pin level travels in the basic configuration command, not the light control one
     await selects(
         CONF_OUT_PIN_LEVEL,
         hub.set_out_pin_level_select,
         options=["low", "high"],
+        apply="set_basic_config()",
+        publish="before",
     )

tests/integration/fixtures/uart_mock_ld2412_out_pin_level.yaml

@@ -0,0 +1,53 @@
+esphome:
+  name: uart-mock-ld2412-outpin
+
+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 ld2412's DEPENDENCIES = ["uart"]
+# The actual UART bus used is the uart_mock component below
+uart:
+  baud_rate: 115200
+  port: /dev/null
+
+# Answers the basic configuration query with gates 3 to 9, a 10 second timeout and a low output pin level
+uart_mock:
+  id: mock_uart
+  baud_rate: 256000
+  on_tx:
+    - then:
+        - lambda: |-
+            // The length and command word arrive as one write of their own
+            static const std::vector<uint8_t> BASIC_CONF_QUERY = {0x02, 0x00, 0x12, 0x00};
+            if (data != BASIC_CONF_QUERY) {
+              return;
+            }
+            id(mock_uart).inject_to_rx_buffer(std::vector<uint8_t>{
+                0xFD, 0xFC, 0xFB, 0xFA,  // header
+                0x09, 0x00,              // length 9
+                0x12, 0x01,              // answer to command 0x0012
+                0x00, 0x00,              // status: success
+                0x03, 0x09,              // min and max gate
+                0x0A, 0x00,              // timeout 10 s
+                0x01,                    // output pin level: low
+                0x04, 0x03, 0x02, 0x01,  // footer
+            });
+
+ld2412:
+  id: ld2412_dev
+  uart_id: mock_uart
+
+# No numbers on purpose: the select must resend what the module reported for them
+select:
+  - platform: ld2412
+    ld2412_id: ld2412_dev
+    out_pin_level:
+      name: "Out Pin Level"

tests/integration/fixtures/uart_mock_ld2412_out_pin_level_held_back.yaml

@@ -0,0 +1,66 @@
+esphome:
+  name: uart-mock-ld2412-outpin-held
+
+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 ld2412's DEPENDENCIES = ["uart"]
+# The actual UART bus used is the uart_mock component below
+uart:
+  baud_rate: 115200
+  port: /dev/null
+
+# Answers only the second basic configuration query, and only inside configuration mode, so the select
+# must hold its write back and ask again the way a real module expects
+uart_mock:
+  id: mock_uart
+  baud_rate: 256000
+  on_tx:
+    - then:
+        - lambda: |-
+            // The length and command word arrive as one write of their own
+            static const std::vector<uint8_t> BASIC_CONF_QUERY = {0x02, 0x00, 0x12, 0x00};
+            static const std::vector<uint8_t> ENABLE_CONF = {0x04, 0x00, 0xFF, 0x00};
+            static const std::vector<uint8_t> DISABLE_CONF = {0x02, 0x00, 0xFE, 0x00};
+            static bool config_mode = false;
+            static int queries = 0;
+            if (data == ENABLE_CONF) {
+              config_mode = true;
+              return;
+            }
+            if (data == DISABLE_CONF) {
+              config_mode = false;
+              return;
+            }
+            if (data != BASIC_CONF_QUERY || !config_mode || ++queries != 2) {
+              return;
+            }
+            id(mock_uart).inject_to_rx_buffer(std::vector<uint8_t>{
+                0xFD, 0xFC, 0xFB, 0xFA,  // header
+                0x09, 0x00,              // length 9
+                0x12, 0x01,              // answer to command 0x0012
+                0x00, 0x00,              // status: success
+                0x03, 0x09,              // min and max gate
+                0x0A, 0x00,              // timeout 10 s
+                0x01,                    // output pin level: low
+                0x04, 0x03, 0x02, 0x01,  // footer
+            });
+
+ld2412:
+  id: ld2412_dev
+  uart_id: mock_uart
+
+# No numbers on purpose: the select must resend what the module reported for them
+select:
+  - platform: ld2412
+    ld2412_id: ld2412_dev
+    out_pin_level:
+      name: "Out Pin Level"

tests/integration/test_uart_mock_ld2412.py

@@ -24,6 +24,11 @@ test_uart_mock_ld2412_engineering_truncated (truncated engineering mode):
 The gate threshold tests (footer_in_payload, truncated_answer, requery_after_giveup) cover
 answers that hold the frame footer or are truncated, writes that fill the gates without a
 number from the module's values, and writes held back until the module reports them.
+
+test_uart_mock_ld2412_out_pin_level checks that the output pin level select sends the basic
+configuration command with the gates and timeout the module reported when no number is configured.
+test_uart_mock_ld2412_out_pin_level_held_back checks that the select holds that write back and asks
+again while the module has not reported, then sends it with the reported values.
 """
 
 from __future__ import annotations
@@ -39,6 +44,7 @@ from aioesphomeapi import (
     EntityState,
     NumberInfo,
     NumberState,
+    SelectInfo,
 )
 import pytest
 
@@ -57,6 +63,9 @@ STILL_COMMAND = "10:00:04:00"
 MOTION_QUERY = "02:00:13:00"
 STILL_QUERY = "02:00:14:00"
 CONFIG_MODE_LEFT = "02:00:FE:00"
+BASIC_CONFIG = "07:00:02:00"
+BASIC_CONFIG_QUERY = "02:00:12:00"
+BASIC_CONFIG_HELD_BACK = "Command 02 held back"
 FACTORY_RESET = "02:00:A2:00"
 # Logged in place of the motion command when the write is held back or given up
 MOTION_HELD_BACK = "Command 03 held back"
@@ -681,3 +690,77 @@ async def test_uart_mock_ld2412_requery_after_giveup(
         assert gate_0_state.state == pytest.approx(25.0)
         assert gate_5_values
         assert all(value == pytest.approx(30.0) for value in gate_5_values)
+
+
+@pytest.mark.asyncio
+async def test_uart_mock_ld2412_out_pin_level(
+    yaml_config: str,
+    run_compiled: RunCompiledFunction,
+    api_client_connected: APIClientConnectedFactory,
+) -> None:
+    """Test that the output pin level select resends the module's own gates and timeout."""
+    # Gates 3 to 9 and the 10 second timeout as the module reported them, with the pin level set to high
+    expected_payload = "03:09:0A:00:00"
+    events = _LogEvents()
+
+    async with (
+        run_compiled(yaml_config, line_callback=events.on_line),
+        api_client_connected() as client,
+    ):
+        entities, _ = await client.list_entities_services()
+        initial_state_helper = await _wait_initial(client, entities)
+        select = require_entity(entities, "out_pin_level", SelectInfo)
+        assert initial_state_helper.initial_states[select.key].state == "low"
+
+        written = events.next_line_with(expected_payload)
+        client.select_command(select.key, "high")
+        await wait_or_fail(
+            written,
+            5.0,
+            "Timeout waiting for the basic configuration command",
+            expected_payload=expected_payload,
+            logged=events.lines,
+        )
+        # Exactly one basic configuration write, and not one built from the defaults
+        assert events.count(BASIC_CONFIG) == 1
+        assert events.count("01:0E:05:00") == 0
+
+
+@pytest.mark.asyncio
+async def test_uart_mock_ld2412_out_pin_level_held_back(
+    yaml_config: str,
+    run_compiled: RunCompiledFunction,
+    api_client_connected: APIClientConnectedFactory,
+) -> None:
+    """Test that the output pin level select waits for the module's settings before it writes."""
+    # Gates 3 to 9 and the 10 second timeout as the module reports them, with the pin level set to high
+    expected_payload = "03:09:0A:00:00"
+    events = _LogEvents(BASIC_CONFIG_HELD_BACK)
+
+    async with (
+        run_compiled(yaml_config, line_callback=events.on_line),
+        api_client_connected() as client,
+    ):
+        entities, _ = await client.list_entities_services()
+        await _wait_initial(client, entities)
+        select = require_entity(entities, "out_pin_level", SelectInfo)
+        # The boot query went unanswered, so nothing was written and the select has no state yet
+        assert events.count(BASIC_CONFIG) == 0
+
+        held_back = events.next_line_with(BASIC_CONFIG_HELD_BACK)
+        written = events.next_line_with(expected_payload)
+        client.select_command(select.key, "high")
+        await wait_or_fail(
+            asyncio.gather(held_back, written),
+            5.0,
+            "Timeout waiting for the held back basic configuration command",
+            expected_payload=expected_payload,
+            logged=events.lines,
+        )
+        # The write was held back once, asked again, and went out only after the answer
+        assert events.count(BASIC_CONFIG_QUERY) == 2
+        assert events.count(BASIC_CONFIG) == 1
+        assert events.index_after(-1, BASIC_CONFIG_HELD_BACK) < events.index_after(
+            -1, BASIC_CONFIG
+        )
+        assert events.count("01:0E:05:00") == 0