embassy_stm32 ADC and DAC drivers don't configure pins correctly

[bgamari]

trafficstars

To use the ADC and DAC peripherals on external pins, it is expected that the user set the pin's mode to ANALOG. However, currently neither embassy_stm32's ADC nor DAC drivers do this.

Moreover, it's unclear how the ADC interface as it exists today would efficiently do so given that the it does not take ownership of the pins which it samples.

[bgamari]

In the case of the ADC this will require interface changes. I propose that we introduce (following the example of gpio::Input):

struct AdcInput<'d, T> {
    pub(create) pin: T,
    phantom: PhantomData<&'d mut T>,
}

impl<'d, T: AdcPin> AdcInput<'d, T> {
    pub fn new(pin: impl Unborrow<Target = T> + 'd) -> Self {
         unborrow!(pin);

        cortex_m::interrupt::free(|_| unsafe {
            let r = pin.block();
            let n = pin.pin() as usize;
            r.moder().modify(|w| w.set_moder(n, vals::Moder::ANALOG));
        });

        Self {
            pin,
            phantom: PhantomData,
        }
    }
}

and modify Adc::read to rather have the type:

impl<'d, T: Instance> Adc<'d, T> {
    pub fn read(&mut self, pin: &AdcInput<T>) -> u16 {
    ...
}

[bobmcwhirter]

Do the pins not start/default to analog?

[bobmcwhirter]

We also have set_as_analog() on pin to shorten that, which we attempt to do when non-analog peripherals drop their pins, to return them to that mode.

[bgamari]

@bobmcwhirter, it turns out that nearly all pins do start in analog state.

I am fine with relying on set_as_analog() to maintain the invariant that unclaimed pins are always returned to analog state. However, this invariant must be documented. There is currently no way someone unfamiliar with the code could know this.

[Dirbaio]

We shouldn't rely on unclaimed pins being in a particular state, we should consider them undefined. Some don't start in analog mode (I think?) and there could also be bootloaders mucking with pins before we run.

AdcInput is a "typestate", the only thing it does is assert the pin is in a certain mode. To be consistent with the way digital pin modes are handled, it probably makes more sense to keep read taking the unclaimed pin, and having it set Analog mode.

[IfitNaoned]

When i run the ADC example on NUCLEO-4L76RG with a potentionmeter like below , the value readed is always the same ( 26 ). I have tried A0 and A5 pins config without sucess.

It's the same bug ?