Skip to content

[RFC] WIP: shared adc #6

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

Closed
wants to merge 1 commit into from
Closed

[RFC] WIP: shared adc #6

wants to merge 1 commit into from

Conversation

geomatsi
Copy link
Contributor

@geomatsi geomatsi commented Aug 5, 2019

Hi @Rahix

Here is an attempt to implement ADC proxy. The first commit is the same as in #5 , so it will be removed as soon as shared-bus is updated for Rust 2018.

Suggested implementation is not complete and has issues. However so far I could not find a proper way to fix them. So the purpose of this PR is to get feedback and find a better approach for shared ADC.

Example 1: using shared ADC for single channel

The following example appears to be working (full example is here) for single channel Measurement wrapper:

pub struct Measurement<'a, ADC, PIN>
where
    PIN: Channel<ADC, ID = u8>,
{
    adc: &'a mut dyn OneShot<ADC, u16, PIN, Error = ()>,
    pin: PIN,
}

impl<'a, ADC, PIN> Measurement<'a, ADC, PIN>
where
    PIN: Channel<ADC, ID = u8>,
{
    pub fn init(adc: &'a mut dyn OneShot<ADC, u16, PIN, Error = ()>, pin: PIN) -> Self {
        Measurement { adc, pin }
    }

    pub fn test(&mut self) -> u16 {
        self.adc.read(&mut self.pin).unwrap()
    }
}

And then create multiple instances each using different channel:

    type AdcType = Adc<stm32::ADC1>;

     ...

    let ch0 = gpioa.pa0.into_analog(&mut gpioa.crl);
    let ch1 = gpioa.pa1.into_analog(&mut gpioa.crl);

    let adc = Adc::adc1(p.ADC1, &mut rcc.apb2, clocks);
    let adc_bus = shared_bus::BusManager::<
        cm::interrupt::Mutex<core::cell::RefCell<AdcType>>,
        AdcType,
    >::new(adc);

    let mut adc_mgr1 = adc_bus.acquire();
    let mut adc_mgr2 = adc_bus.acquire();

    let mut a = Measurement::init(&mut adc_mgr1, ch0);
    let mut b = Measurement::init(&mut adc_mgr2, ch1);

    a.test();
    b.test();

Example 2: using shared ADC for multiple channels

Similar approach does not work for Measurement with multiple channels (full example here):

pub struct Measurement<'a, ADC, PIN>
where
    PIN: Channel<ADC, ID = u8>,
{
    adc: &'a mut dyn OneShot<ADC, u16, PIN, Error = ()>,
    pin1: PIN,
    pin2: PIN,
}

impl<'a, ADC, PIN> Measurement<'a, ADC, PIN>
where
    PIN: Channel<ADC, ID = u8>,
{
    pub fn init(adc: &'a mut dyn OneShot<ADC, u16, PIN, Error = ()>, pin1: PIN, pin2: PIN) -> Self {
        Measurement { adc, pin1, pin2 }
    }

    pub fn test1(&mut self) -> u16 {
        self.adc.read(self.pin1).unwrap()
    }

    pub fn test2(&mut self) -> u16 {
        self.adc.read(self.pin2).unwrap()
    }
}

Due to strict type inference the following snippet results in compilation error:

    let mut gpioa = p.GPIOA.split(&mut rcc.apb2);
    let ch0 = gpioa.pa0.into_analog(&mut gpioa.crl);
    let ch1 = gpioa.pa1.into_analog(&mut gpioa.crl);
    let ch2 = gpioa.pa2.into_analog(&mut gpioa.crl);
    let ch3 = gpioa.pa3.into_analog(&mut gpioa.crl);

    let adc = Adc::adc1(p.ADC1, &mut rcc.apb2, clocks);
    let adc_bus = shared_bus::BusManager::<
        cm::interrupt::Mutex<core::cell::RefCell<AdcType>>,
        AdcType,
    >::new(adc);

    let mut adc_mgr1 = adc_bus.acquire();
    let mut adc_mgr2 = adc_bus.acquire();

    let mut a = Measurement::init(&mut adc_mgr1, ch0, ch1);
    let mut b = Measurement::init(&mut adc_mgr2, ch2, ch3);

Compiler expects ch0 and ch1 to be of the same type which is not the case.

Straightforward attempt to fix includes the following two steps:

  • switching from PIN to trait objects &'a mut dyn Channel<ADC, ID = u8
  • using workaround for method `channel` has no receiver issue

