Skip to content

[usb_host] Read iInterface for connected interfaces - #21781

Open
puddly wants to merge 10 commits into
esphome:devfrom
puddly:puddly/usb-description
Open

puddly wants to merge 10 commits into
esphome:devfrom
puddly:puddly/usb-description

Conversation

@puddly

@puddly puddly commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

What does this implement/fix?

Read the iInterface string descriptor for USB interfaces. Many devices use a different string for the interface description and this technically is the final piece to give ESPHome feature-parity with Linux + udev when listing serial ports.

Tested with puddly/serialx#145:

$ python -m serialx.tools.list_ports 'esphome://something.local:6053/?noise_psk=...'
SerialPortInfo(device='esphome://connect-aux-2-adad44.local:6053/?port_udev_id=usb-Nabu_Casa_ZBT-2_10B41DE58F10-if00', resolved_device='esphome://connect-aux-2-adad44.local:6053/?port_udev_id=usb-Nabu_Casa_ZBT-2_10B41DE58F10-if00', vid=12346, pid=16385, serial_number='10B41DE58F10', manufacturer='Nabu Casa', product='ZBT-2', bcd_device=257, interface_description='Nabu Casa ZBT-2', interface_num=0, port_number=None)

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • New developer-facing feature (adds functionality for component developers; no end-user configuration change)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected) — policy
  • Developer breaking change (an API change that could break external components) — policy
  • Undocumented C++ API change (removal or change of undocumented public methods that lambda users may depend on) — policy
  • Code quality improvements to existing code or addition of tests
  • Other

Related issue or feature (if applicable):

  • fixes

Pull request in esphome.io with documentation (if applicable):

Pull request in developers.esphome.io with developer documentation (if applicable):

  • esphome/developers.esphome.io#<developers.esphome.io PR number goes here>

Test Environment

  • ESP32
  • ESP32 IDF
  • ESP8266
  • RP2040/RP2350
  • BK72xx
  • RTL87xx
  • LN882x
  • nRF52840

Example entry for config.yaml:

# Example config.yaml

Checklist:

  • The code change is tested and works locally.
  • Tests have been added to verify that the new code works (under tests/ folder).

If user exposed functionality or configuration variables are added/changed:

@esphome

esphome Bot commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

To use the changes from this PR as an external component, add the following to your ESPHome configuration YAML file:

external_components:
  - source: github://pr#21781
    components: [api, serial_proxy, usb_host, usb_uart]
    refresh: 1h

(Added by the PR bot)

@codecov

codecov Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.83%. Comparing base (8401d59) to head (28e7d39).
⚠️ Report is 9 commits behind head on dev.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##              dev   #21781   +/-   ##
=======================================
  Coverage   96.83%   96.83%           
=======================================
  Files          91       91           
  Lines       21829    21829           
  Branches     3461     3461           
=======================================
  Hits        21138    21138           
  Misses        525      525           
  Partials      166      166           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@codspeed

codspeed Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Merging this PR will not alter performance

1 warning

⚠️ 21 benchmarks are dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

✅ 210 untouched benchmarks


Comparing puddly:puddly/usb-description (28e7d39) with dev (ee9ea7e)

Open in CodSpeed

@github-actions

github-actions Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Memory Impact Analysis

Components: api, serial_proxy
Platform: esp32-idf

Metric Target Branch This PR Change
RAM 90,984 bytes 90,984 bytes ➡️ +0 bytes (0.00%)
Flash 1,491,134 bytes 1,491,182 bytes 📈 🔸 +48 bytes (+0.00%)
📊 Component Memory Breakdown
Component Target Flash PR Flash Change
[esphome]api 15,343 bytes 15,369 bytes 📈 🔸 +26 bytes (+0.17%)
app_framework 6,669 bytes 6,673 bytes 📈 +4 bytes (+0.06%)
🔍 Symbol-Level Changes (click to expand)

Changed Symbols

Symbol Target Size PR Size Change
esphome::api::UsbDeviceDescriptor::dump_to(esphome::api::DumpBuffer&) const 86 bytes 98 bytes 📈 +12 bytes (+13.95%)
esphome::api::UsbDeviceDescriptor::calc_size_msg(void const*) 61 bytes 69 bytes 📈 +8 bytes (+13.11%)
esphome::api::UsbDeviceDescriptor::encode_msg(void const*, unsigned char*) 54 bytes 60 bytes 📈 +6 bytes (+11.11%)
loop_node_add_handler 219 bytes 223 bytes 📈 +4 bytes (+1.83%)

Note: This analysis measures static RAM and Flash usage only (compile-time allocation).
Dynamic memory (heap) cannot be measured automatically.
⚠️ You must test this PR on a real device to measure free heap and ensure no runtime memory issues.

This analysis runs automatically when components change. Memory usage is measured from a merged configuration with 2 components.

@puddly

puddly commented Oct 28, 2026

Copy link
Copy Markdown
Contributor Author

ESPHome Bot (@esphbot) review

@esphbot

ESPHome Bot (esphbot) commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Previous review — superseded by a newer review below.

@esphbot ESPHome Bot (esphbot) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

