diff --git a/esphome/components/api/api.proto b/esphome/components/api/api.proto index f57579fd27..8d6737fe81 100644 --- a/esphome/components/api/api.proto +++ b/esphome/components/api/api.proto @@ -2733,7 +2733,8 @@ enum SerialProxyParity { SERIAL_PROXY_PARITY_ODD = 2; } -// Configure UART parameters for a serial proxy instance +// Configure UART parameters for a serial proxy instance. Only the subscribed client may +// configure the port; others are refused with PORT_IN_USE (since API 1.17). message SerialProxyConfigureRequest { option (id) = 138; option (source) = SOURCE_CLIENT; @@ -2759,7 +2760,8 @@ message SerialProxyDataReceived { bytes data = 2; // Raw data received from the serial device } -// Write data to a serial device +// Write data to a serial device. Only the subscribed client may write; writes from +// others are ignored (since API 1.17). message SerialProxyWriteRequest { option (id) = 140; option (source) = SOURCE_CLIENT; @@ -2770,7 +2772,8 @@ message SerialProxyWriteRequest { bytes data = 2; // Raw data to write to the serial device } -// Set modem control pin states (RTS and DTR) +// Set modem control pin states (RTS and DTR). Only the subscribed client may set them; +// others are refused with PORT_IN_USE (since API 1.17). message SerialProxySetModemPinsRequest { option (id) = 141; option (source) = SOURCE_CLIENT; @@ -2809,6 +2812,7 @@ enum SerialProxyRequestType { // error the device answers with INVALID_ARGUMENT. SERIAL_PROXY_REQUEST_TYPE_CONFIGURE = 3; // Acknowledges a SerialProxyConfigureRequest SERIAL_PROXY_REQUEST_TYPE_SET_MODEM_PINS = 4; // Acknowledges a SerialProxySetModemPinsRequest + SERIAL_PROXY_REQUEST_TYPE_SET_MODE = 5; // Acknowledges a SerialProxySetModeRequest (since API 1.17) } enum SerialProxyStatus { @@ -2821,7 +2825,8 @@ enum SerialProxyStatus { SERIAL_PROXY_STATUS_INVALID_ARGUMENT = 6; // Invalid instance index or parameter value } -// Generic request message for simple serial proxy operations +// Generic request message for simple serial proxy operations. FLUSH requires an active +// subscription; it is refused with PORT_IN_USE otherwise (since API 1.17). message SerialProxyRequest { option (id) = 144; option (source) = SOURCE_CLIENT; @@ -2856,8 +2861,11 @@ enum SerialProxyMode { SERIAL_PROXY_MODE_PROTOCOL = 1; } +// Only the subscribed client may change the mode; any other caller -- including one that +// never subscribed -- is refused with PORT_IN_USE. PROTOCOL is refused with NOT_SUPPORTED +// when the port has no protocol-aware tap configured. message SerialProxySetModeRequest { - option (id) = 151; + option (id) = 152; option (source) = SOURCE_CLIENT; option (ifdef) = "USE_SERIAL_PROXY"; @@ -2894,7 +2902,7 @@ enum ZigbeeProxyRequestType { } message ZigbeeProxyRequest { - option (id) = 150; + option (id) = 153; option (source) = SOURCE_BOTH; option (ifdef) = "USE_ZIGBEE_PROXY"; diff --git a/esphome/components/api/api_connection.cpp b/esphome/components/api/api_connection.cpp index 3fc46e5c19..59aa93e126 100644 --- a/esphome/components/api/api_connection.cpp +++ b/esphome/components/api/api_connection.cpp @@ -1670,6 +1670,7 @@ void APIConnection::on_serial_proxy_request(const SerialProxyRequest &msg) { break; case enums::SERIAL_PROXY_REQUEST_TYPE_CONFIGURE: case enums::SERIAL_PROXY_REQUEST_TYPE_SET_MODEM_PINS: + case enums::SERIAL_PROXY_REQUEST_TYPE_SET_MODE: // Response-only discriminators; never valid in a request ESP_LOGW(TAG, "Response-only serial proxy request type: %" PRIu32, static_cast(msg.type)); status = enums::SERIAL_PROXY_STATUS_INVALID_ARGUMENT; @@ -1686,9 +1687,13 @@ void APIConnection::on_serial_proxy_set_mode_request(const SerialProxySetModeReq auto &proxies = App.get_serial_proxies(); if (msg.instance >= proxies.size()) { ESP_LOGW(TAG, "Serial proxy instance %" PRIu32 " out of range", msg.instance); + send_serial_proxy_ack(this, msg.instance, enums::SERIAL_PROXY_REQUEST_TYPE_SET_MODE, + enums::SERIAL_PROXY_STATUS_INVALID_ARGUMENT); return; } - proxies[msg.instance]->set_mode(this, msg.mode); + serial_proxy::SerialProxyResult result = proxies[msg.instance]->set_mode_from_client(this, msg.mode); + send_serial_proxy_ack(this, msg.instance, enums::SERIAL_PROXY_REQUEST_TYPE_SET_MODE, + serial_proxy_result_to_status(result)); } void APIConnection::send_serial_proxy_data(const SerialProxyDataReceived &msg) { @@ -1822,7 +1827,7 @@ bool APIConnection::send_hello_response_(const HelloRequest &msg) { HelloResponse resp; resp.api_version_major = 1; - resp.api_version_minor = 16; + resp.api_version_minor = 17; // Send only the version string - the client only logs this for debugging and doesn't use it otherwise resp.server_info = ESPHOME_VERSION_REF; resp.name = StringRef(App.get_name()); diff --git a/esphome/components/api/api_pb2.h b/esphome/components/api/api_pb2.h index 26958e8a50..7755d82966 100644 --- a/esphome/components/api/api_pb2.h +++ b/esphome/components/api/api_pb2.h @@ -356,6 +356,7 @@ enum SerialProxyRequestType : uint32_t { SERIAL_PROXY_REQUEST_TYPE_FLUSH = 2, SERIAL_PROXY_REQUEST_TYPE_CONFIGURE = 3, SERIAL_PROXY_REQUEST_TYPE_SET_MODEM_PINS = 4, + SERIAL_PROXY_REQUEST_TYPE_SET_MODE = 5, }; enum SerialProxyStatus : uint32_t { SERIAL_PROXY_STATUS_OK = 0, @@ -3420,7 +3421,7 @@ class SerialProxyRequestResponse final : public ProtoMessage { }; class SerialProxySetModeRequest final : public ProtoDecodableMessage { public: - static constexpr uint16_t MESSAGE_TYPE = 151; + static constexpr uint16_t MESSAGE_TYPE = 152; static constexpr uint8_t ESTIMATED_SIZE = 6; #ifdef HAS_PROTO_MESSAGE_DUMP const LogString *message_name() const override { return LOG_STR("serial_proxy_set_mode_request"); } @@ -3476,7 +3477,7 @@ class BluetoothSetConnectionParamsResponse final : public ProtoMessage { #ifdef USE_ZIGBEE_PROXY class ZigbeeProxyRequest final : public ProtoDecodableMessage { public: - static constexpr uint16_t MESSAGE_TYPE = 150; + static constexpr uint16_t MESSAGE_TYPE = 153; static constexpr uint8_t ESTIMATED_SIZE = 21; #ifdef HAS_PROTO_MESSAGE_DUMP const LogString *message_name() const override { return LOG_STR("zigbee_proxy_request"); } diff --git a/esphome/components/api/api_pb2_dump.cpp b/esphome/components/api/api_pb2_dump.cpp index 828e06f96d..98dbae5bcc 100644 --- a/esphome/components/api/api_pb2_dump.cpp +++ b/esphome/components/api/api_pb2_dump.cpp @@ -854,6 +854,8 @@ template<> const char *proto_enum_to_string(enums return ESPHOME_PSTR("SERIAL_PROXY_REQUEST_TYPE_CONFIGURE"); case enums::SERIAL_PROXY_REQUEST_TYPE_SET_MODEM_PINS: return ESPHOME_PSTR("SERIAL_PROXY_REQUEST_TYPE_SET_MODEM_PINS"); + case enums::SERIAL_PROXY_REQUEST_TYPE_SET_MODE: + return ESPHOME_PSTR("SERIAL_PROXY_REQUEST_TYPE_SET_MODE"); default: return ESPHOME_PSTR("UNKNOWN"); } diff --git a/esphome/components/api/api_pb2_service.cpp b/esphome/components/api/api_pb2_service.cpp index 1b6ed7e228..edf460c4b6 100644 --- a/esphome/components/api/api_pb2_service.cpp +++ b/esphome/components/api/api_pb2_service.cpp @@ -712,17 +712,6 @@ void APIConnection::read_message_(uint32_t msg_size, uint32_t msg_type, const ui this->on_device_capabilities_request(); break; } -#ifdef USE_ZIGBEE_PROXY - case ZigbeeProxyRequest::MESSAGE_TYPE: { - ZigbeeProxyRequest msg; - msg.decode(msg_data, msg_size); -#ifdef HAS_PROTO_MESSAGE_DUMP - this->log_receive_message_(LOG_STR("on_zigbee_proxy_request"), msg); -#endif - this->on_zigbee_proxy_request(msg); - break; - } -#endif #ifdef USE_SERIAL_PROXY case SerialProxySetModeRequest::MESSAGE_TYPE: { SerialProxySetModeRequest msg; @@ -733,6 +722,17 @@ void APIConnection::read_message_(uint32_t msg_size, uint32_t msg_type, const ui this->on_serial_proxy_set_mode_request(msg); break; } +#endif +#ifdef USE_ZIGBEE_PROXY + case ZigbeeProxyRequest::MESSAGE_TYPE: { + ZigbeeProxyRequest msg; + msg.decode(msg_data, msg_size); +#ifdef HAS_PROTO_MESSAGE_DUMP + this->log_receive_message_(LOG_STR("on_zigbee_proxy_request"), msg); +#endif + this->on_zigbee_proxy_request(msg); + break; + } #endif default: break; diff --git a/esphome/components/serial_proxy/__init__.py b/esphome/components/serial_proxy/__init__.py index 158c9609e4..b6e780fabd 100644 --- a/esphome/components/serial_proxy/__init__.py +++ b/esphome/components/serial_proxy/__init__.py @@ -18,7 +18,7 @@ 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_ID, CONF_MODE, CONF_NAME +from esphome.const import CONF_ID, CONF_NAME from esphome.core import CORE, coroutine_with_priority from esphome.coroutine import CoroPriority from esphome.types import ConfigType @@ -40,16 +40,6 @@ SERIAL_PROXY_PORT_TYPES = { "RS485": SerialProxyPortType.SERIAL_PROXY_PORT_TYPE_RS485, } -SerialProxyMode = api_enums_ns.enum("SerialProxyMode") -# The mode a port starts in. `raw` is a plain byte pipe; `protocol` activates the -# port's tap (if one is configured), letting it observe traffic and inject protocol -# bytes such as acknowledgements. Clients may change it at runtime, so this only -# decides what the device boots into. -SERIAL_PROXY_MODES = { - "RAW": SerialProxyMode.SERIAL_PROXY_MODE_RAW, - "PROTOCOL": SerialProxyMode.SERIAL_PROXY_MODE_PROTOCOL, -} - CONF_DTR_PIN = "dtr_pin" CONF_PORT_TYPE = "port_type" CONF_RTS_PIN = "rts_pin" @@ -74,9 +64,6 @@ CONFIG_SCHEMA = ( cv.GenerateID(): cv.declare_id(SerialProxy), cv.Required(CONF_NAME): cv.string_strict, cv.Required(CONF_PORT_TYPE): cv.enum(SERIAL_PROXY_PORT_TYPES, upper=True), - cv.Optional(CONF_MODE, default="RAW"): cv.enum( - SERIAL_PROXY_MODES, upper=True - ), cv.Optional(CONF_RTS_PIN): pins.gpio_output_pin_schema, cv.Optional(CONF_DTR_PIN): pins.gpio_output_pin_schema, } @@ -101,7 +88,6 @@ async def to_code(config: ConfigType) -> None: cg.add(cg.App.register_serial_proxy(var)) cg.add(var.set_name(config[CONF_NAME])) cg.add(var.set_port_type(config[CONF_PORT_TYPE])) - cg.add(var.set_mode(config[CONF_MODE])) cg.add_define("USE_SERIAL_PROXY") # Track instance count for the FINAL priority define diff --git a/esphome/components/serial_proxy/serial_proxy.cpp b/esphome/components/serial_proxy/serial_proxy.cpp index 5d0e9cbbd7..129745c1c9 100644 --- a/esphome/components/serial_proxy/serial_proxy.cpp +++ b/esphome/components/serial_proxy/serial_proxy.cpp @@ -43,6 +43,7 @@ void SerialProxy::setup() { this->disable_loop(); } +#ifdef USE_SERIAL_PROXY_TAP void SerialProxy::reset_mode_() { // The mode belongs to a session, not to the port. Carrying a departed client's choice // over to the next one would inject protocol bytes into a stream that never asked for @@ -55,6 +56,7 @@ void SerialProxy::reset_mode_() { ESP_LOGD(TAG, "Session ended, returning serial proxy [%" PRIu32 "] to RAW mode", this->instance_index_); this->mode_ = api::enums::SERIAL_PROXY_MODE_RAW; } +#endif void SerialProxy::loop() { #ifdef USE_API @@ -120,11 +122,10 @@ bool SerialProxy::tap_observing_() const { if (this->tap_ == nullptr) { return false; } - // A tap that needs the port is mid-protocol-work of its own -- the boot-time handshake - // with the device, which runs before any client has connected and so before anyone could - // have chosen a mode. Withholding bytes from it there would strand it, so it is served - // regardless of mode. - if (this->tap_->tap_needs_port()) { + // With no subscriber, a tap doing its own protocol work (the boot-time handshake with + // the device, say) is served regardless of mode -- nobody has chosen one yet. Once a + // subscriber holds the port, the mode alone decides, so RAW stays inert. + if (this->api_connection_ == nullptr && this->tap_->tap_needs_port()) { return true; } // Otherwise the mode decides. RAW must be inert: a client that flips to RAW before @@ -135,6 +136,10 @@ bool SerialProxy::tap_observing_() const { void SerialProxy::tap_pump() { #ifdef USE_API + // Nothing would consume the bytes; leave them in the FIFO + if (!this->tap_observing_() && this->api_connection_ == nullptr) { + return; + } const size_t available = this->available(); if (available > 0) { this->read_and_send_(available); @@ -144,28 +149,26 @@ void SerialProxy::tap_pump() { #endif void SerialProxy::dump_config() { - ESP_LOGCONFIG( - TAG, - "Serial Proxy [%" PRIu32 "]:\n" - " Name: %s\n" - " Port Type: %s\n" - " Mode: %s\n" - " RTS Pin: %s\n" - " DTR Pin: %s", - this->instance_index_, this->name_ != nullptr ? this->name_ : "", - this->port_type_ == api::enums::SERIAL_PROXY_PORT_TYPE_RS485 ? LOG_STR_LITERAL("RS485") - : this->port_type_ == api::enums::SERIAL_PROXY_PORT_TYPE_RS232 ? LOG_STR_LITERAL("RS232") - : LOG_STR_LITERAL("TTL"), - this->mode_ == api::enums::SERIAL_PROXY_MODE_PROTOCOL ? LOG_STR_LITERAL("PROTOCOL") : LOG_STR_LITERAL("RAW"), - this->rts_pin_ != nullptr ? LOG_STR_LITERAL("configured") : LOG_STR_LITERAL("not configured"), - this->dtr_pin_ != nullptr ? LOG_STR_LITERAL("configured") : LOG_STR_LITERAL("not configured")); + ESP_LOGCONFIG(TAG, + "Serial Proxy [%" PRIu32 "]:\n" + " Name: %s\n" + " Port Type: %s\n" + " RTS Pin: %s\n" + " DTR Pin: %s", + this->instance_index_, this->name_ != nullptr ? this->name_ : "", + this->port_type_ == api::enums::SERIAL_PROXY_PORT_TYPE_RS485 ? LOG_STR_LITERAL("RS485") + : this->port_type_ == api::enums::SERIAL_PROXY_PORT_TYPE_RS232 ? LOG_STR_LITERAL("RS232") + : LOG_STR_LITERAL("TTL"), + this->rts_pin_ != nullptr ? LOG_STR_LITERAL("configured") : LOG_STR_LITERAL("not configured"), + this->dtr_pin_ != nullptr ? LOG_STR_LITERAL("configured") : LOG_STR_LITERAL("not configured")); } SerialProxyResult SerialProxy::configure(api::APIConnection *api_connection, uint32_t baudrate, bool flow_control, uint8_t parity, uint8_t stop_bits, uint8_t data_size) { #ifdef USE_API - if (this->port_claimed_by_other_(api_connection)) { - ESP_LOGW(TAG, "Ignoring configure request from client without port access [%" PRIu32 "]", this->instance_index_); + if (!this->is_subscriber_(api_connection)) { + ESP_LOGW(TAG, "Ignoring configure request from client without port subscription [%" PRIu32 "]", + this->instance_index_); return SerialProxyResult::SERIAL_PROXY_RESULT_PORT_IN_USE; } #endif @@ -231,20 +234,37 @@ SerialProxyResult SerialProxy::configure(api::APIConnection *api_connection, uin return SerialProxyResult::SERIAL_PROXY_RESULT_OK; } -void SerialProxy::set_mode(api::APIConnection *api_connection, api::enums::SerialProxyMode mode) { +SerialProxyResult SerialProxy::set_mode_from_client(api::APIConnection *api_connection, + api::enums::SerialProxyMode mode) { #ifdef USE_API - if (this->port_claimed_by_other_(api_connection)) { - ESP_LOGW(TAG, "Ignoring mode request from client without port access [%" PRIu32 "]", this->instance_index_); - return; + // Only the live subscriber may change the mode, so the mode cannot outlive a session + if (!this->is_subscriber_(api_connection)) { + ESP_LOGW(TAG, "Ignoring mode request from client without port subscription [%" PRIu32 "]", this->instance_index_); + return SerialProxyResult::SERIAL_PROXY_RESULT_PORT_IN_USE; } #endif + // Values come from a remote client + if (mode != api::enums::SERIAL_PROXY_MODE_RAW && mode != api::enums::SERIAL_PROXY_MODE_PROTOCOL) { + ESP_LOGW(TAG, "Invalid mode: %" PRIu32, static_cast(mode)); + return SerialProxyResult::SERIAL_PROXY_RESULT_INVALID_ARGUMENT; + } + // PROTOCOL on a port with no tap would be a silent no-op; refuse so the client knows +#ifdef USE_SERIAL_PROXY_TAP + const bool has_tap = this->tap_ != nullptr; +#else + const bool has_tap = false; +#endif + if (mode == api::enums::SERIAL_PROXY_MODE_PROTOCOL && !has_tap) { + ESP_LOGW(TAG, "No tap on serial proxy [%" PRIu32 "]; PROTOCOL mode unavailable", this->instance_index_); + return SerialProxyResult::SERIAL_PROXY_RESULT_NOT_SUPPORTED; + } ESP_LOGD(TAG, "Serial proxy [%" PRIu32 "] mode set to %s", this->instance_index_, - mode == api::enums::SERIAL_PROXY_MODE_PROTOCOL ? "PROTOCOL" : "RAW"); + mode == api::enums::SERIAL_PROXY_MODE_PROTOCOL ? LOG_STR_LITERAL("PROTOCOL") : LOG_STR_LITERAL("RAW")); +#ifdef USE_SERIAL_PROXY_TAP const bool leaving_protocol_mode = this->mode_ != api::enums::SERIAL_PROXY_MODE_RAW && mode == api::enums::SERIAL_PROXY_MODE_RAW; this->mode_ = mode; -#ifdef USE_SERIAL_PROXY_TAP // Only for an explicit client request, not for reset_mode_() at the end of a session: // an ordinary disconnect says nothing about the device, whereas a client deliberately // asking for raw bytes usually precedes changing what the device is. @@ -252,14 +272,22 @@ void SerialProxy::set_mode(api::APIConnection *api_connection, api::enums::Seria this->tap_->on_protocol_disabled(); } #endif + return SerialProxyResult::SERIAL_PROXY_RESULT_OK; } void SerialProxy::write_from_client(api::APIConnection *api_connection, const uint8_t *data, size_t len) { #ifdef USE_API - // Bytes from a client other than the live subscriber would interleave with the - // subscriber's traffic on the wire - if (this->port_claimed_by_other_(api_connection)) { - ESP_LOGW(TAG, "Ignoring write from client without port access [%" PRIu32 "]", this->instance_index_); + // Bytes from anyone but the live subscriber would interleave with the subscriber's + // traffic -- or with an active tap's -- on the wire + if (!this->is_subscriber_(api_connection)) { + if (this->api_connection_ != nullptr) { + ESP_LOGW(TAG, "Ignoring write from client that does not hold serial proxy [%" PRIu32 "]", this->instance_index_); + } else { + // A legacy client streaming writes without subscribing would flood WARN, one per + // request; writes are the only high-rate, unacknowledged operation, so keep this + // visible without drowning the log + ESP_LOGV(TAG, "Ignoring write from client without port subscription [%" PRIu32 "]", this->instance_index_); + } return; } #endif @@ -277,8 +305,9 @@ void SerialProxy::write_from_client(api::APIConnection *api_connection, const ui SerialProxyResult SerialProxy::set_modem_pins(api::APIConnection *api_connection, uint32_t line_states) { #ifdef USE_API - if (this->port_claimed_by_other_(api_connection)) { - ESP_LOGW(TAG, "Ignoring modem pin request from client without port access [%" PRIu32 "]", this->instance_index_); + if (!this->is_subscriber_(api_connection)) { + ESP_LOGW(TAG, "Ignoring modem pin request from client without port subscription [%" PRIu32 "]", + this->instance_index_); return SerialProxyResult::SERIAL_PROXY_RESULT_PORT_IN_USE; } #endif @@ -312,8 +341,8 @@ uint32_t SerialProxy::get_modem_pins() const { SerialProxyResult SerialProxy::flush_port(api::APIConnection *api_connection) { #ifdef USE_API // Flushing stalls the port, so it gets the same ownership check as writes - if (this->port_claimed_by_other_(api_connection)) { - ESP_LOGW(TAG, "Ignoring flush from client without port access [%" PRIu32 "]", this->instance_index_); + if (!this->is_subscriber_(api_connection)) { + ESP_LOGW(TAG, "Ignoring flush from client without port subscription [%" PRIu32 "]", this->instance_index_); return SerialProxyResult::SERIAL_PROXY_RESULT_PORT_IN_USE; } #endif @@ -332,11 +361,6 @@ SerialProxyResult SerialProxy::flush_port(api::APIConnection *api_connection) { } #ifdef USE_API -bool SerialProxy::port_claimed_by_other_(api::APIConnection *api_connection) const { - return this->api_connection_ != nullptr && this->api_connection_ != api_connection && - this->api_connection_->is_connection_setup(); -} - SerialProxyResult SerialProxy::serial_proxy_request(api::APIConnection *api_connection, api::enums::SerialProxyRequestType type) { switch (type) { @@ -354,6 +378,10 @@ SerialProxyResult SerialProxy::serial_proxy_request(api::APIConnection *api_conn return SerialProxyResult::SERIAL_PROXY_RESULT_PORT_IN_USE; } ESP_LOGW(TAG, "Previous subscriber disconnected; taking over subscription"); + // End the dead client's session before starting the new one, so its mode + // cannot leak into a session that never asked for it + this->api_connection_ = nullptr; + this->reset_mode_(); } this->api_connection_ = api_connection; this->enable_loop(); @@ -367,7 +395,14 @@ SerialProxyResult SerialProxy::serial_proxy_request(api::APIConnection *api_conn } this->api_connection_ = nullptr; this->reset_mode_(); +#ifdef USE_SERIAL_PROXY_TAP + // Keep the loop alive for a tap that still needs the port (mirrors loop()) + if (this->tap_ == nullptr || !this->tap_->tap_needs_port()) { + this->disable_loop(); + } +#else this->disable_loop(); +#endif ESP_LOGV(TAG, "API connection unsubscribed from serial proxy [%" PRIu32 "]", this->instance_index_); return SerialProxyResult::SERIAL_PROXY_RESULT_OK; default: diff --git a/esphome/components/serial_proxy/serial_proxy.h b/esphome/components/serial_proxy/serial_proxy.h index 19a3a4f063..e3f4264cfa 100644 --- a/esphome/components/serial_proxy/serial_proxy.h +++ b/esphome/components/serial_proxy/serial_proxy.h @@ -71,7 +71,8 @@ class SerialProxyTap { virtual void on_client_tx(const uint8_t *data, size_t len) = 0; /// True when the port must keep reading even with no subscriber attached, so a tap can - /// do its own protocol work while nobody is listening. + /// do its own protocol work while nobody is listening. Honoured only while no + /// subscriber holds the port; with one attached, the port mode alone decides. virtual bool tap_needs_port() const = 0; /// A client explicitly turned protocol handling off for this port. Distinct from the @@ -107,14 +108,8 @@ class SerialProxy final : public uart::UARTDevice, public Component { /// Get the port type api::enums::SerialProxyPortType get_port_type() const { return this->port_type_; } - /// Set the initial mode (from YAML configuration) - void set_mode(api::enums::SerialProxyMode mode) { this->mode_ = mode; } - - /// Get the current mode - api::enums::SerialProxyMode get_mode() const { return this->mode_; } - /// Handle a mode change requested by an API client - void set_mode(api::APIConnection *api_connection, api::enums::SerialProxyMode mode); + SerialProxyResult set_mode_from_client(api::APIConnection *api_connection, api::enums::SerialProxyMode mode); /// Configure UART parameters and apply them /// @param api_connection The API connection requesting the change @@ -165,12 +160,25 @@ class SerialProxy final : public uart::UARTDevice, public Component { void set_tap(SerialProxyTap *tap) { this->tap_ = tap; } /// Write bytes originating from the tap rather than from a client. Bypasses the - /// subscriber ownership check, since the tap is part of the device, not a client of it. - void write_from_tap(const uint8_t *data, size_t len) { this->write_array(data, len); } + /// subscriber ownership check, but only while the tap is being served bytes -- so a + /// port in RAW mode with a subscriber attached stays inert. Returns false when the + /// bytes were dropped for that reason. + bool write_from_tap(const uint8_t *data, size_t len) { + if (!this->tap_observing_()) { + return false; + } + this->write_array(data, len); + return true; + } + + /// Whether the tap is currently being served bytes. Can flip false with no callback + /// (a subscriber attaching in RAW mode, say), so a tap should check before starting + /// protocol work and when a reply seems overdue. + bool tap_is_observed() const { return this->tap_observing_(); } /// Resume reading after a tap's needs change. loop() disables itself when there is /// neither a subscriber nor a tap that wants the port, so a tap starting fresh work - /// must ask for it back. + /// must ask for it back. Must be called from the main loop. void tap_request_port() { this->enable_loop(); } /// Whether the underlying device is present. On a USB UART this tracks enumeration, so @@ -179,7 +187,8 @@ class SerialProxy final : public uart::UARTDevice, public Component { /// Run one read-and-dispatch cycle immediately. Lets a tap make progress before the /// main loop is running -- during setup, for instance, while a component is still - /// blocking on can_proceed(). + /// blocking on can_proceed(). Must not be called from on_device_rx() or + /// on_client_tx(): each nested cycle costs a 256-byte stack frame. void tap_pump(); #endif @@ -189,13 +198,20 @@ class SerialProxy final : public uart::UARTDevice, public Component { /// (slow path with a 256-byte stack buffer) void read_and_send_(size_t available); - /// True when a live subscriber other than the given connection holds the port - bool port_claimed_by_other_(api::APIConnection *api_connection) const; + /// True when the given connection is the live subscriber. Every port operation + /// (write, configure, modem pins, flush, mode) requires this, so an unsubscribed + /// client can never share the wire with the subscriber or an active tap. + bool is_subscriber_(api::APIConnection *api_connection) const { return this->api_connection_ == api_connection; } #endif - /// Return the port to RAW when a subscriber goes away, so the mode never outlives it. - /// Not tap-gated: the mode is a client-visible property whether or not a tap acts on it. +#ifdef USE_SERIAL_PROXY_TAP + /// Return the port to RAW when a subscriber goes away, so the mode never outlives it void reset_mode_(); +#else + /// Without a tap, PROTOCOL is refused, so the mode is fixed at RAW and there is + /// nothing to reset + void reset_mode_() {} +#endif #ifdef USE_SERIAL_PROXY_TAP /// True when the tap should be shown the traffic passing through this port @@ -219,8 +235,10 @@ class SerialProxy final : public uart::UARTDevice, public Component { /// Port type api::enums::SerialProxyPortType port_type_{}; +#ifdef USE_SERIAL_PROXY_TAP /// How the bytes passing through are treated; zero is SERIAL_PROXY_MODE_RAW api::enums::SerialProxyMode mode_{}; +#endif /// Optional GPIO pins for modem control GPIOPin *rts_pin_{nullptr}; diff --git a/tests/benchmarks/stubs/esphome/components/serial_proxy/serial_proxy.h b/tests/benchmarks/stubs/esphome/components/serial_proxy/serial_proxy.h index 6fc20f3350..7da6fff017 100644 --- a/tests/benchmarks/stubs/esphome/components/serial_proxy/serial_proxy.h +++ b/tests/benchmarks/stubs/esphome/components/serial_proxy/serial_proxy.h @@ -40,6 +40,9 @@ class SerialProxy { return SerialProxyResult::SERIAL_PROXY_RESULT_OK; } void write_from_client(api::APIConnection *api_connection, const uint8_t *data, size_t len) {} + SerialProxyResult set_mode_from_client(api::APIConnection *api_connection, api::enums::SerialProxyMode mode) { + return SerialProxyResult::SERIAL_PROXY_RESULT_OK; + } SerialProxyResult set_modem_pins(api::APIConnection *api_connection, uint32_t line_states) { return SerialProxyResult::SERIAL_PROXY_RESULT_OK; } diff --git a/tests/components/serial_proxy/test-tap.esp32-idf.yaml b/tests/components/serial_proxy/test-tap.esp32-idf.yaml new file mode 100644 index 0000000000..5522e53c47 --- /dev/null +++ b/tests/components/serial_proxy/test-tap.esp32-idf.yaml @@ -0,0 +1,14 @@ +substitutions: + tx_pin: GPIO4 + rx_pin: GPIO5 + +# Compile the tap code paths; no tap is attached, so this exercises the +# null-tap branches that a normal build never defines. +esphome: + platformio_options: + build_flags: + - "-DUSE_SERIAL_PROXY_TAP" + +packages: + uart: !include ../../test_build_components/common/uart/esp32-idf.yaml + serial_proxy: !include common.yaml diff --git a/tests/components/zigbee_proxy/.gitignore b/tests/components/zigbee_proxy/.gitignore new file mode 100644 index 0000000000..d8b4157aef --- /dev/null +++ b/tests/components/zigbee_proxy/.gitignore @@ -0,0 +1,5 @@ +# Gitignore settings for ESPHome +# This is an example and may include too much for your use-case. +# You can modify this file to suit your needs. +/.esphome/ +/secrets.yaml diff --git a/tests/components/zigbee_proxy/common.yaml b/tests/components/zigbee_proxy/common.yaml index 91e27efd7d..b2b8cf1f57 100644 --- a/tests/components/zigbee_proxy/common.yaml +++ b/tests/components/zigbee_proxy/common.yaml @@ -20,4 +20,3 @@ serial_proxy: uart_id: zigbee_uart name: Zigbee port_type: TTL - mode: protocol diff --git a/tests/components/zigbee_proxy/test-usb.esp32-s3-idf.yaml b/tests/components/zigbee_proxy/test-usb.esp32-s3-idf.yaml index 93b7707d16..ed9ad9f24a 100644 --- a/tests/components/zigbee_proxy/test-usb.esp32-s3-idf.yaml +++ b/tests/components/zigbee_proxy/test-usb.esp32-s3-idf.yaml @@ -22,7 +22,6 @@ serial_proxy: uart_id: zigbee_usb_channel name: Zigbee port_type: TTL - mode: protocol zigbee_proxy: serial_proxy_id: zigbee_usb_serial