docs(dispatcher): in-depth guide to building custom modules/protocols - #789
Conversation
…rotocols Add doc/en/dispatcher/custom_modules.rst, a worked tutorial covering the stream_frame -> Dispatcher -> transport layering, the CoreDumpService module pattern, a minimal from-scratch "hello" module, payload/packing conventions (including adding typed helpers like put_f32), the USB vendor/WebUSB transport, and discovery + the browser webapp side, closing with a shipping checklist. Wire it into the dispatcher index toctree and refresh the stale module-id table in dispatcher.rst (BLDC haptics, Telemetry, MCP266 were missing). 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 documentation correctness and rendering issues remain.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds an in-depth Dispatcher custom-module/WebUSB tutorial, integrates it into the documentation, and updates module-ID references.
Changes:
- Added guidance on modules, framing, payloads, USB transport, and discovery.
- Linked the tutorial in Dispatcher documentation.
- Refreshed the module-ID table.
File summaries
| File | Summary |
|---|---|
doc/en/dispatcher/index.rst |
Links the new tutorial. |
doc/en/dispatcher/dispatcher.rst |
Updates module-ID documentation. |
doc/en/dispatcher/custom_modules.rst |
Adds the comprehensive custom protocol and WebUSB guide. |
Review details
Suppressed comments (6)
doc/en/dispatcher/custom_modules.rst:498
- The prose says this
UsbTransportcan be reused verbatim, but the block is not functional as shown: it has no constructor orreading = true, never discovers/assignsepInandepOutor claims the interface, and thereforereadLoop()exits whilesend()targets unset endpoints. Include the full implementation fromdispatcher_hub.html, or label this block as abbreviated pseudocode/reference rather than a copyable building block.
class UsbTransport {
async open(anyDevice) {
const options = anyDevice ? { filters: [] } : { filters: [{ vendorId: DEFAULT_VID }] };
this.device = await navigator.usb.requestDevice(options);
await this.device.open();
doc/en/dispatcher/custom_modules.rst:595
- The statement that every shipped module console re-runs discovery is not true for
components/telemetry/web/telemetry.html: its WebUSB path sends a module-3 schema request and its parser ignores 0xFF discovery frames. This makes the guide's claimed cross-console behavior inaccurate; qualify the list or update telemetry before using it as an example.
Your module's own console additionally re-runs the same discovery query
itself on connect (every shipped console does) so it can confirm its module
is actually present on the device it just connected to, independent of
whether the user arrived via the hub or opened the console directly.
doc/en/dispatcher/custom_modules.rst:324
- This wiring calls
usb.write_vendor()from the TinyUSB receive callback and ignores itsboolresult, but the hello payload is unconstrained (up tokMaxPayloadSize). In callback contextwrite_vendor()fails immediately when the whole frame does not currently fit, and frames larger than the FIFO are streamed non-atomically; a large valid PING can leave a partial PONG on the wire or silently drop it. Either route RX/replies through a worker task, or cap the example to the configured FIFO and handle send failure.
HelloModule hello({.send = [&](std::span<const uint8_t> frame) { usb.write_vendor(frame); }});
dispatcher.register_module(
hello_module::kModule, [&](const espp::stream_frame::Frame &f) { hello.handle(f); },
{.name = "Hello", .app = "hello_console.html", .description = "PING/PONG demo module"});
doc/en/dispatcher/custom_modules.rst:299
build()returns an empty vector when the payload exceedsstream_frame::kMaxPayloadSize, but this path passes that empty result directly tosend_, producing no valid PONG for an oversized PING and hiding the failure. Reject or cap the input before building and guard the encoded frame before sending.
send_(build(hello_module::Msg::Pong, std::span<const uint8_t>(
reinterpret_cast<const uint8_t *>(text.data()),
text.size())));
doc/en/dispatcher/custom_modules.rst:517
- This list is not accurate as written:
ota_console.htmlis also a shipped module console, while the BLDC haptics source iscomponents/bldc_haptics/example/webapp/index.htmland the docs workflow only copiescomponents/*/web/*.html; there is no hostedhaptics_console.htmlfor the hub link. Make this explicitly a hosted/partial list or fix the asset andModuleInfo::apppath before describing these as all shipped consoles.
Every shipped module console (``coredump_console.html``, ``mcp266_console.html``,
``can_bridge_console.html``, ``ds402_panel.html``, the BLDC haptics webapp,
``telemetry.html``) carries its own copy of the same three building blocks:
doc/en/dispatcher/custom_modules.rst:420
0x0d36is already used by the coredump example (components/coredump/example/main/coredump_example.cpp:107), so it is not a distinct PID as this comment claims. A custom-module tutorial should not encourage reusing an existing example's PID; use an explicitly allocated application-specific PID or omit the assignment and tell the reader to allocate one.
usb_cfg.pid = 0x0d36; // give your device a distinct PID so a webapp's WebUSB
// filter can find it specifically
- Files reviewed: 3/3 changed files
- Comments generated: 6
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…table Address remaining Copilot review comments on PR #789: - widen the dispatcher.rst module-id table's Protocol column (and both `====` separators) so the "reserved (meta); 0xFF = discovery" cell fits - fix indentation left broken by the earlier WebUSB send() autofix commit - include <cctype> and cast to unsigned char before ::toupper in the PING/PONG snippet, since frame.payload is arbitrary (not-guaranteed-ASCII) bytes and toupper(char) is UB outside the unsigned-char range - correct the claim that shipped JS console helpers are byte-for-byte copies of dispatcher_hub.html: they are wire-compatible variants that differ in correlation-id handling and (telemetry.html) helper names - add Telemetry (module 3) to the component README's module-id table to match the corrected dispatcher.rst table The "module 1 omitted" and "module 3 omitted from custom_modules.rst" comments were already fixed by the two follow-up commits on this branch (2399fd8, b0c530b). 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 4 out of 4 changed files in this pull request and generated 2 comments.
Add module id 1 (coredump example's crash trigger) to the dispatcher.rst and README.md module-id tables, annotated "example only" so it matches custom_modules.rst and doesn't read as a globally reserved/shipped id. Verified the full id set against source: 0 ota::kModule, 1 coredump_example.cpp kCrashModule, 2 haptics_usb_protocol.hpp kModule, 3 espp::Telemetry::kModule, 4 espp::CoreDumpService::kModule, 5 can_bridge_protocol.hpp kModuleId, 6 mcp266_webapp_example kModuleId. Reword the hello_module walkthrough comment that said CoreDumpService "owns its own parser and mutex" — that's true of feed()'s standalone byte-stream path, not why a Dispatcher-routed handler skips the feed()/handle_frame() split. Clarify the split exists so CoreDumpService can also run off a raw stream and serialize flash access, neither of which HelloModule needs. 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 4 out of 4 changed files in this pull request and generated 3 comments.
- Bind the built reply frame to a local variable before passing it to send_(), and note that send_fn's span is only valid for the duration of the call so a callee must copy bytes it needs to retain. - Replace the hard-coded example USB PID with an explicit placeholder that must be swapped for the project's allocated PID. - Make the module-id tables consistent across custom_modules.rst, dispatcher.rst, and README.md: 0xF0-0xFE reserved (meta) as its own row, separate from the 0xFF capability discovery row. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
Keep this branch's wording for the WDI README dependency table and manifest description: the device-role PR said the host-role headers arrive in a follow-up, and this PR is that follow-up. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
Summary
doc/en/dispatcher/custom_modules.rst, an in-depth, worked tutorial for building a custom module/protocol on top ofespp::Dispatcher+espp::stream_frame, and pairing it with a browser web app over the USB vendor (WebUSB) transport.doc/en/dispatcher/index.rst's toctree (and adds a pointer to it from the index blurb).doc/en/dispatcher/dispatcher.rst, which was missing BLDC haptics (2), Telemetry (3), and MCP266 (6).What the guide covers
components/dispatcher/README.md).espp::CoreDumpService(components/coredump/include/coredump_service.hpp) as the canonical five-part shape: module id,Msgenum,Config{send_fn},build(), and thefeed()/handle_frame()lock-then-send-unlocked pattern — then a minimal from-scratch "hello" PING/PONG module built purely fromDispatcher/stream_frameAPIs (no new component/example file added).put_u16/put_u32/get_u16/get_u32, thereplybit vs.type-value conventions used differently by CoreDumpService/Telemetry/MCP266, optional correlation ids, and adding a typed helper (Telemetry::put_f32, cited verbatim fromcomponents/telemetry/include/telemetry.hpp).espp::UsbDevice::VendorFunction,write_vendor()/set_vendor_receive_callback(), Kconfig (CONFIG_TINYUSB_VENDOR_COUNT=1), the RX-queue-to-worker-task pattern fromcoredump_example.cpp, and the browser-sideUsbTransport/crc32/buildFrame/StreamParserJS building blocks shared (verified via grep) across every shipped module console.ModuleInfo,set_device_info()/serve_discovery(), thedescribe()TLV payload layout, and howdispatcher_hub.htmldiscovers and safely links a module's hosted webapp (safeAppName()).Every API name/signature/path was verified against the current source (
dispatcher.hpp,stream_frame.hpp,coredump_service.hpp,usb_device.hpp,telemetry.hpp,mcp266_protocol.hpp, the example.cppfiles, anddispatcher_hub.html) rather than invented.Constraints followed
.rstfiles with a docutils structural parse (tables, section targets, cross-references) — clean, no warnings. A full Sphinx (esp-docs/Doxygen) build was not run (heavy toolchain not set up in this environment).Test plan
docutils.core.publish_doctreeparse of all three.rstfiles — no warnings/errors (tables, backtick-underscore section links, directives all resolve).doc/toolchain set up may want topip install -r doc/requirements.txt && cd doc/en && sphinx-build -b html . _buildto render it.🤖 Generated with Claude Code
https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU