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

Add System.Device.Adc.{AdcChannel, AdcChannelMode, AdcController} and port Mcp3008 device binding to new surface area #90

Closed
joshfree opened this issue Dec 10, 2018 · 16 comments
Labels
area-System.Device.Gpio Contains types for using general-purpose I/O (GPIO) pins Design Discussion Ongoing discussion about design without consensus enhancement New feature or request help wanted Extra attention is needed up-for-grabs Good issue for external contributors to iot

Comments

@joshfree
Copy link
Member

We need to standardize ADC device binding APIs, using Windows.Devices.Adc as a starting point, and update the 10-bit ADC Mcp3008 device binding accordingly.

https://docs.microsoft.com/en-us/uwp/api/windows.devices.adc.adccontroller

https://github.com/dotnet/iot/tree/master/src/devices/Mcp3008

@joshfree joshfree added help wanted Extra attention is needed Design Discussion Ongoing discussion about design without consensus labels Dec 10, 2018
@joshfree
Copy link
Member Author

@joperezr would you be interested in picking this one up next?

@joperezr
Copy link
Member

I was planning on working on testing infrastructure next, but I can also do this one instead.

@shaggygi
Copy link
Contributor

@shaggygi
Copy link
Contributor

@krwq
Copy link
Member

krwq commented Feb 1, 2019

ADS1115 is also in progress: #191 Might be worth taking a closer look soon and convert stuff like FSR to use generic ADC

@joperezr
Copy link
Member

The thing about this is that technically ADC is not really a communication protocol, so I'm not so sure if the right place for this would be the main library with a namespace like System.Device.Adc. IMO, this is something that should probably live on the device bidnings package instead, perhaps some sort of interface that all ADCs need to implement.

@krwq
Copy link
Member

krwq commented Feb 12, 2019

@joperezr ADC is a device so System.Device sounds like a good place 😄

@joperezr
Copy link
Member

joperezr commented Feb 12, 2019

For the namespace we’ve been using System.Device.{CommunicationProtocol} Where Communication protocol is a Device protocol. We currently have Gpio, Pwm, I2c, and Spi. That means that this would be the first namespace where the third part of it is not a protocol. For device bindings. We have been using Iot.Device instead, so this would probably be something like Iot.Device.Adc

@shaggygi
Copy link
Contributor

@joperezr so are you suggesting not having System.Device.Adc in core API similar to System.Device.Gpio/I2c/Spi/Pwm?

If so, I disagree as there are dev boards that have true ADC pins (like BeagleBoard Black). We should have similar classes/interfaces for controllers/drivers as we're doing with Gpio/Pwm. Therefore, dev boards and bindings could implement them. For example, the Mcp3008 could also implement IAdcController like Mcp23xxx implements IGpioController.

If I misunderstood you.... then disregard 😕

@krwq
Copy link
Member

krwq commented Feb 12, 2019

I think we should not think that much about how to package this yet and rather what abstraction is useful right now. Once we are closer to shipping then we have more conversations of what to put where (and it makes sense to put pure abstraction in different place than implementation) but I think we still got some time until then and as long APIs are experimental it is easier to put them in one bag for now.

@joperezr
Copy link
Member

If so, I disagree as there are dev boards that have true ADC pins (like BeagleBoard Black). We should have similar classes/interfaces for controllers/drivers as we're doing with Gpio/Pwm.

I see, I actually wasn't aware that there are some devices that have pins for this particular pourpose which then does mean that it would make sense to have this on the main library. In any case, as @krwq says, I think that because of what @shaggygi brings up it would be fine to put the implementation on the main library for now, and move it later if we don't think it should be there once shipping is close.

@Ellerbach
Copy link
Member

That would be really good to have an IAdcController interface like the IGpioController one. I would be able to implement both on the GrovePi (see #246) to be used fully transparently as a both devices. that would simplify how to create some basic sensors like basic buttons, basic analogic sound sensors, etc.

@krwq krwq added this to the 3.0 milestone Feb 22, 2019
@joshfree
Copy link
Member Author

@joperezr my thinking is we could use https://docs.microsoft.com/en-us/uwp/api/windows.devices.adc.adccontroller as inspiration for an IAdcController

@joshfree joshfree added the area-System.Device.Gpio Contains types for using general-purpose I/O (GPIO) pins label Feb 27, 2019
@krwq
Copy link
Member

krwq commented Mar 5, 2019

We are currently questioning if we should have these interface. For things which take ADC as an input we think the users should use Func<double> - ADC doesn't return data in any specific unit (relative voltage?) so the data is not really bound to ADC and could be anything.

We would need to see some specific examples to see what problems it would solve to have them. I haven't personally had much chance playing with ADC to think of any so would be useful for someone who has to provide something.

@Ellerbach
Copy link
Member

We would need to see some specific examples to see what problems it would solve to have them. I haven't personally had much chance playing with ADC to think of any so would be useful for someone who has to provide something.

All ADC are measuring in byte, int, long or equivalent (some can be negative as well). They all have a reference ADC which can be as well one of those.
For specific calculations, you may need the both values and not just the result. Good to have the result as a float as well but that won't be enough in some of the cases.

@joperezr joperezr added up-for-grabs Good issue for external contributors to iot enhancement New feature or request labels Apr 16, 2019
@joperezr joperezr modified the milestones: 3.0, Future May 14, 2019
@Ellerbach
Copy link
Member

[Triage] All the ADC MCPxxxx bindings has been moved under the System.Device.Adc namespace: https://github.com/dotnet/iot/tree/main/src/devices/Mcp3xxx
So closing this issue.

@ghost ghost locked as resolved and limited conversation to collaborators Aug 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Device.Gpio Contains types for using general-purpose I/O (GPIO) pins Design Discussion Ongoing discussion about design without consensus enhancement New feature or request help wanted Extra attention is needed up-for-grabs Good issue for external contributors to iot
Projects
None yet
Development

No branches or pull requests

5 participants