From 023380bbd27cf07d0ae8a7b2ae9e79efde044b39 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

---
 src/cfdp/pdu/ack.rs | 36 +++++++++++++++++++-----------------
 src/cfdp/pdu/mod.rs |  2 +-
 2 files changed, 20 insertions(+), 18 deletions(-)

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 b62e7e4..6f03f32 100644
--- a/src/cfdp/pdu/mod.rs
+++ b/src/cfdp/pdu/mod.rs
@@ -57,7 +57,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,