embedded-hal icon indicating copy to clipboard operation
embedded-hal copied to clipboard

Tracking issue for ADC trait

Open eldruin opened this issue 3 years ago • 7 comments
trafficstars

The ADC traits (adc::Channel and adc::OneShot) available on the 0.2.x versions have been removed ahead of the 1.0.0 release. See: #376 This issue servers as a tracking issue until we add them back/reject them.

The reasons were:

  • The traits are quite complicated to use in generic code.
  • It seems users would be better served by inherent methods.
  • Also, these traits are nb-only.

ADC traits as of the 0.2.7 release:

Relevant issues/PRs:

  • #10
  • #123
  • #101
  • #110
  • Your issue here!

Please feel free to participate, highlight your current use cases, problems and provide solutions.

eldruin avatar Apr 13 '22 07:04 eldruin

the updated spi traits have me wondering if a similar approach would suit for ADCs... something like a shared AdcDevice that is split into separate channels implementing AdcChannel which drivers then take in the same manner as digital pins or any other injected type.

ADC peripherals would seem to have very similar constraints as sharing as a bus; one ADC peripheral with multiple channels that may be used in different places, and a need to be able to take ownership of the peripheral for more complex behaviors like interleaving or DMA.

(i think this is also relevant to our previous timer / delay traits, though these do not always need to be exclusive you also often need to be able to duplicate delays or split timers into multiple drivers)

ryankurte avatar Apr 28 '22 21:04 ryankurte

Maybe having a trait for an "ADC device" is not really needed? We could just have this

trait AdcChannel
    fn read(&mut self) -> Result<u32, Error>;
}

If the hardware has a single multi-channel ADC it's up to the implementors to do the "sharing" for it. Via RefCell, Mutex, or whatever. Or it could be even a noop for MCU peripheral ADCs: hit the registers directly from read() and ensure the channels are !Send so that the user can't hit the ADC concurrently from multiple threads (irq priority levels).

need to be able to take ownership of the peripheral for more complex behaviors like interleaving or DMA.

I don't think making traits for these is feasible, they're incredibly hardware dependent...

Dirbaio avatar Apr 28 '22 21:04 Dirbaio

Maybe having a trait for an "ADC device" is not really needed? We could just have this

what i'm mostly wondering is whether this is a consistent problem with spi / i2c / adc / etc. that we can extract in a consistent way, but yeah seems to me like AdcChannel could be a good start.

I don't think making traits for these is feasible, they're incredibly hardware dependent...

agreed, but the in the spi model you can still take exclusive ownership of the bus and do whatever is needed via the closure based approach.

ryankurte avatar Apr 29 '22 04:04 ryankurte

A complication in the case of ADCs is that some channel combinations can be configured to work in comparison mode, where the measurements taken correspond to the difference between two (otherwise-independent) channels. See the ads1x1x driver for an example.

eldruin avatar May 11 '22 19:05 eldruin

trait AdcChannel
    fn read(&mut self) -> Result<u32, Error>;
}

I'd strongly vote for a read() interface for ADC as this would abstract away how the ADC is connected. Could be a GPIO or via SPI.

crjeder avatar Jul 14 '22 08:07 crjeder