Fixed setting HardwareTimer interrupt priority#1062
Merged
fpistm merged 2 commits intostm32duino:masterfrom May 13, 2020
Merged
Fixed setting HardwareTimer interrupt priority#1062fpistm merged 2 commits intostm32duino:masterfrom
fpistm merged 2 commits intostm32duino:masterfrom
Conversation
HardwareTimer::setInterruptPriority previously stored the priority fur future use, but it was typically never applied. It was applied only during the call to HAL_TIM_Base_Init in the HardwareTimer constructor. This change sets the priority immediately, in addition to storing it for future use by the HAL.
ABOSTM
suggested changes
May 11, 2020
Member
|
@sjasonsmith |
Contributor
Author
I intend to come back to it, but this is hobby work for me and I am back to my day job at the moment. Let me know if you were planning to make a new release or anything that would require me to rush this change in. Otherwise I expect to update it within the next couple days. |
Member
|
I'm currently preparing the next release and hope to release it this week or at least next week. That's why I ask. 😉 |
Contributor
Author
|
I implemented the requested change to update the Capture/Compare interrupt. I'm only working with Update interrupts in Marlin, so I didn't have a way to test that change. This is still working as expected for the Update interrupt. |
ABOSTM
approved these changes
May 13, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
HardwareTimer interrupt priority is currently set in the constructor inside the call to
HAL_TIM_Base_Init. Setting the priority usingHardwareTimer::setInterruptPrioritystored the priority, but it was only applied if the timer was re-initialized. This would be unlikely to ever occur for typical HardwareTimer users.This change applies the interrupt priority immediately in addition to storing it, so that it impacts already initialized timers.
The motivation for this change is to allow Marlin firmware to utilize the latest version of this framework, without having to maintain workarounds to ensure proper interrupt priorities.
Validation
I verified this fix by reworking Marlin to build with the latest code from this repo. With this fix I am able to delete Marlin's HardwareTimer priority workarounds and custom copy of SoftwareSerial.
Without this change SoftwareSerial bit timing was inconsistent due to conflicting interrupts, causing communication errors.