From 5892922099017423bbeadfd3aad5dd0ebade9cb7 Mon Sep 17 00:00:00 2001 From: Christian Meusel Date: Wed, 31 Jul 2024 18:41:20 +0200 Subject: [PATCH 01/11] Add test for read timeout Duration::MAX --- tests/test_timeout.rs | 47 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/tests/test_timeout.rs b/tests/test_timeout.rs index fc51c9a..907751d 100644 --- a/tests/test_timeout.rs +++ b/tests/test_timeout.rs @@ -178,3 +178,50 @@ fn test_timeout_greater_zero(hw_config: HardwareConfig, #[case] timeout: Duratio assert!(flushed_at + timeout + margin > read_at); assert_eq!(buffer[..read], message[..read]); } + +/// Checks that reading data with a timeout of `Duration::MAX` returns some data and no error. It +/// does not check the actual timeout for obvious reason. +#[rstest] +#[cfg_attr(feature = "ignore-hardware-tests", ignore)] +fn test_timeout_max(hw_config: HardwareConfig) { + let sleep = Duration::from_millis(3000); + let margin = Duration::from_millis(500); + let mut sender = serialport::new(hw_config.port_1, 115200).open().unwrap(); + let mut receiver = serialport::new(hw_config.port_2, 115200) + .timeout(Duration::MAX) + .open() + .unwrap(); + + let message = + b"0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~"; + let mut buffer: [u8; 1024] = [0xff; 1024]; + + sender.clear(ClearBuffer::All).unwrap(); + receiver.clear(ClearBuffer::All).unwrap(); + + let started_at = Instant::now(); + + let sender_thread = thread::spawn(move || { + thread::sleep(sleep); + + sender.write_all(message).unwrap(); + sender.flush().unwrap(); + }); + + let read = receiver.read(&mut buffer).unwrap(); + let read_at = Instant::now(); + + println!( + "read: {} bytes of {} after {} ms", + read, + message.len(), + (Instant::now() - started_at).as_millis() + ); + + assert!(read > 0); + assert!(read_at > started_at + sleep); + assert!(read_at < started_at + sleep + margin); + assert_eq!(buffer[..read], message[..read]); + + sender_thread.join().unwrap(); +} From c7e3233e494e4c442d7c70a2add50a76982d88cc Mon Sep 17 00:00:00 2001 From: Christian Meusel Date: Fri, 2 Aug 2024 16:06:57 +0200 Subject: [PATCH 02/11] Clean up minor Clippy issues in tests They bubbled up when locally using rust-analyzer and Clippy 1.80. --- tests/test_timeout.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_timeout.rs b/tests/test_timeout.rs index 907751d..a0627d2 100644 --- a/tests/test_timeout.rs +++ b/tests/test_timeout.rs @@ -59,7 +59,7 @@ fn test_read_returns_available_data_before_timeout( ); received_message.extend_from_slice(&buffer[..read]); } - _ => assert!(false), + e => panic!("unexpected error {:?}", e), } if received_message.len() >= expected_message.len() { @@ -79,7 +79,7 @@ fn test_read_returns_available_data_before_timeout( println!("send: {} bytes", chunk.len()); - next = next + send_period; + next += send_period; thread::sleep(next - Instant::now()); } }); From 2cce249fa9980231291a26077f989d92d1a818a2 Mon Sep 17 00:00:00 2001 From: Christian Meusel Date: Fri, 2 Aug 2024 16:11:52 +0200 Subject: [PATCH 03/11] Factor out timeout value computations This prepares unit-testing them. --- src/posix/poll.rs | 7 +++++-- src/windows/com.rs | 10 +++++++--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/posix/poll.rs b/src/posix/poll.rs index e45226e..d0aa1c8 100644 --- a/src/posix/poll.rs +++ b/src/posix/poll.rs @@ -24,8 +24,7 @@ fn wait_fd(fd: RawFd, events: PollFlags, timeout: Duration) -> io::Result<()> { let mut fd = PollFd::new(fd, events); - let milliseconds = - timeout.as_secs() as i64 * 1000 + i64::from(timeout.subsec_nanos()) / 1_000_000; + let milliseconds = milliseconds_i64(timeout); #[cfg(target_os = "linux")] let wait_res = { let timespec = TimeSpec::milliseconds(milliseconds); @@ -63,3 +62,7 @@ fn wait_fd(fd: RawFd, events: PollFlags, timeout: Duration) -> io::Result<()> { Err(io::Error::new(io::ErrorKind::Other, EIO.desc())) } + +fn milliseconds_i64(duration: Duration) -> i64 { + duration.as_secs() as i64 * 1000 + i64::from(duration.subsec_nanos()) / 1_000_000 +} diff --git a/src/windows/com.rs b/src/windows/com.rs index 20e243f..07e3340 100644 --- a/src/windows/com.rs +++ b/src/windows/com.rs @@ -154,6 +154,10 @@ impl COMPort { port_name: None, } } + + fn timeout_constant(duration: Duration) -> DWORD { + duration.as_millis() as DWORD + } } impl Drop for COMPort { @@ -248,14 +252,14 @@ impl SerialPort for COMPort { } fn set_timeout(&mut self, timeout: Duration) -> Result<()> { - let milliseconds = timeout.as_millis(); + let timeout_constant = Self::timeout_constant(timeout); let mut timeouts = COMMTIMEOUTS { ReadIntervalTimeout: MAXDWORD, ReadTotalTimeoutMultiplier: MAXDWORD, - ReadTotalTimeoutConstant: milliseconds as DWORD, + ReadTotalTimeoutConstant: timeout_constant, WriteTotalTimeoutMultiplier: 0, - WriteTotalTimeoutConstant: milliseconds as DWORD, + WriteTotalTimeoutConstant: timeout_constant, }; if unsafe { SetCommTimeouts(self.handle, &mut timeouts) } == 0 { From 1ef2022ffc582e0aed66d6367e5987ac58cb39dd Mon Sep 17 00:00:00 2001 From: Christian Meusel Date: Fri, 2 Aug 2024 17:29:05 +0200 Subject: [PATCH 04/11] Add monotonicity tests for timeout value computations --- src/posix/poll.rs | 70 ++++++++++++++++++++++++++++++++++++++++++++++ src/windows/com.rs | 48 +++++++++++++++++++++++++++++++ 2 files changed, 118 insertions(+) diff --git a/src/posix/poll.rs b/src/posix/poll.rs index d0aa1c8..351f6f4 100644 --- a/src/posix/poll.rs +++ b/src/posix/poll.rs @@ -66,3 +66,73 @@ fn wait_fd(fd: RawFd, events: PollFlags, timeout: Duration) -> io::Result<()> { fn milliseconds_i64(duration: Duration) -> i64 { duration.as_secs() as i64 * 1000 + i64::from(duration.subsec_nanos()) / 1_000_000 } + +#[cfg(test)] +mod tests { + use super::*; + + use nix::libc::c_int; + + // TODO: Harmonize with corresponding tests for Windows. + fn monotonicity_test_durations() -> Vec { + vec![ + Duration::ZERO, + Duration::from_nanos(1), + Duration::from_millis(1), + Duration::from_secs(1), + Duration::from_secs(i16::MAX as u64 - 1), + Duration::from_secs(i16::MAX as u64), + Duration::from_secs(i16::MAX as u64 + 1), + Duration::from_secs(i32::MAX as u64 - 1), + Duration::from_secs(i32::MAX as u64), + Duration::from_secs(i32::MAX as u64 + 1), + Duration::from_secs(i64::MAX as u64 - 1), + Duration::from_secs(i64::MAX as u64), + Duration::from_secs(i64::MAX as u64 + 1), + Duration::from_secs(u64::MAX - 1), + Duration::from_secs(u64::MAX), + Duration::from_secs(u64::MAX) + Duration::from_millis(1), + Duration::MAX, + ] + } + + #[test] + fn milliseconds_i64_as_c_int_is_monotonic() { + let mut last = milliseconds_i64(Duration::ZERO) as c_int; + + for (i, d) in monotonicity_test_durations().iter().enumerate() { + let next = milliseconds_i64(*d) as c_int; + dbg!((i, d)); + assert!( + next >= last, + "{next} >= {last} failed for {d:?} at index {i}" + ); + last = next; + } + } + + #[test] + fn milliseconds_i64_as_c_int_zero_is_zero() { + assert_eq!(0, milliseconds_i64(Duration::ZERO) as c_int); + } + + #[test] + fn milliseconds_i64_is_monotonic() { + let mut last = milliseconds_i64(Duration::ZERO); + + for (i, d) in monotonicity_test_durations().iter().enumerate() { + let next = milliseconds_i64(*d); + dbg!((i, d)); + assert!( + next >= last, + "{next} >= {last} failed for {d:?} at index {i}" + ); + last = next; + } + } + + #[test] + fn milliseconds_i64_zero_is_zero() { + assert_eq!(0, milliseconds_i64(Duration::ZERO)); + } +} diff --git a/src/windows/com.rs b/src/windows/com.rs index 07e3340..c788396 100644 --- a/src/windows/com.rs +++ b/src/windows/com.rs @@ -446,3 +446,51 @@ impl SerialPort for COMPort { } } } + +#[cfg(test)] +mod tests { + use super::*; + + // TODO: Harmonize with corresponding tests for POSIX. + fn monotonicity_test_durations() -> Vec { + vec![ + Duration::ZERO, + Duration::from_nanos(1), + Duration::from_millis(1), + Duration::from_secs(1), + Duration::from_secs(i16::MAX as u64 - 1), + Duration::from_secs(i16::MAX as u64), + Duration::from_secs(i16::MAX as u64 + 1), + Duration::from_secs(i32::MAX as u64 - 1), + Duration::from_secs(i32::MAX as u64), + Duration::from_secs(i32::MAX as u64 + 1), + Duration::from_secs(i64::MAX as u64 - 1), + Duration::from_secs(i64::MAX as u64), + Duration::from_secs(i64::MAX as u64 + 1), + Duration::from_secs(u64::MAX - 1), + Duration::from_secs(u64::MAX), + Duration::from_secs(u64::MAX) + Duration::from_millis(1), + Duration::MAX, + ] + } + + #[test] + fn timeout_constant_is_monotonic() { + let mut last = COMPort::timeout_constant(Duration::ZERO); + + for (i, d) in monotonicity_test_durations().iter().enumerate() { + let next = COMPort::timeout_constant(*d); + dbg!((i, d)); + assert!( + next >= last, + "{next} >= {last} failed for {d:?} at index {i}" + ); + last = next; + } + } + + #[test] + fn timeout_constant_zero_is_zero() { + assert_eq!(0, COMPort::timeout_constant(Duration::ZERO)); + } +} From ed1af9ed75ff13b8d18ba3dd6aa8324d9964c976 Mon Sep 17 00:00:00 2001 From: Christian Meusel Date: Wed, 31 Jul 2024 23:34:41 +0200 Subject: [PATCH 05/11] Clamp long timeouts for POSIX The previous timeout calculation overflowed at a cast and resulted in negative timeouts. These in turn resulted in errors from ppoll. --- src/posix/poll.rs | 94 +++++++++++++++++++++++++++++++---------------- 1 file changed, 63 insertions(+), 31 deletions(-) diff --git a/src/posix/poll.rs b/src/posix/poll.rs index 351f6f4..e02bc7c 100644 --- a/src/posix/poll.rs +++ b/src/posix/poll.rs @@ -5,11 +5,12 @@ use std::os::unix::io::RawFd; use std::slice; use std::time::Duration; +use nix::libc::c_int; use nix::poll::{PollFd, PollFlags}; #[cfg(target_os = "linux")] use nix::sys::signal::SigSet; -#[cfg(target_os = "linux")] -use nix::sys::time::{TimeSpec, TimeValLike}; +#[cfg(any(target_os = "linux", test))] +use nix::sys::time::TimeSpec; pub fn wait_read_fd(fd: RawFd, timeout: Duration) -> io::Result<()> { wait_fd(fd, PollFlags::POLLIN, timeout) @@ -24,22 +25,12 @@ fn wait_fd(fd: RawFd, events: PollFlags, timeout: Duration) -> io::Result<()> { let mut fd = PollFd::new(fd, events); - let milliseconds = milliseconds_i64(timeout); - #[cfg(target_os = "linux")] - let wait_res = { - let timespec = TimeSpec::milliseconds(milliseconds); - nix::poll::ppoll( - slice::from_mut(&mut fd), - Some(timespec), - Some(SigSet::empty()), - ) - }; - #[cfg(not(target_os = "linux"))] - let wait_res = nix::poll::poll(slice::from_mut(&mut fd), milliseconds as nix::libc::c_int); - - let wait = match wait_res { + let wait = match poll_clamped(&mut fd, timeout) { Ok(r) => r, - Err(e) => return Err(io::Error::from(crate::Error::from(e))), + Err(e) => { + dbg!(e); + return Err(io::Error::from(crate::Error::from(e))); + } }; // All errors generated by poll or ppoll are already caught by the nix wrapper around libc, so // here we only need to check if there's at least 1 event @@ -63,16 +54,55 @@ fn wait_fd(fd: RawFd, events: PollFlags, timeout: Duration) -> io::Result<()> { Err(io::Error::new(io::ErrorKind::Other, EIO.desc())) } -fn milliseconds_i64(duration: Duration) -> i64 { - duration.as_secs() as i64 * 1000 + i64::from(duration.subsec_nanos()) / 1_000_000 +/// Poll with a duration clamped to the maximum value representable by the `TimeSpec` used by +/// `ppoll`. +#[cfg(target_os = "linux")] +fn poll_clamped(fd: &mut PollFd, timeout: Duration) -> nix::Result { + let spec = clamped_time_spec(timeout); + nix::poll::ppoll(slice::from_mut(fd), Some(spec), Some(SigSet::empty())) +} + +#[cfg(any(target_os = "linux", test))] +fn clamped_time_spec(duration: Duration) -> TimeSpec { + use nix::libc::c_long; + use nix::sys::time::time_t; + + // We need to clamp manually as TimeSpec::from_duration translates durations with more than + // i64::MAX seconds to negative timespans. This happens due to casting to i64 and is still the + // case as of nix 0.29. + let secs_limit = time_t::MAX as u64; + let secs = duration.as_secs(); + if secs <= secs_limit { + TimeSpec::new(secs as time_t, duration.subsec_nanos() as c_long) + } else { + TimeSpec::new(time_t::MAX, 999_999_999) + } +} + +// Poll with a duration clamped to the maximum millisecond value representable by the `c_int` used +// by `poll`. +#[cfg(not(target_os = "linux"))] +fn poll_clamped(fd: &mut PollFd, timeout: Duration) -> nix::Result { + let millis = clamped_millis_c_int(timeout); + nix::poll::poll(slice::from_mut(fd), millis) +} + +#[cfg(any(not(target_os = "linux"), test))] +fn clamped_millis_c_int(duration: Duration) -> c_int { + let secs_limit = (c_int::MAX as u64) / 1000; + let secs = duration.as_secs(); + + if secs <= secs_limit { + secs as c_int * 1000 + duration.subsec_millis() as c_int + } else { + c_int::MAX + } } #[cfg(test)] mod tests { use super::*; - use nix::libc::c_int; - // TODO: Harmonize with corresponding tests for Windows. fn monotonicity_test_durations() -> Vec { vec![ @@ -97,11 +127,11 @@ mod tests { } #[test] - fn milliseconds_i64_as_c_int_is_monotonic() { - let mut last = milliseconds_i64(Duration::ZERO) as c_int; + fn clamped_millis_c_int_is_monotonic() { + let mut last = clamped_millis_c_int(Duration::ZERO); for (i, d) in monotonicity_test_durations().iter().enumerate() { - let next = milliseconds_i64(*d) as c_int; + let next = clamped_millis_c_int(*d); dbg!((i, d)); assert!( next >= last, @@ -112,16 +142,16 @@ mod tests { } #[test] - fn milliseconds_i64_as_c_int_zero_is_zero() { - assert_eq!(0, milliseconds_i64(Duration::ZERO) as c_int); + fn clamped_millis_c_int_zero_is_zero() { + assert_eq!(0, clamped_millis_c_int(Duration::ZERO)); } #[test] - fn milliseconds_i64_is_monotonic() { - let mut last = milliseconds_i64(Duration::ZERO); + fn clamped_time_spec_is_monotonic() { + let mut last = clamped_time_spec(Duration::ZERO); for (i, d) in monotonicity_test_durations().iter().enumerate() { - let next = milliseconds_i64(*d); + let next = clamped_time_spec(*d); dbg!((i, d)); assert!( next >= last, @@ -132,7 +162,9 @@ mod tests { } #[test] - fn milliseconds_i64_zero_is_zero() { - assert_eq!(0, milliseconds_i64(Duration::ZERO)); + fn clamped_time_spec_zero_is_zero() { + let spec = clamped_time_spec(Duration::ZERO); + assert_eq!(0, spec.tv_sec()); + assert_eq!(0, spec.tv_nsec()); } } From 2f857803865fd7d85f5440a7100e75eefbddc181 Mon Sep 17 00:00:00 2001 From: Christian Meusel Date: Thu, 1 Aug 2024 12:08:53 +0200 Subject: [PATCH 06/11] Clamp long timeouts on Windows --- src/windows/com.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/windows/com.rs b/src/windows/com.rs index c788396..06d8dc1 100644 --- a/src/windows/com.rs +++ b/src/windows/com.rs @@ -156,7 +156,15 @@ impl COMPort { } fn timeout_constant(duration: Duration) -> DWORD { - duration.as_millis() as DWORD + let milliseconds = duration.as_millis(); + // In the way we are setting up COMMTIMEOUTS, a timeout_constant of MAXDWORD gets rejected. + // Let's clamp the timeout constant for values of MAXDWORD and above. See remarks at + // https://learn.microsoft.com/en-us/windows/win32/api/winbase/ns-winbase-commtimeouts. + // + // This effectively throws away accuracy for really long timeouts but at least preserves a + // long-ish timeout. But just casting to DWORD would result in presumably unexpected short + // and non-monotonic timeouts from cutting off the higher bits. + u128::min(milliseconds, MAXDWORD as u128 - 1) as DWORD } } From f5f170313751742a12689d25096a470fb7cc0a41 Mon Sep 17 00:00:00 2001 From: Christian Meusel Date: Fri, 2 Aug 2024 19:38:12 +0200 Subject: [PATCH 07/11] Allow using time_t despite deprecation on musl --- src/posix/poll.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/posix/poll.rs b/src/posix/poll.rs index e02bc7c..86c6bb6 100644 --- a/src/posix/poll.rs +++ b/src/posix/poll.rs @@ -63,6 +63,12 @@ fn poll_clamped(fd: &mut PollFd, timeout: Duration) -> nix::Result { } #[cfg(any(target_os = "linux", test))] +// The type time_t is deprecaten on musl. The nix crate internally uses this type and makes an +// exeption for the deprecation for musl. And so do we. +// +// See https://github.com/rust-lang/libc/issues/1848 which is referenced from every exemption used +// in nix. +#[cfg_attr(target_env = "musl", allow(deprecated))] fn clamped_time_spec(duration: Duration) -> TimeSpec { use nix::libc::c_long; use nix::sys::time::time_t; From 967916fc908b0434119dc6c630c857c0fdd85aa2 Mon Sep 17 00:00:00 2001 From: Christian Meusel Date: Fri, 2 Aug 2024 20:25:13 +0200 Subject: [PATCH 08/11] Add warning on clamping timeouts to documentation --- src/lib.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index 200374e..9c9ab6b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -375,6 +375,14 @@ impl SerialPortBuilder { } /// Set the amount of time to wait to receive data before timing out + /// + ///
+ /// + /// The accuracy is limited by the underlying platform's capabilities. Longer timeouts will be + /// clamped to the maximum supported value which is expected to be in the magnitude of a few + /// days. + /// + ///
#[must_use] pub fn timeout(mut self, timeout: Duration) -> Self { self.timeout = timeout; @@ -487,6 +495,14 @@ pub trait SerialPort: Send + io::Read + io::Write { fn set_stop_bits(&mut self, stop_bits: StopBits) -> Result<()>; /// Sets the timeout for future I/O operations. + /// + ///
+ /// + /// The accuracy is limited by the underlying platform's capabilities. Longer timeouts will be + /// clamped to the maximum supported value which is expected to be in the magnitude of a few + /// days. + /// + ///
fn set_timeout(&mut self, timeout: Duration) -> Result<()>; // Functions for setting non-data control signal pins From b622588a791fe0374b6888d1832e2b7f1c107308 Mon Sep 17 00:00:00 2001 From: Christian Meusel Date: Fri, 2 Aug 2024 21:22:54 +0200 Subject: [PATCH 09/11] Clean up naming of modules containing unit tests --- src/posix/enumerate.rs | 2 +- src/windows/enumerate.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/posix/enumerate.rs b/src/posix/enumerate.rs index 44c44a3..209e7cd 100644 --- a/src/posix/enumerate.rs +++ b/src/posix/enumerate.rs @@ -696,7 +696,7 @@ cfg_if! { not(target_env = "musl"), feature = "libudev" ))] -mod test { +mod tests { use super::*; use quickcheck_macros::quickcheck; diff --git a/src/windows/enumerate.rs b/src/windows/enumerate.rs index 57333ec..c303e24 100644 --- a/src/windows/enumerate.rs +++ b/src/windows/enumerate.rs @@ -562,7 +562,7 @@ pub fn available_ports() -> Result> { } #[cfg(test)] -mod test { +mod tests { use super::*; use quickcheck_macros::quickcheck; From 00e9da0b800a1933182067c394b48d89859ba889 Mon Sep 17 00:00:00 2001 From: Christian Meusel Date: Fri, 2 Aug 2024 22:29:33 +0200 Subject: [PATCH 10/11] Share test durations between all tests --- Cargo.toml | 2 +- src/lib.rs | 3 +++ src/posix/poll.rs | 28 +++------------------------- src/tests/mod.rs | 7 +++++++ src/tests/timeout.rs | 41 +++++++++++++++++++++++++++++++++++++++++ src/windows/com.rs | 26 ++------------------------ 6 files changed, 57 insertions(+), 50 deletions(-) create mode 100644 src/tests/mod.rs create mode 100644 src/tests/timeout.rs diff --git a/Cargo.toml b/Cargo.toml index 1608f75..32bbad0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,7 +16,6 @@ categories = ["hardware-support"] [target."cfg(unix)".dependencies] bitflags = "2.4.0" -cfg-if = "1.0.0" nix = { version = "0.26", default-features = false, features = ["fs", "ioctl", "poll", "signal", "term"] } [target.'cfg(all(target_os = "linux", not(target_env = "musl")))'.dependencies] @@ -36,6 +35,7 @@ features = [ ] [dependencies] +cfg-if = "1.0.0" scopeguard = "1.1" serde = { version = "1.0", features = ["derive"], optional = true } diff --git a/src/lib.rs b/src/lib.rs index 9c9ab6b..25242d1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -47,6 +47,9 @@ mod windows; #[cfg(windows)] pub use windows::COMPort; +#[cfg(test)] +pub(crate) mod tests; + /// A type for results generated by interacting with serial ports /// /// The `Err` type is hard-wired to [`serialport::Error`](struct.Error.html). diff --git a/src/posix/poll.rs b/src/posix/poll.rs index 86c6bb6..e193ac0 100644 --- a/src/posix/poll.rs +++ b/src/posix/poll.rs @@ -108,35 +108,13 @@ fn clamped_millis_c_int(duration: Duration) -> c_int { #[cfg(test)] mod tests { use super::*; - - // TODO: Harmonize with corresponding tests for Windows. - fn monotonicity_test_durations() -> Vec { - vec![ - Duration::ZERO, - Duration::from_nanos(1), - Duration::from_millis(1), - Duration::from_secs(1), - Duration::from_secs(i16::MAX as u64 - 1), - Duration::from_secs(i16::MAX as u64), - Duration::from_secs(i16::MAX as u64 + 1), - Duration::from_secs(i32::MAX as u64 - 1), - Duration::from_secs(i32::MAX as u64), - Duration::from_secs(i32::MAX as u64 + 1), - Duration::from_secs(i64::MAX as u64 - 1), - Duration::from_secs(i64::MAX as u64), - Duration::from_secs(i64::MAX as u64 + 1), - Duration::from_secs(u64::MAX - 1), - Duration::from_secs(u64::MAX), - Duration::from_secs(u64::MAX) + Duration::from_millis(1), - Duration::MAX, - ] - } + use crate::tests::timeout::MONOTONIC_DURATIONS; #[test] fn clamped_millis_c_int_is_monotonic() { let mut last = clamped_millis_c_int(Duration::ZERO); - for (i, d) in monotonicity_test_durations().iter().enumerate() { + for (i, d) in MONOTONIC_DURATIONS.iter().enumerate() { let next = clamped_millis_c_int(*d); dbg!((i, d)); assert!( @@ -156,7 +134,7 @@ mod tests { fn clamped_time_spec_is_monotonic() { let mut last = clamped_time_spec(Duration::ZERO); - for (i, d) in monotonicity_test_durations().iter().enumerate() { + for (i, d) in MONOTONIC_DURATIONS.iter().enumerate() { let next = clamped_time_spec(*d); dbg!((i, d)); assert!( diff --git a/src/tests/mod.rs b/src/tests/mod.rs new file mode 100644 index 0000000..deb6b6a --- /dev/null +++ b/src/tests/mod.rs @@ -0,0 +1,7 @@ +use cfg_if::cfg_if; + +cfg_if! { + if #[cfg(test)] { + pub(crate) mod timeout; + } +} diff --git a/src/tests/timeout.rs b/src/tests/timeout.rs new file mode 100644 index 0000000..dd53c17 --- /dev/null +++ b/src/tests/timeout.rs @@ -0,0 +1,41 @@ +use std::time::Duration; + +/// A sequence of strongly monotonic inrceasing durations. Introduced for testing conversions from +/// `Duration` to platform-specific types. +pub(crate) const MONOTONIC_DURATIONS: [Duration; 17] = [ + Duration::ZERO, + Duration::from_nanos(1), + Duration::from_millis(1), + Duration::from_secs(1), + Duration::from_secs(i16::MAX as u64 - 1), + Duration::from_secs(i16::MAX as u64), + Duration::from_secs(i16::MAX as u64 + 1), + Duration::from_secs(i32::MAX as u64 - 1), + Duration::from_secs(i32::MAX as u64), + Duration::from_secs(i32::MAX as u64 + 1), + Duration::from_secs(i64::MAX as u64 - 1), + Duration::from_secs(i64::MAX as u64), + Duration::from_secs(i64::MAX as u64 + 1), + Duration::from_secs(u64::MAX - 1), + Duration::from_secs(u64::MAX), + Duration::new(u64::MAX, 1_000_000), + Duration::MAX, +]; + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn basic_durations_properties() { + assert_eq!(Duration::ZERO, *MONOTONIC_DURATIONS.first().unwrap()); + assert_eq!(Duration::MAX, *MONOTONIC_DURATIONS.last().unwrap()); + + // Check that this array is monotonic. + let mut last = MONOTONIC_DURATIONS[0]; + for next in MONOTONIC_DURATIONS { + assert!(last <= next); + last = next; + } + } +} diff --git a/src/windows/com.rs b/src/windows/com.rs index 06d8dc1..21ed048 100644 --- a/src/windows/com.rs +++ b/src/windows/com.rs @@ -458,35 +458,13 @@ impl SerialPort for COMPort { #[cfg(test)] mod tests { use super::*; - - // TODO: Harmonize with corresponding tests for POSIX. - fn monotonicity_test_durations() -> Vec { - vec![ - Duration::ZERO, - Duration::from_nanos(1), - Duration::from_millis(1), - Duration::from_secs(1), - Duration::from_secs(i16::MAX as u64 - 1), - Duration::from_secs(i16::MAX as u64), - Duration::from_secs(i16::MAX as u64 + 1), - Duration::from_secs(i32::MAX as u64 - 1), - Duration::from_secs(i32::MAX as u64), - Duration::from_secs(i32::MAX as u64 + 1), - Duration::from_secs(i64::MAX as u64 - 1), - Duration::from_secs(i64::MAX as u64), - Duration::from_secs(i64::MAX as u64 + 1), - Duration::from_secs(u64::MAX - 1), - Duration::from_secs(u64::MAX), - Duration::from_secs(u64::MAX) + Duration::from_millis(1), - Duration::MAX, - ] - } + use crate::tests::timeout::MONOTONIC_DURATIONS; #[test] fn timeout_constant_is_monotonic() { let mut last = COMPort::timeout_constant(Duration::ZERO); - for (i, d) in monotonicity_test_durations().iter().enumerate() { + for (i, d) in MONOTONIC_DURATIONS.iter().enumerate() { let next = COMPort::timeout_constant(*d); dbg!((i, d)); assert!( From 5e515cc49dae440c00176476fd2abe877a9d9b58 Mon Sep 17 00:00:00 2001 From: Christian Meusel Date: Fri, 2 Aug 2024 21:43:38 +0200 Subject: [PATCH 11/11] Add changelog for clamping very long timeout values --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e2f3b57..1550890 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,10 @@ project adheres to [Semantic Versioning](https://semver.org/). ### Fixed * Fix looking up `UsbPortInfo::interface` on macOS. [#193](https://github.com/serialport/serialport-rs/pull/193) +* Fix issues with very long timeout values like `Duration::MAX` by clamping to + maximum supported value for underlying platform. + [#207](https://github.com/serialport/serialport-rs/issues/207), + [#208](https://github.com/serialport/serialport-rs/pull/208) ### Removed