Skip to content

Conversation

@haslinghuis
Copy link
Member

@haslinghuis haslinghuis commented Oct 24, 2025

Summary by CodeRabbit

  • Bug Fixes
    • Corrected gyroscope alignment on the MATEKH743 board to improve orientation and gyro accuracy.
    • Updated timer pin mappings to restore proper timer output behavior for affected pins (improving PWM and timer-driven peripherals).
    • Adjusted ADC and timer DMA configuration to stabilize ADC transfers and timer update handling across the board.

@haslinghuis haslinghuis self-assigned this Oct 24, 2025
@haslinghuis haslinghuis added the Bugfix Fixes a problem label Oct 24, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 24, 2025

Walkthrough

Updated the Matek H743 board configuration: adjusted multiple timer pin mapping entries and DMA option macros, and changed the secondary gyro alignment from CW0_DEG_FLIP to CW90_DEG_FLIP in configs/MATEKH743/config.h.

Changes

Cohort / File(s) Change Summary
Timer pin mapping & DMA option updates
configs/MATEKH743/config.h
Modified TIMER_PIN_MAPPING entries for indices 1–7 and 10 (last parameter values updated from 0 → 1..8). Adjusted ADC DMA options (ADC1_DMA_OPT 8→9, ADC3_DMA_OPT 9→10). Removed several old TIMUP*_DMA_OPT definitions and added TIMUP3_DMA_OPT, TIMUP4_DMA_OPT, TIMUP5_DMA_OPT with values 11, 12, 13.
Secondary gyro alignment
configs/MATEKH743/config.h
Changed #define GYRO_2_ALIGN CW0_DEG_FLIP#define GYRO_2_ALIGN CW90_DEG_FLIP.

Sequence Diagram(s)

(omitted — changes are configuration updates without control-flow or runtime sequence modifications)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

  • #14730: Dual-gyro orientation problem on Matek H743 Slim V3 — this PR changes GYRO_2_ALIGN which directly targets the reported incorrect secondary gyro orientation.

Possibly related PRs

Suggested reviewers

  • blckmn
  • nerdCopter

Pre-merge checks and finishing touches

❌ Failed checks (3 warnings)
Check name Status Explanation Resolution
Out of Scope Changes Check ⚠️ Warning The pull request includes substantial changes beyond the gyro alignment fix, including updates to TIMER_PIN_MAPPING entries and ADC/TIM DMA options (ADC1_DMA_OPT, ADC3_DMA_OPT, and multiple TIMUP_* macros). These configuration changes are not mentioned in or directly related to the linked issue #14730, which specifically addresses dual gyro orientation problems. Without explanation of why these additional changes are necessary or which issue they address, they appear to be scope creep or should be submitted in separate pull requests.
Description Check ⚠️ Warning The pull request description is substantially incomplete compared to the repository template. The description consists only of a single line referencing the linked issue, whereas the template requires detailed sections on mandatory review processes, hardware compliance requirements, and a comprehensive checklist for flight controller changes. Even for configuration-only updates, a more thorough explanation of what is being changed and why would be expected to help reviewers understand the fix. Expand the pull request description to include an explanation of the fix (e.g., why changing GYRO_2_ALIGN from CW0_DEG_FLIP to CW90_DEG_FLIP resolves the gyro orientation issue), and provide context for any other configuration changes. If this is a configuration-only fix rather than a new flight controller submission, note that explicitly and follow the appropriate guidelines for that type of contribution.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title "MATEKH743 - Change GYRO_2_ALIGN to CW90_DEG_FLIP" accurately describes a significant change within the pull request that directly addresses the primary objective from the linked issue #14730, which reports gyro orientation problems. The title is concise and specific, clearly identifying both the board and the key configuration change. While the raw summary indicates additional changes to TIMER_PIN_MAPPING and DMA options, the title appropriately focuses on the main fix for the reported bug rather than attempting to enumerate all changes.
Linked Issues Check ✅ Passed The primary objective from linked issue #14730 is to resolve dual gyro orientation problems on the Matek H743 Slim V3, where enabling gyro 2 or both gyros causes incorrect orientation and unstable flight. The GYRO_2_ALIGN change from CW0_DEG_FLIP to CW90_DEG_FLIP directly addresses this root cause by correcting the expected orientation of the second gyro sensor, which should resolve the reported behavior.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch haslinghuis-patch-3

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3750686 and 452f168.

