feat(mcp266 web): VID-only filter + dispatcher module-presence check - #770
Merged
Conversation
Bring the mcp266 WebUSB/Web-Serial console in line with the other espp
dispatcher-module consoles:
- default device filter is now VID-only (any espp device, VID 0x1209) instead
of VID+PID; "show all devices" uses {filters:[{}]} (the invalid
acceptAllDevices, a Web Bluetooth option, is removed -- it throws "Required
member filters is undefined" on WebUSB).
- on connect the console queries the reserved dispatcher discovery module
(0xFF ListModules, reusing Dispatcher::describe's TLV) and warns if the
MCP266 module (6) is not advertised, then continues anyway; firmware without
the discovery module simply never replies and is tolerated silently.
buildFrame() gains an optional module arg so it can emit the 0xFF query.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Aligns the MCP266 WebUSB/Web Serial console with other espp dispatcher-module consoles by broadening device selection and adding a best-effort dispatcher discovery probe on connect.
Changes:
- Switch WebUSB device filtering from VID+PID to VID-only; replace the invalid
acceptAllDevicesoption with a “show all devices” filter. - Add a dispatcher module discovery probe (
0xFF/ListModules) to warn (non-blocking) when module 6 isn’t advertised. - Extend
buildFrame()to support emitting frames for modules other than the MCP266 module.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
✅Static analysis result - no issues found! ✅ |
…imeout
- checkModulePresent(): guard the shared discoveryResolve with a settle() so a
stale probe's 800ms timeout can't cancel a newer probe's resolver.
- pull the 800ms magic number into a named DISCOVERY_TIMEOUT_MS constant.
- fire-and-forget call now has .catch(() => {}) (no unhandled rejection).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The disconnected UI still documents the removed PID filter and must reflect the new VID-only behavior.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
components/mcp266/web/mcp266_console.html:249
- The filter is now VID-only, but the disconnected UI still tells users twice that the default is
VID 0x1209 / PID 0x0d32(lines 123 and 510), andDEFAULT_PIDis consequently unused. Please update both messages to describe the VID-only chooser and remove the stale PID constant so the console does not contradict its actual behavior.
- Files reviewed: 1/1 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 (addresses the review note directly).
- log the missing-module message under a non-fatal "warn" tier (.warn).
Matches the ota/coredump/haptics/canopen consoles.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bring the mcp266 WebUSB/Web-Serial console (dispatcher module 6) in line with the other espp dispatcher-module consoles.
1. Accept any espp device
Default device filter is now VID-only (
{filters:[{vendorId:0x1209}]}) instead of VID+PID; "show all devices" uses{filters:[{}]}. Removes the invalid{acceptAllDevices:true}(a Web Bluetooth option that throws "Required member filters is undefined" on WebUSB, so "show all" never worked).2. Module-presence check
On connect, the console sends a
ListModulesquery on the reserved discovery module (0xFF) and warns (non-blocking) if the MCP266 module (6) isn't advertised, then continues. Firmware without the discovery module never replies and is tolerated silently (800 ms timeout).buildFrame()gains an optionalmoduleargument so it can emit the0xFFquery; both transports'send()return promises so the probe's error handling works on WebUSB and Web Serial alike. Passes a JS parse check.🤖 Generated with Claude Code