Enable servo tab for WING define #4635
Conversation
WalkthroughServos tab exposure now depends on build options containing either "USE_SERVOS" or "USE_WING". Serial backend centralizes connect/disconnect cleanup, updates virtual-connection timestamps to use Changes
Sequence Diagram(s)sequenceDiagram
participant UI
participant TabChecker as updateTabList.js
participant Serial as serial_backend.js
participant FC as FC.CONFIG
participant GUI
Note over FC: After config available / on connection
Serial->>FC: read buildOptions
Serial->>Serial: finishOpen()
alt FC.buildOptions includes "USE_WING"
Serial->>GUI: add "servos" to allowedTabs if missing
Note right of GUI #e6ffed: allowed tabs updated
end
UI->>TabChecker: updateTabList()
TabChecker->>FC: check buildOptions
alt includes "USE_SERVOS" or "USE_WING"
TabChecker->>GUI: enable "servos" tab
else
TabChecker->>GUI: leave "servos" disabled
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
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. Tip 🧪 Early access (models): enabledWe are currently testing Sonnet 4.5 code review models, which should lead to better review quality. However, this model may result in higher noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
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: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/js/utils/updateTabList.js(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: mituritsyn
PR: betaflight/betaflight-configurator#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.
📚 Learning: 2025-06-20T12:35:49.283Z
Learnt from: mituritsyn
PR: betaflight/betaflight-configurator#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/js/utils/updateTabList.js
🧬 Code graph analysis (1)
src/js/utils/updateTabList.js (1)
src/js/fc.js (1)
FC(131-990)
haslinghuis
force-pushed
the
fix-wing-servo
branch
from
8a795df to
321aec8
Compare
haslinghuis
force-pushed
the
fix-wing-servo
branch
from
321aec8 to
4f9f6fc
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/js/utils/updateTabList.js (1)
11-13: Show Servos without expert mode: good; add optional chaining on includes to avoid rare NPE.If
buildOptionsis briefly undefined during early UI init, calling.includes(...)can throw. Guard the method call.Apply:
- $("#tabs ul.mode-connected li.tab_servos").toggle( - ["USE_SERVOS", "USE_WING"].some((option) => FC.CONFIG?.buildOptions?.includes(option)), - ); + $("#tabs ul.mode-connected li.tab_servos").toggle( + ["USE_SERVOS", "USE_WING"].some((option) => FC.CONFIG?.buildOptions?.includes?.(option)), + );For consistency, consider applying the same
includes?.(...)guard to the GPS line in this file as well.Based on learnings
src/js/serial_backend.js (1)
620-624: Unify USE_WING predicate and confirm defaultCloudBuildTabOptions includes “servos”
defaultCloudBuildTabOptionsalready defines"servos", so the tab appears whenUSE_SERVOSis present. To align the special‐case check with the flag‐lookup used elsewhere, apply:- if (FC.CONFIG.buildOptions.some((opt) => opt.includes("USE_WING")) && !GUI.allowedTabs.includes("servos")) { + if (["USE_WING"].some((flag) => FC.CONFIG.buildOptions.includes(flag)) && !GUI.allowedTabs.includes("servos")) { GUI.allowedTabs.push("servos"); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/js/serial_backend.js(1 hunks)src/js/utils/updateTabList.js(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: mituritsyn
PR: betaflight/betaflight-configurator#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.
📚 Learning: 2025-06-20T12:35:49.283Z
Learnt from: mituritsyn
PR: betaflight/betaflight-configurator#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/js/serial_backend.jssrc/js/utils/updateTabList.js
📚 Learning: 2025-09-19T20:42:20.332Z
Learnt from: haslinghuis
PR: betaflight/betaflight-configurator#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/js/utils/updateTabList.js
🧬 Code graph analysis (2)
src/js/serial_backend.js (2)
src/js/fc.js (1)
FC(131-990)src/js/gui.js (1)
GUI(465-465)
src/js/utils/updateTabList.js (1)
src/js/fc.js (1)
FC(131-990)
|
Ignore sonar cloud? |
|
|
Preview URL: https://pr4635.betaflight-app.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/js/serial_backend.js (1)
673-673: Consider modernizing the adjacent jQuery array check for consistency.Line 673 correctly uses the native
includesmethod, but line 667 still uses the older jQuery$.inArray. For consistency, consider updating line 667 as well:- if ($.inArray(tabName, classes) >= 0) { + if (classes.includes(tabName)) { found = true; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/js/serial_backend.js(8 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: mituritsyn
PR: betaflight/betaflight-configurator#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.
📚 Learning: 2025-06-20T12:35:49.283Z
Learnt from: mituritsyn
PR: betaflight/betaflight-configurator#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/js/serial_backend.js
🧬 Code graph analysis (1)
src/js/serial_backend.js (6)
src/js/gui.js (1)
GUI(465-465)src/js/msp/MSPHelper.js (1)
mspHelper(2937-2937)src/js/utils/AutoDetect.js (1)
mspHelper(19-19)src/js/bit.js (1)
bit_check(1-3)src/js/fc.js (1)
FC(131-990)src/js/localization.js (1)
i18n(7-7)
🔇 Additional comments (4)
src/js/serial_backend.js (4)
113-124: LGTM! Disconnection sequence centralized correctly.The refactored disconnection logic properly centralizes cleanup (timeouts, intervals, tab switching) and ensures
finishCloseis called with the appropriate callback. The use of optional chaining onmspHelperis a good defensive practice.
398-400: LGTM! ModernglobalThis.vmusage.The migration from
window.vmtoglobalThis.vmis appropriate and improves cross-environment compatibility. The consistent use of optional chaining and deferred execution across all three locations ensures Vue components are properly mounted before timestamp updates.Also applies to: 567-569, 712-714
463-469: LGTM! Good refactoring to extract helper function.The extraction of
checkReportProblemas a top-level helper reduces code duplication and improves maintainability. The function has a clear single responsibility and is used consistently incheckReportProblems.
512-514: LGTM! Modern iteration syntax.The change from
forEachtofor...ofis a minor modernization that improves readability without affecting functionality.
Summary by CodeRabbit
Bug Fixes
Improvements