[usb_uart] Expose interface_class and port_number (#20659)

Co-authored-by: J. Nick Koston <nick@home-assistant.io>

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

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) {

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);

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 {

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);

esphome/components/usb_uart/ch34x.cpp

@@ -152,14 +152,6 @@ bool USBUartTypeCH34X::config_step(USBUartChannelBase *channel, uint8_t step, bo
   }
 }
 
-std::vector<CdcEps> 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 ||

esphome/components/usb_uart/cp210x.cpp

@@ -88,11 +88,11 @@ std::vector<CdcEps> 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;
 }

esphome/components/usb_uart/ft23xx.cpp

@@ -217,6 +217,7 @@ static optional<CdcEps> 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;
 }
 

esphome/components/usb_uart/pl2303.cpp

@@ -189,7 +189,10 @@ std::vector<CdcEps> 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
     }
   }

esphome/components/usb_uart/usb_uart.cpp

@@ -41,14 +41,17 @@ static optional<CdcEps> 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);

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 Parented<USBUartCo
                                                              : this->cdc_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<CdcEps> parse_descriptors(usb_device_handle_t dev_hdl) override;
 
  private:
   CH34xChipType chiptype_{CHIP_UNKNOWN};

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

tests/components/serial_proxy/test_usb_device_descriptor.cpp

@@ -0,0 +1,51 @@
+#include <gtest/gtest.h>
+
+#include <cstdint>
+#include <vector>
+
+#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<uint8_t> 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<uint32_t>(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<uint8_t> 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<uint8_t> expected = {0x08, 0xBA, 0x60, 0x10, 0x81, 0x20, 0x20, 0x01};
+  EXPECT_EQ(encode(msg), expected);
+}
+
+}  // namespace esphome::api::testing