continue tests

CHANGELOG.md

@@ -8,9 +8,14 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 # [unreleased]
 
-- Bumped `spacepackets` to v0.15
+- Bumped `spacepackets` to v0.16
 - Bumped `defmt` to v1
 
+## Added
+
+- Acknowledged mode support for both source and destination handler.
+- `FaultInfo` structure which is passed to user fault callbacks.
+
 # [v0.2.0] 2024-11-26
 
 - Bumped `thiserror` to v2

examples/python-interop/main.rs

@@ -12,7 +12,7 @@ use std::{
 };
 
 use cfdp::{
-    EntityType, IndicationConfig, LocalEntityConfig, PduOwnedWithInfo, PduProvider,
+    EntityType, FaultInfo, IndicationConfig, LocalEntityConfig, PduOwnedWithInfo, PduProvider,
     RemoteEntityConfig, StdTimerCreator, TransactionId, UserFaultHook,
     dest::DestinationHandler,
     filestore::NativeFilestore,
@@ -64,42 +64,20 @@ pub struct Cli {
 pub struct ExampleFaultHandler {}
 
 impl UserFaultHook for ExampleFaultHandler {
-    fn notice_of_suspension_cb(
+    fn notice_of_suspension_cb(&mut self, fault_info: FaultInfo) {
-        &mut self,
+        panic!("unexpected suspension, {:?}", fault_info);
-        transaction_id: TransactionId,
-        cond: ConditionCode,
-        progress: u64,
-    ) {
-        panic!(
-            "unexpected suspension of transaction {:?}, condition code {:?}, progress {}",
-            transaction_id, cond, progress
-        );
     }
 
-    fn notice_of_cancellation_cb(
+    fn notice_of_cancellation_cb(&mut self, fault_info: FaultInfo) {
-        &mut self,
+        panic!("unexpected cancellation, {:?}", fault_info);
-        transaction_id: TransactionId,
-        cond: ConditionCode,
-        progress: u64,
-    ) {
-        panic!(
-            "unexpected cancellation of transaction {:?}, condition code {:?}, progress {}",
-            transaction_id, cond, progress
-        );
     }
 
-    fn abandoned_cb(&mut self, transaction_id: TransactionId, cond: ConditionCode, progress: u64) {
+    fn abandoned_cb(&mut self, fault_info: FaultInfo) {
-        panic!(
+        panic!("unexpected abandonment, {:?}", fault_info);
-            "unexpected abandonment of transaction {:?}, condition code {:?}, progress {}",
-            transaction_id, cond, progress
-        );
     }
 
-    fn ignore_cb(&mut self, transaction_id: TransactionId, cond: ConditionCode, progress: u64) {
+    fn ignore_cb(&mut self, fault_info: FaultInfo) {
-        panic!(
+        panic!("unexpected ignore, {:?}", fault_info);
-            "ignoring unexpected error in transaction {:?}, condition code {:?}, progress {}",
-            transaction_id, cond, progress
-        );
     }
 }
 

src/dest.rs

@@ -30,7 +30,8 @@
 //!  4. A Finished PDU has been sent back to the remote side.
 //!  5. A Finished PDU ACK was received.
 use crate::{
-    DummyPduProvider, GenericSendError, IndicationConfig, PduProvider, PositiveAckParams,
+    DummyPduProvider, FaultInfo, GenericSendError, IndicationConfig, PduProvider,
+    PositiveAckParams,
     lost_segments::{LostSegmentError, LostSegmentStore},
     user::TransactionFinishedParams,
 };
@@ -669,7 +670,6 @@ impl<
                 if !first_packet && transmission_mode == TransmissionMode::Unacknowledged {
                     return Err(DestError::RecvdMetadataButIsBusy);
                 }
-                //if !self.check_for_deferred_lost_segment_completion(false) {
                 match self.transaction_params.acked_params.as_mut() {
                     Some(acked_params) => {
                         if acked_params.metadata_missing {
@@ -695,7 +695,6 @@ impl<
                         });
                     }
                 }
-                //}
             }
             None => {
                 return Err(DestError::RecvdMetadataButIsBusy);
@@ -1043,6 +1042,7 @@ impl<
     ) -> Result<u32, DestError> {
         let mut sent_packets = 0;
         self.transaction_params.file_size = eof_pdu.file_size();
+        self.transaction_params.checksum = eof_pdu.file_checksum();
         self.transaction_params.acked_params = Some(AcknowledgedModeParams {
             last_start_offset: 0,
             last_end_offset: 0,
@@ -1050,7 +1050,7 @@ impl<
             metadata_missing: true,
             deferred_procedure_active: false,
         });
-        if self.transaction_params.progress > 0 {
+        if self.transaction_params.file_size > 0 {
             self.lost_segment_tracker.reset();
             // Add the whole file to the lost segments map for now.
             self.lost_segment_tracker
@@ -1124,13 +1124,15 @@ impl<
     fn start_deferred_lost_segment_handling(&mut self) {
         match &mut self.transaction_params.acked_params {
             Some(params) => {
+                params.last_start_offset = self.transaction_params.file_size;
+                params.last_end_offset = self.transaction_params.file_size;
                 params.deferred_procedure_active = true;
                 params.nak_activity_counter = 0;
             }
             None => {
                 self.transaction_params.acked_params = Some(AcknowledgedModeParams {
-                    last_start_offset: 0,
+                    last_start_offset: self.transaction_params.file_size,
-                    last_end_offset: 0,
+                    last_end_offset: self.transaction_params.file_size,
                     metadata_missing: false,
                     nak_activity_counter: 0,
                     deferred_procedure_active: true,
@@ -1230,6 +1232,18 @@ impl<
             }
             return Ok(sent_packets);
         }
+        if !first_nak_issuance {
+            self.transaction_params
+                .acked_params
+                .as_mut()
+                .unwrap()
+                .nak_activity_counter += 1;
+            self.transaction_params
+                .deferred_procedure_timer
+                .as_mut()
+                .unwrap()
+                .reset();
+        }
         let pdu_header = PduHeader::new_for_file_directive(self.transaction_params.pdu_conf, 0);
         let max_segment_reqs = NakPduCreatorWithReservedSeqReqsBuf::calculate_max_segment_requests(
             self.transaction_params
@@ -1780,9 +1794,11 @@ impl<
             FaultHandlerCode::IgnoreError => (),
             FaultHandlerCode::AbandonTransaction => (),
         }
-        self.local_cfg
+        self.local_cfg.fault_handler.report_fault(FaultInfo::new(
-            .fault_handler
+            transaction_id,
-            .report_fault(transaction_id, condition_code, progress)
+            condition_code,
+            progress,
+        ))
     }
 
     fn notice_of_cancellation(&self, condition_code: ConditionCode, fault_location: EntityIdTlv) {
@@ -1966,6 +1982,15 @@ mod tests {
             handler
         }
 
+        pub fn fault_handler(&self) -> &FaultHandler<TestFaultHandler> {
+            &self.handler.local_cfg.fault_handler
+        }
+
+        #[inline]
+        fn set_large_file_flag(&mut self, large_file: LargeFileFlag) {
+            self.pdu_conf.file_flag = large_file;
+        }
+
         fn dest_path(&self) -> &PathBuf {
             &self.dest_path
         }
@@ -2007,9 +2032,11 @@ mod tests {
         }
 
         fn set_check_timer_expired(&mut self) {
-            self.expiry_control
+            self.expiry_control.set_check_limit_expired();
-                .check_limit
+        }
-                .store(true, core::sync::atomic::Ordering::Release);
+
+        fn set_nak_activity_timer_expired(&mut self) {
+            self.expiry_control.set_nak_activity_expired();
         }
 
         fn test_user_from_cached_paths(&self, expected_file_size: u64) -> TestCfdpUser {
@@ -2119,6 +2146,23 @@ mod tests {
             assert_eq!(finished_indication.condition_code, ConditionCode::NoError);
         }
 
+        fn check_completion_indication_failure(
+            &mut self,
+            user: &mut TestCfdpUser,
+            cond_code: ConditionCode,
+            file_status: FileStatus,
+        ) {
+            assert_eq!(user.finished_indic_queue.len(), 1);
+            let finished_indication = user.finished_indic_queue.pop_front().unwrap();
+            assert_eq!(
+                finished_indication.id,
+                self.handler.transaction_id().unwrap()
+            );
+            assert_eq!(finished_indication.file_status, file_status);
+            assert_eq!(finished_indication.delivery_code, DeliveryCode::Incomplete);
+            assert_eq!(finished_indication.condition_code, cond_code);
+        }
+
         fn check_eof_ack_pdu(&mut self, cond_code: ConditionCode) {
             assert!(!self.pdu_queue_empty());
             let pdu = self.get_next_pdu().unwrap();
@@ -2147,6 +2191,24 @@ mod tests {
             assert!(finished_pdu.fault_location().is_none());
         }
 
+        fn check_finished_pdu_failure(
+            &mut self,
+            cond_code: ConditionCode,
+            file_status: FileStatus,
+        ) {
+            let pdu = self.get_next_pdu().unwrap();
+            assert_eq!(pdu.pdu_type, PduType::FileDirective);
+            assert_eq!(
+                pdu.file_directive_type.unwrap(),
+                FileDirectiveType::FinishedPdu
+            );
+            let finished_pdu = FinishedPduReader::from_bytes(&pdu.raw_pdu).unwrap();
+            assert_eq!(finished_pdu.delivery_code(), DeliveryCode::Incomplete);
+            assert_eq!(finished_pdu.file_status(), file_status);
+            assert_eq!(finished_pdu.condition_code(), cond_code);
+            //assert!(finished_pdu.fault_location().is_none());
+        }
+
         fn acknowledge_finished_pdu(
             &mut self,
             user: &mut impl CfdpUser,
@@ -2497,9 +2559,12 @@ mod tests {
         let mut fault_handler = tb.handler.local_cfg.fault_handler.user_hook.borrow_mut();
         assert_eq!(fault_handler.ignored_queue.len(), 1);
         let cancelled = fault_handler.ignored_queue.pop_front().unwrap();
-        assert_eq!(cancelled.0, transfer_info.id);
+        assert_eq!(cancelled.transaction_id(), transfer_info.id);
-        assert_eq!(cancelled.1, ConditionCode::FileChecksumFailure);
+        assert_eq!(
-        assert_eq!(cancelled.2, segment_len as u64);
+            cancelled.condition_code(),
+            ConditionCode::FileChecksumFailure
+        );
+        assert_eq!(cancelled.progress(), segment_len as u64);
 
         drop(fault_handler);
         tb.state_check(
@@ -2531,14 +2596,14 @@ mod tests {
         let segment_len: usize = 256;
         let check_checksum_failure =
             |testbench: &mut DestHandlerTestbench, transaction_id: TransactionId| {
-                let mut fault_hook = testbench.handler.local_cfg.user_fault_hook().borrow_mut();
+                let mut fault_hook = testbench.fault_handler().user_hook.borrow_mut();
                 assert!(fault_hook.notice_of_suspension_queue.is_empty());
                 let ignored_queue = &mut fault_hook.ignored_queue;
                 assert_eq!(ignored_queue.len(), 1);
                 let ignored = ignored_queue.pop_front().unwrap();
-                assert_eq!(ignored.0, transaction_id);
+                assert_eq!(ignored.transaction_id(), transaction_id);
-                assert_eq!(ignored.1, ConditionCode::FileChecksumFailure);
+                assert_eq!(ignored.condition_code(), ConditionCode::FileChecksumFailure);
-                assert_eq!(ignored.2, segment_len as u64);
+                assert_eq!(ignored.progress(), segment_len as u64);
             };
 
         let fault_handler = TestFaultHandler::default();
@@ -2587,13 +2652,13 @@ mod tests {
         testbench.state_check(State::Idle, TransactionStep::Idle);
 
         {
-            let mut fault_hook = testbench.handler.local_cfg.user_fault_hook().borrow_mut();
+            let mut fault_hook = testbench.fault_handler().user_hook.borrow_mut();
             let cancelled_queue = &mut fault_hook.notice_of_cancellation_queue;
             assert_eq!(cancelled_queue.len(), 1);
             let cancelled = cancelled_queue.pop_front().unwrap();
-            assert_eq!(cancelled.0, transfer_info.id);
+            assert_eq!(cancelled.transaction_id(), transfer_info.id);
-            assert_eq!(cancelled.1, ConditionCode::CheckLimitReached);
+            assert_eq!(cancelled.condition_code(), ConditionCode::CheckLimitReached);
-            assert_eq!(cancelled.2, segment_len as u64);
+            assert_eq!(cancelled.progress(), segment_len as u64);
         }
 
         assert_eq!(test_user.finished_indic_queue.len(), 1);
@@ -2755,9 +2820,12 @@ mod tests {
         let ignored_queue = &mut fault_hook.ignored_queue;
         assert_eq!(ignored_queue.len(), 1);
         let cancelled = ignored_queue.pop_front().unwrap();
-        assert_eq!(cancelled.0, transaction_id);
+        assert_eq!(cancelled.transaction_id(), transaction_id);
-        assert_eq!(cancelled.1, ConditionCode::FileChecksumFailure);
+        assert_eq!(
-        assert_eq!(cancelled.2, file_size);
+            cancelled.condition_code(),
+            ConditionCode::FileChecksumFailure
+        );
+        assert_eq!(cancelled.progress(), file_size);
         drop(fault_hook);
 
         tb.state_check(
@@ -2783,9 +2851,9 @@ mod tests {
         let cancelled_queue = &mut fault_hook.notice_of_cancellation_queue;
         assert_eq!(cancelled_queue.len(), 1);
         let cancelled = cancelled_queue.pop_front().unwrap();
-        assert_eq!(cancelled.0, transaction_id);
+        assert_eq!(cancelled.transaction_id(), transaction_id);
-        assert_eq!(cancelled.1, ConditionCode::CheckLimitReached);
+        assert_eq!(cancelled.condition_code(), ConditionCode::CheckLimitReached);
-        assert_eq!(cancelled.2, file_size);
+        assert_eq!(cancelled.progress(), file_size);
 
         drop(fault_hook);
 
@@ -2806,13 +2874,13 @@ mod tests {
         let ignored_queue = &mut fault_hook.ignored_queue;
         assert_eq!(ignored_queue.len(), 2);
         let mut ignored = ignored_queue.pop_front().unwrap();
-        assert_eq!(ignored.0, transaction_id);
+        assert_eq!(ignored.transaction_id(), transaction_id);
-        assert_eq!(ignored.1, ConditionCode::FileChecksumFailure);
+        assert_eq!(ignored.condition_code(), ConditionCode::FileChecksumFailure);
-        assert_eq!(ignored.2, file_size);
+        assert_eq!(ignored.progress(), file_size);
         ignored = ignored_queue.pop_front().unwrap();
-        assert_eq!(ignored.0, transaction_id);
+        assert_eq!(ignored.transaction_id(), transaction_id);
-        assert_eq!(ignored.1, ConditionCode::FileChecksumFailure);
+        assert_eq!(ignored.condition_code(), ConditionCode::FileChecksumFailure);
-        assert_eq!(ignored.2, file_size);
+        assert_eq!(ignored.progress(), file_size);
 
         assert_eq!(finished_pdu.delivery_code(), DeliveryCode::Incomplete);
         assert!(finished_pdu.fault_location().is_some());
@@ -2960,9 +3028,9 @@ mod tests {
                 let mut fault_hook = tb.handler.local_cfg.user_fault_hook().borrow_mut();
                 let ignored_queue = &mut fault_hook.ignored_queue;
                 let ignored = ignored_queue.pop_front().unwrap();
-                assert_eq!(ignored.0, transfer_info.id);
+                assert_eq!(ignored.transaction_id(), transfer_info.id);
-                assert_eq!(ignored.1, ConditionCode::FileChecksumFailure);
+                assert_eq!(ignored.condition_code(), ConditionCode::FileChecksumFailure);
-                assert_eq!(ignored.2, 5);
+                assert_eq!(ignored.progress(), 5);
                 user.verify_finished_indication_retained(
                     DeliveryCode::Incomplete,
                     ConditionCode::CancelRequestReceived,
@@ -3205,8 +3273,7 @@ mod tests {
         );
     }
 
-    #[test]
+    fn generic_test_immediate_nak_request(file_flag: LargeFileFlag) {
-    fn test_immediate_nak_request() {
         let file_data_str = "Hello World!";
         let file_data = file_data_str.as_bytes();
         let file_size = file_data.len() as u64;
@@ -3217,6 +3284,7 @@ mod tests {
             TransmissionMode::Acknowledged,
             false,
         );
+        tb.set_large_file_flag(file_flag);
         tb.remote_cfg_mut().immediate_nak_mode = true;
         let mut user = tb.test_user_from_cached_paths(file_size);
         let transfer_info = tb
@@ -3235,6 +3303,7 @@ mod tests {
         assert_eq!(pdu.pdu_type, PduType::FileDirective);
         assert_eq!(pdu.file_directive_type.unwrap(), FileDirectiveType::NakPdu);
         let nak_pdu = NakPduReader::new(&pdu.raw_pdu).unwrap();
+        assert_eq!(nak_pdu.pdu_header().common_pdu_conf().file_flag, file_flag);
         assert_eq!(nak_pdu.start_of_scope(), 0);
         assert_eq!(nak_pdu.end_of_scope(), file_size);
         let seg_reqs: Vec<(u64, u64)> = nak_pdu.get_segment_requests_iterator().unwrap().collect();
@@ -3254,7 +3323,16 @@ mod tests {
     }
 
     #[test]
-    fn test_deferred_nak_request() {
+    fn test_immediate_nak_request() {
+        generic_test_immediate_nak_request(LargeFileFlag::Normal);
+    }
+
+    #[test]
+    fn test_immediate_nak_request_large_file() {
+        generic_test_immediate_nak_request(LargeFileFlag::Large);
+    }
+
+    fn generic_test_deferred_nak_request(file_flag: LargeFileFlag) {
         let file_data_str = "Hello World!";
         let file_data = file_data_str.as_bytes();
         let file_size = file_data.len() as u64;
@@ -3265,6 +3343,7 @@ mod tests {
             TransmissionMode::Acknowledged,
             false,
         );
+        tb.set_large_file_flag(file_flag);
         // Disable this, we only want to check the deferred procedure.
         tb.remote_cfg_mut().immediate_nak_mode = false;
         let mut user = tb.test_user_from_cached_paths(file_size);
@@ -3305,6 +3384,16 @@ mod tests {
         tb.acknowledge_finished_pdu(&mut user, &transfer_info);
     }
 
+    #[test]
+    fn test_deferred_nak_request() {
+        generic_test_deferred_nak_request(LargeFileFlag::Normal);
+    }
+
+    #[test]
+    fn test_deferred_nak_request_large() {
+        generic_test_deferred_nak_request(LargeFileFlag::Large);
+    }
+
     #[test]
     fn test_file_data_before_metadata() {
         let file_data_str = "Hello World!";
@@ -3361,7 +3450,6 @@ mod tests {
         tb.acknowledge_finished_pdu(&mut user, &transfer_info);
     }
 
-    /*
     #[test]
     fn test_eof_before_metadata() {
         let file_data_str = "Hello World!";
@@ -3406,13 +3494,99 @@ mod tests {
         assert_eq!(
             tb.generic_file_data_insert(&mut user, 0, file_data)
                 .expect("file data insertion failed"),
-            0
+            1
         );
         tb.check_completion_indication_success(&mut user);
-        assert_eq!(tb.pdu_queue_len(), 2);
+        assert_eq!(tb.pdu_queue_len(), 1);
-        tb.check_eof_ack_pdu(ConditionCode::NoError);
         tb.check_finished_pdu_success();
         tb.acknowledge_finished_pdu(&mut user, &transfer_info);
     }
-    */
+
+    #[test]
+    fn test_nak_limit_reached() {
+        let file_data_str = "Hello World!";
+        let file_data = file_data_str.as_bytes();
+        let file_size = file_data.len() as u64;
+        let fault_handler = TestFaultHandler::default();
+
+        let mut tb = DestHandlerTestbench::new_with_fixed_paths(
+            fault_handler,
+            TransmissionMode::Acknowledged,
+            false,
+        );
+        // Disable this, we only want to check the deferred procedure.
+        tb.remote_cfg_mut().immediate_nak_mode = false;
+        let mut user = tb.test_user_from_cached_paths(file_size);
+        let transfer_info = tb
+            .generic_transfer_init(&mut user, file_size)
+            .expect("transfer init failed");
+        tb.state_check(State::Busy, TransactionStep::ReceivingFileDataPdus);
+
+        assert_eq!(
+            tb.generic_file_data_insert(&mut user, 4, &file_data[4..])
+                .expect("file data insertion failed"),
+            0
+        );
+        assert_eq!(
+            tb.generic_eof_no_error(&mut user, file_data.to_vec())
+                .expect("EOF no error insertion failed"),
+            2
+        );
+        assert_eq!(tb.pdu_queue_len(), 2);
+        tb.check_eof_ack_pdu(ConditionCode::NoError);
+        assert_eq!(tb.pdu_queue_len(), 1);
+        let pdu = tb.get_next_pdu().unwrap();
+        assert_eq!(pdu.pdu_type, PduType::FileDirective);
+        assert_eq!(pdu.file_directive_type.unwrap(), FileDirectiveType::NakPdu);
+        let nak_pdu = NakPduReader::new(&pdu.raw_pdu).unwrap();
+        assert_eq!(nak_pdu.start_of_scope(), 0);
+        assert_eq!(nak_pdu.end_of_scope(), file_size);
+        let seg_reqs: Vec<(u64, u64)> = nak_pdu.get_segment_requests_iterator().unwrap().collect();
+        assert_eq!(seg_reqs.len(), 1);
+        assert_eq!(seg_reqs[0], (0, 4));
+
+        // Let the NAK timer expire
+        tb.set_nak_activity_timer_expired();
+        assert_eq!(tb.handler.state_machine_no_packet(&mut user).unwrap(), 1);
+        assert_eq!(tb.pdu_queue_len(), 1);
+        let pdu = tb.get_next_pdu().unwrap();
+        assert_eq!(pdu.pdu_type, PduType::FileDirective);
+        assert_eq!(pdu.file_directive_type.unwrap(), FileDirectiveType::NakPdu);
+        let nak_pdu = NakPduReader::new(&pdu.raw_pdu).unwrap();
+        assert_eq!(nak_pdu.start_of_scope(), 0);
+        assert_eq!(nak_pdu.end_of_scope(), file_size);
+        let seg_reqs: Vec<(u64, u64)> = nak_pdu.get_segment_requests_iterator().unwrap().collect();
+        assert_eq!(seg_reqs.len(), 1);
+        assert_eq!(seg_reqs[0], (0, 4));
+
+        // Let the NAK timer expire again.
+        tb.set_nak_activity_timer_expired();
+        assert_eq!(tb.handler.state_machine_no_packet(&mut user).unwrap(), 1);
+        assert_eq!(tb.pdu_queue_len(), 1);
+        tb.check_completion_indication_failure(
+            &mut user,
+            ConditionCode::NakLimitReached,
+            FileStatus::Retained,
+        );
+        tb.check_finished_pdu_failure(ConditionCode::NakLimitReached, FileStatus::Retained);
+
+        {
+            let mut fault_hook = tb.fault_handler().user_hook.borrow_mut();
+            assert_eq!(fault_hook.notice_of_cancellation_queue.len(), 1);
+            let cancellation = fault_hook.notice_of_cancellation_queue.pop_front().unwrap();
+            assert_eq!(cancellation.transaction_id(), transfer_info.id);
+            assert_eq!(
+                cancellation.condition_code(),
+                ConditionCode::NakLimitReached
+            );
+            assert_eq!(cancellation.progress(), file_size);
+        }
+        tb.acknowledge_finished_pdu(&mut user, &transfer_info);
+        tb.check_dest_file = false;
+    }
+
+    #[test]
+    fn test_positive_ack_procedure() {
+        // TODO.
+    }
 }

src/lib.rs

@@ -465,23 +465,13 @@ impl RemoteConfigStore for RemoteEntityConfig {
 /// For each error reported by the [FaultHandler], the appropriate fault handler callback
 /// will be called depending on the [FaultHandlerCode].
 pub trait UserFaultHook {
-    fn notice_of_suspension_cb(
+    fn notice_of_suspension_cb(&mut self, fault_info: FaultInfo);
-        &mut self,
-        transaction_id: TransactionId,
-        cond: ConditionCode,
-        progress: u64,
-    );
 
-    fn notice_of_cancellation_cb(
+    fn notice_of_cancellation_cb(&mut self, fault_info: FaultInfo);
-        &mut self,
-        transaction_id: TransactionId,
-        cond: ConditionCode,
-        progress: u64,
-    );
 
-    fn abandoned_cb(&mut self, transaction_id: TransactionId, cond: ConditionCode, progress: u64);
+    fn abandoned_cb(&mut self, fault_info: FaultInfo);
 
-    fn ignore_cb(&mut self, transaction_id: TransactionId, cond: ConditionCode, progress: u64);
+    fn ignore_cb(&mut self, fault_info: FaultInfo);
 }
 
 /// Dummy fault hook which implements [UserFaultHook] but only provides empty
@@ -490,31 +480,13 @@ pub trait UserFaultHook {
 pub struct DummyFaultHook {}
 
 impl UserFaultHook for DummyFaultHook {
-    fn notice_of_suspension_cb(
+    fn notice_of_suspension_cb(&mut self, _fault_info: FaultInfo) {}
-        &mut self,
-        _transaction_id: TransactionId,
-        _cond: ConditionCode,
-        _progress: u64,
-    ) {
-    }
 
-    fn notice_of_cancellation_cb(
+    fn notice_of_cancellation_cb(&mut self, _fault_info: FaultInfo) {}
-        &mut self,
-        _transaction_id: TransactionId,
-        _cond: ConditionCode,
-        _progress: u64,
-    ) {
-    }
 
-    fn abandoned_cb(
+    fn abandoned_cb(&mut self, _fault_info: FaultInfo) {}
-        &mut self,
-        _transaction_id: TransactionId,
-        _cond: ConditionCode,
-        _progress: u64,
-    ) {
-    }
 
-    fn ignore_cb(&mut self, _transaction_id: TransactionId, _cond: ConditionCode, _progress: u64) {}
+    fn ignore_cb(&mut self, _fault_info: FaultInfo) {}
 }
 
 /// This structure is used to implement the fault handling as specified in chapter 4.8 of the CFDP
@@ -544,6 +516,43 @@ pub struct FaultHandler<UserHandler: UserFaultHook> {
     pub user_hook: RefCell<UserHandler>,
 }
 
+#[derive(Debug, Copy, Clone, PartialEq, Eq)]
+#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+pub struct FaultInfo {
+    transaction_id: TransactionId,
+    condition_code: ConditionCode,
+    progress: u64,
+}
+
+impl FaultInfo {
+    pub const fn new(
+        transaction_id: TransactionId,
+        condition_code: ConditionCode,
+        progress: u64,
+    ) -> Self {
+        Self {
+            transaction_id,
+            condition_code,
+            progress,
+        }
+    }
+
+    #[inline]
+    pub const fn transaction_id(&self) -> TransactionId {
+        self.transaction_id
+    }
+
+    #[inline]
+    pub const fn condition_code(&self) -> ConditionCode {
+        self.condition_code
+    }
+
+    #[inline]
+    pub const fn progress(&self) -> u64 {
+        self.progress
+    }
+}
+
 impl<UserHandler: UserFaultHook> FaultHandler<UserHandler> {
     fn condition_code_to_array_index(conditon_code: ConditionCode) -> Option<usize> {
         Some(match conditon_code {
@@ -594,13 +603,8 @@ impl<UserHandler: UserFaultHook> FaultHandler<UserHandler> {
         self.handler_array[array_idx.unwrap()]
     }
 
-    pub fn report_fault(
+    pub fn report_fault(&self, fault_info: FaultInfo) -> FaultHandlerCode {
-        &self,
+        let array_idx = Self::condition_code_to_array_index(fault_info.condition_code());
-        transaction_id: TransactionId,
-        condition: ConditionCode,
-        progress: u64,
-    ) -> FaultHandlerCode {
-        let array_idx = Self::condition_code_to_array_index(condition);
         if array_idx.is_none() {
             return FaultHandlerCode::IgnoreError;
         }
@@ -608,16 +612,16 @@ impl<UserHandler: UserFaultHook> FaultHandler<UserHandler> {
         let mut handler_mut = self.user_hook.borrow_mut();
         match fh_code {
             FaultHandlerCode::NoticeOfCancellation => {
-                handler_mut.notice_of_cancellation_cb(transaction_id, condition, progress);
+                handler_mut.notice_of_cancellation_cb(fault_info);
             }
             FaultHandlerCode::NoticeOfSuspension => {
-                handler_mut.notice_of_suspension_cb(transaction_id, condition, progress);
+                handler_mut.notice_of_suspension_cb(fault_info);
             }
             FaultHandlerCode::IgnoreError => {
-                handler_mut.ignore_cb(transaction_id, condition, progress);
+                handler_mut.ignore_cb(fault_info);
             }
             FaultHandlerCode::AbandonTransaction => {
-                handler_mut.abandoned_cb(transaction_id, condition, progress);
+                handler_mut.abandoned_cb(fault_info);
             }
         }
         fh_code
@@ -1120,11 +1124,15 @@ pub(crate) mod tests {
                 .store(true, core::sync::atomic::Ordering::Release);
         }
 
-        #[allow(dead_code)]
         pub fn set_positive_ack_expired(&mut self) {
             self.positive_ack
                 .store(true, core::sync::atomic::Ordering::Release);
         }
+
+        pub fn set_nak_activity_expired(&mut self) {
+            self.nak_activity
+                .store(true, core::sync::atomic::Ordering::Release);
+        }
     }
 
     #[derive(Debug)]
@@ -1402,46 +1410,27 @@ pub(crate) mod tests {
 
     #[derive(Default, Debug)]
     pub(crate) struct TestFaultHandler {
-        pub notice_of_suspension_queue: VecDeque<(TransactionId, ConditionCode, u64)>,
+        pub notice_of_suspension_queue: VecDeque<FaultInfo>,
-        pub notice_of_cancellation_queue: VecDeque<(TransactionId, ConditionCode, u64)>,
+        pub notice_of_cancellation_queue: VecDeque<FaultInfo>,
-        pub abandoned_queue: VecDeque<(TransactionId, ConditionCode, u64)>,
+        pub abandoned_queue: VecDeque<FaultInfo>,
-        pub ignored_queue: VecDeque<(TransactionId, ConditionCode, u64)>,
+        pub ignored_queue: VecDeque<FaultInfo>,
     }
 
     impl UserFaultHook for TestFaultHandler {
-        fn notice_of_suspension_cb(
+        fn notice_of_suspension_cb(&mut self, fault_info: FaultInfo) {
-            &mut self,
+            self.notice_of_suspension_queue.push_back(fault_info)
-            transaction_id: TransactionId,
-            cond: ConditionCode,
-            progress: u64,
-        ) {
-            self.notice_of_suspension_queue
-                .push_back((transaction_id, cond, progress))
         }
 
-        fn notice_of_cancellation_cb(
+        fn notice_of_cancellation_cb(&mut self, fault_info: FaultInfo) {
-            &mut self,
+            self.notice_of_cancellation_queue.push_back(fault_info)
-            transaction_id: TransactionId,
-            cond: ConditionCode,
-            progress: u64,
-        ) {
-            self.notice_of_cancellation_queue
-                .push_back((transaction_id, cond, progress))
         }
 
-        fn abandoned_cb(
+        fn abandoned_cb(&mut self, fault_info: FaultInfo) {
-            &mut self,
+            self.abandoned_queue.push_back(fault_info)
-            transaction_id: TransactionId,
-            cond: ConditionCode,
-            progress: u64,
-        ) {
-            self.abandoned_queue
-                .push_back((transaction_id, cond, progress))
         }
 
-        fn ignore_cb(&mut self, transaction_id: TransactionId, cond: ConditionCode, progress: u64) {
+        fn ignore_cb(&mut self, fault_info: FaultInfo) {
-            self.ignored_queue
+            self.ignored_queue.push_back(fault_info)
-                .push_back((transaction_id, cond, progress))
         }
     }
 
@@ -1764,10 +1753,18 @@ pub(crate) mod tests {
             UnsignedByteFieldU8::new(0).into(),
             UnsignedByteFieldU8::new(0).into(),
         );
-        user_hook_dummy.notice_of_cancellation_cb(transaction_id, ConditionCode::NoError, 0);
+        user_hook_dummy.notice_of_cancellation_cb(FaultInfo::new(
-        user_hook_dummy.notice_of_suspension_cb(transaction_id, ConditionCode::NoError, 0);
+            transaction_id,
-        user_hook_dummy.abandoned_cb(transaction_id, ConditionCode::NoError, 0);
+            ConditionCode::NoError,
-        user_hook_dummy.ignore_cb(transaction_id, ConditionCode::NoError, 0);
+            0,
+        ));
+        user_hook_dummy.notice_of_suspension_cb(FaultInfo::new(
+            transaction_id,
+            ConditionCode::NoError,
+            0,
+        ));
+        user_hook_dummy.abandoned_cb(FaultInfo::new(transaction_id, ConditionCode::NoError, 0));
+        user_hook_dummy.ignore_cb(FaultInfo::new(transaction_id, ConditionCode::NoError, 0));
     }
 
     #[test]

src/source.rs

@@ -67,8 +67,8 @@ use spacepackets::{
 use spacepackets::seq_count::SequenceCounter;
 
 use crate::{
-    DummyPduProvider, EntityType, GenericSendError, PduProvider, PositiveAckParams, TimerCreator,
+    DummyPduProvider, EntityType, FaultInfo, GenericSendError, PduProvider, PositiveAckParams,
-    time::Countdown,
+    TimerCreator, time::Countdown,
 };
 
 use super::{
@@ -1092,7 +1092,11 @@ impl<
                     .fault_handler
                     .user_hook
                     .borrow_mut()
-                    .abandoned_cb(transaction_id, cond_code_eof, self.file_params.progress);
+                    .abandoned_cb(FaultInfo::new(
+                        transaction_id,
+                        cond_code_eof,
+                        self.file_params.progress,
+                    ));
                 return Ok(ControlFlow::Break(FsmContext::ResetWhenPossible));
             }
         }
@@ -1157,11 +1161,11 @@ impl<
                 return Ok((sent_packets, FsmContext::ResetWhenPossible));
             }
         }
-        self.local_cfg.fault_handler.report_fault(
+        self.local_cfg.fault_handler.report_fault(FaultInfo::new(
             self.transaction_id().unwrap(),
             cond,
             self.file_params.progress,
-        );
+        ));
         Ok((sent_packets, ctx))
     }
 
@@ -1934,9 +1938,13 @@ mod tests {
         let fh_ref_mut = fault_handler.get_mut();
         assert!(!fh_ref_mut.cancellation_queue_empty());
         assert_eq!(fh_ref_mut.notice_of_cancellation_queue.len(), 1);
-        let (id, cond_code, progress) = fh_ref_mut.notice_of_cancellation_queue.pop_back().unwrap();
+        let FaultInfo {
-        assert_eq!(id, expected_id);
+            transaction_id,
-        assert_eq!(cond_code, ConditionCode::CheckLimitReached);
+            condition_code,
+            progress,
+        } = fh_ref_mut.notice_of_cancellation_queue.pop_back().unwrap();
+        assert_eq!(transaction_id, expected_id);
+        assert_eq!(condition_code, ConditionCode::CheckLimitReached);
         assert_eq!(progress, 0);
         fh_ref_mut.all_queues_empty();
     }
@@ -2184,9 +2192,13 @@ mod tests {
         let fh_ref_mut = fault_handler.get_mut();
         assert!(!fh_ref_mut.cancellation_queue_empty());
         assert_eq!(fh_ref_mut.notice_of_cancellation_queue.len(), 1);
-        let (id, cond_code, progress) = fh_ref_mut.notice_of_cancellation_queue.pop_back().unwrap();
+        let FaultInfo {
-        assert_eq!(id, transfer_info.id);
+            transaction_id,
-        assert_eq!(cond_code, ConditionCode::PositiveAckLimitReached);
+            condition_code,
+            progress,
+        } = fh_ref_mut.notice_of_cancellation_queue.pop_back().unwrap();
+        assert_eq!(transaction_id, transfer_info.id);
+        assert_eq!(condition_code, ConditionCode::PositiveAckLimitReached);
         assert_eq!(progress, file_size);
         fh_ref_mut.all_queues_empty();
 
@@ -2212,9 +2224,13 @@ mod tests {
         let fh_ref_mut = fault_handler.get_mut();
         assert!(!fh_ref_mut.abandoned_queue_empty());
         assert_eq!(fh_ref_mut.abandoned_queue.len(), 1);
-        let (id, cond_code, progress) = fh_ref_mut.abandoned_queue.pop_back().unwrap();
+        let FaultInfo {
-        assert_eq!(id, transfer_info.id);
+            transaction_id,
-        assert_eq!(cond_code, ConditionCode::PositiveAckLimitReached);
+            condition_code,
+            progress,
+        } = fh_ref_mut.abandoned_queue.pop_back().unwrap();
+        assert_eq!(transaction_id, transfer_info.id);
+        assert_eq!(condition_code, ConditionCode::PositiveAckLimitReached);
         assert_eq!(progress, file_size);
         fh_ref_mut.all_queues_empty();
     }

tests/end-to-end.rs

@@ -12,8 +12,8 @@ use std::{
 };
 
 use cfdp::{
-    EntityType, IndicationConfig, LocalEntityConfig, PduOwnedWithInfo, RemoteEntityConfig,
+    EntityType, FaultInfo, IndicationConfig, LocalEntityConfig, PduOwnedWithInfo,
-    StdTimerCreator, TransactionId, UserFaultHook,
+    RemoteEntityConfig, StdTimerCreator, TransactionId, UserFaultHook,
     dest::DestinationHandler,
     filestore::NativeFilestore,
     lost_segments::LostSegmentsList,
@@ -35,42 +35,20 @@ const FILE_DATA: &str = "Hello World!";
 pub struct ExampleFaultHandler {}
 
 impl UserFaultHook for ExampleFaultHandler {
-    fn notice_of_suspension_cb(
+    fn notice_of_suspension_cb(&mut self, fault_info: FaultInfo) {
-        &mut self,
+        panic!("unexpected suspension, {:?}", fault_info);
-        transaction_id: TransactionId,
-        cond: ConditionCode,
-        progress: u64,
-    ) {
-        panic!(
-            "unexpected suspension of transaction {:?}, condition code {:?}, progress {}",
-            transaction_id, cond, progress
-        );
     }
 
-    fn notice_of_cancellation_cb(
+    fn notice_of_cancellation_cb(&mut self, fault_info: FaultInfo) {
-        &mut self,
+        panic!("unexpected cancellation, {:?}", fault_info);
-        transaction_id: TransactionId,
-        cond: ConditionCode,
-        progress: u64,
-    ) {
-        panic!(
-            "unexpected cancellation of transaction {:?}, condition code {:?}, progress {}",
-            transaction_id, cond, progress
-        );
     }
 
-    fn abandoned_cb(&mut self, transaction_id: TransactionId, cond: ConditionCode, progress: u64) {
+    fn abandoned_cb(&mut self, fault_info: FaultInfo) {
-        panic!(
+        panic!("unexpected abandonment, {:?}", fault_info);
-            "unexpected abandonment of transaction {:?}, condition code {:?}, progress {}",
-            transaction_id, cond, progress
-        );
     }
 
-    fn ignore_cb(&mut self, transaction_id: TransactionId, cond: ConditionCode, progress: u64) {
+    fn ignore_cb(&mut self, fault_info: FaultInfo) {
-        panic!(
+        panic!("unexpected ignore, {:?}", fault_info);
-            "ignoring unexpected error in transaction {:?}, condition code {:?}, progress {}",
-            transaction_id, cond, progress
-        );
     }
 }