clean up a little

2025-09-15 14:59:03 +02:00
parent 73cbcfdd2a
commit aeb19081c2
1 changed files with 102 additions and 62 deletions
--- 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<ConditionCode>,
+    delivery_code: Cell<DeliveryCode>,
+    fault_location_finished: Cell<Option<EntityIdTlv>>,
+    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<Countdown: CountdownProvider> {
@@ -122,11 +148,8 @@ struct TransferState<Countdown: CountdownProvider> {
    progress: u64,
    // TODO: Can we delete this for good?
    // file_size_eof: u64,
+    finished_params: FinishedParams,
    metadata_only: bool,
-    condition_code: Cell<ConditionCode>,
-    delivery_code: Cell<DeliveryCode>,
-    fault_location_finished: Cell<Option<EntityIdTlv>>,
-    file_status: FileStatus,
    completion_disposition: Cell<CompletionDisposition>,
    checksum: u32,
    anomaly_tracker: AnomalyTracker,
@@ -142,10 +165,7 @@ impl<CheckTimer: CountdownProvider> Default for TransferState<CheckTimer> {
            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<CheckTimer: CountdownProvider> Default for TransactionParams<CheckTimer> {

 impl<CheckTimer: CountdownProvider> TransactionParams<CheckTimer> {
    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<TransmissionMode> {
        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<TransactionId> {
        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<UserFaultHook> {
+        &self.local_cfg
+    }
+
+    #[inline]
+    pub fn local_cfg_mut(&mut self) -> &mut LocalEntityConfig<UserFaultHook> {
+        &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<u32, DestError> {
        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<UserFaultHook> {
-        &self.local_cfg
-    }
-
    #[inline]
    fn tstate(&self) -> &TransferState<Countdown> {
        &self.transaction_params.transfer_state