From 3f6a5df8e79c9be24530fb53cfa46e09bb0efc0b Mon Sep 17 00:00:00 2001
From: Robin Mueller <robin.mueller.m@pm.me>
Date: Mon, 15 Sep 2025 12:22:24 +0200
Subject: [PATCH] improve ACK PDU

---
 CHANGELOG.md        |  1 +
 src/cfdp/pdu/ack.rs | 36 +++++++++++++++++++-----------------
 src/cfdp/pdu/mod.rs | 12 +++++++++++-
 3 files changed, 31 insertions(+), 18 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 3817278..12e0460 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -45,6 +45,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## Added
 
+- `cfdp::pdu::ack::InvalidAckedDirectiveCodeError` which is returned by the `AckPdu` constructor.
 - `cfdp::pdu::nak::NakPduCreatorWithReservedSegReqsBuf` constructor which exposes the segment
   request buffer mutably to avoid the need for a separate segment request buffer.
 - `SpHeader::packet_len` direct method.
diff --git a/src/cfdp/pdu/ack.rs b/src/cfdp/pdu/ack.rs
index 656bad7..9c9e664 100644
--- a/src/cfdp/pdu/ack.rs
+++ b/src/cfdp/pdu/ack.rs
@@ -10,6 +10,10 @@ use super::{
 #[cfg(feature = "serde")]
 use serde::{Deserialize, Serialize};
 
+#[derive(Debug, Clone, Copy, PartialEq, Eq, thiserror::Error)]
+#[error("invalid directive code of acknowledged PDU")]
+pub struct InvalidAckedDirectiveCodeError(pub FileDirectiveType);
+
 /// ACK PDU abstraction.
 ///
 /// For more information, refer to CFDP chapter 5.2.4.
@@ -29,16 +33,13 @@ impl AckPdu {
         directive_code_of_acked_pdu: FileDirectiveType,
         condition_code: ConditionCode,
         transaction_status: TransactionStatus,
-    ) -> Result<Self, PduError> {
+    ) -> Result<Self, InvalidAckedDirectiveCodeError> {
         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,
-            });
+            return Err(InvalidAckedDirectiveCodeError(directive_code_of_acked_pdu));
         }
         // Force correct direction flag.
         let mut ack_pdu = Self {
@@ -145,12 +146,14 @@ impl AckPdu {
         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(
+        // Unwrap okay, validity of acked directive code was checked.
+        Ok(Self::new(
             pdu_header,
             acked_directive_type,
             condition_code,
             transaction_status,
         )
+        .unwrap())
     }
 
     /// Write [Self] to the provided buffer and returns the written size.
@@ -314,17 +317,16 @@ mod tests {
     fn test_invalid_directive_code_of_acked_pdu() {
         let pdu_conf = common_pdu_conf(CrcFlag::NoCrc, LargeFileFlag::Normal);
         let pdu_header = PduHeader::new_no_file_data(pdu_conf, 0);
-        if let Err(PduError::InvalidDirectiveType { found, expected }) = AckPdu::new(
-            pdu_header,
-            FileDirectiveType::MetadataPdu,
-            ConditionCode::NoError,
-            TransactionStatus::Active,
-        ) {
-            assert!(expected.is_none());
-            assert_eq!(found, FileDirectiveType::MetadataPdu as u8);
-        } else {
-            panic!("ACK PDU construction should have failed");
-        }
+        assert_eq!(
+            AckPdu::new(
+                pdu_header,
+                FileDirectiveType::MetadataPdu,
+                ConditionCode::NoError,
+                TransactionStatus::Active,
+            )
+            .unwrap_err(),
+            InvalidAckedDirectiveCodeError(FileDirectiveType::MetadataPdu)
+        );
     }
 
     #[test]
diff --git a/src/cfdp/pdu/mod.rs b/src/cfdp/pdu/mod.rs
index d8b0057..881f42f 100644
--- a/src/cfdp/pdu/mod.rs
+++ b/src/cfdp/pdu/mod.rs
@@ -1,4 +1,5 @@
 //! CFDP Packet Data Unit (PDU) support.
+use crate::cfdp::pdu::ack::InvalidAckedDirectiveCodeError;
 use crate::cfdp::pdu::nak::InvalidStartOrEndOfScopeError;
 use crate::cfdp::*;
 use crate::crc::CRC_CCITT_FALSE;
@@ -58,7 +59,7 @@ pub enum PduError {
         expected: FileDirectiveType,
     },
     /// 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.
+    /// also happen if an invalid value is passed to the ACK PDU reader.
     #[error("invalid directive type, found {found:?}, expected {expected:?}")]
     InvalidDirectiveType {
         found: u8,
@@ -86,6 +87,15 @@ pub enum PduError {
     TlvLv(#[from] TlvLvError),
 }
 
+impl From<InvalidAckedDirectiveCodeError> for PduError {
+    fn from(value: InvalidAckedDirectiveCodeError) -> Self {
+        Self::InvalidDirectiveType {
+            found: value.0 as u8,
+            expected: None,
+        }
+    }
+}
+
 pub trait WritablePduPacket {
     fn len_written(&self) -> usize;
     fn write_to_bytes(&self, buf: &mut [u8]) -> Result<usize, PduError>;