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 (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds integration with the Changes
Sequence DiagramsequenceDiagram
participant IDE as pioarduino IDE
participant Ext as Extension
participant SPM as SerialPortManager
participant Port as Serial Port
IDE->>Ext: onWillUpload(targetPort)
activate Ext
Ext->>Ext: normalize target & selected (macOS /dev/cu.* vs /dev/tty.*)
alt target matches selected or target undefined
Ext->>SPM: releasePort()
activate SPM
SPM->>SPM: save suspended path & baud
SPM->>Port: disconnect()
deactivate SPM
else target differs
Note over Ext: do not release port
end
deactivate Ext
IDE->>IDE: upload proceeds
IDE->>Ext: onDidUpload(exitCode)
activate Ext
Ext->>Ext: reacquireWithRetry() with backoff
loop retry attempts
Ext->>SPM: reacquirePort()
activate SPM
SPM->>SPM: restore suspended path & baud
SPM->>Port: connect()
deactivate SPM
end
deactivate Ext
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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: 2
🤖 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 472-475: The normalizePortPath function currently only swaps
/dev/cu. to /dev/tty. but leaves casing intact, causing case-sensitive
mismatches (e.g., COM3 vs com3); modify normalizePortPath to also normalize case
(e.g., call toLowerCase or toLocaleLowerCase on the returned path) so port-path
comparisons are case-insensitive, and update any code that compares ports to use
normalizePortPath(port) on both sides (refer to normalizePortPath).
In `@src/serialPortManager.ts`:
- Around line 236-245: reacquirePort currently ignores a failed reconnect
because it awaits this.connect() (which can resolve to false) but always
resolves successfully; change reacquirePort so it surfaces connect() failures by
capturing the result of await this.connect() and either (a) return a boolean
success flag (change signature to Promise<boolean>) and return the connect()
result, or (b) throw an error when connect() returns false so callers can retry;
update any callers of reacquirePort accordingly. Ensure you reference the
existing symbols: method reacquirePort, the call to this.connect(), and the
suspended state fields (_suspendedPath, _suspendedBaudRate) when implementing
the change.
🪄 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: da2a9fb8-2ce4-4085-bee4-8b97743bbc80
📒 Files selected for processing (2)
src/extension.tssrc/serialPortManager.ts
Owner
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
Owner
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
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 450-455: In the catch block handling reconnect attempts (the
try/catch that uses baseDelayMs, attempt, maxAttempts and calls log.appendLine),
avoid logging "retrying" and awaiting the backoff when attempt === maxAttempts;
instead, when attempt === maxAttempts, log a final failure message (or
rethrow/return) and skip the setTimeout so no extra sleep happens — only call
log.appendLine with the retrying message and await the delay when attempt <
maxAttempts.
In `@src/serialPortManager.ts`:
- Around line 221-229: The releasePath for uploads currently swallows port close
failures: update disconnect() so it propagates/throws when serial port.close()
reports an error instead of resolving silently, and ensure releasePort() (method
releasePort) awaits disconnect() and does not catch or convert that rejection
into a success; specifically, in disconnect()/SerialPortManager where you call
port.close(), remove or rework any try/catch that masks the error and rethrow
the close error (or return a rejected promise), and keep releasePort()'s await
this.disconnect() so callers of waitUntil(...) will receive the rejection and
abort/retry the upload.
- Around line 236-249: reacquirePort currently calls connect() which will
disconnect an already active session; to avoid tearing down a user-reconnected
session, add an early check in reacquirePort: after verifying
this._suspendedPath exists, if this._isConnected is true then clear
this._suspendedPath and this._suspendedBaudRate and return without touching
this._selectedPath or calling connect(); otherwise proceed with the existing
logic (set this._selectedPath/this._baudRate and call connect(), and only clear
the suspended snapshot after a successful connection).
🪄 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: 8b35046e-1a88-42b9-83e3-a205cff3fd47
📒 Files selected for processing (2)
src/extension.tssrc/serialPortManager.ts
This was referenced Apr 9, 2026
This was referenced May 5, 2026
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.
Uh oh!
There was an error while loading. Please reload this page.