Skip to content

docs(msp): fix enum doc generator double-counting exclusive #ifdef branches - #11844

Open
sensei-hacker wants to merge 2 commits into
iNavFlight:maintenance-10.xfrom
sensei-hacker:fix/msp-enum-doc-gen-ifdef-branches
Open

docs(msp): fix enum doc generator double-counting exclusive #ifdef branches#11844
sensei-hacker wants to merge 2 commits into
iNavFlight:maintenance-10.xfrom
sensei-hacker:fix/msp-enum-doc-gen-ifdef-branches

Conversation

@sensei-hacker

Copy link
Copy Markdown
Member

Summary

Fixes docs/development/msp/gen_enum_md.py so mutually-exclusive preprocessor branches inside an enum body don't double-count the auto-increment counter.

The bug: for mixerProfileATState_e in src/main/flight/mixer_profile.h:

typedef enum {
    MIXERAT_PHASE_IDLE,                      // 0
    MIXERAT_PHASE_TRANSITION_INITIALIZE,     // 1
    MIXERAT_PHASE_TRANSITIONING,             // 2
#ifdef USE_AUTO_TRANSITION
    MIXERAT_PHASE_POST_SWITCH_FADE,          // (3)
    MIXERAT_PHASE_TAILSITTER_TO_MC_CAPTURE,  // (4)
#endif
#ifndef USE_AUTO_TRANSITION
    MIXERAT_PHASE_DONE,                      // generated (5), correct value is 3
#endif
} mixerProfileATState_e;

MIXERAT_PHASE_DONE exists only in builds without USE_AUTO_TRANSITION, where its true value is 3 — but the generator auto-incremented straight through both branches and emitted (5), double-counting the two members that can never coexist with it.

The fix: ConditionStack frames now record the auto-increment base each branch started from. When an exclusive alternate branch is entered — a sibling #ifndef X after #ifdef X at the same nesting level, or an #else/#elif within one #if family — the counter resets to that base instead of continuing through the other branch's members.

Verification:

  • MIXERAT_PHASE_DONE now generates as (3)
  • Swept all 873 enums: on identical input, the only output change vs the old generator is that one line (no regression on simple or single-branch-optional enums) ✓
  • Generalizes to sibling #ifdef/#ifndef pairs in both orders, #else/#elif families, and nested pairs (verified with synthetic cases); two consecutive same-symbol #ifdef X blocks (not mutually exclusive) are correctly left sequential ✓
  • Regenerated inav_enums_ref.md / inav_enums.json via gen_docs.sh; the remaining diff is enum/MSP drift picked up from source since the last regen (new enums like dronecanAsyncState_e, INA226 sensors) ✓

Tests

  • Regeneration diff old-vs-new on identical all_enums.h: exactly one line changed
  • Synthetic pattern tests: adjacent pair, reverse pair, #else, #elif, nested pair, explicit-value reset, same-symbol non-pair

No firmware code touched; docs tooling + regenerated docs only.

…anches

