From 9976b53f65dd560a2555e39a042b1b7641ddd675 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 26 Nov 2023 15:04:44 +0100 Subject: [PATCH 01/19] added basic state tests for FD and Finished PDU --- src/cfdp/pdu/file_data.rs | 39 ++++++++++++++++++++++----------------- src/cfdp/pdu/finished.rs | 22 ++++++++++++++++++++-- 2 files changed, 42 insertions(+), 19 deletions(-) diff --git a/src/cfdp/pdu/file_data.rs b/src/cfdp/pdu/file_data.rs index 928b369..d37623b 100644 --- a/src/cfdp/pdu/file_data.rs +++ b/src/cfdp/pdu/file_data.rs @@ -239,17 +239,15 @@ impl WritablePduPacket for FileDataPdu<'_, '_> { #[cfg(test)] mod tests { use super::*; + use crate::cfdp::pdu::tests::{TEST_DEST_ID, TEST_SEQ_NUM, TEST_SRC_ID}; use crate::cfdp::pdu::{CommonPduConfig, PduHeader}; - use crate::cfdp::{SegmentMetadataFlag, SegmentationControl}; + use crate::cfdp::{Direction, SegmentMetadataFlag, SegmentationControl, TransmissionMode}; use crate::util::UbfU8; - const SRC_ID: UbfU8 = UbfU8::new(1); - const DEST_ID: UbfU8 = UbfU8::new(2); - const SEQ_NUM: UbfU8 = UbfU8::new(3); - #[test] fn test_basic() { - let common_conf = CommonPduConfig::new_with_byte_fields(SRC_ID, DEST_ID, SEQ_NUM).unwrap(); + let common_conf = + CommonPduConfig::new_with_byte_fields(TEST_SRC_ID, TEST_DEST_ID, TEST_SEQ_NUM).unwrap(); let pdu_header = PduHeader::new_for_file_data_default(common_conf, 0); let file_data: [u8; 4] = [1, 2, 3, 4]; let fd_pdu = FileDataPdu::new_no_seg_metadata(pdu_header, 10, &file_data); @@ -260,11 +258,22 @@ mod tests { fd_pdu.len_written(), fd_pdu.pdu_header.header_len() + core::mem::size_of::() + 4 ); + + assert_eq!(fd_pdu.crc_flag(), CrcFlag::NoCrc); + assert_eq!(fd_pdu.file_flag(), LargeFileFlag::Normal); + assert_eq!(fd_pdu.pdu_type(), PduType::FileData); + assert_eq!(fd_pdu.file_directive_type(), None); + assert_eq!(fd_pdu.transmission_mode(), TransmissionMode::Acknowledged); + assert_eq!(fd_pdu.direction(), Direction::TowardsReceiver); + assert_eq!(fd_pdu.source_id(), TEST_SRC_ID.into()); + assert_eq!(fd_pdu.dest_id(), TEST_DEST_ID.into()); + assert_eq!(fd_pdu.transaction_seq_num(), TEST_SEQ_NUM.into()); } #[test] fn test_serialization() { - let common_conf = CommonPduConfig::new_with_byte_fields(SRC_ID, DEST_ID, SEQ_NUM).unwrap(); + let common_conf = + CommonPduConfig::new_with_byte_fields(TEST_SRC_ID, TEST_DEST_ID, TEST_SEQ_NUM).unwrap(); let pdu_header = PduHeader::new_for_file_data_default(common_conf, 0); let file_data: [u8; 4] = [1, 2, 3, 4]; let fd_pdu = FileDataPdu::new_no_seg_metadata(pdu_header, 10, &file_data); @@ -295,7 +304,8 @@ mod tests { #[test] fn test_write_to_vec() { - let common_conf = CommonPduConfig::new_with_byte_fields(SRC_ID, DEST_ID, SEQ_NUM).unwrap(); + let common_conf = + CommonPduConfig::new_with_byte_fields(TEST_SRC_ID, TEST_DEST_ID, TEST_SEQ_NUM).unwrap(); let pdu_header = PduHeader::new_for_file_data_default(common_conf, 0); let file_data: [u8; 4] = [1, 2, 3, 4]; let fd_pdu = FileDataPdu::new_no_seg_metadata(pdu_header, 10, &file_data); @@ -307,7 +317,8 @@ mod tests { #[test] fn test_deserialization() { - let common_conf = CommonPduConfig::new_with_byte_fields(SRC_ID, DEST_ID, SEQ_NUM).unwrap(); + let common_conf = + CommonPduConfig::new_with_byte_fields(TEST_SRC_ID, TEST_DEST_ID, TEST_SEQ_NUM).unwrap(); let pdu_header = PduHeader::new_for_file_data_default(common_conf, 0); let file_data: [u8; 4] = [1, 2, 3, 4]; let fd_pdu = FileDataPdu::new_no_seg_metadata(pdu_header, 10, &file_data); @@ -321,11 +332,8 @@ mod tests { #[test] fn test_with_seg_metadata_serialization() { - let src_id = UbfU8::new(1); - let dest_id = UbfU8::new(2); - let transaction_seq_num = UbfU8::new(3); let common_conf = - CommonPduConfig::new_with_byte_fields(src_id, dest_id, transaction_seq_num).unwrap(); + CommonPduConfig::new_with_byte_fields(TEST_SRC_ID, TEST_DEST_ID, TEST_SEQ_NUM).unwrap(); let pdu_header = PduHeader::new_for_file_data( common_conf, 0, @@ -386,11 +394,8 @@ mod tests { #[test] fn test_with_seg_metadata_deserialization() { - let src_id = UbfU8::new(1); - let dest_id = UbfU8::new(2); - let transaction_seq_num = UbfU8::new(3); let common_conf = - CommonPduConfig::new_with_byte_fields(src_id, dest_id, transaction_seq_num).unwrap(); + CommonPduConfig::new_with_byte_fields(TEST_SRC_ID, TEST_DEST_ID, TEST_SEQ_NUM).unwrap(); let pdu_header = PduHeader::new_for_file_data( common_conf, 0, diff --git a/src/cfdp/pdu/finished.rs b/src/cfdp/pdu/finished.rs index 1a5f8c2..f200af7 100644 --- a/src/cfdp/pdu/finished.rs +++ b/src/cfdp/pdu/finished.rs @@ -260,9 +260,11 @@ impl WritablePduPacket for FinishedPdu<'_> { #[cfg(test)] mod tests { use super::*; - use crate::cfdp::pdu::tests::{common_pdu_conf, verify_raw_header}; + use crate::cfdp::pdu::tests::{ + common_pdu_conf, verify_raw_header, TEST_DEST_ID, TEST_SEQ_NUM, TEST_SRC_ID, + }; use crate::cfdp::pdu::{FileDirectiveType, PduHeader}; - use crate::cfdp::{ConditionCode, CrcFlag, Direction, LargeFileFlag}; + use crate::cfdp::{ConditionCode, CrcFlag, Direction, LargeFileFlag, TransmissionMode}; fn generic_finished_pdu( crc_flag: CrcFlag, @@ -292,6 +294,22 @@ mod tests { assert_eq!(finished_pdu.filestore_responses(), None); assert_eq!(finished_pdu.fault_location(), None); assert_eq!(finished_pdu.pdu_header().pdu_datafield_len, 2); + + assert_eq!(finished_pdu.crc_flag(), CrcFlag::NoCrc); + assert_eq!(finished_pdu.file_flag(), LargeFileFlag::Normal); + assert_eq!(finished_pdu.pdu_type(), PduType::FileDirective); + assert_eq!( + finished_pdu.file_directive_type(), + Some(FileDirectiveType::FinishedPdu) + ); + assert_eq!( + finished_pdu.transmission_mode(), + TransmissionMode::Acknowledged + ); + assert_eq!(finished_pdu.direction(), Direction::TowardsSender); + assert_eq!(finished_pdu.source_id(), TEST_SRC_ID.into()); + assert_eq!(finished_pdu.dest_id(), TEST_DEST_ID.into()); + assert_eq!(finished_pdu.transaction_seq_num(), TEST_SEQ_NUM.into()); } fn generic_serialization_test_no_error(delivery_code: DeliveryCode, file_status: FileStatus) { From fdf6e1de90c805e425fa9ea2fd469adee7dc321a Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 26 Nov 2023 21:24:24 +0100 Subject: [PATCH 02/19] added ACK PDU impl --- src/cfdp/mod.rs | 34 +++++++ src/cfdp/pdu/ack.rs | 199 ++++++++++++++++++++++++++++++++++++++ src/cfdp/pdu/eof.rs | 32 +++++- src/cfdp/pdu/file_data.rs | 10 +- src/cfdp/pdu/finished.rs | 18 +++- src/cfdp/pdu/metadata.rs | 5 +- src/cfdp/pdu/mod.rs | 7 +- src/cfdp/pdu/nak.rs | 1 + 8 files changed, 290 insertions(+), 16 deletions(-) create mode 100644 src/cfdp/pdu/ack.rs create mode 100644 src/cfdp/pdu/nak.rs diff --git a/src/cfdp/mod.rs b/src/cfdp/mod.rs index 27e5524..cab3351 100644 --- a/src/cfdp/mod.rs +++ b/src/cfdp/mod.rs @@ -48,6 +48,24 @@ pub enum CrcFlag { WithCrc = 1, } +impl From for CrcFlag { + fn from(value: bool) -> Self { + if value { + return CrcFlag::WithCrc; + } + CrcFlag::NoCrc + } +} + +impl From for bool { + fn from(value: CrcFlag) -> Self { + if value == CrcFlag::WithCrc { + return true; + } + false + } +} + /// Always 0 and ignored for File Directive PDUs (CCSDS 727.0-B-5 P.75) #[derive(Debug, Copy, Clone, PartialEq, Eq, TryFromPrimitive, IntoPrimitive)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] @@ -119,6 +137,22 @@ pub enum LargeFileFlag { Large = 1, } +/// Transaction status for the ACK PDU field according to chapter 5.2.4 of the CFDP standard. +#[derive(Debug, Copy, Clone, PartialEq, Eq, TryFromPrimitive, IntoPrimitive)] +#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] +#[repr(u8)] +pub enum TransactionStatus { + /// Transaction is not currently active and the CFDP implementation does not retain a + /// transaction history. + Undefined = 0b00, + Active = 0b01, + /// Transaction was active in the past and was terminated. + Terminated = 0b10, + /// The CFDP implementation does retain a tranaction history, and the transaction is not and + /// never was active at this entity. + Unrecognized = 0b11, +} + /// Checksum types according to the /// [SANA Checksum Types registry](https://sanaregistry.org/r/checksum_identifiers/) #[derive(Debug, Copy, Clone, PartialEq, Eq, TryFromPrimitive, IntoPrimitive)] diff --git a/src/cfdp/pdu/ack.rs b/src/cfdp/pdu/ack.rs new file mode 100644 index 0000000..979015b --- /dev/null +++ b/src/cfdp/pdu/ack.rs @@ -0,0 +1,199 @@ +use crate::{ + cfdp::{ConditionCode, CrcFlag, Direction, TransactionStatus}, + ByteConversionError, +}; + +use super::{ + add_pdu_crc, generic_length_checks_pdu_deserialization, CfdpPdu, FileDirectiveType, PduError, + PduHeader, WritablePduPacket, +}; + +/// ACK PDU abstraction. +/// +/// For more information, refer to CFDP chapter 5.2.4. +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] +pub struct AckPdu { + pdu_header: PduHeader, + directive_code_of_acked_pdu: FileDirectiveType, + condition_code: ConditionCode, + transaction_status: TransactionStatus, +} + +impl AckPdu { + pub fn new( + mut pdu_header: PduHeader, + directive_code_of_acked_pdu: FileDirectiveType, + condition_code: ConditionCode, + transaction_status: TransactionStatus, + ) -> Result { + if directive_code_of_acked_pdu == FileDirectiveType::EofPdu { + pdu_header.pdu_conf.direction = Direction::TowardsSender; + } else if directive_code_of_acked_pdu == FileDirectiveType::FinishedPdu { + pdu_header.pdu_conf.direction = Direction::TowardsReceiver; + } else { + return Err(PduError::InvalidDirectiveType { + found: directive_code_of_acked_pdu as u8, + expected: None, + }); + } + // Force correct direction flag. + let mut ack_pdu = Self { + pdu_header, + directive_code_of_acked_pdu, + condition_code, + transaction_status, + }; + ack_pdu.pdu_header.pdu_datafield_len = ack_pdu.calc_pdu_datafield_len() as u16; + Ok(ack_pdu) + } + + pub fn new_for_eof_pdu( + pdu_header: PduHeader, + condition_code: ConditionCode, + transaction_status: TransactionStatus, + ) -> Self { + // Unwrap okay here, [new] can only fail on invalid directive codes. + Self::new( + pdu_header, + FileDirectiveType::EofPdu, + condition_code, + transaction_status, + ) + .unwrap() + } + + pub fn new_for_finished_pdu( + pdu_header: PduHeader, + condition_code: ConditionCode, + transaction_status: TransactionStatus, + ) -> Self { + // Unwrap okay here, [new] can only fail on invalid directive codes. + Self::new( + pdu_header, + FileDirectiveType::FinishedPdu, + condition_code, + transaction_status, + ) + .unwrap() + } + + pub fn pdu_header(&self) -> &PduHeader { + &self.pdu_header + } + + pub fn directive_code_of_acked_pdu(&self) -> FileDirectiveType { + self.directive_code_of_acked_pdu + } + + pub fn condition_code(&self) -> ConditionCode { + self.condition_code + } + + pub fn transaction_status(&self) -> TransactionStatus { + self.transaction_status + } + + fn calc_pdu_datafield_len(&self) -> usize { + if self.crc_flag() == CrcFlag::WithCrc { + return 5; + } + 3 + } + + pub fn from_bytes(buf: &[u8]) -> Result { + let (pdu_header, mut current_idx) = PduHeader::from_bytes(buf)?; + let full_len_without_crc = pdu_header.verify_length_and_checksum(buf)?; + generic_length_checks_pdu_deserialization(buf, current_idx + 3, full_len_without_crc)?; + let directive_type = FileDirectiveType::try_from(buf[current_idx]).map_err(|_| { + PduError::InvalidDirectiveType { + found: buf[current_idx], + expected: Some(FileDirectiveType::AckPdu), + } + })?; + if directive_type != FileDirectiveType::AckPdu { + return Err(PduError::WrongDirectiveType { + found: directive_type, + expected: FileDirectiveType::AckPdu, + }); + } + current_idx += 1; + let acked_directive_type = + FileDirectiveType::try_from(buf[current_idx] >> 4).map_err(|_| { + PduError::InvalidDirectiveType { + found: buf[current_idx], + expected: None, + } + })?; + if acked_directive_type != FileDirectiveType::EofPdu + && acked_directive_type != FileDirectiveType::FinishedPdu + { + return Err(PduError::InvalidDirectiveType { + found: acked_directive_type as u8, + expected: None, + }); + } + current_idx += 1; + let condition_code = ConditionCode::try_from((buf[current_idx] >> 4) & 0b1111) + .map_err(|_| PduError::InvalidConditionCode((buf[current_idx] >> 4) & 0b1111))?; + let transaction_status = TransactionStatus::try_from(buf[current_idx] & 0b11).unwrap(); + Self::new( + pdu_header, + acked_directive_type, + condition_code, + transaction_status, + ) + } +} + +impl CfdpPdu for AckPdu { + fn pdu_header(&self) -> &PduHeader { + &self.pdu_header + } + + fn file_directive_type(&self) -> Option { + Some(FileDirectiveType::AckPdu) + } +} + +impl WritablePduPacket for AckPdu { + fn write_to_bytes(&self, buf: &mut [u8]) -> Result { + let expected_len = self.len_written(); + if buf.len() < expected_len { + return Err(ByteConversionError::ToSliceTooSmall { + found: buf.len(), + expected: expected_len, + } + .into()); + } + let mut current_idx = self.pdu_header.write_to_bytes(buf)?; + buf[current_idx] = FileDirectiveType::AckPdu as u8; + current_idx += 1; + + buf[current_idx] = (self.directive_code_of_acked_pdu as u8) << 4; + if self.directive_code_of_acked_pdu == FileDirectiveType::FinishedPdu { + // This is the directive subtype code. It needs to be set to 0b0001 if the ACK PDU + // acknowledges a Finished PDU, and to 0b0000 otherwise. + buf[current_idx] |= 0b0001; + } + current_idx += 1; + buf[current_idx] = ((self.condition_code as u8) << 4) | (self.transaction_status as u8); + current_idx += 1; + if self.crc_flag() == CrcFlag::WithCrc { + current_idx = add_pdu_crc(buf, current_idx); + } + Ok(current_idx) + } + + fn len_written(&self) -> usize { + self.pdu_header.header_len() + self.calc_pdu_datafield_len() + } +} + +#[cfg(test)] +mod tests { + #[test] + fn test_basic() { + todo!(); + } +} diff --git a/src/cfdp/pdu/eof.rs b/src/cfdp/pdu/eof.rs index 09be8e9..cfb4e69 100644 --- a/src/cfdp/pdu/eof.rs +++ b/src/cfdp/pdu/eof.rs @@ -63,6 +63,9 @@ impl EofPdu { if let Some(fault_location) = self.fault_location { len += fault_location.len_full(); } + if self.crc_flag() == CrcFlag::WithCrc { + len += 2; + } len } @@ -146,7 +149,7 @@ impl WritablePduPacket for EofPdu { if let Some(fault_location) = self.fault_location { current_idx += fault_location.write_to_be_bytes(buf)?; } - if self.pdu_header.pdu_conf.crc_flag == CrcFlag::WithCrc { + if self.crc_flag() == CrcFlag::WithCrc { current_idx = add_pdu_crc(buf, current_idx); } Ok(current_idx) @@ -160,9 +163,11 @@ impl WritablePduPacket for EofPdu { #[cfg(test)] mod tests { use super::*; - use crate::cfdp::pdu::tests::{common_pdu_conf, verify_raw_header}; + use crate::cfdp::pdu::tests::{ + common_pdu_conf, verify_raw_header, TEST_DEST_ID, TEST_SEQ_NUM, TEST_SRC_ID, + }; use crate::cfdp::pdu::{FileDirectiveType, PduHeader}; - use crate::cfdp::{ConditionCode, CrcFlag, LargeFileFlag}; + use crate::cfdp::{ConditionCode, CrcFlag, LargeFileFlag, PduType, TransmissionMode}; #[test] fn test_basic() { @@ -173,6 +178,19 @@ mod tests { assert_eq!(eof_pdu.file_checksum(), 0x01020304); assert_eq!(eof_pdu.file_size(), 12); assert_eq!(eof_pdu.condition_code(), ConditionCode::NoError); + + assert_eq!(eof_pdu.crc_flag(), CrcFlag::NoCrc); + assert_eq!(eof_pdu.file_flag(), LargeFileFlag::Normal); + assert_eq!(eof_pdu.pdu_type(), PduType::FileDirective); + assert_eq!( + eof_pdu.file_directive_type(), + Some(FileDirectiveType::EofPdu) + ); + assert_eq!(eof_pdu.transmission_mode(), TransmissionMode::Acknowledged); + assert_eq!(eof_pdu.direction(), Direction::TowardsReceiver); + assert_eq!(eof_pdu.source_id(), TEST_SRC_ID.into()); + assert_eq!(eof_pdu.dest_id(), TEST_DEST_ID.into()); + assert_eq!(eof_pdu.transaction_seq_num(), TEST_SEQ_NUM.into()); } #[test] @@ -215,8 +233,7 @@ mod tests { let mut buf: [u8; 64] = [0; 64]; eof_pdu.write_to_bytes(&mut buf).unwrap(); let eof_read_back = EofPdu::from_bytes(&buf); - if eof_read_back.is_err() { - let e = eof_read_back.unwrap_err(); + if let Err(e) = eof_read_back { panic!("deserialization failed with: {e}") } let eof_read_back = eof_read_back.unwrap(); @@ -233,4 +250,9 @@ mod tests { let pdu_vec = eof_pdu.to_vec().unwrap(); assert_eq!(buf[0..written], pdu_vec); } + + #[test] + fn test_with_crc() { + todo!(); + } } diff --git a/src/cfdp/pdu/file_data.rs b/src/cfdp/pdu/file_data.rs index d37623b..852470a 100644 --- a/src/cfdp/pdu/file_data.rs +++ b/src/cfdp/pdu/file_data.rs @@ -144,7 +144,7 @@ impl<'seg_meta, 'file_data> FileDataPdu<'seg_meta, 'file_data> { len += self.segment_metadata.as_ref().unwrap().written_len() } len += self.file_data.len(); - if self.pdu_header.pdu_conf.crc_flag == CrcFlag::WithCrc { + if self.crc_flag() == CrcFlag::WithCrc { len += 2; } len @@ -225,7 +225,7 @@ impl WritablePduPacket for FileDataPdu<'_, '_> { )?; buf[current_idx..current_idx + self.file_data.len()].copy_from_slice(self.file_data); current_idx += self.file_data.len(); - if self.pdu_header.pdu_conf.crc_flag == CrcFlag::WithCrc { + if self.crc_flag() == CrcFlag::WithCrc { current_idx = add_pdu_crc(buf, current_idx); } Ok(current_idx) @@ -242,7 +242,6 @@ mod tests { use crate::cfdp::pdu::tests::{TEST_DEST_ID, TEST_SEQ_NUM, TEST_SRC_ID}; use crate::cfdp::pdu::{CommonPduConfig, PduHeader}; use crate::cfdp::{Direction, SegmentMetadataFlag, SegmentationControl, TransmissionMode}; - use crate::util::UbfU8; #[test] fn test_basic() { @@ -330,6 +329,11 @@ mod tests { assert_eq!(fd_pdu_read_back, fd_pdu); } + #[test] + fn test_with_crc() { + todo!(); + } + #[test] fn test_with_seg_metadata_serialization() { let common_conf = diff --git a/src/cfdp/pdu/finished.rs b/src/cfdp/pdu/finished.rs index f200af7..93cd008 100644 --- a/src/cfdp/pdu/finished.rs +++ b/src/cfdp/pdu/finished.rs @@ -120,14 +120,17 @@ impl<'fs_responses> FinishedPdu<'fs_responses> { } fn calc_pdu_datafield_len(&self) -> usize { - let mut base_len = 2; + let mut datafield_len = 2; if let Some(fs_responses) = self.fs_responses { - base_len += fs_responses.len(); + datafield_len += fs_responses.len(); } if let Some(fault_location) = self.fault_location { - base_len += fault_location.len_full(); + datafield_len += fault_location.len_full(); } - base_len + if self.crc_flag() == CrcFlag::WithCrc { + datafield_len += 2; + } + datafield_len } /// Generates [Self] from a raw bytestream. @@ -246,7 +249,7 @@ impl WritablePduPacket for FinishedPdu<'_> { if let Some(fault_location) = self.fault_location { current_idx += fault_location.write_to_be_bytes(&mut buf[current_idx..])?; } - if self.pdu_header.pdu_conf.crc_flag == CrcFlag::WithCrc { + if self.crc_flag() == CrcFlag::WithCrc { current_idx = add_pdu_crc(buf, current_idx); } Ok(current_idx) @@ -389,4 +392,9 @@ mod tests { let read_back = read_back.unwrap(); assert_eq!(finished_pdu, read_back); } + + #[test] + fn test_with_crc() { + todo!(); + } } diff --git a/src/cfdp/pdu/metadata.rs b/src/cfdp/pdu/metadata.rs index 9f17d04..9c6ed8c 100644 --- a/src/cfdp/pdu/metadata.rs +++ b/src/cfdp/pdu/metadata.rs @@ -187,7 +187,7 @@ impl<'src_name, 'dest_name, 'opts> MetadataPdu<'src_name, 'dest_name, 'opts> { if let Some(opts) = self.options { len += opts.len(); } - if self.pdu_header.pdu_conf.crc_flag == CrcFlag::WithCrc { + if self.crc_flag() == CrcFlag::WithCrc { len += 2; } len @@ -288,7 +288,7 @@ impl WritablePduPacket for MetadataPdu<'_, '_, '_> { buf[current_idx..current_idx + opts.len()].copy_from_slice(opts); current_idx += opts.len(); } - if self.pdu_header.pdu_conf.crc_flag == CrcFlag::WithCrc { + if self.crc_flag() == CrcFlag::WithCrc { current_idx = add_pdu_crc(buf, current_idx); } Ok(current_idx) @@ -453,6 +453,7 @@ pub mod tests { + dest_filename.len_full() + 2 ); + assert_eq!(written, metadata_pdu.len_written()); let pdu_read_back = MetadataPdu::from_bytes(&buf).unwrap(); assert_eq!(pdu_read_back, metadata_pdu); } diff --git a/src/cfdp/pdu/mod.rs b/src/cfdp/pdu/mod.rs index 2e05b78..3d6c611 100644 --- a/src/cfdp/pdu/mod.rs +++ b/src/cfdp/pdu/mod.rs @@ -9,10 +9,12 @@ use core::fmt::{Display, Formatter}; #[cfg(feature = "std")] use std::error::Error; +pub mod ack; pub mod eof; pub mod file_data; pub mod finished; pub mod metadata; +pub mod nak; #[derive(Debug, Copy, Clone, PartialEq, Eq, TryFromPrimitive, IntoPrimitive)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] @@ -41,11 +43,14 @@ pub enum PduError { src_id_len: usize, dest_id_len: usize, }, + /// Wrong directive type, for example when parsing the directive field for a file directive + /// PDU. WrongDirectiveType { found: FileDirectiveType, expected: FileDirectiveType, }, - /// The directive type field contained a value not in the range of permitted values. + /// The directive type field contained a value not in the range of permitted values. This can + /// also happen if an invalid value is passed to the ACK PDU constructor. InvalidDirectiveType { found: u8, expected: Option, diff --git a/src/cfdp/pdu/nak.rs b/src/cfdp/pdu/nak.rs new file mode 100644 index 0000000..8b13789 --- /dev/null +++ b/src/cfdp/pdu/nak.rs @@ -0,0 +1 @@ + From c0805db137d6b49d8a1e44a2deebbd49f14263e2 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 26 Nov 2023 23:52:34 +0100 Subject: [PATCH 03/19] first test --- src/cfdp/pdu/ack.rs | 38 ++++++++++++++++++++++++++++++++++++-- src/cfdp/pdu/eof.rs | 17 ++++++++++++++++- src/cfdp/pdu/file_data.rs | 20 +++++++++++++++++++- src/cfdp/pdu/finished.rs | 20 +++++++++++++++++++- 4 files changed, 90 insertions(+), 5 deletions(-) diff --git a/src/cfdp/pdu/ack.rs b/src/cfdp/pdu/ack.rs index 979015b..38f27d2 100644 --- a/src/cfdp/pdu/ack.rs +++ b/src/cfdp/pdu/ack.rs @@ -63,7 +63,7 @@ impl AckPdu { .unwrap() } - pub fn new_for_finished_pdu( + pub fn new_for_ack_pdu( pdu_header: PduHeader, condition_code: ConditionCode, transaction_status: TransactionStatus, @@ -192,8 +192,42 @@ impl WritablePduPacket for AckPdu { #[cfg(test)] mod tests { + use crate::cfdp::{ + pdu::tests::{common_pdu_conf, TEST_DEST_ID, TEST_SEQ_NUM, TEST_SRC_ID}, + LargeFileFlag, PduType, TransmissionMode, + }; + + use super::*; + #[test] fn test_basic() { - todo!(); + let pdu_conf = common_pdu_conf(CrcFlag::NoCrc, LargeFileFlag::Normal); + let pdu_header = PduHeader::new_no_file_data(pdu_conf, 0); + let ack_pdu = AckPdu::new( + pdu_header, + FileDirectiveType::FinishedPdu, + ConditionCode::NoError, + TransactionStatus::Active, + ) + .expect("creating ACK PDU failed"); + assert_eq!( + ack_pdu.directive_code_of_acked_pdu(), + FileDirectiveType::FinishedPdu + ); + assert_eq!(ack_pdu.condition_code(), ConditionCode::NoError); + assert_eq!(ack_pdu.transaction_status(), TransactionStatus::Active); + + assert_eq!(ack_pdu.crc_flag(), CrcFlag::NoCrc); + assert_eq!(ack_pdu.file_flag(), LargeFileFlag::Normal); + assert_eq!(ack_pdu.pdu_type(), PduType::FileDirective); + assert_eq!( + ack_pdu.file_directive_type(), + Some(FileDirectiveType::AckPdu) + ); + assert_eq!(ack_pdu.transmission_mode(), TransmissionMode::Acknowledged); + assert_eq!(ack_pdu.direction(), Direction::TowardsReceiver); + assert_eq!(ack_pdu.source_id(), TEST_SRC_ID.into()); + assert_eq!(ack_pdu.dest_id(), TEST_DEST_ID.into()); + assert_eq!(ack_pdu.transaction_seq_num(), TEST_SEQ_NUM.into()); } } diff --git a/src/cfdp/pdu/eof.rs b/src/cfdp/pdu/eof.rs index cfb4e69..2ee5fa4 100644 --- a/src/cfdp/pdu/eof.rs +++ b/src/cfdp/pdu/eof.rs @@ -253,6 +253,21 @@ mod tests { #[test] fn test_with_crc() { - todo!(); + let pdu_conf = common_pdu_conf(CrcFlag::WithCrc, LargeFileFlag::Normal); + let pdu_header = PduHeader::new_no_file_data(pdu_conf, 0); + let eof_pdu = EofPdu::new_no_error(pdu_header, 0x01020304, 12); + let mut buf: [u8; 64] = [0; 64]; + let written = eof_pdu.write_to_bytes(&mut buf).unwrap(); + assert_eq!(written, eof_pdu.len_written()); + let eof_from_raw = EofPdu::from_bytes(&buf).expect("creating EOF PDU failed"); + assert_eq!(eof_from_raw, eof_pdu); + buf[written - 1] -= 1; + let crc: u16 = ((buf[written - 2] as u16) << 8) as u16 | buf[written - 1] as u16; + let error = EofPdu::from_bytes(&buf).unwrap_err(); + if let PduError::ChecksumError(e) = error { + assert_eq!(e, crc); + } else { + panic!("expected crc error"); + } } } diff --git a/src/cfdp/pdu/file_data.rs b/src/cfdp/pdu/file_data.rs index 852470a..db60eef 100644 --- a/src/cfdp/pdu/file_data.rs +++ b/src/cfdp/pdu/file_data.rs @@ -331,7 +331,25 @@ mod tests { #[test] fn test_with_crc() { - todo!(); + let mut common_conf = + CommonPduConfig::new_with_byte_fields(TEST_SRC_ID, TEST_DEST_ID, TEST_SEQ_NUM).unwrap(); + common_conf.crc_flag = true.into(); + let pdu_header = PduHeader::new_for_file_data_default(common_conf, 0); + let file_data: [u8; 4] = [1, 2, 3, 4]; + let fd_pdu = FileDataPdu::new_no_seg_metadata(pdu_header, 10, &file_data); + let mut buf: [u8; 64] = [0; 64]; + let written = fd_pdu.write_to_bytes(&mut buf).unwrap(); + assert_eq!(written, fd_pdu.len_written()); + let finished_pdu_from_raw = FileDataPdu::from_bytes(&buf).unwrap(); + assert_eq!(finished_pdu_from_raw, fd_pdu); + buf[written - 1] -= 1; + let crc: u16 = ((buf[written - 2] as u16) << 8) | buf[written - 1] as u16; + let error = FileDataPdu::from_bytes(&buf).unwrap_err(); + if let PduError::ChecksumError(e) = error { + assert_eq!(e, crc); + } else { + panic!("expected crc error"); + } } #[test] diff --git a/src/cfdp/pdu/finished.rs b/src/cfdp/pdu/finished.rs index 93cd008..dc410fb 100644 --- a/src/cfdp/pdu/finished.rs +++ b/src/cfdp/pdu/finished.rs @@ -395,6 +395,24 @@ mod tests { #[test] fn test_with_crc() { - todo!(); + let finished_pdu = generic_finished_pdu( + CrcFlag::WithCrc, + LargeFileFlag::Normal, + DeliveryCode::Complete, + FileStatus::Retained, + ); + let mut buf: [u8; 64] = [0; 64]; + let written = finished_pdu.write_to_bytes(&mut buf).unwrap(); + assert_eq!(written, finished_pdu.len_written()); + let finished_pdu_from_raw = FinishedPdu::from_bytes(&buf).unwrap(); + assert_eq!(finished_pdu_from_raw, finished_pdu); + buf[written - 1] -= 1; + let crc: u16 = ((buf[written - 2] as u16) << 8) as u16 | buf[written - 1] as u16; + let error = FinishedPdu::from_bytes(&buf).unwrap_err(); + if let PduError::ChecksumError(e) = error { + assert_eq!(e, crc); + } else { + panic!("expected crc error"); + } } } From c0bedac058642538620420db29c8274c0787f0ff Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 27 Nov 2023 15:52:47 +0100 Subject: [PATCH 04/19] Serialize/Deserialize --- src/cfdp/pdu/ack.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/cfdp/pdu/ack.rs b/src/cfdp/pdu/ack.rs index 38f27d2..1773735 100644 --- a/src/cfdp/pdu/ack.rs +++ b/src/cfdp/pdu/ack.rs @@ -7,6 +7,8 @@ use super::{ add_pdu_crc, generic_length_checks_pdu_deserialization, CfdpPdu, FileDirectiveType, PduError, PduHeader, WritablePduPacket, }; +#[cfg(feature = "serde")] +use serde::{Deserialize, Serialize}; /// ACK PDU abstraction. /// From dcb697bf6fdb0981a384bedaeb44c47c4b8824e0 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 27 Nov 2023 16:25:55 +0100 Subject: [PATCH 05/19] added some tests for new ACK PDU --- src/cfdp/pdu/ack.rs | 68 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 66 insertions(+), 2 deletions(-) diff --git a/src/cfdp/pdu/ack.rs b/src/cfdp/pdu/ack.rs index 1773735..bc74911 100644 --- a/src/cfdp/pdu/ack.rs +++ b/src/cfdp/pdu/ack.rs @@ -65,7 +65,7 @@ impl AckPdu { .unwrap() } - pub fn new_for_ack_pdu( + pub fn new_for_finished_pdu( pdu_header: PduHeader, condition_code: ConditionCode, transaction_status: TransactionStatus, @@ -195,7 +195,7 @@ impl WritablePduPacket for AckPdu { #[cfg(test)] mod tests { use crate::cfdp::{ - pdu::tests::{common_pdu_conf, TEST_DEST_ID, TEST_SEQ_NUM, TEST_SRC_ID}, + pdu::tests::{common_pdu_conf, verify_raw_header, TEST_DEST_ID, TEST_SEQ_NUM, TEST_SRC_ID}, LargeFileFlag, PduType, TransmissionMode, }; @@ -232,4 +232,68 @@ mod tests { assert_eq!(ack_pdu.dest_id(), TEST_DEST_ID.into()); assert_eq!(ack_pdu.transaction_seq_num(), TEST_SEQ_NUM.into()); } + + fn generic_serialization_test( + condition_code: ConditionCode, + transaction_status: TransactionStatus, + ) { + let pdu_conf = common_pdu_conf(CrcFlag::NoCrc, LargeFileFlag::Normal); + let pdu_header = PduHeader::new_no_file_data(pdu_conf, 0); + let ack_pdu = AckPdu::new_for_finished_pdu(pdu_header, condition_code, transaction_status); + let mut buf: [u8; 64] = [0; 64]; + let res = ack_pdu.write_to_bytes(&mut buf); + assert!(res.is_ok()); + let written = res.unwrap(); + assert_eq!(written, ack_pdu.len_written()); + verify_raw_header(ack_pdu.pdu_header(), &buf); + + assert_eq!(buf[7], FileDirectiveType::AckPdu as u8); + assert_eq!((buf[8] >> 4) & 0b1111, FileDirectiveType::FinishedPdu as u8); + assert_eq!(buf[8] & 0b1111, 0b0001); + assert_eq!(buf[9] >> 4 & 0b1111, condition_code as u8); + assert_eq!(buf[9] & 0b11, transaction_status as u8); + assert_eq!(written, 10); + } + + #[test] + fn test_serialization_no_error() { + generic_serialization_test(ConditionCode::NoError, TransactionStatus::Active); + } + + #[test] + fn test_serialization_fs_error() { + generic_serialization_test(ConditionCode::FileSizeError, TransactionStatus::Terminated); + } + + #[test] + fn test_deserialization() { + let pdu_conf = common_pdu_conf(CrcFlag::NoCrc, LargeFileFlag::Normal); + let pdu_header = PduHeader::new_no_file_data(pdu_conf, 0); + let ack_pdu = AckPdu::new_for_finished_pdu( + pdu_header, + ConditionCode::NoError, + TransactionStatus::Active, + ); + let ack_vec = ack_pdu.to_vec().unwrap(); + let ack_deserialized = + AckPdu::from_bytes(&ack_vec).expect("ACK PDU deserialization failed"); + assert_eq!(ack_deserialized, ack_pdu); + } + + #[test] + fn test_with_crc() { + let pdu_conf = common_pdu_conf(CrcFlag::WithCrc, LargeFileFlag::Normal); + let pdu_header = PduHeader::new_no_file_data(pdu_conf, 0); + let ack_pdu = AckPdu::new_for_finished_pdu( + pdu_header, + ConditionCode::NoError, + TransactionStatus::Active, + ); + let ack_vec = ack_pdu.to_vec().unwrap(); + assert_eq!(ack_vec.len(), ack_pdu.len_written()); + assert_eq!(ack_vec.len(), 12); + let ack_deserialized = + AckPdu::from_bytes(&ack_vec).expect("ACK PDU deserialization failed"); + assert_eq!(ack_deserialized, ack_pdu); + } } From 65d3b4e4e5ec0a5c2fd2198d721f92f02bc21389 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 27 Nov 2023 17:48:05 +0100 Subject: [PATCH 06/19] added NAK PDU impl --- src/cfdp/pdu/mod.rs | 10 +- src/cfdp/pdu/nak.rs | 284 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 293 insertions(+), 1 deletion(-) diff --git a/src/cfdp/pdu/mod.rs b/src/cfdp/pdu/mod.rs index 3d6c611..3f7d344 100644 --- a/src/cfdp/pdu/mod.rs +++ b/src/cfdp/pdu/mod.rs @@ -55,6 +55,8 @@ pub enum PduError { found: u8, expected: Option, }, + InvalidSegmentRequestFormat, + InvalidStartOrEndOfScopeValue, /// Invalid condition code. Contains the raw detected value. InvalidConditionCode(u8), /// Invalid checksum type which is not part of the checksums listed in the @@ -75,9 +77,15 @@ impl Display for PduError { PduError::InvalidEntityLen(raw_id) => { write!( f, - "Invalid PDU entity ID length {raw_id}, only [1, 2, 4, 8] are allowed" + "invalid PDU entity ID length {raw_id}, only [1, 2, 4, 8] are allowed" ) } + PduError::InvalidSegmentRequestFormat => { + write!(f, "invalid segment request format for NAK PDU") + } + PduError::InvalidStartOrEndOfScopeValue => { + write!(f, "invalid start or end of scope for NAK PDU") + } PduError::InvalidTransactionSeqNumLen(raw_id) => { write!( f, diff --git a/src/cfdp/pdu/nak.rs b/src/cfdp/pdu/nak.rs index 8b13789..9619c34 100644 --- a/src/cfdp/pdu/nak.rs +++ b/src/cfdp/pdu/nak.rs @@ -1 +1,285 @@ +use core::marker::PhantomData; +use crate::{ + cfdp::{CrcFlag, Direction, LargeFileFlag}, + ByteConversionError, +}; + +use super::{ + add_pdu_crc, generic_length_checks_pdu_deserialization, CfdpPdu, FileDirectiveType, PduError, + PduHeader, WritablePduPacket, +}; +#[cfg(feature = "serde")] +use serde::{Deserialize, Serialize}; + +/// Special iterator type for the NAK PDU which allows to iterator over both normal and large file +/// segment requests. +pub struct SegmentRequestIter<'a, T> { + seq_req_start: &'a [u8], + current_idx: usize, + phantom: std::marker::PhantomData, +} + +trait FromBytes { + fn from_bytes(bytes: &[u8]) -> Self; +} + +impl FromBytes for u32 { + fn from_bytes(bytes: &[u8]) -> u32 { + u32::from_be_bytes(bytes.try_into().unwrap()) + } +} + +impl FromBytes for u64 { + fn from_bytes(bytes: &[u8]) -> u64 { + u64::from_be_bytes(bytes.try_into().unwrap()) + } +} + +impl<'a, T> Iterator for SegmentRequestIter<'a, T> +where + T: FromBytes, +{ + type Item = (T, T); + + fn next(&mut self) -> Option { + if self.current_idx + std::mem::size_of::() * 2 > self.seq_req_start.len() { + return None; + } + + let start_offset = T::from_bytes( + &self.seq_req_start[self.current_idx..self.current_idx + std::mem::size_of::()], + ); + self.current_idx += std::mem::size_of::(); + + let end_offset = T::from_bytes( + &self.seq_req_start[self.current_idx..self.current_idx + std::mem::size_of::()], + ); + self.current_idx += std::mem::size_of::(); + + Some((start_offset, end_offset)) + } +} + +/// NAK PDU abstraction. +/// +/// For more information, refer to CFDP chapter 5.2.6. +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] +pub struct NakPdu<'seg_reqs> { + pdu_header: PduHeader, + start_of_scope: u64, + end_of_scope: u64, + seg_reqs_raw: &'seg_reqs [u8], +} + +impl<'seg_reqs> NakPdu<'seg_reqs> { + pub fn new( + mut pdu_header: PduHeader, + start_of_scope: u64, + end_of_scope: u64, + seg_reqs_raw: &'seg_reqs [u8], + ) -> Result { + // Force correct direction flag. + pdu_header.pdu_conf.direction = Direction::TowardsSender; + if pdu_header.common_pdu_conf().file_flag == LargeFileFlag::Large { + if seg_reqs_raw.len() % 16 != 0 { + return Err(PduError::InvalidSegmentRequestFormat); + } + } else { + if seg_reqs_raw.len() % 8 != 0 { + return Err(PduError::InvalidSegmentRequestFormat); + } + if start_of_scope > u32::MAX as u64 || end_of_scope > u32::MAX as u64 { + return Err(PduError::InvalidStartOrEndOfScopeValue); + } + } + let mut nak_pdu = Self { + pdu_header, + start_of_scope, + end_of_scope, + seg_reqs_raw, + }; + nak_pdu.pdu_header.pdu_datafield_len = nak_pdu.calc_pdu_datafield_len() as u16; + Ok(nak_pdu) + } + + pub fn start_of_scope(&self) -> u64 { + self.start_of_scope + } + + pub fn end_of_scope(&self) -> u64 { + self.end_of_scope + } + + pub fn get_normal_segment_requests_iterator(&self) -> Option> { + if self.file_flag() == LargeFileFlag::Large { + return None; + } + Some(SegmentRequestIter { + seq_req_start: self.seg_reqs_raw, + current_idx: 0, + phantom: PhantomData, + }) + } + + pub fn get_large_segment_requests_iterator(&self) -> Option> { + if self.file_flag() == LargeFileFlag::Normal { + return None; + } + Some(SegmentRequestIter { + seq_req_start: self.seg_reqs_raw, + current_idx: 0, + phantom: PhantomData, + }) + } + + pub fn num_segment_reqs(&self) -> usize { + if self.file_flag() == LargeFileFlag::Large { + self.seg_reqs_raw.len() / 16 + } else { + self.seg_reqs_raw.len() / 8 + } + } + + pub fn pdu_header(&self) -> &PduHeader { + &self.pdu_header + } + + fn calc_pdu_datafield_len(&self) -> usize { + let mut datafield_len = 1; + if self.file_flag() == LargeFileFlag::Normal { + datafield_len += 8; + datafield_len += self.num_segment_reqs() * 8; + } else { + datafield_len += 16; + datafield_len += self.num_segment_reqs() * 16; + } + if self.crc_flag() == CrcFlag::WithCrc { + datafield_len += 2; + } + datafield_len + } + + pub fn from_bytes(buf: &'seg_reqs [u8]) -> Result { + let (pdu_header, mut current_idx) = PduHeader::from_bytes(buf)?; + let full_len_without_crc = pdu_header.verify_length_and_checksum(buf)?; + // Minimum length of 9: 1 byte directive field and start and end of scope for normal file + // size. + generic_length_checks_pdu_deserialization(buf, 9, full_len_without_crc)?; + let directive_type = FileDirectiveType::try_from(buf[current_idx]).map_err(|_| { + PduError::InvalidDirectiveType { + found: buf[current_idx], + expected: Some(FileDirectiveType::NakPdu), + } + })?; + if directive_type != FileDirectiveType::NakPdu { + return Err(PduError::WrongDirectiveType { + found: directive_type, + expected: FileDirectiveType::AckPdu, + }); + } + current_idx += 1; + let start_of_scope; + let end_of_scope; + if pdu_header.common_pdu_conf().file_flag == LargeFileFlag::Large { + if current_idx + 16 > buf.len() { + return Err(PduError::ByteConversionError( + ByteConversionError::FromSliceTooSmall { + found: buf.len(), + expected: current_idx + 16, + }, + )); + } + start_of_scope = + u64::from_be_bytes(buf[current_idx..current_idx + 8].try_into().unwrap()); + current_idx += 8; + end_of_scope = + u64::from_be_bytes(buf[current_idx..current_idx + 8].try_into().unwrap()); + current_idx += 8; + } else { + start_of_scope = + u64::from_be_bytes(buf[current_idx..current_idx + 4].try_into().unwrap()); + current_idx += 4; + end_of_scope = + u64::from_be_bytes(buf[current_idx..current_idx + 4].try_into().unwrap()); + current_idx += 4; + } + Self::new( + pdu_header, + start_of_scope, + end_of_scope, + &buf[current_idx..full_len_without_crc], + ) + } +} + +impl CfdpPdu for NakPdu<'_> { + fn pdu_header(&self) -> &PduHeader { + &self.pdu_header + } + + fn file_directive_type(&self) -> Option { + Some(FileDirectiveType::NakPdu) + } +} + +impl WritablePduPacket for NakPdu<'_> { + fn write_to_bytes(&self, buf: &mut [u8]) -> Result { + let expected_len = self.len_written(); + if buf.len() < expected_len { + return Err(ByteConversionError::ToSliceTooSmall { + found: buf.len(), + expected: expected_len, + } + .into()); + } + let mut current_idx = self.pdu_header.write_to_bytes(buf)?; + buf[current_idx] = FileDirectiveType::NakPdu as u8; + current_idx += 1; + + if self.file_flag() == LargeFileFlag::Large { + buf[current_idx..current_idx + 8].copy_from_slice(&self.start_of_scope.to_be_bytes()); + current_idx += 8; + buf[current_idx..current_idx + 8].copy_from_slice(&self.end_of_scope.to_be_bytes()); + current_idx += 8; + // Unwrap is okay here, we checked the file flag. + let segments_iter = self.get_large_segment_requests_iterator().unwrap(); + for (next_start_offset, next_end_offset) in segments_iter { + buf[current_idx..current_idx + 8].copy_from_slice(&next_start_offset.to_be_bytes()); + current_idx += 8; + buf[current_idx..current_idx + 8].copy_from_slice(&next_end_offset.to_be_bytes()); + current_idx += 8; + } + } else { + // Unwrap is okay here, the API should prevent invalid values which would trigger a + // panic here. + let start_of_scope = u32::try_from(self.start_of_scope).unwrap(); + let end_of_scope = u32::try_from(self.end_of_scope).unwrap(); + buf[current_idx..current_idx + 4].copy_from_slice(&start_of_scope.to_be_bytes()); + current_idx += 4; + buf[current_idx..current_idx + 4].copy_from_slice(&end_of_scope.to_be_bytes()); + current_idx += 4; + // Unwrap is okay here, we checked the file flag. + let segments_iter = self.get_normal_segment_requests_iterator().unwrap(); + for (next_start_offset, next_end_offset) in segments_iter { + buf[current_idx..current_idx + 4].copy_from_slice(&next_start_offset.to_be_bytes()); + current_idx += 4; + buf[current_idx..current_idx + 4].copy_from_slice(&next_end_offset.to_be_bytes()); + current_idx += 4; + } + } + + if self.crc_flag() == CrcFlag::WithCrc { + current_idx = add_pdu_crc(buf, current_idx); + } + Ok(current_idx) + } + + fn len_written(&self) -> usize { + self.pdu_header.header_len() + self.calc_pdu_datafield_len() + } +} + +#[cfg(test)] +mod tests {} From 6ed023d50dff89b03ce023babd560606d5760441 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 28 Nov 2023 01:19:03 +0100 Subject: [PATCH 07/19] split up NAK PDU API --- src/cfdp/pdu/nak.rs | 403 +++++++++++++++++++++++++++----------------- 1 file changed, 251 insertions(+), 152 deletions(-) diff --git a/src/cfdp/pdu/nak.rs b/src/cfdp/pdu/nak.rs index 9619c34..a663496 100644 --- a/src/cfdp/pdu/nak.rs +++ b/src/cfdp/pdu/nak.rs @@ -9,10 +9,182 @@ use super::{ add_pdu_crc, generic_length_checks_pdu_deserialization, CfdpPdu, FileDirectiveType, PduError, PduHeader, WritablePduPacket, }; + #[cfg(feature = "serde")] use serde::{Deserialize, Serialize}; -/// Special iterator type for the NAK PDU which allows to iterator over both normal and large file +#[derive(Debug, PartialEq, Eq, Clone)] +pub enum SegmentRequests<'a> { + U32Pairs(&'a [(u32, u32)]), + U64Pairs(&'a [(u64, u64)]), +} + +/// NAK PDU abstraction specialized in the creation of NAK PDUs. +/// +/// It exposes a specialized API which simplifies to generate these NAK PDUs with the +/// format according to CFDP chapter 5.2.6. +#[derive(Debug, Clone, PartialEq, Eq)] +#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] +pub struct NakPduCreator<'seg_reqs> { + pdu_header: PduHeader, + start_of_scope: u64, + end_of_scope: u64, + segment_requests: Option>, +} + +impl<'seg_reqs> NakPduCreator<'seg_reqs> { + /// This constructor will set the PDU header [LargeFileFlag] field to the correct value if + /// segment requests are passed to it. If the segment request field is [None], it will remain + /// to whatever was configured for the PDU header. + pub fn new( + mut pdu_header: PduHeader, + start_of_scope: u64, + end_of_scope: u64, + segment_requests: Option>, + ) -> Result { + // Force correct direction flag. + pdu_header.pdu_conf.direction = Direction::TowardsSender; + if let Some(ref seg_reqs) = segment_requests { + match seg_reqs { + SegmentRequests::U32Pairs(_) => { + if start_of_scope > u32::MAX as u64 || end_of_scope > u32::MAX as u64 { + return Err(PduError::InvalidStartOrEndOfScopeValue); + } + pdu_header.pdu_conf.file_flag = LargeFileFlag::Normal; + } + SegmentRequests::U64Pairs(_) => { + pdu_header.pdu_conf.file_flag = LargeFileFlag::Large; + } + } + } + let mut nak_pdu = Self { + pdu_header, + start_of_scope, + end_of_scope, + segment_requests, + }; + nak_pdu.pdu_header.pdu_datafield_len = nak_pdu.calc_pdu_datafield_len() as u16; + Ok(nak_pdu) + } + + pub fn start_of_scope(&self) -> u64 { + self.start_of_scope + } + + pub fn end_of_scope(&self) -> u64 { + self.end_of_scope + } + + pub fn segment_requests(&self) -> Option<&SegmentRequests> { + self.segment_requests.as_ref() + } + + pub fn num_segment_reqs(&self) -> usize { + match &self.segment_requests { + Some(seg_reqs) => match seg_reqs { + SegmentRequests::U32Pairs(pairs) => pairs.len(), + SegmentRequests::U64Pairs(pairs) => pairs.len(), + }, + None => 0, + } + } + + pub fn pdu_header(&self) -> &PduHeader { + &self.pdu_header + } + + fn calc_pdu_datafield_len(&self) -> usize { + let mut datafield_len = 1; + if self.file_flag() == LargeFileFlag::Normal { + datafield_len += 8; + datafield_len += self.num_segment_reqs() * 8; + } else { + datafield_len += 16; + datafield_len += self.num_segment_reqs() * 16; + } + if self.crc_flag() == CrcFlag::WithCrc { + datafield_len += 2; + } + datafield_len + } +} + +impl CfdpPdu for NakPduCreator<'_> { + fn pdu_header(&self) -> &PduHeader { + &self.pdu_header + } + + fn file_directive_type(&self) -> Option { + Some(FileDirectiveType::NakPdu) + } +} + +impl WritablePduPacket for NakPduCreator<'_> { + fn write_to_bytes(&self, buf: &mut [u8]) -> Result { + let expected_len = self.len_written(); + if buf.len() < expected_len { + return Err(ByteConversionError::ToSliceTooSmall { + found: buf.len(), + expected: expected_len, + } + .into()); + } + let mut current_idx = self.pdu_header.write_to_bytes(buf)?; + buf[current_idx] = FileDirectiveType::NakPdu as u8; + current_idx += 1; + + if let Some(ref seg_reqs) = self.segment_requests { + match seg_reqs { + SegmentRequests::U32Pairs(pairs) => { + // Unwrap is okay here, the API should prevent invalid values which would trigger a + // panic here. + let start_of_scope = u32::try_from(self.start_of_scope).unwrap(); + let end_of_scope = u32::try_from(self.end_of_scope).unwrap(); + buf[current_idx..current_idx + 4] + .copy_from_slice(&start_of_scope.to_be_bytes()); + current_idx += 4; + buf[current_idx..current_idx + 4].copy_from_slice(&end_of_scope.to_be_bytes()); + current_idx += 4; + for (next_start_offset, next_end_offset) in *pairs { + buf[current_idx..current_idx + 4] + .copy_from_slice(&next_start_offset.to_be_bytes()); + current_idx += 4; + buf[current_idx..current_idx + 4] + .copy_from_slice(&next_end_offset.to_be_bytes()); + current_idx += 4; + } + } + SegmentRequests::U64Pairs(pairs) => { + buf[current_idx..current_idx + 8] + .copy_from_slice(&self.start_of_scope.to_be_bytes()); + current_idx += 8; + buf[current_idx..current_idx + 8] + .copy_from_slice(&self.end_of_scope.to_be_bytes()); + current_idx += 8; + for (next_start_offset, next_end_offset) in *pairs { + buf[current_idx..current_idx + 8] + .copy_from_slice(&next_start_offset.to_be_bytes()); + current_idx += 8; + buf[current_idx..current_idx + 8] + .copy_from_slice(&next_end_offset.to_be_bytes()); + current_idx += 8; + } + } + } + } + + if self.crc_flag() == CrcFlag::WithCrc { + current_idx = add_pdu_crc(buf, current_idx); + } + Ok(current_idx) + } + + fn len_written(&self) -> usize { + self.pdu_header.header_len() + self.calc_pdu_datafield_len() + } +} + +/// Special iterator type for the NAK PDU which allows to iterate over both normal and large file /// segment requests. pub struct SegmentRequestIter<'a, T> { seq_req_start: &'a [u8], @@ -61,107 +233,31 @@ where } } -/// NAK PDU abstraction. +/// NAK PDU abstraction specialized in the reading NAK PDUs from a raw bytestream. /// -/// For more information, refer to CFDP chapter 5.2.6. -#[derive(Debug, Copy, Clone, PartialEq, Eq)] -#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] -pub struct NakPdu<'seg_reqs> { +/// This is a zero-copy class where the segment requests can be read using a special iterator +/// API without the need to copy them. +/// +/// The NAK format is expected to be conforming to CFDP chapter 5.2.6. +pub struct NakPduReader<'seg_reqs> { pdu_header: PduHeader, start_of_scope: u64, end_of_scope: u64, seg_reqs_raw: &'seg_reqs [u8], } -impl<'seg_reqs> NakPdu<'seg_reqs> { - pub fn new( - mut pdu_header: PduHeader, - start_of_scope: u64, - end_of_scope: u64, - seg_reqs_raw: &'seg_reqs [u8], - ) -> Result { - // Force correct direction flag. - pdu_header.pdu_conf.direction = Direction::TowardsSender; - if pdu_header.common_pdu_conf().file_flag == LargeFileFlag::Large { - if seg_reqs_raw.len() % 16 != 0 { - return Err(PduError::InvalidSegmentRequestFormat); - } - } else { - if seg_reqs_raw.len() % 8 != 0 { - return Err(PduError::InvalidSegmentRequestFormat); - } - if start_of_scope > u32::MAX as u64 || end_of_scope > u32::MAX as u64 { - return Err(PduError::InvalidStartOrEndOfScopeValue); - } - } - let mut nak_pdu = Self { - pdu_header, - start_of_scope, - end_of_scope, - seg_reqs_raw, - }; - nak_pdu.pdu_header.pdu_datafield_len = nak_pdu.calc_pdu_datafield_len() as u16; - Ok(nak_pdu) - } - - pub fn start_of_scope(&self) -> u64 { - self.start_of_scope - } - - pub fn end_of_scope(&self) -> u64 { - self.end_of_scope - } - - pub fn get_normal_segment_requests_iterator(&self) -> Option> { - if self.file_flag() == LargeFileFlag::Large { - return None; - } - Some(SegmentRequestIter { - seq_req_start: self.seg_reqs_raw, - current_idx: 0, - phantom: PhantomData, - }) - } - - pub fn get_large_segment_requests_iterator(&self) -> Option> { - if self.file_flag() == LargeFileFlag::Normal { - return None; - } - Some(SegmentRequestIter { - seq_req_start: self.seg_reqs_raw, - current_idx: 0, - phantom: PhantomData, - }) - } - - pub fn num_segment_reqs(&self) -> usize { - if self.file_flag() == LargeFileFlag::Large { - self.seg_reqs_raw.len() / 16 - } else { - self.seg_reqs_raw.len() / 8 - } - } - - pub fn pdu_header(&self) -> &PduHeader { +impl CfdpPdu for NakPduReader<'_> { + fn pdu_header(&self) -> &PduHeader { &self.pdu_header } - fn calc_pdu_datafield_len(&self) -> usize { - let mut datafield_len = 1; - if self.file_flag() == LargeFileFlag::Normal { - datafield_len += 8; - datafield_len += self.num_segment_reqs() * 8; - } else { - datafield_len += 16; - datafield_len += self.num_segment_reqs() * 16; - } - if self.crc_flag() == CrcFlag::WithCrc { - datafield_len += 2; - } - datafield_len + fn file_directive_type(&self) -> Option { + Some(FileDirectiveType::NakPdu) } +} - pub fn from_bytes(buf: &'seg_reqs [u8]) -> Result { +impl<'seg_reqs> NakPduReader<'seg_reqs> { + pub fn from_bytes(buf: &'seg_reqs [u8]) -> Result { let (pdu_header, mut current_idx) = PduHeader::from_bytes(buf)?; let full_len_without_crc = pdu_header.verify_length_and_checksum(buf)?; // Minimum length of 9: 1 byte directive field and start and end of scope for normal file @@ -205,81 +301,84 @@ impl<'seg_reqs> NakPdu<'seg_reqs> { u64::from_be_bytes(buf[current_idx..current_idx + 4].try_into().unwrap()); current_idx += 4; } - Self::new( + Ok(Self { pdu_header, start_of_scope, end_of_scope, - &buf[current_idx..full_len_without_crc], - ) - } -} - -impl CfdpPdu for NakPdu<'_> { - fn pdu_header(&self) -> &PduHeader { - &self.pdu_header + seg_reqs_raw: &buf[current_idx..full_len_without_crc], + }) } - fn file_directive_type(&self) -> Option { - Some(FileDirectiveType::NakPdu) + pub fn start_of_scope(&self) -> u64 { + self.start_of_scope } -} -impl WritablePduPacket for NakPdu<'_> { - fn write_to_bytes(&self, buf: &mut [u8]) -> Result { - let expected_len = self.len_written(); - if buf.len() < expected_len { - return Err(ByteConversionError::ToSliceTooSmall { - found: buf.len(), - expected: expected_len, - } - .into()); - } - let mut current_idx = self.pdu_header.write_to_bytes(buf)?; - buf[current_idx] = FileDirectiveType::NakPdu as u8; - current_idx += 1; + pub fn end_of_scope(&self) -> u64 { + self.end_of_scope + } + /// This function returns [None] if this NAK PDUs contains segment requests for a large file. + pub fn get_normal_segment_requests_iterator(&self) -> Option> { if self.file_flag() == LargeFileFlag::Large { - buf[current_idx..current_idx + 8].copy_from_slice(&self.start_of_scope.to_be_bytes()); - current_idx += 8; - buf[current_idx..current_idx + 8].copy_from_slice(&self.end_of_scope.to_be_bytes()); - current_idx += 8; - // Unwrap is okay here, we checked the file flag. - let segments_iter = self.get_large_segment_requests_iterator().unwrap(); - for (next_start_offset, next_end_offset) in segments_iter { - buf[current_idx..current_idx + 8].copy_from_slice(&next_start_offset.to_be_bytes()); - current_idx += 8; - buf[current_idx..current_idx + 8].copy_from_slice(&next_end_offset.to_be_bytes()); - current_idx += 8; - } - } else { - // Unwrap is okay here, the API should prevent invalid values which would trigger a - // panic here. - let start_of_scope = u32::try_from(self.start_of_scope).unwrap(); - let end_of_scope = u32::try_from(self.end_of_scope).unwrap(); - buf[current_idx..current_idx + 4].copy_from_slice(&start_of_scope.to_be_bytes()); - current_idx += 4; - buf[current_idx..current_idx + 4].copy_from_slice(&end_of_scope.to_be_bytes()); - current_idx += 4; - // Unwrap is okay here, we checked the file flag. - let segments_iter = self.get_normal_segment_requests_iterator().unwrap(); - for (next_start_offset, next_end_offset) in segments_iter { - buf[current_idx..current_idx + 4].copy_from_slice(&next_start_offset.to_be_bytes()); - current_idx += 4; - buf[current_idx..current_idx + 4].copy_from_slice(&next_end_offset.to_be_bytes()); - current_idx += 4; - } + return None; } - - if self.crc_flag() == CrcFlag::WithCrc { - current_idx = add_pdu_crc(buf, current_idx); - } - Ok(current_idx) + Some(SegmentRequestIter { + seq_req_start: self.seg_reqs_raw, + current_idx: 0, + phantom: PhantomData, + }) } - fn len_written(&self) -> usize { - self.pdu_header.header_len() + self.calc_pdu_datafield_len() + /// This function returns [None] if this NAK PDUs contains segment requests for a normal file. + pub fn get_large_segment_requests_iterator(&self) -> Option> { + if self.file_flag() == LargeFileFlag::Normal { + return None; + } + Some(SegmentRequestIter { + seq_req_start: self.seg_reqs_raw, + current_idx: 0, + phantom: PhantomData, + }) } } #[cfg(test)] -mod tests {} +mod tests { + use crate::cfdp::{ + pdu::tests::{common_pdu_conf, TEST_DEST_ID, TEST_SEQ_NUM, TEST_SRC_ID}, + PduType, TransmissionMode, + }; + + use super::*; + + #[test] + fn test_basic_creator() { + let pdu_conf = common_pdu_conf(CrcFlag::NoCrc, LargeFileFlag::Normal); + let pdu_header = PduHeader::new_no_file_data(pdu_conf, 0); + let nak_pdu = + NakPduCreator::new(pdu_header, 0, 0, None).expect("creating NAK PDU creator failed"); + assert_eq!(nak_pdu.start_of_scope(), 0); + assert_eq!(nak_pdu.end_of_scope(), 0); + assert_eq!(nak_pdu.segment_requests(), None); + assert_eq!(nak_pdu.num_segment_reqs(), 0); + + assert_eq!(nak_pdu.crc_flag(), CrcFlag::NoCrc); + assert_eq!(nak_pdu.file_flag(), LargeFileFlag::Normal); + assert_eq!(nak_pdu.pdu_type(), PduType::FileDirective); + assert_eq!( + nak_pdu.file_directive_type(), + Some(FileDirectiveType::NakPdu), + ); + assert_eq!(nak_pdu.transmission_mode(), TransmissionMode::Acknowledged); + assert_eq!(nak_pdu.direction(), Direction::TowardsSender); + assert_eq!(nak_pdu.source_id(), TEST_SRC_ID.into()); + assert_eq!(nak_pdu.dest_id(), TEST_DEST_ID.into()); + assert_eq!(nak_pdu.transaction_seq_num(), TEST_SEQ_NUM.into()); + } + + #[test] + fn test_serialization_empty() {} + + #[test] + fn test_serialization_one_segment() {} +} From b8dacc12b514fa8336742c1c70d3d01501548e6c Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 28 Nov 2023 22:28:49 +0100 Subject: [PATCH 08/19] add test --- src/cfdp/pdu/nak.rs | 43 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 34 insertions(+), 9 deletions(-) diff --git a/src/cfdp/pdu/nak.rs b/src/cfdp/pdu/nak.rs index a663496..8f935e1 100644 --- a/src/cfdp/pdu/nak.rs +++ b/src/cfdp/pdu/nak.rs @@ -133,18 +133,20 @@ impl WritablePduPacket for NakPduCreator<'_> { buf[current_idx] = FileDirectiveType::NakPdu as u8; current_idx += 1; + let mut write_start_end_of_scope_normal = || { + let start_of_scope = u32::try_from(self.start_of_scope).unwrap(); + let end_of_scope = u32::try_from(self.end_of_scope).unwrap(); + buf[current_idx..current_idx + 4].copy_from_slice(&start_of_scope.to_be_bytes()); + current_idx += 4; + buf[current_idx..current_idx + 4].copy_from_slice(&end_of_scope.to_be_bytes()); + current_idx += 4; + }; if let Some(ref seg_reqs) = self.segment_requests { match seg_reqs { SegmentRequests::U32Pairs(pairs) => { // Unwrap is okay here, the API should prevent invalid values which would trigger a // panic here. - let start_of_scope = u32::try_from(self.start_of_scope).unwrap(); - let end_of_scope = u32::try_from(self.end_of_scope).unwrap(); - buf[current_idx..current_idx + 4] - .copy_from_slice(&start_of_scope.to_be_bytes()); - current_idx += 4; - buf[current_idx..current_idx + 4].copy_from_slice(&end_of_scope.to_be_bytes()); - current_idx += 4; + write_start_end_of_scope_normal(); for (next_start_offset, next_end_offset) in *pairs { buf[current_idx..current_idx + 4] .copy_from_slice(&next_start_offset.to_be_bytes()); @@ -171,6 +173,8 @@ impl WritablePduPacket for NakPduCreator<'_> { } } } + } else { + write_start_end_of_scope_normal(); } if self.crc_flag() == CrcFlag::WithCrc { @@ -345,7 +349,7 @@ impl<'seg_reqs> NakPduReader<'seg_reqs> { #[cfg(test)] mod tests { use crate::cfdp::{ - pdu::tests::{common_pdu_conf, TEST_DEST_ID, TEST_SEQ_NUM, TEST_SRC_ID}, + pdu::tests::{common_pdu_conf, verify_raw_header, TEST_DEST_ID, TEST_SEQ_NUM, TEST_SRC_ID}, PduType, TransmissionMode, }; @@ -377,7 +381,28 @@ mod tests { } #[test] - fn test_serialization_empty() {} + fn test_serialization_empty() { + let pdu_conf = common_pdu_conf(CrcFlag::NoCrc, LargeFileFlag::Normal); + let pdu_header = PduHeader::new_no_file_data(pdu_conf, 0); + let nak_pdu = NakPduCreator::new(pdu_header, 100, 300, None) + .expect("creating NAK PDU creator failed"); + let mut buf: [u8; 64] = [0; 64]; + nak_pdu + .write_to_bytes(&mut buf) + .expect("writing NAK PDU to buffer failed"); + verify_raw_header(nak_pdu.pdu_header(), &buf); + let mut current_idx = nak_pdu.pdu_header().header_len(); + assert_eq!(current_idx + 9, nak_pdu.len_written()); + assert_eq!(buf[current_idx], FileDirectiveType::NakPdu as u8); + current_idx += 1; + let start_of_scope = + u32::from_be_bytes(buf[current_idx..current_idx + 4].try_into().unwrap()); + assert_eq!(start_of_scope, 100); + current_idx += 4; + let end_of_scope = + u32::from_be_bytes(buf[current_idx..current_idx + 4].try_into().unwrap()); + assert_eq!(end_of_scope, 300); + } #[test] fn test_serialization_one_segment() {} From 5ae86619b4594376268d611b8c12b564989fa6d2 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 28 Nov 2023 22:33:34 +0100 Subject: [PATCH 09/19] last push --- src/cfdp/pdu/nak.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/cfdp/pdu/nak.rs b/src/cfdp/pdu/nak.rs index 8f935e1..6f1309e 100644 --- a/src/cfdp/pdu/nak.rs +++ b/src/cfdp/pdu/nak.rs @@ -402,8 +402,12 @@ mod tests { let end_of_scope = u32::from_be_bytes(buf[current_idx..current_idx + 4].try_into().unwrap()); assert_eq!(end_of_scope, 300); + current_idx += 4; + assert_eq!(current_idx, nak_pdu.len_written()); } #[test] - fn test_serialization_one_segment() {} + fn test_serialization_one_segment() { + + } } From 80aa963226939136d47d0f5e1c691df731bbf704 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 30 Nov 2023 20:20:30 +0100 Subject: [PATCH 10/19] that was complicated but it works --- src/cfdp/pdu/nak.rs | 250 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 221 insertions(+), 29 deletions(-) diff --git a/src/cfdp/pdu/nak.rs b/src/cfdp/pdu/nak.rs index 6f1309e..38e242a 100644 --- a/src/cfdp/pdu/nak.rs +++ b/src/cfdp/pdu/nak.rs @@ -33,10 +33,50 @@ pub struct NakPduCreator<'seg_reqs> { } impl<'seg_reqs> NakPduCreator<'seg_reqs> { - /// This constructor will set the PDU header [LargeFileFlag] field to the correct value if - /// segment requests are passed to it. If the segment request field is [None], it will remain - /// to whatever was configured for the PDU header. + /// Please note that the start of scope and the end of scope need to be smaller or equal + /// to [u32::MAX] if the large file flag of the passed PDU configuration is + /// [LargeFileFlag::Normal]. + /// + /// ## Errrors + /// + pub fn new_no_segment_requests( + pdu_header: PduHeader, + start_of_scope: u64, + end_of_scope: u64, + ) -> Result, PduError> { + Self::new_generic(pdu_header, start_of_scope, end_of_scope, None) + } + + /// Default constructor for normal file sizes. pub fn new( + pdu_header: PduHeader, + start_of_scope: u32, + end_of_scope: u32, + segment_requests: &'seg_reqs [(u32, u32)], + ) -> Result { + Self::new_generic( + pdu_header, + start_of_scope.into(), + end_of_scope.into(), + Some(SegmentRequests::U32Pairs(segment_requests)), + ) + } + + pub fn new_large_file_size( + pdu_header: PduHeader, + start_of_scope: u64, + end_of_scope: u64, + segment_requests: &'seg_reqs [(u64, u64)], + ) -> Result { + Self::new_generic( + pdu_header, + start_of_scope, + end_of_scope, + Some(SegmentRequests::U64Pairs(segment_requests)), + ) + } + + fn new_generic( mut pdu_header: PduHeader, start_of_scope: u64, end_of_scope: u64, @@ -44,8 +84,8 @@ impl<'seg_reqs> NakPduCreator<'seg_reqs> { ) -> Result { // Force correct direction flag. pdu_header.pdu_conf.direction = Direction::TowardsSender; - if let Some(ref seg_reqs) = segment_requests { - match seg_reqs { + if let Some(ref segment_requests) = segment_requests { + match segment_requests { SegmentRequests::U32Pairs(_) => { if start_of_scope > u32::MAX as u64 || end_of_scope > u32::MAX as u64 { return Err(PduError::InvalidStartOrEndOfScopeValue); @@ -56,7 +96,7 @@ impl<'seg_reqs> NakPduCreator<'seg_reqs> { pdu_header.pdu_conf.file_flag = LargeFileFlag::Large; } } - } + }; let mut nak_pdu = Self { pdu_header, start_of_scope, @@ -191,22 +231,22 @@ impl WritablePduPacket for NakPduCreator<'_> { /// Special iterator type for the NAK PDU which allows to iterate over both normal and large file /// segment requests. pub struct SegmentRequestIter<'a, T> { - seq_req_start: &'a [u8], + seq_req_raw: &'a [u8], current_idx: usize, phantom: std::marker::PhantomData, } -trait FromBytes { +pub trait SegReqFromBytes { fn from_bytes(bytes: &[u8]) -> Self; } -impl FromBytes for u32 { +impl SegReqFromBytes for u32 { fn from_bytes(bytes: &[u8]) -> u32 { u32::from_be_bytes(bytes.try_into().unwrap()) } } -impl FromBytes for u64 { +impl SegReqFromBytes for u64 { fn from_bytes(bytes: &[u8]) -> u64 { u64::from_be_bytes(bytes.try_into().unwrap()) } @@ -214,25 +254,81 @@ impl FromBytes for u64 { impl<'a, T> Iterator for SegmentRequestIter<'a, T> where - T: FromBytes, + T: SegReqFromBytes, { type Item = (T, T); fn next(&mut self) -> Option { - if self.current_idx + std::mem::size_of::() * 2 > self.seq_req_start.len() { + let value = self.next_at_offset(self.current_idx); + self.current_idx += 2 * std::mem::size_of::(); + value + } +} + +impl<'a, 'b> PartialEq> for SegmentRequestIter<'b, u32> { + fn eq(&self, other: &SegmentRequests) -> bool { + match other { + SegmentRequests::U32Pairs(pairs) => self.compare_pairs(pairs), + SegmentRequests::U64Pairs(pairs) => { + if pairs.is_empty() && self.seq_req_raw.is_empty() { + return true; + } + false + } + } + } +} + +impl<'a, 'b> PartialEq> for SegmentRequestIter<'b, u64> { + fn eq(&self, other: &SegmentRequests) -> bool { + match other { + SegmentRequests::U32Pairs(pairs) => { + if pairs.is_empty() && self.seq_req_raw.is_empty() { + return true; + } + false + } + SegmentRequests::U64Pairs(pairs) => self.compare_pairs(pairs), + } + } +} + +impl<'a, T> SegmentRequestIter<'a, T> +where + T: SegReqFromBytes + PartialEq, +{ + fn compare_pairs(&self, pairs: &[(T, T)]) -> bool { + if pairs.is_empty() && self.seq_req_raw.is_empty() { + return true; + } + let size = std::mem::size_of::(); + if pairs.len() * 2 * size != self.seq_req_raw.len() { + return false; + } + + for (i, pair) in pairs.iter().enumerate() { + let next_val = self.next_at_offset(i * 2 * size).unwrap(); + if next_val != *pair { + return false; + } + } + + true + } +} + +impl SegmentRequestIter<'_, T> { + fn next_at_offset(&self, mut offset: usize) -> Option<(T, T)> { + if offset + std::mem::size_of::() * 2 > self.seq_req_raw.len() { return None; } - let start_offset = T::from_bytes( - &self.seq_req_start[self.current_idx..self.current_idx + std::mem::size_of::()], - ); - self.current_idx += std::mem::size_of::(); - - let end_offset = T::from_bytes( - &self.seq_req_start[self.current_idx..self.current_idx + std::mem::size_of::()], - ); - self.current_idx += std::mem::size_of::(); + let start_offset = + T::from_bytes(&self.seq_req_raw[offset..offset + std::mem::size_of::()]); + offset += std::mem::size_of::(); + let end_offset = + T::from_bytes(&self.seq_req_raw[offset..offset + std::mem::size_of::()]); Some((start_offset, end_offset)) } } @@ -243,6 +339,7 @@ where /// API without the need to copy them. /// /// The NAK format is expected to be conforming to CFDP chapter 5.2.6. +#[derive(Debug, PartialEq, Eq)] pub struct NakPduReader<'seg_reqs> { pdu_header: PduHeader, start_of_scope: u64, @@ -261,6 +358,10 @@ impl CfdpPdu for NakPduReader<'_> { } impl<'seg_reqs> NakPduReader<'seg_reqs> { + pub fn new(buf: &'seg_reqs [u8]) -> Result { + Self::from_bytes(buf) + } + pub fn from_bytes(buf: &'seg_reqs [u8]) -> Result { let (pdu_header, mut current_idx) = PduHeader::from_bytes(buf)?; let full_len_without_crc = pdu_header.verify_length_and_checksum(buf)?; @@ -299,10 +400,10 @@ impl<'seg_reqs> NakPduReader<'seg_reqs> { current_idx += 8; } else { start_of_scope = - u64::from_be_bytes(buf[current_idx..current_idx + 4].try_into().unwrap()); + u32::from_be_bytes(buf[current_idx..current_idx + 4].try_into().unwrap()) as u64; current_idx += 4; end_of_scope = - u64::from_be_bytes(buf[current_idx..current_idx + 4].try_into().unwrap()); + u32::from_be_bytes(buf[current_idx..current_idx + 4].try_into().unwrap()) as u64; current_idx += 4; } Ok(Self { @@ -327,7 +428,7 @@ impl<'seg_reqs> NakPduReader<'seg_reqs> { return None; } Some(SegmentRequestIter { - seq_req_start: self.seg_reqs_raw, + seq_req_raw: self.seg_reqs_raw, current_idx: 0, phantom: PhantomData, }) @@ -339,13 +440,38 @@ impl<'seg_reqs> NakPduReader<'seg_reqs> { return None; } Some(SegmentRequestIter { - seq_req_start: self.seg_reqs_raw, + seq_req_raw: self.seg_reqs_raw, current_idx: 0, phantom: PhantomData, }) } } +impl<'a, 'b> PartialEq> for NakPduReader<'b> { + fn eq(&self, other: &NakPduCreator) -> bool { + if (self.pdu_header() != other.pdu_header() || self.end_of_scope() != other.end_of_scope()) + || (self.start_of_scope() != other.start_of_scope()) + { + return false; + } + if other.segment_requests().is_none() && self.seg_reqs_raw.is_empty() { + return true; + } + if self.file_flag() == LargeFileFlag::Normal { + let normal_iter = self.get_normal_segment_requests_iterator().unwrap(); + if normal_iter != *other.segment_requests().unwrap() { + return false; + } + } else { + let large_iter = self.get_large_segment_requests_iterator().unwrap(); + if large_iter != *other.segment_requests().unwrap() { + return false; + } + } + true + } +} + #[cfg(test)] mod tests { use crate::cfdp::{ @@ -359,8 +485,8 @@ mod tests { fn test_basic_creator() { let pdu_conf = common_pdu_conf(CrcFlag::NoCrc, LargeFileFlag::Normal); let pdu_header = PduHeader::new_no_file_data(pdu_conf, 0); - let nak_pdu = - NakPduCreator::new(pdu_header, 0, 0, None).expect("creating NAK PDU creator failed"); + let nak_pdu = NakPduCreator::new_no_segment_requests(pdu_header, 0, 0) + .expect("creating NAK PDU creator failed"); assert_eq!(nak_pdu.start_of_scope(), 0); assert_eq!(nak_pdu.end_of_scope(), 0); assert_eq!(nak_pdu.segment_requests(), None); @@ -384,7 +510,7 @@ mod tests { fn test_serialization_empty() { let pdu_conf = common_pdu_conf(CrcFlag::NoCrc, LargeFileFlag::Normal); let pdu_header = PduHeader::new_no_file_data(pdu_conf, 0); - let nak_pdu = NakPduCreator::new(pdu_header, 100, 300, None) + let nak_pdu = NakPduCreator::new_no_segment_requests(pdu_header, 100, 300) .expect("creating NAK PDU creator failed"); let mut buf: [u8; 64] = [0; 64]; nak_pdu @@ -407,7 +533,73 @@ mod tests { } #[test] - fn test_serialization_one_segment() { + fn test_serialization_two_segments() { + let pdu_conf = common_pdu_conf(CrcFlag::NoCrc, LargeFileFlag::Normal); + let pdu_header = PduHeader::new_no_file_data(pdu_conf, 0); + let nak_pdu = NakPduCreator::new(pdu_header, 100, 300, &[(0, 0), (32, 64)]) + .expect("creating NAK PDU creator failed"); + let mut buf: [u8; 64] = [0; 64]; + nak_pdu + .write_to_bytes(&mut buf) + .expect("writing NAK PDU to buffer failed"); + verify_raw_header(nak_pdu.pdu_header(), &buf); + let mut current_idx = nak_pdu.pdu_header().header_len(); + assert_eq!(current_idx + 9 + 16, nak_pdu.len_written()); + assert_eq!(buf[current_idx], FileDirectiveType::NakPdu as u8); + current_idx += 1; + let start_of_scope = + u32::from_be_bytes(buf[current_idx..current_idx + 4].try_into().unwrap()); + assert_eq!(start_of_scope, 100); + current_idx += 4; + let end_of_scope = + u32::from_be_bytes(buf[current_idx..current_idx + 4].try_into().unwrap()); + assert_eq!(end_of_scope, 300); + current_idx += 4; + let first_seg_start = + u32::from_be_bytes(buf[current_idx..current_idx + 4].try_into().unwrap()); + assert_eq!(first_seg_start, 0); + current_idx += 4; + let first_seg_end = + u32::from_be_bytes(buf[current_idx..current_idx + 4].try_into().unwrap()); + assert_eq!(first_seg_end, 0); + current_idx += 4; + let second_seg_start = + u32::from_be_bytes(buf[current_idx..current_idx + 4].try_into().unwrap()); + assert_eq!(second_seg_start, 32); + current_idx += 4; + let second_seg_end = + u32::from_be_bytes(buf[current_idx..current_idx + 4].try_into().unwrap()); + assert_eq!(second_seg_end, 64); + current_idx += 4; + assert_eq!(current_idx, nak_pdu.len_written()); + } + #[test] + fn test_deserialization_empty() { + let pdu_conf = common_pdu_conf(CrcFlag::NoCrc, LargeFileFlag::Normal); + let pdu_header = PduHeader::new_no_file_data(pdu_conf, 0); + let nak_pdu = NakPduCreator::new_no_segment_requests(pdu_header, 100, 300) + .expect("creating NAK PDU creator failed"); + let mut buf: [u8; 64] = [0; 64]; + nak_pdu + .write_to_bytes(&mut buf) + .expect("writing NAK PDU to buffer failed"); + let nak_pdu_deser = NakPduReader::from_bytes(&buf).expect("deserializing NAK PDU failed"); + assert_eq!(nak_pdu_deser, nak_pdu); + } + + #[test] + fn test_deserialization_one_file_segment() { + let pdu_conf = common_pdu_conf(CrcFlag::NoCrc, LargeFileFlag::Large); + let pdu_header = PduHeader::new_no_file_data(pdu_conf, 0); + let nak_pdu = + NakPduCreator::new_large_file_size(pdu_header, 100, 300, &[(50, 100), (200, 300)]) + .expect("creating NAK PDU creator failed"); + let mut buf: [u8; 128] = [0; 128]; + nak_pdu + .write_to_bytes(&mut buf) + .expect("writing NAK PDU to buffer failed"); + let nak_pdu_deser = NakPduReader::from_bytes(&buf).expect("deserializing NAK PDU failed"); + assert_eq!(nak_pdu_deser, nak_pdu); } } From 2ba29984268a49ba1b0295dee04965762c577084 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 30 Nov 2023 22:59:35 +0100 Subject: [PATCH 11/19] add more tests --- src/cfdp/pdu/nak.rs | 160 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 139 insertions(+), 21 deletions(-) diff --git a/src/cfdp/pdu/nak.rs b/src/cfdp/pdu/nak.rs index 38e242a..fa747fa 100644 --- a/src/cfdp/pdu/nak.rs +++ b/src/cfdp/pdu/nak.rs @@ -1,9 +1,8 @@ -use core::marker::PhantomData; - use crate::{ cfdp::{CrcFlag, Direction, LargeFileFlag}, ByteConversionError, }; +use core::marker::PhantomData; use super::{ add_pdu_crc, generic_length_checks_pdu_deserialization, CfdpPdu, FileDirectiveType, PduError, @@ -19,6 +18,15 @@ pub enum SegmentRequests<'a> { U64Pairs(&'a [(u64, u64)]), } +impl SegmentRequests<'_> { + pub fn is_empty(&self) -> bool { + match self { + SegmentRequests::U32Pairs(pairs) => pairs.is_empty(), + SegmentRequests::U64Pairs(pairs) => pairs.is_empty(), + } + } +} + /// NAK PDU abstraction specialized in the creation of NAK PDUs. /// /// It exposes a specialized API which simplifies to generate these NAK PDUs with the @@ -54,11 +62,15 @@ impl<'seg_reqs> NakPduCreator<'seg_reqs> { end_of_scope: u32, segment_requests: &'seg_reqs [(u32, u32)], ) -> Result { + let mut passed_segment_requests = None; + if !segment_requests.is_empty() { + passed_segment_requests = Some(SegmentRequests::U32Pairs(segment_requests)); + } Self::new_generic( pdu_header, start_of_scope.into(), end_of_scope.into(), - Some(SegmentRequests::U32Pairs(segment_requests)), + passed_segment_requests, ) } @@ -68,11 +80,15 @@ impl<'seg_reqs> NakPduCreator<'seg_reqs> { end_of_scope: u64, segment_requests: &'seg_reqs [(u64, u64)], ) -> Result { + let mut passed_segment_requests = None; + if !segment_requests.is_empty() { + passed_segment_requests = Some(SegmentRequests::U64Pairs(segment_requests)); + } Self::new_generic( pdu_header, start_of_scope, end_of_scope, - Some(SegmentRequests::U64Pairs(segment_requests)), + passed_segment_requests, ) } @@ -230,6 +246,7 @@ impl WritablePduPacket for NakPduCreator<'_> { /// Special iterator type for the NAK PDU which allows to iterate over both normal and large file /// segment requests. +#[derive(Debug)] pub struct SegmentRequestIter<'a, T> { seq_req_raw: &'a [u8], current_idx: usize, @@ -422,6 +439,17 @@ impl<'seg_reqs> NakPduReader<'seg_reqs> { self.end_of_scope } + pub fn num_segment_reqs(&self) -> usize { + if self.seg_reqs_raw.is_empty() { + return 0; + } + if self.file_flag() == LargeFileFlag::Normal { + self.seg_reqs_raw.len() / 8 + } else { + self.seg_reqs_raw.len() / 16 + } + } + /// This function returns [None] if this NAK PDUs contains segment requests for a large file. pub fn get_normal_segment_requests_iterator(&self) -> Option> { if self.file_flag() == LargeFileFlag::Large { @@ -448,27 +476,30 @@ impl<'seg_reqs> NakPduReader<'seg_reqs> { } impl<'a, 'b> PartialEq> for NakPduReader<'b> { - fn eq(&self, other: &NakPduCreator) -> bool { - if (self.pdu_header() != other.pdu_header() || self.end_of_scope() != other.end_of_scope()) - || (self.start_of_scope() != other.start_of_scope()) + fn eq(&self, other: &NakPduCreator<'a>) -> bool { + if self.pdu_header() != other.pdu_header() + || self.end_of_scope() != other.end_of_scope() + || self.start_of_scope() != other.start_of_scope() { return false; } - if other.segment_requests().is_none() && self.seg_reqs_raw.is_empty() { - return true; - } - if self.file_flag() == LargeFileFlag::Normal { - let normal_iter = self.get_normal_segment_requests_iterator().unwrap(); - if normal_iter != *other.segment_requests().unwrap() { - return false; - } - } else { - let large_iter = self.get_large_segment_requests_iterator().unwrap(); - if large_iter != *other.segment_requests().unwrap() { - return false; + + // Check if both segment requests are empty or None + match (self.seg_reqs_raw.is_empty(), other.segment_requests()) { + (true, None) => true, + (true, Some(seg_reqs)) => seg_reqs.is_empty(), + (false, None) => false, + _ => { + // Compare based on file_flag + if self.file_flag() == LargeFileFlag::Normal { + let normal_iter = self.get_normal_segment_requests_iterator().unwrap(); + normal_iter == *other.segment_requests().unwrap() + } else { + let large_iter = self.get_large_segment_requests_iterator().unwrap(); + large_iter == *other.segment_requests().unwrap() + } } } - true } } @@ -512,6 +543,8 @@ mod tests { let pdu_header = PduHeader::new_no_file_data(pdu_conf, 0); let nak_pdu = NakPduCreator::new_no_segment_requests(pdu_header, 100, 300) .expect("creating NAK PDU creator failed"); + assert_eq!(nak_pdu.start_of_scope(), 100); + assert_eq!(nak_pdu.end_of_scope(), 300); let mut buf: [u8; 64] = [0; 64]; nak_pdu .write_to_bytes(&mut buf) @@ -589,7 +622,7 @@ mod tests { } #[test] - fn test_deserialization_one_file_segment() { + fn test_deserialization_large_segments() { let pdu_conf = common_pdu_conf(CrcFlag::NoCrc, LargeFileFlag::Large); let pdu_header = PduHeader::new_no_file_data(pdu_conf, 0); let nak_pdu = @@ -601,5 +634,90 @@ mod tests { .expect("writing NAK PDU to buffer failed"); let nak_pdu_deser = NakPduReader::from_bytes(&buf).expect("deserializing NAK PDU failed"); assert_eq!(nak_pdu_deser, nak_pdu); + assert_eq!(nak_pdu_deser.start_of_scope(), 100); + assert_eq!(nak_pdu_deser.end_of_scope(), 300); + assert_eq!(nak_pdu_deser.num_segment_reqs(), 2); + assert!(nak_pdu_deser + .get_large_segment_requests_iterator() + .is_some()); + assert!(nak_pdu_deser + .get_normal_segment_requests_iterator() + .is_none()); + assert_eq!( + nak_pdu_deser + .get_large_segment_requests_iterator() + .unwrap() + .count(), + 2 + ); + for (idx, large_segments) in nak_pdu_deser + .get_large_segment_requests_iterator() + .unwrap() + .enumerate() + { + if idx == 0 { + assert_eq!(large_segments.0, 50); + assert_eq!(large_segments.1, 100); + } else { + assert_eq!(large_segments.0, 200); + assert_eq!(large_segments.1, 300); + } + } + } + + #[test] + fn test_deserialization_normal_segments() { + let pdu_conf = common_pdu_conf(CrcFlag::NoCrc, LargeFileFlag::Normal); + let pdu_header = PduHeader::new_no_file_data(pdu_conf, 0); + let nak_pdu = NakPduCreator::new(pdu_header, 100, 300, &[(50, 100), (200, 300)]) + .expect("creating NAK PDU creator failed"); + let mut buf: [u8; 128] = [0; 128]; + nak_pdu + .write_to_bytes(&mut buf) + .expect("writing NAK PDU to buffer failed"); + let nak_pdu_deser = NakPduReader::from_bytes(&buf).expect("deserializing NAK PDU failed"); + assert_eq!(nak_pdu_deser, nak_pdu); + assert_eq!(nak_pdu_deser.start_of_scope(), 100); + assert_eq!(nak_pdu_deser.end_of_scope(), 300); + assert_eq!(nak_pdu_deser.num_segment_reqs(), 2); + assert!(nak_pdu_deser + .get_normal_segment_requests_iterator() + .is_some()); + assert!(nak_pdu_deser + .get_large_segment_requests_iterator() + .is_none()); + assert_eq!( + nak_pdu_deser + .get_normal_segment_requests_iterator() + .unwrap() + .count(), + 2 + ); + for (idx, large_segments) in nak_pdu_deser + .get_normal_segment_requests_iterator() + .unwrap() + .enumerate() + { + if idx == 0 { + assert_eq!(large_segments.0, 50); + assert_eq!(large_segments.1, 100); + } else { + assert_eq!(large_segments.0, 200); + assert_eq!(large_segments.1, 300); + } + } + } + + #[test] + fn test_empty_is_empty() { + let pdu_conf = common_pdu_conf(CrcFlag::NoCrc, LargeFileFlag::Normal); + let pdu_header = PduHeader::new_no_file_data(pdu_conf, 0); + let nak_pdu_0 = + NakPduCreator::new(pdu_header, 100, 300, &[]).expect("creating NAK PDU creator failed"); + let nak_pdu_1 = NakPduCreator::new_no_segment_requests(pdu_header, 100, 300) + .expect("creating NAK PDU creator failed"); + assert_eq!(nak_pdu_0, nak_pdu_1); + // Assert the segment request is mapped to None. + assert!(nak_pdu_0.segment_requests().is_none()); } } From c99b6fddec81cf06ab69afcf316d9a2c862b4550 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 1 Dec 2023 10:22:44 +0100 Subject: [PATCH 12/19] added coverage helper script --- coverage.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100755 coverage.py diff --git a/coverage.py b/coverage.py new file mode 100755 index 0000000..fdd634b --- /dev/null +++ b/coverage.py @@ -0,0 +1,28 @@ +#!/usr/bin/env python3 +import os +import argparse +import webbrowser + + +def generate_cov_report(open_report: bool): + os.environ["RUSTFLAGS"] = "-Cinstrument-coverage" + os.environ["LLVM_PROFILE_FILE"] = "target/coverage/%p-%m.profraw" + os.system("cargo test") + os.system( + "grcov . -s . --binary-path ./target/debug/ -t html --branch --ignore-not-existing " + "-o ./target/debug/coverage/" + ) + if open_report: + coverage_report_path = os.path.abspath("./target/debug/coverage/index.html") + webbrowser.open_new_tab(coverage_report_path) + + +def main(): + parser = argparse.ArgumentParser(description="Generate coverage report and optionally open it in a browser") + parser.add_argument("--open", action="store_true", help="Open the coverage report in a browser") + args = parser.parse_args() + generate_cov_report(args.open) + + +if __name__ == "__main__": + main() From 94ff4fbb5150ad5901b667da8eecd8afd03f1e96 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 1 Dec 2023 10:33:12 +0100 Subject: [PATCH 13/19] added coverage section in README --- README.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/README.md b/README.md index a440074..1568704 100644 --- a/README.md +++ b/README.md @@ -47,3 +47,15 @@ deserializing them with an appropriate `serde` provider like You can check the [documentation](https://docs.rs/spacepackets) of individual modules for various usage examples. + +# Coverage + +Coverage was generated using [`grcov`](https://github.com/mozilla/grcov). If you have not done so +already, install the `llvm-tools-preview`: + +```sh +rustup component add llvm-tools-preview +``` + +After that, you can simply run `coverage.py` to test the project with coverage. You can optionally +supply the `--open` flag to open the coverage report in your webbrowser. From ea1edfb3c1590751c3149e3e2da9feeabafbe2d6 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 1 Dec 2023 12:11:31 +0100 Subject: [PATCH 14/19] improve coverage --- src/cfdp/pdu/nak.rs | 74 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 63 insertions(+), 11 deletions(-) diff --git a/src/cfdp/pdu/nak.rs b/src/cfdp/pdu/nak.rs index fa747fa..1adc9f3 100644 --- a/src/cfdp/pdu/nak.rs +++ b/src/cfdp/pdu/nak.rs @@ -512,17 +512,7 @@ mod tests { use super::*; - #[test] - fn test_basic_creator() { - let pdu_conf = common_pdu_conf(CrcFlag::NoCrc, LargeFileFlag::Normal); - let pdu_header = PduHeader::new_no_file_data(pdu_conf, 0); - let nak_pdu = NakPduCreator::new_no_segment_requests(pdu_header, 0, 0) - .expect("creating NAK PDU creator failed"); - assert_eq!(nak_pdu.start_of_scope(), 0); - assert_eq!(nak_pdu.end_of_scope(), 0); - assert_eq!(nak_pdu.segment_requests(), None); - assert_eq!(nak_pdu.num_segment_reqs(), 0); - + fn check_generic_fields(nak_pdu: &impl CfdpPdu) { assert_eq!(nak_pdu.crc_flag(), CrcFlag::NoCrc); assert_eq!(nak_pdu.file_flag(), LargeFileFlag::Normal); assert_eq!(nak_pdu.pdu_type(), PduType::FileDirective); @@ -537,6 +527,27 @@ mod tests { assert_eq!(nak_pdu.transaction_seq_num(), TEST_SEQ_NUM.into()); } + #[test] + fn test_seg_request_api() { + let seg_req = SegmentRequests::U32Pairs(&[]); + assert!(seg_req.is_empty()); + let seg_req = SegmentRequests::U64Pairs(&[]); + assert!(seg_req.is_empty()); + } + + #[test] + fn test_basic_creator() { + let pdu_conf = common_pdu_conf(CrcFlag::NoCrc, LargeFileFlag::Normal); + let pdu_header = PduHeader::new_no_file_data(pdu_conf, 0); + let nak_pdu = NakPduCreator::new_no_segment_requests(pdu_header, 0, 0) + .expect("creating NAK PDU creator failed"); + assert_eq!(nak_pdu.start_of_scope(), 0); + assert_eq!(nak_pdu.end_of_scope(), 0); + assert_eq!(nak_pdu.segment_requests(), None); + assert_eq!(nak_pdu.num_segment_reqs(), 0); + check_generic_fields(&nak_pdu); + } + #[test] fn test_serialization_empty() { let pdu_conf = common_pdu_conf(CrcFlag::NoCrc, LargeFileFlag::Normal); @@ -619,6 +630,7 @@ mod tests { .expect("writing NAK PDU to buffer failed"); let nak_pdu_deser = NakPduReader::from_bytes(&buf).expect("deserializing NAK PDU failed"); assert_eq!(nak_pdu_deser, nak_pdu); + check_generic_fields(&nak_pdu_deser); } #[test] @@ -720,4 +732,44 @@ mod tests { // Assert the segment request is mapped to None. assert!(nak_pdu_0.segment_requests().is_none()); } + + #[test] + fn test_new_generic_invalid_input() { + let pdu_conf = common_pdu_conf(CrcFlag::NoCrc, LargeFileFlag::Normal); + let pdu_header = PduHeader::new_no_file_data(pdu_conf, 0); + let u32_list = SegmentRequests::U32Pairs(&[(0, 50), (50, 100)]); + if let Err(PduError::InvalidStartOrEndOfScopeValue) = NakPduCreator::new_generic( + pdu_header, + u32::MAX as u64 + 1, + u32::MAX as u64 + 2, + Some(u32_list), + ) { + } else { + panic!("API call did not fail"); + } + } + + #[test] + fn test_target_buf_too_small() { + let pdu_conf = common_pdu_conf(CrcFlag::NoCrc, LargeFileFlag::Normal); + let pdu_header = PduHeader::new_no_file_data(pdu_conf, 0); + let nak_pdu = NakPduCreator::new_no_segment_requests(pdu_header, 100, 300) + .expect("creating NAK PDU creator failed"); + assert_eq!(nak_pdu.start_of_scope(), 100); + assert_eq!(nak_pdu.end_of_scope(), 300); + let mut buf: [u8; 5] = [0; 5]; + let error = nak_pdu.write_to_bytes(&mut buf); + assert!(error.is_err()); + let e = error.unwrap_err(); + match e { + PduError::ByteConversionError(conv_error) => match conv_error { + ByteConversionError::ToSliceTooSmall { found, expected } => { + assert_eq!(expected, nak_pdu.len_written()); + assert_eq!(found, 5); + } + _ => panic!("unexpected error {conv_error}"), + }, + _ => panic!("unexpected error {e}"), + } + } } From 56113ffbcb7d05c4b0568efb98f976d6f91b0936 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 1 Dec 2023 14:18:33 +0100 Subject: [PATCH 15/19] NAK PDU crc check --- src/cfdp/pdu/nak.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/cfdp/pdu/nak.rs b/src/cfdp/pdu/nak.rs index 1adc9f3..df13d86 100644 --- a/src/cfdp/pdu/nak.rs +++ b/src/cfdp/pdu/nak.rs @@ -772,4 +772,26 @@ mod tests { _ => panic!("unexpected error {e}"), } } + + #[test] + fn test_with_crc() { + let pdu_conf = common_pdu_conf(CrcFlag::WithCrc, LargeFileFlag::Normal); + let pdu_header = PduHeader::new_no_file_data(pdu_conf, 0); + let nak_pdu = NakPduCreator::new_no_segment_requests(pdu_header, 0, 0) + .expect("creating NAK PDU creator failed"); + let mut nak_vec = nak_pdu.to_vec().expect("writing NAK to vector failed"); + assert_eq!(nak_vec.len(), pdu_header.header_len() + 9 + 2); + assert_eq!(nak_vec.len(), nak_pdu.len_written()); + let nak_pdu_deser = NakPduReader::new(&nak_vec).expect("reading NAK PDU failed"); + assert_eq!(nak_pdu_deser, nak_pdu); + nak_vec[nak_pdu.len_written() - 1] -= 1; + let nak_pdu_deser = NakPduReader::new(&nak_vec); + assert!(nak_pdu_deser.is_err()); + if let Err(PduError::ChecksumError(raw)) = nak_pdu_deser { + assert_eq!( + raw, + u16::from_be_bytes(nak_vec[nak_pdu.len_written() - 2..].try_into().unwrap()) + ); + } + } } From e422f4f969de7bb547773628d3ceb8b23c537656 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 1 Dec 2023 14:22:35 +0100 Subject: [PATCH 16/19] fix for serde --- src/cfdp/pdu/nak.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/cfdp/pdu/nak.rs b/src/cfdp/pdu/nak.rs index df13d86..5c1f86c 100644 --- a/src/cfdp/pdu/nak.rs +++ b/src/cfdp/pdu/nak.rs @@ -13,6 +13,7 @@ use super::{ use serde::{Deserialize, Serialize}; #[derive(Debug, PartialEq, Eq, Clone)] +#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] pub enum SegmentRequests<'a> { U32Pairs(&'a [(u32, u32)]), U64Pairs(&'a [(u64, u64)]), From e2ae959d0359e9302ed72735f02671bb8b5cadd3 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 1 Dec 2023 14:27:21 +0100 Subject: [PATCH 17/19] serde impl is tricky here.. --- src/cfdp/pdu/nak.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/cfdp/pdu/nak.rs b/src/cfdp/pdu/nak.rs index 5c1f86c..2b48ecb 100644 --- a/src/cfdp/pdu/nak.rs +++ b/src/cfdp/pdu/nak.rs @@ -9,11 +9,7 @@ use super::{ PduHeader, WritablePduPacket, }; -#[cfg(feature = "serde")] -use serde::{Deserialize, Serialize}; - #[derive(Debug, PartialEq, Eq, Clone)] -#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] pub enum SegmentRequests<'a> { U32Pairs(&'a [(u32, u32)]), U64Pairs(&'a [(u64, u64)]), @@ -33,7 +29,6 @@ impl SegmentRequests<'_> { /// It exposes a specialized API which simplifies to generate these NAK PDUs with the /// format according to CFDP chapter 5.2.6. #[derive(Debug, Clone, PartialEq, Eq)] -#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] pub struct NakPduCreator<'seg_reqs> { pdu_header: PduHeader, start_of_scope: u64, From 1f1aa68485e80830ddcc9f86fc3077fea1c0ffb6 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 1 Dec 2023 14:34:16 +0100 Subject: [PATCH 18/19] remove std usages --- src/cfdp/pdu/nak.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/cfdp/pdu/nak.rs b/src/cfdp/pdu/nak.rs index 2b48ecb..5ec93d3 100644 --- a/src/cfdp/pdu/nak.rs +++ b/src/cfdp/pdu/nak.rs @@ -2,13 +2,15 @@ use crate::{ cfdp::{CrcFlag, Direction, LargeFileFlag}, ByteConversionError, }; -use core::marker::PhantomData; +use core::{marker::PhantomData, mem::size_of}; use super::{ add_pdu_crc, generic_length_checks_pdu_deserialization, CfdpPdu, FileDirectiveType, PduError, PduHeader, WritablePduPacket, }; +/// Helper type to encapsulate both normal file size segment requests and large file size segment +/// requests. #[derive(Debug, PartialEq, Eq, Clone)] pub enum SegmentRequests<'a> { U32Pairs(&'a [(u32, u32)]), @@ -246,7 +248,7 @@ impl WritablePduPacket for NakPduCreator<'_> { pub struct SegmentRequestIter<'a, T> { seq_req_raw: &'a [u8], current_idx: usize, - phantom: std::marker::PhantomData, + phantom: core::marker::PhantomData, } pub trait SegReqFromBytes { @@ -273,7 +275,7 @@ where fn next(&mut self) -> Option { let value = self.next_at_offset(self.current_idx); - self.current_idx += 2 * std::mem::size_of::(); + self.current_idx += 2 * size_of::(); value } } @@ -314,7 +316,7 @@ where if pairs.is_empty() && self.seq_req_raw.is_empty() { return true; } - let size = std::mem::size_of::(); + let size = size_of::(); if pairs.len() * 2 * size != self.seq_req_raw.len() { return false; } @@ -332,16 +334,16 @@ where impl SegmentRequestIter<'_, T> { fn next_at_offset(&self, mut offset: usize) -> Option<(T, T)> { - if offset + std::mem::size_of::() * 2 > self.seq_req_raw.len() { + if offset + size_of::() * 2 > self.seq_req_raw.len() { return None; } let start_offset = - T::from_bytes(&self.seq_req_raw[offset..offset + std::mem::size_of::()]); - offset += std::mem::size_of::(); + T::from_bytes(&self.seq_req_raw[offset..offset + size_of::()]); + offset += size_of::(); let end_offset = - T::from_bytes(&self.seq_req_raw[offset..offset + std::mem::size_of::()]); + T::from_bytes(&self.seq_req_raw[offset..offset + size_of::()]); Some((start_offset, end_offset)) } } From e355de3f10f0d034f399aa6ce0930d19b15b74e8 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 1 Dec 2023 14:46:01 +0100 Subject: [PATCH 19/19] fmt --- src/cfdp/pdu/nak.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/cfdp/pdu/nak.rs b/src/cfdp/pdu/nak.rs index 5ec93d3..22d18f2 100644 --- a/src/cfdp/pdu/nak.rs +++ b/src/cfdp/pdu/nak.rs @@ -338,12 +338,10 @@ impl SegmentRequestIter<'_, T> { return None; } - let start_offset = - T::from_bytes(&self.seq_req_raw[offset..offset + size_of::()]); + let start_offset = T::from_bytes(&self.seq_req_raw[offset..offset + size_of::()]); offset += size_of::(); - let end_offset = - T::from_bytes(&self.seq_req_raw[offset..offset + size_of::()]); + let end_offset = T::from_bytes(&self.seq_req_raw[offset..offset + size_of::()]); Some((start_offset, end_offset)) } }