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

esphome/components/ld2410/ld2410.cpp

@@ -9,27 +9,12 @@
 
 #include "esphome/core/application.h"
 #include "esphome/core/helpers.h"
+#include "esphome/core/progmem.h"
 
 namespace esphome::ld2410 {
 
 ESPHOME_LOG_TAG(TAG, "ld2410");
 
-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 = 0x01,
-  DISTANCE_RESOLUTION_0_75 = 0x00,
-};
-
 enum LightFunction : uint8_t {
   LIGHT_FUNCTION_OFF = 0x00,
   LIGHT_FUNCTION_BELOW = 0x01,
@@ -69,73 +54,17 @@ enum AckData : uint8_t {
   COMMAND_STATUS = 7,
 };
 
-// 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.75m");
+// The light function and out pin level 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");
+static_assert(OUT_PIN_LEVEL_LOW == 0 && OUT_PIN_LEVEL_HIGH == 1, "OutPinLevel is the select option index");
 
-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.75m", DISTANCE_RESOLUTION_0_75},
-};
-
-constexpr Uint8ToString DISTANCE_RESOLUTIONS_BY_UINT[] = {
-    {DISTANCE_RESOLUTION_0_2, "0.2m"},
-    {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};
 
 // Commands
 static constexpr uint8_t CMD_ENABLE_CONF = 0xFF;
@@ -469,11 +398,16 @@ bool LD2410Component::handle_ack_data_() {
     }
 
     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;
@@ -483,16 +417,14 @@ bool LD2410Component::handle_ack_data_() {
       this->light_function_ = this->buffer_data_[10];
       this->light_threshold_ = this->buffer_data_[11];
       this->out_pin_level_ = this->buffer_data_[12];
-      const auto *light_function_str = find_str(LIGHT_FUNCTIONS_BY_UINT, this->light_function_);
-      const auto *out_pin_level_str = find_str(OUT_PIN_LEVELS_BY_UINT, this->out_pin_level_);
-      ESP_LOGV(TAG, "Light function: %s, threshold: %u, out pin level: %s", light_function_str, this->light_threshold_,
-               out_pin_level_str);
+      ESP_LOGV(TAG, "Light function: %u, threshold: %u, out pin level: %u", this->light_function_,
+               this->light_threshold_, this->out_pin_level_);
 #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_));
       }
       if (this->out_pin_level_select_ != nullptr) {
-        this->out_pin_level_select_->publish_state(out_pin_level_str);
+        this->out_pin_level_select_->publish_state(static_cast<size_t>(this->out_pin_level_));
       }
 #endif
 #ifdef USE_NUMBER
@@ -638,15 +570,29 @@ void LD2410Component::set_bluetooth(bool enable) {
 }
 
 void LD2410Component::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 LD2410Component::set_distance_resolution(DistanceResolution resolution) {
   this->set_config_mode_(true);
-  const uint8_t cmd_value[2] = {find_uint8(DISTANCE_RESOLUTIONS_BY_STR, state), 0x00};
+  const uint8_t cmd_value[2] = {resolution, 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 LD2410Component::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 LD2410Component::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_(); });
 }
@@ -769,11 +715,15 @@ void LD2410Component::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;
+    }
   }
