trait for new_ads1015() structure?

[pdgilbert]

I am trying to define a function with code for reading a couple of ads1015s so the loop is easier to read. The difficulty is that this requires the type for the structure returned by Ads1x1x::new_ads1015() which is used to read channels. Since the structure type is messy I would like to use a trait, but can only find traits for channels, not for the object doing the reading. The code below is a simplified extract which illustrates the problem.

Click to expand

// read_adc for blue pill stm32f103

#![deny(unsafe_code)]
#![no_std]
#![no_main]

#[cfg(debug_assertions)]
use panic_semihosting as _;

#[cfg(not(debug_assertions))]
use panic_halt as _;

use cortex_m_rt::entry;

use ads1x1x::{channel as AdcChannel, Ads1x1x, FullScaleRange, SlaveAddr};

use nb::block;

use stm32f1xx_hal::{
    delay::Delay,
    device::I2C2,
    i2c::{BlockingI2c, DutyCycle, Mode, Pins},
    pac::{CorePeripherals, Peripherals},
    prelude::*,
};

fn setup() -> (BlockingI2c<I2C2, impl Pins<I2C2>>, Delay) {
    let cp = CorePeripherals::take().unwrap();
    let p = Peripherals::take().unwrap();

    let mut rcc = p.RCC.constrain();
    let clocks = rcc.cfgr.freeze(&mut p.FLASH.constrain().acr);

    let mut gpiob = p.GPIOB.split(&mut rcc.apb2); // for i2c scl and sda

    // can have (scl, sda) using I2C1  on (PB8, PB9 ) or on  (PB6, PB7)
    //     or   (scl, sda) using I2C2  on (PB10, PB11)

    let i2c = BlockingI2c::i2c2(
        p.I2C2,
        (
            gpiob.pb10.into_alternate_open_drain(&mut gpiob.crh), // scl on PB10
            gpiob.pb11.into_alternate_open_drain(&mut gpiob.crh), // sda on PB11
        ),
        //&mut afio.mapr,  need this for i2c1 but not i2c2
        Mode::Fast {
            frequency: 100_000.hz(),
            duty_cycle: DutyCycle::Ratio2to1,
        },
        clocks,
        &mut rcc.apb1,
        1000,
        10,
        1000,
        1000,
    );

    let delay = Delay::new(cp.SYST, clocks);

    (i2c, delay) // return tuple (i2c, delay)
}

fn read_adc(adc_a: impl Ads1x1x, adc_b: impl Ads1x1x) -> (i16, i16, i16, i16, [i16; 3]) {

    // signature needs type of Ads1x1x::new_ads1015() or trait to use impl 

    let bat_ma =
        block!(adc_a.read(&mut AdcChannel::DifferentialA1A3)).unwrap_or(8091);
    let load_ma =
        block!(adc_a.read(&mut AdcChannel::DifferentialA2A3)).unwrap_or(8091);

    // toggle FullScaleRange to measure battery voltage, not just diff across shunt resistor

    adc_a.set_full_scale_range(FullScaleRange::Within4_096V).unwrap();
    let bat_mv = block!(adc_a.read(&mut AdcChannel::SingleA0)).unwrap_or(8091);
    adc_a.set_full_scale_range(FullScaleRange::Within0_256V).unwrap();

    // second adc
    let values_b = [
        block!(adc_b.read(&mut AdcChannel::SingleA0)).unwrap_or(8091),
        block!(adc_b.read(&mut AdcChannel::SingleA1)).unwrap_or(8091),
        block!(adc_b.read(&mut AdcChannel::SingleA2)).unwrap_or(8091),
    ];

    let temp_c = block!(adc_b.read(&mut AdcChannel::SingleA3)).unwrap_or(8091);

    (bat_mv, bat_ma, load_ma, temp_c, values_b)
}

