STELLARH7DEV - Add support for IMU 3+ #794
Conversation
WalkthroughThe configuration for IMU gyros in the STELLARH7DEV board was restructured from a conditional, macro-based selection system to a fixed, explicit four-gyro setup. All gyros are now directly assigned sensor types, pins, SPI instances, alignments, and usage macros without conditional preprocessor logic. Changes
Sequence Diagram(s)sequenceDiagram
participant ConfigHeader as config.h
participant Gyro1 as ICM40609D
participant Gyro2 as ICM45605
participant Gyro3 as ICM45686
participant Gyro4 as ICM42688P
ConfigHeader->>Gyro1: Define pins, SPI, alignment, usage macros
ConfigHeader->>Gyro2: Define pins, SPI, alignment, usage macros
ConfigHeader->>Gyro3: Define pins, SPI, alignment, usage macros
ConfigHeader->>Gyro4: Define pins, SPI, alignment, usage macros, CLKIN
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ 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
Documentation and Community
|
![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
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
configs/STELLARH7DEV/config.h(1 hunks)
🔇 Additional comments (2)
configs/STELLARH7DEV/config.h (2)
118-119: Verify the default gyro selection logic.The
DEFAULT_GYRO_ENABLEDis set toGYRO_MASK(3), which enables gyro 3 by default. Typically, gyro 1 would be the primary/default gyro in flight controller configurations. Please confirm this is intentional.Consider if this should be
GYRO_MASK(1)instead to make gyro 1 the default:-#define DEFAULT_GYRO_ENABLED GYRO_MASK(3) +#define DEFAULT_GYRO_ENABLED GYRO_MASK(1)
133-138: LGTM! GYRO_3 configuration is well-structured.The GYRO_3 configuration follows the established pattern and correctly maps to the ICM45686 sensor with appropriate pin assignments and SPI3 instance.
configs/STELLARH7DEV/config.h
Outdated
| #define USE_IMU1 ICM42688P | ||
| #endif | ||
| #define GYRO_COUNT 4 | ||
| #define DEFAULT_GYRO_ENABLED GYRO_MASK(3) |
There was a problem hiding this comment.
Any reason for this default? Why not something like GYRO_MASK(1) | GYRO_MASK(3) fuse the 40609D and the 45686
There was a problem hiding this comment.
Afaik
- 42688P is the recommended gyro to use at the moment.
- current firmware PR in development for this does not allow fusion
- GYRO_MASK is zero based ?
There was a problem hiding this comment.
GYRO_MASK is 0 based, good catch, I had that wrong.
nerdCopter
changed the title
Summary by CodeRabbit
Summary by CodeRabbit