diff --git a/esphome/components/modbus/__init__.py b/esphome/components/modbus/__init__.py index 89ffc7facf..c9ba00f111 100644 --- a/esphome/components/modbus/__init__.py +++ b/esphome/components/modbus/__init__.py @@ -175,15 +175,34 @@ async def to_code(config: ConfigType) -> None: cg.add(var.set_turnaround_time(config[CONF_TURNAROUND_TIME])) +# The broadcast address (0) is delivered to every device and is never answered (Modbus 4.1), +# so it cannot identify an individual device or read anything back. +BROADCAST_ADDRESS = 0 + + +def reject_broadcast_address( + address: int, usage: str, guidance: str, path: list[str] | None = None +) -> None: + """Raise cv.Invalid if `address` is the Modbus broadcast address (0). + + `usage` names how the address is being used (e.g. "a server device address") and `guidance` + is a sentence telling the user what to do instead. Sharing the leading sentence here keeps the + call sites (server device, modbus_controller) from drifting apart. + """ + if address == BROADCAST_ADDRESS: + raise cv.Invalid( + f"Address 0 is the Modbus broadcast address and cannot be used as {usage}. {guidance}", + path, + ) + + def _validate_server_address(value: Any) -> int: address = cv.hex_uint8_t(value) - # The broadcast address (0) is delivered to every device and is never answered (Modbus 4.1), - # so it cannot identify an individual server device. - if address == 0: - raise cv.Invalid( - "Address 0 is the Modbus broadcast address and cannot be used as a " - "server device address. Assign a unique unit address instead." - ) + reject_broadcast_address( + address, + "a server device address", + "Assign a unique unit address instead.", + ) return address diff --git a/esphome/components/modbus_controller/__init__.py b/esphome/components/modbus_controller/__init__.py index f3cd28d138..188b552a3c 100644 --- a/esphome/components/modbus_controller/__init__.py +++ b/esphome/components/modbus_controller/__init__.py @@ -1,4 +1,6 @@ import binascii +from dataclasses import dataclass +from typing import Any from esphome import automation import esphome.codegen as cg @@ -10,7 +12,9 @@ from esphome.components.modbus.helpers import ( ) import esphome.config_validation as cv from esphome.const import CONF_ADDRESS, CONF_ID, CONF_LAMBDA, CONF_NAME, CONF_OFFSET +from esphome.core import CORE from esphome.cpp_helpers import logging +import esphome.final_validate as fv from esphome.types import ConfigType from .const import ( @@ -19,6 +23,7 @@ from .const import ( CONF_BYTE_OFFSET, CONF_COMMAND_THROTTLE, CONF_CUSTOM_COMMAND, + CONF_CUSTOM_PDU, CONF_FORCE_NEW_RANGE, CONF_MAX_CMD_RETRIES, CONF_MODBUS_CONTROLLER_ID, @@ -41,6 +46,8 @@ AUTO_LOAD = ["modbus"] MULTI_CONF = True +DOMAIN = "modbus_controller" + modbus_controller_ns = cg.esphome_ns.namespace("modbus_controller") ModbusController = modbus_controller_ns.class_("ModbusController", cg.PollingComponent) @@ -48,6 +55,19 @@ SensorItem = modbus_controller_ns.struct("SensorItem") _LOGGER = logging.getLogger(__name__) + +@dataclass +class ModbusControllerData: + # Set once the deprecated 'skip_updates' warning has been emitted so we warn only once total. + skip_updates_warned: bool = False + + +def _get_data() -> ModbusControllerData: + if DOMAIN not in CORE.data: + CORE.data[DOMAIN] = ModbusControllerData() + return CORE.data[DOMAIN] + + # Remove before 2027.2.0 _REMOVED_OPTIONS = { CONF_COMMAND_THROTTLE: "Command spacing is handled by the 'modbus' component - use 'turnaround_time' there instead.", @@ -67,6 +87,32 @@ def _warn_removed_options(config: ConfigType) -> ConfigType: return config +def _reject_broadcast_address(config: ConfigType) -> ConfigType: + """A modbus_controller polls one device, so its address cannot be the broadcast address (0): + a broadcast is never answered (Modbus 4.1), so no register could ever read back.""" + modbus.reject_broadcast_address( + config.get(CONF_ADDRESS), + "a modbus_controller device address", + "Assign the unit address of the device you want to poll.", + [CONF_ADDRESS], + ) + return config + + +# Remove before 2027.3.0. skip_updates (a per-sensor option) no longer does anything: every range is +# polled each update_interval. The key is still accepted so existing configs keep working, with a warning. +def validate_skip_updates_deprecated(value: Any) -> int: + data = _get_data() + if not data.skip_updates_warned: + _LOGGER.warning( + "[modbus_controller] 'skip_updates' no longer has any effect and will be removed in 2027.3.0. " + "To poll some registers less often, add a second modbus_controller with the same address and a " + "slower update_interval, and attach the slow sensors to it." + ) + data.skip_updates_warned = True + return cv.positive_int(value) + + CONFIG_SCHEMA = cv.All( cv.Schema( { @@ -92,13 +138,32 @@ CONFIG_SCHEMA = cv.All( .extend(cv.polling_component_schema("60s")) .extend(modbus.modbus_device_schema(0x01)), _warn_removed_options, + _reject_broadcast_address, ) ModbusItemBaseSchema = cv.Schema( { cv.GenerateID(CONF_MODBUS_CONTROLLER_ID): cv.use_id(ModbusController), cv.Optional(CONF_ADDRESS): cv.positive_int, - cv.Optional(CONF_CUSTOM_COMMAND): cv.ensure_list(cv.hex_uint8_t), + cv.Exclusive( + CONF_CUSTOM_PDU, + "custom_source", + f"{CONF_CUSTOM_PDU} and {CONF_CUSTOM_COMMAND} can't be used together", + ): cv.All( + cv.ensure_list(cv.hex_uint8_t), + cv.Length(min=1, max=modbus.MAX_PDU_SIZE), + ), + # Deprecated: takes a raw frame with a leading device address byte. Auto-migrated to + # custom_pdu in migrate_custom_command (final validate). Remove before 2027.3.0. The upper + # bound is MAX_PDU_SIZE + 1: the extra byte is the address the migration strips. + cv.Exclusive( + CONF_CUSTOM_COMMAND, + "custom_source", + f"{CONF_CUSTOM_PDU} and {CONF_CUSTOM_COMMAND} can't be used together", + ): cv.All( + cv.ensure_list(cv.hex_uint8_t), + cv.Length(min=2, max=modbus.MAX_PDU_SIZE + 1), + ), cv.Exclusive( CONF_OFFSET, "offset", @@ -110,7 +175,7 @@ ModbusItemBaseSchema = cv.Schema( f"{CONF_OFFSET} and {CONF_BYTE_OFFSET} can't be used together", ): cv.positive_int, cv.Optional(CONF_BITMASK, default=0xFFFFFFFF): cv.hex_uint32_t, - cv.Optional(CONF_SKIP_UPDATES, default=0): cv.positive_int, + cv.Optional(CONF_SKIP_UPDATES): validate_skip_updates_deprecated, cv.Optional(CONF_FORCE_NEW_RANGE, default=False): cv.boolean, cv.Optional(CONF_LAMBDA): cv.returning_lambda, cv.Optional(CONF_RESPONSE_SIZE, default=0): cv.positive_int, @@ -119,22 +184,56 @@ ModbusItemBaseSchema = cv.Schema( def validate_modbus_register(config): - if CONF_CUSTOM_COMMAND not in config and CONF_ADDRESS not in config: + # custom_command is the deprecated alias for custom_pdu (migrated later in final validate); treat + # either as "a custom frame is configured" so the address/register_type rules match. + has_custom = CONF_CUSTOM_PDU in config or CONF_CUSTOM_COMMAND in config + if not has_custom and CONF_ADDRESS not in config: raise cv.Invalid( - f" {CONF_ADDRESS} is a required property if '{CONF_CUSTOM_COMMAND}:' isn't used" + f" {CONF_ADDRESS} is a required property if '{CONF_CUSTOM_PDU}:' isn't used" ) - if CONF_CUSTOM_COMMAND in config and CONF_REGISTER_TYPE in config: + if has_custom and CONF_REGISTER_TYPE in config: raise cv.Invalid( - f"can't use '{CONF_REGISTER_TYPE}:' together with '{CONF_CUSTOM_COMMAND}:'", + f"can't use '{CONF_REGISTER_TYPE}:' together with '{CONF_CUSTOM_PDU}:'", ) - if CONF_CUSTOM_COMMAND not in config and CONF_REGISTER_TYPE not in config: + if not has_custom and CONF_REGISTER_TYPE not in config: raise cv.Invalid( - f" {CONF_REGISTER_TYPE} is a required property if '{CONF_CUSTOM_COMMAND}:' isn't used" + f" {CONF_REGISTER_TYPE} is a required property if '{CONF_CUSTOM_PDU}:' isn't used" ) return config +def migrate_custom_command(config: ConfigType) -> None: + """Final-validate: auto-migrate the deprecated custom_command (raw frame incl. device address) + to custom_pdu (PDU only). custom_pdu is always sent to the controller's own address, so a frame + whose address byte does not match the controller's address is a hard error (it targeted a + different unit). Mutates config in place; final validate discards the return value.""" + frame = config.get(CONF_CUSTOM_COMMAND) + if frame is None: + return + fconf = fv.full_config.get() + path = fconf.get_path_for_id(config[CONF_MODBUS_CONTROLLER_ID])[:-1] + controller = fconf.get_config_for_path(path) + # the controller's DEVICE address (from modbus_device_schema) + address = controller[CONF_ADDRESS] + if frame[0] != address: + raise cv.Invalid( + f"'custom_command' begins with device address {frame[0]:#04x}, but this sensor's " + f"modbus_controller uses address {address:#04x}. 'custom_command' is renamed to " + f"'custom_pdu', which is always sent to the controller's own address. Drop the leading " + f"address byte and use 'custom_pdu' if {address:#04x} is correct, or move this sensor to " + f"the modbus_controller for device {frame[0]:#04x}.", + [CONF_CUSTOM_COMMAND], + ) + _LOGGER.warning( + "[modbus_controller] 'custom_command' is deprecated and will be removed in 2027.3.0; " + "auto-migrated to 'custom_pdu' (dropped the leading device address byte). Rename the key " + "and drop that byte to silence this warning." + ) + config[CONF_CUSTOM_PDU] = list(frame[1:]) + del config[CONF_CUSTOM_COMMAND] + + def _final_validate(config: ConfigType) -> None: modbus.final_validate_modbus_device("modbus_controller", role="client")(config) @@ -156,7 +255,7 @@ def modbus_calc_properties(config): value_type = config[CONF_VALUE_TYPE] if reg_count == 0: reg_count = TYPE_REGISTER_MAP[value_type] - if CONF_CUSTOM_COMMAND in config: + if CONF_CUSTOM_PDU in config: if CONF_ADDRESS not in config: # generate a unique modbus address using the hash of the name # CONF_NAME set even if only CONF_ID is used. @@ -173,8 +272,8 @@ def modbus_calc_properties(config): async def add_modbus_base_properties( var, config, sensor_type, lambda_param_type=cg.float_, lambda_return_type=float ): - if CONF_CUSTOM_COMMAND in config: - cg.add(var.set_custom_data(config[CONF_CUSTOM_COMMAND])) + if CONF_CUSTOM_PDU in config: + cg.add(var.set_custom_pdu(config[CONF_CUSTOM_PDU])) if config[CONF_RESPONSE_SIZE] > 0: cg.add(var.set_register_size(config[CONF_RESPONSE_SIZE])) diff --git a/esphome/components/modbus_controller/binary_sensor/__init__.py b/esphome/components/modbus_controller/binary_sensor/__init__.py index 18d017e13f..6ff1975b1e 100644 --- a/esphome/components/modbus_controller/binary_sensor/__init__.py +++ b/esphome/components/modbus_controller/binary_sensor/__init__.py @@ -8,6 +8,7 @@ from .. import ( ModbusItemBaseSchema, SensorItem, add_modbus_base_properties, + migrate_custom_command, modbus_calc_properties, modbus_controller_ns, validate_modbus_register, @@ -17,7 +18,6 @@ from ..const import ( CONF_FORCE_NEW_RANGE, CONF_MODBUS_CONTROLLER_ID, CONF_REGISTER_TYPE, - CONF_SKIP_UPDATES, ) DEPENDENCIES = ["modbus_controller"] @@ -40,6 +40,8 @@ CONFIG_SCHEMA = cv.All( validate_modbus_register, ) +FINAL_VALIDATE_SCHEMA = migrate_custom_command + async def to_code(config): byte_offset, _ = modbus_calc_properties(config) @@ -49,7 +51,6 @@ async def to_code(config): config[CONF_ADDRESS], byte_offset, config[CONF_BITMASK], - config[CONF_SKIP_UPDATES], config[CONF_FORCE_NEW_RANGE], ) await cg.register_component(var, config) diff --git a/esphome/components/modbus_controller/binary_sensor/modbus_binarysensor.h b/esphome/components/modbus_controller/binary_sensor/modbus_binarysensor.h index 62a7fe93d3..f5ddbd82cc 100644 --- a/esphome/components/modbus_controller/binary_sensor/modbus_binarysensor.h +++ b/esphome/components/modbus_controller/binary_sensor/modbus_binarysensor.h @@ -11,13 +11,12 @@ namespace esphome::modbus_controller { class ModbusBinarySensor final : public Component, public binary_sensor::BinarySensor, public SensorItem { public: ModbusBinarySensor(modbus::EntityType register_type, uint16_t start_address, uint8_t offset, uint32_t bitmask, - uint16_t skip_updates, bool force_new_range) { + bool force_new_range) { this->register_type = register_type; this->set_address(start_address); this->set_offset_from_start_address(offset); this->bitmask = bitmask; this->sensor_value_type = SensorValueType::BIT; - this->skip_updates = skip_updates; this->force_new_range = force_new_range; if (modbus::helpers::is_entity_type_binary(register_type)) { diff --git a/esphome/components/modbus_controller/const.py b/esphome/components/modbus_controller/const.py index 0149a3cc49..8412a651b8 100644 --- a/esphome/components/modbus_controller/const.py +++ b/esphome/components/modbus_controller/const.py @@ -4,6 +4,7 @@ CONF_BYTE_OFFSET = "byte_offset" CONF_COMMAND_THROTTLE = "command_throttle" CONF_OFFLINE_SKIP_UPDATES = "offline_skip_updates" CONF_CUSTOM_COMMAND = "custom_command" +CONF_CUSTOM_PDU = "custom_pdu" CONF_FORCE_NEW_RANGE = "force_new_range" CONF_MAX_CMD_RETRIES = "max_cmd_retries" CONF_MODBUS_CONTROLLER_ID = "modbus_controller_id" diff --git a/esphome/components/modbus_controller/modbus_controller.cpp b/esphome/components/modbus_controller/modbus_controller.cpp index 515459f62a..fb7010cb4e 100644 --- a/esphome/components/modbus_controller/modbus_controller.cpp +++ b/esphome/components/modbus_controller/modbus_controller.cpp @@ -14,7 +14,6 @@ ModbusCommandItem::ModbusCommandItem(ModbusController &controller, modbus::Modbu RegisterRange &&range) : modbus::ModbusClientDevice(parent, address), sensors(std::move(range.sensors)), - skip_updates(range.skip_updates), register_type_(range.register_type), start_address_(range.start_address), register_count_(range.register_count), @@ -24,12 +23,14 @@ ModbusCommandItem::ModbusCommandItem(ModbusController &controller, modbus::Modbu ModbusCommandItem::ModbusCommandItem(ModbusController &controller, modbus::ModbusClientHub *parent, uint8_t address, SensorItem *sensor) : modbus::ModbusClientDevice(parent, address), - skip_updates(sensor->skip_updates), start_address_(sensor->start_address), register_count_(sensor->register_count), - function_code_(FunctionCode::CUSTOM), - custom_data_(&sensor->custom_data), + custom_pdu_(&sensor->custom_pdu), controller_(&controller) { + // The PDU's first byte is its real function code; carry it so dump_config, the on_command_sent + // trigger and the response callbacks report the actual code instead of CUSTOM. + if (!sensor->custom_pdu.empty()) + this->function_code_ = static_cast(sensor->custom_pdu.data()[0]); this->sensors.insert(sensor); } @@ -40,13 +41,12 @@ ModbusCommandItem::ModbusCommandItem(ModbusController &controller, modbus::Modbu ModbusCommandItem::ModbusCommandItem(const ModbusCommandItem &other) : modbus::ModbusClientDevice(other.parent_, other.address_), sensors(other.sensors), - skip_updates(other.skip_updates), on_data_func(other.on_data_func), register_type_(other.register_type_), start_address_(other.start_address_), register_count_(other.register_count_), function_code_(other.function_code_), - custom_data_(other.custom_data_), + custom_pdu_(other.custom_pdu_), controller_(other.controller_) { // SmallInlineBuffer is move-only, so deep-copy the bytes explicitly. this->payload.set(other.payload.data(), other.payload.size()); @@ -55,14 +55,13 @@ ModbusCommandItem::ModbusCommandItem(const ModbusCommandItem &other) ModbusCommandItem::ModbusCommandItem(ModbusCommandItem &&other) noexcept : modbus::ModbusClientDevice(other.parent_, other.address_), sensors(std::move(other.sensors)), - skip_updates(other.skip_updates), on_data_func(std::move(other.on_data_func)), payload(std::move(other.payload)), register_type_(other.register_type_), start_address_(other.start_address_), register_count_(other.register_count_), function_code_(other.function_code_), - custom_data_(other.custom_data_), + custom_pdu_(other.custom_pdu_), controller_(other.controller_) { other.parent_ = nullptr; } @@ -74,11 +73,14 @@ void ModbusCommandItem::on_response(std::span request_pdu, std::s auto data = modbus::helpers::server_pdu_payload(response_pdu); if (this->on_data_func) { this->on_data_func(this->register_type_, this->start_address_, data); - } else if (modbus::helpers::is_function_code_write(static_cast(this->function_code_))) { - // write acknowledgement - nothing to publish - } else { + } else if (!this->sensors.empty()) { + // A polling command always has sensors; a factory/write command never does. Test this before the + // write-code branch so a custom_pdu whose function code is a write (e.g. 0x17, whose response + // carries read data) still reaches its sensor instead of being treated as a bare write ack. for (auto *sensor : this->sensors) sensor->parse_and_publish(data); + } else if (modbus::helpers::is_function_code_write(static_cast(this->function_code_))) { + // write acknowledgement - nothing to publish } if (this->controller_ != nullptr) this->controller_->unqueue_command(this); @@ -116,13 +118,11 @@ void ModbusCommandItem::on_sent(std::span request_pdu) { // on_sent is this command's only callback, so drop the one-shot from the queue here, or it would leak. // Test the address the frame went to, not address_: a custom command's frame carries its own address // (frame[0]), which may differ from this controller's. (unqueue_command() is a no-op for a poll.) + // A custom polling command sends its PDU to this controller's own address, so only a factory custom + // command (a raw frame staged in payload) can carry a different address byte. uint8_t wire_address = this->address_; - if (this->function_code_ == FunctionCode::CUSTOM) { - std::span frame = - this->custom_data_ != nullptr ? std::span(*this->custom_data_) : this->payload; - if (!frame.empty()) - wire_address = frame[0]; - } + if (this->function_code_ == FunctionCode::CUSTOM && !this->payload.empty()) + wire_address = this->payload.data()[0]; if (wire_address == modbus::BROADCAST_ADDRESS) this->controller_->unqueue_command(this); } @@ -194,23 +194,11 @@ void ModbusController::sweep_completed_one_shots_() { [](const std::unique_ptr &item) { return item->pending_removal; }); } -void ModbusController::update_range_(ModbusCommandItem &cmd) { - if (this->update_counter_ % (cmd.skip_updates + 1) != 0) { - ESP_LOGVV(TAG, "Skipping update for range 0x%X", cmd.register_address()); - return; - } - // A refusal is already logged by the hub; note the affected range for controller-level diagnostics. - if (!cmd.send()) { - ESP_LOGD(TAG, "Poll refused by hub for range 0x%X", cmd.register_address()); - } -} - void ModbusController::update() { this->sweep_completed_one_shots_(); // reclaim one-shots deferred out of their own callbacks if (this->module_offline_) { - // Offline probing follows the offline cadence alone; per-range skip_updates resumes once the - // device is back online. Requiring both cadences to coincide would leave phase combinations - // where a probe never goes out. + // Offline probing follows the offline cadence alone; regular every-update polling resumes once + // the device is back online. if (offline_retry_due(this->update_counter_, this->module_offline_at_, this->offline_skip_updates_)) { ESP_LOGV(TAG, "Module offline - retrying"); this->cmd_non_responses_ = 0; // allow the probe through can_send() @@ -229,7 +217,9 @@ void ModbusController::update() { if (this->can_send()) { for (auto &cmd : this->polling_command_items_) { ESP_LOGVV(TAG, "Updating range 0x%X", cmd.register_address()); - this->update_range_(cmd); + // A refusal is already logged by the hub; note the affected range for controller-level diagnostics. + if (!cmd.send()) + ESP_LOGD(TAG, "Poll refused by hub for range 0x%X", cmd.register_address()); } } this->update_counter_++; @@ -264,8 +254,8 @@ void ModbusController::create_polling_commands_() { bool range_custom_size = false; SensorItem *prev = nullptr; for (SensorItem *curr : this->sensorset_) { - ESP_LOGV(TAG, "Register: 0x%X count=%d size=%zu offset=%u skip=%u addr=%p", curr->start_address, - curr->register_count, curr->get_register_size(), curr->offset, curr->skip_updates, curr); + ESP_LOGV(TAG, "Register: 0x%X count=%d size=%zu offset=%u addr=%p", curr->start_address, curr->register_count, + curr->get_register_size(), curr->offset, curr); const bool custom_size = curr->get_register_size() != static_cast(curr->register_count) * 2; @@ -296,14 +286,12 @@ void ModbusController::create_polling_commands_() { ESP_LOGV(TAG, "Extend range to include 0x%X", curr->start_address); } else if (range_shared && !range_forced && curr->start_address >= r.start_address && curr->start_address + curr->register_count <= r.start_address + r.register_count && - !range_custom_size && !custom_size && curr->skip_updates == r.skip_updates) { + !range_custom_size && !custom_size) { // The registers already fall inside a range that a shared-address join widened, so this sensor // reads its slice of that response instead of adding an overlapping second poll. The guards keep // it narrow: only a widened range, never a force-isolated one; only where every register in the // range returns two bytes, so interior positions follow from the addresses; only sensors genuinely - // inside it, which is why the lower bound is needed given the walk is not address-ordered; and - // only where the polling rates already match, since joining runs this sensor through the rate - // merge below and would otherwise change one of them. + // inside it, which is why the lower bound is needed given the walk is not address-ordered. const uint16_t addr_delta = curr->start_address - r.start_address; curr->offset = static_cast((curr->addresses_bits() ? addr_delta : addr_delta * 2) + curr->offset_from_start_address); @@ -329,7 +317,7 @@ void ModbusController::create_polling_commands_() { if (!join) { if (have_range) { - ESP_LOGV(TAG, "Add range 0x%X %d skip:%d", r.start_address, r.register_count, r.skip_updates); + ESP_LOGV(TAG, "Add range 0x%X %d", r.start_address, r.register_count); this->create_polling_command_(std::move(r)); } r = {}; @@ -341,11 +329,7 @@ void ModbusController::create_polling_commands_() { r.start_address = curr->start_address; r.register_count = curr->register_count; r.register_type = curr->register_type; - r.skip_updates = curr->skip_updates; have_range = true; - } else if (curr->skip_updates != 0) { - // use the lowest non-zero skip_updates for the whole range (0 is the default and is excluded) - r.skip_updates = (r.skip_updates != 0) ? std::min(r.skip_updates, curr->skip_updates) : curr->skip_updates; } // Every member records its range's first register. The resolved offset is relative to it, so the @@ -355,7 +339,7 @@ void ModbusController::create_polling_commands_() { prev = curr; } if (have_range) { - ESP_LOGV(TAG, "Add last range 0x%X %d skip:%d", r.start_address, r.register_count, r.skip_updates); + ESP_LOGV(TAG, "Add last range 0x%X %d", r.start_address, r.register_count); this->create_polling_command_(std::move(r)); } // Reclaim growth slack; safe here because nothing has registered with the hub yet (see the @@ -380,8 +364,8 @@ void ModbusController::dump_config() { } ESP_LOGCONFIG(TAG, "ranges"); for (auto &it : this->polling_command_items_) { - ESP_LOGCONFIG(TAG, " Range type=%u start=0x%X count=%d skip_updates=%d", static_cast(it.register_type()), - it.register_address(), it.register_count(), it.skip_updates); + ESP_LOGCONFIG(TAG, " Range type=%u start=0x%X count=%d", static_cast(it.register_type()), + it.register_address(), it.register_count()); } #endif } @@ -513,17 +497,19 @@ ModbusCommandItem ModbusCommandItem::create_custom_command( bool ModbusCommandItem::send() { bool accepted; - if (this->function_code_ != FunctionCode::CUSTOM) { + if (this->custom_pdu_ != nullptr) { + // Custom polling command: send the sensor's ready-made PDU (function code + data, no address byte) + // to this controller's own device address; the hub prepends the address and appends the CRC. + accepted = modbus::ModbusClientDevice::queue_pdu(std::span(*this->custom_pdu_)); + } else if (this->function_code_ != FunctionCode::CUSTOM) { accepted = this->queue_pdu(modbus::helpers::create_client_pdu( this->function_code_, this->start_address_, this->register_count_, this->payload.empty() ? nullptr : this->payload.data(), this->payload.size())); } else { - // Custom command: the bytes are a complete raw frame (address + PDU). Send the PDU to the frame's own - // address (which may differ from this controller's); the hub appends the CRC and routes the response - // back to this item by pointer. (send_raw() is deprecated, so queue_pdu() is called with the extracted - // address. Raw-frame semantics are kept here; the custom_pdu migration is a later step.) - std::span frame = - this->custom_data_ != nullptr ? std::span(*this->custom_data_) : this->payload; + // Factory custom command: payload holds a complete raw frame (address + PDU). Send the PDU to the + // frame's own address (which may differ from this controller's); the hub appends the CRC and routes + // the response back to this item by pointer. + std::span frame = this->payload; if (frame.empty()) { ESP_LOGW(TAG, "Empty custom command frame, not sent"); accepted = false; diff --git a/esphome/components/modbus_controller/modbus_controller.h b/esphome/components/modbus_controller/modbus_controller.h index fb0037a0e6..f36705cda4 100644 --- a/esphome/components/modbus_controller/modbus_controller.h +++ b/esphome/components/modbus_controller/modbus_controller.h @@ -157,7 +157,7 @@ class SensorItem { this->range_start_address = address; } - void set_custom_data(const std::vector &data) { custom_data = data; } + void set_custom_pdu(std::initializer_list pdu) { this->custom_pdu.set(pdu.begin(), pdu.size()); } size_t virtual get_register_size() const { if (this->addresses_bits()) { return 1; @@ -186,8 +186,7 @@ class SensorItem { uint8_t offset_from_start_address{0}; /// First register of the range this sensor is polled in; equals start_address for an unpolled item. uint16_t range_start_address{0}; - uint16_t skip_updates{0}; - std::vector custom_data{}; + SmallInlineBuffer<8> custom_pdu{}; bool force_new_range{false}; }; @@ -230,8 +229,7 @@ struct RegisterRange { uint16_t start_address; modbus::EntityType register_type; uint8_t register_count; - uint16_t skip_updates; // the config value - SensorSet sensors; // all sensors of this range + SensorSet sensors; // all sensors of this range }; /// A single modbus command. Each command is its own ModbusClientDevice: it sends its frame to the hub @@ -257,7 +255,6 @@ class ModbusCommandItem : public modbus::ModbusClientDevice { ModbusCommandItem &operator=(ModbusCommandItem &&) = delete; SensorSet sensors; // sensors served by this command (empty for factory/write commands) - uint16_t skip_updates{0}; std::function data)> on_data_func; /// Write data bytes for the command (register/coil values), or the raw frame of a one-shot custom /// command; reads leave it empty. Small-buffer optimized: fixed-size commands (single-register/coil @@ -378,7 +375,7 @@ class ModbusCommandItem : public modbus::ModbusClientDevice { uint16_t register_count_{0}; FunctionCode function_code_{FunctionCode::CUSTOM}; /// Custom polling commands reference the PDU bytes owned by their SensorItem instead of copying them. - const std::vector *custom_data_{nullptr}; + const SmallInlineBuffer<8> *custom_pdu_{nullptr}; ModbusController *controller_{nullptr}; }; @@ -462,19 +459,15 @@ class ModbusController final : public PollingComponent { void create_polling_commands_(); /// build one persistent polling command from a range and add it to polling_command_items_ void create_polling_command_(RegisterRange &&range) { - // A custom range polls the first sensor's custom_data (a ready-made raw frame); it needs the - // sensor constructor so the command references those bytes and decodes the real function code. - // The response still dispatches to every sensor in the range. + // A custom range polls the first sensor's custom_pdu (referenced, not copied); the sensor constructor + // decodes the real function code. The response still dispatches to every sensor in the range. if (range.register_type == EntityType::CUSTOM && !range.sensors.empty()) { auto &cmd = this->polling_command_items_.emplace_back(*this, this->hub_, this->address_, *range.sensors.begin()); cmd.sensors = std::move(range.sensors); - cmd.skip_updates = range.skip_updates; // the range's merged rate, not the first sensor's } else { this->polling_command_items_.emplace_back(*this, this->hub_, this->address_, std::move(range)); } } - /// send a range's polling command if it is due this update - void update_range_(ModbusCommandItem &cmd); /// The hub this controller's commands/entities send through, and the modbus address they target. modbus::ModbusClientHub *hub_{nullptr}; uint8_t address_{0}; @@ -496,7 +489,7 @@ class ModbusController final : public PollingComponent { bool module_offline_{false}; /// update_counter_ value at which the module went offline (for offline_skip_updates timing) uint16_t module_offline_at_{0}; - /// counts update() cycles; drives skip_updates and offline timing + /// counts update() cycles; drives the offline-retry cadence uint16_t update_counter_{0}; /// consecutive non-responses; drives can_send() and offline detection uint8_t cmd_non_responses_{0}; diff --git a/esphome/components/modbus_controller/number/__init__.py b/esphome/components/modbus_controller/number/__init__.py index 7563adfad9..39d04e8d91 100644 --- a/esphome/components/modbus_controller/number/__init__.py +++ b/esphome/components/modbus_controller/number/__init__.py @@ -18,16 +18,17 @@ from .. import ( ModbusItemBaseSchema, SensorItem, add_modbus_base_properties, + migrate_custom_command, modbus_calc_properties, modbus_controller_ns, ) from ..const import ( CONF_BITMASK, CONF_CUSTOM_COMMAND, + CONF_CUSTOM_PDU, CONF_FORCE_NEW_RANGE, CONF_MODBUS_CONTROLLER_ID, CONF_REGISTER_TYPE, - CONF_SKIP_UPDATES, CONF_USE_WRITE_MULTIPLE, CONF_VALUE_TYPE, CONF_WRITE_LAMBDA, @@ -53,9 +54,11 @@ def validate_min_max(config): def validate_modbus_number(config): - if CONF_CUSTOM_COMMAND not in config and CONF_ADDRESS not in config: + # custom_command is the deprecated alias for custom_pdu (migrated later in final validate). + has_custom = CONF_CUSTOM_PDU in config or CONF_CUSTOM_COMMAND in config + if not has_custom and CONF_ADDRESS not in config: raise cv.Invalid( - f" {CONF_ADDRESS} is a required property if '{CONF_CUSTOM_COMMAND}:' isn't used" + f" {CONF_ADDRESS} is a required property if '{CONF_CUSTOM_PDU}:' isn't used" ) return config @@ -83,6 +86,8 @@ CONFIG_SCHEMA = cv.All( validate_modbus_number, ) +FINAL_VALIDATE_SCHEMA = migrate_custom_command + async def to_code(config): byte_offset, reg_count = modbus_calc_properties(config) @@ -94,7 +99,6 @@ async def to_code(config): config[CONF_BITMASK], config[CONF_VALUE_TYPE], reg_count, - config[CONF_SKIP_UPDATES], config[CONF_FORCE_NEW_RANGE], ) diff --git a/esphome/components/modbus_controller/number/modbus_number.h b/esphome/components/modbus_controller/number/modbus_number.h index 1f0d0581eb..538a982f80 100644 --- a/esphome/components/modbus_controller/number/modbus_number.h +++ b/esphome/components/modbus_controller/number/modbus_number.h @@ -13,14 +13,13 @@ using value_to_data_t = std::function(float); class ModbusNumber final : public number::Number, public Component, public SensorItem { public: ModbusNumber(modbus::EntityType register_type, uint16_t start_address, uint8_t offset, uint32_t bitmask, - SensorValueType value_type, int register_count, uint16_t skip_updates, bool force_new_range) { + SensorValueType value_type, int register_count, bool force_new_range) { this->register_type = register_type; this->set_address(start_address); this->set_offset_from_start_address(offset); this->bitmask = bitmask; this->sensor_value_type = value_type; this->register_count = register_count; - this->skip_updates = skip_updates; this->force_new_range = force_new_range; }; diff --git a/esphome/components/modbus_controller/output/__init__.py b/esphome/components/modbus_controller/output/__init__.py index 27d212d58d..178c99caa1 100644 --- a/esphome/components/modbus_controller/output/__init__.py +++ b/esphome/components/modbus_controller/output/__init__.py @@ -12,6 +12,7 @@ from .. import ( ) from ..const import ( CONF_CUSTOM_COMMAND, + CONF_CUSTOM_PDU, CONF_MODBUS_CONTROLLER_ID, CONF_REGISTER_TYPE, CONF_USE_WRITE_MULTIPLE, @@ -37,8 +38,11 @@ CONFIG_SCHEMA = cv.typed_schema( { cv.GenerateID(): cv.declare_id(ModbusBinaryOutput), cv.Required(CONF_ADDRESS): cv.positive_int, + cv.Optional(CONF_CUSTOM_PDU): cv.invalid( + "custom_pdu is not supported for outputs; use a write_lambda instead" + ), cv.Optional(CONF_CUSTOM_COMMAND): cv.invalid( - "custom_command is not supported for outputs" + "custom_command is not supported for outputs; use a write_lambda instead" ), cv.Optional(CONF_WRITE_LAMBDA): cv.returning_lambda, cv.Optional(CONF_USE_WRITE_MULTIPLE, default=False): cv.boolean, @@ -48,8 +52,11 @@ CONFIG_SCHEMA = cv.typed_schema( { cv.GenerateID(): cv.declare_id(ModbusFloatOutput), cv.Required(CONF_ADDRESS): cv.positive_int, + cv.Optional(CONF_CUSTOM_PDU): cv.invalid( + "custom_pdu is not supported for outputs; use a write_lambda instead" + ), cv.Optional(CONF_CUSTOM_COMMAND): cv.invalid( - "custom_command is not supported for outputs" + "custom_command is not supported for outputs; use a write_lambda instead" ), cv.Optional(CONF_VALUE_TYPE, default="U_WORD"): cv.enum( SENSOR_VALUE_TYPE diff --git a/esphome/components/modbus_controller/output/modbus_output.h b/esphome/components/modbus_controller/output/modbus_output.h index 17eb8e3a8f..e79c442aa4 100644 --- a/esphome/components/modbus_controller/output/modbus_output.h +++ b/esphome/components/modbus_controller/output/modbus_output.h @@ -17,7 +17,6 @@ class ModbusFloatOutput final : public output::FloatOutput, public Component, pu this->bitmask = 0xFFFFFFFF; this->register_count = register_count; this->sensor_value_type = value_type; - this->skip_updates = 0; this->set_address(this->start_address + offset); this->set_offset_from_start_address(0); } @@ -48,7 +47,6 @@ class ModbusBinaryOutput final : public output::BinaryOutput, public Component, this->set_address(start_address); this->bitmask = 0xFFFFFFFF; this->sensor_value_type = SensorValueType::BIT; - this->skip_updates = 0; this->register_count = 1; this->set_address(this->start_address + offset); this->set_offset_from_start_address(0); diff --git a/esphome/components/modbus_controller/select/__init__.py b/esphome/components/modbus_controller/select/__init__.py index 5127360770..1d77f9235d 100644 --- a/esphome/components/modbus_controller/select/__init__.py +++ b/esphome/components/modbus_controller/select/__init__.py @@ -4,7 +4,12 @@ from esphome.components.modbus.helpers import SENSOR_VALUE_TYPE, TYPE_REGISTER_M import esphome.config_validation as cv from esphome.const import CONF_ADDRESS, CONF_ID, CONF_LAMBDA, CONF_OPTIMISTIC -from .. import ModbusController, SensorItem, modbus_controller_ns +from .. import ( + ModbusController, + SensorItem, + modbus_controller_ns, + validate_skip_updates_deprecated, +) from ..const import ( CONF_FORCE_NEW_RANGE, CONF_MODBUS_CONTROLLER_ID, @@ -69,7 +74,7 @@ CONFIG_SCHEMA = cv.All( INTEGER_SENSOR_VALUE_TYPE ), cv.Optional(CONF_REGISTER_COUNT): cv.positive_int, - cv.Optional(CONF_SKIP_UPDATES, default=0): cv.positive_int, + cv.Optional(CONF_SKIP_UPDATES): validate_skip_updates_deprecated, cv.Optional(CONF_FORCE_NEW_RANGE, default=False): cv.boolean, cv.Required(CONF_OPTIONSMAP): ensure_option_map(), cv.Optional(CONF_USE_WRITE_MULTIPLE, default=False): cv.boolean, @@ -95,7 +100,6 @@ async def to_code(config): value_type, config[CONF_ADDRESS], reg_count, - config[CONF_SKIP_UPDATES], config[CONF_FORCE_NEW_RANGE], list(options_map.values()), ) diff --git a/esphome/components/modbus_controller/select/modbus_select.h b/esphome/components/modbus_controller/select/modbus_select.h index e1ae578ddf..41ebd4f658 100644 --- a/esphome/components/modbus_controller/select/modbus_select.h +++ b/esphome/components/modbus_controller/select/modbus_select.h @@ -11,8 +11,8 @@ namespace esphome::modbus_controller { class ModbusSelect final : public Component, public select::Select, public SensorItem { public: - ModbusSelect(SensorValueType sensor_value_type, uint16_t start_address, uint8_t register_count, uint16_t skip_updates, - bool force_new_range, std::vector mapping) { + ModbusSelect(SensorValueType sensor_value_type, uint16_t start_address, uint8_t register_count, bool force_new_range, + std::vector mapping) { this->register_type = modbus::EntityType::HOLDING; // not configurable this->sensor_value_type = sensor_value_type; this->set_address(start_address); @@ -20,7 +20,6 @@ class ModbusSelect final : public Component, public select::Select, public Senso this->bitmask = 0xFFFFFFFF; // not configurable this->register_count = register_count; this->response_bytes = 0; // not configurable - this->skip_updates = skip_updates; this->force_new_range = force_new_range; this->mapping_ = std::move(mapping); } diff --git a/esphome/components/modbus_controller/sensor/__init__.py b/esphome/components/modbus_controller/sensor/__init__.py index 5b72586c66..c3c9bd4718 100644 --- a/esphome/components/modbus_controller/sensor/__init__.py +++ b/esphome/components/modbus_controller/sensor/__init__.py @@ -8,6 +8,7 @@ from .. import ( ModbusItemBaseSchema, SensorItem, add_modbus_base_properties, + migrate_custom_command, modbus_calc_properties, modbus_controller_ns, validate_modbus_register, @@ -18,7 +19,6 @@ from ..const import ( CONF_MODBUS_CONTROLLER_ID, CONF_REGISTER_COUNT, CONF_REGISTER_TYPE, - CONF_SKIP_UPDATES, CONF_VALUE_TYPE, ) @@ -44,6 +44,8 @@ CONFIG_SCHEMA = cv.All( validate_modbus_register, ) +FINAL_VALIDATE_SCHEMA = migrate_custom_command + async def to_code(config): byte_offset, reg_count = modbus_calc_properties(config) @@ -56,7 +58,6 @@ async def to_code(config): config[CONF_BITMASK], value_type, reg_count, - config[CONF_SKIP_UPDATES], config[CONF_FORCE_NEW_RANGE], ) await cg.register_component(var, config) diff --git a/esphome/components/modbus_controller/sensor/modbus_sensor.h b/esphome/components/modbus_controller/sensor/modbus_sensor.h index 9d66b2afa7..68dc9e6fcc 100644 --- a/esphome/components/modbus_controller/sensor/modbus_sensor.h +++ b/esphome/components/modbus_controller/sensor/modbus_sensor.h @@ -11,14 +11,13 @@ namespace esphome::modbus_controller { class ModbusSensor final : public Component, public sensor::Sensor, public SensorItem { public: ModbusSensor(modbus::EntityType register_type, uint16_t start_address, uint8_t offset, uint32_t bitmask, - SensorValueType value_type, int register_count, uint16_t skip_updates, bool force_new_range) { + SensorValueType value_type, int register_count, bool force_new_range) { this->register_type = register_type; this->set_address(start_address); this->set_offset_from_start_address(offset); this->bitmask = bitmask; this->sensor_value_type = value_type; this->register_count = register_count; - this->skip_updates = skip_updates; this->force_new_range = force_new_range; } diff --git a/esphome/components/modbus_controller/switch/__init__.py b/esphome/components/modbus_controller/switch/__init__.py index a40c15ab92..35ad12087c 100644 --- a/esphome/components/modbus_controller/switch/__init__.py +++ b/esphome/components/modbus_controller/switch/__init__.py @@ -8,6 +8,7 @@ from .. import ( ModbusItemBaseSchema, SensorItem, add_modbus_base_properties, + migrate_custom_command, modbus_calc_properties, modbus_controller_ns, validate_modbus_register, @@ -17,7 +18,6 @@ from ..const import ( CONF_FORCE_NEW_RANGE, CONF_MODBUS_CONTROLLER_ID, CONF_REGISTER_TYPE, - CONF_SKIP_UPDATES, CONF_USE_WRITE_MULTIPLE, CONF_WRITE_LAMBDA, ) @@ -45,6 +45,8 @@ CONFIG_SCHEMA = cv.All( validate_modbus_register, ) +FINAL_VALIDATE_SCHEMA = migrate_custom_command + async def to_code(config): byte_offset, _ = modbus_calc_properties(config) @@ -54,7 +56,6 @@ async def to_code(config): config[CONF_ADDRESS], byte_offset, config[CONF_BITMASK], - config[CONF_SKIP_UPDATES], config[CONF_FORCE_NEW_RANGE], ) await cg.register_component(var, config) diff --git a/esphome/components/modbus_controller/switch/modbus_switch.h b/esphome/components/modbus_controller/switch/modbus_switch.h index e5b8cf5c21..c21a1939bc 100644 --- a/esphome/components/modbus_controller/switch/modbus_switch.h +++ b/esphome/components/modbus_controller/switch/modbus_switch.h @@ -11,13 +11,12 @@ namespace esphome::modbus_controller { class ModbusSwitch final : public Component, public switch_::Switch, public SensorItem { public: ModbusSwitch(modbus::EntityType register_type, uint16_t start_address, uint8_t offset, uint32_t bitmask, - uint16_t skip_updates, bool force_new_range) { + bool force_new_range) { this->register_type = register_type; this->set_address(start_address); this->set_offset_from_start_address(offset); this->bitmask = bitmask; this->sensor_value_type = SensorValueType::BIT; - this->skip_updates = skip_updates; this->register_count = 1; if (register_type == modbus::EntityType::HOLDING || register_type == modbus::EntityType::COIL) { this->set_address(this->start_address + offset); diff --git a/esphome/components/modbus_controller/text_sensor/__init__.py b/esphome/components/modbus_controller/text_sensor/__init__.py index 93ecd31168..e8447658e2 100644 --- a/esphome/components/modbus_controller/text_sensor/__init__.py +++ b/esphome/components/modbus_controller/text_sensor/__init__.py @@ -8,6 +8,7 @@ from .. import ( ModbusItemBaseSchema, SensorItem, add_modbus_base_properties, + migrate_custom_command, modbus_calc_properties, modbus_controller_ns, validate_modbus_register, @@ -19,7 +20,6 @@ from ..const import ( CONF_REGISTER_COUNT, CONF_REGISTER_TYPE, CONF_RESPONSE_SIZE, - CONF_SKIP_UPDATES, ) DEPENDENCIES = ["modbus_controller"] @@ -55,6 +55,8 @@ CONFIG_SCHEMA = cv.All( validate_modbus_register, ) +FINAL_VALIDATE_SCHEMA = migrate_custom_command + async def to_code(config): byte_offset, reg_count = modbus_calc_properties(config) @@ -70,7 +72,6 @@ async def to_code(config): reg_count, config[CONF_RESPONSE_SIZE], config[CONF_RAW_ENCODE], - config[CONF_SKIP_UPDATES], config[CONF_FORCE_NEW_RANGE], ) diff --git a/esphome/components/modbus_controller/text_sensor/modbus_textsensor.h b/esphome/components/modbus_controller/text_sensor/modbus_textsensor.h index 5bb16eb58a..9e8dce57e7 100644 --- a/esphome/components/modbus_controller/text_sensor/modbus_textsensor.h +++ b/esphome/components/modbus_controller/text_sensor/modbus_textsensor.h @@ -13,14 +13,13 @@ enum class RawEncoding { NONE = 0, HEXBYTES = 1, COMMA = 2, ANSI = 3 }; class ModbusTextSensor final : public Component, public text_sensor::TextSensor, public SensorItem { public: ModbusTextSensor(modbus::EntityType register_type, uint16_t start_address, uint8_t offset, uint8_t register_count, - uint16_t response_bytes, RawEncoding encode, uint16_t skip_updates, bool force_new_range) { + uint16_t response_bytes, RawEncoding encode, bool force_new_range) { this->register_type = register_type; this->set_address(start_address); this->set_offset_from_start_address(offset); this->response_bytes = response_bytes; this->register_count = register_count; this->encode_ = encode; - this->skip_updates = skip_updates; this->bitmask = 0xFFFFFFFF; this->sensor_value_type = SensorValueType::RAW; this->force_new_range = force_new_range; diff --git a/tests/component_tests/modbus_controller/test_custom_pdu.py b/tests/component_tests/modbus_controller/test_custom_pdu.py new file mode 100644 index 0000000000..a5d065c965 --- /dev/null +++ b/tests/component_tests/modbus_controller/test_custom_pdu.py @@ -0,0 +1,47 @@ +"""Schema-level config validation for custom_pdu and the deprecated custom_command alias. + +custom_command took a raw frame with a leading device address byte; custom_pdu takes the PDU only. +The old key is still accepted at the schema level and auto-migrated later in final validate (which a +bare-schema test can't reach), so these tests only cover what the schema itself enforces: the two keys +are mutually exclusive, and custom_pdu takes byte-sized values. +""" + +import pytest +from voluptuous import Invalid, MultipleInvalid + +from esphome.components.modbus_controller import ModbusItemBaseSchema +from esphome.components.modbus_controller.const import ( + CONF_CUSTOM_COMMAND, + CONF_CUSTOM_PDU, +) + + +def test_custom_command_accepted_at_schema_level() -> None: + """custom_command validates at the schema level; migration/rejection happens in final validate.""" + config = ModbusItemBaseSchema( + {CONF_CUSTOM_COMMAND: [0x01, 0x03, 0x00, 0x2A, 0x00, 0x01]} + ) + assert config[CONF_CUSTOM_COMMAND] == [0x01, 0x03, 0x00, 0x2A, 0x00, 0x01] + + +def test_custom_pdu_and_custom_command_mutually_exclusive() -> None: + """Only one custom source may be given; supplying both is a schema error.""" + with pytest.raises((Invalid, MultipleInvalid)): + ModbusItemBaseSchema( + { + CONF_CUSTOM_PDU: [0x03, 0x00, 0x2A, 0x00, 0x01], + CONF_CUSTOM_COMMAND: [0x01, 0x03, 0x00, 0x2A, 0x00, 0x01], + } + ) + + +def test_custom_pdu_accepted() -> None: + """The new key takes PDU bytes (function code + data, no address byte).""" + config = ModbusItemBaseSchema({CONF_CUSTOM_PDU: [0x03, 0x00, 0x2A, 0x00, 0x01]}) + assert config[CONF_CUSTOM_PDU] == [0x03, 0x00, 0x2A, 0x00, 0x01] + + +def test_custom_pdu_rejects_non_byte_values() -> None: + """PDU entries are bytes; a word-sized value is a sign the old raw format is being used.""" + with pytest.raises((Invalid, MultipleInvalid)): + ModbusItemBaseSchema({CONF_CUSTOM_PDU: [0x0103, 0x002A]}) diff --git a/tests/components/modbus_controller/common.yaml b/tests/components/modbus_controller/common.yaml index 67b022cdf5..9c35a2f868 100644 --- a/tests/components/modbus_controller/common.yaml +++ b/tests/components/modbus_controller/common.yaml @@ -108,6 +108,22 @@ select: return value; sensor: + # custom_pdu polls a ready-made PDU (function code + data - no device address byte, no CRC); covers + # the set_custom_pdu codegen path and the custom-range polling constructor. + - platform: modbus_controller + modbus_controller_id: modbus_controller1 + id: modbus_sensor_custom_pdu + name: Test Custom PDU Sensor + custom_pdu: [0x03, 0x00, 0x2A, 0x00, 0x01] + value_type: U_WORD + # Deprecated custom_command (leading byte 0x02 == modbus_controller1's address) drives the + # migrate_custom_command final-validate auto-migration path in CI. + - platform: modbus_controller + modbus_controller_id: modbus_controller1 + id: modbus_sensor_custom_command + name: Test Custom Command Sensor + custom_command: [0x02, 0x03, 0x00, 0x2B, 0x00, 0x01] + value_type: U_WORD - platform: modbus_controller modbus_controller_id: modbus_controller1 id: modbus_sensor1 diff --git a/tests/integration/fixtures/uart_mock_modbus_custom_command.yaml b/tests/integration/fixtures/uart_mock_modbus_custom_pdu.yaml similarity index 83% rename from tests/integration/fixtures/uart_mock_modbus_custom_command.yaml rename to tests/integration/fixtures/uart_mock_modbus_custom_pdu.yaml index 738e691110..188abf90f1 100644 --- a/tests/integration/fixtures/uart_mock_modbus_custom_command.yaml +++ b/tests/integration/fixtures/uart_mock_modbus_custom_pdu.yaml @@ -1,5 +1,5 @@ esphome: - name: uart-mock-modbus-custom-command + name: uart-mock-modbus-custom-pdu host: api: @@ -69,13 +69,14 @@ sensor: address: 0x01 register_type: holding value_type: U_WORD - # Custom command: a raw frame {device address, function code, address hi, address lo, - # count hi, count lo}; the CRC is appended by the hub. Reads holding register 0x0001, - # count 1; the lambda parses the response payload (the register value, big-endian). + # Custom PDU: read holding register 0x0001, count 1. The PDU is + # {function code, address hi, address lo, count hi, count lo}; the device + # address and CRC are added by the hub. The lambda parses the response payload + # (the register value, big-endian). - platform: modbus_controller modbus_controller_id: modbus_controller_1 name: "custom_read" - custom_command: [0x01, 0x03, 0x00, 0x01, 0x00, 0x01] + custom_pdu: [0x03, 0x00, 0x01, 0x00, 0x01] lambda: |- if (data.size() < 2) return {}; return (float) ((data[0] << 8) | data[1]); diff --git a/tests/integration/fixtures/uart_mock_modbus_grouping.yaml b/tests/integration/fixtures/uart_mock_modbus_grouping.yaml index a5394f1d05..d580f5c2e2 100644 --- a/tests/integration/fixtures/uart_mock_modbus_grouping.yaml +++ b/tests/integration/fixtures/uart_mock_modbus_grouping.yaml @@ -134,7 +134,8 @@ sensor: value_type: U_WORD modbus_controller_id: modbus_controller_ok - # F - contiguous registers where the second asks for a slower rate. + # F - contiguous registers that historically carried differing skip_updates rates and were split by + # the rate merge; with per-range rates gone they group like any contiguous pair. - platform: modbus_controller name: "rate_first" address: 0x150 @@ -146,7 +147,6 @@ sensor: address: 0x151 register_type: holding value_type: U_WORD - skip_updates: 5 modbus_controller_id: modbus_controller_ok # B - a wide value and one of its halves share a start address, with a contiguous sensor after them. diff --git a/tests/integration/fixtures/uart_mock_modbus_offline.yaml b/tests/integration/fixtures/uart_mock_modbus_offline.yaml index e4d2dfa294..c34890cd02 100644 --- a/tests/integration/fixtures/uart_mock_modbus_offline.yaml +++ b/tests/integration/fixtures/uart_mock_modbus_offline.yaml @@ -51,9 +51,8 @@ modbus_controller: modbus_id: virtual_modbus_client id: ctl max_cmd_retries: 1 - # offline_skip_updates and the sensor's skip_updates deliberately share a period: offline - # probing must follow the offline cadence alone, or phase combinations like this one can - # leave the device never probing again. + # offline_skip_updates: 1 -> once offline, the controller re-probes every second update cycle; + # the test silences the device to force it offline, then answers again and checks it recovers. offline_skip_updates: 1 update_interval: never on_offline: @@ -71,7 +70,6 @@ sensor: address: 0x03 register_type: holding value_type: U_WORD - skip_updates: 1 # Mirrors the controller's online state so the test can await the transitions. - platform: template name: link_state diff --git a/tests/integration/fixtures/uart_mock_modbus_shared_address.yaml b/tests/integration/fixtures/uart_mock_modbus_shared_address.yaml index 25574d0c42..109603f3b6 100644 --- a/tests/integration/fixtures/uart_mock_modbus_shared_address.yaml +++ b/tests/integration/fixtures/uart_mock_modbus_shared_address.yaml @@ -49,14 +49,10 @@ uart_mock: inject_rx: [0x01, 0x03, 0x02, 0x01, 0x41, 0x79, 0xE4] # 0x101 = 0x0141 = 321 - expect_tx: [0x01, 0x03, 0x01, 0x03, 0x00, 0x01, 0x75, 0xF6] # Read holding 0x103 count 1 inject_rx: [0x01, 0x03, 0x02, 0x01, 0xA5, 0x79, 0xAF] # 0x103 = 0x01A5 = 421 - # A widened shared-address range at 0x200 plus a sensor at 0x201 carrying its own skip_updates. - # The sensor must keep its own range so the polling rates stay independent; if it were folded into - # the widened range it would decode 0x201 from THAT response (2, not 777) and drag the range's - # rate down to its own. + # Two sensors sharing start address 0x200 (a word and a dword) widen the range to 2 registers + # and both decode from the single response. - expect_tx: [0x01, 0x03, 0x02, 0x00, 0x00, 0x02, 0xC5, 0xB3] # Read holding 0x200 count 2 inject_rx: [0x01, 0x03, 0x04, 0x01, 0x41, 0x00, 0x02, 0x2A, 0x1A] # 0x200=0x0141, 0x201=0x0002 - - expect_tx: [0x01, 0x03, 0x02, 0x01, 0x00, 0x01, 0xD4, 0x72] # Read holding 0x201 count 1 - inject_rx: [0x01, 0x03, 0x02, 0x03, 0x09, 0x78, 0xB2] # 0x201 = 0x0309 = 777 modbus: uart_id: virtual_uart_dev @@ -128,26 +124,17 @@ sensor: modbus_controller_id: modbus_controller_ok # Shared address 0x200: the dword widens the range the word opened (or vice versa) - platform: modbus_controller - name: "rate_word" + name: "widen_word" address: 0x200 register_type: holding value_type: U_WORD modbus_controller_id: modbus_controller_ok - platform: modbus_controller - name: "rate_dword" + name: "widen_dword" address: 0x200 register_type: holding value_type: U_DWORD modbus_controller_id: modbus_controller_ok - # Inside the widened range but with its own skip_updates: must NOT be folded in, or the two sensors - # above would silently drop to this sensor's polling rate - - platform: modbus_controller - name: "own_rate" - address: 0x201 - register_type: holding - value_type: U_WORD - skip_updates: 100 - modbus_controller_id: modbus_controller_ok button: - platform: template diff --git a/tests/integration/test_uart_mock_modbus.py b/tests/integration/test_uart_mock_modbus.py index d0b375dd25..09e841b4bb 100644 --- a/tests/integration/test_uart_mock_modbus.py +++ b/tests/integration/test_uart_mock_modbus.py @@ -675,9 +675,7 @@ async def test_uart_mock_modbus_shared_address( wide sensor's span keep polling separately, and that the sensor at the span's tail address does not anchor a re-use join on a mid-range predecessor (which would make it decode that sensor's bytes). - A sensor at 0x201 carrying skip_updates sits inside a widened shared-address range at 0x200 but - keeps its own range, so polling rates stay independent; folding it in would also make it decode - 0x201 out of the shared response (2) instead of its own poll (777). + A word and a dword sharing 0x200 widen that range to two registers and both decode from the one read. """ line_callback, error_log_lines, warning_log_lines = _make_modbus_line_callback() @@ -693,9 +691,8 @@ async def test_uart_mock_modbus_shared_address( "wide_qword": 100, "inside_wide": 321, "tail_of_wide": 421, - "rate_word": 321, - "rate_dword": pytest.approx(21037058), - "own_rate": 777, + "widen_word": 321, + "widen_dword": pytest.approx(21037058), } tracker = SensorTracker(list(expected_values.keys())) futures = tracker.expect_all(expected_values) @@ -710,18 +707,18 @@ async def test_uart_mock_modbus_shared_address( @pytest.mark.asyncio -async def test_uart_mock_modbus_custom_command( +async def test_uart_mock_modbus_custom_pdu( yaml_config: str, run_compiled: RunCompiledFunction, api_client_connected: APIClientConnectedFactory, ) -> None: - """Test a custom_command sensor polling a register served by the mock server. + """Test a custom_pdu sensor reading a register served by the mock server. - The custom_command is a raw frame (device address + PDU); the hub appends the CRC and - routes the response back to the polling command, whose sensor lambda parses the payload. - Guards the custom polling wiring: the command must reference the sensor's custom_data and - decode the real function code, or nothing is ever transmitted. A plain read on the same - register anchors the bus. + The custom_pdu is a raw read-holding PDU (function code + address + count); the + controller prepends its own device address and appends the CRC, sends it, and the + sensor's lambda parses the response payload. Confirms the custom PDU path decodes + the function code and routes the response to the sensor (the gap that hid the + step-2 raw-vs-PDU bug). A plain read on the same register anchors the bus. """ line_callback, error_log_lines, warning_log_lines = _make_modbus_line_callback() @@ -754,9 +751,8 @@ async def test_uart_mock_modbus_offline( publishes. This pins the pooled non-response counter, can_send() gating, the offline retry cadence, and recovery - none of which the responding-path tests touch. - The fixture gives offline_skip_updates and the sensor's skip_updates the same period - on purpose: offline probing must follow the offline cadence alone, since requiring - both cadences to coincide leaves phase combinations where no probe ever goes out. + Offline probing follows the offline cadence alone; regular every-update polling + resumes once the device answers again. """ tracker = SensorTracker(["link_state", "reg"])