Add method to allow the GPT Timer period not to be buffered. #131
Conversation
This file contains 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
During code review of changes I made to the Servo library: arduino-libraries/Servo#116 @iabdalkader requested that I move all of the platform specific timer changes into the core instead of being in the hardware specific sub-directory of the Servo library. This change adds a method to tell the GPT timer, that when I make a change to the pseriod (set_period(p) ) that I want the change to happen now and not be buffered. And updated the set_period method to check for this and directly set the not buffered register with the new period.
KurtE
added a commit
to KurtE/Arduino_Servo
that referenced
this pull request
Sep 10, 2023
I moved the code that disables the period buffer into a new member function. I then checked for this in the set_period(p) call, which will either allow it to go to FSP code, or directly set the appropriate register with the new period. arduino/ArduinoCore-renesas#131
per1234
added
type: enhancement
iabdalkader
reviewed
Sep 12, 2023
iabdalkader
reviewed
Sep 12, 2023
cores/arduino/FspTimer.h
Outdated
| @@ -111,6 +112,8 @@ class FspTimer { | |||
| bool reset(); | |||
| bool set_duty_cycle(uint32_t const duty_cycle_counts, TimerPWMChannel_t pwm_ch); | |||
| bool set_period(uint32_t p); | |||
| bool use_period_buffer(bool buffer_period); | |||
|
|
|||
iabdalkader
reviewed
Sep 12, 2023
cores/arduino/FspTimer.cpp
Outdated
| @@ -470,6 +476,37 @@ bool FspTimer::set_period(uint32_t p) { | |||
|
|
|||
|
|
|||
|
|
|||
| /* -------------------------------------------------------------------------- */ | |||
| bool FspTimer::use_period_buffer(bool buffer_period) { | |||
There was a problem hiding this comment.
I think this should be called set_period_buffer just to match other functions.
Suggested change
| bool FspTimer::use_period_buffer(bool buffer_period) { | |
| bool FspTimer::set_period_buffer(bool period_buffer) { |
Plus - changes then required to make things match up. Note: in the header file, I personally believe there should be a blank line above every public API, plus a comment line or lines describing each of the public APIs, such that they are picked up by context sensitive help. But that is beyond scope of this PR
Fix typo Co-authored-by: Ibrahim Abdelkader <[email protected]>
iabdalkader
approved these changes
Sep 13, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
During code review of changes I made to the Servo library: arduino-libraries/Servo#116
@iabdalkader requested that I move all of the platform specific timer changes into the core instead of being in the hardware specific sub-directory of the Servo library.
This change adds a method to tell the GPT timer, that when I make a change to the pseriod (set_period(p) ) that I want the change to happen now and not be buffered.
And updated the set_period method to check for this and directly set the not buffered register with the new period.