Merge pull request 'fix badge link for new website' (#54) from second-badge-fix into main

Reviewed-on: #54

CHANGELOG.md

@@ -8,6 +8,45 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 # [unreleased]
 
+# [v0.7.0-beta.3] 2023-12-06
+
+## 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.
+- Add `GenericTlv` and `WritableTlv` trait as abstractions for the various TLV types.
+
+## Fixed
+
+- Set the direction field inside the PDU header field correctly explicitely for all CFDP PDU
+  packets.
+
+## Changed
+
+- Split up `FinishedPdu`into `FinishedPduCreator` and `FinishedPduReader` to expose specialized
+  APIs.
+- Split up `MetadataPdu`into `MetadataPduCreator` and `MetadataPduReader` to expose specialized
+  APIs.
+- Cleaned up CUC time implementation. Added `width` and `counter` getter methods.
+- Renamed `SerializablePusPacket` to `WritablePusPacket`.
+- Renamed `UnsignedPfc` to `PfcUnsigned` and `RealPfc` to `PfcReal`.
+- 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.
+- Removed the `PusError::RawDataTooShort` variant which is already covered by
+  `PusError::ByteConversionError` variant.
+- Ranamed `TlvLvError::ByteConversionError` to `TlvLvError::ByteConversion`.
+- Renamed `PusError::IncorrectCrc` to `PusError::ChecksumFailure`.
+- Some more struct variant changes for error enumerations.
+
+## Removed
+
+- `PusError::NoRawData` variant.
+- `cfdp::LenInBytes` which was not used.
+
+# [v0.7.0-beta.2] 2023-09-26
+
 ## Added
 
 - `PacketId` trait impls: `Ord`, `PartialOrd` and `Hash`
@@ -96,7 +135,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

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "spacepackets"
-version = "0.7.0-beta.2"
+version = "0.7.0-beta.3"
 edition = "2021"
 rust-version = "1.61"
 authors = ["Robin Mueller <muellerr@irs.uni-stuttgart.de>"]

README.md

@@ -1,6 +1,7 @@
 [![Crates.io](https://img.shields.io/crates/v/spacepackets)](https://crates.io/crates/spacepackets)
 [![docs.rs](https://img.shields.io/docsrs/spacepackets)](https://docs.rs/spacepackets)
 [![ci](https://github.com/us-irs/spacepackets-rs/actions/workflows/ci.yml/badge.svg?branch=main)](https://github.com/us-irs/spacepackets-rs/actions/workflows/ci.yml)
+[![coverage](https://shields.io/endpoint?url=https://absatsw.irs.uni-stuttgart.de/projects/spacepackets/coverage-rs/latest/coverage.json)](https://absatsw.irs.uni-stuttgart.de/projects/spacepackets/coverage-rs/latest/index.html)
 
 ECSS and CCSDS Spacepackets
 ======
@@ -47,3 +48,16 @@ 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
+cargo install grcov --locked
+```
+
+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.

automation/Dockerfile

@@ -6,10 +6,20 @@ RUN apt-get update
 RUN apt-get --yes upgrade
 # tzdata is a dependency, won't install otherwise
 ARG DEBIAN_FRONTEND=noninteractive
+RUN apt-get --yes install rsync curl
 
 # set CROSS_CONTAINER_IN_CONTAINER to inform `cross` that it is executed from within a container
 ENV CROSS_CONTAINER_IN_CONTAINER=true
 
 RUN rustup install nightly && \
     rustup target add thumbv7em-none-eabihf armv7-unknown-linux-gnueabihf && \
-    rustup component add rustfmt clippy
+    rustup component add rustfmt clippy llvm-tools-preview
+
+RUN curl -sSL https://github.com/mozilla/grcov/releases/download/v0.8.19/grcov-x86_64-unknown-linux-gnu.tar.bz2 | tar -xj --directory /usr/local/bin
+
+# SSH stuff to allow deployment to doc server
+RUN adduser --uid 114 jenkins
+
+# Add documentation server to known hosts
+RUN echo "|1|/LzCV4BuTmTb2wKnD146l9fTKgQ=|NJJtVjvWbtRt8OYqFgcYRnMQyVw= ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBNL8ssTonYtgiR/6RRlSIK9WU1ywOcJmxFTLcEblAwH7oifZzmYq3XRfwXrgfMpylEfMFYfCU8JRqtmi19xc21A=" >> /etc/ssh/ssh_known_hosts
+RUN echo "|1|CcBvBc3EG03G+XM5rqRHs6gK/Gg=|oGeJQ+1I8NGI2THIkJsW92DpTzs= ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBNL8ssTonYtgiR/6RRlSIK9WU1ywOcJmxFTLcEblAwH7oifZzmYq3XRfwXrgfMpylEfMFYfCU8JRqtmi19xc21A=" >> /etc/ssh/ssh_known_hosts

automation/Jenkinsfile

@@ -1,57 +1,78 @@
 pipeline {
 
-    agent {
-        dockerfile {
-            dir 'automation'
-            reuseNode true
-        }
+  agent {
+    dockerfile {
+      dir 'automation'
+      reuseNode true
+      args '--network host'
     }
+  }
 
-    stages {
-        stage('Rust Toolchain Info') {
-            steps {
-                sh 'rustc --version'
-            }
-        }
-        stage('Clippy') {
-            steps {
-                sh 'cargo clippy'
-            }
-        }
-        stage('Docs') {
-            steps {
-                sh 'cargo +nightly doc --all-features'
-            }
-        }
-        stage('Rustfmt') {
-            steps {
-                sh 'cargo fmt --all --check'
-            }
-        }
-        stage('Test') {
-            steps {
-                sh 'cargo test --all-features'
-            }
-        }
-        stage('Check with all features') {
-            steps {
-                sh 'cargo check --all-features'
-            }
-        }
-        stage('Check with no features') {
-            steps {
-                sh 'cargo check --no-default-features'
-            }
-        }
-        stage('Check Cross Embedded Bare Metal') {
-            steps {
-                sh 'cargo check --target thumbv7em-none-eabihf --no-default-features'
-            }
-        }
-        stage('Check Cross Embedded Linux') {
-            steps {
-                sh 'cargo check --target armv7-unknown-linux-gnueabihf'
-            }
+  stages {
+    stage('Rust Toolchain Info') {
+      steps {
+        sh 'rustc --version'
+      }
+    }
+    stage('Clippy') {
+      steps {
+        sh 'cargo clippy'
+      }
+    }
+    stage('Docs') {
+        steps {
+          sh 'cargo +nightly doc --all-features'
         }
     }
+    stage('Rustfmt') {
+        steps {
+          sh 'cargo fmt --all --check'
+        }
+    }
+    stage('Test') {
+      steps {
+        sh 'cargo test --all-features'
+      }
+    }
+    stage('Check with all features') {
+      steps {
+        sh 'cargo check --all-features'
+      }
+    }
+    stage('Check with no features') {
+      steps {
+        sh 'cargo check --no-default-features'
+      }
+    }
+    stage('Check Cross Embedded Bare Metal') {
+      steps {
+        sh 'cargo check --target thumbv7em-none-eabihf --no-default-features'
+      }
+    }
+    stage('Check Cross Embedded Linux') {
+      steps {
+        sh 'cargo check --target armv7-unknown-linux-gnueabihf'
+      }
+    }
+    stage('Run test with Coverage') {
+      when {
+        anyOf {
+          branch 'main';
+          branch pattern: 'cov-deployment*'
+        }
+      }
+      steps {
+        withEnv(['RUSTFLAGS=-Cinstrument-coverage', 'LLVM_PROFILE_FILE=target/coverage/%p-%m.profraw']) {
+          echo "Executing tests with coverage"
+          sh 'cargo clean'
+          sh 'cargo test --all-features'
+          sh 'grcov . -s . --binary-path ./target/debug -t html --branch --ignore-not-existing -o ./target/debug/coverage/'
+          sshagent(credentials: ['documentation-buildfix']) {
+            // Deploy to Apache webserver
+            sh 'rsync --mkpath -r --delete ./target/debug/coverage/ buildfix@documentation.irs.uni-stuttgart.de:/projects/spacepackets/coverage-rs/latest/'
+          }
+        }
+      }
+    }
+  }
 }

coverage.py

@@ -0,0 +1,54 @@
+#!/usr/bin/env python3
+import os
+import logging
+import argparse
+import webbrowser
+
+
+_LOGGER = logging.getLogger()
+
+
+def generate_cov_report(open_report: bool, format: str):
+    logging.basicConfig(level=logging.INFO)
+    os.environ["RUSTFLAGS"] = "-Cinstrument-coverage"
+    os.environ["LLVM_PROFILE_FILE"] = "target/coverage/%p-%m.profraw"
+    _LOGGER.info("Executing tests with coverage")
+    os.system("cargo test --all-features")
+
+    out_path = "./target/debug/coverage"
+    if format == "lcov":
+        out_path = "./target/debug/lcov.info"
+    os.system(
+        f"grcov . -s . --binary-path ./target/debug/ -t {format} --branch --ignore-not-existing "
+        f"-o {out_path}"
+    )
+    if format == "lcov":
+        os.system(
+            "genhtml -o ./target/debug/coverage/ --show-details --highlight --ignore-errors source "
+            "--legend ./target/debug/lcov.info"
+        )
+    if open_report:
+        coverage_report_path = os.path.abspath("./target/debug/coverage/index.html")
+        webbrowser.open_new_tab(coverage_report_path)
+    _LOGGER.info("Done")
+
+
+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"
+    )
+    parser.add_argument(
+        "--format",
+        choices=["html", "lcov"],
+        default="html",
+        help="Choose report format (html or lcov)",
+    )
+    args = parser.parse_args()
+    generate_cov_report(args.open, args.format)
+
+
+if __name__ == "__main__":
+    main()

release-checklist.md

@@ -4,7 +4,7 @@ Checklist for new releases
 # Pre-Release
 
 1. Make sure any new modules are documented sufficiently enough and check docs with
-   `cargo +nightly doc --all-features --open`.
+   `cargo +nightly doc --all-features --config 'rustdocflags=["--cfg", "doc_cfg"]' --open`.
 2. Bump version specifier in `Cargo.toml`.
 3. Update `CHANGELOG.md`: Convert `unreleased` section into version section with date and add new
    `unreleased` section.

src/cfdp/lv.rs

@@ -165,7 +165,9 @@ impl<'data> Lv<'data> {
 
 #[cfg(test)]
 pub mod tests {
-    use crate::cfdp::lv::Lv;
+    use super::*;
+    use alloc::string::ToString;
+
     use crate::cfdp::TlvLvError;
     use crate::ByteConversionError;
     use std::string::String;
@@ -176,7 +178,7 @@ pub mod tests {
         let lv_res = Lv::new(&lv_data);
         assert!(lv_res.is_ok());
         let lv = lv_res.unwrap();
-        assert!(lv.value().len() > 0);
+        assert!(!lv.value().is_empty());
         let val = lv.value();
         assert_eq!(val[0], 1);
         assert_eq!(val[1], 2);
@@ -259,6 +261,10 @@ pub mod tests {
         let error = lv.unwrap_err();
         if let TlvLvError::DataTooLarge(size) = error {
             assert_eq!(size, u8::MAX as usize + 1);
+            assert_eq!(
+                error.to_string(),
+                "data with size 256 larger than allowed 255 bytes"
+            );
         } else {
             panic!("invalid exception {:?}", error)
         }

src/cfdp/mod.rs

@@ -48,6 +48,24 @@ pub enum CrcFlag {
     WithCrc = 1,
 }
 
+impl From<bool> for CrcFlag {
+    fn from(value: bool) -> Self {
+        if value {
+            return CrcFlag::WithCrc;
+        }
+        CrcFlag::NoCrc
+    }
+}
+
+impl From<CrcFlag> 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))]
@@ -76,17 +94,6 @@ pub enum FaultHandlerCode {
     AbandonTransaction = 0b0100,
 }
 
-#[derive(Debug, Copy, Clone, PartialEq, Eq, TryFromPrimitive, IntoPrimitive)]
-#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
-#[repr(u8)]
-pub enum LenInBytes {
-    ZeroOrNone = 0,
-    OneByte = 1,
-    TwoBytes = 2,
-    ThreeBytes = 4,
-    FourBytes = 8,
-}
-
 #[derive(Debug, Copy, Clone, PartialEq, Eq, TryFromPrimitive, IntoPrimitive)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 #[repr(u8)]
@@ -119,6 +126,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)]
@@ -146,9 +169,12 @@ pub const NULL_CHECKSUM_U32: [u8; 4] = [0; 4];
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 pub enum TlvLvError {
     DataTooLarge(usize),
-    ByteConversionError(ByteConversionError),
+    ByteConversion(ByteConversionError),
     /// First value: Found value. Second value: Expected value if there is one.
-    InvalidTlvTypeField((u8, Option<u8>)),
+    InvalidTlvTypeField {
+        found: u8,
+        expected: Option<u8>,
+    },
     /// Logically invalid value length detected. The value length may not exceed 255 bytes.
     /// Depending on the concrete TLV type, the value length may also be logically invalid.
     InvalidValueLength(usize),
@@ -161,7 +187,7 @@ pub enum TlvLvError {
 
 impl From<ByteConversionError> for TlvLvError {
     fn from(value: ByteConversionError) -> Self {
-        Self::ByteConversionError(value)
+        Self::ByteConversion(value)
     }
 }
 
@@ -176,17 +202,17 @@ impl Display for TlvLvError {
                     u8::MAX
                 )
             }
-            TlvLvError::ByteConversionError(e) => {
-                write!(f, "{}", e)
+            TlvLvError::ByteConversion(e) => {
+                write!(f, "tlv or lv byte conversion: {}", e)
             }
-            TlvLvError::InvalidTlvTypeField((found, expected)) => {
+            TlvLvError::InvalidTlvTypeField { found, expected } => {
                 write!(
                     f,
-                    "invalid TLV type field, found {found}, possibly expected {expected:?}"
+                    "invalid TLV type field, found {found}, expected {expected:?}"
                 )
             }
             TlvLvError::InvalidValueLength(len) => {
-                write!(f, "invalid value length {len} detected")
+                write!(f, "invalid value length {len}")
             }
             TlvLvError::SecondNameMissing => {
                 write!(f, "second name missing for filestore request or response")
@@ -202,8 +228,65 @@ impl Display for TlvLvError {
 impl Error for TlvLvError {
     fn source(&self) -> Option<&(dyn Error + 'static)> {
         match self {
-            TlvLvError::ByteConversionError(e) => Some(e),
+            TlvLvError::ByteConversion(e) => Some(e),
             _ => None,
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    #[cfg(feature = "serde")]
+    use crate::tests::generic_serde_test;
+
+    #[test]
+    fn test_crc_from_bool() {
+        assert_eq!(CrcFlag::from(false), CrcFlag::NoCrc);
+    }
+
+    #[test]
+    fn test_crc_flag_to_bool() {
+        let is_true: bool = CrcFlag::WithCrc.into();
+        assert!(is_true);
+        let is_false: bool = CrcFlag::NoCrc.into();
+        assert!(!is_false);
+    }
+
+    #[test]
+    fn test_default_checksum_type() {
+        let checksum = ChecksumType::default();
+        assert_eq!(checksum, ChecksumType::NullChecksum);
+    }
+
+    #[test]
+    fn test_fault_handler_code_from_u8() {
+        let fault_handler_code_raw = FaultHandlerCode::NoticeOfSuspension as u8;
+        let fault_handler_code = FaultHandlerCode::try_from(fault_handler_code_raw).unwrap();
+        assert_eq!(fault_handler_code, FaultHandlerCode::NoticeOfSuspension);
+    }
+
+    #[test]
+    #[cfg(feature = "serde")]
+    fn test_serde_impl_pdu_type() {
+        generic_serde_test(PduType::FileData);
+    }
+
+    #[test]
+    #[cfg(feature = "serde")]
+    fn test_serde_impl_direction() {
+        generic_serde_test(Direction::TowardsReceiver);
+    }
+
+    #[test]
+    #[cfg(feature = "serde")]
+    fn test_serde_impl_transmission_mode() {
+        generic_serde_test(TransmissionMode::Unacknowledged);
+    }
+
+    #[test]
+    #[cfg(feature = "serde")]
+    fn test_serde_fault_handler_code() {
+        generic_serde_test(FaultHandlerCode::NoticeOfCancellation);
+    }
+}

src/cfdp/pdu/ack.rs

@@ -0,0 +1,334 @@
+use crate::{
+    cfdp::{ConditionCode, CrcFlag, Direction, TransactionStatus},
+    ByteConversionError,
+};
+
+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.
+///
+/// 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<Self, PduError> {
+        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<AckPdu, PduError> {
+        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<FileDirectiveType> {
+        Some(FileDirectiveType::AckPdu)
+    }
+}
+
+impl WritablePduPacket for AckPdu {
+    fn write_to_bytes(&self, buf: &mut [u8]) -> Result<usize, PduError> {
+        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 {
+    use crate::cfdp::{
+        pdu::tests::{common_pdu_conf, verify_raw_header, TEST_DEST_ID, TEST_SEQ_NUM, TEST_SRC_ID},
+        LargeFileFlag, PduType, TransmissionMode,
+    };
+
+    use super::*;
+    #[cfg(feature = "serde")]
+    use crate::tests::generic_serde_test;
+
+    fn verify_state(ack_pdu: &AckPdu, expected_crc_flag: CrcFlag, expected_dir: Direction) {
+        assert_eq!(ack_pdu.condition_code(), ConditionCode::NoError);
+        assert_eq!(ack_pdu.transaction_status(), TransactionStatus::Active);
+
+        assert_eq!(ack_pdu.crc_flag(), expected_crc_flag);
+        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(), expected_dir);
+        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());
+    }
+
+    #[test]
+    fn test_basic() {
+        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
+        );
+        verify_state(&ack_pdu, CrcFlag::NoCrc, Direction::TowardsReceiver);
+    }
+
+    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);
+    }
+
+    #[test]
+    fn test_for_eof_pdu() {
+        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_eof_pdu(
+            pdu_header,
+            ConditionCode::NoError,
+            TransactionStatus::Active,
+        );
+        assert_eq!(
+            ack_pdu.directive_code_of_acked_pdu(),
+            FileDirectiveType::EofPdu
+        );
+        verify_state(&ack_pdu, CrcFlag::WithCrc, Direction::TowardsSender);
+    }
+
+    #[test]
+    #[cfg(feature = "serde")]
+    fn test_ack_pdu_serialization() {
+        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_eof_pdu(
+            pdu_header,
+            ConditionCode::NoError,
+            TransactionStatus::Active,
+        );
+        generic_serde_test(ack_pdu);
+    }
+}

src/cfdp/pdu/eof.rs

@@ -2,12 +2,14 @@ use crate::cfdp::pdu::{
     add_pdu_crc, generic_length_checks_pdu_deserialization, read_fss_field, write_fss_field,
     FileDirectiveType, PduError, PduHeader,
 };
-use crate::cfdp::tlv::EntityIdTlv;
-use crate::cfdp::{ConditionCode, CrcFlag, LargeFileFlag};
+use crate::cfdp::tlv::{EntityIdTlv, WritableTlv};
+use crate::cfdp::{ConditionCode, CrcFlag, Direction, LargeFileFlag};
 use crate::ByteConversionError;
 #[cfg(feature = "serde")]
 use serde::{Deserialize, Serialize};
 
+use super::{CfdpPdu, WritablePduPacket};
+
 /// Finished PDU abstraction.
 ///
 /// For more information, refer to CFDP chapter 5.2.2.
@@ -22,7 +24,9 @@ pub struct EofPdu {
 }
 
 impl EofPdu {
-    pub fn new_no_error(pdu_header: PduHeader, file_checksum: u32, file_size: u64) -> Self {
+    pub fn new_no_error(mut pdu_header: PduHeader, file_checksum: u32, file_size: u64) -> Self {
+        // Force correct direction flag.
+        pdu_header.pdu_conf.direction = Direction::TowardsReceiver;
         let mut eof_pdu = Self {
             pdu_header,
             condition_code: ConditionCode::NoError,
@@ -38,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
     }
@@ -63,39 +63,12 @@ impl EofPdu {
         if let Some(fault_location) = self.fault_location {
             len += fault_location.len_full();
         }
+        if self.crc_flag() == CrcFlag::WithCrc {
+            len += 2;
+        }
         len
     }
 
-    pub fn write_to_bytes(&self, buf: &mut [u8]) -> Result<usize, PduError> {
-        let expected_len = self.written_len();
-        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::EofPdu as u8;
-        current_idx += 1;
-        buf[current_idx] = (self.condition_code as u8) << 4;
-        current_idx += 1;
-        buf[current_idx..current_idx + 4].copy_from_slice(&self.file_checksum.to_be_bytes());
-        current_idx += 4;
-        current_idx += write_fss_field(
-            self.pdu_header.pdu_conf.file_flag,
-            self.file_size,
-            &mut buf[current_idx..],
-        )?;
-        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 {
-            current_idx = add_pdu_crc(buf, current_idx);
-        }
-        Ok(current_idx)
-    }
-
     pub fn from_bytes(buf: &[u8]) -> Result<EofPdu, PduError> {
         let (pdu_header, mut current_idx) = PduHeader::from_bytes(buf)?;
         let full_len_without_crc = pdu_header.verify_length_and_checksum(buf)?;
@@ -141,22 +114,89 @@ impl EofPdu {
     }
 }
 
+impl CfdpPdu for EofPdu {
+    fn pdu_header(&self) -> &PduHeader {
+        &self.pdu_header
+    }
+
+    fn file_directive_type(&self) -> Option<FileDirectiveType> {
+        Some(FileDirectiveType::EofPdu)
+    }
+}
+
+impl WritablePduPacket for EofPdu {
+    fn write_to_bytes(&self, buf: &mut [u8]) -> Result<usize, PduError> {
+        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::EofPdu as u8;
+        current_idx += 1;
+        buf[current_idx] = (self.condition_code as u8) << 4;
+        current_idx += 1;
+        buf[current_idx..current_idx + 4].copy_from_slice(&self.file_checksum.to_be_bytes());
+        current_idx += 4;
+        current_idx += write_fss_field(
+            self.pdu_header.pdu_conf.file_flag,
+            self.file_size,
+            &mut buf[current_idx..],
+        )?;
+        if let Some(fault_location) = self.fault_location {
+            current_idx += fault_location.write_to_bytes(buf)?;
+        }
+        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 {
-    use crate::cfdp::pdu::eof::EofPdu;
-    use crate::cfdp::pdu::tests::{common_pdu_conf, verify_raw_header};
+    use super::*;
+    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};
+    #[cfg(feature = "serde")]
+    use crate::tests::generic_serde_test;
+
+    fn verify_state(&eof_pdu: &EofPdu, file_flag: LargeFileFlag) {
+        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(), file_flag);
+        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]
     fn test_basic() {
         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.file_checksum(), 0x01020304);
-        assert_eq!(eof_pdu.file_size(), 12);
-        assert_eq!(eof_pdu.condition_code(), ConditionCode::NoError);
+        assert_eq!(eof_pdu.len_written(), pdu_header.header_len() + 2 + 4 + 4);
+        verify_state(&eof_pdu, LargeFileFlag::Normal);
     }
 
     #[test]
@@ -168,7 +208,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;
@@ -199,11 +239,59 @@ 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_ok() {
-            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();
         assert_eq!(eof_read_back, eof_pdu);
     }
+
+    #[test]
+    fn test_write_to_vec() {
+        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);
+        let mut buf: [u8; 64] = [0; 64];
+        let written = eof_pdu.write_to_bytes(&mut buf).unwrap();
+        let pdu_vec = eof_pdu.to_vec().unwrap();
+        assert_eq!(buf[0..written], pdu_vec);
+    }
+
+    #[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 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");
+        }
+    }
+
+    #[test]
+    fn test_with_large_file_flag() {
+        let pdu_conf = common_pdu_conf(CrcFlag::NoCrc, LargeFileFlag::Large);
+        let pdu_header = PduHeader::new_no_file_data(pdu_conf, 0);
+        let eof_pdu = EofPdu::new_no_error(pdu_header, 0x01020304, 12);
+        verify_state(&eof_pdu, LargeFileFlag::Large);
+        assert_eq!(eof_pdu.len_written(), pdu_header.header_len() + 2 + 8 + 4);
+    }
+
+    #[test]
+    #[cfg(feature = "serde")]
+    fn test_eof_serde() {
+        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);
+        generic_serde_test(eof_pdu);
+    }
 }

src/cfdp/pdu/file_data.rs

@@ -8,6 +8,8 @@ use num_enum::{IntoPrimitive, TryFromPrimitive};
 #[cfg(feature = "serde")]
 use serde::{Deserialize, Serialize};
 
+use super::{CfdpPdu, FileDirectiveType, WritablePduPacket};
+
 #[derive(Debug, Copy, Clone, PartialEq, Eq, TryFromPrimitive, IntoPrimitive)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 #[repr(u8)]
@@ -41,6 +43,14 @@ impl<'seg_meta> SegmentMetadata<'seg_meta> {
         })
     }
 
+    pub fn record_continuation_state(&self) -> RecordContinuationState {
+        self.record_continuation_state
+    }
+
+    pub fn metadata(&self) -> Option<&'seg_meta [u8]> {
+        self.metadata
+    }
+
     pub fn written_len(&self) -> usize {
         // Map empty metadata to 0 and slice to its length.
         1 + self.metadata.map_or(0, |meta| meta.len())
@@ -142,14 +152,11 @@ 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
     }
-    pub fn written_len(&self) -> usize {
-        self.pdu_header.header_len() + self.calc_pdu_datafield_len()
-    }
 
     pub fn offset(&self) -> u64 {
         self.offset
@@ -163,38 +170,7 @@ impl<'seg_meta, 'file_data> FileDataPdu<'seg_meta, 'file_data> {
         self.segment_metadata.as_ref()
     }
 
-    pub fn write_to_bytes(&self, buf: &mut [u8]) -> Result<usize, PduError> {
-        if buf.len() < self.written_len() {
-            return Err(ByteConversionError::ToSliceTooSmall {
-                found: buf.len(),
-                expected: self.written_len(),
-            }
-            .into());
-        }
-        let mut current_idx = self.pdu_header.write_to_bytes(buf)?;
-        if self.segment_metadata.is_some() {
-            current_idx += self
-                .segment_metadata
-                .as_ref()
-                .unwrap()
-                .write_to_bytes(&mut buf[current_idx..])?;
-        }
-        current_idx += write_fss_field(
-            self.pdu_header.common_pdu_conf().file_flag,
-            self.offset,
-            &mut buf[current_idx..],
-        )?;
-        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 {
-            current_idx = add_pdu_crc(buf, current_idx);
-        }
-        Ok(current_idx)
-    }
-
-    pub fn from_bytes<'longest: 'seg_meta + 'file_data>(
-        buf: &'longest [u8],
-    ) -> Result<Self, PduError> {
+    pub fn from_bytes<'buf: 'seg_meta + 'file_data>(buf: &'buf [u8]) -> Result<Self, PduError> {
         let (pdu_header, mut current_idx) = PduHeader::from_bytes(buf)?;
         let full_len_without_crc = pdu_header.verify_length_and_checksum(buf)?;
         let min_expected_len = current_idx + core::mem::size_of::<u32>();
@@ -221,21 +197,64 @@ 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<FileDirectiveType> {
+        None
+    }
+}
+
+impl WritablePduPacket for FileDataPdu<'_, '_> {
+    fn write_to_bytes(&self, buf: &mut [u8]) -> Result<usize, PduError> {
+        if buf.len() < self.len_written() {
+            return Err(ByteConversionError::ToSliceTooSmall {
+                found: buf.len(),
+                expected: self.len_written(),
+            }
+            .into());
+        }
+        let mut current_idx = self.pdu_header.write_to_bytes(buf)?;
+        if self.segment_metadata.is_some() {
+            current_idx += self
+                .segment_metadata
+                .as_ref()
+                .unwrap()
+                .write_to_bytes(&mut buf[current_idx..])?;
+        }
+        current_idx += write_fss_field(
+            self.pdu_header.common_pdu_conf().file_flag,
+            self.offset,
+            &mut buf[current_idx..],
+        )?;
+        buf[current_idx..current_idx + self.file_data.len()].copy_from_slice(self.file_data);
+        current_idx += self.file_data.len();
+        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 {
-    use crate::cfdp::pdu::file_data::{FileDataPdu, RecordContinuationState, SegmentMetadata};
+    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::util::UbfU8;
+    use crate::cfdp::{Direction, SegmentMetadataFlag, SegmentationControl, TransmissionMode};
+    #[cfg(feature = "serde")]
+    use postcard::{from_bytes, to_allocvec};
 
     #[test]
     fn test_basic() {
-        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_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);
@@ -243,18 +262,25 @@ 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
         );
+
+        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 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_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);
@@ -284,12 +310,22 @@ mod tests {
     }
 
     #[test]
-    fn test_deserialization() {
-        let src_id = UbfU8::new(1);
-        let dest_id = UbfU8::new(2);
-        let transaction_seq_num = UbfU8::new(3);
+    fn test_write_to_vec() {
         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_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();
+        let pdu_vec = fd_pdu.to_vec().unwrap();
+        assert_eq!(buf[0..written], pdu_vec);
+    }
+
+    #[test]
+    fn test_deserialization() {
+        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);
@@ -301,13 +337,33 @@ mod tests {
         assert_eq!(fd_pdu_read_back, fd_pdu);
     }
 
+    #[test]
+    fn test_with_crc() {
+        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]
     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,
@@ -323,7 +379,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()
@@ -363,16 +419,13 @@ 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]
     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,
@@ -394,4 +447,39 @@ mod tests {
         let fd_pdu_read_back = fd_pdu_read_back.unwrap();
         assert_eq!(fd_pdu_read_back, fd_pdu);
     }
+
+    #[test]
+    #[cfg(feature = "serde")]
+    fn test_serde_serialization() {
+        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);
+        let output = to_allocvec(&fd_pdu).unwrap();
+        let output_converted_back: FileDataPdu = from_bytes(&output).unwrap();
+        assert_eq!(output_converted_back, fd_pdu);
+    }
+
+    #[test]
+    #[cfg(feature = "serde")]
+    fn test_serde_serialization_with_seg_metadata() {
+        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(
+            common_conf,
+            0,
+            SegmentMetadataFlag::Present,
+            SegmentationControl::WithRecordBoundaryPreservation,
+        );
+        let file_data: [u8; 4] = [1, 2, 3, 4];
+        let seg_metadata: [u8; 4] = [4, 3, 2, 1];
+        let segment_meta =
+            SegmentMetadata::new(RecordContinuationState::StartAndEnd, Some(&seg_metadata))
+                .unwrap();
+        let fd_pdu = FileDataPdu::new_with_seg_metadata(pdu_header, segment_meta, 10, &file_data);
+        let output = to_allocvec(&fd_pdu).unwrap();
+        let output_converted_back: FileDataPdu = from_bytes(&output).unwrap();
+        assert_eq!(output_converted_back, fd_pdu);
+    }
 }

src/cfdp/pdu/finished.rs

@@ -1,13 +1,17 @@
 use crate::cfdp::pdu::{
     add_pdu_crc, generic_length_checks_pdu_deserialization, FileDirectiveType, PduError, PduHeader,
 };
-use crate::cfdp::tlv::{EntityIdTlv, Tlv, TlvType, TlvTypeField};
-use crate::cfdp::{ConditionCode, CrcFlag, PduType, TlvLvError};
+use crate::cfdp::tlv::{
+    EntityIdTlv, FilestoreResponseTlv, GenericTlv, Tlv, TlvType, TlvTypeField, WritableTlv,
+};
+use crate::cfdp::{ConditionCode, CrcFlag, Direction, PduType, TlvLvError};
 use crate::ByteConversionError;
 use num_enum::{IntoPrimitive, TryFromPrimitive};
 #[cfg(feature = "serde")]
 use serde::{Deserialize, Serialize};
 
+use super::{CfdpPdu, WritablePduPacket};
+
 #[derive(Debug, Copy, Clone, PartialEq, Eq, TryFromPrimitive, IntoPrimitive)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 #[repr(u8)]
@@ -30,17 +34,17 @@ pub enum FileStatus {
 ///
 /// For more information, refer to CFDP chapter 5.2.3.
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
-#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
-pub struct FinishedPdu<'fs_responses> {
+pub struct FinishedPduCreator<'fs_responses> {
     pdu_header: PduHeader,
     condition_code: ConditionCode,
     delivery_code: DeliveryCode,
     file_status: FileStatus,
-    fs_responses: Option<&'fs_responses [u8]>,
+    fs_responses:
+        &'fs_responses [FilestoreResponseTlv<'fs_responses, 'fs_responses, 'fs_responses>],
     fault_location: Option<EntityIdTlv>,
 }
 
-impl<'fs_responses> FinishedPdu<'fs_responses> {
+impl<'fs_responses> FinishedPduCreator<'fs_responses> {
     /// Default finished PDU: No error (no fault location field) and no filestore responses.
     pub fn new_default(
         pdu_header: PduHeader,
@@ -52,7 +56,7 @@ impl<'fs_responses> FinishedPdu<'fs_responses> {
             ConditionCode::NoError,
             delivery_code,
             file_status,
-            None,
+            &[],
             None,
         )
     }
@@ -69,7 +73,7 @@ impl<'fs_responses> FinishedPdu<'fs_responses> {
             condition_code,
             delivery_code,
             file_status,
-            None,
+            &[],
             Some(fault_location),
         )
     }
@@ -79,10 +83,16 @@ impl<'fs_responses> FinishedPdu<'fs_responses> {
         condition_code: ConditionCode,
         delivery_code: DeliveryCode,
         file_status: FileStatus,
-        fs_responses: Option<&'fs_responses [u8]>,
+        fs_responses: &'fs_responses [FilestoreResponseTlv<
+            'fs_responses,
+            'fs_responses,
+            'fs_responses,
+        >],
         fault_location: Option<EntityIdTlv>,
     ) -> Self {
         pdu_header.pdu_type = PduType::FileDirective;
+        // Enforce correct direction bit.
+        pdu_header.pdu_conf.direction = Direction::TowardsSender;
         let mut finished_pdu = Self {
             pdu_header,
             condition_code,
@@ -94,13 +104,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 written_len(&self) -> usize {
-        self.pdu_header.header_len() + self.calc_pdu_datafield_len()
-    }
 
     pub fn condition_code(&self) -> ConditionCode {
         self.condition_code
@@ -114,7 +117,8 @@ impl<'fs_responses> FinishedPdu<'fs_responses> {
         self.file_status
     }
 
-    pub fn filestore_responses(&self) -> Option<&'fs_responses [u8]> {
+    // If there are no filestore responses, an empty slice will be returned.
+    pub fn filestore_responses(&self) -> &[FilestoreResponseTlv<'_, '_, '_>] {
         self.fs_responses
     }
 
@@ -123,18 +127,33 @@ impl<'fs_responses> FinishedPdu<'fs_responses> {
     }
 
     fn calc_pdu_datafield_len(&self) -> usize {
-        let mut base_len = 2;
-        if let Some(fs_responses) = self.fs_responses {
-            base_len += fs_responses.len();
+        let mut datafield_len = 2;
+        for fs_response in self.fs_responses {
+            datafield_len += fs_response.len_full();
         }
         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
+    }
+}
+
+impl CfdpPdu for FinishedPduCreator<'_> {
+    fn pdu_header(&self) -> &PduHeader {
+        &self.pdu_header
     }
 
-    pub fn write_to_bytes(&self, buf: &mut [u8]) -> Result<usize, PduError> {
-        let expected_len = self.written_len();
+    fn file_directive_type(&self) -> Option<FileDirectiveType> {
+        Some(FileDirectiveType::FinishedPdu)
+    }
+}
+
+impl WritablePduPacket for FinishedPduCreator<'_> {
+    fn write_to_bytes(&self, buf: &mut [u8]) -> Result<usize, PduError> {
+        let expected_len = self.len_written();
         if buf.len() < expected_len {
             return Err(ByteConversionError::ToSliceTooSmall {
                 found: buf.len(),
@@ -150,21 +169,73 @@ impl<'fs_responses> FinishedPdu<'fs_responses> {
             | ((self.delivery_code as u8) << 2)
             | self.file_status as u8;
         current_idx += 1;
-        if let Some(fs_responses) = self.fs_responses {
-            buf[current_idx..current_idx + fs_responses.len()].copy_from_slice(fs_responses);
-            current_idx += fs_responses.len();
+        for fs_responses in self.fs_responses {
+            current_idx += fs_responses.write_to_bytes(&mut buf[current_idx..])?;
         }
         if let Some(fault_location) = self.fault_location {
-            current_idx += fault_location.write_to_be_bytes(&mut buf[current_idx..])?;
+            current_idx += fault_location.write_to_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)
     }
 
+    fn len_written(&self) -> usize {
+        self.pdu_header.header_len() + self.calc_pdu_datafield_len()
+    }
+}
+
+/// Helper structure to loop through all filestore responses of a read Finished PDU. It should be
+/// noted that iterators in Rust are not fallible, but the TLV creation can fail, for example if
+/// the raw TLV data is invalid for some reason. In that case, the iterator will yield [None]
+/// because there is no way to recover from this.
+///
+/// The user can accumulate the length of all TLVs yielded by the iterator and compare it against
+/// the full length of the options to check whether the iterator was able to parse all TLVs
+/// successfully.
+pub struct FilestoreResponseIterator<'buf> {
+    responses_buf: &'buf [u8],
+    current_idx: usize,
+}
+
+impl<'buf> Iterator for FilestoreResponseIterator<'buf> {
+    type Item = FilestoreResponseTlv<'buf, 'buf, 'buf>;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        if self.current_idx == self.responses_buf.len() {
+            return None;
+        }
+        let tlv = FilestoreResponseTlv::from_bytes(&self.responses_buf[self.current_idx..]);
+        // There are not really fallible iterators so we can't continue here..
+        if tlv.is_err() {
+            return None;
+        }
+        let tlv = tlv.unwrap();
+        self.current_idx += tlv.len_full();
+        Some(tlv)
+    }
+}
+
+#[derive(Debug, Copy, Clone, PartialEq, Eq)]
+#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+pub struct FinishedPduReader<'buf> {
+    pdu_header: PduHeader,
+    condition_code: ConditionCode,
+    delivery_code: DeliveryCode,
+    file_status: FileStatus,
+    fs_responses_raw: &'buf [u8],
+    fault_location: Option<EntityIdTlv>,
+}
+
+impl<'buf> FinishedPduReader<'buf> {
     /// Generates [Self] from a raw bytestream.
-    pub fn from_bytes(buf: &'fs_responses [u8]) -> Result<Self, PduError> {
+    pub fn new(buf: &'buf [u8]) -> Result<Self, PduError> {
+        Self::from_bytes(buf)
+    }
+
+    /// Generates [Self] from a raw bytestream.
+    pub fn from_bytes(buf: &'buf [u8]) -> Result<Self, PduError> {
         let (pdu_header, mut current_idx) = PduHeader::from_bytes(buf)?;
         let full_len_without_crc = pdu_header.verify_length_and_checksum(buf)?;
         let min_expected_len = current_idx + 2;
@@ -188,24 +259,51 @@ impl<'fs_responses> FinishedPdu<'fs_responses> {
         let delivery_code = DeliveryCode::try_from((buf[current_idx] >> 2) & 0b1).unwrap();
         let file_status = FileStatus::try_from(buf[current_idx] & 0b11).unwrap();
         current_idx += 1;
-        let (fs_responses, fault_location) =
+        let (fs_responses_raw, fault_location) =
             Self::parse_tlv_fields(current_idx, full_len_without_crc, buf)?;
         Ok(Self {
             pdu_header,
             condition_code,
             delivery_code,
             file_status,
-            fs_responses,
+            fs_responses_raw,
             fault_location,
         })
     }
 
+    pub fn fs_responses_raw(&self) -> &[u8] {
+        self.fs_responses_raw
+    }
+
+    pub fn fs_responses_iter(&self) -> FilestoreResponseIterator<'_> {
+        FilestoreResponseIterator {
+            responses_buf: self.fs_responses_raw,
+            current_idx: 0,
+        }
+    }
+
+    pub fn condition_code(&self) -> ConditionCode {
+        self.condition_code
+    }
+
+    pub fn delivery_code(&self) -> DeliveryCode {
+        self.delivery_code
+    }
+
+    pub fn file_status(&self) -> FileStatus {
+        self.file_status
+    }
+
+    pub fn fault_location(&self) -> Option<EntityIdTlv> {
+        self.fault_location
+    }
+
     fn parse_tlv_fields(
         mut current_idx: usize,
         full_len_without_crc: usize,
-        buf: &'fs_responses [u8],
-    ) -> Result<(Option<&'fs_responses [u8]>, Option<EntityIdTlv>), PduError> {
-        let mut fs_responses = None;
+        buf: &[u8],
+    ) -> Result<(&[u8], Option<EntityIdTlv>), PduError> {
+        let mut fs_responses: &[u8] = &[];
         let mut fault_location = None;
         let start_of_fs_responses = current_idx;
         // There are leftover filestore response(s) and/or a fault location field.
@@ -216,12 +314,12 @@ impl<'fs_responses> FinishedPdu<'fs_responses> {
                     if tlv_type == TlvType::FilestoreResponse {
                         current_idx += next_tlv.len_full();
                         if current_idx == full_len_without_crc {
-                            fs_responses = Some(&buf[start_of_fs_responses..current_idx]);
+                            fs_responses = &buf[start_of_fs_responses..current_idx];
                         }
                     } else if tlv_type == TlvType::EntityId {
                         // At least one FS response is included.
-                        if current_idx > full_len_without_crc {
-                            fs_responses = Some(&buf[start_of_fs_responses..current_idx]);
+                        if current_idx > start_of_fs_responses {
+                            fs_responses = &buf[start_of_fs_responses..current_idx];
                         }
                         fault_location = Some(EntityIdTlv::from_bytes(&buf[current_idx..])?);
                         current_idx += fault_location.as_ref().unwrap().len_full();
@@ -232,11 +330,19 @@ impl<'fs_responses> FinishedPdu<'fs_responses> {
                             return Err(PduError::FormatError);
                         }
                     } else {
-                        return Err(TlvLvError::InvalidTlvTypeField((tlv_type as u8, None)).into());
+                        return Err(TlvLvError::InvalidTlvTypeField {
+                            found: tlv_type.into(),
+                            expected: Some(TlvType::FilestoreResponse.into()),
+                        }
+                        .into());
                     }
                 }
                 TlvTypeField::Custom(raw) => {
-                    return Err(TlvLvError::InvalidTlvTypeField((raw, None)).into());
+                    return Err(TlvLvError::InvalidTlvTypeField {
+                        found: raw,
+                        expected: None,
+                    }
+                    .into());
                 }
             }
         }
@@ -244,21 +350,55 @@ impl<'fs_responses> FinishedPdu<'fs_responses> {
     }
 }
 
+impl CfdpPdu for FinishedPduReader<'_> {
+    fn pdu_header(&self) -> &PduHeader {
+        &self.pdu_header
+    }
+
+    fn file_directive_type(&self) -> Option<FileDirectiveType> {
+        Some(FileDirectiveType::FinishedPdu)
+    }
+}
+
+impl PartialEq<FinishedPduCreator<'_>> for FinishedPduReader<'_> {
+    fn eq(&self, other: &FinishedPduCreator<'_>) -> bool {
+        self.pdu_header == other.pdu_header
+            && self.condition_code == other.condition_code
+            && self.delivery_code == other.delivery_code
+            && self.file_status == other.file_status
+            && self.fault_location == other.fault_location
+            && self
+                .fs_responses_iter()
+                .zip(other.filestore_responses().iter())
+                .all(|(a, b)| a == *b)
+    }
+}
+
+impl PartialEq<FinishedPduReader<'_>> for FinishedPduCreator<'_> {
+    fn eq(&self, other: &FinishedPduReader<'_>) -> bool {
+        other.eq(self)
+    }
+}
+
 #[cfg(test)]
 mod tests {
-    use crate::cfdp::pdu::finished::{DeliveryCode, FileStatus, FinishedPdu};
-    use crate::cfdp::pdu::tests::{common_pdu_conf, verify_raw_header};
+    use super::*;
+    use crate::cfdp::lv::Lv;
+    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::tlv::FilestoreResponseTlv;
+    use crate::cfdp::{ConditionCode, CrcFlag, Direction, LargeFileFlag, TransmissionMode};
 
     fn generic_finished_pdu(
         crc_flag: CrcFlag,
         fss: LargeFileFlag,
         delivery_code: DeliveryCode,
         file_status: FileStatus,
-    ) -> FinishedPdu<'static> {
+    ) -> FinishedPduCreator<'static> {
         let pdu_header = PduHeader::new_no_file_data(common_pdu_conf(crc_flag, fss), 0);
-        FinishedPdu::new_default(pdu_header, delivery_code, file_status)
+        FinishedPduCreator::new_default(pdu_header, delivery_code, file_status)
     }
 
     #[test]
@@ -270,11 +410,31 @@ mod tests {
             FileStatus::Retained,
         );
         assert_eq!(finished_pdu.condition_code(), ConditionCode::NoError);
+        assert_eq!(
+            finished_pdu.pdu_header().pdu_conf.direction,
+            Direction::TowardsSender
+        );
         assert_eq!(finished_pdu.delivery_code(), DeliveryCode::Complete);
         assert_eq!(finished_pdu.file_status(), FileStatus::Retained);
-        assert_eq!(finished_pdu.filestore_responses(), None);
+        assert_eq!(finished_pdu.filestore_responses(), &[]);
         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) {
@@ -288,8 +448,13 @@ 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, 9);
+        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,
+            Direction::TowardsSender
+        );
         verify_raw_header(finished_pdu.pdu_header(), &buf);
         let mut current_idx = finished_pdu.pdu_header().header_len();
         assert_eq!(buf[current_idx], FileDirectiveType::FinishedPdu as u8);
@@ -321,6 +486,20 @@ mod tests {
         generic_serialization_test_no_error(DeliveryCode::Incomplete, FileStatus::Unreported);
     }
 
+    #[test]
+    fn test_write_to_vec() {
+        let finished_pdu = generic_finished_pdu(
+            CrcFlag::NoCrc,
+            LargeFileFlag::Normal,
+            DeliveryCode::Complete,
+            FileStatus::Retained,
+        );
+        let mut buf: [u8; 64] = [0; 64];
+        let written = finished_pdu.write_to_bytes(&mut buf).unwrap();
+        let pdu_vec = finished_pdu.to_vec().unwrap();
+        assert_eq!(buf[0..written], pdu_vec);
+    }
+
     #[test]
     fn test_deserialization_simple() {
         let finished_pdu = generic_finished_pdu(
@@ -331,9 +510,169 @@ mod tests {
         );
         let mut buf: [u8; 64] = [0; 64];
         finished_pdu.write_to_bytes(&mut buf).unwrap();
-        let read_back = FinishedPdu::from_bytes(&buf);
+        let read_back = FinishedPduReader::from_bytes(&buf);
         assert!(read_back.is_ok());
         let read_back = read_back.unwrap();
         assert_eq!(finished_pdu, read_back);
+        // Use all getter functions here explicitely once.
+        assert_eq!(finished_pdu.pdu_header(), read_back.pdu_header());
+        assert_eq!(finished_pdu.condition_code(), read_back.condition_code());
+        assert_eq!(finished_pdu.fault_location(), read_back.fault_location());
+        assert_eq!(finished_pdu.file_status(), read_back.file_status());
+        assert_eq!(finished_pdu.delivery_code(), read_back.delivery_code());
+    }
+
+    #[test]
+    fn test_serialization_buf_too_small() {
+        let finished_pdu = generic_finished_pdu(
+            CrcFlag::NoCrc,
+            LargeFileFlag::Normal,
+            DeliveryCode::Complete,
+            FileStatus::Retained,
+        );
+        let mut buf: [u8; 8] = [0; 8];
+        let error = finished_pdu.write_to_bytes(&mut buf);
+        assert!(error.is_err());
+        if let PduError::ByteConversion(ByteConversionError::ToSliceTooSmall { found, expected }) =
+            error.unwrap_err()
+        {
+            assert_eq!(found, 8);
+            assert_eq!(expected, 9);
+        } else {
+            panic!("expected to_slice_too_small error");
+        }
+    }
+
+    #[test]
+    fn test_with_crc() {
+        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 = FinishedPduReader::new(&buf).unwrap();
+        assert_eq!(finished_pdu, finished_pdu_from_raw);
+        buf[written - 1] -= 1;
+        let crc: u16 = ((buf[written - 2] as u16) << 8) as u16 | buf[written - 1] as u16;
+        let error = FinishedPduReader::new(&buf).unwrap_err();
+        if let PduError::ChecksumError(e) = error {
+            assert_eq!(e, crc);
+        } else {
+            panic!("expected crc error");
+        }
+    }
+
+    #[test]
+    fn test_with_fault_location() {
+        let pdu_header =
+            PduHeader::new_no_file_data(common_pdu_conf(CrcFlag::NoCrc, LargeFileFlag::Normal), 0);
+        let finished_pdu = FinishedPduCreator::new_with_error(
+            pdu_header,
+            ConditionCode::NakLimitReached,
+            DeliveryCode::Incomplete,
+            FileStatus::DiscardDeliberately,
+            EntityIdTlv::new(TEST_DEST_ID.into()),
+        );
+        let finished_pdu_vec = finished_pdu.to_vec().unwrap();
+        assert_eq!(finished_pdu_vec.len(), 12);
+        assert_eq!(finished_pdu_vec[9], TlvType::EntityId.into());
+        assert_eq!(finished_pdu_vec[10], 1);
+        assert_eq!(finished_pdu_vec[11], TEST_DEST_ID.value());
+        assert_eq!(
+            finished_pdu.fault_location().unwrap().entity_id(),
+            &TEST_DEST_ID.into()
+        );
+    }
+
+    #[test]
+    fn test_deserialization_with_fault_location() {
+        let pdu_header =
+            PduHeader::new_no_file_data(common_pdu_conf(CrcFlag::NoCrc, LargeFileFlag::Normal), 0);
+        let entity_id_tlv = EntityIdTlv::new(TEST_DEST_ID.into());
+        let finished_pdu = FinishedPduCreator::new_with_error(
+            pdu_header,
+            ConditionCode::NakLimitReached,
+            DeliveryCode::Incomplete,
+            FileStatus::DiscardDeliberately,
+            entity_id_tlv,
+        );
+        let finished_pdu_vec = finished_pdu.to_vec().unwrap();
+        let finished_pdu_deserialized = FinishedPduReader::from_bytes(&finished_pdu_vec).unwrap();
+        assert_eq!(finished_pdu, finished_pdu_deserialized);
+    }
+
+    #[test]
+    fn test_deserialization_with_fs_responses() {
+        let entity_id_tlv = EntityIdTlv::new(TEST_DEST_ID.into());
+        let first_name = "first.txt";
+        let first_name_lv = Lv::new_from_str(first_name).unwrap();
+        let fs_response_0 = FilestoreResponseTlv::new_no_filestore_message(
+            crate::cfdp::tlv::FilestoreActionCode::CreateFile,
+            0,
+            first_name_lv,
+            None,
+        )
+        .unwrap();
+        let fs_response_1 = FilestoreResponseTlv::new_no_filestore_message(
+            crate::cfdp::tlv::FilestoreActionCode::DeleteFile,
+            0,
+            first_name_lv,
+            None,
+        )
+        .unwrap();
+        let fs_responses = &[fs_response_0, fs_response_1];
+
+        let pdu_header =
+            PduHeader::new_no_file_data(common_pdu_conf(CrcFlag::NoCrc, LargeFileFlag::Normal), 0);
+        let finished_pdu = FinishedPduCreator::new_generic(
+            pdu_header,
+            ConditionCode::NakLimitReached,
+            DeliveryCode::Incomplete,
+            FileStatus::DiscardDeliberately,
+            fs_responses,
+            Some(entity_id_tlv),
+        );
+        let finished_pdu_vec = finished_pdu.to_vec().unwrap();
+        let finished_pdu_deserialized = FinishedPduReader::from_bytes(&finished_pdu_vec).unwrap();
+        assert_eq!(finished_pdu_deserialized, finished_pdu);
+    }
+
+    #[test]
+    fn test_deserialization_with_fs_responses_and_fault_location() {
+        let first_name = "first.txt";
+        let first_name_lv = Lv::new_from_str(first_name).unwrap();
+        let fs_response_0 = FilestoreResponseTlv::new_no_filestore_message(
+            crate::cfdp::tlv::FilestoreActionCode::CreateFile,
+            0,
+            first_name_lv,
+            None,
+        )
+        .unwrap();
+        let fs_response_1 = FilestoreResponseTlv::new_no_filestore_message(
+            crate::cfdp::tlv::FilestoreActionCode::DeleteFile,
+            0,
+            first_name_lv,
+            None,
+        )
+        .unwrap();
+        let fs_responses = &[fs_response_0, fs_response_1];
+
+        let pdu_header =
+            PduHeader::new_no_file_data(common_pdu_conf(CrcFlag::NoCrc, LargeFileFlag::Normal), 0);
+        let finished_pdu = FinishedPduCreator::new_generic(
+            pdu_header,
+            ConditionCode::NakLimitReached,
+            DeliveryCode::Incomplete,
+            FileStatus::DiscardDeliberately,
+            fs_responses,
+            None,
+        );
+        let finished_pdu_vec = finished_pdu.to_vec().unwrap();
+        let finished_pdu_deserialized = FinishedPduReader::from_bytes(&finished_pdu_vec).unwrap();
+        assert_eq!(finished_pdu_deserialized, finished_pdu);
     }
 }

src/cfdp/pdu/metadata.rs

@@ -3,14 +3,16 @@ use crate::cfdp::pdu::{
     add_pdu_crc, generic_length_checks_pdu_deserialization, read_fss_field, write_fss_field,
     FileDirectiveType, PduError, PduHeader,
 };
-use crate::cfdp::tlv::Tlv;
-use crate::cfdp::{ChecksumType, CrcFlag, LargeFileFlag, PduType};
+use crate::cfdp::tlv::{Tlv, WritableTlv};
+use crate::cfdp::{ChecksumType, CrcFlag, Direction, LargeFileFlag, PduType};
 use crate::ByteConversionError;
 #[cfg(feature = "alloc")]
 use alloc::vec::Vec;
 #[cfg(feature = "serde")]
 use serde::{Deserialize, Serialize};
 
+use super::{CfdpPdu, WritablePduPacket};
+
 #[derive(Default, Debug, Copy, Clone, PartialEq, Eq)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 pub struct MetadataGenericParams {
@@ -48,6 +50,160 @@ pub fn build_metadata_opts_from_vec(
     build_metadata_opts_from_slice(buf, tlvs.as_slice())
 }
 
+/// Metadata PDU creator abstraction.
+///
+/// This abstraction exposes a specialized API for creating metadata PDUs as specified in
+/// CFDP chapter 5.2.5.
+#[derive(Debug, Copy, Clone, PartialEq, Eq)]
+pub struct MetadataPduCreator<'src_name, 'dest_name, 'opts> {
+    pdu_header: PduHeader,
+    metadata_params: MetadataGenericParams,
+    src_file_name: Lv<'src_name>,
+    dest_file_name: Lv<'dest_name>,
+    options: &'opts [Tlv<'opts>],
+}
+
+impl<'src_name, 'dest_name, 'opts> MetadataPduCreator<'src_name, 'dest_name, 'opts> {
+    pub fn new_no_opts(
+        pdu_header: PduHeader,
+        metadata_params: MetadataGenericParams,
+        src_file_name: Lv<'src_name>,
+        dest_file_name: Lv<'dest_name>,
+    ) -> Self {
+        Self::new(
+            pdu_header,
+            metadata_params,
+            src_file_name,
+            dest_file_name,
+            &[],
+        )
+    }
+
+    pub fn new_with_opts(
+        pdu_header: PduHeader,
+        metadata_params: MetadataGenericParams,
+        src_file_name: Lv<'src_name>,
+        dest_file_name: Lv<'dest_name>,
+        options: &'opts [Tlv<'opts>],
+    ) -> Self {
+        Self::new(
+            pdu_header,
+            metadata_params,
+            src_file_name,
+            dest_file_name,
+            options,
+        )
+    }
+
+    pub fn new(
+        mut pdu_header: PduHeader,
+        metadata_params: MetadataGenericParams,
+        src_file_name: Lv<'src_name>,
+        dest_file_name: Lv<'dest_name>,
+        options: &'opts [Tlv<'opts>],
+    ) -> Self {
+        pdu_header.pdu_type = PduType::FileDirective;
+        pdu_header.pdu_conf.direction = Direction::TowardsReceiver;
+        let mut pdu = Self {
+            pdu_header,
+            metadata_params,
+            src_file_name,
+            dest_file_name,
+            options,
+        };
+        pdu.pdu_header.pdu_datafield_len = pdu.calc_pdu_datafield_len() as u16;
+        pdu
+    }
+
+    pub fn metadata_params(&self) -> &MetadataGenericParams {
+        &self.metadata_params
+    }
+
+    pub fn src_file_name(&self) -> Lv<'src_name> {
+        self.src_file_name
+    }
+
+    pub fn dest_file_name(&self) -> Lv<'dest_name> {
+        self.dest_file_name
+    }
+
+    pub fn options(&self) -> &'opts [Tlv<'opts>] {
+        self.options
+    }
+
+    fn calc_pdu_datafield_len(&self) -> usize {
+        // One directve type octet and one byte of the directive parameter field.
+        let mut len = 2;
+        if self.pdu_header.common_pdu_conf().file_flag == LargeFileFlag::Large {
+            len += 8;
+        } else {
+            len += 4;
+        }
+        len += self.src_file_name.len_full();
+        len += self.dest_file_name.len_full();
+        for tlv in self.options() {
+            len += tlv.len_full()
+        }
+        if self.crc_flag() == CrcFlag::WithCrc {
+            len += 2;
+        }
+        len
+    }
+}
+
+impl CfdpPdu for MetadataPduCreator<'_, '_, '_> {
+    fn pdu_header(&self) -> &PduHeader {
+        &self.pdu_header
+    }
+
+    fn file_directive_type(&self) -> Option<FileDirectiveType> {
+        Some(FileDirectiveType::MetadataPdu)
+    }
+}
+
+impl WritablePduPacket for MetadataPduCreator<'_, '_, '_> {
+    fn write_to_bytes(&self, buf: &mut [u8]) -> Result<usize, PduError> {
+        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::MetadataPdu as u8;
+        current_idx += 1;
+        buf[current_idx] = ((self.metadata_params.closure_requested as u8) << 7)
+            | (self.metadata_params.checksum_type as u8);
+        current_idx += 1;
+        current_idx += write_fss_field(
+            self.pdu_header.common_pdu_conf().file_flag,
+            self.metadata_params.file_size,
+            &mut buf[current_idx..],
+        )?;
+        current_idx += self
+            .src_file_name
+            .write_to_be_bytes(&mut buf[current_idx..])?;
+        current_idx += self
+            .dest_file_name
+            .write_to_be_bytes(&mut buf[current_idx..])?;
+        for opt in self.options() {
+            opt.write_to_bytes(&mut buf[current_idx..current_idx + opt.len_full()])?;
+            current_idx += opt.len_full();
+        }
+        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()
+    }
+}
+
 /// Helper structure to loop through all options of a metadata PDU. It should be noted that
 /// iterators in Rust are not fallible, but the TLV creation can fail, for example if the raw TLV
 /// data is invalid for some reason. In that case, the iterator will yield [None] because there
@@ -79,165 +235,28 @@ impl<'opts> Iterator for OptionsIter<'opts> {
     }
 }
 
-/// Metadata PDU abstraction.
+/// Metadata PDU reader abstraction.
 ///
-/// For more information, refer to CFDP chapter 5.2.5.
-#[derive(Debug, Copy, Clone, PartialEq, Eq)]
+/// This abstraction exposes a specialized API for reading a metadata PDU with minimal copying
+/// involved.
+#[derive(Debug)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
-pub struct MetadataPdu<'src_name, 'dest_name, 'opts> {
+pub struct MetadataPduReader<'buf> {
     pdu_header: PduHeader,
     metadata_params: MetadataGenericParams,
     #[cfg_attr(feature = "serde", serde(borrow))]
-    src_file_name: Lv<'src_name>,
+    src_file_name: Lv<'buf>,
     #[cfg_attr(feature = "serde", serde(borrow))]
-    dest_file_name: Lv<'dest_name>,
-    options: Option<&'opts [u8]>,
+    dest_file_name: Lv<'buf>,
+    options: &'buf [u8],
 }
 
-impl<'src_name, 'dest_name, 'opts> MetadataPdu<'src_name, 'dest_name, 'opts> {
-    pub fn new_no_opts(
-        pdu_header: PduHeader,
-        metadata_params: MetadataGenericParams,
-        src_file_name: Lv<'src_name>,
-        dest_file_name: Lv<'dest_name>,
-    ) -> Self {
-        Self::new(
-            pdu_header,
-            metadata_params,
-            src_file_name,
-            dest_file_name,
-            None,
-        )
+impl<'raw> MetadataPduReader<'raw> {
+    pub fn new(buf: &'raw [u8]) -> Result<Self, PduError> {
+        Self::from_bytes(buf)
     }
 
-    pub fn new_with_opts(
-        pdu_header: PduHeader,
-        metadata_params: MetadataGenericParams,
-        src_file_name: Lv<'src_name>,
-        dest_file_name: Lv<'dest_name>,
-        options: &'opts [u8],
-    ) -> Self {
-        Self::new(
-            pdu_header,
-            metadata_params,
-            src_file_name,
-            dest_file_name,
-            Some(options),
-        )
-    }
-
-    pub fn new(
-        mut pdu_header: PduHeader,
-        metadata_params: MetadataGenericParams,
-        src_file_name: Lv<'src_name>,
-        dest_file_name: Lv<'dest_name>,
-        options: Option<&'opts [u8]>,
-    ) -> Self {
-        pdu_header.pdu_type = PduType::FileDirective;
-        let mut pdu = Self {
-            pdu_header,
-            metadata_params,
-            src_file_name,
-            dest_file_name,
-            options,
-        };
-        pdu.pdu_header.pdu_datafield_len = pdu.calc_pdu_datafield_len() as u16;
-        pdu
-    }
-
-    pub fn metadata_params(&self) -> &MetadataGenericParams {
-        &self.metadata_params
-    }
-
-    pub fn src_file_name(&self) -> Lv<'src_name> {
-        self.src_file_name
-    }
-
-    pub fn dest_file_name(&self) -> Lv<'dest_name> {
-        self.dest_file_name
-    }
-
-    pub fn options(&self) -> Option<&'opts [u8]> {
-        self.options
-    }
-
-    /// Yield an iterator which can be used to loop through all options. Returns [None] if the
-    /// options field is empty.
-    pub fn options_iter(&self) -> Option<OptionsIter<'opts>> {
-        self.options?;
-        Some(OptionsIter {
-            opt_buf: self.options.unwrap(),
-            current_idx: 0,
-        })
-    }
-
-    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;
-        if self.pdu_header.common_pdu_conf().file_flag == LargeFileFlag::Large {
-            len += 8;
-        } else {
-            len += 4;
-        }
-        len += self.src_file_name.len_full();
-        len += self.dest_file_name.len_full();
-        if let Some(opts) = self.options {
-            len += opts.len();
-        }
-        if self.pdu_header.pdu_conf.crc_flag == CrcFlag::WithCrc {
-            len += 2;
-        }
-        len
-    }
-
-    pub fn pdu_header(&self) -> &PduHeader {
-        &self.pdu_header
-    }
-
-    pub fn write_to_bytes(&self, buf: &mut [u8]) -> Result<usize, PduError> {
-        let expected_len = self.written_len();
-        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::MetadataPdu as u8;
-        current_idx += 1;
-        buf[current_idx] = ((self.metadata_params.closure_requested as u8) << 7)
-            | (self.metadata_params.checksum_type as u8);
-        current_idx += 1;
-        current_idx += write_fss_field(
-            self.pdu_header.common_pdu_conf().file_flag,
-            self.metadata_params.file_size,
-            &mut buf[current_idx..],
-        )?;
-        current_idx += self
-            .src_file_name
-            .write_to_be_bytes(&mut buf[current_idx..])?;
-        current_idx += self
-            .dest_file_name
-            .write_to_be_bytes(&mut buf[current_idx..])?;
-        if let Some(opts) = self.options {
-            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 {
-            current_idx = add_pdu_crc(buf, current_idx);
-        }
-        Ok(current_idx)
-    }
-
-    pub fn from_bytes<'longest: 'src_name + 'dest_name + 'opts>(
-        buf: &'longest [u8],
-    ) -> Result<Self, PduError> {
+    pub fn from_bytes(buf: &'raw [u8]) -> Result<Self, PduError> {
         let (pdu_header, mut current_idx) = PduHeader::from_bytes(buf)?;
         let full_len_without_crc = pdu_header.verify_length_and_checksum(buf)?;
         let is_large_file = pdu_header.pdu_conf.file_flag == LargeFileFlag::Large;
@@ -274,46 +293,83 @@ impl<'src_name, 'dest_name, 'opts> MetadataPdu<'src_name, 'dest_name, 'opts> {
         let dest_file_name = Lv::from_bytes(&buf[current_idx..])?;
         current_idx += dest_file_name.len_full();
         // All left-over bytes are options.
-        let mut options = None;
-        if current_idx < full_len_without_crc {
-            options = Some(&buf[current_idx..full_len_without_crc]);
-        }
         Ok(Self {
             pdu_header,
             metadata_params,
             src_file_name,
             dest_file_name,
-            options,
+            options: &buf[current_idx..full_len_without_crc],
         })
     }
+
+    /// Yield an iterator which can be used to loop through all options. Returns [None] if the
+    /// options field is empty.
+    pub fn options_iter(&self) -> Option<OptionsIter<'_>> {
+        Some(OptionsIter {
+            opt_buf: self.options,
+            current_idx: 0,
+        })
+    }
+
+    pub fn options(&self) -> &'raw [u8] {
+        self.options
+    }
+
+    pub fn metadata_params(&self) -> &MetadataGenericParams {
+        &self.metadata_params
+    }
+
+    pub fn src_file_name(&self) -> Lv {
+        self.src_file_name
+    }
+
+    pub fn dest_file_name(&self) -> Lv {
+        self.dest_file_name
+    }
+}
+
+impl CfdpPdu for MetadataPduReader<'_> {
+    fn pdu_header(&self) -> &PduHeader {
+        &self.pdu_header
+    }
+
+    fn file_directive_type(&self) -> Option<FileDirectiveType> {
+        Some(FileDirectiveType::MetadataPdu)
+    }
 }
 
 #[cfg(test)]
 pub mod tests {
+    use alloc::string::ToString;
+
     use crate::cfdp::lv::Lv;
     use crate::cfdp::pdu::metadata::{
         build_metadata_opts_from_slice, build_metadata_opts_from_vec, MetadataGenericParams,
-        MetadataPdu,
+        MetadataPduCreator, MetadataPduReader,
     };
-    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::{CfdpPdu, PduError, 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;
 
-    const SRC_FILENAME: &'static str = "hello-world.txt";
-    const DEST_FILENAME: &'static str = "hello-world2.txt";
+    const SRC_FILENAME: &str = "hello-world.txt";
+    const DEST_FILENAME: &str = "hello-world2.txt";
 
     fn generic_metadata_pdu<'opts>(
         crc_flag: CrcFlag,
         fss: LargeFileFlag,
-        opts: Option<&'opts [u8]>,
+        opts: &'opts [Tlv],
     ) -> (
         Lv<'static>,
         Lv<'static>,
-        MetadataPdu<'static, 'static, 'opts>,
+        MetadataPduCreator<'static, 'static, 'opts>,
     ) {
         let pdu_header = PduHeader::new_no_file_data(common_pdu_conf(crc_flag, fss), 0);
         let metadata_params = MetadataGenericParams::new(false, ChecksumType::Crc32, 0x1010);
@@ -323,7 +379,7 @@ pub mod tests {
         (
             src_filename,
             dest_filename,
-            MetadataPdu::new(
+            MetadataPduCreator::new(
                 pdu_header,
                 metadata_params,
                 src_filename,
@@ -336,9 +392,9 @@ pub mod tests {
     #[test]
     fn test_basic() {
         let (src_filename, dest_filename, metadata_pdu) =
-            generic_metadata_pdu(CrcFlag::NoCrc, LargeFileFlag::Normal, None);
+            generic_metadata_pdu(CrcFlag::NoCrc, LargeFileFlag::Normal, &[]);
         assert_eq!(
-            metadata_pdu.written_len(),
+            metadata_pdu.len_written(),
             metadata_pdu.pdu_header().header_len()
                 + 1
                 + 1
@@ -348,13 +404,28 @@ 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!(metadata_pdu.options().is_empty());
+        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]
     fn test_serialization() {
         let (src_filename, dest_filename, metadata_pdu) =
-            generic_metadata_pdu(CrcFlag::NoCrc, LargeFileFlag::Normal, None);
+            generic_metadata_pdu(CrcFlag::NoCrc, LargeFileFlag::Normal, &[]);
         let mut buf: [u8; 64] = [0; 64];
         let res = metadata_pdu.write_to_bytes(&mut buf);
         assert!(res.is_ok());
@@ -386,22 +457,41 @@ pub mod tests {
         assert_eq!(current_idx, written);
     }
 
+    #[test]
+    fn test_write_to_vec() {
+        let (_, _, metadata_pdu) = generic_metadata_pdu(CrcFlag::NoCrc, LargeFileFlag::Normal, &[]);
+        let mut buf: [u8; 64] = [0; 64];
+        let pdu_vec = metadata_pdu.to_vec().unwrap();
+        let written = metadata_pdu.write_to_bytes(&mut buf).unwrap();
+        assert_eq!(buf[0..written], pdu_vec);
+    }
+
+    fn compare_read_pdu_to_written_pdu(written: &MetadataPduCreator, read: &MetadataPduReader) {
+        assert_eq!(written.metadata_params(), read.metadata_params());
+        assert_eq!(written.src_file_name(), read.src_file_name());
+        assert_eq!(written.dest_file_name(), read.dest_file_name());
+        let opts = written.options();
+        for (tlv_written, tlv_read) in opts.iter().zip(read.options_iter().unwrap()) {
+            assert_eq!(tlv_written, &tlv_read);
+        }
+    }
+
     #[test]
     fn test_deserialization() {
-        let (_, _, metadata_pdu) =
-            generic_metadata_pdu(CrcFlag::NoCrc, LargeFileFlag::Normal, None);
+        let (_, _, metadata_pdu) = generic_metadata_pdu(CrcFlag::NoCrc, LargeFileFlag::Normal, &[]);
         let mut buf: [u8; 64] = [0; 64];
         metadata_pdu.write_to_bytes(&mut buf).unwrap();
-        let pdu_read_back = MetadataPdu::from_bytes(&buf);
+        let pdu_read_back = MetadataPduReader::from_bytes(&buf);
         assert!(pdu_read_back.is_ok());
         let pdu_read_back = pdu_read_back.unwrap();
-        assert_eq!(pdu_read_back, metadata_pdu);
+        compare_read_pdu_to_written_pdu(&metadata_pdu, &pdu_read_back);
     }
 
     #[test]
     fn test_with_crc_flag() {
         let (src_filename, dest_filename, metadata_pdu) =
-            generic_metadata_pdu(CrcFlag::WithCrc, LargeFileFlag::Normal, None);
+            generic_metadata_pdu(CrcFlag::WithCrc, LargeFileFlag::Normal, &[]);
+        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());
@@ -416,14 +506,15 @@ pub mod tests {
                 + dest_filename.len_full()
                 + 2
         );
-        let pdu_read_back = MetadataPdu::from_bytes(&buf).unwrap();
-        assert_eq!(pdu_read_back, metadata_pdu);
+        assert_eq!(written, metadata_pdu.len_written());
+        let pdu_read_back = MetadataPduReader::new(&buf).unwrap();
+        compare_read_pdu_to_written_pdu(&metadata_pdu, &pdu_read_back);
     }
 
     #[test]
     fn test_with_large_file_flag() {
         let (src_filename, dest_filename, metadata_pdu) =
-            generic_metadata_pdu(CrcFlag::NoCrc, LargeFileFlag::Large, None);
+            generic_metadata_pdu(CrcFlag::NoCrc, LargeFileFlag::Large, &[]);
         let mut buf: [u8; 64] = [0; 64];
         let write_res = metadata_pdu.write_to_bytes(&mut buf);
         assert!(write_res.is_ok());
@@ -437,8 +528,8 @@ pub mod tests {
                 + src_filename.len_full()
                 + dest_filename.len_full()
         );
-        let pdu_read_back = MetadataPdu::from_bytes(&buf).unwrap();
-        assert_eq!(pdu_read_back, metadata_pdu);
+        let pdu_read_back = MetadataPduReader::new(&buf).unwrap();
+        compare_read_pdu_to_written_pdu(&metadata_pdu, &pdu_read_back);
     }
 
     #[test]
@@ -481,13 +572,9 @@ pub mod tests {
         let msg_to_user: [u8; 4] = [1, 2, 3, 4];
         let tlv2 = Tlv::new(TlvType::MsgToUser, &msg_to_user).unwrap();
         let tlv_vec = vec![tlv1, tlv2];
-        let mut opts_buf: [u8; 32] = [0; 32];
-        let opts_len = build_metadata_opts_from_vec(&mut opts_buf, &tlv_vec).unwrap();
-        let (src_filename, dest_filename, metadata_pdu) = generic_metadata_pdu(
-            CrcFlag::NoCrc,
-            LargeFileFlag::Normal,
-            Some(&opts_buf[..opts_len]),
-        );
+        let opts_len = tlv1.len_full() + tlv2.len_full();
+        let (src_filename, dest_filename, metadata_pdu) =
+            generic_metadata_pdu(CrcFlag::NoCrc, LargeFileFlag::Normal, &tlv_vec);
         let mut buf: [u8; 128] = [0; 128];
         let write_res = metadata_pdu.write_to_bytes(&mut buf);
         assert!(write_res.is_ok());
@@ -502,8 +589,8 @@ pub mod tests {
                 + dest_filename.len_full()
                 + opts_len
         );
-        let pdu_read_back = MetadataPdu::from_bytes(&buf).unwrap();
-        assert_eq!(pdu_read_back, metadata_pdu);
+        let pdu_read_back = MetadataPduReader::from_bytes(&buf).unwrap();
+        compare_read_pdu_to_written_pdu(&metadata_pdu, &pdu_read_back);
         let opts_iter = pdu_read_back.options_iter();
         assert!(opts_iter.is_some());
         let opts_iter = opts_iter.unwrap();
@@ -512,9 +599,48 @@ pub mod tests {
             assert_eq!(tlv_vec[idx], opt);
             accumulated_len += opt.len_full();
         }
-        assert_eq!(accumulated_len, pdu_read_back.options().unwrap().len());
+        assert_eq!(accumulated_len, pdu_read_back.options().len());
     }
 
+    #[test]
+    fn test_invalid_directive_code() {
+        let (_, _, metadata_pdu) = generic_metadata_pdu(CrcFlag::NoCrc, LargeFileFlag::Large, &[]);
+        let mut metadata_vec = metadata_pdu.to_vec().unwrap();
+        metadata_vec[7] = 0xff;
+        let metadata_error = MetadataPduReader::from_bytes(&metadata_vec);
+        assert!(metadata_error.is_err());
+        let error = metadata_error.unwrap_err();
+        if let PduError::InvalidDirectiveType { found, expected } = error {
+            assert_eq!(found, 0xff);
+            assert_eq!(expected, Some(FileDirectiveType::MetadataPdu));
+            assert_eq!(
+                error.to_string(),
+                "invalid directive type value 255, expected Some(MetadataPdu)"
+            );
+        } else {
+            panic!("Expected InvalidDirectiveType error, got {:?}", error);
+        }
+    }
+
+    #[test]
+    fn test_wrong_directive_code() {
+        let (_, _, metadata_pdu) = generic_metadata_pdu(CrcFlag::NoCrc, LargeFileFlag::Large, &[]);
+        let mut metadata_vec = metadata_pdu.to_vec().unwrap();
+        metadata_vec[7] = FileDirectiveType::EofPdu as u8;
+        let metadata_error = MetadataPduReader::from_bytes(&metadata_vec);
+        assert!(metadata_error.is_err());
+        let error = metadata_error.unwrap_err();
+        if let PduError::WrongDirectiveType { found, expected } = error {
+            assert_eq!(found, FileDirectiveType::EofPdu);
+            assert_eq!(expected, FileDirectiveType::MetadataPdu);
+            assert_eq!(
+                error.to_string(),
+                "found directive type EofPdu, expected MetadataPdu"
+            );
+        } else {
+            panic!("Expected InvalidDirectiveType error, got {:?}", error);
+        }
+    }
     #[test]
     fn test_corrects_pdu_header() {
         let pdu_header = PduHeader::new_for_file_data(
@@ -527,8 +653,12 @@ pub mod tests {
         let src_filename = Lv::new_from_str(SRC_FILENAME).expect("Generating string LV failed");
         let dest_filename =
             Lv::new_from_str(DEST_FILENAME).expect("Generating destination LV failed");
-        let metadata_pdu =
-            MetadataPdu::new_no_opts(pdu_header, metadata_params, src_filename, dest_filename);
+        let metadata_pdu = MetadataPduCreator::new_no_opts(
+            pdu_header,
+            metadata_params,
+            src_filename,
+            dest_filename,
+        );
         assert_eq!(metadata_pdu.pdu_header().pdu_type(), PduType::FileDirective);
     }
 }

src/cfdp/pdu/mod.rs

@@ -3,14 +3,18 @@ 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;
 
+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))]
@@ -28,7 +32,7 @@ pub enum FileDirectiveType {
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 pub enum PduError {
-    ByteConversionError(ByteConversionError),
+    ByteConversion(ByteConversionError),
     /// Found version ID invalid, not equal to [CFDP_VERSION_2].
     CfdpVersionMissmatch(u8),
     /// Invalid length for the entity ID detected. Only the values 1, 2, 4 and 8 are supported.
@@ -39,15 +43,19 @@ 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<FileDirectiveType>,
     },
+    InvalidStartOrEndOfScopeValue,
     /// Invalid condition code. Contains the raw detected value.
     InvalidConditionCode(u8),
     /// Invalid checksum type which is not part of the checksums listed in the
@@ -68,9 +76,12 @@ 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::InvalidStartOrEndOfScopeValue => {
+                write!(f, "invalid start or end of scope for NAK PDU")
+            }
             PduError::InvalidTransactionSeqNumLen(raw_id) => {
                 write!(
                     f,
@@ -92,7 +103,7 @@ impl Display for PduError {
                     "missmatch of PDU source length {src_id_len} and destination length {dest_id_len}"
                 )
             }
-            PduError::ByteConversionError(e) => {
+            PduError::ByteConversion(e) => {
                 write!(f, "{}", e)
             }
             PduError::FileSizeTooLarge(value) => {
@@ -130,7 +141,7 @@ impl Display for PduError {
 impl Error for PduError {
     fn source(&self) -> Option<&(dyn Error + 'static)> {
         match self {
-            PduError::ByteConversionError(e) => Some(e),
+            PduError::ByteConversion(e) => Some(e),
             PduError::TlvLvError(e) => Some(e),
             _ => None,
         }
@@ -139,7 +150,7 @@ impl Error for PduError {
 
 impl From<ByteConversionError> for PduError {
     fn from(value: ByteConversionError) -> Self {
-        Self::ByteConversionError(value)
+        Self::ByteConversion(value)
     }
 }
 
@@ -149,8 +160,65 @@ 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)
+    }
+}
+
+/// 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<FileDirectiveType>;
+}
+
 /// 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,
@@ -271,6 +339,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.
@@ -442,7 +522,7 @@ impl PduHeader {
     /// function.
     pub fn from_bytes(buf: &[u8]) -> Result<(Self, usize), PduError> {
         if buf.len() < FIXED_HEADER_LEN {
-            return Err(PduError::ByteConversionError(
+            return Err(PduError::ByteConversion(
                 ByteConversionError::FromSliceTooSmall {
                     found: buf.len(),
                     expected: FIXED_HEADER_LEN,
@@ -605,23 +685,26 @@ pub(crate) fn add_pdu_crc(buf: &mut [u8], mut current_idx: usize) -> usize {
 
 #[cfg(test)]
 mod tests {
+    use alloc::string::ToString;
+
     use crate::cfdp::pdu::{CommonPduConfig, PduError, PduHeader, FIXED_HEADER_LEN};
     use crate::cfdp::{
         CrcFlag, Direction, LargeFileFlag, PduType, SegmentMetadataFlag, SegmentationControl,
         TransmissionMode, CFDP_VERSION_2,
     };
     use crate::util::{
-        UbfU8, UnsignedByteField, UnsignedByteFieldU16, UnsignedByteFieldU8, UnsignedEnum,
+        UbfU16, UbfU8, UnsignedByteField, UnsignedByteFieldU16, UnsignedByteFieldU8, UnsignedEnum,
     };
     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;
@@ -632,7 +715,6 @@ mod tests {
         assert_eq!((buf[0] >> 5) & 0b111, CFDP_VERSION_2);
         // File directive
         assert_eq!((buf[0] >> 4) & 1, pdu_conf.pdu_type as u8);
-        // Towards receiver
         assert_eq!((buf[0] >> 3) & 1, pdu_conf.pdu_conf.direction as u8);
         // Acknowledged
         assert_eq!((buf[0] >> 2) & 1, pdu_conf.pdu_conf.trans_mode as u8);
@@ -678,7 +760,7 @@ mod tests {
                             .try_into()
                             .unwrap()
                     ),
-                    ubf.value() as u64
+                    ubf.value()
                 ),
                 _ => panic!("invalid entity ID length"),
             }
@@ -713,6 +795,17 @@ 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();
@@ -758,6 +851,7 @@ mod tests {
         // 4 byte fixed header plus three bytes src, dest ID and transaction ID
         assert_eq!(res.unwrap(), 7);
         verify_raw_header(&pdu_header, &buf);
+        assert_eq!(pdu_header.pdu_datafield_len(), 5);
     }
 
     #[test]
@@ -851,6 +945,10 @@ mod tests {
         let error = res.unwrap_err();
         if let PduError::CfdpVersionMissmatch(raw_version) = error {
             assert_eq!(raw_version, CFDP_VERSION_2 + 1);
+            assert_eq!(
+                error.to_string(),
+                "cfdp version missmatch, found 2, expected 1"
+            );
         } else {
             panic!("invalid exception: {}", error);
         }
@@ -862,7 +960,7 @@ mod tests {
         let res = PduHeader::from_bytes(&buf);
         assert!(res.is_err());
         let error = res.unwrap_err();
-        if let PduError::ByteConversionError(ByteConversionError::FromSliceTooSmall {
+        if let PduError::ByteConversion(ByteConversionError::FromSliceTooSmall {
             found,
             expected,
         }) = error
@@ -888,13 +986,17 @@ mod tests {
         let header = PduHeader::from_bytes(&buf[0..6]);
         assert!(header.is_err());
         let error = header.unwrap_err();
-        if let PduError::ByteConversionError(ByteConversionError::FromSliceTooSmall {
+        if let PduError::ByteConversion(ByteConversionError::FromSliceTooSmall {
             found,
             expected,
         }) = error
         {
             assert_eq!(found, 6);
             assert_eq!(expected, 7);
+            assert_eq!(
+                error.to_string(),
+                "source slice with size 6 too small, expected at least 7 bytes"
+            );
         }
     }
 
@@ -922,6 +1024,10 @@ mod tests {
         let error = pdu_conf_res.unwrap_err();
         if let PduError::InvalidEntityLen(len) = error {
             assert_eq!(len, 3);
+            assert_eq!(
+                error.to_string(),
+                "invalid PDU entity ID length 3, only [1, 2, 4, 8] are allowed"
+            );
         } else {
             panic!("Invalid exception: {}", error)
         }
@@ -942,6 +1048,10 @@ mod tests {
         {
             assert_eq!(src_id_len, 1);
             assert_eq!(dest_id_len, 2);
+            assert_eq!(
+                error.to_string(),
+                "missmatch of PDU source length 1 and destination length 2"
+            );
         }
     }
 
@@ -992,4 +1102,20 @@ mod tests {
             panic!("invalid exception {:?}", error)
         }
     }
+
+    #[test]
+    fn test_pdu_error_clonable_and_comparable() {
+        let pdu_error = PduError::InvalidEntityLen(0);
+        let pdu_error_2 = pdu_error;
+        assert_eq!(pdu_error, pdu_error_2);
+    }
+
+    #[test]
+    fn test_pdu_config_clonable_and_comparable() {
+        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 = common_pdu_cfg_0;
+        assert_eq!(common_pdu_cfg_0, common_pdu_cfg_1);
+    }
 }

src/cfdp/pdu/nak.rs

@@ -0,0 +1,803 @@
+use crate::{
+    cfdp::{CrcFlag, Direction, LargeFileFlag},
+    ByteConversionError,
+};
+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)]),
+    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
+/// format according to CFDP chapter 5.2.6.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct NakPduCreator<'seg_reqs> {
+    pdu_header: PduHeader,
+    start_of_scope: u64,
+    end_of_scope: u64,
+    segment_requests: Option<SegmentRequests<'seg_reqs>>,
+}
+
+impl<'seg_reqs> NakPduCreator<'seg_reqs> {
+    /// 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<NakPduCreator<'seg_reqs>, 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<NakPduCreator, PduError> {
+        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(),
+            passed_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<NakPduCreator, PduError> {
+        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,
+            passed_segment_requests,
+        )
+    }
+
+    fn new_generic(
+        mut pdu_header: PduHeader,
+        start_of_scope: u64,
+        end_of_scope: u64,
+        segment_requests: Option<SegmentRequests<'seg_reqs>>,
+    ) -> Result<NakPduCreator, PduError> {
+        // Force correct direction flag.
+        pdu_header.pdu_conf.direction = Direction::TowardsSender;
+        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);
+                    }
+                    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<FileDirectiveType> {
+        Some(FileDirectiveType::NakPdu)
+    }
+}
+
+impl WritablePduPacket for NakPduCreator<'_> {
+    fn write_to_bytes(&self, buf: &mut [u8]) -> Result<usize, PduError> {
+        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;
+
+        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.
+                    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());
+                        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;
+                    }
+                }
+            }
+        } else {
+            write_start_end_of_scope_normal();
+        }
+
+        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.
+#[derive(Debug)]
+pub struct SegmentRequestIter<'a, T> {
+    seq_req_raw: &'a [u8],
+    current_idx: usize,
+    phantom: core::marker::PhantomData<T>,
+}
+
+pub trait SegReqFromBytes {
+    fn from_bytes(bytes: &[u8]) -> Self;
+}
+
+impl SegReqFromBytes for u32 {
+    fn from_bytes(bytes: &[u8]) -> u32 {
+        u32::from_be_bytes(bytes.try_into().unwrap())
+    }
+}
+
+impl SegReqFromBytes 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: SegReqFromBytes,
+{
+    type Item = (T, T);
+
+    fn next(&mut self) -> Option<Self::Item> {
+        let value = self.next_at_offset(self.current_idx);
+        self.current_idx += 2 * size_of::<T>();
+        value
+    }
+}
+
+impl<'a, 'b> PartialEq<SegmentRequests<'a>> 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<SegmentRequests<'a>> 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 = size_of::<T>();
+        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<T: SegReqFromBytes> SegmentRequestIter<'_, T> {
+    fn next_at_offset(&self, mut offset: usize) -> Option<(T, T)> {
+        if offset + size_of::<T>() * 2 > self.seq_req_raw.len() {
+            return None;
+        }
+
+        let start_offset = T::from_bytes(&self.seq_req_raw[offset..offset + size_of::<T>()]);
+        offset += size_of::<T>();
+
+        let end_offset = T::from_bytes(&self.seq_req_raw[offset..offset + size_of::<T>()]);
+        Some((start_offset, end_offset))
+    }
+}
+
+/// NAK PDU abstraction specialized in the reading NAK PDUs from a raw bytestream.
+///
+/// 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.
+#[derive(Debug, PartialEq, Eq)]
+pub struct NakPduReader<'seg_reqs> {
+    pdu_header: PduHeader,
+    start_of_scope: u64,
+    end_of_scope: u64,
+    seg_reqs_raw: &'seg_reqs [u8],
+}
+
+impl CfdpPdu for NakPduReader<'_> {
+    fn pdu_header(&self) -> &PduHeader {
+        &self.pdu_header
+    }
+
+    fn file_directive_type(&self) -> Option<FileDirectiveType> {
+        Some(FileDirectiveType::NakPdu)
+    }
+}
+
+impl<'seg_reqs> NakPduReader<'seg_reqs> {
+    pub fn new(buf: &'seg_reqs [u8]) -> Result<NakPduReader, PduError> {
+        Self::from_bytes(buf)
+    }
+
+    pub fn from_bytes(buf: &'seg_reqs [u8]) -> Result<NakPduReader, PduError> {
+        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::ByteConversion(
+                    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 =
+                u32::from_be_bytes(buf[current_idx..current_idx + 4].try_into().unwrap()) as u64;
+            current_idx += 4;
+            end_of_scope =
+                u32::from_be_bytes(buf[current_idx..current_idx + 4].try_into().unwrap()) as u64;
+            current_idx += 4;
+        }
+        Ok(Self {
+            pdu_header,
+            start_of_scope,
+            end_of_scope,
+            seg_reqs_raw: &buf[current_idx..full_len_without_crc],
+        })
+    }
+
+    pub fn start_of_scope(&self) -> u64 {
+        self.start_of_scope
+    }
+
+    pub fn end_of_scope(&self) -> u64 {
+        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<SegmentRequestIter<'_, u32>> {
+        if self.file_flag() == LargeFileFlag::Large {
+            return None;
+        }
+        Some(SegmentRequestIter {
+            seq_req_raw: self.seg_reqs_raw,
+            current_idx: 0,
+            phantom: PhantomData,
+        })
+    }
+
+    /// This function returns [None] if this NAK PDUs contains segment requests for a normal file.
+    pub fn get_large_segment_requests_iterator(&self) -> Option<SegmentRequestIter<'_, u64>> {
+        if self.file_flag() == LargeFileFlag::Normal {
+            return None;
+        }
+        Some(SegmentRequestIter {
+            seq_req_raw: self.seg_reqs_raw,
+            current_idx: 0,
+            phantom: PhantomData,
+        })
+    }
+}
+
+impl<'a, 'b> PartialEq<NakPduCreator<'a>> for NakPduReader<'b> {
+    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;
+        }
+
+        // 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()
+                }
+            }
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use alloc::string::ToString;
+
+    use crate::cfdp::{
+        pdu::tests::{common_pdu_conf, verify_raw_header, TEST_DEST_ID, TEST_SEQ_NUM, TEST_SRC_ID},
+        PduType, TransmissionMode,
+    };
+
+    use super::*;
+
+    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);
+        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_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);
+        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)
+            .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);
+        current_idx += 4;
+        assert_eq!(current_idx, nak_pdu.len_written());
+    }
+
+    #[test]
+    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);
+        check_generic_fields(&nak_pdu_deser);
+    }
+
+    #[test]
+    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 =
+            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);
+        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());
+    }
+
+    #[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)]);
+        //let error = NakPduCreator::new_generic(pdu_header, 100, 300, Some(u32_list));
+        let error = NakPduCreator::new_generic(
+            pdu_header,
+            u32::MAX as u64 + 1,
+            u32::MAX as u64 + 2,
+            Some(u32_list),
+        );
+        assert!(error.is_err());
+        let error = error.unwrap_err();
+        if let PduError::InvalidStartOrEndOfScopeValue = error {
+            assert_eq!(
+                error.to_string(),
+                "invalid start or end of scope for NAK PDU"
+            );
+        } else {
+            panic!("unexpected error {error}");
+        }
+    }
+
+    #[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::ByteConversion(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}"),
+        }
+    }
+
+    #[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())
+            );
+        }
+    }
+}

src/cfdp/tlv/mod.rs

@@ -5,6 +5,10 @@ use crate::cfdp::lv::{
 use crate::cfdp::TlvLvError;
 use crate::util::{UnsignedByteField, UnsignedByteFieldError, UnsignedEnum};
 use crate::ByteConversionError;
+#[cfg(feature = "alloc")]
+use alloc::vec;
+#[cfg(feature = "alloc")]
+use alloc::vec::Vec;
 use num_enum::{IntoPrimitive, TryFromPrimitive};
 #[cfg(feature = "serde")]
 use serde::{Deserialize, Serialize};
@@ -13,6 +17,39 @@ pub mod msg_to_user;
 
 pub const MIN_TLV_LEN: usize = 2;
 
+pub trait GenericTlv {
+    fn tlv_type_field(&self) -> TlvTypeField;
+
+    /// Checks whether the type field contains one of the standard types specified in the CFDP
+    /// standard and is part of the [TlvType] enum.
+    fn is_standard_tlv(&self) -> bool {
+        if let TlvTypeField::Standard(_) = self.tlv_type_field() {
+            return true;
+        }
+        false
+    }
+
+    /// Returns the standard TLV type if the TLV field is not a custom field
+    fn tlv_type(&self) -> Option<TlvType> {
+        if let TlvTypeField::Standard(tlv_type) = self.tlv_type_field() {
+            Some(tlv_type)
+        } else {
+            None
+        }
+    }
+}
+
+pub trait WritableTlv {
+    fn write_to_bytes(&self, buf: &mut [u8]) -> Result<usize, ByteConversionError>;
+    fn len_written(&self) -> usize;
+    #[cfg(feature = "alloc")]
+    fn to_vec(&self) -> Vec<u8> {
+        let mut buf = vec![0; self.len_written()];
+        self.write_to_bytes(&mut buf).unwrap();
+        buf
+    }
+}
+
 #[derive(Debug, Copy, Clone, PartialEq, Eq, TryFromPrimitive, IntoPrimitive)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 #[repr(u8)]
@@ -95,6 +132,13 @@ impl<'data> Tlv<'data> {
         })
     }
 
+    pub fn new_with_custom_type(tlv_type: u8, data: &[u8]) -> Result<Tlv, TlvLvError> {
+        Ok(Tlv {
+            tlv_type_field: TlvTypeField::Custom(tlv_type),
+            lv: Lv::new(data)?,
+        })
+    }
+
     /// Creates a TLV with an empty value field.
     pub fn new_empty(tlv_type: TlvType) -> Tlv<'data> {
         Tlv {
@@ -103,53 +147,24 @@ impl<'data> Tlv<'data> {
         }
     }
 
-    /// Checks whether the type field contains one of the standard types specified in the CFDP
-    /// standard and is part of the [TlvType] enum.
-    pub fn is_standard_tlv(&self) -> bool {
-        if let TlvTypeField::Standard(_) = self.tlv_type_field {
-            return true;
-        }
-        false
-    }
-
-    /// Returns the standard TLV type if the TLV field is not a custom field
-    pub fn tlv_type(&self) -> Option<TlvType> {
-        if let TlvTypeField::Standard(tlv_type) = self.tlv_type_field {
-            Some(tlv_type)
-        } else {
-            None
-        }
-    }
-
-    pub fn tlv_type_field(&self) -> TlvTypeField {
-        self.tlv_type_field
-    }
-
-    pub fn write_to_bytes(&self, buf: &mut [u8]) -> Result<usize, ByteConversionError> {
-        generic_len_check_data_serialization(buf, self.value().len(), MIN_TLV_LEN)?;
-        buf[0] = self.tlv_type_field.into();
-        self.lv.write_to_be_bytes_no_len_check(&mut buf[1..]);
-        Ok(self.len_full())
-    }
-
     pub fn value(&self) -> &[u8] {
         self.lv.value()
     }
 
+    /// Checks whether the value field is empty.
+    pub fn is_empty(&self) -> bool {
+        self.value().is_empty()
+    }
+
     /// Helper method to retrieve the length of the value. Simply calls the [slice::len] method of
     /// [Self::value]
     pub fn len_value(&self) -> usize {
-        self.lv.len_value()
+        self.value().len()
     }
 
     /// Returns the full raw length, including the length byte.
     pub fn len_full(&self) -> usize {
-        self.lv.len_full() + 1
-    }
-
-    /// Checks whether the value field is empty.
-    pub fn is_empty(&self) -> bool {
-        self.lv.is_empty()
+        self.len_value() + 2
     }
 
     /// Creates a TLV give a raw bytestream. Please note that is is not necessary to pass the
@@ -175,14 +190,34 @@ impl<'data> Tlv<'data> {
     }
 }
 
+impl WritableTlv for Tlv<'_> {
+    fn write_to_bytes(&self, buf: &mut [u8]) -> Result<usize, ByteConversionError> {
+        generic_len_check_data_serialization(buf, self.value().len(), MIN_TLV_LEN)?;
+        buf[0] = self.tlv_type_field.into();
+        self.lv.write_to_be_bytes_no_len_check(&mut buf[1..]);
+        Ok(self.len_full())
+    }
+    fn len_written(&self) -> usize {
+        self.len_full()
+    }
+}
+
+impl GenericTlv for Tlv<'_> {
+    fn tlv_type_field(&self) -> TlvTypeField {
+        self.tlv_type_field
+    }
+}
+
 pub(crate) fn verify_tlv_type(raw_type: u8, expected_tlv_type: TlvType) -> Result<(), TlvLvError> {
-    let tlv_type = TlvType::try_from(raw_type)
-        .map_err(|_| TlvLvError::InvalidTlvTypeField((raw_type, Some(expected_tlv_type as u8))))?;
+    let tlv_type = TlvType::try_from(raw_type).map_err(|_| TlvLvError::InvalidTlvTypeField {
+        found: raw_type,
+        expected: Some(expected_tlv_type.into()),
+    })?;
     if tlv_type != expected_tlv_type {
-        return Err(TlvLvError::InvalidTlvTypeField((
-            tlv_type as u8,
-            Some(expected_tlv_type as u8),
-        )));
+        return Err(TlvLvError::InvalidTlvTypeField {
+            found: tlv_type as u8,
+            expected: Some(expected_tlv_type as u8),
+        });
     }
     Ok(())
 }
@@ -208,6 +243,10 @@ impl EntityIdTlv {
         Ok(())
     }
 
+    pub fn entity_id(&self) -> &UnsignedByteField {
+        &self.entity_id
+    }
+
     pub fn len_value(&self) -> usize {
         self.entity_id.size()
     }
@@ -216,13 +255,6 @@ impl EntityIdTlv {
         2 + self.entity_id.size()
     }
 
-    pub fn write_to_be_bytes(&self, buf: &mut [u8]) -> Result<usize, ByteConversionError> {
-        Self::len_check(buf)?;
-        buf[0] = TlvType::EntityId as u8;
-        buf[1] = self.entity_id.size() as u8;
-        self.entity_id.write_to_be_bytes(&mut buf[2..])
-    }
-
     pub fn from_bytes(buf: &[u8]) -> Result<Self, TlvLvError> {
         Self::len_check(buf)?;
         verify_tlv_type(buf[0], TlvType::EntityId)?;
@@ -241,13 +273,32 @@ impl EntityIdTlv {
         self.entity_id
             .write_to_be_bytes(&mut buf[2..2 + self.entity_id.size()])?;
         Tlv::new(TlvType::EntityId, &buf[2..2 + self.entity_id.size()]).map_err(|e| match e {
-            TlvLvError::ByteConversionError(e) => e,
+            TlvLvError::ByteConversion(e) => e,
             // All other errors are impossible.
             _ => panic!("unexpected TLV error"),
         })
     }
 }
 
+impl WritableTlv for EntityIdTlv {
+    fn write_to_bytes(&self, buf: &mut [u8]) -> Result<usize, ByteConversionError> {
+        Self::len_check(buf)?;
+        buf[0] = TlvType::EntityId as u8;
+        buf[1] = self.entity_id.size() as u8;
+        Ok(2 + self.entity_id.write_to_be_bytes(&mut buf[2..])?)
+    }
+
+    fn len_written(&self) -> usize {
+        self.len_full()
+    }
+}
+
+impl GenericTlv for EntityIdTlv {
+    fn tlv_type_field(&self) -> TlvTypeField {
+        TlvTypeField::Standard(TlvType::EntityId)
+    }
+}
+
 impl<'data> TryFrom<Tlv<'data>> for EntityIdTlv {
     type Error = TlvLvError;
 
@@ -255,17 +306,17 @@ impl<'data> TryFrom<Tlv<'data>> for EntityIdTlv {
         match value.tlv_type_field {
             TlvTypeField::Standard(tlv_type) => {
                 if tlv_type != TlvType::EntityId {
-                    return Err(TlvLvError::InvalidTlvTypeField((
-                        tlv_type as u8,
-                        Some(TlvType::EntityId as u8),
-                    )));
+                    return Err(TlvLvError::InvalidTlvTypeField {
+                        found: tlv_type as u8,
+                        expected: Some(TlvType::EntityId as u8),
+                    });
                 }
             }
             TlvTypeField::Custom(val) => {
-                return Err(TlvLvError::InvalidTlvTypeField((
-                    val,
-                    Some(TlvType::EntityId as u8),
-                )));
+                return Err(TlvLvError::InvalidTlvTypeField {
+                    found: val,
+                    expected: Some(TlvType::EntityId as u8),
+                });
             }
         }
         let len_value = value.value().len();
@@ -285,23 +336,50 @@ impl<'data> TryFrom<Tlv<'data>> for EntityIdTlv {
     }
 }
 
+pub fn fs_request_has_second_filename(action_code: FilestoreActionCode) -> bool {
+    if action_code == FilestoreActionCode::RenameFile
+        || action_code == FilestoreActionCode::AppendFile
+        || action_code == FilestoreActionCode::ReplaceFile
+    {
+        return true;
+    }
+    false
+}
+
+#[derive(Debug, Copy, Clone, PartialEq, Eq)]
+#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+struct FilestoreTlvBase<'first_name, 'second_name> {
+    pub action_code: FilestoreActionCode,
+    #[cfg_attr(feature = "serde", serde(borrow))]
+    pub first_name: Lv<'first_name>,
+    #[cfg_attr(feature = "serde", serde(borrow))]
+    pub second_name: Option<Lv<'second_name>>,
+}
+
+impl FilestoreTlvBase<'_, '_> {
+    fn base_len_value(&self) -> usize {
+        let mut len = 1 + self.first_name.len_full();
+        if let Some(second_name) = self.second_name {
+            len += second_name.len_full();
+        }
+        len
+    }
+}
+
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 pub struct FilestoreRequestTlv<'first_name, 'second_name> {
-    action_code: FilestoreActionCode,
     #[cfg_attr(feature = "serde", serde(borrow))]
-    first_name: Lv<'first_name>,
-    #[cfg_attr(feature = "serde", serde(borrow))]
-    second_name: Option<Lv<'second_name>>,
+    base: FilestoreTlvBase<'first_name, 'second_name>,
 }
 
 impl<'first_name, 'second_name> FilestoreRequestTlv<'first_name, 'second_name> {
-    pub fn new_create_file(first_name: Lv<'first_name>) -> Result<Self, TlvLvError> {
-        Self::new(FilestoreActionCode::CreateFile, first_name, None)
+    pub fn new_create_file(file_name: Lv<'first_name>) -> Result<Self, TlvLvError> {
+        Self::new(FilestoreActionCode::CreateFile, file_name, None)
     }
 
-    pub fn new_delete_file(first_name: Lv<'first_name>) -> Result<Self, TlvLvError> {
-        Self::new(FilestoreActionCode::DeleteFile, first_name, None)
+    pub fn new_delete_file(file_name: Lv<'first_name>) -> Result<Self, TlvLvError> {
+        Self::new(FilestoreActionCode::DeleteFile, file_name, None)
     }
 
     pub fn new_rename_file(
@@ -363,14 +441,14 @@ impl<'first_name, 'second_name> FilestoreRequestTlv<'first_name, 'second_name> {
     /// only one is passed. It will also returns [None] if the cumulative length of the first
     /// name and the second name exceeds 255 bytes.
     ///
-    /// This is the case for the rename, append and replace filestore request.
+    /// Two file paths are required for the rename, append and replace filestore request.
     pub fn new(
         action_code: FilestoreActionCode,
         first_name: Lv<'first_name>,
         second_name: Option<Lv<'second_name>>,
     ) -> Result<Self, TlvLvError> {
         let mut base_value_len = first_name.len_full();
-        if Self::has_second_filename(action_code) {
+        if fs_request_has_second_filename(action_code) {
             if second_name.is_none() {
                 return Err(TlvLvError::SecondNameMissing);
             }
@@ -380,71 +458,34 @@ impl<'first_name, 'second_name> FilestoreRequestTlv<'first_name, 'second_name> {
             return Err(TlvLvError::InvalidValueLength(base_value_len));
         }
         Ok(Self {
-            action_code,
-            first_name,
-            second_name,
+            base: FilestoreTlvBase {
+                action_code,
+                first_name,
+                second_name,
+            },
         })
     }
 
-    pub fn has_second_filename(action_code: FilestoreActionCode) -> bool {
-        if action_code == FilestoreActionCode::RenameFile
-            || action_code == FilestoreActionCode::AppendFile
-            || action_code == FilestoreActionCode::ReplaceFile
-        {
-            return true;
-        }
-        false
-    }
-
     pub fn action_code(&self) -> FilestoreActionCode {
-        self.action_code
+        self.base.action_code
     }
 
     pub fn first_name(&self) -> Lv<'first_name> {
-        self.first_name
+        self.base.first_name
     }
 
     pub fn second_name(&self) -> Option<Lv<'second_name>> {
-        self.second_name
+        self.base.second_name
     }
 
     pub fn len_value(&self) -> usize {
-        let mut len = 1 + self.first_name.len_full();
-        if let Some(second_name) = self.second_name {
-            len += second_name.len_full();
-        }
-        len
+        self.base.base_len_value()
     }
 
     pub fn len_full(&self) -> usize {
         2 + self.len_value()
     }
 
-    pub fn write_to_bytes(&self, buf: &mut [u8]) -> Result<usize, ByteConversionError> {
-        if buf.len() < self.len_full() {
-            return Err(ByteConversionError::ToSliceTooSmall {
-                found: buf.len(),
-                expected: self.len_full(),
-            });
-        }
-        buf[0] = TlvType::FilestoreRequest as u8;
-        buf[1] = self.len_value() as u8;
-        buf[2] = (self.action_code as u8) << 4;
-        let mut current_idx = 3;
-        // Length checks were already performed.
-        self.first_name.write_to_be_bytes_no_len_check(
-            &mut buf[current_idx..current_idx + self.first_name.len_full()],
-        );
-        current_idx += self.first_name.len_full();
-        if let Some(second_name) = self.second_name {
-            second_name.write_to_be_bytes_no_len_check(
-                &mut buf[current_idx..current_idx + second_name.len_full()],
-            );
-            current_idx += second_name.len_full();
-        }
-        Ok(current_idx)
-    }
-
     pub fn from_bytes<'longest: 'first_name + 'second_name>(
         buf: &'longest [u8],
     ) -> Result<Self, TlvLvError> {
@@ -465,26 +506,253 @@ impl<'first_name, 'second_name> FilestoreRequestTlv<'first_name, 'second_name> {
         let mut second_name = None;
 
         current_idx += first_name.len_full();
-        if Self::has_second_filename(action_code) {
+        if fs_request_has_second_filename(action_code) {
             if current_idx >= 2 + len {
                 return Err(TlvLvError::SecondNameMissing);
             }
             second_name = Some(Lv::from_bytes(&buf[current_idx..])?);
         }
         Ok(Self {
+            base: FilestoreTlvBase {
+                action_code,
+                first_name,
+                second_name,
+            },
+        })
+    }
+}
+
+impl WritableTlv for FilestoreRequestTlv<'_, '_> {
+    fn write_to_bytes(&self, buf: &mut [u8]) -> Result<usize, ByteConversionError> {
+        if buf.len() < self.len_full() {
+            return Err(ByteConversionError::ToSliceTooSmall {
+                found: buf.len(),
+                expected: self.len_full(),
+            });
+        }
+        buf[0] = TlvType::FilestoreRequest as u8;
+        buf[1] = self.len_value() as u8;
+        buf[2] = (self.base.action_code as u8) << 4;
+        let mut current_idx = 3;
+        // Length checks were already performed.
+        self.base.first_name.write_to_be_bytes_no_len_check(
+            &mut buf[current_idx..current_idx + self.base.first_name.len_full()],
+        );
+        current_idx += self.base.first_name.len_full();
+        if let Some(second_name) = self.base.second_name {
+            second_name.write_to_be_bytes_no_len_check(
+                &mut buf[current_idx..current_idx + second_name.len_full()],
+            );
+            current_idx += second_name.len_full();
+        }
+        Ok(current_idx)
+    }
+
+    fn len_written(&self) -> usize {
+        self.len_full()
+    }
+}
+
+impl GenericTlv for FilestoreRequestTlv<'_, '_> {
+    fn tlv_type_field(&self) -> TlvTypeField {
+        TlvTypeField::Standard(TlvType::FilestoreRequest)
+    }
+}
+
+#[derive(Debug, Copy, Clone, PartialEq, Eq)]
+#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+pub struct FilestoreResponseTlv<'first_name, 'second_name, 'fs_msg> {
+    #[cfg_attr(feature = "serde", serde(borrow))]
+    base: FilestoreTlvBase<'first_name, 'second_name>,
+    status_code: u8,
+    #[cfg_attr(feature = "serde", serde(borrow))]
+    filestore_message: Lv<'fs_msg>,
+}
+
+impl<'first_name, 'second_name, 'fs_msg> FilestoreResponseTlv<'first_name, 'second_name, 'fs_msg> {
+    /// This function will return [None] if the respective action code requires two names but
+    /// only one is passed. It will also returns [None] if the cumulative length of the first
+    /// name and the second name exceeds 255 bytes.
+    ///
+    /// Two file paths are required for the rename, append and replace filestore request.
+    pub fn new_no_filestore_message(
+        action_code: FilestoreActionCode,
+        status_code: u8,
+        first_name: Lv<'first_name>,
+        second_name: Option<Lv<'second_name>>,
+    ) -> Result<Self, TlvLvError> {
+        Self::new(
             action_code,
+            status_code,
             first_name,
             second_name,
+            Lv::new_empty(),
+        )
+    }
+    pub fn new(
+        action_code: FilestoreActionCode,
+        status_code: u8,
+        first_name: Lv<'first_name>,
+        second_name: Option<Lv<'second_name>>,
+        filestore_message: Lv<'fs_msg>,
+    ) -> Result<Self, TlvLvError> {
+        let mut base_value_len = first_name.len_full();
+        if Self::has_second_filename(action_code) {
+            if second_name.is_none() {
+                return Err(TlvLvError::SecondNameMissing);
+            }
+            base_value_len += second_name.as_ref().unwrap().len_full();
+        }
+        if base_value_len > u8::MAX as usize {
+            return Err(TlvLvError::InvalidValueLength(base_value_len));
+        }
+        Ok(Self {
+            base: FilestoreTlvBase {
+                action_code,
+                first_name,
+                second_name,
+            },
+            status_code,
+            filestore_message,
         })
     }
+
+    pub fn has_second_filename(action_code: FilestoreActionCode) -> bool {
+        if action_code == FilestoreActionCode::RenameFile
+            || action_code == FilestoreActionCode::AppendFile
+            || action_code == FilestoreActionCode::ReplaceFile
+        {
+            return true;
+        }
+        false
+    }
+
+    pub fn action_code(&self) -> FilestoreActionCode {
+        self.base.action_code
+    }
+
+    pub fn status_code(&self) -> u8 {
+        self.status_code
+    }
+
+    pub fn first_name(&self) -> Lv<'first_name> {
+        self.base.first_name
+    }
+
+    pub fn second_name(&self) -> Option<Lv<'second_name>> {
+        self.base.second_name
+    }
+
+    pub fn len_value(&self) -> usize {
+        self.base.base_len_value() + self.filestore_message.len_full()
+    }
+
+    pub fn len_full(&self) -> usize {
+        2 + self.len_value()
+    }
+
+    pub fn from_bytes<'buf: 'first_name + 'second_name + 'fs_msg>(
+        buf: &'buf [u8],
+    ) -> Result<Self, TlvLvError> {
+        if buf.len() < 2 {
+            return Err(ByteConversionError::FromSliceTooSmall {
+                found: buf.len(),
+                expected: 2,
+            }
+            .into());
+        }
+        verify_tlv_type(buf[0], TlvType::FilestoreResponse)?;
+        let len = buf[1] as usize;
+        let mut current_idx = 2;
+        let len_check = |current_idx: &mut usize, add_len: usize| -> Result<(), TlvLvError> {
+            if *current_idx + add_len > buf.len() {
+                return Err(ByteConversionError::FromSliceTooSmall {
+                    found: buf.len(),
+                    expected: *current_idx,
+                }
+                .into());
+            }
+            Ok(())
+        };
+        len_check(&mut current_idx, len)?;
+        let action_code = FilestoreActionCode::try_from((buf[2] >> 4) & 0b1111)
+            .map_err(|_| TlvLvError::InvalidFilestoreActionCode((buf[2] >> 4) & 0b1111))?;
+        let status_code = buf[2] & 0b1111;
+        current_idx += 1;
+        let first_name = Lv::from_bytes(&buf[current_idx..])?;
+        len_check(&mut current_idx, first_name.len_full())?;
+        current_idx += first_name.len_full();
+
+        let mut second_name = None;
+        if Self::has_second_filename(action_code) {
+            if current_idx >= 2 + len {
+                return Err(TlvLvError::SecondNameMissing);
+            }
+            let second_name_lv = Lv::from_bytes(&buf[current_idx..])?;
+            current_idx += second_name_lv.len_full();
+            second_name = Some(second_name_lv);
+        }
+        let filestore_message = Lv::from_bytes(&buf[current_idx..])?;
+        len_check(&mut current_idx, filestore_message.len_full())?;
+        Ok(Self {
+            base: FilestoreTlvBase {
+                action_code,
+                first_name,
+                second_name,
+            },
+            status_code,
+            filestore_message,
+        })
+    }
+}
+
+impl WritableTlv for FilestoreResponseTlv<'_, '_, '_> {
+    fn write_to_bytes(&self, buf: &mut [u8]) -> Result<usize, ByteConversionError> {
+        if buf.len() < self.len_full() {
+            return Err(ByteConversionError::ToSliceTooSmall {
+                found: buf.len(),
+                expected: self.len_full(),
+            });
+        }
+        buf[0] = TlvType::FilestoreResponse as u8;
+        buf[1] = self.len_value() as u8;
+        buf[2] = ((self.base.action_code as u8) << 4) | (self.status_code & 0b1111);
+        let mut current_idx = 3;
+        // Length checks were already performed.
+        self.base.first_name.write_to_be_bytes_no_len_check(
+            &mut buf[current_idx..current_idx + self.base.first_name.len_full()],
+        );
+        current_idx += self.base.first_name.len_full();
+        if let Some(second_name) = self.base.second_name {
+            current_idx += second_name.write_to_be_bytes_no_len_check(
+                &mut buf[current_idx..current_idx + second_name.len_full()],
+            );
+        }
+        current_idx += self.filestore_message.write_to_be_bytes_no_len_check(
+            &mut buf[current_idx..current_idx + self.filestore_message.len_full()],
+        );
+        Ok(current_idx)
+    }
+
+    fn len_written(&self) -> usize {
+        self.len_full()
+    }
+}
+
+impl GenericTlv for FilestoreResponseTlv<'_, '_, '_> {
+    fn tlv_type_field(&self) -> TlvTypeField {
+        TlvTypeField::Standard(TlvType::FilestoreResponse)
+    }
 }
 
 #[cfg(test)]
 mod tests {
+    use super::*;
     use crate::cfdp::lv::Lv;
     use crate::cfdp::tlv::{FilestoreActionCode, FilestoreRequestTlv, Tlv, TlvType, TlvTypeField};
     use crate::cfdp::TlvLvError;
-    use crate::util::{UbfU8, UnsignedEnum};
+    use crate::util::{UbfU16, UbfU8, UnsignedEnum};
+    use alloc::string::ToString;
 
     const TLV_TEST_STR_0: &str = "hello.txt";
     const TLV_TEST_STR_1: &str = "hello2.txt";
@@ -544,6 +812,45 @@ mod tests {
         assert_eq!(tlv_from_raw.value()[0], 5);
     }
 
+    #[test]
+    fn test_entity_id_tlv() {
+        let entity_id = UbfU16::new(0x0102);
+        let entity_id_tlv = EntityIdTlv::new(entity_id.into());
+        let mut buf: [u8; 16] = [0; 16];
+        let written_len = entity_id_tlv.write_to_bytes(&mut buf).unwrap();
+        assert_eq!(written_len, entity_id_tlv.len_full());
+        assert_eq!(entity_id_tlv.len_value(), 2);
+        assert!(entity_id_tlv.is_standard_tlv());
+        assert_eq!(entity_id_tlv.tlv_type().unwrap(), TlvType::EntityId);
+        assert_eq!(buf[0], TlvType::EntityId as u8);
+        assert_eq!(buf[1], 2);
+        assert_eq!(u16::from_be_bytes(buf[2..4].try_into().unwrap()), 0x0102);
+        let entity_id_as_vec = entity_id_tlv.to_vec();
+        assert_eq!(entity_id_as_vec, buf[0..written_len].to_vec());
+    }
+
+    #[test]
+    fn test_entity_id_from_generic_tlv() {
+        let entity_id = UbfU16::new(0x0102);
+        let entity_id_tlv = EntityIdTlv::new(entity_id.into());
+        let mut buf: [u8; 16] = [0; 16];
+        let entity_id_as_tlv: Tlv = entity_id_tlv.to_tlv(&mut buf).unwrap();
+        let entity_id_converted_back: EntityIdTlv = entity_id_as_tlv.try_into().unwrap();
+        assert_eq!(entity_id_converted_back, entity_id_tlv);
+    }
+
+    #[test]
+    fn test_entity_id_from_raw() {
+        let entity_id = UbfU16::new(0x0102);
+        let entity_id_tlv = EntityIdTlv::new(entity_id.into());
+        let mut buf: [u8; 16] = [0; 16];
+        let _ = entity_id_tlv.write_to_bytes(&mut buf).unwrap();
+        let entity_tlv_from_raw =
+            EntityIdTlv::from_bytes(&buf).expect("creating entity ID TLV failed");
+        assert_eq!(entity_tlv_from_raw, entity_id_tlv);
+        assert_eq!(entity_tlv_from_raw.entity_id(), &entity_id.into());
+    }
+
     #[test]
     fn test_empty() {
         let tlv_empty = Tlv::new_empty(TlvType::MsgToUser);
@@ -584,6 +891,41 @@ mod tests {
         assert!(tlv_empty.value().is_empty());
     }
 
+    #[test]
+    fn test_write_buf_too_small() {
+        let mut buf: [u8; 2] = [0; 2];
+        let fs_request =
+            FilestoreRequestTlv::new_create_file(Lv::new_from_str(TLV_TEST_STR_0).unwrap())
+                .unwrap();
+        let error = fs_request.write_to_bytes(&mut buf);
+        assert!(error.is_err());
+        let error = error.unwrap_err();
+        if let ByteConversionError::ToSliceTooSmall { found, expected } = error {
+            assert_eq!(found, 2);
+            assert_eq!(expected, 13);
+        } else {
+            panic!("unexpected error {:?}", error);
+        }
+    }
+
+    #[test]
+    fn test_read_from_buf_too_small() {
+        let buf: [u8; 1] = [0; 1];
+        let error = FilestoreRequestTlv::from_bytes(&buf);
+        assert!(error.is_err());
+        let error = error.unwrap_err();
+        if let TlvLvError::ByteConversion(ByteConversionError::FromSliceTooSmall {
+            found,
+            expected,
+        }) = error
+        {
+            assert_eq!(found, 1);
+            assert_eq!(expected, 2);
+        } else {
+            panic!("unexpected error {:?}", error);
+        }
+    }
+
     #[test]
     fn test_buf_too_large() {
         let buf_too_large: [u8; u8::MAX as usize + 1] = [0; u8::MAX as usize + 1];
@@ -592,6 +934,10 @@ mod tests {
         let error = tlv_res.unwrap_err();
         if let TlvLvError::DataTooLarge(size) = error {
             assert_eq!(size, u8::MAX as usize + 1);
+            assert_eq!(
+                error.to_string(),
+                "data with size 256 larger than allowed 255 bytes"
+            );
         } else {
             panic!("unexpected error {:?}", error);
         }
@@ -607,6 +953,7 @@ mod tests {
         assert!(tlv.is_ok());
         let tlv = tlv.unwrap();
         assert_eq!(tlv.tlv_type_field(), TlvTypeField::Custom(3));
+        assert!(!tlv.is_standard_tlv());
         assert_eq!(tlv.value().len(), 1);
         assert_eq!(tlv.len_full(), 3);
     }
@@ -614,7 +961,7 @@ mod tests {
     fn generic_fs_request_test_one_file(
         action_code: FilestoreActionCode,
     ) -> FilestoreRequestTlv<'static, 'static> {
-        assert!(!FilestoreRequestTlv::has_second_filename(action_code));
+        assert!(!fs_request_has_second_filename(action_code));
         let first_name = Lv::new_from_str(TLV_TEST_STR_0).unwrap();
         let fs_request = match action_code {
             FilestoreActionCode::CreateFile => FilestoreRequestTlv::new_create_file(first_name),
@@ -644,7 +991,7 @@ mod tests {
     fn generic_fs_request_test_two_files(
         action_code: FilestoreActionCode,
     ) -> FilestoreRequestTlv<'static, 'static> {
-        assert!(FilestoreRequestTlv::has_second_filename(action_code));
+        assert!(fs_request_has_second_filename(action_code));
         let first_name = Lv::new_from_str(TLV_TEST_STR_0).unwrap();
         let second_name = Lv::new_from_str(TLV_TEST_STR_1).unwrap();
         let fs_request = match action_code {
@@ -665,7 +1012,12 @@ mod tests {
             fs_request.len_value(),
             1 + first_name.len_full() + second_name.len_full()
         );
+        assert_eq!(
+            fs_request.tlv_type_field(),
+            TlvTypeField::Standard(TlvType::FilestoreRequest)
+        );
         assert_eq!(fs_request.len_full(), fs_request.len_value() + 2);
+        assert_eq!(fs_request.len_written(), fs_request.len_full());
         assert_eq!(fs_request.action_code(), action_code);
         assert_eq!(fs_request.first_name(), first_name);
         assert!(fs_request.second_name().is_some());
@@ -795,4 +1147,150 @@ mod tests {
         let req_conv_back = req_conv_back.unwrap();
         assert_eq!(req_conv_back, req);
     }
+
+    #[test]
+    fn test_fs_response_state_one_path() {
+        let lv_0 = Lv::new_from_str(TLV_TEST_STR_0).unwrap();
+        let response = FilestoreResponseTlv::new_no_filestore_message(
+            FilestoreActionCode::CreateFile,
+            0b0001,
+            lv_0,
+            None,
+        )
+        .expect("creating response failed");
+        assert_eq!(response.status_code(), 0b0001);
+        assert_eq!(response.action_code(), FilestoreActionCode::CreateFile);
+        assert_eq!(response.first_name(), lv_0);
+        assert!(response.second_name().is_none());
+    }
+    #[test]
+    fn test_fs_response_state_two_paths() {
+        let lv_0 = Lv::new_from_str(TLV_TEST_STR_0).unwrap();
+        let lv_1 = Lv::new_from_str(TLV_TEST_STR_1).unwrap();
+        let response = FilestoreResponseTlv::new_no_filestore_message(
+            FilestoreActionCode::RenameFile,
+            0b0001,
+            lv_0,
+            Some(lv_1),
+        )
+        .expect("creating response failed");
+        assert_eq!(response.status_code(), 0b0001);
+        assert_eq!(response.action_code(), FilestoreActionCode::RenameFile);
+        assert_eq!(response.first_name(), lv_0);
+        assert!(response.second_name().is_some());
+        assert!(response.second_name().unwrap() == lv_1);
+        assert_eq!(
+            response.len_full(),
+            2 + 1 + lv_0.len_full() + lv_1.len_full() + 1
+        );
+    }
+
+    #[test]
+    fn test_fs_response_serialization() {
+        let lv_0 = Lv::new_from_str(TLV_TEST_STR_0).unwrap();
+        let response = FilestoreResponseTlv::new_no_filestore_message(
+            FilestoreActionCode::CreateFile,
+            0b0001,
+            lv_0,
+            None,
+        )
+        .expect("creating response failed");
+        let mut buf: [u8; 32] = [0; 32];
+        let written_len = response.write_to_bytes(&mut buf).unwrap();
+        assert_eq!(written_len, 2 + 1 + lv_0.len_full() + 1);
+        assert_eq!(buf[0], TlvType::FilestoreResponse as u8);
+        assert_eq!(buf[1], written_len as u8 - 2);
+        assert_eq!(
+            (buf[2] >> 4) & 0b1111,
+            FilestoreActionCode::CreateFile as u8
+        );
+        assert_eq!(buf[2] & 0b1111, 0b0001);
+        let lv_read_back = Lv::from_bytes(&buf[3..]).unwrap();
+        assert_eq!(lv_0, lv_read_back);
+        let current_idx = 3 + lv_0.len_full();
+        let fs_msg_empty = Lv::from_bytes(&buf[current_idx..]).unwrap();
+        assert!(fs_msg_empty.is_empty());
+    }
+
+    #[test]
+    fn test_fs_response_deserialization() {
+        let lv_0 = Lv::new_from_str(TLV_TEST_STR_0).unwrap();
+        let response = FilestoreResponseTlv::new_no_filestore_message(
+            FilestoreActionCode::CreateFile,
+            0b0001,
+            lv_0,
+            None,
+        )
+        .expect("creating response failed");
+        let mut buf: [u8; 32] = [0; 32];
+        response.write_to_bytes(&mut buf).unwrap();
+        let response_read_back = FilestoreResponseTlv::from_bytes(&buf).unwrap();
+        assert_eq!(response_read_back, response);
+    }
+
+    #[test]
+    fn test_entity_it_tlv_to_tlv() {
+        let entity_id = UbfU16::new(0x0102);
+        let entity_id_tlv = EntityIdTlv::new(entity_id.into());
+        let mut binding = [0; 16];
+        let tlv = entity_id_tlv.to_tlv(&mut binding).unwrap();
+        assert_eq!(
+            tlv.tlv_type_field(),
+            TlvTypeField::Standard(TlvType::EntityId)
+        );
+        assert_eq!(tlv.len_full(), 4);
+        assert_eq!(tlv.len_value(), 2);
+        assert_eq!(tlv.value(), &[0x01, 0x02]);
+    }
+
+    #[test]
+    fn test_invalid_tlv_conversion() {
+        let msg_to_user_tlv = Tlv::new_empty(TlvType::MsgToUser);
+        let error = EntityIdTlv::try_from(msg_to_user_tlv);
+        assert!(error.is_err());
+        let error = error.unwrap_err();
+        if let TlvLvError::InvalidTlvTypeField { found, expected } = error {
+            assert_eq!(found, TlvType::MsgToUser as u8);
+            assert_eq!(expected, Some(TlvType::EntityId as u8));
+            assert_eq!(
+                error.to_string(),
+                "invalid TLV type field, found 2, expected Some(6)"
+            );
+        } else {
+            panic!("unexpected error");
+        }
+    }
+
+    #[test]
+    fn test_entity_id_invalid_value_len() {
+        let entity_id = UbfU16::new(0x0102);
+        let entity_id_tlv = EntityIdTlv::new(entity_id.into());
+        let mut buf: [u8; 32] = [0; 32];
+        entity_id_tlv.write_to_bytes(&mut buf).unwrap();
+        buf[1] = 12;
+        let error = EntityIdTlv::from_bytes(&buf);
+        assert!(error.is_err());
+        let error = error.unwrap_err();
+        if let TlvLvError::InvalidValueLength(len) = error {
+            assert_eq!(len, 12);
+            assert_eq!(error.to_string(), "invalid value length 12");
+        } else {
+            panic!("unexpected error");
+        }
+    }
+
+    #[test]
+    fn test_custom_tlv() {
+        let custom_tlv = Tlv::new_with_custom_type(20, &[]).unwrap();
+        assert!(custom_tlv.tlv_type().is_none());
+        if let TlvTypeField::Custom(val) = custom_tlv.tlv_type_field() {
+            assert_eq!(val, 20);
+        } else {
+            panic!("unexpected type field");
+        }
+        let tlv_as_vec = custom_tlv.to_vec();
+        assert_eq!(tlv_as_vec.len(), 2);
+        assert_eq!(tlv_as_vec[0], 20);
+        assert_eq!(tlv_as_vec[1], 0);
+    }
 }

src/cfdp/tlv/msg_to_user.rs

@@ -1,5 +1,5 @@
 //! Abstractions for the Message to User CFDP TLV subtype.
-use super::{Tlv, TlvLvError, TlvType, TlvTypeField};
+use super::{GenericTlv, Tlv, TlvLvError, TlvType, TlvTypeField, WritableTlv};
 use crate::ByteConversionError;
 use delegate::delegate;
 
@@ -18,8 +18,6 @@ impl<'data> MsgToUserTlv<'data> {
 
     delegate! {
         to self.tlv {
-            pub fn tlv_type_field(&self) -> TlvTypeField;
-            pub fn write_to_bytes(&self, buf: &mut [u8]) -> Result<usize, ByteConversionError>;
             pub fn value(&self) -> &[u8];
             /// Helper method to retrieve the length of the value. Simply calls the [slice::len] method of
             /// [Self::value]
@@ -59,29 +57,48 @@ impl<'data> MsgToUserTlv<'data> {
         let msg_to_user = Self {
             tlv: Tlv::from_bytes(buf)?,
         };
-        match msg_to_user.tlv_type_field() {
+        match msg_to_user.tlv.tlv_type_field() {
             TlvTypeField::Standard(tlv_type) => {
                 if tlv_type != TlvType::MsgToUser {
-                    return Err(TlvLvError::InvalidTlvTypeField((
-                        tlv_type as u8,
-                        Some(TlvType::MsgToUser as u8),
-                    )));
+                    return Err(TlvLvError::InvalidTlvTypeField {
+                        found: tlv_type as u8,
+                        expected: Some(TlvType::MsgToUser as u8),
+                    });
                 }
             }
             TlvTypeField::Custom(raw) => {
-                return Err(TlvLvError::InvalidTlvTypeField((
-                    raw,
-                    Some(TlvType::MsgToUser as u8),
-                )));
+                return Err(TlvLvError::InvalidTlvTypeField {
+                    found: raw,
+                    expected: Some(TlvType::MsgToUser as u8),
+                });
             }
         }
         Ok(msg_to_user)
     }
 }
 
+impl WritableTlv for MsgToUserTlv<'_> {
+    fn len_written(&self) -> usize {
+        self.len_full()
+    }
+
+    delegate!(
+        to self.tlv {
+            fn write_to_bytes(&self, buf: &mut [u8]) -> Result<usize, ByteConversionError>;
+        }
+    );
+}
+
+impl GenericTlv for MsgToUserTlv<'_> {
+    fn tlv_type_field(&self) -> TlvTypeField {
+        self.tlv.tlv_type_field()
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
+
     #[test]
     fn test_basic() {
         let custom_value: [u8; 4] = [1, 2, 3, 4];
@@ -90,6 +107,10 @@ mod tests {
         let msg_to_user = msg_to_user.unwrap();
         assert!(msg_to_user.is_standard_tlv());
         assert_eq!(msg_to_user.tlv_type().unwrap(), TlvType::MsgToUser);
+        assert_eq!(
+            msg_to_user.tlv_type_field(),
+            TlvTypeField::Standard(TlvType::MsgToUser)
+        );
         assert_eq!(msg_to_user.value(), custom_value);
         assert_eq!(msg_to_user.value().len(), 4);
         assert_eq!(msg_to_user.len_value(), 4);
@@ -99,6 +120,48 @@ mod tests {
         assert!(!msg_to_user.is_reserved_cfdp_msg());
     }
 
+    #[test]
+    fn test_reserved_msg_serialization() {
+        let custom_value: [u8; 4] = [1, 2, 3, 4];
+        let msg_to_user = MsgToUserTlv::new(&custom_value).unwrap();
+        let mut buf: [u8; 6] = [0; 6];
+        msg_to_user.write_to_bytes(&mut buf).unwrap();
+        assert_eq!(
+            buf,
+            [
+                TlvType::MsgToUser as u8,
+                custom_value.len() as u8,
+                1,
+                2,
+                3,
+                4
+            ]
+        );
+    }
+
+    #[test]
+    fn test_reserved_msg_deserialization() {
+        let custom_value: [u8; 3] = [1, 2, 3];
+        let msg_to_user = MsgToUserTlv::new(&custom_value).unwrap();
+        let msg_to_user_vec = msg_to_user.to_vec();
+        let msg_to_user_from_bytes = MsgToUserTlv::from_bytes(&msg_to_user_vec).unwrap();
+        assert!(!msg_to_user.is_reserved_cfdp_msg());
+        assert_eq!(msg_to_user_from_bytes, msg_to_user);
+        assert_eq!(msg_to_user_from_bytes.value(), msg_to_user.value());
+        assert_eq!(msg_to_user_from_bytes.tlv_type(), msg_to_user.tlv_type());
+    }
+    #[test]
+    fn test_reserved_msg_deserialization_invalid_type() {
+        let trash: [u8; 5] = [TlvType::FlowLabel as u8, 3, 1, 2, 3];
+        let error = MsgToUserTlv::from_bytes(&trash).unwrap_err();
+        if let TlvLvError::InvalidTlvTypeField { found, expected } = error {
+            assert_eq!(found, TlvType::FlowLabel as u8);
+            assert_eq!(expected, Some(TlvType::MsgToUser as u8));
+        } else {
+            panic!("Wrong error type returned: {:?}", error);
+        }
+    }
+
     #[test]
     fn test_reserved_msg() {
         let reserved_str = "cfdp";

src/ecss/hk.rs

@@ -29,3 +29,33 @@ pub enum Subservice {
     TcGenerateOneShotDiag = 28,
     TcModifyDiagCollectionInterval = 32,
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    #[test]
+    fn test_try_from_u8() {
+        let hk_report_subservice_raw = 25;
+        let hk_report: Subservice = Subservice::try_from(hk_report_subservice_raw).unwrap();
+        assert_eq!(hk_report, Subservice::TmHkPacket);
+    }
+
+    #[test]
+    fn test_into_u8() {
+        let hk_report_raw: u8 = Subservice::TmHkPacket.into();
+        assert_eq!(hk_report_raw, 25);
+    }
+
+    #[test]
+    fn test_partial_eq() {
+        let hk_report_raw = Subservice::TmHkPacket;
+        assert_ne!(hk_report_raw, Subservice::TcGenerateOneShotHk);
+        assert_eq!(hk_report_raw, Subservice::TmHkPacket);
+    }
+    #[test]
+    fn test_copy_clone() {
+        let hk_report = Subservice::TmHkPacket;
+        let hk_report_copy = hk_report;
+        assert_eq!(hk_report, hk_report_copy);
+    }
+}

src/ecss/mod.rs

@@ -74,6 +74,7 @@ pub enum PusServiceId {
 /// All PUS versions. Only PUS C is supported by this library.
 #[derive(PartialEq, Eq, Copy, Clone, Debug)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[non_exhaustive]
 pub enum PusVersion {
     EsaPus = 0,
     PusA = 1,
@@ -95,8 +96,9 @@ impl TryFrom<u8> for PusVersion {
 }
 
 /// ECSS Packet Type Codes (PTC)s.
-#[derive(Debug, Copy, Clone, PartialEq, Eq)]
+#[derive(Debug, Copy, Clone, Eq, PartialEq, IntoPrimitive, TryFromPrimitive)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[repr(u8)]
 pub enum PacketTypeCodes {
     Boolean = 1,
     Enumerated = 2,
@@ -115,9 +117,10 @@ pub enum PacketTypeCodes {
 pub type Ptc = PacketTypeCodes;
 
 /// ECSS Packet Field Codes (PFC)s for the unsigned [Ptc].
-#[derive(Debug, Copy, Clone, PartialEq, Eq)]
+#[derive(Debug, Copy, Clone, Eq, PartialEq, IntoPrimitive, TryFromPrimitive)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
-pub enum UnsignedPfc {
+#[repr(u8)]
+pub enum PfcUnsigned {
     OneByte = 4,
     TwelveBits = 8,
     TwoBytes = 12,
@@ -131,9 +134,10 @@ pub enum UnsignedPfc {
 }
 
 /// ECSS Packet Field Codes (PFC)s for the real (floating point) [Ptc].
-#[derive(Debug, Copy, Clone, PartialEq, Eq)]
+#[derive(Debug, Copy, Clone, Eq, PartialEq, IntoPrimitive, TryFromPrimitive)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
-pub enum RealPfc {
+#[repr(u8)]
+pub enum PfcReal {
     /// 4 octets simple precision format (IEEE)
     Float = 1,
     /// 8 octets simple precision format (IEEE)
@@ -148,9 +152,7 @@ pub enum RealPfc {
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 pub enum PusError {
     VersionNotSupported(PusVersion),
-    IncorrectCrc(u16),
-    RawDataTooShort(usize),
-    NoRawData,
+    ChecksumFailure(u16),
     /// CRC16 needs to be calculated first
     CrcCalculationMissing,
     ByteConversion(ByteConversionError),
@@ -162,23 +164,14 @@ impl Display for PusError {
             PusError::VersionNotSupported(v) => {
                 write!(f, "PUS version {v:?} not supported")
             }
-            PusError::IncorrectCrc(crc) => {
-                write!(f, "crc16 {crc:#04x} is incorrect")
-            }
-            PusError::RawDataTooShort(size) => {
-                write!(
-                    f,
-                    "deserialization error, provided raw data with size {size} too short"
-                )
-            }
-            PusError::NoRawData => {
-                write!(f, "no raw data provided")
+            PusError::ChecksumFailure(crc) => {
+                write!(f, "checksum verification for crc16 {crc:#06x} failed")
             }
             PusError::CrcCalculationMissing => {
                 write!(f, "crc16 was not calculated")
             }
             PusError::ByteConversion(e) => {
-                write!(f, "low level byte conversion error: {e}")
+                write!(f, "pus error: {e}")
             }
         }
     }
@@ -214,7 +207,11 @@ pub trait PusPacket: CcsdsPacket {
 
 pub(crate) fn crc_from_raw_data(raw_data: &[u8]) -> Result<u16, PusError> {
     if raw_data.len() < 2 {
-        return Err(PusError::RawDataTooShort(raw_data.len()));
+        return Err(ByteConversionError::FromSliceTooSmall {
+            found: raw_data.len(),
+            expected: 2,
+        }
+        .into());
     }
     Ok(u16::from_be_bytes(
         raw_data[raw_data.len() - 2..raw_data.len()]
@@ -250,11 +247,14 @@ pub(crate) fn crc_procedure(
 pub(crate) fn user_data_from_raw(
     current_idx: usize,
     total_len: usize,
-    raw_data_len: usize,
     slice: &[u8],
 ) -> Result<&[u8], PusError> {
     match current_idx {
-        _ if current_idx > total_len - 2 => Err(PusError::RawDataTooShort(raw_data_len)),
+        _ if current_idx > total_len - 2 => Err(ByteConversionError::FromSliceTooSmall {
+            found: total_len - 2,
+            expected: current_idx,
+        }
+        .into()),
         _ => Ok(&slice[current_idx..total_len - 2]),
     }
 }
@@ -265,7 +265,7 @@ pub(crate) fn verify_crc16_ccitt_false_from_raw_to_pus_error(
 ) -> Result<(), PusError> {
     verify_crc16_ccitt_false_from_raw(raw_data)
         .then(|| ())
-        .ok_or(PusError::IncorrectCrc(crc16))
+        .ok_or(PusError::ChecksumFailure(crc16))
 }
 
 pub(crate) fn verify_crc16_ccitt_false_from_raw(raw_data: &[u8]) -> bool {
@@ -315,11 +315,11 @@ pub trait EcssEnumerationExt: EcssEnumeration + Debug + Copy + Clone + PartialEq
 
 #[derive(Debug, Copy, Clone, Eq, PartialEq)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
-pub struct GenericEcssEnumWrapper<TYPE> {
+pub struct GenericEcssEnumWrapper<TYPE: Copy> {
     field: GenericUnsignedByteField<TYPE>,
 }
 
-impl<TYPE> GenericEcssEnumWrapper<TYPE> {
+impl<TYPE: Copy> GenericEcssEnumWrapper<TYPE> {
     pub const fn ptc() -> PacketTypeCodes {
         PacketTypeCodes::Enumerated
     }
@@ -331,7 +331,7 @@ impl<TYPE> GenericEcssEnumWrapper<TYPE> {
     }
 }
 
-impl<TYPE: ToBeBytes> UnsignedEnum for GenericEcssEnumWrapper<TYPE> {
+impl<TYPE: Copy + ToBeBytes> UnsignedEnum for GenericEcssEnumWrapper<TYPE> {
     fn size(&self) -> usize {
         (self.pfc() / 8) as usize
     }
@@ -341,7 +341,7 @@ impl<TYPE: ToBeBytes> UnsignedEnum for GenericEcssEnumWrapper<TYPE> {
     }
 }
 
-impl<TYPE: ToBeBytes> EcssEnumeration for GenericEcssEnumWrapper<TYPE> {
+impl<TYPE: Copy + ToBeBytes> EcssEnumeration for GenericEcssEnumWrapper<TYPE> {
     fn pfc(&self) -> u8 {
         size_of::<TYPE>() as u8 * 8_u8
     }
@@ -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)
     }
@@ -376,13 +376,22 @@ pub trait SerializablePusPacket {
 
 #[cfg(test)]
 mod tests {
+    use alloc::string::ToString;
+
     use crate::ecss::{EcssEnumU16, EcssEnumU32, EcssEnumU8, UnsignedEnum};
     use crate::ByteConversionError;
 
+    use super::*;
+    #[cfg(feature = "serde")]
+    use crate::tests::generic_serde_test;
+
     #[test]
     fn test_enum_u8() {
         let mut buf = [0, 0, 0];
         let my_enum = EcssEnumU8::new(1);
+        assert_eq!(EcssEnumU8::ptc(), Ptc::Enumerated);
+        assert_eq!(my_enum.size(), 1);
+        assert_eq!(my_enum.pfc(), 8);
         my_enum
             .write_to_be_bytes(&mut buf[1..2])
             .expect("To byte conversion of u8 failed");
@@ -396,6 +405,8 @@ mod tests {
         my_enum
             .write_to_be_bytes(&mut buf[1..3])
             .expect("To byte conversion of u8 failed");
+        assert_eq!(my_enum.size(), 2);
+        assert_eq!(my_enum.pfc(), 16);
         assert_eq!(buf[1], 0x1f);
         assert_eq!(buf[2], 0x2f);
     }
@@ -448,4 +459,78 @@ mod tests {
             }
         }
     }
+
+    #[test]
+    fn test_pus_error_display() {
+        let unsupport_version = PusError::VersionNotSupported(super::PusVersion::EsaPus);
+        let write_str = unsupport_version.to_string();
+        assert_eq!(write_str, "PUS version EsaPus not supported")
+    }
+
+    #[test]
+    fn test_service_id_from_u8() {
+        let verification_id_raw = 1;
+        let verification_id = PusServiceId::try_from(verification_id_raw).unwrap();
+        assert_eq!(verification_id, PusServiceId::Verification);
+    }
+
+    #[test]
+    fn test_ptc_from_u8() {
+        let ptc_raw = Ptc::AbsoluteTime as u8;
+        let ptc = Ptc::try_from(ptc_raw).unwrap();
+        assert_eq!(ptc, Ptc::AbsoluteTime);
+    }
+
+    #[test]
+    fn test_unsigned_pfc_from_u8() {
+        let pfc_raw = PfcUnsigned::OneByte as u8;
+        let pfc = PfcUnsigned::try_from(pfc_raw).unwrap();
+        assert_eq!(pfc, PfcUnsigned::OneByte);
+    }
+
+    #[test]
+    fn test_real_pfc_from_u8() {
+        let pfc_raw = PfcReal::Double as u8;
+        let pfc = PfcReal::try_from(pfc_raw).unwrap();
+        assert_eq!(pfc, PfcReal::Double);
+    }
+
+    #[test]
+    fn test_pus_error_eq_impl() {
+        assert_eq!(
+            PusError::VersionNotSupported(PusVersion::EsaPus),
+            PusError::VersionNotSupported(PusVersion::EsaPus)
+        );
+    }
+
+    #[test]
+    fn test_pus_error_clonable() {
+        let pus_error = PusError::ChecksumFailure(0x0101);
+        let cloned = pus_error;
+        assert_eq!(pus_error, cloned);
+    }
+
+    #[test]
+    #[cfg(feature = "serde")]
+    fn test_serde_pus_service_id() {
+        generic_serde_test(PusServiceId::Verification);
+    }
+
+    #[test]
+    #[cfg(feature = "serde")]
+    fn test_serde_ptc() {
+        generic_serde_test(Ptc::AbsoluteTime);
+    }
+
+    #[test]
+    #[cfg(feature = "serde")]
+    fn test_serde_pfc_unsigned() {
+        generic_serde_test(PfcUnsigned::EightBytes);
+    }
+
+    #[test]
+    #[cfg(feature = "serde")]
+    fn test_serde_pfc_real() {
+        generic_serde_test(PfcReal::Double);
+    }
 }

src/ecss/scheduling.rs

@@ -76,6 +76,8 @@ pub enum TimeWindowType {
 #[cfg(test)]
 mod tests {
     use super::*;
+    #[cfg(feature = "serde")]
+    use crate::tests::generic_serde_test;
 
     #[test]
     fn test_bool_conv_0() {
@@ -102,4 +104,22 @@ mod tests {
         let subservice: Subservice = 22u8.try_into().unwrap();
         assert_eq!(subservice, Subservice::TcCreateScheduleGroup);
     }
+
+    #[test]
+    #[cfg(feature = "serde")]
+    fn test_serde_subservice_id() {
+        generic_serde_test(Subservice::TcEnableScheduling);
+    }
+
+    #[test]
+    #[cfg(feature = "serde")]
+    fn test_serde_sched_status() {
+        generic_serde_test(SchedStatus::Enabled);
+    }
+
+    #[test]
+    #[cfg(feature = "serde")]
+    fn test_serde_time_window_type() {
+        generic_serde_test(TimeWindowType::SelectAll);
+    }
 }

src/ecss/tc.rs

@@ -5,13 +5,13 @@
 //!
 //! ```rust
 //! use spacepackets::{CcsdsPacket, SpHeader};
-//! use spacepackets::ecss::{PusPacket, SerializablePusPacket};
+//! use spacepackets::ecss::{PusPacket, WritablePusPacket};
 //! use spacepackets::ecss::tc::{PusTcCreator, PusTcReader, PusTcSecondaryHeader};
 //!
 //! // Create a ping telecommand with no user application data
 //! let mut sph = SpHeader::tc_unseg(0x02, 0x34, 0).unwrap();
 //! let tc_header = PusTcSecondaryHeader::new_simple(17, 1);
-//! let pus_tc = PusTcCreator::new(&mut sph, tc_header, None, true);
+//! let pus_tc = PusTcCreator::new_no_app_data(&mut sph, tc_header, true);
 //! println!("{:?}", pus_tc);
 //! assert_eq!(pus_tc.service(), 17);
 //! assert_eq!(pus_tc.subservice(), 1);
@@ -34,12 +34,13 @@
 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};
 use core::mem::size_of;
 use delegate::delegate;
+use num_enum::{IntoPrimitive, TryFromPrimitive};
 #[cfg(feature = "serde")]
 use serde::{Deserialize, Serialize};
 use zerocopy::AsBytes;
@@ -58,7 +59,9 @@ const PUS_VERSION: PusVersion = PusVersion::PusC;
 /// Marker trait for PUS telecommand structures.
 pub trait IsPusTelecommand {}
 
-#[derive(Copy, Clone, PartialEq, Debug)]
+#[derive(Debug, Eq, PartialEq, Copy, Clone, IntoPrimitive, TryFromPrimitive)]
+#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[repr(u8)]
 enum AckOpts {
     Acceptance = 0b1000,
     Start = 0b0100,
@@ -216,7 +219,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 +350,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
@@ -405,14 +408,22 @@ pub mod legacy_tc {
         pub fn from_bytes(slice: &'raw_data [u8]) -> Result<(Self, usize), PusError> {
             let raw_data_len = slice.len();
             if raw_data_len < PUS_TC_MIN_LEN_WITHOUT_APP_DATA {
-                return Err(PusError::RawDataTooShort(raw_data_len));
+                return Err(ByteConversionError::FromSliceTooSmall {
+                    found: raw_data_len,
+                    expected: PUS_TC_MIN_LEN_WITHOUT_APP_DATA,
+                }
+                .into());
             }
             let mut current_idx = 0;
             let (sp_header, _) = SpHeader::from_be_bytes(&slice[0..CCSDS_HEADER_LEN])?;
             current_idx += CCSDS_HEADER_LEN;
             let total_len = sp_header.total_len();
             if raw_data_len < total_len || total_len < PUS_TC_MIN_LEN_WITHOUT_APP_DATA {
-                return Err(PusError::RawDataTooShort(raw_data_len));
+                return Err(ByteConversionError::FromSliceTooSmall {
+                    found: raw_data_len,
+                    expected: total_len,
+                }
+                .into());
             }
             let sec_header = zc::PusTcSecondaryHeader::from_bytes(
                 &slice[current_idx..current_idx + PUC_TC_SECONDARY_HEADER_LEN],
@@ -424,7 +435,7 @@ pub mod legacy_tc {
                 sp_header,
                 sec_header: PusTcSecondaryHeader::try_from(sec_header).unwrap(),
                 raw_data: Some(raw_data),
-                app_data: user_data_from_raw(current_idx, total_len, raw_data_len, slice)?,
+                app_data: user_data_from_raw(current_idx, total_len, slice)?,
                 calc_crc_on_serialization: false,
                 crc16: Some(crc_from_raw_data(raw_data)?),
             };
@@ -447,8 +458,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 +467,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(),
@@ -560,14 +571,14 @@ impl<'raw_data> PusTcCreator<'raw_data> {
     pub fn new(
         sp_header: &mut SpHeader,
         sec_header: PusTcSecondaryHeader,
-        app_data: Option<&'raw_data [u8]>,
+        app_data: &'raw_data [u8],
         set_ccsds_len: bool,
     ) -> Self {
         sp_header.set_packet_type(PacketType::Tc);
         sp_header.set_sec_header_flag();
         let mut pus_tc = Self {
             sp_header: *sp_header,
-            app_data: app_data.unwrap_or(&[]),
+            app_data,
             sec_header,
         };
         if set_ccsds_len {
@@ -588,11 +599,19 @@ impl<'raw_data> PusTcCreator<'raw_data> {
         Self::new(
             sph,
             PusTcSecondaryHeader::new(service, subservice, ACK_ALL, 0),
-            app_data,
+            app_data.unwrap_or(&[]),
             set_ccsds_len,
         )
     }
 
+    pub fn new_no_app_data(
+        sp_header: &mut SpHeader,
+        sec_header: PusTcSecondaryHeader,
+        set_ccsds_len: bool,
+    ) -> Self {
+        Self::new(sp_header, sec_header, &[], set_ccsds_len)
+    }
+
     pub fn sp_header(&self) -> &SpHeader {
         &self.sp_header
     }
@@ -619,7 +638,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 +672,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 +681,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(),
@@ -748,14 +767,29 @@ impl<'raw_data> PusTcReader<'raw_data> {
     pub fn new(slice: &'raw_data [u8]) -> Result<(Self, usize), PusError> {
         let raw_data_len = slice.len();
         if raw_data_len < PUS_TC_MIN_LEN_WITHOUT_APP_DATA {
-            return Err(PusError::RawDataTooShort(raw_data_len));
+            return Err(ByteConversionError::FromSliceTooSmall {
+                found: raw_data_len,
+                expected: PUS_TC_MIN_LEN_WITHOUT_APP_DATA,
+            }
+            .into());
         }
         let mut current_idx = 0;
         let (sp_header, _) = SpHeader::from_be_bytes(&slice[0..CCSDS_HEADER_LEN])?;
         current_idx += CCSDS_HEADER_LEN;
         let total_len = sp_header.total_len();
-        if raw_data_len < total_len || total_len < PUS_TC_MIN_LEN_WITHOUT_APP_DATA {
-            return Err(PusError::RawDataTooShort(raw_data_len));
+        if raw_data_len < total_len {
+            return Err(ByteConversionError::FromSliceTooSmall {
+                found: raw_data_len,
+                expected: total_len,
+            }
+            .into());
+        }
+        if total_len < PUS_TC_MIN_LEN_WITHOUT_APP_DATA {
+            return Err(ByteConversionError::FromSliceTooSmall {
+                found: total_len,
+                expected: PUS_TC_MIN_LEN_WITHOUT_APP_DATA,
+            }
+            .into());
         }
         let sec_header = zc::PusTcSecondaryHeader::from_bytes(
             &slice[current_idx..current_idx + PUC_TC_SECONDARY_HEADER_LEN],
@@ -767,7 +801,7 @@ impl<'raw_data> PusTcReader<'raw_data> {
             sp_header,
             sec_header: PusTcSecondaryHeader::try_from(sec_header).unwrap(),
             raw_data,
-            app_data: user_data_from_raw(current_idx, total_len, raw_data_len, slice)?,
+            app_data: user_data_from_raw(current_idx, total_len, slice)?,
             crc16: crc_from_raw_data(raw_data)?,
         };
         verify_crc16_ccitt_false_from_raw_to_pus_error(raw_data, pus_tc.crc16)?;
@@ -847,19 +881,22 @@ impl PartialEq<PusTcReader<'_>> for PusTcCreator<'_> {
 
 #[cfg(all(test, feature = "std"))]
 mod tests {
-    use crate::ecss::tc::{
-        GenericPusTcSecondaryHeader, PusTcCreator, PusTcReader, PusTcSecondaryHeader, ACK_ALL,
-    };
+    use std::error::Error;
+
+    use super::*;
     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::string::ToString;
     use alloc::vec::Vec;
+    #[cfg(feature = "serde")]
+    use postcard::{from_bytes, to_allocvec};
 
     fn base_ping_tc_full_ctor() -> PusTcCreator<'static> {
         let mut sph = SpHeader::tc_unseg(0x02, 0x34, 0).unwrap();
         let tc_header = PusTcSecondaryHeader::new_simple(17, 1);
-        PusTcCreator::new(&mut sph, tc_header, None, true)
+        PusTcCreator::new_no_app_data(&mut sph, tc_header, true)
     }
 
     fn base_ping_tc_simple_ctor() -> PusTcCreator<'static> {
@@ -886,6 +923,10 @@ mod tests {
             .write_to_bytes(test_buf.as_mut_slice())
             .expect("Error writing TC to buffer");
         assert_eq!(size, 13);
+        assert_eq!(
+            pus_tc.crc16().unwrap(),
+            u16::from_be_bytes(test_buf[size - 2..size].try_into().unwrap())
+        );
     }
 
     #[test]
@@ -905,6 +946,20 @@ mod tests {
         verify_crc_no_app_data(&test_buf);
     }
 
+    #[test]
+    fn test_writing_into_vec() {
+        let pus_tc = base_ping_tc_simple_ctor();
+        let tc_vec = pus_tc.to_vec().expect("Error writing TC to buffer");
+        assert_eq!(tc_vec.len(), 13);
+        let (tc_from_raw, size) = PusTcReader::new(tc_vec.as_slice())
+            .expect("Creating PUS TC struct from raw buffer failed");
+        assert_eq!(size, 13);
+        verify_test_tc_with_reader(&tc_from_raw, false, 13);
+        assert!(tc_from_raw.user_data().is_empty());
+        verify_test_tc_raw(&tc_vec);
+        verify_crc_no_app_data(&tc_vec);
+    }
+
     #[test]
     fn test_update_func() {
         let mut sph = SpHeader::tc_unseg(0x02, 0x34, 0).unwrap();
@@ -926,11 +981,31 @@ mod tests {
         assert_eq!(size, 16);
         verify_test_tc_with_reader(&tc_from_raw, true, 16);
         let user_data = tc_from_raw.user_data();
+        assert_eq!(tc_from_raw.user_data(), tc_from_raw.app_data());
+        assert_eq!(tc_from_raw.raw_data(), &test_buf[..size]);
+        assert_eq!(
+            tc_from_raw.crc16().unwrap(),
+            u16::from_be_bytes(test_buf[size - 2..size].try_into().unwrap())
+        );
         assert_eq!(user_data[0], 1);
         assert_eq!(user_data[1], 2);
         assert_eq!(user_data[2], 3);
     }
 
+    #[test]
+    fn test_reader_eq() {
+        let pus_tc = base_ping_tc_simple_ctor_with_app_data(&[1, 2, 3]);
+        let mut test_buf: [u8; 32] = [0; 32];
+        pus_tc
+            .write_to_bytes(test_buf.as_mut_slice())
+            .expect("Error writing TC to buffer");
+        let (tc_from_raw_0, _) =
+            PusTcReader::new(&test_buf).expect("Creating PUS TC struct from raw buffer failed");
+        let (tc_from_raw_1, _) =
+            PusTcReader::new(&test_buf).expect("Creating PUS TC struct from raw buffer failed");
+        assert_eq!(tc_from_raw_0, tc_from_raw_1);
+    }
+
     #[test]
     fn test_vec_ser_deser() {
         let pus_tc = base_ping_tc_simple_ctor();
@@ -951,10 +1026,19 @@ mod tests {
             .write_to_bytes(test_buf.as_mut_slice())
             .expect("Error writing TC to buffer");
         test_buf[12] = 0;
+        test_buf[11] = 0;
         let res = PusTcReader::new(&test_buf);
         assert!(res.is_err());
         let err = res.unwrap_err();
-        assert!(matches!(err, PusError::IncorrectCrc { .. }));
+        if let PusError::ChecksumFailure(crc) = err {
+            assert_eq!(crc, 0);
+            assert_eq!(
+                err.to_string(),
+                "checksum verification for crc16 0x0000 failed"
+            );
+        } else {
+            panic!("unexpected error {err}");
+        }
     }
 
     #[test]
@@ -990,15 +1074,21 @@ mod tests {
         let res = pus_tc.write_to_bytes(test_buf.as_mut_slice());
         assert!(res.is_err());
         let err = res.unwrap_err();
-        match err {
-            PusError::ByteConversion(err) => match err {
-                ByteConversionError::ToSliceTooSmall { found, expected } => {
-                    assert_eq!(expected, pus_tc.len_packed());
-                    assert_eq!(found, 12);
+        if let PusError::ByteConversion(e) = err {
+            assert_eq!(
+                e,
+                ByteConversionError::ToSliceTooSmall {
+                    found: 12,
+                    expected: 13
                 }
-                _ => panic!("Unexpected error"),
-            },
-            _ => panic!("Unexpected error"),
+            );
+            assert_eq!(
+                err.to_string(),
+                "pus error: target slice with size 12 is too small, expected size of at least 13"
+            );
+            assert_eq!(err.source().unwrap().to_string(), e.to_string());
+        } else {
+            panic!("unexpected error {err}");
         }
     }
 
@@ -1067,13 +1157,20 @@ mod tests {
 
     fn verify_test_tc_generic(tc: &(impl CcsdsPacket + PusPacket + GenericPusTcSecondaryHeader)) {
         assert_eq!(PusPacket::service(tc), 17);
+        assert_eq!(GenericPusTcSecondaryHeader::service(tc), 17);
         assert_eq!(PusPacket::subservice(tc), 1);
+        assert_eq!(GenericPusTcSecondaryHeader::subservice(tc), 1);
         assert!(tc.sec_header_flag());
         assert_eq!(PusPacket::pus_version(tc), PusC);
         assert_eq!(tc.seq_count(), 0x34);
         assert_eq!(tc.source_id(), 0);
         assert_eq!(tc.apid(), 0x02);
         assert_eq!(tc.ack_flags(), ACK_ALL);
+        assert_eq!(PusPacket::pus_version(tc), PusVersion::PusC);
+        assert_eq!(
+            GenericPusTcSecondaryHeader::pus_version(tc),
+            PusVersion::PusC
+        );
     }
     fn verify_test_tc_raw(slice: &impl AsRef<[u8]>) {
         // Reference comparison implementation:
@@ -1125,4 +1222,59 @@ mod tests {
         assert_eq!(pus_tc, PusTcReader::new(&buf).unwrap().0);
         assert_eq!(PusTcReader::new(&buf).unwrap().0, pus_tc);
     }
+
+    #[test]
+    fn test_ack_opts_from_raw() {
+        let ack_opts_raw = AckOpts::Start as u8;
+        let ack_opts = AckOpts::try_from(ack_opts_raw).unwrap();
+        assert_eq!(ack_opts, AckOpts::Start);
+    }
+
+    #[test]
+    fn test_reader_buf_too_small() {
+        let app_data = &[1, 2, 3, 4];
+        let pus_tc = base_ping_tc_simple_ctor_with_app_data(app_data);
+        let mut buf = [0; 32];
+        let written_len = pus_tc.write_to_bytes(&mut buf).unwrap();
+        let error = PusTcReader::new(&buf[0..PUS_TC_MIN_LEN_WITHOUT_APP_DATA + 1]);
+        assert!(error.is_err());
+        let error = error.unwrap_err();
+        if let PusError::ByteConversion(ByteConversionError::FromSliceTooSmall {
+            found,
+            expected,
+        }) = error
+        {
+            assert_eq!(found, PUS_TC_MIN_LEN_WITHOUT_APP_DATA + 1);
+            assert_eq!(expected, written_len);
+        } else {
+            panic!("unexpected error {error}")
+        }
+    }
+
+    #[test]
+    fn test_reader_input_too_small() {
+        let buf: [u8; 5] = [0; 5];
+        let error = PusTcReader::new(&buf);
+        assert!(error.is_err());
+        let error = error.unwrap_err();
+        if let PusError::ByteConversion(ByteConversionError::FromSliceTooSmall {
+            found,
+            expected,
+        }) = error
+        {
+            assert_eq!(found, 5);
+            assert_eq!(expected, PUS_TC_MIN_LEN_WITHOUT_APP_DATA);
+        } else {
+            panic!("unexpected error {error}")
+        }
+    }
+
+    #[test]
+    #[cfg(feature = "serde")]
+    fn test_serialization_tc_serde() {
+        let pus_tc = base_ping_tc_simple_ctor();
+        let output = to_allocvec(&pus_tc).unwrap();
+        let output_converted_back: PusTcCreator = from_bytes(&output).unwrap();
+        assert_eq!(output_converted_back, pus_tc);
+    }
 }

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
@@ -381,14 +381,29 @@ pub mod legacy_tm {
         ) -> Result<(Self, usize), PusError> {
             let raw_data_len = slice.len();
             if raw_data_len < PUS_TM_MIN_LEN_WITHOUT_SOURCE_DATA {
-                return Err(PusError::RawDataTooShort(raw_data_len));
+                return Err(ByteConversionError::FromSliceTooSmall {
+                    found: raw_data_len,
+                    expected: PUS_TM_MIN_LEN_WITHOUT_SOURCE_DATA,
+                }
+                .into());
             }
             let mut current_idx = 0;
             let (sp_header, _) = SpHeader::from_be_bytes(&slice[0..CCSDS_HEADER_LEN])?;
             current_idx += 6;
             let total_len = sp_header.total_len();
-            if raw_data_len < total_len || total_len < PUS_TM_MIN_LEN_WITHOUT_SOURCE_DATA {
-                return Err(PusError::RawDataTooShort(raw_data_len));
+            if raw_data_len < total_len {
+                return Err(ByteConversionError::FromSliceTooSmall {
+                    found: raw_data_len,
+                    expected: PUS_TM_MIN_LEN_WITHOUT_SOURCE_DATA,
+                }
+                .into());
+            }
+            if total_len < PUS_TM_MIN_LEN_WITHOUT_SOURCE_DATA {
+                return Err(ByteConversionError::FromSliceTooSmall {
+                    found: total_len,
+                    expected: PUS_TM_MIN_LEN_WITHOUT_SOURCE_DATA,
+                }
+                .into());
             }
             let sec_header_zc = zc::PusTmSecHeaderWithoutTimestamp::from_bytes(
                 &slice[current_idx..current_idx + PUC_TM_MIN_SEC_HEADER_LEN],
@@ -405,7 +420,7 @@ pub mod legacy_tm {
                 sp_header,
                 sec_header: PusTmSecondaryHeader::try_from(zc_sec_header_wrapper).unwrap(),
                 raw_data: Some(&slice[0..total_len]),
-                source_data: user_data_from_raw(current_idx, total_len, raw_data_len, slice)?,
+                source_data: user_data_from_raw(current_idx, total_len, slice)?,
                 calc_crc_on_serialization: false,
                 crc16: Some(crc_from_raw_data(raw_data)?),
             };
@@ -423,8 +438,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 +447,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(),
@@ -541,21 +556,21 @@ impl<'raw_data> PusTmCreator<'raw_data> {
     ///     automatically
     /// * `sec_header` - Information contained in the secondary header, including the service
     ///     and subservice type
-    /// * `app_data` - Custom application data
+    /// * `source_data` - Custom application data
     /// * `set_ccsds_len` - Can be used to automatically update the CCSDS space packet data length
     ///     field. If this is not set to true, [PusTm::update_ccsds_data_len] can be called to set
     ///     the correct value to this field manually
     pub fn new(
         sp_header: &mut SpHeader,
         sec_header: PusTmSecondaryHeader<'raw_data>,
-        source_data: Option<&'raw_data [u8]>,
+        source_data: &'raw_data [u8],
         set_ccsds_len: bool,
     ) -> Self {
         sp_header.set_packet_type(PacketType::Tm);
         sp_header.set_sec_header_flag();
         let mut pus_tm = Self {
             sp_header: *sp_header,
-            source_data: source_data.unwrap_or(&[]),
+            source_data,
             sec_header,
             calc_crc_on_serialization: true,
         };
@@ -577,8 +592,22 @@ impl<'raw_data> PusTmCreator<'raw_data> {
         let stamp_size = time_provider.write_to_bytes(stamp_buf)?;
         let sec_header =
             PusTmSecondaryHeader::new_simple(service, subservice, &stamp_buf[0..stamp_size]);
-        Ok(Self::new(sp_header, sec_header, source_data, set_ccsds_len))
+        Ok(Self::new(
+            sp_header,
+            sec_header,
+            source_data.unwrap_or(&[]),
+            set_ccsds_len,
+        ))
     }
+
+    pub fn new_no_source_data(
+        sp_header: &mut SpHeader,
+        sec_header: PusTmSecondaryHeader<'raw_data>,
+        set_ccsds_len: bool,
+    ) -> Self {
+        Self::new(sp_header, sec_header, &[], set_ccsds_len)
+    }
+
     pub fn timestamp(&self) -> &[u8] {
         self.sec_header.timestamp
     }
@@ -608,7 +637,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 +679,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 +688,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(),
@@ -760,14 +789,29 @@ impl<'raw_data> PusTmReader<'raw_data> {
     pub fn new(slice: &'raw_data [u8], timestamp_len: usize) -> Result<(Self, usize), PusError> {
         let raw_data_len = slice.len();
         if raw_data_len < PUS_TM_MIN_LEN_WITHOUT_SOURCE_DATA {
-            return Err(PusError::RawDataTooShort(raw_data_len));
+            return Err(ByteConversionError::FromSliceTooSmall {
+                found: raw_data_len,
+                expected: PUS_TM_MIN_LEN_WITHOUT_SOURCE_DATA,
+            }
+            .into());
         }
         let mut current_idx = 0;
         let (sp_header, _) = SpHeader::from_be_bytes(&slice[0..CCSDS_HEADER_LEN])?;
         current_idx += 6;
         let total_len = sp_header.total_len();
-        if raw_data_len < total_len || total_len < PUS_TM_MIN_LEN_WITHOUT_SOURCE_DATA {
-            return Err(PusError::RawDataTooShort(raw_data_len));
+        if raw_data_len < total_len {
+            return Err(ByteConversionError::FromSliceTooSmall {
+                found: raw_data_len,
+                expected: total_len,
+            }
+            .into());
+        }
+        if total_len < PUS_TM_MIN_LEN_WITHOUT_SOURCE_DATA {
+            return Err(ByteConversionError::FromSliceTooSmall {
+                found: total_len,
+                expected: PUS_TM_MIN_LEN_WITHOUT_SOURCE_DATA,
+            }
+            .into());
         }
         let sec_header_zc = zc::PusTmSecHeaderWithoutTimestamp::from_bytes(
             &slice[current_idx..current_idx + PUC_TM_MIN_SEC_HEADER_LEN],
@@ -784,7 +828,7 @@ impl<'raw_data> PusTmReader<'raw_data> {
             sp_header,
             sec_header: PusTmSecondaryHeader::try_from(zc_sec_header_wrapper).unwrap(),
             raw_data: &slice[0..total_len],
-            source_data: user_data_from_raw(current_idx, total_len, raw_data_len, slice)?,
+            source_data: user_data_from_raw(current_idx, total_len, slice)?,
             crc16: crc_from_raw_data(raw_data)?,
         };
         verify_crc16_ccitt_false_from_raw_to_pus_error(raw_data, pus_tm.crc16)?;
@@ -811,7 +855,10 @@ impl<'raw_data> PusTmReader<'raw_data> {
 
 impl PartialEq for PusTmReader<'_> {
     fn eq(&self, other: &Self) -> bool {
-        self.raw_data == other.raw_data
+        self.sec_header == other.sec_header
+            && self.source_data == other.source_data
+            && self.sp_header == other.sp_header
+            && self.crc16 == other.crc16
     }
 }
 
@@ -943,43 +990,60 @@ impl<'raw> PusTmZeroCopyWriter<'raw> {
 
 #[cfg(test)]
 mod tests {
+    use alloc::string::ToString;
+
     use super::*;
     use crate::ecss::PusVersion::PusC;
+    use crate::time::cds::TimeProvider;
+    #[cfg(feature = "serde")]
+    use crate::time::CcsdsTimeProvider;
     use crate::SpHeader;
+    #[cfg(feature = "serde")]
+    use postcard::{from_bytes, to_allocvec};
 
     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);
-        PusTmCreator::new(&mut sph, tm_header, None, true)
+        let tm_header = PusTmSecondaryHeader::new_simple(17, 2, timestamp);
+        PusTmCreator::new_no_source_data(&mut sph, tm_header, 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);
-        PusTmCreator::new(&mut sph, tc_header, Some(src_data), true)
+        let tc_header = PusTmSecondaryHeader::new_simple(3, 5, timestamp);
+        PusTmCreator::new(&mut sph, tc_header, 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_basic_simple_api() {
+        let mut sph = SpHeader::tm_unseg(0x123, 0x234, 0).unwrap();
+        let time_provider = TimeProvider::new_with_u16_days(0, 0);
+        let mut stamp_buf: [u8; 8] = [0; 8];
+        let pus_tm =
+            PusTmCreator::new_simple(&mut sph, 17, 2, &time_provider, &mut stamp_buf, None, true)
+                .unwrap();
+        verify_ping_reply(&pus_tm, false, 22, &[64, 0, 0, 0, 0, 0, 0]);
+    }
 
     #[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)
             .expect("Serialization failed");
         assert_eq!(ser_len, 22);
-        verify_raw_ping_reply(&buf);
+        verify_raw_ping_reply(pus_tm.crc16().unwrap(), &buf);
     }
 
     #[test]
@@ -999,7 +1063,7 @@ mod tests {
     #[test]
     fn test_setters() {
         let timestamp = dummy_timestamp();
-        let mut pus_tm = base_ping_reply_full_ctor(&timestamp);
+        let mut pus_tm = base_ping_reply_full_ctor(timestamp);
         pus_tm.set_sc_time_ref_status(0b1010);
         pus_tm.set_dest_id(0x7fff);
         pus_tm.set_msg_counter(0x1f1f);
@@ -1010,10 +1074,21 @@ mod tests {
         assert_eq!(pus_tm.apid(), 0x7ff);
     }
 
+    #[test]
+    fn test_write_into_vec() {
+        let timestamp = dummy_timestamp();
+        let pus_tm = base_ping_reply_full_ctor(timestamp);
+        let tm_vec = pus_tm.to_vec().expect("Serialization failed");
+        assert_eq!(tm_vec.len(), 22);
+        let (tm_deserialized, size) =
+            PusTmReader::new(tm_vec.as_slice(), 7).expect("Deserialization failed");
+        assert_eq!(tm_vec.len(), size);
+        verify_ping_reply_with_reader(&tm_deserialized, false, 22, dummy_timestamp());
+    }
     #[test]
     fn test_deserialization_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)
@@ -1021,14 +1096,39 @@ mod tests {
         assert_eq!(ser_len, 22);
         let (tm_deserialized, size) = PusTmReader::new(&buf, 7).expect("Deserialization failed");
         assert_eq!(ser_len, size);
+        assert_eq!(tm_deserialized.user_data(), tm_deserialized.source_data());
+        assert_eq!(tm_deserialized.raw_data(), &buf[..ser_len]);
+        assert_eq!(tm_deserialized.crc16().unwrap(), pus_tm.crc16().unwrap());
         verify_ping_reply_with_reader(&tm_deserialized, false, 22, dummy_timestamp());
     }
+    #[test]
+    fn test_deserialization_faulty_crc() {
+        let timestamp = dummy_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)
+            .expect("Serialization failed");
+        assert_eq!(ser_len, 22);
+        buf[ser_len - 2] = 0;
+        buf[ser_len - 1] = 0;
+        let tm_error = PusTmReader::new(&buf, 7);
+        assert!(tm_error.is_err());
+        let tm_error = tm_error.unwrap_err();
+        if let PusError::ChecksumFailure(crc) = tm_error {
+            assert_eq!(crc, 0);
+            assert_eq!(
+                tm_error.to_string(),
+                "checksum verification for crc16 0x0000 failed"
+            );
+        }
+    }
 
     #[test]
     fn test_manual_field_update() {
         let mut sph = SpHeader::tm_unseg(0x123, 0x234, 0).unwrap();
         let tc_header = PusTmSecondaryHeader::new_simple(17, 2, dummy_timestamp());
-        let mut tm = PusTmCreator::new(&mut sph, tc_header, None, false);
+        let mut tm = PusTmCreator::new_no_source_data(&mut sph, tc_header, false);
         tm.calc_crc_on_serialization = false;
         assert_eq!(tm.data_len(), 0x00);
         let mut buf: [u8; 32] = [0; 32];
@@ -1045,7 +1145,7 @@ mod tests {
     #[test]
     fn test_target_buf_too_small() {
         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; 16] = [0; 16];
         let res = pus_tm.write_to_bytes(&mut buf);
         assert!(res.is_err());
@@ -1069,12 +1169,12 @@ 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());
         assert_eq!(res.unwrap(), 22);
-        verify_raw_ping_reply(vec.as_slice());
+        verify_raw_ping_reply(pus_tm.crc16().unwrap(), vec.as_slice());
     }
 
     #[test]
@@ -1090,7 +1190,7 @@ mod tests {
         assert_eq!(vec.len(), 26);
     }
 
-    fn verify_raw_ping_reply(buf: &[u8]) {
+    fn verify_raw_ping_reply(crc16: u16, buf: &[u8]) {
         // Secondary header is set -> 0b0000_1001 , APID occupies last bit of first byte
         assert_eq!(buf[0], 0x09);
         // Rest of APID 0x123
@@ -1113,9 +1213,10 @@ mod tests {
         assert_eq!(&buf[13..20], dummy_timestamp());
         let mut digest = CRC_CCITT_FALSE.digest();
         digest.update(&buf[0..20]);
-        let crc16 = digest.finalize();
+        let crc16_calced = digest.finalize();
         assert_eq!(((crc16 >> 8) & 0xff) as u8, buf[20]);
         assert_eq!((crc16 & 0xff) as u8, buf[21]);
+        assert_eq!(crc16, crc16_calced);
     }
 
     fn verify_ping_reply(
@@ -1124,8 +1225,9 @@ 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);
+        assert_eq!(tm.source_data(), tm.user_data());
         verify_ping_reply_generic(tm, has_user_data, exp_full_len);
     }
 
@@ -1147,7 +1249,9 @@ mod tests {
     ) {
         assert!(tm.is_tm());
         assert_eq!(PusPacket::service(tm), 17);
+        assert_eq!(GenericPusTmSecondaryHeader::service(tm), 17);
         assert_eq!(PusPacket::subservice(tm), 2);
+        assert_eq!(GenericPusTmSecondaryHeader::subservice(tm), 2);
         assert!(tm.sec_header_flag());
         if has_user_data {
             assert!(!tm.user_data().is_empty());
@@ -1155,6 +1259,11 @@ mod tests {
         assert_eq!(PusPacket::pus_version(tm), PusC);
         assert_eq!(tm.apid(), 0x123);
         assert_eq!(tm.seq_count(), 0x234);
+        assert_eq!(PusPacket::pus_version(tm), PusVersion::PusC);
+        assert_eq!(
+            GenericPusTmSecondaryHeader::pus_version(tm),
+            PusVersion::PusC
+        );
         assert_eq!(tm.data_len(), exp_full_len as u16 - 7);
         assert_eq!(tm.dest_id(), 0x0000);
         assert_eq!(tm.msg_counter(), 0x0000);
@@ -1191,6 +1300,10 @@ mod tests {
         writer.set_msg_count(100);
         writer.set_seq_count(MAX_SEQ_COUNT);
         writer.set_apid(MAX_APID);
+        assert!(!writer.set_apid(MAX_APID + 1));
+        assert!(!writer.set_apid(MAX_SEQ_COUNT + 1));
+        assert_eq!(writer.service(), 17);
+        assert_eq!(writer.subservice(), 2);
         writer.finish();
         // This performs all necessary checks, including the CRC check.
         let (tm_read_back, tm_size_read_back) =
@@ -1201,4 +1314,95 @@ mod tests {
         assert_eq!(tm_read_back.seq_count(), MAX_SEQ_COUNT);
         assert_eq!(tm_read_back.apid(), MAX_APID);
     }
+
+    #[test]
+    fn test_sec_header_without_stamp() {
+        let sec_header = PusTmSecondaryHeader::new_simple_no_timestamp(17, 1);
+        assert_eq!(sec_header.timestamp, &[]);
+    }
+
+    #[test]
+    fn test_reader_partial_eq() {
+        let timestamp = dummy_timestamp();
+        let pus_tm = base_ping_reply_full_ctor(timestamp);
+        let mut buf = [0; 32];
+        pus_tm.write_to_bytes(&mut buf).unwrap();
+        let (tm_0, _) = PusTmReader::new(&buf, timestamp.len()).unwrap();
+        let (tm_1, _) = PusTmReader::new(&buf, timestamp.len()).unwrap();
+        assert_eq!(tm_0, tm_1);
+    }
+    #[test]
+    fn test_reader_buf_too_small_2() {
+        let timestamp = dummy_timestamp();
+        let pus_tm = base_ping_reply_full_ctor(timestamp);
+        let mut buf = [0; 32];
+        let written = pus_tm.write_to_bytes(&mut buf).unwrap();
+        let tm_error = PusTmReader::new(
+            &buf[0..PUS_TM_MIN_LEN_WITHOUT_SOURCE_DATA + 1],
+            timestamp.len(),
+        );
+        assert!(tm_error.is_err());
+        let tm_error = tm_error.unwrap_err();
+        if let PusError::ByteConversion(ByteConversionError::FromSliceTooSmall {
+            found,
+            expected,
+        }) = tm_error
+        {
+            assert_eq!(found, PUS_TM_MIN_LEN_WITHOUT_SOURCE_DATA + 1);
+            assert_eq!(expected, written);
+        } else {
+            panic!("unexpected error {tm_error}")
+        }
+    }
+    #[test]
+    fn test_reader_buf_too_small() {
+        let timestamp = dummy_timestamp();
+        let pus_tm = base_ping_reply_full_ctor(timestamp);
+        let mut buf = [0; 32];
+        pus_tm.write_to_bytes(&mut buf).unwrap();
+        let tm_error = PusTmReader::new(&buf[0..5], timestamp.len());
+        assert!(tm_error.is_err());
+        let tm_error = tm_error.unwrap_err();
+        if let PusError::ByteConversion(ByteConversionError::FromSliceTooSmall {
+            found,
+            expected,
+        }) = tm_error
+        {
+            assert_eq!(found, 5);
+            assert_eq!(expected, PUS_TM_MIN_LEN_WITHOUT_SOURCE_DATA);
+        } else {
+            panic!("unexpected error {tm_error}")
+        }
+    }
+
+    #[test]
+    #[cfg(feature = "serde")]
+    fn test_serialization_creator_serde() {
+        let mut sph = SpHeader::tm_unseg(0x123, 0x234, 0).unwrap();
+        let time_provider = TimeProvider::new_with_u16_days(0, 0);
+        let mut stamp_buf: [u8; 8] = [0; 8];
+        let pus_tm =
+            PusTmCreator::new_simple(&mut sph, 17, 2, &time_provider, &mut stamp_buf, None, true)
+                .unwrap();
+
+        let output = to_allocvec(&pus_tm).unwrap();
+        let output_converted_back: PusTmCreator = from_bytes(&output).unwrap();
+        assert_eq!(output_converted_back, pus_tm);
+    }
+
+    #[test]
+    #[cfg(feature = "serde")]
+    fn test_serialization_reader_serde() {
+        let mut sph = SpHeader::tm_unseg(0x123, 0x234, 0).unwrap();
+        let time_provider = TimeProvider::new_with_u16_days(0, 0);
+        let mut stamp_buf: [u8; 8] = [0; 8];
+        let pus_tm =
+            PusTmCreator::new_simple(&mut sph, 17, 2, &time_provider, &mut stamp_buf, None, true)
+                .unwrap();
+        let pus_tm_vec = pus_tm.to_vec().unwrap();
+        let (tm_reader, _) = PusTmReader::new(&pus_tm_vec, time_provider.len_as_bytes()).unwrap();
+        let output = to_allocvec(&tm_reader).unwrap();
+        let output_converted_back: PusTmReader = from_bytes(&output).unwrap();
+        assert_eq!(output_converted_back, tm_reader);
+    }
 }

src/lib.rs

@@ -206,7 +206,7 @@ impl PartialEq for PacketId {
 
 impl PartialOrd for PacketId {
     fn partial_cmp(&self, other: &Self) -> Option<core::cmp::Ordering> {
-        self.raw().partial_cmp(&other.raw())
+        Some(self.cmp(other))
     }
 }
 
@@ -749,7 +749,7 @@ pub mod zc {
 }
 
 #[cfg(all(test, feature = "std"))]
-mod tests {
+pub(crate) mod tests {
     use std::collections::HashSet;
 
     #[cfg(feature = "serde")]
@@ -759,9 +759,13 @@ mod tests {
     };
     use crate::{SequenceFlags, SpHeader};
     use alloc::vec;
+    #[cfg(feature = "serde")]
+    use core::fmt::Debug;
     use num_traits::pow;
     #[cfg(feature = "serde")]
     use postcard::{from_bytes, to_allocvec};
+    #[cfg(feature = "serde")]
+    use serde::{de::DeserializeOwned, Serialize};
 
     const CONST_SP: SpHeader = SpHeader::new(
         PacketId::const_tc(true, 0x36),
@@ -771,10 +775,19 @@ mod tests {
 
     const PACKET_ID_TM: PacketId = PacketId::const_tm(true, 0x22);
 
+    #[cfg(feature = "serde")]
+    pub(crate) fn generic_serde_test<T: Serialize + DeserializeOwned + PartialEq + Debug>(
+        value: T,
+    ) {
+        let output: alloc::vec::Vec<u8> = to_allocvec(&value).unwrap();
+        let output_converted_back: T = from_bytes(&output).unwrap();
+        assert_eq!(output_converted_back, value);
+    }
+
     #[test]
     fn verify_const_packet_id() {
         assert_eq!(PACKET_ID_TM.apid(), 0x22);
-        assert_eq!(PACKET_ID_TM.sec_header_flag, true);
+        assert!(PACKET_ID_TM.sec_header_flag);
         assert_eq!(PACKET_ID_TM.ptype, PacketType::Tm);
         let const_tc_id = PacketId::const_tc(true, 0x23);
         assert_eq!(const_tc_id.ptype, PacketType::Tc);
@@ -880,6 +893,7 @@ mod tests {
     fn test_invalid_seq_count() {
         let mut psc = PacketSequenceCtrl::new(SequenceFlags::ContinuationSegment, 77)
             .expect("PSC creation failed");
+        assert_eq!(psc.seq_count(), 77);
         assert!(!psc.set_seq_count(0xffff));
     }
 

src/time/cuc.rs

@@ -7,6 +7,7 @@ use chrono::Datelike;
 use core::fmt::Debug;
 use core::ops::{Add, AddAssign};
 use core::time::Duration;
+use core::u64;
 
 const MIN_CUC_LEN: usize = 2;
 
@@ -93,10 +94,15 @@ pub fn fractional_part_from_subsec_ns(
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 pub enum CucError {
     InvalidCounterWidth(u8),
-    InvalidFractionResolution(FractionalResolution),
     /// Invalid counter supplied.
-    InvalidCounter(u8, u64),
-    InvalidFractions(FractionalResolution, u64),
+    InvalidCounter {
+        width: u8,
+        counter: u64,
+    },
+    InvalidFractions {
+        resolution: FractionalResolution,
+        value: u64,
+    },
 }
 
 impl Display for CucError {
@@ -105,14 +111,14 @@ impl Display for CucError {
             CucError::InvalidCounterWidth(w) => {
                 write!(f, "invalid cuc counter byte width {w}")
             }
-            CucError::InvalidFractionResolution(w) => {
-                write!(f, "invalid cuc fractional part byte width {w:?}")
+            CucError::InvalidCounter { width, counter } => {
+                write!(f, "invalid cuc counter {counter} for width {width}")
             }
-            CucError::InvalidCounter(w, c) => {
-                write!(f, "invalid cuc counter {c} for width {w}")
-            }
-            CucError::InvalidFractions(w, c) => {
-                write!(f, "invalid cuc fractional part {c} for width {w:?}")
+            CucError::InvalidFractions { resolution, value } => {
+                write!(
+                    f,
+                    "invalid cuc fractional part {value} for resolution {resolution:?}"
+                )
             }
         }
     }
@@ -278,6 +284,8 @@ impl TimeProviderCcsdsEpoch {
         .map_err(|e| e.into())
     }
 
+    /// Generates a CUC timestamp from a UNIX timestamp with a width of 4. This width is able
+    /// to accomodate all possible UNIX timestamp values.
     pub fn from_unix_stamp(
         unix_stamp: &UnixTimestamp,
         res: FractionalResolution,
@@ -289,9 +297,6 @@ impl TimeProviderCcsdsEpoch {
                 unix_stamp.as_date_time().unwrap(),
             ));
         }
-        if ccsds_epoch > u32::MAX as i64 {
-            return Err(CucError::InvalidCounter(4, ccsds_epoch as u64).into());
-        }
         let mut fractions = None;
         if let Some(subsec_millis) = unix_stamp.subsecond_millis {
             fractions = fractional_part_from_subsec_ns(res, subsec_millis as u64 * 10_u64.pow(6));
@@ -308,12 +313,19 @@ impl TimeProviderCcsdsEpoch {
         self.counter
     }
 
+    pub fn width(&self) -> u8 {
+        self.counter.0
+    }
+
+    pub fn counter(&self) -> u32 {
+        self.counter.1
+    }
+
     pub fn width_fractions_pair(&self) -> Option<FractionalPart> {
         self.fractions
     }
 
     pub fn set_fractions(&mut self, fractions: FractionalPart) -> Result<(), CucError> {
-        Self::verify_fractions_width(fractions.0)?;
         Self::verify_fractions_value(fractions)?;
         self.fractions = Some(fractions);
         self.update_p_field_fractions();
@@ -343,10 +355,12 @@ impl TimeProviderCcsdsEpoch {
     ) -> Result<Self, CucError> {
         Self::verify_counter_width(counter.0)?;
         if counter.1 > (2u64.pow(counter.0 as u32 * 8) - 1) as u32 {
-            return Err(CucError::InvalidCounter(counter.0, counter.1 as u64));
+            return Err(CucError::InvalidCounter {
+                width: counter.0,
+                counter: counter.1 as u64,
+            });
         }
         if let Some(fractions) = fractions {
-            Self::verify_fractions_width(fractions.0)?;
             Self::verify_fractions_value(fractions)?;
         }
         Ok(Self {
@@ -432,16 +446,12 @@ impl TimeProviderCcsdsEpoch {
         Ok(())
     }
 
-    fn verify_fractions_width(width: FractionalResolution) -> Result<(), CucError> {
-        if width as u8 > 3 {
-            return Err(CucError::InvalidFractionResolution(width));
-        }
-        Ok(())
-    }
-
     fn verify_fractions_value(val: FractionalPart) -> Result<(), CucError> {
         if val.1 > 2u32.pow((val.0 as u32) * 8) - 1 {
-            return Err(CucError::InvalidFractions(val.0, val.1 as u64));
+            return Err(CucError::InvalidFractions {
+                resolution: val.0,
+                value: val.1 as u64,
+            });
         }
         Ok(())
     }
@@ -713,6 +723,7 @@ impl Add<Duration> for &TimeProviderCcsdsEpoch {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use alloc::string::ToString;
     use chrono::{Datelike, Timelike};
     #[allow(unused_imports)]
     use std::println;
@@ -721,6 +732,8 @@ mod tests {
     fn test_basic_zero_epoch() {
         let zero_cuc = TimeProviderCcsdsEpoch::new(0);
         assert_eq!(zero_cuc.len_as_bytes(), 5);
+        assert_eq!(zero_cuc.width(), zero_cuc.width_counter_pair().0);
+        assert_eq!(zero_cuc.counter(), zero_cuc.width_counter_pair().1);
         assert_eq!(zero_cuc.ccdsd_time_code(), CcsdsTimeCodes::CucCcsdsEpoch);
         let counter = zero_cuc.width_counter_pair();
         assert_eq!(counter.0, 4);
@@ -861,6 +874,7 @@ mod tests {
 
     #[test]
     fn invalid_buf_len_for_read() {}
+
     #[test]
     fn write_read_three_byte_cntr_stamp() {
         let mut buf = [0; 4];
@@ -1102,12 +1116,7 @@ mod tests {
         assert_eq!(fractions.1, 2_u32.pow(3 * 8) - 2);
     }
 
-    #[test]
-    fn add_duration_basic() {
-        let mut cuc_stamp = TimeProviderCcsdsEpoch::new(200);
-        cuc_stamp.set_fractional_resolution(FractionalResolution::FifteenUs);
-        let duration = Duration::from_millis(2500);
-        cuc_stamp += duration;
+    fn check_stamp_after_addition(cuc_stamp: &TimeProviderCcsdsEpoch) {
         assert_eq!(cuc_stamp.width_counter_pair().1, 202);
         let fractions = cuc_stamp.width_fractions_pair().unwrap().1;
         let expected_val =
@@ -1120,6 +1129,41 @@ mod tests {
         assert!(cuc_stamp2.subsecond_millis().unwrap() <= 1);
     }
 
+    #[test]
+    fn add_duration_basic() {
+        let mut cuc_stamp = TimeProviderCcsdsEpoch::new(200);
+        cuc_stamp.set_fractional_resolution(FractionalResolution::FifteenUs);
+        let duration = Duration::from_millis(2500);
+        cuc_stamp += duration;
+        check_stamp_after_addition(&cuc_stamp);
+    }
+
+    #[test]
+    fn add_duration_basic_on_ref() {
+        let mut cuc_stamp = TimeProviderCcsdsEpoch::new(200);
+        cuc_stamp.set_fractional_resolution(FractionalResolution::FifteenUs);
+        let duration = Duration::from_millis(2500);
+        let new_stamp = cuc_stamp + duration;
+        check_stamp_after_addition(&new_stamp);
+    }
+
+    #[test]
+    fn add_duration_basic_no_fractions() {
+        let mut cuc_stamp = TimeProviderCcsdsEpoch::new(200);
+        let duration = Duration::from_millis(2000);
+        cuc_stamp += duration;
+        assert_eq!(cuc_stamp.counter(), 202);
+        assert_eq!(cuc_stamp.width_fractions_pair(), None);
+    }
+
+    #[test]
+    fn add_duration_basic_on_ref_no_fractions() {
+        let cuc_stamp = TimeProviderCcsdsEpoch::new(200);
+        let duration = Duration::from_millis(2000);
+        let new_stamp = cuc_stamp + duration;
+        assert_eq!(new_stamp.counter(), 202);
+        assert_eq!(new_stamp.width_fractions_pair(), None);
+    }
     #[test]
     fn add_duration_overflow() {
         let mut cuc_stamp =
@@ -1128,4 +1172,58 @@ mod tests {
         cuc_stamp += duration;
         assert_eq!(cuc_stamp.counter.1, 10);
     }
+
+    #[test]
+    fn test_invalid_width_param() {
+        let error = TimeProviderCcsdsEpoch::new_generic(WidthCounterPair(8, 0), None);
+        assert!(error.is_err());
+        let error = error.unwrap_err();
+        if let CucError::InvalidCounterWidth(width) = error {
+            assert_eq!(width, 8);
+            assert_eq!(error.to_string(), "invalid cuc counter byte width 8");
+        } else {
+            panic!("unexpected error: {}", error);
+        }
+    }
+
+    #[test]
+    fn test_from_dt() {
+        let dt = Utc.with_ymd_and_hms(2021, 1, 1, 0, 0, 0).unwrap();
+        let cuc =
+            TimeProviderCcsdsEpoch::from_date_time(&dt, FractionalResolution::Seconds).unwrap();
+        assert_eq!(cuc.counter(), dt.timestamp() as u32);
+    }
+
+    #[test]
+    fn test_new_u16_width() {
+        let cuc = TimeProviderCcsdsEpoch::new_u16_counter(0);
+        assert_eq!(cuc.width(), 2);
+        assert_eq!(cuc.counter(), 0);
+    }
+
+    #[test]
+    fn from_unix_stamp() {
+        let unix_stamp = UnixTimestamp::new(0, 0).unwrap();
+        let cuc =
+            TimeProviderCcsdsEpoch::from_unix_stamp(&unix_stamp, FractionalResolution::Seconds)
+                .expect("failed to create cuc from unix stamp");
+        assert_eq!(
+            cuc.counter(),
+            (-DAYS_CCSDS_TO_UNIX * SECONDS_PER_DAY as i32) as u32
+        );
+    }
+
+    #[test]
+    fn test_invalid_counter() {
+        let cuc_error = TimeProviderCcsdsEpoch::new_generic(WidthCounterPair(1, 256), None);
+        assert!(cuc_error.is_err());
+        let cuc_error = cuc_error.unwrap_err();
+        if let CucError::InvalidCounter { width, counter } = cuc_error {
+            assert_eq!(width, 1);
+            assert_eq!(counter, 256);
+            assert_eq!(cuc_error.to_string(), "invalid cuc counter 256 for width 1");
+        } else {
+            panic!("unexpected error: {}", cuc_error);
+        }
+    }
 }

src/time/mod.rs

@@ -82,13 +82,13 @@ impl Display for TimestampError {
                 )
             }
             TimestampError::Cds(e) => {
-                write!(f, "cds error {e}")
+                write!(f, "cds error: {e}")
             }
             TimestampError::Cuc(e) => {
-                write!(f, "cuc error {e}")
+                write!(f, "cuc error: {e}")
             }
             TimestampError::ByteConversion(e) => {
-                write!(f, "byte conversion error {e}")
+                write!(f, "time stamp: {e}")
             }
             TimestampError::DateBeforeCcsdsEpoch(e) => {
                 write!(f, "datetime with date before ccsds epoch: {e}")
@@ -307,12 +307,18 @@ impl From<DateTime<Utc>> for UnixTimestamp {
 
 impl PartialOrd for UnixTimestamp {
     fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
+impl Ord for UnixTimestamp {
+    fn cmp(&self, other: &Self) -> Ordering {
         if self == other {
-            return Some(Ordering::Equal);
+            return Ordering::Equal;
         }
         match self.unix_seconds.cmp(&other.unix_seconds) {
-            Ordering::Less => return Some(Ordering::Less),
-            Ordering::Greater => return Some(Ordering::Greater),
+            Ordering::Less => return Ordering::Less,
+            Ordering::Greater => return Ordering::Greater,
             _ => (),
         }
 
@@ -323,27 +329,21 @@ impl PartialOrd for UnixTimestamp {
         {
             Ordering::Less => {
                 return if self.unix_seconds < 0 {
-                    Some(Ordering::Greater)
+                    Ordering::Greater
                 } else {
-                    Some(Ordering::Less)
+                    Ordering::Less
                 }
             }
             Ordering::Greater => {
                 return if self.unix_seconds < 0 {
-                    Some(Ordering::Less)
+                    Ordering::Less
                 } else {
-                    Some(Ordering::Greater)
+                    Ordering::Greater
                 }
             }
             Ordering::Equal => (),
         }
-        Some(Ordering::Equal)
-    }
-}
-
-impl Ord for UnixTimestamp {
-    fn cmp(&self, other: &Self) -> Ordering {
-        PartialOrd::partial_cmp(self, other).unwrap()
+        Ordering::Equal
     }
 }
 
@@ -412,7 +412,11 @@ impl Add<Duration> for &UnixTimestamp {
 
 #[cfg(all(test, feature = "std"))]
 mod tests {
-    use super::*;
+    use alloc::string::ToString;
+    use chrono::{Datelike, Timelike};
+    use std::format;
+
+    use super::{cuc::CucError, *};
 
     #[test]
     fn test_days_conversion() {
@@ -426,6 +430,14 @@ mod tests {
         assert!(sec_floats > 0.0);
     }
 
+    #[test]
+    fn test_ms_of_day() {
+        let ms = ms_of_day(0.0);
+        assert_eq!(ms, 0);
+        let ms = ms_of_day(5.0);
+        assert_eq!(ms, 5000);
+    }
+
     #[test]
     fn test_ccsds_epoch() {
         let now = SystemTime::now()
@@ -534,6 +546,25 @@ mod tests {
         assert!(stamp1.subsecond_millis().is_none());
     }
 
+    #[test]
+    fn test_as_dt() {
+        let stamp = UnixTimestamp::new_only_seconds(0);
+        let dt = stamp.as_date_time().unwrap();
+        assert_eq!(dt.year(), 1970);
+        assert_eq!(dt.month(), 1);
+        assert_eq!(dt.day(), 1);
+        assert_eq!(dt.hour(), 0);
+        assert_eq!(dt.minute(), 0);
+        assert_eq!(dt.second(), 0);
+    }
+
+    #[test]
+    fn test_from_now() {
+        let stamp_now = UnixTimestamp::from_now().unwrap();
+        let dt_now = stamp_now.as_date_time().unwrap();
+        assert!(dt_now.year() >= 2020);
+    }
+
     #[test]
     fn test_addition_spillover() {
         let mut stamp0 = UnixTimestamp::new(1, 900).unwrap();
@@ -544,4 +575,11 @@ mod tests {
         assert_eq!(stamp0.unix_seconds, 3);
         assert_eq!(stamp0.subsecond_millis().unwrap(), 100);
     }
+
+    #[test]
+    fn test_cuc_error_printout() {
+        let cuc_error = CucError::InvalidCounterWidth(12);
+        let stamp_error = TimestampError::from(cuc_error);
+        assert_eq!(stamp_error.to_string(), format!("cuc error: {cuc_error}"));
+    }
 }

src/util.rs

@@ -207,17 +207,21 @@ impl UnsignedEnum for UnsignedByteField {
 
 #[derive(Debug, Copy, Clone, Eq, PartialEq)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
-pub struct GenericUnsignedByteField<TYPE> {
+pub struct GenericUnsignedByteField<TYPE: Copy> {
     value: TYPE,
 }
 
-impl<TYPE> GenericUnsignedByteField<TYPE> {
+impl<TYPE: Copy> GenericUnsignedByteField<TYPE> {
     pub const fn new(val: TYPE) -> Self {
         Self { value: val }
     }
+
+    pub const fn value(&self) -> TYPE {
+        self.value
+    }
 }
 
-impl<TYPE: ToBeBytes> UnsignedEnum for GenericUnsignedByteField<TYPE> {
+impl<TYPE: Copy + ToBeBytes> UnsignedEnum for GenericUnsignedByteField<TYPE> {
     fn size(&self) -> usize {
         self.value.written_len()
     }
@@ -344,8 +348,8 @@ pub mod tests {
             .expect("writing to raw buffer failed");
         assert_eq!(len, 1);
         assert_eq!(buf[0], 5);
-        for i in 1..8 {
-            assert_eq!(buf[i], 0);
+        for val in buf.iter().skip(1) {
+            assert_eq!(*val, 0);
         }
     }
 
@@ -360,8 +364,8 @@ pub mod tests {
         assert_eq!(len, 2);
         let raw_val = u16::from_be_bytes(buf[0..2].try_into().unwrap());
         assert_eq!(raw_val, 3823);
-        for i in 2..8 {
-            assert_eq!(buf[i], 0);
+        for val in buf.iter().skip(2) {
+            assert_eq!(*val, 0);
         }
     }
 
@@ -376,9 +380,9 @@ pub mod tests {
         assert_eq!(len, 4);
         let raw_val = u32::from_be_bytes(buf[0..4].try_into().unwrap());
         assert_eq!(raw_val, 80932);
-        for i in 4..8 {
+        (4..8).for_each(|i| {
             assert_eq!(buf[i], 0);
-        }
+        });
     }
 
     #[test]
@@ -544,8 +548,8 @@ pub mod tests {
             .expect("writing to raw buffer failed");
         let raw_val = u16::from_be_bytes(buf[0..2].try_into().unwrap());
         assert_eq!(raw_val, 3823);
-        for i in 2..8 {
-            assert_eq!(buf[i], 0);
+        for val in buf.iter().skip(2) {
+            assert_eq!(*val, 0);
         }
     }