-
Notifications
You must be signed in to change notification settings - Fork 201
Add configuration file for RP2350 based MADFLIGHT_FC3 #940
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
base: master
Are you sure you want to change the base?
Conversation
WalkthroughAdds a new public board configuration header Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (13)📓 Common learnings📚 Learning: 2025-07-03T04:33:57.480ZApplied to files:
📚 Learning: 2025-07-14T16:16:56.822ZApplied to files:
📚 Learning: 2025-07-03T04:33:57.480ZApplied to files:
📚 Learning: 2025-06-23T18:43:31.746ZApplied to files:
📚 Learning: 2025-07-28T16:44:59.686ZApplied to files:
📚 Learning: 2025-08-20T02:54:58.596ZApplied to files:
📚 Learning: 2025-07-14T15:38:06.837ZApplied to files:
📚 Learning: 2025-07-23T13:43:50.456ZApplied to files:
📚 Learning: 2025-09-26T16:32:46.694ZApplied to files:
📚 Learning: 2025-07-23T13:43:50.456ZApplied to files:
📚 Learning: 2025-07-25T20:06:07.492ZApplied to files:
📚 Learning: 2025-08-22T17:08:23.283ZApplied to files:
🔇 Additional comments (5)
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. Comment |
Updated SDCARD configuration to use SPI0 pins and commented out unused sections.
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: 0
🧹 Nitpick comments (1)
configs/MADFLIGHT_FC3/config.h (1)
58-58: Remove the orphaned GYRO_1_CLKIN_PIN definition.The RP2350 MCU lacks timer infrastructure needed for USE_GYRO_CLKIN (confirmed by compile error). Unlike STM32-based boards, there's no TIMER_PIN_MAP support on RP2350. GYRO_1_CLKIN_PIN at line 71 is unused and misleading—delete it. The board functions properly with dual gyro drivers (ICM42688P, ICM42605) enabled for auto-detection, as proven by HELLBENDER_0001 (another RP2350B board) which omits this entirely.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
configs/MADFLIGHT_FC3/config.h(1 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: ot0tot
Repo: betaflight/config PR: 0
File: :0-0
Timestamp: 2025-07-03T04:33:57.480Z
Learning: Always verify that the BOARD_NAME define in each config.h matches the directory name it resides in (e.g., configs/AXISFLYINGF7AIO/config.h must have #define BOARD_NAME AXISFLYINGF7AIO). This check is required for all Betaflight board configuration reviews.
Learnt from: haslinghuis
Repo: betaflight/config PR: 835
File: configs/HDZERO_HALO_MPU6000/config.h:44-56
Timestamp: 2025-07-14T16:16:56.822Z
Learning: For Betaflight board configurations of boards already in production, pin assignments in config.h files reflect the actual hardware design and cannot be changed, even if they present potential conflicts with debug interfaces. The configuration must match the physical board layout, not ideal software design preferences.
Learnt from: osirisinferi
Repo: betaflight/config PR: 0
File: :0-0
Timestamp: 2025-09-26T16:32:46.694Z
Learning: When reviewing hardware configuration files like Betaflight configs, always check for pin assignment conflicts by verifying that no physical pins (like PB10, PB11, etc.) are assigned to multiple peripheral functions simultaneously.
Learnt from: ot0tot
Repo: betaflight/config PR: 0
File: :0-0
Timestamp: 2025-07-03T04:33:57.480Z
Learning: Always verify that the `BOARD_NAME` define in config.h files exactly matches the directory name where the config file is located. For example, if the config file is in configs/AXISFLYINGF7AIO/config.h, then the BOARD_NAME should be defined as "AXISFLYINGF7AIO". This ensures proper board identification and configuration loading in Betaflight.
Learnt from: haslinghuis
Repo: betaflight/config PR: 879
File: configs/AIRBOTSUPERF4V2/config.h:42-45
Timestamp: 2025-08-22T17:08:23.283Z
Learning: In Betaflight board configurations, OSD feature flags like USE_OSD_SD (analog/MAX7456) and USE_OSD_HD (digital/MSP DisplayPort) are typically defined at build time by the build system, not in the individual board config.h files. Board configs can conditionally define OSD-related settings based on these build-time flags.
Learnt from: haslinghuis
Repo: betaflight/config PR: 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.
Learnt from: haslinghuis
Repo: betaflight/config PR: 656
File: configs/IFLIGHT_F745_AIO/config.h:31-35
Timestamp: 2025-07-28T16:44:59.686Z
Learning: In Betaflight board configurations, when a board design uses the same physical gyro socket but different sensor variants across production batches (e.g., MPU6000 in older batches, ICM42688P in newer batches), the correct approach is to enable both sensor drivers (USE_GYRO_SPI_MPU6000 and USE_GYRO_SPI_ICM42688P) in the same configuration. This allows runtime auto-detection of which sensor is actually present without requiring separate board configurations for each hardware variant.
Learnt from: haslinghuis
Repo: betaflight/config PR: 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.
📚 Learning: 2025-07-03T04:33:57.480Z
Learnt from: ot0tot
Repo: betaflight/config PR: 0
File: :0-0
Timestamp: 2025-07-03T04:33:57.480Z
Learning: Always verify that the BOARD_NAME define in each config.h matches the directory name it resides in (e.g., configs/AXISFLYINGF7AIO/config.h must have #define BOARD_NAME AXISFLYINGF7AIO). This check is required for all Betaflight board configuration reviews.
Applied to files:
configs/MADFLIGHT_FC3/config.h
📚 Learning: 2025-07-14T16:16:56.822Z
Learnt from: haslinghuis
Repo: betaflight/config PR: 835
File: configs/HDZERO_HALO_MPU6000/config.h:44-56
Timestamp: 2025-07-14T16:16:56.822Z
Learning: For Betaflight board configurations of boards already in production, pin assignments in config.h files reflect the actual hardware design and cannot be changed, even if they present potential conflicts with debug interfaces. The configuration must match the physical board layout, not ideal software design preferences.
Applied to files:
configs/MADFLIGHT_FC3/config.h
📚 Learning: 2025-06-23T18:43:31.746Z
Learnt from: haslinghuis
Repo: betaflight/config PR: 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/MADFLIGHT_FC3/config.h
📚 Learning: 2025-07-03T04:33:57.480Z
Learnt from: ot0tot
Repo: betaflight/config PR: 0
File: :0-0
Timestamp: 2025-07-03T04:33:57.480Z
Learning: Always verify that the `BOARD_NAME` define in config.h files exactly matches the directory name where the config file is located. For example, if the config file is in configs/AXISFLYINGF7AIO/config.h, then the BOARD_NAME should be defined as "AXISFLYINGF7AIO". This ensures proper board identification and configuration loading in Betaflight.
Applied to files:
configs/MADFLIGHT_FC3/config.h
📚 Learning: 2025-08-22T17:08:23.283Z
Learnt from: haslinghuis
Repo: betaflight/config PR: 879
File: configs/AIRBOTSUPERF4V2/config.h:42-45
Timestamp: 2025-08-22T17:08:23.283Z
Learning: In Betaflight board configurations, OSD feature flags like USE_OSD_SD (analog/MAX7456) and USE_OSD_HD (digital/MSP DisplayPort) are typically defined at build time by the build system, not in the individual board config.h files. Board configs can conditionally define OSD-related settings based on these build-time flags.
Applied to files:
configs/MADFLIGHT_FC3/config.h
📚 Learning: 2025-08-20T02:54:58.596Z
Learnt from: ot0tot
Repo: betaflight/config PR: 872
File: configs/HGLRCH743/config.h:82-83
Timestamp: 2025-08-20T02:54:58.596Z
Learning: For Betaflight board configuration reviews, the definitive source for validating UART pin assignments is the serial_uart_{platform}.c file in the Betaflight codebase (e.g., serial_uart_stm32h7xx.c for STM32H7xx platforms), not generic MCU datasheets. These files contain the exact pin mappings that Betaflight supports for each UART peripheral.
Applied to files:
configs/MADFLIGHT_FC3/config.h
📚 Learning: 2025-07-28T16:44:59.686Z
Learnt from: haslinghuis
Repo: betaflight/config PR: 656
File: configs/IFLIGHT_F745_AIO/config.h:31-35
Timestamp: 2025-07-28T16:44:59.686Z
Learning: In Betaflight board configurations, when a board design uses the same physical gyro socket but different sensor variants across production batches (e.g., MPU6000 in older batches, ICM42688P in newer batches), the correct approach is to enable both sensor drivers (USE_GYRO_SPI_MPU6000 and USE_GYRO_SPI_ICM42688P) in the same configuration. This allows runtime auto-detection of which sensor is actually present without requiring separate board configurations for each hardware variant.
Applied to files:
configs/MADFLIGHT_FC3/config.h
📚 Learning: 2025-07-25T20:06:07.492Z
Learnt from: haslinghuis
Repo: betaflight/config PR: 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/MADFLIGHT_FC3/config.h
📚 Learning: 2025-07-25T20:06:07.492Z
Learnt from: haslinghuis
Repo: betaflight/config PR: 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/MADFLIGHT_FC3/config.h
📚 Learning: 2025-09-26T16:32:46.694Z
Learnt from: osirisinferi
Repo: betaflight/config PR: 0
File: :0-0
Timestamp: 2025-09-26T16:32:46.694Z
Learning: When reviewing hardware configuration files like Betaflight configs, always check for pin assignment conflicts by verifying that no physical pins (like PB10, PB11, etc.) are assigned to multiple peripheral functions simultaneously.
Applied to files:
configs/MADFLIGHT_FC3/config.h
🔇 Additional comments (6)
configs/MADFLIGHT_FC3/config.h (6)
24-26: LGTM: Board identifiers are correct.The BOARD_NAME "MADFLIGHT_FC3" correctly matches the directory name "configs/MADFLIGHT_FC3/" as required.
87-100: SDIO configuration remains work-in-progress.The commented SDIO configuration indicates a known issue where the 4-bit SDIO mode compiles but doesn't function properly in the configurator. This is documented for future investigation.
If SDIO support is a priority for this board, consider opening a separate issue to track the investigation of why SDIO appears as "Not configured" despite compiling successfully.
106-122: LGTM: External pin mappings are conflict-free.All external pin assignments (PA0-PA11, PA14-PA15) are properly defined without conflicts with internal pin usage (PA26-PA46). The unused pins PA12 and PA13 are appropriately documented.
124-130: LGTM: External barometer configuration is correct.Multiple barometer drivers are enabled for auto-detection on I2C1, which is the standard approach. Per Betaflight learnings, these barometer drivers do not require explicit
USE_I2Cdefinition as the I2C dependency is handled automatically by the build system.
132-137: LGTM: External magnetometer configuration is correct.Multiple magnetometer drivers are enabled for auto-detection on I2C1 (same bus as the barometer). Per Betaflight learnings, USE_MAG is a build-time option while MAG_I2C_INSTANCE is predefined in the board configuration to assist with hardware mapping when the feature is enabled.
75-85: ****The review comment is based on a flawed premise.
USE_SPI_DEVICE_0does not exist anywhere in the codebase—a repository-wide search returns no results. Moreover, HELLBENDER_0001, another RP2350-based board with SPI0 pins defined, also does not includeUSE_SPI_DEVICE_0and functions without it. The pattern across all boards shows thatUSE_SPI_DEVICE_*flags are platform or device-specific and not universally required. The MADFLIGHT_FC3 configuration is consistent with other RP2350 boards in the repository.Likely an incorrect or invalid review comment.
|
Did some testing on with the config:
Can't fly (yet) because I'm using an old DJI Phantom with PWM ESCs which are not supported for RP2350... |
| #define GYRO_1_EXTI_PIN PA27 | ||
| #define GYRO_1_CS_PIN PA29 | ||
| #define GYRO_1_CLKIN_PIN PA26 // for ICM42688P, needs #define USE_GYRO_CLKIN | ||
| #define GYRO_1_CLKIN_PIN PA26 // for ICM42688P,ICP45686 needs #define USE_GYRO_CLKIN |
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.
Haven't checked - was 45686 implemented with support for GYRO_CLKIN ?
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.
No it is not implemented in bf, but I added the pin on the board. Maybe something to add to: betaflight/betaflight#14733 ?
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.
Bugfixes and improvements need separate PR :)
|
This should be the final update to this PR, the board is in production. Decided to go for ICM45686 instead of ICM42688P. The schematic is here: https://madflight.com/img/madflight-FC3v2.pdf Let me know if anything else is needed for your review. |
Pull-Request requirements
Mandatory Review for All New Flight Controllers
Hardware Compliance Requirements
Checklist (✓/✕, or y/n)
Summary by CodeRabbit