diff --git a/esphome/components/canbus/__init__.py b/esphome/components/canbus/__init__.py index b7de235dd1..b70bac7265 100644 --- a/esphome/components/canbus/__init__.py +++ b/esphome/components/canbus/__init__.py @@ -144,20 +144,26 @@ async def register_canbus(var: MockObj, config: ConfigType) -> None: # Actions +CANBUS_SEND_ACTION_SCHEMA = cv.maybe_simple_value( + { + cv.GenerateID(CONF_CANBUS_ID): cv.use_id(CanbusComponent), + cv.Optional(CONF_CAN_ID): cv.int_range(min=0, max=0x1FFFFFFF), + cv.Optional(CONF_USE_EXTENDED_ID, default=False): cv.boolean, + cv.Optional(CONF_REMOTE_TRANSMISSION_REQUEST, default=False): cv.boolean, + # One classic CAN frame + cv.Required(CONF_DATA): cv.templatable( + cv.All(validate_raw_data, cv.Length(max=8)) + ), + }, + validate_id, + key=CONF_DATA, +) + + @automation.register_action( "canbus.send", canbus_ns.class_("CanbusSendAction", automation.Action), - cv.maybe_simple_value( - { - cv.GenerateID(CONF_CANBUS_ID): cv.use_id(CanbusComponent), - cv.Optional(CONF_CAN_ID): cv.int_range(min=0, max=0x1FFFFFFF), - cv.Optional(CONF_USE_EXTENDED_ID, default=False): cv.boolean, - cv.Optional(CONF_REMOTE_TRANSMISSION_REQUEST, default=False): cv.boolean, - cv.Required(CONF_DATA): cv.templatable(validate_raw_data), - }, - validate_id, - key=CONF_DATA, - ), + CANBUS_SEND_ACTION_SCHEMA, synchronous=True, ) async def canbus_action_to_code( @@ -177,15 +183,11 @@ async def canbus_action_to_code( var.set_remote_transmission_request(config[CONF_REMOTE_TRANSMISSION_REQUEST]) ) - data = config[CONF_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: - if isinstance(data, bytes): - data = [int(x) for x in data] - # 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, + "canbus_data", + ) return var diff --git a/esphome/components/canbus/canbus.cpp b/esphome/components/canbus/canbus.cpp index c5ba59a645..1032a389c6 100644 --- a/esphome/components/canbus/canbus.cpp +++ b/esphome/components/canbus/canbus.cpp @@ -22,10 +22,11 @@ void Canbus::dump_config() { } canbus::Error Canbus::send_data(uint32_t can_id, bool use_extended_id, bool remote_transmission_request, - const std::vector &data) { + const uint8_t *data, size_t len) { struct CanFrame can_message; - uint8_t size = static_cast(data.size()); + // Clamp before narrowing, so a lambda payload of 256 bytes or more cannot wrap to a short length + const uint8_t size = static_cast(std::min(len, CAN_MAX_DATA_LENGTH)); if (use_extended_id) { ESP_LOGD(TAG, "send extended id=0x%08" PRIx32 " rtr=%s size=%d", can_id, TRUEFALSE(remote_transmission_request), size); @@ -33,8 +34,6 @@ canbus::Error Canbus::send_data(uint32_t can_id, bool use_extended_id, bool remo ESP_LOGD(TAG, "send standard id=0x%03" PRIx32 " rtr=%s size=%d", can_id, TRUEFALSE(remote_transmission_request), size); } - if (size > CAN_MAX_DATA_LENGTH) - size = CAN_MAX_DATA_LENGTH; can_message.can_data_length_code = size; can_message.can_id = can_id; can_message.use_extended_id = use_extended_id; diff --git a/esphome/components/canbus/canbus.h b/esphome/components/canbus/canbus.h index 1bc4d6e345..8685c026b0 100644 --- a/esphome/components/canbus/canbus.h +++ b/esphome/components/canbus/canbus.h @@ -69,8 +69,12 @@ class Canbus : public Component { float get_setup_priority() const override { return setup_priority::HARDWARE; } void loop() override; + canbus::Error send_data(uint32_t can_id, bool use_extended_id, bool remote_transmission_request, const uint8_t *data, + size_t len); canbus::Error send_data(uint32_t can_id, bool use_extended_id, bool remote_transmission_request, - const std::vector &data); + const std::vector &data) { + return this->send_data(can_id, use_extended_id, remote_transmission_request, data.data(), data.size()); + } canbus::Error send_data(uint32_t can_id, bool use_extended_id, const std::vector &data) { // for backwards compatibility only return this->send_data(can_id, use_extended_id, false, data); @@ -107,18 +111,7 @@ class Canbus : public Component { }; template class CanbusSendAction 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->data_.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->data_.data = data; - this->len_ = len; // Length >= 0 indicates static mode - } + TEMPLATABLE_BYTES(data) void set_can_id(uint32_t can_id) { this->can_id_ = can_id; } @@ -132,26 +125,17 @@ template class CanbusSendAction final : public Action, pu auto can_id = this->can_id_.has_value() ? *this->can_id_ : this->parent_->can_id_; auto use_extended_id = this->use_extended_id_.has_value() ? *this->use_extended_id_ : this->parent_->use_extended_id_; - 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_->send_data(can_id, use_extended_id, this->remote_transmission_request_, data); + this->data_.template visit( + [&](const uint8_t *data, size_t len) { + this->parent_->send_data(can_id, use_extended_id, this->remote_transmission_request_, data, len); + }, + x...); } protected: optional can_id_{}; optional use_extended_id_{}; bool remote_transmission_request_{false}; - 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_; }; class CanbusTrigger final : public Trigger, uint32_t, bool>, public Component { diff --git a/tests/component_tests/canbus/__init__.py b/tests/component_tests/canbus/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/component_tests/canbus/config/payload_tables.yaml b/tests/component_tests/canbus/config/payload_tables.yaml new file mode 100644 index 0000000000..abbf0bab8a --- /dev/null +++ b/tests/component_tests/canbus/config/payload_tables.yaml @@ -0,0 +1,21 @@ +--- +esphome: + name: test + on_boot: + then: + - canbus.send: [0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08] + - canbus.send: + can_id: 0x100 + data: [0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08] + - canbus.send: !lambda return {0x09}; + +esp32: + board: esp32dev + +canbus: + - platform: esp32_can + id: can_bus + rx_pin: 4 + tx_pin: 5 + can_id: 4 + bit_rate: 50kbps diff --git a/tests/component_tests/canbus/test_canbus_payload_tables.py b/tests/component_tests/canbus/test_canbus_payload_tables.py new file mode 100644 index 0000000000..958ec082e4 --- /dev/null +++ b/tests/component_tests/canbus/test_canbus_payload_tables.py @@ -0,0 +1,41 @@ +"""Tests for canbus constant payloads in shared PROGMEM tables.""" + +from collections.abc import Callable +from pathlib import Path +import re + +import pytest + +from esphome.components.canbus import CANBUS_SEND_ACTION_SCHEMA +import esphome.config_validation as cv + + +def test_constant_payloads_share_progmem_tables( + generate_main: Callable[[str | Path], str], + component_config_path: Callable[[str], Path], +) -> None: + """Equal send payloads share one table; lambdas stay templates.""" + main_cpp = generate_main(component_config_path("payload_tables.yaml")) + + tables = re.findall( + r"static constexpr uint8_t (\w+)\[\] PROGMEM = (\{[^}]*\});", main_cpp + ) + assert [v for _, v in tables] == [ + "{0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08}" + ] + assert main_cpp.count(f"set_data_static({tables[0][0]}, 8);") == 2 + assert "set_data_template(" in main_cpp + + +@pytest.mark.parametrize("size", [8, 9]) +@pytest.mark.parametrize("make", [lambda n: [0x01] * n, lambda n: "a" * n]) +def test_send_one_frame_limit( + size: int, make: Callable[[int], list[int] | str] +) -> None: + """Payloads up to one 8 byte CAN frame are accepted, longer ones rejected.""" + config = {"canbus_id": "can_bus", "data": make(size)} + if size <= 8: + assert len(CANBUS_SEND_ACTION_SCHEMA(config)["data"]) == size + else: + with pytest.raises(cv.Invalid, match="at most 8"): + CANBUS_SEND_ACTION_SCHEMA(config)