Fix macro redefinition and unit test failures caused by shared platform.h leak

[sensei-hacker]

Fix unit test CI build failure on maintenance-10.x (macro leak from shared platform.h)

Summary

CI's test job (cmake -DTOOLCHAIN=none .. && ninja check) fails to build on maintenance-10.x. gimbal_serial_unittest fails to link, and several other unit tests print "macro redefined" warnings along the way. This PR fixes the link failure and eliminates every redefinition warning, verified with zero regressions on production/hardware builds.

Scope note: fixing the reported failure unmasked 5 more pre-existing, unrelated test failures that were always broken but never reached, because ninja's default fail-fast behavior stopped scheduling new work at the first failure (gimbal_serial_unittest, at build step 44/122). Those 5 turned out to be fixable within reasonable scope too (see "Fix, part 2" below) — all 17 unit test targets now pass, 196/196 gtest cases, ninja check exits 0.

Root cause

Two commits in the same MAVLink PR series (same author), merged back-to-back onto maintenance-10.x:

• 053c3977d6 ("Split MAVLink telemetry into modules with multi-port runtime") added #include "target/common.h" to src/main/telemetry/telemetry.h.
• be9aaa3b82 ("Add MAVLink unit tests for the multi-port core"), its direct child, added the same include to src/test/unit/platform.h:

  +#include "target/common.h"
   #include "target.h"

platform.h is included by every unit test translation unit, not just mavlink_unittest.cc. target/common.h unconditionally #defines a large baseline feature set. be9aaa3b82's include collided with several tests' own explicit -D feature macros (harmless-value but noisy "redefined" warnings), and in one case silently changed which code compiles:

src/main/io/gimbal_serial.c's vtable initializer was guarded by:

#if (defined(USE_HEADTRACKER) && defined(USE_HEADTRACKER_SERIAL))
static headTrackerVTable_t headTrackerVTable = {
    .process = headtrackerSerialProcess,
    .getDeviceType = headtrackerSerialGetDeviceType,
};

but the actual definitions of those two functions are additionally gated by #ifndef GIMBAL_UNIT_TEST. Before be9aaa3b82, USE_HEADTRACKER_SERIAL was never defined in the unit test build, so the reference and the (absent) definitions agreed — nothing to link. After be9aaa3b82, target/common.h unconditionally defines USE_HEADTRACKER_SERIAL too, so the vtable reference compiles in while the definitions stay compiled out:

undefined reference to `headtrackerSerialProcess'
undefined reference to `headtrackerSerialGetDeviceType'

target/common.h's inclusion in platform.h is load-bearing, not removable. I confirmed this empirically: removing it breaks settings-code-generation (settings.rb, run for every unit test target via enable_settings()) for 14 of 17 targets. Tracing further, this is because 053c3977d6 is also what settings-gen now silently depends on — settings.rb's check_conditions() builds one combined probe file (platform.h + every settings group header, in settings.yaml order) and only evaluates #if defined(MACRO) conditions at the very end, so it sees a macro as "on" if it's defined anywhere in that file. resolve_types() instead probes each struct inline, at that struct's own position in the same header sequence, so a macro only counts if something earlier already defined it. Once telemetry.h (late in the include order) started pulling in target/common.h, check_conditions() correctly saw feature macros as defined, but resolve_types() — probing structs from headers earlier in the list, e.g. sensors/gyro.h — didn't yet have them defined, and choked on fields that "should" exist (gyroConfig_t has no member named 'dynamicGyroNotchQ', etc.). platform.h including target/common.h first (via be9aaa3b82) papers over this by making every macro visible before any group header is reached. This ordering bug in settings.rb is real, pre-existing, and separate from this PR's fix — it's being tracked as its own follow-up (see below), not touched here.

Fix

