Skip to content

compass: add MAG debug mode for raw magnetometer logging - #11924

Open
Raffi1202 wants to merge 2 commits into
iNavFlight:maintenance-10.xfrom
Raffi1202:fix/blackbox-mag-raw
Open

Raffi1202 wants to merge 2 commits into
iNavFlight:maintenance-10.xfrom
Raffi1202:fix/blackbox-mag-raw

Conversation

@Raffi1202

@Raffi1202 Raffi1202 commented Sep 10, 2026

Copy link
Copy Markdown

Problem

Fixes #10758. @and-sh asked for the raw magnetometer data to be saved to the blackbox so the compass can be calibrated with an external tool; @AlexC176 confirmed the need for out-of-application calibration. The magADC fields in the log are written after the magZero/magGain correction and after sensor and board alignment, so on a board with a stored calibration the original sample cannot be recovered from them.

Cause

src/main/sensors/compass.c:493 applies the magZero/magGain correction and compass.c:514-515 apply sensor and board alignment to mag.magADC; src/main/blackbox/blackbox.c:1772 logs that corrected value. mag.dev.magADCRaw is never published, and debugType_e has no compass entry (src/main/build/debug.h:85). Separately, src/main/drivers/compass/compass_mlx90393.c:108 ignores the busReadBuf result and returns true (line 114), so a failed bus read would yield zeros.

Change

Adds DEBUG_MAG at the end of debugType_e, to debugModeNames in cli.c and to debug_modes in settings.yaml, so existing debug numbers do not shift. compassUpdate() writes mag.dev.magADCRaw[X..Z] into debug[0..2] right after the driver read, before calibration and alignment, as DEBUG_ACC does with acc.dev.ADCRaw (acceleration.c:582). mlx90393Read() now returns false when busReadBuf fails, so the existing failure branch in compassUpdate() skips the sample instead of logging zeros (raised by the Qodo review on this PR). Enable with set debug_mode = MAG; the values appear in blackbox debug[0..2] and in the CLI debug command.

Test

Not run on hardware or SITL. Cause verified by reading compass.c:493/514 and blackbox.c:1772 on maintenance-10.x. Fork CI ran only the Settings.md consistency check, on f0dca86: https://github.com/Raffi1202/inav/actions/runs/34532494452 (success). No firmware build has run; the upstream "Build firmware" runs for both commits are waiting for approval (https://github.com/iNavFlight/inav/actions/runs/34619099396).

Flash / RAM

Not measured yet. The upstream firmware CI has not been released for this PR, so no size report exists.

Docs

docs/Blackbox.md lists MAG among the debug modes; docs/Settings.md regenerated from settings.yaml (checked by the docs workflow run above).

Raw magnetometer samples were not obtainable from a blackbox log. The
existing magADC fields are written after the magZero/magGain correction
and after sensor and board alignment have been applied, so they cannot
be used to solve for hard and soft iron offsets externally. On a board
that already has a stored calibration the original sample cannot be
recovered from them at all.

Publish mag.dev.magADCRaw into debug[0..2] straight after the sensor
read, before calibration and before alignment, mirroring how DEBUG_ACC
already exposes acc.dev.ADCRaw in acceleration.c.

Enable with:

    set debug_mode = MAG

The three axes then appear as debug[0] (X), debug[1] (Y) and debug[2]
(Z) in the blackbox log and in the CLI "debug" command.

This reuses the existing debug channel, so the log format is unchanged.
debug_mode defaults to NONE and the debug columns are only emitted when
debug_mode is not NONE, so logs from users who do not opt in are
unaffected. DEBUG_MAG is appended to the end of debugType_e so the
numbering of the existing debug modes is preserved.

Closes iNavFlight#10758
@Raffi1202
Raffi1202 marked this pull request as ready for review September 11, 2026 15:39
@qodo-code-review

Copy link
Copy Markdown
Contributor

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Add MAG debug mode for raw magnetometer logging

✨ Enhancement 📝 Documentation ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• Exposes uncalibrated, unaligned magnetometer axes through the existing debug channels.
• Adds MAG to CLI and generated debug-mode settings without renumbering existing modes.
• Documents raw magnetometer capture for external hard- and soft-iron calibration.
Diagram

