[sx126x] Keep constant send_packet payloads in shared flash tables (#20273)

esphome/components/sx126x/__init__.py

@@ -146,6 +146,11 @@ def validate_raw_data(value: Any) -> bytes | list[int]:
     )
 
 
+MAX_PACKET_SIZE = 255
+# The radio sends packets of 1 to MAX_PACKET_SIZE bytes.
+validate_packet_data = cv.All(validate_raw_data, cv.Length(min=1, max=MAX_PACKET_SIZE))
+
+
 def validate_config(config: ConfigType) -> ConfigType:
     lora_bws = [
         "7_8kHz",
@@ -205,7 +210,9 @@ CONFIG_SCHEMA = (
             cv.Optional(CONF_ON_PACKET): automation.validate_automation(single=True),
             cv.Optional(CONF_PA_POWER, default=17): cv.int_range(min=-3, max=22),
             cv.Optional(CONF_PA_RAMP, default="40us"): cv.enum(RAMP),
-            cv.Optional(CONF_PAYLOAD_LENGTH, default=0): cv.int_range(min=0, max=255),
+            cv.Optional(CONF_PAYLOAD_LENGTH, default=0): cv.int_range(
+                min=0, max=MAX_PACKET_SIZE
+            ),
             cv.Optional(CONF_PREAMBLE_DETECT, default=2): cv.int_range(min=0, max=4),
             cv.Optional(CONF_PREAMBLE_SIZE, default=8): cv.int_range(min=1, max=65535),
             cv.Required(CONF_RST_PIN): pins.gpio_output_pin_schema,
@@ -314,7 +321,7 @@ automation.register_apply_action(
 SEND_PACKET_ACTION_SCHEMA = cv.maybe_simple_value(
     {
         cv.GenerateID(): cv.use_id(SX126x),
-        cv.Required(CONF_DATA): cv.templatable(validate_raw_data),
+        cv.Required(CONF_DATA): cv.templatable(validate_packet_data),
     },
     key=CONF_DATA,
 )
@@ -334,15 +341,11 @@ async def send_packet_action_to_code(
 ) -> MockObj:
     var = cg.new_Pvariable(action_id, template_arg)
     await cg.register_parented(var, config[CONF_ID])
-    data = config[CONF_DATA]
-    if isinstance(data, bytes):
-        data = list(data)
-    if cg.is_template(data):
-        templ = await cg.templatable(data, args, cg.std_vector.template(cg.uint8))
-        cg.add(var.set_data_template(templ))
-    else:
-        # Generate static array in flash to avoid RAM copy
-        arr_id = ID(f"{action_id}_data", is_declaration=True, type=cg.uint8)
-        arr = cg.static_const_array(arr_id, cg.ArrayInitializer(*data))
-        cg.add(var.set_data_static(arr, len(data)))
+    await automation.templatable_bytes(
+        config[CONF_DATA],
+        args,
+        var.set_data_template,
+        var.set_data_static,
+        "sx126x_data",
+    )
     return var

esphome/components/sx126x/automation.h

@@ -7,35 +7,12 @@
 namespace esphome::sx126x {
 
 template<typename... Ts> class SendPacketAction final : public Action<Ts...>, public Parented<SX126x> {
- public:
-  void set_data_template(std::vector<uint8_t> (*func)(Ts...)) {
-    this->data_.func = func;
-    this->len_ = -1;  // Sentinel value indicates template mode
-  }
-
-  void set_data_static(const uint8_t *data, size_t len) {
-    this->data_.data = data;
-    this->len_ = len;  // Length >= 0 indicates static mode
-  }
+  TEMPLATABLE_BYTES(data)
 
   void play(const Ts &...x) override {
-    std::vector<uint8_t> data;
-    if (this->len_ >= 0) {
-      // Static mode: copy from flash to vector
-      data.assign(this->data_.data, this->data_.data + this->len_);
-    } else {
-      // Template mode: call function
-      data = this->data_.func(x...);
-    }
-    this->parent_->transmit_packet(data);
+    this->data_.template visit<SX126X_MAX_PACKET_SIZE>(
+        [this](const uint8_t *data, size_t len) { this->parent_->transmit_packet(data, len); }, x...);
   }
-
- protected:
-  ssize_t len_{-1};  // -1 = template mode, >=0 = static mode with length
-  union Data {
-    std::vector<uint8_t> (*func)(Ts...);  // Function pointer (stateless lambdas)
-    const uint8_t *data;                  // Pointer to static data in flash
-  } data_;
 };
 
 }  // namespace esphome::sx126x

esphome/components/sx126x/sx126x.cpp

@@ -42,13 +42,13 @@ uint8_t SX126x::read_fifo_(uint8_t offset, std::vector<uint8_t> &packet) {
   return status;
 }
 
-void SX126x::write_fifo_(uint8_t offset, const std::vector<uint8_t> &packet) {
+void SX126x::write_fifo_(uint8_t offset, const uint8_t *data, size_t len) {
   this->enable();
   this->wait_busy_();
   this->transfer_byte(RADIO_WRITE_BUFFER);
   this->transfer_byte(offset);
-  for (const uint8_t &byte : packet) {
-    this->transfer_byte(byte);
+  for (size_t i = 0; i < len; i++) {
+    this->transfer_byte(data[i]);
   }
   this->disable();
   delayMicroseconds(SWITCHING_DELAY_US);
@@ -280,7 +280,7 @@ size_t SX126x::get_max_packet_size() {
   if (this->payload_length_ > 0) {
     return this->payload_length_;
   }
-  return 255;
+  return SX126X_MAX_PACKET_SIZE;
 }
 
 void SX126x::set_packet_params_(uint8_t payload_length) {
@@ -312,12 +312,12 @@ void SX126x::set_packet_params_(uint8_t payload_length) {
   }
 }
 
-SX126xError SX126x::transmit_packet(const std::vector<uint8_t> &packet) {
-  if (this->payload_length_ > 0 && this->payload_length_ != packet.size()) {
+SX126xError SX126x::transmit_packet(const uint8_t *data, size_t len) {
+  if (this->payload_length_ > 0 && this->payload_length_ != len) {
     ESP_LOGE(TAG, "Packet size does not match config");
     return SX126xError::INVALID_PARAMS;
   }
-  if (packet.empty() || packet.size() > this->get_max_packet_size()) {
+  if (len == 0 || len > this->get_max_packet_size()) {
     ESP_LOGE(TAG, "Packet size out of range");
     return SX126xError::INVALID_PARAMS;
   }
@@ -325,9 +325,9 @@ SX126xError SX126x::transmit_packet(const std::vector<uint8_t> &packet) {
   SX126xError ret = SX126xError::NONE;
   this->set_mode_standby(STDBY_XOSC);
   if (this->payload_length_ == 0) {
-    this->set_packet_params_(packet.size());
+    this->set_packet_params_(len);
   }
-  this->write_fifo_(0x00, packet);
+  this->write_fifo_(0x00, data, len);
   this->set_mode_tx();
 
   // wait until transmit completes, typically the delay will be less than 100 ms

esphome/components/sx126x/sx126x.h

@@ -10,6 +10,8 @@
 
 namespace esphome::sx126x {
 
+static constexpr size_t SX126X_MAX_PACKET_SIZE = 255;
+
 enum SX126xBw : uint8_t {
   // FSK
   SX126X_BW_4800,
@@ -97,7 +99,10 @@ class SX126x final : public Component,
   void set_tcxo_delay(uint32_t tcxo_delay) { this->tcxo_delay_ = tcxo_delay; }
   void run_image_cal();
   void configure();
-  SX126xError transmit_packet(const std::vector<uint8_t> &packet);
+  SX126xError transmit_packet(const uint8_t *data, size_t len);
+  SX126xError transmit_packet(const std::vector<uint8_t> &packet) {
+    return this->transmit_packet(packet.data(), packet.size());
+  }
   void register_listener(SX126xListener *listener) { this->listeners_.push_back(listener); }
   Trigger<std::vector<uint8_t>, float, float> *get_packet_trigger() { return &this->packet_trigger_; }
 
@@ -107,7 +112,7 @@ class SX126x final : public Component,
   void configure_lora_();
   void set_packet_params_(uint8_t payload_length);
   uint8_t read_fifo_(uint8_t offset, std::vector<uint8_t> &packet);
-  void write_fifo_(uint8_t offset, const std::vector<uint8_t> &packet);
+  void write_fifo_(uint8_t offset, const uint8_t *data, size_t len);
   void write_opcode_(uint8_t opcode, uint8_t *data, uint8_t size);
   uint8_t read_opcode_(uint8_t opcode, uint8_t *data, uint8_t size);
   void write_register_(uint16_t reg, uint8_t *data, uint8_t size);

tests/component_tests/sx126x/config/packet_tables.yaml

@@ -0,0 +1,27 @@
+esphome:
+  name: test
+  on_boot:
+    then:
+      - sx126x.send_packet: [0xC5, 0x51, 0x78, 0x82]
+      - sx126x.send_packet: [0xC5, 0x51, 0x78, 0x82]
+      - sx126x.send_packet: "hi"
+      - sx126x.send_packet: !lambda return {0x09};
+
+esp32:
+  board: esp32dev
+
+spi:
+  clk_pin: 18
+  mosi_pin: 23
+  miso_pin: 19
+
+sx126x:
+  cs_pin: 12
+  rst_pin: 13
+  busy_pin: 25
+  dio1_pin: 26
+  frequency: 433920000
+  hw_version: sx1262
+  modulation: LORA
+  bandwidth: 125_0kHz
+  rf_switch: true

tests/component_tests/sx126x/test_packet_tables.py

@@ -0,0 +1,39 @@
+"""Tests for SX126x constant packets in shared PROGMEM tables."""
+
+from collections.abc import Callable
+from pathlib import Path
+import re
+
+import pytest
+
+from esphome.components.sx126x import validate_packet_data
+import esphome.config_validation as cv
+
+
+def test_constant_packets_share_progmem_tables(
+    generate_main: Callable[[str | Path], str],
+    component_config_path: Callable[[str], Path],
+) -> None:
+    """Equal constant packets share one table; lambdas stay templates."""
+    main_cpp = generate_main(component_config_path("packet_tables.yaml"))
+
+    tables = dict(
+        re.findall(
+            r"static constexpr uint8_t (\w+)\[\] PROGMEM = (\{[^}]*\});", main_cpp
+        )
+    )
+    assert sorted(tables.values()) == sorted(
+        ["{0xC5, 0x51, 0x78, 0x82}", "{0x68, 0x69}"]
+    )
+    shared = next(k for k, v in tables.items() if v == "{0xC5, 0x51, 0x78, 0x82}")
+    assert main_cpp.count(f"set_data_static({shared}, 4);") == 2
+    assert "set_data_template(" in main_cpp
+
+
+def test_packet_data_length_limit() -> None:
+    """Constant packets must be 1 to 255 bytes long."""
+    assert len(validate_packet_data([0x01] * 255)) == 255
+    with pytest.raises(cv.Invalid):
+        validate_packet_data([0x01] * 256)
+    with pytest.raises(cv.Invalid):
+        validate_packet_data([])