-
Notifications
You must be signed in to change notification settings - Fork 997
board/teensy40: Add ADC support #1458
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
45b25ed to
d1e521f
Compare
d1e521f to
4f4d73d
Compare
|
Please rebase this branch against dev to resolve merge conflicts. Thanks. |
4f4d73d to
c4a7f26
Compare
|
rebased against |
9567c1c to
e8068db
Compare
src/machine/machine_mimxrt1062.go
Outdated
|
|
||
| // Configure initializes the receiver ADC's Pin and the ADC1/ADC2 instances for | ||
| // immediate usage with default settings. | ||
| func (a ADC) Configure() { a.ConfigureMode(ADCConfig{}) } |
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 think it makes sense to change the API of ADC.Configure (for all chips) to the API you're using here. The reason is that an ADC actually has a number of important configurations for different tradeoffs:
- Reference voltage (often VCC, but sometimes 1.1V or some other value).
- Accuracy vs time. I know that the nrf chips have this tradeoff (where more accurate sampling takes a lot longer but is necessary if there is a big resistor on the input) and I would guess other ADCs have something similar.
Of course, it's not necessary to implement them all right away but with this I think it makes sense to add an ADCConfig to the peripheral.
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.
Agreed. However, unless you are wanting to maintain backwards compatibility, I would think the ADC peripheral should be like all other peripherals and receive its ADCConfig as argument to Configure, and completely ditch the ConfigureMode that I'm using here.
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.
@aykevl would you rather I keep this code as is (i.e., keep it API-compatible with other machine package implementations)?
or should I introduce a new API-incompatible func (*ADC) Configure(ADCConfig), and remove my current func (*ADC) ConfigureMode(ADCConfig), under the presumption that you are planning to change those others to match this new API-incompatible function
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 think it would actually be better to (temporarily) break compatibility and meanwhile also update the signature of the other ADC implementations.
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.
updated to use the more conventional API: func (*ADC) Configure(ADCConfig)
0495df1 to
51c407b
Compare
51c407b to
b192826
Compare
|
rebased against dev |
682ec29 to
d2343d8
Compare
|
rebased against dev (3eb9dca) |
|
Looks like this PR is ready to go @ardnew Do you want to squash commits together so I can merge, or would you prefer that I do so, or something else entirely? |
|
I am going to squash/merge in the interest of getting this unstuck (which is entirely my fault for not doing sooner). Thanks @ardnew |
machine/teensy40: add ADC support
machine/teensy40: add ADC support
This PR is baselined on top of the Teensy 4.0 external interrupt support PR (#1484)
This PR adds analog input support (ADC1/ADC2) for Teensy 4.0 on 14 GPIO pins (
A0-A13).I mirrored the API used in machine
feather-m4, assuming it is probably a good example for TinyGo convention.This has been tested using a cheap potentiometer with
src/examples/adc/adc.go. Note you must also change the pin fromADC0to one of this machine's analog pins, e.g.A0. All other code can be left as-is (although the call tomachine.InitADC()is unnecessary - it is implemented as an empty stub for backwards compatibility with existing code such as this example).The following describes each analog pin, its equivalent digital pin, its IOMUX pad, and finally which ADC channel(s) it is connected to. It is defined in
src/machine/board_teensy40.go: