diff --git a/gsd-parser/src/lib.rs b/gsd-parser/src/lib.rs index b1e4bad..7153b87 100644 --- a/gsd-parser/src/lib.rs +++ b/gsd-parser/src/lib.rs @@ -1,3 +1,6 @@ +#![expect(clippy::result_large_err)] +#![deny(clippy::as_conversions)] + use std::collections::BTreeMap; use std::path::Path; use std::sync::Arc; @@ -139,6 +142,23 @@ pub enum UserPrmDataType { BitArea(u8, u8), } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct PrmValueRangeError(pub(crate) ()); + +impl From for PrmValueRangeError { + fn from(_value: std::num::TryFromIntError) -> Self { + PrmValueRangeError(()) + } +} + +impl std::fmt::Display for PrmValueRangeError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + "value for user_prm is not in range for data type".fmt(f) + } +} + +impl std::error::Error for PrmValueRangeError {} + impl UserPrmDataType { pub fn size(self) -> usize { match self { @@ -153,42 +173,42 @@ impl UserPrmDataType { } } - pub fn write_value_to_slice(self, value: i64, s: &mut [u8]) { + pub fn write_value_to_slice(self, value: i64, s: &mut [u8]) -> Result<(), PrmValueRangeError> { match self { UserPrmDataType::Unsigned8 => { - assert!(0 <= value && value <= 255); - s[..1].copy_from_slice(&(value as u8).to_be_bytes()); + s[..1].copy_from_slice(&u8::try_from(value)?.to_be_bytes()); } UserPrmDataType::Unsigned16 => { - assert!(0 <= value && value <= 65535); - s[..2].copy_from_slice(&(value as u16).to_be_bytes()); + s[..2].copy_from_slice(&u16::try_from(value)?.to_be_bytes()); } UserPrmDataType::Unsigned32 => { - assert!(0 <= value && value <= 4294967295); - s[..4].copy_from_slice(&(value as u32).to_be_bytes()); + s[..4].copy_from_slice(&u32::try_from(value)?.to_be_bytes()); } UserPrmDataType::Signed8 => { - assert!(-127 <= value && value <= 127); - s[..1].copy_from_slice(&(value as i8).to_be_bytes()); + s[..1].copy_from_slice(&i8::try_from(value)?.to_be_bytes()); } UserPrmDataType::Signed16 => { - assert!(-32767 <= value && value <= 32767); - s[..2].copy_from_slice(&(value as i16).to_be_bytes()); + s[..2].copy_from_slice(&u16::try_from(value)?.to_be_bytes()); } UserPrmDataType::Signed32 => { - assert!(2147483647 <= value && value <= 2147483647); - s[..4].copy_from_slice(&(value as i32).to_be_bytes()); + s[..4].copy_from_slice(&i32::try_from(value)?.to_be_bytes()); } UserPrmDataType::Bit(b) => { + if value != 0 && value != 1 { + return Err(PrmValueRangeError(())); + } assert!(value == 0 || value == 1); - s[0] |= (value as u8) << b; + s[0] |= u8::try_from(value)? << b; } UserPrmDataType::BitArea(first, last) => { let bit_size = last - first + 1; - assert!(value >= 0 && value < 2i64.pow(bit_size as u32)); - s[0] = (value as u8) << first; + if value < 0 || value >= 2i64.pow(u32::from(bit_size)) { + return Err(PrmValueRangeError(())); + } + s[0] = u8::try_from(value)? << first; } } + Ok(()) } } @@ -199,6 +219,28 @@ pub enum PrmValueConstraint { Unconstrained, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct PrmValueConstraintError { + value: i64, + constraint: PrmValueConstraint, +} + +impl std::fmt::Display for PrmValueConstraintError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match &self.constraint { + PrmValueConstraint::MinMax(min, max) => { + write!(f, "value {} not in range {min}..={max}", self.value) + } + PrmValueConstraint::Enum(values) => { + write!(f, "value {} not in set {values:?}", self.value) + } + PrmValueConstraint::Unconstrained => unreachable!(), + } + } +} + +impl std::error::Error for PrmValueConstraintError {} + impl PrmValueConstraint { pub fn is_valid(&self, value: i64) -> bool { match self { @@ -208,21 +250,29 @@ impl PrmValueConstraint { } } - pub fn assert_valid(&self, value: i64) { + pub fn assert_valid(&self, value: i64) -> Result<(), PrmValueConstraintError> { match self { PrmValueConstraint::MinMax(min, max) => { - assert!( - *min <= value && value <= *max, - "value {value} not in range {min}..={max}", - ); + if *min > value || value > *max { + Err(PrmValueConstraintError { + value, + constraint: self.clone(), + }) + } else { + Ok(()) + } } PrmValueConstraint::Enum(values) => { - assert!( - values.contains(&value), - "value {value} not in set {values:?}", - ); + if !values.contains(&value) { + Err(PrmValueConstraintError { + value, + constraint: self.clone(), + }) + } else { + Ok(()) + } } - PrmValueConstraint::Unconstrained => (), + PrmValueConstraint::Unconstrained => Ok(()), } } } @@ -350,20 +400,60 @@ pub struct GenericStationDescription { pub unit_diag: UnitDiag, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum SetPrmError { + PrmNotFound(String), + PrmWithoutTexts(String), + PrmTextNotFound { prm: String, text: String }, + ValueConstraint(PrmValueConstraintError), + ValueRange { value: i64, ty: UserPrmDataType }, +} + +impl From for SetPrmError { + fn from(value: PrmValueConstraintError) -> Self { + Self::ValueConstraint(value) + } +} + +impl std::fmt::Display for SetPrmError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + SetPrmError::PrmNotFound(name) => { + write!(f, "prm {name} was not found") + } + SetPrmError::ValueConstraint(prm_value_constraint_error) => { + prm_value_constraint_error.fmt(f) + } + SetPrmError::ValueRange { value, ty } => { + write!(f, "value {value} is out of range for type {ty:?}") + } + SetPrmError::PrmWithoutTexts(name) => { + write!(f, "prm {name} does not have texts") + } + SetPrmError::PrmTextNotFound { prm, text } => { + write!(f, "prm {prm} does not have text {text:?}") + } + } + } +} + +impl std::error::Error for SetPrmError {} + +#[derive(Debug, PartialEq, Eq)] pub struct PrmBuilder<'a> { desc: &'a UserPrmData, prm: Vec, } impl<'a> PrmBuilder<'a> { - pub fn new(desc: &'a UserPrmData) -> Self { + pub fn new(desc: &'a UserPrmData) -> Result { let mut this = Self { desc, prm: Vec::new(), }; this.write_const_prm_data(); - this.write_default_prm_data(); - this + this.write_default_prm_data()?; + Ok(this) } fn update_prm_data_len(&mut self, offset: usize, size: usize) { @@ -381,44 +471,61 @@ impl<'a> PrmBuilder<'a> { } } - fn write_default_prm_data(&mut self) { + fn write_default_prm_data(&mut self) -> Result<(), PrmValueRangeError> { for (offset, data_ref) in self.desc.data_ref.iter() { let size = data_ref.data_type.size(); self.update_prm_data_len(*offset, size); data_ref .data_type - .write_value_to_slice(data_ref.default_value, &mut self.prm[(*offset as usize)..]); + .write_value_to_slice(data_ref.default_value, &mut self.prm[(*offset)..])?; } + Ok(()) } - pub fn set_prm(&mut self, prm: &str, value: i64) -> &mut Self { + pub fn set_prm(&mut self, prm: &str, value: i64) -> Result<&mut Self, SetPrmError> { let (offset, data_ref) = self .desc .data_ref .iter() .find(|(_, r)| r.name == prm) - .unwrap(); - data_ref.constraint.assert_valid(value); + .ok_or_else(|| SetPrmError::PrmNotFound(prm.to_string()))?; + data_ref.constraint.assert_valid(value)?; data_ref .data_type - .write_value_to_slice(value, &mut self.prm[(*offset as usize)..]); - self + .write_value_to_slice(value, &mut self.prm[(*offset)..]) + .map_err(|_e| SetPrmError::ValueRange { + value, + ty: data_ref.data_type, + })?; + Ok(self) } - pub fn set_prm_from_text(&mut self, prm: &str, value: &str) -> &mut Self { + pub fn set_prm_from_text(&mut self, prm: &str, value: &str) -> Result<&mut Self, SetPrmError> { let (offset, data_ref) = self .desc .data_ref .iter() .find(|(_, r)| r.name == prm) - .unwrap(); - let text_ref = data_ref.text_ref.as_ref().unwrap(); - let value = *text_ref.get(value).unwrap(); - data_ref.constraint.assert_valid(value); + .ok_or_else(|| SetPrmError::PrmNotFound(prm.to_string()))?; + let text_ref = data_ref + .text_ref + .as_ref() + .ok_or_else(|| SetPrmError::PrmWithoutTexts(prm.to_string()))?; + let value = *text_ref + .get(value) + .ok_or_else(|| SetPrmError::PrmTextNotFound { + prm: prm.to_string(), + text: value.to_string(), + })?; + data_ref.constraint.assert_valid(value)?; data_ref .data_type - .write_value_to_slice(value, &mut self.prm[(*offset as usize)..]); - self + .write_value_to_slice(value, &mut self.prm[(*offset)..]) + .map_err(|_e| SetPrmError::ValueRange { + value, + ty: data_ref.data_type, + })?; + Ok(self) } pub fn as_bytes(&self) -> &[u8] { @@ -433,9 +540,9 @@ impl<'a> PrmBuilder<'a> { pub fn parse_from_file>(file: P) -> GenericStationDescription { use std::io::Read; - let mut f = std::fs::File::open(file.as_ref()).unwrap(); + let mut f = std::fs::File::open(file.as_ref()).expect("TODO"); let mut source_bytes = Vec::new(); - f.read_to_end(&mut source_bytes).unwrap(); + f.read_to_end(&mut source_bytes).expect("TODO"); let source = String::from_utf8_lossy(&source_bytes); match parser::parse(file.as_ref(), &source) { diff --git a/gsd-parser/src/parser.rs b/gsd-parser/src/parser.rs index b45073a..d1d8ca7 100644 --- a/gsd-parser/src/parser.rs +++ b/gsd-parser/src/parser.rs @@ -57,7 +57,6 @@ where Ok(match pair.as_rule() { gsd_parser::Rule::number_list => pair .into_inner() - .into_iter() .map(|p| parse_number::(p)) .collect::>>()?, gsd_parser::Rule::dec_number | gsd_parser::Rule::hex_number => { @@ -96,7 +95,7 @@ pub fn parse( fn parse_inner(source: &str) -> ParseResult { use pest::Parser; - let gsd_pairs = gsd_parser::GsdParser::parse(gsd_parser::Rule::gsd, &source)? + let gsd_pairs = gsd_parser::GsdParser::parse(gsd_parser::Rule::gsd, source)? .next() .expect("pest grammar wrong?"); @@ -276,8 +275,10 @@ fn parse_inner(source: &str) -> ParseResult { "ext_user_prm_data_ref" => { let offset = parse_number(value_pair)?; let data_id = parse_number(pairs.next().unwrap())?; - let data_ref = - user_prm_data_definitions.get(&data_id).unwrap().clone(); + let data_ref = user_prm_data_definitions + .get(&data_id) + .expect("TODO") + .clone(); module_prm_data.data_ref.push((offset, data_ref)); } "ext_user_prm_data_const" => { @@ -483,7 +484,10 @@ fn parse_inner(source: &str) -> ParseResult { "ext_user_prm_data_ref" => { let offset = parse_number(value_pair)?; let data_id = parse_number(pairs.next().unwrap())?; - let data_ref = user_prm_data_definitions.get(&data_id).unwrap().clone(); + let data_ref = user_prm_data_definitions + .get(&data_id) + .expect("TODO") + .clone(); gsd.user_prm_data.data_ref.push((offset, data_ref)); // The presence of this keywords means `User_Prm_Data` and // `User_Prm_Data_Len` should be ignored. diff --git a/gsd-parser/tests/regress.rs b/gsd-parser/tests/regress.rs index 302ecb4..57f69a2 100644 --- a/gsd-parser/tests/regress.rs +++ b/gsd-parser/tests/regress.rs @@ -11,7 +11,7 @@ fn regress(#[files("tests/data/*.[gG][sS][dD]")] gsd_file: PathBuf) { fn regress_prm(#[files("tests/data/*.[gG][sS][dD]")] gsd_file: PathBuf) { let name = gsd_file.file_stem().unwrap().to_string_lossy().to_string(); let gsd = gsd_parser::parse_from_file(gsd_file); - let mut prm = gsd_parser::PrmBuilder::new(&gsd.user_prm_data); + let mut prm = gsd_parser::PrmBuilder::new(&gsd.user_prm_data).unwrap(); // Try setting all the available parameters to some reasonable values. for (_, prm_ref) in gsd.user_prm_data.data_ref.iter() { @@ -22,16 +22,32 @@ fn regress_prm(#[files("tests/data/*.[gG][sS][dD]")] gsd_file: PathBuf) { // Fallback when the list only has one text... texts.keys().next().unwrap() }; - prm.set_prm_from_text(&prm_ref.name, text); + prm.set_prm_from_text(&prm_ref.name, text).unwrap(); + + // Test that trying a wrong text doens't panic + let res = prm.set_prm_from_text(&prm_ref.name, "InvalidTextAllTheWay"); + assert!(res.is_err()); } else { let v = match &prm_ref.constraint { gsd_parser::PrmValueConstraint::MinMax(_, max) => *max, gsd_parser::PrmValueConstraint::Enum(values) => *values.last().unwrap(), gsd_parser::PrmValueConstraint::Unconstrained => 1, }; - prm.set_prm(&prm_ref.name, v); + prm.set_prm(&prm_ref.name, v).unwrap(); + + // Test that an invalid value results in an error rather than a panic + let res = prm.set_prm(&prm_ref.name, i64::MIN); + assert!(res.is_err()); + + // Test that trying a text PRM doesn't panic + let res = prm.set_prm_from_text(&prm_ref.name, "InvalidTextAllTheWay"); + assert!(res.is_err()); } } + // Test that a non-existent PRM doesn't panic + let res = prm.set_prm("ThisPrmNeverEverExistsEver", 0); + assert!(res.is_err()); + insta::assert_debug_snapshot!(format!("{}-PRM", name).as_ref(), prm.as_bytes()); } diff --git a/gsdtool/src/main.rs b/gsdtool/src/main.rs index a006dad..1de5610 100644 --- a/gsdtool/src/main.rs +++ b/gsdtool/src/main.rs @@ -78,7 +78,7 @@ fn run_config_wizard(args: &ConfigWizardOptions) { println!(); println!("{}", style("Global parameters:").bold()); - let mut prm = gsd_parser::PrmBuilder::new(&gsd.user_prm_data); + let mut prm = gsd_parser::PrmBuilder::new(&gsd.user_prm_data).expect("TODO"); let mut global_parameters = vec![]; let mut had_parameters = false; for (_, prm_ref) in gsd.user_prm_data.data_ref.iter() { @@ -103,8 +103,9 @@ fn run_config_wizard(args: &ConfigWizardOptions) { .interact() .unwrap(); - let sel_text = &texts_list[selection]; - prm.set_prm_from_text(&prm_ref.name, sel_text); + let sel_text = &texts_list.get(selection).expect("TODO"); + prm.set_prm_from_text(&prm_ref.name, sel_text) + .expect("TODO"); global_parameters.push((prm_ref.name.to_owned(), sel_text.to_string())); } else if let gsd_parser::PrmValueConstraint::MinMax(min, max) = prm_ref.constraint { @@ -121,8 +122,8 @@ fn run_config_wizard(args: &ConfigWizardOptions) { .interact() .unwrap(); - let value: i64 = str::parse(&value).unwrap(); - prm.set_prm(&prm_ref.name, value); + let value: i64 = str::parse(&value).expect("TODO"); + prm.set_prm(&prm_ref.name, value).expect("TODO"); global_parameters.push((prm_ref.name.to_owned(), value.to_string())); } else if let gsd_parser::PrmValueConstraint::Enum(values) = &prm_ref.constraint { @@ -141,8 +142,8 @@ fn run_config_wizard(args: &ConfigWizardOptions) { .interact() .unwrap(); - let value: i64 = values[selection]; - prm.set_prm(&prm_ref.name, value); + let value: i64 = values.get(selection).copied().expect("TODO"); + prm.set_prm(&prm_ref.name, value).expect("TODO"); global_parameters.push((prm_ref.name.to_owned(), value.to_string())); } else { @@ -159,8 +160,8 @@ fn run_config_wizard(args: &ConfigWizardOptions) { .interact() .unwrap(); - let value: i64 = str::parse(&value_str).unwrap(); - prm.set_prm(&prm_ref.name, value); + let value: i64 = str::parse(&value_str).expect("TODO"); + prm.set_prm(&prm_ref.name, value).expect("TODO"); global_parameters.push((prm_ref.name.to_owned(), value_str)); } @@ -254,7 +255,7 @@ fn run_config_wizard(args: &ConfigWizardOptions) { module_config.append(&mut module.config.to_vec()); - let mut prm = gsd_parser::PrmBuilder::new(&module.module_prm_data); + let mut prm = gsd_parser::PrmBuilder::new(&module.module_prm_data).expect("TODO"); let mut module_parameters = vec![]; for (_, prm_ref) in module.module_prm_data.data_ref.iter() { if !prm_ref.visible || !prm_ref.changeable { @@ -279,7 +280,8 @@ fn run_config_wizard(args: &ConfigWizardOptions) { .unwrap(); let sel_text = &texts_list[selection]; - prm.set_prm_from_text(&prm_ref.name, sel_text); + prm.set_prm_from_text(&prm_ref.name, sel_text) + .expect("TODO"); module_parameters.push((prm_ref.name.to_owned(), sel_text.to_string())); } else if let gsd_parser::PrmValueConstraint::MinMax(min, max) = prm_ref.constraint @@ -297,8 +299,8 @@ fn run_config_wizard(args: &ConfigWizardOptions) { .interact() .unwrap(); - let value: i64 = str::parse(&value).unwrap(); - prm.set_prm(&prm_ref.name, value); + let value: i64 = str::parse(&value).expect("TODO"); + prm.set_prm(&prm_ref.name, value).expect("TODO"); module_parameters.push((prm_ref.name.to_owned(), value.to_string())); } else if let gsd_parser::PrmValueConstraint::Enum(values) = &prm_ref.constraint { @@ -317,8 +319,8 @@ fn run_config_wizard(args: &ConfigWizardOptions) { .interact() .unwrap(); - let value: i64 = values[selection]; - prm.set_prm(&prm_ref.name, value); + let value: i64 = values.get(selection).copied().expect("TODO"); + prm.set_prm(&prm_ref.name, value).expect("TODO"); module_parameters.push((prm_ref.name.to_owned(), value.to_string())); } else { @@ -335,8 +337,8 @@ fn run_config_wizard(args: &ConfigWizardOptions) { .interact() .unwrap(); - let value: i64 = str::parse(&value_str).unwrap(); - prm.set_prm(&prm_ref.name, value); + let value: i64 = str::parse(&value_str).expect("TODO"); + prm.set_prm(&prm_ref.name, value).expect("TODO"); module_parameters.push((prm_ref.name.to_owned(), value_str)); } diff --git a/src/dp/diagnostics.rs b/src/dp/diagnostics.rs index dcefd27..931d95b 100644 --- a/src/dp/diagnostics.rs +++ b/src/dp/diagnostics.rs @@ -152,7 +152,7 @@ impl ChannelError { 8 => ChannelError::LowerLimitUndershoot, 9 => ChannelError::Error, v @ 16..=31 => ChannelError::Vendor(v), - r @ _ => ChannelError::Reserved(r), + r => ChannelError::Reserved(r), } } diff --git a/src/dp/peripheral.rs b/src/dp/peripheral.rs index 20e76c6..35f2db2 100644 --- a/src/dp/peripheral.rs +++ b/src/dp/peripheral.rs @@ -314,7 +314,7 @@ impl<'a> Peripheral<'a> { /// Get the last diagnostics information received from this peripheral. #[inline] - pub fn last_diagnostics(&self) -> Option { + pub fn last_diagnostics(&self) -> Option> { self.diag.as_ref().map(|diag| PeripheralDiagnostics { flags: diag.flags, ident_number: diag.ident_number, @@ -399,7 +399,7 @@ impl<'a> Peripheral<'a> { // Groups buf[6] = self.options.groups; // User Prm Data - buf[7..].copy_from_slice(&user_parameters); + buf[7..].copy_from_slice(user_parameters); }, )) } else { @@ -420,7 +420,7 @@ impl<'a> Peripheral<'a> { }, config.len(), |buf| { - buf.copy_from_slice(&config); + buf.copy_from_slice(config); }, )) } else { @@ -598,7 +598,7 @@ impl<'a> Peripheral<'a> { if data_ok { if t.pdu.len() == self.pi_i.len() { - self.pi_i.copy_from_slice(&t.pdu); + self.pi_i.copy_from_slice(t.pdu); self.state = PeripheralState::DataExchange; Some(PeripheralEvent::DataExchanged) } else { diff --git a/src/dp/scan.rs b/src/dp/scan.rs index 8b7e1b8..635123e 100644 --- a/src/dp/scan.rs +++ b/src/dp/scan.rs @@ -12,6 +12,7 @@ pub enum DpScanEvent { PeripheralLost(crate::Address), } +#[derive(Debug)] pub struct DpScanner { stations: bitvec::BitArr!(for 128), cursor: crate::Address, @@ -19,6 +20,12 @@ pub struct DpScanner { current_address_done: bool, } +impl Default for DpScanner { + fn default() -> Self { + Self::new() + } +} + impl DpScanner { pub fn new() -> Self { Self { diff --git a/src/fdl/active.rs b/src/fdl/active.rs index cf14e07..5dcdf7a 100644 --- a/src/fdl/active.rs +++ b/src/fdl/active.rs @@ -1,6 +1,12 @@ //! Implementation of an FDL active station. #![deny(unused_must_use)] +#![allow(clippy::unneeded_struct_pattern)] +#![allow(clippy::wildcard_in_or_patterns)] +#![allow(clippy::extra_unused_lifetimes)] +#![allow(clippy::unit_arg)] +#![allow(clippy::useless_conversion)] + use crate::fdl::FdlApplication; use crate::phy::ProfibusPhy; @@ -48,11 +54,11 @@ enum GapState { impl GapState { pub fn increment_wait(&mut self) { - match self { - GapState::Waiting { - ref mut rotation_count, - } => *rotation_count += 1, - _ => (), + if let GapState::Waiting { + ref mut rotation_count, + } = self + { + *rotation_count += 1; } } } @@ -938,8 +944,7 @@ impl FdlActiveStation { high_prio_only: bool, ) -> Option { if let Some(tx_res) = phy.transmit_telegram(now, |tx| { - let res = app.transmit_telegram(now, self, tx, high_prio_only); - res + app.transmit_telegram(now, self, tx, high_prio_only) }) { if let Some(addr) = tx_res.expects_reply() { let data = *self.state.get_use_token_data(); diff --git a/src/fdl/live_list.rs b/src/fdl/live_list.rs index ef15b20..008fc6d 100644 --- a/src/fdl/live_list.rs +++ b/src/fdl/live_list.rs @@ -18,6 +18,12 @@ pub struct LiveList { current_address_done: bool, } +impl Default for LiveList { + fn default() -> Self { + Self::new() + } +} + impl LiveList { pub fn new() -> Self { Self { @@ -28,7 +34,7 @@ impl LiveList { } } - pub fn iter_stations(&self) -> impl Iterator + DoubleEndedIterator + '_ { + pub fn iter_stations(&self) -> impl DoubleEndedIterator + '_ { self.stations.iter_ones().map(|a| u8::try_from(a).unwrap()) } diff --git a/src/fdl/mod.rs b/src/fdl/mod.rs index b291b66..c53f779 100644 --- a/src/fdl/mod.rs +++ b/src/fdl/mod.rs @@ -77,7 +77,6 @@ impl FdlApplication for () { addr: u8, telegram: Telegram, ) { - () } fn handle_timeout(&mut self, now: crate::time::Instant, fdl: &FdlActiveStation, addr: u8) { diff --git a/src/fdl/parameters.rs b/src/fdl/parameters.rs index 8c3323e..1ddb1f8 100644 --- a/src/fdl/parameters.rs +++ b/src/fdl/parameters.rs @@ -106,7 +106,7 @@ fn watchdog_factors(dur: crate::time::Duration) -> Option> let timeout_10ms: u32 = (dur.total_millis() / 10).try_into().or(Err(()))?; for f1 in 1..256 { - let f2 = (timeout_10ms + f1 - 1) / f1; + let f2 = timeout_10ms.div_ceil(f1); if f2 < 256 { return Ok((u8::try_from(f1).unwrap(), u8::try_from(f2).unwrap())); diff --git a/src/fdl/telegram.rs b/src/fdl/telegram.rs index fd3d75f..99f3fef 100644 --- a/src/fdl/telegram.rs +++ b/src/fdl/telegram.rs @@ -1,5 +1,6 @@ #![cfg_attr(test, allow(non_local_definitions))] +#[expect(clippy::identity_op)] #[derive(Debug, PartialEq, Eq, Clone, Copy)] #[cfg_attr(test, derive(proptest_derive::Arbitrary))] #[repr(u8)] @@ -207,6 +208,15 @@ pub enum FunctionCode { }, } +#[expect(clippy::enum_variant_names)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[repr(u8)] +pub enum FcParseError { + InvalidRequestType, + InvalidResponseState, + InvalidResponseStatus, +} + impl FunctionCode { pub fn to_byte(self) -> u8 { match self { @@ -217,18 +227,20 @@ impl FunctionCode { } } - pub fn from_byte(b: u8) -> Result { + pub fn from_byte(b: u8) -> Result { if b & (1 << 6) != 0 { let fcv = b & (1 << 4) != 0; let fcb = b & (1 << 5) != 0; - let req = RequestType::from_u8(b & 0x8F).ok_or(())?; + let req = RequestType::from_u8(b & 0x8F).ok_or(FcParseError::InvalidRequestType)?; Ok(Self::Request { fcb: FrameCountBit::from_fcv_fcb(fcv, fcb), req, }) } else { - let state = ResponseState::from_u8((b & 0x30) >> 4).ok_or(())?; - let status = ResponseStatus::from_u8(b & 0x0F).ok_or(())?; + let state = ResponseState::from_u8((b & 0x30) >> 4) + .ok_or(FcParseError::InvalidResponseState)?; + let status = + ResponseStatus::from_u8(b & 0x0F).ok_or(FcParseError::InvalidResponseStatus)?; Ok(Self::Response { state, status }) } } diff --git a/src/fdl/token_ring.rs b/src/fdl/token_ring.rs index b38774c..3d1488a 100644 --- a/src/fdl/token_ring.rs +++ b/src/fdl/token_ring.rs @@ -67,9 +67,7 @@ impl TokenRing { } } - pub fn iter_active_stations( - &self, - ) -> impl Iterator + DoubleEndedIterator + '_ { + pub fn iter_active_stations(&self) -> impl DoubleEndedIterator + '_ { self.active_stations .iter_ones() .map(|a| u8::try_from(a).unwrap()) @@ -153,7 +151,7 @@ impl TokenRing { .find(|a| *a < self.this_station) { previous - } else if let Some(last) = self.iter_active_stations().rev().next() { + } else if let Some(last) = self.iter_active_stations().next_back() { last } else { self.this_station diff --git a/src/lib.rs b/src/lib.rs index 46193de..def0005 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -77,6 +77,9 @@ // TODO: Remove this once the crate has matured. #![allow(dead_code)] #![allow(unused_variables)] +#![allow(clippy::len_zero)] +#![allow(clippy::collapsible_if)] +#![allow(clippy::manual_range_contains)] #![cfg_attr(not(any(feature = "std", test)), no_std)] mod consts; diff --git a/src/phy/serial.rs b/src/phy/serial.rs index 7774f05..f020082 100644 --- a/src/phy/serial.rs +++ b/src/phy/serial.rs @@ -16,17 +16,11 @@ enum PhyData<'a> { impl PhyData<'_> { pub fn is_rx(&self) -> bool { - match self { - PhyData::Rx { .. } => true, - _ => false, - } + matches!(self, PhyData::Rx { .. }) } pub fn is_tx(&self) -> bool { - match self { - PhyData::Tx { .. } => true, - _ => false, - } + matches!(self, PhyData::Tx { .. }) } pub fn make_rx(&mut self) { diff --git a/src/phy/simulator.rs b/src/phy/simulator.rs index 37f1e90..69a7587 100644 --- a/src/phy/simulator.rs +++ b/src/phy/simulator.rs @@ -176,10 +176,9 @@ impl SimulatorBus { &self.stream[t.index..t.index + t.length] } - pub fn get_telegram(&self, t: &CapturedTelegram) -> Option { + pub fn get_telegram(&self, t: &CapturedTelegram) -> Option> { crate::fdl::Telegram::deserialize(self.get_telegram_data(t)) - .map(Result::ok) - .flatten() + .and_then(Result::ok) .map(|(t, _)| t) }