feat(canopen web): VID-only filter + module-presence check (can_bridge / ds402) - #769
Conversation
Bring the can_bridge and ds402 WebUSB consoles in line with the ota / coredump
/ haptics consoles:
- default device filter is now VID-only (any espp device, VID 0x1209) instead
of VID+PID, so a multi-module device is offered in every console; "show all
devices" uses {filters:[{}]}. Removes the invalid acceptAllDevices (a Web
Bluetooth option that throws "Required member filters is undefined" on WebUSB).
- on connect each console queries the reserved dispatcher discovery module
(0xFF ListModules, reusing Dispatcher::describe's TLV) and warns if the CAN
bridge module (5) is not advertised, then continues anyway; firmware without
the discovery module simply never replies and is tolerated silently.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
✅Static analysis result - no issues found! ✅ |
There was a problem hiding this comment.
🟡 Changes recommended
The new discovery check uses a shared resolver that can be cleared by an earlier timeout during quick reconnects, and the UI hint text still claims the default filter is VID+PID after switching to VID-only.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the CANopen WebUSB/WebSerial consoles to (1) broaden device selection to a VID-only WebUSB filter (plus a working “show all devices” path) and (2) add a best-effort dispatcher “discovery” probe that warns when the CAN bridge module (module 5) is not advertised.
Changes:
- Switch WebUSB
requestDevice()options to a VID-only filter and replace the invalidacceptAllDevicesusage with{filters:[{}]}for the “any device” path. - Add a discovery-based module presence check using reserved dispatcher module
0xFF/ListModules (0x00)and integrate discovery replies into the RX frame loop. - Log a warning when the device responds to discovery but does not advertise the CAN bridge module.
File summaries
| File | Description |
|---|---|
| components/canopen/web/ds402_panel.html | Updates WebUSB filtering and adds dispatcher discovery probe to warn if module 5 isn’t advertised. |
| components/canopen/web/can_bridge_console.html | Same as ds402 panel: VID-only filtering + discovery-based module presence warning. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…te hint
- checkModulePresent(): the 800ms timeout unconditionally nulled the shared
discoveryResolve, so a stale probe's timer could cancel a newer probe. Guard
with a settle() that only clears the resolver when it is still the active one.
- pull the 800ms magic number into a named DISCOVERY_TIMEOUT_MS constant.
- fire-and-forget call now has .catch(() => {}) (no unhandled rejection).
- "Default filter" hint text corrected to VID-only (was VID+PID).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🔵 Needs a closer look
Discovery parsing must reject unsupported or malformed payloads to prevent false missing-module warnings.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
components/canopen/web/can_bridge_console.html:885
- Reject unsupported discovery versions and require full TLV consumption before using the module list. The version byte exists so this wire format can evolve (
components/dispatcher/include/dispatcher.hpp:76-78); currently a future or remainder-bearing payload can be interpreted as v1 and produce a false “CAN bridge missing” warning instead of being ignored. The existing hub parser also enforces exact consumption atcomponents/dispatcher/web/dispatcher_hub.html:175-180.
components/canopen/web/ds402_panel.html:1164 - Reject unsupported discovery versions and require full TLV consumption before using the module list. The version byte exists so this wire format can evolve (
components/dispatcher/include/dispatcher.hpp:76-78); currently a future or remainder-bearing payload can be interpreted as v1 and produce a false “CAN bridge missing” warning instead of being ignored. The existing hub parser also enforces exact consumption atcomponents/dispatcher/web/dispatcher_hub.html:175-180.
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Balanced
- requestUsbDevice(): "show all" uses an empty filter {} with a VID-only
fallback if a WebUSB impl rejects it.
- log the missing-module message under a non-fatal "warn" tier (.warn) instead
of the error tier.
Matches the ota/coredump/haptics/mcp266 consoles.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Bring the CANopen WebUSB/Web-Serial consoles (
can_bridge_console,ds402_panel, both dispatcher module 5) in line with the ota / coredump / haptics consoles.1. Accept any espp device
Default device filter is now VID-only (
{filters:[{vendorId:0x1209}]}) instead of VID+PID, so a device that bundles the CAN bridge with other modules (and therefore has a different PID) still shows up. "Show all devices" now uses{filters:[{}]}.This also fixes a real bug: the "show all devices" path used
{acceptAllDevices:true}, which is a Web Bluetooth option — WebUSB'srequestDevicerequires afiltersmember and throws "Required member filters is undefined", so that path never worked.2. Module-presence check
On connect, each console sends a
ListModulesquery on the reserved dispatcher discovery module (0xFF, reusingespp::Dispatcher::describe's TLV) and, if the device answers, warns (non-blocking) when the CAN bridge module (5) isn't in the advertised list — naming what was detected. Firmware without the discovery module never replies and is tolerated silently (800 ms timeout); the normal status poll still surfaces a genuinely missing module.Both files pass a JS parse check; the discovery hook is placed in the frame loop with
continue(notreturn) so a discovery frame never aborts processing of the rest of an RX batch.🤖 Generated with Claude Code