Improve pwmio documentation #7649
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
adafruit#7644 pointed out the need for better documentation. To the best of my ability I noted the current behavior. I think that there may be some ports that do not actually read back the 'set' frequency value, but they are ports marked as beta status (mimxrt10xx) or not maintained by us (cxd56).
|
There's also a trade-off to be made between frequency precision and duty-cycle precision. Right now we favor duty-cycle precision, but I think it would be acceptable to drop one bit of duty-cycle to attain better frequency precision at low frequencies. Or even make the duty-cycle precision an optional parameter to the API. |
dhalbert
approved these changes
Feb 26, 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.
#7644 pointed out the need for better documentation.
To the best of my ability I noted the current behavior. I think that there may be some ports that do not actually read back the 'set' frequency value, but they are ports marked as beta status (mimxrt10xx) or not maintained by us (cxd56).
This isn't to say that we shouldn't improve the implementation, for instance, to make sure that out-of-range values are handled uniformly across ports. I think there's some subtlety to this; for instance on rp2040 we might want to draw a distinction between a way-too-low frequency (1) and an on-the-cusp frequency (7).
(I think we might also want to change frequency property/parameter to be a float in 9, because many attainable frequencies are not exact integers, and in many cases at the low end the attainable frequencies are within 1Hz of one another.)