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

impl spi ?

Open pdgilbert opened this issue 4 years ago • 3 comments

(Possibly related to #118) I am trying to use a setup() function to return a spi object and the cs pin to use in my main code. I can do this with specific types for the return object like this stripped down (but building ok) example:

Click to expand
#![deny(unsafe_code)]
#![no_std]
#![no_main]

use cortex_m_rt::entry;
use panic_rtt_target as _;

use embedded_hal::spi::{Mode, Phase, Polarity};
pub const MODE: Mode = Mode {
    phase: Phase::CaptureOnSecondTransition,
    polarity: Polarity::IdleHigh,
};

#[cfg(feature = "stm32f1xx")]
use stm32f1xx_hal::{
    gpio::{
        gpioa::{PA4, PA5, PA6, PA7},
        Alternate, Floating, Input,
    },
    gpio::{Output, PushPull},
    pac::{Peripherals, SPI1},
    prelude::*,
    spi::{Spi, Spi1NoRemap, Pins},
};

#[cfg(feature = "stm32f1xx")]
fn setup() -> (Spi<SPI1, Spi1NoRemap, (PA5<Alternate<PushPull>>, PA6<Input<Floating>>, PA7<Alternate<PushPull>>), u8>,
    PA4<Output<PushPull>>) {
//fn setup() -> (Spi<SPI1, Spi1NoRemap, impl Pins<Spi1NoRemap, SPI1>, u8>, PA4<Output<PushPull>>) {

    let dp = Peripherals::take().unwrap();

    let mut flash = dp.FLASH.constrain();
    let mut rcc = dp.RCC.constrain();

    let clocks = rcc.cfgr.freeze(&mut flash.acr);

    let mut afio = dp.AFIO.constrain(&mut rcc.apb2);
    let mut gpioa = dp.GPIOA.split(&mut rcc.apb2);

    // SPI1
    let  sck = gpioa.pa5.into_alternate_push_pull(&mut gpioa.crl);
    let miso = gpioa.pa6;
    let mosi = gpioa.pa7.into_alternate_push_pull(&mut gpioa.crl);
    let  cs  = gpioa.pa4.into_push_pull_output(&mut gpioa.crl);

    let spi = Spi::spi1(
        dp.SPI1,
        (sck, miso, mosi),
        &mut afio.mapr,
        MODE,
        1_u32.mhz(),
        clocks,
        &mut rcc.apb2,
    );

    (spi, cs)
}

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

    let (_spi, _cs) = setup();

    loop {
    }
}

However, I cannot figure out how to use impl trait for the spi. I think it should be something like the line commented out in the above, but I get various errors with all variations I have tried. With other HALs I can do as follows:

#[cfg(feature = "stm32f3xx")]
fn setup() -> (Spi<SPI1, (impl SckPin<SPI1>, impl MisoPin<SPI1>, impl MosiPin<SPI1>), u8>,  PA1<Output<PushPull>>) {

#[cfg(feature = "stm32f4xx")]
fn setup() -> (Spi<SPI1, impl Pins<SPI1>>, PA1<Output<PushPull>>) {

#[cfg(feature = "stm32f7xx")]
fn setup() -> (Spi<SPI1, impl Pins<SPI1>, Enabled<u8>>, PA1<Output<PushPull>>,

#[cfg(feature = "stm32h7xx")]
fn setup() -> (Spi<SPI1, Enabled, >,  PA1<Output<PushPull>>) {

#[cfg(feature = "stm32l0xx")]
fn setup() -> (Spi<SPI1, impl Pins<SPI1>>, PA1<Output<PushPull>>) {

#[cfg(feature = "stm32l1xx")]
fn setup() -> (Spi<SPI1, impl Pins<SPI1>>, PA4<Output<PushPull>>) {

#[cfg(feature = "stm32l4xx")]
fn setup() -> (Spi<SPI1, (impl SckPin<SPI1>, impl MisoPin<SPI1>, impl MosiPin<SPI1>)>, PA1<Output<PushPull>>) {

What is the proper incantation for stm32f1xx-hal?

pdgilbert avatar Jun 04 '21 22:06 pdgilbert

You could write:

fn setup() -> (Spi<SPI1, Spi1NoRemap, (impl Sck<Spi1NoRemap>, impl Miso<Spi1NoRemap>, impl Mosi<Spi1NoRemap>), u8>,
    PA4<Output<PushPull>>)

if we make Sck, Miso, Mosi traits public. But is it safe? Not sure.

burrbull avatar Jun 05 '21 09:06 burrbull

I don't understand the technical aspects of this, so can't comment on whether it is safe. FWIW, from a user's pont of view it would be nice if it were similar to some other HALs, of which

Spi<SPI1, impl Pins<SPI1>>

seems to be the cleanest. I'm guessing there is a reason that doesn't work, but could

Spi<SPI1, impl Pins<SPI1, Spi1NoRemap>>

be a possibility?

pdgilbert avatar Jun 05 '21 21:06 pdgilbert

Now Spi in f1xx can take Pins in any sequence:

pins_impl!(
    (SCK, MISO, MOSI), (Sck, Miso, Mosi), (_Sck, _Miso, _Mosi);
    (SCK, MOSI, MISO), (Sck, Mosi, Miso), (_Sck, _Mosi, _Miso);
    (MOSI, SCK, MISO), (Mosi, Sck, Miso), (_Mosi, _Sck, _Miso);
    (MOSI, MISO, SCK), (Mosi, Miso, Sck), (_Mosi, _Miso, _Sck);
    (MISO, MOSI, SCK), (Miso, Mosi, Sck), (_Miso, _Mosi, _Sck);
    (MISO, SCK, MOSI), (Miso, Sck, Mosi), (_Miso, _Sck, _Mosi);
);

The only way to do that you want is to get rid of from this functionality and use only 1 variant everywhere.

burrbull avatar Jun 06 '21 06:06 burrbull