Check the unknown name no-op before any select write so no timer can race it

esphome/components/ld2412/ld2412.cpp

@@ -9,6 +9,7 @@
 
 #include "esphome/core/application.h"
 #include "esphome/core/helpers.h"
+#include "esphome/core/progmem.h"
 
 #include <algorithm>
 #include <cmath>
@@ -17,23 +18,6 @@ namespace esphome::ld2412 {
 
 ESPHOME_LOG_TAG(TAG, "ld2412");
 
-enum BaudRate : uint8_t {
-  BAUD_RATE_9600 = 1,
-  BAUD_RATE_19200 = 2,
-  BAUD_RATE_38400 = 3,
-  BAUD_RATE_57600 = 4,
-  BAUD_RATE_115200 = 5,
-  BAUD_RATE_230400 = 6,
-  BAUD_RATE_256000 = 7,
-  BAUD_RATE_460800 = 8,
-};
-
-enum DistanceResolution : uint8_t {
-  DISTANCE_RESOLUTION_0_2 = 0x03,
-  DISTANCE_RESOLUTION_0_5 = 0x01,
-  DISTANCE_RESOLUTION_0_75 = 0x00,
-};
-
 enum LightFunction : uint8_t {
   LIGHT_FUNCTION_OFF = 0x00,
   LIGHT_FUNCTION_BELOW = 0x01,
@@ -82,77 +66,18 @@ enum AckData : uint8_t {
   ACK_PAYLOAD = 10,
 };
 
-// Memory-efficient lookup tables
-struct StringToUint8 {
-  const char *str;
-  const uint8_t value;
-};
+// Index + 1 is the BaudRate value; the select options use the same order
+PROGMEM_STRING_TABLE(BaudRateStrings, "9600", "19200", "38400", "57600", "115200", "230400", "256000", "460800");
+static_assert(BAUD_RATE_9600 == 1 && BAUD_RATE_460800 == BaudRateStrings::COUNT, "BaudRate is index + 1");
+// Same order as DISTANCE_RESOLUTIONS
+PROGMEM_STRING_TABLE(DistanceResolutionStrings, "0.2m", "0.5m", "0.75m");
+// The light function select options are in enum order
+static_assert(LIGHT_FUNCTION_OFF == 0 && LIGHT_FUNCTION_BELOW == 1 && LIGHT_FUNCTION_ABOVE == 2,
+              "LightFunction is the select option index");
+// Device values of the out pin level select options, in option order
+static constexpr uint8_t OUT_PIN_LEVELS[] PROGMEM = {OUT_PIN_LEVEL_LOW, OUT_PIN_LEVEL_HIGH};
 
-struct Uint8ToString {
-  const uint8_t value;
-  const char *str;
-};
-
-constexpr StringToUint8 BAUD_RATES_BY_STR[] = {
-    {"9600", BAUD_RATE_9600},     {"19200", BAUD_RATE_19200},   {"38400", BAUD_RATE_38400},
-    {"57600", BAUD_RATE_57600},   {"115200", BAUD_RATE_115200}, {"230400", BAUD_RATE_230400},
-    {"256000", BAUD_RATE_256000}, {"460800", BAUD_RATE_460800},
-};
-
-constexpr StringToUint8 DISTANCE_RESOLUTIONS_BY_STR[] = {
-    {"0.2m", DISTANCE_RESOLUTION_0_2},
-    {"0.5m", DISTANCE_RESOLUTION_0_5},
-    {"0.75m", DISTANCE_RESOLUTION_0_75},
-};
-
-constexpr Uint8ToString DISTANCE_RESOLUTIONS_BY_UINT[] = {
-    {DISTANCE_RESOLUTION_0_2, "0.2m"},
-    {DISTANCE_RESOLUTION_0_5, "0.5m"},
-    {DISTANCE_RESOLUTION_0_75, "0.75m"},
-};
-
-constexpr StringToUint8 LIGHT_FUNCTIONS_BY_STR[] = {
-    {"off", LIGHT_FUNCTION_OFF},
-    {"below", LIGHT_FUNCTION_BELOW},
-    {"above", LIGHT_FUNCTION_ABOVE},
-};
-
-constexpr Uint8ToString LIGHT_FUNCTIONS_BY_UINT[] = {
-    {LIGHT_FUNCTION_OFF, "off"},
-    {LIGHT_FUNCTION_BELOW, "below"},
-    {LIGHT_FUNCTION_ABOVE, "above"},
-};
-
-constexpr StringToUint8 OUT_PIN_LEVELS_BY_STR[] = {
-    {"low", OUT_PIN_LEVEL_LOW},
-    {"high", OUT_PIN_LEVEL_HIGH},
-};
-
-constexpr Uint8ToString OUT_PIN_LEVELS_BY_UINT[] = {
-    {OUT_PIN_LEVEL_LOW, "low"},
-    {OUT_PIN_LEVEL_HIGH, "high"},
-};
-
-constexpr uint32_t BAUD_RATES[] = {9600, 19200, 38400, 57600, 115200, 230400, 256000, 460800};
-
-// Helper functions for lookups
-template<size_t N> uint8_t find_uint8(const StringToUint8 (&arr)[N], const char *str) {
-  for (const auto &entry : arr) {
-    if (strcmp(str, entry.str) == 0) {
-      return entry.value;
-    }
-  }
-  return 0xFF;  // Not found
-}
-
-template<size_t N> const char *find_str(const Uint8ToString (&arr)[N], uint8_t value) {
-  for (const auto &entry : arr) {
-    if (value == entry.value) {
-      return entry.str;
-    }
-  }
-  return "";  // Not found
-}
+static constexpr uint32_t BAUD_RATES[] PROGMEM = {9600, 19200, 38400, 57600, 115200, 230400, 256000, 460800};
 
 static constexpr uint8_t DEFAULT_PRESENCE_TIMEOUT = 5;  // Default used when number component is not defined
 // Commands
@@ -532,11 +457,16 @@ bool LD2412Component::handle_ack_data_() {
       break;
     }
     case CMD_QUERY_DISTANCE_RESOLUTION: {
-      const auto *distance_resolution = find_str(DISTANCE_RESOLUTIONS_BY_UINT, this->buffer_data_[10]);
-      ESP_LOGV(TAG, "Distance resolution: %s", distance_resolution);
+      const auto index = ld24xx::find_index(DISTANCE_RESOLUTIONS, this->buffer_data_[10]);
+      if (!index) {
+        ESP_LOGW(TAG, "Unknown distance resolution %u", this->buffer_data_[10]);
+        break;
+      }
+      ESP_LOGV(TAG, "Distance resolution: %s",
+               LOG_STR_ARG(DistanceResolutionStrings::get_log_str(*index, DistanceResolutionStrings::LAST_INDEX)));
 #ifdef USE_SELECT
       if (this->distance_resolution_select_ != nullptr) {
-        this->distance_resolution_select_->publish_state(distance_resolution);
+        this->distance_resolution_select_->publish_state(*index);
       }
 #endif
       break;
@@ -545,11 +475,10 @@ bool LD2412Component::handle_ack_data_() {
     case CMD_QUERY_LIGHT_CONTROL: {
       this->light_function_ = this->buffer_data_[10];
       this->light_threshold_ = this->buffer_data_[11];
-      const auto *light_function_str = find_str(LIGHT_FUNCTIONS_BY_UINT, this->light_function_);
-      ESP_LOGV(TAG, "Light function: %s, threshold: %u", light_function_str, this->light_threshold_);
+      ESP_LOGV(TAG, "Light function: %u, threshold: %u", this->light_function_, this->light_threshold_);
 #ifdef USE_SELECT
       if (this->light_function_select_ != nullptr) {
-        this->light_function_select_->publish_state(light_function_str);
+        this->light_function_select_->publish_state(static_cast<size_t>(this->light_function_));
       }
 #endif
 #ifdef USE_NUMBER
@@ -667,9 +596,10 @@ bool LD2412Component::handle_ack_data_() {
       */
       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);
+        if (const auto index = ld24xx::find_index(OUT_PIN_LEVELS, this->out_pin_level_)) {
+          this->out_pin_level_select_->publish_state(*index);
+        }
       }
 #endif
 #endif
@@ -729,15 +659,29 @@ void LD2412Component::set_bluetooth(bool enable) {
 }
 
 void LD2412Component::set_distance_resolution(const char *state) {
+  const int16_t index = progmem_string_table_find<DistanceResolutionStrings>(state, strlen(state));
+  if (index >= 0) {
+    this->set_distance_resolution(static_cast<DistanceResolution>(progmem_read_byte(&DISTANCE_RESOLUTIONS[index])));
+  }
+}
+
+void LD2412Component::set_distance_resolution(DistanceResolution resolution) {
   this->set_config_mode_(true);
-  const uint8_t cmd_value[6] = {find_uint8(DISTANCE_RESOLUTIONS_BY_STR, state), 0x00, 0x00, 0x00, 0x00, 0x00};
+  const uint8_t cmd_value[6] = {resolution, 0x00, 0x00, 0x00, 0x00, 0x00};
   this->send_command_(CMD_SET_DISTANCE_RESOLUTION, cmd_value, sizeof(cmd_value));
   this->set_timeout(200, [this]() { this->restart_and_read_all_info(); });
 }
 
 void LD2412Component::set_baud_rate(const char *state) {
+  const int16_t index = progmem_string_table_find<BaudRateStrings>(state, strlen(state));
+  if (index >= 0) {
+    this->set_baud_rate(static_cast<BaudRate>(index + 1));
+  }
+}
+
+void LD2412Component::set_baud_rate(BaudRate rate) {
   this->set_config_mode_(true);
-  const uint8_t cmd_value[2] = {find_uint8(BAUD_RATES_BY_STR, state), 0x00};
+  const uint8_t cmd_value[2] = {rate, 0x00};
   this->send_command_(CMD_SET_BAUD_RATE, cmd_value, sizeof(cmd_value));
   this->set_timeout(200, [this]() { this->restart_(); });
 }
@@ -819,9 +763,10 @@ void LD2412Component::set_basic_config() {
 #endif
 #ifdef USE_SELECT
   if (this->out_pin_level_select_ != nullptr) {
-    if (!this->out_pin_level_select_->has_state())
+    const auto index = this->out_pin_level_select_->active_index();
+    if (!index)
       return;
-    out_pin_level = find_uint8(OUT_PIN_LEVELS_BY_STR, this->out_pin_level_select_->current_option().c_str());
+    out_pin_level = progmem_read_byte(&OUT_PIN_LEVELS[*index]);
   }
 #endif
 
@@ -893,8 +838,10 @@ void LD2412Component::set_light_out_control() {
   }
 #endif
 #ifdef USE_SELECT
-  if (this->light_function_select_ != nullptr && this->light_function_select_->has_state()) {
-    this->light_function_ = find_uint8(LIGHT_FUNCTIONS_BY_STR, this->light_function_select_->current_option().c_str());
+  if (this->light_function_select_ != nullptr) {
+    if (const auto index = this->light_function_select_->active_index()) {
+      this->light_function_ = *index;
+    }
   }
 #endif
   uint8_t value[2] = {this->light_function_, this->light_threshold_};

esphome/components/ld2412/ld2412.h

@@ -36,6 +36,27 @@ 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
 
+enum BaudRate : uint8_t {
+  BAUD_RATE_9600 = 1,
+  BAUD_RATE_19200 = 2,
+  BAUD_RATE_38400 = 3,
+  BAUD_RATE_57600 = 4,
+  BAUD_RATE_115200 = 5,
+  BAUD_RATE_230400 = 6,
+  BAUD_RATE_256000 = 7,
+  BAUD_RATE_460800 = 8,
+};
+
+enum DistanceResolution : uint8_t {
+  DISTANCE_RESOLUTION_0_2 = 0x03,
+  DISTANCE_RESOLUTION_0_5 = 0x01,
+  DISTANCE_RESOLUTION_0_75 = 0x00,
+};
+
+// Device values of the distance resolution select options, in option order; in flash on ESP8266
+static constexpr uint8_t DISTANCE_RESOLUTIONS[] PROGMEM = {DISTANCE_RESOLUTION_0_2, DISTANCE_RESOLUTION_0_5,
+                                                           DISTANCE_RESOLUTION_0_75};
+
 class LD2412Component final : public Component, public uart::UARTDevice {
 #ifdef USE_BINARY_SENSOR
   SUB_BINARY_SENSOR(dynamic_background_correction_status)
@@ -103,7 +124,9 @@ class LD2412Component final : public Component, public uart::UARTDevice {
   void restart_and_read_all_info();
   void set_bluetooth(bool enable);
   void set_distance_resolution(const char *state);
+  void set_distance_resolution(DistanceResolution resolution);
   void set_baud_rate(const char *state);
+  void set_baud_rate(BaudRate rate);
   void factory_reset();
   void start_dynamic_background_correction();
 

esphome/components/ld2412/select/__init__.py

@@ -52,6 +52,7 @@ CONFIG_SCHEMA = {
 async def to_code(config: ConfigType) -> None:
     hub = await cg.get_variable(config[CONF_LD2412_ID])
     selects = select.sub_selects(config, parent=hub)
+    # Option lists follow the order of the C++ enums and tables; the hub maps by index
     await selects(
         CONF_BAUD_RATE,
         hub.set_baud_rate_select,

esphome/components/ld2412/select/baud_rate_select.cpp

@@ -4,7 +4,7 @@ namespace esphome::ld2412 {
 
 void BaudRateSelect::control(size_t index) {
   this->publish_state(index);
-  this->parent_->set_baud_rate(this->option_at(index));
+  this->parent_->set_baud_rate(static_cast<BaudRate>(index + 1));
 }
 
 }  // namespace esphome::ld2412

esphome/components/ld2412/select/distance_resolution_select.cpp

@@ -4,7 +4,8 @@ namespace esphome::ld2412 {
 
 void DistanceResolutionSelect::control(size_t index) {
   this->publish_state(index);
-  this->parent_->set_distance_resolution(this->option_at(index));
+  this->parent_->set_distance_resolution(
+      static_cast<DistanceResolution>(progmem_read_byte(&DISTANCE_RESOLUTIONS[index])));
 }
 
 }  // namespace esphome::ld2412

esphome/components/ld24xx/ld24xx.h

@@ -1,6 +1,7 @@
 #pragma once
 
 #include "esphome/core/defines.h"
+#include "esphome/core/hal.h"
 #include "esphome/core/helpers.h"
 
 #include <memory>
@@ -24,10 +25,17 @@
 
 namespace esphome::ld24xx {
 
-// Helper to find index of value in constexpr array
-template<size_t N> optional<size_t> find_index(const uint32_t (&arr)[N], uint32_t value) {
+// Index of value in a constant table that may live in flash, or none
+template<typename T, size_t N> optional<size_t> find_index(const T (&arr)[N], T value) {
+  static_assert(sizeof(T) == 1 || sizeof(T) == 4, "flash tables are read as bytes or aligned 32 bit words");
   for (size_t i = 0; i < N; i++) {
-    if (arr[i] == value)
+    T entry;
+    if constexpr (sizeof(T) == 1) {
+      entry = progmem_read_byte(reinterpret_cast<const uint8_t *>(&arr[i]));
+    } else {
+      entry = arr[i];  // an aligned 32 bit read from flash is safe on ESP8266
+    }
+    if (entry == value)
       return i;
   }
   return {};

tests/integration/fixtures/uart_mock_ld2412_selects.yaml

@@ -0,0 +1,68 @@
+esphome:
+  name: uart-mock-ld2412-selects
+
+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 distance resolution query with 0x01, the module's code for 0.5m
+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> RESOLUTION_QUERY = {0x02, 0x00, 0x11, 0x00};
+            if (data != RESOLUTION_QUERY) {
+              return;
+            }
+            id(mock_uart).inject_to_rx_buffer(std::vector<uint8_t>{
+                0xFD, 0xFC, 0xFB, 0xFA,  // header
+                0x06, 0x00,              // length 6
+                0x11, 0x01,              // answer to command 0x0011
+                0x00, 0x00,              // status: success
+                0x01, 0x00,              // resolution code 0x01 = 0.5m
+                0x04, 0x03, 0x02, 0x01,  // footer
+            });
+
+ld2412:
+  id: ld2412_dev
+  uart_id: mock_uart
+
+select:
+  - platform: ld2412
+    ld2412_id: ld2412_dev
+    distance_resolution:
+      name: "Distance Resolution"
+    baud_rate:
+      name: "Baud Rate"
+
+# Lambdas reach the name taking setters directly, which the API cannot do with an unknown name
+button:
+  - platform: template
+    name: "Bogus Names"
+    on_press:
+      - lambda: |-
+          id(ld2412_dev).set_distance_resolution("bogus");
+          id(ld2412_dev).set_baud_rate("bogus");
+          ESP_LOGI("test", "bogus names done");
+  - platform: template
+    name: "Named Values"
+    on_press:
+      - lambda: |-
+          id(ld2412_dev).set_distance_resolution("0.75m");
+          id(ld2412_dev).set_baud_rate("57600");

tests/integration/test_uart_mock_ld2412.py

@@ -24,6 +24,10 @@ 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_selects checks that the distance resolution and baud rate selects show the
+module's values and send the device code of the chosen option, that the name taking setters map a
+name the same way, and that an unknown name sends nothing at all.
 """
 
 from __future__ import annotations
@@ -39,6 +43,7 @@ from aioesphomeapi import (
     EntityState,
     NumberInfo,
     NumberState,
+    SelectInfo,
 )
 import pytest
 
@@ -57,6 +62,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"
+# Set distance resolution (6 payload bytes) and set baud rate (2 payload bytes)
+SET_RESOLUTION = "08:00:01:00"
+SET_BAUD_RATE = "04:00:A1:00"
 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 +689,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_selects(
+    yaml_config: str,
+    run_compiled: RunCompiledFunction,
+    api_client_connected: APIClientConnectedFactory,
+) -> None:
+    """Test that the selects show the module's values and send the device code of the chosen option."""
+    # 0.2m is device code 0x03; the module answered 0x01, which is 0.5m
+    expected_resolution = "TX 6 bytes: 03:00:00:00:00:00"
+    # 19200 is the second baud rate, so its code is 2
+    expected_baud_rate = "TX 2 bytes: 02:00"
+    # From the name taking setters: 0.75m is device code 0x00 and 57600 is the fourth baud rate
+    named_resolution = "TX 6 bytes: 00:00:00:00:00:00"
+    named_baud_rate = "TX 2 bytes: 04:00"
+    bogus_done = "bogus names done"
+    events = _LogEvents(bogus_done)
+
+    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)
+        resolution = require_entity(entities, "distance_resolution", SelectInfo)
+        baud_rate = require_entity(entities, "baud_rate", SelectInfo)
+        assert initial_state_helper.initial_states[resolution.key].state == "0.5m"
+        # The mock bus runs at 256000, which the select reports at boot
+        assert initial_state_helper.initial_states[baud_rate.key].state == "256000"
+
+        # An unknown name writes nothing, not even the configuration mode frames around a command.
+        # This runs before any select write, so no restart timer from a write can send a frame meanwhile.
+        before = len(events.lines)
+        done = events.next_line_with(bogus_done)
+        client.button_command(require_entity(entities, "bogus_names", ButtonInfo).key)
+        await wait_or_fail(done, 5.0, "Timeout waiting for the bogus names lambda")
+        assert [line for line in events.lines[before:] if _is_tx(line)] == []
+
+        resolution_written = events.next_line_with(expected_resolution)
+        client.select_command(resolution.key, "0.2m")
+        await wait_or_fail(
+            resolution_written,
+            5.0,
+            "Timeout waiting for the distance resolution command",
+            logged=events.lines,
+        )
+        assert events.count(SET_RESOLUTION) == 1
+
+        baud_rate_written = events.next_line_with(expected_baud_rate)
+        client.select_command(baud_rate.key, "19200")
+        await wait_or_fail(
+            baud_rate_written,
+            5.0,
+            "Timeout waiting for the baud rate command",
+            logged=events.lines,
+        )
+        assert events.count(SET_BAUD_RATE) == 1
+        # The payload line follows the length and command line directly
+        start = events.index_after(-1, SET_BAUD_RATE)
+        assert expected_baud_rate in events.lines[start + 1]
+
+        # A known name maps to the same device code as the select
+        named_resolution_written = events.next_line_with(named_resolution)
+        named_baud_rate_written = events.next_line_with(named_baud_rate)
+        client.button_command(require_entity(entities, "named_values", ButtonInfo).key)
+        await wait_or_fail(
+            asyncio.gather(named_resolution_written, named_baud_rate_written),
+            5.0,
+            "Timeout waiting for the named setter commands",
+            logged=events.lines,
+        )
+        assert events.count(SET_RESOLUTION) == 2
+        assert events.count(SET_BAUD_RATE) == 2