Skip to content
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

atsamd51 atsame5x: change to allow specifying the ADC Bus number #1835

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

sago35
Copy link
Member

@sago35 sago35 commented Apr 23, 2021

For PB08 and PB09, and PA08 and PA09, it was not possible to select which ADC instance to use until now.
Since the two ADC instances can be sampled at the same time, we want to be able to select which ADC instance to use.

https://ww1.microchip.com/downloads/en/DeviceDoc/SAM_D5x_E5x_Family_Data_Sheet_DS60001507G.pdf
The SAM D5x/E5x has two ADC instances, ADC0 and ADC1.
The two inputs can be sampled simultaneously, as each ADC includes sample and hold circuits.

ADC0 ADC1
PB03 15
PC00 10
PC01 11
PC02 4
PC03 5
PA02 0
PA03 1
PB04 6
PB05 7
PD00 14
PD01 15
PB06 8
PB07 9
PB08 2 0
PB09 3 1
PA04 4
PA05 5
PA06 6
PA07 7
PA08 8 2
PA09 9 3
PA10 10
PA11 11
PC30 12
PC31 13
PB00 12
PB01 13
PB02 14

I tried to change the source code, but there may be a better way.
Also, as I was making changes in src/examples/adc, I ended up with incompatible changes in environments other than atsamx5x.
Would it be better to make sure that all environments except atsamx5x are not incompatible?

@@ -39,4 +39,5 @@ func (p Pin) Low() {

type ADC struct {
Pin Pin
Bus uint8
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Due to this change, src/examples/adc needs to be modified

@sago35
Copy link
Member Author

sago35 commented Apr 23, 2021

This is not directly related to this PR, but I noticed that the following sources can be unified.
I'll discuss it in another PR / Issue.

  • machine_atsamd51.go
  • machine_atsame5x.go

@sago35
Copy link
Member Author

sago35 commented May 3, 2021

This is not directly related to this PR, but I noticed that the following sources can be unified.
I'll discuss it in another PR / Issue.

* machine_atsamd51.go

* machine_atsame5x.go

Fixed with #1845

@deadprogram
Copy link
Member

I think there is probably a different way you can solve this @sago35 that does not require breaking the current API.

For example, your function getADCBus() could lookup which bus matches that pin. In the case that you really need to set which bus to use, then perhaps adding the bus number to https://github.com/tinygo-org/tinygo/blob/dev/src/machine/adc.go#L8

What do you think?

@sago35
Copy link
Member Author

sago35 commented May 10, 2021

The original version of getADCBus() was implemented in such a way that if there were two candidates, it would return one of them.
When a new getADCBus() decides which bus to use, there are two rules to follow.

If there is only one bus that the Pin can use, it will return that bus.
If there are two busses available for the Pin, the bus is determined from the information stored in the ADC-struct.

Hmm, I can't think of a good way to do this.

@sago35
Copy link
Member Author

sago35 commented May 10, 2021

The dev branch is in progress, so I rebased it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants