From 04715fb32bf711b89b04bb606fad55f977f0fcd1 Mon Sep 17 00:00:00 2001 From: puddly <32534428+puddly@users.noreply.github.com> Date: Wed, 14 Oct 2026 17:41:12 -0400 Subject: [PATCH] [usb_uart] Expose `interface_class` and `port_number` (#20659) Co-authored-by: J. Nick Koston --- esphome/components/api/api.proto | 7 ++- esphome/components/api/api_pb2.cpp | 4 ++ esphome/components/api/api_pb2.h | 2 + esphome/components/api/api_pb2_dump.cpp | 2 + .../components/serial_proxy/serial_proxy.cpp | 2 + esphome/components/usb_uart/ch34x.cpp | 8 --- esphome/components/usb_uart/cp210x.cpp | 10 ++-- esphome/components/usb_uart/ft23xx.cpp | 1 + esphome/components/usb_uart/pl2303.cpp | 5 +- esphome/components/usb_uart/usb_uart.cpp | 6 ++- esphome/components/usb_uart/usb_uart.h | 21 +++++++- tests/components/serial_proxy/__init__.py | 16 ++++++ .../test_usb_device_descriptor.cpp | 51 +++++++++++++++++++ 13 files changed, 115 insertions(+), 20 deletions(-) create mode 100644 tests/components/serial_proxy/__init__.py create mode 100644 tests/components/serial_proxy/test_usb_device_descriptor.cpp diff --git a/esphome/components/api/api.proto b/esphome/components/api/api.proto index f8f31d81df0..d55cf10c47d 100644 --- a/esphome/components/api/api.proto +++ b/esphome/components/api/api.proto @@ -3043,12 +3043,15 @@ enum SerialProxyIdentityFlag { // IDs below are empty } -// The descriptor fields of a USB device as seen by the host stack +// The descriptor fields of a USB device as seen by the host stack, and the port's index +// among those a vendor driver exposes on the interface message UsbDeviceDescriptor { uint32 vendor_id = 1; uint32 product_id = 2; uint32 bcd_device = 3; - uint32 interface_number = 4; // bInterfaceNumber the host driver binds to + uint32 interface_number = 4; // bInterfaceNumber the port is bound through + uint32 interface_class = 5; // bInterfaceClass of that interface + uint32 port_number = 6; } // The identity of the device behind a port. Identifies one physical device among others of diff --git a/esphome/components/api/api_pb2.cpp b/esphome/components/api/api_pb2.cpp index 8847249b173..bcfa86763a1 100644 --- a/esphome/components/api/api_pb2.cpp +++ b/esphome/components/api/api_pb2.cpp @@ -4175,6 +4175,8 @@ uint8_t *UsbDeviceDescriptor::encode_msg(const void *self, uint8_t *__restrict__ pos = ProtoEncode::encode_uint32(pos PROTO_ENCODE_DEBUG_ARG, 2, msg.product_id); pos = ProtoEncode::encode_uint32(pos PROTO_ENCODE_DEBUG_ARG, 3, msg.bcd_device); pos = ProtoEncode::encode_uint32(pos PROTO_ENCODE_DEBUG_ARG, 4, msg.interface_number); + pos = ProtoEncode::encode_uint32(pos PROTO_ENCODE_DEBUG_ARG, 5, msg.interface_class); + pos = ProtoEncode::encode_uint32(pos PROTO_ENCODE_DEBUG_ARG, 6, msg.port_number); return pos; } uint32_t UsbDeviceDescriptor::calc_size_msg(const void *self) { @@ -4184,6 +4186,8 @@ uint32_t UsbDeviceDescriptor::calc_size_msg(const void *self) { size += ProtoSize::calc_uint32(1, msg.product_id); size += ProtoSize::calc_uint32(1, msg.bcd_device); size += ProtoSize::calc_uint32(1, msg.interface_number); + size += ProtoSize::calc_uint32(1, msg.interface_class); + size += ProtoSize::calc_uint32(1, msg.port_number); return size; } uint8_t *SerialProxyIdentity::encode_msg(const void *self, uint8_t *__restrict__ pos PROTO_ENCODE_DEBUG_PARAM) { diff --git a/esphome/components/api/api_pb2.h b/esphome/components/api/api_pb2.h index 14204e97aaa..7c8082a8c7c 100644 --- a/esphome/components/api/api_pb2.h +++ b/esphome/components/api/api_pb2.h @@ -4083,6 +4083,8 @@ class UsbDeviceDescriptor final : public ProtoMessage { uint32_t product_id{0}; uint32_t bcd_device{0}; uint32_t interface_number{0}; + uint32_t interface_class{0}; + uint32_t port_number{0}; static uint8_t *encode_msg(const void *self, uint8_t *pos PROTO_ENCODE_DEBUG_PARAM); uint8_t *encode(ProtoWriteBuffer &buffer PROTO_ENCODE_DEBUG_PARAM) const { return encode_msg(this, buffer.get_pos() PROTO_ENCODE_DEBUG_ARG); diff --git a/esphome/components/api/api_pb2_dump.cpp b/esphome/components/api/api_pb2_dump.cpp index 3c6f5302b55..ec224c46e29 100644 --- a/esphome/components/api/api_pb2_dump.cpp +++ b/esphome/components/api/api_pb2_dump.cpp @@ -2942,6 +2942,8 @@ const char *UsbDeviceDescriptor::dump_to(DumpBuffer &out) const { dump_field(out, ESPHOME_PSTR("product_id"), this->product_id); dump_field(out, ESPHOME_PSTR("bcd_device"), this->bcd_device); dump_field(out, ESPHOME_PSTR("interface_number"), this->interface_number); + dump_field(out, ESPHOME_PSTR("interface_class"), this->interface_class); + dump_field(out, ESPHOME_PSTR("port_number"), this->port_number); return out.c_str(); } const char *SerialProxyIdentity::dump_to(DumpBuffer &out) const { diff --git a/esphome/components/serial_proxy/serial_proxy.cpp b/esphome/components/serial_proxy/serial_proxy.cpp index 9623eb21b3c..03d289225f2 100644 --- a/esphome/components/serial_proxy/serial_proxy.cpp +++ b/esphome/components/serial_proxy/serial_proxy.cpp @@ -418,6 +418,8 @@ void SerialProxy::fill_identity_([[maybe_unused]] IdentityScratch &scratch, api: msg.usb.product_id = scratch.product_id; msg.usb.bcd_device = scratch.bcd_device; msg.usb.interface_number = this->usb_channel_->get_interface_number(); + msg.usb.interface_class = this->usb_channel_->get_interface_class(); + msg.usb.port_number = this->usb_channel_->get_port_number(); msg.manufacturer = StringRef(scratch.manufacturer); msg.product = StringRef(scratch.product); msg.serial_number = StringRef(scratch.serial_number); diff --git a/esphome/components/usb_uart/ch34x.cpp b/esphome/components/usb_uart/ch34x.cpp index ed5a034b82f..dce07a64a53 100644 --- a/esphome/components/usb_uart/ch34x.cpp +++ b/esphome/components/usb_uart/ch34x.cpp @@ -152,14 +152,6 @@ bool USBUartTypeCH34X::config_step(USBUartChannelBase *channel, uint8_t step, bo } } -std::vector USBUartTypeCH34X::parse_descriptors(usb_device_handle_t dev_hdl) { - auto result = USBUartTypeCdcAcm::parse_descriptors(dev_hdl); - // ch34x doesn't use the interrupt endpoint, and we don't have endpoints to spare - for (auto &cdc_dev : result) { - cdc_dev.interrupt_interface_number = 0xFF; - } - return result; -} } // namespace esphome::usb_uart #endif // USE_ESP32_VARIANT_ESP32P4 || USE_ESP32_VARIANT_ESP32S2 || USE_ESP32_VARIANT_ESP32S3 || diff --git a/esphome/components/usb_uart/cp210x.cpp b/esphome/components/usb_uart/cp210x.cpp index e920da6c267..34b76f88541 100644 --- a/esphome/components/usb_uart/cp210x.cpp +++ b/esphome/components/usb_uart/cp210x.cpp @@ -88,11 +88,11 @@ std::vector USBUartTypeCP210X::parse_descriptors(usb_device_handle_t dev ESP_LOGE(TAG, "in_ep: usb_parse_endpoint_descriptor_by_index failed"); continue; } - if (in_ep->bEndpointAddress & usb_host::USB_DIR_IN) { - cdc_devs.push_back({CdcEps{nullptr, in_ep, out_ep, data_desc->bInterfaceNumber}}); - } else { - cdc_devs.push_back({CdcEps{nullptr, out_ep, in_ep, data_desc->bInterfaceNumber}}); - } + CdcEps eps = in_ep->bEndpointAddress & usb_host::USB_DIR_IN + ? CdcEps{nullptr, in_ep, out_ep, data_desc->bInterfaceNumber} + : CdcEps{nullptr, out_ep, in_ep, data_desc->bInterfaceNumber}; + eps.bulk_interface_class = data_desc->bInterfaceClass; + cdc_devs.push_back(eps); } return cdc_devs; } diff --git a/esphome/components/usb_uart/ft23xx.cpp b/esphome/components/usb_uart/ft23xx.cpp index e10da78aaf0..805a4adfce3 100644 --- a/esphome/components/usb_uart/ft23xx.cpp +++ b/esphome/components/usb_uart/ft23xx.cpp @@ -217,6 +217,7 @@ static optional get_uart(const usb_config_desc_t *config_desc, uint8_t i } eps.bulk_interface_number = intf_desc->bInterfaceNumber; + eps.bulk_interface_class = intf_desc->bInterfaceClass; return eps; } diff --git a/esphome/components/usb_uart/pl2303.cpp b/esphome/components/usb_uart/pl2303.cpp index 36107c093c4..0430b1eeacb 100644 --- a/esphome/components/usb_uart/pl2303.cpp +++ b/esphome/components/usb_uart/pl2303.cpp @@ -189,7 +189,10 @@ std::vector USBUartTypePL2303::parse_descriptors(usb_device_handle_t dev } if (in_ep && out_ep) { - cdc_devs.push_back(CdcEps{notify_ep, in_ep, out_ep, intf->bInterfaceNumber, intf->bInterfaceNumber}); + CdcEps eps{notify_ep, in_ep, out_ep, intf->bInterfaceNumber, intf->bInterfaceNumber}; + eps.bulk_interface_class = intf->bInterfaceClass; + eps.interrupt_interface_class = intf->bInterfaceClass; + cdc_devs.push_back(eps); break; // PL2303 is single-port } } diff --git a/esphome/components/usb_uart/usb_uart.cpp b/esphome/components/usb_uart/usb_uart.cpp index f61642566cb..744b79f4e43 100644 --- a/esphome/components/usb_uart/usb_uart.cpp +++ b/esphome/components/usb_uart/usb_uart.cpp @@ -41,14 +41,17 @@ static optional get_cdc(const usb_config_desc_t *config_desc, uint8_t in if (ep->bmAttributes == USB_BM_ATTRIBUTES_XFER_INT) { eps.notify_ep = ep; eps.interrupt_interface_number = intf_desc->bInterfaceNumber; + eps.interrupt_interface_class = intf_desc->bInterfaceClass; } else if (ep->bmAttributes == USB_BM_ATTRIBUTES_XFER_BULK && ep->bEndpointAddress & usb_host::USB_DIR_IN && (eps.bulk_interface_number == 0xFF || eps.bulk_interface_number == intf_desc->bInterfaceNumber)) { eps.in_ep = ep; eps.bulk_interface_number = intf_desc->bInterfaceNumber; + eps.bulk_interface_class = intf_desc->bInterfaceClass; } else if (ep->bmAttributes == USB_BM_ATTRIBUTES_XFER_BULK && !(ep->bEndpointAddress & usb_host::USB_DIR_IN) && (eps.bulk_interface_number == 0xFF || eps.bulk_interface_number == intf_desc->bInterfaceNumber)) { eps.out_ep = ep; eps.bulk_interface_number = intf_desc->bInterfaceNumber; + eps.bulk_interface_class = intf_desc->bInterfaceClass; } else { ESP_LOGE(TAG, "Unexpected endpoint attributes: %02X", ep->bmAttributes); continue; @@ -431,8 +434,7 @@ void USBUartTypeCdcAcm::on_connected() { if (channel->cdc_dev_.interrupt_interface_number != 0xFF && channel->cdc_dev_.interrupt_interface_number != channel->cdc_dev_.bulk_interface_number) { if (!this->claim_comm_interface_) { - ESP_LOGD(TAG, "Skipping comm interface %d (claim_comm_interface: false)", - channel->cdc_dev_.interrupt_interface_number); + ESP_LOGD(TAG, "Comm interface %d left unclaimed", channel->cdc_dev_.interrupt_interface_number); } else { auto err_comm = usb_host_interface_claim(this->handle_, this->device_handle_, channel->cdc_dev_.interrupt_interface_number, 0); diff --git a/esphome/components/usb_uart/usb_uart.h b/esphome/components/usb_uart/usb_uart.h index b54641884e6..ba073404e2f 100644 --- a/esphome/components/usb_uart/usb_uart.h +++ b/esphome/components/usb_uart/usb_uart.h @@ -40,6 +40,9 @@ struct CdcEps { // must remain valid even when the interface itself is not claimed. uint8_t interrupt_interface_number{0xFF}; bool interrupt_interface_claimed{false}; + // bInterfaceClass of each interface above, which decides how a host binds the port + uint8_t bulk_interface_class{0}; + uint8_t interrupt_interface_class{0}; }; enum CH34xChipType : uint8_t { @@ -187,6 +190,16 @@ class USBUartChannelBase : public uart::UARTComponent, public Parentedcdc_dev_.bulk_interface_number; } + /// bInterfaceClass of the interface `get_interface_number()` names + uint8_t get_interface_class() const { + return this->cdc_dev_.interrupt_interface_number != 0xFF ? this->cdc_dev_.interrupt_interface_class + : this->cdc_dev_.bulk_interface_class; + } + + /// Index of this port among the ports on its interface. Every driver here exposes one + /// port per interface; a multiplexed driver would index its channels. + uint8_t get_port_number() const { return 0; } + protected: // Not directly instantiable; construct a concrete channel type instead. USBUartChannelBase(uint8_t index, uint16_t buffer_size) : input_buffer_(RingBuffer(buffer_size)), index_(index) {} @@ -318,13 +331,17 @@ class USBUartTypeCP210X : public USBUartTypeCdcAcm { }; class USBUartTypeCH34X : public USBUartTypeCdcAcm { public: - USBUartTypeCH34X(uint16_t vid, uint16_t pid) : USBUartTypeCdcAcm(vid, pid) {} + USBUartTypeCH34X(uint16_t vid, uint16_t pid) : USBUartTypeCdcAcm(vid, pid) { + // ch34x doesn't use the interrupt endpoint, and we don't have endpoints to spare. The + // comm interface is left in place unclaimed: on the CDC-style chips it is still the + // interface a host binds, and so the one the port is identified by. + this->claim_comm_interface_ = false; + } void dump_config() override; protected: bool config_step(USBUartChannelBase *channel, uint8_t step, bool reload, bool ok, const uint8_t *response) override; bool config_device_step(uint8_t step, bool ok, const uint8_t *response) override; - std::vector parse_descriptors(usb_device_handle_t dev_hdl) override; private: CH34xChipType chiptype_{CHIP_UNKNOWN}; diff --git a/tests/components/serial_proxy/__init__.py b/tests/components/serial_proxy/__init__.py new file mode 100644 index 00000000000..913e8d8b37e --- /dev/null +++ b/tests/components/serial_proxy/__init__.py @@ -0,0 +1,16 @@ +import esphome.codegen as cg +from tests.testing_helpers import ComponentManifestOverride + + +def override_manifest(manifest: ComponentManifestOverride) -> None: + # The harness gives a multi-instance component an empty list and never calls its to_code, + # so take a single empty config instead and emit what the proxy messages and handlers + # need. No proxy is configured, so the count is 0, which keeps the capability messages as + # the api tests expect them; the USB identity stays out, since the USB channel exists on the ESP32 only. + manifest.multi_conf = False + + async def to_code_testing(config): + cg.add_define("USE_SERIAL_PROXY") + cg.add_define("SERIAL_PROXY_COUNT", 0) + + manifest.to_code = to_code_testing diff --git a/tests/components/serial_proxy/test_usb_device_descriptor.cpp b/tests/components/serial_proxy/test_usb_device_descriptor.cpp new file mode 100644 index 00000000000..c76757bbee6 --- /dev/null +++ b/tests/components/serial_proxy/test_usb_device_descriptor.cpp @@ -0,0 +1,51 @@ +#include + +#include +#include + +#include "esphome/components/api/api_buffer.h" +#include "esphome/components/api/api_pb2.h" +#include "esphome/components/api/proto.h" + +namespace esphome::api::testing { + +// Encodes into a buffer of exactly the calculated size, so any overrun trips the debug bounds check. +static std::vector encode(const UsbDeviceDescriptor &msg) { + const uint32_t size = msg.calculate_size(); + APIBuffer buf; + EXPECT_TRUE(buf.resize(size == 0 ? 1 : size)); + uint8_t *pos = buf.data(); +#ifdef ESPHOME_DEBUG_API + uint8_t *proto_debug_end_ = buf.data() + buf.size(); +#endif + uint8_t *end = UsbDeviceDescriptor::encode_msg(&msg, pos PROTO_ENCODE_DEBUG_ARG); + EXPECT_EQ(static_cast(end - buf.data()), size); + return {buf.data(), end}; +} + +TEST(UsbDeviceDescriptor, EncodesTheInterfaceClassAndPort) { + UsbDeviceDescriptor msg; + msg.vendor_id = 0x10C4; + msg.product_id = 0xEA60; + msg.bcd_device = 0x0100; + msg.interface_number = 1; + msg.interface_class = 0xFF; + msg.port_number = 2; + + const std::vector expected = {0x08, 0xC4, 0x21, 0x10, 0xE0, 0xD4, 0x03, 0x18, 0x80, + 0x02, 0x20, 0x01, 0x28, 0xFF, 0x01, 0x30, 0x02}; + EXPECT_EQ(encode(msg), expected); +} + +TEST(UsbDeviceDescriptor, ZeroClassAndPortLeaveTheWireAsBefore) { + // The new fields are additive with zero defaults, so an old client sees the message it always did + UsbDeviceDescriptor msg; + msg.vendor_id = 0x303A; + msg.product_id = 0x1001; + msg.interface_number = 1; + + const std::vector expected = {0x08, 0xBA, 0x60, 0x10, 0x81, 0x20, 0x20, 0x01}; + EXPECT_EQ(encode(msg), expected); +} + +} // namespace esphome::api::testing