-
Notifications
You must be signed in to change notification settings - Fork 955
machine: refactor PWM support #1121
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
Conversation
What is the status with this PR now? |
58602f9
to
0c90dfb
Compare
I now have a PWM abstraction for four chips: atsamd21, atsamd51, nrf52xxx, and atmega328p. The abstraction works with a WIP servo and tone driver, proving that it can be used for purposes that require tight control over frequency and duty cycle. Next up: update the documentation so that it is usable and possibly also add a method or something so that it remains possible to easily drive LEDs using PWM without worrying about PWM instances. |
I updated the PR again, I think it is now more or less ready. I have also updated the documentation. I'm not entirely ready yet, but here is the near-final result: https://github.com/tinygo-org/tinygo-site/blob/pwm/content/microcontrollers/machine/pwm.md For individual boards, I've started adding documentation on pin to PWM mapping: https://github.com/tinygo-org/tinygo-site/blob/pwm/content/microcontrollers/circuit-playground-express.md#pins To be implemented at a later date:
|
Been studying this PR for a while and I think the latest version is pretty great. What would be the next steps to help it land? |
@conejoninja @ardnew @sago35 or anyone else have any opinions? |
I didn't really look at it recently but it should be ready now. What is missing is some work on the documentation, but that can perhaps happen after this PR lands. Of course, this is a breaking change so might need a few more eyes for review. |
I don't fully understand the details, but looks good to me. documentation is clear enough and it would be great to finally have a servo driver, so this is a first step |
I'm not real certain what you mean by this. Do you mean a common API using various chip's timer peripherals (e.g., TC/TCC on SAMD21) as general-purpose timers (with common functionality such as interrupt on overflow, etc.) and/or system timers such as SysTick and Sleep? If so, that seems like an entirely separate effort from this PWM support |
Yes, this. PWM and timers may seem like different things but in practice they aren't. Most of the time a single peripheral can be used as both a PWM and a timer. In fact, the core of a PWM is a timer. The nrf chips do have a dedicated PWM (that would be harder to use as a timer) and have a dedicated RTC (that can't easily be used as a PWM) but most other chips I've looked at (avr, sam, stm32) don't make a clear distinction. For most chips, I do not intend to use these PWM peripherals as system times however maybe it would be a good idea on AVR: adding an API to use one of the timer/PWM peripherals as a system timer (because AVRs usually don't have a system timer). |
It looks very good. |
So after merge conflicts are solved, this PR can finally be merged? :) |
@aykevl reminder that this PR needs a merge conflict resolved. Thanks. |
Rebased. |
Looks like just one last merge conflict to be resolved. |
This commit refactors PWM support in the machine package to be more flexible. The new API can be used to produce tones at a specific frequency and control servos in a portable way, by abstracting over counter widths and prescalers.
Rebased again. It was conflicting with #1718. |
By popular demand, now merging this PR. ♾️ |
thank u <3 |
This commit lays the groundwork and refactors the SAM D21 implementation.
This is very much WIP, as the API hasn't been agreed upon yet. It is really just a proof of concept to make sure the API works in the real world. The API implemented here gives a lot of flexibility and control over PWM while keeping device specific details to a minimum. It also tries to be as simple to use as possible.