graph TD
  A["Mag Sensor"] --> B["Raw XYZ"] --> C{"MAG enabled?"} -->|Yes| D["Debug array"] --> E["Blackbox log"]
  D --> F["CLI debug"]
  B --> G["Calibration"] --> H["Alignment"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Add dedicated blackbox fields
  • ➕ Provides explicit magnetometer field names and semantics.
  • ➕ Avoids requiring users to reserve the shared debug mode.
  • ➖ Changes the blackbox schema and associated tooling.
  • ➖ Adds logging and flash overhead outside an opt-in diagnostic workflow.
  • ➖ Requires broader compatibility and migration review.

Recommendation: Keep the PR's existing debug-channel approach. It matches the established raw accelerometer pattern, preserves the blackbox format and existing debug-mode numbering, and imposes no cost unless MAG debugging is selected. Dedicated fields would only be preferable if raw magnetometer capture became a permanent, broadly consumed logging requirement.

Files changed (6) +7 / -2

Enhancement (3) +4 / -1
debug.hDefine the MAG debug mode identifier +1/-0

Define the MAG debug mode identifier

• Appends DEBUG_MAG immediately before DEBUG_COUNT, preserving every existing debug mode's numeric value.

src/main/build/debug.h

cli.cExpose MAG in CLI debug mode names +2/-1

Expose MAG in CLI debug mode names

• Adds MAG to the debug mode name array so users can select and inspect the mode through the CLI.

src/main/fc/cli.c

compass.cPublish raw magnetometer axes to debug channels +1/-0

Publish raw magnetometer axes to debug channels

• Writes magADCRaw X, Y, and Z into debug channels 0–2 immediately after a successful sensor read. Capture occurs before zero/gain calibration and sensor or board alignment.

src/main/sensors/compass.c

Documentation (2) +2 / -0
Blackbox.mdDocument MAG blackbox debug logging +1/-0

Document MAG blackbox debug logging

• Adds MAG to the available blackbox debug modes and explains that it records raw, uncalibrated magnetometer samples for external calibration.

docs/Blackbox.md

Settings.mdList MAG as a supported debug mode +1/-0

List MAG as a supported debug mode

• Adds MAG to the generated debug_mode setting value reference.

docs/Settings.md

Other (1) +1 / -1
settings.yamlRegister MAG in the debug mode settings table +1/-1

Register MAG in the debug mode settings table

• Extends the debug_modes settings lookup with MAG, enabling generated configuration handling for the new mode.

src/main/fc/settings.yaml

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Sep 11, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Bus faults skew external compass fits ✓ Resolved 🐞 Bug ☼ Reliability
Description
compassUpdate records magADCRaw whenever the compass driver returns true, but mlx90393Read
ignores the bus-read result, parses its zero-initialized buffer, and unconditionally reports
success. When an MLX90393 transaction fails, three artificial zero coordinates reach both the debug
channel and blackbox log as valid samples, contaminating any external ellipsoid fit that consumes
them.
Code

src/main/sensors/compass.c[426]

+        DEBUG_SET(DEBUG_MAG, axis, mag.dev.magADCRaw[axis]);
Evidence
The new logging line publishes every sample accepted by compassUpdate. The MLX90393 implementation
reads into a zero-initialized buffer without checking whether the transaction succeeded and always
returns true, while blackbox copies the resulting debug values directly into its records.

src/main/sensors/compass.c[417-426]
src/main/drivers/compass/compass_mlx90393.c[103-114]
src/main/blackbox/blackbox.c[1821-1823]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new raw magnetometer logger trusts each driver's success result, but `mlx90393Read` reports success after a failed bus transaction and supplies three zero values. Those values are consequently recorded as real measurements and can skew external calibration.
## Fix Focus Areas
- src/main/drivers/compass/compass_mlx90393.c[103-114]
- src/main/sensors/compass.c[417-426]
## Recommended Fix
Check the return value from `busReadBuf` in `mlx90393Read` and return false without replacing `magADCRaw` when the transaction fails. This allows the existing failure branch in `compassUpdate` to skip publishing the invalid sample.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can switch off images and animations for a plain-text comment

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/main/sensors/compass.c
@github-actions

Copy link
Copy Markdown

RAM / Flash usage vs. base commit 76ee415 — commit 99f7a68

Target Flash Δ RAM Δ
MATEKF405 +8 B (+0.00%) ±0 B (±0.00%)
MATEKF722 -8 B (-0.00%) ±0 B (±0.00%)
MATEKF765 +24 B (+0.00%) ±0 B (±0.00%)
MATEKH743 +24 B (+0.00%) ±0 B (±0.00%)

See RAM/flash optimization guide for techniques to reduce usage.

@github-actions

Copy link
Copy Markdown

Test firmware build ready — commit 99f7a68

Download firmware for PR #11924

249 targets built. Find your board's .hex file by name on that page (e.g. MATEKF405SE.hex). Files are individually downloadable — no GitHub login required.

Development build for testing only. Use Full Chip Erase when flashing.

@sensei-hacker

Copy link
Copy Markdown
Member

FYI, in case useful: PR #11708 just added MSP2_INAV_MAG_UNALIGNED, an MSP command that reports the compass reading with zero/gain calibration applied but no alignment rotation. It's not the same thing you're adding here (yours is truly raw, pre-calibration, for external calibration tools via blackbox; that one is post-calibration, pre-alignment, for the Configurator's alignment wizard) — but if anyone building external tooling would rather poll over MSP than parse blackbox logs, the plumbing for a similarly-shaped fully-raw MSP message would be a small addition alongside it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants