From c0a7dfd9f8878494e45f842567411928ef4ab440 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 12 Sep 2025 15:56:17 +0200 Subject: [PATCH] Add acknowledged destination handler --- .github/workflows/ci.yml | 2 +- Cargo.toml | 5 +- examples/python-interop/main.rs | 13 +- src/dest.rs | 1531 ++++++++++++++++++++++--------- src/lib.rs | 263 ++++-- src/lost_segments.rs | 1339 +++++++++++++++++++++++++++ src/source.rs | 380 ++++---- src/time.rs | 2 +- tests/end-to-end.rs | 6 +- 9 files changed, 2802 insertions(+), 739 deletions(-) create mode 100644 src/lost_segments.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0e5f31a..52ed3b7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -29,7 +29,7 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 - - uses: dtolnay/rust-toolchain@1.85.0 + - uses: dtolnay/rust-toolchain@1.86.0 - run: cargo check --release cross-check: diff --git a/Cargo.toml b/Cargo.toml index 2d03fa7..9436268 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,7 +2,7 @@ name = "cfdp-rs" version = "0.2.0" edition = "2024" -rust-version = "1.85.0" +rust-version = "1.86.0" authors = ["Robin Mueller "] description = "High level CCSDS File Delivery Protocol components" homepage = "https://egit.irs.uni-stuttgart.de/rust/cfdp" @@ -22,6 +22,7 @@ derive-new = ">=0.6, <=0.7" hashbrown = { version = ">=0.14, <=0.15", optional = true } spacepackets = { git = "https://egit.irs.uni-stuttgart.de/rust/spacepackets.git", version = "0.16", default-features = false } thiserror = { version = "2", default-features = false } +heapless = "0.9" serde = { version = "1", optional = true } defmt = { version = "1", optional = true } @@ -36,7 +37,7 @@ alloc = [ "hashbrown", "spacepackets/alloc" ] -serde = ["dep:serde", "spacepackets/serde", "hashbrown/serde"] +serde = ["dep:serde", "spacepackets/serde", "hashbrown/serde", "heapless/serde"] defmt = ["dep:defmt", "spacepackets/defmt"] [dev-dependencies] diff --git a/examples/python-interop/main.rs b/examples/python-interop/main.rs index ef21d7b..b8f2f64 100644 --- a/examples/python-interop/main.rs +++ b/examples/python-interop/main.rs @@ -13,9 +13,10 @@ use std::{ use cfdp::{ EntityType, IndicationConfig, LocalEntityConfig, PduOwnedWithInfo, PduProvider, - RemoteEntityConfig, StdTimerCreator, TransactionId, UserFaultHookProvider, + RemoteEntityConfig, StdTimerCreator, TransactionId, UserFaultHook, dest::DestinationHandler, filestore::NativeFilestore, + lost_segments::LostSegmentsList, request::{PutRequestOwned, StaticPutRequestCacher}, source::SourceHandler, user::{CfdpUser, FileSegmentRecvdParams, MetadataReceivedParams, TransactionFinishedParams}, @@ -62,7 +63,7 @@ pub struct Cli { #[derive(Default)] pub struct ExampleFaultHandler {} -impl UserFaultHookProvider for ExampleFaultHandler { +impl UserFaultHook for ExampleFaultHandler { fn notice_of_suspension_cb( &mut self, transaction_id: TransactionId, @@ -261,7 +262,7 @@ impl UdpServer { while let Ok(tm) = receiver.try_recv() { debug!("Sending PDU: {:?}", tm); pdu_printout(&tm); - let result = self.socket.send_to(tm.pdu(), self.remote_addr()); + let result = self.socket.send_to(tm.raw_pdu(), self.remote_addr()); if let Err(e) = result { warn!("Sending TM with UDP socket failed: {e}") } @@ -284,7 +285,7 @@ fn pdu_printout(pdu: &PduOwnedWithInfo) { spacepackets::cfdp::pdu::FileDirectiveType::AckPdu => (), spacepackets::cfdp::pdu::FileDirectiveType::MetadataPdu => { let meta_pdu = - MetadataPduReader::new(pdu.pdu()).expect("creating metadata pdu failed"); + MetadataPduReader::new(pdu.raw_pdu()).expect("creating metadata pdu failed"); debug!("Metadata PDU: {:?}", meta_pdu) } spacepackets::cfdp::pdu::FileDirectiveType::NakPdu => (), @@ -292,7 +293,8 @@ fn pdu_printout(pdu: &PduOwnedWithInfo) { spacepackets::cfdp::pdu::FileDirectiveType::KeepAlivePdu => (), }, spacepackets::cfdp::PduType::FileData => { - let fd_pdu = FileDataPdu::from_bytes(pdu.pdu()).expect("creating file data pdu failed"); + let fd_pdu = + FileDataPdu::from_bytes(pdu.raw_pdu()).expect("creating file data pdu failed"); debug!("File data PDU: {:?}", fd_pdu); } } @@ -367,6 +369,7 @@ fn main() { NativeFilestore::default(), remote_cfg_python, StdTimerCreator::default(), + LostSegmentsList::default(), ); let mut cfdp_user_dest = ExampleCfdpUser::new(EntityType::Receiving); diff --git a/src/dest.rs b/src/dest.rs index b36f648..f780c50 100644 --- a/src/dest.rs +++ b/src/dest.rs @@ -24,31 +24,40 @@ //! //! 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. //! 5. A Finished PDU ACK was received. -use crate::{DummyPduProvider, GenericSendError, PduProvider, user::TransactionFinishedParams}; -use core::str::{Utf8Error, from_utf8, from_utf8_unchecked}; +use crate::{ + DummyPduProvider, GenericSendError, IndicationConfig, PduProvider, PositiveAckParams, + lost_segments::{LostSegmentError, LostSegmentStore}, + user::TransactionFinishedParams, +}; +use core::{ + cell::{Cell, RefCell}, + str::{Utf8Error, from_utf8, from_utf8_unchecked}, +}; use super::{ - CountdownProvider, EntityType, LocalEntityConfig, PacketTarget, PduSendProvider, - RemoteEntityConfig, RemoteEntityConfigProvider, State, TimerContext, TimerCreatorProvider, - TransactionId, UserFaultHookProvider, + Countdown, EntityType, LocalEntityConfig, PacketTarget, PduSendProvider, RemoteConfigStore, + RemoteEntityConfig, State, TimerContext, TimerCreator, TransactionId, UserFaultHook, filestore::{FilestoreError, VirtualFilestore}, user::{CfdpUser, FileSegmentRecvdParams, MetadataReceivedParams}, }; use smallvec::SmallVec; use spacepackets::{ cfdp::{ - ChecksumType, ConditionCode, FaultHandlerCode, PduType, TransmissionMode, + ChecksumType, ConditionCode, FaultHandlerCode, LargeFileFlag, PduType, TransactionStatus, + TransmissionMode, pdu::{ CfdpPdu, CommonPduConfig, FileDirectiveType, PduError, PduHeader, + ack::AckPdu, eof::EofPdu, file_data::FileDataPdu, finished::{DeliveryCode, FileStatus, FinishedPduCreator}, metadata::{MetadataGenericParams, MetadataPduReader}, + nak::NakPduCreatorWithReservedSeqReqsBuf, }, tlv::{EntityIdTlv, GenericTlv, ReadableTlv, TlvType, msg_to_user::MsgToUserTlv}, }, @@ -56,7 +65,7 @@ use spacepackets::{ }; #[derive(Debug)] -struct FileProperties { +struct FileNames { src_file_name: [u8; u8::MAX as usize], src_file_name_len: usize, dest_file_name: [u8; u8::MAX as usize], @@ -65,6 +74,39 @@ struct FileProperties { dest_file_path_len: usize, } +#[derive(Debug, Default, Clone, Copy)] +pub struct AnomalyTracker { + invalid_ack_directive_code: u8, + lost_segment_errors: u8, +} + +impl AnomalyTracker { + #[inline] + pub fn invalid_ack_directive_code(&mut self) -> u8 { + self.invalid_ack_directive_code + } + + #[inline] + pub fn lost_segment_errors(&mut self) -> u8 { + self.lost_segment_errors + } + + #[inline] + pub fn increment_lost_segment_errors(&mut self) { + self.lost_segment_errors = self.lost_segment_errors.wrapping_add(1); + } + + #[inline] + pub fn increment_invalid_ack_directive_code(&mut self) { + self.invalid_ack_directive_code = self.invalid_ack_directive_code.wrapping_add(1); + } + + #[inline] + pub fn reset(&mut self) { + *self = Default::default(); + } +} + #[derive(Debug, PartialEq, Eq, Copy, Clone)] enum CompletionDisposition { Completed = 0, @@ -76,74 +118,92 @@ enum CompletionDisposition { #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[cfg_attr(feature = "defmt", derive(defmt::Format))] pub enum TransactionStep { - Idle = 0, - TransactionStart = 1, - ReceivingFileDataPdus = 2, - ReceivingFileDataPdusWithCheckLimitHandling = 3, - SendingAckPdu = 4, - TransferCompletion = 5, - SendingFinishedPdu = 6, + Idle, + TransactionStart, + /// Special state which is only used for acknowledged mode. The CFDP entity is still waiting + /// for a missing metadata PDU to be re-sent. Until then, all arriving file data PDUs will only + /// update the internal lost segment tracker. When the EOF PDU arrives, the state will be left. + /// Please note that deferred lost segment handling might also be active when this state is set. + WaitingForMetadata, + ReceivingFileDataPdus, + /// This is the check timer step as specified in chapter 4.6.3.3 b) of the standard. + /// The destination handler will still check for file data PDUs which might lead to a full + /// file transfer completion. + ReceivingFileDataPdusWithCheckLimitHandling, + WaitingForMissingData, + //SendingAckPdu, + TransferCompletion, + WaitingForFinishedAck, } -// This contains transfer state parameters for destination transaction. #[derive(Debug)] -struct TransferState { - transaction_id: Option, - metadata_params: MetadataGenericParams, - progress: u64, - // TODO: Can we delete this for good? - // file_size_eof: u64, - metadata_only: bool, - condition_code: ConditionCode, - delivery_code: DeliveryCode, - fault_location_finished: Option, +struct FinishedParams { + condition_code: Cell, + delivery_code: Cell, + fault_location_finished: Cell>, file_status: FileStatus, - completion_disposition: CompletionDisposition, - checksum: u32, - current_check_count: u32, - current_check_timer: Option, } -impl Default for TransferState { +impl Default for FinishedParams { fn default() -> Self { Self { - transaction_id: None, - metadata_params: Default::default(), - progress: Default::default(), - // file_size_eof: Default::default(), - metadata_only: false, - condition_code: ConditionCode::NoError, - delivery_code: DeliveryCode::Incomplete, - fault_location_finished: None, + condition_code: Cell::new(ConditionCode::NoError), + delivery_code: Cell::new(DeliveryCode::Incomplete), + fault_location_finished: Cell::new(None), file_status: FileStatus::Unreported, - completion_disposition: CompletionDisposition::Completed, - checksum: 0, - current_check_count: 0, - current_check_timer: None, } } } +impl FinishedParams { + pub fn reset(&mut self) { + self.condition_code.set(ConditionCode::NoError); + self.delivery_code.set(DeliveryCode::Incomplete); + self.file_status = FileStatus::Unreported; + } +} + +#[derive(Debug)] +pub struct AcknowledgedModeParams { + last_start_offset: u64, + last_end_offset: u64, + metadata_missing: bool, + nak_activity_counter: u32, + deferred_procedure_active: bool, +} + // This contains parameters for destination transaction. #[derive(Debug)] -struct TransactionParams { - tstate: TransferState, +struct TransactionParams { pdu_conf: CommonPduConfig, - file_properties: FileProperties, - cksum_buf: [u8; 1024], + file_names: FileNames, msgs_to_user_size: usize, // TODO: Should we make this configurable? msgs_to_user_buf: [u8; 1024], remote_cfg: Option, + transaction_id: Option, + metadata_params: MetadataGenericParams, + file_size: u64, + progress: u64, + acked_params: Option, + deferred_procedure_timer: Option, + finished_params: FinishedParams, + positive_ack_params: Option>, + metadata_only: bool, + completion_disposition: Cell, + checksum: u32, + anomaly_tracker: AnomalyTracker, + current_check_count: u32, + current_check_timer: Option, } -impl TransactionParams { +impl TransactionParams { fn transmission_mode(&self) -> TransmissionMode { self.pdu_conf.trans_mode } } -impl Default for FileProperties { +impl Default for FileNames { fn default() -> Self { Self { src_file_name: [0; u8::MAX as usize], @@ -156,39 +216,51 @@ impl Default for FileProperties { } } -impl TransactionParams { +impl TransactionParams { fn file_size(&self) -> u64 { - self.tstate.metadata_params.file_size + self.metadata_params.file_size } fn metadata_params(&self) -> &MetadataGenericParams { - &self.tstate.metadata_params + &self.metadata_params } } -impl Default for TransactionParams { +impl Default for TransactionParams { fn default() -> Self { Self { pdu_conf: Default::default(), - cksum_buf: [0; 1024], msgs_to_user_size: 0, + file_size: 0, msgs_to_user_buf: [0; 1024], - tstate: Default::default(), - file_properties: Default::default(), + file_names: Default::default(), remote_cfg: None, + transaction_id: None, + metadata_params: Default::default(), + progress: Default::default(), + deferred_procedure_timer: None, + acked_params: None, + metadata_only: false, + positive_ack_params: None, + finished_params: FinishedParams::default(), + completion_disposition: Cell::new(CompletionDisposition::Completed), + checksum: 0, + current_check_count: 0, + anomaly_tracker: AnomalyTracker::default(), + current_check_timer: None, } } } -impl TransactionParams { +impl TransactionParams { fn reset(&mut self) { - self.tstate.condition_code = ConditionCode::NoError; - self.tstate.delivery_code = DeliveryCode::Incomplete; - self.tstate.file_status = FileStatus::Unreported; + self.finished_params.reset(); + self.anomaly_tracker.reset(); } } #[derive(Debug, thiserror::Error)] +#[non_exhaustive] pub enum DestError { /// File directive expected, but none specified #[error("expected file directive")] @@ -198,10 +270,17 @@ pub enum DestError { pdu_type: PduType, directive_type: Option, }, - #[error("can not process file data PDUs in current state")] - WrongStateForFileData, - #[error("can not process EOF PDUs in current state")] - WrongStateForEof, + #[error("first packet must be metadata PDU for unacknowledged transfers")] + FirstPacketNotMetadata, + #[error( + "can not process PDU with type {pdu_type:?} and directive field {file_directive_type:?} in \ + current transaction step {step:?}" + )] + WrongStepForPdu { + step: TransactionStep, + pdu_type: PduType, + file_directive_type: Option, + }, // Received new metadata PDU while being already being busy with a file transfer. #[error("busy with transfer")] RecvdMetadataButIsBusy, @@ -209,21 +288,21 @@ pub enum DestError { EmptySrcFileField, #[error("empty dest file field")] EmptyDestFileField, - #[error("packets to be sent are still left")] - PacketToSendLeft, #[error("pdu error {0}")] Pdu(#[from] PduError), #[error("io error {0}")] #[cfg(feature = "std")] Io(#[from] std::io::Error), - #[error("file store error {0}")] + #[error("file store error: {0}")] Filestore(#[from] FilestoreError), + #[error("lost segment error: {0}")] + LostSegmentError(#[from] LostSegmentError), #[error("path conversion error {0}")] PathConversion(#[from] Utf8Error), #[error("error building dest path from source file name and dest folder")] PathConcat, #[error("no remote entity configuration found for {0:?}")] - NoRemoteCfgFound(UnsignedByteField), + NoRemoteConfigFound(UnsignedByteField), #[error("issue sending PDU: {0}")] SendError(#[from] GenericSendError), #[error("cfdp feature not implemented")] @@ -256,21 +335,23 @@ pub enum DestError { /// run them inside a thread pool, or move the newly created handler to a new thread.""" pub struct DestinationHandler< PduSender: PduSendProvider, - UserFaultHook: UserFaultHookProvider, + UserFaultHookInstance: UserFaultHook, Vfs: VirtualFilestore, - RemoteCfgTable: RemoteEntityConfigProvider, - CheckTimerCreator: TimerCreatorProvider, - CheckTimerProvider: CountdownProvider, + RemoteConfigStoreInstance: RemoteConfigStore, + TimerCreatorInstance: TimerCreator, + CountdownInstance: Countdown, + LostSegmentTracker: LostSegmentStore, > { - local_cfg: LocalEntityConfig, - step: TransactionStep, + local_cfg: LocalEntityConfig, + step: core::cell::Cell, state: State, - tparams: TransactionParams, - packet_buf: alloc::vec::Vec, + transaction_params: TransactionParams, + pdu_and_cksum_buffer: RefCell>, pub pdu_sender: PduSender, pub vfs: Vfs, - pub remote_cfg_table: RemoteCfgTable, - pub check_timer_creator: CheckTimerCreator, + pub remote_cfg_table: RemoteConfigStoreInstance, + pub check_timer_creator: TimerCreatorInstance, + lost_segment_tracker: LostSegmentTracker, } #[cfg(feature = "std")] @@ -278,30 +359,64 @@ pub type StdDestinationHandler = DestinationHandler< PduSender, UserFaultHook, crate::filestore::NativeFilestore, - crate::StdRemoteEntityConfigProvider, + crate::RemoteConfigStoreStd, crate::StdTimerCreator, crate::StdCountdown, + crate::lost_segments::LostSegmentsList, >; +#[cfg(feature = "std")] +impl + StdDestinationHandler +{ + #[cfg(feature = "std")] + pub fn new_std( + local_cfg: LocalEntityConfig, + pdu_and_cksum_buf_size: usize, + pdu_sender: PduSender, + ) -> Self { + Self::new( + local_cfg, + pdu_and_cksum_buf_size, + pdu_sender, + crate::filestore::NativeFilestore::default(), + crate::RemoteConfigStoreStd::default(), + crate::StdTimerCreator::default(), + crate::lost_segments::LostSegmentsList::default(), + ) + } +} + impl< PduSender: PduSendProvider, - UserFaultHook: UserFaultHookProvider, + UserFaultHookInstance: UserFaultHook, Vfs: VirtualFilestore, - RemoteCfgTable: RemoteEntityConfigProvider, - TimerCreator: TimerCreatorProvider, - Countdown: CountdownProvider, -> DestinationHandler + RemoteConfigStoreInstance: RemoteConfigStore, + TimerCreatorInstance: TimerCreator, + CountdownInstance: Countdown, + LostSegmentTracker: LostSegmentStore, +> + DestinationHandler< + PduSender, + UserFaultHookInstance, + Vfs, + RemoteConfigStoreInstance, + TimerCreatorInstance, + CountdownInstance, + LostSegmentTracker, + > { /// Constructs a new destination handler. /// /// # Arguments /// /// * `local_cfg` - The local CFDP entity configuration. - /// * `max_packet_len` - The maximum expected generated packet size in bytes. Each time a - /// packet is sent, it will be buffered inside an internal buffer. The length of this buffer - /// will be determined by this parameter. This parameter can either be a known upper bound, - /// or it can specifically be determined by the largest packet size parameter of all remote - /// entity configurations in the passed `remote_cfg_table`. + /// + /// * `pdu_and_cksum_buf_size` - The handler requires a buffer to generate PDUs and perform + /// checksum calculations. This parameter can either be a known upper bound for the packet + /// size, for example 2048 or 4096 bytes. + /// It can also specifically be determined by the largest packet size parameter of all + /// remote entity configurations in the passed `remote_cfg_table`. /// * `pdu_sender` - [PduSendProvider] used to send generated PDU packets. /// * `vfs` - [VirtualFilestore] implementation used by the handler, which decouples the CFDP /// implementation from the underlying filestore/filesystem. This allows to use this handler @@ -312,23 +427,25 @@ impl< /// timers required by various tasks. This allows to use this handler for embedded systems /// where the standard time APIs might not be available. pub fn new( - local_cfg: LocalEntityConfig, - max_packet_len: usize, + local_cfg: LocalEntityConfig, + pdu_and_cksum_buf_size: usize, pdu_sender: PduSender, vfs: Vfs, - remote_cfg_table: RemoteCfgTable, - timer_creator: TimerCreator, + remote_cfg_table: RemoteConfigStoreInstance, + timer_creator: TimerCreatorInstance, + lost_segment_tracker: LostSegmentTracker, ) -> Self { Self { local_cfg, - step: TransactionStep::Idle, + step: Cell::new(TransactionStep::Idle), state: State::Idle, - tparams: Default::default(), - packet_buf: alloc::vec![0; max_packet_len], + transaction_params: Default::default(), + pdu_and_cksum_buffer: core::cell::RefCell::new(alloc::vec![0; pdu_and_cksum_buf_size]), pdu_sender, vfs, remote_cfg_table, check_timer_creator: timer_creator, + lost_segment_tracker, } } @@ -387,7 +504,7 @@ impl< EntityIdTlv::new(self.local_cfg.id), ); - self.step = TransactionStep::TransferCompletion; + self.set_step(TransactionStep::TransferCompletion); return true; } } @@ -396,22 +513,58 @@ 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; } - Some(self.tparams.transmission_mode()) + Some(self.transaction_params.transmission_mode()) } + #[inline] pub fn transaction_id(&self) -> Option { - self.tstate().transaction_id + self.transaction_params.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, packet_to_insert: &impl PduProvider, - ) -> Result<(), DestError> { + ) -> Result { + let mut sent_packets = 0; if packet_to_insert.packet_target()? != PacketTarget::DestEntity { // Unwrap is okay here, a PacketInfo for a file data PDU should always have the // destination as the target. @@ -425,14 +578,18 @@ impl< if packet_to_insert.file_directive_type().is_none() { return Err(DestError::DirectiveFieldEmpty); } - self.handle_file_directive( + sent_packets += self.handle_file_directive( cfdp_user, packet_to_insert.file_directive_type().unwrap(), - packet_to_insert.pdu(), - ) + packet_to_insert.raw_pdu(), + )?; + } + PduType::FileData => { + let fd_pdu = FileDataPdu::from_bytes(packet_to_insert.raw_pdu())?; + self.handle_file_data(cfdp_user, fd_pdu)?; } - PduType::FileData => self.handle_file_data(cfdp_user, packet_to_insert.pdu()), } + Ok(sent_packets) } fn handle_file_directive( @@ -440,9 +597,13 @@ impl< cfdp_user: &mut impl CfdpUser, pdu_directive: FileDirectiveType, raw_packet: &[u8], - ) -> Result<(), DestError> { + ) -> Result { + let mut sent_packets = 0; match pdu_directive { - FileDirectiveType::EofPdu => self.handle_eof_pdu(cfdp_user, raw_packet)?, + FileDirectiveType::EofPdu => { + let eof_pdu = EofPdu::from_bytes(raw_packet)?; + sent_packets += self.handle_eof_pdu(cfdp_user, eof_pdu)? + } FileDirectiveType::FinishedPdu | FileDirectiveType::NakPdu | FileDirectiveType::KeepAlivePdu => { @@ -452,172 +613,439 @@ impl< }); } FileDirectiveType::AckPdu => { - return Err(DestError::NotImplemented); + let ack_pdu = AckPdu::from_bytes(raw_packet)?; + self.handle_ack_pdu(ack_pdu)?; + } + FileDirectiveType::MetadataPdu => { + let metadata_pdu = MetadataPduReader::from_bytes(raw_packet)?; + self.handle_metadata_pdu(metadata_pdu, self.step() == TransactionStep::Idle)? } - FileDirectiveType::MetadataPdu => self.handle_metadata_pdu(raw_packet)?, FileDirectiveType::PromptPdu => self.handle_prompt_pdu(raw_packet)?, }; + Ok(sent_packets) + } + + fn handle_ack_pdu(&mut self, ack_pdu: AckPdu) -> Result<(), DestError> { + if ack_pdu.directive_code_of_acked_pdu() != FileDirectiveType::FinishedPdu { + self.transaction_params + .anomaly_tracker + .increment_invalid_ack_directive_code(); + } + // We are done. + self.reset(); Ok(()) } - fn handle_metadata_pdu(&mut self, raw_packet: &[u8]) -> Result<(), DestError> { + fn first_packet_handling(&mut self, pdu_config: &CommonPduConfig) -> Result<(), DestError> { + self.transaction_params.reset(); + let remote_cfg = self.remote_cfg_table.get(pdu_config.source_id().value()); + if remote_cfg.is_none() { + return Err(DestError::NoRemoteConfigFound(pdu_config.source_id())); + } + self.transaction_params.remote_cfg = Some(*remote_cfg.unwrap()); + self.transaction_params.transaction_id = Some(TransactionId::new( + pdu_config.source_id(), + pdu_config.transaction_seq_num, + )); + self.transaction_params.pdu_conf = *pdu_config; + self.transaction_params.pdu_conf.direction = spacepackets::cfdp::Direction::TowardsSender; + self.state = State::Busy; + Ok(()) + } + + fn handle_metadata_pdu( + &mut self, + metadata_pdu: MetadataPduReader, + first_packet: bool, + ) -> Result<(), DestError> { if self.state != State::Idle { return Err(DestError::RecvdMetadataButIsBusy); } - let metadata_pdu = MetadataPduReader::from_bytes(raw_packet)?; - self.tparams.reset(); - self.tparams.tstate.metadata_params = *metadata_pdu.metadata_params(); + if first_packet { + self.first_packet_handling(metadata_pdu.pdu_header().common_pdu_conf())?; + } + + self.transaction_params.metadata_params = *metadata_pdu.metadata_params(); let remote_cfg = self.remote_cfg_table.get(metadata_pdu.source_id().value()); if remote_cfg.is_none() { - return Err(DestError::NoRemoteCfgFound(metadata_pdu.dest_id())); + return Err(DestError::NoRemoteConfigFound(metadata_pdu.dest_id())); } - self.tparams.remote_cfg = Some(*remote_cfg.unwrap()); - // TODO: Support for metadata only PDUs. let src_name = metadata_pdu.src_file_name(); let dest_name = metadata_pdu.dest_file_name(); if src_name.is_empty() && dest_name.is_empty() { - self.tparams.tstate.metadata_only = true; + self.transaction_params.metadata_only = true; } - if !self.tparams.tstate.metadata_only && src_name.is_empty() { + if !self.transaction_params.metadata_only && src_name.is_empty() { return Err(DestError::EmptySrcFileField); } - if !self.tparams.tstate.metadata_only && dest_name.is_empty() { + if !self.transaction_params.metadata_only && dest_name.is_empty() { return Err(DestError::EmptyDestFileField); } - if !self.tparams.tstate.metadata_only { - self.tparams.file_properties.src_file_name[..src_name.len_value()] + if !self.transaction_params.metadata_only { + self.transaction_params.file_names.src_file_name[..src_name.len_value()] .copy_from_slice(src_name.value()); - self.tparams.file_properties.src_file_name_len = src_name.len_value(); + self.transaction_params.file_names.src_file_name_len = src_name.len_value(); if dest_name.is_empty() { return Err(DestError::EmptyDestFileField); } - self.tparams.file_properties.dest_file_name[..dest_name.len_value()] + self.transaction_params.file_names.dest_file_name[..dest_name.len_value()] .copy_from_slice(dest_name.value()); - self.tparams.file_properties.dest_file_name_len = dest_name.len_value(); - self.tparams.pdu_conf = *metadata_pdu.pdu_header().common_pdu_conf(); - self.tparams.msgs_to_user_size = 0; + self.transaction_params.file_names.dest_file_name_len = dest_name.len_value(); + self.transaction_params.msgs_to_user_size = 0; } if !metadata_pdu.options().is_empty() { for option_tlv in metadata_pdu.options_iter().unwrap() { if option_tlv.is_standard_tlv() && option_tlv.tlv_type().unwrap() == TlvType::MsgToUser { - self.tparams + self.transaction_params .msgs_to_user_buf .copy_from_slice(option_tlv.raw_data().unwrap()); - self.tparams.msgs_to_user_size += option_tlv.len_full(); + self.transaction_params.msgs_to_user_size += option_tlv.len_full(); } } } - self.state = State::Busy; - self.step = TransactionStep::TransactionStart; + self.set_step(TransactionStep::TransactionStart); Ok(()) } + fn handle_file_data_without_previous_metadata( + &mut self, + fd_pdu: &FileDataPdu, + ) -> Result { + let mut packets_sent = 0; + if fd_pdu.transmission_mode() == TransmissionMode::Unacknowledged { + // In acknowledged mode, we need to wait for the metadata PDU first. + return Err(DestError::FirstPacketNotMetadata); + } + self.first_packet_handling(fd_pdu.pdu_header().common_pdu_conf())?; + self.transaction_params.progress = fd_pdu.offset() + fd_pdu.file_data().len() as u64; + self.transaction_params.acked_params = Some(AcknowledgedModeParams { + last_start_offset: 0, + last_end_offset: 0, + nak_activity_counter: 0, + metadata_missing: true, + deferred_procedure_active: false, + }); + if fd_pdu.file_data().len() as u64 > 0 { + self.lost_segment_tracker + .add_lost_segment((0, self.transaction_params.progress))?; + let ack_params = self.transaction_params.acked_params.as_mut().unwrap(); + ack_params.last_start_offset = self.transaction_params.progress; + ack_params.last_end_offset = self.transaction_params.progress; + } + if self + .transaction_params + .remote_cfg + .as_ref() + .unwrap() + .immediate_nak_mode + { + let mut num_seg_reqs = 1; + if fd_pdu.file_data().len() as u64 > 0 { + num_seg_reqs += 1; + } + let pdu_header = PduHeader::new_no_file_data(self.transaction_params.pdu_conf, 0); + let mut nak_pdu = NakPduCreatorWithReservedSeqReqsBuf::new( + self.pdu_and_cksum_buffer.get_mut(), + pdu_header, + num_seg_reqs, + ) + .map_err(PduError::from)?; + let buf_mut = nak_pdu.segment_request_buffer_mut(); + let mut current_offset = 0; + let increment = if pdu_header.common_pdu_conf().file_flag == LargeFileFlag::Large { + 8 + } else { + 4 + }; + buf_mut[0..current_offset].fill(0); + current_offset += increment; + buf_mut[current_offset..current_offset + increment].fill(0); + current_offset += increment; + if fd_pdu.file_data().len() as u64 > 0 { + buf_mut[0..current_offset].fill(0); + current_offset += increment; + if pdu_header.common_pdu_conf().file_flag == LargeFileFlag::Large { + buf_mut[current_offset..current_offset + increment] + .copy_from_slice(&self.transaction_params.progress.to_be_bytes()); + } else { + buf_mut[current_offset..current_offset + increment] + .copy_from_slice(&(self.transaction_params.progress as u32).to_be_bytes()); + } + nak_pdu + .set_start_and_end_of_scope(0, self.transaction_params.progress) + .map_err(PduError::from)?; + } + let written_size = nak_pdu.finish(); + self.pdu_sender.send_file_directive_pdu( + FileDirectiveType::NakPdu, + &self.pdu_and_cksum_buffer.borrow()[0..written_size], + )?; + packets_sent += 1; + } + Ok(packets_sent) + } + fn handle_file_data( &mut self, user: &mut impl CfdpUser, - raw_packet: &[u8], - ) -> Result<(), DestError> { - if self.state == State::Idle - || (self.step != TransactionStep::ReceivingFileDataPdus - && self.step != TransactionStep::ReceivingFileDataPdusWithCheckLimitHandling) - { - return Err(DestError::WrongStateForFileData); - } - let fd_pdu = FileDataPdu::from_bytes(raw_packet)?; - if self.local_cfg.indication_cfg.file_segment_recv { - user.file_segment_recvd_indication(&FileSegmentRecvdParams { - id: self.tstate().transaction_id.unwrap(), - offset: fd_pdu.offset(), - length: fd_pdu.file_data().len(), - segment_metadata: fd_pdu.segment_metadata(), - }); + fd_pdu: FileDataPdu, + ) -> Result { + let mut packets_sent = 0; + let mut handle_indication = |id: TransactionId, indication_config: &IndicationConfig| { + if indication_config.file_segment_recv { + user.file_segment_recvd_indication(&FileSegmentRecvdParams { + id, + offset: fd_pdu.offset(), + length: fd_pdu.file_data().len(), + segment_metadata: fd_pdu.segment_metadata(), + }); + } + }; + let step = self.step.get(); + if self.state == State::Idle { + if step == TransactionStep::Idle { + packets_sent += self.handle_file_data_without_previous_metadata(&fd_pdu)?; + self.set_step(TransactionStep::WaitingForMetadata); + handle_indication( + self.transaction_id().unwrap(), + &self.local_cfg.indication_cfg, + ); + return Ok(packets_sent); + } + if step != TransactionStep::ReceivingFileDataPdus + && step != TransactionStep::ReceivingFileDataPdusWithCheckLimitHandling + { + return Err(DestError::WrongStepForPdu { + pdu_type: PduType::FileData, + file_directive_type: None, + step, + }); + } } + handle_indication( + self.transaction_id().unwrap(), + &self.local_cfg.indication_cfg, + ); if let Err(e) = self.vfs.write_data( // Safety: It was already verified that the path is valid during the transaction start. unsafe { from_utf8_unchecked( //from_utf8( - &self.tparams.file_properties.dest_path_buf - [0..self.tparams.file_properties.dest_file_path_len], + &self.transaction_params.file_names.dest_path_buf + [0..self.transaction_params.file_names.dest_file_path_len], ) }, fd_pdu.offset(), fd_pdu.file_data(), ) { - self.declare_fault(ConditionCode::FilestoreRejection); + if self.declare_fault(ConditionCode::FilestoreRejection) + == FaultHandlerCode::AbandonTransaction + { + self.abandon_transaction(); + } return Err(e.into()); } - self.tstate_mut().progress += fd_pdu.file_data().len() as u64; - Ok(()) + self.transaction_params.progress += fd_pdu.file_data().len() as u64; + Ok(packets_sent) } fn handle_eof_pdu( &mut self, cfdp_user: &mut impl CfdpUser, - raw_packet: &[u8], - ) -> Result<(), DestError> { - if self.state == State::Idle || self.step != TransactionStep::ReceivingFileDataPdus { - return Err(DestError::WrongStateForEof); - } - let eof_pdu = EofPdu::from_bytes(raw_packet)?; + eof_pdu: EofPdu, + ) -> Result { + let mut sent_packets = 0; if self.local_cfg.indication_cfg.eof_recv { // Unwrap is okay here, application logic ensures that transaction ID is valid here. - cfdp_user.eof_recvd_indication(self.tparams.tstate.transaction_id.as_ref().unwrap()); + cfdp_user + .eof_recvd_indication(self.transaction_params.transaction_id.as_ref().unwrap()); + } + if self.state == State::Idle { + if self.transmission_mode().unwrap() == TransmissionMode::Unacknowledged { + return Err(DestError::WrongStepForPdu { + pdu_type: PduType::FileDirective, + file_directive_type: Some(FileDirectiveType::EofPdu), + step: self.step(), + }); + } + self.handle_eof_without_previous_metadata(&eof_pdu)?; + return Ok(sent_packets); } let regular_transfer_finish = if eof_pdu.condition_code() == ConditionCode::NoError { - self.handle_no_error_eof_pdu(&eof_pdu)? + self.handle_eof_no_error(&eof_pdu)? } else { - // This is an EOF (Cancel), perform Cancel Response Procedures according to chapter - // 4.6.6 of the standard. - self.trigger_notice_of_completion_cancelled( - eof_pdu.condition_code(), - EntityIdTlv::new(self.tparams.remote_cfg.unwrap().entity_id), - ); - self.tparams.tstate.progress = eof_pdu.file_size(); - if eof_pdu.file_size() > 0 { - self.tparams.tstate.delivery_code = DeliveryCode::Incomplete; - } else { - self.tparams.tstate.delivery_code = DeliveryCode::Complete; - } - // TODO: The cancel EOF also supplies a checksum and a progress number. We could cross - // check that checksum, but how would we deal with a checksum failure? The standard - // does not specify anything for this case.. It could be part of the status report - // issued to the user though. + self.handle_eof_cancel(&eof_pdu); true }; if regular_transfer_finish { - self.file_transfer_complete_transition(); + sent_packets += self.file_transfer_complete_transition()?; } + Ok(sent_packets) + } + + fn handle_eof_without_previous_metadata(&mut self, eof_pdu: &EofPdu) -> Result<(), DestError> { + self.transaction_params.progress = eof_pdu.file_size(); + self.transaction_params.file_size = eof_pdu.file_size(); + self.transaction_params.acked_params = Some(AcknowledgedModeParams { + last_start_offset: 0, + last_end_offset: 0, + nak_activity_counter: 0, + metadata_missing: true, + deferred_procedure_active: false, + }); + if self.transaction_params.progress > 0 { + self.lost_segment_tracker.reset(); + // Add the whole file to the lost segments map for now. + self.lost_segment_tracker + .add_lost_segment((0, eof_pdu.file_size()))?; + } + if eof_pdu.condition_code() != ConditionCode::NoError { + self.handle_eof_cancel(eof_pdu); + } + self.acknowledge_eof_pdu(eof_pdu)?; + if self.transaction_params.completion_disposition.get() == CompletionDisposition::Cancelled + { + self.set_step(TransactionStep::TransferCompletion); + return Ok(()); + } + if let Some(ack_params) = &self.transaction_params.acked_params { + if ack_params.metadata_missing || !self.lost_segment_tracker.is_empty() { + self.start_deferred_lost_segment_handling(); + return Ok(()); + } + } + self.set_step(TransactionStep::TransferCompletion); Ok(()) } + fn handle_eof_cancel(&mut self, eof_pdu: &EofPdu) { + // This is an EOF (Cancel), perform Cancel Response Procedures according to chapter + // 4.6.6 of the standard. Set remote ID as fault location. + self.trigger_notice_of_completion_cancelled( + eof_pdu.condition_code(), + EntityIdTlv::new(self.transaction_params.remote_cfg.unwrap().entity_id), + ); + // Store this as progress for the checksum calculation as well. + self.transaction_params.progress = eof_pdu.file_size(); + if let Some(ack_params) = &self.transaction_params.acked_params { + if ack_params.metadata_missing { + return; + } + } + if self.transaction_params.progress == 0 { + // Empty file, no file data PDU. + self.transaction_params + .finished_params + .delivery_code + .set(DeliveryCode::Complete); + return; + } + if self.checksum_verify(self.transaction_params.progress, eof_pdu.file_checksum()) { + self.transaction_params + .finished_params + .delivery_code + .set(DeliveryCode::Complete); + return; + } + self.transaction_params + .finished_params + .delivery_code + .set(DeliveryCode::Incomplete); + } + + fn acknowledge_eof_pdu(&mut self, eof_pdu: &EofPdu) -> Result<(), DestError> { + let pdu_header = PduHeader::new_no_file_data(self.transaction_params.pdu_conf, 0); + let ack_pdu = AckPdu::new_for_eof_pdu( + pdu_header, + eof_pdu.condition_code(), + TransactionStatus::Active, + ); + let written_len = ack_pdu.write_to_bytes(self.pdu_and_cksum_buffer.get_mut())?; + self.pdu_sender.send_file_directive_pdu( + FileDirectiveType::AckPdu, + &self.pdu_and_cksum_buffer.borrow()[0..written_len], + )?; + Ok(()) + } + + fn start_deferred_lost_segment_handling(&mut self) { + match &mut self.transaction_params.acked_params { + Some(params) => { + 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, + metadata_missing: false, + nak_activity_counter: 0, + deferred_procedure_active: true, + }) + } + } + self.transaction_params.deferred_procedure_timer = Some( + self.check_timer_creator + .create_countdown(TimerContext::NakActivity { + expiry_time: self + .transaction_params + .remote_cfg + .as_ref() + .unwrap() + .nak_timer_interval, + }), + ); + } + + fn deferred_lost_segment_handling(&mut self) { + // TODO. + } + fn trigger_notice_of_completion_cancelled( - &mut self, + &self, cond_code: ConditionCode, fault_location: EntityIdTlv, ) { - self.tparams.tstate.completion_disposition = CompletionDisposition::Cancelled; - self.tparams.tstate.condition_code = cond_code; - self.tparams.tstate.fault_location_finished = Some(fault_location); + self.transaction_params + .completion_disposition + .set(CompletionDisposition::Cancelled); + self.transaction_params + .finished_params + .condition_code + .set(cond_code); + self.transaction_params + .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 // the delivery is really complete, and we need the EOF checksum for that.. - if self.tparams.tstate.progress == 0 { - self.tparams.tstate.delivery_code = DeliveryCode::Complete; + if self.transaction_params.progress == 0 { + self.transaction_params + .finished_params + .delivery_code + .set(DeliveryCode::Complete); } } /// Returns whether the transfer can be completed regularly. - fn handle_no_error_eof_pdu(&mut self, eof_pdu: &EofPdu) -> Result { + fn handle_eof_no_error(&mut self, eof_pdu: &EofPdu) -> Result { // CFDP 4.6.1.2.9: Declare file size error if progress exceeds file size - if self.tparams.tstate.progress > eof_pdu.file_size() - && self.declare_fault(ConditionCode::FileSizeError) != FaultHandlerCode::IgnoreError - { - return Ok(false); - } else if (self.tparams.tstate.progress < eof_pdu.file_size()) - && self.tparams.transmission_mode() == TransmissionMode::Acknowledged + if self.transaction_params.progress > eof_pdu.file_size() { + match self.declare_fault(ConditionCode::FileSizeError) { + FaultHandlerCode::IgnoreError => (), + FaultHandlerCode::AbandonTransaction => { + self.abandon_transaction(); + return Ok(false); + } + FaultHandlerCode::NoticeOfCancellation | FaultHandlerCode::NoticeOfSuspension => { + return Ok(false); + } + } + } else if (self.transaction_params.progress < eof_pdu.file_size()) + && self.transaction_params.transmission_mode() == TransmissionMode::Acknowledged { // CFDP 4.6.4.3.1: The end offset of the last received file segment and the file // size as stated in the EOF PDU is not the same, so we need to add that segment to @@ -628,123 +1056,166 @@ impl< // ) } - self.tparams.tstate.checksum = eof_pdu.file_checksum(); - if self.tparams.transmission_mode() == TransmissionMode::Unacknowledged - && !self.checksum_verify(self.tparams.tstate.checksum) + self.transaction_params.checksum = eof_pdu.file_checksum(); + if self.transaction_params.transmission_mode() == TransmissionMode::Unacknowledged + && !self.checksum_verify( + self.transaction_params.progress, + self.transaction_params.checksum, + ) { - if self.declare_fault(ConditionCode::FileChecksumFailure) - != FaultHandlerCode::IgnoreError - { - return Ok(false); - } self.start_check_limit_handling(); return Ok(false); } + self.transaction_params + .finished_params + .delivery_code + .set(DeliveryCode::Complete); + self.transaction_params + .finished_params + .condition_code + .set(ConditionCode::NoError); Ok(true) } - fn file_transfer_complete_transition(&mut self) { - if self.tparams.transmission_mode() == TransmissionMode::Unacknowledged { - self.step = TransactionStep::TransferCompletion; - } else { - // TODO: Prepare ACK PDU somehow. - self.step = TransactionStep::SendingAckPdu; + fn file_transfer_complete_transition(&mut self) -> Result { + let mut sent_packets = 0; + if self.transaction_params.transmission_mode() == TransmissionMode::Acknowledged { + let pdu_header = PduHeader::new_no_file_data(self.transaction_params.pdu_conf, 0); + let ack_pdu = AckPdu::new( + pdu_header, + FileDirectiveType::EofPdu, + self.transaction_params.finished_params.condition_code.get(), + TransactionStatus::Active, + ) + .unwrap(); + let written_len = ack_pdu.write_to_bytes(self.pdu_and_cksum_buffer.get_mut())?; + self.pdu_sender.send_file_directive_pdu( + FileDirectiveType::AckPdu, + &self.pdu_and_cksum_buffer.borrow()[0..written_len], + )?; + sent_packets += 1; } + self.set_step(TransactionStep::TransferCompletion); + Ok(sent_packets) } - fn checksum_verify(&mut self, checksum: u32) -> bool { - let mut file_delivery_complete = false; - if self.tparams.metadata_params().checksum_type == ChecksumType::NullChecksum - || self.tparams.tstate.metadata_only + fn checksum_verify(&mut self, verify_len: u64, checksum: u32) -> bool { + if self.transaction_params.metadata_params().checksum_type == ChecksumType::NullChecksum + || self.transaction_params.metadata_only { - file_delivery_complete = true; - } else { - match self.vfs.checksum_verify( - checksum, - // Safety: It was already verified that the path is valid during the transaction start. - unsafe { - from_utf8_unchecked( - &self.tparams.file_properties.dest_path_buf - [0..self.tparams.file_properties.dest_file_path_len], - ) - }, - self.tparams.metadata_params().checksum_type, - self.tparams.tstate.progress, - &mut self.tparams.cksum_buf, - ) { - Ok(checksum_success) => { - file_delivery_complete = checksum_success; - if !checksum_success { - self.tparams.tstate.delivery_code = DeliveryCode::Incomplete; - self.tparams.tstate.condition_code = ConditionCode::FileChecksumFailure; - } + return true; + } + match self.vfs.checksum_verify( + checksum, + // Safety: It was already verified that the path is valid during the transaction start. + unsafe { + from_utf8_unchecked( + &self.transaction_params.file_names.dest_path_buf + [0..self.transaction_params.file_names.dest_file_path_len], + ) + }, + self.transaction_params.metadata_params().checksum_type, + verify_len, + self.pdu_and_cksum_buffer.get_mut(), + ) { + Ok(false) => { + if self.declare_fault(ConditionCode::FileChecksumFailure) + == FaultHandlerCode::AbandonTransaction + { + self.abandon_transaction(); } - Err(e) => match e { - FilestoreError::ChecksumTypeNotImplemented(_) => { - self.declare_fault(ConditionCode::UnsupportedChecksumType); - // For this case, the applicable algorithm shall be the the null checksum, - // which is always succesful. - file_delivery_complete = true; + false + } + Ok(true) => true, + Err(e) => match e { + FilestoreError::ChecksumTypeNotImplemented(_) => { + if self.declare_fault(ConditionCode::UnsupportedChecksumType) + == FaultHandlerCode::AbandonTransaction + { + self.abandon_transaction(); } - _ => { - self.declare_fault(ConditionCode::FilestoreRejection); - // Treat this equivalent to a failed checksum procedure. + // For this case, the applicable algorithm shall be the the null checksum, + // which is always succesful. + true + } + _ => { + if self.declare_fault(ConditionCode::FilestoreRejection) + == FaultHandlerCode::AbandonTransaction + { + self.abandon_transaction(); } - }, - }; + // Treat this equivalent to a failed checksum procedure. + false + } + }, } - if file_delivery_complete { - self.tparams.tstate.delivery_code = DeliveryCode::Complete; - self.tparams.tstate.condition_code = ConditionCode::NoError; - } - file_delivery_complete } fn start_check_limit_handling(&mut self) { - self.step = TransactionStep::ReceivingFileDataPdusWithCheckLimitHandling; - self.tparams.tstate.current_check_timer = Some(self.check_timer_creator.create_countdown( - TimerContext::CheckLimit { - local_id: self.local_cfg.id, - remote_id: self.tparams.remote_cfg.unwrap().entity_id, - entity_type: EntityType::Receiving, - }, - )); - self.tparams.tstate.current_check_count = 0; + self.set_step(TransactionStep::ReceivingFileDataPdusWithCheckLimitHandling); + self.transaction_params.current_check_timer = Some( + self.check_timer_creator + .create_countdown(TimerContext::CheckLimit { + local_id: self.local_cfg.id, + remote_id: self.transaction_params.remote_cfg.unwrap().entity_id, + entity_type: EntityType::Receiving, + }), + ); + self.transaction_params.current_check_count = 0; } - fn check_limit_handling(&mut self) { - if self.tparams.tstate.current_check_timer.is_none() { - return; + fn check_limit_handling(&mut self) -> Result<(), DestError> { + if self.transaction_params.current_check_timer.is_none() { + return Ok(()); } - let check_timer = self.tparams.tstate.current_check_timer.as_ref().unwrap(); + let check_timer = self + .transaction_params + .current_check_timer + .as_ref() + .unwrap(); if check_timer.has_expired() { - if self.checksum_verify(self.tparams.tstate.checksum) { - self.file_transfer_complete_transition(); - return; + if self.checksum_verify( + self.transaction_params.progress, + self.transaction_params.checksum, + ) { + self.transaction_params + .finished_params + .condition_code + .set(ConditionCode::NoError); + self.transaction_params + .finished_params + .delivery_code + .set(DeliveryCode::Complete); + self.set_step(TransactionStep::TransferCompletion); + return Ok(()); } - if self.tparams.tstate.current_check_count + 1 - >= self.tparams.remote_cfg.unwrap().check_limit + if self.transaction_params.current_check_count + 1 + >= self.transaction_params.remote_cfg.unwrap().check_limit { - self.declare_fault(ConditionCode::CheckLimitReached); + if self.declare_fault(ConditionCode::CheckLimitReached) + == FaultHandlerCode::AbandonTransaction + { + self.abandon_transaction(); + } } else { - self.tparams.tstate.current_check_count += 1; - self.tparams - .tstate + self.transaction_params.current_check_count += 1; + self.transaction_params .current_check_timer .as_mut() .unwrap() .reset(); } } + Ok(()) } - 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) } fn fsm_busy(&mut self, cfdp_user: &mut impl CfdpUser) -> Result { let mut sent_packets = 0; - if self.step == TransactionStep::TransactionStart { + if self.step() == TransactionStep::TransactionStart { let result = self.transaction_start(cfdp_user); if let Err(e) = result { // If we can not even start the transaction properly, reset the handler. @@ -754,54 +1225,40 @@ impl< return Err(e); } } - if self.step == TransactionStep::ReceivingFileDataPdusWithCheckLimitHandling { - self.check_limit_handling(); + if self.step() == TransactionStep::ReceivingFileDataPdusWithCheckLimitHandling { + self.check_limit_handling()?; } - if self.step == TransactionStep::TransferCompletion { + if self.step() == TransactionStep::TransferCompletion { sent_packets += self.transfer_completion(cfdp_user)?; } - if self.step == TransactionStep::SendingAckPdu { - return Err(DestError::NotImplemented); - } - if self.step == TransactionStep::SendingFinishedPdu { - self.reset(); + if self.step() == TransactionStep::WaitingForFinishedAck { + sent_packets += self.handle_positive_ack_procedures()?; } Ok(sent_packets) } - /// Get the step, which denotes the exact step of a pending CFDP transaction when applicable. - pub fn step(&self) -> TransactionStep { - self.step - } - - /// Get the step, which denotes whether the CFDP handler is active, and which CFDP class - /// is used if it is active. - pub fn state(&self) -> State { - self.state - } - fn transaction_start(&mut self, cfdp_user: &mut impl CfdpUser) -> Result<(), DestError> { + let id = self.transaction_id().unwrap(); let dest_name = from_utf8( - &self.tparams.file_properties.dest_file_name - [..self.tparams.file_properties.dest_file_name_len], + &self.transaction_params.file_names.dest_file_name + [..self.transaction_params.file_names.dest_file_name_len], )?; - self.tparams.file_properties.dest_path_buf[0..dest_name.len()] + self.transaction_params.file_names.dest_path_buf[0..dest_name.len()] .copy_from_slice(dest_name.as_bytes()); - self.tparams.file_properties.dest_file_path_len = dest_name.len(); - let source_id = self.tparams.pdu_conf.source_id(); - let id = TransactionId::new(source_id, self.tparams.pdu_conf.transaction_seq_num); + self.transaction_params.file_names.dest_file_path_len = dest_name.len(); + let source_id = self.transaction_params.pdu_conf.source_id(); let src_name = from_utf8( - &self.tparams.file_properties.src_file_name - [0..self.tparams.file_properties.src_file_name_len], + &self.transaction_params.file_names.src_file_name + [0..self.transaction_params.file_names.src_file_name_len], )?; let mut msgs_to_user = SmallVec::<[MsgToUserTlv<'_>; 16]>::new(); let mut num_msgs_to_user = 0; - if self.tparams.msgs_to_user_size > 0 { + if self.transaction_params.msgs_to_user_size > 0 { let mut index = 0; - while index < self.tparams.msgs_to_user_size { + while index < self.transaction_params.msgs_to_user_size { // This should never panic as the validity of the options was checked beforehand. let msgs_to_user_tlv = - MsgToUserTlv::from_bytes(&self.tparams.msgs_to_user_buf[index..]) + MsgToUserTlv::from_bytes(&self.transaction_params.msgs_to_user_buf[index..]) .expect("message to user creation failed unexpectedly"); msgs_to_user.push(msgs_to_user_tlv); index += msgs_to_user_tlv.len_full(); @@ -811,12 +1268,11 @@ impl< let metadata_recvd_params = MetadataReceivedParams { id, source_id, - file_size: self.tparams.file_size(), + file_size: self.transaction_params.file_size(), src_file_name: src_name, dest_file_name: dest_name, msgs_to_user: &msgs_to_user[..num_msgs_to_user], }; - self.tparams.tstate.transaction_id = Some(id); cfdp_user.metadata_recvd_indication(&metadata_recvd_params); if self.vfs.exists(dest_name)? && self.vfs.is_dir(dest_name)? { @@ -830,76 +1286,138 @@ impl< return Err(DestError::PathConcat); } let source_name = source_file_name.unwrap(); - self.tparams.file_properties.dest_path_buf[dest_name.len()] = b'/'; - self.tparams.file_properties.dest_path_buf + self.transaction_params.file_names.dest_path_buf[dest_name.len()] = b'/'; + self.transaction_params.file_names.dest_path_buf [dest_name.len() + 1..dest_name.len() + 1 + source_name.len()] .copy_from_slice(source_name.as_bytes()); - self.tparams.file_properties.dest_file_path_len += 1 + source_name.len(); + self.transaction_params.file_names.dest_file_path_len += 1 + source_name.len(); } let dest_path_str = from_utf8( - &self.tparams.file_properties.dest_path_buf - [0..self.tparams.file_properties.dest_file_path_len], + &self.transaction_params.file_names.dest_path_buf + [0..self.transaction_params.file_names.dest_file_path_len], )?; if self.vfs.exists(dest_path_str)? { self.vfs.truncate_file(dest_path_str)?; } else { self.vfs.create_file(dest_path_str)?; } - self.tparams.tstate.file_status = FileStatus::Retained; - self.step = TransactionStep::ReceivingFileDataPdus; + self.transaction_params.finished_params.file_status = FileStatus::Retained; + drop(msgs_to_user); + self.set_step(TransactionStep::ReceivingFileDataPdus); Ok(()) } fn transfer_completion(&mut self, cfdp_user: &mut impl CfdpUser) -> Result { let mut sent_packets = 0; self.notice_of_completion(cfdp_user)?; - if self.tparams.transmission_mode() == TransmissionMode::Acknowledged - || self.tparams.metadata_params().closure_requested + if self.transaction_params.transmission_mode() == TransmissionMode::Acknowledged + || self.transaction_params.metadata_params().closure_requested { sent_packets += self.send_finished_pdu()?; + self.start_positive_ack_procedure(); + if self.transaction_params.transmission_mode() == TransmissionMode::Acknowledged { + self.set_step(TransactionStep::WaitingForFinishedAck); + return Ok(sent_packets); + } } - self.step = TransactionStep::SendingFinishedPdu; + self.reset(); Ok(sent_packets) } + fn start_positive_ack_procedure(&mut self) { + self.transaction_params.positive_ack_params = Some(PositiveAckParams { + ack_timer: self + .check_timer_creator + .create_countdown(TimerContext::PositiveAck { + expiry_time: self + .transaction_params + .remote_cfg + .as_ref() + .unwrap() + .positive_ack_timer_interval, + }), + ack_counter: 0, + }) + } + + fn handle_positive_ack_procedures(&mut self) -> Result { + // 1) Do we have positive-ack params? + let params = match self.transaction_params.positive_ack_params.as_mut() { + Some(p) => p, + None => return Ok(0), + }; + + // 2) Has the timer expired? + if !params.ack_timer.has_expired() { + return Ok(0); + } + + // 3) Load the remote limit (avoid a panicking unwrap later) + let expiration_limit = self + .transaction_params + .remote_cfg + .as_ref() + .unwrap() + .positive_ack_timer_expiration_limit; + + // 4) If bumping the counter would exceed the limit, fault & maybe abandon + if params.ack_counter + 1 >= expiration_limit { + if self.declare_fault(ConditionCode::PositiveAckLimitReached) + == FaultHandlerCode::AbandonTransaction + { + self.abandon_transaction(); + } + return Ok(0); + } + + // 5) Otherwise reset the timer, increment, and send the PDU + params.ack_timer.reset(); + params.ack_counter += 1; + self.send_finished_pdu() + } + fn notice_of_completion(&mut self, cfdp_user: &mut impl CfdpUser) -> Result<(), DestError> { - if self.tstate().completion_disposition == CompletionDisposition::Completed { + if self.transaction_params.completion_disposition.get() == CompletionDisposition::Completed + { // TODO: Execute any filestore requests } else if self - .tparams + .transaction_params .remote_cfg .as_ref() .unwrap() .disposition_on_cancellation - && self.tstate().delivery_code == DeliveryCode::Incomplete + && self.transaction_params.finished_params.delivery_code.get() + == DeliveryCode::Incomplete { // Safety: We already verified that the path is valid during the transaction start. let dest_path = unsafe { from_utf8_unchecked( - &self.tparams.file_properties.dest_path_buf - [0..self.tparams.file_properties.dest_file_path_len], + &self.transaction_params.file_names.dest_path_buf + [0..self.transaction_params.file_names.dest_file_path_len], ) }; 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.transaction_params.finished_params.file_status = FileStatus::DiscardDeliberately; } - let tstate = self.tstate(); + let tstate = &self.transaction_params; let transaction_finished_params = TransactionFinishedParams { id: tstate.transaction_id.unwrap(), - condition_code: tstate.condition_code, - delivery_code: tstate.delivery_code, - 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(()) } - fn declare_fault(&mut self, condition_code: ConditionCode) -> FaultHandlerCode { + // When the fault handler code [FaultHandlerCode::AbandonTransaction] is returned, the caller + // must call [Self::abandon_transaction] as soon as it is possible. + fn declare_fault(&self, condition_code: ConditionCode) -> FaultHandlerCode { // Cache those, because they might be reset when abandoning the transaction. - let transaction_id = self.tstate().transaction_id.unwrap(); - let progress = self.tstate().progress; + let transaction_id = self.transaction_id().unwrap(); + let progress = self.transaction_params.progress; let fh_code = self .local_cfg .fault_handler @@ -910,25 +1428,27 @@ impl< } FaultHandlerCode::NoticeOfSuspension => self.notice_of_suspension(), FaultHandlerCode::IgnoreError => (), - FaultHandlerCode::AbandonTransaction => self.abandon_transaction(), + FaultHandlerCode::AbandonTransaction => (), } self.local_cfg .fault_handler .report_fault(transaction_id, condition_code, progress) } - fn notice_of_cancellation( - &mut self, - condition_code: ConditionCode, - fault_location: EntityIdTlv, - ) { - self.step = TransactionStep::TransferCompletion; - self.tstate_mut().condition_code = condition_code; - self.tstate_mut().fault_location_finished = Some(fault_location); - self.tstate_mut().completion_disposition = CompletionDisposition::Cancelled; + fn notice_of_cancellation(&self, condition_code: ConditionCode, fault_location: EntityIdTlv) { + self.set_step_internal(TransactionStep::TransferCompletion); + let tstate = &self.transaction_params; + tstate.finished_params.condition_code.set(condition_code); + tstate + .completion_disposition + .set(CompletionDisposition::Cancelled); + tstate + .finished_params + .fault_location_finished + .set(Some(fault_location)); } - fn notice_of_suspension(&mut self) { + fn notice_of_suspension(&self) { // TODO: Implement suspension handling. } @@ -936,55 +1456,45 @@ impl< self.reset(); } - /// 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.step = TransactionStep::Idle; - self.state = State::Idle; - // self.packets_to_send_ctx.packet_available = false; - self.tparams.reset(); + #[inline] + fn set_step_internal(&self, step: TransactionStep) { + self.step.set(step); + } + + #[inline] + fn set_step(&mut self, step: TransactionStep) { + self.set_step_internal(step); } fn send_finished_pdu(&mut self) -> Result { - let tstate = self.tstate(); + let tstate = &self.transaction_params; - let pdu_header = PduHeader::new_no_file_data(self.tparams.pdu_conf, 0); - let finished_pdu = if tstate.condition_code == ConditionCode::NoError - || tstate.condition_code == ConditionCode::UnsupportedChecksumType + let pdu_header = PduHeader::new_no_file_data(self.transaction_params.pdu_conf, 0); + 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, tstate.file_status) + FinishedPduCreator::new_default( + pdu_header, + tstate.finished_params.delivery_code.get(), + tstate.finished_params.file_status, + ) } else { FinishedPduCreator::new_generic( pdu_header, - tstate.condition_code, - tstate.delivery_code, - tstate.file_status, + tstate.finished_params.condition_code.get(), + tstate.finished_params.delivery_code.get(), + tstate.finished_params.file_status, &[], - tstate.fault_location_finished, + tstate.finished_params.fault_location_finished.get(), ) }; - finished_pdu.write_to_bytes(&mut self.packet_buf)?; - self.pdu_sender.send_pdu( - finished_pdu.pdu_type(), - finished_pdu.file_directive_type(), - &self.packet_buf[0..finished_pdu.len_written()], + finished_pdu.write_to_bytes(self.pdu_and_cksum_buffer.get_mut())?; + self.pdu_sender.send_file_directive_pdu( + FileDirectiveType::FinishedPdu, + &self.pdu_and_cksum_buffer.borrow()[0..finished_pdu.len_written()], )?; Ok(1) } - - pub fn local_cfg(&self) -> &LocalEntityConfig { - &self.local_cfg - } - - fn tstate(&self) -> &TransferState { - &self.tparams.tstate - } - - fn tstate_mut(&mut self) -> &mut TransferState { - &mut self.tparams.tstate - } } #[cfg(test)] @@ -1009,8 +1519,9 @@ mod tests { }; use crate::{ - CRC_32, FaultHandler, IndicationConfig, PduRawWithInfo, StdRemoteEntityConfigProvider, + CRC_32, FaultHandler, IndicationConfig, PduRawWithInfo, RemoteConfigStoreStd, filestore::NativeFilestore, + lost_segments::LostSegmentsList, tests::{ LOCAL_ID, REMOTE_ID, SentPdu, TestCfdpSender, TestCfdpUser, TestCheckTimer, TestCheckTimerCreator, TestFaultHandler, TimerExpiryControl, basic_remote_cfg_table, @@ -1023,9 +1534,10 @@ mod tests { TestCfdpSender, TestFaultHandler, NativeFilestore, - StdRemoteEntityConfigProvider, + RemoteConfigStoreStd, TestCheckTimerCreator, TestCheckTimer, + LostSegmentsList, >; struct DestHandlerTestbench { @@ -1171,11 +1683,9 @@ mod tests { let result = self.handler.state_machine(user, Some(&packet_info)); if self.indication_cfg().file_segment_recv { assert!(!user.file_seg_recvd_queue.is_empty()); - assert_eq!(user.file_seg_recvd_queue.back().unwrap().offset, offset); - assert_eq!( - user.file_seg_recvd_queue.back().unwrap().length, - file_data_chunk.len() - ); + let file_seg = user.file_seg_recvd_queue.pop_front().unwrap(); + assert_eq!(file_seg.offset, offset); + assert_eq!(file_seg.length, file_data_chunk.len()); } result } @@ -1219,7 +1729,17 @@ mod tests { // Specifying some checks in the drop method avoids some boilerplate. impl Drop for DestHandlerTestbench { fn drop(&mut self) { - assert!(self.all_fault_queues_empty()); + if !self.all_fault_queues_empty() { + let fh_queues = self.handler.local_cfg.user_fault_hook().borrow(); + println!( + "fault queues not empyt. cancellation {}, suspension {}, ignored {}, abandon {}", + fh_queues.notice_of_cancellation_queue.len(), + fh_queues.notice_of_suspension_queue.len(), + fh_queues.ignored_queue.len(), + fh_queues.abandoned_queue.len() + ); + } + assert!(self.all_fault_queues_empty(), "fault queues not empty, "); assert!(self.handler.pdu_sender.queue_empty()); if self.check_handler_idle_at_drop { self.state_check(State::Idle, TransactionStep::Idle); @@ -1261,6 +1781,7 @@ mod tests { NativeFilestore::default(), basic_remote_cfg_table(LOCAL_ID, 1024, true), TestCheckTimerCreator::new(expiry_control), + LostSegmentsList::default(), ) } @@ -1357,6 +1878,7 @@ mod tests { tb.generic_eof_no_error(&mut test_user, Vec::new()) .expect("EOF no error insertion failed"); tb.check_completion_indication_success(&mut test_user); + assert!(test_user.indication_queues_empty()); } #[test] @@ -1376,6 +1898,7 @@ mod tests { tb.generic_eof_no_error(&mut test_user, file_data.to_vec()) .expect("EOF no error insertion failed"); tb.check_completion_indication_success(&mut test_user); + assert!(test_user.indication_queues_empty()); } #[test] @@ -1403,6 +1926,7 @@ mod tests { tb.generic_eof_no_error(&mut test_user, random_data.to_vec()) .expect("EOF no error insertion failed"); tb.check_completion_indication_success(&mut test_user); + assert!(test_user.indication_queues_empty()); } #[test] @@ -1425,30 +1949,34 @@ mod tests { .expect("file data insertion 0 failed"); tb.generic_eof_no_error(&mut test_user, random_data.to_vec()) .expect("EOF no error insertion failed"); + + // Checksum failure. + 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, transaction_id); + assert_eq!(cancelled.1, ConditionCode::FileChecksumFailure); + assert_eq!(cancelled.2, segment_len as u64); + + drop(fault_handler); tb.state_check( State::Busy, TransactionStep::ReceivingFileDataPdusWithCheckLimitHandling, ); + tb.set_check_timer_expired(); + tb.generic_file_data_insert( &mut test_user, segment_len as u64, &random_data[segment_len..], ) .expect("file data insertion 1 failed"); - tb.set_check_timer_expired(); tb.handler .state_machine_no_packet(&mut test_user) .expect("fsm failure"); - 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, transaction_id); - assert_eq!(cancelled.1, ConditionCode::FileChecksumFailure); - assert_eq!(cancelled.2, segment_len as u64); - drop(fault_handler); tb.check_completion_indication_success(&mut test_user); + assert!(test_user.indication_queues_empty()); } #[test] @@ -1458,6 +1986,17 @@ mod tests { rng.fill(&mut random_data); let file_size = random_data.len() as u64; let segment_len = 256; + let check_checksum_failure = + |testbench: &mut DestHandlerTestbench, transaction_id: TransactionId| { + let mut fault_hook = testbench.handler.local_cfg.user_fault_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); + }; let fault_handler = TestFaultHandler::default(); let mut testbench = DestHandlerTestbench::new_with_fixed_paths(fault_handler, false); @@ -1473,10 +2012,13 @@ mod tests { testbench .generic_eof_no_error(&mut test_user, random_data.to_vec()) .expect("EOF no error insertion failed"); + check_checksum_failure(&mut testbench, transaction_id); + testbench.state_check( State::Busy, TransactionStep::ReceivingFileDataPdusWithCheckLimitHandling, ); + testbench.set_check_timer_expired(); testbench .handler @@ -1486,30 +2028,36 @@ mod tests { State::Busy, TransactionStep::ReceivingFileDataPdusWithCheckLimitHandling, ); + check_checksum_failure(&mut testbench, transaction_id); + testbench.set_check_timer_expired(); testbench .handler .state_machine_no_packet(&mut test_user) .expect("fsm error"); + check_checksum_failure(&mut testbench, transaction_id); + testbench.state_check(State::Idle, TransactionStep::Idle); - let mut fault_hook = testbench.handler.local_cfg.user_fault_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 cancelled = ignored_queue.pop_front().unwrap(); - assert_eq!(cancelled.0, transaction_id); - assert_eq!(cancelled.1, ConditionCode::FileChecksumFailure); - assert_eq!(cancelled.2, segment_len as u64); + { + let mut fault_hook = testbench.handler.local_cfg.user_fault_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, transaction_id); + assert_eq!(cancelled.1, ConditionCode::CheckLimitReached); + assert_eq!(cancelled.2, segment_len as u64); + } - 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, segment_len as u64); - - drop(fault_hook); + assert_eq!(test_user.finished_indic_queue.len(), 1); + let finished_indication = test_user.finished_indic_queue.pop_front().unwrap(); + assert_eq!(finished_indication.id, transaction_id); + assert_eq!( + finished_indication.condition_code, + ConditionCode::CheckLimitReached + ); + assert_eq!(finished_indication.delivery_code, DeliveryCode::Incomplete); + assert_eq!(finished_indication.file_status, FileStatus::Retained); assert!(testbench.handler.pdu_sender.queue_empty()); @@ -1520,6 +2068,7 @@ mod tests { assert_eq!(read_content.len(), segment_len); assert_eq!(read_content, &random_data[0..segment_len]); assert!(fs::remove_file(testbench.dest_path().as_path()).is_ok()); + assert!(test_user.indication_queues_empty()); } fn check_finished_pdu_success(sent_pdu: &SentPdu) { @@ -1555,6 +2104,7 @@ mod tests { let sent_pdu = tb.handler.pdu_sender.retrieve_next_pdu().unwrap(); check_finished_pdu_success(&sent_pdu); tb.check_completion_indication_success(&mut test_user); + assert!(test_user.indication_queues_empty()); } #[test] @@ -1687,6 +2237,19 @@ mod tests { finished_pdu.condition_code(), ConditionCode::CheckLimitReached ); + + let mut fault_hook = tb.handler.local_cfg.user_fault_hook().borrow_mut(); + 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); + 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!(finished_pdu.delivery_code(), DeliveryCode::Incomplete); assert!(finished_pdu.fault_location().is_some()); assert_eq!( @@ -1695,6 +2258,11 @@ mod tests { ); assert_eq!(finished_pdu.fs_responses_raw(), &[]); assert!(tb.handler.pdu_sender.queue_empty()); + user.verify_finished_indication_retained( + DeliveryCode::Incomplete, + ConditionCode::CheckLimitReached, + transaction_id, + ); tb.expected_full_data = faulty_file_data.to_vec(); } @@ -1713,21 +2281,22 @@ mod tests { ); dest_path_buf.push(src_path.file_name().unwrap()); tb.dest_path = dest_path_buf; - let mut test_user = tb.test_user_from_cached_paths(0); - tb.generic_transfer_init(&mut test_user, 0) + let mut user = tb.test_user_from_cached_paths(0); + tb.generic_transfer_init(&mut user, 0) .expect("transfer init failed"); tb.state_check(State::Busy, TransactionStep::ReceivingFileDataPdus); - tb.generic_eof_no_error(&mut test_user, Vec::new()) + tb.generic_eof_no_error(&mut user, Vec::new()) .expect("EOF no error insertion failed"); - tb.check_completion_indication_success(&mut test_user); + tb.check_completion_indication_success(&mut user); } #[test] fn test_tranfer_cancellation_empty_file_with_eof_pdu() { let fault_handler = TestFaultHandler::default(); let mut tb = DestHandlerTestbench::new_with_fixed_paths(fault_handler, false); - let mut test_user = tb.test_user_from_cached_paths(0); - tb.generic_transfer_init(&mut test_user, 0) + let mut user = tb.test_user_from_cached_paths(0); + let id = tb + .generic_transfer_init(&mut user, 0) .expect("transfer init failed"); tb.state_check(State::Busy, TransactionStep::ReceivingFileDataPdus); let cancel_eof = EofPdu::new( @@ -1740,28 +2309,37 @@ mod tests { let packets = tb .handler .state_machine( - &mut test_user, + &mut user, Some(&PduRawWithInfo::new(&cancel_eof.to_vec().unwrap()).unwrap()), ) .expect("state machine call with EOF insertion failed"); assert_eq!(packets, 0); + user.verify_finished_indication_retained( + DeliveryCode::Complete, + ConditionCode::CancelRequestReceived, + id, + ); } - fn generic_tranfer_cancellation_partial_file_with_eof_pdu(with_closure: bool) { + fn generic_tranfer_cancellation_partial_file_with_eof_pdu( + with_closure: bool, + insert_packet: bool, + ) { let file_data_str = "Hello World!"; let file_data = file_data_str.as_bytes(); let file_size = 5; let fault_handler = TestFaultHandler::default(); let mut tb = DestHandlerTestbench::new_with_fixed_paths(fault_handler, with_closure); - let mut test_user = tb.test_user_from_cached_paths(file_size); - tb.generic_transfer_init(&mut test_user, file_size) + let mut user = tb.test_user_from_cached_paths(file_size); + let id = tb + .generic_transfer_init(&mut user, file_size) .expect("transfer init failed"); tb.state_check(State::Busy, TransactionStep::ReceivingFileDataPdus); - tb.generic_file_data_insert(&mut test_user, 0, &file_data[0..5]) - .expect("file data insertion failed"); - // Checksum is currently ignored on remote side.. we still supply it, according to the - // standard. + if insert_packet { + tb.generic_file_data_insert(&mut user, 0, &file_data[0..5]) + .expect("file data insertion failed"); + } let mut digest = CRC_32.digest(); digest.update(&file_data[0..5]); let checksum = digest.finalize(); @@ -1775,7 +2353,7 @@ mod tests { let packets = tb .handler .state_machine( - &mut test_user, + &mut user, Some(&PduRawWithInfo::new(&cancel_eof.to_vec().unwrap()).unwrap()), ) .expect("state machine call with EOF insertion failed"); @@ -1792,7 +2370,30 @@ mod tests { finished_pdu.condition_code(), ConditionCode::CancelRequestReceived ); - assert_eq!(finished_pdu.delivery_code(), DeliveryCode::Incomplete); + if insert_packet { + // Checksum success, so data is complete. + assert_eq!(finished_pdu.delivery_code(), DeliveryCode::Complete); + user.verify_finished_indication_retained( + DeliveryCode::Complete, + ConditionCode::CancelRequestReceived, + id, + ); + } else { + // Checksum failure, so data is incomplete. + assert_eq!(finished_pdu.delivery_code(), DeliveryCode::Incomplete); + tb.check_dest_file = false; + 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, id); + assert_eq!(ignored.1, ConditionCode::FileChecksumFailure); + assert_eq!(ignored.2, 5); + user.verify_finished_indication_retained( + DeliveryCode::Incomplete, + ConditionCode::CancelRequestReceived, + id, + ); + } assert_eq!(finished_pdu.file_status(), FileStatus::Retained); assert_eq!( finished_pdu @@ -1801,6 +2402,11 @@ mod tests { EntityIdTlv::new(LOCAL_ID.into()) ); } else { + user.verify_finished_indication_retained( + DeliveryCode::Complete, + ConditionCode::CancelRequestReceived, + id, + ); assert_eq!(packets, 0); } tb.expected_file_size = file_size; @@ -1808,28 +2414,39 @@ mod tests { } #[test] - fn test_tranfer_cancellation_partial_file_with_eof_pdu_no_closure() { - generic_tranfer_cancellation_partial_file_with_eof_pdu(false); + fn test_tranfer_cancellation_partial_file_with_eof_pdu_no_closure_complete() { + generic_tranfer_cancellation_partial_file_with_eof_pdu(false, true); } + #[test] - fn test_tranfer_cancellation_partial_file_with_eof_pdu_with_closure() { - generic_tranfer_cancellation_partial_file_with_eof_pdu(true); + fn test_tranfer_cancellation_partial_file_with_eof_pdu_with_closure_complete() { + generic_tranfer_cancellation_partial_file_with_eof_pdu(true, true); + } + + #[test] + fn test_tranfer_cancellation_partial_file_with_eof_pdu_with_closure_incomplete() { + generic_tranfer_cancellation_partial_file_with_eof_pdu(true, false); } #[test] fn test_tranfer_cancellation_empty_file_with_cancel_api() { let fault_handler = TestFaultHandler::default(); let mut tb = DestHandlerTestbench::new_with_fixed_paths(fault_handler, false); - let mut test_user = tb.test_user_from_cached_paths(0); - let transaction_id = tb - .generic_transfer_init(&mut test_user, 0) + let mut user = tb.test_user_from_cached_paths(0); + let id = tb + .generic_transfer_init(&mut user, 0) .expect("transfer init failed"); tb.state_check(State::Busy, TransactionStep::ReceivingFileDataPdus); - tb.handler.cancel_request(&transaction_id); + tb.handler.cancel_request(&id); let packets = tb .handler - .state_machine_no_packet(&mut test_user) + .state_machine_no_packet(&mut user) .expect("state machine call with EOF insertion failed"); + user.verify_finished_indication_retained( + DeliveryCode::Complete, + ConditionCode::CancelRequestReceived, + id, + ); assert_eq!(packets, 0); } @@ -1837,15 +2454,15 @@ mod tests { fn test_tranfer_cancellation_empty_file_with_cancel_api_and_closure() { let fault_handler = TestFaultHandler::default(); let mut tb = DestHandlerTestbench::new_with_fixed_paths(fault_handler, true); - let mut test_user = tb.test_user_from_cached_paths(0); - let transaction_id = tb - .generic_transfer_init(&mut test_user, 0) + let mut user = tb.test_user_from_cached_paths(0); + let id = tb + .generic_transfer_init(&mut user, 0) .expect("transfer init failed"); tb.state_check(State::Busy, TransactionStep::ReceivingFileDataPdus); - tb.handler.cancel_request(&transaction_id); + tb.handler.cancel_request(&id); let packets = tb .handler - .state_machine_no_packet(&mut test_user) + .state_machine_no_packet(&mut user) .expect("state machine call with EOF insertion failed"); assert_eq!(packets, 1); let next_pdu = tb.get_next_pdu().unwrap(); @@ -1867,6 +2484,11 @@ mod tests { finished_pdu.fault_location(), Some(EntityIdTlv::new(REMOTE_ID.into())) ); + user.verify_finished_indication_retained( + DeliveryCode::Complete, + ConditionCode::CancelRequestReceived, + id, + ); } #[test] @@ -1877,18 +2499,18 @@ mod tests { let fault_handler = TestFaultHandler::default(); let mut tb = DestHandlerTestbench::new_with_fixed_paths(fault_handler, true); - let mut test_user = tb.test_user_from_cached_paths(file_size); - let transaction_id = tb - .generic_transfer_init(&mut test_user, file_size) + let mut user = tb.test_user_from_cached_paths(file_size); + let id = tb + .generic_transfer_init(&mut user, file_size) .expect("transfer init failed"); tb.state_check(State::Busy, TransactionStep::ReceivingFileDataPdus); - tb.generic_file_data_insert(&mut test_user, 0, &file_data[0..5]) + tb.generic_file_data_insert(&mut user, 0, &file_data[0..5]) .expect("file data insertion failed"); - tb.handler.cancel_request(&transaction_id); + tb.handler.cancel_request(&id); let packets = tb .handler - .state_machine_no_packet(&mut test_user) + .state_machine_no_packet(&mut user) .expect("state machine call with EOF insertion failed"); assert_eq!(packets, 1); let next_pdu = tb.get_next_pdu().unwrap(); @@ -1911,6 +2533,11 @@ mod tests { ); tb.expected_file_size = file_size; tb.expected_full_data = file_data[0..file_size as usize].to_vec(); + user.verify_finished_indication_retained( + DeliveryCode::Incomplete, + ConditionCode::CancelRequestReceived, + id, + ); } // Only incomplete received files will be removed. @@ -1924,18 +2551,23 @@ mod tests { .get_mut(LOCAL_ID.value()) .unwrap(); remote_cfg_mut.disposition_on_cancellation = true; - let mut test_user = tb.test_user_from_cached_paths(0); - let transaction_id = tb - .generic_transfer_init(&mut test_user, 0) + let mut user = tb.test_user_from_cached_paths(0); + let id = tb + .generic_transfer_init(&mut user, 0) .expect("transfer init failed"); tb.state_check(State::Busy, TransactionStep::ReceivingFileDataPdus); - tb.handler.cancel_request(&transaction_id); + tb.handler.cancel_request(&id); let packets = tb .handler - .state_machine_no_packet(&mut test_user) + .state_machine_no_packet(&mut user) .expect("state machine call with EOF insertion failed"); assert_eq!(packets, 0); + user.verify_finished_indication_retained( + DeliveryCode::Complete, + ConditionCode::CancelRequestReceived, + id, + ); } #[test] @@ -1953,21 +2585,28 @@ mod tests { .get_mut(LOCAL_ID.value()) .unwrap(); remote_cfg_mut.disposition_on_cancellation = true; - let mut test_user = tb.test_user_from_cached_paths(file_size); + let mut user = tb.test_user_from_cached_paths(file_size); let transaction_id = tb - .generic_transfer_init(&mut test_user, file_size) + .generic_transfer_init(&mut user, file_size) .expect("transfer init failed"); tb.state_check(State::Busy, TransactionStep::ReceivingFileDataPdus); - tb.generic_file_data_insert(&mut test_user, 0, &file_data[0..5]) + tb.generic_file_data_insert(&mut user, 0, &file_data[0..5]) .expect("file data insertion failed"); tb.handler.cancel_request(&transaction_id); let packets = tb .handler - .state_machine_no_packet(&mut test_user) + .state_machine_no_packet(&mut user) .expect("state machine call with EOF insertion failed"); assert_eq!(packets, 0); // File was disposed. assert!(!Path::exists(tb.dest_path())); + assert_eq!(user.finished_indic_queue.len(), 1); + user.verify_finished_indication( + DeliveryCode::Incomplete, + ConditionCode::CancelRequestReceived, + transaction_id, + FileStatus::DiscardDeliberately, + ); } } diff --git a/src/lib.rs b/src/lib.rs index adef6df..2f8cb66 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -93,13 +93,14 @@ extern crate std; #[cfg(feature = "alloc")] pub mod dest; pub mod filestore; +pub mod lost_segments; pub mod request; #[cfg(feature = "alloc")] pub mod source; pub mod time; pub mod user; -use crate::time::CountdownProvider; +use crate::time::Countdown; use core::{cell::RefCell, fmt::Debug, hash::Hash}; use crc::{CRC_32_ISCSI, CRC_32_ISO_HDLC, Crc}; @@ -180,8 +181,8 @@ pub enum TimerContext { /// The timer will be used to perform the Positive Acknowledgement Procedures as specified in /// 4.7. 1of the CFDP standard. The expiration period will be provided by the Positive ACK timer /// interval of the remote entity configuration. -pub trait TimerCreatorProvider { - type Countdown: CountdownProvider; +pub trait TimerCreator { + type Countdown: Countdown; fn create_countdown(&self, timer_context: TimerContext) -> Self::Countdown; } @@ -256,12 +257,12 @@ pub struct RemoteEntityConfig { pub crc_on_transmission_by_default: bool, pub default_transmission_mode: TransmissionMode, pub default_crc_type: ChecksumType, - pub positive_ack_timer_interval_seconds: f32, + pub positive_ack_timer_interval: Duration, pub positive_ack_timer_expiration_limit: u32, pub check_limit: u32, pub disposition_on_cancellation: bool, pub immediate_nak_mode: bool, - pub nak_timer_interval_seconds: f32, + pub nak_timer_interval: Duration, pub nak_timer_expiration_limit: u32, } @@ -283,26 +284,33 @@ impl RemoteEntityConfig { default_transmission_mode, default_crc_type, check_limit: 2, - positive_ack_timer_interval_seconds: 10.0, + positive_ack_timer_interval: Duration::from_secs(10), positive_ack_timer_expiration_limit: 2, disposition_on_cancellation: false, immediate_nak_mode: true, - nak_timer_interval_seconds: 10.0, + nak_timer_interval: Duration::from_secs(10), nak_timer_expiration_limit: 2, } } } -pub trait RemoteEntityConfigProvider { +#[derive(Debug, PartialEq, Eq, thiserror::Error)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] +pub enum RemoteConfigStoreError { + #[error("store is full")] + Full, +} + +pub trait RemoteConfigStore { /// Retrieve the remote entity configuration for the given remote ID. fn get(&self, remote_id: u64) -> Option<&RemoteEntityConfig>; + fn get_mut(&mut self, remote_id: u64) -> Option<&mut RemoteEntityConfig>; + /// Add a new remote configuration. Return [true] if the configuration was /// inserted successfully, and [false] if a configuration already exists. - fn add_config(&mut self, cfg: &RemoteEntityConfig) -> bool; - /// Remote a configuration. Returns [true] if the configuration was removed successfully, - /// and [false] if no configuration exists for the given remote ID. - fn remove_config(&mut self, remote_id: u64) -> bool; + fn add_config(&mut self, cfg: &RemoteEntityConfig) -> Result; } /// This is a thin wrapper around a [hashbrown::HashMap] to store remote entity configurations. @@ -310,20 +318,26 @@ pub trait RemoteEntityConfigProvider { #[cfg(feature = "alloc")] #[derive(Default, Debug)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] -pub struct StdRemoteEntityConfigProvider(pub hashbrown::HashMap); +pub struct RemoteConfigStoreStd(pub hashbrown::HashMap); #[cfg(feature = "std")] -impl RemoteEntityConfigProvider for StdRemoteEntityConfigProvider { +impl RemoteConfigStore for RemoteConfigStoreStd { fn get(&self, remote_id: u64) -> Option<&RemoteEntityConfig> { self.0.get(&remote_id) } + fn get_mut(&mut self, remote_id: u64) -> Option<&mut RemoteEntityConfig> { self.0.get_mut(&remote_id) } - fn add_config(&mut self, cfg: &RemoteEntityConfig) -> bool { - self.0.insert(cfg.entity_id.value(), *cfg).is_some() + + fn add_config(&mut self, cfg: &RemoteEntityConfig) -> Result { + Ok(self.0.insert(cfg.entity_id.value(), *cfg).is_some()) } - fn remove_config(&mut self, remote_id: u64) -> bool { +} + +#[cfg(feature = "std")] +impl RemoteConfigStoreStd { + pub fn remove_config(&mut self, remote_id: u64) -> bool { self.0.remove(&remote_id).is_some() } } @@ -334,10 +348,10 @@ impl RemoteEntityConfigProvider for StdRemoteEntityConfigProvider { #[derive(Default, Debug)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] -pub struct VecRemoteEntityConfigProvider(pub alloc::vec::Vec); +pub struct RemoteConfigList(pub alloc::vec::Vec); #[cfg(feature = "alloc")] -impl RemoteEntityConfigProvider for VecRemoteEntityConfigProvider { +impl RemoteConfigStore for RemoteConfigList { fn get(&self, remote_id: u64) -> Option<&RemoteEntityConfig> { self.0 .iter() @@ -350,12 +364,19 @@ impl RemoteEntityConfigProvider for VecRemoteEntityConfigProvider { .find(|cfg| cfg.entity_id.value() == remote_id) } - fn add_config(&mut self, cfg: &RemoteEntityConfig) -> bool { + fn add_config(&mut self, cfg: &RemoteEntityConfig) -> Result { + for other_cfg in self.0.iter() { + if cfg.entity_id.value() == other_cfg.entity_id.value() { + return Ok(false); + } + } self.0.push(*cfg); - true + Ok(true) } +} - fn remove_config(&mut self, remote_id: u64) -> bool { +impl RemoteConfigList { + pub fn remove_config(&mut self, remote_id: u64) -> bool { for (idx, cfg) in self.0.iter().enumerate() { if cfg.entity_id.value() == remote_id { self.0.remove(idx); @@ -366,10 +387,55 @@ impl RemoteEntityConfigProvider for VecRemoteEntityConfigProvider { } } -/// A remote entity configurations also implements the [RemoteEntityConfigProvider], but the -/// [RemoteEntityConfigProvider::add_config] and [RemoteEntityConfigProvider::remove_config] -/// are no-ops and always returns [false]. -impl RemoteEntityConfigProvider for RemoteEntityConfig { +/// This is a thin wrapper around a [alloc::vec::Vec] to store remote entity configurations. +/// It implements the full [RemoteEntityConfigProvider] trait. +#[derive(Default, Debug)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] +pub struct RemoteConfigListHeapless(pub heapless::vec::Vec); + +impl RemoteConfigStore for RemoteConfigListHeapless { + fn get(&self, remote_id: u64) -> Option<&RemoteEntityConfig> { + self.0 + .iter() + .find(|&cfg| cfg.entity_id.value() == remote_id) + } + + fn get_mut(&mut self, remote_id: u64) -> Option<&mut RemoteEntityConfig> { + self.0 + .iter_mut() + .find(|cfg| cfg.entity_id.value() == remote_id) + } + + fn add_config(&mut self, cfg: &RemoteEntityConfig) -> Result { + if self.0.is_full() { + return Err(RemoteConfigStoreError::Full); + } + for other_cfg in self.0.iter() { + if cfg.entity_id.value() == other_cfg.entity_id.value() { + return Ok(false); + } + } + self.0.push(*cfg).unwrap(); + Ok(true) + } +} + +impl RemoteConfigListHeapless { + pub fn remove_config(&mut self, remote_id: u64) -> bool { + for (idx, cfg) in self.0.iter().enumerate() { + if cfg.entity_id.value() == remote_id { + self.0.remove(idx); + return true; + } + } + false + } +} + +/// A remote entity configurations also implements the [RemoteConfigStore], but the +/// [RemoteConfigStore::add_config] always returns [RemoteConfigStoreError::Full]. +impl RemoteConfigStore for RemoteEntityConfig { fn get(&self, remote_id: u64) -> Option<&RemoteEntityConfig> { if remote_id == self.entity_id.value() { return Some(self); @@ -384,12 +450,8 @@ impl RemoteEntityConfigProvider for RemoteEntityConfig { None } - fn add_config(&mut self, _cfg: &RemoteEntityConfig) -> bool { - false - } - - fn remove_config(&mut self, _remote_id: u64) -> bool { - false + fn add_config(&mut self, _cfg: &RemoteEntityConfig) -> Result { + Err(RemoteConfigStoreError::Full) } } @@ -403,7 +465,7 @@ impl RemoteEntityConfigProvider for RemoteEntityConfig { /// /// For each error reported by the [FaultHandler], the appropriate fault handler callback /// will be called depending on the [FaultHandlerCode]. -pub trait UserFaultHookProvider { +pub trait UserFaultHook { fn notice_of_suspension_cb( &mut self, transaction_id: TransactionId, @@ -428,7 +490,7 @@ pub trait UserFaultHookProvider { #[derive(Default, Debug, PartialEq, Eq, Copy, Clone)] pub struct DummyFaultHook {} -impl UserFaultHookProvider for DummyFaultHook { +impl UserFaultHook for DummyFaultHook { fn notice_of_suspension_cb( &mut self, _transaction_id: TransactionId, @@ -476,14 +538,14 @@ impl UserFaultHookProvider for DummyFaultHook { /// These defaults can be overriden by using the [Self::set_fault_handler] method. /// Please note that in any case, fault handler overrides can be specified by the sending CFDP /// entity. -pub struct FaultHandler { +pub struct FaultHandler { handler_array: [FaultHandlerCode; 10], // Could also change the user fault handler trait to have non mutable methods, but that limits // flexbility on the user side.. pub user_hook: RefCell, } -impl FaultHandler { +impl FaultHandler { fn condition_code_to_array_index(conditon_code: ConditionCode) -> Option { Some(match conditon_code { ConditionCode::PositiveAckLimitReached => 0, @@ -589,17 +651,17 @@ impl Default for IndicationConfig { } /// Each CFDP entity handler has a [LocalEntityConfig]uration. -pub struct LocalEntityConfig { +pub struct LocalEntityConfig { pub id: UnsignedByteField, pub indication_cfg: IndicationConfig, - pub fault_handler: FaultHandler, + pub fault_handler: FaultHandler, } -impl LocalEntityConfig { +impl LocalEntityConfig { pub fn new( id: UnsignedByteField, indication_cfg: IndicationConfig, - hook: UserFaultHook, + hook: UserFaultHookInstance, ) -> Self { Self { id, @@ -609,12 +671,12 @@ impl LocalEntityConfig { } } -impl LocalEntityConfig { - pub fn user_fault_hook_mut(&mut self) -> &mut RefCell { +impl LocalEntityConfig { + pub fn user_fault_hook_mut(&mut self) -> &mut RefCell { &mut self.fault_handler.user_hook } - pub fn user_fault_hook(&self) -> &RefCell { + pub fn user_fault_hook(&self) -> &RefCell { &self.fault_handler.user_hook } } @@ -638,6 +700,14 @@ pub trait PduSendProvider { file_directive_type: Option, raw_pdu: &[u8], ) -> Result<(), GenericSendError>; + + fn send_file_directive_pdu( + &self, + file_directive_type: FileDirectiveType, + raw_pdu: &[u8], + ) -> Result<(), GenericSendError> { + self.send_pdu(PduType::FileDirective, Some(file_directive_type), raw_pdu) + } } #[cfg(feature = "std")] @@ -683,7 +753,7 @@ pub mod std_mod { } } - impl CountdownProvider for StdCountdown { + impl Countdown for StdCountdown { fn has_expired(&self) -> bool { if self.start_time.elapsed() > self.expiry_time { return true; @@ -714,7 +784,7 @@ pub mod std_mod { } } - impl TimerCreatorProvider for StdTimerCreator { + impl TimerCreator for StdTimerCreator { type Countdown = StdCountdown; fn create_countdown(&self, timer_context: TimerContext) -> Self::Countdown { @@ -800,7 +870,7 @@ pub enum PacketTarget { pub trait PduProvider { fn pdu_type(&self) -> PduType; fn file_directive_type(&self) -> Option; - fn pdu(&self) -> &[u8]; + fn raw_pdu(&self) -> &[u8]; fn packet_target(&self) -> Result; } @@ -815,7 +885,7 @@ impl PduProvider for DummyPduProvider { None } - fn pdu(&self) -> &[u8] { + fn raw_pdu(&self) -> &[u8] { &[] } @@ -927,7 +997,7 @@ impl PduProvider for PduRawWithInfo<'_> { self.file_directive_type } - fn pdu(&self) -> &[u8] { + fn raw_pdu(&self) -> &[u8] { self.raw_packet } @@ -989,7 +1059,7 @@ pub mod alloc_mod { self.file_directive_type } - fn pdu(&self) -> &[u8] { + fn raw_pdu(&self) -> &[u8] { &self.pdu } @@ -999,6 +1069,12 @@ pub mod alloc_mod { } } +#[derive(Debug)] +struct PositiveAckParams { + ack_timer: CountdownInstance, + ack_counter: u32, +} + #[cfg(test)] pub(crate) mod tests { use core::{ @@ -1016,6 +1092,7 @@ pub(crate) mod tests { CommonPduConfig, FileDirectiveType, PduHeader, eof::EofPdu, file_data::FileDataPdu, + finished::{DeliveryCode, FileStatus}, metadata::{MetadataGenericParams, MetadataPduCreator}, }, }, @@ -1057,7 +1134,7 @@ pub(crate) mod tests { expiry_control: TimerExpiryControl, } - impl CountdownProvider for TestCheckTimer { + impl Countdown for TestCheckTimer { fn has_expired(&self) -> bool { match self.context { TimerContext::CheckLimit { @@ -1117,7 +1194,7 @@ pub(crate) mod tests { } } - impl TimerCreatorProvider for TestCheckTimerCreator { + impl TimerCreator for TestCheckTimerCreator { type Countdown = TestCheckTimer; fn create_countdown(&self, timer_context: TimerContext) -> Self::Countdown { @@ -1135,6 +1212,7 @@ pub(crate) mod tests { } } + #[derive(Debug)] pub struct FileSegmentRecvdParamsNoSegMetadata { #[allow(dead_code)] pub id: TransactionId, @@ -1142,8 +1220,9 @@ pub(crate) mod tests { pub length: usize, } - #[derive(Default)] + #[derive(Default, Debug)] pub struct TestCfdpUser { + pub check_queues_empty_on_drop: bool, pub next_expected_seq_num: u64, pub expected_full_src_name: String, pub expected_full_dest_name: String, @@ -1164,6 +1243,7 @@ pub(crate) mod tests { expected_file_size: u64, ) -> Self { Self { + check_queues_empty_on_drop: true, next_expected_seq_num, expected_full_src_name, expected_full_dest_name, @@ -1181,6 +1261,36 @@ pub(crate) mod tests { assert_eq!(id.source_id, LOCAL_ID.into()); assert_eq!(id.seq_num().value(), self.next_expected_seq_num); } + + pub fn indication_queues_empty(&self) -> bool { + self.finished_indic_queue.is_empty() + && self.metadata_recv_queue.is_empty() + && self.file_seg_recvd_queue.is_empty() + } + + pub fn verify_finished_indication_retained( + &mut self, + delivery_code: DeliveryCode, + cond_code: ConditionCode, + id: TransactionId, + ) { + self.verify_finished_indication(delivery_code, cond_code, id, FileStatus::Retained); + } + + pub fn verify_finished_indication( + &mut self, + delivery_code: DeliveryCode, + cond_code: ConditionCode, + id: TransactionId, + file_status: FileStatus, + ) { + assert_eq!(self.finished_indic_queue.len(), 1); + let finished_indication = self.finished_indic_queue.pop_front().unwrap(); + assert_eq!(finished_indication.id, id); + assert_eq!(finished_indication.condition_code, cond_code); + assert_eq!(finished_indication.delivery_code, delivery_code); + assert_eq!(finished_indication.file_status, file_status); + } } impl CfdpUser for TestCfdpUser { @@ -1270,6 +1380,20 @@ pub(crate) mod tests { } } + impl Drop for TestCfdpUser { + fn drop(&mut self) { + if self.check_queues_empty_on_drop { + assert!( + self.indication_queues_empty(), + "indication queues not empty on drop: finished: {}, metadata: {}, file seg: {}", + self.finished_indic_queue.len(), + self.metadata_recv_queue.len(), + self.file_seg_recvd_queue.len() + ); + } + } + } + #[derive(Default, Debug)] pub(crate) struct TestFaultHandler { pub notice_of_suspension_queue: VecDeque<(TransactionId, ConditionCode, u64)>, @@ -1278,7 +1402,7 @@ pub(crate) mod tests { pub ignored_queue: VecDeque<(TransactionId, ConditionCode, u64)>, } - impl UserFaultHookProvider for TestFaultHandler { + impl UserFaultHook for TestFaultHandler { fn notice_of_suspension_cb( &mut self, transaction_id: TransactionId, @@ -1383,8 +1507,8 @@ pub(crate) mod tests { dest_id: impl Into, max_packet_len: usize, crc_on_transmission_by_default: bool, - ) -> StdRemoteEntityConfigProvider { - let mut table = StdRemoteEntityConfigProvider::default(); + ) -> RemoteConfigStoreStd { + let mut table = RemoteConfigStoreStd::default(); let remote_entity_cfg = RemoteEntityConfig::new_with_default_values( dest_id.into(), max_packet_len, @@ -1393,7 +1517,7 @@ pub(crate) mod tests { TransmissionMode::Unacknowledged, ChecksumType::Crc32, ); - table.add_config(&remote_entity_cfg); + table.add_config(&remote_entity_cfg).unwrap(); table } @@ -1534,9 +1658,10 @@ pub(crate) mod tests { TransmissionMode::Unacknowledged, ChecksumType::Crc32, ); - assert!(!remote_entity_cfg.add_config(&dummy)); - // Removal is no-op. - assert!(!remote_entity_cfg.remove_config(REMOTE_ID.value())); + assert_eq!( + remote_entity_cfg.add_config(&dummy).unwrap_err(), + RemoteConfigStoreError::Full + ); let remote_entity_retrieved = remote_entity_cfg.get(REMOTE_ID.value()).unwrap(); assert_eq!(remote_entity_retrieved.entity_id, REMOTE_ID.into()); // Does not exist. @@ -1554,9 +1679,9 @@ pub(crate) mod tests { TransmissionMode::Unacknowledged, ChecksumType::Crc32, ); - let mut remote_cfg_provider = StdRemoteEntityConfigProvider::default(); + let mut remote_cfg_provider = RemoteConfigStoreStd::default(); assert!(remote_cfg_provider.0.is_empty()); - remote_cfg_provider.add_config(&remote_entity_cfg); + remote_cfg_provider.add_config(&remote_entity_cfg).unwrap(); assert_eq!(remote_cfg_provider.0.len(), 1); let remote_entity_cfg_2 = RemoteEntityConfig::new_with_default_values( LOCAL_ID.into(), @@ -1568,7 +1693,9 @@ pub(crate) mod tests { ); let cfg_0 = remote_cfg_provider.get(REMOTE_ID.value()).unwrap(); assert_eq!(cfg_0.entity_id, REMOTE_ID.into()); - remote_cfg_provider.add_config(&remote_entity_cfg_2); + remote_cfg_provider + .add_config(&remote_entity_cfg_2) + .unwrap(); assert_eq!(remote_cfg_provider.0.len(), 2); let cfg_1 = remote_cfg_provider.get(LOCAL_ID.value()).unwrap(); assert_eq!(cfg_1.entity_id, LOCAL_ID.into()); @@ -1582,7 +1709,7 @@ pub(crate) mod tests { #[test] fn test_remote_cfg_provider_vector() { - let mut remote_cfg_provider = VecRemoteEntityConfigProvider::default(); + let mut remote_cfg_provider = RemoteConfigList::default(); let remote_entity_cfg = RemoteEntityConfig::new_with_default_values( REMOTE_ID.into(), 1024, @@ -1592,7 +1719,7 @@ pub(crate) mod tests { ChecksumType::Crc32, ); assert!(remote_cfg_provider.0.is_empty()); - remote_cfg_provider.add_config(&remote_entity_cfg); + remote_cfg_provider.add_config(&remote_entity_cfg).unwrap(); assert_eq!(remote_cfg_provider.0.len(), 1); let remote_entity_cfg_2 = RemoteEntityConfig::new_with_default_values( LOCAL_ID.into(), @@ -1604,7 +1731,11 @@ pub(crate) mod tests { ); let cfg_0 = remote_cfg_provider.get(REMOTE_ID.value()).unwrap(); assert_eq!(cfg_0.entity_id, REMOTE_ID.into()); - remote_cfg_provider.add_config(&remote_entity_cfg_2); + assert!( + remote_cfg_provider + .add_config(&remote_entity_cfg_2) + .unwrap() + ); assert_eq!(remote_cfg_provider.0.len(), 2); let cfg_1 = remote_cfg_provider.get(LOCAL_ID.value()).unwrap(); assert_eq!(cfg_1.entity_id, LOCAL_ID.into()); @@ -1634,7 +1765,7 @@ pub(crate) mod tests { let dummy_pdu_provider = DummyPduProvider(()); assert_eq!(dummy_pdu_provider.pdu_type(), PduType::FileData); assert!(dummy_pdu_provider.file_directive_type().is_none()); - assert_eq!(dummy_pdu_provider.pdu(), &[]); + assert_eq!(dummy_pdu_provider.raw_pdu(), &[]); assert_eq!( dummy_pdu_provider.packet_target(), Ok(PacketTarget::SourceEntity) diff --git a/src/lost_segments.rs b/src/lost_segments.rs new file mode 100644 index 0000000..8970d6e --- /dev/null +++ b/src/lost_segments.rs @@ -0,0 +1,1339 @@ +//! # Lost Segment Store Module +//! +//! The core abstraction provided by this module in the [LostSegmentStore]. +//! +//! The two concrete implementations provided are: +//! +//! * [LostSegmentsList]: A hash set based implementation which can grow dynamically andcan +//! optionally be bounded. Suitable for systems where dynamic allocation is allowed. +//! * [LostSegmentsListHeapless]: A fixed-size list based implementation where the size +//! of the lost segment list is statically known at compile-time. Suitable for resource +//! constrained devices where dyanamic allocation is not allowed or possible. + +use spacepackets::cfdp::{LargeFileFlag, pdu::nak::NakPduCreatorWithReservedSeqReqsBuf}; + +#[derive(Debug, PartialEq, Eq, thiserror::Error)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +#[non_exhaustive] +pub enum LostSegmentError { + #[error("store is full")] + StoreFull, + #[error("segment is empty")] + EmptySegment, + #[error("segment start {0} is larger than segment end {1}")] + StartLargerThanEnd(u64, u64), + #[error("large file segments are not supported")] + LargeFileSegmentNotSupported, + #[error("invalid segment boundary detected for lost segment ({0}, {1})")] + InvalidSegmentBoundary(u64, u64), +} + +#[derive(Debug, PartialEq, Eq, thiserror::Error)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +#[non_exhaustive] +pub enum LostSegmentWriteError { + #[error("number of segments mismatch: expected {expected}, actual {actual}")] + NumberOfSegmentsMismatch { expected: usize, actual: usize }, + #[error("buffer size not equal to required size")] + BufferSizeMissmatch { expected: usize, actual: usize }, + #[error("large file segment not compatible to normal file size")] + LargeSegmentForNormalFileSize, +} + +/// Generic trait to model a lost segment store. +/// +/// The destination handler can use this store to keep track of lost segments and re-requesting +/// them. This abstraction allow using different data structures as a backend. +pub trait LostSegmentStore { + // Iteration + type Iter<'a>: Iterator + 'a + where + Self: 'a; + + /// Iterate over all lost segments stored. + fn iter(&self) -> Self::Iter<'_>; + + /// Current number of lost segments stored. + fn number_of_segments(&self) -> usize; + + /// Implementations may explicitely omit support for large file segments to save memory if + /// large file sizes are not used. + fn supports_large_file_size(&self) -> bool; + fn capacity(&self) -> Option; + fn reset(&mut self); + + /// Checks whether a segment is already in the store. + /// + /// Implementors should be check whether the provided segment is a subset of an existing + /// segments. + fn segment_in_store(&self, segment: (u64, u64)) -> bool; + + /// Add a new lost segment. + /// + /// For efficiency reason, the implementors must not check whether the new segments already + /// exists in the store which is provided by the [Self::segment_in_store] method. + /// + /// Therefore, the caller must ensure that no duplicate segments are added. + fn add_lost_segment(&mut self, lost_seg: (u64, u64)) -> Result<(), LostSegmentError>; + + /// Remove a lost segment. + /// + /// Implementors should also be able to remove lost segments which are a subset of an existing + /// section but can return an error if an segment to remove only partially overlaps an existing + /// segment. + /// + /// Returns whether a segment was removed. + fn remove_lost_segment( + &mut self, + segment_to_remove: (u64, u64), + ) -> Result; + + /// The lost segment store may additionally have the capability to coalesce overlapping or + /// adjacent segments. + fn coalesce_lost_segments(&mut self) -> Result<(), LostSegmentError>; + + #[inline] + fn is_empty(&self) -> bool { + self.number_of_segments() == 0 + } + + /// Write the segments to the raw byte format of the NAK PDU segment requests as specified by + /// the CFDP standard 5.2.6.1 (NAK PDU). + fn write_segments_to_bytes( + &self, + buf: &mut [u8], + file_flag: LargeFileFlag, + ) -> Result { + let len_per_segment = if file_flag == LargeFileFlag::Large { + 16 + } else { + 8 + }; + let written_len = self.number_of_segments() * len_per_segment; + if written_len != buf.len() { + return Err(LostSegmentWriteError::BufferSizeMissmatch { + expected: written_len, + actual: buf.len(), + }); + } + let mut current_index = 0; + for segment in self.iter() { + match file_flag { + LargeFileFlag::Normal => { + if segment.0 > u32::MAX as u64 || segment.1 > u32::MAX as u64 { + return Err(LostSegmentWriteError::LargeSegmentForNormalFileSize); + } + buf[current_index..current_index + 4] + .copy_from_slice(&(segment.0 as u32).to_be_bytes()); + current_index += 4; + buf[current_index..current_index + 4] + .copy_from_slice(&(segment.1 as u32).to_be_bytes()); + current_index += 4; + } + LargeFileFlag::Large => { + buf[current_index..current_index + 8] + .copy_from_slice(&(segment.0).to_be_bytes()); + current_index += 8; + buf[current_index..current_index + 8] + .copy_from_slice(&(segment.1).to_be_bytes()); + current_index += 8; + } + } + } + Ok(current_index) + } + + /// Write the segments to the raw byte buffer of the supplied + /// [NAK builder][NakPduCreatorWithReservedSeqReqsBuf]. + /// + /// Please note that this function only works if all the segment requests fit into the NAK + /// builder buffer. In any other case, you should write a custom iteration and serialization + /// sequence which spreads the lost segments across multiple packets. + fn write_to_nak_segment_list( + &self, + nak_builder: &mut NakPduCreatorWithReservedSeqReqsBuf, + ) -> Result { + let file_flag = nak_builder.pdu_header().common_pdu_conf().file_flag; + if nak_builder.num_segment_reqs() != self.number_of_segments() { + return Err(LostSegmentWriteError::NumberOfSegmentsMismatch { + expected: self.number_of_segments(), + actual: nak_builder.num_segment_reqs(), + }); + } + self.write_segments_to_bytes(nak_builder.segment_request_buffer_mut(), file_flag) + } +} + +/// Implementation based on a [alloc::vec::Vec] which can grow dynamically. +/// +/// Optionally, a maximum capacity can be specified at creation time. This container allocates at +/// run-time! +#[cfg(feature = "alloc")] +#[derive(Debug, Default)] +pub struct LostSegmentsList { + list: alloc::vec::Vec<(u64, u64)>, + opt_capacity: Option, +} + +#[cfg(feature = "alloc")] +impl LostSegmentsList { + pub fn new(opt_capacity: Option) -> Self { + Self { + list: alloc::vec::Vec::new(), + opt_capacity, + } + } +} + +#[cfg(feature = "alloc")] +impl LostSegmentStore for LostSegmentsList { + type Iter<'a> + = core::iter::Cloned> + where + Self: 'a; + + fn iter(&self) -> Self::Iter<'_> { + self.list.iter().cloned() + } + + #[inline] + fn number_of_segments(&self) -> usize { + self.list.len() + } + + #[inline] + fn supports_large_file_size(&self) -> bool { + true + } + + #[inline] + fn capacity(&self) -> Option { + self.opt_capacity + } + + #[inline] + fn reset(&mut self) { + self.list.clear(); + } + + fn segment_in_store(&self, segment: (u64, u64)) -> bool { + for (seg_start, seg_end) in &self.list { + if segment.0 >= *seg_start && segment.1 <= *seg_end { + return true; + } + } + false + } + + #[inline] + fn add_lost_segment(&mut self, lost_seg: (u64, u64)) -> Result<(), LostSegmentError> { + if lost_seg.1 == lost_seg.0 { + return Err(LostSegmentError::EmptySegment); + } + if lost_seg.0 > lost_seg.1 { + return Err(LostSegmentError::StartLargerThanEnd(lost_seg.0, lost_seg.1)); + } + if let Some(capacity) = self.opt_capacity { + if self.list.len() == capacity { + return Err(LostSegmentError::StoreFull); + } + } + self.list.push((lost_seg.0, lost_seg.1)); + Ok(()) + } + + fn coalesce_lost_segments(&mut self) -> Result<(), LostSegmentError> { + // Remove empty/invalid ranges + self.list.retain(|&(s, e)| e > s); + if self.list.len() <= 1 { + return Ok(()); + } + + // Sort by start, then end (no extra allocs) + self.list + .as_mut_slice() + .sort_unstable_by_key(|&(start, _)| start); + + // In-place merge; merges overlapping or adjacent [s, e) where next.s <= prev.e + let mut w = 0usize; + for i in 0..self.list.len() { + if w == 0 { + self.list[w] = self.list[i]; + w = 1; + continue; + } + + let (prev_s, mut prev_e) = self.list[w - 1]; + let (s, e) = self.list[i]; + + if s <= prev_e { + // Extend previous + if e > prev_e { + prev_e = e; + self.list[w - 1] = (prev_s, prev_e); + } + } else { + // Start new merged interval + self.list[w] = (s, e); + w += 1; + } + } + + // Truncate to merged length + self.list.truncate(w); + Ok(()) + } + + #[inline] + fn remove_lost_segment( + &mut self, + segment_to_remove: (u64, u64), + ) -> Result { + if segment_to_remove.1 == segment_to_remove.0 { + return Err(LostSegmentError::EmptySegment); + } + if segment_to_remove.0 > segment_to_remove.1 { + return Err(LostSegmentError::StartLargerThanEnd( + segment_to_remove.0, + segment_to_remove.1, + )); + } + + // Binary search for the first candidate. + let idx = match self + .list + .binary_search_by_key(&segment_to_remove.0, |&(s, _)| s) + { + Ok(idx) => idx, + Err(insertion) => insertion.saturating_sub(1), + }; + + // --- single linear scan ------------------------------------------------- + let mut i = idx; + let list_len = self.list.len(); + while i < self.list.len() && self.list[i].0 <= segment_to_remove.1 { + let seg = &mut self.list[i]; + + // no overlap + if seg.1 < segment_to_remove.0 { + i += 1; + continue; + } + + // exact match → remove whole segment + if seg == &segment_to_remove { + self.list.remove(i); + // keep `i` unchanged: we swapped the tail element forward + return Ok(true); + } + + // partial overlap → forbidden + if (segment_to_remove.0 < seg.0 && segment_to_remove.1 > seg.0) + || (segment_to_remove.1 > seg.1 && segment_to_remove.0 < seg.1) + { + return Err(LostSegmentError::InvalidSegmentBoundary( + segment_to_remove.0, + segment_to_remove.1, + )); + } + + // Removal of subset. + + let mut changed = false; + + // Removal touches right edge only → shorten from the right + if segment_to_remove.1 == seg.1 { + seg.1 = segment_to_remove.0; + changed = true; + } + // Removal touches left edge only → shorten from the left + if segment_to_remove.0 == seg.0 { + seg.0 = segment_to_remove.1; + changed = true; + } + // Removal is strictly inside → split into two parts + if segment_to_remove.0 > seg.0 && segment_to_remove.1 < seg.1 { + if list_len == self.opt_capacity.unwrap_or(usize::MAX) { + return Err(LostSegmentError::StoreFull); + } + // Right remainder. + let end_of_right_remainder = seg.1; + // Left remainder. + seg.1 = segment_to_remove.0; + self.list + .insert(i + 1, (segment_to_remove.1, end_of_right_remainder)); + changed = true; + } + + // when both sides remain we truncated the current segment already + if changed { + return Ok(true); + } + + i += 1; + } + Ok(false) + } +} + +/// Implementation based on a [heapless::Vec] with a statically known container size. +#[derive(Default, Debug)] +pub struct LostSegmentsListHeapless { + list: heapless::vec::Vec<(T, T), N>, +} + +/// Type definition for segment list which only supports normal file sizes. This can be used +/// to save memory required for the lost segment list. +pub type LostSegmentsListNormalFilesHeapless = LostSegmentsListHeapless; + +impl LostSegmentsListHeapless { + pub fn new() -> Self { + Self { + list: heapless::Vec::new(), + } + } + + #[inline] + fn num_lost_segments(&self) -> usize { + self.list.len() + } + + #[inline] + fn capacity(&self) -> Option { + Some(N) + } + + #[inline] + fn reset(&mut self) { + self.list.clear(); + } +} + +impl LostSegmentsListHeapless { + fn coalesce_lost_segments(&mut self) -> Result<(), LostSegmentError> { + // Remove empty/invalid ranges + self.list.retain(|&(s, e)| e > s); + if self.list.len() <= 1 { + return Ok(()); + } + + // Sort by start, then end (no extra allocs) + self.list + .as_mut_slice() + .sort_unstable_by(|a, b| a.0.cmp(&b.0).then(a.1.cmp(&b.1))); + + // In-place merge; merges overlapping or adjacent [s, e) where next.s <= prev.e + let mut w = 0usize; + for i in 0..self.list.len() { + if w == 0 { + self.list[w] = self.list[i]; + w = 1; + continue; + } + + let (prev_s, mut prev_e) = self.list[w - 1]; + let (s, e) = self.list[i]; + + if s <= prev_e { + // Extend previous + if e > prev_e { + prev_e = e; + self.list[w - 1] = (prev_s, prev_e); + } + } else { + // Start new merged interval + self.list[w] = (s, e); + w += 1; + } + } + + // Truncate to merged length + self.list.truncate(w); + Ok(()) + } +} + +impl LostSegmentStore for LostSegmentsListHeapless { + type Iter<'a> + = core::iter::Cloned> + where + Self: 'a; + + fn iter(&self) -> Self::Iter<'_> { + self.list.iter().cloned() + } + + fn add_lost_segment(&mut self, lost_seg: (u64, u64)) -> Result<(), LostSegmentError> { + if lost_seg.1 == lost_seg.0 { + return Err(LostSegmentError::EmptySegment); + } + if lost_seg.0 > lost_seg.1 { + return Err(LostSegmentError::StartLargerThanEnd(lost_seg.0, lost_seg.1)); + } + if self.list.is_full() { + return Err(LostSegmentError::StoreFull); + } + self.list.push(lost_seg).ok(); + Ok(()) + } + + fn remove_lost_segment( + &mut self, + segment_to_remove: (u64, u64), + ) -> Result { + if segment_to_remove.1 == segment_to_remove.0 { + return Err(LostSegmentError::EmptySegment); + } + if segment_to_remove.0 > segment_to_remove.1 { + return Err(LostSegmentError::StartLargerThanEnd( + segment_to_remove.0, + segment_to_remove.1, + )); + } + + // Binary search for the first candidate. + let idx = match self + .list + .binary_search_by_key(&segment_to_remove.0, |&(s, _)| s) + { + Ok(idx) => idx, + Err(insertion) => insertion.saturating_sub(1), + }; + + // --- single linear scan ------------------------------------------------- + let mut i = idx; + let list_len = self.list.len(); + while i < self.list.len() && self.list[i].0 <= segment_to_remove.1 { + let seg = &mut self.list[i]; + + // no overlap + if seg.1 < segment_to_remove.0 { + i += 1; + continue; + } + + // exact match → remove whole segment + if seg == &segment_to_remove { + self.list.remove(i); + return Ok(true); + } + + // partial overlap → forbidden + if (segment_to_remove.0 < seg.0 && segment_to_remove.1 > seg.0) + || (segment_to_remove.1 > seg.1 && segment_to_remove.0 < seg.1) + { + return Err(LostSegmentError::InvalidSegmentBoundary( + segment_to_remove.0, + segment_to_remove.1, + )); + } + + // Removal of subset. + + let mut changed = false; + + // Removal touches right edge only → shorten from the right + if segment_to_remove.1 == seg.1 { + seg.1 = segment_to_remove.0; + changed = true; + } + // Removal touches left edge only → shorten from the left + if segment_to_remove.0 == seg.0 { + seg.0 = segment_to_remove.1; + changed = true; + } + // Removal is strictly inside → split into two parts + if segment_to_remove.0 > seg.0 && segment_to_remove.1 < seg.1 { + if list_len == N { + return Err(LostSegmentError::StoreFull); + } + // Right remainder. + let end_of_right_remainder = seg.1; + // Left remainder. + seg.1 = segment_to_remove.0; + self.list + .insert(i + 1, (segment_to_remove.1, end_of_right_remainder)) + .unwrap(); + changed = true; + } + + // when both sides remain we truncated the current segment already + if changed { + return Ok(true); + } + + i += 1; + } + Ok(false) + } + + fn coalesce_lost_segments(&mut self) -> Result<(), LostSegmentError> { + self.coalesce_lost_segments() + } + + fn number_of_segments(&self) -> usize { + self.num_lost_segments() + } + + fn supports_large_file_size(&self) -> bool { + true + } + + fn capacity(&self) -> Option { + self.capacity() + } + + fn reset(&mut self) { + self.reset(); + } + + fn segment_in_store(&self, segment: (u64, u64)) -> bool { + for (seg_start, seg_end) in &self.list { + if segment.0 >= *seg_start && segment.1 <= *seg_end { + return true; + } + } + false + } +} + +impl LostSegmentStore for LostSegmentsListHeapless { + type Iter<'a> + = core::iter::Map, fn(&(u32, u32)) -> (u64, u64)> + where + Self: 'a; + + fn iter(&self) -> Self::Iter<'_> { + self.list.iter().map(|pair| (pair.0 as u64, pair.1 as u64)) + } + + fn add_lost_segment(&mut self, lost_seg: (u64, u64)) -> Result<(), LostSegmentError> { + if lost_seg.1 == lost_seg.0 { + return Err(LostSegmentError::EmptySegment); + } + if lost_seg.0 > lost_seg.1 { + return Err(LostSegmentError::StartLargerThanEnd(lost_seg.0, lost_seg.1)); + } + if lost_seg.1 > u32::MAX as u64 || lost_seg.0 > u32::MAX as u64 { + return Err(LostSegmentError::LargeFileSegmentNotSupported); + } + if self.list.is_full() { + return Err(LostSegmentError::StoreFull); + } + self.list.push((lost_seg.0 as u32, lost_seg.1 as u32)).ok(); + Ok(()) + } + + fn remove_lost_segment( + &mut self, + segment_to_remove: (u64, u64), + ) -> Result { + if segment_to_remove.0 > u32::MAX as u64 || segment_to_remove.1 > u32::MAX as u64 { + return Err(LostSegmentError::LargeFileSegmentNotSupported); + } + if segment_to_remove.1 == segment_to_remove.0 { + return Err(LostSegmentError::EmptySegment); + } + if segment_to_remove.0 > segment_to_remove.1 { + return Err(LostSegmentError::StartLargerThanEnd( + segment_to_remove.0, + segment_to_remove.1, + )); + } + + // Binary search for the first candidate. + let idx = match self + .list + .binary_search_by_key(&segment_to_remove.0, |&(s, _)| s as u64) + { + Ok(idx) => idx, + Err(insertion) => insertion.saturating_sub(1), + }; + + // --- single linear scan ------------------------------------------------- + let mut i = idx; + let list_len = self.list.len(); + while i < self.list.len() && self.list[i].0 as u64 <= segment_to_remove.1 { + let seg = &mut self.list[i]; + + // no overlap + if (seg.1 as u64) < segment_to_remove.0 { + i += 1; + continue; + } + + // exact match → remove whole segment + if seg.0 as u64 == segment_to_remove.0 && seg.1 as u64 == segment_to_remove.1 { + self.list.remove(i); + return Ok(true); + } + + // partial overlap → forbidden + if (segment_to_remove.0 < seg.0 as u64 && segment_to_remove.1 > seg.0 as u64) + || (segment_to_remove.1 > seg.1 as u64 && segment_to_remove.0 < seg.1 as u64) + { + return Err(LostSegmentError::InvalidSegmentBoundary( + segment_to_remove.0, + segment_to_remove.1, + )); + } + + // Removal of subset. + + let mut changed = false; + + // Removal touches right edge only → shorten from the right + if segment_to_remove.1 == seg.1 as u64 { + seg.1 = segment_to_remove.0 as u32; + changed = true; + } + // Removal touches left edge only → shorten from the left + if segment_to_remove.0 == seg.0 as u64 { + seg.0 = segment_to_remove.1 as u32; + changed = true; + } + // Removal is strictly inside → split into two parts + if segment_to_remove.0 > seg.0 as u64 && segment_to_remove.1 < seg.1 as u64 { + if list_len == N { + return Err(LostSegmentError::StoreFull); + } + // Right remainder. + let end_of_right_remainder = seg.1; + // Left remainder. + seg.1 = segment_to_remove.0 as u32; + self.list + .insert(i + 1, (segment_to_remove.1 as u32, end_of_right_remainder)) + .unwrap(); + changed = true; + } + + // when both sides remain we truncated the current segment already + if changed { + return Ok(true); + } + + i += 1; + } + Ok(false) + } + + fn coalesce_lost_segments(&mut self) -> Result<(), LostSegmentError> { + self.coalesce_lost_segments() + } + + #[inline] + fn number_of_segments(&self) -> usize { + self.num_lost_segments() + } + + #[inline] + fn supports_large_file_size(&self) -> bool { + false + } + + #[inline] + fn capacity(&self) -> Option { + self.capacity() + } + + #[inline] + fn reset(&mut self) { + self.reset(); + } + + fn segment_in_store(&self, segment: (u64, u64)) -> bool { + for (seg_start, seg_end) in &self.list { + if segment.0 >= *seg_start as u64 && segment.1 <= *seg_end as u64 { + return true; + } + } + false + } +} + +#[cfg(test)] +mod tests { + use std::vec::Vec; + + use super::*; + + fn generic_basic_state_test( + store: &impl LostSegmentStore, + supports_large_file_size: bool, + capacity: Option, + ) { + assert_eq!(store.supports_large_file_size(), supports_large_file_size); + assert_eq!(store.number_of_segments(), 0); + assert!(store.is_empty()); + assert_eq!(store.capacity(), capacity); + assert_eq!(store.iter().count(), 0); + } + + fn generic_error_tests(store: &mut impl LostSegmentStore) { + matches!( + store.add_lost_segment((0, 0)).unwrap_err(), + LostSegmentError::EmptySegment + ); + matches!( + store.add_lost_segment((10, 0)).unwrap_err(), + LostSegmentError::StartLargerThanEnd(10, 0) + ); + matches!( + store.remove_lost_segment((0, 0)).unwrap_err(), + LostSegmentError::EmptySegment + ); + matches!( + store.remove_lost_segment((10, 0)).unwrap_err(), + LostSegmentError::StartLargerThanEnd(10, 0) + ); + } + + fn generic_add_segments_test(store: &mut impl LostSegmentStore) { + store.add_lost_segment((0, 20)).unwrap(); + assert_eq!(store.number_of_segments(), 1); + for segment in store.iter() { + assert_eq!(segment, (0, 20)); + } + store.add_lost_segment((20, 40)).unwrap(); + let mut segments: Vec<(u64, u64)> = store.iter().collect(); + segments.sort_unstable(); + assert_eq!(segments.len(), 2); + assert_eq!(segments[0], (0, 20)); + assert_eq!(segments[1], (20, 40)); + } + + fn generic_reset_test(store: &mut impl LostSegmentStore) { + store.add_lost_segment((0, 20)).unwrap(); + assert_eq!(store.number_of_segments(), 1); + store.reset(); + assert_eq!(store.number_of_segments(), 0); + assert!(store.is_empty()); + assert_eq!(store.iter().count(), 0); + } + + fn generic_removal_test(store: &mut impl LostSegmentStore) { + store.add_lost_segment((0, 20)).unwrap(); + store.add_lost_segment((20, 40)).unwrap(); + assert_eq!(store.number_of_segments(), 2); + assert!(store.remove_lost_segment((0, 20)).unwrap()); + assert_eq!(store.number_of_segments(), 1); + assert!(!store.remove_lost_segment((0, 20)).unwrap()); + assert_eq!(store.number_of_segments(), 1); + assert!(store.remove_lost_segment((20, 40)).unwrap()); + assert_eq!(store.number_of_segments(), 0); + } + + fn generic_partial_removal_test_right_aligned(store: &mut impl LostSegmentStore) { + store.add_lost_segment((0, 20)).unwrap(); + assert_eq!(store.number_of_segments(), 1); + store.remove_lost_segment((0, 10)).unwrap(); + assert_eq!(store.number_of_segments(), 1); + for list in store.iter() { + assert_eq!(list, (10, 20)); + } + store.remove_lost_segment((10, 20)).unwrap(); + assert!(store.is_empty()); + } + + fn generic_partial_removal_test_left_aligned(store: &mut impl LostSegmentStore) { + store.add_lost_segment((0, 20)).unwrap(); + assert_eq!(store.number_of_segments(), 1); + store.remove_lost_segment((10, 20)).unwrap(); + assert_eq!(store.number_of_segments(), 1); + for list in store.iter() { + assert_eq!(list, (0, 10)); + } + store.remove_lost_segment((0, 10)).unwrap(); + assert!(store.is_empty()); + } + + fn generic_partial_removal_test_fully_contained(store: &mut impl LostSegmentStore) { + store.add_lost_segment((0, 20)).unwrap(); + assert_eq!(store.number_of_segments(), 1); + store.remove_lost_segment((5, 15)).unwrap(); + assert_eq!(store.number_of_segments(), 2); + let seg_list = store.iter().collect::>(); + assert!(seg_list.contains(&(0, 5))); + assert!(seg_list.contains(&(15, 20))); + store.remove_lost_segment((0, 5)).unwrap(); + assert_eq!(store.number_of_segments(), 1); + store.remove_lost_segment((15, 20)).unwrap(); + assert!(store.is_empty()); + } + + fn generic_partial_removal_fails_test_0(store: &mut impl LostSegmentStore) { + store.add_lost_segment((10, 20)).unwrap(); + assert_eq!(store.number_of_segments(), 1); + assert_eq!( + store.remove_lost_segment((5, 15)).unwrap_err(), + LostSegmentError::InvalidSegmentBoundary(5, 15) + ); + } + + fn generic_partial_removal_fails_test_1(store: &mut impl LostSegmentStore) { + store.add_lost_segment((10, 20)).unwrap(); + assert_eq!(store.number_of_segments(), 1); + assert_eq!( + store.remove_lost_segment((15, 25)).unwrap_err(), + LostSegmentError::InvalidSegmentBoundary(15, 25) + ); + } + + fn generic_partial_removal_fails_test_2(store: &mut impl LostSegmentStore) { + store.add_lost_segment((10, 20)).unwrap(); + assert_eq!(store.number_of_segments(), 1); + assert_eq!( + store.remove_lost_segment((10, 25)).unwrap_err(), + LostSegmentError::InvalidSegmentBoundary(10, 25) + ); + } + + fn generic_partial_removal_fails_test_3(store: &mut impl LostSegmentStore) { + store.add_lost_segment((10, 20)).unwrap(); + assert_eq!(store.number_of_segments(), 1); + assert_eq!( + store.remove_lost_segment((5, 20)).unwrap_err(), + LostSegmentError::InvalidSegmentBoundary(5, 20) + ); + } + + fn generic_coalescing_simple_test(store: &mut impl LostSegmentStore) { + store.add_lost_segment((0, 20)).unwrap(); + store.add_lost_segment((20, 40)).unwrap(); + store.add_lost_segment((40, 60)).unwrap(); + store.coalesce_lost_segments().unwrap(); + for segment in store.iter() { + assert_eq!(segment, (0, 60)); + } + assert_eq!(store.number_of_segments(), 1); + } + + fn generic_coalescing_simple_with_gaps_test(store: &mut impl LostSegmentStore) { + store.add_lost_segment((0, 20)).unwrap(); + store.add_lost_segment((20, 40)).unwrap(); + store.add_lost_segment((40, 60)).unwrap(); + + store.add_lost_segment((80, 100)).unwrap(); + store.add_lost_segment((110, 120)).unwrap(); + store.add_lost_segment((120, 130)).unwrap(); + store.coalesce_lost_segments().unwrap(); + let mut segments: Vec<(u64, u64)> = store.iter().collect(); + segments.sort_unstable(); + assert_eq!(segments.len(), 3); + assert_eq!(segments[0], (0, 60)); + assert_eq!(segments[1], (80, 100)); + assert_eq!(segments[2], (110, 130)); + } + + fn generic_coalescing_overlapping_simple_test(store: &mut impl LostSegmentStore) { + store.add_lost_segment((0, 20)).unwrap(); + store.add_lost_segment((10, 30)).unwrap(); + store.coalesce_lost_segments().unwrap(); + for segment in store.iter() { + assert_eq!(segment, (0, 30)); + } + assert_eq!(store.number_of_segments(), 1); + } + + fn generic_coalescing_overlapping_adjacent_test(store: &mut impl LostSegmentStore) { + store.add_lost_segment((0, 20)).unwrap(); + store.add_lost_segment((10, 30)).unwrap(); + store.add_lost_segment((20, 40)).unwrap(); + store.coalesce_lost_segments().unwrap(); + for segment in store.iter() { + assert_eq!(segment, (0, 40)); + } + assert_eq!(store.number_of_segments(), 1); + } + + fn generic_useless_coalescing_test(store: &mut impl LostSegmentStore) { + // Is okay, does nothing. + assert!(store.coalesce_lost_segments().is_ok()); + assert_eq!(store.number_of_segments(), 0); + assert!(store.is_empty()); + store.add_lost_segment((10, 20)).unwrap(); + // Is okay, does nothing. + assert!(store.coalesce_lost_segments().is_ok()); + for segment in store.iter() { + assert_eq!(segment, (10, 20)); + } + } + + #[test] + fn test_basic_map_state_list() { + let store = LostSegmentsList::default(); + generic_basic_state_test(&store, true, None); + } + + #[test] + fn test_basic_errors_list() { + let mut store = LostSegmentsList::default(); + generic_error_tests(&mut store); + } + + #[test] + fn test_add_segments_list() { + let mut store = LostSegmentsList::default(); + generic_add_segments_test(&mut store); + } + + #[test] + fn test_reset_list() { + let mut store = LostSegmentsList::default(); + generic_reset_test(&mut store); + } + + #[test] + fn test_removal_map() { + let mut store = LostSegmentsList::default(); + generic_removal_test(&mut store); + } + + #[test] + fn test_partial_removal_list_0() { + let mut store = LostSegmentsList::default(); + generic_partial_removal_test_right_aligned(&mut store); + } + + #[test] + fn test_partial_removal_list_1() { + let mut store = LostSegmentsList::default(); + generic_partial_removal_test_left_aligned(&mut store); + } + + #[test] + fn test_partial_removal_list_2() { + let mut store = LostSegmentsList::default(); + generic_partial_removal_test_fully_contained(&mut store); + } + + #[test] + fn test_partial_removal_list_fails_0() { + let mut store = LostSegmentsList::default(); + generic_partial_removal_fails_test_0(&mut store); + } + + #[test] + fn test_partial_removal_list_fails_1() { + let mut store = LostSegmentsList::default(); + generic_partial_removal_fails_test_1(&mut store); + } + + #[test] + fn test_partial_removal_list_fails_2() { + let mut store = LostSegmentsList::default(); + generic_partial_removal_fails_test_2(&mut store); + } + + #[test] + fn test_partial_removal_list_fails_3() { + let mut store = LostSegmentsList::default(); + generic_partial_removal_fails_test_3(&mut store); + } + + #[test] + fn test_cap_limit_list() { + let mut store = LostSegmentsList::new(Some(4)); + for i in 0..4 { + store.add_lost_segment((i * 20, (i + 1) * 20)).unwrap(); + } + matches!( + store.add_lost_segment((80, 100)), + Err(LostSegmentError::StoreFull) + ); + } + + #[test] + fn test_basic_list_state_list() { + let store = LostSegmentsListNormalFilesHeapless::<12>::default(); + generic_basic_state_test(&store, false, Some(12)); + let store = LostSegmentsListNormalFilesHeapless::<12>::new(); + generic_basic_state_test(&store, false, Some(12)); + } + #[test] + fn test_basic_errors_list_heapless() { + let mut store = LostSegmentsListNormalFilesHeapless::<12>::default(); + generic_error_tests(&mut store); + } + + #[test] + fn test_add_segments_list_heapless() { + let mut store = LostSegmentsListNormalFilesHeapless::<12>::default(); + generic_add_segments_test(&mut store); + } + + #[test] + fn test_reset_list_heapless() { + let mut store = LostSegmentsListNormalFilesHeapless::<12>::default(); + generic_reset_test(&mut store); + } + + #[test] + fn test_removal_list() { + let mut store = LostSegmentsListNormalFilesHeapless::<12>::default(); + generic_removal_test(&mut store); + } + + #[test] + fn test_partial_removal_list_heapless_0() { + let mut store = LostSegmentsListNormalFilesHeapless::<12>::default(); + generic_partial_removal_test_right_aligned(&mut store); + } + + #[test] + fn test_partial_removal_list_heapless_1() { + let mut store = LostSegmentsListNormalFilesHeapless::<12>::default(); + generic_partial_removal_test_left_aligned(&mut store); + } + + #[test] + fn test_partial_removal_list_heapless_2() { + let mut store = LostSegmentsListNormalFilesHeapless::<12>::default(); + generic_partial_removal_test_fully_contained(&mut store); + } + + #[test] + fn test_partial_removal_heapless_list_fails_0() { + let mut store = LostSegmentsListNormalFilesHeapless::<12>::default(); + generic_partial_removal_fails_test_0(&mut store); + } + + #[test] + fn test_partial_removal_heapless_list_fails_1() { + let mut store = LostSegmentsListNormalFilesHeapless::<12>::default(); + generic_partial_removal_fails_test_1(&mut store); + } + + #[test] + fn test_partial_removal_heapless_list_fails_2() { + let mut store = LostSegmentsListNormalFilesHeapless::<12>::default(); + generic_partial_removal_fails_test_2(&mut store); + } + + #[test] + fn test_partial_removal_heapless_list_fails_3() { + let mut store = LostSegmentsListNormalFilesHeapless::<12>::default(); + generic_partial_removal_fails_test_3(&mut store); + } + + fn generic_cap_limit_list_by_removal_test(store: &mut impl LostSegmentStore) { + for i in 0..4 { + store.add_lost_segment((i * 20, (i + 1) * 20)).unwrap(); + } + // This splits the segments, and the insert attempt should fail. + matches!( + store.remove_lost_segment((85, 95)), + Err(LostSegmentError::StoreFull) + ); + } + + fn generic_cap_limit_list_by_addition_test(store: &mut impl LostSegmentStore) { + for i in 0..4 { + store.add_lost_segment((i * 20, (i + 1) * 20)).unwrap(); + } + // This splits the segments, and the insert attempt should fail. + matches!( + store.add_lost_segment((100, 120)), + Err(LostSegmentError::StoreFull) + ); + } + + #[test] + fn test_cap_limit_list_by_removal() { + let mut store = LostSegmentsList::new(Some(4)); + generic_cap_limit_list_by_removal_test(&mut store); + } + + #[test] + fn test_cap_limit_list_heapless_by_removal() { + let mut store = LostSegmentsListNormalFilesHeapless::<4>::default(); + generic_cap_limit_list_by_removal_test(&mut store); + } + + #[test] + fn test_cap_limit_list_heapless() { + let mut store = LostSegmentsListNormalFilesHeapless::<4>::default(); + generic_cap_limit_list_by_addition_test(&mut store); + } + + #[test] + fn test_large_file_size_unsupported() { + let mut store = LostSegmentsListNormalFilesHeapless::<4>::default(); + matches!( + store.add_lost_segment((0, u32::MAX as u64 + 1)), + Err(LostSegmentError::LargeFileSegmentNotSupported) + ); + } + + #[test] + fn test_large_file_size_unsupported_2() { + let mut store = LostSegmentsListNormalFilesHeapless::<4>::default(); + matches!( + store.remove_lost_segment((0, u32::MAX as u64 + 1)), + Err(LostSegmentError::LargeFileSegmentNotSupported) + ); + } + + #[test] + fn test_basic_list_state_list_large() { + let store = LostSegmentsListHeapless::<12, u64>::default(); + generic_basic_state_test(&store, true, Some(12)); + } + #[test] + fn test_basic_errors_list_large() { + let mut store = LostSegmentsListHeapless::<12, u64>::default(); + generic_error_tests(&mut store); + } + + #[test] + fn test_add_segments_list_large() { + let mut store = LostSegmentsListHeapless::<12, u64>::default(); + generic_add_segments_test(&mut store); + } + + #[test] + fn test_reset_list_large() { + let mut store = LostSegmentsListHeapless::<12, u64>::default(); + generic_reset_test(&mut store); + } + + #[test] + fn test_removal_list_large() { + let mut store = LostSegmentsListHeapless::<12, u64>::default(); + generic_removal_test(&mut store); + } + + #[test] + fn test_partial_removal_list_heapless_large_0() { + let mut store = LostSegmentsListHeapless::<12, u64>::default(); + generic_partial_removal_test_right_aligned(&mut store); + } + + #[test] + fn test_partial_removal_list_heapless_large_1() { + let mut store = LostSegmentsListHeapless::<12, u64>::default(); + generic_partial_removal_test_left_aligned(&mut store); + } + + #[test] + fn test_partial_removal_list_heapless_large_2() { + let mut store = LostSegmentsListHeapless::<12, u64>::default(); + generic_partial_removal_test_fully_contained(&mut store); + } + + #[test] + fn test_partial_removal_heapless_list_large_fails_0() { + let mut store = LostSegmentsListHeapless::<12, u64>::default(); + generic_partial_removal_fails_test_0(&mut store); + } + + #[test] + fn test_partial_removal_heapless_list_large_fails_1() { + let mut store = LostSegmentsListHeapless::<12, u64>::default(); + generic_partial_removal_fails_test_1(&mut store); + } + + #[test] + fn test_partial_removal_heapless_list_large_fails_2() { + let mut store = LostSegmentsListHeapless::<12, u64>::default(); + generic_partial_removal_fails_test_2(&mut store); + } + + #[test] + fn test_partial_removal_heapless_list_large_fails_3() { + let mut store = LostSegmentsListHeapless::<12, u64>::default(); + generic_partial_removal_fails_test_3(&mut store); + } + + #[test] + fn test_cap_limit_list_large() { + let mut store = LostSegmentsListHeapless::<4, u64>::default(); + generic_cap_limit_list_by_removal_test(&mut store); + } + + #[test] + fn test_coalescing_simple_in_map() { + let mut store = LostSegmentsList::default(); + generic_coalescing_simple_test(&mut store); + } + + #[test] + fn test_useless_coalescing_map() { + let mut store = LostSegmentsList::default(); + generic_useless_coalescing_test(&mut store); + } + + #[test] + fn test_useless_coalescing_list() { + let mut store = LostSegmentsListNormalFilesHeapless::<4>::default(); + generic_useless_coalescing_test(&mut store); + } + + #[test] + fn test_coalescing_simple_in_list() { + let mut store = LostSegmentsListNormalFilesHeapless::<4>::default(); + generic_coalescing_simple_test(&mut store); + } + + #[test] + fn test_coalescing_simple_in_list_large() { + let mut store = LostSegmentsListHeapless::<4, u64>::default(); + generic_coalescing_simple_test(&mut store); + } + + #[test] + fn test_coalescing_overlapping_simple_in_map() { + let mut store = LostSegmentsList::default(); + generic_coalescing_overlapping_simple_test(&mut store); + } + + #[test] + fn test_coalescing_overlapping_simple_in_list() { + let mut store = LostSegmentsListNormalFilesHeapless::<4>::default(); + generic_coalescing_overlapping_simple_test(&mut store); + } + + #[test] + fn test_coalescing_overlapping_simple_in_list_large() { + let mut store = LostSegmentsListHeapless::<4, u64>::default(); + generic_coalescing_overlapping_simple_test(&mut store); + } + + #[test] + fn test_coalescing_overlapping_adjacent_in_map() { + let mut store = LostSegmentsList::default(); + generic_coalescing_overlapping_adjacent_test(&mut store); + } + + #[test] + fn test_coalescing_overlapping_adjacent_in_list() { + let mut store = LostSegmentsListNormalFilesHeapless::<4>::default(); + generic_coalescing_overlapping_adjacent_test(&mut store); + } + + #[test] + fn test_coalescing_overlapping_adjacent_in_list_large() { + let mut store = LostSegmentsListHeapless::<4, u64>::default(); + generic_coalescing_overlapping_adjacent_test(&mut store); + } + + #[test] + fn test_coalescing_simple_with_gaps_in_map() { + let mut store = LostSegmentsList::default(); + generic_coalescing_simple_with_gaps_test(&mut store); + } + + #[test] + fn test_coalescing_simple_with_gaps_in_list() { + let mut store = LostSegmentsListNormalFilesHeapless::<8>::default(); + generic_coalescing_simple_with_gaps_test(&mut store); + } + + #[test] + fn test_coalescing_simple_with_gaps_in_list_large() { + let mut store = LostSegmentsListHeapless::<8, u64>::default(); + generic_coalescing_simple_with_gaps_test(&mut store); + } +} diff --git a/src/source.rs b/src/source.rs index 3eb46a7..b0660f6 100644 --- a/src/source.rs +++ b/src/source.rs @@ -40,7 +40,6 @@ use core::{ cell::{Cell, RefCell}, ops::ControlFlow, str::Utf8Error, - time::Duration, }; use spacepackets::{ @@ -68,13 +67,13 @@ use spacepackets::{ use spacepackets::seq_count::SequenceCounter; use crate::{ - DummyPduProvider, EntityType, GenericSendError, PduProvider, TimerCreatorProvider, - time::CountdownProvider, + DummyPduProvider, EntityType, GenericSendError, PduProvider, PositiveAckParams, TimerCreator, + time::Countdown, }; use super::{ - LocalEntityConfig, PacketTarget, PduSendProvider, RemoteEntityConfig, - RemoteEntityConfigProvider, State, TransactionId, UserFaultHookProvider, + LocalEntityConfig, PacketTarget, PduSendProvider, RemoteConfigStore, RemoteEntityConfig, State, + TransactionId, UserFaultHook, filestore::{FilestoreError, VirtualFilestore}, request::{ReadablePutRequest, StaticPutRequestCacher}, user::{CfdpUser, TransactionFinishedParams}, @@ -94,7 +93,6 @@ pub enum TransactionStep { SendingEof = 6, WaitingForEofAck = 7, WaitingForFinished = 8, - //SendingAckOfFinished = 9, NoticeOfCompletion = 10, } @@ -204,12 +202,6 @@ pub enum PutRequestError { FilestoreError(#[from] FilestoreError), } -#[derive(Debug)] -struct PositiveAckParams { - ack_timer: Countdown, - ack_counter: u32, -} - #[derive(Debug, Default, Clone, Copy)] pub struct AnomalyTracker { invalid_ack_directive_code: u8, @@ -251,18 +243,18 @@ pub enum FsmContext { /// thread pool, or move the newly created handler to a new thread. pub struct SourceHandler< PduSender: PduSendProvider, - UserFaultHook: UserFaultHookProvider, + UserFaultHookInstance: UserFaultHook, Vfs: VirtualFilestore, - RemoteCfgTable: RemoteEntityConfigProvider, - TimerCreator: TimerCreatorProvider, - Countdown: CountdownProvider, + RemoteConfigStoreInstance: RemoteConfigStore, + TimerCreatorInstance: TimerCreator, + CountdownInstance: Countdown, SequenceCounterInstance: SequenceCounter, > { - local_cfg: LocalEntityConfig, + local_cfg: LocalEntityConfig, pdu_sender: PduSender, pdu_and_cksum_buffer: RefCell>, put_request_cacher: StaticPutRequestCacher, - remote_cfg_table: RemoteCfgTable, + remote_cfg_table: RemoteConfigStoreInstance, vfs: Vfs, state_helper: StateHelper, // Transfer related state information @@ -271,29 +263,29 @@ pub struct SourceHandler< file_params: FileParams, // PDU configuration is cached so it can be re-used for all PDUs generated for file transfers. pdu_conf: CommonPduConfig, - check_timer: RefCell>, - positive_ack_params: RefCell>>, - timer_creator: TimerCreator, + check_timer: RefCell>, + positive_ack_params: RefCell>>, + timer_creator: TimerCreatorInstance, seq_count_provider: SequenceCounterInstance, anomalies: AnomalyTracker, } impl< PduSender: PduSendProvider, - UserFaultHook: UserFaultHookProvider, + UserFaultHookInstance: UserFaultHook, Vfs: VirtualFilestore, - RemoteCfgTable: RemoteEntityConfigProvider, - TimerCreator: TimerCreatorProvider, - Countdown: CountdownProvider, + RemoteConfigStoreInstance: RemoteConfigStore, + TimerCreatorInstance: TimerCreator, + CountdownInstance: Countdown, SequenceCounterInstance: SequenceCounter, > SourceHandler< PduSender, - UserFaultHook, + UserFaultHookInstance, Vfs, - RemoteCfgTable, - TimerCreator, - Countdown, + RemoteConfigStoreInstance, + TimerCreatorInstance, + CountdownInstance, SequenceCounterInstance, > { @@ -321,13 +313,13 @@ impl< /// which contains an incrementing counter. #[allow(clippy::too_many_arguments)] pub fn new( - cfg: LocalEntityConfig, + cfg: LocalEntityConfig, pdu_sender: PduSender, vfs: Vfs, put_request_cacher: StaticPutRequestCacher, pdu_and_cksum_buf_size: usize, - remote_cfg_table: RemoteCfgTable, - timer_creator: TimerCreator, + remote_cfg_table: RemoteConfigStoreInstance, + timer_creator: TimerCreatorInstance, seq_count_provider: SequenceCounterInstance, ) -> Self { Self { @@ -416,7 +408,7 @@ impl< } #[inline] - pub fn local_cfg(&self) -> &LocalEntityConfig { + pub fn local_cfg(&self) -> &LocalEntityConfig { &self.local_cfg } @@ -551,16 +543,16 @@ impl< .expect("PDU directive type unexpectedly not set") { FileDirectiveType::FinishedPdu => { - let finished_pdu = FinishedPduReader::new(packet_to_insert.pdu())?; + let finished_pdu = FinishedPduReader::new(packet_to_insert.raw_pdu())?; self.handle_finished_pdu(&finished_pdu)? } FileDirectiveType::NakPdu => { - let nak_pdu = NakPduReader::new(packet_to_insert.pdu())?; + let nak_pdu = NakPduReader::new(packet_to_insert.raw_pdu())?; sent_packets += self.handle_nak_pdu(&nak_pdu)?; } FileDirectiveType::KeepAlivePdu => self.handle_keep_alive_pdu(), FileDirectiveType::AckPdu => { - let ack_pdu = AckPdu::from_bytes(packet_to_insert.pdu())?; + let ack_pdu = AckPdu::from_bytes(packet_to_insert.raw_pdu())?; self.handle_ack_pdu(&ack_pdu)? } FileDirectiveType::EofPdu @@ -789,12 +781,11 @@ impl< ack_timer: self .timer_creator .create_countdown(crate::TimerContext::PositiveAck { - expiry_time: Duration::from_secs( - self.tstate_ref() - .remote_cfg - .borrow() - .positive_ack_timer_interval_seconds as u64, - ), + expiry_time: self + .tstate_ref() + .remote_cfg + .borrow() + .positive_ack_timer_interval, }), ack_counter: 0, }) @@ -844,7 +835,8 @@ impl< FileDirectiveType::FinishedPdu, condition_code, transaction_status, - )?; + ) + .map_err(PduError::from)?; self.pdu_send_helper(&ack_pdu)?; Ok(()) } @@ -1203,7 +1195,7 @@ mod tests { use super::*; use crate::{ - CRC_32, FaultHandler, IndicationConfig, PduRawWithInfo, StdRemoteEntityConfigProvider, + CRC_32, FaultHandler, IndicationConfig, PduRawWithInfo, RemoteConfigStoreStd, filestore::NativeFilestore, request::PutRequestOwned, source::TransactionStep, @@ -1229,7 +1221,7 @@ mod tests { TestCfdpSender, TestFaultHandler, NativeFilestore, - StdRemoteEntityConfigProvider, + RemoteConfigStoreStd, TestCheckTimerCreator, TestCheckTimer, SequenceCounterSimple, @@ -1387,7 +1379,7 @@ mod tests { transfer_info: &TransferInfo, seg_reqs: &[(u32, u32)], ) { - let nak_pdu = NakPduCreator::new( + let nak_pdu = NakPduCreator::new_normal_file_size( transfer_info.pdu_header, 0, transfer_info.file_size as u32, @@ -1693,18 +1685,21 @@ mod tests { Some(false), ) .expect("creating put request failed"); - let mut cfdp_user = tb.create_user(0, file_size); - let transaction_info = tb.common_file_transfer_init_with_metadata_check( - &mut cfdp_user, - put_request, - file_size, - ); + let mut user = tb.create_user(0, file_size); + let transfer_info = + tb.common_file_transfer_init_with_metadata_check(&mut user, put_request, file_size); tb.common_eof_pdu_check( - &mut cfdp_user, - transaction_info.closure_requested, + &mut user, + transfer_info.closure_requested, EofParams::new_success(file_size, CRC_32.digest().finalize()), 1, - ) + ); + user.verify_finished_indication( + DeliveryCode::Complete, + ConditionCode::NoError, + transfer_info.id, + FileStatus::Unreported, + ); } #[test] @@ -1719,53 +1714,55 @@ mod tests { Some(false), ) .expect("creating put request failed"); - let mut cfdp_user = tb.create_user(0, file_size); - let transaction_info = tb.common_file_transfer_init_with_metadata_check( - &mut cfdp_user, - put_request, - file_size, - ); + let mut user = tb.create_user(0, file_size); + let transaction_info = + tb.common_file_transfer_init_with_metadata_check(&mut user, put_request, file_size); tb.common_eof_pdu_check( - &mut cfdp_user, + &mut user, transaction_info.closure_requested, EofParams::new_success(file_size, CRC_32.digest().finalize()), 1, ); - tb.acknowledge_eof_pdu(&mut cfdp_user, &transaction_info); - tb.finish_handling(&mut cfdp_user, &transaction_info); + tb.acknowledge_eof_pdu(&mut user, &transaction_info); + tb.finish_handling(&mut user, &transaction_info); tb.common_finished_pdu_ack_check(); + user.verify_finished_indication_retained( + DeliveryCode::Complete, + ConditionCode::NoError, + transaction_info.id, + ); } #[test] fn test_tiny_file_transfer_not_acked_no_closure() { - let mut cfdp_user = TestCfdpUser::default(); + let mut user = TestCfdpUser::default(); let mut tb = SourceHandlerTestbench::new(TransmissionMode::Unacknowledged, false, 512); - tb.common_tiny_file_transfer(&mut cfdp_user, false); + tb.common_tiny_file_transfer(&mut user, false); } #[test] fn test_tiny_file_transfer_acked() { - let mut cfdp_user = TestCfdpUser::default(); + let mut user = TestCfdpUser::default(); let mut tb = SourceHandlerTestbench::new(TransmissionMode::Acknowledged, false, 512); - let (_data, transfer_info) = tb.common_tiny_file_transfer(&mut cfdp_user, false); - tb.acknowledge_eof_pdu(&mut cfdp_user, &transfer_info); - tb.finish_handling(&mut cfdp_user, &transfer_info); + let (_data, transfer_info) = tb.common_tiny_file_transfer(&mut user, false); + tb.acknowledge_eof_pdu(&mut user, &transfer_info); + tb.finish_handling(&mut user, &transfer_info); tb.common_finished_pdu_ack_check(); } #[test] fn test_tiny_file_transfer_not_acked_with_closure() { let mut tb = SourceHandlerTestbench::new(TransmissionMode::Unacknowledged, false, 512); - let mut cfdp_user = TestCfdpUser::default(); - let (_data, transfer_info) = tb.common_tiny_file_transfer(&mut cfdp_user, true); - tb.finish_handling(&mut cfdp_user, &transfer_info) + let mut user = TestCfdpUser::default(); + let (_data, transfer_info) = tb.common_tiny_file_transfer(&mut user, true); + tb.finish_handling(&mut user, &transfer_info) } #[test] fn test_two_segment_file_transfer_not_acked_no_closure() { let mut tb = SourceHandlerTestbench::new(TransmissionMode::Unacknowledged, false, 128); - let mut cfdp_user = TestCfdpUser::default(); + let mut user = TestCfdpUser::default(); let mut file = OpenOptions::new() .write(true) .open(&tb.srcfile) @@ -1775,14 +1772,14 @@ mod tests { file.write_all(&rand_data) .expect("writing file content failed"); drop(file); - let (_, fd_pdus) = tb.generic_file_transfer(&mut cfdp_user, false, rand_data.to_vec()); + let (_, fd_pdus) = tb.generic_file_transfer(&mut user, false, rand_data.to_vec()); assert_eq!(fd_pdus, 2); } #[test] fn test_two_segment_file_transfer_not_acked_with_closure() { let mut tb = SourceHandlerTestbench::new(TransmissionMode::Unacknowledged, false, 128); - let mut cfdp_user = TestCfdpUser::default(); + let mut user = TestCfdpUser::default(); let mut file = OpenOptions::new() .write(true) .open(&tb.srcfile) @@ -1793,14 +1790,14 @@ mod tests { .expect("writing file content failed"); drop(file); let (transfer_info, fd_pdus) = - tb.generic_file_transfer(&mut cfdp_user, true, rand_data.to_vec()); + tb.generic_file_transfer(&mut user, true, rand_data.to_vec()); assert_eq!(fd_pdus, 2); - tb.finish_handling(&mut cfdp_user, &transfer_info) + tb.finish_handling(&mut user, &transfer_info) } #[test] fn test_two_segment_file_transfer_acked() { - let mut cfdp_user = TestCfdpUser::default(); + let mut user = TestCfdpUser::default(); let mut tb = SourceHandlerTestbench::new(TransmissionMode::Acknowledged, false, 128); let mut file = OpenOptions::new() .write(true) @@ -1812,10 +1809,10 @@ mod tests { .expect("writing file content failed"); drop(file); let (transfer_info, fd_pdus) = - tb.generic_file_transfer(&mut cfdp_user, true, rand_data.to_vec()); + tb.generic_file_transfer(&mut user, true, rand_data.to_vec()); assert_eq!(fd_pdus, 2); - tb.acknowledge_eof_pdu(&mut cfdp_user, &transfer_info); - tb.finish_handling(&mut cfdp_user, &transfer_info); + tb.acknowledge_eof_pdu(&mut user, &transfer_info); + tb.finish_handling(&mut user, &transfer_info); tb.common_finished_pdu_ack_check(); } @@ -1831,19 +1828,21 @@ mod tests { Some(true), ) .expect("creating put request failed"); - let mut cfdp_user = tb.create_user(0, file_size); - let transaction_info = tb.common_file_transfer_init_with_metadata_check( - &mut cfdp_user, - put_request, - file_size, - ); + let mut user = tb.create_user(0, file_size); + let transaction_info = + tb.common_file_transfer_init_with_metadata_check(&mut user, put_request, file_size); tb.common_eof_pdu_check( - &mut cfdp_user, + &mut user, transaction_info.closure_requested, EofParams::new_success(file_size, CRC_32.digest().finalize()), 1, ); - tb.finish_handling(&mut cfdp_user, &transaction_info) + tb.finish_handling(&mut user, &transaction_info); + user.verify_finished_indication_retained( + DeliveryCode::Complete, + ConditionCode::NoError, + transaction_info.id, + ); } #[test] @@ -1905,15 +1904,12 @@ mod tests { Some(true), ) .expect("creating put request failed"); - let mut cfdp_user = tb.create_user(0, file_size); - let transaction_info = tb.common_file_transfer_init_with_metadata_check( - &mut cfdp_user, - put_request, - file_size, - ); + let mut user = tb.create_user(0, file_size); + let transaction_info = + tb.common_file_transfer_init_with_metadata_check(&mut user, put_request, file_size); let expected_id = tb.handler.transaction_id().unwrap(); tb.common_eof_pdu_check( - &mut cfdp_user, + &mut user, transaction_info.closure_requested, EofParams::new_success(file_size, CRC_32.digest().finalize()), 1, @@ -1924,10 +1920,7 @@ mod tests { // cancellation -> leads to an EOF PDU with the appropriate error code. tb.expiry_control.set_check_limit_expired(); - assert_eq!( - tb.handler.state_machine_no_packet(&mut cfdp_user).unwrap(), - 1 - ); + assert_eq!(tb.handler.state_machine_no_packet(&mut user).unwrap(), 1); assert!(!tb.pdu_queue_empty()); let next_pdu = tb.get_next_sent_pdu().unwrap(); let eof_pdu = EofPdu::from_bytes(&next_pdu.raw_pdu).expect("invalid EOF PDU format"); @@ -1960,9 +1953,9 @@ mod tests { Some(false), ) .expect("creating put request failed"); - let mut cfdp_user = tb.create_user(0, filesize); - assert_eq!(cfdp_user.transaction_indication_call_count, 0); - assert_eq!(cfdp_user.eof_sent_call_count, 0); + let mut user = tb.create_user(0, filesize); + assert_eq!(user.transaction_indication_call_count, 0); + assert_eq!(user.eof_sent_call_count, 0); tb.put_request(&put_request) .expect("put_request call failed"); @@ -1971,7 +1964,7 @@ mod tests { assert!(tb.get_next_sent_pdu().is_none()); let id = tb.handler.transaction_id().unwrap(); tb.handler - .cancel_request(&mut cfdp_user, &id) + .cancel_request(&mut user, &id) .expect("transaction cancellation failed"); assert_eq!(tb.handler.state(), State::Idle); assert_eq!(tb.handler.step(), TransactionStep::Idle); @@ -2014,12 +2007,9 @@ mod tests { ) .expect("creating put request failed"); let file_size = rand_data.len() as u64; - let mut cfdp_user = tb.create_user(0, file_size); - let transaction_info = tb.common_file_transfer_init_with_metadata_check( - &mut cfdp_user, - put_request, - file_size, - ); + let mut user = tb.create_user(0, file_size); + let transaction_info = + tb.common_file_transfer_init_with_metadata_check(&mut user, put_request, file_size); let mut chunks = rand_data.chunks( calculate_max_file_seg_len_for_max_packet_len_and_pdu_header( &transaction_info.pdu_header, @@ -2038,7 +2028,7 @@ mod tests { let expected_id = tb.handler.transaction_id().unwrap(); assert!( tb.handler - .cancel_request(&mut cfdp_user, &expected_id) + .cancel_request(&mut user, &expected_id) .expect("cancellation failed") ); assert_eq!(tb.handler.state(), State::Idle); @@ -2077,18 +2067,10 @@ mod tests { Some(false), ) .expect("creating put request failed"); - let mut cfdp_user = tb.create_user(0, file_size); - let transfer_info = tb.common_file_transfer_init_with_metadata_check( - &mut cfdp_user, - put_request, - file_size, - ); - tb.common_eof_pdu_check( - &mut cfdp_user, - transfer_info.closure_requested, - eof_params, - 1, - ); + let mut user = tb.create_user(0, file_size); + let transfer_info = + tb.common_file_transfer_init_with_metadata_check(&mut user, put_request, file_size); + tb.common_eof_pdu_check(&mut user, transfer_info.closure_requested, eof_params, 1); assert!(tb.pdu_queue_empty()); @@ -2096,19 +2078,19 @@ mod tests { tb.expiry_control.set_positive_ack_expired(); let sent_packets = tb .handler - .state_machine_no_packet(&mut cfdp_user) + .state_machine_no_packet(&mut user) .expect("source handler FSM failure"); assert_eq!(sent_packets, 1); - tb.common_eof_pdu_check( - &mut cfdp_user, - transfer_info.closure_requested, - eof_params, - 2, - ); + tb.common_eof_pdu_check(&mut user, transfer_info.closure_requested, eof_params, 2); - tb.acknowledge_eof_pdu(&mut cfdp_user, &transfer_info); - tb.finish_handling(&mut cfdp_user, &transfer_info); + tb.acknowledge_eof_pdu(&mut user, &transfer_info); + tb.finish_handling(&mut user, &transfer_info); tb.common_finished_pdu_ack_check(); + user.verify_finished_indication_retained( + DeliveryCode::Complete, + ConditionCode::NoError, + transfer_info.id, + ); } #[test] @@ -2124,18 +2106,10 @@ mod tests { Some(false), ) .expect("creating put request failed"); - let mut cfdp_user = tb.create_user(0, file_size); - let transfer_info = tb.common_file_transfer_init_with_metadata_check( - &mut cfdp_user, - put_request, - file_size, - ); - tb.common_eof_pdu_check( - &mut cfdp_user, - transfer_info.closure_requested, - eof_params, - 1, - ); + let mut user = tb.create_user(0, file_size); + let transfer_info = + tb.common_file_transfer_init_with_metadata_check(&mut user, put_request, file_size); + tb.common_eof_pdu_check(&mut user, transfer_info.closure_requested, eof_params, 1); assert!(tb.pdu_queue_empty()); @@ -2143,34 +2117,29 @@ mod tests { tb.expiry_control.set_positive_ack_expired(); let sent_packets = tb .handler - .state_machine_no_packet(&mut cfdp_user) + .state_machine_no_packet(&mut user) .expect("source handler FSM failure"); assert_eq!(sent_packets, 1); - tb.common_eof_pdu_check( - &mut cfdp_user, - transfer_info.closure_requested, - eof_params, - 2, - ); + tb.common_eof_pdu_check(&mut user, transfer_info.closure_requested, eof_params, 2); // Enforce a postive ack timer expiry -> leads to a re-send of the EOF PDU. tb.expiry_control.set_positive_ack_expired(); let sent_packets = tb .handler - .state_machine_no_packet(&mut cfdp_user) + .state_machine_no_packet(&mut user) .expect("source handler FSM failure"); assert_eq!(sent_packets, 1); eof_params.condition_code = ConditionCode::PositiveAckLimitReached; - tb.common_eof_pdu_check( - &mut cfdp_user, - transfer_info.closure_requested, - eof_params, - 3, - ); + tb.common_eof_pdu_check(&mut user, transfer_info.closure_requested, eof_params, 3); // This boilerplate handling is still expected. In a real-life use-case I would expect // this to fail as well, leading to a transaction abandonment. This is tested separately. - tb.acknowledge_eof_pdu(&mut cfdp_user, &transfer_info); - tb.finish_handling(&mut cfdp_user, &transfer_info); + tb.acknowledge_eof_pdu(&mut user, &transfer_info); + tb.finish_handling(&mut user, &transfer_info); tb.common_finished_pdu_ack_check(); + user.verify_finished_indication_retained( + DeliveryCode::Complete, + ConditionCode::NoError, + transfer_info.id, + ); } #[test] @@ -2186,18 +2155,10 @@ mod tests { Some(false), ) .expect("creating put request failed"); - let mut cfdp_user = tb.create_user(0, file_size); - let transfer_info = tb.common_file_transfer_init_with_metadata_check( - &mut cfdp_user, - put_request, - file_size, - ); - tb.common_eof_pdu_check( - &mut cfdp_user, - transfer_info.closure_requested, - eof_params, - 1, - ); + let mut user = tb.create_user(0, file_size); + let transfer_info = + tb.common_file_transfer_init_with_metadata_check(&mut user, put_request, file_size); + tb.common_eof_pdu_check(&mut user, transfer_info.closure_requested, eof_params, 1); assert!(tb.pdu_queue_empty()); @@ -2205,29 +2166,19 @@ mod tests { tb.expiry_control.set_positive_ack_expired(); let sent_packets = tb .handler - .state_machine_no_packet(&mut cfdp_user) + .state_machine_no_packet(&mut user) .expect("source handler FSM failure"); assert_eq!(sent_packets, 1); - tb.common_eof_pdu_check( - &mut cfdp_user, - transfer_info.closure_requested, - eof_params, - 2, - ); + tb.common_eof_pdu_check(&mut user, transfer_info.closure_requested, eof_params, 2); // Enforce a postive ack timer expiry -> positive ACK limit reached -> Cancel EOF sent. tb.expiry_control.set_positive_ack_expired(); let sent_packets = tb .handler - .state_machine_no_packet(&mut cfdp_user) + .state_machine_no_packet(&mut user) .expect("source handler FSM failure"); assert_eq!(sent_packets, 1); eof_params.condition_code = ConditionCode::PositiveAckLimitReached; - tb.common_eof_pdu_check( - &mut cfdp_user, - transfer_info.closure_requested, - eof_params, - 3, - ); + tb.common_eof_pdu_check(&mut user, transfer_info.closure_requested, eof_params, 3); // Cancellation fault should have been triggered. let fault_handler = tb.test_fault_handler_mut(); let fh_ref_mut = fault_handler.get_mut(); @@ -2243,22 +2194,17 @@ mod tests { tb.expiry_control.set_positive_ack_expired(); let sent_packets = tb .handler - .state_machine_no_packet(&mut cfdp_user) + .state_machine_no_packet(&mut user) .expect("source handler FSM failure"); assert_eq!(sent_packets, 1); - tb.common_eof_pdu_check( - &mut cfdp_user, - transfer_info.closure_requested, - eof_params, - 4, - ); + tb.common_eof_pdu_check(&mut user, transfer_info.closure_requested, eof_params, 4); // Enforce a postive ack timer expiry -> positive ACK limit reached -> Transaction // abandoned tb.expiry_control.set_positive_ack_expired(); let sent_packets = tb .handler - .state_machine_no_packet(&mut cfdp_user) + .state_machine_no_packet(&mut user) .expect("source handler FSM failure"); assert_eq!(sent_packets, 0); // Abandonment fault should have been triggered. @@ -2276,21 +2222,21 @@ mod tests { #[test] fn test_nak_for_whole_file() { let mut tb = SourceHandlerTestbench::new(TransmissionMode::Acknowledged, false, 512); - let mut cfdp_user = TestCfdpUser::default(); - let (data, transfer_info) = tb.common_tiny_file_transfer(&mut cfdp_user, true); + let mut user = TestCfdpUser::default(); + let (data, transfer_info) = tb.common_tiny_file_transfer(&mut user, true); let seg_reqs = &[(0, transfer_info.file_size as u32)]; - tb.nak_for_file_segments(&mut cfdp_user, &transfer_info, seg_reqs); + tb.nak_for_file_segments(&mut user, &transfer_info, seg_reqs); tb.check_next_file_pdu(0, data.as_bytes()); tb.all_fault_queues_empty(); - tb.acknowledge_eof_pdu(&mut cfdp_user, &transfer_info); - tb.finish_handling(&mut cfdp_user, &transfer_info); + tb.acknowledge_eof_pdu(&mut user, &transfer_info); + tb.finish_handling(&mut user, &transfer_info); tb.common_finished_pdu_ack_check(); } #[test] fn test_nak_for_file_segment() { - let mut cfdp_user = TestCfdpUser::default(); + let mut user = TestCfdpUser::default(); let mut tb = SourceHandlerTestbench::new(TransmissionMode::Acknowledged, false, 128); let mut file = OpenOptions::new() .write(true) @@ -2302,14 +2248,14 @@ mod tests { .expect("writing file content failed"); drop(file); let (transfer_info, fd_pdus) = - tb.generic_file_transfer(&mut cfdp_user, false, rand_data.to_vec()); + tb.generic_file_transfer(&mut user, false, rand_data.to_vec()); assert_eq!(fd_pdus, 2); - tb.nak_for_file_segments(&mut cfdp_user, &transfer_info, &[(0, 90)]); + tb.nak_for_file_segments(&mut user, &transfer_info, &[(0, 90)]); tb.check_next_file_pdu(0, &rand_data[0..90]); tb.all_fault_queues_empty(); - tb.acknowledge_eof_pdu(&mut cfdp_user, &transfer_info); - tb.finish_handling(&mut cfdp_user, &transfer_info); + tb.acknowledge_eof_pdu(&mut user, &transfer_info); + tb.finish_handling(&mut user, &transfer_info); tb.common_finished_pdu_ack_check(); } @@ -2325,21 +2271,18 @@ mod tests { Some(false), ) .expect("creating put request failed"); - let mut cfdp_user = tb.create_user(0, file_size); - let transfer_info = tb.common_file_transfer_init_with_metadata_check( - &mut cfdp_user, - put_request, - file_size, - ); + let mut user = tb.create_user(0, file_size); + let transfer_info = + tb.common_file_transfer_init_with_metadata_check(&mut user, put_request, file_size); tb.common_eof_pdu_check( - &mut cfdp_user, + &mut user, transfer_info.closure_requested, EofParams::new_success(file_size, CRC_32.digest().finalize()), 1, ); // NAK to cause re-transmission of metadata PDU. - let nak_pdu = NakPduCreator::new( + let nak_pdu = NakPduCreator::new_normal_file_size( transfer_info.pdu_header, 0, transfer_info.file_size as u32, @@ -2350,7 +2293,7 @@ mod tests { let packet_info = PduRawWithInfo::new(&nak_pdu_vec).unwrap(); let sent_packets = tb .handler - .state_machine(&mut cfdp_user, Some(&packet_info)) + .state_machine(&mut user, Some(&packet_info)) .unwrap(); assert_eq!(sent_packets, 1); let next_pdu = tb.get_next_sent_pdu().unwrap(); @@ -2358,8 +2301,13 @@ mod tests { tb.metadata_check(&next_pdu, file_size); tb.all_fault_queues_empty(); - tb.acknowledge_eof_pdu(&mut cfdp_user, &transfer_info); - tb.finish_handling(&mut cfdp_user, &transfer_info); + tb.acknowledge_eof_pdu(&mut user, &transfer_info); + tb.finish_handling(&mut user, &transfer_info); tb.common_finished_pdu_ack_check(); + user.verify_finished_indication_retained( + DeliveryCode::Complete, + ConditionCode::NoError, + transfer_info.id, + ); } } diff --git a/src/time.rs b/src/time.rs index 23d69d2..8a8f433 100644 --- a/src/time.rs +++ b/src/time.rs @@ -1,7 +1,7 @@ use core::fmt::Debug; /// Generic abstraction for a check/countdown timer. Should also be cheap to copy and clone. -pub trait CountdownProvider: Debug { +pub trait Countdown: Debug { fn has_expired(&self) -> bool; fn reset(&mut self); } diff --git a/tests/end-to-end.rs b/tests/end-to-end.rs index 2f7ba87..b94d51e 100644 --- a/tests/end-to-end.rs +++ b/tests/end-to-end.rs @@ -13,9 +13,10 @@ use std::{ use cfdp::{ EntityType, IndicationConfig, LocalEntityConfig, PduOwnedWithInfo, RemoteEntityConfig, - StdTimerCreator, TransactionId, UserFaultHookProvider, + StdTimerCreator, TransactionId, UserFaultHook, dest::DestinationHandler, filestore::NativeFilestore, + lost_segments::LostSegmentsList, request::{PutRequestOwned, StaticPutRequestCacher}, source::SourceHandler, user::{CfdpUser, FileSegmentRecvdParams, MetadataReceivedParams, TransactionFinishedParams}, @@ -33,7 +34,7 @@ const FILE_DATA: &str = "Hello World!"; #[derive(Default)] pub struct ExampleFaultHandler {} -impl UserFaultHookProvider for ExampleFaultHandler { +impl UserFaultHook for ExampleFaultHandler { fn notice_of_suspension_cb( &mut self, transaction_id: TransactionId, @@ -230,6 +231,7 @@ fn end_to_end_test(with_closure: bool) { NativeFilestore::default(), remote_cfg_of_source, StdTimerCreator::default(), + LostSegmentsList::default(), ); let mut cfdp_user_dest = ExampleCfdpUser::new(EntityType::Receiving, completion_signal_dest);