diff --git a/esphome/components/modbus_client/__init__.py b/esphome/components/modbus_client/__init__.py index 66ddcd7722d..a4f6ef585ac 100644 --- a/esphome/components/modbus_client/__init__.py +++ b/esphome/components/modbus_client/__init__.py @@ -13,7 +13,7 @@ from esphome.const import ( CONF_ON_RESPONSE, CONF_VALUE, ) -from esphome.core import ID, Lambda +from esphome.core import Lambda from esphome.types import ConfigType, TemplateArgsType CODEOWNERS = ["@exciton"] @@ -487,9 +487,10 @@ async def write_multiple_registers_to_code(config, action_id, template_arg, args templ = await cg.templatable(values, args, cg.std_vector.template(cg.uint16)) cg.add(var.set_values_template(templ)) else: - # A static list goes to flash, so play() sends straight from there without allocating. - arr_id = ID(f"{action_id}_values", is_declaration=True, type=cg.uint16) - arr = cg.static_const_array(arr_id, cg.ArrayInitializer(*values)) + # Shared flash table; play() sends without allocating (ESP8266 copies it to the stack first). + arr = cg.shared_progmem_array( + "modbus_client_registers", cg.uint16, cg.ArrayInitializer(*values) + ) cg.add(var.set_values_static(arr, len(values))) return await register_client_action( var, config, args, [], command_direction="write" @@ -514,8 +515,9 @@ async def write_multiple_coils_to_code(config, action_id, template_arg, args): for i, coil in enumerate(values): if coil: packed[i // 8] |= 1 << (i % 8) - arr_id = ID(f"{action_id}_values", is_declaration=True, type=cg.uint8) - arr = cg.static_const_array(arr_id, cg.ArrayInitializer(*packed)) + arr = cg.shared_progmem_array( + "modbus_client_coils", cg.uint8, cg.ArrayInitializer(*packed) + ) cg.add(var.set_values_static(arr, len(values))) return await register_client_action( var, config, args, [], command_direction="write" @@ -581,8 +583,9 @@ async def read_write_multiple_registers_to_code(config, action_id, template_arg, templ = await cg.templatable(values, args, cg.std_vector.template(cg.uint16)) cg.add(var.set_values_template(templ)) else: - # A static list goes to flash, so play() sends straight from there without allocating. - arr_id = ID(f"{action_id}_values", is_declaration=True, type=cg.uint16) - arr = cg.static_const_array(arr_id, cg.ArrayInitializer(*values)) + # Shared flash table; play() sends without allocating (ESP8266 copies it to the stack first). + arr = cg.shared_progmem_array( + "modbus_client_registers", cg.uint16, cg.ArrayInitializer(*values) + ) cg.add(var.set_values_static(arr, len(values))) return await register_client_action(var, config, args, [(_REGISTER_SPAN, "values")]) diff --git a/esphome/components/modbus_client/modbus_client.h b/esphome/components/modbus_client/modbus_client.h index 4c1d11da838..ea8abb92c47 100644 --- a/esphome/components/modbus_client/modbus_client.h +++ b/esphome/components/modbus_client/modbus_client.h @@ -3,6 +3,8 @@ #include "esphome/components/modbus/modbus.h" #include "esphome/components/modbus/modbus_helpers.h" #include "esphome/core/automation.h" +#include "esphome/core/hal.h" +#include "esphome/core/helpers.h" #include #include @@ -161,6 +163,22 @@ class ModbusClientSendAction : public ClientActionBase, /// success status.) Each typed callback still checks succeeded() before firing its trigger: that branch /// is unreachable today, and is kept so a future change to that interception cannot silently deliver an /// exception as a successful reply. +// Static lists up to this size are staged on the stack on ESP8266; longer ones use the heap. +static constexpr size_t STATIC_REGISTERS_ON_STACK = 16; +static constexpr size_t STATIC_COIL_BYTES_ON_STACK = 16; + +/// Calls fn with a RAM-readable span of a static values table. On ESP8266 the table is in PROGMEM, which the +/// PDU builders cannot read directly, so it is copied to the stack first (the heap above N elements). +template void with_static_values(const T *data, size_t len, F &&fn) { +#ifdef USE_ESP8266 + SmallBufferWithHeapFallback buf(len); + progmem_memcpy(buf.get(), data, len * sizeof(T)); + fn(std::span(buf.get(), len)); +#else + fn(std::span(data, len)); +#endif +} + template class TypedClientActionBase : public ClientActionBase { public: Trigger, std::span> *get_custom_response_trigger() { @@ -290,7 +308,7 @@ class WriteSingleCoilAction : public TypedClientActionBase, public WriteC }; /// modbus_client.write_multiple_registers: on_response is the acknowledgement (no arguments). -/// A `values:` list is emitted as a flash array and sent straight from there; only a lambda builds a +/// A `values:` list is a shared flash table (staged on the stack on ESP8266); only a lambda builds a /// vector, and only when it runs. Same split as canbus's send action, and for the same reason: a static /// list must not allocate on every play(). template @@ -298,7 +316,7 @@ class WriteMultipleRegistersAction : public TypedClientActionBase, public public: TEMPLATABLE_VALUE(uint16_t, start_address) - /// Static config: the registers live in flash, so play() neither allocates nor copies. + /// Static config: the registers are a shared flash table. void set_values_static(const uint16_t *values, size_t len) { this->values_.data = values; this->len_ = static_cast(len); @@ -317,9 +335,11 @@ class WriteMultipleRegistersAction : public TypedClientActionBase, public // An empty or over-long set rejects into an empty PDU inside the builder, which logs the reason; // the empty PDU then resolves via on_not_sent like any refused send. if (this->len_ >= 0) { - this->send_or_resolve_(modbus::helpers::create_write_registers_pdu( - start, std::span(this->values_.data, static_cast(this->len_))), - this->write_command_options_(x...)); + with_static_values( + this->values_.data, static_cast(this->len_), [&](std::span values) { + this->send_or_resolve_(modbus::helpers::create_write_registers_pdu(start, values), + this->write_command_options_(x...)); + }); return; } const std::vector values = this->values_.func(x...); @@ -367,11 +387,11 @@ class WriteMultipleCoilsAction : public TypedClientActionBase, public Wri const uint16_t start = this->start_address_.value(x...); if (this->count_ >= 0) { const auto count = static_cast(this->count_); - this->send_or_resolve_( - modbus::helpers::create_write_coils_pdu( - start, modbus::PackedBits(std::span(this->values_.packed, modbus::packed_bit_bytes(count)), - count)), - this->write_command_options_(x...)); + with_static_values( + this->values_.packed, modbus::packed_bit_bytes(count), [&](std::span packed) { + this->send_or_resolve_(modbus::helpers::create_write_coils_pdu(start, modbus::PackedBits(packed, count)), + this->write_command_options_(x...)); + }); return; } // The builder packs and bound-checks; an over-long set is rejected and logged there. @@ -402,7 +422,7 @@ class ReadWriteMultipleRegistersAction : public TypedClientActionBase, pu TEMPLATABLE_VALUE(uint16_t, read_count) TEMPLATABLE_VALUE(uint16_t, write_address) - /// Static config: the write registers live in flash, so play() neither allocates nor copies. + /// Static config: the write registers are a shared flash table. void set_values_static(const uint16_t *values, size_t len) { this->values_.data = values; this->len_ = static_cast(len); @@ -421,10 +441,12 @@ class ReadWriteMultipleRegistersAction : public TypedClientActionBase, pu const uint16_t write_start = this->write_address_.value(x...); // An out-of-range read/write count builds an empty PDU (the builder logs why), resolving via on_not_sent. if (this->len_ >= 0) { - this->send_or_resolve_(modbus::helpers::create_read_write_multiple_registers_pdu( - read_start, read_count, write_start, - std::span(this->values_.data, static_cast(this->len_))), - this->command_options_(x...)); + with_static_values( + this->values_.data, static_cast(this->len_), [&](std::span values) { + this->send_or_resolve_( + modbus::helpers::create_read_write_multiple_registers_pdu(read_start, read_count, write_start, values), + this->command_options_(x...)); + }); return; } const std::vector values = this->values_.func(x...); diff --git a/tests/component_tests/modbus_client/config/write_values_tables.yaml b/tests/component_tests/modbus_client/config/write_values_tables.yaml new file mode 100644 index 00000000000..56c3bb255f2 --- /dev/null +++ b/tests/component_tests/modbus_client/config/write_values_tables.yaml @@ -0,0 +1,35 @@ +esphome: + name: test + on_boot: + then: + - modbus_client.write_multiple_registers: + address: 0x01 + start_address: 0x10 + values: [0x1234, 0xABCD] + - modbus_client.read_write_multiple_registers: + address: 0x01 + write_address: 0x20 + values: [0x1234, 0xABCD] + read_address: 0x30 + - modbus_client.write_multiple_coils: + address: 0x01 + start_address: 0x40 + values: [true, false, true, true, false, false, true, false, true] + - modbus_client.write_multiple_coils: + address: 0x01 + start_address: 0x50 + values: [true, false, true, true, false, false, true, false, true] + - modbus_client.write_multiple_registers: + address: 0x01 + start_address: 0x60 + values: !lambda return {1}; + +esp32: + board: esp32dev + +uart: + tx_pin: 17 + rx_pin: 16 + baud_rate: 9600 + +modbus: diff --git a/tests/component_tests/modbus_client/test_write_values_tables.py b/tests/component_tests/modbus_client/test_write_values_tables.py new file mode 100644 index 00000000000..720df7ec497 --- /dev/null +++ b/tests/component_tests/modbus_client/test_write_values_tables.py @@ -0,0 +1,22 @@ +"""Tests for modbus_client constant write values in shared PROGMEM tables.""" + +from collections.abc import Callable +from pathlib import Path +import re + + +def test_constant_values_share_progmem_tables( + generate_main: Callable[[str | Path], str], + component_config_path: Callable[[str], Path], +) -> None: + """Equal value lists share one table across actions; lambdas stay templates.""" + main_cpp = generate_main(component_config_path("write_values_tables.yaml")) + + tables = re.findall( + r"static constexpr (uint16_t|uint8_t) \w+\[\] PROGMEM = (\{[^}]*\});", main_cpp + ) + # Coils pack LSB first: 0b01001101, then bit 8 in the second byte + assert tables == [("uint16_t", "{0x1234, 0xABCD}"), ("uint8_t", "{77, 1}")] + assert main_cpp.count("set_values_static(modbus_client_registers, 2);") == 2 + assert main_cpp.count("set_values_static(modbus_client_coils, 9);") == 2 + assert "set_values_template(" in main_cpp