Don't ignore errors from setting baud rates

CHANGELOG.md

@@ -9,6 +9,9 @@ project adheres to [Semantic Versioning](https://semver.org/).
 ### Added
 ### Changed
 ### Fixed
+* Fix ignoring errors from setting baud rate and ignoring unsupported baud
+  rates.
+  [#213](https://github.com/serialport/serialport-rs/pull/213)
 ### Removed
 
 

Cargo.toml

@@ -51,6 +51,7 @@ os_str_bytes = ">=6.0, <6.6.0"
 quickcheck = "1.0.3"
 quickcheck_macros = "1.0.0"
 rstest = { version = "0.12.0", default-features = false }
+rstest_reuse = "0.6.0"
 rustversion = "1.0.16"
 
 [features]

src/posix/termios.rs

@@ -213,11 +213,12 @@ pub(crate) fn set_stop_bits(termios: &mut Termios, stop_bits: StopBits) {
         ))
     )
 ))]
-pub(crate) fn set_baud_rate(termios: &mut Termios, baud_rate: u32) {
+pub(crate) fn set_baud_rate(termios: &mut Termios, baud_rate: u32) -> Result<()> {
     termios.c_cflag &= !nix::libc::CBAUD;
     termios.c_cflag |= nix::libc::BOTHER;
     termios.c_ispeed = baud_rate;
     termios.c_ospeed = baud_rate;
+    Ok(())
 }
 
 // BSDs use the baud rate as the constant value so there's no translation necessary
@@ -227,9 +228,10 @@ pub(crate) fn set_baud_rate(termios: &mut Termios, baud_rate: u32) {
     target_os = "netbsd",
     target_os = "openbsd"
 ))]
-pub(crate) fn set_baud_rate(termios: &mut Termios, baud_rate: u32) {
-    // Ignore the return value because this should never fail
-    unsafe { libc::cfsetspeed(termios, baud_rate.into()) };
+pub(crate) fn set_baud_rate(termios: &mut Termios, baud_rate: u32) -> Result<()> {
+    let res = unsafe { libc::cfsetspeed(termios, baud_rate.into()) };
+    nix::errno::Errno::result(res)?;
+    Ok(())
 }
 
 #[cfg(all(
@@ -240,7 +242,9 @@ pub(crate) fn set_baud_rate(termios: &mut Termios, baud_rate: u32) {
         target_arch = "powerpc64"
     )
 ))]