📒 Files selected for processing (1)
  • configs/MATEKH743/config.h (2 hunks)
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: haslinghuis
PR: betaflight/config#0
File: :0-0
Timestamp: 2025-06-02T16:16:20.197Z
Learning: In STM32F405-based flight controller configurations, timer pin mappings must be carefully analyzed for DMA stream conflicts, especially between motor timers, LED strip timers, and SPI DMA assignments. Changes to TIMER_PIN_MAPPING should include verification that all required peripherals (motors, LED strip, blackbox SPI) can coexist without DMA conflicts.
Learnt from: haslinghuis
PR: betaflight/config#792
File: configs/FLYWOOF411/config.h:33-33
Timestamp: 2025-05-28T15:48:18.449Z
Learning: The FLYWOOF411 board does not actually use dual gyros despite having GYRO_2_SPI_INSTANCE defined in its configuration. It was incorrectly targeted for GYRO_COUNT and dual gyro support.
📚 Learning: 2025-06-02T16:16:20.197Z
Learnt from: haslinghuis
PR: betaflight/config#0
File: :0-0
Timestamp: 2025-06-02T16:16:20.197Z
Learning: In STM32F405-based flight controller configurations, timer pin mappings must be carefully analyzed for DMA stream conflicts, especially between motor timers, LED strip timers, and SPI DMA assignments. Changes to TIMER_PIN_MAPPING should include verification that all required peripherals (motors, LED strip, blackbox SPI) can coexist without DMA conflicts.

Applied to files:

  • configs/MATEKH743/config.h
📚 Learning: 2025-05-31T17:01:39.423Z
Learnt from: haslinghuis
PR: betaflight/config#798
File: configs/FURYF4OSD/config.h:88-88
Timestamp: 2025-05-31T17:01:39.423Z
Learning: For STM32F4xx platforms, PB1 pin timer definitions are:
1 = DEF_TIM(TIM1, CH3N, PB1, 0, 0)
2 = DEF_TIM(TIM3, CH4, PB1, 0, 0) 
3 = DEF_TIM(TIM8, CH3N, PB1, 0, 0)
Timer instance 2 (TIM3, CH4) is the correct mapping for PB1 motor control on STM32F4xx platforms as defined in src/platform/STM32/timer_stm32f4xx.c

Applied to files:

  • configs/MATEKH743/config.h
📚 Learning: 2025-07-14T15:41:14.364Z
Learnt from: ot0tot
PR: betaflight/config#834
File: configs/RADIOLINKF405/config.h:79-88
Timestamp: 2025-07-14T15:41:14.364Z
Learning: In STM32F405 configurations, PB1 typically maps to TIM3_CH4 and PC9 typically maps to TIM8_CH4. These are different timers and do not share DMA resources, so there is no conflict when both pins are used simultaneously (e.g., PB1 for LED_STRIP and PC9 for MOTOR4).

Applied to files:

  • configs/MATEKH743/config.h
📚 Learning: 2025-08-28T20:45:36.652Z
Learnt from: haslinghuis
PR: betaflight/config#888
File: configs/ZEX_ATHENA_MINI/config.h:0-0
Timestamp: 2025-08-28T20:45:36.652Z
Learning: In STM32H743 Betaflight configurations, PB0 can use multiple timer options including TIM1_CH2N, TIM3_CH3, and TIM8_CH2N as defined in timer_stm32h7xx.c. For CAMERA_CONTROL_PIN assignments, TIM3_CH3 (timer index 3) is the appropriate choice.

Applied to files:

  • configs/MATEKH743/config.h
