From aeb19081c2c288c5d10f7b63d9a3befaeac79708 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 15 Sep 2025 14:59:03 +0200 Subject: [PATCH] clean up a little --- src/dest.rs | 164 ++++++++++++++++++++++++++++++++-------------------- 1 file changed, 102 insertions(+), 62 deletions(-) diff --git a/src/dest.rs b/src/dest.rs index aae3ac9..e0cda44 100644 --- a/src/dest.rs +++ b/src/dest.rs @@ -24,7 +24,7 @@ //! //! 3. Finished PDU has been sent back to the remote side. //! -//! ### Acknowledged mode (*not implemented yet*) +//! ### Acknowledged mode //! //! 3. An EOF ACK PDU has been sent back to the remote side. //! 4. A Finished PDU has been sent back to the remote side. @@ -114,6 +114,32 @@ pub enum TransactionStep { WaitingForFinishedAck, } +#[derive(Debug)] +struct FinishedParams { + condition_code: Cell, + delivery_code: Cell, + fault_location_finished: Cell>, + file_status: FileStatus, +} + +impl Default for FinishedParams { + fn default() -> Self { + Self { + condition_code: Cell::new(ConditionCode::NoError), + delivery_code: Cell::new(DeliveryCode::Incomplete), + fault_location_finished: Cell::new(None), + file_status: FileStatus::Unreported, + } + } +} + +impl FinishedParams { + pub fn reset(&mut self) { + self.condition_code.set(ConditionCode::NoError); + self.delivery_code.set(DeliveryCode::Incomplete); + self.file_status = FileStatus::Unreported; + } +} // This contains transfer state parameters for destination transaction. #[derive(Debug)] struct TransferState { @@ -122,11 +148,8 @@ struct TransferState { progress: u64, // TODO: Can we delete this for good? // file_size_eof: u64, + finished_params: FinishedParams, metadata_only: bool, - condition_code: Cell, - delivery_code: Cell, - fault_location_finished: Cell>, - file_status: FileStatus, completion_disposition: Cell, checksum: u32, anomaly_tracker: AnomalyTracker, @@ -142,10 +165,7 @@ impl Default for TransferState { progress: Default::default(), // file_size_eof: Default::default(), metadata_only: false, - condition_code: Cell::new(ConditionCode::NoError), - delivery_code: Cell::new(DeliveryCode::Incomplete), - fault_location_finished: Cell::new(None), - file_status: FileStatus::Unreported, + finished_params: FinishedParams::default(), completion_disposition: Cell::new(CompletionDisposition::Completed), checksum: 0, current_check_count: 0, @@ -211,13 +231,7 @@ impl Default for TransactionParams { impl TransactionParams { fn reset(&mut self) { - self.transfer_state - .condition_code - .set(ConditionCode::NoError); - self.transfer_state - .delivery_code - .set(DeliveryCode::Incomplete); - self.transfer_state.file_status = FileStatus::Unreported; + self.transfer_state.finished_params.reset(); self.transfer_state.anomaly_tracker.reset(); } } @@ -432,6 +446,7 @@ impl< /// Returns [None] if the state machine is IDLE, and the transmission mode of the current /// request otherwise. + #[inline] pub fn transmission_mode(&self) -> Option { if self.state == State::Idle { return None; @@ -439,10 +454,44 @@ impl< Some(self.transaction_params.transmission_mode()) } + #[inline] pub fn transaction_id(&self) -> Option { self.tstate().transaction_id } + /// Get the step, which denotes the exact step of a pending CFDP transaction when applicable. + #[inline] + pub fn step(&self) -> TransactionStep { + self.step.get() + } + + /// Get the step, which denotes whether the CFDP handler is active, and which CFDP class + /// is used if it is active. + #[inline] + pub fn state(&self) -> State { + self.state + } + + #[inline] + pub fn local_cfg(&self) -> &LocalEntityConfig { + &self.local_cfg + } + + #[inline] + pub fn local_cfg_mut(&mut self) -> &mut LocalEntityConfig { + &mut self.local_cfg + } + + /// This function is public to allow completely resetting the handler, but it is explicitely + /// discouraged to do this. CFDP has mechanism to detect issues and errors on itself. + /// Resetting the handler might interfere with these mechanisms and lead to unexpected + /// behaviour. + pub fn reset(&mut self) { + self.set_step(TransactionStep::Idle); + self.state = State::Idle; + self.transaction_params.reset(); + } + fn insert_packet( &mut self, cfdp_user: &mut impl CfdpUser, @@ -655,11 +704,13 @@ impl< if eof_pdu.file_size() > 0 { self.transaction_params .transfer_state + .finished_params .delivery_code .set(DeliveryCode::Incomplete); } else { self.transaction_params .transfer_state + .finished_params .delivery_code .set(DeliveryCode::Complete); } @@ -686,10 +737,12 @@ impl< .set(CompletionDisposition::Cancelled); self.transaction_params .transfer_state + .finished_params .condition_code .set(cond_code); self.transaction_params .transfer_state + .finished_params .fault_location_finished .set(Some(fault_location)); // For anything larger than 0, we'd have to do a checksum check to verify whether @@ -697,6 +750,7 @@ impl< if self.transaction_params.transfer_state.progress == 0 { self.transaction_params .transfer_state + .finished_params .delivery_code .set(DeliveryCode::Complete); } @@ -750,7 +804,11 @@ impl< let ack_pdu = AckPdu::new( pdu_header, FileDirectiveType::EofPdu, - self.transaction_params.transfer_state.condition_code.get(), + self.transaction_params + .transfer_state + .finished_params + .condition_code + .get(), TransactionStatus::Active, ) .unwrap(); @@ -790,10 +848,12 @@ impl< if !checksum_success { self.transaction_params .transfer_state + .finished_params .delivery_code .set(DeliveryCode::Incomplete); self.transaction_params .transfer_state + .finished_params .condition_code .set(ConditionCode::FileChecksumFailure); } @@ -824,10 +884,12 @@ impl< if file_delivery_complete { self.transaction_params .transfer_state + .finished_params .delivery_code .set(DeliveryCode::Complete); self.transaction_params .transfer_state + .finished_params .condition_code .set(ConditionCode::NoError); } @@ -889,7 +951,7 @@ impl< Ok(sent_packets) } - pub fn handle_prompt_pdu(&mut self, _raw_packet: &[u8]) -> Result<(), DestError> { + fn handle_prompt_pdu(&mut self, _raw_packet: &[u8]) -> Result<(), DestError> { Err(DestError::NotImplemented) } @@ -917,19 +979,6 @@ impl< Ok(sent_packets) } - /// Get the step, which denotes the exact step of a pending CFDP transaction when applicable. - #[inline] - pub fn step(&self) -> TransactionStep { - self.step.get() - } - - /// Get the step, which denotes whether the CFDP handler is active, and which CFDP class - /// is used if it is active. - #[inline] - pub fn state(&self) -> State { - self.state - } - fn transaction_start(&mut self, cfdp_user: &mut impl CfdpUser) -> Result<(), DestError> { let dest_name = from_utf8( &self.transaction_params.file_properties.dest_file_name @@ -998,7 +1047,10 @@ impl< } else { self.vfs.create_file(dest_path_str)?; } - self.transaction_params.transfer_state.file_status = FileStatus::Retained; + self.transaction_params + .transfer_state + .finished_params + .file_status = FileStatus::Retained; drop(msgs_to_user); self.set_step(TransactionStep::ReceivingFileDataPdus); Ok(()) @@ -1031,7 +1083,7 @@ impl< .as_ref() .unwrap() .disposition_on_cancellation - && self.tstate().delivery_code.get() == DeliveryCode::Incomplete + && self.tstate().finished_params.delivery_code.get() == DeliveryCode::Incomplete { // Safety: We already verified that the path is valid during the transaction start. let dest_path = unsafe { @@ -1043,14 +1095,14 @@ impl< if self.vfs.exists(dest_path)? && self.vfs.is_file(dest_path)? { self.vfs.remove_file(dest_path)?; } - self.tstate_mut().file_status = FileStatus::DiscardDeliberately; + self.tstate_mut().finished_params.file_status = FileStatus::DiscardDeliberately; } let tstate = self.tstate(); let transaction_finished_params = TransactionFinishedParams { id: tstate.transaction_id.unwrap(), - condition_code: tstate.condition_code.get(), - delivery_code: tstate.delivery_code.get(), - file_status: tstate.file_status, + condition_code: tstate.finished_params.condition_code.get(), + delivery_code: tstate.finished_params.delivery_code.get(), + file_status: tstate.finished_params.file_status, }; cfdp_user.transaction_finished_indication(&transaction_finished_params); Ok(()) @@ -1082,11 +1134,14 @@ impl< fn notice_of_cancellation(&self, condition_code: ConditionCode, fault_location: EntityIdTlv) { self.set_step_internal(TransactionStep::TransferCompletion); let tstate = self.tstate(); - tstate.condition_code.set(condition_code); + tstate.finished_params.condition_code.set(condition_code); tstate .completion_disposition .set(CompletionDisposition::Cancelled); - tstate.fault_location_finished.set(Some(fault_location)); + tstate + .finished_params + .fault_location_finished + .set(Some(fault_location)); } fn notice_of_suspension(&self) { @@ -1107,36 +1162,26 @@ impl< self.set_step_internal(step); } - /// This function is public to allow completely resetting the handler, but it is explicitely - /// discouraged to do this. CFDP has mechanism to detect issues and errors on itself. - /// Resetting the handler might interfere with these mechanisms and lead to unexpected - /// behaviour. - pub fn reset(&mut self) { - self.set_step(TransactionStep::Idle); - self.state = State::Idle; - self.transaction_params.reset(); - } - fn send_finished_pdu(&mut self) -> Result { let tstate = self.tstate(); let pdu_header = PduHeader::new_no_file_data(self.transaction_params.pdu_conf, 0); - let finished_pdu = if tstate.condition_code.get() == ConditionCode::NoError - || tstate.condition_code.get() == ConditionCode::UnsupportedChecksumType + let finished_pdu = if tstate.finished_params.condition_code.get() == ConditionCode::NoError + || tstate.finished_params.condition_code.get() == ConditionCode::UnsupportedChecksumType { FinishedPduCreator::new_default( pdu_header, - tstate.delivery_code.get(), - tstate.file_status, + tstate.finished_params.delivery_code.get(), + tstate.finished_params.file_status, ) } else { FinishedPduCreator::new_generic( pdu_header, - tstate.condition_code.get(), - tstate.delivery_code.get(), - tstate.file_status, + tstate.finished_params.condition_code.get(), + tstate.finished_params.delivery_code.get(), + tstate.finished_params.file_status, &[], - tstate.fault_location_finished.get(), + tstate.finished_params.fault_location_finished.get(), ) }; finished_pdu.write_to_bytes(self.pdu_and_cksum_buffer.get_mut())?; @@ -1147,11 +1192,6 @@ impl< Ok(1) } - #[inline] - pub fn local_cfg(&self) -> &LocalEntityConfig { - &self.local_cfg - } - #[inline] fn tstate(&self) -> &TransferState { &self.transaction_params.transfer_state