diff --git a/src/cfdp/pdu/ack.rs b/src/cfdp/pdu/ack.rs index 1c4c241..91bfbca 100644 --- a/src/cfdp/pdu/ack.rs +++ b/src/cfdp/pdu/ack.rs @@ -63,14 +63,13 @@ impl AckPdu { condition_code: ConditionCode, transaction_status: TransactionStatus, ) -> Self { - // Unwrap okay here, [new] can only fail on invalid directive codes. Self::new( pdu_header, FileDirectiveType::Eof, condition_code, transaction_status, ) - .unwrap() + .expect("only fails when directive codes are invalid") } /// Constructor for an ACK PDU acknowledging a Finished PDU. @@ -81,14 +80,13 @@ impl AckPdu { condition_code: ConditionCode, transaction_status: TransactionStatus, ) -> Self { - // Unwrap okay here, [new] can only fail on invalid directive codes. Self::new( pdu_header, FileDirectiveType::Finished, condition_code, transaction_status, ) - .unwrap() + .expect("only fails when directive codes are invalid") } /// PDU header. diff --git a/src/ecss/mod.rs b/src/ecss/mod.rs index 4a1ad12..00c9c8f 100644 --- a/src/ecss/mod.rs +++ b/src/ecss/mod.rs @@ -197,8 +197,9 @@ pub enum PusError { #[error("checksum verification for crc16 {0:#06x} failed")] ChecksumFailure(u16), /// CRC16 needs to be calculated first - //#[error("crc16 was not calculated")] - //CrcCalculationMissing, + #[error("crc16 was not calculated")] + CrcCalculationMissing, + /// Raised when converting to and from raw byte slices #[error("pus error: {0}")] ByteConversion(#[from] ByteConversionError), } diff --git a/src/ecss/tc.rs b/src/ecss/tc.rs index 0597d10..d706dc1 100644 --- a/src/ecss/tc.rs +++ b/src/ecss/tc.rs @@ -148,7 +148,7 @@ pub mod zc { impl TryFrom for PusTcSecondaryHeader { type Error = PusError; fn try_from(value: crate::ecss::tc::PusTcSecondaryHeader) -> Result { - if value.version != PusVersion::PusC { + if !matches!(value.version, PusVersion::PusC) { return Err(PusError::VersionNotSupported(value.version.raw_value())); } Ok(PusTcSecondaryHeader { @@ -418,7 +418,8 @@ impl<'app_data> PusTcCreator<'app_data> { let mut digest = CRC_CCITT_FALSE.digest(); let sph_zc = crate::zc::SpHeader::from(self.sp_header); digest.update(sph_zc.as_bytes()); - let pus_tc_header = zc::PusTcSecondaryHeader::try_from(self.sec_header).unwrap(); + let pus_tc_header = zc::PusTcSecondaryHeader::try_from(self.sec_header) + .expect("the PUS version is hardcoded to PUS C"); digest.update(pus_tc_header.as_bytes()); digest.update(self.app_data); digest.finalize() @@ -429,7 +430,8 @@ impl<'app_data> PusTcCreator<'app_data> { let mut digest = CRC_CCITT_FALSE_NO_TABLE.digest(); let sph_zc = crate::zc::SpHeader::from(self.sp_header); digest.update(sph_zc.as_bytes()); - let pus_tc_header = zc::PusTcSecondaryHeader::try_from(self.sec_header).unwrap(); + let pus_tc_header = zc::PusTcSecondaryHeader::try_from(self.sec_header) + .expect("the PUS version is hardcoded to PUS C"); digest.update(pus_tc_header.as_bytes()); digest.update(self.app_data); digest.finalize() @@ -450,7 +452,8 @@ impl<'app_data> PusTcCreator<'app_data> { let start_idx = vec.len(); vec.extend_from_slice(sph_zc.as_bytes()); // The PUS version is hardcoded to PUS C - let pus_tc_header = zc::PusTcSecondaryHeader::try_from(self.sec_header).unwrap(); + let pus_tc_header = zc::PusTcSecondaryHeader::try_from(self.sec_header) + .expect("the PUS version is hardcoded to PUS C"); vec.extend_from_slice(pus_tc_header.as_bytes()); vec.extend_from_slice(self.app_data); if self.has_checksum() { @@ -664,11 +667,11 @@ impl<'buf> PusTcCreatorWithReservedAppData<'buf> { sp_header.write_to_be_bytes(&mut buf[0..CCSDS_HEADER_LEN])?; curr_idx += CCSDS_HEADER_LEN; let sec_header_len = size_of::(); - let sec_header_zc = zc::PusTcSecondaryHeader::try_from(sec_header).unwrap(); - // Unwrap okay, this can not fail. + let sec_header_zc = zc::PusTcSecondaryHeader::try_from(sec_header) + .expect("the PUS version is hardcoded to PUS C"); sec_header_zc .write_to(&mut buf[curr_idx..curr_idx + sec_header_len]) - .unwrap(); + .expect("buffer is large enough for secondary header"); curr_idx += sec_header_len; let app_data_offset = curr_idx; curr_idx += app_data_len; @@ -916,8 +919,10 @@ impl<'raw_data> PusTcReader<'raw_data> { /// a CRC-16-CCITT checksum which is also verified. pub fn new(slice: &'raw_data [u8]) -> Result { let pus_tc = Self::new_no_checksum_verification(slice, true)?; - // Unwrap for CRC16 okay, should always have some value. - verify_crc16_ccitt_false_from_raw_to_pus_error(pus_tc.raw_data(), pus_tc.crc16().unwrap())?; + verify_crc16_ccitt_false_from_raw_to_pus_error( + pus_tc.raw_data(), + pus_tc.crc16().ok_or(PusError::CrcCalculationMissing)?, + )?; Ok(pus_tc) } @@ -925,10 +930,9 @@ impl<'raw_data> PusTcReader<'raw_data> { /// binary size and memory usage. pub fn new_checksum_no_table(slice: &'raw_data [u8]) -> Result { let pus_tc = Self::new_no_checksum_verification(slice, true)?; - // Unwrap for CRC16 okay, should always have some value. verify_crc16_ccitt_false_from_raw_to_pus_error_no_table( pus_tc.raw_data(), - pus_tc.crc16().unwrap(), + pus_tc.crc16().ok_or(PusError::CrcCalculationMissing)?, )?; Ok(pus_tc) } @@ -976,10 +980,10 @@ impl<'raw_data> PusTcReader<'raw_data> { .unwrap(); current_idx += PUC_TC_SECONDARY_HEADER_LEN; let raw_data = &slice[0..total_len]; - let mut crc16 = None; - if has_checksum { - crc16 = Some(crc_from_raw_data(&slice[total_len - 2..total_len])?); - } + let crc16 = match has_checksum { + true => Some(crc_from_raw_data(&slice[total_len - 2..total_len])?), + false => None, + }; Ok(Self { sp_header, sec_header: PusTcSecondaryHeader::try_from(sec_header).unwrap(), diff --git a/src/ecss/tm_pus_a.rs b/src/ecss/tm_pus_a.rs index 47ddcef..dfe2dcd 100644 --- a/src/ecss/tm_pus_a.rs +++ b/src/ecss/tm_pus_a.rs @@ -192,11 +192,8 @@ impl<'stamp> PusTmSecondaryHeader<'stamp> { .into()); } let pus_version = PusVersion::try_from(u4::new((buf[0] >> 4) & 0x0F)); - if let Err(version_raw) = pus_version { - return Err(PusError::VersionNotSupported(version_raw)); - } - let pus_version = pus_version.unwrap(); - if pus_version != PusVersion::PusA { + let pus_version = pus_version.map_err(PusError::VersionNotSupported)?; + if !matches!(pus_version, PusVersion::PusA) { return Err(PusError::VersionNotSupported(pus_version.raw_value())); } let mut msg_counter = None; @@ -466,9 +463,9 @@ impl<'time, 'src_data> PusTmCreator<'time, 'src_data> { if let Some(dest_id) = self.sec_header.dest_id { let mut dest_id_buf: [u8; core::mem::size_of::()] = [0; core::mem::size_of::()]; - // Unwrap okay, this can never fail because we created a buffer with the largest - // possible size. - let len = dest_id.write_to_be_bytes(&mut dest_id_buf).unwrap(); + let len = dest_id + .write_to_be_bytes(&mut dest_id_buf) + .expect("buffer is maximum possible size; this operation cannot fail"); digest.update(&dest_id_buf[0..len]); } digest.update(self.sec_header.timestamp); diff --git a/src/lib.rs b/src/lib.rs index f880ce5..4718b75 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1250,9 +1250,8 @@ impl CcsdsPacketCreatorCommon { #[cfg(feature = "alloc")] pub fn to_vec(&self, len_written: usize, packet_data: &[u8]) -> alloc::vec::Vec { let mut vec = alloc::vec![0u8; len_written]; - // Can not fail, unless we messed up the len_written method.. self.write_to_bytes(&mut vec, len_written, packet_data) - .unwrap(); + .expect("buffer size shall match the len_written"); vec } } diff --git a/src/time/cds.rs b/src/time/cds.rs index 09e246e..3c0b5ef 100644 --- a/src/time/cds.rs +++ b/src/time/cds.rs @@ -296,11 +296,6 @@ impl CdsTime { /// Please note that a precision value of 0 will be converted to [None] (no precision). pub fn set_submillis(&mut self, prec: SubmillisPrecision, value: u32) -> bool { self.pfield &= !(0b11); - if let SubmillisPrecision::Absent = prec { - // self.submillis_precision = prec; - self.submillis = 0; - return true; - } // self.submillis_precision = prec; match prec { SubmillisPrecision::Microseconds => { @@ -314,7 +309,10 @@ impl CdsTime { self.pfield |= SubmillisPrecision::Picoseconds as u8; self.submillis = value; } - _ => (), + SubmillisPrecision::Absent => { + self.submillis = 0; + } + SubmillisPrecision::Reserved => (), } true } @@ -353,23 +351,18 @@ impl CdsTime { )); } let pfield = buf[0]; - match CcsdsTimeCode::try_from((pfield >> 4) & 0b111) { - Ok(cds_type) => match cds_type { - CcsdsTimeCode::Cds => (), - _ => { - return Err(TimestampError::InvalidTimeCode { - expected: CcsdsTimeCode::Cds, - found: cds_type as u8, - }) - } - }, - _ => { - return Err(TimestampError::InvalidTimeCode { - expected: CcsdsTimeCode::Cds, - found: (pfield >> 4) & 0b111, - }); + let cds_type = CcsdsTimeCode::try_from((pfield >> 4) & 0b111).map_err(|_| { + TimestampError::InvalidTimeCode { + expected: CcsdsTimeCode::Cds, + found: (pfield >> 4) & 0b111, } - }; + })?; + if !matches!(cds_type, CcsdsTimeCode::Cds) { + return Err(TimestampError::InvalidTimeCode { + expected: CcsdsTimeCode::Cds, + found: cds_type as u8, + }); + } if ((pfield >> 3) & 0b1) == 1 { return Err(TimestampError::CustomEpochNotSupported); } @@ -394,14 +387,10 @@ impl CdsTime { if length_of_day_segment_from_pfield(pfield) == LengthOfDaySegment::Long24Bits { init_len += 1 } - match pfield & 0b11 { - 0b01 => { - init_len += 2; - } - 0b10 => { - init_len += 4; - } - _ => (), + match precision_from_pfield(pfield) { + SubmillisPrecision::Microseconds => init_len += 2, + SubmillisPrecision::Picoseconds => init_len += 4, + SubmillisPrecision::Absent | SubmillisPrecision::Reserved => (), } init_len } @@ -534,17 +523,19 @@ impl CdsTime { Ok(match self.submillis_precision() { SubmillisPrecision::Microseconds => ConversionFromNow::new_with_submillis_us_prec()?, SubmillisPrecision::Picoseconds => ConversionFromNow::new_with_submillis_ps_prec()?, - _ => ConversionFromNow::new()?, + SubmillisPrecision::Absent | SubmillisPrecision::Reserved => ConversionFromNow::new()?, }) } fn generate_p_field(day_seg_len: LengthOfDaySegment, submillis_prec: SubmillisPrecision) -> u8 { let mut pfield = P_FIELD_BASE | ((day_seg_len as u8) << 2); - match submillis_prec { - SubmillisPrecision::Microseconds => pfield |= SubmillisPrecision::Microseconds as u8, - SubmillisPrecision::Picoseconds => pfield |= SubmillisPrecision::Picoseconds as u8, - SubmillisPrecision::Reserved => pfield |= SubmillisPrecision::Reserved as u8, - _ => (), + if matches!( + submillis_prec, + SubmillisPrecision::Microseconds + | SubmillisPrecision::Picoseconds + | SubmillisPrecision::Reserved + ) { + pfield |= submillis_prec as u8 } pfield } @@ -652,17 +643,17 @@ impl CdsTime { match submillis_precision { SubmillisPrecision::Microseconds => { provider.set_submillis( - SubmillisPrecision::Microseconds, + submillis_precision, u16::from_be_bytes(buf[8..10].try_into().unwrap()) as u32, ); } SubmillisPrecision::Picoseconds => { provider.set_submillis( - SubmillisPrecision::Picoseconds, + submillis_precision, u32::from_be_bytes(buf[8..12].try_into().unwrap()), ); } - _ => (), + SubmillisPrecision::Absent | SubmillisPrecision::Reserved => (), } Ok(provider) } @@ -763,17 +754,17 @@ impl CdsTime { match submillis_precision { SubmillisPrecision::Microseconds => { provider.set_submillis( - SubmillisPrecision::Microseconds, + submillis_precision, u16::from_be_bytes(buf[7..9].try_into().unwrap()) as u32, ); } SubmillisPrecision::Picoseconds => { provider.set_submillis( - SubmillisPrecision::Picoseconds, + submillis_precision, u32::from_be_bytes(buf[7..11].try_into().unwrap()), ); } - _ => (), + SubmillisPrecision::Absent | SubmillisPrecision::Reserved => (), } Ok(provider) } @@ -937,7 +928,7 @@ impl CdsTime { SubmillisPrecision::Picoseconds => { buf[8..12].copy_from_slice(self.submillis().to_be_bytes().as_slice()); } - _ => (), + SubmillisPrecision::Absent | SubmillisPrecision::Reserved => (), } Ok(self.len_as_bytes()) } @@ -970,24 +961,20 @@ impl PartialOrd for CdsTime return Some(Ordering::Less), - Ordering::Greater => return Some(Ordering::Greater), - _ => (), + let ordering = self.ccsds_days_as_u32().cmp(&other.ccsds_days_as_u32()); + if ordering != Ordering::Equal { + return Some(ordering); } - match self.ms_of_day().cmp(&other.ms_of_day()) { - Ordering::Less => return Some(Ordering::Less), - Ordering::Greater => return Some(Ordering::Greater), - _ => (), + let ordering = self.ms_of_day().cmp(&other.ms_of_day()); + if ordering != Ordering::Equal { + return Some(ordering); } - match self + let ordering = self .precision_as_ns() .unwrap_or(0) - .cmp(&other.precision_as_ns().unwrap_or(0)) - { - Ordering::Less => return Some(Ordering::Less), - Ordering::Greater => return Some(Ordering::Greater), - _ => (), + .cmp(&other.precision_as_ns().unwrap_or(0)); + if ordering != Ordering::Equal { + return Some(ordering); } Some(Ordering::Equal) } @@ -1051,7 +1038,7 @@ impl ConversionFromUnix { let submillis = match precision { SubmillisPrecision::Microseconds => (subsec_nanos / 1_000) % 1000, SubmillisPrecision::Picoseconds => (subsec_nanos % 10_u32.pow(6)) * 1000, - _ => 0, + SubmillisPrecision::Absent | SubmillisPrecision::Reserved => 0, }; Ok(Self { ccsds_days: unix_to_ccsds_days(unix_days) as u32, @@ -1161,16 +1148,12 @@ impl ConversionFromChronoDatetime { // The contained values in the conversion should be all positive now let unix_conversion = ConversionFromUnix::new(dt.timestamp(), dt.timestamp_subsec_nanos(), prec)?; - let mut submillis = 0; - match prec { - SubmillisPrecision::Microseconds => { - submillis = dt.timestamp_subsec_micros() % 1000; - } - SubmillisPrecision::Picoseconds => { - submillis = (dt.timestamp_subsec_nanos() % 10_u32.pow(6)) * 1000; - } - _ => (), - } + + let submillis = match prec { + SubmillisPrecision::Microseconds => dt.timestamp_subsec_micros() % 1000, + SubmillisPrecision::Picoseconds => (dt.timestamp_subsec_nanos() % 10_u32.pow(6)) * 1000, + SubmillisPrecision::Absent | SubmillisPrecision::Reserved => 0, + }; Ok(Self { unix_conversion, submillis_prec: prec, @@ -1207,17 +1190,12 @@ impl ConversionFromNow { // so it is okay to unwrap let unix_conversion = ConversionFromUnix::new(epoch as i64, now.subsec_nanos(), prec).unwrap(); - let mut submillis = 0; - match prec { - SubmillisPrecision::Microseconds => { - submillis = now.subsec_micros() % 1000; - } - SubmillisPrecision::Picoseconds => { - submillis = (now.subsec_nanos() % 10_u32.pow(6)) * 1000; - } - _ => (), - } + let submillis = match prec { + SubmillisPrecision::Microseconds => now.subsec_micros() % 1000, + SubmillisPrecision::Picoseconds => (now.subsec_nanos() % 10_u32.pow(6)) * 1000, + SubmillisPrecision::Absent | SubmillisPrecision::Reserved => 0, + }; Ok(Self { unix_conversion, submillis_prec: prec, diff --git a/src/time/cuc.rs b/src/time/cuc.rs index 4ca8e13..4685c38 100644 --- a/src/time/cuc.rs +++ b/src/time/cuc.rs @@ -866,8 +866,8 @@ impl Add for CucTime { fn add(self, duration: Duration) -> Self::Output { let (new_counter, new_fractional_part) = get_time_values_after_duration_addition(&self, duration); - // The generated fractional part should always be valid, so its okay to unwrap here. - Self::new_with_fractions(new_counter, new_fractional_part).unwrap() + Self::new_with_fractions(new_counter, new_fractional_part) + .expect("The generated fractional part should always be valid") } } @@ -877,8 +877,8 @@ impl Add for &CucTime { fn add(self, duration: Duration) -> Self::Output { let (new_counter, new_fractional_part) = get_time_values_after_duration_addition(self, duration); - // The generated fractional part should always be valid, so its okay to unwrap here. - Self::Output::new_with_fractions(new_counter, new_fractional_part).unwrap() + Self::Output::new_with_fractions(new_counter, new_fractional_part) + .expect("The generated fractional part should always be valid") } }