📚 Learning: 2025-08-28T20:45:36.652Z
Learnt from: haslinghuis
PR: betaflight/config#888
File: configs/ZEX_ATHENA_MINI/config.h:0-0
Timestamp: 2025-08-28T20:45:36.652Z
Learning: In STM32H743 Betaflight configurations, PA15 maps to TIM2_CH1 (timer index 2) as defined in timer_stm32h7xx.c. When used for GYRO_CLKIN functionality, it's recommended to use the macro name GYRO_1_CLKIN_PIN in TIMER_PIN_MAPPING for consistency with other pin definitions.

Applied to files:

  • configs/MATEKH743/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/MATEKH743/config.h
📚 Learning: 2025-06-08T22:02:28.961Z
Learnt from: haslinghuis
PR: betaflight/config#814
File: configs/JHEF405PRO/config.h:109-109
Timestamp: 2025-06-08T22:02:28.961Z
Learning: The JHEF405PRO board does not actually use dual gyros despite having GYRO_2_SPI_INSTANCE defined in its configuration. It should not receive GYRO_COUNT or other dual-gyro related definitions since only the first gyro is supported.

Applied to files:

  • configs/MATEKH743/config.h
📚 Learning: 2025-07-14T16:16:50.628Z
Learnt from: haslinghuis
PR: betaflight/config#835
File: configs/HDZERO_HALO_MPU6000/config.h:29-35
Timestamp: 2025-07-14T16:16:50.628Z
Learning: CW0_DEG is not a universal default gyro alignment setting in Betaflight configurations. Different boards use different gyro alignments (CW0_DEG, CW90_DEG, CW180_DEG, CW270_DEG, CW0_DEG_FLIP, etc.) based on their physical sensor mounting orientation.

Applied to files:

  • configs/MATEKH743/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/MATEKH743/config.h
📚 Learning: 2025-05-28T15:48:18.449Z
Learnt from: haslinghuis
PR: betaflight/config#792
File: configs/FLYWOOF411/config.h:33-33
Timestamp: 2025-05-28T15:48:18.449Z
Learning: The FLYWOOF411 board does not actually use dual gyros despite having GYRO_2_SPI_INSTANCE defined in its configuration. It was incorrectly targeted for GYRO_COUNT and dual gyro support.

Applied to files:

  • configs/MATEKH743/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.

Applied to files:

  • configs/MATEKH743/config.h
🔇 Additional comments (3)
configs/MATEKH743/config.h (3)

162-162: Gyro 2 alignment fix confirmed; bench-verify for correctness.

