From 68ee5a2f049d443d3d384ef316765af7b39a8513 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Mon, 31 Aug 2026 22:09:07 -0500 Subject: [PATCH 01/10] feat(canopen): USB<->CAN bridge example + WebUSB/Web Serial CAN console Adds a new can_bridge_example that turns an ESP32-S3 into a WebUSB / Web Serial CAN interface, and the hosted CAN console web app that drives it. - Firmware bridges the Twai (CAN 2.0) controller to the host over USB using the stream_frame framing and an espp::Dispatcher (this example owns module id 5). The same framed protocol is exposed on BOTH the vendor interface (WebUSB) and a CDC interface (Web Serial); device->host frames go to whichever transport the host last used. The system console stays on USB-Serial-JTAG. - Protocol (can_bridge_protocol.hpp): CAN_TX / SET_CONFIG / START / STOP / GET_STATUS requests (0x5X) and CAN_RX / OK / ERROR / STATUS replies (0xDX); a CAN frame encodes as [id u32][flags u8][dlc u8][data]. Supports normal (master, ACK) and listen-only (passive sniff) modes; the bus starts stopped and the host configures baudrate/mode then starts it. - Web app (components/canopen/web/can_console.html): dual WebUSB/Web Serial connect, bus config + live status/counters, a send-frame panel with full id/ext/rtr/dlc/hex validation, and a capped live RX+TX monitor table with pause/clear/autoscroll. Self-contained single file; node --check clean. The DS402 canopen_example is unchanged. Example builds clean (esp32s3) and is added to the CI build matrix; the web app is auto-hosted from components/*/web/. Co-Authored-By: Claude Fable 5 --- .github/workflows/build.yml | 2 + .../canopen/can_bridge_example/CMakeLists.txt | 34 + .../canopen/can_bridge_example/README.md | 62 + .../can_bridge_example/main/CMakeLists.txt | 5 + .../main/can_bridge_example.cpp | 293 +++++ .../main/can_bridge_protocol.hpp | 111 ++ .../can_bridge_example/sdkconfig.defaults | 31 + components/canopen/web/can_console.html | 1064 +++++++++++++++++ doc/en/buses/canopen.rst | 11 + 9 files changed, 1613 insertions(+) create mode 100644 components/canopen/can_bridge_example/CMakeLists.txt create mode 100644 components/canopen/can_bridge_example/README.md create mode 100644 components/canopen/can_bridge_example/main/CMakeLists.txt create mode 100644 components/canopen/can_bridge_example/main/can_bridge_example.cpp create mode 100644 components/canopen/can_bridge_example/main/can_bridge_protocol.hpp create mode 100644 components/canopen/can_bridge_example/sdkconfig.defaults create mode 100644 components/canopen/web/can_console.html diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 1341f95019..756f68a10d 100755 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -101,6 +101,8 @@ jobs: target: esp32s3 - path: 'components/canopen/example' target: esp32 + - path: 'components/canopen/can_bridge_example' + target: esp32s3 - path: 'components/chsc6x/example' target: esp32s3 - path: 'components/cdr/example' diff --git a/components/canopen/can_bridge_example/CMakeLists.txt b/components/canopen/can_bridge_example/CMakeLists.txt new file mode 100644 index 0000000000..5c7547c13e --- /dev/null +++ b/components/canopen/can_bridge_example/CMakeLists.txt @@ -0,0 +1,34 @@ +# The following lines of boilerplate have to be in your project's CMakeLists +# in this exact order for cmake to work correctly +cmake_minimum_required(VERSION 3.20) + +# NOTE: the IDF component manager is intentionally left ENABLED here (unlike most +# espp examples) so it can fetch the managed `espressif/esp_tinyusb` dependency +# declared by the usb_device component. To avoid the manager scanning every espp +# component manifest (some board components declare target-specific constraints +# that would fail on esp32s3), EXTRA_COMPONENT_DIRS is narrowed to just the +# components this example uses; the in-repo espp components there satisfy the +# `espp/*` dependencies locally. +include($ENV{IDF_PATH}/tools/cmake/project.cmake) + +set(EXTRA_COMPONENT_DIRS + "../../../components/base_component" + "../../../components/dispatcher" + "../../../components/format" + "../../../components/logger" + "../../../components/stream_frame" + "../../../components/task" + "../../../components/twai" + "../../../components/usb_device" +) + +set( + COMPONENTS + "main esptool_py base_component dispatcher format logger stream_frame task twai usb_device esp_tinyusb" + CACHE STRING + "List of components to include" + ) + +project(can_bridge_example) + +set(CMAKE_CXX_STANDARD 20) diff --git a/components/canopen/can_bridge_example/README.md b/components/canopen/can_bridge_example/README.md new file mode 100644 index 0000000000..77be4d8935 --- /dev/null +++ b/components/canopen/can_bridge_example/README.md @@ -0,0 +1,62 @@ +# USB <-> CAN Bridge Example + +Turns an ESP32-S3 into a **WebUSB / Web Serial CAN interface**: the hosted +[CAN console web app](https://esp-cpp.github.io/espp/apps/can_console.html) +connects over the native USB and can + +- **send** CAN frames as a normal, ACK-ing bus participant ("master"), and +- **inspect** the bus — stream every received frame; in *listen-only* mode the + node is a passive sniffer that never ACKs or transmits. + +It bridges the ESP32-S3 TWAI (CAN 2.0) controller to the host over USB using the +espp `stream_frame` framing and an `espp::Dispatcher` (this example owns +**module id 5**). The same framed protocol is exposed on both the USB **vendor** +interface (WebUSB) and a **CDC** interface (Web Serial), so the web app can use +either transport. The system console/logs stay on the separate built-in +USB-Serial-JTAG. + +## Wiring + +Connect the TWAI TX/RX GPIOs to a CAN transceiver (e.g. SN65HVD230, TJA1050) on +a properly terminated (120 Ω) bus. Defaults (change in `can_bridge_example.cpp`): + +| Signal | GPIO | +|--------|------| +| TWAI TX | 5 | +| TWAI RX | 4 | + +Listen-only mode monitors an existing bus without a transceiver ACKing, but a +transceiver is still required to receive the differential signal. + +## Protocol (module 5) + +Framed with `stream_frame` (magic `OT` / type / len / crc32) and routed by +`espp::Dispatcher`. Host→device requests use type high-nibble 5; device→host +replies/events use high-nibble D. + +| Type | Dir | Meaning | +|------|-----|---------| +| `0x50` CAN_TX | H→D | transmit a CAN frame | +| `0x51` SET_CONFIG | H→D | `[baudrate u32][mode u8][rsv u8]` (mode 0=normal, 1=listen-only) | +| `0x52` START | H→D | bring the bus up with the current config | +| `0x53` STOP | H→D | take the bus down | +| `0x54` GET_STATUS | H→D | request a STATUS reply | +| `0xD0` CAN_RX | D→H | a received CAN frame | +| `0xD1` OK | D→H | ack | +| `0xD2` ERROR | D→H | `[code u32][utf8 message]` | +| `0xD3` STATUS | D→H | `[baudrate u32][mode u8][running u8][rx u32][tx u32][err u32]` | + +A CAN frame is encoded as `[id u32][flags u8][dlc u8][data: dlc bytes]`, where +`flags` bit0 = extended (29-bit) and bit1 = RTR. + +The bus starts **stopped**: the host sets baudrate/mode with `SET_CONFIG`, then +`START`. `SET_CONFIG` is rejected while the bus is running (stop first). + +## Build & flash + +``` +idf.py set-target esp32s3 +idf.py build flash monitor +``` + +Then open the CAN console web app and Connect (WebUSB or Web Serial). diff --git a/components/canopen/can_bridge_example/main/CMakeLists.txt b/components/canopen/can_bridge_example/main/CMakeLists.txt new file mode 100644 index 0000000000..4d9c19253e --- /dev/null +++ b/components/canopen/can_bridge_example/main/CMakeLists.txt @@ -0,0 +1,5 @@ +idf_component_register( + SRC_DIRS "." + INCLUDE_DIRS "." + REQUIRES usb_device twai dispatcher stream_frame task logger +) diff --git a/components/canopen/can_bridge_example/main/can_bridge_example.cpp b/components/canopen/can_bridge_example/main/can_bridge_example.cpp new file mode 100644 index 0000000000..9758780708 --- /dev/null +++ b/components/canopen/can_bridge_example/main/can_bridge_example.cpp @@ -0,0 +1,293 @@ +// USB <-> CAN (TWAI) bridge example. +// +// Turns an ESP32-S3 into a WebUSB / Web Serial CAN interface: the hosted CAN +// console web app connects over the USB vendor interface and can +// - send CAN frames (as a normal, ACK-ing bus participant — "master"), and +// - inspect the bus (stream every received frame; in listen-only mode the +// node is a passive sniffer that never ACKs/transmits). +// +// The vendor stream is framed with the espp stream_frame codec and routed by an +// espp::Dispatcher; this example owns module id 5 (see can_bridge_protocol.hpp). +// A CDC interface carries the system console/logs. Wire the TX/RX GPIOs to a CAN +// transceiver (e.g. SN65HVD230) on a terminated bus. + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "dispatcher.hpp" +#include "logger.hpp" +#include "stream_frame.hpp" +#include "task.hpp" +#include "twai.hpp" +#include "usb_device.hpp" + +#include "can_bridge_protocol.hpp" + +using namespace std::chrono_literals; +namespace sf = espp::stream_frame; + +// TWAI GPIOs — change to match your board / transceiver wiring. +static constexpr int kCanTxGpio = 5; +static constexpr int kCanRxGpio = 4; + +extern "C" void app_main(void) { + espp::Logger logger({.tag = "CAN Bridge", .level = espp::Logger::Verbosity::INFO}); + logger.info("Starting USB<->CAN bridge example"); + + // --- USB: vendor (WebUSB) for the framed protocol + CDC for the console ---- + espp::UsbDevice::Config usb_cfg; + usb_cfg.manufacturer = "espp"; + usb_cfg.product = "espp CAN Bridge"; + usb_cfg.log_level = espp::Logger::Verbosity::WARN; + espp::UsbDevice::VendorFunction vendor; + vendor.interface_name = "espp CAN Bridge (WebUSB)"; + vendor.webusb = true; + vendor.landing_page_url = "esp-cpp.github.io/espp/apps/can_console.html"; + usb_cfg.vendor = vendor; + // The CDC interface carries the SAME framed protocol as the vendor interface, + // so the web app can connect over Web Serial as well as WebUSB. (The system + // console/logs go to the built-in USB-Serial-JTAG, kept separate — see + // sdkconfig.defaults.) + espp::UsbDevice::CdcFunction cdc; + cdc.interface_name = "espp CAN Bridge (CDC)"; + usb_cfg.cdc = cdc; + espp::UsbDevice usb(usb_cfg); + + // Device -> host frames go to whichever transport the host last talked on + // (only one is connected at a time). Device -> host sends come from TWO tasks + // (the RX worker's request replies and the TWAI receive task's streamed + // frames), so serialize them. + enum class Transport { Vendor, Cdc }; + std::atomic active_transport{Transport::Vendor}; + std::mutex tx_mutex; + auto send = [&](std::span bytes) { + std::lock_guard lock(tx_mutex); + if (active_transport.load() == Transport::Cdc) + usb.write_cdc(bytes); + else + usb.write_vendor(bytes); + }; + auto send_frame = [&](uint8_t type, std::span payload = {}) { + send(sf::build_frame(type, payload)); + }; + auto reply_error = [&](const std::error_code &ec, const std::string &context) { + std::vector p; + sf::put_u32(p, static_cast(ec.value())); + const std::string msg = context + ": " + ec.message(); + p.insert(p.end(), msg.begin(), msg.end()); + send_frame(can_bridge::kError, p); + }; + + // --- CAN bus state (recreated on START so baudrate/mode can change) --------- + std::mutex bus_mutex; // guards twai + config below + std::unique_ptr twai; + uint32_t baudrate = 500000; + uint8_t mode = can_bridge::kModeNormal; + std::atomic rx_count{0}, tx_count{0}, err_count{0}; + + // TWAI receive task context: stream each frame to the host as CAN_RX. + auto on_can_rx = [&](const espp::Twai::Message &m) { + can_bridge::CanFrame f; + f.id = m.id; + f.extended = m.extended; + f.rtr = m.rtr; + f.dlc = m.dlc; + f.data = m.data; + rx_count.fetch_add(1); + send_frame(can_bridge::kCanRx, can_bridge::encode_frame(f)); + }; + auto on_can_err = [&](twai_error_flags_t) { err_count.fetch_add(1); }; + + auto send_status = [&]() { + std::vector p; + bool running; + { + std::lock_guard lock(bus_mutex); + running = static_cast(twai); + sf::put_u32(p, baudrate); + p.push_back(mode); + p.push_back(running ? 1 : 0); + } + sf::put_u32(p, rx_count.load()); + sf::put_u32(p, tx_count.load()); + sf::put_u32(p, err_count.load()); + send_frame(can_bridge::kStatus, p); + }; + + auto start_bus = [&](std::error_code &ec) -> bool { + std::lock_guard lock(bus_mutex); + if (twai) + return true; // already running + espp::Twai::Config cfg; + cfg.tx_gpio = kCanTxGpio; + cfg.rx_gpio = kCanRxGpio; + cfg.baudrate = baudrate; + cfg.mode = (mode == can_bridge::kModeListenOnly) ? espp::Twai::Mode::LISTEN_ONLY + : espp::Twai::Mode::NORMAL; + cfg.auto_start = false; // start() explicitly so we get an error code + cfg.on_receive = on_can_rx; + cfg.on_error = on_can_err; + auto node = std::make_unique(cfg); + if (!node->start(ec)) + return false; // node destructs, uninstalling the driver + twai = std::move(node); + return true; + }; + auto stop_bus = [&]() { + std::lock_guard lock(bus_mutex); + if (twai) { + std::error_code ec; + twai->stop(ec); + twai.reset(); + } + }; + + // --- Dispatcher: CAN bridge protocol on module id 5 ------------------------ + espp::Dispatcher dispatcher; + dispatcher.register_module( + can_bridge::kModuleId, [&](uint8_t type, std::span payload) { + std::error_code ec; + switch (type) { + case can_bridge::kCanTx: { + can_bridge::CanFrame f; + if (!can_bridge::decode_frame(payload, f)) { + reply_error(std::make_error_code(std::errc::invalid_argument), "malformed CAN_TX"); + break; + } + std::lock_guard lock(bus_mutex); + if (!twai) { + reply_error(std::make_error_code(std::errc::not_connected), "bus not started"); + break; + } + espp::Twai::Message m; + m.id = f.id; + m.extended = f.extended; + m.rtr = f.rtr; + m.dlc = f.dlc; + m.data = f.data; + if (twai->transmit(m, ec)) { + tx_count.fetch_add(1); + send_frame(can_bridge::kOk); + } else { + reply_error(ec, "transmit failed"); + } + break; + } + case can_bridge::kSetConfig: { + if (payload.size() < 6) { + reply_error(std::make_error_code(std::errc::invalid_argument), + "SET_CONFIG needs u32 baudrate + u8 mode + u8 reserved"); + break; + } + { + std::lock_guard lock(bus_mutex); + if (twai) { + reply_error(std::make_error_code(std::errc::device_or_resource_busy), + "stop the bus before reconfiguring"); + break; + } + baudrate = sf::get_u32(payload); + mode = payload[4]; + } + send_frame(can_bridge::kOk); + send_status(); + break; + } + case can_bridge::kStart: + if (start_bus(ec)) { + send_frame(can_bridge::kOk); + send_status(); + } else { + reply_error(ec, "start failed"); + } + break; + case can_bridge::kStop: + stop_bus(); + send_frame(can_bridge::kOk); + send_status(); + break; + case can_bridge::kGetStatus: + send_status(); + break; + default: + reply_error(std::make_error_code(std::errc::not_supported), "unknown CAN bridge message"); + break; + } + }); + + // --- USB RX plumbing: queue in the TinyUSB task, dispatch from a worker ----- + // transmit() can block up to its timeout, so it must not run in the TinyUSB + // callback context. + std::mutex rx_mutex; + std::condition_variable rx_cv; + std::deque> rx_queue; + size_t rx_queued_bytes = 0; + bool rx_overflow = false; + static constexpr size_t kMaxQueuedRxBytes = 8 * sf::kMaxFrameSize; + auto enqueue_rx = [&](Transport source, std::span data) { + active_transport.store(source); // reply on the transport the host is using + { + std::lock_guard lock(rx_mutex); + if (rx_queued_bytes + data.size() > kMaxQueuedRxBytes) { + rx_queue.clear(); + rx_queued_bytes = 0; + rx_overflow = true; + } else { + rx_queue.emplace_back(data.begin(), data.end()); + rx_queued_bytes += data.size(); + } + } + rx_cv.notify_one(); + }; + usb.set_vendor_receive_callback( + [&](std::span data) { enqueue_rx(Transport::Vendor, data); }); + usb.set_cdc_receive_callback( + [&](std::span data) { enqueue_rx(Transport::Cdc, data); }); + + std::error_code usb_ec; + if (!usb.initialize(usb_ec)) + logger.error("Failed to initialize USB device: {}", usb_ec.message()); + + espp::Task rx_task({.callback = [&](std::mutex &, std::condition_variable &) -> bool { + std::deque> chunks; + bool overflowed = false; + { + std::unique_lock lock(rx_mutex); + rx_cv.wait_for(lock, 100ms, + [&] { return !rx_queue.empty() || rx_overflow; }); + std::swap(chunks, rx_queue); + rx_queued_bytes = 0; + overflowed = rx_overflow; + rx_overflow = false; + } + if (overflowed) { + // Bytes were dropped: a frame straddling the gap + // would be stitched incorrectly, so resync. + dispatcher.reset(); + return false; + } + for (const auto &chunk : chunks) + dispatcher.feed(chunk); + return false; // keep running + }, + .task_config = {.name = "can_bridge_rx", .stack_size_bytes = 8192}}); + rx_task.start(); + + logger.info("CAN bridge ready. Connect the CAN console web app over WebUSB."); + logger.info("Bus starts stopped; the host sets baudrate/mode (SET_CONFIG) then START."); + + // Idle; all work happens in the TWAI receive task and the RX worker. + while (true) { + std::this_thread::sleep_for(1s); + } +} diff --git a/components/canopen/can_bridge_example/main/can_bridge_protocol.hpp b/components/canopen/can_bridge_example/main/can_bridge_protocol.hpp new file mode 100644 index 0000000000..c0faefb27d --- /dev/null +++ b/components/canopen/can_bridge_example/main/can_bridge_protocol.hpp @@ -0,0 +1,111 @@ +#pragma once + +// Wire protocol for the USB <-> CAN (TWAI) bridge example. +// +// Framed with the espp stream_frame codec and routed by espp::Dispatcher on +// MODULE ID 5 (message-type high nibble 5 for host->device requests, 0xD for +// device->host replies/events — the 0x80 reply bit sits in the high nibble, so +// both map to dispatcher module 5). The hosted CAN console web app speaks this +// exact protocol over WebUSB / Web Serial. +// +// A CAN frame is encoded as a compact payload: +// [id u32 LE][flags u8][dlc u8][data: dlc bytes] +// flags: bit0 = extended (29-bit id), bit1 = remote-transmission-request (RTR) +// so a frame payload is 6..14 bytes (dlc 0..8). + +#include +#include +#include +#include + +#include "stream_frame.hpp" + +namespace can_bridge { + +/// Dispatcher module id owned by the CAN bridge protocol. +static constexpr uint8_t kModuleId = 5; + +/// Host -> device (requests, high nibble 5). +enum : uint8_t { + kCanTx = 0x50, ///< transmit a CAN frame (payload: encoded CanFrame) + kSetConfig = 0x51, ///< set bus config (payload: u32 baudrate, u8 mode, u8 reserved) + kStart = 0x52, ///< bring the bus up with the current config (no payload) + kStop = 0x53, ///< take the bus down (no payload) + kGetStatus = 0x54, ///< request the current config + counters (no payload) +}; + +/// Device -> host (replies / events, high nibble D). +enum : uint8_t { + kCanRx = 0xD0, ///< a received CAN frame (payload: encoded CanFrame) + kOk = 0xD1, ///< ack for a request (payload: empty) + kError = 0xD2, ///< failure (payload: u32 code + UTF-8 message) + kStatus = 0xD3, ///< status reply (see StatusPayload below) +}; + +/// Bus mode requested by SET_CONFIG. +enum : uint8_t { + kModeNormal = 0, ///< actively participate (ACK frames) — "master" / send+receive + kModeListenOnly = 1, ///< passive monitor (never ACK/transmit) — "inspection" +}; + +/// CAN-frame flag bits used in the encoded payload. +enum : uint8_t { + kFlagExtended = 0x01, ///< 29-bit extended identifier + kFlagRtr = 0x02, ///< remote-transmission-request frame (no data) +}; + +/// A decoded classic-CAN (2.0) frame (mirrors espp::Twai::Message fields). +struct CanFrame { + uint32_t id{0}; + bool extended{false}; + bool rtr{false}; + uint8_t dlc{0}; + std::array data{}; +}; + +/// Encode a CanFrame to its wire payload: [id u32][flags u8][dlc u8][data]. +inline std::vector encode_frame(const CanFrame &f) { + std::vector p; + const uint8_t dlc = f.dlc > 8 ? 8 : f.dlc; + p.reserve(6 + dlc); + espp::stream_frame::put_u32(p, f.id); + uint8_t flags = 0; + if (f.extended) + flags |= kFlagExtended; + if (f.rtr) + flags |= kFlagRtr; + p.push_back(flags); + p.push_back(dlc); + // RTR frames carry no data; otherwise append dlc data bytes. + if (!f.rtr) + p.insert(p.end(), f.data.begin(), f.data.begin() + dlc); + return p; +} + +/// Decode a CAN-frame wire payload. Returns false if malformed (too short, dlc +/// out of range, or fewer data bytes than dlc for a non-RTR frame). +inline bool decode_frame(std::span payload, CanFrame &out) { + if (payload.size() < 6) + return false; + out.id = espp::stream_frame::get_u32(payload); + const uint8_t flags = payload[4]; + const uint8_t dlc = payload[5]; + out.extended = (flags & kFlagExtended) != 0; + out.rtr = (flags & kFlagRtr) != 0; + if (dlc > 8) + return false; + out.dlc = dlc; + out.data = {}; + if (!out.rtr) { + if (payload.size() < static_cast(6) + dlc) + return false; + for (uint8_t i = 0; i < dlc; ++i) + out.data[i] = payload[6 + i]; + } + return true; +} + +// Status reply payload layout (kStatus): +// [baudrate u32][mode u8][running u8][rx_count u32][tx_count u32][err_count u32] + +} // namespace can_bridge diff --git a/components/canopen/can_bridge_example/sdkconfig.defaults b/components/canopen/can_bridge_example/sdkconfig.defaults new file mode 100644 index 0000000000..fc50e6354e --- /dev/null +++ b/components/canopen/can_bridge_example/sdkconfig.defaults @@ -0,0 +1,31 @@ +# The USB vendor / WebUSB + CDC transports use the native USB-OTG peripheral, +# available on the ESP32-S3 (also S2 / P4) -- NOT the classic ESP32. Pin the +# target so a bare `idf.py build` does not fall back to esp32. +CONFIG_IDF_TARGET="esp32s3" + +# Common ESP-related +CONFIG_ESP_SYSTEM_EVENT_TASK_STACK_SIZE=4096 +CONFIG_ESP_MAIN_TASK_STACK_SIZE=8192 + +# Keep the system console/logs on the built-in USB-Serial-JTAG peripheral so it +# stays separate from the native USB-OTG (vendor / CDC) interfaces created by +# espp::UsbDevice for the framed CAN protocol. (On an S3 devkit these are two +# distinct USB connectors.) +CONFIG_ESP_CONSOLE_USB_SERIAL_JTAG=y + +# Enable the TinyUSB vendor + CDC class drivers. The vendor interface carries +# the framed protocol for WebUSB; the CDC interface carries the SAME framed +# protocol for Web Serial. HID count is set only so the usb_device component's +# HID code paths compile (no HID interface is instantiated here). esp_tinyusb +# gates each class behind these counts. +CONFIG_TINYUSB_VENDOR_COUNT=1 +CONFIG_TINYUSB_CDC_ENABLED=y +CONFIG_TINYUSB_CDC_COUNT=1 +CONFIG_TINYUSB_HID_COUNT=1 + +# Give the vendor RX/TX FIFOs headroom for CAN-RX frame bursts on a busy bus. +CONFIG_TINYUSB_VENDOR_RX_BUFSIZE=2048 +CONFIG_TINYUSB_VENDOR_TX_BUFSIZE=2048 + +# C++ +CONFIG_COMPILER_CXX_EXCEPTIONS=y diff --git a/components/canopen/web/can_console.html b/components/canopen/web/can_console.html new file mode 100644 index 0000000000..4649a1a1e3 --- /dev/null +++ b/components/canopen/web/can_console.html @@ -0,0 +1,1064 @@ + + + + + + espp CAN Bridge Console (WebUSB / Web Serial) + + + + + +
+
+

espp CAN Bridge Console (WebUSB / Web Serial)

+ Disconnected +
+ +
+ This browser supports neither WebUSB nor Web Serial. Use a Chromium-based + browser (Chrome / Edge / Opera) on a secure origin (https, localhost or file://). +
+ + +
+

Device

+
+ + + + +
+

Not connected. WebUSB default filter: VID 0x1209 / PID 0x0d32 (espp default); the vendor (0xFF) interface is discovered from the descriptors at runtime.

+
+ + +
+

Bus configuration

+
+
+ Baud rate + +
+
+ Mode + +
+
+ + + +
+
+ Bus: unknown + Baud: - + Mode: - + RX: 0 + TX: 0 + Err: 0 +
+
+ + +
+

Send frame

+
+
+ ID (hex) + +
+ + +
+ DLC (blank = auto) + +
+
+ Data (hex bytes, e.g. "DE AD BE EF") + +
+ +
+ +
+ + +
+

Monitor

+
+ + + + +
+ 0 frames (0 dropped) +
+
+ + + + + + + +
TimeDirIDTypeDLCDataASCII
+
+ +
+ + +
+

Log

+
+ + +
+
+
+ +
+ Speaks the espp stream_frame protocol (magic "OT" + type + len + + payload + CRC-32) carrying the USB<->CAN bridge message set. Works over + WebUSB (vendor 0xFF interface) or Web Serial; unrecognized frame types on the + stream are ignored so the bridge can multiplex other protocols. +
+
+ + + + diff --git a/doc/en/buses/canopen.rst b/doc/en/buses/canopen.rst index f207ef278a..47e0d758c3 100644 --- a/doc/en/buses/canopen.rst +++ b/doc/en/buses/canopen.rst @@ -35,6 +35,17 @@ performing SDO transfers; with `Twai` this is automatically the case since its canopen_example +USB <-> CAN bridge +------------------- + +The ``can_bridge_example`` (``components/canopen/can_bridge_example``) turns an +ESP32-S3 into a WebUSB / Web Serial CAN interface: it bridges the `Twai` +(CAN 2.0) controller to the host over USB using the ``stream_frame`` framing and +an :doc:`../dispatcher/index` (module id 5), so the hosted +`CAN console `_ web app can +send frames (as a bus master) and inspect the bus (streaming every received +frame, optionally in passive listen-only mode) directly from a Chromium browser. + .. ---------------------------- API Reference ---------------------------------- API Reference From 5626ab51c1a6b31f63f743fb10c22bfcc71b6bf5 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Tue, 1 Sep 2026 07:35:01 -0500 Subject: [PATCH 02/10] refactor(canopen): migrate CAN bridge to v2 stream_frame framing + address review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rebased onto the v2 stream_frame/dispatcher branch and migrated the CAN bridge: - send_frame() now builds v2 frames on module 5, deriving the reply flag from the type's high bit (0xD_ replies); the dispatcher handler takes the whole stream_frame::Frame (module already routed). - Protocol header comment corrected: every frame is module 5 with the type in the type byte and the reply flag set for 0xD_ replies (the old 'high nibble' wording described the retired v1 nibble scheme — the PR comment). - can_console.html migrated to the v2 9-byte header (module 5). Also addresses the other review comments: - top comment fixed: both vendor (WebUSB) and CDC (Web Serial) carry the framed protocol; the console/logs go to USB-Serial-JTAG (not CDC). - build.yml: can_bridge_example ordered before canopen/example (alphabetical). can_bridge_example builds clean (esp32s3); webapp node --check clean and the GET_STATUS/CAN_RX frames verified. Co-Authored-By: Claude Fable 5 --- .github/workflows/build.yml | 4 +- .../main/can_bridge_example.cpp | 152 +++++++++--------- .../main/can_bridge_protocol.hpp | 11 +- components/canopen/web/can_console.html | 69 +++++--- 4 files changed, 131 insertions(+), 105 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 756f68a10d..21d5b33aea 100755 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -99,10 +99,10 @@ jobs: target: esp32 - path: 'components/byte90/example' target: esp32s3 - - path: 'components/canopen/example' - target: esp32 - path: 'components/canopen/can_bridge_example' target: esp32s3 + - path: 'components/canopen/example' + target: esp32 - path: 'components/chsc6x/example' target: esp32s3 - path: 'components/cdr/example' diff --git a/components/canopen/can_bridge_example/main/can_bridge_example.cpp b/components/canopen/can_bridge_example/main/can_bridge_example.cpp index 9758780708..718e5e1368 100644 --- a/components/canopen/can_bridge_example/main/can_bridge_example.cpp +++ b/components/canopen/can_bridge_example/main/can_bridge_example.cpp @@ -6,10 +6,12 @@ // - inspect the bus (stream every received frame; in listen-only mode the // node is a passive sniffer that never ACKs/transmits). // -// The vendor stream is framed with the espp stream_frame codec and routed by an -// espp::Dispatcher; this example owns module id 5 (see can_bridge_protocol.hpp). -// A CDC interface carries the system console/logs. Wire the TX/RX GPIOs to a CAN -// transceiver (e.g. SN65HVD230) on a terminated bus. +// Both the vendor (WebUSB) and CDC (Web Serial) interfaces carry the SAME +// framed protocol (espp stream_frame codec, routed by an espp::Dispatcher; this +// example owns module id 5 — see can_bridge_protocol.hpp), so the web app can +// connect over either transport. The system console/logs go to the separate +// built-in USB-Serial-JTAG. Wire the TX/RX GPIOs to a CAN transceiver (e.g. +// SN65HVD230) on a terminated bus. #include #include @@ -78,7 +80,10 @@ extern "C" void app_main(void) { usb.write_vendor(bytes); }; auto send_frame = [&](uint8_t type, std::span payload = {}) { - send(sf::build_frame(type, payload)); + // Reply/event types (kCanRx/kOk/kError/kStatus) carry the high bit; map it + // to the frame reply flag. All CAN-bridge frames are module kModuleId. + const bool reply = (type & 0x80) != 0; + send(sf::build_frame(reply, can_bridge::kModuleId, type, payload)); }; auto reply_error = [&](const std::error_code &ec, const std::string &context) { std::vector p; @@ -154,76 +159,77 @@ extern "C" void app_main(void) { // --- Dispatcher: CAN bridge protocol on module id 5 ------------------------ espp::Dispatcher dispatcher; - dispatcher.register_module( - can_bridge::kModuleId, [&](uint8_t type, std::span payload) { - std::error_code ec; - switch (type) { - case can_bridge::kCanTx: { - can_bridge::CanFrame f; - if (!can_bridge::decode_frame(payload, f)) { - reply_error(std::make_error_code(std::errc::invalid_argument), "malformed CAN_TX"); - break; - } - std::lock_guard lock(bus_mutex); - if (!twai) { - reply_error(std::make_error_code(std::errc::not_connected), "bus not started"); - break; - } - espp::Twai::Message m; - m.id = f.id; - m.extended = f.extended; - m.rtr = f.rtr; - m.dlc = f.dlc; - m.data = f.data; - if (twai->transmit(m, ec)) { - tx_count.fetch_add(1); - send_frame(can_bridge::kOk); - } else { - reply_error(ec, "transmit failed"); - } - break; - } - case can_bridge::kSetConfig: { - if (payload.size() < 6) { - reply_error(std::make_error_code(std::errc::invalid_argument), - "SET_CONFIG needs u32 baudrate + u8 mode + u8 reserved"); - break; - } - { - std::lock_guard lock(bus_mutex); - if (twai) { - reply_error(std::make_error_code(std::errc::device_or_resource_busy), - "stop the bus before reconfiguring"); - break; - } - baudrate = sf::get_u32(payload); - mode = payload[4]; - } - send_frame(can_bridge::kOk); - send_status(); - break; - } - case can_bridge::kStart: - if (start_bus(ec)) { - send_frame(can_bridge::kOk); - send_status(); - } else { - reply_error(ec, "start failed"); - } - break; - case can_bridge::kStop: - stop_bus(); - send_frame(can_bridge::kOk); - send_status(); - break; - case can_bridge::kGetStatus: - send_status(); - break; - default: - reply_error(std::make_error_code(std::errc::not_supported), "unknown CAN bridge message"); + dispatcher.register_module(can_bridge::kModuleId, [&](const espp::stream_frame::Frame &frame) { + const uint8_t type = frame.type; + std::span payload = frame.payload; + std::error_code ec; + switch (type) { + case can_bridge::kCanTx: { + can_bridge::CanFrame f; + if (!can_bridge::decode_frame(payload, f)) { + reply_error(std::make_error_code(std::errc::invalid_argument), "malformed CAN_TX"); + break; + } + std::lock_guard lock(bus_mutex); + if (!twai) { + reply_error(std::make_error_code(std::errc::not_connected), "bus not started"); + break; + } + espp::Twai::Message m; + m.id = f.id; + m.extended = f.extended; + m.rtr = f.rtr; + m.dlc = f.dlc; + m.data = f.data; + if (twai->transmit(m, ec)) { + tx_count.fetch_add(1); + send_frame(can_bridge::kOk); + } else { + reply_error(ec, "transmit failed"); + } + break; + } + case can_bridge::kSetConfig: { + if (payload.size() < 6) { + reply_error(std::make_error_code(std::errc::invalid_argument), + "SET_CONFIG needs u32 baudrate + u8 mode + u8 reserved"); + break; + } + { + std::lock_guard lock(bus_mutex); + if (twai) { + reply_error(std::make_error_code(std::errc::device_or_resource_busy), + "stop the bus before reconfiguring"); break; } - }); + baudrate = sf::get_u32(payload); + mode = payload[4]; + } + send_frame(can_bridge::kOk); + send_status(); + break; + } + case can_bridge::kStart: + if (start_bus(ec)) { + send_frame(can_bridge::kOk); + send_status(); + } else { + reply_error(ec, "start failed"); + } + break; + case can_bridge::kStop: + stop_bus(); + send_frame(can_bridge::kOk); + send_status(); + break; + case can_bridge::kGetStatus: + send_status(); + break; + default: + reply_error(std::make_error_code(std::errc::not_supported), "unknown CAN bridge message"); + break; + } + }); // --- USB RX plumbing: queue in the TinyUSB task, dispatch from a worker ----- // transmit() can block up to its timeout, so it must not run in the TinyUSB diff --git a/components/canopen/can_bridge_example/main/can_bridge_protocol.hpp b/components/canopen/can_bridge_example/main/can_bridge_protocol.hpp index c0faefb27d..cd6cf077d6 100644 --- a/components/canopen/can_bridge_example/main/can_bridge_protocol.hpp +++ b/components/canopen/can_bridge_example/main/can_bridge_protocol.hpp @@ -2,11 +2,12 @@ // Wire protocol for the USB <-> CAN (TWAI) bridge example. // -// Framed with the espp stream_frame codec and routed by espp::Dispatcher on -// MODULE ID 5 (message-type high nibble 5 for host->device requests, 0xD for -// device->host replies/events — the 0x80 reply bit sits in the high nibble, so -// both map to dispatcher module 5). The hosted CAN console web app speaks this -// exact protocol over WebUSB / Web Serial. +// Framed with the espp stream_frame v2 codec and routed by espp::Dispatcher on +// MODULE ID 5. Every frame sets module = 5; the `type` field carries the +// message id below (host->device request opcodes 0x5X, device->host +// reply/event opcodes 0xD_), and the frame's reply flag is set for the 0xD_ +// replies (build derives it from the type's high bit). The hosted CAN console +// web app speaks this exact protocol over WebUSB / Web Serial. // // A CAN frame is encoded as a compact payload: // [id u32 LE][flags u8][dlc u8][data: dlc bytes] diff --git a/components/canopen/web/can_console.html b/components/canopen/web/can_console.html index 4649a1a1e3..cf032a773c 100644 --- a/components/canopen/web/can_console.html +++ b/components/canopen/web/can_console.html @@ -13,25 +13,29 @@ the device over EITHER WebUSB (vendor 0xFF interface, bulk IN/OUT) OR Web Serial (navigator.serial). Both transports carry the same frames. - Wire format (all multi-byte fields LITTLE-ENDIAN): - [magic u16 = 0x4F54 "OT"][type u8][len u32][payload...][crc32 u32] - - magic bytes on the wire: 0x54 ('T') then 0x4F ('O'). + Wire format v2 (all multi-byte fields LITTLE-ENDIAN): + [magic u16 = 0x4F54 "OT"][flags u8][module u8][type u8][len u32][payload...][crc32 u32] + - magic bytes on the wire: 0x54 ('T') then 0x4F ('O'). Header is 9 bytes. + - flags u8: bit0 = reply (0 host->device request, 1 device->host + reply/event); bits 4-7 = protocol version = 1. So a request byte = 0x10, + a reply = 0x11. + - module u8: the CAN bridge is module 5. All CAN frames use module 5. - crc32 is the standard zlib / IEEE-802.3 CRC-32 (poly 0xEDB88320 - reflected, init/final xor 0xFFFFFFFF) over the 7 header bytes + payload. + reflected, init/final xor 0xFFFFFFFF) over the 9 header bytes + payload. Golden check value: crc32("123456789") === 0xCBF43926. - - the device may interleave frames of OTHER protocols (e.g. OTA) on the - same stream; the parser passes through / ignores any type it does not - recognize instead of erroring. + - the device may interleave frames of OTHER protocols/modules (e.g. OTA) on + the same stream; the parser passes through / ignores any frame whose + module/type it does not recognize instead of erroring. - CAN bridge message types - ------------------------ - Host -> device: + CAN bridge message types (module 5) + ----------------------------------- + Host -> device (request, reply bit 0): 0x50 CAN_TX transmit a CAN frame. payload = encoded CAN frame 0x51 SET_CONFIG configure the bus. payload = [baud u32][mode u8][rsv u8=0] 0x52 START bring the bus up. no payload 0x53 STOP take the bus down. no payload 0x54 GET_STATUS request a STATUS reply. no payload - Device -> host: + Device -> host (reply/event, reply bit 1): 0xD0 CAN_RX a received CAN frame. payload = encoded CAN frame 0xD1 OK ack for a request. no payload 0xD2 ERROR [code u32][utf8 message...] @@ -346,10 +350,11 @@

Log

- Speaks the espp stream_frame protocol (magic "OT" + type + len + - payload + CRC-32) carrying the USB<->CAN bridge message set. Works over - WebUSB (vendor 0xFF interface) or Web Serial; unrecognized frame types on the - stream are ignored so the bridge can multiplex other protocols. + Speaks the espp stream_frame v2 protocol (magic "OT" + flags + + module + type + len + payload + CRC-32) carrying the USB<->CAN bridge + message set on module 5. Works over WebUSB (vendor 0xFF interface) or Web + Serial; frames for other modules/types on the stream are ignored so the + bridge can multiplex other protocols.
@@ -362,9 +367,14 @@

Log

const DEFAULT_VID = 0x1209, DEFAULT_PID = 0x0d32; const VENDOR_CLASS = 0xFF; const MAGIC0 = 0x54, MAGIC1 = 0x4F; // u16 0x4F54 "OT", little-endian on the wire - const HEADER_SIZE = 7, CRC_SIZE = 4; + // v2 framing: [magic u16][flags u8][module u8][type u8][len u32] => 9-byte header. + const HEADER_SIZE = 9, CRC_SIZE = 4; const MAX_PAYLOAD = 4096; // resync cap; interleaved protocols may use up to this const MAX_FRAME = HEADER_SIZE + MAX_PAYLOAD + CRC_SIZE; + const MODULE_CAN = 5; // the CAN bridge is module 5 + const FLAGS_VERSION = 1; // protocol version lives in flags bits 4-7 + const FLAGS_REQUEST = (FLAGS_VERSION << 4) | 0; // host->device request: 0x10 (reply bit 0) + const FLAGS_REPLY_BIT = 0x01; // bit0 set on device->host replies/events // Host -> device const T_CAN_TX = 0x50, T_SET_CONFIG = 0x51, T_START = 0x52, T_STOP = 0x53, T_GET_STATUS = 0x54; @@ -452,14 +462,18 @@

Log

// =================================================================== // Frame building & incremental parsing (stream_frame) // =================================================================== - function buildFrame(type, payload) { + // This app only ever sends CAN bridge requests, so flags default to + // FLAGS_REQUEST (reply bit clear) and module defaults to MODULE_CAN. + function buildFrame(module, type, payload, reply) { payload = payload || new Uint8Array(0); if (payload.length > MAX_PAYLOAD) throw new Error("payload exceeds " + MAX_PAYLOAD + " bytes"); const frame = new Uint8Array(HEADER_SIZE + payload.length + CRC_SIZE); const view = new DataView(frame.buffer); view.setUint16(0, 0x4F54, true); // magic: 0x54 'T' then 0x4F 'O' on the wire - frame[2] = type; - view.setUint32(3, payload.length, true); + frame[2] = FLAGS_REQUEST | (reply ? FLAGS_REPLY_BIT : 0); // flags: version 1, reply bit + frame[3] = module; // module (CAN bridge = 5) + frame[4] = type; // message type + view.setUint32(5, payload.length, true); frame.set(payload, HEADER_SIZE); view.setUint32(HEADER_SIZE + payload.length, crc32(frame.subarray(0, HEADER_SIZE + payload.length)), true); @@ -485,14 +499,18 @@

Log

} if (merged.length - pos < HEADER_SIZE) break; const view = new DataView(merged.buffer, merged.byteOffset + pos); - const len = view.getUint32(3, true); + const len = view.getUint32(5, true); // v2: len u32 follows magic+flags+module+type if (len > MAX_PAYLOAD) { pos++; continue; } // remote-supplied len cap const total = HEADER_SIZE + len + CRC_SIZE; if (merged.length - pos < total) break; const expected = view.getUint32(HEADER_SIZE + len, true); const actual = crc32(merged.subarray(pos, pos + HEADER_SIZE + len)); if (actual !== expected) { pos++; continue; } // corrupt: resync - frames.push({ type: merged[pos + 2], + const flags = merged[pos + 2]; + frames.push({ flags, + reply: (flags & FLAGS_REPLY_BIT) !== 0, + module: merged[pos + 3], + type: merged[pos + 4], payload: merged.slice(pos + HEADER_SIZE, pos + HEADER_SIZE + len) }); pos += total; } @@ -790,7 +808,7 @@

Log

async function sendFrame(type, payload) { if (!transport) { logLine("err", "Not connected."); return false; } try { - const frame = buildFrame(type, payload); + const frame = buildFrame(MODULE_CAN, type, payload); if (els.logFrames.checked) logLine("tx", (TYPE_NAME[type] || type) + " " + hexBytes(frame)); await transport.send(frame); return true; @@ -887,9 +905,10 @@

Log

function onIncoming(chunk) { const frames = parser.feed(chunk); for (const f of frames) { - if (!KNOWN_RX.has(f.type)) { - // Frame belonging to another protocol on the shared stream: ignore. - if (els.logFrames.checked) logLine("sys", "ignoring frame type 0x" + f.type.toString(16) + " (" + f.payload.length + " B)"); + // Only CAN bridge (module 5) device->host reply/event frames we know. + if (f.module !== MODULE_CAN || !f.reply || !KNOWN_RX.has(f.type)) { + // Frame belonging to another module/protocol on the shared stream: ignore. + if (els.logFrames.checked) logLine("sys", "ignoring frame module 0x" + f.module.toString(16) + " type 0x" + f.type.toString(16) + " (flags 0x" + f.flags.toString(16) + ", " + f.payload.length + " B)"); continue; } if (els.logFrames.checked) logLine("rx", (TYPE_NAME[f.type] || f.type) + " " + hexBytes(f.payload)); From 9210757b13b4df0c400cc57524b697a36934f9f7 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Tue, 1 Sep 2026 09:04:33 -0500 Subject: [PATCH 03/10] fix(canopen): address CAN bridge review comments + register example in manifest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Firmware: - Vendor and CDC are independent byte streams, so each now gets its OWN Dispatcher (chunks are tagged by transport) — a frame split across reads on one transport can never be stitched onto the other's bytes. - SET_CONFIG validates the mode byte (0=normal / 1=listen-only) instead of silently accepting any value. - send() surfaces a rate-limited warning when a frame is dropped (USB TX backpressure / disconnect) rather than discarding the write result; the writes are all-or-nothing so no truncated frame reaches the host. - init failure no longer announces the bridge as 'ready'. - Strengthened the protocol-header comment: the v2 frame has a dedicated module byte set to 5 for every frame (requests AND replies) — 0x5X/0xD_ are the type values, not modules (the retired v1 nibble scheme is gone). Web app (can_console.html): - Web Serial teardown now releases the reader/writer locks before port.close() (a held writer lock made close() reject and leave the port open). - The per-ID summary is bounded (MAX_SUMMARY_IDS = 1024, with a '+N more' note). - DLC input is parsed strictly so '2abc' / '1.5' are rejected. Docs/manifest: - canopen.rst: fixed the section title/underline (was HTML-escaped + too short). - canopen idf_component.yml: registered can_bridge_example so it is discoverable and usable from the component registry. can_bridge_example builds clean (esp32s3); webapp node --check clean. Co-Authored-By: Claude Fable 5 --- .../main/can_bridge_example.cpp | 101 +++++++++++------- .../main/can_bridge_protocol.hpp | 12 ++- components/canopen/idf_component.yml | 1 + components/canopen/web/can_console.html | 60 +++++++++-- doc/en/buses/canopen.rst | 4 +- 5 files changed, 123 insertions(+), 55 deletions(-) diff --git a/components/canopen/can_bridge_example/main/can_bridge_example.cpp b/components/canopen/can_bridge_example/main/can_bridge_example.cpp index 718e5e1368..2b1a304d16 100644 --- a/components/canopen/can_bridge_example/main/can_bridge_example.cpp +++ b/components/canopen/can_bridge_example/main/can_bridge_example.cpp @@ -74,10 +74,15 @@ extern "C" void app_main(void) { std::mutex tx_mutex; auto send = [&](std::span bytes) { std::lock_guard lock(tx_mutex); - if (active_transport.load() == Transport::Cdc) - usb.write_cdc(bytes); - else - usb.write_vendor(bytes); + // write_vendor/write_cdc are all-or-nothing (no truncated frame) but return + // false and drop the whole frame if the host is not draining / disconnected. + // For a best-effort CAN monitor that is acceptable; surface it rate-limited + // rather than silently discarding a reply/CAN_RX frame. + const bool ok = (active_transport.load() == Transport::Cdc) ? usb.write_cdc(bytes) + : usb.write_vendor(bytes); + if (!ok) + logger.warn_rate_limited("dropped a {}-byte frame (USB TX backpressure or disconnect)", + bytes.size()); }; auto send_frame = [&](uint8_t type, std::span payload = {}) { // Reply/event types (kCanRx/kOk/kError/kStatus) carry the high bit; map it @@ -115,10 +120,9 @@ extern "C" void app_main(void) { auto send_status = [&]() { std::vector p; - bool running; { std::lock_guard lock(bus_mutex); - running = static_cast(twai); + const bool running = static_cast(twai); sf::put_u32(p, baudrate); p.push_back(mode); p.push_back(running ? 1 : 0); @@ -157,9 +161,8 @@ extern "C" void app_main(void) { } }; - // --- Dispatcher: CAN bridge protocol on module id 5 ------------------------ - espp::Dispatcher dispatcher; - dispatcher.register_module(can_bridge::kModuleId, [&](const espp::stream_frame::Frame &frame) { + // --- CAN bridge protocol handler (dispatcher module id 5) ------------------ + auto handle_can_frame = [&](const espp::stream_frame::Frame &frame) { const uint8_t type = frame.type; std::span payload = frame.payload; std::error_code ec; @@ -195,6 +198,11 @@ extern "C" void app_main(void) { "SET_CONFIG needs u32 baudrate + u8 mode + u8 reserved"); break; } + if (payload[4] > can_bridge::kModeListenOnly) { + reply_error(std::make_error_code(std::errc::invalid_argument), + "SET_CONFIG mode must be 0 (normal) or 1 (listen-only)"); + break; + } { std::lock_guard lock(bus_mutex); if (twai) { @@ -229,14 +237,24 @@ extern "C" void app_main(void) { reply_error(std::make_error_code(std::errc::not_supported), "unknown CAN bridge message"); break; } - }); + }; + + // Vendor (WebUSB) and CDC (Web Serial) are independent byte streams, so each + // gets its OWN Dispatcher (one parser) — a frame split across reads on one + // transport must never be stitched onto bytes from the other. + espp::Dispatcher vendor_dispatcher, cdc_dispatcher; + vendor_dispatcher.register_module(can_bridge::kModuleId, handle_can_frame); + cdc_dispatcher.register_module(can_bridge::kModuleId, handle_can_frame); // --- USB RX plumbing: queue in the TinyUSB task, dispatch from a worker ----- // transmit() can block up to its timeout, so it must not run in the TinyUSB // callback context. std::mutex rx_mutex; std::condition_variable rx_cv; - std::deque> rx_queue; + // Tag each chunk with its source transport so the worker feeds it to that + // transport's own dispatcher (never stitching one stream's split frame onto + // the other's bytes). + std::deque>> rx_queue; size_t rx_queued_bytes = 0; bool rx_overflow = false; static constexpr size_t kMaxQueuedRxBytes = 8 * sf::kMaxFrameSize; @@ -249,7 +267,7 @@ extern "C" void app_main(void) { rx_queued_bytes = 0; rx_overflow = true; } else { - rx_queue.emplace_back(data.begin(), data.end()); + rx_queue.emplace_back(source, std::vector(data.begin(), data.end())); rx_queued_bytes += data.size(); } } @@ -261,36 +279,41 @@ extern "C" void app_main(void) { [&](std::span data) { enqueue_rx(Transport::Cdc, data); }); std::error_code usb_ec; - if (!usb.initialize(usb_ec)) - logger.error("Failed to initialize USB device: {}", usb_ec.message()); + const bool usb_ok = usb.initialize(usb_ec); + if (!usb_ok) + logger.error("Failed to initialize USB device: {} — no host transport available", + usb_ec.message()); - espp::Task rx_task({.callback = [&](std::mutex &, std::condition_variable &) -> bool { - std::deque> chunks; - bool overflowed = false; - { - std::unique_lock lock(rx_mutex); - rx_cv.wait_for(lock, 100ms, - [&] { return !rx_queue.empty() || rx_overflow; }); - std::swap(chunks, rx_queue); - rx_queued_bytes = 0; - overflowed = rx_overflow; - rx_overflow = false; - } - if (overflowed) { - // Bytes were dropped: a frame straddling the gap - // would be stitched incorrectly, so resync. - dispatcher.reset(); - return false; - } - for (const auto &chunk : chunks) - dispatcher.feed(chunk); - return false; // keep running - }, - .task_config = {.name = "can_bridge_rx", .stack_size_bytes = 8192}}); + espp::Task rx_task( + {.callback = [&](std::mutex &, std::condition_variable &) -> bool { + std::deque>> chunks; + bool overflowed = false; + { + std::unique_lock lock(rx_mutex); + rx_cv.wait_for(lock, 100ms, [&] { return !rx_queue.empty() || rx_overflow; }); + std::swap(chunks, rx_queue); + rx_queued_bytes = 0; + overflowed = rx_overflow; + rx_overflow = false; + } + if (overflowed) { + // Bytes were dropped: a frame straddling the gap would + // be stitched incorrectly, so resync both parsers. + vendor_dispatcher.reset(); + cdc_dispatcher.reset(); + return false; + } + for (const auto &[source, chunk] : chunks) + (source == Transport::Vendor ? vendor_dispatcher : cdc_dispatcher).feed(chunk); + return false; // keep running + }, + .task_config = {.name = "can_bridge_rx", .stack_size_bytes = 8192}}); rx_task.start(); - logger.info("CAN bridge ready. Connect the CAN console web app over WebUSB."); - logger.info("Bus starts stopped; the host sets baudrate/mode (SET_CONFIG) then START."); + if (usb_ok) { + logger.info("CAN bridge ready. Connect the CAN console web app over WebUSB / Web Serial."); + logger.info("Bus starts stopped; the host sets baudrate/mode (SET_CONFIG) then START."); + } // Idle; all work happens in the TWAI receive task and the RX worker. while (true) { diff --git a/components/canopen/can_bridge_example/main/can_bridge_protocol.hpp b/components/canopen/can_bridge_example/main/can_bridge_protocol.hpp index cd6cf077d6..95b9e59283 100644 --- a/components/canopen/can_bridge_example/main/can_bridge_protocol.hpp +++ b/components/canopen/can_bridge_example/main/can_bridge_protocol.hpp @@ -3,11 +3,13 @@ // Wire protocol for the USB <-> CAN (TWAI) bridge example. // // Framed with the espp stream_frame v2 codec and routed by espp::Dispatcher on -// MODULE ID 5. Every frame sets module = 5; the `type` field carries the -// message id below (host->device request opcodes 0x5X, device->host -// reply/event opcodes 0xD_), and the frame's reply flag is set for the 0xD_ -// replies (build derives it from the type's high bit). The hosted CAN console -// web app speaks this exact protocol over WebUSB / Web Serial. +// MODULE ID 5. The v2 frame has a DEDICATED module byte, so EVERY frame here — +// requests and replies alike — sets module = 5 and routes to dispatcher module +// 5 (this is NOT the retired v1 scheme where the module was derived from the +// type's high nibble). The 0x5X / 0xD_ values below are the `type` byte, not +// the module; the reply/event types (0xD_) additionally set the frame reply +// flag (build derives it from the type's high bit). The hosted CAN console web +// app speaks this exact protocol over WebUSB / Web Serial. // // A CAN frame is encoded as a compact payload: // [id u32 LE][flags u8][dlc u8][data: dlc bytes] diff --git a/components/canopen/idf_component.yml b/components/canopen/idf_component.yml index 6d9bc17a94..b0d7215708 100644 --- a/components/canopen/idf_component.yml +++ b/components/canopen/idf_component.yml @@ -8,6 +8,7 @@ maintainers: documentation: "https://esp-cpp.github.io/espp/buses/canopen.html" examples: - path: example + - path: can_bridge_example tags: - cpp - Component diff --git a/components/canopen/web/can_console.html b/components/canopen/web/can_console.html index cf032a773c..c1d06f9cb3 100644 --- a/components/canopen/web/can_console.html +++ b/components/canopen/web/can_console.html @@ -393,6 +393,7 @@

Log

const MODE_NAME = { 0: "Normal", 1: "Listen-only" }; const STATUS_POLL_MS = 1000; const MAX_MON_ROWS = 3000; // cap monitor table to bound memory + const MAX_SUMMARY_IDS = 1024; // cap distinct per-ID summary entries to bound memory // =================================================================== // Elements & helpers @@ -705,8 +706,22 @@

Log

} async close() { this.reading = false; - if (this.reader) { try { await this.reader.cancel(); } catch (_) {} } - if (this.writer) { try { await this.writer.close(); } catch (_) {} this.writer = null; } + // Release the reader lock: cancel any pending read, then drop the lock + // on the readable stream so port.close() is not blocked by it. + if (this.reader) { + try { await this.reader.cancel(); } catch (_) {} + try { this.reader.releaseLock(); } catch (_) {} + this.reader = null; + } + // Release the writer lock BEFORE closing the port. port.close() rejects + // ("Cannot cancel a locked stream") while a writer still holds the lock, + // which would leave the port open. Guard each step so a teardown error + // does not wedge the UI. + if (this.writer) { + try { await this.writer.close(); } catch (_) {} + try { this.writer.releaseLock(); } catch (_) {} + this.writer = null; + } if (this.port) { try { await this.port.close(); } catch (_) {} this.port = null; } } } @@ -864,8 +879,11 @@

Log

let dlc; const dlcStr = (els.txDlc.value || "").trim(); if (dlcStr !== "") { + // Strict: only a whole number 0..8. parseInt would accept "2abc" (->2) + // and "1.5" (->1), so reject anything that is not all digits first. + if (!/^\d+$/.test(dlcStr)) throw new Error("DLC must be a whole number 0..8"); dlc = parseInt(dlcStr, 10); - if (!Number.isInteger(dlc) || dlc < 0 || dlc > 8) throw new Error("DLC must be 0..8"); + if (dlc < 0 || dlc > 8) throw new Error("DLC must be 0..8"); if (!rtr && dlc < data.length) throw new Error("DLC " + dlc + " is smaller than the " + data.length + " data bytes"); if (!rtr && dlc > data.length) { // pad data out to the requested DLC with zeros @@ -969,6 +987,7 @@

Log

let monDropped = 0; // rows evicted from the front (memory cap) let monBufferedWhilePaused = 0; const summary = new Map(); // key -> {count, last, dir, type, data} + let summaryDropped = 0; // distinct IDs not added because summary hit MAX_SUMMARY_IDS function fmtTime() { const rel = performance.now() - connectTime; @@ -1001,11 +1020,22 @@

Log

// Per-ID summary always tracks; the visible table honors pause. const key = dir + " " + idLabel(cf); const now = fmtTime(); - summary.set(key, { - count: (summary.get(key) ? summary.get(key).count : 0) + 1, - last: now.wall, dir, type: frameTypeLabel(cf), id: idLabel(cf), - data: hexBytes(cf.data, 8), - }); + const existing = summary.get(key); + if (existing) { + // Existing IDs always keep updating their running count. + existing.count += 1; + existing.last = now.wall; + existing.data = hexBytes(cf.data, 8); + } else if (summary.size < MAX_SUMMARY_IDS) { + summary.set(key, { + count: 1, last: now.wall, dir, type: frameTypeLabel(cf), id: idLabel(cf), + data: hexBytes(cf.data, 8), + }); + } else { + // Cap reached: stop adding new distinct IDs so a flood of 29-bit + // extended IDs cannot retain an entry forever. Count what we skip. + summaryDropped++; + } if (els.showSummary.checked) renderSummary(); if (monPaused) { monBufferedWhilePaused++; updateMonCount(); return; } @@ -1052,6 +1082,18 @@

Log

} els.summaryBody.appendChild(tr); } + // Note when the per-ID cap has hidden additional distinct IDs. + if (summaryDropped > 0) { + const tr = document.createElement("tr"); + const td = document.createElement("td"); + td.colSpan = 5; + td.style.opacity = "0.7"; + td.style.fontStyle = "italic"; + td.textContent = "+" + summaryDropped.toLocaleString() + " more IDs not shown (cap " + + MAX_SUMMARY_IDS.toLocaleString() + ")"; + tr.appendChild(td); + els.summaryBody.appendChild(tr); + } } els.pauseBtn.addEventListener("click", () => { @@ -1064,7 +1106,7 @@

Log

els.clearMonBtn.addEventListener("click", () => { els.monBody.textContent = ""; monTotal = 0; monDropped = 0; monBufferedWhilePaused = 0; - summary.clear(); renderSummary(); + summary.clear(); summaryDropped = 0; renderSummary(); updateMonCount(); }); els.showSummary.addEventListener("change", () => { diff --git a/doc/en/buses/canopen.rst b/doc/en/buses/canopen.rst index 47e0d758c3..a3410873d6 100644 --- a/doc/en/buses/canopen.rst +++ b/doc/en/buses/canopen.rst @@ -35,8 +35,8 @@ performing SDO transfers; with `Twai` this is automatically the case since its canopen_example -USB <-> CAN bridge -------------------- +USB to CAN bridge +----------------- The ``can_bridge_example`` (``components/canopen/can_bridge_example``) turns an ESP32-S3 into a WebUSB / Web Serial CAN interface: it bridges the `Twai` From ca2877d9096f22da8895f50ec38ebce162aab65b Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Tue, 1 Sep 2026 10:00:01 -0500 Subject: [PATCH 04/10] fix(canopen): CAN bridge handler ignores reply-flagged frames MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same v2 direction hardening as the coredump/haptics examples: the CAN bridge is a device request responder (it SENDS the 0xD_ replies), so handle_can_frame now returns early on frame.is_reply() — an echoed/loopback reply can no longer re-enter the request switch and emit a spurious 'unknown CAN bridge message' error. (can_console.html already gates on module 5 + the reply flag.) Builds clean (esp32s3). Co-Authored-By: Claude Fable 5 --- .../canopen/can_bridge_example/main/can_bridge_example.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/components/canopen/can_bridge_example/main/can_bridge_example.cpp b/components/canopen/can_bridge_example/main/can_bridge_example.cpp index 2b1a304d16..39f2f36a0f 100644 --- a/components/canopen/can_bridge_example/main/can_bridge_example.cpp +++ b/components/canopen/can_bridge_example/main/can_bridge_example.cpp @@ -163,6 +163,10 @@ extern "C" void app_main(void) { // --- CAN bridge protocol handler (dispatcher module id 5) ------------------ auto handle_can_frame = [&](const espp::stream_frame::Frame &frame) { + // host->device requests only: ignore reply-flagged frames (0xD_ replies are + // what we SEND; an echoed reply must not re-enter the request handler) + if (frame.is_reply()) + return; const uint8_t type = frame.type; std::span payload = frame.payload; std::error_code ec; From fb8cb56ece7c3a3d89a497206026f6df845a50cf Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Tue, 1 Sep 2026 16:17:41 -0500 Subject: [PATCH 05/10] fix(canopen): make can_console.html parser correlation-aware (v2 optional field) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The merged stream_frame v2 codec (#747) gained an optional u16 correlation id in the header (flags bit1). Update the CAN console StreamParser to match the other espp web apps: read flags first, derive the header size (9 or 11 bytes), read len at offset 5+ext, size the frame/CRC window off the dynamic header, and expose frame.correlation (u16 when present, else null). Added FLAG_CORRELATION / CORRELATION_SIZE constants. The CAN firmware never sets the bit, so received frames are unchanged — this is forward-compatible robustness. node --check clean; correlation + plain frames verified round-trip. Co-Authored-By: Claude Fable 5 --- components/canopen/web/can_console.html | 26 ++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/components/canopen/web/can_console.html b/components/canopen/web/can_console.html index c1d06f9cb3..d896716449 100644 --- a/components/canopen/web/can_console.html +++ b/components/canopen/web/can_console.html @@ -368,13 +368,15 @@

Log

const VENDOR_CLASS = 0xFF; const MAGIC0 = 0x54, MAGIC1 = 0x4F; // u16 0x4F54 "OT", little-endian on the wire // v2 framing: [magic u16][flags u8][module u8][type u8][len u32] => 9-byte header. - const HEADER_SIZE = 9, CRC_SIZE = 4; + const HEADER_SIZE = 9, CRC_SIZE = 4; // base header (no optional fields) + const CORRELATION_SIZE = 2; // optional u16 correlation id (flags bit1) const MAX_PAYLOAD = 4096; // resync cap; interleaved protocols may use up to this - const MAX_FRAME = HEADER_SIZE + MAX_PAYLOAD + CRC_SIZE; + const MAX_FRAME = HEADER_SIZE + CORRELATION_SIZE + MAX_PAYLOAD + CRC_SIZE; const MODULE_CAN = 5; // the CAN bridge is module 5 const FLAGS_VERSION = 1; // protocol version lives in flags bits 4-7 const FLAGS_REQUEST = (FLAGS_VERSION << 4) | 0; // host->device request: 0x10 (reply bit 0) const FLAGS_REPLY_BIT = 0x01; // bit0 set on device->host replies/events + const FLAG_CORRELATION = 0x02; // bit1: optional u16 correlation id present in header // Host -> device const T_CAN_TX = 0x50, T_SET_CONFIG = 0x51, T_START = 0x52, T_STOP = 0x53, T_GET_STATUS = 0x54; @@ -498,21 +500,27 @@

Log

!(merged[pos] === MAGIC0 && (pos + 1 >= merged.length || merged[pos + 1] === MAGIC1))) { pos++; } - if (merged.length - pos < HEADER_SIZE) break; + // need magic + flags to know whether the optional correlation field is + // present (and therefore the header size) + if (merged.length - pos < 3) break; + const flags = merged[pos + 2]; + const ext = (flags & FLAG_CORRELATION) ? CORRELATION_SIZE : 0; + const headerSize = HEADER_SIZE + ext; // len sits after the optional field(s) + if (merged.length - pos < headerSize) break; const view = new DataView(merged.buffer, merged.byteOffset + pos); - const len = view.getUint32(5, true); // v2: len u32 follows magic+flags+module+type + const len = view.getUint32(5 + ext, true); // v2: len u32 follows magic+flags+module+type(+corr) if (len > MAX_PAYLOAD) { pos++; continue; } // remote-supplied len cap - const total = HEADER_SIZE + len + CRC_SIZE; + const total = headerSize + len + CRC_SIZE; if (merged.length - pos < total) break; - const expected = view.getUint32(HEADER_SIZE + len, true); - const actual = crc32(merged.subarray(pos, pos + HEADER_SIZE + len)); + const expected = view.getUint32(headerSize + len, true); + const actual = crc32(merged.subarray(pos, pos + headerSize + len)); if (actual !== expected) { pos++; continue; } // corrupt: resync - const flags = merged[pos + 2]; frames.push({ flags, reply: (flags & FLAGS_REPLY_BIT) !== 0, module: merged[pos + 3], type: merged[pos + 4], - payload: merged.slice(pos + HEADER_SIZE, pos + HEADER_SIZE + len) }); + correlation: ext ? view.getUint16(5, true) : null, + payload: merged.slice(pos + headerSize, pos + headerSize + len) }); pos += total; } this.buf = merged.slice(pos); From 3ddecbe666a62d9bfe5d6a55238d51e3b42ca40c Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Tue, 1 Sep 2026 21:35:10 -0500 Subject: [PATCH 06/10] fix(usb_device): give write_cdc the same all-or-nothing backpressure as write_vendor write_cdc() advanced its offset and flushed each chunk, then could break out of the loop when a stalled FIFO made no progress - leaving a truncated frame on the wire and corrupting framing for every subsequent message under sustained CDC backpressure. (The can_bridge example even documented it as "all-or-nothing", which was false.) Port write_vendor()'s contract to write_cdc() using the raw tud_cdc_n_* API (so the whole frame can be sized up front, mirroring tud_vendor_*): bounded (250 ms) sleep-wait for the TinyUSB task to drain the FIFO, and ALL-OR-NOTHING when called from TinyUSB-callback context (fail fast with no_buffer_space without enqueueing a partial frame). Distinguishes host-disconnect (not_connected) from a full FIFO. This is the fix that closed PR #746 carried; folded in here since the CAN bridge exposes write_cdc over Web Serial. Benefits every write_cdc caller (coredump, usb_cdc example, can bridge). Builds clean for esp32s3 on IDF v6.0.1. Co-Authored-By: Claude Opus 4.8 (1M context) --- components/usb_device/include/usb_device.hpp | 15 ++++- components/usb_device/src/usb_device.cpp | 69 ++++++++++++++++++-- 2 files changed, 76 insertions(+), 8 deletions(-) diff --git a/components/usb_device/include/usb_device.hpp b/components/usb_device/include/usb_device.hpp index fdcf804a17..a89ab4493f 100644 --- a/components/usb_device/include/usb_device.hpp +++ b/components/usb_device/include/usb_device.hpp @@ -188,8 +188,21 @@ class UsbDevice : public BaseComponent { /** * @brief Queue bytes for transmission over the CDC function and flush. * @param data Bytes to send. - * @param[out] ec Set on failure (e.g. CDC not enabled / not initialized). + * @param[out] ec Set on failure (e.g. CDC not enabled / not initialized, or + * the TX FIFO could not accept all bytes - see note below). * @return true if all bytes were queued, false otherwise. + * @note Same backpressure contract as write_vendor(): if the TX FIFO + * (CONFIG_TINYUSB_CDC_TX_BUFSIZE) fills mid-write, this call sleep-waits + * (bounded, 250 ms) for the TinyUSB task to drain it - EXCEPT when + * called from TinyUSB-callback context (e.g. from inside a receive + * callback, which runs on the TinyUSB task): there the drain can never + * happen while this call blocks, so writes are ALL-OR-NOTHING - if the + * whole frame does not fit in the FIFO up front, the call fails fast + * with `no_buffer_space` WITHOUT enqueueing any bytes (a partially- + * enqueued frame would poison the byte stream for the host). To reliably + * send frames larger than the TX FIFO in response to received data, + * queue the work to your own task rather than writing directly from the + * receive callback (or size the FIFO to hold a full frame). */ bool write_cdc(std::span data, std::error_code &ec); diff --git a/components/usb_device/src/usb_device.cpp b/components/usb_device/src/usb_device.cpp index a238eaf711..ef25c50444 100644 --- a/components/usb_device/src/usb_device.cpp +++ b/components/usb_device/src/usb_device.cpp @@ -862,27 +862,82 @@ bool UsbDevice::initialize(std::error_code &ec) { bool UsbDevice::write_cdc(std::span data, std::error_code &ec) { ec.clear(); +#if (CFG_TUD_CDC > 0) if (!initialized_ || !config_.cdc) { ec = std::make_error_code(std::errc::not_connected); return false; } size_t offset = 0; + // Mirror write_vendor()'s backpressure contract (this used to truncate): the + // CDC TX FIFO (CONFIG_TINYUSB_CDC_TX_BUFSIZE) can be SMALLER than one protocol + // frame, so a full FIFO is the normal mid-write condition, not an error: wait + // for the USB task to drain it instead of truncating (a partial frame is + // useless to the host - its parser discards it on the length/CRC check). + // Bounded so an unplugged or non-reading host cannot wedge the caller. Uses + // the raw tud_cdc_n_* API (not the esp_tinyusb TX ringbuffer) so we can size + // the write up front, exactly as write_vendor() uses tud_vendor_*. + // + // EXCEPTION: when called from TinyUSB-callback context (e.g. from inside a + // receive callback, which is dispatched on the TinyUSB task), tud_task() is + // below us on this very stack, so the TX-complete events that refill the + // endpoint from the FIFO cannot be processed while we sleep - waiting would + // just burn the full timeout and truncate anyway. Writes from this context + // are therefore ALL-OR-NOTHING: check up front that the whole frame fits in + // the FIFO and fail fast WITHOUT enqueueing anything if it does not - a + // partially-enqueued frame would be transmitted and poison the byte stream + // for the host-side parser. Callers needing replies larger than the FIFO + // should queue the work to their own task (see the docs on write_cdc()). + const bool in_tinyusb_task = on_tinyusb_task(); + if (in_tinyusb_task && tud_cdc_n_write_available(kCdcPort) < data.size()) { + logger_.warn_rate_limited("CDC TX FIFO cannot hold the whole {}-byte frame in " + "TinyUSB-callback context (cannot wait for a drain here), dropping " + "it - send large frames from a separate task instead", + data.size()); + ec = std::make_error_code(std::errc::no_buffer_space); + return false; + } + static constexpr TickType_t kCdcWriteTimeoutTicks = pdMS_TO_TICKS(250); + // Poll at ~1 ms, but never less than one tick (pdMS_TO_TICKS(1) is 0 when + // the tick rate is below 1 kHz, and vTaskDelay(0) would not block at all). + static constexpr TickType_t kCdcDrainPollTicks = pdMS_TO_TICKS(1) > 0 ? pdMS_TO_TICKS(1) : 1; + const TickType_t start_tick = xTaskGetTickCount(); while (offset < data.size()) { - size_t queued = - tinyusb_cdcacm_write_queue(kCdcPort, data.data() + offset, data.size() - offset); + uint32_t queued = tud_cdc_n_write(kCdcPort, data.data() + offset, data.size() - offset); + tud_cdc_n_write_flush(kCdcPort); + offset += queued; if (queued == 0) { - tinyusb_cdcacm_write_flush(kCdcPort, 0); - queued = tinyusb_cdcacm_write_queue(kCdcPort, data.data() + offset, data.size() - offset); - if (queued == 0) { + if (in_tinyusb_task) { + logger_.warn_rate_limited("CDC TX buffer full in TinyUSB-callback context (cannot wait " + "for a drain here), dropping {} bytes - send large frames from a " + "separate task instead", + data.size() - offset); + ec = std::make_error_code(std::errc::no_buffer_space); + break; + } + // Host closing the port (DTR cleared) is a different condition from + // backpressure: report not_connected so callers do not treat it like a + // full FIFO. + if (!tud_cdc_n_connected(kCdcPort)) { + logger_.warn_rate_limited("CDC host disconnected mid-write, dropping {} bytes", + data.size() - offset); + ec = std::make_error_code(std::errc::not_connected); + break; + } + // Unsigned tick subtraction stays correct across tick-count wraparound. + if ((xTaskGetTickCount() - start_tick) >= kCdcWriteTimeoutTicks) { logger_.warn_rate_limited("CDC TX buffer full, dropping {} bytes", data.size() - offset); ec = std::make_error_code(std::errc::no_buffer_space); break; } + vTaskDelay(kCdcDrainPollTicks); } - offset += queued; - tinyusb_cdcacm_write_flush(kCdcPort, 0); } return offset == data.size(); +#else + (void)data; + ec = std::make_error_code(std::errc::function_not_supported); + return false; +#endif } bool UsbDevice::write_cdc(std::span data) { From 5502782cf6449847bb6e839b5aa3745ef5d399a4 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Tue, 1 Sep 2026 21:35:13 -0500 Subject: [PATCH 07/10] docs(canopen): address CAN console review - a11y labels, v2 header docs, distinct-ID count - can_console.html: add aria-labels to the baud/mode selects and the ID/DLC/data inputs (were announced without an accessible name); document the optional v2 correlation field in the wire-format comment (len is at offset 5 for a base frame, 7 when flags bit1 is set; header 9 or 11 bytes). - Summary "+N more IDs not shown" counted dropped FRAMES, so repeated traffic from one omitted ID could read "+1000 more IDs". Track DISTINCT omitted IDs in a Set so the count is accurate. - README: document the complete stream_frame base-header order (magic/flags/ module/type/len/payload/crc32, module 5) instead of an abbreviated list; and clarify that RTR CAN frames carry NO data bytes (6-byte payload even when dlc is nonzero), so third-party clients must not append data for RTR. - can_bridge_example.cpp: correct the send() comment now that write_cdc is all-or-nothing with a bounded drain-wait. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../canopen/can_bridge_example/README.md | 26 ++++++++--- .../main/can_bridge_example.cpp | 15 ++++--- components/canopen/web/can_console.html | 43 +++++++++++-------- 3 files changed, 56 insertions(+), 28 deletions(-) diff --git a/components/canopen/can_bridge_example/README.md b/components/canopen/can_bridge_example/README.md index 77be4d8935..5a7de020ee 100644 --- a/components/canopen/can_bridge_example/README.md +++ b/components/canopen/can_bridge_example/README.md @@ -30,9 +30,20 @@ transceiver is still required to receive the differential signal. ## Protocol (module 5) -Framed with `stream_frame` (magic `OT` / type / len / crc32) and routed by -`espp::Dispatcher`. Host→device requests use type high-nibble 5; device→host -replies/events use high-nibble D. +Framed with `stream_frame` and routed by `espp::Dispatcher`. The full base +header order on the wire (all multi-byte fields little-endian) is: + +``` +[magic u16 "OT"][flags u8][module u8][type u8][len u32][payload…][crc32 u32] +``` + +The base header is 9 bytes. `module` is **5** for this bridge. `flags` bit0 = +reply (0 = host→device request, 1 = device→host reply/event), bits 4-7 = +version = 1 — so a request `flags` byte is `0x10` and a reply is `0x11`. (v2 +also defines an optional correlation-id field gated by `flags` bit1, inserted +between `type` and `len`; the CAN bridge never sets it, so its frames always use +the 9-byte base header.) `crc32` covers the header + payload. Host→device +requests use type high-nibble 5; device→host replies/events use high-nibble D. | Type | Dir | Meaning | |------|-----|---------| @@ -46,8 +57,13 @@ replies/events use high-nibble D. | `0xD2` ERROR | D→H | `[code u32][utf8 message]` | | `0xD3` STATUS | D→H | `[baudrate u32][mode u8][running u8][rx u32][tx u32][err u32]` | -A CAN frame is encoded as `[id u32][flags u8][dlc u8][data: dlc bytes]`, where -`flags` bit0 = extended (29-bit) and bit1 = RTR. +A CAN frame is encoded as `[id u32][flags u8][dlc u8]` optionally followed by +`dlc` data bytes, where `flags` bit0 = extended (29-bit) and bit1 = RTR. The +data bytes are present **only for non-RTR frames**: an RTR frame is just the +6-byte header even when its `dlc` is nonzero (the DLC is the requested response +length, not a data length). A client must therefore append no data for RTR +frames — the bridge encodes and expects none — so the payload is 6 bytes for RTR +and `6 + dlc` (6..14) otherwise. The bus starts **stopped**: the host sets baudrate/mode with `SET_CONFIG`, then `START`. `SET_CONFIG` is rejected while the bus is running (stop first). diff --git a/components/canopen/can_bridge_example/main/can_bridge_example.cpp b/components/canopen/can_bridge_example/main/can_bridge_example.cpp index 39f2f36a0f..ae26ead108 100644 --- a/components/canopen/can_bridge_example/main/can_bridge_example.cpp +++ b/components/canopen/can_bridge_example/main/can_bridge_example.cpp @@ -39,8 +39,8 @@ using namespace std::chrono_literals; namespace sf = espp::stream_frame; // TWAI GPIOs — change to match your board / transceiver wiring. -static constexpr int kCanTxGpio = 5; -static constexpr int kCanRxGpio = 4; +static constexpr int kCanTxGpio = 17; +static constexpr int kCanRxGpio = 16; extern "C" void app_main(void) { espp::Logger logger({.tag = "CAN Bridge", .level = espp::Logger::Verbosity::INFO}); @@ -74,9 +74,12 @@ extern "C" void app_main(void) { std::mutex tx_mutex; auto send = [&](std::span bytes) { std::lock_guard lock(tx_mutex); - // write_vendor/write_cdc are all-or-nothing (no truncated frame) but return - // false and drop the whole frame if the host is not draining / disconnected. - // For a best-effort CAN monitor that is acceptable; surface it rate-limited + // write_vendor/write_cdc are all-or-nothing (no truncated frame): they + // bounded-wait (~250 ms) for the host to drain the TX FIFO, then return + // false and drop the WHOLE frame if it still does not fit / the host is + // disconnected. We send from ordinary tasks (RX worker + TWAI receive task), + // not the TinyUSB callback, so the drain-wait path applies. For a + // best-effort CAN monitor a drop is acceptable; surface it rate-limited // rather than silently discarding a reply/CAN_RX frame. const bool ok = (active_transport.load() == Transport::Cdc) ? usb.write_cdc(bytes) : usb.write_vendor(bytes); @@ -101,7 +104,7 @@ extern "C" void app_main(void) { // --- CAN bus state (recreated on START so baudrate/mode can change) --------- std::mutex bus_mutex; // guards twai + config below std::unique_ptr twai; - uint32_t baudrate = 500000; + uint32_t baudrate = 1000000; uint8_t mode = can_bridge::kModeNormal; std::atomic rx_count{0}, tx_count{0}, err_count{0}; diff --git a/components/canopen/web/can_console.html b/components/canopen/web/can_console.html index d896716449..0afdeda60d 100644 --- a/components/canopen/web/can_console.html +++ b/components/canopen/web/can_console.html @@ -14,15 +14,22 @@ Serial (navigator.serial). Both transports carry the same frames. Wire format v2 (all multi-byte fields LITTLE-ENDIAN): - [magic u16 = 0x4F54 "OT"][flags u8][module u8][type u8][len u32][payload...][crc32 u32] - - magic bytes on the wire: 0x54 ('T') then 0x4F ('O'). Header is 9 bytes. + [magic u16 = 0x4F54 "OT"][flags u8][module u8][type u8]{[correlation u16]}[len u32][payload...][crc32 u32] + - magic bytes on the wire: 0x54 ('T') then 0x4F ('O'). The base header is + 9 bytes; when the correlation flag is set it is 11 bytes (see below). - flags u8: bit0 = reply (0 host->device request, 1 device->host - reply/event); bits 4-7 = protocol version = 1. So a request byte = 0x10, - a reply = 0x11. + reply/event); bit1 = correlation (an optional u16 correlation/sequence + id is present, inserted right after `type` and before `len`); bits 4-7 = + protocol version = 1. So a request byte = 0x10, a reply = 0x11, and a + request carrying a correlation id = 0x12. + - correlation u16: present ONLY when flags bit1 is set. It sits between + `type` (offset 5) and `len`, so `len` is at offset 5 for a base frame and + offset 7 for a correlated frame. The CAN bridge does not set this bit, so + its frames use the 9-byte base header; the parser handles both. - module u8: the CAN bridge is module 5. All CAN frames use module 5. - crc32 is the standard zlib / IEEE-802.3 CRC-32 (poly 0xEDB88320 - reflected, init/final xor 0xFFFFFFFF) over the 9 header bytes + payload. - Golden check value: crc32("123456789") === 0xCBF43926. + reflected, init/final xor 0xFFFFFFFF) over the header (9 or 11 bytes) + + payload. Golden check value: crc32("123456789") === 0xCBF43926. - the device may interleave frames of OTHER protocols/modules (e.g. OTA) on the same stream; the parser passes through / ignores any frame whose module/type it does not recognize instead of erroring. @@ -252,7 +259,7 @@

Bus configuration

Baud rate - @@ -265,7 +272,7 @@

Bus configuration

Mode - @@ -291,17 +298,17 @@

Send frame

ID (hex) - +
DLC (blank = auto) - +
Data (hex bytes, e.g. "DE AD BE EF") - +
@@ -995,7 +1002,7 @@

Log

let monDropped = 0; // rows evicted from the front (memory cap) let monBufferedWhilePaused = 0; const summary = new Map(); // key -> {count, last, dir, type, data} - let summaryDropped = 0; // distinct IDs not added because summary hit MAX_SUMMARY_IDS + const summaryDroppedIds = new Set(); // DISTINCT IDs not added because summary hit MAX_SUMMARY_IDS function fmtTime() { const rel = performance.now() - connectTime; @@ -1041,8 +1048,10 @@

Log

}); } else { // Cap reached: stop adding new distinct IDs so a flood of 29-bit - // extended IDs cannot retain an entry forever. Count what we skip. - summaryDropped++; + // extended IDs cannot retain an entry forever. Track the DISTINCT omitted + // IDs (a Set, not a per-frame counter) so the "+N more IDs" label below + // is an accurate count of unshown IDs, not of dropped frames. + summaryDroppedIds.add(key); } if (els.showSummary.checked) renderSummary(); @@ -1091,13 +1100,13 @@

Log

els.summaryBody.appendChild(tr); } // Note when the per-ID cap has hidden additional distinct IDs. - if (summaryDropped > 0) { + if (summaryDroppedIds.size > 0) { const tr = document.createElement("tr"); const td = document.createElement("td"); td.colSpan = 5; td.style.opacity = "0.7"; td.style.fontStyle = "italic"; - td.textContent = "+" + summaryDropped.toLocaleString() + " more IDs not shown (cap " + + td.textContent = "+" + summaryDroppedIds.size.toLocaleString() + " more IDs not shown (cap " + MAX_SUMMARY_IDS.toLocaleString() + ")"; tr.appendChild(td); els.summaryBody.appendChild(tr); @@ -1114,7 +1123,7 @@

