cleans up lifetimes and ownership

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"]

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<I2C> {
+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: I2C,
     address: u8,
     bank: [u8; 2],
     dirty: bool,
 }
 
-impl<I2C> Mcp23017<I2C>
+impl<I2C> Debug for Mcp23017Driver<I2C> {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        write!(f, "Mcp23017Driver {{ address: {} }}", self.address)
+    }
+}
+
+impl<I2C> Mcp23017Driver<I2C>
 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<I2C> Mcp23017 for Mcp23017Driver<I2C>
+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(())

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: SPI,
     vref: f64
 }
 
+impl<SPI> Debug for Mcp3208<SPI> {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        write!(f, "Mcp3208 {{ vref: {} }}", self.vref)
+    }
+}
+
 impl<SPI> Mcp3208<SPI>
 where
     SPI : SpiDevice<u8>,
@@ -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<f64> {
+        trace!("Mcp3208::read_single(self: {self:?}, channel: {channel})");
         ensure!(channel < 8, "Invalid Channel {channel}");
 
         // We don't care what the third byte written is

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<f64>;
 }
 
@@ -34,5 +37,5 @@ mod bno085;
 mod imu;
 mod error;
 
-mod mcp23017;
+pub mod mcp23017;
 mod mcp3208;

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<Mcp23017<I2c>>,
+    // gpio: Gpio,
+    i2c_bus: Mutex<I2c>,
     mcp3208: RefCell<Mcp3208<SimpleHalSpiDevice>>,
 }
 
 impl RaspiHardware {
     pub fn new() -> Result<Self> {
-        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<f64> {

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);
 

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(())
 }