-  if (this->out_pin_level_select_ != nullptr && this->out_pin_level_select_->has_state()) {
-    this->out_pin_level_ = find_uint8(OUT_PIN_LEVELS_BY_STR, this->out_pin_level_select_->current_option().c_str());
+  if (this->out_pin_level_select_ != nullptr) {
+    if (const auto index = this->out_pin_level_select_->active_index()) {
+      this->out_pin_level_ = *index;
+    }
   }
 #endif
   this->set_config_mode_(true);

esphome/components/ld2410/ld2410.h

@@ -37,6 +37,25 @@ using namespace ld24xx;
 static constexpr uint8_t MAX_LINE_LENGTH = 50;
 static constexpr uint8_t TOTAL_GATES = 9;  // Total number of gates supported by the LD2410
 
+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 = 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_75};
+
 class LD2410Component final : public Component, public uart::UARTDevice {
 #ifdef USE_BINARY_SENSOR
   SUB_BINARY_SENSOR(out_pin_presence_status)
@@ -99,7 +118,9 @@ class LD2410Component 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();
 
  protected:

esphome/components/ld2410/select/__init__.py

@@ -52,6 +52,7 @@ CONFIG_SCHEMA = {
 async def to_code(config: ConfigType) -> None:
     hub = await cg.get_variable(config[CONF_LD2410_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_DISTANCE_RESOLUTION,
         hub.set_distance_resolution_select,

esphome/components/ld2410/select/baud_rate_select.cpp

@@ -4,7 +4,7 @@ namespace esphome::ld2410 {
 
 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::ld2410

esphome/components/ld2410/select/distance_resolution_select.cpp

@@ -4,7 +4,8 @@ namespace esphome::ld2410 {
 
 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::ld2410

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_ld2410_selects.yaml

@@ -0,0 +1,68 @@
+esphome:
+  name: uart-mock-ld2410-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 ld2410'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 0x00, the module's code for 0.75m
+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, 0xAB, 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
+                0xAB, 0x01,              // answer to command 0x00AB
+                0x00, 0x00,              // status: success
+                0x00, 0x00,              // resolution code 0x00 = 0.75m
+                0x04, 0x03, 0x02, 0x01,  // footer
+            });
+
+ld2410:
+  id: ld2410_dev
+  uart_id: mock_uart
+
+select:
+  - platform: ld2410
+    ld2410_id: ld2410_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(ld2410_dev).set_distance_resolution("bogus");
+          id(ld2410_dev).set_baud_rate("bogus");
+          ESP_LOGI("test", "bogus names done");
+  - platform: template
+    name: "Named Values"
+    on_press:
+      - lambda: |-
+          id(ld2410_dev).set_distance_resolution("0.75m");
+          id(ld2410_dev).set_baud_rate("57600");

tests/integration/test_uart_mock_ld2410_selects.py

@@ -0,0 +1,119 @@
+"""Integration test for the LD2410 selects with mock UART.
+
+The module answers the distance resolution query with 0x00, so the select starts at 0.75m, and the
+baud rate select starts at the bus speed. Choosing an option must send the device code of that
+option: 0.2m is 0x01 and 19200 is the second baud rate. The name taking setters map a name the
+same way from a lambda, and an unknown name sends nothing at all.
+"""
+
+from __future__ import annotations
+
+import asyncio
+
+from aioesphomeapi import ButtonInfo, SelectInfo
+import pytest
+
+from .state_utils import InitialStateHelper, require_entity
+from .types import APIClientConnectedFactory, RunCompiledFunction
+from .wait_utils import wait_or_fail
+
+# Length and command word of each frame as the uart mock logs it; the payload is logged on its own TX line
+SET_RESOLUTION = "04:00:AA:00"
+SET_BAUD_RATE = "04:00:A1:00"
+
+
+class _TxLines:
+    """Record the TX lines and the marker lines, and hand out the payload line after an awaited command."""
+
+    def __init__(self, *markers: str) -> None:
+        self._markers = markers
+        self.lines: list[str] = []
+        # Command text, whether its line was seen, and the future for the line after it
+        self._awaited: list[tuple[str, bool, asyncio.Future[str]]] = []
+
+    def on_line(self, line: str) -> None:
+        is_tx = "uart_mock" in line and "TX " in line
+        if not is_tx and not any(marker in line for marker in self._markers):
+            return
+        self.lines.append(line)
+        for i, (text, seen, future) in enumerate(self._awaited):
+            if seen:
+                if not future.done():
+                    future.set_result(line)
+            elif text in line:
+                self._awaited[i] = (text, True, future)
+
+    def next_payload_after(self, text: str) -> asyncio.Future[str]:
+        """Resolve with the TX line logged right after the next line holding ``text``."""
+        future: asyncio.Future[str] = asyncio.get_running_loop().create_future()
+        self._awaited.append((text, False, future))
+        return future
+
+
+@pytest.mark.asyncio
+async def test_uart_mock_ld2410_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."""
+    bogus_done = "bogus names done"
+    events = _TxLines(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 = InitialStateHelper(entities)
+        client.subscribe_states(initial_state_helper.on_state_wrapper(lambda s: None))
+        await initial_state_helper.wait_for_initial_states()
+        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.75m"
+        # 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)
+        bogus = events.next_payload_after(bogus_done)
+        client.button_command(require_entity(entities, "bogus_names", ButtonInfo).key)
+
+        # Each payload line follows its length and command line directly
+        resolution_payload = events.next_payload_after(SET_RESOLUTION)
+        client.select_command(resolution.key, "0.2m")
+        await wait_or_fail(
+            asyncio.gather(bogus, resolution_payload),
+            5.0,
+            "Timeout waiting for the distance resolution command",
+            logged=events.lines,
+        )
+        marker = next(
+            i for i in range(before, len(events.lines)) if bogus_done in events.lines[i]
+        )
+        assert all("TX " not in line for line in events.lines[before:marker])
+        assert "TX 2 bytes: 01:00" in resolution_payload.result()
+
+        baud_rate_payload = events.next_payload_after(SET_BAUD_RATE)
+        client.select_command(baud_rate.key, "19200")
+        await wait_or_fail(
+            baud_rate_payload,
+            5.0,
+            "Timeout waiting for the baud rate command",
+            logged=events.lines,
+        )
+        assert "TX 2 bytes: 02:00" in baud_rate_payload.result()
+
+        named_resolution = events.next_payload_after(SET_RESOLUTION)
+        named_baud_rate = events.next_payload_after(SET_BAUD_RATE)
+        client.button_command(require_entity(entities, "named_values", ButtonInfo).key)
+        await wait_or_fail(
+            asyncio.gather(named_resolution, named_baud_rate),
+            5.0,
+            "Timeout waiting for the named setter commands",
+            logged=events.lines,
+        )
+        # 0.75m is device code 0x00 and 57600 is the fourth baud rate
+        assert "TX 2 bytes: 00:00" in named_resolution.result()
+        assert "TX 2 bytes: 04:00" in named_baud_rate.result()