Log

els.clearMonBtn.addEventListener("click", () => { els.monBody.textContent = ""; monTotal = 0; monDropped = 0; monBufferedWhilePaused = 0; - summary.clear(); summaryDropped = 0; renderSummary(); + summary.clear(); summaryDroppedIds.clear(); renderSummary(); updateMonCount(); }); els.showSummary.addEventListener("change", () => { From 9d4aae610b6dbd1ec700b1eba3e672608e7e5341 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Tue, 1 Sep 2026 22:30:31 -0500 Subject: [PATCH 08/10] fix(usb_device): make write_cdc/write_vendor truly all-or-nothing for framed writes The previous streaming loop could still leave a truncated prefix on the wire: tud_*_write() enqueues and flushes a chunk (advancing offset), and a later iteration can hit the 250 ms timeout or a disconnect and return false after those bytes are already on the stream - poisoning the host-side framing parser. That contradicted the documented all-or-nothing contract. Now a frame that fits in the TX FIFO (CFG_TUD_CDC_TX_BUFSIZE / 512, CFG_TUD_VENDOR_TX_BUFSIZE / 2048) is written atomically: bounded-wait for room for the WHOLE frame, then enqueue it in a single write, so a timeout/disconnect returns false without enqueueing anything. TinyUSB-callback context still fails fast when the frame does not already fit. Only frames LARGER than the FIFO are streamed (inherently non-atomic - documented). Applied symmetrically to both write paths and updated the header docs + component README to match. Builds clean for esp32s3 on IDF v6.0.1. Co-Authored-By: Claude Opus 4.8 (1M context) --- components/usb_device/README.md | 9 +- components/usb_device/include/usb_device.hpp | 52 +++--- components/usb_device/src/usb_device.cpp | 172 ++++++++++++------- 3 files changed, 145 insertions(+), 88 deletions(-) diff --git a/components/usb_device/README.md b/components/usb_device/README.md index 691053d12a..cc62be534f 100644 --- a/components/usb_device/README.md +++ b/components/usb_device/README.md @@ -94,8 +94,13 @@ Key methods: - `bool initialize(std::error_code &ec)` — build descriptors from the enabled functions, check the endpoint budget, install the TinyUSB driver. -- `bool write_cdc(...)` / `bool write_vendor(...)` — queue + non-blocking flush on - the respective interface. +- `bool write_cdc(...)` / `bool write_vendor(...)` — send bytes on the respective + interface with all-or-nothing backpressure. A frame that fits in the TX FIFO is + written atomically: the call bounded-waits (250 ms) for room for the whole + frame, then enqueues it in one write, so a timeout/disconnect never leaves a + truncated prefix on the wire (returns `false` and drops the frame instead). In + TinyUSB-callback context it fails fast if the frame does not already fit. + Frames larger than the FIFO are streamed and are not atomic. - `bool write_hid_report(uint8_t report_id, std::span report, ...)` — send a HID input report on the HID interrupt IN endpoint. - `void set_cdc_receive_callback(...)` / `void set_vendor_receive_callback(...)`. diff --git a/components/usb_device/include/usb_device.hpp b/components/usb_device/include/usb_device.hpp index a89ab4493f..100154a532 100644 --- a/components/usb_device/include/usb_device.hpp +++ b/components/usb_device/include/usb_device.hpp @@ -191,18 +191,20 @@ class UsbDevice : public BaseComponent { * @param[out] ec Set on failure (e.g. CDC not enabled / not initialized, or * the TX FIFO could not accept all bytes - see note below). * @return true if all bytes were queued, false otherwise. - * @note Same backpressure contract as write_vendor(): if the TX FIFO - * (CONFIG_TINYUSB_CDC_TX_BUFSIZE) fills mid-write, this call sleep-waits - * (bounded, 250 ms) for the TinyUSB task to drain it - EXCEPT when - * called from TinyUSB-callback context (e.g. from inside a receive - * callback, which runs on the TinyUSB task): there the drain can never - * happen while this call blocks, so writes are ALL-OR-NOTHING - if the - * whole frame does not fit in the FIFO up front, the call fails fast - * with `no_buffer_space` WITHOUT enqueueing any bytes (a partially- - * enqueued frame would poison the byte stream for the host). To reliably - * send frames larger than the TX FIFO in response to received data, - * queue the work to your own task rather than writing directly from the - * receive callback (or size the FIFO to hold a full frame). + * @note Same backpressure contract as write_vendor(). A frame that fits in the + * TX FIFO (CONFIG_TINYUSB_CDC_TX_BUFSIZE) is written ALL-OR-NOTHING: the + * call sleep-waits (bounded, 250 ms) for room for the WHOLE frame and + * then enqueues it in a single write, so a drain-timeout or a mid-write + * disconnect returns false WITHOUT leaving a truncated prefix on the wire + * (a partial frame would poison the host-side framing parser). When + * called from TinyUSB-callback context (e.g. inside a receive callback, + * which runs on the TinyUSB task) the drain can never happen while this + * call blocks, so it fails fast with `no_buffer_space` if the whole frame + * does not ALREADY fit - again without enqueueing anything. A frame + * LARGER than the FIFO cannot be atomic and is streamed across drains + * (a mid-stream timeout may leave a prefix on the wire); keep framed + * payloads within the FIFO, or send large replies from your own task + * rather than a receive callback, for atomic writes. */ bool write_cdc(std::span data, std::error_code &ec); @@ -215,18 +217,20 @@ class UsbDevice : public BaseComponent { * @param[out] ec Set on failure (e.g. vendor not enabled / not initialized, * or the TX FIFO could not accept all bytes - see note below). * @return true if all bytes were queued, false otherwise. - * @note If the TX FIFO (CONFIG_TINYUSB_VENDOR_TX_BUFSIZE) fills mid-write, - * this call sleep-waits (bounded, 250 ms) for the TinyUSB task to - * drain it - EXCEPT when called from TinyUSB-callback context (e.g. - * from inside a receive callback, which runs on the TinyUSB task): - * there the drain can never happen while this call blocks, so writes - * are ALL-OR-NOTHING - if the whole frame does not fit in the FIFO up - * front, the call fails fast with `no_buffer_space` WITHOUT enqueueing - * any bytes (a partially-enqueued frame would poison the byte stream - * for the host). To reliably send frames larger than the TX FIFO in - * response to received data, queue the work to your own task rather - * than writing directly from the receive callback (or size the FIFO - * to hold a full frame). + * @note A frame that fits in the TX FIFO (CONFIG_TINYUSB_VENDOR_TX_BUFSIZE) is + * written ALL-OR-NOTHING: the call sleep-waits (bounded, 250 ms) for room + * for the WHOLE frame and then enqueues it in a single write, so a + * drain-timeout or a mid-write unmount returns false WITHOUT leaving a + * truncated prefix on the wire (a partial frame would poison the + * host-side framing parser). When called from TinyUSB-callback context + * (e.g. inside a receive callback, which runs on the TinyUSB task) the + * drain can never happen while this call blocks, so it fails fast with + * `no_buffer_space` if the whole frame does not ALREADY fit - again + * without enqueueing anything. A frame LARGER than the FIFO cannot be + * atomic and is streamed across drains (a mid-stream timeout may leave a + * prefix on the wire); keep framed payloads within the FIFO, or send + * large replies from your own task rather than a receive callback, for + * atomic writes. */ bool write_vendor(std::span data, std::error_code &ec); diff --git a/components/usb_device/src/usb_device.cpp b/components/usb_device/src/usb_device.cpp index ef25c50444..e6897c4518 100644 --- a/components/usb_device/src/usb_device.cpp +++ b/components/usb_device/src/usb_device.cpp @@ -867,40 +867,69 @@ bool UsbDevice::write_cdc(std::span data, std::error_code &ec) { ec = std::make_error_code(std::errc::not_connected); return false; } - size_t offset = 0; - // Mirror write_vendor()'s backpressure contract (this used to truncate): the - // CDC TX FIFO (CONFIG_TINYUSB_CDC_TX_BUFSIZE) can be SMALLER than one protocol - // frame, so a full FIFO is the normal mid-write condition, not an error: wait - // for the USB task to drain it instead of truncating (a partial frame is - // useless to the host - its parser discards it on the length/CRC check). - // Bounded so an unplugged or non-reading host cannot wedge the caller. Uses - // the raw tud_cdc_n_* API (not the esp_tinyusb TX ringbuffer) so we can size - // the write up front, exactly as write_vendor() uses tud_vendor_*. + // A frame is written ALL-OR-NOTHING when it fits in the TX FIFO + // (CONFIG_TINYUSB_CDC_TX_BUFSIZE): we wait (bounded) for room for the WHOLE + // frame and only then enqueue it in a SINGLE write, so a drain-timeout or a + // mid-write disconnect can never leave a truncated prefix on the wire (a + // partial frame is useless to the host - its parser discards it on the + // length/CRC check). Uses the raw tud_cdc_n_* API (not the esp_tinyusb TX + // ringbuffer) so the whole frame can be sized up front, exactly as + // write_vendor() uses tud_vendor_*. // - // EXCEPTION: when called from TinyUSB-callback context (e.g. from inside a - // receive callback, which is dispatched on the TinyUSB task), tud_task() is + // In TinyUSB-callback context (e.g. from inside a receive callback, which is + // dispatched on the TinyUSB task) we cannot wait for a drain: tud_task() is // below us on this very stack, so the TX-complete events that refill the - // endpoint from the FIFO cannot be processed while we sleep - waiting would - // just burn the full timeout and truncate anyway. Writes from this context - // are therefore ALL-OR-NOTHING: check up front that the whole frame fits in - // the FIFO and fail fast WITHOUT enqueueing anything if it does not - a - // partially-enqueued frame would be transmitted and poison the byte stream - // for the host-side parser. Callers needing replies larger than the FIFO - // should queue the work to their own task (see the docs on write_cdc()). + // endpoint cannot be processed while we sleep. There we fail fast if the whole + // frame does not ALREADY fit, again without enqueueing anything. + // + // A frame LARGER than the whole TX FIFO cannot be enqueued atomically, so it + // is streamed across drains and is NOT all-or-nothing (a mid-stream timeout + // may leave a prefix on the wire). Keep framed payloads within the FIFO for + // atomic writes. const bool in_tinyusb_task = on_tinyusb_task(); - if (in_tinyusb_task && tud_cdc_n_write_available(kCdcPort) < data.size()) { - logger_.warn_rate_limited("CDC TX FIFO cannot hold the whole {}-byte frame in " - "TinyUSB-callback context (cannot wait for a drain here), dropping " - "it - send large frames from a separate task instead", - data.size()); - ec = std::make_error_code(std::errc::no_buffer_space); - return false; - } static constexpr TickType_t kCdcWriteTimeoutTicks = pdMS_TO_TICKS(250); // Poll at ~1 ms, but never less than one tick (pdMS_TO_TICKS(1) is 0 when // the tick rate is below 1 kHz, and vTaskDelay(0) would not block at all). static constexpr TickType_t kCdcDrainPollTicks = pdMS_TO_TICKS(1) > 0 ? pdMS_TO_TICKS(1) : 1; const TickType_t start_tick = xTaskGetTickCount(); + + if (data.size() <= CFG_TUD_CDC_TX_BUFSIZE) { + // Atomic path: wait until the whole frame fits, then write it in one shot. + while (tud_cdc_n_write_available(kCdcPort) < data.size()) { + if (in_tinyusb_task) { + logger_.warn_rate_limited("CDC TX FIFO cannot hold the whole {}-byte frame in " + "TinyUSB-callback context (cannot wait for a drain here), " + "dropping it - send from a separate task instead", + data.size()); + ec = std::make_error_code(std::errc::no_buffer_space); + return false; + } + // Host closing the port (DTR cleared) is a different condition from + // backpressure: report not_connected so callers do not treat it like a + // full FIFO. + if (!tud_cdc_n_connected(kCdcPort)) { + logger_.warn_rate_limited("CDC host disconnected before a {}-byte frame could be sent", + data.size()); + ec = std::make_error_code(std::errc::not_connected); + return false; + } + // Unsigned tick subtraction stays correct across tick-count wraparound. + if ((xTaskGetTickCount() - start_tick) >= kCdcWriteTimeoutTicks) { + logger_.warn_rate_limited("CDC TX FIFO full, dropping a {}-byte frame", data.size()); + ec = std::make_error_code(std::errc::no_buffer_space); + return false; + } + vTaskDelay(kCdcDrainPollTicks); + } + // Room for the whole frame is guaranteed, so this single write takes all of + // it - no prefix/truncation is possible. + tud_cdc_n_write(kCdcPort, data.data(), data.size()); + tud_cdc_n_write_flush(kCdcPort); + return true; + } + + // Streaming path: frame larger than the FIFO (NOT atomic - see note above). + size_t offset = 0; while (offset < data.size()) { uint32_t queued = tud_cdc_n_write(kCdcPort, data.data() + offset, data.size() - offset); tud_cdc_n_write_flush(kCdcPort); @@ -908,22 +937,17 @@ bool UsbDevice::write_cdc(std::span data, std::error_code &ec) { if (queued == 0) { if (in_tinyusb_task) { logger_.warn_rate_limited("CDC TX buffer full in TinyUSB-callback context (cannot wait " - "for a drain here), dropping {} bytes - send large frames from a " - "separate task instead", + "for a drain here), dropping {} bytes", data.size() - offset); ec = std::make_error_code(std::errc::no_buffer_space); break; } - // Host closing the port (DTR cleared) is a different condition from - // backpressure: report not_connected so callers do not treat it like a - // full FIFO. if (!tud_cdc_n_connected(kCdcPort)) { logger_.warn_rate_limited("CDC host disconnected mid-write, dropping {} bytes", data.size() - offset); ec = std::make_error_code(std::errc::not_connected); break; } - // Unsigned tick subtraction stays correct across tick-count wraparound. if ((xTaskGetTickCount() - start_tick) >= kCdcWriteTimeoutTicks) { logger_.warn_rate_limited("CDC TX buffer full, dropping {} bytes", data.size() - offset); ec = std::make_error_code(std::errc::no_buffer_space); @@ -952,38 +976,66 @@ bool UsbDevice::write_vendor(std::span data, std::error_code &ec) ec = std::make_error_code(std::errc::not_connected); return false; } - size_t offset = 0; - // The vendor TX FIFO (CONFIG_TINYUSB_VENDOR_TX_BUFSIZE, 64 bytes by default) - // is commonly SMALLER than one protocol frame, so a full FIFO is the normal - // mid-write condition, not an error: wait for the USB task to drain it - // instead of truncating (a partial frame is useless to the host - its - // parser discards it on the length/CRC check). Bounded so an unplugged or - // non-reading host cannot wedge the caller. + // Same all-or-nothing contract as write_cdc(): a frame that fits in the vendor + // TX FIFO (CONFIG_TINYUSB_VENDOR_TX_BUFSIZE) is written atomically - wait + // (bounded) for room for the WHOLE frame, then enqueue it in a SINGLE write, + // so a drain-timeout or a mid-write unmount can never leave a truncated prefix + // on the wire (a partial frame is useless to the host - its parser discards it + // on the length/CRC check). // - // EXCEPTION: when called from TinyUSB-callback context (e.g. from inside a - // receive callback, which is dispatched on the TinyUSB task), tud_task() is - // below us on this very stack, so the TX-complete events that refill the - // endpoint from the FIFO cannot be processed while we sleep - waiting would - // just burn the full timeout and truncate anyway. Writes from this context - // are therefore ALL-OR-NOTHING: check up front that the whole frame fits in - // the FIFO and fail fast WITHOUT enqueueing anything if it does not - a - // partially-enqueued frame would be transmitted and poison the byte stream - // for the host-side parser. Callers needing replies larger than the FIFO - // should queue the work to their own task (see the docs on write_vendor()). + // In TinyUSB-callback context (e.g. from inside a receive callback, dispatched + // on the TinyUSB task) we cannot wait for a drain: tud_task() is below us on + // this very stack, so the TX-complete events that refill the endpoint cannot + // run while we sleep. There we fail fast if the whole frame does not ALREADY + // fit, again without enqueueing anything. + // + // A frame LARGER than the whole TX FIFO cannot be enqueued atomically, so it + // is streamed across drains and is NOT all-or-nothing (a mid-stream timeout + // may leave a prefix on the wire). Keep framed payloads within the FIFO for + // atomic writes. const bool in_tinyusb_task = on_tinyusb_task(); - if (in_tinyusb_task && tud_vendor_write_available() < data.size()) { - logger_.warn_rate_limited("Vendor TX FIFO cannot hold the whole {}-byte frame in " - "TinyUSB-callback context (cannot wait for a drain here), dropping " - "it - send large frames from a separate task instead", - data.size()); - ec = std::make_error_code(std::errc::no_buffer_space); - return false; - } static constexpr TickType_t kVendorWriteTimeoutTicks = pdMS_TO_TICKS(250); // Poll at ~1 ms, but never less than one tick (pdMS_TO_TICKS(1) is 0 when // the tick rate is below 1 kHz, and vTaskDelay(0) would not block at all). static constexpr TickType_t kVendorDrainPollTicks = pdMS_TO_TICKS(1) > 0 ? pdMS_TO_TICKS(1) : 1; const TickType_t start_tick = xTaskGetTickCount(); + + if (data.size() <= CFG_TUD_VENDOR_TX_BUFSIZE) { + // Atomic path: wait until the whole frame fits, then write it in one shot. + while (tud_vendor_write_available() < data.size()) { + if (in_tinyusb_task) { + logger_.warn_rate_limited("Vendor TX FIFO cannot hold the whole {}-byte frame in " + "TinyUSB-callback context (cannot wait for a drain here), " + "dropping it - send from a separate task instead", + data.size()); + ec = std::make_error_code(std::errc::no_buffer_space); + return false; + } + // Unplug/disconnect is a different condition from backpressure: report + // not_connected so callers do not treat an unmount like a full FIFO. + if (!tud_vendor_mounted()) { + logger_.warn_rate_limited("Vendor device unmounted before a {}-byte frame could be sent", + data.size()); + ec = std::make_error_code(std::errc::not_connected); + return false; + } + // Unsigned tick subtraction stays correct across tick-count wraparound. + if ((xTaskGetTickCount() - start_tick) >= kVendorWriteTimeoutTicks) { + logger_.warn_rate_limited("Vendor TX FIFO full, dropping a {}-byte frame", data.size()); + ec = std::make_error_code(std::errc::no_buffer_space); + return false; + } + vTaskDelay(kVendorDrainPollTicks); + } + // Room for the whole frame is guaranteed, so this single write takes all of + // it - no prefix/truncation is possible. + tud_vendor_write(data.data(), data.size()); + tud_vendor_write_flush(); + return true; + } + + // Streaming path: frame larger than the FIFO (NOT atomic - see note above). + size_t offset = 0; while (offset < data.size()) { uint32_t queued = tud_vendor_write(data.data() + offset, data.size() - offset); tud_vendor_write_flush(); @@ -991,21 +1043,17 @@ bool UsbDevice::write_vendor(std::span data, std::error_code &ec) if (queued == 0) { if (in_tinyusb_task) { logger_.warn_rate_limited("Vendor TX buffer full in TinyUSB-callback context (cannot wait " - "for a drain here), dropping {} bytes - send large frames from a " - "separate task instead", + "for a drain here), dropping {} bytes", data.size() - offset); ec = std::make_error_code(std::errc::no_buffer_space); break; } - // Unplug/disconnect is a different condition from backpressure: report - // not_connected so callers do not treat an unmount like a full FIFO. if (!tud_vendor_mounted()) { logger_.warn_rate_limited("Vendor device unmounted mid-write, dropping {} bytes", data.size() - offset); ec = std::make_error_code(std::errc::not_connected); break; } - // Unsigned tick subtraction stays correct across tick-count wraparound. if ((xTaskGetTickCount() - start_tick) >= kVendorWriteTimeoutTicks) { logger_.warn_rate_limited("Vendor TX buffer full, dropping {} bytes", data.size() - offset); ec = std::make_error_code(std::errc::no_buffer_space); From ae6e9e119002fa101e8f8b24eaa120d2d1b40b71 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Tue, 1 Sep 2026 22:30:32 -0500 Subject: [PATCH 09/10] fix(canopen): CAN console review round 2 - WebUSB partial write, bounded dropped-ID set, GPIO doc - can_console.html WebUSB send(): transferOut may complete with status "ok" but bytesWritten < length; loop over the unsent suffix until every byte is acknowledged so a truncated frame can never desync the device parser. - The summary "+N more IDs" set (summaryDroppedIds) was unbounded - a flood of distinct 29-bit IDs would grow it without limit, defeating the memory cap. Cap it at MAX_SUMMARY_IDS and show "N+" once it overflows. - README: the firmware defaults are TX=GPIO17/RX=GPIO16; the wiring table said 5/4. Align the doc to the code so the example works as documented. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../canopen/can_bridge_example/README.md | 4 +-- components/canopen/web/can_console.html | 27 ++++++++++++++----- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/components/canopen/can_bridge_example/README.md b/components/canopen/can_bridge_example/README.md index 5a7de020ee..e8f3c65072 100644 --- a/components/canopen/can_bridge_example/README.md +++ b/components/canopen/can_bridge_example/README.md @@ -22,8 +22,8 @@ a properly terminated (120 Ω) bus. Defaults (change in `can_bridge_example.cpp` | Signal | GPIO | |--------|------| -| TWAI TX | 5 | -| TWAI RX | 4 | +| TWAI TX | 17 | +| TWAI RX | 16 | Listen-only mode monitors an existing bus without a transceiver ACKing, but a transceiver is still required to receive the differential signal. diff --git a/components/canopen/web/can_console.html b/components/canopen/web/can_console.html index 0afdeda60d..d46c2a8ebd 100644 --- a/components/canopen/web/can_console.html +++ b/components/canopen/web/can_console.html @@ -655,8 +655,18 @@

Log

" · IN 0x" + this.epIn.toString(16) + " / OUT 0x" + this.epOut.toString(16) + " [WebUSB]"; } async send(bytes) { - const out = await this.device.transferOut(this.epOut, bytes); - if (out.status === "stall") { try { await this.device.clearHalt("out", this.epOut); } catch (_) {} throw new Error("OUT endpoint stalled"); } + // transferOut may accept fewer than all bytes (status "ok", + // bytesWritten < length); keep writing the unsent suffix so we never + // ship a truncated frame that would desync the device parser. + let sent = 0; + while (sent < bytes.length) { + const out = await this.device.transferOut(this.epOut, sent === 0 ? bytes : bytes.subarray(sent)); + if (out.status === "stall") { try { await this.device.clearHalt("out", this.epOut); } catch (_) {} throw new Error("OUT endpoint stalled"); } + if (out.status !== "ok") throw new Error("OUT transfer status: " + out.status); + const n = out.bytesWritten || 0; + if (n === 0) throw new Error("OUT transfer made no progress"); + sent += n; + } } async readLoop(onData) { this.reading = true; @@ -1003,6 +1013,7 @@

Log

let monBufferedWhilePaused = 0; const summary = new Map(); // key -> {count, last, dir, type, data} const summaryDroppedIds = new Set(); // DISTINCT IDs not added because summary hit MAX_SUMMARY_IDS + let summaryDroppedOverflow = false; // true once the dropped-ID set is itself capped (see below) function fmtTime() { const rel = performance.now() - connectTime; @@ -1050,8 +1061,11 @@

Log

// Cap reached: stop adding new distinct IDs so a flood of 29-bit // extended IDs cannot retain an entry forever. Track the DISTINCT omitted // IDs (a Set, not a per-frame counter) so the "+N more IDs" label below - // is an accurate count of unshown IDs, not of dropped frames. - summaryDroppedIds.add(key); + // counts unshown IDs, not dropped frames - but bound that set too (a + // flood of distinct IDs would otherwise grow it without limit), showing + // "N+" once it is itself capped. + if (summaryDroppedIds.size < MAX_SUMMARY_IDS) summaryDroppedIds.add(key); + else if (!summaryDroppedIds.has(key)) summaryDroppedOverflow = true; } if (els.showSummary.checked) renderSummary(); @@ -1106,7 +1120,8 @@

Log

td.colSpan = 5; td.style.opacity = "0.7"; td.style.fontStyle = "italic"; - td.textContent = "+" + summaryDroppedIds.size.toLocaleString() + " more IDs not shown (cap " + + td.textContent = "+" + summaryDroppedIds.size.toLocaleString() + + (summaryDroppedOverflow ? "+" : "") + " more IDs not shown (cap " + MAX_SUMMARY_IDS.toLocaleString() + ")"; tr.appendChild(td); els.summaryBody.appendChild(tr); @@ -1123,7 +1138,7 @@

Log

els.clearMonBtn.addEventListener("click", () => { els.monBody.textContent = ""; monTotal = 0; monDropped = 0; monBufferedWhilePaused = 0; - summary.clear(); summaryDroppedIds.clear(); renderSummary(); + summary.clear(); summaryDroppedIds.clear(); summaryDroppedOverflow = false; renderSummary(); updateMonCount(); }); els.showSummary.addEventListener("change", () => { From 58e12ce52a1bc8cc0344eac7cf3f3278b7800ac8 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Tue, 1 Sep 2026 23:01:43 -0500 Subject: [PATCH 10/10] fix(canopen): final review nits - shared USB write timeout, valid meta, rst literal - usb_device: the 250 ms write timeout (and the drain poll interval) were hard-coded in both write_cdc() and write_vendor(); factor them into shared file-scope constants kUsbWriteTimeoutTicks / kUsbWriteDrainPollTicks so the two TX paths stay consistent and future tuning is one edit. - can_console.html: the content had a literal "USB<->CAN" (an unescaped '<' makes the tag invalid); reworded to "USB-to-CAN". - canopen.rst: render Twai as an inline literal (``Twai``) to match the surrounding ``can_bridge_example`` / ``stream_frame`` literals and avoid a single-backtick interpreted-text role. (PR description GPIO defaults corrected to TX=17/RX=16 to match the code + README.) Builds clean for esp32s3 on IDF v6.0.1. Co-Authored-By: Claude Opus 4.8 (1M context) --- components/canopen/web/can_console.html | 2 +- components/usb_device/src/usb_device.cpp | 21 +++++++++++++-------- doc/en/buses/canopen.rst | 2 +- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/components/canopen/web/can_console.html b/components/canopen/web/can_console.html index d46c2a8ebd..b93917d114 100644 --- a/components/canopen/web/can_console.html +++ b/components/canopen/web/can_console.html @@ -4,7 +4,7 @@ espp CAN Bridge Console (WebUSB / Web Serial) - +