-
Notifications
You must be signed in to change notification settings - Fork 205
Add optional MAG definitions for BETAFPVF405 and BETAFPVF405_ELRS targets #880
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds three preprocessor macros — Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
configs/BETAFPVF405/config.h (1)
42-46: Avoid hiding driver macros behind USE_MAG; guard each macro independentlyAs written, if USE_MAG is defined upstream, the driver macros (USE_MAG_QMC5883, USE_MAG_IST8310) will be skipped entirely. That can silently omit needed drivers while still enabling the MAG feature. Prefer individually guarding each macro.
Apply this diff within this block:
-#ifndef USE_MAG -#define USE_MAG -#define USE_MAG_QMC5883 -#define USE_MAG_IST8310 -#endif +#ifndef USE_MAG +#define USE_MAG +#endif +#ifndef USE_MAG_QMC5883 +#define USE_MAG_QMC5883 +#endif +#ifndef USE_MAG_IST8310 +#define USE_MAG_IST8310 +#endifIf the goal is to keep magnetometer support truly optional by default (no added flash unless explicitly requested), consider not defining USE_MAG or any driver here and only provide the I2C pin mapping; users can enable the feature at build time.
configs/BETAFPVF405_ELRS/config.h (1)
42-46: Guard MAG driver macros independently to avoid accidental omissionSame pattern here: wrapping drivers under a single #ifndef USE_MAG means pre-defining USE_MAG upstream would skip driver inclusion.
Apply this diff within this block:
-#ifndef USE_MAG -#define USE_MAG -#define USE_MAG_QMC5883 -#define USE_MAG_IST8310 -#endif +#ifndef USE_MAG +#define USE_MAG +#endif +#ifndef USE_MAG_QMC5883 +#define USE_MAG_QMC5883 +#endif +#ifndef USE_MAG_IST8310 +#define USE_MAG_IST8310 +#endifOptional: if “optional MAG” is intended (not compiled by default), omit USE_MAG and driver defines here and only keep I2C pins; enable via build flags when needed.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
configs/BETAFPVF405/config.h(2 hunks)configs/BETAFPVF405_ELRS/config.h(2 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: haslinghuis
PR: betaflight/config#822
File: configs/AXISFLYINGH7MINI/config.h:29-37
Timestamp: 2025-06-23T18:43:31.746Z
Learning: In Betaflight configuration files, feature enablement macros like USE_MAG are build options that can be controlled at compile time, while hardware instance definitions like MAG_I2C_INSTANCE are predefined in board configurations to assist with hardware mapping when those features are enabled at build time.
📚 Learning: 2025-06-23T18:43:31.746Z
Learnt from: haslinghuis
PR: betaflight/config#822
File: configs/AXISFLYINGH7MINI/config.h:29-37
Timestamp: 2025-06-23T18:43:31.746Z
Learning: In Betaflight configuration files, feature enablement macros like USE_MAG are build options that can be controlled at compile time, while hardware instance definitions like MAG_I2C_INSTANCE are predefined in board configurations to assist with hardware mapping when those features are enabled at build time.
Applied to files:
configs/BETAFPVF405/config.hconfigs/BETAFPVF405_ELRS/config.h
📚 Learning: 2025-08-21T11:11:19.213Z
Learnt from: haslinghuis
PR: betaflight/config#870
File: configs/ZEX_ATHENA_STD_PRO/config.h:54-55
Timestamp: 2025-08-21T11:11:19.213Z
Learning: For STM32H743 in Betaflight, UART4 can use PB8 (UART4_RX) and PB9 (UART4_TX) as confirmed in the official Betaflight codebase at src/platform/STM32/serial_uart_stm32h7xx.c. This contradicts some generic STM32 documentation that might not show all supported pin configurations used by Betaflight.
Applied to files:
configs/BETAFPVF405/config.hconfigs/BETAFPVF405_ELRS/config.h
📚 Learning: 2025-08-21T11:11:19.213Z
Learnt from: haslinghuis
PR: betaflight/config#870
File: configs/ZEX_ATHENA_STD_PRO/config.h:54-55
Timestamp: 2025-08-21T11:11:19.213Z
Learning: For STM32H743 in Betaflight, UART4 can use PB8 (UART4_RX) and PB9 (UART4_TX) with GPIO_AF8_UART4, as confirmed in the official Betaflight codebase at src/platform/STM32/serial_uart_stm32h7xx.c. This is a valid pin mapping despite what some generic STM32 documentation might suggest.
Applied to files:
configs/BETAFPVF405/config.hconfigs/BETAFPVF405_ELRS/config.h
📚 Learning: 2025-08-19T18:34:22.887Z
Learnt from: osirisinferi
PR: betaflight/config#872
File: configs/HGLRCH743/config.h:82-83
Timestamp: 2025-08-19T18:34:22.887Z
Learning: For STM32H743, both UART5_TX (PB6) and UART5_RX (PB5) use alternate function AF14, not AF8. The Port B alternate functions table in the STM32H743 datasheet confirms PB5 supports UART5_RX and PB6 supports UART5_TX, both using AF14.
Applied to files:
configs/BETAFPVF405/config.hconfigs/BETAFPVF405_ELRS/config.h
📚 Learning: 2025-07-14T15:41:14.364Z
Learnt from: ot0tot
PR: betaflight/config#834
File: configs/RADIOLINKF405/config.h:79-88
Timestamp: 2025-07-14T15:41:14.364Z
Learning: In STM32F405 configurations, PB1 typically maps to TIM3_CH4 and PC9 typically maps to TIM8_CH4. These are different timers and do not share DMA resources, so there is no conflict when both pins are used simultaneously (e.g., PB1 for LED_STRIP and PC9 for MOTOR4).
Applied to files:
configs/BETAFPVF405/config.hconfigs/BETAFPVF405_ELRS/config.h
📚 Learning: 2025-07-25T20:06:07.492Z
Learnt from: haslinghuis
PR: betaflight/config#719
File: configs/SPEDIXG473/config.h:29-38
Timestamp: 2025-07-25T20:06:07.492Z
Learning: BMP280 and DPS310 barometer drivers in Betaflight do not require USE_I2C to be explicitly defined in board configurations. Out of 264 boards using these drivers, 259 (98%) work without USE_I2C defined, indicating that the I2C dependency is handled automatically by the build system.
Applied to files:
configs/BETAFPVF405_ELRS/config.h
📚 Learning: 2025-07-25T20:06:07.492Z
Learnt from: haslinghuis
PR: betaflight/config#719
File: configs/SPEDIXG473/config.h:29-38
Timestamp: 2025-07-25T20:06:07.492Z
Learning: BMP280 and DPS310 barometer drivers in Betaflight do not require USE_I2C to be explicitly defined in board configurations. Many existing boards successfully use USE_BARO_BMP280 and USE_BARO_DPS310 without defining USE_I2C, indicating that the I2C dependency is handled automatically by the build system or these sensors support alternative communication methods.
Applied to files:
configs/BETAFPVF405_ELRS/config.h
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Mark Haslinghuis <[email protected]>
|
PB8 is already mapped to ESC_SERIAL and RX_PPM in the target. It cannot be used for those and I2C. |
Wouldn't be the first that the I²C config (exclusively |
|
@osirisinferi guess these targets were submitted before our manufacturer guideline requirements kicked in - as we all learn in the progress to get the best possible hardware out. |
@haslinghuis Didn't mean to "point fingers", but thought maybe someone would like to fix these 🙂 Do you have schematics of the older boards? |
|
No worrries, schematics were not required before the program. |
Co-authored-by: Mark Haslinghuis <[email protected]>
Hi. Please check.
This configures the I2C interface on the board for use with an external magnetometer.
The target board has an I2C pad.
Thank you.
Summary by CodeRabbit