1. src/main/io/gimbal_serial.c — added the missing !defined(GIMBAL_UNIT_TEST) condition to the vtable's #if guard, matching the guard already used for the function definitions a few dozen lines down. This is the fix for the actual link failure.
2. src/main/target/common.h — wrapped the entire unconditional "always-on baseline" block (~90 bare, presence-only #define USE_X-style feature-toggle macros) in #ifndef X / #define X / #endif guards, rather than patching only the handful of macros that happened to surface as CI warnings. These are toggle-only flags with no competing values, so the guard is a safe no-op wherever the macro isn't already defined — true for every real hardware target. I deliberately did not guard value-bearing macros (SCHEDULER_DELAY_LIMIT 10, NAV_MAX_WAYPOINTS 120, USE_BOOTLOG 2048, etc.) since blindly keeping "whichever definition came first" could silently mask a genuine value conflict rather than a harmless duplicate toggle. Verified structurally (#if/#ifndef count matches #endif count exactly; the set of macro names actually defined is byte-identical before/after) and via build (SITL, MATEKH743, KAKUTEF7, and MATEKF411 — a flash-constrained target that exercises the #undef block at the bottom of this file — all produce byte-identical/flash-identical output to the unmodified base commit, zero new warnings).
3. src/main/common/maths.h — wrapped M_Ef in #ifndef M_Ef. Unrelated to the above: INAV's own M_Ef collides with glibc's GNU-extension M_Ef (pulled in via <cmath> in maths_unittest.cc), and reproduces identically on release/9.1 and master today. The two values agree to full float precision, so this is a safe no-op guard.
4. src/test/unit/target.h — the unit test harness's own fake-target stub (test-only, not shared with any real hardware target), included by platform.h right after target/common.h. It independently redefines USE_GPS_PROTO_UBLOX and USE_TELEMETRY, colliding with common.h's definitions the same way — wrapped both in #ifndef guards for consistency.

Result: zero "macro redefined" warnings anywhere in ninja check's output (down from 23+), gimbal_serial_unittest builds, links, and passes, and 11 other previously-passing unit tests confirmed still passing with no regressions.

(Two small follow-ups from code review: USE_EZ_TUNE/USE_ADAPTIVE_FILTER at the end of common.h were missed in the initial sweep and are now guarded too for consistency; a one-line comment was added explaining why M_Ef is guarded when M_PIf/M_LN2f aren't.)

Fix, part 2: the 5 unmasked failures

Once gimbal_serial_unittest no longer blocked the build, 5 more targets failed — confirmed pre-existing on unmodified maintenance-10.x, not caused by this PR, and structurally different from the redefinition bug above (a macro becoming newly enabled for a test that was never built to handle it, not a macro being redefined — no #ifndef guard can fix this class of bug by construction). All 5 are now fixed:

time_unittest, rcdevice_unittest, telemetry_hott_unittest (compile errors) — drivers/time.h declares usTicks as a fixed uint32_t (a raw hardware tick counter, unrelated to USE_64BIT_TIME, which only widens the derived timeUs_t microsecond counter). Each test file's own mock had drifted from that: time_unittest.cc declared extern timeUs_t usTicks instead of uint32_t, and rcdevice_unittest.cc/telemetry_hott_unittest.cc each defined their own micros() mock hardcoded to return uint32_t instead of timeUs_t (drivers/time.h's actual declared return type). Before USE_64BIT_TIME was reachable in unit tests, timeUs_t silently defaulted to uint32_t, so these mismatches were invisible — the types happened to coincide. USE_64BIT_TIME is genuinely, deliberately always-on in common.h (not part of the leak — this was already latent, just never exercised in the test harness before). Fixed each mock to match drivers/time.h's real signatures.

gps_null_port_unittest (link errors) — this test's depends list in CMakeLists.txt is deliberately minimal (io/gps.c only, with only USE_GPS_PROTO_UBLOX explicitly set), reflecting its narrow original intent (test only null-port/proto-selection behavior). USE_GPS_FIX_ESTIMATION, USE_GPS_PROTO_MSP, USE_GPS_PROTO_DRONECAN, and USE_SIMULATOR all now leak in unconditionally from common.h, enabling extra code branches in gps.c that reference production globals (posControl, baro, positionEstimationConfig(), rMat, etc.) this minimal test was never built to link. This test already has its own marker macro (GPS_NULL_PORT_UNIT_TEST), following the same convention as GIMBAL_UNIT_TEST — but it wasn't referenced anywhere in gps.c yet. Added && !defined(GPS_NULL_PORT_UNIT_TEST) to the 10 relevant guard sites (verified: a plain -U compiler flag does not work here, since common.h's own #ifndef-guarded definitions re-establish the macro regardless — the fix has to be a source-level guard). One further USE_GPS_FIX_ESTIMATION occurrence, a STATE(GPS_ESTIMATED_FIX) flag read with no unlinked-symbol reference, was deliberately left untouched.

osd_unittest (link errors, two independent bugs):

1. displayport_msp_osd.c's entire body is gated by one outer #if defined(USE_OSD) && defined(USE_MSP_OSD) guard — neither macro is in this test's own -D list, both leak in from common.h, so the entire real DJI/MSP-displayport driver compiles in (~30 undefined symbols: armingFlags, cliMode, mspSerialPushPort, bitArray*, etc.), none of which is in the test's minimal depends list (io/osd_utils.c, io/displayport_msp_osd.c, common/typeconversion.c). Added && !defined(OSD_UNIT_TEST) to that guard — same pattern as everywhere else, correctly makes the file an empty translation unit for this test again.

2. Independently: osd_utils.c's osdFormatCentiNumber (the function actually under test) calls isDJICompatibleVideoSystem(osdConfig()), defined in displayport_msp_dji_compat.h. That header already has a dedicated #ifdef OSD_UNIT_TEST fallback (isDJICompatibleVideoSystem(osdConfigPtr) (true), never evaluating its argument) — but the header's outer guard didn't exclude OSD_UNIT_TEST directly, so it took the real branch instead, referencing osdConfig()/osdConfig_System. Added && !defined(OSD_UNIT_TEST) to that outer guard, routing to the existing test fallback.

   (src/test/unit/CMakeLists.txt previously tried to reach this same fallback a different way — defining DISABLE_MSP_BF_COMPAT, which is a typo; the real macro checked by the header is DISABLE_MSP_DJI_COMPAT. I deliberately did not fix the typo to the "correct" name: doing so would define DISABLE_MSP_DJI_COMPAT for real, which excludes osdConfig_t.highlight_djis_missing_characters — a field settings.yaml references completely unconditionally, no matching condition: key. That's a separate, dormant, pre-existing bug (currently unreachable, since no real target ever defines DISABLE_MSP_DJI_COMPAT either) that I don't want to trigger here. The header-guard fix above solves the original problem without needing that macro defined at all, so the typo'd flag was simply removed from CMakeLists.txt rather than "corrected.")

Result: ninja check now exits 0. All 17 unit test targets pass, 196/196 gtest cases, zero regressions, zero new warnings anywhere.

What I deliberately did not change

src/test/unit/CMakeLists.txt still explicitly declares macros like USE_ADSB, USE_SERIAL_GIMBAL, USE_HEADTRACKER, USE_TELEMETRY per-test, even though target/common.h's now-guarded definitions make most of these redundant today. Left as-is because: (a) not all of them are actually redundant — USE_TELEMETRY_MAVLINK on mavlink_unittest is real, since common.h only defines it when MCU_FLASH_SIZE > 512, which isn't set for unit test builds; and (b) the commit message on be9aaa3b82 says "later stack PRs re-add their own test sections" for the mavlink_multiport2 suite, so editing these lines now risks conflicting with in-flight follow-on work for no functional benefit.

Testing

• cmake -DTOOLCHAIN=none .. && ninja check (the exact CI command): exits 0. All 17 unit test targets build, link, and pass — 196/196 gtest cases. Zero "redefined" warnings anywhere in the build.
• SITL, MATEKH743 (H7), KAKUTEF7 (F7), MATEKF405 (F4): clean builds, zero new warnings, byte-identical/flash-identical output vs. unmodified base commit (a914d2dbe2).
• MATEKF411 (flash-constrained, MCU_FLASH_SIZE <= 512): clean build, and directly verified via a targeted probe that the #undef USE_SERIALRX_SPEKTRUM / #undef USE_TELEMETRY_SRXL block at the bottom of common.h still correctly un-defines macros that were set via the new #ifndef guard pattern.
• gps.c/displayport_msp_osd.c/displayport_msp_dji_compat.h guard additions specifically verified inert for production: none of GPS_NULL_PORT_UNIT_TEST/OSD_UNIT_TEST are ever defined by any real target.

Follow-ups (not in this PR)

1. settings.rb ordering bug (described above under "Root cause") — check_conditions() and resolve_types() disagree about macro visibility depending on include order. Currently masked by platform.h including target/common.h first; will resurface confusingly if that ordering ever changes. Needs its own scoped fix/review in the Ruby codegen tooling. Reported to project tracking separately.
2. Dormant settings.yaml/osd.h mismatch — osd.h's highlight_djis_missing_characters field is guarded by #ifndef DISABLE_MSP_DJI_COMPAT, but settings.yaml's osd_highlight_djis_missing_font_symbols entry references that field completely unconditionally, with no matching condition: key — and the settings.yaml DSL only supports positive "macro is defined" conditions, not negation, so there's no direct way to express "field only exists when macro X is not defined" today. Currently unreachable in practice (no real target defines DISABLE_MSP_DJI_COMPAT), avoided in this PR by not defining that macro for osd_unittest either — but it's a landmine for whoever eventually does need that macro defined (e.g. a future target or test). Worth its own follow-up.

Note for future test additions

src/test/unit/platform.h is shared across every unit test translation unit — anything added there has global blast radius across all other tests, not just the one being worked on. Same is true of src/main/telemetry/telemetry.h in this case, since it's included by settings-gen's combined probe. Worth keeping in mind for the rest of the mavlink_multiport2 test stack (cc @xznhj8129, since this PR series is what surfaced both issues above).

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[github-actions]

Test firmware build ready — commit 3374b7b

Download firmware for PR #11746

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