Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add read_until_timeout #17

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add read_until_timeout #17

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 44 additions & 3 deletions src/serial_port.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::io::{IoSlice, IoSliceMut};
use std::num::NonZeroUsize;
use std::path::{Path, PathBuf};
use std::time::Duration;

Expand Down Expand Up @@ -93,6 +94,47 @@ impl SerialPort {
self.inner.is_read_vectored()
}

/// Will read a byte at a time until either a timeout is triggered, the size is reached, or memory is exhausted.
/// This differs from `read()` which will either timeout or read a single byte.
/// # Examples
///
/// ```
/// use SerialTester::read_to_timeout;
///
/// let mut port = serial2::Serialport::open("COM2", 9600);
/// assert_eq!(read_to_timeout(&mut port, None), );
/// assert_eq!(port, [0x48, 0x49, 0x20, 0x46, 0x45, 0x52, 0x52, 0x49, 0x53]);
/// ```
pub fn read_until_timeout(&self, size: Option<NonZeroUsize>) -> std::io::Result<Vec<u8>> {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of this

pub fn read_until_timeout(&self, size: Option<NonZeroUsize>) -> std::io::Result<Vec<u8>> {

You could do this

pub fn read_until_timeout(&self, buffer: &mut [u8]) -> usize{

the function would then read into the buffer slice and return the number of bytes successfully into the buffer. If a timeout occured, then obviously the return value will < buff.len()

this aligns with how many other apis work as well, and is also nostd ( embedded ) friendly as the buffer could just be a slice into a static allocated array

Copy link

@DanielJoyce DanielJoyce Apr 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, the timeout bounds is kinda common for all of these methods, so does not really need to be explained.

pub fn read_into(...)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My primary use is an API that doesn't really care about the size and more about the timeout, which is why I named it such.

You do showcase something interesting which is that this API is in essence very similar to read_exact. Just it special cases timeout as being Ok, which you can do manually with something like

match port.read_exact(&mut buf) {
    Ok(_) => (),
    Err(err) if err.kind() == std::io::ErrorKind::TimedOut => (),
    Err(err) => return Err(err),
}

Though this does rely on the implementation detail of this library which is that the buffer is written to in the loop.

Copy link
Owner

@de-vri-es de-vri-es Apr 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

read_exact using a loop is something you can rely on. There is no other way, since the contract of the underlying read syscall is that it can return less than the requested bytes. At-least on Unix, and we try to get that behavior on Windows too.

I'm still considering if this should be fixed by a timeout returned Ok(0) instead of an error or not.

If we do, read_exact will still fail with UnexpectedEof (instead of TimedOut). But read_to_end() will return Ok.

From the standard library documentation of read():

If the return value of this method is Ok(n), then implementations must guarantee that 0 <= n <= buf.len(). A nonzero n value indicates that the buffer buf has been filled in with n bytes of data from this source. If n is 0, then it can indicate one of two scenarios:

  1. This reader has reached its “end of file” and will likely no longer be able to produce bytes. Note that this does not mean that the reader will always no longer be able to produce bytes. As an example, on Linux, this method will call the recv syscall for a TcpStream, where returning zero indicates the connection was shut down correctly. While for File, it is possible to reach the end of file and get zero as result, but if more data is appended to the file, future calls to read will return more data.
  2. The buffer specified was 0 bytes in length.

So the question is: should we mimick the guarantees of a TcpStream or a File? Conceptually, a serial port is more similar to a TcpStream than a File, so that kinda hints to keeping the timeout as an error instead of Ok(0).

But then a convenience function that reads until a timeout (and returns Ok) is certainly useful.

let mut output = Vec::new();

loop {
let mut byte = [0];

// read to the byte
let res = self.read(&mut byte);

match res {
Ok(_) => {
output.push(byte[0]);

match size {
Some(max_size) => {
if output.len() == max_size.into() {
break Ok(output);
}
},
None => continue,
}
},
Err(err) => match err.kind() {
std::io::ErrorKind::TimedOut => break Ok(output),
_ => break Err(err),
},
}
}
}

/// Write bytes to the serial port.
///
/// This is identical to [`std::io::Write::write()`], except that this function takes a const reference `&self`.
Expand Down Expand Up @@ -288,7 +330,7 @@ impl From<SerialPort> for std::os::unix::io::OwnedFd {
impl From<std::os::unix::io::OwnedFd> for SerialPort {
fn from(value: std::os::unix::io::OwnedFd) -> Self {
Self {
inner: sys::SerialPort::from_file(value.into())
inner: sys::SerialPort::from_file(value.into()),
}
}
}
Expand Down Expand Up @@ -338,7 +380,7 @@ impl From<SerialPort> for std::os::windows::io::OwnedHandle {
impl From<std::os::windows::io::OwnedHandle> for SerialPort {
fn from(value: std::os::windows::io::OwnedHandle) -> Self {
Self {
inner: sys::SerialPort::from_file(value.into())
inner: sys::SerialPort::from_file(value.into()),
}
}
}
Expand All @@ -364,7 +406,6 @@ impl std::os::windows::io::IntoRawHandle for SerialPort {
}
}


/// Convert an [`RawHandle`][std::os::windows::io::RawHandle] into a `SerialPort`.
///
/// The file handle must have been created with the `FILE_FLAG_OVERLAPPED` flag for the serial port to function correctly.
Expand Down
Loading