eldruin / xca9548a-rs

Platform-agnostic Rust driver for the TCA954xA and PCA954xA I2C switch/multiplexer family.
Apache License 2.0
3 stars 8 forks source link

`split` API prevents using interrupts #4

Open UnTraDe opened 1 month ago

UnTraDe commented 1 month ago

It seems to me that get_interrupt_status() doesn't need to borrow self as mutable. Currently it is impossible to use it after you use split and pass it on to other drivers:

image

Since get_interrupt_status() use self.do_on_acquired(), is there any reason for it to need &mut self? Am I missing something here? And if I do, what would be the correct usage for this API? I'd be happy to send a PR if needed 😀

eldruin commented 3 weeks ago

Hi, could you post the rest of the code? Which Rust version are you using?

UnTraDe commented 3 weeks ago

Rust version 1.80

use crate::modules::sampler::AtomicTimestamp;

use super::{ReadySignal, Sampler};
use crate::modules::utils::ms_to_freertos_ticks;
use crate::modules::utils::TaskHandle;
use crossbeam::sync::Parker;
use embedded_hal::i2c::I2c;
use esp_idf_svc::hal::peripheral::Peripheral;
use esp_idf_svc::hal::{
    delay::Ets,
    gpio::*,
    i2c::{I2cConfig, I2cDriver},
    task::{notify, wait_notification},
};
use nau7802::Nau7802;

use std::sync::{
    atomic::{AtomicI32, Ordering},
    Arc, Mutex,
};
use xca9548a::Xca9545a;
pub const LOAD_CELLS: usize = 3;

pub struct Nau {
    samples: [Arc<AtomicI32>; LOAD_CELLS],
    sampling_rate: super::SamplingRate,
    ready_signal: Arc<Mutex<ReadySignal>>,
    timestamp: Arc<AtomicTimestamp>,
    metrics: super::Metrics,
}

impl Nau {
    pub fn new<I2C: esp_idf_svc::hal::i2c::I2c>(
        i2c: impl Peripheral<P = I2C> + 'static,
        sda: impl Peripheral<P = impl InputPin + OutputPin> + 'static,
        scl: impl Peripheral<P = impl InputPin + OutputPin> + 'static,
        mux_interrupt_pin: Gpio15,
        sampling_rate: super::SamplingRate,
    ) -> anyhow::Result<Self> {
        log::info!("init nau sampler");
        let cfg = I2cConfig {
            baudrate: esp_idf_svc::hal::units::Hertz(100_000),
            sda_pullup_enabled: false,
            scl_pullup_enabled: false,
            timeout: None,
            intr_flags: enumset::EnumSet::empty(), // TODO(odel): set the intr flags
        };
        log::info!("init i2c driver");
        let inst = I2cDriver::new(i2c, sda, scl, &cfg).unwrap();
        log::info!("init xca9545a");
        let xca = Xca9545a::new(inst, xca9548a::SlaveAddr::Alternative(false, false, false));
        let samples = [
            Arc::new(AtomicI32::new(0)),
            Arc::new(AtomicI32::new(0)),
            Arc::new(AtomicI32::new(0)),
        ];

        let timestamp = Arc::new(AtomicTimestamp::default());
        let ready_signal = Arc::new(Mutex::new(ReadySignal::from_atomics(
            timestamp.clone(),
            samples.clone().to_vec(),
        )));

        let samples_copy = samples.clone();
        let timestamp_copy = timestamp.clone();
        let ready_signal_copy = ready_signal.clone();
        log::info!("spawning sample loop");
        let mut mux_interrupt_driver: PinDriver<Gpio15, Input> =
            PinDriver::input(mux_interrupt_pin).unwrap();

        std::thread::spawn(move || {
            match sample_loop(
                xca,
                sampling_rate,
                samples_copy,
                timestamp_copy,
                ready_signal_copy,
                &mut mux_interrupt_driver,
            ) {
                Ok(_) => log::info!("sample loop exited successfully"),
                Err(e) => log::error!("sample loop exited with error: {:?}", e), // TODO retry and notify someone somehow
            }
        });

        Ok(Self {
            sampling_rate,
            samples,
            ready_signal,
            timestamp,
            metrics: super::Metrics::default(),
        })
    }
}

