feat(ota): Python OTA-over-USB host tool + idf.py ota-usb build target - #784
feat(ota): Python OTA-over-USB host tool + idf.py ota-usb build target#784finger563 wants to merge 8 commits into
Conversation
…d target
Adds `espp_ota`, a host CLI/library that updates an espp device over USB using
the existing stream_frame framing + OTA stream protocol (dispatcher module 0) —
the same protocol the ota example serves and ota_console.html drives from the
browser. No device-side changes; this is the missing host counterpart.
Package (components/ota/python/espp_ota/), layered like espp_odrive:
- frame.py — stream_frame v2 codec (stdlib only): build_frame + a
resynchronizing StreamParser. Verified against the C++ golden
CRC (crc32("123456789")==0xCBF43926).
- protocol.py — OTA opcodes (Begin/Data/Finish/Abort -> Ok/Error/Progress) +
dispatcher discovery request.
- transport.py — pyusb vendor-interface transport (discovers the 0xFF interface
and its bulk IN/OUT endpoints; VID/PID default 0x1209:0x0d32).
pyusb is imported lazily so the codec/protocol stay stdlib-only.
- client.py — the session driver (BEGIN -> chunked DATA -> FINISH), one
request in flight, progress callback, PROGRESS/ERROR handling.
- cli.py — `python -m espp_ota {flash,list,discover}`; VID/PID/serial via
flags or ESPP_OTA_* env vars.
Seamless build -> OTA: components/ota/project_include.cmake registers an
`ota-usb` build target (the OTA counterpart to `idf.py flash`), so any project
using the ota component can run `idf.py ota-usb` to build the app and flash it
over USB in one step. The build dependency on gen_project_binary is wired via a
deferred call (that target is created after project_include runs); on CMake <3.19
`idf.py build ota-usb` is the explicit form.
Packaging: espp_ota is added to the espp wheel (wheel.packages) with a `usb`
extra (pyusb) and an `espp-ota` console script, mirroring espp_odrive.
Verified: codec + full-OTA-against-a-mock-device host tests pass with plain
python3; the CMake target + deferred app-build dependency validated in an
isolated harness (ninja runs the build step before the flash step); graceful
error when pyusb is absent. On-device USB flashing needs hardware.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
|
✅Static analysis result - no issues found! ✅ |
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved moderate findings affect OTA cleanup, reply validation, USB lifecycle and error handling, timeout propagation, and serial filtering.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a pure-Python OTA-over-USB host tool and integrates it with ESP-IDF through an idf.py ota-usb target.
Changes:
- Adds framing, protocol, PyUSB transport, client, and CLI layers.
- Packages the tool with an optional
pyusbdependency and console entry point. - Adds documentation, host tests, and deferred CMake integration.
File summaries
| File | Reviewed changes and findings |
|---|---|
pyproject.toml |
Packages espp_ota, its USB extra, and CLI entry point. |
components/ota/README.md |
Documents build-to-OTA usage. |
components/ota/python/tests/test_ota_host.py |
Adds mock-device OTA tests. Nit (2 votes): add independent CRC golden and resynchronization fixtures. |
components/ota/python/README.md |
Documents installation, CLI, library, and protocol usage. |
components/ota/python/espp_ota/transport.py |
Implements PyUSB transport. Moderate: address configuration selection, driver reattachment, failed-open cleanup, USB error conversion, and timeout classification. |
components/ota/python/espp_ota/protocol.py |
Defines OTA messages and payload helpers. |
components/ota/python/espp_ota/frame.py |
Implements frame encoding and parsing. |
components/ota/python/espp_ota/client.py |
Implements OTA sessions and discovery. Moderate: add abort cleanup, validate reply/discovery frames, preserve original errors during abort, and pass transaction timeouts to writes. |
components/ota/python/espp_ota/cli.py |
Provides CLI commands. Moderate (1 vote): apply --serial to device listing. |
components/ota/python/espp_ota/__main__.py |
Enables module execution. |
components/ota/python/espp_ota/__init__.py |
Exposes the public API. |
components/ota/project_include.cmake |
Registers the ota-usb build target. |
Review details
Suppressed comments (9)
components/ota/python/espp_ota/cli.py:111
- The
listparser exposes--serial, but this call dropsargs.serial, soespp-ota list --serial ...prints every matching VID/PID device instead of applying the documented selector. Thread the serial predicate throughlist_devices(or filter each device by serial) before producing the list.
found = list_devices(vid=args.vid, pid=pid)
components/ota/python/espp_ota/client.py:110
- This method is marked best-effort but suppresses only
OtaError. A USB transport can raiseTransportError(or another I/O exception), so failure cleanup throughabort()can itself escape and mask the original flash error; suppress transport-side exceptions here as well.
def abort(self) -> None:
try:
self._transact(_p.make_abort(), self._data_to)
except OtaError:
pass # best-effort
components/ota/python/espp_ota/client.py:61
- The timeout selected by each transaction is not used for the USB write: BEGIN and FINISH calls pass their 60-second timeouts into
_transact, but this line always uses the DATA timeout instead. Passtimeout_mshere so the transport honors the timeout configured for the current operation.
self._t.write(request, timeout_ms=self._data_to)
components/ota/python/espp_ota/transport.py:110
open()assumes that libusb already selected a configuration and callsget_active_configuration()immediately. A freshly enumerated device can have no active configuration, causing the CLI to fail before it discovers the vendor interface; the browser path explicitly selects configuration 1 when needed. Handle the no-active-configuration case by selecting the device's configuration before walking its interfaces.
cfg = dev.get_active_configuration()
components/ota/python/espp_ota/transport.py:158
open()may detach a kernel driver at lines 115-120, butclose()only releases the interface and disposes resources. libusb does not automatically reattach a detached Linux kernel driver, so closing after a flash can leave that interface unavailable until the device is replugged; track whether detachment succeeded and reattach it after release.
if self._claimed and self._itf_num is not None:
self._util.release_interface(self._dev, self._itf_num)
except Exception:
pass
try:
components/ota/python/espp_ota/transport.py:113
- After
_devis assigned, an exception fromget_active_configuration(), interface discovery, orclaim_interface()exitsopen()without cleanup._cmd_flashonly closes after_open_transport()succeeds, and__enter__does not call__exit__whenopen()fails, so a failed open can leak USB resources and leave a detached kernel driver unrecovered.
cfg = dev.get_active_configuration()
itf, ep_in, ep_out = self._find_vendor_interface(cfg)
self._itf_num = itf.bInterfaceNumber
self._ep_in, self._ep_out = ep_in, ep_out
components/ota/python/espp_ota/transport.py:185
- Non-timeout USB errors are re-raised as the backend's
USBError, butcli.main()catches onlyOtaErrorandTransportError. A disconnect or access failure during a read therefore escapes as a traceback instead of the CLI's actionable error path; wrap non-timeout failures inTransportErrorhere.
except self._core.USBError as exc:
# errno 110 == ETIMEDOUT; pyusb>=1.1 raises the USBTimeoutError subclass.
if getattr(exc, "errno", None) in (110, None) or "timeout" in str(exc).lower():
return b""
raise
components/ota/python/espp_ota/transport.py:175
Endpoint.write()can also raise a rawUSBError, which bypasses the CLI'sTransportErrorhandler and produces a traceback instead of the normal actionable error path. Convert the endpoint exception toTransportError, matching the read-side error contract.
def write(self, data: bytes, timeout_ms: int = 5000) -> None:
n = self._ep_out.write(data, timeout_ms)
if n != len(data):
raise TransportError(f"short write: {n}/{len(data)} bytes")
components/ota/python/espp_ota/transport.py:183
- Treating
errno is Noneas a timeout hides non-timeoutUSBErrorinstances, since backend/disconnect errors can also omit an errno. The client then spins until the deadline and reports a misleading reply timeout instead of surfacing the transport failure; only accept a timeout-specific errno/subclass/message here.
if getattr(exc, "errno", None) in (110, None) or "timeout" in str(exc).lower():
- Files reviewed: 12/12 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
client.py: - _transact now requires the reply flag (fr.is_reply) on module-0 frames, so a device-originated request whose type collides with OK/ERROR/PROGRESS can't be mistaken for a reply (matches the browser probe). - flash() wraps the BEGIN/DATA/FINISH session in best-effort ABORT cleanup on any failure (device ERROR, timeout, Ctrl-C, transport error) before re-raising, so the device doesn't stay "busy" and reject a retry's BEGIN. abort() now swallows all errors (the link may already be gone). - discover() reads until the actual discovery reply (module 0xFF, reply flag, ListModules type) instead of returning the first frame seen. cli.py: open the transport with a `with` statement (context manager) instead of try/finally in the flash and discover commands. transport.py: comment the three best-effort empty-except blocks (kernel-driver detach, interface release, resource dispose). tests: add byte-level golden fixtures (CRC vector 0xCBF43926, exact BEGIN(0) and discovery request frames) and StreamParser resync fixtures (leading garbage, a CRC-corrupted frame, a split frame) so wire-encoding / resync correctness is asserted independently of the mock loopback. Host tests pass with python3. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
Feedback from on-device testing: - Progress only appeared after the flash finished when run via `idf.py ota-usb`, because idf.py captures the child's stderr (not a TTY) and the carriage-return in-place bar never flushed until the final newline. _ProgressBar now detects a non-TTY stderr and emits throttled newline-terminated "OTA NN%" lines that flush live through the capturing parent; it still draws the in-place bar on a real TTY. - Errors were a plain hard-to-spot line. They now print with a distinct "espp_ota ERROR:" prefix (stands out even in idf.py's plain capture) and in red+bold on a color terminal (respects NO_COLOR / FORCE_COLOR / CLICOLOR_FORCE). No protocol/transport change; host tests still pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
Replaces the plain per-line progress with a proper UI (new ui.py), mirroring the `rich` library that ships in the ESP-IDF Python env: - On a real terminal: a rich progress bar (spinner, bar, %, bytes, transfer speed, ETA) and colorized status/success/error lines. - Under `idf.py ota-usb` (output captured, not a TTY): rich's live display can't animate through idf.py's line capture, so we emit throttled lines ending in "(NN %)" — the pattern idf.py re-renders in place, exactly how esptool's progress shows under `idf.py flash`. Result is a live in-place text bar there. - No rich installed: degrades to plain text. Errors keep the distinct "espp_ota ERROR:" prefix and are red+bold when color is supported. `rich` added to the `usb` extra (`pip install "espp[usb]"`); it's already present in the IDF env so `idf.py ota-usb` has it for free. The frame/protocol/transport layers are unchanged; codec + resync + full-OTA host tests still pass. (Protocol speed-up is a separate, backwards-compatible follow-up.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
The previous attempt relied on idf.py re-rendering lines ending in "(NN %)" in place, but idf.py only does that for its own build progression (needs force_progression + its stdout being a tty), so under `idf.py ota-usb` the lines just scrolled. idf.py captures the target's stdout/stderr as pipes, but the process still has a controlling terminal. So the progress bar now opens that terminal directly (/dev/tty, or CONOUT$ on Windows) and renders the rich bar there, bypassing the capture — a real animated bar under idf.py. Falls back to the tty stream with a manual \r bar when rich is absent, and to periodic plain lines when there is no terminal at all (CI / fully redirected). Status/error messages still go to stderr (which idf.py forwards, with color). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
- The "Connected to <vid>:<pid>" line is now a prominent bold-cyan status line (Console.note) on its own, with the flash target + human-readable size beneath it; the transport description spells out "(interface N)". - Add the on-device idf.py ota-usb screenshots (mid-flash progress bar and the completed flash) to the tool README and the ota docs page. Images are hosted GitHub user-content, not committed to the repo. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.
- pyproject: `rich` is no longer forced by the `usb` extra. `usb = ["pyusb"]` again; a new `usb-ui = ["pyusb", "rich"]` adds the nicer UI (docs updated). - frame.py: StreamParser.reset() now also clears dropped_bytes so a reused parser fully resets. - client.py: _transact() uses its `timeout_ms` argument for the write side too, so BEGIN/FINISH writes get their own (larger) timeout instead of the DATA one. - transport.py: read() only treats a genuine timeout (USBTimeoutError, or errno 110/ETIMEDOUT) as empty; other USBError I/O failures propagate instead of being swallowed by the previous `errno is None` catch-all. - project_include.cmake: prepend the package dir to PYTHONPATH (host path separator) instead of overwriting an existing PYTHONPATH. - ui.py: comment the two best-effort empty-except blocks. Host tests pass with python3. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
Add inline explanations to the two `except Exception: pass` blocks the code-quality check still flagged (client.abort() best-effort cleanup, and the isatty() guard in ui._open_progress_stream()). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
There was a problem hiding this comment.
🟡 Changes recommended
Retry recovery and routine USB-error handling have unresolved failure paths.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 14/14 changed files
- Comments generated: 3
- Review effort level: Balanced
| except (OtaError, TransportError) as exc: | ||
| CON.error(exc) | ||
| return 1 | ||
| except FileNotFoundError as exc: | ||
| CON.error(exc) | ||
| return 2 |
| # The device keeps its OTA session across a host disconnect, so a failed | ||
| # or interrupted flash would leave it "busy" and reject a retry's BEGIN. | ||
| # On any failure (device ERROR, timeout, Ctrl-C, transport error) send a | ||
| # best-effort ABORT to release the session before propagating. | ||
| try: | ||
| self._transact(_p.make_begin(size), self._begin_to) |
| * **Standalone, real terminal** — if `rich` is available (it ships in the | ||
| ESP-IDF Python environment, and `pip install "espp[usb]"` pulls it in) we draw | ||
| a `rich` progress bar (spinner, bar, %, bytes, transfer speed, ETA) and print |
Adds
espp_ota, a pure-Python host tool that OTA-updates an espp device overUSB using the existing
stream_frameframing + OTA stream protocol (dispatchermodule 0) — the same protocol the
otaexampleserves and
ota_console.htmldrives fromthe browser. No device-side changes; this is the missing host counterpart, plus
a build-system hook so it's painless.
Seamless: build → OTA with one command
components/ota/project_include.cmakeregisters anota-usbbuild target, so anyproject that uses the
otacomponent gets the OTA counterpart toidf.py flash:The build dependency on
gen_project_binaryis wired via a deferred call (thattarget is created after
project_include.cmakeruns); on CMake < 3.19 theexplicit
idf.py build ota-usbform works. Device/port overrides come fromESPP_OTA_*env vars.The tool (
components/ota/python/espp_ota/)Layered like
espp_odrive:frame.py—stream_framev2 codec + resynchronizingStreamParser(stdlib only).protocol.py— OTA opcodes (Begin/Data/Finish/Abort → Ok/Error/Progress).transport.py—pyusbvendor-interface transport (discovers the 0xFFinterface + its bulk IN/OUT endpoints; VID/PID default
0x1209:0x0d32).pyusbis imported lazily so the codec/protocol stay stdlib-only.
client.py— the session driver (one request in flight, progress, error handling).cli.py—python -m espp_ota {flash,list,discover}.Also shipped in the espp wheel (
wheel.packages) with ausbextra (pyusb)and an
espp-otaconsole script.Testing
components/ota/python/tests/test_ota_host.py, plainpython3):codec round-trips + resync, CRC against the C++ golden
0xCBF43926, and afull OTA against a mock device (correct chunking, progress, FINISH, and the
ERROR path).
harness (
ninja ota-usbruns the build step before the flash step).pyusbis absent.🤖 Generated with Claude Code