diff --git a/esphome/components/uart/__init__.py b/esphome/components/uart/__init__.py index 5eea8060ff9..d1ac806b5fd 100644 --- a/esphome/components/uart/__init__.py +++ b/esphome/components/uart/__init__.py @@ -137,6 +137,10 @@ def validate_raw_data(value): ) +# Lengths are stored as uint16_t; reject longer payloads at validation. +validate_raw_payload = cv.All(validate_raw_data, cv.Length(max=65535)) + + def validate_rx_pin(value): value = pins.internal_gpio_input_pin_schema(value) if CORE.is_esp8266 and value[CONF_NUMBER] >= 16: @@ -596,7 +600,7 @@ async def register_uart_device(var, config): cv.maybe_simple_value( { cv.GenerateID(): cv.use_id(UARTComponent), - cv.Required(CONF_DATA): cv.templatable(validate_raw_data), + cv.Required(CONF_DATA): cv.templatable(validate_raw_payload), }, key=CONF_DATA, ), @@ -605,18 +609,9 @@ async def register_uart_device(var, config): async def uart_write_to_code(config, action_id, template_arg, args): 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, "uart_data" + ) return var diff --git a/esphome/components/uart/automation.h b/esphome/components/uart/automation.h index e5a9fa7c7bc..b134c8af9f6 100644 --- a/esphome/components/uart/automation.h +++ b/esphome/components/uart/automation.h @@ -8,36 +8,15 @@ namespace esphome::uart { template class UARTWriteAction final : public Action, public Parented { - public: - void set_data_template(std::vector (*func)(Ts...)) { - // Stateless lambdas (generated by ESPHome) implicitly convert to function pointers - this->code_.func = func; - this->len_ = -1; // Sentinel value indicates template mode - } - - // Store pointer to static data in flash (no RAM copy) - void set_data_static(const uint8_t *data, size_t len) { - this->code_.data = data; - this->len_ = len; // Length >= 0 indicates static mode - } + TEMPLATABLE_BYTES(data) void play(const Ts &...x) override { - if (this->len_ >= 0) { - // Static mode: use pointer and length - this->parent_->write_array(this->code_.data, static_cast(this->len_)); + if (this->data_.is_static()) { + this->parent_->write_array_progmem(this->data_.data(), this->data_.size()); } else { - // Template mode: call function - auto val = this->code_.func(x...); - this->parent_->write_array(val); + this->parent_->write_array(this->data_.value(x...)); } } - - protected: - ssize_t len_{-1}; // -1 = template mode, >=0 = static mode with length - union Code { - std::vector (*func)(Ts...); // Function pointer (stateless lambdas) - const uint8_t *data; // Pointer to static data in flash - } code_; }; } // namespace esphome::uart diff --git a/esphome/components/uart/button/__init__.py b/esphome/components/uart/button/__init__.py index 95fe21271db..821d8264dc4 100644 --- a/esphome/components/uart/button/__init__.py +++ b/esphome/components/uart/button/__init__.py @@ -1,10 +1,10 @@ +from esphome import automation import esphome.codegen as cg from esphome.components import button, uart import esphome.config_validation as cv from esphome.const import CONF_DATA -from esphome.core import HexInt -from .. import uart_ns, validate_raw_data +from .. import uart_ns, validate_raw_payload CODEOWNERS = ["@ssieb"] @@ -17,7 +17,7 @@ CONFIG_SCHEMA = ( button.button_schema(UARTButton) .extend( { - cv.Required(CONF_DATA): validate_raw_data, + cv.Required(CONF_DATA): validate_raw_payload, } ) .extend(uart.UART_DEVICE_SCHEMA) @@ -31,6 +31,4 @@ async def to_code(config): await uart.register_uart_device(var, config) data = config[CONF_DATA] - if isinstance(data, bytes): - data = [HexInt(x) for x in data] - cg.add(var.set_data(cg.ArrayInitializer(*data))) + cg.add(var.set_data(automation.progmem_bytes("uart_data", data), len(data))) diff --git a/esphome/components/uart/button/uart_button.cpp b/esphome/components/uart/button/uart_button.cpp index b55207cb46e..99b5ee548d0 100644 --- a/esphome/components/uart/button/uart_button.cpp +++ b/esphome/components/uart/button/uart_button.cpp @@ -7,7 +7,7 @@ ESPHOME_LOG_TAG(TAG, "uart.button"); void UARTButton::press_action() { ESP_LOGD(TAG, "'%s': Sending data", this->get_name().c_str()); - this->write_array(this->data_.data(), this->data_.size()); + this->write_array_progmem(this->data_, this->data_len_); } void UARTButton::dump_config() { LOG_BUTTON("", "UART Button", this); } diff --git a/esphome/components/uart/button/uart_button.h b/esphome/components/uart/button/uart_button.h index 47f45d4899c..903f945fd30 100644 --- a/esphome/components/uart/button/uart_button.h +++ b/esphome/components/uart/button/uart_button.h @@ -4,20 +4,21 @@ #include "esphome/components/uart/uart.h" #include "esphome/components/button/button.h" -#include - namespace esphome::uart { class UARTButton final : public button::Button, public UARTDevice, public Component { public: - void set_data(std::vector &&data) { this->data_ = std::move(data); } - void set_data(std::initializer_list data) { this->data_ = std::vector(data); } + void set_data(const uint8_t *data, uint16_t len) { + this->data_ = data; + this->data_len_ = len; + } void dump_config() override; protected: void press_action() override; - std::vector data_; + const uint8_t *data_{nullptr}; + uint16_t data_len_{0}; }; } // namespace esphome::uart diff --git a/esphome/components/uart/switch/__init__.py b/esphome/components/uart/switch/__init__.py index 290bbed5d30..3288ceeee20 100644 --- a/esphome/components/uart/switch/__init__.py +++ b/esphome/components/uart/switch/__init__.py @@ -1,10 +1,10 @@ +from esphome import automation import esphome.codegen as cg from esphome.components import switch, uart import esphome.config_validation as cv from esphome.const import CONF_DATA, CONF_SEND_EVERY -from esphome.core import HexInt -from .. import uart_ns, validate_raw_data +from .. import uart_ns, validate_raw_payload DEPENDENCIES = ["uart"] @@ -18,11 +18,11 @@ CONFIG_SCHEMA = ( .extend( { cv.Required(CONF_DATA): cv.Any( - validate_raw_data, + validate_raw_payload, cv.Schema( { - cv.Optional(CONF_TURN_OFF): validate_raw_data, - cv.Optional(CONF_TURN_ON): validate_raw_data, + cv.Optional(CONF_TURN_OFF): validate_raw_payload, + cv.Optional(CONF_TURN_ON): validate_raw_payload, } ), ), @@ -40,20 +40,16 @@ async def to_code(config): await uart.register_uart_device(var, config) data = config[CONF_DATA] - if isinstance(data, dict): - if data_on := data.get(CONF_TURN_ON): - if isinstance(data_on, bytes): - data_on = [HexInt(x) for x in data_on] - cg.add(var.set_data_on(cg.ArrayInitializer(*data_on))) - if data_off := data.get(CONF_TURN_OFF): - if isinstance(data_off, bytes): - data_off = [HexInt(x) for x in data_off] - cg.add(var.set_data_off(cg.ArrayInitializer(*data_off))) - else: - data = config[CONF_DATA] - if isinstance(data, bytes): - data = [HexInt(x) for x in data] - cg.add(var.set_data_on(cg.ArrayInitializer(*data))) + single_state = not isinstance(data, dict) + if single_state: + data = {CONF_TURN_ON: data} + for key, setter in ( + (CONF_TURN_ON, var.set_data_on), + (CONF_TURN_OFF, var.set_data_off), + ): + if payload := data.get(key): + cg.add(setter(automation.progmem_bytes("uart_data", payload), len(payload))) + if single_state: cg.add(var.set_single_state(True)) if CONF_SEND_EVERY in config: cg.add(var.set_send_every(config[CONF_SEND_EVERY])) diff --git a/esphome/components/uart/switch/uart_switch.cpp b/esphome/components/uart/switch/uart_switch.cpp index 6b6aa273f7a..1af497ffcfd 100644 --- a/esphome/components/uart/switch/uart_switch.cpp +++ b/esphome/components/uart/switch/uart_switch.cpp @@ -17,13 +17,13 @@ void UARTSwitch::loop() { } void UARTSwitch::write_command_(bool state) { - if (state && !this->data_on_.empty()) { + if (state && this->data_on_len_ != 0) { ESP_LOGD(TAG, "'%s': Sending on data", this->get_name().c_str()); - this->write_array(this->data_on_.data(), this->data_on_.size()); + this->write_array_progmem(this->data_on_, this->data_on_len_); } - if (!state && !this->data_off_.empty()) { + if (!state && this->data_off_len_ != 0) { ESP_LOGD(TAG, "'%s': Sending off data", this->get_name().c_str()); - this->write_array(this->data_off_.data(), this->data_off_.size()); + this->write_array_progmem(this->data_off_, this->data_off_len_); } } diff --git a/esphome/components/uart/switch/uart_switch.h b/esphome/components/uart/switch/uart_switch.h index c924c7d4e54..96d8833d7a8 100644 --- a/esphome/components/uart/switch/uart_switch.h +++ b/esphome/components/uart/switch/uart_switch.h @@ -5,7 +5,6 @@ #include "esphome/components/switch/switch.h" #include -#include namespace esphome::uart { @@ -13,10 +12,14 @@ class UARTSwitch final : public switch_::Switch, public UARTDevice, public Compo public: void loop() override; - void set_data_on(std::vector &&data) { this->data_on_ = std::move(data); } - void set_data_on(std::initializer_list data) { this->data_on_ = std::vector(data); } - void set_data_off(std::vector &&data) { this->data_off_ = std::move(data); } - void set_data_off(std::initializer_list data) { this->data_off_ = std::vector(data); } + void set_data_on(const uint8_t *data, uint16_t len) { + this->data_on_ = data; + this->data_on_len_ = len; + } + void set_data_off(const uint8_t *data, uint16_t len) { + this->data_off_ = data; + this->data_off_len_ = len; + } void set_send_every(uint32_t send_every) { this->send_every_ = send_every; } void set_single_state(bool single) { this->single_state_ = single; } @@ -25,8 +28,10 @@ class UARTSwitch final : public switch_::Switch, public UARTDevice, public Compo protected: void write_command_(bool state); void write_state(bool state) override; - std::vector data_on_; - std::vector data_off_; + const uint8_t *data_on_{nullptr}; + const uint8_t *data_off_{nullptr}; + uint16_t data_on_len_{0}; + uint16_t data_off_len_{0}; bool single_state_{false}; uint32_t send_every_; uint32_t last_transmission_; diff --git a/esphome/components/uart/uart.h b/esphome/components/uart/uart.h index d1c67733634..7a34a20583e 100644 --- a/esphome/components/uart/uart.h +++ b/esphome/components/uart/uart.h @@ -25,6 +25,7 @@ class UARTDevice { size_t get_rx_timeout() { return this->parent_->get_rx_timeout(); } void write_array(const uint8_t *data, size_t len) { this->parent_->write_array(data, len); } + void write_array_progmem(const uint8_t *data, size_t len) { this->parent_->write_array_progmem(data, len); } void write_array(const std::vector &data) { this->parent_->write_array(data); } template void write_array(const std::array &data) { this->parent_->write_array(data.data(), data.size()); diff --git a/esphome/components/uart/uart_component.cpp b/esphome/components/uart/uart_component.cpp index 82bc5927024..e77e37cd089 100644 --- a/esphome/components/uart/uart_component.cpp +++ b/esphome/components/uart/uart_component.cpp @@ -46,4 +46,17 @@ void UARTComponent::set_rx_full_threshold_ms(uint8_t time) { this->set_rx_full_threshold(val); } +#ifdef USE_ESP8266 +void UARTComponent::write_array_progmem(const uint8_t *data, size_t len) { + uint8_t buf[32]; + while (len > 0) { + const size_t chunk = std::min(len, sizeof(buf)); + progmem_memcpy(buf, data, chunk); + this->write_array(buf, chunk); + data += chunk; + len -= chunk; + } +} +#endif + } // namespace esphome::uart diff --git a/esphome/components/uart/uart_component.h b/esphome/components/uart/uart_component.h index 6d446fd6a6f..14709d44906 100644 --- a/esphome/components/uart/uart_component.h +++ b/esphome/components/uart/uart_component.h @@ -65,6 +65,13 @@ class UARTComponent { // @param len Length of the array. virtual void write_array(const uint8_t *data, size_t len) = 0; + // Writes bytes that may live in PROGMEM; ESP8266 cannot read flash bytewise, so it copies chunks. +#ifdef USE_ESP8266 + void write_array_progmem(const uint8_t *data, size_t len); +#else + void write_array_progmem(const uint8_t *data, size_t len) { this->write_array(data, len); } +#endif + // Reads a single byte from the UART bus. // @param data Pointer to the byte where the read data will be stored. // @return True if a byte was successfully read, false otherwise. diff --git a/tests/component_tests/uart/__init__.py b/tests/component_tests/uart/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/component_tests/uart/config/payload_tables.yaml b/tests/component_tests/uart/config/payload_tables.yaml new file mode 100644 index 00000000000..e93ab4754d0 --- /dev/null +++ b/tests/component_tests/uart/config/payload_tables.yaml @@ -0,0 +1,34 @@ +--- +esphome: + name: test + on_boot: + then: + - uart.write: [0x01, 0x02, 0x03] + - uart.write: !lambda return {0x09}; + - uart.write: "" + +esp32: + board: esp32dev + +uart: + tx_pin: 17 + rx_pin: 16 + baud_rate: 9600 + +switch: + - platform: uart + id: shared_switch + name: Shared Switch + data: [0x01, 0x02, 0x03] + - platform: uart + id: on_off_switch + name: On Off Switch + data: + turn_on: "ON" + turn_off: [0x04] + +button: + - platform: uart + id: shared_button + name: Shared Button + data: [0x01, 0x02, 0x03] diff --git a/tests/component_tests/uart/test_payload_tables.py b/tests/component_tests/uart/test_payload_tables.py new file mode 100644 index 00000000000..2ba341136a7 --- /dev/null +++ b/tests/component_tests/uart/test_payload_tables.py @@ -0,0 +1,28 @@ +"""Tests for UART constant payloads in shared PROGMEM tables.""" + +from collections.abc import Callable +from pathlib import Path +import re + + +def test_constant_payloads_share_progmem_tables( + generate_main: Callable[[str | Path], str], + component_config_path: Callable[[str], Path], +) -> None: + """Equal payloads share one table across switch, button and write action.""" + main_cpp = generate_main(component_config_path("payload_tables.yaml")) + + tables = dict( + re.findall( + r"static constexpr uint8_t (\w+)\[\] PROGMEM = (\{[^}]*\});", main_cpp + ) + ) + shared = next(k for k, v in tables.items() if v == "{0x01, 0x02, 0x03}") + assert sorted(tables.values()) == sorted( + ["{0x01, 0x02, 0x03}", "{0x4F, 0x4E}", "{0x04}"] + ) + assert f"shared_switch->set_data_on({shared}, 3);" in main_cpp + assert f"shared_button->set_data({shared}, 3);" in main_cpp + assert f"set_data_static({shared}, 3);" in main_cpp + assert "set_data_static(nullptr, 0);" in main_cpp + assert "set_data_template(" in main_cpp