The enum-body parser advanced the auto-increment counter through both
members of mutually-exclusive branches (#ifdef X / #ifndef X siblings,
and #else / #elif within one family), so values in the alternate branch
were inflated. E.g. MIXERAT_PHASE_DONE generated as (5) but is 3 in any
build where it exists. Track the counter base per open branch and reset
to it when an exclusive alternate branch is entered; regenerate the MSP
enum reference docs (which also picks up enum drift since the last regen).
@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

Fix conditional enum numbering in MSP documentation generator

🐞 Bug fix 📝 Documentation 🕐 20-40 Minutes

Grey Divider

AI Description

• Reset enum counters across mutually exclusive preprocessor branches.
• Correct MIXERAT_PHASE_DONE from conditional value 5 to 3.
• Regenerate MSP references with new messages and current enum source drift.
Diagram

graph TD
  A["Enum Headers"] --> B["Enum Parser"] --> C{"Branch directive?"}
  C -->|Yes| D["Condition Stack"] --> E["Counter Base"] --> F["Doc Renderer"] --> G["Enum Outputs"]
  C -->|No| E
Loading
High-Level Assessment

The branch-base snapshot approach is appropriate for a documentation parser that intentionally avoids evaluating C expressions. It fixes known exclusive branch forms without requiring an impractical build-matrix preprocessing pass or a full symbolic preprocessor.

Files changed (4) +178 / -32

Bug fix (1) +84 / -21
gen_enum_md.pyRestore enum counters at exclusive branch boundaries +84/-21

Restore enum counters at exclusive branch boundaries

• Extends conditional stack frames with branch starting values, symbols, and polarity. The parser now restores the shared counter base for opposite '#ifdef'/'#ifndef' siblings and for '#else' or '#elif' alternatives, including nested branches.

docs/development/msp/gen_enum_md.py

Documentation (3) +94 / -11
README.mdDocument new ADSB and wind MSP messages +53/-0

Document new ADSB and wind MSP messages

• Adds index entries and payload documentation for single-vehicle ADSB polling, ADSB slot counts, and estimated wind data. These regenerated additions reflect MSP specification drift since the previous documentation build.

docs/development/msp/README.md

inav_enums.jsonRegenerate machine-readable enum reference +18/-6

Regenerate machine-readable enum reference

• Corrects 'MIXERAT_PHASE_DONE' from conditional value '(5)' to '(3)'. The regeneration also captures current source enums for terrain hold, INA226 sensors, DroneCAN async state, and current-meter ownership.

docs/development/msp/inav_enums.json

inav_enums_ref.mdRegenerate human-readable enum reference +23/-5

Regenerate human-readable enum reference

• Publishes the corrected conditional value for 'MIXERAT_PHASE_DONE' and mirrors newly discovered enum members and types from current firmware headers.

docs/development/msp/inav_enums_ref.md

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

qodo-free-for-open-source-projects Bot commented Aug 31, 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. Stale branch base reset ✓ Resolved 🐞 Bug ≡ Correctness
Description
ConditionStack.closed remains valid after intervening enumerators, so a later opposite-polarity
block for the same symbol is incorrectly treated as the prior block's alternate and resets
current_numeric to a stale base. For example, after A, #ifdef X B, #endif C = 10, #ifndef X D,
this PR emits D as (1) instead of the correct (11), whereas the old sequential counter handled
the explicit reset correctly.
Code

docs/development/msp/gen_enum_md.py[R109-112]

+        if prev and frame['sym'] is not None and prev['sym'] == frame['sym'] \
+                and prev['polarity'] != frame['polarity']:
+            # mutually-exclusive sibling: restart numbering from the sibling's base
+            frame['base'] = prev['base']
Evidence
endif() stores each closed symbol frame by nesting depth, _push() later reuses it solely from
matching depth/symbol/polarity, and the enumerator path never invalidates that cached frame.
Consequently an intervening explicit assignment updates current_numeric to 10, but the later
#ifndef X overwrites it with the earlier base before auto-incrementing.

docs/development/msp/gen_enum_md.py[107-114]
docs/development/msp/gen_enum_md.py[137-144]
docs/development/msp/gen_enum_md.py[276-295]

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 sibling-branch cache survives ordinary enum members, causing a later opposite-polarity conditional to reuse a stale numeric base even though the two blocks are not adjacent alternate branches.
## Issue Context
Only an immediately corresponding opposite-polarity sibling should reuse the closed branch base. Parsing any intervening enumerator must invalidate the closed frame at that nesting level; add regression coverage including an intervening explicit assignment.
## Fix Focus Areas
- docs/development/msp/gen_enum_md.py[107-114]
- docs/development/msp/gen_enum_md.py[262-297]

ⓘ 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 type 'qodo, fix this' on a finding and the fix lands right on your PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread docs/development/msp/gen_enum_md.py
Only adjacent #ifdef/#ifndef blocks are exclusive alternates. A member
parsed between them advances the counter in every build, so a later
opposite-polarity block must continue from that value instead of
restarting from the earlier branch's base (e.g. A, #ifdef X B, #endif,
C = 10, #ifndef X D -> D must be 11, not 1).
@sensei-hacker

Copy link
Copy Markdown
Member Author

Addressed the review finding in f62230c: the sibling-branch cache is now invalidated by note_item() whenever an enumerator is parsed at that nesting level, so a later opposite-polarity block is only treated as an exclusive alternate when it immediately follows the earlier branch. Verified: A, #ifdef X {B}, #endif, C = 10, #ifndef X {D} now yields D = (11) as the reviewer's example requires, while MIXERAT_PHASE_DONE stays (3) and the full 873-enum sweep still shows exactly one changed line.

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.

1 participant