stm32l4-multi: fix STM32N6 dshot driver to detect non DMA timers#597
stm32l4-multi: fix STM32N6 dshot driver to detect non DMA timers#597winderdoot wants to merge 1 commit into
Conversation
Summary of ChangesHello @winderdoot, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request implements a crucial bug fix for the STM32N6 DSHOT driver by introducing a mechanism to ensure that DSHOT operations are only performed on timers that support Direct Memory Access (DMA). This prevents potential issues arising from attempting to use non-DMA compatible timers, thereby enhancing the robustness and reliability of the DSHOT functionality within the system. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a safeguard in the STM32N6 DSHOT driver to prevent the use of timers that are not compatible with DMA. This is achieved by defining a PWM_TIM_DMA bitmask for DMA-capable timers and adding a validation check in pwm_setBitSequence. The change is logical and correctly placed to fail early. My review includes a suggestion to improve code style for better readability and maintainability by adhering to common line length conventions.
| #define PWM_TIM_GP2 ((1 << pwm_tim9) | (1 << pwm_tim12) | (1 << pwm_tim15)) | ||
| #define PWM_TIM_NO_MASTER_SLAVE ((1 << pwm_tim10) | (1 << pwm_tim11) | (1 << pwm_tim13) | (1 << pwm_tim14) | (1 << pwm_tim16) | (1 << pwm_tim17)) | ||
| #define PWM_TIM_32BIT ((1 << pwm_tim2) | (1 << pwm_tim3) | (1 << pwm_tim4) | (1 << pwm_tim5)) | ||
| #define PWM_TIM_DMA ((1 << pwm_tim1) | (1 << pwm_tim2) | (1 << pwm_tim3) | (1 << pwm_tim4) | (1 << pwm_tim5) | (1 << pwm_tim8) | (1 << pwm_tim15) | (1 << pwm_tim16) | (1 << pwm_tim17)) |
There was a problem hiding this comment.
This line is very long and exceeds common style guide recommendations for line length (e.g., 80 or 100 characters), which can make the code harder to read and maintain. Consider splitting it across multiple lines for improved readability.
| #define PWM_TIM_DMA ((1 << pwm_tim1) | (1 << pwm_tim2) | (1 << pwm_tim3) | (1 << pwm_tim4) | (1 << pwm_tim5) | (1 << pwm_tim8) | (1 << pwm_tim15) | (1 << pwm_tim16) | (1 << pwm_tim17)) | |
| #define PWM_TIM_DMA ((1 << pwm_tim1) | (1 << pwm_tim2) | (1 << pwm_tim3) | (1 << pwm_tim4) | (1 << pwm_tim5) | \ | |
| (1 << pwm_tim8) | (1 << pwm_tim15) | (1 << pwm_tim16) | (1 << pwm_tim17)) |
1 participant
JIRA: RTOS-1102
Small fix to include guarding against using a non DMA compatible TIMx peripheral for DSHOT.
Description
Motivation and Context
Types of changes
How Has This Been Tested?
Checklist:
Special treatment