Update TAKERF722SE #918
Update TAKERF722SE #918
Conversation
WalkthroughUpdated TAKERF722SE board config: removed BMI270 accel/gyro macro; added generic and specific barometer macros (BMP280, DPS310); moved SERIALRX_UART from USART3 to USART2; added MSP_DISPLAYPORT_UART set to USART1. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 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 (5)📚 Learning: 2025-07-23T13:43:50.456ZApplied to files:
📚 Learning: 2025-08-20T02:54:58.596ZApplied to files:
📚 Learning: 2025-07-23T13:43:50.456ZApplied to files:
📚 Learning: 2025-07-25T20:06:07.492ZApplied to files:
📚 Learning: 2025-07-25T20:06:07.492ZApplied to files:
🔇 Additional comments (3)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
configs/TAKERF722SE/config.h (1)
109-111: UART assignments valid; confirm PA3 sharing between PPM and Serial RXMSP_DISPLAYPORT_UART=SERIAL_PORT_USART1 and SERIALRX_UART=SERIAL_PORT_USART2 use valid identifiers. Note PA3 is both RX_PPM_PIN (Line 55) and UART2 RX (Line 64); that overlap is common but mutually exclusive at runtime. Please confirm this pin‑share is intended for this target and documented for users.
If helpful, add a brief comment above RX_PPM_PIN noting it shares PA3 with USART2 RX.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
configs/TAKERF722SE/config.h(3 hunks)
🧰 Additional context used
🧠 Learnings (9)
📚 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/TAKERF722SE/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/TAKERF722SE/config.h
📚 Learning: 2025-07-28T16:44:59.686Z
Learnt from: haslinghuis
PR: betaflight/config#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/TAKERF722SE/config.h
📚 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/TAKERF722SE/config.h
📚 Learning: 2025-05-28T15:45:15.608Z
Learnt from: haslinghuis
PR: betaflight/config#792
File: configs/MERAKRCF722/config.h:32-32
Timestamp: 2025-05-28T15:45:15.608Z
Learning: The presence of GYRO_2_SPI_INSTANCE definition in a board config does not necessarily mean the board uses dual gyros. Some boards have GYRO_2_SPI_INSTANCE defined but it's not actually used, so they should not receive GYRO_COUNT or other dual-gyro related definitions. Only boards that actually utilize dual gyros should get these definitions.
Applied to files:
configs/TAKERF722SE/config.h
📚 Learning: 2025-05-28T15:42:05.402Z
Learnt from: haslinghuis
PR: betaflight/config#792
File: configs/BEEROTORF4/config.h:30-30
Timestamp: 2025-05-28T15:42:05.402Z
Learning: The DEFAULT_GYRO_ENABLED macro with both gyros enabled (GYRO_MASK(0) | GYRO_MASK(1)) should only be added to board configurations that have DEFAULT_GYRO_TO_USE set to GYRO_CONFIG_USE_GYRO_BOTH. Boards without this setting should only get the GYRO_COUNT definition to specify the number of available gyros.
Applied to files:
configs/TAKERF722SE/config.h
📚 Learning: 2025-07-03T15:17:30.040Z
Learnt from: haslinghuis
PR: betaflight/config#0
File: :0-0
Timestamp: 2025-07-03T15:17:30.040Z
Learning: In Betaflight configurations, when a target name suggests dual IMUs (like JHEF7DUAL) but specific hardware variants only have one gyro available, the preferred solution is to use DEFAULT_GYRO_TO_USE macro to specify which gyro to use by default rather than pruning gyro defines or creating redundant configurations. This approach maintains compatibility when the same target is used by multiple hardware variants from the same manufacturer.
Applied to files:
configs/TAKERF722SE/config.h
📚 Learning: 2025-07-23T13:43:50.456Z
Learnt from: ot0tot
PR: betaflight/config#0
File: :0-0
Timestamp: 2025-07-23T13:43:50.456Z
Learning: For Betaflight board configuration reviews, always verify that serial port defines (like ESC_SENSOR_UART, MSP_UART, GPS_UART, etc.) use valid SERIAL_PORT identifiers that match those defined in the Betaflight codebase at src/main/io/serial.h. Common valid identifiers include SERIAL_PORT_USART1, SERIAL_PORT_USART2, SERIAL_PORT_USART3, SERIAL_PORT_UART4, SERIAL_PORT_UART5, etc. Invalid identifiers like SERIAL_PORT_UART3 (should be SERIAL_PORT_USART3) will cause build failures.
Applied to files:
configs/TAKERF722SE/config.h
📚 Learning: 2025-07-23T13:43:50.456Z
Learnt from: ot0tot
PR: betaflight/config#0
File: :0-0
Timestamp: 2025-07-23T13:43:50.456Z
Learning: For Betaflight board configuration reviews, always verify that serial port defines use valid identifiers from the serialPortIdentifier_e enum in src/main/io/serial.h. Valid identifiers include: SERIAL_PORT_USB_VCP (20), SERIAL_PORT_SOFTSERIAL1 (30), SERIAL_PORT_SOFTSERIAL2 (31), SERIAL_PORT_LPUART1 (40), SERIAL_PORT_UART0 (50, if SERIAL_UART_FIRST_INDEX == 0), SERIAL_PORT_USART1 (51 or 50), SERIAL_PORT_USART2, SERIAL_PORT_USART3, SERIAL_PORT_UART4, SERIAL_PORT_UART5, SERIAL_PORT_USART6, SERIAL_PORT_USART7, SERIAL_PORT_USART8, SERIAL_PORT_UART9, SERIAL_PORT_USART10. While some aliases exist (like SERIAL_PORT_UART3 = SERIAL_PORT_USART3), prefer using the primary identifiers as referenced in the serialPortIdentifiers array to avoid build failures.
Applied to files:
configs/TAKERF722SE/config.h
🔇 Additional comments (2)
configs/TAKERF722SE/config.h (2)
34-36: Baro enablement looks correct; no explicit USE_I2C neededUSE_BARO plus BMP280 and DPS310 with BARO_I2C_INSTANCE=I2CDEV_1 and I2C1 pins defined is consistent. Many boards ship exactly this without defining USE_I2C, so this is fine. (Based on learnings)
122-122: PINIO1_BOX value verified: 40 matches BOX_PINIO1 in upstream Betaflight master (as of 2025-09-29). No changes needed.
|
|
||
| // serial 5 1024 115200 57600 0 115200 | ||
| #define MSP_DISPLAYPORT_UART SERIAL_PORT_USART1 | ||
| #define SERIALRX_UART SERIAL_PORT_USART2 |
There was a problem hiding this comment.
Default UART port functions are generally only used for peripherals that are integrated onto the board itself, or BNF quads where the factory setup will consistently use those specific ports. If you cannot ensure consistency, it's best to leave this up to the user or factory setup
There was a problem hiding this comment.
In the design of FC, UART2 is specifically used for RX communication. However, the target has not been modified before.
There was a problem hiding this comment.
If that's how it comes set up in the BNFs that use it, that's probably fine
configs/TAKERF722SE/config.h
Outdated
| #define MAX7456_SPI_INSTANCE SPI2 | ||
| #define PINIO1_CONFIG 129 | ||
| #define PINIO1_BOX 0 | ||
| #define PINIO1_BOX 40 |
There was a problem hiding this comment.
Why are you changing this? What does PINIO 1 control?
There was a problem hiding this comment.
Because some users have requested that USER1 be used to control the on/off of the 12V power supply.
There was a problem hiding this comment.
What is PINIO1 connected to on the FC? Can you post the schematic in the Discord?
There was a problem hiding this comment.
This is the schematic diagram of the PINIO1 connection. It can control a pad to output 12V voltage.
There was a problem hiding this comment.
So for the past several years the VTX power on this FC was tied to the arm switch? That makes no sense. Have you changed the design?
There was a problem hiding this comment.
If you are making changes to the FC design, this requires a new target.
There was a problem hiding this comment.
I believe that the new design and the old design can use the same target; there is no need to create a new one. However, the new design is still under planning. If you don't agree with the modifications made by pinio1, I can first cancel them and then we can have a discussion later. We need to add a barometer now.
There was a problem hiding this comment.
Does this FC have a barometer? The product page and images make no mention of it.
https://geprc.com/product/geprc-taker-f722-se-flight-controller/
There was a problem hiding this comment.
Our subsequent plan is to add the "spl06-001 baro" to it.
There was a problem hiding this comment.
- The SPL06 is a poor quality baro, use something better.
- Any revisions to the FC require a new review. The design as-is does not meet the current standards.
3 participants
Added bmp280 and dps310, set the default UART, and added the configuration for pinio1.
Summary by CodeRabbit
New Features
Changes