Configuration is in place (line 162: #define GYRO_2_ALIGN CW90_DEG_FLIP). Before merge, test:

  • Gyro 1 only: correct axes
  • Gyro 2 only: correct axes with CW90_DEG_FLIP
  • Both gyros: stable attitude, no mid-flight orientation switch

133-137: <!-- [scratchpad_start]
[task verify_dma_opts_across_h743_boards]
Found script results comparing DMA options across board configs.

[observations]

  • MATEKH743 config (lines 133-137): ADC1_DMA_OPT=9, ADC3_DMA_OPT=10, TIMUP3_DMA_OPT=11, TIMUP4_DMA_OPT=12, TIMUP5_DMA_OPT=13
  • SKYSTARSH7HD (same MCU, H743): ADC1_DMA_OPT=9, ADC3_DMA_OPT=10, TIMUP3_DMA_OPT=11, TIMUP4_DMA_OPT=12, TIMUP5_DMA_OPT=13 (IDENTICAL)
  • DARWINH7DUAL (same MCU, H743): ADC1_DMA_OPT=9, ADC3_DMA_OPT=10, TIMUP3_DMA_OPT=11, TIMUP4_DMA_OPT=12, TIMUP5_DMA_OPT=13 (IDENTICAL)
  • Other H743 boards use different patterns (8/9 or 0/variations)
  • Values 9-13 are NOT unique to MATEKH743; they're reused across multiple H743 boards
  • No evidence these values are "untested" - multiple boards already use this exact pattern

[analysis]

  • Original review claim of "unique, untested values" is INCORRECT - these exact values are used on SKYSTARSH7HD and DARWINH7DUAL
  • The values themselves appear to be valid and established across the codebase
  • However, original concern remains valid: Are these DMA changes RELATED to gyro fix or unrelated cleanup?
  • Cannot determine from script output whether these changes are necessary for gyro fix or are separate

[pending]

  • Need to know: Is this PR specifically about gyro fix? If so, why are DMA options being changed?
  • Need to verify: Does PR description/title indicate scope (gyro fix only vs board config updates)?

[actions]

  • Review PR title/description context to determine if DMA changes are in scope
  • Assess if DMA changes are documented as necessary for gyro fix or are cosmetic/cleanup changes

[done]
Verified that DMA option values 9-13 are NOT unique/untested - they appear identically on SKYSTARSH7HD and DARWINH7DUAL.
[scratchpad_end]

DMA option values are not unique; already in use on multiple H743 boards. However, verify if these changes belong in a gyro-fix PR.

The script output reveals that the DMA option values (ADC1_DMA_OPT=9, ADC3_DMA_OPT=10, TIMUP3/4/5_DMA_OPT=11/12/13) are not unique or untested—they are identically defined on SKYSTARSH7HD and DARWINH7DUAL, which use the same STM32H743 MCU. This contradicts the original review's claim.

However, the core concern remains: Are these DMA changes necessary for the gyro fix, or are they unrelated configuration adjustments that should be in a separate PR? Without access to the PR description or PR context, I cannot determine scope.

To complete verification, check whether:

  1. The PR title/description explicitly includes "board config" or "DMA remapping" changes
  2. The gyro fix is documented as requiring these DMA changes
  3. These changes were made to resolve an actual conflict or are cosmetic/organizational

If the DMA changes are not essential to the gyro fix, they should be reverted per the original review guidance.


118-124: Verify scope and intent: Were TIMER_PIN_MAP changes necessary for the gyro fix?

The TIMER_PIN_MAP changes (sequential DMA opt values 0–8) follow a pattern used in multiple other H743 configs (WDIYH743, TJRCH743, KAKUTEH7) and appear to be a standard, working configuration. However, these changes fall outside the stated PR objective (fixing GYRO_2_ALIGN orientation).

Options:

  1. If the timer mapping changes are incidental (e.g., copied from a reference config): revert them to minimize scope.
  2. If intentional and necessary: provide clear justification and evidence that DMA conflicts are resolved (e.g., test results, datasheet analysis).
  3. If in doubt: split into two PRs—one for gyro orientation (low-risk) and one for timer mapping (reviewable separately with rationale).

The sequential DMA pattern itself is not problematic; other boards use it. The issue is whether it's in-scope for this PR.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cspro1
Copy link

cspro1 commented Oct 24, 2025

Thanks! That was extremely fast. Looking forward to the release and will continue to fly gyro 1 till then.

@ot0tot
Copy link
Contributor

ot0tot commented Oct 24, 2025

Also fixed DMA assignments

@haslinghuis haslinghuis merged commit 58914e0 into master Oct 25, 2025
2 checks passed
@haslinghuis haslinghuis deleted the haslinghuis-patch-3 branch October 25, 2025 14:50
@cspro1
Copy link

cspro1 commented Oct 25, 2025

Thanks guys! Looking forward to the new release with these changes. This FC is on a Foxeer Aura 10 build and the extra gyro for noise filtering will be nice to have back. I had noticed in the older BF that it made tuning a little easier with the two than just one enabled. Or, in other words, the older BF flew a little better on stock pids than 2025.12 did yesterday. So Im hoping that having the extra gyro working again will give me a solid platform to start tuning from. Thanks again for the prompt resolve!!

Also...i love the new approach to how yall are handling Betaflight going foward. Excellent work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bugfix Fixes a problem

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BF 2025.12 issue with Matek H743 slim V3 Dual Gyros

5 participants