-
Notifications
You must be signed in to change notification settings - Fork 1k
Bug in SetCCRRegister() #531
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Hi @cdrose |
|
fpistm
added a commit
to fpistm/Arduino_Core_STM32
that referenced
this issue
May 29, 2019
TIM channel values used was confusing and required to map them correctly while there is no added value to do this. Use the HAL definition: TIM_CHANNEL_x will avoid any mismatch. Fix stm32duino#531 Signed-off-by: Frederic.Pillon <[email protected]>
Merged
@fpistm |
@cdrose |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I have been doing some work with CCP timer outputs and there are a few bugs to fix.
In timer.c attachIntHandleOC() case1 for compare channel 1 is correct, but case 2, 3 and 4 have != HAL_OK when the logic should be == HAL_OK, which then causes the call to HAL_TIM_OC_Start_IT() to be skipped and the channel is not enabled.
eg.
There is also a bug in the SetCCRRegister() function in timer.c, the __HAL_TIM_SET_COMPARE is meant to be passed the TIM_CHANNEL_X constants (0x00, 0x04, 0x08, 0x0C) but the conversion from integer 1-4 to these values is incorrect. Just multiplying by 4 results in an off by 1 error (either that or we should pass in integer 0-3 but thats not consistent with other usage in timer.c
I suggest a switch instead to make the intent more clear and avoid the odd conversion function:
I would also assume this fix needs to be applied to getCCRRegister() too though I havent tested that.
The text was updated successfully, but these errors were encountered: