Skip to content

feature: API to open Terminal from pioarduino#51

Merged
Jason2866 merged 3 commits into
mainfrom
caller_api
May 8, 2026
Merged

feature: API to open Terminal from pioarduino#51
Jason2866 merged 3 commits into
mainfrom
caller_api

Conversation

@Jason2866
Copy link
Copy Markdown
Owner

@Jason2866 Jason2866 commented May 8, 2026

Summary by CodeRabbit

  • New Features
    • The monitor command now accepts optional parameters for configurable port selection and baud rate.
    • Automatic connection is now supported when opening the monitor with port parameters.
    • Programmatic APIs are available to set serial port and baud rate dynamically.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 8, 2026

Review Change Stack

Warning

Rate limit exceeded

@Jason2866 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 29 minutes and 21 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4a39ab17-7188-4924-a86e-9a1fc2ab49c3

📥 Commits

Reviewing files that changed from the base of the PR and between eed2fdb and fcf419c.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (4)
  • CHANGELOG.md
  • package.json
  • src/extension.ts
  • src/serialPortManager.ts
📝 Walkthrough

Walkthrough

The PR enables programmatic configuration of serial port selection and baud rate. SerialPortManager exposes two new public setters; the esp-decoder.openMonitor command accepts optional port, baud rate, and auto-connect parameters, applies them via the new setters, shows the monitor UI, and optionally performs automatic connection with error handling.

Changes

Programmatic Port Configuration API

Layer / File(s) Summary
SerialPortManager API
src/serialPortManager.ts
New public methods setPort(path: string) and setBaudRate(rate: number) allow external callers to override port and baud rate; setBaudRate validates the rate is finite and positive.
OpenMonitor Command Enhancement
src/extension.ts
Command accepts optional { port?, baudRate?, autoConnect? } parameters, applies them via SerialPortManager setters, opens the monitor UI in editor or panel mode, and optionally auto-connects with port-aware disconnect and error handling.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • Jason2866/ESP-Decoder#29: Modifies SerialPortManager constructor and connection/disconnection/error handling logic; this PR adds programmatic setter APIs to the same class.

Poem

🐰 A port, a rate, a command so sweet,
Now programmed paths make integration neat,
Auto-connect flows through the manager's door,
Configuration blooms where setters explore! 🔌✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Title check ⚠️ Warning The PR title references opening a Terminal from pioarduino, but the actual changes implement an API to open the ESP Decoder monitor with optional port/baud rate configuration and auto-connect functionality. Update the title to accurately reflect the main change, such as: 'feature: Add API to programmatically open monitor with port configuration and auto-connect'
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch caller_api

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/extension.ts`:
- Around line 115-123: The current auto-connect logic only reconnects when the
port changes, so if opts.port matches serialManager.selectedPath but
opts.baudRate changed the live connection stays at the old speed; update the
branch around serialManager.connect()/disconnect() to also handle baud-rate-only
changes: detect when serialManager.isConnected && serialManager.selectedPath ===
opts.port but opts.baudRate !== serialManager.baudRate (or wherever the current
baud rate is stored) and then either call a method to apply the new rate (e.g.,
serialManager.setBaudRate(opts.baudRate) if available) or perform await
serialManager.disconnect() followed by await serialManager.connect() so the new
opts.baudRate takes effect; reference serialManager.isConnected,
serialManager.selectedPath, serialManager.baudRate (or stored value),
opts.baudRate, connect(), disconnect(), and setBaudRate() in the change.

In `@src/serialPortManager.ts`:
- Around line 99-101: The setPort method currently assigns path directly to
_selectedPath allowing whitespace-only or empty values; update setPort(path:
string) to sanitize and validate the input by trimming whitespace (e.g., path =
path.trim()), then check for a non-empty string before mutating
this._selectedPath; if the trimmed path is empty, either clear _selectedPath or
leave it unchanged and surface an error (throw or return a boolean) so later
calls like connect() cannot operate on an invalid port value.
- Around line 103-108: setBaudRate currently accepts finite positive non-integer
values (e.g. 115200.5) which defers validation to open-time; change the guard in
setBaudRate to require an integer (use Number.isInteger(rate) && rate > 0)
before assigning this._baudRate so decimals are rejected up-front; reference the
setBaudRate method and the this._baudRate field when making 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: fff4b9f7-f716-4bee-8b9d-9169dc7463fa

📥 Commits

Reviewing files that changed from the base of the PR and between 8d404cc and eed2fdb.

📒 Files selected for processing (2)
  • src/extension.ts
  • src/serialPortManager.ts

Comment thread src/extension.ts Outdated
Comment thread src/serialPortManager.ts Outdated
Comment thread src/serialPortManager.ts Outdated
@Jason2866
Copy link
Copy Markdown
Owner Author

@coderabbitai are all review comments fixed by a40c19e

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 8, 2026

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== PR Review Comments ==="
gh pr view 51 --repo Jason2866/ESP-Decoder --json reviews,comments

echo ""
echo "=== PR Review Thread Comments ==="
gh api repos/Jason2866/ESP-Decoder/pulls/51/reviews

echo ""
echo "=== PR Inline Review Comments ==="
gh api repos/Jason2866/ESP-Decoder/pulls/51/comments --jq '.[] | {id, path, line, body, user: .user.login, diff_hunk}'

echo ""
echo "=== Commit diff for a40c19e ==="
gh api repos/Jason2866/ESP-Decoder/commits/a40c19e12471958539950a0b2d33d5f079c6ef9f --jq '.files[] | {filename, patch}'

Length of output: 35967


@Jason2866 Yes, all three review comments are fully addressed by commit a40c19e. Here's a summary of what was fixed:

  1. src/extension.ts — Auto-connect misses baud-rate-only reconnect
    prevPort and prevBaud are now captured before applying the new options. The reconnect guard now checks portChanged || baudChanged, so a baud-rate-only change correctly triggers a disconnect + reconnect.

  2. src/serialPortManager.tssetPort input sanitization
    setPort now trims the input, returns false (no-op) for empty/whitespace-only strings, and returns true on success — preventing an invalid _selectedPath from reaching connect().

  3. src/serialPortManager.ts — Integer-only baud rates in setBaudRate
    The guard was changed from Number.isFinite(rate) && rate > 0 to Number.isInteger(rate) && rate > 0 (inverted with early return), so decimal values like 115200.5 are rejected up-front rather than deferred to open-time.

@Jason2866 Jason2866 merged commit a561e56 into main May 8, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant