PLTXFxx new configs#1072
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds two new Betaflight board configuration headers for Pilotix boards and registers the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 3
🧹 Nitpick comments (4)
configs/PLTXF435V3BF/config.h (2)
90-90: PA14 (SWCLK) used for PINIO2.Combined with PA13 usage, SWD debugging will be completely disabled on this board.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@configs/PLTXF435V3BF/config.h` at line 90, PINIO2_PIN is currently defined as PA14 which is SWCLK and together with PA13 (SWDIO) will disable SWD debugging; change PINIO2_PIN in config.h to a non-SWD GPIO (pick an available port/pin that is not PA13/PA14) or make the PINIO2_PIN definition configurable via a board option (e.g., `#ifdef`) so SWD pins remain free; update any references to PINIO2_PIN (e.g., initialization in the board setup code) to match the new pin name and ensure no code assumes PA14.
73-73: PA13 (SWDIO) used for MAX7456 chip select.Same concern as the F405 target — standard SWD debugging will be unavailable. Confirm alternate debug/recovery methods exist.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@configs/PLTXF435V3BF/config.h` at line 73, MAX7456_SPI_CS_PIN is set to PA13 (SWDIO) which disables standard SWD debugging; change MAX7456_SPI_CS_PIN to a non-SWD GPIO (e.g., any free PAx/PBx/PCx that is not PA13/PA14) or move the MAX7456 CS to a spare board pin, update the define in config.h from PA13 accordingly, and add a comment documenting the new pin choice and any alternate debug/recovery method (bootloader pins or UART recovery) if you must keep PA13.configs/PLTXF405V3BF/config.h (2)
73-73: PA13 (SWDIO) used for MAX7456 chip select.PA13 is the SWDIO debug pin. Using it for
MAX7456_SPI_CS_PINwill disable standard SWD debugging. Ensure the hardware provides alternate debug access (e.g., SWO or boot-pin recovery) or this is an intentional design trade-off.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@configs/PLTXF405V3BF/config.h` at line 73, MAX7456_SPI_CS_PIN is defined as PA13 which is the SWDIO debug pin; update the configuration to avoid disabling SWD: either pick a non-SWD GPIO for MAX7456_SPI_CS_PIN (replace the macro value with an alternate free pin), or explicitly document the trade-off by adding a clear comment next to the `#define` (and ensure hardware provides alternate debug like SWO/boot-pin recovery). Refer to the MAX7456_SPI_CS_PIN macro in config.h when making the change.
90-90: PA14 (SWCLK) used for PINIO2.PA14 is the SWCLK debug pin. Combined with PA13 usage, standard SWD debugging will be completely unavailable. Confirm this is intentional and alternate debug/recovery methods exist.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@configs/PLTXF405V3BF/config.h` at line 90, PINIO2_PIN is currently defined as PA14 which is the SWCLK debug pin; using PA14 together with PA13 disables standard SWD debugging—either choose a non-SWD GPIO for PINIO2 (change the `#define` PINIO2_PIN from PA14 to an alternate available pin) or explicitly document/implement an alternate recovery method (e.g., bootloader entry pin, USB DFU, or isolate SWD lines) and add a clear comment in configs/PLTXF405V3BF/config.h near PINIO2_PIN explaining the decision; update any board init code that references PINIO2_PIN accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@configs/PLTXF405V3BF/config.h`:
- Around line 26-27: BOARD_NAME in configs/PLTXF405V3BF/config.h is incorrect
(defined as PILOTIXF405V3BF) and must exactly match the directory name; update
the BOARD_NAME macro to PLTXF405V3BF in config.h, and also add/register the
MANUFACTURER_ID PLTX in the Manufacturers.md registry (ensure the PLTX entry
follows the existing format for other manufacturers) so MANUFACTURER_ID PLTX is
accepted.
In `@configs/PLTXF435V3BF/config.h`:
- Around line 26-27: The BOARD_NAME macro is incorrect — update the BOARD_NAME
define from PILOTIXF435V3BF to exactly match the directory name PLTXF435V3BF
(modify the BOARD_NAME symbol in config.h), and ensure MANUFACTURER_ID PLTX is
registered in Manufacturers.md (add an entry for PLTX if missing) so the
manufacturer ID aligns with the config's MANUFACTURER_ID symbol.
- Line 108: The BOARD_NAME macro is incorrect: change the definition of
BOARD_NAME (currently set to PILOTIXF435V3BF) to match the directory name
PLTXF435V3BF; leave ADC1_DMA_OPT (value 11) as-is since it is correct for
AT32F435 boards. Update the BOARD_NAME macro only (search for the symbol
BOARD_NAME in this file and replace its value with "PLTXF435V3BF") and run a
quick build or linter to ensure no other references expect the old name.
---
Nitpick comments:
In `@configs/PLTXF405V3BF/config.h`:
- Line 73: MAX7456_SPI_CS_PIN is defined as PA13 which is the SWDIO debug pin;
update the configuration to avoid disabling SWD: either pick a non-SWD GPIO for
MAX7456_SPI_CS_PIN (replace the macro value with an alternate free pin), or
explicitly document the trade-off by adding a clear comment next to the `#define`
(and ensure hardware provides alternate debug like SWO/boot-pin recovery). Refer
to the MAX7456_SPI_CS_PIN macro in config.h when making the change.
- Line 90: PINIO2_PIN is currently defined as PA14 which is the SWCLK debug pin;
using PA14 together with PA13 disables standard SWD debugging—either choose a
non-SWD GPIO for PINIO2 (change the `#define` PINIO2_PIN from PA14 to an alternate
available pin) or explicitly document/implement an alternate recovery method
(e.g., bootloader entry pin, USB DFU, or isolate SWD lines) and add a clear
comment in configs/PLTXF405V3BF/config.h near PINIO2_PIN explaining the
decision; update any board init code that references PINIO2_PIN accordingly.
In `@configs/PLTXF435V3BF/config.h`:
- Line 90: PINIO2_PIN is currently defined as PA14 which is SWCLK and together
with PA13 (SWDIO) will disable SWD debugging; change PINIO2_PIN in config.h to a
non-SWD GPIO (pick an available port/pin that is not PA13/PA14) or make the
PINIO2_PIN definition configurable via a board option (e.g., `#ifdef`) so SWD pins
remain free; update any references to PINIO2_PIN (e.g., initialization in the
board setup code) to match the new pin name and ensure no code assumes PA14.
- Line 73: MAX7456_SPI_CS_PIN is set to PA13 (SWDIO) which disables standard SWD
debugging; change MAX7456_SPI_CS_PIN to a non-SWD GPIO (e.g., any free
PAx/PBx/PCx that is not PA13/PA14) or move the MAX7456 CS to a spare board pin,
update the define in config.h from PA13 accordingly, and add a comment
documenting the new pin choice and any alternate debug/recovery method
(bootloader pins or UART recovery) if you must keep PA13.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e92cbcbe-30f3-4f48-809e-cacdaedba6b0
📒 Files selected for processing (2)
configs/PLTXF405V3BF/config.hconfigs/PLTXF435V3BF/config.h
| TIMER_PIN_MAP( 9, GYRO_1_CLKIN_PIN, 2, -1) | ||
|
|
||
| #define ADC_INSTANCE ADC1 | ||
| #define ADC1_DMA_OPT 11 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check ADC1_DMA_OPT values used by other AT32F435 boards
echo "=== ADC DMA options in AT32F435 configs ==="
rg -n "ADC.*DMA_OPT" configs/ | rg -i "f435"Repository: betaflight/config
Length of output: 1801
🏁 Script executed:
#!/bin/bash
# Verify BOARD_NAME matches directory and check config context
echo "=== BOARD_NAME in PLTXF435V3BF ==="
grep -n "BOARD_NAME" configs/PLTXF435V3BF/config.h | head -5
# Check first 120 lines to see ADC config context
echo ""
echo "=== Config file structure (lines 1-50) ==="
head -50 configs/PLTXF435V3BF/config.h | cat -n
# Verify ADC1_DMA_OPT context around line 108
echo ""
echo "=== Context around ADC1_DMA_OPT (lines 100-115) ==="
sed -n '100,115p' configs/PLTXF435V3BF/config.h | cat -nRepository: betaflight/config
Length of output: 2644
Fix BOARD_NAME mismatch.
The directory is PLTXF435V3BF but BOARD_NAME is defined as PILOTIXF435V3BF (line 26). The board name must match the directory name. Additionally, regarding ADC1_DMA_OPT = 11: this value is standard for AT32F435 boards (15 out of 24 boards use this value), so no issue exists there.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@configs/PLTXF435V3BF/config.h` at line 108, The BOARD_NAME macro is
incorrect: change the definition of BOARD_NAME (currently set to
PILOTIXF435V3BF) to match the directory name PLTXF435V3BF; leave ADC1_DMA_OPT
(value 11) as-is since it is correct for AT32F435 boards. Update the BOARD_NAME
macro only (search for the symbol BOARD_NAME in this file and replace its value
with "PLTXF435V3BF") and run a quick build or linter to ensure no other
references expect the old name.
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@configs/PILOTIXF405V3BF/config.h`:
- Around line 95-105: TIMER_PIN_MAPPING currently assigns the same DMA index (0)
to MOTOR1_PIN (TIM3 CH3) and MOTOR2_PIN (TIM3 CH4) which conflicts on STM32F405;
update the TIMER_PIN_MAP entry for MOTOR2_PIN so its DMA index is changed from 0
to a different index (e.g., 2) to avoid sharing DMA with MOTOR1_PIN, leaving all
other TIMER_PIN_MAP entries (MOTOR1_PIN, MOTOR3_PIN, MOTOR4_PIN, SERVO1_PIN,
SERVO2_PIN, LED_STRIP_PIN, BEEPER_PIN, RX_PPM_PIN, GYRO_1_CLKIN_PIN) unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2d2e5c4c-b6db-49c1-9fce-239d9b704757
📒 Files selected for processing (2)
configs/PILOTIXF405V3BF/config.hconfigs/PILOTIXF435V3BF/config.h
| #define PINIO1_CONFIG 129 | ||
| #define PINIO2_CONFIG 129 | ||
| #define PINIO1_BOX 40 | ||
| #define PINIO2_BOX 41 | ||
| #define PINIO3_BOX 42 |
There was a problem hiding this comment.
Recommend adding names for the PINIOs, e.g.:
| #define PINIO1_CONFIG 129 | |
| #define PINIO2_CONFIG 129 | |
| #define PINIO1_BOX 40 | |
| #define PINIO2_BOX 41 | |
| #define PINIO3_BOX 42 | |
| #define PINIO1_CONFIG 129 | |
| #define PINIO2_CONFIG 129 | |
| #define PINIO1_BOX 40 | |
| #define PINIO2_BOX 41 | |
| #define PINIO3_BOX 42 | |
| #define BOX_USER1_NAME "VTX PWR" | |
| #define BOX_USER3_NAME "CAM 1,2" |
Not sure what PINIO2 does though.
| #define PINIO2_BOX 41 | ||
| #define PINIO3_BOX 42 | ||
|
|
||
| #define DEFAULT_CURRENT_METER_SCALE 206 |
There was a problem hiding this comment.
Will this FC be part of a stack? As it doesn't have a current meter of its own like on AIO FCs. The DEFAULT_CURRENT_METER_SCALE should only be set on stacks with therefore a known, fixed current meter scale of the accompanied ESC or on an AIO.
There was a problem hiding this comment.
The FC could be sold in both ways: as a solo FC as well as a part of stack. We will remove this default value.
| #define MSP_UART SERIAL_PORT_UART5 | ||
| #define GPS_UART SERIAL_PORT_UART4 | ||
| #define SERIALRX_UART SERIAL_PORT_USART2 |
There was a problem hiding this comment.
Recommend adding guards around the GPS UART, as GPS is user configurable settings and we might as well add the ESC telemetry RX and MSP VTX UART:
| #define MSP_UART SERIAL_PORT_UART5 | |
| #define GPS_UART SERIAL_PORT_UART4 | |
| #define SERIALRX_UART SERIAL_PORT_USART2 | |
| #ifdef USE_OSD_HD | |
| #define MSP_DISPLAYPORT_UART SERIAL_PORT_USART1 | |
| #endif | |
| #define SERIALRX_UART SERIAL_PORT_USART2 | |
| #define ESC_SENSOR_UART SERIAL_PORT_USART3 | |
| #ifdef USE_GPS | |
| #define GPS_UART SERIAL_PORT_UART4 | |
| #endif | |
| #define MSP_UART SERIAL_PORT_UART5 |
And why not order them by UART 🙂
| #define PINIO1_CONFIG 129 | ||
| #define PINIO2_CONFIG 129 | ||
| #define PINIO1_BOX 40 | ||
| #define PINIO2_BOX 41 | ||
| #define PINIO3_BOX 42 |
There was a problem hiding this comment.
See my comment for the F405 board.
|
|
||
| #define MSP_UART SERIAL_PORT_UART5 | ||
| #define GPS_UART SERIAL_PORT_UART4 | ||
| #define SERIALRX_UART SERIAL_PORT_USART2 |
There was a problem hiding this comment.
See my comment for the F405 board.
| #define PINIO2_BOX 41 | ||
| #define PINIO3_BOX 42 | ||
|
|
||
| #define DEFAULT_CURRENT_METER_SCALE 84 |
There was a problem hiding this comment.
See my comment for the F405 board above. Also, this value is completely different than the F405 board?
Co-authored-by: Osiris Inferi <github@flut.nl.eu.org>
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 1
♻️ Duplicate comments (1)
configs/PILOTIXF405V3BF/config.h (1)
27-27:⚠️ Potential issue | 🔴 CriticalAdd
PLTXtoManufacturers.mdbefore this PR can be merged.The
MANUFACTURER_IDmust be registered inManufacturers.mdper repo policy. Currently,PLTXis not present in that file. Add an entry forPLTX(or update it if the PR modifiesManufacturers.mdelsewhere).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@configs/PILOTIXF405V3BF/config.h` at line 27, Add a registry entry for the new manufacturer identifier PLTX in Manufacturers.md to satisfy repo policy; ensure the entry matches the existing document format (identifier "PLTX", human-readable manufacturer name and any required metadata like a numeric ID or contact info if present) so that the `#define` MANUFACTURER_ID PLTX in config.h is backed by a corresponding Manufacturers.md record; follow the exact formatting and ordering used by other entries in Manufacturers.md.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@configs/PILOTIXF405V3BF/config.h`:
- Around line 89-91: PINIO3_PIN and PINIO3_BOX are defined but PINIO3_CONFIG is
missing (PINIO1_CONFIG and PINIO2_CONFIG are set to 129), so either add an
explicit PINIO3_CONFIG with the intended value (e.g., match 129 or the correct
PINIO_CONFIG_* constant for the required idle level) or remove
PINIO3_PIN/PINIO3_BOX if the pin isn't used; do the same fix for the duplicate
occurrence referenced around lines 119-123, and optionally add human-readable
BOX_USER1_NAME/BOX_USER2_NAME entries to label the PINIO boxes in the
Configurator.
---
Duplicate comments:
In `@configs/PILOTIXF405V3BF/config.h`:
- Line 27: Add a registry entry for the new manufacturer identifier PLTX in
Manufacturers.md to satisfy repo policy; ensure the entry matches the existing
document format (identifier "PLTX", human-readable manufacturer name and any
required metadata like a numeric ID or contact info if present) so that the
`#define` MANUFACTURER_ID PLTX in config.h is backed by a corresponding
Manufacturers.md record; follow the exact formatting and ordering used by other
entries in Manufacturers.md.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b9998122-e280-4019-8796-6499f76cc055
📒 Files selected for processing (1)
configs/PILOTIXF405V3BF/config.h
Co-authored-by: Osiris Inferi <github@flut.nl.eu.org>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
configs/PILOTIXF435V3BF/config.h (1)
93-103:⚠️ Potential issue | 🔴 CriticalCritical:
TIMER_PIN_MAPPINGis missing for the F435 board.This config defines
MOTOR1_PIN–MOTOR4_PIN,SERVO1_PIN/SERVO2_PIN,LED_STRIP_PIN,BEEPER_PIN,RX_PPM_PIN, andGYRO_1_CLKIN_PIN, but there is noTIMER_PIN_MAPPINGblock anywhere in the file. Without it, none of these timer-driven outputs (DShot motors, servo PWM, WS2812 LED strip, beeper PWM, PPM RX, gyro clock-in) will be wired to a timer/DMA, and the board will not be usable.A previous review round already provided a concrete mapping that accounts for PB8/PB9 having different timer occurrences on AT32F435 than on STM32F405, and uses unique DMA opts (DMAMUX requires that). Please re-add it. Also note that on AT32F435 each DMA-enabled channel must have a unique DMA opt (DMAMUX), so MOTOR1/MOTOR2/MOTOR3/MOTOR4/LED_STRIP cannot share the same opt index.
🛠️ Proposed addition (adapt if needed)
`#define` ADC_INSTANCE ADC1 `#define` ADC1_DMA_OPT 1 `#define` SYSTEM_HSE_MHZ 8 + +#define TIMER_PIN_MAPPING \ + TIMER_PIN_MAP( 0, MOTOR1_PIN , 2, 0) \ + TIMER_PIN_MAP( 1, MOTOR2_PIN , 2, 1) \ + TIMER_PIN_MAP( 2, MOTOR3_PIN , 1, 2) \ + TIMER_PIN_MAP( 3, MOTOR4_PIN , 1, 3) \ + TIMER_PIN_MAP( 4, SERVO1_PIN, 2, -1) \ + TIMER_PIN_MAP( 5, SERVO2_PIN, 2, -1) \ + TIMER_PIN_MAP( 6, LED_STRIP_PIN , 1, 4) \ + TIMER_PIN_MAP( 7, BEEPER_PIN, 2, -1) \ + TIMER_PIN_MAP( 8, RX_PPM_PIN , 2, -1) \ + TIMER_PIN_MAP( 9, GYRO_1_CLKIN_PIN, 3, -1)Based on learnings: "when GYRO_1_CLKIN_PIN … is defined, it must be included in the TIMER_PIN_MAPPING" and "F435 has a DMA mux, so requires unique DMA opts for all DMA enabled pins" (osirisinferi, PR 1072).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@configs/PILOTIXF435V3BF/config.h` around lines 93 - 103, Add a TIMER_PIN_MAPPING block that wires MOTOR1_PIN–MOTOR4_PIN, SERVO1_PIN/SERVO2_PIN, LED_STRIP_PIN, BEEPER_PIN, RX_PPM_PIN and GYRO_1_CLKIN_PIN to concrete timers/DMA opts for the F435 board (use the same symbol TIMER_PIN_MAPPING), assigning the correct timer instances (e.g. TIM1/TIM2/TIM3/TIM15 or whichever timers are used by the existing pins) and ensure each DMA-enabled entry uses a unique DMA opt value (DMAMUX requires unique opts for each DMA channel); explicitly handle PB8/PB9 timer differences on AT32F435 vs F405 by selecting the timers that exist for those pins on F435; include GYRO_1_CLKIN_PIN in the mapping as required when it is defined.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@configs/PILOTIXF435V3BF/config.h`:
- Around line 93-103: Add a TIMER_PIN_MAPPING block that wires
MOTOR1_PIN–MOTOR4_PIN, SERVO1_PIN/SERVO2_PIN, LED_STRIP_PIN, BEEPER_PIN,
RX_PPM_PIN and GYRO_1_CLKIN_PIN to concrete timers/DMA opts for the F435 board
(use the same symbol TIMER_PIN_MAPPING), assigning the correct timer instances
(e.g. TIM1/TIM2/TIM3/TIM15 or whichever timers are used by the existing pins)
and ensure each DMA-enabled entry uses a unique DMA opt value (DMAMUX requires
unique opts for each DMA channel); explicitly handle PB8/PB9 timer differences
on AT32F435 vs F405 by selecting the timers that exist for those pins on F435;
include GYRO_1_CLKIN_PIN in the mapping as required when it is defined.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7fa1a1c3-faf0-4920-a087-2a994784c2e9
📒 Files selected for processing (3)
Manufacturers.mdconfigs/PILOTIXF405V3BF/config.hconfigs/PILOTIXF435V3BF/config.h
✅ Files skipped from review due to trivial changes (1)
- Manufacturers.md
3 participants
New Pilotix targets:
PLTXF405V3BF and PLTXF435V3BF
Checklist (✓/✕, or y/n)
Summary by CodeRabbit
New Features
Documentation