From 16568c0453440132a9af928e7ed1253a7e4eb3da Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 7 Oct 2026 10:24:14 -1000 Subject: [PATCH] [sx126x] Keep constant send_packet payloads in shared flash tables (#20273) --- esphome/components/sx126x/__init__.py | 29 +++++++------- esphome/components/sx126x/automation.h | 29 ++------------ esphome/components/sx126x/sx126x.cpp | 18 ++++----- esphome/components/sx126x/sx126x.h | 9 ++++- tests/component_tests/sx126x/__init__.py | 0 .../sx126x/config/packet_tables.yaml | 27 +++++++++++++ .../sx126x/test_packet_tables.py | 39 +++++++++++++++++++ 7 files changed, 101 insertions(+), 50 deletions(-) create mode 100644 tests/component_tests/sx126x/__init__.py create mode 100644 tests/component_tests/sx126x/config/packet_tables.yaml create mode 100644 tests/component_tests/sx126x/test_packet_tables.py diff --git a/esphome/components/sx126x/__init__.py b/esphome/components/sx126x/__init__.py index ce47a57050..63c3711b10 100644 --- a/esphome/components/sx126x/__init__.py +++ b/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 diff --git a/esphome/components/sx126x/automation.h b/esphome/components/sx126x/automation.h index 411de12341..0dbf3cb42c 100644 --- a/esphome/components/sx126x/automation.h +++ b/esphome/components/sx126x/automation.h @@ -7,35 +7,12 @@ namespace esphome::sx126x { template class SendPacketAction final : public Action, public Parented { - public: - void set_data_template(std::vector (*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 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( + [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 (*func)(Ts...); // Function pointer (stateless lambdas) - const uint8_t *data; // Pointer to static data in flash - } data_; }; } // namespace esphome::sx126x diff --git a/esphome/components/sx126x/sx126x.cpp b/esphome/components/sx126x/sx126x.cpp index 376676ce85..673464d5eb 100644 --- a/esphome/components/sx126x/sx126x.cpp +++ b/esphome/components/sx126x/sx126x.cpp @@ -42,13 +42,13 @@ uint8_t SX126x::read_fifo_(uint8_t offset, std::vector &packet) { return status; } -void SX126x::write_fifo_(uint8_t offset, const std::vector &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 &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 &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 diff --git a/esphome/components/sx126x/sx126x.h b/esphome/components/sx126x/sx126x.h index b3dfe6590a..536bb93c48 100644 --- a/esphome/components/sx126x/sx126x.h +++ b/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 &packet); + SX126xError transmit_packet(const uint8_t *data, size_t len); + SX126xError transmit_packet(const std::vector &packet) { + return this->transmit_packet(packet.data(), packet.size()); + } void register_listener(SX126xListener *listener) { this->listeners_.push_back(listener); } Trigger, 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 &packet); - void write_fifo_(uint8_t offset, const std::vector &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); diff --git a/tests/component_tests/sx126x/__init__.py b/tests/component_tests/sx126x/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/component_tests/sx126x/config/packet_tables.yaml b/tests/component_tests/sx126x/config/packet_tables.yaml new file mode 100644 index 0000000000..d8b2f04281 --- /dev/null +++ b/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 diff --git a/tests/component_tests/sx126x/test_packet_tables.py b/tests/component_tests/sx126x/test_packet_tables.py new file mode 100644 index 0000000000..9ee3379f08 --- /dev/null +++ b/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([])