-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Split pulseio.PWMOut into pwmio #3299
Conversation
This gives us better granularity when implementing new ports because PWMOut is commonly implemented before PulseIn and PulseOut. Fixes micropython#3211
@dhalbert We should be able to do beta releases after this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like CI is concerned about stubs, not sure what that's about. I found a couple of mis-labeled comments but it looks good to me overall - you covered everything in Display and and the config settings that I can think of. Either this or ESP32-PulseIO will need some tweaks depending on which goes in first, of course.
Ok, everything is passing now. Please take another look. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have one very minor question/comment about include style but it looks good to me. Thanks for putting this together.
Looks good, thanks! |
Upstream in CircuitPython, they moved PWM functionality out of pulsio into a dedicated module. See: adafruit/circuitpython#3299
This gives us better granularity when implementing new ports because
PWMOut is commonly implemented before PulseIn and PulseOut.
Fixes #3211