fn sample_loop(
    mux: Xca9545a<I2cDriver<'_>>,
    sampling_rate: super::SamplingRate,
    samples: [Arc<AtomicI32>; LOAD_CELLS],
    timestamp: Arc<AtomicTimestamp>,
    ready_signal: Arc<Mutex<ReadySignal>>,
    mux_interrupt_driver: &mut PinDriver<Gpio15, Input>,
) -> anyhow::Result<()> {
    let parts = mux.split();

    log::info!("initing nau0...");

    let mut nau0 = Nau7802::new(parts.i2c0, &mut Ets).unwrap();

    // we repeat the start sequence since we don't want to set the AVDDS bit, which the library does on init while calling set_ldo
    startup_sequence(&mut nau0, sampling_rate.into())
        .map_err(|e| anyhow::anyhow!("nau7802 startup failed: {e:?}"))?;

    log::info!("initing nau1...");
    let mut nau1 = Nau7802::new(parts.i2c1, &mut Ets).unwrap();

    startup_sequence(&mut nau1, sampling_rate.into())
        .map_err(|e| anyhow::anyhow!("nau7802 startup failed: {e:?}"))?;

    log::info!("initing nau2...");
    let mut nau2 = Nau7802::new(parts.i2c2, &mut Ets).unwrap();

    startup_sequence(&mut nau2, sampling_rate.into())
        .map_err(|e| anyhow::anyhow!("nau7802 startup failed: {e:?}"))?;

    mux_interrupt_driver.set_interrupt_type(InterruptType::LowLevel)?;
    let handle = TaskHandle(esp_idf_svc::hal::task::current().unwrap());

    unsafe {
        mux_interrupt_driver.subscribe(move || {
            let _ = &handle;
            notify(handle.0, std::num::NonZeroU32::new_unchecked(1));
        })?;
    }

    mux_interrupt_driver.enable_interrupt()?;

    loop {
        let is_notified_on_time = wait_notification(ms_to_freertos_ticks(20));
        if is_notified_on_time.is_some() {
            let mux_interrupt_status_result = mux.get_interrupt_status();

            if let Ok(mux_interrupt_status) = mux_interrupt_status_result {
                if mux_interrupt_status & 0b001 != 0 {
                    match nau0.read() {
                        Ok(data) => {
                            samples[0].store(data, std::sync::atomic::Ordering::SeqCst);
                            let raw_ts = unsafe { esp_idf_svc::sys::esp_timer_get_time() };
                            timestamp
                                .milliseconds
                                .store((raw_ts / 1000) as _, Ordering::SeqCst);
                            timestamp
                                .microseconds
                                .store((raw_ts % 1000) as _, Ordering::SeqCst);
                            ready_signal.lock().unwrap().set_ready(0);
                        }
                        Err(nb::Error::WouldBlock) => {
                            continue;
                        }
                        Err(e) => {
                            log::error!("error: {:?}", e);
                        }
                    }
                }

                if mux_interrupt_status & 0b010 != 0 {
                    match nau1.read() {
                        Ok(data) => {
                            samples[1].store(data, std::sync::atomic::Ordering::SeqCst);
                            let raw_ts = unsafe { esp_idf_svc::sys::esp_timer_get_time() };
                            timestamp
                                .milliseconds
                                .store((raw_ts / 1000) as _, Ordering::SeqCst);
                            timestamp
                                .microseconds
                                .store((raw_ts % 1000) as _, Ordering::SeqCst);
                            ready_signal.lock().unwrap().set_ready(1);
                        }
                        Err(nb::Error::WouldBlock) => {
                            continue;
                        }
                        Err(e) => {
                            log::error!("error: {:?}", e);
                            // TODO notify about it somehow (metrics?)
                        }
                    }
                }

                if mux_interrupt_status & 0b100 != 0 {
                    match nau2.read() {
                        Ok(data) => {
                            samples[2].store(data, std::sync::atomic::Ordering::SeqCst);
                            let raw_ts = unsafe { esp_idf_svc::sys::esp_timer_get_time() };
                            timestamp
                                .milliseconds
                                .store((raw_ts / 1000) as _, Ordering::SeqCst);
                            timestamp
                                .microseconds
                                .store((raw_ts % 1000) as _, Ordering::SeqCst);
                            ready_signal.lock().unwrap().set_ready(2);
                        }
                        Err(nb::Error::WouldBlock) => {
                            continue;
                        }
                        Err(e) => {
                            log::error!("error: {:?}", e);
                            // TODO notify about it somehow (metrics?)
                        }
                    }
                }
            }

            mux_interrupt_driver.enable_interrupt()?;
        }
    }
}
eldruin commented 3 weeks ago

Thanks! I have changed the signature of the method to require an immutable reference. Could you try the current git version?

UnTraDe commented 3 weeks ago

It works! Thank you very much!