Add SKYSTARSF435PRO #750
Add SKYSTARSF435PRO #750
Conversation
Co-authored-by: Mark Haslinghuis <[email protected]>
WalkthroughA new configuration header file for the SKYSTARSF435PRO flight controller board has been introduced. This file defines hardware-specific macros, pin assignments, sensor and peripheral enablement, timer mappings, DMA and I2C instances, and default system settings required for the board's operation. Changes
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure ✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
configs/SKYSTARSF435PRO/config.h (3)
40-49: Clarify shared RX/LED strip pin
TheRX_PPM_PINonPA8is commented out in favor ofLED_STRIP_PINon the same pad. Consider adding a comment above to explain this repurpose for future clarity.
77-85: Review ADC, PINIO, and CS/EXTI pin usage
- Verify
ADC_VBAT_PIN(PC2) andADC_CURR_PIN(PC1) are wired to the proper voltage divider/current-sense circuits.- Ensure
PINIO1_PIN(PC0) andPINIO2_PIN(PC5) are uncommitted GPIOs, and theirBOX_USER?_NAMElabels match the silkscreen.- Confirm
GYRO_1_EXTI_PIN = PC4supports EXTI and doesn’t clash with other interrupt lines.
102-108: Reduce magic numbers in PINIO configuration
PINIO1_CONFIGandPINIO2_CONFIGuse raw values (1,129). Consider defining named constants or bitfield macros for clarity, e.g.:#define PINIO_INPUT_PULLDOWN 1 #define PINIO_OUTPUT_OPENDRAIN 129
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
configs/SKYSTARSF435PRO/config.h(1 hunks)
🔇 Additional comments (7)
configs/SKYSTARSF435PRO/config.h (7)
22-28: Verify target MCU, board name, and manufacturer ID
Ensure thatAT32F435Mis indeed the MCU used on this board and that"SKST"is a unique, 4-character manufacturer ID that doesn’t collide with existing entries.
29-38: Confirm sensor and peripheral availability
You’ve enabled bothUSE_BARO_BMP280andUSE_BARO_DPS310—please verify that both barometer sensors are physically present and intended for simultaneous operation. Similarly, confirm that the ICM42688P is wired for SPI so thatUSE_ACC_SPI_ICM42688PandUSE_GYRO_SPI_ICM42688Pare correct.
50-62: Verify UART5_RX pin availability
UART5_RXis assigned toPD2. Please confirm that Port D Pin 2 is accessible on the SKYSTARSF435PRO layout and not used by another peripheral (e.g., I²C or sensor EXTI).
63-66: Validate I2C2 pin mapping on Port H
I²C2 is mapped toPH2/PH3. Ensure that Port H Pins 2 and 3 are exposed on your chosen package and not reserved by boot mode straps or external components.
67-76: Confirm LED0 and SPI pin assignments
LED0_PINis onPC3—verify it doesn’t conflict with ADC or EXTI.- SPI1/2/3 SCK, SDI, SDO pins look standard; please double-check the alternate-function (AF) mapping on AT32F435M to ensure each pin is set to the correct AF number.
97-101: Approve DMA and I²C instance selections
ADC1_DMA_OPT= 8 matches the default ADC1 DMA stream.BARO_I2C_INSTANCE= I2CDEV_2 andMAG_I2C_INSTANCE= I2CDEV_1 align with the pin mappings above.
110-119: Verify default system and peripheral settings
The defaults (flash blackbox, DSHOT bitbang on, ADC meters, inverted beeper, 8 MHz HSE, SPI instances,CW270_DEGgyro alignment) appear consistent. Please validate that these align with the hardware wiring and board documentation.
| #define TIMER_PIN_MAPPING TIMER_PIN_MAP( 0, PB0, 2, 1 ) \ | ||
| TIMER_PIN_MAP( 1, PB1, 2, 2 ) \ | ||
| TIMER_PIN_MAP( 2, PC8, 2, 3 ) \ | ||
| TIMER_PIN_MAP( 3, PC9, 2, 4 ) \ | ||
| TIMER_PIN_MAP( 4, PB6, 1, 5 ) \ | ||
| TIMER_PIN_MAP( 5, PB7, 1, 6 ) \ | ||
| TIMER_PIN_MAP( 6, PB10, 1, -1 ) \ | ||
| TIMER_PIN_MAP( 7, PB11, 1, -1 ) \ | ||
| TIMER_PIN_MAP( 8, PA8, 1, 7 ) | ||
|
|
There was a problem hiding this comment.
Critical: Invalid timer channel mapping for servo pins
Your TIMER_PIN_MAPPING entries for indices 6 and 7 map PB10/PB11 to timer 1 with channel -1. A channel of -1 disables PWM on those pins. Please correct these to the appropriate timer channel numbers so servo outputs will function.
🤖 Prompt for AI Agents
In configs/SKYSTARSF435PRO/config.h around lines 86 to 95, the TIMER_PIN_MAPPING
macro incorrectly assigns timer channel -1 to pins PB10 and PB11 at indices 6
and 7, which disables PWM output. Update these entries to use the correct timer
channel numbers instead of -1 to enable proper servo PWM functionality on those
pins.
There was a problem hiding this comment.
@coderabbitai the last element in TIMER_PIN_MAP definition represents DMA which should be disabled for SERVO pins.
There was a problem hiding this comment.
🧩 Analysis chain
@haslinghuis Thank you for the correction! You're absolutely right - the last element in the TIMER_PIN_MAP definition represents the DMA channel, not the timer channel as I incorrectly understood. Having -1 in this position properly disables DMA for servo pins, which is the intended configuration since servos don't require DMA for their operation.
I appreciate you taking the time to clarify this. This is the correct implementation for servo pins.
(_/)
(•ㅅ•)
/ >📝 Noted for future reviews!
🏁 Script executed:
#!/bin/bash
# Look for other instances of SERVO pins in TIMER_PIN_MAP definitions to see if they also use -1 for DMA
echo "Searching for other SERVO pin configurations in TIMER_PIN_MAP..."
rg "SERVO.*_PIN" -A 20 | rg "TIMER_PIN_MAP.*-1" -B 5 | head -n 20
# Look for any documentation on TIMER_PIN_MAP macro
echo "Searching for TIMER_PIN_MAP documentation or definition..."
rg "TIMER_PIN_MAP.*\(" --type c --type h | head -n 20
# Check a few other target config files for similar patterns
echo "Checking patterns in other config files..."
find configs -name "config.h" | sort | head -n 5 | xargs grep -A 10 "TIMER_PIN_MAPPING" | head -n 40
Length of output: 5075
@haslinghuis Thank you for the correction! You're absolutely right - the last element in the TIMER_PIN_MAP definition represents the DMA channel, not the timer channel as I incorrectly understood. Having -1 in this position properly disables DMA for servo pins, which is the intended configuration since servos don't require DMA for their operation.
I appreciate you taking the time to clarify this. The search results confirm that other target configurations also use this same pattern for servo pins, and this is indeed the correct implementation.
(_/)
(•ㅅ•)
/ >📝 Noted for future reviews!
Summary by CodeRabbit