Adc API

[japaric]

This API has not been designed yet. What methods should it provide?

[japaric]

Looking only at STM micros the trait for single conversion mode would have to look like this:

trait Adc {
    type Bits;
    type Error;

    /// Starts a single ADC conversion (on a single channel)
    fn start(&self) -> Self::Error;

    /// Reads the converted value (Adc.start must be called first)
    fn read(&self) -> nb::Result<Self::Bits, Self::Error>;
}

adc.start().unwrap(); // SWSTART = 1
let value: u16 = block!(adc.read()).unwrap();

Anything more complicated (multiple channels or continuous conversion mode) will probably require something DMA based. Thoughts @adamgreig?

[adamgreig]

I've been thinking about how I'd want an ADC trait to look for a few days now but keep running into various issues. I think your concept above is reasonably nice for the single conversion case, though:

• Sample seems a nicer trait name than Bits, and perhaps convert() or convert_single() rather than start() too.

• Many ADCs can convert continuously and you can just read the latest sample in a non-blocking sense; but I think this trait could be implemented to support the user having already set that up, so that each read() only blocks until there's new data available and start no-ops if the ADC is already started continuously. This is where you want your firmware to immediately access the most recently sampled value without waiting, but you don't care if you miss samples in between.

• You so commonly need to select the right channel on a mux before reading that I'm not sure it's useful to have an abstraction that doesn't cover it, even given as we're trying to not have configuration here. If you're writing some abstract driver or firmware and you're passed in something that you can read samples from, odds are good something else will need to read samples from the same ADC but a different channel. I think the better trait to implement would be AdcChannel which acquires access to the ADC, samples the selected channel, and returns that. But it's a bit messy and mixes configuration...

I think in general ADCs are commonly used either:

1. You need to intermittently record a slowly varying signal, such as reading a configuration trimpot, or sampling a temperature sensor for one-off reports, or reading a voltage rail for telemetry, etc. You want to do a single sample of one channel and often block until it's done. In this case I think you usually want to be able to easily sample different channels (where AdcChannel might be useful). This is what the above trait targets.

2. You need to sample a quickly-changing signal, where the sampling time base is important (so you probably clock off a timer), and you're likely processing large blocks of samples from the same channel at once, so you're probably using DMA. Maybe you're chaining two or three ADCs to simultaneously sample more channels, or to interleave sampling a single channel at a higher rate. Examples are things like capturing audio signals, demodulating radio basebands, datalogging, etc. I think most of the work in this case is involved in configuration -- you need to configure the timer, the ADC, the DMA -- and then you press 'start' and the samples start flowing, so there's less to offer in an abtract trait for runtime use only. Mostly it would come down to the DMA circular buffer being discussed in DMA based API proposal #14 and you just process each block of data as they arrive, so there's little need for the Adc trait to be involved here.

I'd be interested to hear of use cases for ADCs different to those two. There's things like analogue watchdogs where the ADC interrupts after going above/below a threshold, but I think that's again a case of a bunch of device-specific configuration followed by an interrupt, so doesn't make sense in the embedded-hal context.

[japaric]

Thanks for the input @adamgreig

I think it may make sense to have three traits then: one for single conversion,
one for continuous conversion and one for selecting the channel. Tentatively:

trait Single {
    type Error;
    type Sample;

    fn start(&self) -> Result<(), Self::Error>;
    fn convert(&self) -> nb::Result<Self::Sample, Self::Error>;
}

single.start();
let sample = block!(single.convert()).unwrap();

trait Continuous {
    type Sample;

    fn read(&self) -> Self::Sample;
}

let sample = continuous.read();

trait Select {
    type Channel;
    type Error;

    fn select(&self, channel: Self::Channel) -> Result<(), Self::Error>;
}

I was wondering about failure modes of the continuous conversion mode. I suppose
that in that case you (a) don't care about missing samples between two
consecutive reads, but (b) also, I suppose, don't care about calling read
too fast that you end up grabbing the same value in each call. If neither (a)
and (b) are considered errors then is there any error that this mode would have
to deal with?

Single.convert returns nb::Result so you can block! on it, or you could
create a futures based API that with a single method, maybe called fread,
would start a conversion and return a future that represents the result of the
conversion.

As for Select you can compose it with Single using generics: where A: Single + Select. I'm not sure if it makes sense to use Select with
Continuous but there's nothing stopping you from using them together.

[adamgreig]

In continuous mode it'd be an error if the ADC wasn't running at all, and I don't think you can statically assert that it is? read() couldn't start the ADC if not started since then it'd have to block until the first sample arrives, which means returning a Result anyway. Perhaps it just panics in that case.

[japaric]

I don't think you can statically assert that it is?

That's probably doable using session types or other type level wizardy but would probably complicate the trait too much, or may not even be abstractable into a trait.

We could add a check to Continuous.read but it wouldn't be zero cost IMO. I expect users of Continuous would want to just read the value without checks or waiting. I think we could just say that it's up to the caller to ensure the ADC is running before calling read. The trait could include methods to check this: is_enabled(&self) -> bool and maybe even enable(&self) to enable the ADC. Then usage would look like this:

fn foo<A>(adc: A) where A: adc::Continuous {
    if !adc.is_enabled() {
        adc.enable();
        delay(100);
    }

    loop {
        let sample = adc.read();
        ..;
    }
}

[tl8roy]

2 fairly common things that I remember doing when I was using ADCs in PICs was setting the sample rate and the analogue reference. Those are more configuration options, so might be better in a sub struct but the sample rate is a pain in Arduino, but the Ref is a straight function call. Also, Read Resolution might be handy to have around.

On continuous readings, I think there should be an error returned if you try to read without starting the ADC. Maybe have another function read_fast() that doesn't have the checks and is intended for fast operations.

[burrbull]

#101

[jonas-schievink]

embedded-hal/src/adc.rs

Lines 51 to 54 in 62a5dc6

	// `channel` is a function due to [this reported
	// issue](https://github.com/rust-lang/rust/issues/54973). Something about blanket impls
	// combined with `type ID; const CHANNEL: Self::ID;` causes problems.
	//const CHANNEL: Self::ID;

This can now be an associated constant as the upstream issue was fixed.

[iFreilicht]

In continuous mode it'd be an error if the ADC wasn't running at all, and I don't think you can statically assert that it is? read() couldn't start the ADC if not started since then it'd have to block until the first sample arrives, which means returning a Result anyway. Perhaps it just panics in that case.

Maybe this could be solved similarly to how digital IO is handled? So, there'd be a trait that can be implemented for IO pins IntoContinuousAnalog<AdcChannel> providing fn into_continuous_analog(self, adc: StartedContinuousAdcChannel). You'd only implement it for the channels that are available for each pin. So to initialize, you'd do something like:

let dp = my_hal::Peripherals::take().unwrap();
let pins = my_hal::pins!(dp);
let adc = dp.ADC1.start();

let a1 = pins.pb6.into_continuous_analog(adc.channel2);

This way, the compiler verifies statically that no channel is reused, and that the adc is started.