SerialBuf reading issue

[sturdy kayak]

I am having an issue of missing bytes and general reliability of connection with my serial port connection. I have created what I believe to be a well commented repo explaining the issue. In short, reading the data via the serialport crate works perfectly, but it doesn't read it by lines. I modified and wrote my own that attempts to read it by lines, however it seems to have difficulties collecting all of the data, and reliably starting. (see Issues.md). If anyone has experience with BufReader and knows what I'm doing wrong and if it will fix my issue with missing bytes in my manually coded section your assistance would be greatly appreciated!

I also understand this is not a Tauri specific question and I will happily remove this post if need be. Thank you!

[tribal glacier]

Trying to understand your code right now 🤔 Hard to fully fix it on my end since I don't have a device to test with but lets see hyaaa...

General tip reading your code btw, make more smaller functions in order to not end up in nesting hell where you're currently like 9 layers deep or something, and make those functions return a Result<T> that you can instead error handle once instead of all over your code, and make use of the ? syntax to raise the error, and use syntaxes like if let Ok(value) = something() {} in any cases where you don't care about the error/none states. Just some general tips that will help with code quality and readability because this code is getting a bit hard to follow 🙂

The Current Issue you have in the Issues.md looks like it's just related to that serial_buf gets dropped in the context where you made it, so you either have to move it (will probably complain about that I'm guessing) by removing the & or .clone() it so that it can escape the context, or have stored_buffer be extended by the contents in serial_buf instead so the serial_buf is safe to drop

As a general note on BufReader, it's possible it loses data according to its docs if the same stream is used multiple times, seeing as you've defined things in a loop that may be the reason why data is sometimes incomplete, not sure, hard to follow the code 😅 Also it doesn't convert from bytes to anything, it receives bytes, it only creates strings when you run .lines() on it (like you do), but since it is a potentially failing function the way you've written your code it's supposed to error out when it e.g. lacks some bytes to construct proper utf-8 strings (note ascii, Rust loves utf-8)

[sturdy kayak]

I was reluctant to share my nested hellscape, but its what I had written. I'll refactor this and send another message in here if I still encounter the same issue. I also saw that issue of losing data if its moved multiple times. I didn't think about how the loops could do that. Thank you!

[tribal glacier]

fn read_data(port: SerialPort) -> Result<(), io::Error> {
    println!("Receiving data on {} at {} baud:", &port_name, &baud_rate);
    let mut serial_buf: Vec<u8> = vec![];
    loop {
        let t= port.read(&mut [0; 1])?;
        let mut byte = [0; 1];
        let data =  port.read(&mut byte);
        if byte[0] == b'\n' {
            if serial_buf != stored_reading {
                let utf_string = match std::str::from_utf8(&serial_buf) {
                    Ok(s) => {
                        s
                    },
                    Err(e) => {
                        let _ = io::stdout().write_all(&serial_buf);
                        panic!("Invalid UTF-8 sequence: {}", e);
                    }
                };
                println!("{}", utf_string);
                // replace the old reading with the new reading
                stored_reading = serial_buf.clone();
                // should I clear the buffer?
            }
            serial_buf.clear();
        } else {
            serial_buf.push(byte[0]);
            // for byte in &serial_buf {
            //     println!("{:X}", byte);
            // }
        }
        if serial_buf.len() > 14 {
            serial_buf.clear();
        }
    
        }
    }
}

fn main() {
    let port_name = "COM9"; // port for my computer only. change
    let baud_rate = 1200;
    let mut port = serialport::new(port_name, baud_rate)
        .timeout(Duration::from_millis(80))    //Is this the time given to open the port, or the time given to read from the port?
        .open()
        .expect("failed to get port");

    // Define a vector to hold the bytes we read to compare to for changes
    let mut stored_buffer = "";
    let mut stored_reading: Vec<u8> = vec![];

    match read_data(port) {
        Ok(_) => {},
        Err(error) match error.kind() {
            io::ErrorKind::TimedOut => {},
            _ => {}
        }
    }
}

[tribal glacier]

Not a functioning example, just to illustrate what sort of code structure changes you could make

[tribal glacier]

After cleaning the code a bit it makes it more apparent that you're reading bytes in multiple places, which could definitely be the reason the received data in the end doesn't result in a proper utf-8 string

This part I'm thinking could be what's breaking the string

        let mut byte = [0; 1];
        let data =  port.read(&mut byte);

[sturdy kayak]

would that issue be negated by using the bufReader's .read_until('\n')? I was hoping that by switching to the BufReader I would make some of these issues irrelevant.

I understand! I figured it out conceptually. I'm misusing my serialport instance

[tribal glacier]

.read_until could work but just .lines should work, or using .read_vectored on the SerialPort itself. As far as I can tell most of your manual handling of bytes isn't necessary

[sturdy kayak]

I think I'm seeing that now. And I'm quite glad my manual handling isn't needed. I didn't want to manage it like that. Thank you again, and I will update this thread if this doesn't work.

[sturdy kayak]

