Refactor: Split OMNIBUSF4 family into 4 target directories#11196
Open
sensei-hacker wants to merge 9 commits intoiNavFlight:maintenance-9.xfrom
Open
Refactor: Split OMNIBUSF4 family into 4 target directories#11196sensei-hacker wants to merge 9 commits intoiNavFlight:maintenance-9.xfrom
sensei-hacker wants to merge 9 commits intoiNavFlight:maintenance-9.xfrom
Conversation
Contributor
|
You are nearing your monthly Qodo Merge usage quota. For more information, please visit here. PR Compliance Guide 🔍All compliance sections have been disabled in the configurations. |
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Split OMNIBUSF4 mega-target into 4 directories to improve maintainability: - DYSF4: DYSF4PRO, DYSF4PROV2 - OMNIBUSF4: Base OMNIBUSF4 variant only - OMNIBUSF4PRO: OMNIBUSF4PRO, OMNIBUSF4V3, OMNIBUSF4V3_ICM - OMNIBUSF4V3_SS: Softserial variants (S6_SS, S5S6_SS, S5_S6_2SS) Each directory now contains only the code relevant to its variants, with irrelevant preprocessor conditionals removed. Changes: - Reduced DYSF4/target.h from 271 to 169 lines (37% reduction) - Reduced preprocessor conditionals from 18 to 4 in DYSF4 files - All 11 targets build successfully and produce identical binaries
sensei-hacker
force-pushed
the
refactor/split-omnibusf4-targets
branch
from
e61568d to
cebc906
Compare
Remove preprocessor conditionals that can never be true in split directories: - OMNIBUSF4PRO: Remove checks for _S6_SS variants (in different directory) - OMNIBUSF4V3_SS: Remove checks for OMNIBUSF4PRO/V3 (in different directory) - Remove OMNIBUSF4PRO_LEDSTRIPM5 references (target no longer exists) Update comments to accurately describe what targets each directory contains. All affected targets build successfully.
The OMNIBUSF4 directory now only contains the base OMNIBUSF4 target. All other variants (OMNIBUSF4PRO, OMNIBUSF4V3, DYSF4, etc.) are in separate directories. Used unifdef to remove all conditionals for non-OMNIBUSF4 targets: - Removed checks for OMNIBUSF4PRO, OMNIBUSF4V3, DYSF4PRO, DYSF4PROV2 - Removed checks for softserial variants (_S6_SS, etc.) - Removed LEDSTRIPM5 references Results: - target.h: 280 → 147 lines (47% reduction, 133 lines removed) - target.c: cleaned (7 conditionals removed) - All conditionals removed: 0 remaining in both files - OMNIBUSF4 target builds successfully The OMNIBUSF4 directory now contains only the code specific to the base OMNIBUSF4 variant with no dead branches.
1. Added alias for OMNIBUSF4V3_ICM → OMNIBUSF4V3 in OMNIBUSF4PRO/target.h This fixes a critical bug where OMNIBUSF4V3_ICM would get wrong configuration: - Wrong IMU alignment (CW180 instead of CW270) - Missing MPU6500/BMI270 support - Wrong barometer (I2C-only instead of SPI BMP280) - Missing UART inverter support - Wrong SPI3_NSS_PIN (PB3 instead of PA15) - Wrong blackbox (SPI flash instead of SD card) - Wrong timer configuration 2. Removed dead #else clauses after OMNIBUS split: - DYSF4/target.h: Removed unreachable fallback cases - OMNIBUSF4PRO/target.h: All #else branches now dead after alias fix - OMNIBUSF4V3_SS/target.h: Removed unreachable softserial fallback All affected targets verified to build successfully.
- DYSF4/target.h: Removed ICM20608 comment (no ICM support in this directory) - OMNIBUSF4/target.h: Removed ICM20608 comment (no ICM support in this directory) - OMNIBUSF4/target.h: Removed obsolete relationship comments from consolidated file ICM20608 support only exists in OMNIBUSF4PRO and OMNIBUSF4V3_SS directories.
The #else branch at lines 44-46 was unreachable after adding OMNIBUSF4V3_ICM aliasing. All three targets (OMNIBUSF4PRO, OMNIBUSF4V3, OMNIBUSF4V3_ICM) always use TIM4/PB9, never TIM12/PB15.
1. Removed outdated 'pad labelled CH5/CH6 on OMNIBUSF4PRO' comments from: - OMNIBUSF4/target.h and target.c (wrong target) - DYSF4/target.h and target.c (wrong target) - OMNIBUSF4V3_SS/target.c (wrong target) These comments are only accurate in the OMNIBUSF4PRO directory where PC8/PC9 really are labelled CH5/CH6 on the physical board. 2. Removed redundant conditional from OMNIBUSF4PRO/target.c: #if (defined(OMNIBUSF4PRO) || defined(OMNIBUSF4V3)) This is always true since all 3 targets in this directory (OMNIBUSF4PRO, OMNIBUSF4V3, OMNIBUSF4V3_ICM) have one of these defined.
Remove orphaned indentation from preprocessor defines that were previously inside conditional blocks but are now at top level after the target split. Files changed: - OMNIBUSF4/target.h: Fixed 2-space and 4-space orphaned indents - DYSF4/target.h: Fixed 2-space orphaned indents - OMNIBUSF4V3_SS/target.h: Fixed extensive 2-space orphaned indents - OMNIBUSF4PRO/target.h: Fixed 4-space orphaned indent on ADC_CHANNEL_3_PIN Valid indentation inside active #if blocks was preserved. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
The upstream added OMNIBUSF4V3_ICM support to the monolithic OMNIBUSF4/target.h. This PR moves OMNIBUSF4V3_ICM to the new OMNIBUSF4PRO/ directory alongside V3. Resolution: - OMNIBUSF4/target.h: keep PR version (base target only, no PRO/V3/ICM) - OMNIBUSF4/CMakeLists.txt: keep PR version (only OMNIBUSF4 here) - OMNIBUSF4PRO/target.h: add ICM42605 support for OMNIBUSF4V3_ICM, which was present in upstream but missing from the split target
sensei-hacker
added
Ready to merge
Testing Required
Release Notes
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
Summary
Splits the OMNIBUSF4 target family from 1 directory into 4 for better organization and maintainability.
It had nine different targets tangled together.
I used unifdef to split them, than used
clang -E -dDandgcc -Eto try to verify that the splitting is correct.Changes
New 4-way directory structure:
DYSF4/ (2 targets):
DYSF4PRO,DYSF4PROV2OMNIBUSF4/ (1 target):
OMNIBUSF4OMNIBUSF4PRO/ (3 targets):
OMNIBUSF4PRO,OMNIBUSF4V3,OMNIBUSF4V3_ICMOMNIBUSF4V3_SS/ (3 targets):
OMNIBUSF4V3_S6_SS,OMNIBUSF4V3_S5S6_SS,OMNIBUSF4V3_S5_S6_2SSDescription
This description is generated by an AI tool. It may have inaccuracies
Split OMNIBUSF4 family from 3 directories into 4 separate target directories
Removed conditional compilation blocks from target files for cleaner separation
All 9 targets verified to produce identical preprocessed output before/after split
Improved maintainability by isolating hardware-specific configurations per directory
Diagram Walkthrough
File Walkthrough
8 files
New DYSF4 target header with I2C1 configurationNew DYSF4 target timer hardware configurationSimplified OMNIBUSF4 header removing conditional blocksSimplified OMNIBUSF4 timer configuration removing conditionalsNew OMNIBUSF4PRO header with base variant configurationsNew OMNIBUSF4PRO timer hardware with conditional blocksNew OMNIBUSF4V3_SS header with softserial configurationsNew OMNIBUSF4V3_SS timer hardware with softserial variants4 files
New DYSF4 build configuration for two targetsUpdated OMNIBUSF4 build to single target onlyNew OMNIBUSF4PRO build configuration for three targetsNew OMNIBUSF4V3_SS build configuration for three targets