-pub(crate) fn set_baud_rate(termios: &mut Termios, baud_rate: u32) {
+pub(crate) fn set_baud_rate(termios: &mut Termios, baud_rate: u32) -> Result<()> {
+    use crate::{Error, ErrorKind};
+
     use self::libc::{
         B1000000, B1152000, B1500000, B2000000, B2500000, B3000000, B3500000, B4000000, B460800,
         B500000, B576000, B921600,
@@ -281,8 +285,9 @@ pub(crate) fn set_baud_rate(termios: &mut Termios, baud_rate: u32) {
         3_000_000 => B3000000,
         3_500_000 => B3500000,
         4_000_000 => B4000000,
-        _ => return,
+        _ => return Err(Error::new(ErrorKind::InvalidInput, "Unsupported baud rate")),
     };
     let res = unsafe { libc::cfsetspeed(termios, baud_rate) };
-    nix::errno::Errno::result(res).expect("cfsetspeed failed");
+    nix::errno::Errno::result(res)?;
+    Ok(())
 }

src/posix/tty.rs

@@ -176,7 +176,7 @@ impl TTYPort {
         termios::set_data_bits(&mut termios, builder.data_bits);
         termios::set_stop_bits(&mut termios, builder.stop_bits);
         #[cfg(not(any(target_os = "ios", target_os = "macos")))]
-        termios::set_baud_rate(&mut termios, builder.baud_rate);
+        termios::set_baud_rate(&mut termios, builder.baud_rate)?;
         #[cfg(any(target_os = "ios", target_os = "macos"))]
         termios::set_termios(fd.0, &termios, builder.baud_rate)?;
         #[cfg(not(any(target_os = "ios", target_os = "macos")))]
@@ -630,7 +630,7 @@ impl SerialPort for TTYPort {
     ))]
     fn set_baud_rate(&mut self, baud_rate: u32) -> Result<()> {
         let mut termios = termios::get_termios(self.fd)?;
-        termios::set_baud_rate(&mut termios, baud_rate);
+        termios::set_baud_rate(&mut termios, baud_rate)?;
         termios::set_termios(self.fd, &termios)
     }
 

tests/test_baudrate.rs

@@ -0,0 +1,191 @@
+mod config;
+
+use config::{hw_config, HardwareConfig};
+use rstest::rstest;
+use rstest_reuse::{self, apply, template};
+use serialport::SerialPort;
+use std::ops::Range;
+
+const RESET_BAUD_RATE: u32 = 300;
+
+/// Returs an acceptance interval for the actual baud rate returned from the device after setting
+/// the supplied value. For example, the CP2102 driver on Linux returns the baud rate actually
+/// configured a the device rather than the the value set.
+fn accepted_actual_baud_for(baud: u32) -> Range<u32> {
+    let delta = baud / 200;
+    baud.checked_sub(delta).unwrap()..baud.checked_add(delta).unwrap()
+}
+
+fn check_baud_rate(port: &dyn SerialPort, baud: u32) {
+    let accepted = accepted_actual_baud_for(baud);
+    let actual = port.baud_rate().unwrap();
+    assert!(accepted.contains(&actual));
+}
+
+#[template]
+#[rstest]
+#[case(9600)]
+#[case(57600)]
+#[case(115200)]
+fn standard_baud_rates(#[case] baud: u32) {}
+
+#[template]
+#[rstest]
+#[case(1000)]
+#[case(42000)]
+#[case(100000)]
+fn non_standard_baud_rates(#[case] baud: u32) {}
+
+/// Test cases for setting the baud rate via [`SerialPortBuilder`].
+mod builder {
+    use super::*;
+
+    #[apply(standard_baud_rates)]
+    #[cfg_attr(feature = "ignore-hardware-tests", ignore)]
+    fn test_standard_baud_rate(hw_config: HardwareConfig, #[case] baud: u32) {
+        let port = serialport::new(hw_config.port_1, RESET_BAUD_RATE)
+            .baud_rate(baud)
+            .open()
+            .unwrap();
+        check_baud_rate(port.as_ref(), baud);
+    }
+
+    #[apply(non_standard_baud_rates)]
+    #[cfg_attr(
+        any(
+            feature = "ignore-hardware-tests",
+            not(all(target_os = "linux", target_env = "musl")),
+        ),
+        ignore
+    )]
+    fn test_non_standard_baud_rate_fails_where_not_supported(
+        hw_config: HardwareConfig,
+        #[case] baud: u32,
+    ) {
+        let res = serialport::new(hw_config.port_1, RESET_BAUD_RATE)
+            .baud_rate(baud)
+            .open();
+        assert!(res.is_err());
+    }
+
+    #[apply(non_standard_baud_rates)]
+    #[cfg_attr(
+        any(
+            feature = "ignore-hardware-tests",
+            all(target_os = "linux", target_env = "musl"),
+        ),
+        ignore
+    )]
+    fn test_non_standard_baud_rate_succeeds_where_supported(
+        hw_config: HardwareConfig,
+        #[case] baud: u32,
+    ) {
+        let port = serialport::new(hw_config.port_1, RESET_BAUD_RATE)
+            .baud_rate(baud)
+            .open()
+            .unwrap();
+        check_baud_rate(port.as_ref(), baud);
+    }
+}
+
+/// Test cases for setting the baud rate via [`serialport::new`].
+mod new {
+    use super::*;
+
+    #[apply(standard_baud_rates)]
+    #[cfg_attr(feature = "ignore-hardware-tests", ignore)]
+    fn test_standard_baud_rate(hw_config: HardwareConfig, #[case] baud: u32) {
+        let port = serialport::new(hw_config.port_1, baud).open().unwrap();
+        check_baud_rate(port.as_ref(), baud);
+    }
+
+    #[apply(non_standard_baud_rates)]
+    #[cfg_attr(
+        any(
+            feature = "ignore-hardware-tests",
+            not(all(target_os = "linux", target_env = "musl")),
+        ),
+        ignore
+    )]
+    fn test_non_standard_baud_rate_fails_where_not_supported(
+        hw_config: HardwareConfig,
+        #[case] baud: u32,
+    ) {
+        assert!(serialport::new(hw_config.port_1, baud).open().is_err());
+    }
+
+    #[apply(non_standard_baud_rates)]
+    #[cfg_attr(
+        any(
+            feature = "ignore-hardware-tests",
+            all(target_os = "linux", target_env = "musl"),
+        ),
+        ignore
+    )]
+    fn test_non_standard_baud_rate_succeeds_where_supported(
+        hw_config: HardwareConfig,
+        #[case] baud: u32,
+    ) {
+        let port = serialport::new(hw_config.port_1, baud).open().unwrap();
+        check_baud_rate(port.as_ref(), baud);
+    }
+}
+
+/// Test cases for setting the baud rate via [`SerialPort::set_baud`].
+mod set_baud {
+    use super::*;
+
+    #[apply(standard_baud_rates)]
+    #[cfg_attr(feature = "ignore-hardware-tests", ignore)]
+    fn test_standard_baud_rate(hw_config: HardwareConfig, #[case] baud: u32) {
+        let mut port = serialport::new(hw_config.port_1, RESET_BAUD_RATE)
+            .open()
+            .unwrap();
+        check_baud_rate(port.as_ref(), RESET_BAUD_RATE);
+
+        port.set_baud_rate(baud).unwrap();
+        check_baud_rate(port.as_ref(), baud);
+    }
+
+    #[apply(non_standard_baud_rates)]
+    #[cfg_attr(
+        any(
+            feature = "ignore-hardware-tests",
+            not(all(target_os = "linux", target_env = "musl")),
+        ),
+        ignore
+    )]
+    fn test_non_standard_baud_rate_fails_where_not_supported(
+        hw_config: HardwareConfig,
+        #[case] baud: u32,
+    ) {
+        let mut port = serialport::new(hw_config.port_1, RESET_BAUD_RATE)
+            .open()
+            .unwrap();
+        check_baud_rate(port.as_ref(), RESET_BAUD_RATE);
+
+        assert!(port.set_baud_rate(baud).is_err());
+        check_baud_rate(port.as_ref(), RESET_BAUD_RATE);
+    }
+
+    #[apply(non_standard_baud_rates)]
+    #[cfg_attr(
+        any(
+            feature = "ignore-hardware-tests",
+            all(target_os = "linux", target_env = "musl"),
+        ),
+        ignore
+    )]
+    fn test_non_standard_baud_rate_succeeds_where_supported(
+        hw_config: HardwareConfig,
+        #[case] baud: u32,
+    ) {
+        let mut port = serialport::new(hw_config.port_1, RESET_BAUD_RATE)
+            .open()
+            .unwrap();
+        check_baud_rate(port.as_ref(), RESET_BAUD_RATE);
+
+        port.set_baud_rate(baud).unwrap();
+        check_baud_rate(port.as_ref(), baud);
+    }
+}