From 824ac1ca549998ead10fb992a14cde5656a14dc0 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 24 Sep 2025 15:57:18 +0200 Subject: [PATCH] continue tests --- CHANGELOG.md | 7 +- examples/python-interop/main.rs | 40 ++--- src/dest.rs | 266 ++++++++++++++++++++++++++------ src/lib.rs | 163 ++++++++++--------- src/source.rs | 44 ++++-- tests/end-to-end.rs | 42 ++--- 6 files changed, 355 insertions(+), 207 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bc40490..ac03890 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/examples/python-interop/main.rs b/examples/python-interop/main.rs index b8f2f64..8b4a0e1 100644 --- a/examples/python-interop/main.rs +++ b/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( - &mut self, - transaction_id: TransactionId, - cond: ConditionCode, - progress: u64, - ) { - panic!( - "unexpected suspension of transaction {:?}, condition code {:?}, progress {}", - transaction_id, cond, progress - ); + fn notice_of_suspension_cb(&mut self, fault_info: FaultInfo) { + panic!("unexpected suspension, {:?}", fault_info); } - fn notice_of_cancellation_cb( - &mut self, - transaction_id: TransactionId, - cond: ConditionCode, - progress: u64, - ) { - panic!( - "unexpected cancellation of transaction {:?}, condition code {:?}, progress {}", - transaction_id, cond, progress - ); + fn notice_of_cancellation_cb(&mut self, fault_info: FaultInfo) { + panic!("unexpected cancellation, {:?}", fault_info); } - fn abandoned_cb(&mut self, transaction_id: TransactionId, cond: ConditionCode, progress: u64) { - panic!( - "unexpected abandonment of transaction {:?}, condition code {:?}, progress {}", - transaction_id, cond, progress - ); + fn abandoned_cb(&mut self, fault_info: FaultInfo) { + panic!("unexpected abandonment, {:?}", fault_info); } - fn ignore_cb(&mut self, transaction_id: TransactionId, cond: ConditionCode, progress: u64) { - panic!( - "ignoring unexpected error in transaction {:?}, condition code {:?}, progress {}", - transaction_id, cond, progress - ); + fn ignore_cb(&mut self, fault_info: FaultInfo) { + panic!("unexpected ignore, {:?}", fault_info); } } diff --git a/src/dest.rs b/src/dest.rs index 5f3d85e..b9f2f32 100644 --- a/src/dest.rs +++ b/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 { 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_end_offset: 0, + last_start_offset: self.transaction_params.file_size, + 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 - .fault_handler - .report_fault(transaction_id, condition_code, progress) + self.local_cfg.fault_handler.report_fault(FaultInfo::new( + transaction_id, + 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 { + &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 - .check_limit - .store(true, core::sync::atomic::Ordering::Release); + self.expiry_control.set_check_limit_expired(); + } + + 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.1, ConditionCode::FileChecksumFailure); - assert_eq!(cancelled.2, segment_len as u64); + assert_eq!(cancelled.transaction_id(), transfer_info.id); + assert_eq!( + 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.1, ConditionCode::FileChecksumFailure); - assert_eq!(ignored.2, segment_len as u64); + assert_eq!(ignored.transaction_id(), transaction_id); + assert_eq!(ignored.condition_code(), ConditionCode::FileChecksumFailure); + 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.1, ConditionCode::CheckLimitReached); - assert_eq!(cancelled.2, segment_len as u64); + assert_eq!(cancelled.transaction_id(), transfer_info.id); + assert_eq!(cancelled.condition_code(), ConditionCode::CheckLimitReached); + 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.1, ConditionCode::FileChecksumFailure); - assert_eq!(cancelled.2, file_size); + assert_eq!(cancelled.transaction_id(), transaction_id); + assert_eq!( + 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.1, ConditionCode::CheckLimitReached); - assert_eq!(cancelled.2, file_size); + assert_eq!(cancelled.transaction_id(), transaction_id); + assert_eq!(cancelled.condition_code(), ConditionCode::CheckLimitReached); + 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.1, ConditionCode::FileChecksumFailure); - assert_eq!(ignored.2, file_size); + assert_eq!(ignored.transaction_id(), transaction_id); + assert_eq!(ignored.condition_code(), ConditionCode::FileChecksumFailure); + assert_eq!(ignored.progress(), file_size); ignored = ignored_queue.pop_front().unwrap(); - assert_eq!(ignored.0, transaction_id); - assert_eq!(ignored.1, ConditionCode::FileChecksumFailure); - assert_eq!(ignored.2, file_size); + assert_eq!(ignored.transaction_id(), transaction_id); + assert_eq!(ignored.condition_code(), ConditionCode::FileChecksumFailure); + 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.1, ConditionCode::FileChecksumFailure); - assert_eq!(ignored.2, 5); + assert_eq!(ignored.transaction_id(), transfer_info.id); + assert_eq!(ignored.condition_code(), ConditionCode::FileChecksumFailure); + assert_eq!(ignored.progress(), 5); user.verify_finished_indication_retained( DeliveryCode::Incomplete, ConditionCode::CancelRequestReceived, @@ -3205,8 +3273,7 @@ mod tests { ); } - #[test] - fn test_immediate_nak_request() { + fn generic_test_immediate_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; @@ -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); - tb.check_eof_ack_pdu(ConditionCode::NoError); + assert_eq!(tb.pdu_queue_len(), 1); 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. + } } diff --git a/src/lib.rs b/src/lib.rs index 6d15fd1..a5f74ee 100644 --- a/src/lib.rs +++ b/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( - &mut self, - transaction_id: TransactionId, - cond: ConditionCode, - progress: u64, - ); + fn notice_of_suspension_cb(&mut self, fault_info: FaultInfo); - fn notice_of_cancellation_cb( - &mut self, - transaction_id: TransactionId, - cond: ConditionCode, - progress: u64, - ); + fn notice_of_cancellation_cb(&mut self, fault_info: FaultInfo); - 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( - &mut self, - _transaction_id: TransactionId, - _cond: ConditionCode, - _progress: u64, - ) { - } + fn notice_of_suspension_cb(&mut self, _fault_info: FaultInfo) {} - fn notice_of_cancellation_cb( - &mut self, - _transaction_id: TransactionId, - _cond: ConditionCode, - _progress: u64, - ) { - } + fn notice_of_cancellation_cb(&mut self, _fault_info: FaultInfo) {} - 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) {} } /// 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 { pub user_hook: RefCell, } +#[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 FaultHandler { fn condition_code_to_array_index(conditon_code: ConditionCode) -> Option { Some(match conditon_code { @@ -594,13 +603,8 @@ impl FaultHandler { self.handler_array[array_idx.unwrap()] } - pub fn report_fault( - &self, - transaction_id: TransactionId, - condition: ConditionCode, - progress: u64, - ) -> FaultHandlerCode { - let array_idx = Self::condition_code_to_array_index(condition); + pub fn report_fault(&self, fault_info: FaultInfo) -> FaultHandlerCode { + let array_idx = Self::condition_code_to_array_index(fault_info.condition_code()); if array_idx.is_none() { return FaultHandlerCode::IgnoreError; } @@ -608,16 +612,16 @@ impl FaultHandler { 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_cancellation_queue: VecDeque<(TransactionId, ConditionCode, u64)>, - pub abandoned_queue: VecDeque<(TransactionId, ConditionCode, u64)>, - pub ignored_queue: VecDeque<(TransactionId, ConditionCode, u64)>, + pub notice_of_suspension_queue: VecDeque, + pub notice_of_cancellation_queue: VecDeque, + pub abandoned_queue: VecDeque, + pub ignored_queue: VecDeque, } impl UserFaultHook for TestFaultHandler { - fn notice_of_suspension_cb( - &mut self, - transaction_id: TransactionId, - cond: ConditionCode, - progress: u64, - ) { - self.notice_of_suspension_queue - .push_back((transaction_id, cond, progress)) + fn notice_of_suspension_cb(&mut self, fault_info: FaultInfo) { + self.notice_of_suspension_queue.push_back(fault_info) } - fn notice_of_cancellation_cb( - &mut self, - transaction_id: TransactionId, - cond: ConditionCode, - progress: u64, - ) { - self.notice_of_cancellation_queue - .push_back((transaction_id, cond, progress)) + fn notice_of_cancellation_cb(&mut self, fault_info: FaultInfo) { + self.notice_of_cancellation_queue.push_back(fault_info) } - fn abandoned_cb( - &mut self, - transaction_id: TransactionId, - cond: ConditionCode, - progress: u64, - ) { - self.abandoned_queue - .push_back((transaction_id, cond, progress)) + fn abandoned_cb(&mut self, fault_info: FaultInfo) { + self.abandoned_queue.push_back(fault_info) } - fn ignore_cb(&mut self, transaction_id: TransactionId, cond: ConditionCode, progress: u64) { - self.ignored_queue - .push_back((transaction_id, cond, progress)) + fn ignore_cb(&mut self, fault_info: FaultInfo) { + self.ignored_queue.push_back(fault_info) } } @@ -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_suspension_cb(transaction_id, ConditionCode::NoError, 0); - user_hook_dummy.abandoned_cb(transaction_id, ConditionCode::NoError, 0); - user_hook_dummy.ignore_cb(transaction_id, ConditionCode::NoError, 0); + user_hook_dummy.notice_of_cancellation_cb(FaultInfo::new( + transaction_id, + ConditionCode::NoError, + 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] diff --git a/src/source.rs b/src/source.rs index 4a9f24e..55df9cc 100644 --- a/src/source.rs +++ b/src/source.rs @@ -67,8 +67,8 @@ use spacepackets::{ use spacepackets::seq_count::SequenceCounter; use crate::{ - DummyPduProvider, EntityType, GenericSendError, PduProvider, PositiveAckParams, TimerCreator, - time::Countdown, + DummyPduProvider, EntityType, FaultInfo, GenericSendError, PduProvider, PositiveAckParams, + 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(); - assert_eq!(id, expected_id); - assert_eq!(cond_code, ConditionCode::CheckLimitReached); + let FaultInfo { + transaction_id, + 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(); - assert_eq!(id, transfer_info.id); - assert_eq!(cond_code, ConditionCode::PositiveAckLimitReached); + let FaultInfo { + transaction_id, + 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(); - assert_eq!(id, transfer_info.id); - assert_eq!(cond_code, ConditionCode::PositiveAckLimitReached); + let FaultInfo { + transaction_id, + 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(); } diff --git a/tests/end-to-end.rs b/tests/end-to-end.rs index b94d51e..64132b5 100644 --- a/tests/end-to-end.rs +++ b/tests/end-to-end.rs @@ -12,8 +12,8 @@ use std::{ }; use cfdp::{ - EntityType, IndicationConfig, LocalEntityConfig, PduOwnedWithInfo, RemoteEntityConfig, - StdTimerCreator, TransactionId, UserFaultHook, + EntityType, FaultInfo, IndicationConfig, LocalEntityConfig, PduOwnedWithInfo, + 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( - &mut self, - transaction_id: TransactionId, - cond: ConditionCode, - progress: u64, - ) { - panic!( - "unexpected suspension of transaction {:?}, condition code {:?}, progress {}", - transaction_id, cond, progress - ); + fn notice_of_suspension_cb(&mut self, fault_info: FaultInfo) { + panic!("unexpected suspension, {:?}", fault_info); } - fn notice_of_cancellation_cb( - &mut self, - transaction_id: TransactionId, - cond: ConditionCode, - progress: u64, - ) { - panic!( - "unexpected cancellation of transaction {:?}, condition code {:?}, progress {}", - transaction_id, cond, progress - ); + fn notice_of_cancellation_cb(&mut self, fault_info: FaultInfo) { + panic!("unexpected cancellation, {:?}", fault_info); } - fn abandoned_cb(&mut self, transaction_id: TransactionId, cond: ConditionCode, progress: u64) { - panic!( - "unexpected abandonment of transaction {:?}, condition code {:?}, progress {}", - transaction_id, cond, progress - ); + fn abandoned_cb(&mut self, fault_info: FaultInfo) { + panic!("unexpected abandonment, {:?}", fault_info); } - fn ignore_cb(&mut self, transaction_id: TransactionId, cond: ConditionCode, progress: u64) { - panic!( - "ignoring unexpected error in transaction {:?}, condition code {:?}, progress {}", - transaction_id, cond, progress - ); + fn ignore_cb(&mut self, fault_info: FaultInfo) { + panic!("unexpected ignore, {:?}", fault_info); } }