From 8e6ed92eea495839b1cc8a4c1e1096991102b86e Mon Sep 17 00:00:00 2001 From: Sergey Savelyev Date: Sat, 20 Sep 2025 10:38:02 -0700 Subject: [PATCH] cleans up lifetimes and ownership --- Cargo.lock | 25 ++++++++++++++--- flight/Cargo.toml | 5 ++-- flight/src/hardware/mcp23017/mod.rs | 43 +++++++++++++++++++++++++---- flight/src/hardware/mcp3208/mod.rs | 14 ++++++++-- flight/src/hardware/mod.rs | 9 ++++-- flight/src/hardware/raspi/mod.rs | 41 ++++++++++++++------------- flight/src/lib.rs | 37 ++++++++++++++++++++----- flight/src/logger.rs | 4 +-- 8 files changed, 134 insertions(+), 44 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 52064f5..ca7d15c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -85,6 +85,12 @@ version = "0.8.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "773648b94d0e5d620f64f280777445740e61fe701025087ec8b57f45c791888b" +[[package]] +name = "critical-section" +version = "1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "790eea4361631c5e7d22598ecd5723ff611904e3344ce8720784c93e3d83d40b" + [[package]] name = "embedded-hal" version = "0.2.7" @@ -101,6 +107,16 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "361a90feb7004eca4019fb28352a9465666b24f840f5c3cddf0ff13920590b89" +[[package]] +name = "embedded-hal-bus" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "513e0b3a8fb7d3013a8ae17a834283f170deaf7d0eeab0a7c1a36ad4dd356d22" +dependencies = [ + "critical-section", + "embedded-hal 1.0.0", +] + [[package]] name = "embedded-hal-mock" version = "0.11.1" @@ -243,13 +259,14 @@ dependencies = [ "anyhow", "chrono", "embedded-hal 1.0.0", + "embedded-hal-bus", "embedded-hal-mock", "fern", "hex", "log", "nalgebra", "num-traits", - "rppal", + "rpi-pal", "thiserror", ] @@ -401,10 +418,10 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "60a357793950651c4ed0f3f52338f53b2f809f32d83a07f72909fa13e4c6c1e3" [[package]] -name = "rppal" -version = "0.22.1" +name = "rpi-pal" +version = "0.22.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c1ce3b019009cff02cb6b0e96e7cc2e5c5b90187dc1a490f8ef1521d0596b026" +checksum = "aed1991fa7d497e81f82b7eac494ad7d1f58d3b5f7fd1a13e30867375dfbc553" dependencies = [ "embedded-hal 0.2.7", "embedded-hal 1.0.0", diff --git a/flight/Cargo.toml b/flight/Cargo.toml index 8398a34..3d62180 100644 --- a/flight/Cargo.toml +++ b/flight/Cargo.toml @@ -9,8 +9,9 @@ fern = "0.7.1" log = "0.4.27" chrono = "0.4.40" embedded-hal = "1.0.0" +embedded-hal-bus = { version = "0.3.0", features = ["std"] } embedded-hal-mock = { version = "0.11.1", optional = true } -rppal = { version = "0.22.1", features = ["hal"], optional = true } +rpi-pal = { version = "0.22.2", features = ["hal"], optional = true } nalgebra = "0.33.2" hex = "0.4.3" thiserror = "2.0.12" @@ -20,5 +21,5 @@ num-traits = "0.2.19" embedded-hal-mock = { version = "0.11.1" } [features] -raspi = ["dep:rppal"] +raspi = ["dep:rpi-pal"] sim = ["dep:embedded-hal-mock"] diff --git a/flight/src/hardware/mcp23017/mod.rs b/flight/src/hardware/mcp23017/mod.rs index 66ed9fc..ce6bd8c 100644 --- a/flight/src/hardware/mcp23017/mod.rs +++ b/flight/src/hardware/mcp23017/mod.rs @@ -1,15 +1,30 @@ +use std::fmt::{Debug, Formatter}; +use std::time::Instant; use anyhow::{bail, Result}; use embedded_hal::i2c::I2c; +use log::{info, trace}; use crate::hardware::error::I2cError; -pub struct Mcp23017 { +pub trait Mcp23017 { + fn init(&mut self) -> Result<()>; + fn set_pin(&mut self, pin: u8, value: bool) -> Result<()>; + fn flush(&mut self) -> Result<()>; +} + +pub struct Mcp23017Driver { i2c: I2C, address: u8, bank: [u8; 2], dirty: bool, } -impl Mcp23017 +impl Debug for Mcp23017Driver { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!(f, "Mcp23017Driver {{ address: {} }}", self.address) + } +} + +impl Mcp23017Driver where I2C: I2c, I2C::Error : Send, @@ -17,6 +32,7 @@ where I2C::Error : 'static { pub fn new(i2c: I2C, address: u8) -> Self { + trace!("Mcp23017Driver::new(i2c, address: {address:07b})"); Self { i2c, address, @@ -24,12 +40,27 @@ where dirty: false, } } +} + + +impl Mcp23017 for Mcp23017Driver +where + I2C: I2c, + I2C::Error : Send, + I2C::Error : Sync, + I2C::Error : 'static +{ + fn init(&mut self) -> Result<()> { + trace!("Mcp23017Driver::init(self: {self:?})"); + // Set each pin as an output (Addresses 0x00 & 0x01) + let data: [u8; _] = [0x00, 0x00, 0x00]; + self.i2c.write(self.address, &data).map_err(I2cError)?; - pub fn init(&self) -> Result<()> { Ok(()) } - pub fn set_pin(&mut self, pin: u8, value: bool) -> Result<()> { + fn set_pin(&mut self, pin: u8, value: bool) -> Result<()> { + trace!("Mcp23017Driver::set_pin(self: {self:?}, pin: {pin}, value: {value})"); let (pin_bank, dirty_flag, pin_index) = match pin { 0..8 => { (&mut self.bank[0], &mut self.dirty, pin as u32) @@ -53,9 +84,11 @@ where Ok(()) } - pub fn flush(&mut self) -> Result<()> { + fn flush(&mut self) -> Result<()> { + trace!("Mcp23017Driver::flush(self: {self:?})"); if self.dirty { let data: [u8; _] = [0x12, self.bank[0], self.bank[1]]; + // This blocks while writing self.i2c.write(self.address, &data).map_err(I2cError)?; } Ok(()) diff --git a/flight/src/hardware/mcp3208/mod.rs b/flight/src/hardware/mcp3208/mod.rs index 81206ed..29ec579 100644 --- a/flight/src/hardware/mcp3208/mod.rs +++ b/flight/src/hardware/mcp3208/mod.rs @@ -1,12 +1,20 @@ -use embedded_hal::spi::{Operation, SpiDevice}; -use anyhow::{ensure, Result}; +use std::fmt::{Debug, Formatter}; use crate::hardware::error::SpiError; +use anyhow::{ensure, Result}; +use embedded_hal::spi::SpiDevice; +use log::trace; pub struct Mcp3208 { spi: SPI, vref: f64 } +impl Debug for Mcp3208 { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!(f, "Mcp3208 {{ vref: {} }}", self.vref) + } +} + impl Mcp3208 where SPI : SpiDevice, @@ -15,6 +23,7 @@ where SPI::Error : 'static, { pub fn new(spi: SPI, vref: f64) -> Self { + trace!("Mcp3208::new(spi, vref: {vref})"); Self { spi, vref @@ -22,6 +31,7 @@ where } pub fn read_single(&mut self, channel: u8) -> Result { + trace!("Mcp3208::read_single(self: {self:?}, channel: {channel})"); ensure!(channel < 8, "Invalid Channel {channel}"); // We don't care what the third byte written is diff --git a/flight/src/hardware/mod.rs b/flight/src/hardware/mod.rs index fdd7076..d7fbd44 100644 --- a/flight/src/hardware/mod.rs +++ b/flight/src/hardware/mod.rs @@ -1,8 +1,11 @@ +use crate::hardware::mcp23017::Mcp23017; use anyhow::Result; -use embedded_hal::i2c::I2c; pub trait Hardware { - fn set_mcp0_pin(&self, pin: u8, value: bool) -> Result<()>; + + fn get_mcp23017_a(&self) -> impl Mcp23017; + fn get_mcp23017_b(&self) -> impl Mcp23017; + fn get_battery_voltage(&self) -> Result; } @@ -34,5 +37,5 @@ mod bno085; mod imu; mod error; -mod mcp23017; +pub mod mcp23017; mod mcp3208; diff --git a/flight/src/hardware/raspi/mod.rs b/flight/src/hardware/raspi/mod.rs index dd0da3e..7f210dd 100644 --- a/flight/src/hardware/raspi/mod.rs +++ b/flight/src/hardware/raspi/mod.rs @@ -1,49 +1,52 @@ use std::cell::RefCell; +use std::sync::Mutex; use crate::hardware::Hardware; use anyhow::Result; -use embedded_hal::i2c::SevenBitAddress; -use log::{debug, info}; -use rppal::gpio::Gpio; -use rppal::i2c::I2c; -use rppal::spi::{Bus, Mode, SlaveSelect, Spi}; -use crate::hardware::mcp23017::Mcp23017; +use embedded_hal_bus::i2c::MutexDevice; +use log::{debug, info, trace}; +// use rpi_pal::gpio::Gpio; +use rpi_pal::i2c::I2c; +use rpi_pal::spi::{Bus, Mode, SlaveSelect, Spi}; +use crate::hardware::mcp23017::{Mcp23017, Mcp23017Driver}; use crate::hardware::mcp3208::Mcp3208; -use rppal::spi::SimpleHalSpiDevice; +use rpi_pal::spi::SimpleHalSpiDevice; const CLOCK_1MHZ: u32 = 1_000_000; pub struct RaspiHardware { - gpio: Gpio, - mcp23017_a: RefCell>, + // gpio: Gpio, + i2c_bus: Mutex, mcp3208: RefCell>, } impl RaspiHardware { pub fn new() -> Result { - let device = rppal::system::DeviceInfo::new()?; + trace!("RaspiHardware::new()"); + + let device = rpi_pal::system::DeviceInfo::new()?; info!("Running on {}", device.model()); debug!("SOC: {}", device.soc()); - let mut i2c = I2c::with_bus(1u8)?; - i2c.set_timeout(1000)?; - Ok(Self { - gpio: Gpio::new()?, - mcp23017_a: Mcp23017::new(i2c, 0b0100000).into(), + // gpio: Gpio::new()?, + i2c_bus: Mutex::new(I2c::with_bus(0u8)?), mcp3208: Mcp3208::new(SimpleHalSpiDevice::new(Spi::new( Bus::Spi1, SlaveSelect::Ss1, CLOCK_1MHZ, Mode::Mode0, - )?), 3.3f64).into() + )?), 3.3f64).into(), }) } } impl Hardware for RaspiHardware { - fn set_mcp0_pin(&self, pin: u8, value: bool) -> Result<()> { - self.mcp23017_a.borrow_mut().set_pin(pin, value)?; - self.mcp23017_a.borrow_mut().flush() + fn get_mcp23017_a(&self) -> impl Mcp23017 { + Mcp23017Driver::new(MutexDevice::new(&self.i2c_bus), 0b0100000) + } + + fn get_mcp23017_b(&self) -> impl Mcp23017 { + Mcp23017Driver::new(MutexDevice::new(&self.i2c_bus), 0b0100001) } fn get_battery_voltage(&self) -> Result { diff --git a/flight/src/lib.rs b/flight/src/lib.rs index 1d505dc..9088259 100644 --- a/flight/src/lib.rs +++ b/flight/src/lib.rs @@ -5,6 +5,7 @@ use crate::logger::setup_logger; use anyhow::Result; use log::info; use crate::hardware::Hardware; +use crate::hardware::mcp23017::Mcp23017; mod hardware; mod logger; @@ -18,14 +19,36 @@ pub fn run() -> Result<()> { let hal = initialize()?; - for i in 0..100 { - sleep(Duration::from_millis(1000)); - info!("Battery Voltage: {}", hal.get_battery_voltage()?); - } + let mut mcp23017_a = hal.get_mcp23017_a(); + let mut mcp23017_b = hal.get_mcp23017_b(); - // hal.set_mcp0_pin(15u8, true)?; - // sleep(Duration::from_secs(1)); - // hal.set_mcp0_pin(15u8, false)?; + info!("Battery Voltage: {}", hal.get_battery_voltage()?); + + mcp23017_a.init()?; + mcp23017_b.init()?; + + mcp23017_a.set_pin(7u8, true)?; + mcp23017_a.flush()?; + + sleep(Duration::from_secs(1)); + + mcp23017_b.set_pin(7u8, true)?; + mcp23017_b.flush()?; + + sleep(Duration::from_secs(1)); + + mcp23017_a.set_pin(7u8, false)?; + mcp23017_a.flush()?; + + sleep(Duration::from_secs(1)); + + mcp23017_b.set_pin(7u8, false)?; + mcp23017_b.flush()?; + + // Explicitly drop these to allow the borrow checker to be sure that + // dropping the hal is safe + drop(mcp23017_a); + drop(mcp23017_b); drop(hal); diff --git a/flight/src/logger.rs b/flight/src/logger.rs index 229e7ea..6a9db07 100644 --- a/flight/src/logger.rs +++ b/flight/src/logger.rs @@ -2,7 +2,7 @@ use anyhow::Result; use std::env; use std::fs::create_dir_all; use std::str::FromStr; -use log::trace; +use log::debug; pub fn setup_logger() -> Result<()> { let log_file = env::var("LOG_FILE").or_else(|_| { @@ -37,6 +37,6 @@ pub fn setup_logger() -> Result<()> { .chain(fern::log_file(log_file.clone())?) .apply()?; - trace!("Logging to {} at level {}", log_file, log_level); + debug!("Logging to {} at level {}", log_file, log_level); Ok(()) }