Merge pull request 'improve ACK PDU' (#157) from improve-ack-pdu into main

Reviewed-on: #157

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.

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 {
+            return Err(InvalidAckedDirectiveCodeError(directive_code_of_acked_pdu));
-                found: directive_code_of_acked_pdu as u8,
-                expected: None,
-            });
         }
         // 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(
+        assert_eq!(
+            AckPdu::new(
                 pdu_header,
                 FileDirectiveType::MetadataPdu,
                 ConditionCode::NoError,
                 TransactionStatus::Active,
-        ) {
+            )
-            assert!(expected.is_none());
+            .unwrap_err(),
-            assert_eq!(found, FileDirectiveType::MetadataPdu as u8);
+            InvalidAckedDirectiveCodeError(FileDirectiveType::MetadataPdu)
-        } else {
+        );
-            panic!("ACK PDU construction should have failed");
-        }
     }
 
     #[test]

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>;