feat: Migrate Ports Tab to Vue #4726
Conversation
WalkthroughReplaces the legacy ports tab (JS + HTML) with a Vue 3 single-file component PortsTab.vue, updates tab mounting to use the Vue mounter, adjusts tab mount lifecycle, registers the new component, and removes the old ports module and template; minor CapacitorSerial whitespace and mount-time tweaks included. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
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 |
Erics1337
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/js/protocols/CapacitorSerial.js (1)
53-60: WraphexStringToUint8Arraycall in try/catch to handle malformed data.The enhanced validation in
hexStringToUint8Arraynow throws on invalid input. If the native plugin ever sends malformed hex data, this exception will propagate uncaught and break the data receive flow.handleDataReceived(event) { - // Convert hex string to Uint8Array - const data = this.hexStringToUint8Array(event.data); - this.bytesReceived += data.length; - - // Dispatch receive event with the data - this.dispatchEvent(new CustomEvent("receive", { detail: data })); + try { + // Convert hex string to Uint8Array + const data = this.hexStringToUint8Array(event.data); + this.bytesReceived += data.length; + + // Dispatch receive event with the data + this.dispatchEvent(new CustomEvent("receive", { detail: data })); + } catch (error) { + console.error(`${logHead} Error parsing received data:`, error); + } }
🧹 Nitpick comments (10)
src/js/protocols/CapacitorSerial.js (1)
129-129: Add braces afterifconditions for consistency.Static analysis flags missing braces on early-return
ifstatements. While these work correctly, adding braces improves readability and aligns with typical style guidelines.Example fix for one instance:
- if (!this.pluginAvailable) return; + if (!this.pluginAvailable) { + return; + }Apply similar changes at lines 143, 167, 242, and 267-270.
Also applies to: 143-143, 167-167, 242-242, 267-270
src/js/main.js (1)
301-304: Consider consistent commenting style.The servos case has a comment explaining the Vue tab approach, but the ports case (line 264) doesn't. For consistency, either add a similar comment to ports or remove the comment from servos.
src/js/vue_components.js (1)
25-27: Consider if global registration is necessary for tab components.Tab components are dynamically mounted via
VueTabComponentsregistry invue_tab_mounter.js, so they may not need global registration inBetaflightComponents. However, keeping them registered globally allows using<ServosTab>or<PortsTab>directly in templates if needed in the future.src/components/tabs/ServosTab.vue (2)
17-17: Replace deprecatedwidthattribute with CSS.The
widthattribute on<th>is deprecated in HTML5. Use CSS instead.- <th width="110px">{{ $t("servosName") }}</th> + <th style="width: 110px">{{ $t("servosName") }}</th>
165-168: Consider makingrateOptionsa computed or top-level constant.
rateOptionsis built once in setup and never changes. Moving it outsidesetup()as a module-level constant would be cleaner and avoid recreating the array on each component instantiation.+// Generate rate options from 100 to -100 (module-level constant) +const RATE_OPTIONS = Array.from({ length: 201 }, (_, i) => 100 - i); + export default defineComponent({ name: "ServosTab", components: { BaseTab, }, setup() { // ... - // Generate rate options from 100 to -100 - const rateOptions = []; - for (let i = 100; i > -101; i--) { - rateOptions.push(i); - } + const rateOptions = RATE_OPTIONS;src/components/tabs/PortsTab.vue (5)
47-55: Form labels lack visible text for accessibility.The
<label>elements are empty and rely on CSS for visual indication. While theforattribute correctly associates them with inputs, screen readers need accessible text.<input type="checkbox" class="togglemedium" :id="`msp-${index}`" v-model="port.msp" :disabled="port.identifier === 20" /> - <label :for="`msp-${index}`"></label> + <label :for="`msp-${index}`"> + <span class="sr-only">{{ $t("portsMSP") }}</span> + </label>Apply the same pattern for the Serial RX checkbox label at line 70.
Also applies to: 64-71
323-335: Refactor nested callbacks using async/await.The nested callback structure exceeds 4 levels and can be simplified. Since
MSP.promise()is already used on line 324, extend its usage.- const loadConfig = () => { - MSP.promise(MSPCodes.MSP_VTX_CONFIG).then(() => { - mspHelper.loadSerialConfig(() => { - ports.length = 0; - FC.SERIAL_CONFIG.ports.forEach((p) => { - ports.push(transformPortData(p)); - }); - nextTick(() => { - GUI.content_ready(); - }); - }); - }); - }; + const loadConfig = async () => { + await MSP.promise(MSPCodes.MSP_VTX_CONFIG); + await new Promise((resolve) => mspHelper.loadSerialConfig(resolve)); + + ports.length = 0; + FC.SERIAL_CONFIG.ports.forEach((p) => { + ports.push(transformPortData(p)); + }); + + await nextTick(); + GUI.content_ready(); + };
391-403: Deep callback nesting in saveConfig.The save flow has 4+ levels of nesting. Consider using async/await for clarity.
- mspHelper.sendSerialConfig(() => { - MSP.send_message( - MSPCodes.MSP_SET_FEATURE_CONFIG, - mspHelper.crunch(MSPCodes.MSP_SET_FEATURE_CONFIG), - false, - () => { - mspHelper.writeConfiguration(true, () => { - gui_log(i18n.getMessage("portsEepromSave")); - }); - }, - ); - }); + await new Promise((resolve) => mspHelper.sendSerialConfig(resolve)); + await MSP.promise( + MSPCodes.MSP_SET_FEATURE_CONFIG, + mspHelper.crunch(MSPCodes.MSP_SET_FEATURE_CONFIG), + ); + await new Promise((resolve) => mspHelper.writeConfiguration(true, resolve)); + gui_log(i18n.getMessage("portsEepromSave"));This requires making
saveConfigan async function.
405-428: Reduce cognitive complexity in updateFeatures.This function exceeds the allowed cognitive complexity (16 vs 15). Refactor using a mapping approach.
const updateFeatures = () => { - // Logic from original ports.js lines 440-498 - let enableRxSerial = false; - let enableTelemetry = false; - let enableBlackbox = false; - let enableEsc = false; - let enableGps = false; - - for (const port of FC.SERIAL_CONFIG.ports) { - const func = port.functions; - if (func.includes("RX_SERIAL")) enableRxSerial = true; - if (func.some((e) => e.startsWith("TELEMETRY"))) enableTelemetry = true; - if (func.includes("BLACKBOX")) enableBlackbox = true; - if (func.includes("ESC_SENSOR")) enableEsc = true; - if (func.includes("GPS")) enableGps = true; - } - - const featureConfig = FC.FEATURE_CONFIG.features; - enableRxSerial ? featureConfig.enable("RX_SERIAL") : featureConfig.disable("RX_SERIAL"); - if (enableTelemetry) featureConfig.enable("TELEMETRY"); // Note: never disabled in original? - enableBlackbox ? featureConfig.enable("BLACKBOX") : featureConfig.disable("BLACKBOX"); - enableEsc ? featureConfig.enable("ESC_SENSOR") : featureConfig.disable("ESC_SENSOR"); - enableGps ? featureConfig.enable("GPS") : featureConfig.disable("GPS"); + const allFunctions = FC.SERIAL_CONFIG.ports.flatMap((p) => p.functions); + const featureConfig = FC.FEATURE_CONFIG.features; + + const featureMap = { + RX_SERIAL: allFunctions.includes("RX_SERIAL"), + BLACKBOX: allFunctions.includes("BLACKBOX"), + ESC_SENSOR: allFunctions.includes("ESC_SENSOR"), + GPS: allFunctions.includes("GPS"), + }; + + for (const [feature, enabled] of Object.entries(featureMap)) { + enabled ? featureConfig.enable(feature) : featureConfig.disable(feature); + } + + // TELEMETRY is only enabled, never disabled (per original behavior) + if (allFunctions.some((f) => f.startsWith("TELEMETRY"))) { + featureConfig.enable("TELEMETRY"); + } };
297-303: Potential false positive for disabled rules when buildOptions is empty.When
FC.CONFIG.buildOptions.lengthis 0 (falsy), the function returnsfalse, meaning all rules withdependsOnare enabled. This is correct behavior for builds without build options, but the logic is slightly confusing.Consider making the intent clearer:
const isRuleDisabled = (rule) => { - return ( - FC.CONFIG.buildOptions.length && - rule.dependsOn !== undefined && - !FC.CONFIG.buildOptions.includes(rule.dependsOn) - ); + // Rules are enabled if: no build options exist, rule has no dependency, or dependency is met + if (!FC.CONFIG.buildOptions.length || rule.dependsOn === undefined) { + return false; + } + return !FC.CONFIG.buildOptions.includes(rule.dependsOn); };
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/components/tabs/BaseTab.vue(1 hunks)src/components/tabs/PortsTab.vue(1 hunks)src/components/tabs/ServosTab.vue(1 hunks)src/js/main.js(3 hunks)src/js/protocols/CapacitorSerial.js(8 hunks)src/js/tabs/ports.js(0 hunks)src/js/tabs/servos.js(0 hunks)src/js/vue_components.js(2 hunks)src/js/vue_tab_mounter.js(1 hunks)src/tabs/ports.html(0 hunks)src/tabs/servos.html(0 hunks)
💤 Files with no reviewable changes (4)
- src/js/tabs/ports.js
- src/tabs/servos.html
- src/js/tabs/servos.js
- src/tabs/ports.html
🧰 Additional context used
🧠 Learnings (13)
📚 Learning: 2025-06-20T12:35:49.283Z
Learnt from: mituritsyn
Repo: betaflight/betaflight-configurator PR: 4526
File: src/js/gui.js:43-43
Timestamp: 2025-06-20T12:35:49.283Z
Learning: In the Betaflight Configurator codebase, tabs in `defaultCloudBuildTabOptions` are conditionally displayed based on firmware build options. The logic in `serial_backend.js` checks `FC.CONFIG.buildOptions` and only adds tabs to `GUI.allowedTabs` if the firmware was built with support for that feature.
Applied to files:
src/components/tabs/ServosTab.vuesrc/js/vue_components.jssrc/components/tabs/PortsTab.vue
📚 Learning: 2025-09-10T18:26:10.136Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4601
File: package.json:125-125
Timestamp: 2025-09-10T18:26:10.136Z
Learning: In betaflight-configurator, dependency updates are handled incrementally - Vue dependencies are updated separately from Vite dependencies for better isolation and maintainability.
Applied to files:
src/js/vue_components.js
📚 Learning: 2025-12-01T17:09:13.412Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-01T17:09:13.412Z
Learning: In betaflight-configurator’s CapacitorBluetooth.js, native plugin listeners (notification, services, connectionState) must be attached exactly once per app lifetime. Re-attaching them during writes/connect cycles leads to duplicate handlers, increased JS workload, MSP queue pressure, and can prevent the MSP_EEPROM_WRITE callback from firing (so reinitializeConnection is not reached). Implement an idempotent _attachNativeListenersOnce() guard.
Applied to files:
src/js/protocols/CapacitorSerial.js
📚 Learning: 2025-11-24T15:07:25.227Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4706
File: src/js/protocols/CapacitorSerial.js:47-47
Timestamp: 2025-11-24T15:07:25.227Z
Learning: In betaflight-configurator's CapacitorSerial protocol (src/js/protocols/CapacitorSerial.js), the fire-and-forget this.loadDevices() call in the constructor is intentional background pre-loading. The actual critical device list loading for UI population is orchestrated by port_handler.js via serial.getDevices("serial"), which properly awaits CapacitorSerial.getDevices() → loadDevices() in a complete async chain. The constructor's async call does not cause race conditions.
Applied to files:
src/js/protocols/CapacitorSerial.js
📚 Learning: 2025-11-28T22:41:59.374Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:135-143
Timestamp: 2025-11-28T22:41:59.374Z
Learning: In betaflight-configurator, WebSerial.js uses a constant path "serial" for all devices, and CapacitorBluetooth.js uses constant path "bluetooth", consistent with the single-device-at-a-time workflow. WebBluetooth.js uses counter-based paths (bluetooth_${counter}), but that's specific to its implementation. The path strategy varies by protocol based on their specific requirements.
Applied to files:
src/js/protocols/CapacitorSerial.js
📚 Learning: 2025-11-28T22:51:11.691Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-28T22:51:11.691Z
Learning: In betaflight-configurator, Web Bluetooth API's `services` option filters scan results to only devices advertising those UUIDs, while `optionalServices` requests permission to access services after connection without filtering. PWA (WebBluetooth.js) uses `{ acceptAllDevices: true, optionalServices: uuids }` to show all devices while requesting post-connection service access. CapacitorBluetooth.js should match this pattern rather than using `services` as a scan filter.
Applied to files:
src/js/protocols/CapacitorSerial.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/protocols/CapacitorSerial.js
📚 Learning: 2025-11-22T21:18:08.814Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4706
File: src/js/protocols/CapacitorSerial.js:11-46
Timestamp: 2025-11-22T21:18:08.814Z
Learning: In betaflight-configurator, protocol implementations (WebSerial, CapacitorSerial, WebBluetooth, etc.) are instantiated once in the Serial class constructor and stored as singletons in the _protocols array for the entire application lifetime. They are never destroyed or recreated, so cleanup methods to remove event listeners are not needed.
Applied to files:
src/js/protocols/CapacitorSerial.js
📚 Learning: 2025-08-22T16:43:20.901Z
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.
Applied to files:
src/js/protocols/CapacitorSerial.js
📚 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/protocols/CapacitorSerial.js
📚 Learning: 2025-11-06T17:26:50.201Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4687
File: src/js/protocols/TauriSerial.js:299-310
Timestamp: 2025-11-06T17:26:50.201Z
Learning: In tauri-plugin-serialplugin Android development, the Android project must be initialized via `tauri android init` before the USB support patches (AndroidManifest.xml permissions, device_filter.xml, MainActivity.kt) can be applied. Without initialization, the plugin returns empty results from available_ports_direct() because the generated Android project structure and USB permissions don't exist yet.
Applied to files:
src/js/protocols/CapacitorSerial.js
📚 Learning: 2025-06-27T22:06:49.210Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4532
File: src/js/VirtualFC.js:234-234
Timestamp: 2025-06-27T22:06:49.210Z
Learning: In the betaflight-configurator codebase, the VirtualFC.js AUX_CONFIG array must stay synchronized with the betaflight firmware's msp_box.c definitions to ensure proper auxiliary mode functionality. Changes to mode names should follow the firmware source code even if it potentially breaks backward compatibility with saved presets.
Applied to files:
src/components/tabs/PortsTab.vue
📚 Learning: 2025-09-19T20:42:20.332Z
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.
Applied to files:
src/components/tabs/PortsTab.vue
🧬 Code graph analysis (3)
src/js/main.js (1)
src/js/vue_tab_mounter.js (1)
mountVueTab(31-80)
src/js/vue_tab_mounter.js (2)
src/js/vue_components.js (2)
VueTabComponents(11-14)VueTabComponents(11-14)src/js/gui.js (1)
GUI(473-473)
src/js/vue_components.js (2)
src/components/init.js (1)
app(60-64)src/js/vue_loader.js (1)
app(42-42)
🪛 GitHub Check: SonarCloud Code Analysis
src/js/vue_tab_mounter.js
[warning] 63-63: Prefer globalThis over window.
[warning] 62-62: Prefer globalThis over window.
src/components/tabs/ServosTab.vue
[warning] 6-6: Anchors must have content and the content must be accessible by a screen reader.
[warning] 17-17: Remove this deprecated "width" attribute.
[failure] 255-255: Refactor this code to not nest functions more than 4 levels deep.
[warning] 171-171: Move function 'getBarStyle' to the outer scope.
src/js/protocols/CapacitorSerial.js
[failure] 129-129: Expected { after 'if' condition.
[failure] 143-143: Expected { after 'if' condition.
[failure] 167-167: Expected { after 'if' condition.
[failure] 242-242: Expected { after 'if' condition.
[failure] 268-268: Expected { after 'if' condition.
src/components/tabs/PortsTab.vue
[failure] 417-417: Expected { after 'if' condition.
[failure] 419-419: Expected { after 'if' condition.
[failure] 397-397: Refactor this code to not nest functions more than 4 levels deep.
[failure] 369-369: Expected { after 'if' condition.
[warning] 54-54: A form label must be associated with a control.
[failure] 371-371: Expected { after 'if' condition.
[failure] 327-327: Refactor this code to not nest functions more than 4 levels deep.
[failure] 370-370: Expected { after 'if' condition.
[failure] 424-424: Expected { after 'if' condition.
[failure] 372-372: Expected { after 'if' condition.
[failure] 373-373: Expected { after 'if' condition.
[failure] 415-415: Expected { after 'if' condition.
[failure] 405-405: Refactor this function to reduce its Cognitive Complexity from 16 to the 15 allowed.
[failure] 418-418: Expected { after 'if' condition.
[warning] 70-70: A form label must be associated with a control.
[failure] 443-443: Expected { after 'if' condition.
[failure] 330-330: Refactor this code to not nest functions more than 4 levels deep.
[warning] 6-6: Anchors must have content and the content must be accessible by a screen reader.
[failure] 416-416: Expected { after 'if' condition.
⏰ 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
🔇 Additional comments (14)
src/js/protocols/CapacitorSerial.js (2)
312-325: Good addition of input validation.The 0x prefix stripping, character validation, and even-length check provide robust input handling for hex string conversion.
15-48: LGTM!The
pluginAvailableguard pattern is a good defensive measure that prevents native plugin calls when the plugin is unavailable, ensuring graceful degradation on unsupported platforms.src/components/tabs/BaseTab.vue (2)
32-49: LGTM!The setup correctly manages tab lifecycle: setting
GUI.active_tabon mount and cleaning up intervals/timeouts on unmount. ThebetaflightModelinjection with null fallback is appropriate.
1-5: Clean template structure.The dynamic class binding
tab-${tabName}follows the existing tab naming convention and the slot pattern enables flexible content composition.src/js/main.js (1)
263-265: LGTM!The Vue tab mounting integration for the ports tab is clean and maintains compatibility with the existing
content_readycallback pattern.src/js/vue_components.js (1)
10-14: LGTM!The
VueTabComponentsregistry provides a clean lookup mechanism for dynamic tab mounting invue_tab_mounter.js.src/js/vue_tab_mounter.js (3)
31-80: LGTM! Well-structured tab mounting utility.The implementation correctly:
- Validates component existence and target element
- Cleans up previous tab before mounting new one
- Configures i18n and provides the betaflight model
- Defers callback execution to ensure Vue has completed mounting
85-89: LGTM!The
unmountVueTabcleanup function properly handles the singleton app instance and null checks.
61-64: Consider usingglobalThisinstead ofwindow.Static analysis suggests preferring
globalThisoverwindowfor broader compatibility (e.g., web workers, SSR). However, if the codebase consistently useswindow.vmelsewhere, maintaining consistency may be more important.// Provide the global betaflight model - if (window.vm) { - currentTabApp.provide("betaflightModel", window.vm); + if (globalThis.vm) { + currentTabApp.provide("betaflightModel", globalThis.vm); }src/components/tabs/ServosTab.vue (3)
29-74: LGTM!The servo configuration table implementation is well-structured with proper v-for loops, reactive bindings, and event handlers. The channel forwarding checkbox logic using
:checkedand@changecorrectly implements radio-like behavior within each row.
302-306: LGTM!Proper cleanup of locally tracked intervals in
onUnmounted. This prevents memory leaks and ensures the component doesn't leave orphaned interval handlers.
253-261: Nested callback structure is consistent with codebase patterns.The 4-level nested callbacks follow the established callback-based MSP pattern used throughout betaflight-configurator. The sequential dependency between these MSP calls (servo configurations → servo mix rules → RC → box names → UI initialization) is appropriately expressed through nested callbacks.
MSP.promise()is not available in this codebase, so async/await refactoring is not applicable here. The current structure is the correct approach for this context.src/components/tabs/PortsTab.vue (2)
168-219: LGTM!The
functionRulesarray is well-structured with clear definitions for groups, max ports, sharing rules, and firmware dependencies. The conditional rule addition for API version 1.45+ and localization loop are correctly implemented.
252-275: Port identifier mapping uses separate ranges for different hardware platforms.Identifiers 1-7 and 51-57 map to overlapping UART names for different MCU peripheral types or flight controller variants. This aligns with Betaflight's multi-platform architecture but could benefit from clarification in comments about which ranges apply to which hardware families.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/components/tabs/PortsTab.vue (4)
42-162: Consider restricting non‑MSP functions on USB VCP for parity with legacy UI.Right now only the MSP checkbox is disabled for
identifier === 20(USB VCP); Serial RX, telemetry, sensors, peripherals, and baudrate selects remain fully editable. On older Configurator builds, USB VCP was typically MSP‑only and other functions weren’t assignable on that port.If legacy
ports.jsalso prevented configuring non‑MSP functions on VCP, mirroring that here (e.g., disabling the RX checkbox and the select elements whenport.identifier === 20) would avoid confusing users with options that the FC can’t meaningfully use.
198-261: functionRules metadata is only partially used; either wire it in or trim for clarity.
maxPorts,sharableWith, andnotSharableWithare defined on many rules, but the current logic only consultsdependsOnviaisRuleDisabled(). If you’re not yet enforcingmaxPortsor sharing constraints, this metadata is effectively dead and can confuse future readers.Either:
- Implement the remaining rule logic (e.g., to disable options once
maxPortsis reached, or enforce sharing constraints), or- Drop the unused properties for now and reintroduce them when the corresponding behavior is ported.
This keeps the rules table aligned with the actual behavior.
354-381: Add basic error handling around MSP_VTX_CONFIG load to avoid stuck “loading” states.
loadConfig()only proceeds onMSP.promise(MSPCodes.MSP_VTX_CONFIG).then(...); if that promise rejects (e.g., older firmware or transient MSP error),mspHelper.loadSerialConfigandGUI.content_ready()are never called, so the Ports tab may appear to hang.Consider adding a
.catch/.finallyto either:
- Log the failure and still call
mspHelper.loadSerialConfig(handleSerialConfigLoaded), or- At least call
GUI.content_ready()so the tab becomes usable even if VTX config isn’t available.That keeps the tab resilient when VTX support is missing or misconfigured.
450-468: Add braces togetEnabledFeaturesFromPortsifstatements (and satisfy Sonar).The single‑line
ifstatements ingetEnabledFeaturesFromPortsare functionally correct but violate the project’s brace style and match the Sonar “Expected { after 'if' condition” warnings.You can fix this and improve readability with:
for (const port of ports) { const func = port.functions; - if (func.includes("RX_SERIAL")) flags.rxSerial = true; - if (func.some((e) => e.startsWith("TELEMETRY"))) flags.telemetry = true; - if (func.includes("BLACKBOX")) flags.blackbox = true; - if (func.includes("ESC_SENSOR")) flags.esc = true; - if (func.includes("GPS")) flags.gps = true; + if (func.includes("RX_SERIAL")) { + flags.rxSerial = true; + } + if (func.some((e) => e.startsWith("TELEMETRY"))) { + flags.telemetry = true; + } + if (func.includes("BLACKBOX")) { + flags.blackbox = true; + } + if (func.includes("ESC_SENSOR")) { + flags.esc = true; + } + if (func.includes("GPS")) { + flags.gps = true; + } }This should clear the static analysis findings and keep the style consistent with the nearby
saveConfigmapping.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/components/tabs/PortsTab.vue(1 hunks)src/components/tabs/ServosTab.vue(1 hunks)src/js/protocols/CapacitorSerial.js(8 hunks)src/js/vue_tab_mounter.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/js/protocols/CapacitorSerial.js
- src/components/tabs/ServosTab.vue
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
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.
📚 Learning: 2025-09-19T20:42:20.332Z
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.
Applied to files:
src/components/tabs/PortsTab.vue
📚 Learning: 2025-09-19T20:41:44.286Z
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.
Applied to files:
src/components/tabs/PortsTab.vue
📚 Learning: 2025-06-20T12:35:49.283Z
Learnt from: mituritsyn
Repo: betaflight/betaflight-configurator PR: 4526
File: src/js/gui.js:43-43
Timestamp: 2025-06-20T12:35:49.283Z
Learning: In the Betaflight Configurator codebase, tabs in `defaultCloudBuildTabOptions` are conditionally displayed based on firmware build options. The logic in `serial_backend.js` checks `FC.CONFIG.buildOptions` and only adds tabs to `GUI.allowedTabs` if the firmware was built with support for that feature.
Applied to files:
src/components/tabs/PortsTab.vue
📚 Learning: 2025-06-27T22:06:49.210Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4532
File: src/js/VirtualFC.js:234-234
Timestamp: 2025-06-27T22:06:49.210Z
Learning: In the betaflight-configurator codebase, the VirtualFC.js AUX_CONFIG array must stay synchronized with the betaflight firmware's msp_box.c definitions to ensure proper auxiliary mode functionality. Changes to mode names should follow the firmware source code even if it potentially breaks backward compatibility with saved presets.
Applied to files:
src/components/tabs/PortsTab.vue
📚 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/components/tabs/PortsTab.vue
📚 Learning: 2025-09-02T07:45:48.606Z
Learnt from: blckmn
Repo: betaflight/betaflight-configurator PR: 4583
File: src/js/tabs/firmware_flasher.js:949-961
Timestamp: 2025-09-02T07:45:48.606Z
Learning: In src/js/tabs/firmware_flasher.js, the config file loading code path after firmware loading (in the load_file click handler) cannot be reached when UF2 firmware is loaded, according to the maintainer blckmn. This code is maintained for backward compatibility with unified target settings and will be refactored in a future PR.
Applied to files:
src/components/tabs/PortsTab.vue
📚 Learning: 2025-08-27T22:07:26.353Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4579
File: src/js/data_storage.js:9-9
Timestamp: 2025-08-27T22:07:26.353Z
Learning: In the betaflight-configurator CalVer refactoring, the generateVirtualApiVersions function in src/js/utils/common.js works correctly despite the apparent mathematical issue with bounds logic where compareVersions.minor() returns different ranges for API_VERSION_MAX_SUPPORTED (25.12.0) vs API_VERSION_ACCEPTED (1.44.0).
Applied to files:
src/components/tabs/PortsTab.vue
🧬 Code graph analysis (1)
src/js/vue_tab_mounter.js (3)
src/js/main.js (2)
tabName(139-139)tabName(173-173)src/js/vue_components.js (2)
VueTabComponents(11-14)VueTabComponents(11-14)src/js/gui.js (1)
GUI(473-473)
🪛 GitHub Check: SonarCloud Code Analysis
src/components/tabs/PortsTab.vue
[failure] 461-461: Expected { after 'if' condition.
[failure] 464-464: Expected { after 'if' condition.
[failure] 463-463: Expected { after 'if' condition.
[failure] 462-462: Expected { after 'if' condition.
[failure] 465-465: Expected { after 'if' condition.
🔇 Additional comments (1)
src/js/vue_tab_mounter.js (1)
13-80: Vue tab mounting lifecycle looks solid.Mount/unmount flow is clean: you guard against missing components/
#content, unmount any previous app, clear the container, wire up i18n andbetaflightModel, then resetGUI.tab_switch_in_progressand invoke the legacycontentReadyCallback. This should integrate well with the existing jQuery tab system without leaking Vue apps.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/tabs/PortsTab.vue(1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
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.
📚 Learning: 2025-09-19T20:42:20.332Z
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.
Applied to files:
src/components/tabs/PortsTab.vue
📚 Learning: 2025-09-19T20:41:44.286Z
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.
Applied to files:
src/components/tabs/PortsTab.vue
📚 Learning: 2025-06-20T12:35:49.283Z
Learnt from: mituritsyn
Repo: betaflight/betaflight-configurator PR: 4526
File: src/js/gui.js:43-43
Timestamp: 2025-06-20T12:35:49.283Z
Learning: In the Betaflight Configurator codebase, tabs in `defaultCloudBuildTabOptions` are conditionally displayed based on firmware build options. The logic in `serial_backend.js` checks `FC.CONFIG.buildOptions` and only adds tabs to `GUI.allowedTabs` if the firmware was built with support for that feature.
Applied to files:
src/components/tabs/PortsTab.vue
📚 Learning: 2025-06-27T22:06:49.210Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4532
File: src/js/VirtualFC.js:234-234
Timestamp: 2025-06-27T22:06:49.210Z
Learning: In the betaflight-configurator codebase, the VirtualFC.js AUX_CONFIG array must stay synchronized with the betaflight firmware's msp_box.c definitions to ensure proper auxiliary mode functionality. Changes to mode names should follow the firmware source code even if it potentially breaks backward compatibility with saved presets.
Applied to files:
src/components/tabs/PortsTab.vue
📚 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/components/tabs/PortsTab.vue
📚 Learning: 2025-09-02T07:45:48.606Z
Learnt from: blckmn
Repo: betaflight/betaflight-configurator PR: 4583
File: src/js/tabs/firmware_flasher.js:949-961
Timestamp: 2025-09-02T07:45:48.606Z
Learning: In src/js/tabs/firmware_flasher.js, the config file loading code path after firmware loading (in the load_file click handler) cannot be reached when UF2 firmware is loaded, according to the maintainer blckmn. This code is maintained for backward compatibility with unified target settings and will be refactored in a future PR.
Applied to files:
src/components/tabs/PortsTab.vue
📚 Learning: 2025-08-27T22:07:26.353Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4579
File: src/js/data_storage.js:9-9
Timestamp: 2025-08-27T22:07:26.353Z
Learning: In the betaflight-configurator CalVer refactoring, the generateVirtualApiVersions function in src/js/utils/common.js works correctly despite the apparent mathematical issue with bounds logic where compareVersions.minor() returns different ranges for API_VERSION_MAX_SUPPORTED (25.12.0) vs API_VERSION_ACCEPTED (1.44.0).
Applied to files:
src/components/tabs/PortsTab.vue
🪛 GitHub Check: SonarCloud Code Analysis
src/components/tabs/PortsTab.vue
[failure] 463-463: Expected { after 'if' condition.
[failure] 461-461: Expected { after 'if' condition.
[failure] 462-462: Expected { after 'if' condition.
[failure] 465-465: Expected { after 'if' condition.
[failure] 464-464: Expected { after 'if' condition.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/components/tabs/PortsTab.vue (3)
198-261: Function rules metadata (maxPorts/sharing) is currently unused for disabling options.
functionRulescarries rich metadata (maxPorts,sharableWith,notSharableWith) butgetRules/isRuleDisabledonly usegroupsanddependsOn. That means we’re not enforcing port-count limits or cross-group exclusivity via disabled options (beyond the telemetry/peripheral mutual-exclusion handled in the change handlers), which may diverge from legacyports.jsbehavior.It would be good to either:
- Port the old logic that uses
maxPortsand the sharing constraints to compute disabled states, or- Explicitly confirm that the legacy tab also ignored those fields and that this metadata is intentionally unused.
Example of how this could be wired in without changing the public template API:
- const isRuleDisabled = (rule) => { - return ( - FC.CONFIG.buildOptions.length && - rule.dependsOn !== undefined && - !FC.CONFIG.buildOptions.includes(rule.dependsOn) - ); - }; + const isRuleDisabled = (rule) => { + // Build-option gating + if (FC.CONFIG.buildOptions.length && rule.dependsOn && !FC.CONFIG.buildOptions.includes(rule.dependsOn)) { + return true; + } + + // Optional: enforce maxPorts / sharing based on current ports state + const allFunctions = FC.SERIAL_CONFIG?.ports?.flatMap((p) => p.functions || []) ?? []; + const usedCount = allFunctions.filter((f) => f === rule.name).length; + if (rule.maxPorts && usedCount >= rule.maxPorts) { + return true; + } + + return false; + };(You’d likely want a more faithful port of the legacy logic, including
sharableWith/notSharableWith, but this illustrates the idea.)If you’d like, I can sketch a closer port of the original
functionRulesavailability logic fromports.jsinto a composable helper to keepsetup()cleaner.Also applies to: 323-335
364-381: Consider handlingMSP_VTX_CONFIGfailures so ports still load.
loadConfigalways waits onMSP.promise(MSPCodes.MSP_VTX_CONFIG)before callingmspHelper.loadSerialConfig. IfMSP_VTX_CONFIGisn’t supported or the promise rejects,handleSerialConfigLoadedis never reached, soportsnever populate andGUI.content_ready()is never called, leaving the tab in a “loading” state.To keep behavior robust on targets without VTX table support (or when the request fails), you could fall back to loading serial config even if
MSP_VTX_CONFIGfails:- const loadConfig = () => { - MSP.promise(MSPCodes.MSP_VTX_CONFIG).then(() => { - mspHelper.loadSerialConfig(handleSerialConfigLoaded); - }); - }; + const loadConfig = () => { + MSP.promise(MSPCodes.MSP_VTX_CONFIG) + .catch(() => { + // Ignore VTX config failure; ports tab should still work. + }) + .finally(() => { + mspHelper.loadSerialConfig(handleSerialConfigLoaded); + }); + };This keeps the MSP callback usage simple and avoids the infinite-loading style issues we’ve seen when MSP flows get too clever. Based on learnings, MSP callback handling is sensitive, so keeping this pattern straightforward is safer.
336-343: Verify SERIAL_CONFIG shape and AUTO baudrate mapping against legacy behavior.The transform/save path currently:
- Normalizes GPS/Blackbox baudrates to
"AUTO"in component state and then maps"AUTO"→"57600"(GPS) and"115200"(Blackbox) on save.- Leaves telemetry
"AUTO"as"AUTO"inFC.SERIAL_CONFIG.ports.- Rebuilds each port with only
{ identifier, msp_baudrate, telemetry_baudrate, gps_baudrate, blackbox_baudrate, functions }.Two things worth double‑checking against the old
ports.jsand firmware expectations:
AUTO representation:
- Ensure that GPS/Blackbox AUTO really need to be serialized as
"57600"/"115200"rather than a sentinel value (e.g.,0) and that telemetry AUTO should remain"AUTO"inFC.SERIAL_CONFIG.ports. Any mismatch here could cause unexpected baud changes on save.Port object shape:
- If
FC.SERIAL_CONFIG.portscontains additional fields (e.g., options/flags/inversion) today, rebuilding the array with a narrowed field set will drop them. In that case, we should carry them through from the original objects rather than omitting them.For example, if ports include an
optionsfield, the reconstruction might need to look like:- FC.SERIAL_CONFIG.ports = ports.map((p) => { + FC.SERIAL_CONFIG.ports = ports.map((p, idx) => { + const original = FC.SERIAL_CONFIG.ports[idx] || {}; const functions = []; // ...existing function pushes... return { - identifier: p.identifier, - msp_baudrate: p.msp_baudrate, - telemetry_baudrate: p.telemetry_baudrate, - gps_baudrate: p.gps_baudrate === "AUTO" ? "57600" : p.gps_baudrate, - blackbox_baudrate: p.blackbox_baudrate === "AUTO" ? "115200" : p.blackbox_baudrate, - functions: functions, + ...original, + identifier: p.identifier, + msp_baudrate: p.msp_baudrate, + telemetry_baudrate: p.telemetry_baudrate, + gps_baudrate: p.gps_baudrate === "AUTO" ? "57600" : p.gps_baudrate, + blackbox_baudrate: p.blackbox_baudrate === "AUTO" ? "115200" : p.blackbox_baudrate, + functions, }; });If you can confirm the current
FC.SERIAL_CONFIG.portsstructure, I can help adjusttransformPortData/saveConfigto be fully schema‑preserving while keeping the AUTO mapping intact.Also applies to: 399-428, 450-497
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/tabs/PortsTab.vue(1 hunks)
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
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: 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.
📚 Learning: 2025-06-20T12:35:49.283Z
Learnt from: mituritsyn
Repo: betaflight/betaflight-configurator PR: 4526
File: src/js/gui.js:43-43
Timestamp: 2025-06-20T12:35:49.283Z
Learning: In the Betaflight Configurator codebase, tabs in `defaultCloudBuildTabOptions` are conditionally displayed based on firmware build options. The logic in `serial_backend.js` checks `FC.CONFIG.buildOptions` and only adds tabs to `GUI.allowedTabs` if the firmware was built with support for that feature.
Applied to files:
src/components/tabs/PortsTab.vue
📚 Learning: 2025-09-19T20:42:20.332Z
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.
Applied to files:
src/components/tabs/PortsTab.vue
📚 Learning: 2025-09-19T20:41:44.286Z
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.
Applied to files:
src/components/tabs/PortsTab.vue
📚 Learning: 2025-06-27T22:06:49.210Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4532
File: src/js/VirtualFC.js:234-234
Timestamp: 2025-06-27T22:06:49.210Z
Learning: In the betaflight-configurator codebase, the VirtualFC.js AUX_CONFIG array must stay synchronized with the betaflight firmware's msp_box.c definitions to ensure proper auxiliary mode functionality. Changes to mode names should follow the firmware source code even if it potentially breaks backward compatibility with saved presets.
Applied to files:
src/components/tabs/PortsTab.vue
📚 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/components/tabs/PortsTab.vue
📚 Learning: 2025-09-02T07:45:48.606Z
Learnt from: blckmn
Repo: betaflight/betaflight-configurator PR: 4583
File: src/js/tabs/firmware_flasher.js:949-961
Timestamp: 2025-09-02T07:45:48.606Z
Learning: In src/js/tabs/firmware_flasher.js, the config file loading code path after firmware loading (in the load_file click handler) cannot be reached when UF2 firmware is loaded, according to the maintainer blckmn. This code is maintained for backward compatibility with unified target settings and will be refactored in a future PR.
Applied to files:
src/components/tabs/PortsTab.vue
📚 Learning: 2025-08-27T22:07:26.353Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4579
File: src/js/data_storage.js:9-9
Timestamp: 2025-08-27T22:07:26.353Z
Learning: In the betaflight-configurator CalVer refactoring, the generateVirtualApiVersions function in src/js/utils/common.js works correctly despite the apparent mathematical issue with bounds logic where compareVersions.minor() returns different ranges for API_VERSION_MAX_SUPPORTED (25.12.0) vs API_VERSION_ACCEPTED (1.44.0).
Applied to files:
src/components/tabs/PortsTab.vue
📚 Learning: 2025-11-28T22:51:11.691Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-28T22:51:11.691Z
Learning: In betaflight-configurator, Web Bluetooth API's `services` option filters scan results to only devices advertising those UUIDs, while `optionalServices` requests permission to access services after connection without filtering. PWA (WebBluetooth.js) uses `{ acceptAllDevices: true, optionalServices: uuids }` to show all devices while requesting post-connection service access. CapacitorBluetooth.js should match this pattern rather than using `services` as a scan filter.
Applied to files:
src/components/tabs/PortsTab.vue
- Create PortsTab.vue component - Register PortsTab in Vue components - Update main.js to use Vue mounter for 'ports' tab - Delete legacy ports.js and ports.html
…sually hidden labels for accessibility.
Erics1337
force-pushed
the
vue-tab-ports
branch
from
23f929b to
dcc5237
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/components/tabs/PortsTab.vue (1)
340-357: Consider guardingMSP_VTX_CONFIGso serial config still loads when VTX tables are unsupported
loadConfigcurrently does:const loadConfig = () => { MSP.promise(MSPCodes.MSP_VTX_CONFIG).then(() => { mspHelper.loadSerialConfig(handleSerialConfigLoaded); }); };If the FC firmware does not implement
MSP_VTX_CONFIG, this promise will reject andloadSerialConfig(and thushandleSerialConfigLoaded/GUI.content_ready()) will never run, leaving the Ports table empty for those targets.If we still support any API versions without
MSP_VTX_CONFIG, please consider a.catch/.finallypattern so serial config always loads, e.g.:- const loadConfig = () => { - MSP.promise(MSPCodes.MSP_VTX_CONFIG).then(() => { - mspHelper.loadSerialConfig(handleSerialConfigLoaded); - }); - }; + const loadConfig = () => { + MSP.promise(MSPCodes.MSP_VTX_CONFIG) + .catch(() => { + // Older firmware or VTX tables not supported – ignore and proceed. + }) + .finally(() => { + mspHelper.loadSerialConfig(handleSerialConfigLoaded); + }); + };If configurator’s supported API range guarantees
MSP_VTX_CONFIGis always present, this is less critical but still avoids unhandled rejections.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/components/tabs/PortsTab.vue(1 hunks)src/js/main.js(1 hunks)src/js/protocols/CapacitorSerial.js(5 hunks)src/js/tabs/ports.js(0 hunks)src/js/vue_components.js(2 hunks)src/js/vue_tab_mounter.js(1 hunks)src/tabs/ports.html(0 hunks)
💤 Files with no reviewable changes (2)
- src/js/tabs/ports.js
- src/tabs/ports.html
🚧 Files skipped from review as they are similar to previous changes (1)
- src/js/main.js
🧰 Additional context used
🧠 Learnings (19)
📓 Common learnings
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.
📚 Learning: 2025-09-10T18:26:10.136Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4601
File: package.json:125-125
Timestamp: 2025-09-10T18:26:10.136Z
Learning: In betaflight-configurator, dependency updates are handled incrementally - Vue dependencies are updated separately from Vite dependencies for better isolation and maintainability.
Applied to files:
src/js/vue_components.jssrc/js/vue_tab_mounter.js
📚 Learning: 2025-06-20T12:35:49.283Z
Learnt from: mituritsyn
Repo: betaflight/betaflight-configurator PR: 4526
File: src/js/gui.js:43-43
Timestamp: 2025-06-20T12:35:49.283Z
Learning: In the Betaflight Configurator codebase, tabs in `defaultCloudBuildTabOptions` are conditionally displayed based on firmware build options. The logic in `serial_backend.js` checks `FC.CONFIG.buildOptions` and only adds tabs to `GUI.allowedTabs` if the firmware was built with support for that feature.
Applied to files:
src/components/tabs/PortsTab.vue
📚 Learning: 2025-09-19T20:42:20.332Z
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.
Applied to files:
src/components/tabs/PortsTab.vuesrc/js/vue_tab_mounter.js
📚 Learning: 2025-09-19T20:41:44.286Z
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.
Applied to files:
src/components/tabs/PortsTab.vuesrc/js/vue_tab_mounter.js
📚 Learning: 2025-06-27T22:06:49.210Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4532
File: src/js/VirtualFC.js:234-234
Timestamp: 2025-06-27T22:06:49.210Z
Learning: In the betaflight-configurator codebase, the VirtualFC.js AUX_CONFIG array must stay synchronized with the betaflight firmware's msp_box.c definitions to ensure proper auxiliary mode functionality. Changes to mode names should follow the firmware source code even if it potentially breaks backward compatibility with saved presets.
Applied to files:
src/components/tabs/PortsTab.vue
📚 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/components/tabs/PortsTab.vue
📚 Learning: 2025-09-02T07:45:48.606Z
Learnt from: blckmn
Repo: betaflight/betaflight-configurator PR: 4583
File: src/js/tabs/firmware_flasher.js:949-961
Timestamp: 2025-09-02T07:45:48.606Z
Learning: In src/js/tabs/firmware_flasher.js, the config file loading code path after firmware loading (in the load_file click handler) cannot be reached when UF2 firmware is loaded, according to the maintainer blckmn. This code is maintained for backward compatibility with unified target settings and will be refactored in a future PR.
Applied to files:
src/components/tabs/PortsTab.vue
📚 Learning: 2025-08-27T22:07:26.353Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4579
File: src/js/data_storage.js:9-9
Timestamp: 2025-08-27T22:07:26.353Z
Learning: In the betaflight-configurator CalVer refactoring, the generateVirtualApiVersions function in src/js/utils/common.js works correctly despite the apparent mathematical issue with bounds logic where compareVersions.minor() returns different ranges for API_VERSION_MAX_SUPPORTED (25.12.0) vs API_VERSION_ACCEPTED (1.44.0).
Applied to files:
src/components/tabs/PortsTab.vue
📚 Learning: 2025-11-28T22:51:11.691Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-28T22:51:11.691Z
Learning: In betaflight-configurator, Web Bluetooth API's `services` option filters scan results to only devices advertising those UUIDs, while `optionalServices` requests permission to access services after connection without filtering. PWA (WebBluetooth.js) uses `{ acceptAllDevices: true, optionalServices: uuids }` to show all devices while requesting post-connection service access. CapacitorBluetooth.js should match this pattern rather than using `services` as a scan filter.
Applied to files:
src/components/tabs/PortsTab.vuesrc/js/protocols/CapacitorSerial.js
📚 Learning: 2025-12-01T17:09:13.412Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-01T17:09:13.412Z
Learning: In betaflight-configurator’s CapacitorBluetooth.js, native plugin listeners (notification, services, connectionState) must be attached exactly once per app lifetime. Re-attaching them during writes/connect cycles leads to duplicate handlers, increased JS workload, MSP queue pressure, and can prevent the MSP_EEPROM_WRITE callback from firing (so reinitializeConnection is not reached). Implement an idempotent _attachNativeListenersOnce() guard.
Applied to files:
src/js/protocols/CapacitorSerial.js
📚 Learning: 2025-11-24T15:07:25.227Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4706
File: src/js/protocols/CapacitorSerial.js:47-47
Timestamp: 2025-11-24T15:07:25.227Z
Learning: In betaflight-configurator's CapacitorSerial protocol (src/js/protocols/CapacitorSerial.js), the fire-and-forget this.loadDevices() call in the constructor is intentional background pre-loading. The actual critical device list loading for UI population is orchestrated by port_handler.js via serial.getDevices("serial"), which properly awaits CapacitorSerial.getDevices() → loadDevices() in a complete async chain. The constructor's async call does not cause race conditions.
Applied to files:
src/js/protocols/CapacitorSerial.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/protocols/CapacitorSerial.js
📚 Learning: 2025-11-28T22:41:59.374Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:135-143
Timestamp: 2025-11-28T22:41:59.374Z
Learning: In betaflight-configurator, WebSerial.js uses a constant path "serial" for all devices, and CapacitorBluetooth.js uses constant path "bluetooth", consistent with the single-device-at-a-time workflow. WebBluetooth.js uses counter-based paths (bluetooth_${counter}), but that's specific to its implementation. The path strategy varies by protocol based on their specific requirements.
Applied to files:
src/js/protocols/CapacitorSerial.js
📚 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/protocols/CapacitorSerial.js
📚 Learning: 2025-08-22T16:43:20.901Z
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.
Applied to files:
src/js/protocols/CapacitorSerial.js
📚 Learning: 2025-11-22T21:18:08.814Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4706
File: src/js/protocols/CapacitorSerial.js:11-46
Timestamp: 2025-11-22T21:18:08.814Z
Learning: In betaflight-configurator, protocol implementations (WebSerial, CapacitorSerial, WebBluetooth, etc.) are instantiated once in the Serial class constructor and stored as singletons in the _protocols array for the entire application lifetime. They are never destroyed or recreated, so cleanup methods to remove event listeners are not needed.
Applied to files:
src/js/protocols/CapacitorSerial.js
📚 Learning: 2025-12-02T17:46:47.859Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-02T17:46:47.859Z
Learning: BetaflightBluetoothPlugin.java: maintainers prefer write(PluginCall) to be “write-only” — no per-call UUID parsing or characteristic lookups. Resolve and cache the write characteristic once in onServicesDiscovered() (prefer PROPERTY_WRITE_NO_RESPONSE; else PROPERTY_WRITE). write() should read payload params from call, use the cached writeCharacteristic, pick write type, and submitWrite; clear the cache in cleanupGatt().
Applied to files:
src/js/protocols/CapacitorSerial.js
📚 Learning: 2025-08-27T19:36:13.733Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4579
File: src/js/tabs/pid_tuning.js:80-82
Timestamp: 2025-08-27T19:36:13.733Z
Learning: MSP_STATUS_EX calls in serial_backend.js and setup.js are intentionally unguarded because they are used for status fetching on page load to initialize profiles and are safe to call on older firmware versions. Only conditional/optional MSP_STATUS_EX usage (like in pid_tuning.js) needs version gating to API_VERSION_25_12.
Applied to files:
src/js/protocols/CapacitorSerial.js
🧬 Code graph analysis (2)
src/js/vue_components.js (2)
src/components/init.js (1)
app(60-64)src/js/vue_loader.js (1)
app(42-42)
src/js/vue_tab_mounter.js (2)
src/js/main.js (2)
tabName(139-139)tabName(173-173)src/js/gui.js (1)
GUI(473-473)
⏰ 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
🔇 Additional comments (4)
src/js/vue_components.js (1)
10-18: PortsTab Vue integration and registration look consistentThe new
PortsTabimport,VueTabComponents.portsentry, and global"PortsTab"registration align with the existing tab pattern and themountVueTab("ports", ...)usage. No issues from a wiring/registration perspective.Also applies to: 30-34
src/js/vue_tab_mounter.js (1)
62-64: Tab mount lifecycle andbetaflightModelinjection changes look correct
- Switching from
window.vmtoglobalThis.vmstill accesses the same global app instance in browsers while being more robust.- Always scheduling the zero‑delay timeout to clear
GUI.tab_switch_in_progress(and conditionally callcontentReadyCallbackinside it) makes tab switching more reliable when callers omit the callback.This stays within the existing lifecycle model and improves robustness.
Also applies to: 71-77
src/components/tabs/PortsTab.vue (1)
363-404: Ports save path, AUTO baud mapping, and feature auto-enabling look faithful to legacy behaviorThe
saveConfig+getEnabledFeaturesFromPorts+updateFeaturespipeline appears consistent and well-structured:
- Rebuilds
FC.SERIAL_CONFIG.portsfrom the reactiveportsmodel, with clear mapping from booleans / selected rules back to thefunctionsarray.- Maps
"AUTO"GPS and Blackbox baudrates to concrete defaults ("57600"and"115200"respectively) when persisting, while keeping"AUTO"in component state, which matches the described AUTO‑baud behavior.- Derives
rxSerial,telemetry,blackbox,esc, andgpsflags from the persistedfunctionsand updatesFC.FEATURE_CONFIG.featuresaccordingly, preserving the historical behavior of only enabling TELEMETRY without auto‑disabling it when telemetry is no longer present.- Uses
vtxTableNotConfiguredto show the VTX‑table warning only when the MSP‑provided table is marked available but has zero bands/channels/powerlevels.Overall this path looks solid and aligns with the existing configurator feature‑autoconfig semantics.
Also applies to: 426-473, 475-483
src/js/protocols/CapacitorSerial.js (1)
12-32: State initialization must occur before plugin availability check to ensure stable defaultsThe constructor currently performs an early return
if (!BetaflightSerial)before initializing instance properties likethis.ports,this.connected,this.bitrate, etc. If the plugin is unavailable, these properties remainundefinedinstead of safe defaults, which can cause downstream errors when code expects them to exist.Initialize all state unconditionally at the start of the constructor, then check availability afterward:
class CapacitorSerial extends EventTarget { constructor() { super(); - - if (!BetaflightSerial) { - console.error(`${logHead} Native BetaflightSerial plugin is not available`); - return; - } - this.connected = false; this.openRequested = false; this.transmitting = false; this.connectionInfo = null; this.bitrate = 0; this.bytesSent = 0; this.bytesReceived = 0; this.ports = []; this.currentDevice = null; this.connectionId = null; + + if (!BetaflightSerial) { + console.error(`${logHead} Native BetaflightSerial plugin is not available`); + return; + }This ensures the instance is always in a valid state regardless of plugin availability.
haslinghuis
left a comment
haslinghuis
left a comment
There was a problem hiding this comment.
Please remove Capacitor part
This reverts commit 0cceae8.
|
|
Preview URL: https://pr4726.betaflight-app-preview.pages.dev |
nerdCopter
left a comment
nerdCopter
left a comment
There was a problem hiding this comment.
- approving, briefly tested ports tab.
3 participants
Ports Tab Vue Migration
Description
Migrated the
Portstab from legacy jQuery/HTML to a Vue 3 componentPortsTab.vue.This is part of the ongoing Vue migration (Phase 4).
Changes
src/components/tabs/PortsTab.vueimplementing:src/js/tabs/ports.jsandsrc/tabs/ports.html.functionRulesmechanism for determining column content.featureConfigauto-enabling logic (e.g. enabling GPS feature when GPS port is configured).Testing
FC.SERIAL_CONFIGis updated correctly.FC.FEATURE_CONFIGis updated (e.g. enabling GPS).Dependencies
BaseTab.vue(introduced in previous phase).Summary by CodeRabbit
New Features
Refactor
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.