Auto connect esp device to ESP-Decoder Terminal#25
Closed
Jason2866 wants to merge 7 commits into
Closed
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds a new Changes
Sequence DiagramsequenceDiagram
participant Ext as Extension (Polling)
participant USB as USB Enumerator
participant SPM as SerialPortManager
participant WV as Webview Panel
participant User as User
Ext->>USB: listPorts() (startup)
USB-->>Ext: ports[]
Ext->>Ext: identify ESP-like ports
alt autoConnect = "on" and ESP present
Ext->>SPM: setPort(path)
Ext->>SPM: connect()
SPM-->>Ext: connect result
Ext->>WV: syncState()
Ext->>User: show success message
end
Note over Ext: Poll every 2s
Ext->>USB: listPorts()
USB-->>Ext: ports[]
Ext->>Ext: diff knownPorts -> newPorts
alt New ESP detected
alt autoConnect = "ask"
Ext->>User: prompt to connect
User-->>Ext: choice (yes/no)
Ext->>Ext: persist autoConnect = on/off (if chosen)
alt yes
Ext->>SPM: setPort(path)
Ext->>SPM: connect()
SPM-->>Ext: connect result
Ext->>WV: syncState()
Ext->>User: show success message
end
else autoConnect = "on"
Ext->>SPM: setPort(path)
Ext->>SPM: connect()
SPM-->>Ext: connect result
Ext->>WV: syncState()
Ext->>User: show success message
else autoConnect = "off"
Note right of Ext: No action
end
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/extension.ts`:
- Around line 553-565: When auto-connecting, the code calls
serial.setPort(espPort.path) then awaits serial.connect() but only calls
viewProvider.syncState() on success, leaving the webview showing the old port
after a failed open; update the logic in the block around
serial.setPort/serial.connect (and the analogous block that also uses
setPort/connect later) to always call viewProvider.syncState() after setPort
regardless of connect() result (or call it in both success and failure
branches), so the webview reflects the newly selected port even if auto-connect
fails.
- Around line 507-528: The current isEspDevice() returns true for generic bridge
chips causing unintended auto-connections; change the logic so high-confidence
detection (used for autoConnect = "on") only returns true for Espressif-specific
matches (ESP_VENDOR_IDS membership or manufacturer includes 'espressif'), and
move CP210x/CH340/FTDI/WCH checks into a separate lower-confidence predicate
(e.g., isBridgeChip()) that callers can use only when autoConnect = "ask";
update callers that currently call isEspDevice() during the auto-connect flow to
use the stricter Espressif-only check for automatic connect and use
isBridgeChip() only for prompting/ask flows.
- Around line 579-584: The background poller should call a non-UI, log-only scan
instead of SerialPortManager.listPorts() which currently triggers
showErrorMessage(); modify the poller loop that currently invokes
serial.listPorts().then(...) to either call a new silent API (e.g.,
SerialPortManager.listPortsSilent()) or wrap serial.listPorts() in a try/catch
that only logs errors (using log.error) without calling showErrorMessage, and
implement simple throttling by counting consecutive failures and disabling the
poller (or backing off) after N failures; update references in the startup scan
and the poller code paths that call tryAutoConnectToExisting and update
knownPorts accordingly so the silent path mirrors the successful-port handling
but never surfaces repeated errors to the user.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f6d30e40-9ed9-4648-9507-0c026ae01d1d
📒 Files selected for processing (4)
package.jsonsrc/extension.tssrc/serialPortManager.tssrc/webviewPanel.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/extension.ts`:
- Around line 589-655: The USB poller (usbPollingTimer) must be paused while a
platform upload is in progress to avoid auto-reconnecting after onWillUpload()
releases the monitor port; add a shared flag (e.g., isUploading) that
onWillUpload() sets true and the corresponding onDidUpload()/onDidFailUpload()
handlers set false, then early-return from the usbPollingTimer handler when
isUploading is true (and also have reacquireWithRetry() respect this flag before
attempting to re-open the port). Update references to serial.isConnected and
serial.setPort usage inside the poller to only run when not isUploading and
ensure viewProvider.syncState() behavior is unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Owner
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
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
1 participant
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.
Summary by CodeRabbit
New Features
Bug Fixes