Fix: Virtual callback is handled in serial.js already

[haslinghuis]

• Callback for virtual connection already handled in serial.js is no longer used in virtualSerial protocol
• Removed unused openCanceled parameter
• Removed unused callback for disconnect

Summary by CodeRabbit

• Breaking Changes
  • Connection API simplified: connect now takes only port and options; callback-based signature removed.
  • Connection behavior changed: connections are immediately marked active and configured, and disconnect now returns a boolean result instead of using a callback.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

VirtualSerial.connect and disconnect signatures were simplified: connect(port, options) now sets connected, connectionId, and bitrate unconditionally and returns a boolean; disconnect() now returns a boolean and no longer accepts a callback. The openCanceled property and related callback usage were removed.

Changes

Cohort / File(s)	Summary
VirtualSerial API changes src/js/protocols/VirtualSerial.js	Removed openCanceled and callback usage. Changed connect(port, options, callback) → connect(port, options) returning boolean and setting connected, connectionId, bitrate. Changed disconnect(callback) → disconnect() returning boolean instead of invoking a callback.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Verify all callers no longer pass/connect callbacks to connect or disconnect.
• Check code paths that depended on callback-based completion or openCanceled.
• Run/update tests that reference old signatures.

Possibly related PRs

• Refactor: Use serial path #4548 — Modifies VirtualSerial.connect API and related connection handling.
• Fix cleanup of autodetect event handlers #4664 — Changes VirtualSerial.connect/disconnect signatures and affects callers like AutoDetect.cleanup().

Suggested reviewers

• nerdCopter
• blckmn
• KarateBrot

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The pull request description is incomplete and does not follow the required template structure for this repository.	Add required sections from the template: issue references (Fixes #), ensure commit message is descriptive, and verify compliance with coding style guidelines and PR size constraints.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check	✅ Passed	The PR title 'Fix: Virtual callback is handled in serial.js already' clearly summarizes the main change: removing unnecessary callback handling from VirtualSerial since it's already handled elsewhere.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/js/protocols/VirtualSerial.js (1)

28-39: Consider removing callback from disconnect for API consistency.

The disconnect method still accepts a callback parameter while connect no longer does. If callback handling is centralized in serial.js (as stated in the PR description), consider updating disconnect to match the new pattern for API consistency.

Example:

-    disconnect(callback) {
+    disconnect() {
         this.connected = false;
         this.outputBuffer = [];
         this.transmitting = false;
         if (this.connectionId) {
             this.connectionId = false;
             this.bitrate = 0;
-            if (callback) {
-                callback(true);
-            }
         }
     }

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1de94ca and 2c42a66.

📒 Files selected for processing (1)

• src/js/protocols/VirtualSerial.js (1 hunks)

🧰 Additional context used

🧠 Learnings (1)

📓 Common learnings

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-14T20:22:03.580Z
Learning: In betaflight-configurator's msp.js, the bluetooth CRC bypass check at line 266 uses serial.protocol === "bluetooth". When protocol names are unified to "ble", this check must be updated to serial.protocol === "ble".

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4510
File: src/js/msp.js:384-391
Timestamp: 2025-09-19T20:41:44.286Z
Learning: When fixing MSP duplicate handling in Betaflight Configurator, avoid complex changes to callback resolution mechanisms as they can break tab switching functionality. Simple duplicate detection based on code and payload size is safer than complex requestKey-based approaches.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4510
File: src/js/msp.js:384-391
Timestamp: 2025-09-19T20:42:20.332Z
Learning: Complex MSP duplicate handling fixes in Betaflight Configurator can cause infinite loading messages when changing tabs due to disruption of the callback resolution mechanism. Simple code-only duplicate detection (using this.callbacks.some((instance) => instance.code === code)) is the safer approach that preserves tab switching functionality.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4576
File: src/js/port_usage.js:17-23
Timestamp: 2025-08-22T16:43:20.901Z
Learning: In betaflight-configurator, the serial facade architecture requires accessing metrics like bitrate, bytesReceived, and bytesSent from serial._protocol rather than the top-level serial object. This change maintains compatibility with existing port utilization calculations that have been stable for over 11 years.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[github-actions]

🎉 Do you want to test this code? 🎉

• 🌐 Progressive Web Application (PWA)
• 🤖 Android APK

⚠️ CAUTION: The build may be unstable and result in corrupted configurations or data loss. Use only for testing! ⚠️

[nerdCopter]

• brief successful test, flash, paste diff, pid-tab. (with reset permissions)