Skip to content

Conversation

@haslinghuis
Copy link
Member

@haslinghuis haslinghuis commented Sep 26, 2025

Summary by CodeRabbit

  • New Features

    • Added support for additional magnetometer hardware on newer firmware: LIS2MDL, IST8310, and QMC5883P (where applicable).
  • Improvements

    • Hardware option lists now adapt dynamically to firmware version so correct magnetometer/accelerometer choices appear automatically.
    • Beta firmware users receive pre-initialized hardware lists by default.
    • Labels and lists streamlined for consistency across hardware selections.

@haslinghuis haslinghuis added this to the 2025.12.0 milestone Sep 26, 2025
@haslinghuis haslinghuis self-assigned this Sep 26, 2025
@coderabbitai
Copy link

coderabbitai bot commented Sep 26, 2025

Walkthrough

Makes ACC_HARDWARE_COMPLETE a local constant and introduces a local MAG_HARDWARE_COMPLETE while changing MAG_HARDWARE to an exported mutable array. In beta firmware path both ACC and MAG arrays are initialized from their *_COMPLETE copies, then MAG_HARDWARE is conditionally augmented by firmware version and both arrays are wrapped read-only at the end.

Changes

Cohort / File(s) Summary
Flightlog field defs
src/flightlog_fielddefs.js
- Change export const ACC_HARDWARE_COMPLETEconst ACC_HARDWARE_COMPLETE (no longer exported)
- Add const MAG_HARDWARE_COMPLETE = makeReadOnly([...]) (local, non-exported)
- Change export const MAG_HARDWARE = makeReadOnly([...])export let MAG_HARDWARE = [] and initialize from MAG_HARDWARE_COMPLETE.slice(0) in beta path
- Initialize ACC_HARDWARE from ACC_HARDWARE_COMPLETE.slice(0) in beta path
- Add firmware-based MAG augmentations: for >=4.5.0 insert LIS2MDL before LIS3MDL and push IST8310; for >=2025.12.0 push QMC5883P
- Include MAG_HARDWARE in final makeReadOnly(...) wrapping alongside ACC_HARDWARE and other read-only exports
- Comment changed from “ACC hardware names” to “Hardware names” in beta branch

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant App
  participant adjustFieldDefsList
  participant Firmware as FirmwareVersion
  participant Arrays as "ACC/MAG arrays"

  App->>adjustFieldDefsList: call with firmware context
  adjustFieldDefsList->>Firmware: check beta flag and version thresholds
  alt Beta firmware
    adjustFieldDefsList->>Arrays: ACC_HARDWARE = ACC_HARDWARE_COMPLETE.slice(0)
    adjustFieldDefsList->>Arrays: MAG_HARDWARE = MAG_HARDWARE_COMPLETE.slice(0)
  end
  alt Version >= 4.5.0
    adjustFieldDefsList->>Arrays: MAG_HARDWARE.splice(indexOf("LIS3MDL"),0,"LIS2MDL")
    adjustFieldDefsList->>Arrays: MAG_HARDWARE.push("IST8310")
  end
  alt Version >= 2025.12.0
    adjustFieldDefsList->>Arrays: MAG_HARDWARE.push("QMC5883P")
  end
  adjustFieldDefsList->>Arrays: wrap ACC_HARDWARE and MAG_HARDWARE with makeReadOnly(...)
  adjustFieldDefsList-->>App: return updated field defs
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The pull request description is completely empty and retains none of the required sections or context from the repository’s template, providing no summary, rationale, or testing information for the changes. Please remove the boilerplate template text and replace it with a filled-in description that summarizes the MAG_HARDWARE changes, explains the purpose and impact, outlines any testing performed, and references relevant issues as per the repository guidelines.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The title “Update MAG_HARDWARE” is concise, clearly indicates the primary change of making MAG_HARDWARE mutable and augmenting its contents, and aligns with the main focus of the changeset.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 45ae805 and bc9c74d.

📒 Files selected for processing (1)
  • src/flightlog_fielddefs.js (6 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-24T23:23:01.244Z
Learnt from: haslinghuis
PR: betaflight/blackbox-log-viewer#859
File: src/flightlog_fields_presenter.js:1315-1325
Timestamp: 2025-09-24T23:23:01.244Z
Learning: In the blackbox-log-viewer codebase, firmware version pre-release identifiers (like "-rc1", "-rc2") are stripped from the `firmwareVersion` property, so Betaflight 4.5.0-RC1 would be exposed as just "4.5.0" for version comparison purposes.

Applied to files:

  • src/flightlog_fielddefs.js

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.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

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

536-539: Guard against missing LIS3MDL index

If LIS3MDL is ever absent, splice(-1, …) inserts at the wrong position. Add a guard and fallback to push.

Apply:

-      MAG_HARDWARE.splice(MAG_HARDWARE.indexOf("LIS3MDL"), 0, "LIS2MDL");
+      {
+        const i = MAG_HARDWARE.indexOf("LIS3MDL");
+        if (i !== -1) {
+          MAG_HARDWARE.splice(i, 0, "LIS2MDL");
+        } else {
+          MAG_HARDWARE.push("LIS2MDL");
+        }
+      }
       MAG_HARDWARE.push("IST8310");
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 334ae08 and 45ae805.

📒 Files selected for processing (1)
  • src/flightlog_fielddefs.js (6 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-24T23:23:01.244Z
Learnt from: haslinghuis
PR: betaflight/blackbox-log-viewer#859
File: src/flightlog_fields_presenter.js:1315-1325
Timestamp: 2025-09-24T23:23:01.244Z
Learning: In the blackbox-log-viewer codebase, firmware version pre-release identifiers (like "-rc1", "-rc2") are stripped from the `firmwareVersion` property, so Betaflight 4.5.0-RC1 would be exposed as just "4.5.0" for version comparison purposes.

Applied to files:

  • src/flightlog_fielddefs.js
🔇 Additional comments (5)
src/flightlog_fielddefs.js (5)

420-432: Public MAG lists: good API shape

Exporting MAG_HARDWARE as mutable (then frozen per firmware) and adding MAG_HARDWARE_COMPLETE aligns with the ACC pattern and enables version-specific tailoring.


510-515: Init symmetry for MAG_HARDWARE

Initializing MAG_HARDWARE alongside ACC_HARDWARE for BF >= 3.3.0 is correct. Comment rename is fine.


571-572: Freezing MAG_HARDWARE post-adjustment is correct

Prevents accidental mutation after construction, while keeping the binding reassignable for subsequent adjust calls.


382-406: ACC_HARDWARE_COMPLETE isn’t referenced outside this file; no export is needed.


550-551: Verify firmware support for QMC5883P
No other UI strings or parser references were found—only this fielddef. Confirm that firmware’s MAG_HARDWARE enum (e.g., in flight_log.h) actually includes QMC5883P by version 2025.12.0.

coderabbitai[bot]
coderabbitai bot previously approved these changes Sep 26, 2025
nerdCopter
nerdCopter previously approved these changes Sep 26, 2025
@sonarqubecloud
Copy link

@github-actions
Copy link

Preview URL: https://pr861.betaflight-blackbox.pages.dev

@haslinghuis haslinghuis merged commit c7224a5 into betaflight:master Sep 29, 2025
5 checks passed
@haslinghuis haslinghuis deleted the add-qmc5883p branch September 29, 2025 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants