-
Notifications
You must be signed in to change notification settings - Fork 231
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
Reimplement simple pwm #272
Conversation
Fixed my typo which tried to use simple_pwm on all MCU types instead of just the atmega ones. |
This PR should probably have been a Draft from the beginning, sorry about the noise.. I'll add simple_pwm implementations for the remaining MCU's and fix the documentation errors that I've made this coming weekend. |
I didn't set out to implement this for all the (currently) supported MCUs from the beginning, but it was rather fun, so I just kept going. This should now be fairly "complete" as far as the current MCU support goes. The tests break because they depend on a new release of avr-device (due to the missing timer-patches for atmega1280), so I guess the "waiting-for-avr-device" label would be appropriate here. Pros
Cons
As said; any feedback is greatly appreciated. |
Thank you SO MUCH for this PR! I know nothing about microcontrollers and that means I'm hesitant to use workaround from other issues that uses unfamiliar for me internal names. Using this branch I was able to play with my LED too :D |
69c9d9b
to
e615e7e
Compare
No worries, I'm guessing that you maintain these libraries as a hobby, and it wasn't my intention to cause any stress by opening a fairly big unsolicited pull request. I'm just happy to tinker with this. :) |
The only difference seems to be the amount of flash memory available
e615e7e
to
d89427a
Compare
Rebased once more to add simple_pwm for the attiny84. |
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.
Hey, thank you very much again for this changeset! Sorry for all the delays from my side...
I took the time to review it now, everything looks good enough that I will merge it as is. I think it is better to get this into the project ASAP and iterate on anything else in the future.
Thanks btw for the cleanly rebased commit series. Not many people take the time to do this so I appreciate it a lot.
I tested the Uno example. It works fine but for some reason the built-in LED on pin 13 seems to behave weirdly while it runs... Configuring the pin as output to drive it explicitly fixes this. But I do not think we should worry about it here.
Finally one minor nit for the future: You should configure your editor to always save files with a trailing newline. In some files here, it is missing, but I will fix this for you after merging.
Thank you very much for picking this up! |
This is a reimplementation of the "simple" fastpwm functionality from the old-branch.
I've only been able to test this on a Arduino Uno, a Leonardo and a Trinket, but I've tried to add implementations for some of the other MCUs (atmega48p, atmega168, atmega328pb, atmega2650) as well, based on the code in the old-branch.
I also added implementations for atmega32u8, attiny85, attiny88 and atmega1280 as well, which wasn't available in the old branch IIRC.
Any feedback would be greatly appreciated.
Closes #251.