forked from ARMmbed/mbed-os
-
Notifications
You must be signed in to change notification settings - Fork 18
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
Increase RPi Pico PWM range and resolution to the max supported by HW #203
Merged
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
… +1 to top_count, fixes missing pwm_config_set_wrap() call.
multiplemonomials
force-pushed
the
dev/rpi-pico-full-pwm-support
branch
from
January 15, 2024 09:03
a59db58
to
1494794
Compare
multiplemonomials
changed the title
[draft] Attempt to increase RPi Pico PWM range and resolution to the max supported
Attempt to increase RPi Pico PWM range and resolution to the max supported
Jan 15, 2024
multiplemonomials
changed the title
Attempt to increase RPi Pico PWM range and resolution to the max supported
Increase RPi Pico PWM range and resolution to the max supported by HW
Jan 15, 2024
JohnK1987
reviewed
Jan 15, 2024
JohnK1987
reviewed
Jan 15, 2024
JohnK1987
approved these changes
Jan 16, 2024
Do you know when platformIO will update their mbed version with this patch ? It doesn't work for now with the mbed version provided with platformIO. Thanks |
Sadly I don't think PlatformIO supports the mbed-ce fork :/ |
Thanks, i posted on platformIO issues, we will see. |
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.
Summary of changes
@Olivier_Corrio on the Mbed forums reported an issue with the PWM functionality on RPi Pico. After some investigation, I confirmed the issue: The initial implementation of PWM always uses a top_count of 1000 and only varies the divider to set the PWM period. I think they did it this way to avoid floating point math, but it is sub-optimalfor three reasons:
This PR redesigns the math that sets the PWM period so that both the divider and the top_count are calculated dynamically in order to match the requested period as closely as possible. It does mean that setting the PWM period will be a tad more performance intensive, but this isn't an operation done commonly at runtime, and the benefits to range and accuracy are significant, so I think we should be OK!
Bonus: I also fixed a bug where the RPi Pico HAL was returning values in the range [0, 3.3] instead of [0, 1] from the ADC. This means that AnalogIn::read() now matches the spec behavior.
Impact of changes
Improved range and accuracy of PWM for RPi Pico
Migration actions required
None
Documentation
None
Pull request type
Test results
I ran my new (create just for this!) PWM and ADC test shield test on the new implementation, and it passed!
Reviewers