Fix: Remove static CRC table duplication in MAVLink helpers

[sensei-hacker]

Summary

Removes duplicated static data in the MAVLink helper functions. mavlink_helpers.h defines every helper — and the mavlink_message_crcs[] table inside mavlink_get_msg_entry() — with internal (static) linkage via the MAVLINK_HELPER macro, so every TU that includes the MAVLink headers compiles its own private copy of the functions and of the ~3.7 KB CRC table. A plain grep for "static inline" misses this because the macro hides the static keyword from the call site.

Changes

• Define MAVLINK_SEPARATE_HELPERS in the two INAV headers that include storm32/mavlink.h (src/main/rx/mavlink.h, src/main/mavlink/mavlink_types.h), so the vendored protocol.h emits only extern prototypes (its own documented mechanism, #ifdef MAVLINK_SEPARATE_HELPERS block).
• Add src/main/mavlink/mavlink_helpers.c — a single translation unit that compiles all helper definitions once with external linkage.
• Wire the new file into src/main/CMakeLists.txt and the mavlink_unittest.cc build (via extra_sources, since the unit-test helper requires a sibling .h for depends entries and this file's header is the vendored one).

No lib/ files are touched, so the fix survives generate.sh regeneration.

Testing

• Firmware build (BLUEBERRYF405, arm-none-eabi 13.2): clean, zero errors and warnings.
• Binary measurement (arm-none-eabi-nm --size-sort -S -C):
  • Before: mavlink_message_crcs.0 + .1, 2 × 4,044 B = 8,088 B duplicated
  • After: 1 copy (4,044 B) — flash 667,416 → 661,336 B (−6,080 B), RAM 122,884 → 122,500 B (−384 B)
  • (Table size varies by dialect set; the storm32 set here is 337 entries vs 312 in earlier measurements — same mechanism.)
• Unit tests: mavlink_unittest — 96/96 pass.

Notes

• The vendored protocol.h SEPARATE_HELPERS prototype block declares 13 of 22 non-convenience helpers; the other 9 (mavlink_get_channel_buffer, mavlink_sign_packet, mavlink_get_crc_extra, etc.) are referenced only within mavlink_helpers.h itself, so no implicit declarations occur. Verified: no INAV source or test calls them.
• Channel parse state (m_mavlink_status[], per-channel buffers) becomes globally shared instead of per-TU — no observable INAV change (only mavlink_runtime.c parses MAVLink), and the RAM delta is consistent with exactly two live copies before.

[qodo-code-review]

ⓘ 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]

PR Summary by Qodo

Deduplicate MAVLink helper definitions and CRC table

🐞 Bug fix ⚙️ Configuration changes 🕐 20-40 Minutes

AI Description

• Compile MAVLink helpers once with external linkage, eliminating per-translation-unit CRC table
 copies.
• Configure MAVLink consumers to use vendored extern prototypes without modifying generated library
 files.
• Include the shared helper unit in firmware and MAVLink unit-test builds.

Diagram

graph TD
  A["Firmware modules"] -->|include| B["Wrapper headers"] -->|select externs| C["Vendored protocol"]
  D["Build targets"] -->|compile once| E["Helper TU"] -->|emits| F["Helper definitions"] -->|owns one| G[("CRC table")]
  A -->|call| F

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Patch the vendored MAVLink headers

• ➕ Could expose a dedicated implementation macro or move the CRC table directly.
• ➖ Would modify generated library files.
• ➖ Could be overwritten by MAVLink regeneration.
• ➖ Creates an ongoing vendor-maintenance burden.

2. Rely on linker deduplication or LTO

• ➕ Would require fewer source-level changes.
• ➖ Does not reliably merge address-distinct function-local static data.
• ➖ Makes flash savings toolchain- and optimization-dependent.
• ➖ Preserves duplicated per-translation-unit parser state.

Recommendation: Use the PR's documented MAVLINK_SEPARATE_HELPERS mechanism. It establishes deterministic single ownership, avoids generated-file changes, and applies consistently to firmware and tests; reviewers should specifically confirm that globally shared channel state matches INAV's single parsing path.

Files changed (5) +51 / -0

Bug fix (3) +45 / -0

mavlink_helpers.cProvide one external MAVLink helper implementation +37/-0

Provide one external MAVLink helper implementation

• Introduces the sole translation unit that emits externally linked MAVLink helpers and their static data. It aligns channel-buffer sizing with INAV targets and suppresses expected unused-helper diagnostics.

src/main/mavlink/mavlink_helpers.c

mavlink_types.hRequest external helpers from the telemetry wrapper +4/-0

Request external helpers from the telemetry wrapper

• Defines MAVLINK_SEPARATE_HELPERS before including the storm32 dialect, guarded against macro redefinition. Consumers now receive prototypes instead of private helper implementations.

src/main/mavlink/mavlink_types.h

mavlink.hRequest external helpers from the receiver wrapper +4/-0

Request external helpers from the receiver wrapper

• Enables separate MAVLink helpers for receiver consumers with a guarded macro definition. This prevents receiver translation units from emitting duplicate helpers and CRC data.

src/main/rx/mavlink.h

Other (2) +6 / -0

CMakeLists.txtCompile the shared MAVLink helper translation unit +1/-0

Compile the shared MAVLink helper translation unit

• Adds the new helper implementation file to common firmware sources so every target links one externally visible helper set.

src/main/CMakeLists.txt

CMakeLists.txtLink shared helpers into MAVLink unit tests +5/-0

Link shared helpers into MAVLink unit tests

• Adds mavlink_helpers.c through extra_sources so the unit-test build avoids the dependency helper's nonexistent sibling-header transformation.

src/test/unit/CMakeLists.txt

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Tip of the day

💡 Did you know, you can start a comment with 'qodo' or '@qodo' to chat about any finding

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[github-actions]

RAM / Flash usage vs. base branch — commit b29a5dc

Target	Flash Δ	RAM Δ
MATEKF405 ⚠️	-6080 B (-0.91%)	-384 B (-0.26%)
MATEKF722	±0 B (±0.00%)	±0 B (±0.00%)
MATEKF765 ⚠️	-6080 B (-0.88%)	-392 B (-0.26%)
MATEKH743 ⚠️	-6144 B (-0.85%)	-384 B (-0.25%)

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

[github-actions]

Test firmware build ready — commit b29a5dc

Download firmware for PR #11825

246 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.