diff --git a/esphome/components/sx127x/__init__.py b/esphome/components/sx127x/__init__.py index 9f95c61844..68c56cdff4 100644 --- a/esphome/components/sx127x/__init__.py +++ b/esphome/components/sx127x/__init__.py @@ -135,6 +135,11 @@ def validate_raw_data(value: Any) -> bytes | list[int]: ) +MAX_PACKET_SIZE = 255 +# The radio's payload length register is 8 bits, and empty packets are rejected. +validate_packet_data = cv.All(validate_raw_data, cv.Length(min=1, max=MAX_PACKET_SIZE)) + + def validate_config(config: ConfigType) -> ConfigType: if config[CONF_MODULATION] == "LORA": bws = [ @@ -198,7 +203,9 @@ CONFIG_SCHEMA = ( cv.Optional(CONF_PA_POWER, default=17): cv.int_range(min=0, max=17), cv.Optional(CONF_PA_RAMP, default="40us"): cv.enum(RAMP), cv.Optional(CONF_PACKET_MODE): cv.boolean, - cv.Optional(CONF_PAYLOAD_LENGTH, default=0): cv.int_range(min=0, max=256), + cv.Optional(CONF_PAYLOAD_LENGTH, default=0): cv.int_range( + min=0, max=MAX_PACKET_SIZE + ), cv.Optional(CONF_PREAMBLE_DETECT, default=0): cv.int_range(min=0, max=3), cv.Optional(CONF_PREAMBLE_ERRORS, default=0): cv.int_range(min=0, max=31), cv.Optional(CONF_PREAMBLE_POLARITY, default=0xAA): cv.All( @@ -286,7 +293,7 @@ for _name, _call in ( SEND_PACKET_ACTION_SCHEMA = cv.maybe_simple_value( { cv.GenerateID(): cv.use_id(SX127x), - cv.Required(CONF_DATA): cv.templatable(validate_raw_data), + cv.Required(CONF_DATA): cv.templatable(validate_packet_data), }, key=CONF_DATA, ) @@ -306,15 +313,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, + "sx127x_data", + ) return var diff --git a/esphome/components/sx127x/automation.h b/esphome/components/sx127x/automation.h index 2ed1614681..8d409f0b01 100644 --- a/esphome/components/sx127x/automation.h +++ b/esphome/components/sx127x/automation.h @@ -7,35 +7,13 @@ namespace esphome::sx127x { template class SendPacketAction final : public Action, public Parented { + TEMPLATABLE_BYTES(data) + 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 - } - 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::sx127x diff --git a/esphome/components/sx127x/sx127x.cpp b/esphome/components/sx127x/sx127x.cpp index cd81f08914..9af30400a7 100644 --- a/esphome/components/sx127x/sx127x.cpp +++ b/esphome/components/sx127x/sx127x.cpp @@ -43,11 +43,11 @@ void SX127x::read_fifo_(std::vector &packet) { this->disable(); } -void SX127x::write_fifo_(const std::vector &packet) { +void SX127x::write_fifo_(const uint8_t *data, size_t len) { this->enable(); this->write_byte(REG_FIFO | 0x80); - for (const auto &byte : packet) { - this->transfer_byte(byte); + for (size_t i = 0; i < len; i++) { + this->transfer_byte(data[i]); } this->disable(); } @@ -250,18 +250,17 @@ size_t SX127x::get_max_packet_size() { return this->payload_length_; } if (this->modulation_ == MOD_LORA) { - return 256; - } else { - return 64; + return SX127X_MAX_PACKET_SIZE; } + return 64; } -SX127xError SX127x::transmit_packet(const std::vector &packet) { - if (this->payload_length_ > 0 && this->payload_length_ != packet.size()) { +SX127xError SX127x::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 SX127xError::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 SX127xError::INVALID_PARAMS; } @@ -275,18 +274,18 @@ SX127xError SX127x::transmit_packet(const std::vector &packet) { if (this->modulation_ == MOD_LORA) { this->set_mode_standby(); if (this->payload_length_ == 0) { - this->write_register_(REG_PAYLOAD_LENGTH, packet.size()); + this->write_register_(REG_PAYLOAD_LENGTH, len); } this->write_register_(REG_IRQ_FLAGS, 0xFF); this->write_register_(REG_FIFO_ADDR_PTR, 0); - this->write_fifo_(packet); + this->write_fifo_(data, len); this->set_mode_tx(); } else { this->set_mode_standby(); if (this->payload_length_ == 0) { - this->write_register_(REG_FIFO, packet.size()); + this->write_register_(REG_FIFO, len); } - this->write_fifo_(packet); + this->write_fifo_(data, len); this->set_mode_tx(); } diff --git a/esphome/components/sx127x/sx127x.h b/esphome/components/sx127x/sx127x.h index 070a6eeb96..27031d98b2 100644 --- a/esphome/components/sx127x/sx127x.h +++ b/esphome/components/sx127x/sx127x.h @@ -9,6 +9,8 @@ namespace esphome::sx127x { +static constexpr size_t SX127X_MAX_PACKET_SIZE = 255; // the payload length register is 8 bits + enum SX127xBw : uint8_t { SX127X_BW_2_6, SX127X_BW_3_1, @@ -81,7 +83,10 @@ class SX127x final : public Component, void set_sync_value(const std::vector &sync_value) { this->sync_value_ = sync_value; } void run_image_cal(); void configure(); - SX127xError transmit_packet(const std::vector &packet); + SX127xError transmit_packet(const uint8_t *data, size_t len); + SX127xError transmit_packet(const std::vector &packet) { + return this->transmit_packet(packet.data(), packet.size()); + } void register_listener(SX127xListener *listener) { this->listeners_.push_back(listener); } Trigger, float, float> *get_packet_trigger() { return &this->packet_trigger_; } @@ -90,7 +95,7 @@ class SX127x final : public Component, void configure_fsk_ook_(); void configure_lora_(); void set_mode_(uint8_t modulation, uint8_t mode); - void write_fifo_(const std::vector &packet); + void write_fifo_(const uint8_t *data, size_t len); void read_fifo_(std::vector &packet); void write_register_(uint8_t reg, uint8_t value); void call_listeners_(const std::vector &packet, float rssi, float snr); diff --git a/tests/component_tests/sx127x/__init__.py b/tests/component_tests/sx127x/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/component_tests/sx127x/config/packet_tables.yaml b/tests/component_tests/sx127x/config/packet_tables.yaml new file mode 100644 index 0000000000..a1cf7b3f00 --- /dev/null +++ b/tests/component_tests/sx127x/config/packet_tables.yaml @@ -0,0 +1,24 @@ +esphome: + name: test + on_boot: + then: + - sx127x.send_packet: [0xC5, 0x51, 0x78, 0x82] + - sx127x.send_packet: [0xC5, 0x51, 0x78, 0x82] + - sx127x.send_packet: "hi" + - sx127x.send_packet: !lambda return {0x09}; + +esp32: + board: esp32dev + +spi: + clk_pin: 18 + mosi_pin: 23 + miso_pin: 19 + +sx127x: + cs_pin: 12 + rst_pin: 13 + dio0_pin: 26 + frequency: 433920000 + modulation: LORA + preamble_size: 8 diff --git a/tests/component_tests/sx127x/test_packet_tables.py b/tests/component_tests/sx127x/test_packet_tables.py new file mode 100644 index 0000000000..f7573bb1ca --- /dev/null +++ b/tests/component_tests/sx127x/test_packet_tables.py @@ -0,0 +1,51 @@ +"""Tests for SX127x constant packets in shared PROGMEM tables.""" + +from collections.abc import Callable +from pathlib import Path +import re + +import pytest + +from esphome.components.sx127x import CONFIG_SCHEMA, 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 are limited to the 255 bytes the length register holds.""" + 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("x" * 256) + with pytest.raises(cv.Invalid): + validate_packet_data([]) + + +def test_payload_length_limit() -> None: + """payload_length goes into an 8 bit register, so 256 is rejected.""" + validator = next( + v for k, v in CONFIG_SCHEMA.schema.items() if str(k) == "payload_length" + ) + assert validator(255) == 255 + with pytest.raises(cv.Invalid): + validator(256)