From 7650429c5bf6d54feeae21bd1111699f60211d27 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 24 Nov 2023 17:09:23 +0100 Subject: [PATCH 1/2] custom impl for CommonPduConfig PartialEq --- CHANGELOG.md | 6 ++- src/cfdp/pdu/eof.rs | 12 +++++- src/cfdp/pdu/file_data.rs | 11 +++++- src/cfdp/pdu/finished.rs | 15 ++++++-- src/cfdp/pdu/metadata.rs | 41 +++++++++++++++++---- src/cfdp/pdu/mod.rs | 77 ++++++++++++++++++++++++++++++++++++--- 6 files changed, 141 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0952f2a..90be3b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## Added - Add `WritablePduPacket` trait which is a common trait of all CFDP PDU implementations. +- Add `CfdpPdu` trait which exposes fields and attributes common to all CFDP PDUs. ## Fixed @@ -21,6 +22,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Renamed `SerializablePusPacket` to `WritablePusPacket`. - Renamed `WritablePduPacket.written_len` and `SerializablePusPacket.len_packed` to `len_written`. +- Introduce custom implementation of `PartialEq` for `CommonPduConfig` which only compares the + values for the source entity ID, destination entity ID and transaction sequence number field to + allow those fields to have different widths. # [v0.7.0-beta.2] 2023-09-26 @@ -112,7 +116,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). `PusTm` object. ## Changed - + - The `EcssEnumeration` now requires the `UnsignedEnum` trait and only adds the `pfc` method to it. - Renamed `byte_width` usages to `size` (part of new `UnsignedEnum` trait) - Moved `ecss::CRC_CCITT_FALSE` CRC constant to the root module. This CRC type is not just used by diff --git a/src/cfdp/pdu/eof.rs b/src/cfdp/pdu/eof.rs index b251f47..09be8e9 100644 --- a/src/cfdp/pdu/eof.rs +++ b/src/cfdp/pdu/eof.rs @@ -8,7 +8,7 @@ use crate::ByteConversionError; #[cfg(feature = "serde")] use serde::{Deserialize, Serialize}; -use super::WritablePduPacket; +use super::{CfdpPdu, WritablePduPacket}; /// Finished PDU abstraction. /// @@ -111,6 +111,16 @@ impl EofPdu { } } +impl CfdpPdu for EofPdu { + fn pdu_header(&self) -> &PduHeader { + &self.pdu_header + } + + fn file_directive_type(&self) -> Option { + Some(FileDirectiveType::EofPdu) + } +} + impl WritablePduPacket for EofPdu { fn write_to_bytes(&self, buf: &mut [u8]) -> Result { let expected_len = self.len_written(); diff --git a/src/cfdp/pdu/file_data.rs b/src/cfdp/pdu/file_data.rs index 4c334a6..928b369 100644 --- a/src/cfdp/pdu/file_data.rs +++ b/src/cfdp/pdu/file_data.rs @@ -8,7 +8,7 @@ use num_enum::{IntoPrimitive, TryFromPrimitive}; #[cfg(feature = "serde")] use serde::{Deserialize, Serialize}; -use super::WritablePduPacket; +use super::{CfdpPdu, FileDirectiveType, WritablePduPacket}; #[derive(Debug, Copy, Clone, PartialEq, Eq, TryFromPrimitive, IntoPrimitive)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] @@ -191,6 +191,15 @@ impl<'seg_meta, 'file_data> FileDataPdu<'seg_meta, 'file_data> { }) } } +impl CfdpPdu for FileDataPdu<'_, '_> { + fn pdu_header(&self) -> &PduHeader { + &self.pdu_header + } + + fn file_directive_type(&self) -> Option { + None + } +} impl WritablePduPacket for FileDataPdu<'_, '_> { fn write_to_bytes(&self, buf: &mut [u8]) -> Result { diff --git a/src/cfdp/pdu/finished.rs b/src/cfdp/pdu/finished.rs index d56d5a0..1a5f8c2 100644 --- a/src/cfdp/pdu/finished.rs +++ b/src/cfdp/pdu/finished.rs @@ -8,7 +8,7 @@ use num_enum::{IntoPrimitive, TryFromPrimitive}; #[cfg(feature = "serde")] use serde::{Deserialize, Serialize}; -use super::WritablePduPacket; +use super::{CfdpPdu, WritablePduPacket}; #[derive(Debug, Copy, Clone, PartialEq, Eq, TryFromPrimitive, IntoPrimitive)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] @@ -98,9 +98,6 @@ impl<'fs_responses> FinishedPdu<'fs_responses> { finished_pdu.pdu_header.pdu_datafield_len = finished_pdu.calc_pdu_datafield_len() as u16; finished_pdu } - pub fn pdu_header(&self) -> &PduHeader { - &self.pdu_header - } pub fn condition_code(&self) -> ConditionCode { self.condition_code @@ -214,6 +211,16 @@ impl<'fs_responses> FinishedPdu<'fs_responses> { } } +impl CfdpPdu for FinishedPdu<'_> { + fn pdu_header(&self) -> &PduHeader { + &self.pdu_header + } + + fn file_directive_type(&self) -> Option { + Some(FileDirectiveType::FinishedPdu) + } +} + impl WritablePduPacket for FinishedPdu<'_> { fn write_to_bytes(&self, buf: &mut [u8]) -> Result { let expected_len = self.len_written(); diff --git a/src/cfdp/pdu/metadata.rs b/src/cfdp/pdu/metadata.rs index d68444c..9f17d04 100644 --- a/src/cfdp/pdu/metadata.rs +++ b/src/cfdp/pdu/metadata.rs @@ -11,7 +11,7 @@ use alloc::vec::Vec; #[cfg(feature = "serde")] use serde::{Deserialize, Serialize}; -use super::WritablePduPacket; +use super::{CfdpPdu, WritablePduPacket}; #[derive(Default, Debug, Copy, Clone, PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] @@ -193,10 +193,6 @@ impl<'src_name, 'dest_name, 'opts> MetadataPdu<'src_name, 'dest_name, 'opts> { len } - pub fn pdu_header(&self) -> &PduHeader { - &self.pdu_header - } - pub fn from_bytes<'longest: 'src_name + 'dest_name + 'opts>( buf: &'longest [u8], ) -> Result { @@ -250,6 +246,16 @@ impl<'src_name, 'dest_name, 'opts> MetadataPdu<'src_name, 'dest_name, 'opts> { } } +impl CfdpPdu for MetadataPdu<'_, '_, '_> { + fn pdu_header(&self) -> &PduHeader { + &self.pdu_header + } + + fn file_directive_type(&self) -> Option { + Some(FileDirectiveType::MetadataPdu) + } +} + impl WritablePduPacket for MetadataPdu<'_, '_, '_> { fn write_to_bytes(&self, buf: &mut [u8]) -> Result { let expected_len = self.len_written(); @@ -300,12 +306,15 @@ pub mod tests { build_metadata_opts_from_slice, build_metadata_opts_from_vec, MetadataGenericParams, MetadataPdu, }; - use crate::cfdp::pdu::tests::{common_pdu_conf, verify_raw_header}; - use crate::cfdp::pdu::WritablePduPacket; + use crate::cfdp::pdu::tests::{ + common_pdu_conf, verify_raw_header, TEST_DEST_ID, TEST_SEQ_NUM, TEST_SRC_ID, + }; + use crate::cfdp::pdu::{CfdpPdu, WritablePduPacket}; use crate::cfdp::pdu::{FileDirectiveType, PduHeader}; use crate::cfdp::tlv::{Tlv, TlvType}; use crate::cfdp::{ - ChecksumType, CrcFlag, LargeFileFlag, PduType, SegmentMetadataFlag, SegmentationControl, + ChecksumType, CrcFlag, Direction, LargeFileFlag, PduType, SegmentMetadataFlag, + SegmentationControl, TransmissionMode, }; use std::vec; @@ -351,6 +360,21 @@ pub mod tests { assert_eq!(metadata_pdu.src_file_name(), src_filename); assert_eq!(metadata_pdu.dest_file_name(), dest_filename); assert_eq!(metadata_pdu.options(), None); + assert_eq!(metadata_pdu.crc_flag(), CrcFlag::NoCrc); + assert_eq!(metadata_pdu.file_flag(), LargeFileFlag::Normal); + assert_eq!(metadata_pdu.pdu_type(), PduType::FileDirective); + assert_eq!( + metadata_pdu.file_directive_type(), + Some(FileDirectiveType::MetadataPdu) + ); + assert_eq!( + metadata_pdu.transmission_mode(), + TransmissionMode::Acknowledged + ); + assert_eq!(metadata_pdu.direction(), Direction::TowardsReceiver); + assert_eq!(metadata_pdu.source_id(), TEST_SRC_ID.into()); + assert_eq!(metadata_pdu.dest_id(), TEST_DEST_ID.into()); + assert_eq!(metadata_pdu.transaction_seq_num(), TEST_SEQ_NUM.into()); } #[test] @@ -414,6 +438,7 @@ pub mod tests { fn test_with_crc_flag() { let (src_filename, dest_filename, metadata_pdu) = generic_metadata_pdu(CrcFlag::WithCrc, LargeFileFlag::Normal, None); + assert_eq!(metadata_pdu.crc_flag(), CrcFlag::WithCrc); let mut buf: [u8; 64] = [0; 64]; let write_res = metadata_pdu.write_to_bytes(&mut buf); assert!(write_res.is_ok()); diff --git a/src/cfdp/pdu/mod.rs b/src/cfdp/pdu/mod.rs index fbb2e10..16e8cb3 100644 --- a/src/cfdp/pdu/mod.rs +++ b/src/cfdp/pdu/mod.rs @@ -165,8 +165,51 @@ pub trait WritablePduPacket { } } +/// Abstraction trait for fields and properties common for all PDUs. +pub trait CfdpPdu { + fn pdu_header(&self) -> &PduHeader; + + fn source_id(&self) -> UnsignedByteField { + self.pdu_header().common_pdu_conf().source_entity_id + } + + fn dest_id(&self) -> UnsignedByteField { + self.pdu_header().common_pdu_conf().dest_entity_id + } + + fn transaction_seq_num(&self) -> UnsignedByteField { + self.pdu_header().common_pdu_conf().transaction_seq_num + } + + fn transmission_mode(&self) -> TransmissionMode { + self.pdu_header().common_pdu_conf().trans_mode + } + fn direction(&self) -> Direction { + self.pdu_header().common_pdu_conf().direction + } + + fn crc_flag(&self) -> CrcFlag { + self.pdu_header().common_pdu_conf().crc_flag + } + + fn file_flag(&self) -> LargeFileFlag { + self.pdu_header().common_pdu_conf().file_flag + } + + fn pdu_type(&self) -> PduType { + self.pdu_header().pdu_type() + } + + fn file_directive_type(&self) -> Option; +} + /// Common configuration fields for a PDU. -#[derive(Debug, Copy, Clone, PartialEq, Eq)] +/// +/// Please note that this structure has a custom implementation of [PartialEq] which only +/// compares the values for source entity ID, destination entity ID and transaction sequence +/// number. This permits that those fields can have different widths, as long as the value is the +/// same. +#[derive(Debug, Copy, Clone, Eq)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] pub struct CommonPduConfig { source_entity_id: UnsignedByteField, @@ -287,6 +330,18 @@ impl Default for CommonPduConfig { } } +impl PartialEq for CommonPduConfig { + fn eq(&self, other: &Self) -> bool { + self.source_entity_id.value() == other.source_entity_id.value() + && self.dest_entity_id.value() == other.dest_entity_id.value() + && self.transaction_seq_num.value() == other.transaction_seq_num.value() + && self.trans_mode == other.trans_mode + && self.file_flag == other.file_flag + && self.crc_flag == other.crc_flag + && self.direction == other.direction + } +} + pub const FIXED_HEADER_LEN: usize = 4; /// Abstraction for the PDU header common to all CFDP PDUs. @@ -627,17 +682,18 @@ mod tests { TransmissionMode, CFDP_VERSION_2, }; use crate::util::{ - UbfU8, UnsignedByteField, UnsignedByteFieldU16, UnsignedByteFieldU8, UnsignedEnum, + UbfU8, UnsignedByteField, UnsignedByteFieldU16, UnsignedByteFieldU8, UnsignedEnum, UbfU16, }; use crate::ByteConversionError; use std::format; + pub(crate) const TEST_SRC_ID: UbfU8 = UbfU8::new(5); + pub(crate) const TEST_DEST_ID: UbfU8 = UbfU8::new(10); + pub(crate) const TEST_SEQ_NUM: UbfU8 = UbfU8::new(20); + pub(crate) fn common_pdu_conf(crc_flag: CrcFlag, fss: LargeFileFlag) -> CommonPduConfig { - let src_id = UbfU8::new(5); - let dest_id = UbfU8::new(10); - let transaction_seq_num = UbfU8::new(20); let mut pdu_conf = - CommonPduConfig::new_with_byte_fields(src_id, dest_id, transaction_seq_num) + CommonPduConfig::new_with_byte_fields(TEST_SRC_ID, TEST_DEST_ID, TEST_SEQ_NUM) .expect("Generating common PDU config"); pdu_conf.crc_flag = crc_flag; pdu_conf.file_flag = fss; @@ -728,6 +784,15 @@ mod tests { assert_eq!(pdu_header.header_len(), 7); } + #[test] + fn test_common_pdu_conf_partial_eq() { + let common_pdu_cfg_0 = CommonPduConfig::new_with_byte_fields(UbfU8::new(1), UbfU8::new(2), UbfU8::new(3)) + .expect("common config creation failed"); + let common_pdu_cfg_1 = CommonPduConfig::new_with_byte_fields(UbfU16::new(1), UbfU16::new(2), UbfU16::new(3)) + .expect("common config creation failed"); + assert_eq!(common_pdu_cfg_0, common_pdu_cfg_1); + } + #[test] fn test_basic_state_default() { let default_conf = CommonPduConfig::default(); From a23e5107f25369f5bd1457be52eca5036de65e5c Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sat, 25 Nov 2023 18:16:21 +0100 Subject: [PATCH 2/2] fmt --- src/cfdp/pdu/mod.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/cfdp/pdu/mod.rs b/src/cfdp/pdu/mod.rs index 16e8cb3..2e05b78 100644 --- a/src/cfdp/pdu/mod.rs +++ b/src/cfdp/pdu/mod.rs @@ -682,7 +682,7 @@ mod tests { TransmissionMode, CFDP_VERSION_2, }; use crate::util::{ - UbfU8, UnsignedByteField, UnsignedByteFieldU16, UnsignedByteFieldU8, UnsignedEnum, UbfU16, + UbfU16, UbfU8, UnsignedByteField, UnsignedByteFieldU16, UnsignedByteFieldU8, UnsignedEnum, }; use crate::ByteConversionError; use std::format; @@ -786,10 +786,12 @@ mod tests { #[test] fn test_common_pdu_conf_partial_eq() { - let common_pdu_cfg_0 = CommonPduConfig::new_with_byte_fields(UbfU8::new(1), UbfU8::new(2), UbfU8::new(3)) - .expect("common config creation failed"); - let common_pdu_cfg_1 = CommonPduConfig::new_with_byte_fields(UbfU16::new(1), UbfU16::new(2), UbfU16::new(3)) - .expect("common config creation failed"); + let common_pdu_cfg_0 = + CommonPduConfig::new_with_byte_fields(UbfU8::new(1), UbfU8::new(2), UbfU8::new(3)) + .expect("common config creation failed"); + let common_pdu_cfg_1 = + CommonPduConfig::new_with_byte_fields(UbfU16::new(1), UbfU16::new(2), UbfU16::new(3)) + .expect("common config creation failed"); assert_eq!(common_pdu_cfg_0, common_pdu_cfg_1); }