From f5a2865b5149294afb2d3b5f1cfd98526a58ad89 Mon Sep 17 00:00:00 2001 From: techmccat Date: Fri, 25 Jul 2025 16:13:08 +0200 Subject: [PATCH 1/3] fix (spi): fix spi, add hal-1 example Fixed visibility of methods set_bidi and set_tx_only, which were not supposed to be public Fixed SPI reads never happening at all because of a broken pointer cast --- examples/spi-hal-one.rs | 107 ++++++++++++++++++++++++++++++++++++++++ src/spi.rs | 7 ++- 2 files changed, 113 insertions(+), 1 deletion(-) create mode 100644 examples/spi-hal-one.rs diff --git a/examples/spi-hal-one.rs b/examples/spi-hal-one.rs new file mode 100644 index 00000000..fc28ba95 --- /dev/null +++ b/examples/spi-hal-one.rs @@ -0,0 +1,107 @@ +// This example is to test the SPI without any external devices, +// it writes to the MOSI line and logs whatever is received on the MISO line +// +// MISO and MOSI should be connected to make the assertions work + +#![no_main] +#![no_std] + +use crate::hal::{ + prelude::*, + pwr::PwrExt, + rcc::Config, + spi, + stm32::Peripherals, + time::RateExtU32, +}; + +use cortex_m_rt::entry; +use stm32g4xx_hal as hal; + +#[macro_use] +mod utils; +use utils::logger::info; + +#[entry] +fn main() -> ! { + utils::logger::init(); + info!("Logger init"); + + let dp = Peripherals::take().unwrap(); + let rcc = dp.RCC.constrain(); + let pwr = dp.PWR.constrain().freeze(); + let mut rcc = rcc.freeze( + Config::hsi(), + pwr + ); + + // let gpioa = dp.GPIOA.split(&mut rcc); + let gpioa = dp.GPIOA.split(&mut rcc); + let sclk = gpioa.pa5.into_alternate(); + let miso = gpioa.pa6.into_alternate(); + let mosi = gpioa.pa7.into_alternate(); + + // 1/8 SPI/SysClk ratio seems to be the upper limit for continuous transmission + // one byte at a time + // 1/4 works well when writing two packed bytes at once + // At 1/2 the clock stays on for ~80% of the time + let mut spi = dp + .SPI1 + .spi((sclk, miso, mosi), spi::MODE_0, 8.MHz(), &mut rcc); + let mut cs = gpioa.pa8.into_push_pull_output(); + cs.set_high(); + + // Odd number of bits to test packing edge case + const MESSAGE: &[u8] = "Hello world, but longer".as_bytes(); + cs.set_low(); + spi.write(MESSAGE).unwrap(); + SpiBus::::flush(&mut spi).unwrap(); + cs.set_high(); + + let received = &mut [0u8; MESSAGE.len()]; + spi.read(received).unwrap(); + + cortex_m::asm::delay(100); + cs.set_low(); + spi.transfer(received, MESSAGE).unwrap(); + // downside of having 8 and 16 bit impls on the same struct is you have to specify which flush + // impl to call, although internally they call the same function + SpiBus::::flush(&mut spi).unwrap(); + cs.set_high(); + + info!("Received {:?}", core::str::from_utf8(received).ok()); + assert_eq!(MESSAGE, received); + + cs.set_low(); + spi.transfer_in_place(received).unwrap(); + SpiBus::::flush(&mut spi).unwrap(); + cs.set_high(); + + info!("Received {:?}", core::str::from_utf8(received).ok()); + assert_eq!(MESSAGE, received); + + // Switch between 8 and 16 bit frames on the fly + const TX_16B: &[u16] = &[0xf00f, 0xfeef, 0xfaaf]; + let rx_16b = &mut [0u16; TX_16B.len()]; + + cs.set_low(); + spi.transfer(rx_16b, TX_16B).unwrap(); + // internally works the same as SpiBus::::flush() + SpiBus::::flush(&mut spi).unwrap(); + cs.set_high(); + + info!("Received {:?}", rx_16b); + assert_eq!(TX_16B, rx_16b); + + cs.set_low(); + spi.transfer_in_place(rx_16b).unwrap(); + SpiBus::::flush(&mut spi).unwrap(); + cs.set_high(); + + info!("Received {:?}", rx_16b); + assert_eq!(TX_16B, rx_16b); + + loop { + cortex_m::asm::nop(); + } +} diff --git a/src/spi.rs b/src/spi.rs index f17fef18..f2201d2f 100644 --- a/src/spi.rs +++ b/src/spi.rs @@ -186,7 +186,7 @@ impl Spi { fn read_unchecked(&mut self) -> W { // NOTE(read_volatile) read only 1 byte (the svd2rust API only allows // reading a half-word) - unsafe { ptr::read_volatile(&self.spi.dr() as *const _ as *const W) } + unsafe { ptr::read_volatile(self.spi.dr().as_ptr() as *const W) } } #[inline] fn write_unchecked(&mut self, word: W) { @@ -194,12 +194,16 @@ impl Spi { let dr = self.spi.dr().as_ptr() as *mut W; unsafe { ptr::write_volatile(dr, word) }; } + /// disables rx #[inline] pub fn set_tx_only(&mut self) { + // very counter-intuitively, setting spi bidi mode on disables rx while transmitting + // it's made for half-duplex spi, which they called bidirectional in the manual self.spi .cr1() .modify(|_, w| w.bidimode().bidirectional().bidioe().output_enabled()); } + /// re-enables rx if it was disabled #[inline] pub fn set_bidi(&mut self) { self.spi @@ -314,6 +318,7 @@ impl> embedded_hal::spi::SpiBus for Spi Date: Wed, 30 Jul 2025 11:47:00 +0200 Subject: [PATCH 2/3] spi: use hal1 in example, make spi-hal-one a test --- examples/spi-example.rs | 20 +-- examples/spi-hal-one.rs | 107 ------------- tests/nucleo-g474_w_jumpers.rs | 265 +++++++++++++++++++++------------ 3 files changed, 183 insertions(+), 209 deletions(-) delete mode 100644 examples/spi-hal-one.rs diff --git a/examples/spi-example.rs b/examples/spi-example.rs index 34194167..649062f4 100644 --- a/examples/spi-example.rs +++ b/examples/spi-example.rs @@ -8,7 +8,6 @@ use hal::{ delay::DelayFromCountDownTimer, gpio::{AF5, PA5, PA6, PA7}, - hal_02::spi::FullDuplex, prelude::*, pwr::PwrExt, rcc::Config, @@ -49,17 +48,20 @@ fn main() -> ! { // "Hello world!" let message = b"Hello world!"; - let mut received_byte: u8; + let mut received_msg = [0u8; 12]; loop { - for &byte in message { - cs.set_low(); - spi.send(byte).unwrap(); - received_byte = nb::block!(FullDuplex::read(&mut spi)).unwrap(); - cs.set_high(); + cs.set_low(); + // write only, nothing is received + spi.write(message).unwrap(); + cs.set_high(); + + cs.set_low(); + // transmit and receive at the same time + spi.transfer(&mut received_msg, message).unwrap(); + info!("{received_msg:?}"); + cs.set_high(); - info!("{}", received_byte as char); - } delay_tim2.delay_ms(1000); } } diff --git a/examples/spi-hal-one.rs b/examples/spi-hal-one.rs deleted file mode 100644 index fc28ba95..00000000 --- a/examples/spi-hal-one.rs +++ /dev/null @@ -1,107 +0,0 @@ -// This example is to test the SPI without any external devices, -// it writes to the MOSI line and logs whatever is received on the MISO line -// -// MISO and MOSI should be connected to make the assertions work - -#![no_main] -#![no_std] - -use crate::hal::{ - prelude::*, - pwr::PwrExt, - rcc::Config, - spi, - stm32::Peripherals, - time::RateExtU32, -}; - -use cortex_m_rt::entry; -use stm32g4xx_hal as hal; - -#[macro_use] -mod utils; -use utils::logger::info; - -#[entry] -fn main() -> ! { - utils::logger::init(); - info!("Logger init"); - - let dp = Peripherals::take().unwrap(); - let rcc = dp.RCC.constrain(); - let pwr = dp.PWR.constrain().freeze(); - let mut rcc = rcc.freeze( - Config::hsi(), - pwr - ); - - // let gpioa = dp.GPIOA.split(&mut rcc); - let gpioa = dp.GPIOA.split(&mut rcc); - let sclk = gpioa.pa5.into_alternate(); - let miso = gpioa.pa6.into_alternate(); - let mosi = gpioa.pa7.into_alternate(); - - // 1/8 SPI/SysClk ratio seems to be the upper limit for continuous transmission - // one byte at a time - // 1/4 works well when writing two packed bytes at once - // At 1/2 the clock stays on for ~80% of the time - let mut spi = dp - .SPI1 - .spi((sclk, miso, mosi), spi::MODE_0, 8.MHz(), &mut rcc); - let mut cs = gpioa.pa8.into_push_pull_output(); - cs.set_high(); - - // Odd number of bits to test packing edge case - const MESSAGE: &[u8] = "Hello world, but longer".as_bytes(); - cs.set_low(); - spi.write(MESSAGE).unwrap(); - SpiBus::::flush(&mut spi).unwrap(); - cs.set_high(); - - let received = &mut [0u8; MESSAGE.len()]; - spi.read(received).unwrap(); - - cortex_m::asm::delay(100); - cs.set_low(); - spi.transfer(received, MESSAGE).unwrap(); - // downside of having 8 and 16 bit impls on the same struct is you have to specify which flush - // impl to call, although internally they call the same function - SpiBus::::flush(&mut spi).unwrap(); - cs.set_high(); - - info!("Received {:?}", core::str::from_utf8(received).ok()); - assert_eq!(MESSAGE, received); - - cs.set_low(); - spi.transfer_in_place(received).unwrap(); - SpiBus::::flush(&mut spi).unwrap(); - cs.set_high(); - - info!("Received {:?}", core::str::from_utf8(received).ok()); - assert_eq!(MESSAGE, received); - - // Switch between 8 and 16 bit frames on the fly - const TX_16B: &[u16] = &[0xf00f, 0xfeef, 0xfaaf]; - let rx_16b = &mut [0u16; TX_16B.len()]; - - cs.set_low(); - spi.transfer(rx_16b, TX_16B).unwrap(); - // internally works the same as SpiBus::::flush() - SpiBus::::flush(&mut spi).unwrap(); - cs.set_high(); - - info!("Received {:?}", rx_16b); - assert_eq!(TX_16B, rx_16b); - - cs.set_low(); - spi.transfer_in_place(rx_16b).unwrap(); - SpiBus::::flush(&mut spi).unwrap(); - cs.set_high(); - - info!("Received {:?}", rx_16b); - assert_eq!(TX_16B, rx_16b); - - loop { - cortex_m::asm::nop(); - } -} diff --git a/tests/nucleo-g474_w_jumpers.rs b/tests/nucleo-g474_w_jumpers.rs index 6b3e5805..9605ee70 100644 --- a/tests/nucleo-g474_w_jumpers.rs +++ b/tests/nucleo-g474_w_jumpers.rs @@ -2,46 +2,122 @@ #![no_main] #![allow(clippy::uninlined_format_args)] -// Requires a jumper from A1<->A2 (arduino naming) aka PA1<->PA4 +//! Requires jumpers between +//! - A1<->A2 (arduino naming) aka PA1<->PA4 +//! - D11<->D12 (arduino naming) aka PA6<->PA7 #[path = "../examples/utils/mod.rs"] mod utils; -use utils::logger::println; - mod common; -use stm32g4xx_hal::adc::{self, AdcClaim, AdcCommonExt}; -use stm32g4xx_hal::comparator::{self, ComparatorSplit}; -use stm32g4xx_hal::dac::{self, DacExt, DacOut}; -use stm32g4xx_hal::delay::{self, SYSTDelayExt}; -use stm32g4xx_hal::gpio::{self, GpioExt}; -use stm32g4xx_hal::opamp::{self, Opamp1, OpampEx}; -use stm32g4xx_hal::rcc::{self, RccExt}; - -use hal::stm32; -use stm32g4xx_hal as hal; - #[embedded_test::tests] mod tests { + use crate::utils::logger::{info, println}; + use embedded_hal::delay::DelayNs; use stm32g4xx_hal::{ - adc::{self}, - comparator::{self, ComparatorExt}, - dac::DacOut, - opamp::{self, IntoFollower, IntoPga}, + adc::{self, AdcClaim, AdcCommonExt}, + comparator::{self, ComparatorExt, ComparatorSplit}, + dac::{self, DacExt, DacOut}, + delay, gpio, + opamp::{self, IntoFollower, IntoPga, Opamp1}, + pac, + prelude::*, + rcc, spi, stasis::Freeze, + time::RateExtU32, }; use crate::VREF_ADC_BITS; + type Spi1 = spi::Spi< + pac::SPI1, + ( + gpio::gpioa::PA5, + gpio::gpioa::PA6, + gpio::gpioa::PA7, + ), + >; + struct Peripherals { + opamp: opamp::Disabled, + comp: comparator::COMP1, + value_dac: dac::Dac1Ch1<{ dac::M_EXT_PIN }, dac::Enabled>, + ref_dac: dac::Dac3Ch1<{ dac::M_INT_SIG }, dac::Enabled>, + adc: adc::Adc, + pa1: gpio::gpioa::PA1, + pa2: gpio::gpioa::PA2, + rcc: rcc::Rcc, + delay: delay::SystDelay, + spi: Spi1, + cs: gpio::gpioa::PA8>, + } + + #[init] + fn init() -> Peripherals { + //op1+ PA1 -> A1 + //DAC1_OUT1 PA4 -> A2 + + let cp = pac::CorePeripherals::take().unwrap(); + let dp = pac::Peripherals::take().unwrap(); + let mut rcc = dp.RCC.constrain(); + let mut delay = cp.SYST.delay(&rcc.clocks); + + let adc12_common = dp.ADC12_COMMON.claim(Default::default(), &mut rcc); + let adc = adc12_common.claim(dp.ADC1, &mut delay); + + let gpioa = dp.GPIOA.split(&mut rcc); + let pa1 = gpioa.pa1.into_analog(); + let pa2 = gpioa.pa2.into_analog(); + let pa4 = gpioa.pa4.into_analog(); + + let dac1ch1 = dp.DAC1.constrain(pa4, &mut rcc); + let dac3ch1 = dp.DAC3.constrain(dac::Dac3IntSig1, &mut rcc); + + let mut value_dac = dac1ch1.calibrate_buffer(&mut delay).enable(&mut rcc); + let mut ref_dac = dac3ch1.enable(&mut rcc); // TODO: should calibrate_buffer be available on dacs without outputs? + value_dac.set_value(0); + ref_dac.set_value(0); + + let (opamp, ..) = dp.OPAMP.split(&mut rcc); + let (comp, ..) = dp.COMP.split(&mut rcc); + + let sclk = gpioa.pa5.into_alternate(); + let miso = gpioa.pa6.into_alternate(); + let mosi = gpioa.pa7.into_alternate(); + + // 1/8 SPI/SysClk ratio seems to be the upper limit for continuous transmission + // one byte at a time + // 1/4 works well when writing two packed bytes at once + // At 1/2 the clock stays on for ~80% of the time + let spi = dp + .SPI1 + .spi((sclk, miso, mosi), spi::MODE_0, 8.MHz(), &mut rcc); + let mut cs = gpioa.pa8.into_push_pull_output(); + cs.set_high(); + + Peripherals { + opamp, + comp, + value_dac, + ref_dac, + adc, + pa1, + pa2, + rcc, + delay, + spi, + cs, + } + } + #[test] fn dac() { use super::*; // TODO: Is it ok to steal these? - let cp = unsafe { stm32::CorePeripherals::steal() }; - let dp = unsafe { stm32::Peripherals::steal() }; + let cp = unsafe { pac::CorePeripherals::steal() }; + let dp = unsafe { pac::Peripherals::steal() }; let mut rcc = dp.RCC.constrain(); let mut delay = cp.SYST.delay(&rcc.clocks); @@ -50,7 +126,7 @@ mod tests { let pa4 = gpioa.pa4.into_analog(); let dac1ch1 = dp.DAC1.constrain(pa4, &mut rcc); - let gpioa = unsafe { &*stm32::GPIOA::PTR }; + let gpioa = unsafe { &*pac::GPIOA::PTR }; // dac_manual will have its value set manually let mut dac = dac1ch1.calibrate_buffer(&mut delay).enable(&mut rcc); @@ -65,15 +141,15 @@ mod tests { } #[test] - fn opamp_follower_dac_adc() { - let super::Peripherals { + fn opamp_follower_dac_adc(state: Peripherals) { + let Peripherals { opamp, mut value_dac, mut adc, pa1, mut delay, .. - } = super::setup_opamp_comp_dac(); + } = state; let opamp = opamp.follower(pa1); delay.delay_ms(10); @@ -92,8 +168,8 @@ mod tests { } #[test] - fn opamp_follower_ext_pin_dac_adc() { - let super::Peripherals { + fn opamp_follower_ext_pin_dac_adc(state: Peripherals) { + let Peripherals { opamp, mut value_dac, mut adc, @@ -101,7 +177,7 @@ mod tests { pa2, mut delay, .. - } = super::setup_opamp_comp_dac(); + } = state; let (pa2, [pa2_token]) = pa2.freeze(); let _opamp = opamp.follower(pa1).enable_output(pa2_token); delay.delay_ms(10); @@ -121,15 +197,15 @@ mod tests { } #[test] - fn opamp_pga_dac_adc() { - let super::Peripherals { + fn opamp_pga_dac_adc(state: Peripherals) { + let Peripherals { opamp, mut value_dac, mut adc, pa1, mut delay, .. - } = super::setup_opamp_comp_dac(); + } = state; let opamp = opamp.pga(pa1, opamp::Gain::Gain2); let sample_time = adc::config::SampleTime::Cycles_640_5; @@ -154,15 +230,15 @@ mod tests { /// | / /// #[test] - fn comp_dac_vref() { - let super::Peripherals { + fn comp_dac_vref(state: Peripherals) { + let Peripherals { comp, mut value_dac, pa1, rcc, mut delay, .. - } = super::setup_opamp_comp_dac(); + } = state; let r = comparator::refint_input::VRefint; let ref_setpoint = VREF_ADC_BITS; @@ -197,15 +273,15 @@ mod tests { /// | / /// #[test] - fn comp_dac_half_vref() { - let super::Peripherals { + fn comp_dac_half_vref(state: Peripherals) { + let Peripherals { comp, mut value_dac, pa1, rcc, mut delay, .. - } = super::setup_opamp_comp_dac(); + } = state; let r = comparator::refint_input::VRefintM12; let ref_setpoint = VREF_ADC_BITS / 2; @@ -240,9 +316,8 @@ mod tests { /// | / /// #[test] - fn comp_dac_dac() { - use super::*; - let super::Peripherals { + fn comp_dac_dac(state: Peripherals) { + let Peripherals { comp, mut value_dac, ref_dac, @@ -250,7 +325,7 @@ mod tests { rcc, mut delay, .. - } = super::setup_opamp_comp_dac(); + } = state; let (mut ref_dac, [comp_ref]) = ref_dac.freeze(); let comp = comp .comparator(pa1, comp_ref, comparator::Config::default(), &rcc.clocks) @@ -277,9 +352,65 @@ mod tests { } } } + + #[test] + fn spi_loopback(state: Peripherals) { + let mut spi = state.spi; + let mut cs = state.cs; + + // Odd number of bits to test packing edge case + const MESSAGE: &[u8] = "Hello world, but longer".as_bytes(); + cs.set_low(); + spi.write(MESSAGE).unwrap(); + SpiBus::::flush(&mut spi).unwrap(); + cs.set_high(); + + let received = &mut [0u8; MESSAGE.len()]; + spi.read(received).unwrap(); + + cortex_m::asm::delay(100); + cs.set_low(); + spi.transfer(received, MESSAGE).unwrap(); + // downside of having 8 and 16 bit impls on the same struct is you have to specify which flush + // impl to call, although internally they call the same function + SpiBus::::flush(&mut spi).unwrap(); + cs.set_high(); + + info!("Received {:?}", core::str::from_utf8(received).ok()); + assert_eq!(MESSAGE, received); + + cs.set_low(); + spi.transfer_in_place(received).unwrap(); + SpiBus::::flush(&mut spi).unwrap(); + cs.set_high(); + + info!("Received {:?}", core::str::from_utf8(received).ok()); + assert_eq!(MESSAGE, received); + + // Switch between 8 and 16 bit frames on the fly + const TX_16B: &[u16] = &[0xf00f, 0xfeef, 0xfaaf]; + let mut rx_16b = [0u16; TX_16B.len()]; + + cs.set_low(); + spi.transfer(&mut rx_16b, TX_16B).unwrap(); + // internally works the same as SpiBus::::flush() + SpiBus::::flush(&mut spi).unwrap(); + cs.set_high(); + + info!("Received {:?}", &rx_16b); + assert_eq!(TX_16B, rx_16b); + + cs.set_low(); + spi.transfer_in_place(&mut rx_16b).unwrap(); + SpiBus::::flush(&mut spi).unwrap(); + cs.set_high(); + + info!("Received {:?}", &rx_16b); + assert_eq!(TX_16B, rx_16b); + } } -fn is_pax_low(gpioa: &stm32::gpioa::RegisterBlock, x: u8) -> bool { +fn is_pax_low(gpioa: &stm32g4xx_hal::pac::gpioa::RegisterBlock, x: u8) -> bool { gpioa.idr().read().idr(x).is_low() } @@ -287,55 +418,3 @@ fn is_pax_low(gpioa: &stm32::gpioa::RegisterBlock, x: u8) -> bool { /// Vref+ = 3.3V for nucleo /// 1.212V/3.3V*4095 = 1504 adc value const VREF_ADC_BITS: u16 = 1504; - -struct Peripherals { - opamp: opamp::Disabled, - comp: comparator::COMP1, - value_dac: dac::Dac1Ch1<{ dac::M_EXT_PIN }, dac::Enabled>, - ref_dac: dac::Dac3Ch1<{ dac::M_INT_SIG }, dac::Enabled>, - adc: adc::Adc, - pa1: gpio::gpioa::PA1, - pa2: gpio::gpioa::PA2, - rcc: rcc::Rcc, - delay: delay::SystDelay, -} - -fn setup_opamp_comp_dac() -> Peripherals { - //op1+ PA1 -> A1 - //DAC1_OUT1 PA4 -> A2 - - let cp = stm32::CorePeripherals::take().unwrap(); - let dp = stm32::Peripherals::take().unwrap(); - let mut rcc = dp.RCC.constrain(); - let mut delay = cp.SYST.delay(&rcc.clocks); - - let adc12_common = dp.ADC12_COMMON.claim(Default::default(), &mut rcc); - let adc = adc12_common.claim(dp.ADC1, &mut delay); - - let gpioa = dp.GPIOA.split(&mut rcc); - let pa1 = gpioa.pa1.into_analog(); - let pa2 = gpioa.pa2.into_analog(); - let pa4 = gpioa.pa4.into_analog(); - - let dac1ch1 = dp.DAC1.constrain(pa4, &mut rcc); - let dac3ch1 = dp.DAC3.constrain(dac::Dac3IntSig1, &mut rcc); - - let mut value_dac = dac1ch1.calibrate_buffer(&mut delay).enable(&mut rcc); - let mut ref_dac = dac3ch1.enable(&mut rcc); // TODO: should calibrate_buffer be available on dacs without outputs? - value_dac.set_value(0); - ref_dac.set_value(0); - - let (opamp, ..) = dp.OPAMP.split(&mut rcc); - let (comp, ..) = dp.COMP.split(&mut rcc); - Peripherals { - opamp, - comp, - value_dac, - ref_dac, - adc, - pa1, - pa2, - rcc, - delay, - } -} From 775b8e5186a1367d8c2a23008550a20ceba186a3 Mon Sep 17 00:00:00 2001 From: techmccat Date: Wed, 30 Jul 2025 12:07:13 +0200 Subject: [PATCH 3/3] spi example: replace log with utils::logger --- examples/spi-example.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/spi-example.rs b/examples/spi-example.rs index 649062f4..bfbb3ef7 100644 --- a/examples/spi-example.rs +++ b/examples/spi-example.rs @@ -18,8 +18,8 @@ use hal::{ }; use cortex_m_rt::entry; -use log::info; use stm32g4xx_hal as hal; +use utils::logger::info; #[macro_use] mod utils; @@ -59,7 +59,7 @@ fn main() -> ! { cs.set_low(); // transmit and receive at the same time spi.transfer(&mut received_msg, message).unwrap(); - info!("{received_msg:?}"); + info!("{:?}", &received_msg); cs.set_high(); delay_tim2.delay_ms(1000);