Merge branch 'esp8266-native-converter-hardening' into esp8266-native-library-converter

This commit is contained in:
J. Nick Koston
2026-08-23 13:57:07 -05:00
13 changed files with 329 additions and 146 deletions
+18 -41
View File
@@ -21,9 +21,8 @@ import subprocess
from esphome.core import EsphomeError
from esphome.helpers import write_file
# Everything idedata generation may raise after a successful link. Broad on
# purpose, and shared by every consumer: idedata is a bonus artifact, so
# these must be caught and warned about, never allowed to fail the build.
# Everything idedata generation may raise after a successful link; idedata
# is a bonus artifact, so consumers warn instead of failing the build
IDEDATA_BEST_EFFORT_ERRORS = (
EsphomeError,
LookupError,
@@ -44,12 +43,8 @@ _ESPHOME_SRC_MARKER = "/src/esphome/"
def _is_esphome_src(file: str) -> bool:
"""Whether ``file`` is an ESPHome C++ translation unit.
``compile_commands.json`` ``file`` paths use the OS-native separator, so on
Windows they contain backslashes; normalize to ``/`` before testing the
marker, otherwise no source matches and the build-include union is empty.
"""
"""Whether ``file`` is an ESPHome C++ translation unit; normalized to
``/`` first since Windows compile DBs use backslashes."""
return _ESPHOME_SRC_MARKER in file.replace("\\", "/") and file.endswith(
_CXX_SUFFIXES
)
@@ -120,11 +115,8 @@ def _expand_response_files(tokens: list[str], directory: Path) -> list[str]:
def _pick_entry(entries: list[dict]) -> dict:
"""Pick a representative ESPHome C++ translation unit.
All ESPHome sources share the same component flags/defines, so any one of
them yields the cxx_path / cxx_flags / defines we need.
"""
"""Pick a representative ESPHome C++ TU; all share the same component
flags/defines."""
for entry in entries:
if _is_esphome_src(entry["file"]):
return entry
@@ -151,11 +143,8 @@ def parse_entry(
tokens = _expand_response_files(_split_command(entry["command"]), directory)
def _include(raw: str) -> str:
# Include paths in compile_commands are interpreted relative to the
# entry's ``directory`` (e.g. build-local ``-Iconfig``); resolve them
# so the cached idedata is usable regardless of the consumer's cwd.
# Emit forward slashes (``normpath`` yields ``\`` on Windows) so the
# paths match the absolute, already-forward-slash entries in the JSON.
# Resolve against the entry's ``directory`` so cached idedata works
# from any cwd; emit forward slashes to match the JSON's own entries
raw = raw.strip()
if raw and not Path(raw).is_absolute():
raw = os.path.normpath(directory / raw)
@@ -165,14 +154,11 @@ def parse_entry(
if launcher is not None and tokens[:1] == [launcher]:
tokens = tokens[1:]
if not tokens:
# _split_command("") is [] by design, and a command that is only
# the launcher strips to nothing; fail like _pick_entry does
# instead of an IndexError traceback
# An empty command, or one that was only the launcher; fail by name
raise ValueError(f"empty compile command for {entry.get('file')}")
if _is_launcher(tokens[0]) and len(tokens) > 1 and not tokens[1].startswith("-"):
# A stale compile DB built with a launcher the current run no longer
# configures: the real compiler is the next token. Warn: the DB is
# stale and worth regenerating.
# Stale DB built with a launcher this run no longer configures; the
# real compiler is the next token
_LOGGER.warning("Stripping unconfigured launcher %s", tokens[0])
tokens = tokens[1:]
# token0 is the compiler path; the rest of the command already uses forward
@@ -316,11 +302,8 @@ def load_or_build_idedata(
def reject_launcher_compiler(cxx_path: str) -> None:
"""Reject a compile DB that names a launcher (ccache) as the compiler.
Reject before the toolchain probe, which would fail opaquely on a
launcher; the unusable compile DB must never be cached or consumed.
"""
"""Reject a compile DB naming a launcher (ccache) as the compiler; it
must never be probed, cached, or consumed."""
if _is_launcher(cxx_path):
raise EsphomeError(
f"compile_commands.json names the launcher {cxx_path} as the "
@@ -353,13 +336,9 @@ def idedata_from_build(compile_commands: Path, launcher: str | None = None) -> d
)
def _shape(entry: dict) -> str:
# The command minus its TU-specific paths: entries sharing a shape
# carry identical include sets (one ninja rule), so tokenize once
# per shape instead of once per TU. Response-file commands never
# dedupe: per-object .rsp names strip to one shape while the files
# may hold different include sets.
# Keyed on directory too: relative -I paths resolve against it, so
# identical commands in different dirs mean different include sets
# directory + command minus TU-specific paths: same shape means the
# same include set, so tokenize once per shape. Response-file
# commands never dedupe (the .rsp contents differ per object)
command = entry["command"]
directory = entry.get("directory", "")
if "@" in command:
@@ -382,10 +361,8 @@ def idedata_from_build(compile_commands: Path, launcher: str | None = None) -> d
build_includes.setdefault(inc, None)
if not has_esphome_tu:
# _pick_entry fell back to an arbitrary C++ entry: idedata built
# from it breaks clang-tidy/IDE consumers, and a one-time warning
# would be cached into permanence. The best-effort call sites
# downgrade this to a build warning.
# An arbitrary fallback TU breaks clang-tidy/IDE consumers, and a
# warning would be cached into permanence; call sites downgrade this
raise EsphomeError(
f"No ESPHome translation unit found in {compile_commands}; "
"refusing to cache unusable idedata"
+76 -3
View File
@@ -1,17 +1,23 @@
from __future__ import annotations
import logging
from typing import Any, Literal
from typing import Any, Literal, NamedTuple
from esphome import pins
import esphome.codegen as cg
from esphome.components import uart
import esphome.config_validation as cv
from esphome.const import CONF_ADDRESS, CONF_DISABLE_CRC, CONF_FLOW_CONTROL_PIN, CONF_ID
from esphome.const import (
CONF_ADDRESS,
CONF_CONTINUOUS,
CONF_DISABLE_CRC,
CONF_FLOW_CONTROL_PIN,
CONF_ID,
)
from esphome.cpp_generator import MockObj
from esphome.cpp_helpers import gpio_pin_expression
import esphome.final_validate as fv
from esphome.types import ConfigType
from esphome.types import ConfigType, TemplateArgsType
_LOGGER = logging.getLogger(__name__)
@@ -48,6 +54,73 @@ CONF_TURNAROUND_TIME = "turnaround_time"
MODBUS_ROLES = ["client", "server"]
class _CommandOption(NamedTuple):
"""One per-command option forwarded to the hub (modbus::CommandOptions)."""
conf_key: str
field: str # the C++ field, and so the set_<field>() setter name
validator: Any # the static (non-templatable) validator for the key
cpp_type: Any # the C++ type the value is generated as
default: Any
# Per-direction command options. Single-sourcing the schema and the setter generation here keeps
# them from drifting; the C++ side must add the matching field per the rules documented on
# CommandOptions (modbus.h).
_COMMAND_OPTIONS: dict[str, list[_CommandOption]] = {
"read": [_CommandOption(CONF_CONTINUOUS, "continuous", cv.boolean, bool, False)],
"write": [],
}
def _command_options(direction: str) -> list[_CommandOption]:
try:
return _COMMAND_OPTIONS[direction]
except KeyError:
raise ValueError(f"unknown command-options direction {direction!r}") from None
def command_options_schema(
*, direction: Literal["read", "write"], templatable: bool = False
) -> dict[cv.Optional, Any]:
"""Schema fragment for the per-command options a component forwards to the hub
(modbus::CommandOptions). Extend this into any schema that queues commands. Keys are
direction-specific so a schema never offers an option the hub would strip (e.g.
continuous on a write); the write side has no options yet. For actions (templatable=True the
keys also accept lambdas), register the values with register_templatable_command_options().
"""
return {
cv.Optional(option.conf_key, default=option.default): (
cv.templatable(option.validator) if templatable else option.validator
)
for option in _command_options(direction)
}
async def register_templatable_command_options(
var: MockObj, config: ConfigType, args: TemplateArgsType, direction: str
) -> None:
"""Generate the set_<option>() calls for the given direction's command options present in config.
Pass the same direction the action's command_options_schema() used, so the keys generated match
the ones the schema offered - a write action never emits a read option's setter. Options the
schema did not add are simply absent. The consumer's C++ class declares a matching
TEMPLATABLE_VALUE per option (e.g. TEMPLATABLE_VALUE(bool, continuous)).
"""
for option in _command_options(direction):
if option.conf_key not in config:
continue
value = config[option.conf_key]
# Skip codegen when the value is its C++ zero (TemplatableFn::value() returns T{} when
# unset): behaviourally identical, and saves a thunk plus a setup() call per action.
if cg.is_template(value) or value != type(value)():
cg.add(
getattr(var, f"set_{option.field}")(
await cg.templatable(value, args, option.cpp_type)
)
)
CONFIG_SCHEMA = cv.typed_schema(
{
"client": cv.Schema(
+10 -13
View File
@@ -146,7 +146,7 @@ bool ModbusClientHub::tx_buffer_empty() {
// other states are mid-transaction or owed bookkeeping, not queued sends - and a READY continuous
// poll does not count either, since it ranks below every one-shot, so a new send goes out first.
for (const auto &cmd : this->tx_buffer_) {
if (cmd.state == FrameState::READY && !cmd.continuous)
if (cmd.state == FrameState::READY && !cmd.options.continuous)
return false;
}
return true;
@@ -946,7 +946,7 @@ bool ModbusDeviceCommand::notify_retired() {
bool ModbusDeviceCommand::response(std::span<const uint8_t> response_pdu) {
this->state = this->state == FrameState::WAITING_RETIRED ? FrameState::RETIRED : FrameState::RECEIVED_RESPONSE;
// A continuous poll is never consumed by its own response; a one-shot consumes one request here.
if (!this->continuous)
if (!this->options.continuous)
this->decrement_pending();
if (this->device == nullptr)
return false;
@@ -1070,15 +1070,12 @@ bool ModbusClientHub::queue_pdu(uint8_t address, std::span<const uint8_t> pdu, M
return false;
}
// Normalize the caller's options in place (the param is a by-value copy) so everything stored or
// merged below carries effective options, never the raw request.
// continuous is ignored for every mutating code (re-writing a value forever is never intended).
const bool mutates = priority == CommandPriority::WRITE;
bool continuous = false;
if (options.continuous) {
if (mutates) {
ESP_LOGV(TAG, "continuous is ignored for a mutating function (0x%X, address %" PRIu8 ")", pdu[0], address);
} else {
continuous = true;
}
if (options.continuous && priority == CommandPriority::WRITE) {
ESP_LOGW(TAG, "continuous is ignored for a mutating function (0x%X, address %" PRIu8 ")", pdu[0], address);
options.continuous = false;
}
// A duplicate of a live entry with the same owner is not queued twice; it resolves against that
@@ -1104,10 +1101,10 @@ bool ModbusClientHub::queue_pdu(uint8_t address, std::span<const uint8_t> pdu, M
}
return false; // dropped: no entry, no callbacks - the refusal is the return value
}
if (continuous) {
if (options.continuous) {
item.make_continuous(true);
ESP_LOGV(TAG, "Frame already active for %" PRIu8 ", now polled continuously", address);
} else if (item.continuous) {
} else if (item.options.continuous) {
// A one-shot duplicate downgrades the poll to a one-shot: it runs one more cycle to serve this
// request, then stops (mirrors continuous incoming converting a one-shot the other way).
item.make_continuous(false);
@@ -1140,7 +1137,7 @@ bool ModbusClientHub::queue_pdu(uint8_t address, std::span<const uint8_t> pdu, M
#endif
ESP_LOGV(TAG, "Adding frame to tx queue: %" PRIu8 ":%s", address,
format_hex_pretty_to(hex_buf, pdu.data(), pdu.size()));
this->tx_buffer_.emplace_back(device, address, pdu, continuous, this->next_seq_++);
this->tx_buffer_.emplace_back(device, address, pdu, options, this->next_seq_++);
return true;
}
+31 -17
View File
@@ -118,6 +118,14 @@ enum class FrameState : uint8_t {
};
// Per-command send options. Append-only; pass via designated initializers ({.continuous = true}).
// The queue entry stores this struct whole, so a new field arrives at the queue with no plumbing -
// but it arrives inert. Every new field must define three rules before it does anything:
// 1. normalization in queue_pdu() (is it valid for this function code? e.g. continuous is
// stripped for mutating codes),
// 2. a merge rule for when a duplicate send absorbs into a live entry (continuous
// upgrades/downgrades via make_continuous(); a new field needs its own answer),
// 3. teardown: retire() resets the whole struct; silent_retire() leaves it, relying on the sweep
// to erase the entry.
struct CommandOptions {
// A continuous poll lives in the queue until cancelled or failed; ignored for mutating codes.
bool continuous{false};
@@ -126,26 +134,29 @@ struct CommandOptions {
struct ModbusDeviceCommand {
ModbusClientDevice *device;
ModbusFrame frame;
FrameState state{FrameState::READY};
// A continuous poll is a subscription: pending fixed at 1, removed only by cancellation or failure.
bool continuous{false};
// Accepted requests this entry stands for, capped at max_pending(); drains one terminal each.
uint8_t pending{1};
// Place-in-line stamp (hub's free-running counter); selection takes the oldest for round-robin
// fairness within a class. Meant to wrap.
// fairness within a class. Meant to wrap. Declared ahead of the byte fields so the tail packs
// densely and a growing CommandOptions eats trailing padding before enlarging the struct.
uint16_t seq{0};
FrameState state{FrameState::READY};
// Accepted requests this entry stands for, capped at max_pending(); drains one terminal each.
// A continuous poll is a subscription: pending fixed at 1, removed only by cancellation or failure.
uint8_t pending{1};
// The entry's LIVE effective options, not a record of the caller's request: queue_pdu() normalizes
// before storing, duplicate absorption mutates continuous via make_continuous(), and retire() resets
// the struct (silent_retire() leaves it, relying on the sweep to erase the entry). See the
// CommandOptions comment for the rules a new field must define.
CommandOptions options;
// Build a command from a PDU span (caller bounds it to MAX_PDU_SIZE); fully initialized here.
// Build a command from a PDU span (caller bounds it to MAX_PDU_SIZE) and pre-normalized options;
// fully initialized here.
ModbusDeviceCommand(ModbusClientDevice *device, uint8_t address, std::span<const uint8_t> pdu,
bool continuous = false, uint16_t seq = 0)
: device(device),
frame(address, pdu.data(), static_cast<uint16_t>(pdu.size())),
continuous(continuous),
seq(seq) {}
CommandOptions options = {}, uint16_t seq = 0)
: device(device), frame(address, pdu.data(), static_cast<uint16_t>(pdu.size())), seq(seq), options(options) {}
// Transmit ordering class, derived (never stored): a continuous poll ranks below every one-shot.
CommandPriority priority() const {
return this->continuous ? CommandPriority::CONTINUOUS : classify(this->frame.pdu()[0]);
return this->options.continuous ? CommandPriority::CONTINUOUS : classify(this->frame.pdu()[0]);
}
// Wire-derived class: mutating codes rank WRITE; exception-flagged codes are excluded.
static CommandPriority classify(uint8_t function_code) {
@@ -161,7 +172,7 @@ struct ModbusDeviceCommand {
uint8_t max_pending() const {
const uint8_t fc = this->frame.pdu()[0];
const bool requeueable = !helpers::is_function_code_exception(fc) && helpers::is_function_code_read_only(fc);
return (requeueable && !this->continuous) ? 2 : 1;
return (requeueable && !this->options.continuous) ? 2 : 1;
}
// Device-scoped clear: detach with no callback (device-less, pending 0). An entry still waiting for
// a response keeps its state as a reply-ignoring shell that resolves silently; any other goes RETIRED.
@@ -196,11 +207,11 @@ struct ModbusDeviceCommand {
// retroactively inflating that no-op.
void make_continuous(bool continuous) {
if (continuous) {
this->continuous = true;
this->options.continuous = true;
this->pending = 1;
} else {
this->increment_pending();
this->continuous = false;
this->options.continuous = false;
}
}
// Address-scoped clear: keep pending and device so the sweep delivers one on_not_sent() per un-run
@@ -218,7 +229,7 @@ struct ModbusDeviceCommand {
} else if (!this->waiting_state()) { // an already-retired shell stays put; off the wire -> RETIRED
this->state = FrameState::RETIRED;
}
this->continuous = false;
this->options = {}; // reset every option so a future field is torn down without editing here
}
// True while the entry is still waiting for a response; the erase pass exempts these even at pending 0.
@@ -253,6 +264,9 @@ struct ModbusDeviceCommand {
bool notify_retired();
/// True if this command carries the same wire frame (address + PDU) as the given one.
/// Cancellation matches the exact frame, not the action instance: a continuous poll whose
/// start_address (or other field) is templated produces one poll per distinct frame, and a later
/// cancel built from different argument values will not reach the polls it does not byte-match.
bool same_frame(uint8_t address, std::span<const uint8_t> pdu) const {
const auto own_pdu = this->frame.pdu();
return own_pdu.size() == pdu.size() && this->frame.address() == address &&
+57 -13
View File
@@ -7,6 +7,7 @@ from esphome.components import modbus
import esphome.config_validation as cv
from esphome.const import (
CONF_ADDRESS,
CONF_CONTINUOUS,
CONF_COUNT,
CONF_ID,
CONF_ON_ERROR,
@@ -156,16 +157,45 @@ _ACTION_BASE_SCHEMA = cv.Schema(
}
)
MODBUS_CLIENT_SEND_SCHEMA = _ACTION_BASE_SCHEMA.extend(
{
cv.Required(CONF_PDU): cv.templatable(
cv.All(
cv.ensure_list(cv.hex_uint8_t),
cv.Length(min=1, max=modbus.MAX_PDU_SIZE),
)
),
cv.Optional(CONF_ON_RESPONSE): _handler_schema(),
}
# The write codes recognised by modbus::helpers::is_function_code_write() - keep in sync. 0x17
# (read/write multiple) is included: it mutates, so the hub treats it as a write despite its read half.
_WRITE_FUNCTION_CODES = frozenset({0x05, 0x06, 0x0F, 0x10, 0x16, 0x17})
def _no_continuous_on_write(config: ConfigType) -> ConfigType:
"""Reject `continuous: true` on a static write PDU: continuous polling only applies to reads.
Only the fully-static case is decidable here; the hub strips the flag from mutating PDUs at
runtime, so a templated pdu or continuous falls through to that backstop."""
pdu = config[CONF_PDU]
if (
isinstance(pdu, list)
and config.get(CONF_CONTINUOUS) is True
# Masking the exception bit (0x90 -> 0x10) makes this check stricter than the runtime hub,
# whose classify() treats an exception-flagged code as a read and leaves continuous in place.
and pdu[0] & 0x7F in _WRITE_FUNCTION_CODES
):
raise cv.Invalid(
f"'{CONF_CONTINUOUS}: true' does not apply to a write PDU (function code "
f"0x{pdu[0]:02X}); continuous polling only applies to reads",
path=[CONF_CONTINUOUS],
)
return config
MODBUS_CLIENT_SEND_SCHEMA = cv.All(
_ACTION_BASE_SCHEMA.extend(
{
cv.Required(CONF_PDU): cv.templatable(
cv.All(
cv.ensure_list(cv.hex_uint8_t),
cv.Length(min=1, max=modbus.MAX_PDU_SIZE),
)
),
**modbus.command_options_schema(direction="read", templatable=True),
cv.Optional(CONF_ON_RESPONSE): _handler_schema(),
}
),
_no_continuous_on_write,
)
@@ -174,6 +204,7 @@ async def register_client_action(
config: ConfigType,
args: TemplateArgsType,
response_args: TemplateArgsType,
command_direction: str = "read",
) -> cg.MockObj:
"""Wire the shared action plumbing: hub parent, templated device address, outcome triggers.
@@ -235,6 +266,12 @@ async def register_client_action(
await automation.build_automation(
var.get_not_sent_trigger(), [(_PDU_SPAN, "request")], not_sent_conf
)
# Wire any command options the action's schema opted into (e.g. continuous on reads). Pass the
# matching direction so a write action never generates a read option's setter; the write side
# has no options yet, so this is a no-op there.
await modbus.register_templatable_command_options(
var, config, args, command_direction
)
return var
@@ -318,6 +355,7 @@ def _read_schema(max_count: int) -> cv.All:
cv.Optional(CONF_COUNT, default=1): cv.templatable(
cv.int_range(min=1, max=max_count)
),
**modbus.command_options_schema(direction="read", templatable=True),
}
),
_no_address_overflow(CONF_COUNT),
@@ -379,7 +417,9 @@ async def read_input_registers_to_code(config, action_id, template_arg, args):
async def _write_single_to_code(config, action_id, template_arg, args, value_type):
var = cg.new_Pvariable(action_id, template_arg)
cg.add(var.set_value(await cg.templatable(config[CONF_VALUE], args, value_type)))
return await register_client_action(var, config, args, [])
return await register_client_action(
var, config, args, [], command_direction="write"
)
@automation.register_action(
@@ -458,7 +498,9 @@ async def write_multiple_registers_to_code(config, action_id, template_arg, args
arr_id = ID(f"{action_id}_values", is_declaration=True, type=cg.uint16)
arr = cg.static_const_array(arr_id, cg.ArrayInitializer(*values))
cg.add(var.set_values_static(arr, len(values)))
return await register_client_action(var, config, args, [])
return await register_client_action(
var, config, args, [], command_direction="write"
)
@automation.register_action(
@@ -482,7 +524,9 @@ async def write_multiple_coils_to_code(config, action_id, template_arg, args):
arr_id = ID(f"{action_id}_values", is_declaration=True, type=cg.uint8)
arr = cg.static_const_array(arr_id, cg.ArrayInitializer(*packed))
cg.add(var.set_values_static(arr, len(values)))
return await register_client_action(var, config, args, [])
return await register_client_action(
var, config, args, [], command_direction="write"
)
# Read/write multiple registers (FC 0x17) writes one register block and reads another in a single
@@ -68,8 +68,8 @@ template<typename... Ts> class ClientActionBase : public Action<Ts...>, public m
/// resolves through on_sent() alone), so resolve refusals here via on_not_sent.
/// Takes a span, not a PduBuffer: the builders return right-sized buffers (a read PDU is 5 bytes), and
/// a PduBuffer parameter would widen each one to the 253-byte maximum just to cross the call.
void send_or_resolve_(std::span<const uint8_t> pdu) {
if (!this->queue_pdu(pdu))
void send_or_resolve_(std::span<const uint8_t> pdu, modbus::CommandOptions options = {}) {
if (!this->queue_pdu(pdu, options))
this->on_not_sent(pdu);
}
@@ -80,6 +80,26 @@ template<typename... Ts> class ClientActionBase : public Action<Ts...>, public m
retry_func_t retry_func_{nullptr};
};
/// The read-side per-command options (modbus::CommandOptions), declared once for every action that
/// sends a read. Each option is templatable, so it cannot be built in Python the way modbus_controller
/// builds its static struct; declaring the values here instead of per action means a new read option
/// costs one TEMPLATABLE_VALUE plus one field below, and every read action picks it up.
/// The read/write split mirrors _COMMAND_OPTIONS in the modbus component's Python
/// (command_options_schema(direction="read") adds exactly these keys). When a write-side option
/// arrives it gets a WriteCommandOptions twin, so write actions never carry read-only members.
template<typename... Ts> class ReadCommandOptions {
public:
// Poll: re-queue after each success until downgraded (replay with false) or failed. The hub strips
// it for mutating function codes at the door (see modbus::CommandOptions).
TEMPLATABLE_VALUE(bool, continuous)
protected:
/// The options for this send, with every templatable value resolved against the action's arguments.
modbus::CommandOptions command_options_(const Ts &...x) const {
return {.continuous = this->continuous_.value(x...)};
}
};
/// modbus_client.send: fire a raw PDU (function code + data; the hub adds address and CRC). The reply is
/// delivered raw - on_response(request, response) - deliberately bypassing the typed dispatch, so
/// non-standard/custom transactions pass through untouched.
@@ -87,7 +107,8 @@ template<typename... Ts> class ClientActionBase : public Action<Ts...>, public m
/// modbus::helpers::create_*_pdu() builders and return it directly (smaller builder results convert).
/// A PduBuffer drops bytes past modbus::MAX_PDU_SIZE without reporting it (the hub's oversize check
/// cannot fire - that limit is the capacity), so an over-long lambda-built PDU is silently truncated.
template<typename... Ts> class ModbusClientSendAction : public ClientActionBase<Ts...> {
template<typename... Ts>
class ModbusClientSendAction : public ClientActionBase<Ts...>, public ReadCommandOptions<Ts...> {
public:
TEMPLATABLE_VALUE(modbus::helpers::PduBuffer, pdu)
@@ -95,7 +116,7 @@ template<typename... Ts> class ModbusClientSendAction : public ClientActionBase<
return &this->response_trigger_;
}
void play(const Ts &...x) override { this->send_or_resolve_(this->pdu_.value(x...)); }
void play(const Ts &...x) override { this->send_or_resolve_(this->pdu_.value(x...), this->command_options_(x...)); }
void on_response(std::span<const uint8_t> request_pdu, std::span<const uint8_t> response_pdu) override {
this->response_trigger_.trigger(request_pdu, response_pdu);
@@ -140,7 +161,8 @@ template<typename... Ts> class TypedClientActionBase : public ClientActionBase<T
/// modbus_client.read_holding_registers / read_input_registers: on_response delivers the registers in
/// host byte order as `values` (only valid for the duration of the trigger).
template<typename... Ts> class ReadRegistersAction : public TypedClientActionBase<Ts...> {
template<typename... Ts>
class ReadRegistersAction : public TypedClientActionBase<Ts...>, public ReadCommandOptions<Ts...> {
public:
explicit ReadRegistersAction(bool holding) : holding_(holding) {}
TEMPLATABLE_VALUE(uint16_t, start_address)
@@ -152,7 +174,8 @@ template<typename... Ts> class ReadRegistersAction : public TypedClientActionBas
const auto function_code =
this->holding_ ? modbus::FunctionCode::READ_HOLDING_REGISTERS : modbus::FunctionCode::READ_INPUT_REGISTERS;
this->send_or_resolve_(
modbus::helpers::create_read_pdu(function_code, this->start_address_.value(x...), this->count_.value(x...)));
modbus::helpers::create_read_pdu(function_code, this->start_address_.value(x...), this->count_.value(x...)),
this->command_options_(x...));
}
void on_read_registers(modbus::EntityType entity_type, uint16_t start_address, std::span<const uint16_t> registers,
modbus::ResponseStatus status) override {
@@ -167,7 +190,7 @@ template<typename... Ts> class ReadRegistersAction : public TypedClientActionBas
/// modbus_client.read_coils / read_discrete_inputs: on_response delivers the bits as a PackedBits view
/// (bit 0 = the bit at start_address; only valid for the duration of the trigger).
template<typename... Ts> class ReadBitsAction : public TypedClientActionBase<Ts...> {
template<typename... Ts> class ReadBitsAction : public TypedClientActionBase<Ts...>, public ReadCommandOptions<Ts...> {
public:
explicit ReadBitsAction(bool coils) : coils_(coils) {}
TEMPLATABLE_VALUE(uint16_t, start_address)
@@ -179,7 +202,8 @@ template<typename... Ts> class ReadBitsAction : public TypedClientActionBase<Ts.
const auto function_code =
this->coils_ ? modbus::FunctionCode::READ_COILS : modbus::FunctionCode::READ_DISCRETE_INPUTS;
this->send_or_resolve_(
modbus::helpers::create_read_pdu(function_code, this->start_address_.value(x...), this->count_.value(x...)));
modbus::helpers::create_read_pdu(function_code, this->start_address_.value(x...), this->count_.value(x...)),
this->command_options_(x...));
}
void on_read_bits(modbus::EntityType entity_type, uint16_t start_address, modbus::PackedBits bits,
modbus::ResponseStatus status) override {
+1 -3
View File
@@ -133,9 +133,7 @@ def _print_summary(size_json: Path, partitions_csv: Path | None) -> None:
_LOGGER.warning("Skipping Flash summary: %s", e)
return
if app_size <= 0:
# A "from 0 bytes" denominator is meaningless to a reader. The skip
# costs CI's memory-impact extraction its Flash match, which is the
# loud outcome a broken partition table deserves.
# Skipping also fails CI's Flash extraction, the right outcome here
_LOGGER.warning(
"Skipping Flash summary: app partition size is %s in %s",
app_size,
+1 -1
View File
@@ -12,7 +12,7 @@ pyserial==3.5
platformio==6.1.19
esptool==5.3.1
click==8.3.3
aioesphomeapi==45.13.1
aioesphomeapi==46.0.0
aiohappyeyeballs==2.7.1 # Happy Eyeballs for requests downloads; already pulled in by aioesphomeapi
zeroconf==0.150.0
puremagic==2.2.0
@@ -16,7 +16,13 @@ from esphome.components.modbus_client import (
CONFIG_SCHEMA,
MODBUS_CLIENT_SEND_SCHEMA,
)
from esphome.const import CONF_ADDRESS, CONF_ID, CONF_ON_ERROR, CONF_ON_RESPONSE
from esphome.const import (
CONF_ADDRESS,
CONF_CONTINUOUS,
CONF_ID,
CONF_ON_ERROR,
CONF_ON_RESPONSE,
)
from esphome.core import Lambda
from esphome.types import ConfigType
@@ -118,6 +124,29 @@ def test_on_no_response_retry_lambda_accepted() -> None:
)
def test_continuous_on_write_pdu_rejected() -> None:
"""A literal write-code PDU with continuous: true is rejected at config time (reads only)."""
with pytest.raises(cv.Invalid, match="does not apply to a write PDU"):
MODBUS_CLIENT_SEND_SCHEMA(
{
CONF_ADDRESS: 0x01,
CONF_PDU: [0x06, 0x00, 0x01, 0x00, 0x0A],
CONF_CONTINUOUS: True,
}
)
def test_continuous_on_read_pdu_accepted() -> None:
"""A literal read-code PDU with continuous: true is fine - continuous polling applies to reads."""
MODBUS_CLIENT_SEND_SCHEMA(
{
CONF_ADDRESS: 0x01,
CONF_PDU: [0x03, 0x00, 0x10, 0x00, 0x01],
CONF_CONTINUOUS: True,
}
)
# The standalone component block. The compile fixtures cover the accepted shapes end to end; these pin
# the parts a fixture cannot express - a rejection, and a module flag whose absence breaks other
# components rather than this one.
@@ -322,14 +322,14 @@ TEST(ModbusClientHubPriority, ContinuousReadRequeuesOnSuccessOnly) {
device.read_holding_registers(0x100, 2, {.continuous = true});
ASSERT_EQ(hub.queued_frames(), 1u);
EXPECT_TRUE(hub.queued(0).continuous);
EXPECT_TRUE(hub.queued(0).options.continuous);
hub.force_send_next();
// A matching successful response cycles the continuous entry back to READY.
const uint8_t ok_response[] = {0x03, 0x04, 0x00, 0x2A, 0x01, 0x00};
hub.receive_frame_for_test(0x02, ok_response);
ASSERT_EQ(hub.queued_frames(), 1u);
EXPECT_TRUE(hub.queued(0).continuous);
EXPECT_TRUE(hub.queued(0).options.continuous);
// An exception response ends the poll.
hub.force_send_next();
@@ -346,13 +346,13 @@ TEST(ModbusClientHubPriority, RetriedContinuousReadStaysContinuous) {
device.read_holding_registers(0x100, 2, {.continuous = true});
ASSERT_EQ(hub.queued_frames(), 1u);
ASSERT_TRUE(hub.queued(0).continuous);
ASSERT_TRUE(hub.queued(0).options.continuous);
hub.force_send_next();
hub.timeout_waiting(); // no response -> device requests retry
ASSERT_EQ(hub.queued_frames(), 1u);
EXPECT_TRUE(hub.queued(0).continuous); // the retried poll stays continuous
EXPECT_TRUE(hub.queued(0).options.continuous); // the retried poll stays continuous
}
// A one-shot duplicate downgrades a continuous poll to a one-shot (the mirror of a continuous
@@ -363,16 +363,16 @@ TEST(ModbusClientHubPriority, DuplicateSendDowngradesContinuous) {
device.read_holding_registers(0x100, 2, {.continuous = true});
ASSERT_EQ(hub.queued_frames(), 1u);
ASSERT_TRUE(hub.queued(0).continuous);
ASSERT_TRUE(hub.queued(0).options.continuous);
device.read_holding_registers(0x100, 2); // one-shot duplicate downgrades the poll
ASSERT_EQ(hub.queued_frames(), 1u);
EXPECT_FALSE(hub.queued(0).continuous);
EXPECT_FALSE(hub.queued(0).options.continuous);
EXPECT_EQ(hub.queued(0).pending, 1u);
// It runs one more cycle to serve the request, then stops - not re-queued as a poll.
hub.force_send_next();
EXPECT_FALSE(hub.waiting_command().continuous);
EXPECT_FALSE(hub.waiting_command().options.continuous);
const uint8_t ok_response[] = {0x03, 0x04, 0x00, 0x2A, 0x01, 0x00};
hub.receive_frame_for_test(0x02, ok_response);
EXPECT_EQ(hub.queued_frames(), 0u);
@@ -407,16 +407,16 @@ TEST(ModbusClientHubPriority, DowngradeAfterTerminalKeepsRequestAlive) {
device.read_holding_registers(0x100, 2, {.continuous = true});
ASSERT_EQ(hub.queued_frames(), 1u);
ASSERT_TRUE(hub.queued(0).continuous);
ASSERT_TRUE(hub.queued(0).options.continuous);
hub.force_send_next();
const uint8_t exception_response[] = {0x83, 0x02};
hub.receive_frame_for_test(0x02, exception_response); // exception ends the poll; on_error re-sends
EXPECT_EQ(device.error_count_, 1); // one terminal delivered so far
ASSERT_EQ(hub.queued_frames(), 1u); // the re-send survived the sweep instead of being erased
EXPECT_FALSE(hub.queued(0).continuous); // downgraded to a one-shot
EXPECT_EQ(hub.queued(0).pending, 1u); // debt restored so the request runs
EXPECT_EQ(device.error_count_, 1); // one terminal delivered so far
ASSERT_EQ(hub.queued_frames(), 1u); // the re-send survived the sweep instead of being erased
EXPECT_FALSE(hub.queued(0).options.continuous); // downgraded to a one-shot
EXPECT_EQ(hub.queued(0).pending, 1u); // debt restored so the request runs
// And it runs to its own terminal - a good response this time - then the entry is gone.
hub.force_send_next();
@@ -434,18 +434,18 @@ TEST(ModbusClientHubPriority, ContinuousRequestUpgradesQueuedDuplicate) {
device.read_holding_registers(0x100, 2);
ASSERT_EQ(hub.queued_frames(), 1u);
ASSERT_FALSE(hub.queued(0).continuous);
ASSERT_FALSE(hub.queued(0).options.continuous);
device.read_holding_registers(0x100, 2, {.continuous = true});
ASSERT_EQ(hub.queued_frames(), 1u);
EXPECT_TRUE(hub.queued(0).continuous);
EXPECT_TRUE(hub.queued(0).options.continuous);
// And it behaves as a poll from here: success cycles it back to READY.
hub.force_send_next();
const uint8_t ok_response[] = {0x03, 0x04, 0x00, 0x2A, 0x01, 0x00};
hub.receive_frame_for_test(0x02, ok_response);
ASSERT_EQ(hub.queued_frames(), 1u);
EXPECT_TRUE(hub.queued(0).continuous);
EXPECT_TRUE(hub.queued(0).options.continuous);
}
// The transmit order is one key with three levels: writes, then one-shot reads, then continuous
@@ -473,7 +473,7 @@ TEST(ModbusClientHubPriority, WritesThenOneShotReadsThenContinuousPolls) {
EXPECT_EQ(hub.waiting_command().frame.pdu()[1], 0x02); // then the one-shot read
hub.timeout_waiting();
hub.force_send_next();
EXPECT_TRUE(hub.waiting_command().continuous); // and the poll takes what is left
EXPECT_TRUE(hub.waiting_command().options.continuous); // and the poll takes what is left
}
// continuous is ignored for writes: the frame still sends at WRITE priority, once.
@@ -485,7 +485,7 @@ TEST(ModbusClientHubPriority, ContinuousIgnoredForWrites) {
device.queue_pdu(write_pdu, {.continuous = true});
ASSERT_EQ(hub.queued_frames(), 1u);
EXPECT_EQ(hub.queued(0).priority(), CommandPriority::WRITE);
EXPECT_FALSE(hub.queued(0).continuous);
EXPECT_FALSE(hub.queued(0).options.continuous);
}
// A queued continuous poll does not count against immediate-send readiness: it ranks below every
@@ -496,7 +496,7 @@ TEST(ModbusClientHubPriority, ContinuousPollDoesNotBlockImmediateSend) {
EXPECT_TRUE(hub.tx_buffer_empty()); // nothing queued
device.read_holding_registers(0x100, 2, {.continuous = true});
ASSERT_TRUE(hub.queued(0).continuous);
ASSERT_TRUE(hub.queued(0).options.continuous);
EXPECT_TRUE(hub.tx_buffer_empty()); // a READY continuous poll still leaves room to send now
device.read_holding_registers(0x200, 2); // a one-shot does count
@@ -1878,8 +1878,8 @@ TEST(ModbusClientHubPriority, ResendFromOnResponseAbsorbsIntoCompletingCommand)
const uint8_t ok_response[] = {0x03, 0x04, 0x00, 0x2A, 0x01, 0x00};
hub.receive_frame_for_test(0x02, ok_response); // handler re-sends the identical frame mid-completion
ASSERT_EQ(hub.queued_frames(), 1u); // absorbed into the same entry, not a fresh twin
EXPECT_FALSE(hub.queued(0).continuous); // the one-shot re-send downgraded the poll
ASSERT_EQ(hub.queued_frames(), 1u); // absorbed into the same entry, not a fresh twin
EXPECT_FALSE(hub.queued(0).options.continuous); // the one-shot re-send downgraded the poll
}
// An exception-flagged function code is never silently re-sendable, even though the read check
@@ -51,6 +51,7 @@ button:
# A pdu lambda can hand-assemble bytes or return a modbus::helpers::create_*_pdu() builder result.
- modbus_client.send:
address: 0x01
continuous: true
pdu: !lambda "return modbus::helpers::create_read_pdu(modbus::FunctionCode::READ_HOLDING_REGISTERS, 0x0010, 1);"
- modbus_client.send:
address: !lambda "return 1;"
@@ -91,6 +92,7 @@ button:
address: !lambda "return 1;"
start_address: 0x10
count: 2
continuous: true
on_response:
then:
- lambda: 'ESP_LOGI("modbus_client.test", "first=%u n=%u", values[0], (unsigned) values.size());'
@@ -98,6 +100,7 @@ button:
then:
- logger.log: "typed read timeout"
- modbus_client.read_input_registers:
continuous: !lambda "return false;"
address: 0x01
start_address: 0x20
on_custom_response:
@@ -113,12 +116,14 @@ button:
address: 0x01
start_address: 0x03
count: 16
continuous: true
on_response:
then:
- lambda: 'ESP_LOGI("modbus_client.test", "coil0=%d n=%u", bits[0], (unsigned) bits.size());'
- modbus_client.read_discrete_inputs:
address: 0x01
start_address: 0x00
continuous: true
on_error:
then:
- lambda: 'ESP_LOGW("modbus_client.test", "fc 0x%X exception %d", request.empty() ? 0 : request[0], (int) exception_code);'
+22 -26
View File
@@ -1,5 +1,4 @@
from collections.abc import Generator
import errno
import io
import logging
import os
@@ -178,37 +177,34 @@ def _run_probe_on_pty(
output = b""
deadline = time.monotonic() + 60
try:
try:
proc = subprocess.Popen(
_probe_command(fixture_path),
stdout=follower,
stderr=follower if stderr_to_pty else subprocess.PIPE,
stdin=follower,
env=probe_env,
)
finally:
os.close(follower)
while True:
timeout = deadline - time.monotonic()
if timeout <= 0 or not select.select([controller], [], [], timeout)[0]:
pytest.fail(f"pty probe produced no EOF in time; got {output!r}")
try:
chunk = os.read(controller, 1024)
except OSError as err:
# macOS raises EIO once the child closes its end of the pty;
# anything else is a real failure, not end-of-stream.
if err.errno != errno.EIO:
raise
break
if not chunk:
break
proc = subprocess.Popen(
_probe_command(fixture_path),
stdout=follower,
stderr=follower if stderr_to_pty else subprocess.PIPE,
stdin=follower,
env=probe_env,
)
# The parent keeps the follower open until the child has exited and
# the controller is drained: macOS discards buffered pty output once
# the last follower closes, so closing it early loses the probe's
# output whenever the child finishes before the first read.
while proc.poll() is None:
if time.monotonic() > deadline:
pytest.fail(f"pty probe did not exit in time; got {output!r}")
if select.select([controller], [], [], 0.01)[0]:
output += os.read(controller, 4096)
# Everything the child wrote is already buffered, so drain without waiting.
while select.select([controller], [], [], 0)[0] and (
chunk := os.read(controller, 4096)
):
output += chunk
stderr_text = ""
if proc.stderr is not None:
stderr_text = proc.stderr.read().decode(errors="replace")
proc.stderr.close()
assert proc.wait(60) == 0, stderr_text
assert proc.returncode == 0, stderr_text
finally:
os.close(follower)
os.close(controller)
if proc is not None and proc.poll() is None:
proc.kill()
@@ -471,6 +471,32 @@ def test_spaced_linkflag_survives_relexing(tmp_path) -> None:
]
def test_env_attribute_access_warns_without_call(caplog) -> None:
"""hasattr()/truthiness on an unsupported method is diagnosable; dunder
protocol probes stay silent."""
env = _FakeSConsEnv(
board_mcu="esp8266", pio_env="esphome_esp8266", pio_platform="espressif8266"
)
assert env.GetProjectOption
assert caplog.text.count("env.GetProjectOption is not supported") == 1
assert not hasattr(env, "__deepcopy__")
assert "__deepcopy__" not in caplog.text
def test_env_unmodelled_subscript_degrades_one_branch(caplog) -> None:
"""env[...] on an unmodelled var returns '' instead of KeyError
discarding the whole capture."""
env = _FakeSConsEnv(
board_mcu="esp8266", pio_env="esphome_esp8266", pio_platform="espressif8266"
)
assert env["PIOFRAMEWORK"] == ""
assert env["PIOFRAMEWORK"] == ""
assert caplog.text.count("env['PIOFRAMEWORK'] is not modelled") == 1
assert env["BOARD_MCU"] == "esp8266"
env.Append(LIBS=["still_captured"])
assert env.result.libs == ["still_captured"]
def test_uncaptured_append_key_warns_once(caplog) -> None:
"""A loop of Appends to the same uncaptured key warns once."""