Skip to content

Conversation

@haslinghuis
Copy link
Member

@haslinghuis haslinghuis commented Dec 1, 2025

  • when request fails port_handler should fall back to default port selected.

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced handling for cancelled or denied device permissions, ensuring the system automatically reverts to default device selection when permission requests fail.

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

@haslinghuis haslinghuis added this to the 2025.12 milestone Dec 1, 2025
@haslinghuis haslinghuis self-assigned this Dec 1, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 1, 2025

Walkthrough

On permission request cancellation or failure for any protocol device, this.portPicker.selectedPort is reset to DEFAULT_PORT to ensure a fallback when permission is not granted.

Changes

Cohort / File(s) Change Summary
Permission Handling
src/js/port_handler.js
Reset selectedPort to DEFAULT_PORT on permission request cancellation or failure for protocol devices

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Single file with localized logic change
  • Straightforward fallback reset mechanism
  • No structural or multi-component interactions

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning No pull request description was provided by the author; only the template instructions are present. Add a detailed pull request description explaining the bug, the fix, and any relevant context. Reference the issue number using 'Fixes #' format.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: setting default port when permission request fails, which aligns with the changeset's focus on resetting selectedPort to DEFAULT_PORT on permission failure.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch haslinghuis-patch-1

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.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 1, 2025

Copy link
Contributor

@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: 0

🧹 Nitpick comments (1)
src/js/port_handler.js (1)

168-174: Good fix for permission cancellation. Consider handling errors consistently.

The change correctly resets selectedPort to DEFAULT_PORT when permission is cancelled or fails, which aligns with the PR objective.

For consistency, consider also resetting in the catch block so errors leave the UI in the same predictable state:

 } catch (error) {
     console.error(`${this.logHead} Error requesting permission for ${protocol} device:`, error);
+    this.portPicker.selectedPort = DEFAULT_PORT;
 }
📜 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 41cb8c1 and caee2a7.

📒 Files selected for processing (1)
  • src/js/port_handler.js (1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-06-09T00:32:21.385Z
Learning: In the betaflight-configurator codebase, port paths use counter prefixes (e.g., "bluetooth1", "bluetooth2", "serial1") rather than direct protocol identifiers. The protocol selection logic correctly uses `portPath.startsWith("bluetooth")` to detect bluetooth ports regardless of the counter suffix, rather than direct string matching against protocol map keys.
Learnt from: blckmn
Repo: betaflight/betaflight-configurator PR: 4521
File: src/js/protocols/WebSerial.js:148-151
Timestamp: 2025-06-19T22:13:09.136Z
Learning: In WebSerial.js, there's a timing issue where the cached `this.ports` array doesn't immediately reflect newly permitted devices after `requestPermissionDevice()` completes. The `getDevices()` method needs to refresh the device list from the browser API to return accurate data immediately following a permission request and user acceptance.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4379
File: src/js/protocols/TauriSerial.js:203-259
Timestamp: 2025-10-25T21:16:32.474Z
Learning: In TauriSerial (src/js/protocols/TauriSerial.js), the requestPermissionDevice() method is not needed and not invoked. Tauri automatically discovers serial devices through the constructor's loadDevices() and startDeviceMonitoring() calls, bypassing the browser permission model that WebSerial requires. Devices are auto-detected via a 1-second polling interval without user permission prompts.
📚 Learning: 2025-06-19T22:13:09.136Z
Learnt from: blckmn
Repo: betaflight/betaflight-configurator PR: 4521
File: src/js/protocols/WebSerial.js:148-151
Timestamp: 2025-06-19T22:13:09.136Z
Learning: In WebSerial.js, there's a timing issue where the cached `this.ports` array doesn't immediately reflect newly permitted devices after `requestPermissionDevice()` completes. The `getDevices()` method needs to refresh the device list from the browser API to return accurate data immediately following a permission request and user acceptance.

Applied to files:

  • src/js/port_handler.js
📚 Learning: 2025-10-25T21:16:32.474Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4379
File: src/js/protocols/TauriSerial.js:203-259
Timestamp: 2025-10-25T21:16:32.474Z
Learning: In TauriSerial (src/js/protocols/TauriSerial.js), the requestPermissionDevice() method is not needed and not invoked. Tauri automatically discovers serial devices through the constructor's loadDevices() and startDeviceMonitoring() calls, bypassing the browser permission model that WebSerial requires. Devices are auto-detected via a 1-second polling interval without user permission prompts.

Applied to files:

  • src/js/port_handler.js
📚 Learning: 2025-06-09T00:32:21.385Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-06-09T00:32:21.385Z
Learning: In the betaflight-configurator codebase, port paths use counter prefixes (e.g., "bluetooth1", "bluetooth2", "serial1") rather than direct protocol identifiers. The protocol selection logic correctly uses `portPath.startsWith("bluetooth")` to detect bluetooth ports regardless of the counter suffix, rather than direct string matching against protocol map keys.

Applied to files:

  • src/js/port_handler.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build

@github-actions
Copy link
Contributor

github-actions bot commented Dec 1, 2025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: App

Development

Successfully merging this pull request may close these issues.

3 participants