use std::time::Duration;
use std::io::{self, Write};
use std::io::{BufReader, BufRead};
use serialport::SerialPort;

fn read_data(port: &mut Box<dyn SerialPort>) -> Result<(), io::Error> {
    let mut reader = BufReader::new(port);
    let mut line = String::new();
    let mut stored_reading = String::new();

    loop {
        if let Ok(_bytes) = reader.read_line(&mut line){
            if line != stored_reading && _bytes == 16 {
                println!("Data: {}", line);
                stored_reading = line.clone();
            }
            line.clear();
        }
    }
}

fn main() {
    let port_name = "COM9"; // port for my computer only. change
    let baud_rate = 1200;
    println!("Connecting on {} with {} baud.", &port_name, &baud_rate);
    if let Ok(mut port) = serialport::new(port_name, baud_rate)
        .timeout(Duration::from_millis(10))
        .open(){
            let _ = read_data(&mut port);
        }
}

I'm not sure about my && bytes == 16, but I need to ensure that I return complete lines, and having a .timeout seems to affect my ability to do that, but without the .timeout the buffer just endlessly fills. However I think that it is working, and I want to severely thank you for your suggestion on error handling with if let Ok() Its made a world of difference in my ability to read my own code

[tribal glacier]

Much more readable 😄 Just note that in the main function you probably want to use a match instead as a top level error catcher, then in read_data you can write let bytes_ = reader.read_line(&mut line)?; instead. The ? syntax automatically raises the error if there is one so your top level error handler gets it thererror state. Note that I put _ after instead of before bytes so it's bytes_ instead of _bytes because a leading _ means that the variable is unused in Rust, which in your case it is, so you suffix the _ instead of prefix it if it's just to avoid keywords 🙂

I think it's not .timeout you want on the builder but .data_bits or .stop_bits perhaps, making it time based seems highly unstable. I'm not the most well versed on serial communication, but I'd assume if you set .data_bits to Eight you should be getting exactly 8 bits per read, and as long as it's ascii communication you'll be fine just turning it into a string, but if you get something more advanced you'll need to maybe create a 16 bit buffer to push to that you try converting to a sting instead, e.g. if you get utf8 based strings
https://docs.rs/serialport/latest/serialport/struct.SerialPortBuilder.html#implementations

[sturdy kayak]

If I don't use timeout it will run for a minute + before printing anything. but when it prints it prints with \n, so I know that it is getting newline characters. I'll play around with it. right now its a proof of concept, I'll keep playing with it.

is this the correct implementation of your match suggestion? namely with the Ok() in the .open{ .. }

use serialport::SerialPort;
use std::time::Duration;
use std::io::{self, BufReader, BufRead};
use tauri::AppHandle;


pub fn read_data(port: &mut Box<dyn SerialPort>, app: AppHandle) -> Result<(), io::Error> {
    let mut reader = BufReader::new(port);
    let mut line = String::new();
    let mut stored_reading = String::new();

    loop {
        let bytes_ = reader.read_line(&mut line)?;
        if line != stored_reading && _bytes == 16 {
            app.emit("new_reading", Some(line.clone()));
            stored_reading = line.clone();
        }
        line.clear();
    }
}

pub fn setup_connection(app: AppHandle) {
    let port_name = "COM9";
    let baud_rate = 1200;
    println!("Connecting on {} with {} baud.", &port_name, &baud_rate);

    match serialport::new(port_name, baud_rate)
        .data_bits(DataBits::Eight)
        .open(){
            Ok(mut port) => {
                let _ = read_data(&mut port);
            },
            Err(e) => {
                eprintln!("Error: {}", e);
            }
        }
}

and on a sidenote, are there any simple examples on how to spawn a new thread?

pub mod serial;

pub fn run() {
    tauri::Builder::default()
        .plugin(tauri_plugin_shell::init())
        // .invoke_handler(tauri::generate_handler![greet])
        .setup(|app| {
            let app_handle = app.handle().clone();
            std::thread::spawn(move || {
                serial::setup_connection(app.handle().clone());
            });
            Ok(())
        })
        .run(tauri::generate_context!())
        .expect("error while running tauri application");
}

I searched the support thread on here and saw that icb recommended spawning each new thread in the setup, and then passing a clone of app handle. But I seem to be doing that wrong. github page

[tribal glacier]

Yea that looks exactly like what I had in mind 😄 Down the line you'll wanna add your own custom Error enum and then having that match statement catch all sorts of different types of errors, then all your functions return your own Error type and can be written nicely with the ? syntax all over the place 🙂

[tribal glacier]

Looks correct to me the way you've done it already, that'll spawn a new thread. The alternative is to use Tokio for spawning, that technically however spawns a task not a thread, it'll be scheduled to be ran on one of Tokio's worker threads. Is there anything in particular about thread handling you wanna see? Otherwise the official docs are where I go to for documentation on it https://doc.rust-lang.org/stable/std/thread/fn.spawn.html

[tribal glacier]

If you wanna use the Tokio runtime you use it like so:

.setup(|app| {
  tauri::async_runtime::spawn(async move {
    // Your code here
  });
})