However this brings errors for ADC reading in test mehods:

     |
36 |         self.adc.read(self.pin1).unwrap()
     |                       ^^^^^^^^^ expected type parameter, found trait embedded_hal::adc::Channel
     |

Regards,
Sergey

Add support for ADC bus proxy.

Signed-off-by: Sergey Matyukevich <[email protected]>
@geomatsi
Copy link
Contributor Author

Hi @Rahix

I did more experiments trying to make use of suggested shared ADC implementation. So far I failed to fix the second (flexible multi channel) example: Sized issues for dyn Trait and other trait objects issues on the way down the rabbit hole.

On the other hand, suggested shared ADC implementation works fine if we focus on less flexible, but arguably more practical example supporting multiple channels.

Example 3: using shared ADC for multiple specific channels

Unlike the second example, here we create two objects with specific list of channels assigned to each object from the very beginning. Full example is available here. Here are the major building blocks:

type AdcType = Adc<ADC1>;
type AdcProxy<'a> = BusProxy<'a, Mutex<RefCell<Adc<ADC1>>>, Adc<ADC1>>;

type Chan0 = hal::gpio::gpioa::PA0<hal::gpio::Analog>;
type Chan1 = hal::gpio::gpioa::PA1<hal::gpio::Analog>;
type Chan2 = hal::gpio::gpioa::PA2<hal::gpio::Analog>;
type Chan3 = hal::gpio::gpioa::PA3<hal::gpio::Analog>;
type Chan4 = hal::gpio::gpioa::PA4<hal::gpio::Analog>;

pub struct M1<'a> {
    adc: AdcProxy<'a>,
    ch0: Chan0,
    ch1: Chan1,
}

impl<'a> M1<'a> {
    pub fn init(adc: AdcProxy<'a>, ch0: Chan0, ch1: Chan1) -> Self {
        M1 { adc, ch0, ch1 }
    }

    pub fn get_ch0(&mut self) -> u16 {
        self.adc.read(&mut self.ch0).unwrap()
    }

    pub fn get_ch1(&mut self) -> u16 {
        self.adc.read(&mut self.ch1).unwrap()
    }
}

pub struct M2<'a> {
    adc: AdcProxy<'a>,
    ch2: Chan2,
    ch3: Chan3,
    ch4: Chan4,
}

impl<'a> M2<'a> {
    pub fn init(adc: AdcProxy<'a>, ch2: Chan2, ch3: Chan3, ch4: Chan4) -> Self {
        M2 { adc, ch2, ch3, ch4 }
    }

    pub fn get_ch2(&mut self) -> u16 {
        self.adc.read(&mut self.ch2).unwrap()
    }

    pub fn get_ch3(&mut self) -> u16 {
        self.adc.read(&mut self.ch3).unwrap()
    }

    pub fn get_ch4(&mut self) -> u16 {
        self.adc.read(&mut self.ch4).unwrap()
    }
}

...

let adc = Adc::adc1(p.ADC1, &mut rcc.apb2, clocks);
let adc_bus = BusManager::<Mutex<RefCell<AdcType>>, AdcType>::new(adc);
let adc_mgr1 = adc_bus.acquire();
let adc_mgr2 = adc_bus.acquire();

let mut a = M1::init(adc_mgr1, ch0, ch1);
let mut b = M2::init(adc_mgr2, ch2, ch3, ch4);

This example works with suggested shared ADC implementation w/o any additional fixes for embedded-hal or stm32f1xx-hal.

Regards,
Sergey

@geomatsi
Copy link
Contributor Author

Hi @Rahix ,
Any thoughts on this ? Do you think it can be merged as is taking into account that Example 1 and Example 3 work fine ?

Regards,
Sergey

@Rahix
Copy link
Owner

Rahix commented Aug 29, 2019

Hi Sergey,

I am really busy with non-embedded-rust stuff right now, so I did not have time to look at this yet ... I'll ping you once I'm back. Sorry!

@geomatsi
Copy link
Contributor Author

Sure, no hurry at all. Let us get back to this topic when you have time.

Thanks,
Sergey

@Rahix
Copy link
Owner

Rahix commented Aug 19, 2020

Hi Sergey,

sorry for not responding until now. As you might have seen, I'm currently doing a major rework of this crate (#15) and I'm definitely planning on trying to integrate your work from this PR over there once the other stuff is working!

@geomatsi
Copy link
Contributor Author

Sure. I will keep an eye on your work and will try to help with review and testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants