Separate thread for loop in struct implementation

Question

I'm working with a struct where I need to read the GPIO pin of a Raspberry Pi, and increment a 'register' within the struct every time the pin goes high. Concurrently with this, I would like to be able to sample the register every now and then to see what the current value is.

When implementing this, my thought was to spawn a thread that continuously loops checking if the pin has gone from Low to High, and increment the register from within the thread. Then, from the parent thread, I can read the value of the register and report it.

After doing some research, it seems that a scoped thread would not be the correct implementation of this, because the child thread would never hand over ownership of the register to the parent thread.

Rather, I believe I should use an Arc/Mutex combination guarding the register and only momentarily take control over the lock to increment the register. Is this the correct interpretation of multithreading in Rust?

Assuming the above is correct, I'm unsure of how to implement this in Rust.

struct GpioReader {
    register: Arc<Mutex<i64>>,
    input_pin: Arc<Mutex<InputPin>>,
}

impl GpioReader {
    pub fn new(input_pin: InputPin) -> Self {
        Self {
            register: Arc::New(Mutex::from(0)),
            input_pin: Arc::new(Mutex::from(input_pin))
        }
    }

    pub fn start(&self) {
        let pin = self.input_pin.lock().unwrap(); // ???
        let register = self.register.lock().unwrap(); // ???

        let handle = spawn(move || loop {
            match pin.read() { // ???
                High => register += 1, // ???
                Low => (),
            }

            sleep(Duration::from_millis(SLEEP_TIME));
        });
        handle.join().expect("Failed to join thread.");
    }

    pub fn get_register(&self) -> i64 {
        let reg_val = self.register.lock().unwrap();

        return reg_val;
    }
}

Given the above, how do I declare the pin and register variables in such a way that I can read off the pin and increment the register within the loop? My best guess is I'll have to instantiate some kind of reference to these members of the struct outside of the loop, and then pass the reference into the loop at which point I can use the lock() method of the Arc.

Edit: Using RaspberryPi 3A+ running Raspbian. The InputPin in question is from the rppal crate.

"The GPIO pin of a Raspberry Pi" - What do you mean with that? An actual Raspberry Pi running a Linux operating system? Or something bare-bone, and maybe RP2040 based? I'm asking because InputPin is an embedded-hal thing and doesn't apply to Linux based Raspberry Pis. So, which Raspberry Pi are you trying to program exactly? I am guessing the Raspberry Pi Pico, but without that information, no answer can be constructed. — Finomnis
– Finomnis, Commented Jan 31, 2023 at 19:53
@Finomnis Raspberry Pi 3A+ running 32 bit Raspbian. The rppal crate has an InputPin struct with the method read() to assess the state of the pin. — reeslabree
– reeslabree, Commented Jan 31, 2023 at 20:13
Got it. Independent of that, please provide a minimal reproducible example in the future. Yours is missing use statements, and Arc::New is misspelled. — Finomnis
– Finomnis, Commented Jan 31, 2023 at 20:21
Why do you need Arc for InputPin? Do you intend do use the pin from multiple places in your code simultaneously? — Finomnis
– Finomnis, Commented Jan 31, 2023 at 20:25
Is that thread meant to be a constantly running loop in the background? — PitaJ
– PitaJ, Commented Jan 31, 2023 at 20:31

Finomnis · Accepted Answer · 2023-01-31 20:48:18Z

Mutex<i64> is an anti-pattern. Replace it with AtomicI64.
Arc is meant to be cloned with Arc::clone() to create new references to the same object.
Don't use shared ownership if not necessary. InputPin is only used from within the thread, so move it in instead.
I'm unsure why you do handle.join(). If you want it to continue in the background, don't wait for it with .join().

use std::{
    sync::{
        atomic::{AtomicI64, Ordering},
        Arc,
    },
    thread::{self, sleep},
    time::Duration,
};

use rppal::gpio::InputPin;

struct GpioReader {
    register: Arc<AtomicI64>,
    input_pin: Option<InputPin>,
}

const SLEEP_TIME: Duration = Duration::from_millis(1000);

impl GpioReader {
    pub fn new(input_pin: InputPin) -> Self {
        Self {
            register: Arc::new(AtomicI64::new(0)),
            input_pin: Some(input_pin),
        }
    }

    pub fn start(&mut self) {
        let register = Arc::clone(&self.register);
        let pin = self.input_pin.take().expect("Thread already running!");

        let handle = thread::spawn(move || loop {
            match pin.read() {
                High => {
                    register.fetch_add(1, Ordering::Relaxed);
                }
                Low => (),
            }

            sleep(SLEEP_TIME);
        });
    }

    pub fn get_register(&self) -> i64 {
        self.register.load(Ordering::Relaxed)
    }
}

If you want to stop the thread automatically when the GpioReader object is dropped, you can use Weak to signal it to the thread:

use std::{
    sync::{
        atomic::{AtomicI64, Ordering},
        Arc,
    },
    thread::{self, sleep},
    time::Duration,
};

use rppal::gpio::InputPin;

struct GpioReader {
    register: Arc<AtomicI64>,
    input_pin: Option<InputPin>,
}

const SLEEP_TIME: Duration = Duration::from_millis(1000);

impl GpioReader {
    pub fn new(input_pin: InputPin) -> Self {
        Self {
            register: Arc::new(AtomicI64::new(0)),
            input_pin: Some(input_pin),
        }
    }

    pub fn start(&mut self) {
        let register = Arc::downgrade(&self.register);
        let pin = self.input_pin.take().expect("Thread already running!");

        let handle = thread::spawn(move || loop {
            if let Some(register) = register.upgrade() {
                match pin.read() {
                    High => {
                        register.fetch_add(1, Ordering::Relaxed);
                    }
                    Low => (),
                }

                sleep(SLEEP_TIME);
            } else {
                // Original `register` got dropped, cancel the thread
                break;
            }
        });
    }

    pub fn get_register(&self) -> i64 {
        self.register.load(Ordering::Relaxed)
    }
}

Collectives™ on Stack Overflow

Separate thread for loop in struct implementation

1 Answer 1

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related