Important issues found.

  • Two different interface_description values do not make two clients disjoint

@puddly

puddly commented Oct 29, 2026

Copy link
Copy Markdown
Contributor Author

Regarding bot point 1: this is an architectural issue in ESPHome and can be fixed in a follow-up PR. There are a few devices on the market that do this (HUSBZB-1 has a Zigbee interface and a Z-Wave interface) but it's very rare in practice. That being said, it should be fixed. But not in this PR.

ESPHome Bot (@esphbot) Re-review.

@esphbot

Copy link
Copy Markdown
Contributor

❌ Unknown command re. Did you mean repo?

Code & Development

  • 🔎 @esphbot audit — Audit a project for optimizations, simplifications, and issues — creates tracker issues for findings
  • 🧠 @esphbot brainstorm — Break down a topic into detailed sub-issues grouped under a master issue
  • 🔎 @esphbot check_need (alias: need, needs) — Analyze whether a PR's changes or an issue's request is still relevant
  • \U0001f41b @esphbot debug (alias: dbg) — Run a structured debug loop on a failed issue
  • 🧠 @esphbot deepplan (alias: deeplan) — Deep design an idea — explores approaches, posts spec as GitHub issue, queues /plan
  • 📚 @esphbot doc (alias: docs) — Investigate a project codebase and produce structured documentation under docs/
  • 💡 @esphbot explain (alias: xp) — Explain a PR's changes in simple words with examples and alternative approaches
  • 🐞 @esphbot fix — Queue a fix mission for a GitHub or Jira issue — understand, plan, test, implement, and submit a PR. Can also batch-queue all open GitHub issues from a repo URL. Use --now to queue at the top.
  • 🔨 @esphbot implement (alias: impl) — Queue an implementation mission for a GitHub or Jira issue
  • 🧠 @esphbot plan — Plan an idea or iterate on an existing tracker issue
  • 🧠🔨 @esphbot planimplement (alias: planimp, planimpl, planit, plandoit, doit) — Queue /plan then /implement for an issue — plan insights feed the implementation
  • 📊 @esphbot profile (alias: perf, benchmark) — Queue a performance profiling mission
  • 🛠️ @esphbot refactor (alias: rf) — Queue a refactoring mission for a PR, issue, or file
  • 🔍 @esphbot review (alias: rv, rereview, re_review) — Queue a code review for one or more PRs/issues. Use --now to queue at the top. Flags: --architecture (SOLID/layering focus), --errors (silent-failure-hunter pass), --comments (comment quality), --plan-url (plan alignment check), --force (review even if closed/merged)
  • 🛡️ @esphbot security_audit (alias: security, secu) — SDLC security audit — finds critical vulnerabilities and creates tracker issues for each
  • 🏭 @esphbot speckit — Run the spec-kit SDD pipeline for a project goal or tracker issue
  • 🌿 @esphbot speckit_from_branch — Run spec-kit plan onward from a human-authored spec already pushed to a branch
  • 🔬 @esphbot ultrareview (alias: urv, ultra_review) — Queue the most thorough review Kōan can run: architecture-focused main pass + silent-failure-hunter pass in a single comment. Use --now to queue at the top.

