Fix gyro enable box #4611
Fix gyro enable box #4611
Conversation
WalkthroughIntroduces a new “Gyro Enable” UI for firmware 1.47+, relocates per-gyro controls to a dedicated container, adjusts detection/visibility logic, rebuilds gyro_enable_mask from new checkboxes on save, adds a new i18n help string, and marks the legacy gyro alignment block as deprecated without removing it. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant UI as Config Tab (UI)
participant FW as Firmware Detector
participant FC as FC State
rect rgb(245,245,250)
U->>UI: Open Configuration tab
UI->>FW: Get firmware version
alt FW >= 1.47 (new UI)
UI->>FC: Read detected gyros + flags
UI->>UI: Render gyro_enable_box with per-gyro rows
note right of UI: Show .gyro_notfound if none<br/>Hide legacy gyro_box
else FW < 1.47 (legacy)
UI->>UI: Show legacy gyro alignment UI
note right of UI: Hide gyro_enable_box
end
end
rect rgb(240,255,245)
U->>UI: Click Save
alt FW >= 1.47
UI->>UI: Build gyro_enable_mask from #gyro_X_enable
UI->>FC: Update SENSOR_ALIGNMENT.gyro_enable_mask
else FW < 1.47
UI->>FC: Save legacy gyro state
end
UI-->>U: Save complete
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 Pre-merge checks❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
|
|
|
Preview URL: https://8518b6c5.betaflight-configurator.pages.dev |
![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
🧹 Nitpick comments (2)
locales/en/messages.json (1)
1504-1506: Tighten wording; align terminology with “Active IMU”; fix comma splice and spelling.Use IMU consistently (gyro + accelerometer), avoid the comma splice, and match British spelling commonly used elsewhere in this file.
- "configurationGyroActiveIMUHelp": { - "message": "Select which gyro to use as the primary gyro. The primary gyro is used for stabilization and control. If you have only one gyro, it will be selected automatically.<br><br>It is not always possible to enable multiple gyros, when they are of different types or brands." - }, + "configurationGyroActiveIMUHelp": { + "message": "Select the Active IMU (gyro + accelerometer). The primary IMU is used for stabilisation and control. If only one IMU is present, it is selected automatically.<br><br>Enabling multiple gyros may not be possible, especially when they are of different types or brands." + },src/tabs/configuration.html (1)
189-189: Consider internationalizing the deprecation labelThe "DEPRECATED" text is hardcoded in the comment. While this is just a comment and not user-facing, it would be more consistent to follow the pattern used elsewhere in the codebase.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
locales/en/messages.json(1 hunks)src/js/tabs/configuration.js(2 hunks)src/tabs/configuration.html(2 hunks)
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-08-11T19:10:56.992Z
Learnt from: haslinghuis
PR: betaflight/betaflight-configurator#4567
File: locales/en/messages.json:7724-7728
Timestamp: 2025-08-11T19:10:56.992Z
Learning: In the Betaflight Configurator project, only English language entries are added directly to locales/en/messages.json. Translations for other languages are handled through an outsourced process and should not be suggested as part of code reviews.
Applied to files:
locales/en/messages.json
📚 Learning: 2025-05-26T11:37:09.758Z
Learnt from: haslinghuis
PR: betaflight/betaflight-configurator#4490
File: locales/en/messages.json:1534-1545
Timestamp: 2025-05-26T11:37:09.758Z
Learning: In the Betaflight Configurator repository, only English messages should be added directly to `locales/en/messages.json`; other language translations are handled via Crowdin automation and should not be manually updated in PRs.
Applied to files:
locales/en/messages.json
📚 Learning: 2025-08-22T22:43:45.415Z
Learnt from: haslinghuis
PR: betaflight/betaflight-configurator#4577
File: src/js/tabs/configuration.js:580-590
Timestamp: 2025-08-22T22:43:45.415Z
Learning: In src/js/tabs/configuration.js, hiding the entire .mag_align_box for API versions < 1.47 is intentional behavior because these MAG alignment options were not present or have changed significantly between API versions. The code has been reorganized for better maintenance.
Applied to files:
src/tabs/configuration.htmlsrc/js/tabs/configuration.js
📚 Learning: 2025-08-13T15:13:10.214Z
Learnt from: haslinghuis
PR: betaflight/betaflight-configurator#4569
File: src/js/tabs/receiver.js:957-960
Timestamp: 2025-08-13T15:13:10.214Z
Learning: In src/js/tabs/receiver.js, the updateInterpolationView() function already contains logic to hide ".rcSmoothing-feedforward-cutoff" when FC.RX_CONFIG.rcSmoothingFeedforwardCutoff === 0, which handles the API ≥ 1.47 case since feedforward cutoff isn't initialized from DOM for newer APIs.
Applied to files:
src/js/tabs/configuration.js
📚 Learning: 2025-05-24T15:12:50.241Z
Learnt from: haslinghuis
PR: betaflight/betaflight-configurator#4485
File: src/js/sensor_types.js:96-99
Timestamp: 2025-05-24T15:12:50.241Z
Learning: In src/js/sensor_types.js, the sonar element selection UI is only displayed when firmware version >= API_VERSION_1_47, so sonar sensor types can be added directly to the static elements array without requiring additional version guards.
Applied to files:
src/js/tabs/configuration.js
📚 Learning: 2025-06-27T22:06:49.210Z
Learnt from: haslinghuis
PR: betaflight/betaflight-configurator#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/js/tabs/configuration.js
🔇 Additional comments (9)
src/tabs/configuration.html (1)
235-251: LGTM! Well-structured new gyro enable UI blockThe new gyro enable section follows the established UI patterns:
- Properly wrapped in a grey GUI box with title bar
- Uses i18n keys for localization
- Includes help icon with tooltip
- Has proper error message display area
src/js/tabs/configuration.js (8)
243-244: Good approach hiding deprecated UI for newer firmwareProperly hides the entire deprecated gyro alignment box parent for firmware 1.47+, maintaining backward compatibility while preventing UI confusion.
258-263: Good safety check for gyro_enable_mask initializationThe code properly handles edge cases:
- Initializes undefined masks to all enabled
- Prevents all gyros from being disabled (safety measure)
This ensures at least one gyro remains active for flight control.
328-328: Correct container targeting for gyro enable configurationThe change properly targets the new
.gyro_enable_configurationcontainer introduced in the HTML template, ensuring gyro boxes are rendered in the correct location.
333-333: Fixed notfound element selectorThe change correctly updates from
.gyro_alignment_inputs_notfoundto.gyro_notfound, matching the new HTML structure.
336-336: Proper hiding of gyro enable box when not neededCorrectly hides the parent of
.gyro_enable_boxwhen there's only one gyro or no gyros detected, preventing unnecessary UI clutter.
339-340: Good backward compatibility handlingProperly hides the new gyro enable box for firmware versions prior to 1.47, ensuring older firmware versions continue to use the legacy UI.
302-314: Excellent safety implementation preventing all gyros from being disabledThe code prevents users from disabling all gyros, which would make the drone unflyable. It:
- Checks if disabling would result in zero enabled gyros
- Keeps the checkbox checked if that would happen
- Shows a warning message to the user
This is a critical safety feature.
604-614: gyro_enable_mask construction verified — IDs match and save logic correct.createGyroBox uses id="gyro_${gyroIndex + 1}enable" which matches the save selector (#gyro${i + 1}_enable). The save routine initializes FC.SENSOR_ALIGNMENT.gyro_enable_mask = 0 and ORs 1 << i for each checked box; MAX_GYROS = 8. There is an earlier default assignment (FC.SENSOR_ALIGNMENT.gyro_enable_mask = (1 << MAX_GYROS) - 1) but it is overwritten by the save logic.
Summary by CodeRabbit