-
-
Notifications
You must be signed in to change notification settings - Fork 155
Fix semver #858
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix semver #858
Conversation
WalkthroughUpdated firmware-version gating thresholds from "4.6.0" to "2025.12.0" across multiple modules; adjusted field/hardware/debug mappings and FFT_FREQ mappings to follow the new threshold; preserved full firmware precision when parsing Betaflight headers; increased several UI parameter upper bounds and moved RC-smoothing UI gating to the new threshold. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Log as Log File
participant Parser as flightlog_parser
participant Defs as flightlog_fielddefs
participant Presenter as flightlog_fields_presenter
participant UI as header_dialog / UI
Log->>Parser: supply header (firmware string)
Parser->>Parser: parse major.minor(.patch) preserving integer parts
Parser->>Parser: set firmware, firmwarePatch, firmwareVersion
UI->>Parser: request firmwareVersion
Parser-->>UI: return firmwareVersion
UI->>Defs: request field/hardware/debug defs with firmwareVersion
alt firmwareVersion >= 2025.12.0
Defs-->>UI: provide updated defs (MULTI_GYRO, ACC_HARDWARE changes, DEBUG_MODE additions)
Presenter-->>UI: use expanded FFT_FREQ mappings
else
Defs-->>UI: provide legacy defs (DUAL_GYRO, older ACC list)
Presenter-->>UI: use legacy FFT_FREQ mappings
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 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)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/flightlog.js (1)
1785-1795: Verify mask remap vs UI for fields_disabled_mask (ATTITUDE/ACC/DEBUG offsets).For BF >= 2025.12.0 you remap/extend bits (e.g., ATTITUDE at bit 8, ACC moves to bit 9, etc.). The header dialog’s “Selected Fields” list (src/header_dialog.js, builtSelectedFieldsList) still enumerates the legacy 0..13 mapping and doesn’t include ATTITUDE/SERVO. When this threshold becomes active, toggles will drift.
- Suggest adding ATTITUDE/SERVO to the UI list and gating the bit positions by firmwareVersion here too.
- Until then, a small comment noting the future mismatch would help.
src/flightlog_fields_presenter.js (1)
1317-1489: Fix typo: SPA debug mapping key missing closing bracket.'debug[all' should be 'debug[all]'. Without the bracket, friendly names for SPA won’t resolve.
- DEBUG_FRIENDLY_FIELD_NAMES.SPA = { - 'debug[all': 'SPA', + DEBUG_FRIENDLY_FIELD_NAMES.SPA = { + 'debug[all]': 'SPA', 'debug[0]': 'Setpoint PID Attenuation [roll]', 'debug[1]': 'Setpoint PID Attenuation [pitch]', 'debug[2]': 'Setpoint PID Attenuation [yaw]', };src/header_dialog.js (2)
1577-1587: RC smoothing indices changed; confirm data layout for rc_smoothing_active_cutoffs_ff_sp_thr.For >= 2025.12.0 you hide FF and map only [SP, THR] from indexes [0],[1]. In 4.3–4.5 you map [FF, SP, THR] as [0],[1],[2]. Please confirm the header format actually changes to a 2‑element tuple in new firmware; otherwise this will mis-assign values.
1713-1718: DOM reordering guarded for < 2025.12.0.Looks harmless. Consider guarding with element existence checks to avoid null derefs if IDs change.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/flightlog.js(1 hunks)src/flightlog_fielddefs.js(3 hunks)src/flightlog_fields_presenter.js(3 hunks)src/flightlog_parser.js(1 hunks)src/header_dialog.js(16 hunks)
🔇 Additional comments (7)
src/flightlog_fields_presenter.js (3)
1317-1489: Nice: expanded debug mappings for >= 2025.12.0.The FFT_FREQ, MULTI_GYRO*, AUTOPILOT_* additions look consistent with fielddefs updates.
1978-1993: LGTM: FFT_FREQ unit switch aligns with new field order.Using deg/s for debug[0] on >= 2025.12.0 and Hz otherwise matches the name remap.
2730-2749: LGTM: ConvertDebugFieldValue mirrors the display logic for FFT_FREQ.The conversion paths are consistent with decodeDebugFieldToFriendly.
src/header_dialog.js (2)
101-111: Bounds bumped to 9999.9.9 across many params: OK; sanity-check semver lib limits.This effectively “un-caps” visibility checks. Confirm your semver implementation handles large majors (e.g., 9999.9.9) without surprises.
Also applies to: 131-141, 143-159, 173-183, 187-195, 197-207, 211-219, 221-225, 247-255, 257-261, 269-279, 281-291, 299-303, 311-321, 329-333, 337-339, 347-351, 353-357, 361-363, 367-375, 377-399, 401-405, 409-417, 425-429, 433-441, 445-447, 451-459, 463-471, 475-483, 487-495, 499-507, 521-525, 529-537, 541-549, 553-561, 565-573, 577-585, 589-597, 601-609, 613-621, 647-651, 655-663, 667-675, 679-687, 691-699, 703-711, 715-723, 727-735, 739-747, 751-759, 763-771, 775-783
329-333: Gate motor_idle on >= 2025.12.0.Looks fine; ensure logs with this header actually set motor_idle so the field isn’t rendered “missing” by default.
src/flightlog_fielddefs.js (2)
532-558: Debug list evolution for >= 2025.12.0 looks consistent.
- D_MAX vs D_MIN swap pre-threshold is sensible.
- ACC_HARDWARE pruning and IIM42653 insertion OK.
- MULTI_GYRO renames align with presenter mappings.
Please verify firmware outputs the renamed debug modes (MULTI_GYRO_, AUTOPILOT_, OPTICALFLOW, TPA, SPA, S_TERM, TASK, GIMBAL, WING_SETPOINT).
564-568: Flight mode names switch to POST_4_5 only for >= 2025.12.0.De-risking by pushing the boundary out is fine. Ensure any UI text elsewhere that relied on the old 4.6.0 split doesn’t assume POST_4_5 until the new threshold.
|
|
Preview URL: https://pr858.betaflight-blackbox.pages.dev |
nerdCopter
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- only briefly tested, did not deep inspect.


Summary by CodeRabbit
New Features
Chores