From c21ddf3cf0fe84a28ed49e20110c1c2e14cd30ff Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 6 Dec 2023 01:12:33 +0100 Subject: [PATCH] that reduced coverage again.. --- coverage.py | 2 +- src/cfdp/tlv/mod.rs | 56 +++++++++++++++++++++++++------- src/ecss/mod.rs | 2 +- src/ecss/tc.rs | 25 +++++++++------ src/time/cds.rs | 10 +++--- src/time/cuc.rs | 78 ++++++++++++++++++++++++++++++--------------- 6 files changed, 120 insertions(+), 53 deletions(-) diff --git a/coverage.py b/coverage.py index 73b81e9..cfbe006 100755 --- a/coverage.py +++ b/coverage.py @@ -13,7 +13,7 @@ def generate_cov_report(open_report: bool, format: str): os.environ["RUSTFLAGS"] = "-Cinstrument-coverage" os.environ["LLVM_PROFILE_FILE"] = "target/coverage/%p-%m.profraw" _LOGGER.info("Executing tests with coverage") - os.system("cargo test") + os.system("cargo test --all-features") out_path = "./target/debug/coverage" if format == "lcov": diff --git a/src/cfdp/tlv/mod.rs b/src/cfdp/tlv/mod.rs index d1ab1b2..dc63cd8 100644 --- a/src/cfdp/tlv/mod.rs +++ b/src/cfdp/tlv/mod.rs @@ -355,9 +355,20 @@ struct FilestoreTlvBase<'first_name, 'second_name> { pub second_name: Option>, } +impl FilestoreTlvBase<'_, '_> { + fn base_len_value(&self) -> usize { + let mut len = 1 + self.first_name.len_full(); + if let Some(second_name) = self.second_name { + len += second_name.len_full(); + } + len + } +} + #[derive(Debug, Copy, Clone, PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] pub struct FilestoreRequestTlv<'first_name, 'second_name> { + #[cfg_attr(feature = "serde", serde(borrow))] base: FilestoreTlvBase<'first_name, 'second_name>, } @@ -467,11 +478,7 @@ impl<'first_name, 'second_name> FilestoreRequestTlv<'first_name, 'second_name> { } pub fn len_value(&self) -> usize { - let mut len = 1 + self.base.first_name.len_full(); - if let Some(second_name) = self.base.second_name { - len += second_name.len_full(); - } - len + self.base.base_len_value() } pub fn len_full(&self) -> usize { @@ -554,8 +561,10 @@ impl GenericTlv for FilestoreRequestTlv<'_, '_> { #[derive(Debug, Copy, Clone, PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] pub struct FilestoreResponseTlv<'first_name, 'second_name, 'fs_msg> { + #[cfg_attr(feature = "serde", serde(borrow))] base: FilestoreTlvBase<'first_name, 'second_name>, status_code: u8, + #[cfg_attr(feature = "serde", serde(borrow))] filestore_message: Lv<'fs_msg>, } @@ -634,12 +643,7 @@ impl<'first_name, 'second_name, 'fs_msg> FilestoreResponseTlv<'first_name, 'seco } pub fn len_value(&self) -> usize { - let mut len = 1 + self.base.first_name.len_full(); - if let Some(second_name) = self.base.second_name { - len += second_name.len_full(); - } - len += self.filestore_message.len_full(); - len + self.base.base_len_value() + self.filestore_message.len_full() } pub fn len_full(&self) -> usize { @@ -948,6 +952,7 @@ mod tests { assert!(tlv.is_ok()); let tlv = tlv.unwrap(); assert_eq!(tlv.tlv_type_field(), TlvTypeField::Custom(3)); + assert!(!tlv.is_standard_tlv()); assert_eq!(tlv.value().len(), 1); assert_eq!(tlv.len_full(), 3); } @@ -1006,7 +1011,12 @@ mod tests { fs_request.len_value(), 1 + first_name.len_full() + second_name.len_full() ); + assert_eq!( + fs_request.tlv_type_field(), + TlvTypeField::Standard(TlvType::FilestoreRequest) + ); assert_eq!(fs_request.len_full(), fs_request.len_value() + 2); + assert_eq!(fs_request.len_written(), fs_request.len_full()); assert_eq!(fs_request.action_code(), action_code); assert_eq!(fs_request.first_name(), first_name); assert!(fs_request.second_name().is_some()); @@ -1138,7 +1148,7 @@ mod tests { } #[test] - fn test_fs_response_state() { + fn test_fs_response_state_one_path() { let lv_0 = Lv::new_from_str(TLV_TEST_STR_0).unwrap(); let response = FilestoreResponseTlv::new_no_filestore_message( FilestoreActionCode::CreateFile, @@ -1148,9 +1158,31 @@ mod tests { ) .expect("creating response failed"); assert_eq!(response.status_code(), 0b0001); + assert_eq!(response.action_code(), FilestoreActionCode::CreateFile); assert_eq!(response.first_name(), lv_0); assert!(response.second_name().is_none()); } + #[test] + fn test_fs_response_state_two_paths() { + let lv_0 = Lv::new_from_str(TLV_TEST_STR_0).unwrap(); + let lv_1 = Lv::new_from_str(TLV_TEST_STR_1).unwrap(); + let response = FilestoreResponseTlv::new_no_filestore_message( + FilestoreActionCode::RenameFile, + 0b0001, + lv_0, + Some(lv_1), + ) + .expect("creating response failed"); + assert_eq!(response.status_code(), 0b0001); + assert_eq!(response.action_code(), FilestoreActionCode::RenameFile); + assert_eq!(response.first_name(), lv_0); + assert!(response.second_name().is_some()); + assert!(response.second_name().unwrap() == lv_1); + assert_eq!( + response.len_full(), + 2 + 1 + lv_0.len_full() + lv_1.len_full() + 1 + ); + } #[test] fn test_fs_response_serialization() { diff --git a/src/ecss/mod.rs b/src/ecss/mod.rs index 9922129..3dd3ad4 100644 --- a/src/ecss/mod.rs +++ b/src/ecss/mod.rs @@ -171,7 +171,7 @@ impl Display for PusError { write!(f, "crc16 was not calculated") } PusError::ByteConversion(e) => { - write!(f, "low level byte conversion error: {e}") + write!(f, "pus error: {e}") } } } diff --git a/src/ecss/tc.rs b/src/ecss/tc.rs index 64ad575..c761126 100644 --- a/src/ecss/tc.rs +++ b/src/ecss/tc.rs @@ -873,6 +873,8 @@ impl PartialEq> for PusTcCreator<'_> { #[cfg(all(test, feature = "std"))] mod tests { + use std::error::Error; + use super::*; use crate::ecss::PusVersion::PusC; use crate::ecss::{PusError, PusPacket, WritablePusPacket}; @@ -1062,16 +1064,21 @@ mod tests { let res = pus_tc.write_to_bytes(test_buf.as_mut_slice()); assert!(res.is_err()); let err = res.unwrap_err(); - match err { - PusError::ByteConversion(err) => { - if let ByteConversionError::ToSliceTooSmall { found, expected } = err { - assert_eq!(expected, pus_tc.len_written()); - assert_eq!(found, 12); - } else { - panic!("Unexpected error") + if let PusError::ByteConversion(e) = err { + assert_eq!( + e, + ByteConversionError::ToSliceTooSmall { + found: 12, + expected: 13 } - } - _ => panic!("Unexpected error"), + ); + assert_eq!( + err.to_string(), + "pus error: target slice with size 12 is too small, expected size of at least 13" + ); + assert_eq!(err.source().unwrap().to_string(), e.to_string()); + } else { + panic!("unexpected error {err}"); } } diff --git a/src/time/cds.rs b/src/time/cds.rs index 909c006..3442819 100644 --- a/src/time/cds.rs +++ b/src/time/cds.rs @@ -408,7 +408,9 @@ impl DynCdsTimeProvider for TimeProvider {} /// # Example /// /// ``` -/// use spacepackets::time::cds::{TimeProvider, LengthOfDaySegment, get_dyn_time_provider_from_bytes}; +/// use spacepackets::time::cds::{ +/// TimeProvider, LengthOfDaySegment, get_dyn_time_provider_from_bytes, SubmillisPrecision, +/// }; /// use spacepackets::time::{TimeWriter, CcsdsTimeCodes, CcsdsTimeProvider}; /// /// let timestamp_now = TimeProvider::new_with_u16_days(24, 24); @@ -423,7 +425,7 @@ impl DynCdsTimeProvider for TimeProvider {} /// assert_eq!(dyn_provider.len_of_day_seg(), LengthOfDaySegment::Short16Bits); /// assert_eq!(dyn_provider.ccsds_days_as_u32(), 24); /// assert_eq!(dyn_provider.ms_of_day(), 24); -/// assert_eq!(dyn_provider.submillis_precision(), None); +/// assert_eq!(dyn_provider.submillis_precision(), SubmillisPrecision::Absent); /// } /// ``` #[cfg(feature = "alloc")] @@ -510,7 +512,7 @@ impl TimeProvider { /// using picosecond resolution, but significantly simplifies comparison of timestamps. pub fn precision_as_ns(&self) -> Option { match self.submillis_precision() { - SubmillisPrecision::Microseconds => Some(self.submillis as u32 * 1000), + SubmillisPrecision::Microseconds => Some(self.submillis * 1000), SubmillisPrecision::Picoseconds => Some(self.submillis / 1000), _ => None, } @@ -1159,7 +1161,7 @@ impl CcsdsTimeProvider for TimeProvider { - ns_since_last_sec += self.submillis() as u32 * 1000; + ns_since_last_sec += self.submillis() * 1000; } SubmillisPrecision::Picoseconds => { ns_since_last_sec += self.submillis() / 1000; diff --git a/src/time/cuc.rs b/src/time/cuc.rs index de1e728..b5dd95b 100644 --- a/src/time/cuc.rs +++ b/src/time/cuc.rs @@ -94,7 +94,6 @@ pub fn fractional_part_from_subsec_ns( #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] pub enum CucError { InvalidCounterWidth(u8), - InvalidFractionResolution(FractionalResolution), /// Invalid counter supplied. InvalidCounter { width: u8, @@ -112,9 +111,6 @@ impl Display for CucError { CucError::InvalidCounterWidth(w) => { write!(f, "invalid cuc counter byte width {w}") } - CucError::InvalidFractionResolution(w) => { - write!(f, "invalid cuc fractional part byte width {w:?}") - } CucError::InvalidCounter { width, counter } => { write!(f, "invalid cuc counter {counter} for width {width}") } @@ -288,6 +284,8 @@ impl TimeProviderCcsdsEpoch { .map_err(|e| e.into()) } + /// Generates a CUC timestamp from a UNIX timestamp with a width of 4. This width is able + /// to accomodate all possible UNIX timestamp values. pub fn from_unix_stamp( unix_stamp: &UnixTimestamp, res: FractionalResolution, @@ -299,13 +297,6 @@ impl TimeProviderCcsdsEpoch { unix_stamp.as_date_time().unwrap(), )); } - if ccsds_epoch > u32::MAX as i64 { - return Err(CucError::InvalidCounter { - width: 4, - counter: ccsds_epoch as u64, - } - .into()); - } let mut fractions = None; if let Some(subsec_millis) = unix_stamp.subsecond_millis { fractions = fractional_part_from_subsec_ns(res, subsec_millis as u64 * 10_u64.pow(6)); @@ -335,7 +326,6 @@ impl TimeProviderCcsdsEpoch { } pub fn set_fractions(&mut self, fractions: FractionalPart) -> Result<(), CucError> { - Self::verify_fractions_width(fractions.0)?; Self::verify_fractions_value(fractions)?; self.fractions = Some(fractions); self.update_p_field_fractions(); @@ -371,7 +361,6 @@ impl TimeProviderCcsdsEpoch { }); } if let Some(fractions) = fractions { - Self::verify_fractions_width(fractions.0)?; Self::verify_fractions_value(fractions)?; } Ok(Self { @@ -457,13 +446,6 @@ impl TimeProviderCcsdsEpoch { Ok(()) } - fn verify_fractions_width(width: FractionalResolution) -> Result<(), CucError> { - if width as u8 > 3 { - return Err(CucError::InvalidFractionResolution(width)); - } - Ok(()) - } - fn verify_fractions_value(val: FractionalPart) -> Result<(), CucError> { if val.1 > 2u32.pow((val.0 as u32) * 8) - 1 { return Err(CucError::InvalidFractions { @@ -1134,12 +1116,7 @@ mod tests { assert_eq!(fractions.1, 2_u32.pow(3 * 8) - 2); } - #[test] - fn add_duration_basic() { - let mut cuc_stamp = TimeProviderCcsdsEpoch::new(200); - cuc_stamp.set_fractional_resolution(FractionalResolution::FifteenUs); - let duration = Duration::from_millis(2500); - cuc_stamp += duration; + fn check_stamp_after_addition(cuc_stamp: &TimeProviderCcsdsEpoch) { assert_eq!(cuc_stamp.width_counter_pair().1, 202); let fractions = cuc_stamp.width_fractions_pair().unwrap().1; let expected_val = @@ -1152,6 +1129,41 @@ mod tests { assert!(cuc_stamp2.subsecond_millis().unwrap() <= 1); } + #[test] + fn add_duration_basic() { + let mut cuc_stamp = TimeProviderCcsdsEpoch::new(200); + cuc_stamp.set_fractional_resolution(FractionalResolution::FifteenUs); + let duration = Duration::from_millis(2500); + cuc_stamp += duration; + check_stamp_after_addition(&cuc_stamp); + } + + #[test] + fn add_duration_basic_on_ref() { + let mut cuc_stamp = TimeProviderCcsdsEpoch::new(200); + cuc_stamp.set_fractional_resolution(FractionalResolution::FifteenUs); + let duration = Duration::from_millis(2500); + let new_stamp = cuc_stamp + duration; + check_stamp_after_addition(&new_stamp); + } + + #[test] + fn add_duration_basic_no_fractions() { + let mut cuc_stamp = TimeProviderCcsdsEpoch::new(200); + let duration = Duration::from_millis(2000); + cuc_stamp += duration; + assert_eq!(cuc_stamp.counter(), 202); + assert_eq!(cuc_stamp.width_fractions_pair(), None); + } + + #[test] + fn add_duration_basic_on_ref_no_fractions() { + let cuc_stamp = TimeProviderCcsdsEpoch::new(200); + let duration = Duration::from_millis(2000); + let new_stamp = cuc_stamp + duration; + assert_eq!(new_stamp.counter(), 202); + assert_eq!(new_stamp.width_fractions_pair(), None); + } #[test] fn add_duration_overflow() { let mut cuc_stamp = @@ -1200,4 +1212,18 @@ mod tests { (-DAYS_CCSDS_TO_UNIX * SECONDS_PER_DAY as i32) as u32 ); } + + #[test] + fn test_invalid_counter() { + let cuc_error = TimeProviderCcsdsEpoch::new_generic(WidthCounterPair(1, 256), None); + assert!(cuc_error.is_err()); + let cuc_error = cuc_error.unwrap_err(); + if let CucError::InvalidCounter { width, counter } = cuc_error { + assert_eq!(width, 1); + assert_eq!(counter, 256); + assert_eq!(cuc_error.to_string(), "invalid cuc counter 256 for width 1"); + } else { + panic!("unexpected error: {}", cuc_error); + } + } }