Pull Requests

  • ❓ @esphbot ask (alias: question) — Ask a question about a PR or issue and get an AI reply posted to GitHub
  • 🔀 @esphbot gh_request — Route a natural-language GitHub request to the appropriate action (fix, rebase, review, reply, etc.)
  • 🔄 @esphbot rebase (alias: rb) — Queue a PR rebase (ex: /rebase https://github.com/owner/repo/pull/42). Bare rebase = rebase only; add --fix to also apply review feedback. Use --now to queue at the top.
  • 🔁 @esphbot recreate (alias: rc) — Recreate a diverged PR from scratch on current upstream (ex: /recreate https://github.com/owner/repo/pull/42)
  • 🔍 @esphbot reviewrebase (alias: rr) — Queue /review then /rebase --fix for a PR — review insights feed the rebase, which then applies the feedback
  • 🔄 @esphbot squash (alias: sq) — Squash PR commits into one (ex: /squash https://github.com/owner/repo/pull/42)

Usage: @esphbot <command> in any PR or issue comment.

@puddly

puddly commented Oct 29, 2026

Copy link
Copy Markdown
Contributor Author

ESPHome Bot (@esphbot) review

@esphbot

Copy link
Copy Markdown
Contributor

puddly: Regarding bot point 1: this is an architectural issue in ESPHome and can be fixed in a follow-up PR....

Agreed, deferring to follow-up. Now marked [Deferred], non-blocking. Follow-up idea: record matched interface in handle_get_desc_state_(). Then limit usb_uart channels to it. Docs should say: filter selects device, not port.

@esphbot

Copy link
Copy Markdown
Contributor

PR Review — [usb_host] Read iInterface for connected interfaces

Merge-ready. Both suggestions from the last review are fixed, and the overlap issue is deferred to a follow-up at the author's request.

Strengths:

  • handle_get_desc_state_() now stops at the first match (!this->interface_matched_ && in the loop condition).
  • read_interface_string_() now clears string_done_ before submitting.
  • Strings are read through a dedicated full-size transfer. Its data stage is rounded up to the EP0 MPS, the same way the host stack does its own descriptor reads.
  • string_result_() checks status, length, descriptor type and bLength against the bytes received.
  • The acquire/release pairing between the USB task and the main loop is correct.
  • The new protobuf field 7 is additive and covered by a gtest. I checked the expected bytes by hand and they are right.
  • copy_descriptor_string now accepts any span size. It cannot truncate the 128-byte buffers, since a string descriptor holds at most 126 characters.

Issues:

  • [Deferred] Composite devices with several iInterface strings break the overlap check. Downgraded per puddly, to be fixed in a follow-up PR.
  • on_interface_string() should bounds-check string_channel_ before indexing channels_ (a cheap guard against a narrow remaining race).

✅ Resolved since last review (1)

Previously-flagged issues verified fixed
  • esphome/components/usb_host/usb_host_client.cpp:438 Filter pass keeps reading interface strings after a match

🟢 Suggestions

1. [Deferred] Different interface_description values do not make clients disjoint on composite devices
esphome/components/usb_host/__init__.py:81-88

Downgraded per puddly: "this is an architectural issue in ESPHome and can be fixed in a follow-up PR ... it should be fixed. But not in this PR."

The problem is unchanged. _clients_overlap treats two different interface_description values as disjoint. The C++ filter, though, accepts a device when any of its interfaces carries the string, so a composite device like the HUSBZB-1 (one Zigbee interface, one Z-Wave interface) matches both clients. USBUartTypeCdcAcm::on_connected() then binds channels to parse_descriptors() results in order, so both clients claim the first CDC interface.

For the follow-up, either:

  • record the interface(s) that matched in handle_get_desc_state_() and limit channel assignment to them, or
  • count only a value-vs-wildcard difference as a separator for this key.

Until then, it would help if the docs PR (esphome/esphome.io#7947) said that this filter selects a device, not a port.

if wildcard not in (a, b) and a != b:
    return False

Checklist

  • Input validation at boundaries (config overlap validation) — suggestion #1
  • Thread safety between USB task and main loop
  • No unnecessary repeated I/O
  • Backward-compatible API/protobuf change
  • Tests cover new behavior
  • No hardcoded secrets
ℹ️ Triage summary

1 finding(s) deferred at a reviewer's request.


Automated review by Kōan (Claude) HEAD=73c4cc0 1 min 21s

@esphbot ESPHome Bot (esphbot) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tip

No blocking issues found — ready to merge.

@puddly
puddly marked this pull request as ready for review October 29, 2026 15:17
Copilot AI balanced review requested due to automatic review settings October 29, 2026 15:17
@esphome

esphome Bot commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

👋 Hi there! This PR modifies 10 file(s) with codeowners.

Keith Burzinski (@kbx81), Clyde Stubbs (@clydebarrow), @esphome/core - As codeowner(s) of the affected files, your review would be appreciated! 🙏

Note: Automatic review request may have failed, but you're still welcome to review.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved USB descriptor-transfer lifecycle bugs and unchecked allocation can cause assertions or crashes, while overlapping filters can make clients claim the same interface.

5 open issues
What changed in this PR

This PR adds USB interface descriptions to ESPHome’s USB host and serial proxy support, bringing serial-port identity reporting closer to Linux/udev.

Changes:

  • Reads and caches iInterface strings for connected USB UART channels.
  • Adds interface-description filters and exposes descriptions through the native API.
  • Extends filter-validation, component-build, and protobuf-encoding tests.
File Description
tests/​unit_tests/​components/​test_usb_host.py Adds interface-filter overlap cases.
tests/​components/​usb_uart/​common.yaml Exercises USB UART interface filtering.
tests/​components/​usb_host/​test.esp32-s3-idf.yaml Exercises interface filtering on ESP32-S3.
tests/​components/​usb_host/​test.esp32-p4-idf.yaml Exercises interface filtering on ESP32-P4.
tests/​components/​serial_proxy/​test_usb_device_descriptor.cpp Tests interface-description encoding.
esphome/​components/​usb_uart/​usb_uart.h Adds channel-description storage and read hooks.
esphome/​components/​usb_uart/​usb_uart.cpp Reads descriptions before enabling channels.
esphome/​components/​usb_host/​usb_host.h Declares descriptor-transfer state and helpers.
esphome/​components/​usb_host/​usb_host_client.cpp Implements asynchronous string reads and filtering.
esphome/​components/​usb_host/​__init__.py Adds interface-description validation and code generation.
esphome/​components/​serial_proxy/​serial_proxy.cpp Includes descriptions in port identity reports.
esphome/​components/​api/​api.proto Adds the interface-description protocol field.
esphome/​components/​api/​api_pb2.h Adds generated field storage.
esphome/​components/​api/​api_pb2.cpp Updates generated encoding and size calculation.
esphome/​components/​api/​api_pb2_dump.cpp Includes descriptions in protocol debug output.

Review effort: Balanced

Comment thread esphome/components/usb_host/__init__.py Resolved
@bdraco

Copy link
Copy Markdown
Member

esphome/backlog#193

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants