improvements for writable abstractions

CHANGELOG.md

@@ -8,11 +8,20 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 # [unreleased]
 
+## Added
+
+- Add `WritablePduPacket` trait which is a common trait of all CFDP PDU implementations.
+
 ## Fixed
 
-- Set the directtion field inside the PDU header field correctl explicitely for all CFDP PDU
+- Set the direction field inside the PDU header field correctly explicitely for all CFDP PDU
   packets.
 
+## Changed
+
+- Renamed `SerializablePusPacket` to `WritablePusPacket`.
+- Renamed `WritablePduPacket.written_len` and `SerializablePusPacket.written_len` to `len_written`.
+
 # [v0.7.0-beta.2] 2023-09-26
 
 ## Added

src/cfdp/pdu/eof.rs

@@ -42,10 +42,6 @@ impl EofPdu {
         &self.pdu_header
     }
 
-    pub fn written_len(&self) -> usize {
-        self.pdu_header.header_len() + self.calc_pdu_datafield_len()
-    }
-
     pub fn condition_code(&self) -> ConditionCode {
         self.condition_code
     }
@@ -117,7 +113,7 @@ impl EofPdu {
 
 impl WritablePduPacket for EofPdu {
     fn write_to_bytes(&self, buf: &mut [u8]) -> Result<usize, PduError> {
-        let expected_len = self.written_len();
+        let expected_len = self.len_written();
         if buf.len() < expected_len {
             return Err(ByteConversionError::ToSliceTooSmall {
                 found: buf.len(),
@@ -145,6 +141,10 @@ impl WritablePduPacket for EofPdu {
         }
         Ok(current_idx)
     }
+
+    fn len_written(&self) -> usize {
+        self.pdu_header.header_len() + self.calc_pdu_datafield_len()
+    }
 }
 
 #[cfg(test)]
@@ -159,7 +159,7 @@ mod tests {
         let pdu_conf = common_pdu_conf(CrcFlag::NoCrc, LargeFileFlag::Normal);
         let pdu_header = PduHeader::new_no_file_data(pdu_conf, 0);
         let eof_pdu = EofPdu::new_no_error(pdu_header, 0x01020304, 12);
-        assert_eq!(eof_pdu.written_len(), pdu_header.header_len() + 2 + 4 + 4);
+        assert_eq!(eof_pdu.len_written(), pdu_header.header_len() + 2 + 4 + 4);
         assert_eq!(eof_pdu.file_checksum(), 0x01020304);
         assert_eq!(eof_pdu.file_size(), 12);
         assert_eq!(eof_pdu.condition_code(), ConditionCode::NoError);
@@ -174,7 +174,7 @@ mod tests {
         let res = eof_pdu.write_to_bytes(&mut buf);
         assert!(res.is_ok());
         let written = res.unwrap();
-        assert_eq!(written, eof_pdu.written_len());
+        assert_eq!(written, eof_pdu.len_written());
         verify_raw_header(eof_pdu.pdu_header(), &buf);
         let mut current_idx = eof_pdu.pdu_header().header_len();
         buf[current_idx] = FileDirectiveType::EofPdu as u8;

src/cfdp/pdu/file_data.rs

@@ -149,9 +149,6 @@ impl<'seg_meta, 'file_data> FileDataPdu<'seg_meta, 'file_data> {
         }
         len
     }
-    pub fn written_len(&self) -> usize {
-        self.pdu_header.header_len() + self.calc_pdu_datafield_len()
-    }
 
     pub fn offset(&self) -> u64 {
         self.offset
@@ -197,10 +194,10 @@ impl<'seg_meta, 'file_data> FileDataPdu<'seg_meta, 'file_data> {
 
 impl WritablePduPacket for FileDataPdu<'_, '_> {
     fn write_to_bytes(&self, buf: &mut [u8]) -> Result<usize, PduError> {
-        if buf.len() < self.written_len() {
+        if buf.len() < self.len_written() {
             return Err(ByteConversionError::ToSliceTooSmall {
                 found: buf.len(),
-                expected: self.written_len(),
+                expected: self.len_written(),
             }
             .into());
         }
@@ -224,6 +221,10 @@ impl WritablePduPacket for FileDataPdu<'_, '_> {
         }
         Ok(current_idx)
     }
+
+    fn len_written(&self) -> usize {
+        self.pdu_header.header_len() + self.calc_pdu_datafield_len()
+    }
 }
 
 #[cfg(test)]
@@ -247,7 +248,7 @@ mod tests {
         assert_eq!(fd_pdu.offset(), 10);
         assert!(fd_pdu.segment_metadata().is_none());
         assert_eq!(
-            fd_pdu.written_len(),
+            fd_pdu.len_written(),
             fd_pdu.pdu_header.header_len() + core::mem::size_of::<u32>() + 4
         );
     }
@@ -327,7 +328,7 @@ mod tests {
         assert!(fd_pdu.segment_metadata().is_some());
         assert_eq!(*fd_pdu.segment_metadata().unwrap(), segment_meta);
         assert_eq!(
-            fd_pdu.written_len(),
+            fd_pdu.len_written(),
             fd_pdu.pdu_header.header_len()
                 + 1
                 + seg_metadata.len()
@@ -367,7 +368,7 @@ mod tests {
         current_idx += 1;
         assert_eq!(buf[current_idx], 4);
         current_idx += 1;
-        assert_eq!(current_idx, fd_pdu.written_len());
+        assert_eq!(current_idx, fd_pdu.len_written());
     }
 
     #[test]

src/cfdp/pdu/finished.rs

@@ -102,10 +102,6 @@ impl<'fs_responses> FinishedPdu<'fs_responses> {
         &self.pdu_header
     }
 
-    pub fn written_len(&self) -> usize {
-        self.pdu_header.header_len() + self.calc_pdu_datafield_len()
-    }
-
     pub fn condition_code(&self) -> ConditionCode {
         self.condition_code
     }
@@ -220,7 +216,7 @@ impl<'fs_responses> FinishedPdu<'fs_responses> {
 
 impl WritablePduPacket for FinishedPdu<'_> {
     fn write_to_bytes(&self, buf: &mut [u8]) -> Result<usize, PduError> {
-        let expected_len = self.written_len();
+        let expected_len = self.len_written();
         if buf.len() < expected_len {
             return Err(ByteConversionError::ToSliceTooSmall {
                 found: buf.len(),
@@ -248,6 +244,10 @@ impl WritablePduPacket for FinishedPdu<'_> {
         }
         Ok(current_idx)
     }
+
+    fn len_written(&self) -> usize {
+        self.pdu_header.header_len() + self.calc_pdu_datafield_len()
+    }
 }
 
 #[cfg(test)]
@@ -298,7 +298,7 @@ mod tests {
         let written = finished_pdu.write_to_bytes(&mut buf);
         assert!(written.is_ok());
         let written = written.unwrap();
-        assert_eq!(written, finished_pdu.written_len());
+        assert_eq!(written, finished_pdu.len_written());
         assert_eq!(written, finished_pdu.pdu_header().header_len() + 2);
         assert_eq!(
             finished_pdu.pdu_header().pdu_conf.direction,

src/cfdp/pdu/metadata.rs

@@ -174,10 +174,6 @@ impl<'src_name, 'dest_name, 'opts> MetadataPdu<'src_name, 'dest_name, 'opts> {
         })
     }
 
-    pub fn written_len(&self) -> usize {
-        self.pdu_header.header_len() + self.calc_pdu_datafield_len()
-    }
-
     fn calc_pdu_datafield_len(&self) -> usize {
         // One directve type octet and one byte of the directive parameter field.
         let mut len = 2;
@@ -256,7 +252,7 @@ impl<'src_name, 'dest_name, 'opts> MetadataPdu<'src_name, 'dest_name, 'opts> {
 
 impl WritablePduPacket for MetadataPdu<'_, '_, '_> {
     fn write_to_bytes(&self, buf: &mut [u8]) -> Result<usize, PduError> {
-        let expected_len = self.written_len();
+        let expected_len = self.len_written();
         if buf.len() < expected_len {
             return Err(ByteConversionError::ToSliceTooSmall {
                 found: buf.len(),
@@ -291,17 +287,21 @@ impl WritablePduPacket for MetadataPdu<'_, '_, '_> {
         }
         Ok(current_idx)
     }
+
+    fn len_written(&self) -> usize {
+        self.pdu_header.header_len() + self.calc_pdu_datafield_len()
+    }
 }
 
 #[cfg(test)]
 pub mod tests {
-    use super::*;
     use crate::cfdp::lv::Lv;
     use crate::cfdp::pdu::metadata::{
         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::{FileDirectiveType, PduHeader};
     use crate::cfdp::tlv::{Tlv, TlvType};
     use crate::cfdp::{
@@ -340,7 +340,7 @@ pub mod tests {
         let (src_filename, dest_filename, metadata_pdu) =
             generic_metadata_pdu(CrcFlag::NoCrc, LargeFileFlag::Normal, None);
         assert_eq!(
-            metadata_pdu.written_len(),
+            metadata_pdu.len_written(),
             metadata_pdu.pdu_header().header_len()
                 + 1
                 + 1

src/cfdp/pdu/mod.rs

@@ -3,6 +3,8 @@ use crate::cfdp::*;
 use crate::util::{UnsignedByteField, UnsignedByteFieldU8, UnsignedEnum};
 use crate::ByteConversionError;
 use crate::CRC_CCITT_FALSE;
+#[cfg(feature = "alloc")]
+use alloc::vec::Vec;
 use core::fmt::{Display, Formatter};
 #[cfg(feature = "std")]
 use std::error::Error;
@@ -150,7 +152,17 @@ impl From<TlvLvError> for PduError {
 }
 
 pub trait WritablePduPacket {
+    fn len_written(&self) -> usize;
     fn write_to_bytes(&self, buf: &mut [u8]) -> Result<usize, PduError>;
+    #[cfg(feature = "alloc")]
+    fn to_vec(&self) -> Result<Vec<u8>, PduError> {
+        // This is the correct way to do this. See
+        // [this issue](https://github.com/rust-lang/rust-clippy/issues/4483) for caveats of more
+        // "efficient" implementations.
+        let mut vec = alloc::vec![0; self.len_written()];
+        self.write_to_bytes(&mut vec)?;
+        Ok(vec)
+    }
 }
 
 /// Common configuration fields for a PDU.

src/ecss/mod.rs

@@ -360,15 +360,15 @@ pub type EcssEnumU64 = GenericEcssEnumWrapper<u64>;
 /// Generic trait for PUS packet abstractions which can written to a raw slice as their raw
 /// byte representation. This is especially useful for generic abstractions which depend only
 /// on the serialization of those packets.
-pub trait SerializablePusPacket {
-    fn len_packed(&self) -> usize;
+pub trait WritablePusPacket {
+    fn len_written(&self) -> usize;
     fn write_to_bytes(&self, slice: &mut [u8]) -> Result<usize, PusError>;
     #[cfg(feature = "alloc")]
     fn to_vec(&self) -> Result<Vec<u8>, PusError> {
         // This is the correct way to do this. See
         // [this issue](https://github.com/rust-lang/rust-clippy/issues/4483) for caveats of more
         // "efficient" implementations.
-        let mut vec = alloc::vec![0; self.len_packed()];
+        let mut vec = alloc::vec![0; self.len_written()];
         self.write_to_bytes(&mut vec)?;
         Ok(vec)
     }

src/ecss/tc.rs

@@ -34,7 +34,7 @@
 use crate::ecss::{
     ccsds_impl, crc_from_raw_data, sp_header_impls, user_data_from_raw,
     verify_crc16_ccitt_false_from_raw_to_pus_error, CrcType, PusError, PusPacket, PusVersion,
-    SerializablePusPacket,
+    WritablePusPacket,
 };
 use crate::{ByteConversionError, CcsdsPacket, PacketType, SequenceFlags, CCSDS_HEADER_LEN};
 use crate::{SpHeader, CRC_CCITT_FALSE};
@@ -216,7 +216,7 @@ pub mod legacy_tc {
     };
     use crate::ecss::{
         ccsds_impl, crc_from_raw_data, crc_procedure, sp_header_impls,
-        verify_crc16_ccitt_false_from_raw_to_pus_error, PusError, PusPacket, SerializablePusPacket,
+        verify_crc16_ccitt_false_from_raw_to_pus_error, PusError, PusPacket, WritablePusPacket,
         CCSDS_HEADER_LEN,
     };
     use crate::ecss::{user_data_from_raw, PusVersion};
@@ -347,7 +347,7 @@ pub mod legacy_tc {
         /// is set correctly.
         pub fn update_ccsds_data_len(&mut self) {
             self.sp_header.data_len =
-                self.len_packed() as u16 - size_of::<crate::zc::SpHeader>() as u16 - 1;
+                self.len_written() as u16 - size_of::<crate::zc::SpHeader>() as u16 - 1;
         }
 
         /// This function should be called before the TC packet is serialized if
@@ -447,8 +447,8 @@ pub mod legacy_tc {
         }
     }
 
-    impl SerializablePusPacket for PusTc<'_> {
-        fn len_packed(&self) -> usize {
+    impl WritablePusPacket for PusTc<'_> {
+        fn len_written(&self) -> usize {
             PUS_TC_MIN_LEN_WITHOUT_APP_DATA + self.app_data.len()
         }
 
@@ -456,7 +456,7 @@ pub mod legacy_tc {
         fn write_to_bytes(&self, slice: &mut [u8]) -> Result<usize, PusError> {
             let mut curr_idx = 0;
             let tc_header_len = size_of::<zc::PusTcSecondaryHeader>();
-            let total_size = self.len_packed();
+            let total_size = self.len_written();
             if total_size > slice.len() {
                 return Err(ByteConversionError::ToSliceTooSmall {
                     found: slice.len(),
@@ -619,7 +619,7 @@ impl<'raw_data> PusTcCreator<'raw_data> {
     /// is set correctly.
     pub fn update_ccsds_data_len(&mut self) {
         self.sp_header.data_len =
-            self.len_packed() as u16 - size_of::<crate::zc::SpHeader>() as u16 - 1;
+            self.len_written() as u16 - size_of::<crate::zc::SpHeader>() as u16 - 1;
     }
 
     /// This function should be called before the TC packet is serialized if
@@ -653,8 +653,8 @@ impl<'raw_data> PusTcCreator<'raw_data> {
     }
 }
 
-impl SerializablePusPacket for PusTcCreator<'_> {
-    fn len_packed(&self) -> usize {
+impl WritablePusPacket for PusTcCreator<'_> {
+    fn len_written(&self) -> usize {
         PUS_TC_MIN_LEN_WITHOUT_APP_DATA + self.app_data.len()
     }
 
@@ -662,7 +662,7 @@ impl SerializablePusPacket for PusTcCreator<'_> {
     fn write_to_bytes(&self, slice: &mut [u8]) -> Result<usize, PusError> {
         let mut curr_idx = 0;
         let tc_header_len = size_of::<zc::PusTcSecondaryHeader>();
-        let total_size = self.len_packed();
+        let total_size = self.len_written();
         if total_size > slice.len() {
             return Err(ByteConversionError::ToSliceTooSmall {
                 found: slice.len(),
@@ -851,7 +851,7 @@ mod tests {
         GenericPusTcSecondaryHeader, PusTcCreator, PusTcReader, PusTcSecondaryHeader, ACK_ALL,
     };
     use crate::ecss::PusVersion::PusC;
-    use crate::ecss::{PusError, PusPacket, SerializablePusPacket};
+    use crate::ecss::{PusError, PusPacket, WritablePusPacket};
     use crate::{ByteConversionError, SpHeader};
     use crate::{CcsdsPacket, SequenceFlags};
     use alloc::vec::Vec;
@@ -993,7 +993,7 @@ mod tests {
         match err {
             PusError::ByteConversion(err) => match err {
                 ByteConversionError::ToSliceTooSmall { found, expected } => {
-                    assert_eq!(expected, pus_tc.len_packed());
+                    assert_eq!(expected, pus_tc.len_written());
                     assert_eq!(found, 12);
                 }
                 _ => panic!("Unexpected error"),

src/ecss/tm.rs

@@ -3,7 +3,7 @@
 use crate::ecss::{
     calc_pus_crc16, ccsds_impl, crc_from_raw_data, sp_header_impls, user_data_from_raw,
     verify_crc16_ccitt_false_from_raw_to_pus_error, CrcType, PusError, PusPacket, PusVersion,
-    SerializablePusPacket,
+    WritablePusPacket,
 };
 use crate::{
     ByteConversionError, CcsdsPacket, PacketType, SequenceFlags, SpHeader, CCSDS_HEADER_LEN,
@@ -203,7 +203,7 @@ pub mod legacy_tm {
     use crate::ecss::PusVersion;
     use crate::ecss::{
         ccsds_impl, crc_from_raw_data, crc_procedure, sp_header_impls, user_data_from_raw,
-        verify_crc16_ccitt_false_from_raw_to_pus_error, PusError, PusPacket, SerializablePusPacket,
+        verify_crc16_ccitt_false_from_raw_to_pus_error, PusError, PusPacket, WritablePusPacket,
         CCSDS_HEADER_LEN,
     };
     use crate::SequenceFlags;
@@ -314,7 +314,7 @@ pub mod legacy_tm {
         /// is set correctly
         pub fn update_ccsds_data_len(&mut self) {
             self.sp_header.data_len =
-                self.len_packed() as u16 - size_of::<crate::zc::SpHeader>() as u16 - 1;
+                self.len_written() as u16 - size_of::<crate::zc::SpHeader>() as u16 - 1;
         }
 
         /// This function should be called before the TM packet is serialized if
@@ -423,8 +423,8 @@ pub mod legacy_tm {
         }
     }
 
-    impl SerializablePusPacket for PusTm<'_> {
-        fn len_packed(&self) -> usize {
+    impl WritablePusPacket for PusTm<'_> {
+        fn len_written(&self) -> usize {
             PUS_TM_MIN_LEN_WITHOUT_SOURCE_DATA
                 + self.sec_header.timestamp.len()
                 + self.source_data.len()
@@ -432,7 +432,7 @@ pub mod legacy_tm {
         /// Write the raw PUS byte representation to a provided buffer.
         fn write_to_bytes(&self, slice: &mut [u8]) -> Result<usize, PusError> {
             let mut curr_idx = 0;
-            let total_size = self.len_packed();
+            let total_size = self.len_written();
             if total_size > slice.len() {
                 return Err(ByteConversionError::ToSliceTooSmall {
                     found: slice.len(),
@@ -608,7 +608,7 @@ impl<'raw_data> PusTmCreator<'raw_data> {
     /// is set correctly
     pub fn update_ccsds_data_len(&mut self) {
         self.sp_header.data_len =
-            self.len_packed() as u16 - size_of::<crate::zc::SpHeader>() as u16 - 1;
+            self.len_written() as u16 - size_of::<crate::zc::SpHeader>() as u16 - 1;
     }
 
     /// This function should be called before the TM packet is serialized if
@@ -650,8 +650,8 @@ impl<'raw_data> PusTmCreator<'raw_data> {
     }
 }
 
-impl SerializablePusPacket for PusTmCreator<'_> {
-    fn len_packed(&self) -> usize {
+impl WritablePusPacket for PusTmCreator<'_> {
+    fn len_written(&self) -> usize {
         PUS_TM_MIN_LEN_WITHOUT_SOURCE_DATA
             + self.sec_header.timestamp.len()
             + self.source_data.len()
@@ -659,7 +659,7 @@ impl SerializablePusPacket for PusTmCreator<'_> {
     /// Write the raw PUS byte representation to a provided buffer.
     fn write_to_bytes(&self, slice: &mut [u8]) -> Result<usize, PusError> {
         let mut curr_idx = 0;
-        let total_size = self.len_packed();
+        let total_size = self.len_written();
         if total_size > slice.len() {
             return Err(ByteConversionError::ToSliceTooSmall {
                 found: slice.len(),
@@ -949,31 +949,31 @@ mod tests {
 
     fn base_ping_reply_full_ctor(timestamp: &[u8]) -> PusTmCreator {
         let mut sph = SpHeader::tm_unseg(0x123, 0x234, 0).unwrap();
-        let tm_header = PusTmSecondaryHeader::new_simple(17, 2, &timestamp);
+        let tm_header = PusTmSecondaryHeader::new_simple(17, 2, timestamp);
         PusTmCreator::new(&mut sph, tm_header, None, true)
     }
 
     fn base_hk_reply<'a>(timestamp: &'a [u8], src_data: &'a [u8]) -> PusTmCreator<'a> {
         let mut sph = SpHeader::tm_unseg(0x123, 0x234, 0).unwrap();
-        let tc_header = PusTmSecondaryHeader::new_simple(3, 5, &timestamp);
+        let tc_header = PusTmSecondaryHeader::new_simple(3, 5, timestamp);
         PusTmCreator::new(&mut sph, tc_header, Some(src_data), true)
     }
 
     fn dummy_timestamp() -> &'static [u8] {
-        return &[0, 1, 2, 3, 4, 5, 6];
+        &[0, 1, 2, 3, 4, 5, 6]
     }
 
     #[test]
     fn test_basic() {
         let timestamp = dummy_timestamp();
-        let pus_tm = base_ping_reply_full_ctor(&timestamp);
+        let pus_tm = base_ping_reply_full_ctor(timestamp);
         verify_ping_reply(&pus_tm, false, 22, dummy_timestamp());
     }
 
     #[test]
     fn test_serialization_no_source_data() {
         let timestamp = dummy_timestamp();
-        let pus_tm = base_ping_reply_full_ctor(&timestamp);
+        let pus_tm = base_ping_reply_full_ctor(timestamp);
         let mut buf: [u8; 32] = [0; 32];
         let ser_len = pus_tm
             .write_to_bytes(&mut buf)
@@ -1069,7 +1069,7 @@ mod tests {
     #[cfg(feature = "alloc")]
     fn test_append_to_vec() {
         let timestamp = dummy_timestamp();
-        let pus_tm = base_ping_reply_full_ctor(&timestamp);
+        let pus_tm = base_ping_reply_full_ctor(timestamp);
         let mut vec = Vec::new();
         let res = pus_tm.append_to_vec(&mut vec);
         assert!(res.is_ok());
@@ -1124,7 +1124,7 @@ mod tests {
         exp_full_len: usize,
         exp_timestamp: &[u8],
     ) {
-        assert_eq!(tm.len_packed(), exp_full_len);
+        assert_eq!(tm.len_written(), exp_full_len);
         assert_eq!(tm.timestamp(), exp_timestamp);
         verify_ping_reply_generic(tm, has_user_data, exp_full_len);
     }