feat(serial_plotter): binary telemetry service + WebUSB transport (Phase 2) - #783
feat(serial_plotter): binary telemetry service + WebUSB transport (Phase 2)#783finger563 wants to merge 5 commits into
Conversation
…example Phase 2 of the Serial Plotter: a firmware-side binary telemetry transport the web app can plot over WebUSB / Web Serial, alongside the existing text/CSV path. - include/telemetry_service.hpp: espp::Telemetry, a small device->host protocol on the stream_frame framing (dispatcher module 3). Firmware declares named float channels (SCHEMA) and pushes SAMPLE frames (device timestamp + one float per channel, batchable); host requests are GET_SCHEMA and SET_STREAM (enable/disable + rate). Follows the CoreDumpService pattern: build frames under a mutex, invoke the user send callback with the lock released; usable with a Dispatcher (handle()) or standalone (feed()). - CMakeLists.txt / idf_component.yml: the component now ships firmware, so it registers include/ and depends on base_component + stream_frame again. - example/: an esp32s3 app that streams four synthetic channels over USB vendor + CDC, wires a Dispatcher per transport (module 3 + discovery advertising app="serial_plotter.html"), and honors GET_SCHEMA / SET_STREAM. The web-app WebUSB transport that decodes SCHEMA/SAMPLE into the same uPlot plot lands in a follow-up commit. Verified: the example builds clean against ESP-IDF v6.1 (esp32s3). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
…p + docs Adds a WebUSB transport to serial_plotter.html that plots the espp::Telemetry binary stream, alongside the existing text/CSV Web Serial path. - A new "USB" button connects over WebUSB (claims the vendor 0xFF interface's bulk IN/OUT pair), requests the schema (GET_SCHEMA), and starts the stream (SET_STREAM). It decodes stream_frame frames (vendored codec: magic/flags/ crc32 matching components/stream_frame), routes module 3, maps SCHEMA -> series and SAMPLE -> the same ring buffers / uPlot plot, using the device timestamp (u32 microseconds, unwrapped) as the X axis. Serial and USB are mutually exclusive; pause/clear/filter/save/modes all work over USB too. - Docs: a serial_plotter component doc page (Telemetry API via include-build-file) + index + example include, registered in the main toctree and the Doxygen INPUT list; web_apps.rst and the README/meta note the WebUSB path. Verified: the JS crc32 matches the C++ golden (0xCBF43926); schema/sample frames built + parsed + decoded correctly (headless); the app loads clean with the USB button and the CSV/serial path still plots (browser). Live WebUSB I/O uses the same UsbTransport pattern as the other espp consoles. 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
telemetry_service.hpp has likely C++20 build breaks (missing direct includes and std::vector passed to a std::span parameter) that should be fixed before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds Phase 2 of the serial_plotter component by introducing a firmware-side binary telemetry service (espp::Telemetry) and a matching WebUSB transport in the existing browser plotter, plus docs and an ESP32-S3 USB streaming example.
Changes:
- Add
espp::Telemetryheader-only service usingstream_frame(module 3) with SCHEMA/SAMPLE and GET_SCHEMA/SET_STREAM handling. - Extend
serial_plotter.htmlwith a WebUSB transport that decodesstream_frametelemetry and feeds the existing plotting pipeline. - Add example + documentation wiring (Sphinx + Doxygen) and update component metadata to include firmware dependencies.
File summaries
| File | Description |
|---|---|
| doc/en/web_apps.rst | Mentions Serial Plotter’s new WebUSB binary telemetry path in the web apps overview. |
| doc/en/serial_plotter/serial_plotter.rst | New component docs page describing Web Serial vs WebUSB and the Telemetry protocol. |
| doc/en/serial_plotter/serial_plotter_example.md | Adds docs include of the example README. |
| doc/en/serial_plotter/index.rst | Adds Serial Plotter section entry to the docs tree. |
| doc/en/index.rst | Registers Serial Plotter docs section in the main docs toctree. |
| doc/Doxyfile | Adds Telemetry header + example source to Doxygen inputs/example paths. |
| components/serial_plotter/web/serial_plotter.html | Implements WebUSB connect + stream_frame parsing and telemetry schema/sample decoding. |
| components/serial_plotter/README.md | Updates README to describe both transports and the Telemetry firmware path. |
| components/serial_plotter/include/telemetry_service.hpp | Introduces the espp::Telemetry firmware protocol/service implementation. |
| components/serial_plotter/idf_component.yml | Adds firmware deps and example metadata now that the component ships code. |
| components/serial_plotter/example/sdkconfig.defaults | Configures ESP32-S3 USB + TinyUSB vendor/CDC settings for telemetry streaming. |
| components/serial_plotter/example/README.md | Documents how to build/use the USB telemetry example with the web app. |
| components/serial_plotter/example/main/telemetry_example.cpp | Implements ESP32-S3 demo producing synthetic channels over vendor + CDC with Dispatcher routing. |
| components/serial_plotter/example/main/CMakeLists.txt | Registers the example’s main component requirements. |
| components/serial_plotter/example/CMakeLists.txt | Adds a managed-deps-friendly example project configuration and component search path narrowing. |
| components/serial_plotter/CMakeLists.txt | Updates component registration to export include/ and depend on base_component + stream_frame. |
Review details
Suppressed comments (2)
components/serial_plotter/include/telemetry_service.hpp:180
- send_fn expects a std::span, but this call passes a std::vector<uint8_t>, which can fail to compile under C++20. Pass an explicit span over the vector storage.
s(frame);
components/serial_plotter/include/telemetry_service.hpp:283
- send_fn expects a std::span, but send_frame passes a std::vector<uint8_t>. This can fail to compile under C++20; pass an explicit span over the vector bytes.
if (s)
s(frame);
- Files reviewed: 16/16 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.
Copilot review + cppcheck fixes on the Telemetry service: - Include <algorithm> and <string_view> directly (used by std::min / std::string_view) instead of relying on transitive includes. - Pass an explicit std::span<const uint8_t> over the vector at every send_fn call site, so it compiles under C++20 (no reliance on C++23 span-from-range). - Cap the channel count at 255 (kMaxChannels) in the constructor and set_channels, and derive the SCHEMA channel count and the serialized channels from the same bound, so a >255-channel config can't emit a malformed frame (the count is a u8). Warns when truncating. - Example: return early if usb.initialize() fails instead of running the tasks with no host transport. - static analysis (cppcheck functionConst): make send_schema, send_frame, send_ok, and send_error const (none mutate state; Logger::warn is const, warn_rate_limited is not, so emit stays non-const). Verified: builds clean on ESP-IDF v6.1 (esp32s3); cppcheck reports no findings on the header. 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
WebUSB lifecycle and pause behavior, CDC compatibility, schema bounds, concurrency guarantees, and CI coverage need correction.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (6)
Previously missed (6) — in code that hasn't changed since the last review.
components/serial_plotter/example/main/telemetry_example.cpp:52
- This binary CDC path is not consumable by the Serial Plotter web app: its Web Serial loop decodes every chunk as UTF-8 and calls
feedText()(serial_plotter.html:764-799), while onlyUsbTransportfeedsSfParser. Consequently selecting the example's CDC port cannot request a schema or plot telemetry. Either add a binary Web Serial mode or remove the CDC telemetry path and its claims.
components/serial_plotter/include/telemetry_service.hpp:91 - Concurrent
emit()and request handling can invoke this callback simultaneously because each path releasesmutex_before sending. The type does not document thatsendmust be thread-safe, despite promising concurrent-safe use; a transport without the example's separate TX mutex can race or interleave frames. Serialize callback invocations internally, or explicitly require callers to provide a serialized/thread-safe callback.
components/serial_plotter/web/serial_plotter.html:946 open()can fail after the device has been opened or an interface claimed, but this catch dropstwithout closing it. That leaves the WebUSB device/interface open and can make the next connection attempt fail; closetbefore returning, as the other web transports do.
components/serial_plotter/web/serial_plotter.html:958SET_STREAMis sent only during connect/disconnect. The shared Pause handler merely togglespausedand drops incoming samples, so a WebUSB device continues transmitting while paused and is never re-enabled through this request on resume. SendSET_STREAMwith enabled=0/1 when Pause changes in USB mode (while retaining local-only pause for text serial).
components/serial_plotter/example/CMakeLists.txt:7- This raises the example to C++23 even though the repository standard and the directly related examples use C++20 (
components/dispatcher/example/CMakeLists.txt:22,components/stream_frame/example/CMakeLists.txt:21, andcomponents/usb_device/example/CMakeLists.txt:35). This code needs no C++23 feature, so the higher setting unnecessarily narrows compiler/ESP-IDF compatibility.
components/serial_plotter/example/CMakeLists.txt:49 - The new firmware example is absent from
.github/workflows/build.yml; the matrix jumps fromseeed-studio-round-displaytoserialization. Add an alphabetically placedcomponents/serial_plotter/exampleentry targetingesp32s3(using the manager-disabled command if intended) so this firmware path is continuously built.
- Files reviewed: 16/16 changed files
- Comments generated: 2
- Review effort level: Balanced
| explicit Telemetry(const Config &config) | ||
| : BaseComponent("Telemetry", config.log_level) | ||
| , channels_(config.channels) | ||
| , send_(config.send) | ||
| , streaming_(config.stream_on_start) | ||
| , period_ms_(config.period_ms) { | ||
| clamp_channels(); |
| const uint8_t len = static_cast<uint8_t>(std::min<size_t>(name.size(), 255)); | ||
| p.push_back(len); | ||
| p.insert(p.end(), name.begin(), name.begin() + len); |
- Example is now WebUSB-only: the web app's Web Serial path parses text/CSV, so a binary-telemetry CDC interface it can't consume was misleading. Drop the CDC function (device enumerates just the vendor interface) and update the README / docs. The Telemetry service stays transport-agnostic (CDC/UART/socket work); only the demo is WebUSB. (CDC stays enabled in sdkconfig because usb_device includes the TinyUSB CDC header unconditionally, but no CDC interface is made.) - Serialize the send callback internally (a dedicated send_mutex_ + deliver()), so concurrent emit()/request handling can't interleave two frames' bytes even if the transport's send isn't itself thread-safe; document it. - WebUSB: close the transport if open() fails partway (don't leave the device claimed), and send SET_STREAM(0/1) on Pause/Resume so the device actually stops/starts streaming (text serial keeps local-only pause). - Example builds at C++20 (repo standard; no C++23 feature needed) and is added to the CI build matrix (esp32s3, manager-off). - static analysis: pass send_frame's frame by const ref (passedByValue). Verified: example builds clean on ESP-IDF v6.1 (esp32s3, gnu++20); cppcheck reports no findings; the web app parses. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
…SB-only) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
Phase 2 of the Serial Plotter (#781): a firmware-side binary telemetry transport the web app plots over WebUSB, alongside the existing text/CSV Web Serial path.
Firmware:
espp::Telemetry(include/telemetry_service.hpp)A small device→host protocol on the
stream_frameframing (dispatcher module 3), so it shares one USB vendor / CDC stream with other modules viaDispatcher. Firmware declares namedfloatchannels (SCHEMA) and pushes SAMPLE frames — a device timestamp (u32 µs) + one float per channel, batchable — withemit(...). Host requests:GET_SCHEMA,SET_STREAM(enable/disable + rate).Follows the
CoreDumpServicepattern: frames built under a mutex, thesendcallback invoked with the lock released; usable with aDispatcher(handle()) or standalone (feed()).emit()and request handling are concurrency-safe.The component now ships firmware, so it registers
include/and depends onbase_component+stream_frameagain (the deps removed when it was webapp-only).Example (
example/)An esp32s3 app streams four synthetic channels (
sine,cosine,noise,ramp) over USB vendor (WebUSB) + CDC (Web Serial), wires aDispatcherper transport (module 3 + discovery advertisingapp="serial_plotter.html"), and honorsGET_SCHEMA/SET_STREAM.Web app (
web/serial_plotter.html)A new USB button connects over WebUSB (claims the vendor
0xFFinterface's bulk IN/OUT pair), requests the schema, starts the stream, decodesstream_frameframes (vendored codec matchingcomponents/stream_frame), and maps SCHEMA → series and SAMPLE → the same ring buffers / uPlot plot, using the device timestamp as X. Serial and USB are mutually exclusive; pause / clear / filter / save / 2D / 3D modes all work over USB too.Docs
New
doc/en/serial_plotter/component page (Telemetry API viainclude-build-file) + example include, registered in the main toctree and the Doxygen INPUT list;web_apps.rst, the README, and the app<meta>note the WebUSB path.Testing
example/builds clean against ESP-IDF v6.1 (esp32s3) — image generated.crc32matches the C++ golden (0xCBF43926); SCHEMA/SAMPLE frames build + parse + decode correctly (headless).UsbTransportpattern as the other espp consoles (dispatcher_hub.html); an on-device WebUSB smoke test is the one thing not exercisable in CI.🤖 Generated with Claude Code