#[entry]
fn main() -> ! {

    let (i2c, mut delay) = setup();

    let manager = shared_bus::BusManager::<cortex_m::interrupt::Mutex<_>, _>::new(i2c);

    let mut adc_a = Ads1x1x::new_ads1015(manager.acquire(), SlaveAddr::Alternative(false, false)); //addr = GND
    let mut adc_b = Ads1x1x::new_ads1015(manager.acquire(), SlaveAddr::Alternative(false, true)); //addr =  V

    adc_a
        .set_full_scale_range(FullScaleRange::Within0_256V)
        .unwrap();
    adc_b
        .set_full_scale_range(FullScaleRange::Within4_096V)
        .unwrap();


    loop {
        let (_bat_mv, _bat_ma, _load_ma, _temp_c, _values_b) = read_adc(adc_a, adc_b);
        
        // do other things
        delay.delay_ms(2000_u16); // sleep for 2s
    }
}

Since I don't have the trait right it gives

fn read_adc(adc_a: impl Ads1x1x, adc_b: impl Ads1x1x) -> (i16, i16, i16, i16, [i16; 3]) {
                          ^^^^^^^ not a trait

Is there a way to specify the trait to do this, or just a better way to approach the whole thing?

[eldruin]

If you split read_adc() into two functions so that you can change the FSR outside of it, you could have two functions that simply take the ADC as an embedded_hal::adc::OneShot trait.
Would that be a solution for you?

[pdgilbert]

Yes, this would work. But now I'm stuck trying to figure out the type arguments

error[E0107]: wrong number of type arguments: expected 3, found 0
  --> examples/zz.rs:64:27
   |
64 | fn read_adc_a(adc_a: impl embedded_hal::adc::OneShot) -> (i16, i16, i16) {
   |                           ^^^^^^^^^^^^^^^^^^^^^^^^^^ expected 3 type arguments

(I expect there should be an easy way to find type arguments in documentation or compiler error messages, but I have not figure it out.)

[eldruin]

Ah, that solution I proposed cannot work because the OneShot trait is channel-specific and you want to pass the whole ADC and measure several channels inside the function.
You can use your function as initially proposed if you define the concrete types of the ADC.
An example of doing this is in this example.
You will only need to replace I2cdev with something like BlockingI2c<I2C2, ...> although putting the pin types there will bloat the type up a bit.
I have a look at adding a trait to simplify this.

[eldruin]

I have added the DynamicOneShot trait to ease the usage in functions in release 0.2.2. See this example.
Closing.

[pdgilbert]

Thanks for adding this trait. It works well except when I try to use the method .set_full_scale_range in the read function. (I do this because I am measuring a voltage across a shunt resistor and also to ground.) I'm not sure if I just don't have the proper syntax figured out, I am a newbie still, or if there is a problem de-referencing the method. I have tried various syntax without luck:

113 |     adc_a.set_full_scale_range(FullScaleRange::Within4_096V).unwrap();
    |           ^^^^^^^^^^^^^^^^^^^^ method not found in `&mut A`

113 |     *adc_a.set_full_scale_range(FullScaleRange::Within4_096V).unwrap();
    |            ^^^^^^^^^^^^^^^^^^^^ method not found in `&mut A`

113 |     (*adc_a).set_full_scale_range(FullScaleRange::Within4_096V).unwrap();
    |              ^^^^^^^^^^^^^^^^^^^^ method not found in `A`

A stripped down version of the code follows.

Click to expand

//  setup for bluepill   stm32f1xx_hal

#![deny(unsafe_code)]
#![no_std]
#![no_main]

#[cfg(debug_assertions)]
use panic_semihosting as _;

#[cfg(not(debug_assertions))]
use panic_halt as _;

use cortex_m_rt::entry;

use ads1x1x::{Ads1x1x, ChannelSelection, DynamicOneShot, FullScaleRange, SlaveAddr};

use core::fmt::Write;

use embedded_graphics::{
    mono_font::{ascii::FONT_8X13, MonoTextStyle, MonoTextStyleBuilder}, //FONT_6X10  FONT_8X13
    pixelcolor::BinaryColor,
    prelude::*,
    text::{Baseline, Text},
};

use ssd1306::{mode::BufferedGraphicsMode, prelude::*, I2CDisplayInterface, Ssd1306};

use nb::block;

pub trait LED {
    fn on(&mut self) -> ();
    fn off(&mut self) -> ();

    // default methods
    fn blink(&mut self, time: u16, delay: &mut Delay) -> () {
        self.on();
        delay.delay_ms(time);
        self.off()
    }
}

use stm32f1xx_hal::{
    delay::Delay,
    device::I2C2,
    gpio::{gpioc::PC13, Output, PushPull},
    i2c::{BlockingI2c, DutyCycle, Mode, Pins},
    pac::{CorePeripherals, Peripherals},
    prelude::*,
};

fn setup() -> (BlockingI2c<I2C2, impl Pins<I2C2>>, impl LED, Delay) {
    let cp = CorePeripherals::take().unwrap();
    let p = Peripherals::take().unwrap();

    let rcc = p.RCC.constrain();
    let clocks = rcc.cfgr.freeze(&mut p.FLASH.constrain().acr);

    let mut gpiob = p.GPIOB.split(); // for i2c scl and sda

    let i2c = BlockingI2c::i2c2(
        p.I2C2,
        (
            gpiob.pb10.into_alternate_open_drain(&mut gpiob.crh), // scl on PB10
            gpiob.pb11.into_alternate_open_drain(&mut gpiob.crh), // sda on PB11
        ),
        Mode::Fast {
            frequency: 100_000.hz(),
            duty_cycle: DutyCycle::Ratio2to1,
        },
        clocks,
        1000,
        10,
        1000,
        1000,
    );

    let delay = Delay::new(cp.SYST, clocks);

    // led
    let mut gpioc = p.GPIOC.split();
    let led = gpioc.pc13.into_push_pull_output(&mut gpioc.crh); // led on pc13

    impl LED for PC13<Output<PushPull>> {
        fn on(&mut self) -> () {
            self.set_low()
        }
        fn off(&mut self) -> () {
            self.set_high()
        }
    }

    (i2c, led, delay)
}

pub fn read_all<E, A: DynamicOneShot<Error = E>>(adc_a: &mut A, adc_b: &mut A
      ) -> (i16, i16, i16, i16, [i16; 3]) {
    let scale_cur = 10; // calibrated to get mA/mV depends on FullScaleRange above and values of shunt resistors
    let scale_a = 2; // calibrated to get mV    depends on FullScaleRange
    let scale_b = 2; // calibrated to get mV    depends on FullScaleRange

    let scale_temp = 5; //divides
    let offset_temp = 50;

    let bat_ma  = block!(adc_a.read(ChannelSelection::DifferentialA1A3)).unwrap_or(8091) / scale_cur;
    let load_ma = block!(adc_a.read(ChannelSelection::DifferentialA2A3)).unwrap_or(8091) / scale_cur;

    // toggle FullScaleRange to measure battery voltage, not just diff across shunt resistor
    // set FullScaleRange to measure expected max voltage.
    // This is very small for diff across low value shunt resistors
    //   but up to 5v for single pin with usb power.
    // +- 6.144v , 4.096v, 2.048v, 1.024v, 0.512v, 0.256v

    adc_a.set_full_scale_range(FullScaleRange::Within4_096V).unwrap();
    let bat_mv = block!(adc_a.read(ChannelSelection::SingleA0)).unwrap_or(8091) * scale_a;
    adc_a.set_full_scale_range(FullScaleRange::Within0_256V).unwrap();

    // second adc
    let values_b = [
        block!(adc_b.read(ChannelSelection::SingleA0)).unwrap_or(8091) * scale_b,
        block!(adc_b.read(ChannelSelection::SingleA1)).unwrap_or(8091) * scale_b,
        block!(adc_b.read(ChannelSelection::SingleA2)).unwrap_or(8091) * scale_b,
    ];

    let temp_c = block!(adc_b.read(ChannelSelection::SingleA3)).unwrap_or(8091) / scale_temp
        - offset_temp;

    (bat_mv, bat_ma, load_ma, temp_c, values_b )
}

fn display<S>(
    bat_mv: i16,
    bat_ma: i16,
    load_ma: i16,
    temp_c: i16,
    values_b: [i16; 3],
    text_style: MonoTextStyle<BinaryColor>,
    disp: &mut Ssd1306<impl WriteOnlyDataCommand, S, BufferedGraphicsMode<S>>,
) -> ()
where
    S: DisplaySize,
{
    let mut lines: [heapless::String<32>; 4] = [
        heapless::String::new(),
        heapless::String::new(),
        heapless::String::new(),
        heapless::String::new(),
    ];

    // Many SSD1306 modules have a yellow strip at the top of the display, so first line may be yellow.
    // it is now possible to use \n in place of separate writes, with one line rather than vector.
    write!(lines[0], "bat:{:4}mV{:4}mA", bat_mv, bat_ma).unwrap();
    write!(lines[1], "load:    {:5}mA", load_ma).unwrap();
    write!(
        lines[2],
        "B:{:4} {:4} {:4}",
        values_b[0], values_b[1], values_b[2]
    )
    .unwrap();
    write!(lines[3], "temperature{:3} C", temp_c).unwrap();

    disp.clear();
    for i in 0..lines.len() {
        // start from 0 requires that the top is used for font baseline
        Text::with_baseline(
            &lines[i],
            Point::new(0, i as i32 * 16),
            text_style,
            Baseline::Top,
        )
        .draw(&mut *disp)
        .unwrap();
    }
    disp.flush().unwrap();
    ()
}

#[entry]
fn main() -> ! {
    let (i2c, mut led, mut delay) = setup();

    let manager = shared_bus::BusManager::<cortex_m::interrupt::Mutex<_>, _>::new(i2c);
    let interface = I2CDisplayInterface::new(manager.acquire());

    let mut disp = Ssd1306::new(interface, DisplaySize128x64, DisplayRotation::Rotate0)
        .into_buffered_graphics_mode();
    disp.init().unwrap();

    let text_style = MonoTextStyleBuilder::new()
        .font(&FONT_8X13) //.&FONT_6X10  &FONT_8X13
        .text_color(BinaryColor::On)
        .build();

    Text::with_baseline("Display initialized ...", Point::zero(), text_style, Baseline::Top)
        .draw(&mut disp)
        .unwrap();

    let mut adc_a = Ads1x1x::new_ads1015(manager.acquire(), SlaveAddr::Alternative(false, false)); //addr = GND
    let mut adc_b = Ads1x1x::new_ads1015(manager.acquire(), SlaveAddr::Alternative(false, true)); //addr =  V

    adc_a.set_full_scale_range(FullScaleRange::Within4_096V).unwrap();
    adc_b.set_full_scale_range(FullScaleRange::Within4_096V).unwrap();

    loop {
        // Blink LED to check that everything is actually running.
        led.blink(10_u16, &mut delay);

        let (bat_mv, bat_ma, load_ma, temp_c, values_b ) = read_all(&mut adc_a, &mut adc_b);

        display(bat_mv, bat_ma, load_ma, temp_c, values_b, text_style, &mut disp);

        delay.delay_ms(2000_u16); // sleep for 2s
    }
}

[eldruin]

Ah, I forgot about that. Could you just use the actual driver type then, instead of this trait?
You just need to use the concrete type (Depending on your IDE, it might help you there) like here
Then you can use it easily in a function like here
I would start with adc_a only.

[pdgilbert]

I tried using the actual driver type, switching from linux to no std, etc. I have

type Adc = Ads1x1x<I2cInterface<BlockingI2c<I2C2, impl Pins<I2C2>>>, Ads1015, Resolution16Bit, ads1x1x::mode::OneShot>;

but then I get

`impl Trait` in type aliases is unstable

and it looks downhill from there. I guess I have to spell out the whole concrete type for every hal. I have a reasonable work around just leaving that one read in the main loop ( I just have to disambiguate), so I think I will leave it like that until there is an easy way to make it clean.