diff --git a/examples/cfdp-libre-cube-crosstest/tmtccmd-client.py b/examples/cfdp-libre-cube-crosstest/tmtccmd-client.py index c1729a2..a29087c 100755 --- a/examples/cfdp-libre-cube-crosstest/tmtccmd-client.py +++ b/examples/cfdp-libre-cube-crosstest/tmtccmd-client.py @@ -14,7 +14,7 @@ from logging import basicConfig from pathlib import Path from queue import Empty -from typing import Any, cast +from typing import TYPE_CHECKING, Any, cast from common import REMOTE_CFG_FOR_DEST_ENTITY, UDP_SERVER_PORT, UDP_TM_SERVER_PORT from common import REMOTE_ENTITY_ID as REMOTE_ENTITY_ID_RAW @@ -24,7 +24,6 @@ TransactionId, TransmissionMode, ) -from spacepackets.cfdp.pdu import AbstractFileDirectiveBase from spacepackets.cfdp.pdu.helper import PduFactory from spacepackets.countdown import Countdown from spacepackets.seqcount import SeqCountProvider @@ -50,6 +49,9 @@ TransactionParams, ) +if TYPE_CHECKING: + from spacepackets.cfdp.pdu import AbstractFileDirectiveBase + SOURCE_ENTITY_ID = ByteFieldU16(SOURCE_ENTITY_ID_RAW) DEST_ENTITY_ID = ByteFieldU16(REMOTE_ENTITY_ID_RAW) @@ -292,7 +294,7 @@ def source_entity_handler( ready = select.select([tm_client], [], [], 0) if ready[0]: data, _ = tm_client.recvfrom(4096) - packet = cast(AbstractFileDirectiveBase, PduFactory.from_raw(data)) + packet = cast("AbstractFileDirectiveBase", PduFactory.from_raw(data)) packet_received = True except Empty: pass diff --git a/justfile b/justfile new file mode 100644 index 0000000..77a6e2d --- /dev/null +++ b/justfile @@ -0,0 +1,11 @@ +# Run with `just all` +all: fmt check test + +fmt: + ruff format + +check: + ruff check + +test: + pytest diff --git a/src/cfdppy/handler/dest.py b/src/cfdppy/handler/dest.py index 9b6f4b7..776cdab 100644 --- a/src/cfdppy/handler/dest.py +++ b/src/cfdppy/handler/dest.py @@ -81,7 +81,6 @@ class CompletionDisposition(enum.Enum): @dataclass class _DestFileParams(_FileParamsBase): file_name: Path - file_size_eof: int | None @classmethod def empty(cls) -> _DestFileParams: @@ -91,38 +90,41 @@ def empty(cls) -> _DestFileParams: crc32=b"", file_size=None, file_name=Path(), - file_size_eof=None, + # file_size_eof=None, metadata_only=False, ) def reset(self) -> None: super().reset() self.file_name = Path() - self.file_size_eof = None class TransactionStep(enum.Enum): IDLE = 0 TRANSACTION_START = 1 """Metadata was received, which triggered a transaction start.""" + WAITING_FOR_METADATA = 2 """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.""" + RECEIVING_FILE_DATA = 3 + RECV_FILE_DATA_WITH_CHECK_LIMIT_HANDLING = 4 """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.""" - SENDING_EOF_ACK_PDU = 5 - """Sending the ACK (EOF) packet.""" + WAITING_FOR_MISSING_DATA = 6 """Only relevant for acknowledged mode: Wait for lost metadata and file segments as part of the deferred lost segments detection procedure.""" + TRANSFER_COMPLETION = 7 """File transfer complete. Perform checksum verification and notice of completion. Please note that this does not necessarily mean that the file transfer was completed successfully.""" + SENDING_FINISHED_PDU = 8 WAITING_FOR_FINISHED_ACK = 9 @@ -162,6 +164,7 @@ def coalesce_lost_segments(self) -> None: if len(self.lost_segments) <= 1: return merged_segments = [] + # Initialize to the first entry. current_start, current_end = next(iter(self.lost_segments.items())) for seg_start, seg_end in self.lost_segments.items(): @@ -220,6 +223,8 @@ def remove_lost_segment(self, segment_to_remove: tuple[int, int]) -> bool: @dataclass class _AckedModeParams: lost_seg_tracker: LostSegmentTracker = field(default=LostSegmentTracker()) + # Extra parameter: Missing metadata is not tracked inside the lost segment tracker, so we + # need an extra parameter for this. metadata_missing: bool = False last_start_offset: int = 0 last_end_offset: int = 0 @@ -520,11 +525,11 @@ def __idle_fsm(self, packet: GenericPduPacket | None) -> None: self._start_transaction(metadata_pdu) else: raise ValueError( - f"unexpected configuration error: {pdu_holder.pdu} in " f"IDLE state machine" + f"unexpected configuration error: {pdu_holder.pdu} in IDLE state machine" ) def __non_idle_fsm(self, packet: GenericPduPacket | None) -> None: - self._fsm_advancement_after_packets_were_sent() + self._assert_all_packets_were_sent() pdu_holder = PduHolder(packet) if ( self.states.step @@ -554,20 +559,10 @@ def __non_idle_fsm(self, packet: GenericPduPacket | None) -> None: if self.states.step == TransactionStep.WAITING_FOR_FINISHED_ACK: self._handle_waiting_for_finished_ack(pdu_holder) - def _fsm_advancement_after_packets_were_sent(self) -> None: + def _assert_all_packets_were_sent(self) -> None: """Advance the internal FSM after all packets to be sent were retrieved from the handler.""" if len(self._pdus_to_be_sent) > 0: raise UnretrievedPdusToBeSent(f"{len(self._pdus_to_be_sent)} packets left to send") - if self.states.step == TransactionStep.SENDING_EOF_ACK_PDU: - if ( - self._params.acked_params.lost_seg_tracker.num_lost_segments > 0 - or self._params.acked_params.metadata_missing - ): - self._start_deferred_lost_segment_handling() - else: - if self._params.completion_disposition != CompletionDisposition.CANCELED: - self._checksum_verify() - self.states.step = TransactionStep.TRANSFER_COMPLETION def _start_transaction(self, metadata_pdu: MetadataPdu) -> bool: if self.states.state != CfdpState.IDLE: @@ -595,7 +590,7 @@ def _start_transaction_missing_metadata_recv_eof(self, eof_pdu: EofPdu) -> None: def _handle_eof_without_previous_metadata(self, eof_pdu: EofPdu) -> None: self._params.fp.progress = eof_pdu.file_size - self._params.fp.file_size_eof = eof_pdu.file_size + self._params.fp.file_size = eof_pdu.file_size self._params.acked_params.metadata_missing = True if self._params.fp.progress > 0: # Clear old list, deferred procedure for the whole file is now active. @@ -609,11 +604,23 @@ def _handle_eof_without_previous_metadata(self, eof_pdu: EofPdu) -> None: assert self._params.transaction_id is not None self.user.eof_recv_indication(self._params.transaction_id) self._prepare_eof_ack_packet() - self.states.step = TransactionStep.SENDING_EOF_ACK_PDU + self._eof_ack_pdu_done() + + def _eof_ack_pdu_done(self) -> None: + if ( + self._params.acked_params.lost_seg_tracker.num_lost_segments > 0 + or self._params.acked_params.metadata_missing + ): + self._start_deferred_lost_segment_handling() + + else: + if self._params.completion_disposition != CompletionDisposition.CANCELED: + self._checksum_verify() + self.states.step = TransactionStep.TRANSFER_COMPLETION def _start_transaction_missing_metadata_recv_fd(self, fd_pdu: FileDataPdu) -> None: self._common_first_packet_not_metadata_pdu_handler(fd_pdu) - self._handle_fd_without_previous_metadata(True, fd_pdu) + self._handle_fd_without_previous_metadata(fd_pdu) def _handle_finished_pdu_sent(self) -> None: if ( @@ -625,19 +632,16 @@ def _handle_finished_pdu_sent(self) -> None: return self._reset_internal(False) - def _handle_fd_without_previous_metadata(self, first_pdu: bool, fd_pdu: FileDataPdu) -> None: + def _handle_fd_without_previous_metadata(self, fd_pdu: FileDataPdu) -> None: self._params.fp.progress = fd_pdu.offset + len(fd_pdu.file_data) if len(fd_pdu.file_data) > 0: - start = fd_pdu.offset - if first_pdu: - start = 0 # I will just wait until the metadata has been received with re-requesting the file # data PDU. How does the standard expect me to process file data PDUs where I do not # even know the filenames? How would I even generically do this? # I will add this file segment (and all others which came before and might be missing # as well) to the lost segment list. self._params.acked_params.lost_seg_tracker.add_lost_segment( - (start, self._params.fp.progress) + (0, self._params.fp.progress) ) # This is a bit tricky: We need to set those variables to an appropriate value so # the removal of handled lost segments works properly. However, we can not set the @@ -649,8 +653,7 @@ def _handle_fd_without_previous_metadata(self, first_pdu: bool, fd_pdu: FileData # Re-request the metadata PDU. if self._params.remote_cfg.immediate_nak_mode: lost_segments: list[tuple[int, int]] = [] - if first_pdu: - lost_segments.append((0, 0)) + lost_segments.append((0, 0)) if len(fd_pdu.file_data) > 0: lost_segments.append((0, self._params.fp.progress)) if len(lost_segments) > 0: @@ -698,7 +701,7 @@ def _handle_metadata_packet(self, metadata_pdu: MetadataPdu) -> None: # sender. if self._params.remote_cfg is None: _LOGGER.warning( - "No remote configuration found for remote ID" f" {metadata_pdu.dest_entity_id}" + f"No remote configuration found for remote ID {metadata_pdu.dest_entity_id}" ) raise NoRemoteEntityCfgFound(metadata_pdu.dest_entity_id) if not self._params.fp.metadata_only: @@ -753,7 +756,7 @@ def _handle_waiting_for_missing_metadata(self, packet_holder: PduHolder) -> None if packet_holder.pdu is None: return if packet_holder.pdu_type == PduType.FILE_DATA: - self._handle_fd_without_previous_metadata(True, packet_holder.to_file_data_pdu()) + self._handle_fd_without_previous_metadata(packet_holder.to_file_data_pdu()) elif packet_holder.pdu_directive_type == DirectiveType.METADATA_PDU: self._handle_metadata_packet(packet_holder.to_metadata_pdu()) if self._params.acked_params.deferred_lost_segment_detection_active: @@ -832,8 +835,8 @@ def _handle_fd_pdu(self, file_data_pdu: FileDataPdu) -> None: self._params.finished_params.file_status = FileStatus.FILE_RETAINED if ( - self._params.fp.file_size_eof is not None - and (offset + len(file_data_pdu.file_data) > self._params.fp.file_size_eof) + self._params.fp.file_size is not None + and (offset + len(file_data_pdu.file_data) > self._params.fp.file_size) and ( self._declare_fault(ConditionCode.FILE_SIZE_ERROR) != FaultHandlerCode.IGNORE_ERROR @@ -895,7 +898,7 @@ def _deferred_lost_segment_handling(self) -> None: if not self._params.acked_params.deferred_lost_segment_detection_active: return assert self._params.remote_cfg is not None - assert self._params.fp.file_size_eof is not None + assert self._params.fp.file_size is not None if ( self._params.acked_params.lost_seg_tracker.num_lost_segments == 0 and not self._params.acked_params.metadata_missing @@ -941,7 +944,7 @@ def _deferred_lost_segment_handling(self) -> None: NakPdu( self._params.pdu_conf, 0, - self._params.fp.file_size_eof, + self._params.fp.file_size, next_segment_reqs, ) ) @@ -951,7 +954,7 @@ def _deferred_lost_segment_handling(self) -> None: NakPdu( self._params.pdu_conf, 0, - self._params.fp.file_size_eof, + self._params.fp.file_size, next_segment_reqs, ) ) @@ -962,7 +965,7 @@ def _deferred_lost_segment_handling(self) -> None: def _handle_eof_pdu(self, eof_pdu: EofPdu) -> bool | None: """Returns whether to exit the FSM prematurely.""" self._params.fp.crc32 = eof_pdu.file_checksum - self._params.fp.file_size_eof = eof_pdu.file_size + self._params.fp.file_size = eof_pdu.file_size if self.cfg.indication_cfg.eof_recv_indication_required: assert self._params.transaction_id is not None self.user.eof_recv_indication(self._params.transaction_id) @@ -978,7 +981,7 @@ def _handle_eof_pdu(self, eof_pdu: EofPdu) -> bool | None: EntityIdTlv(self._params.remote_cfg.entity_id.as_bytes), ) # Store this as progress for the checksum calculation. - self._params.fp.progress = self._params.fp.file_size_eof + self._params.fp.progress = self._params.fp.file_size self._params.finished_params.delivery_code = DeliveryCode.DATA_INCOMPLETE self._file_transfer_complete_transition() return False @@ -986,22 +989,18 @@ def _handle_eof_pdu(self, eof_pdu: EofPdu) -> bool | None: def _handle_no_error_eof(self) -> bool: """Returns whether the transfer can be completed regularly.""" # CFDP 4.6.1.2.9: Declare file size error if progress exceeds file size - if self._params.fp.progress > self._params.fp.file_size_eof: # type: ignore + if self._params.fp.progress > self._params.fp.file_size: # type: ignore if self._declare_fault(ConditionCode.FILE_SIZE_ERROR) != FaultHandlerCode.IGNORE_ERROR: return False elif ( - self._params.fp.progress < self._params.fp.file_size_eof # type: ignore + self._params.fp.progress < self._params.fp.file_size # type: ignore ) and self.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 # the lost segments for the deferred lost segment detection procedure. self._params.acked_params.lost_seg_tracker.add_lost_segment( - (self._params.fp.progress, self._params.fp.file_size_eof) # type: ignore + (self._params.fp.progress, self._params.fp.file_size) # type: ignore ) - if self._params.fp.file_size_eof != self._params.fp.file_size: - # Can or should this ever happen for a No Error EOF? Treat this like a non-fatal - # error for now. - _LOGGER.warning("missmatch of EOF file size and Metadata File Size for success EOF") if ( self.transmission_mode == TransmissionMode.UNACKNOWLEDGED and not self._checksum_verify() @@ -1022,8 +1021,8 @@ def _start_deferred_lost_segment_handling(self) -> None: self.states.step = TransactionStep.WAITING_FOR_MISSING_DATA self._params.acked_params.deferred_lost_segment_detection_active = True self._params.acked_params.lost_seg_tracker.coalesce_lost_segments() - self._params.acked_params.last_start_offset = self._params.fp.file_size_eof # type: ignore - self._params.acked_params.last_end_offset = self._params.fp.file_size_eof # type: ignore + self._params.acked_params.last_start_offset = self._params.fp.file_size # type: ignore + self._params.acked_params.last_end_offset = self._params.fp.file_size # type: ignore self._deferred_lost_segment_handling() def _prepare_eof_ack_packet(self) -> None: @@ -1062,7 +1061,7 @@ def _file_transfer_complete_transition(self) -> None: self.states.step = TransactionStep.TRANSFER_COMPLETION elif self.transmission_mode == TransmissionMode.ACKNOWLEDGED: self._prepare_eof_ack_packet() - self.states.step = TransactionStep.SENDING_EOF_ACK_PDU + self._eof_ack_pdu_done() def _trigger_notice_of_completion_canceled( self, condition_code: ConditionCode, fault_location: EntityIdTlv @@ -1102,8 +1101,6 @@ def _notice_of_completion(self) -> None: self.user.transaction_finished_indication(finished_indic_params) def _prepare_finished_pdu(self) -> None: - if self.states.packets_ready: - raise UnretrievedPdusToBeSent # TODO: Fault location handling. Set remote entity ID for file copy # operations cancelled with an EOF (Cancel) PDU, and the local ID for file # copy operations cancelled with the local API. diff --git a/src/cfdppy/handler/source.py b/src/cfdppy/handler/source.py index 6a66366..4a320bd 100644 --- a/src/cfdppy/handler/source.py +++ b/src/cfdppy/handler/source.py @@ -80,11 +80,14 @@ class TransactionStep(enum.Enum): IDLE = 0 TRANSACTION_START = 1 + # The following three are used for the Copy File Procedure SENDING_METADATA = 3 SENDING_FILE_DATA = 4 + RETRANSMITTING = 5 """Re-transmitting missing packets in acknowledged mode.""" + SENDING_EOF = 6 WAITING_FOR_EOF_ACK = 7 WAITING_FOR_FINISHED = 8 @@ -886,12 +889,9 @@ def _prepare_progressing_file_data_pdu(self) -> None: :return: True if a packet was prepared, False if PDU handling is done and the next steps in the Copy File procedure can be performed """ - if self._params.fp.file_size < self._params.fp.segment_len: - read_len = self._params.fp.file_size - elif self._params.fp.progress + self._params.fp.segment_len > self._params.fp.file_size: - read_len = self._params.fp.file_size - self._params.fp.progress - else: - read_len = self._params.fp.segment_len + read_len = min( + self._params.fp.segment_len, self._params.fp.file_size - self._params.fp.progress + ) self._prepare_file_data_pdu(self._params.fp.progress, read_len) self._params.fp.progress += read_len @@ -930,7 +930,7 @@ def _get_next_transfer_seq_num(self) -> None: next_seq_num = self.seq_num_provider.get_and_increment() if self.seq_num_provider.max_bit_width not in [8, 16, 32]: raise ValueError( - "Invalid bit width for sequence number provider, must be one of [8," " 16, 32]" + "Invalid bit width for sequence number provider, must be one of [8, 16, 32]" ) self._params.pdu_conf.transaction_seq_num = ByteFieldGenerator.from_int( self.seq_num_provider.max_bit_width // 8, next_seq_num diff --git a/src/cfdppy/request.py b/src/cfdppy/request.py index e3ab949..e313b07 100644 --- a/src/cfdppy/request.py +++ b/src/cfdppy/request.py @@ -75,7 +75,7 @@ def __str_for_metadata_only(self) -> str: print_str = f"Metadata Only Put Request with Destination ID {self.destination_id.value}" if self.msgs_to_user is not None: for idx, msg_to_user in enumerate(self.msgs_to_user): - msg_to_user = cast(MessageToUserTlv, msg_to_user) + msg_to_user = cast("MessageToUserTlv", msg_to_user) if msg_to_user.is_reserved_cfdp_message(): reserved_msg = msg_to_user.to_reserved_msg_tlv() assert reserved_msg is not None diff --git a/tests/test_dest_handler.py b/tests/test_dest_handler.py index fcd77b8..3d3f100 100644 --- a/tests/test_dest_handler.py +++ b/tests/test_dest_handler.py @@ -171,6 +171,7 @@ def _generic_transfer_init( expected_init_packets: int, expected_init_state: CfdpState, expected_init_step: TransactionStep, + expected_file_size: None | int = None, expected_originating_id: TransactionId | None = None, ) -> FsmResult: checksum_type = ChecksumType.NULL_CHECKSUM @@ -185,7 +186,7 @@ def _generic_transfer_init( ) file_transfer_init = MetadataPdu(params=metadata_params, pdu_conf=self.src_pdu_conf) self._state_checker(None, expected_init_packets, expected_init_state, expected_init_step) - self.assertEqual(self.dest_handler.file_size, None) + self.assertEqual(self.dest_handler.file_size, expected_file_size) fsm_res = self.dest_handler.state_machine(file_transfer_init) self.assertEqual(self.dest_handler.file_size, file_size) return fsm_res @@ -210,7 +211,7 @@ def _insert_file_segment( self.cfdp_user.file_segment_recv_indication.assert_called_once() self.assertEqual(self.cfdp_user.file_segment_recv_indication.call_count, 1) seg_recv_params = cast( - FileSegmentRecvdParams, + "FileSegmentRecvdParams", self.cfdp_user.file_segment_recv_indication.call_args.args[0], ) self.assertEqual(seg_recv_params.transaction_id, self.transaction_id) @@ -358,7 +359,7 @@ def _generic_transfer_finished_indication_check( ): self.cfdp_user.transaction_finished_indication.assert_called_once() finished_params_from_callback = cast( - TransactionFinishedParams, + "TransactionFinishedParams", self.cfdp_user.transaction_finished_indication.call_args.args[0], ) self.assertEqual(finished_params_from_callback.transaction_id, self.transaction_id) diff --git a/tests/test_dest_handler_acked.py b/tests/test_dest_handler_acked.py index bd8b54f..f96eda0 100644 --- a/tests/test_dest_handler_acked.py +++ b/tests/test_dest_handler_acked.py @@ -36,8 +36,7 @@ def test_acked_empty_transfer(self): self._generic_regular_transfer_init(0) fsm_res = self._generic_insert_eof_pdu(0, NULL_CHECKSUM_U32) self._generic_eof_recv_indication_check(fsm_res) - self._generic_verify_eof_ack_packet(fsm_res) - fsm_res = self.dest_handler.state_machine() + self._generic_verify_eof_ack_packet(fsm_res, TransactionStep.WAITING_FOR_FINISHED_ACK) finished_pdu = self._generic_no_error_finished_pdu_check_acked(fsm_res) self._generic_verify_transfer_completion(fsm_res, b"") self._generic_insert_finished_pdu_ack(finished_pdu) @@ -53,8 +52,7 @@ def test_acked_small_file_transfer(self): self._insert_file_segment(file_content, 0) fsm_res = self._generic_insert_eof_pdu(len(file_content), crc32) self._generic_eof_recv_indication_check(fsm_res) - self._generic_verify_eof_ack_packet(fsm_res) - fsm_res = self.dest_handler.state_machine() + self._generic_verify_eof_ack_packet(fsm_res, TransactionStep.WAITING_FOR_FINISHED_ACK) finished_pdu = self._generic_no_error_finished_pdu_check_acked(fsm_res) self._generic_verify_transfer_completion(fsm_res, file_content) self._generic_insert_finished_pdu_ack(finished_pdu) @@ -75,9 +73,11 @@ def test_cancelled_file_transfer(self): fsm_res = self.dest_handler.state_machine(eof_pdu) # Should contain an ACK PDU now. self._generic_verify_eof_ack_packet( - fsm_res, condition_code_of_acked_pdu=ConditionCode.CANCEL_REQUEST_RECEIVED + fsm_res, + TransactionStep.WAITING_FOR_FINISHED_ACK, + condition_code_of_acked_pdu=ConditionCode.CANCEL_REQUEST_RECEIVED, ) - fsm_res = self.dest_handler.state_machine() + # fsm_res = self.dest_handler.state_machine() finished_pdu = self._generic_finished_pdu_check_acked( fsm_res, expected_condition_code=ConditionCode.CANCEL_REQUEST_RECEIVED, @@ -105,8 +105,7 @@ def test_deferred_missing_file_segment_handling(self): self._insert_file_segment(file_content[0:5], 0) fsm_res = self._generic_insert_eof_pdu(len(file_content), crc32) self._generic_eof_recv_indication_check(fsm_res) - self._generic_verify_eof_ack_packet(fsm_res) - self.dest_handler.state_machine() + self._generic_verify_eof_ack_packet(fsm_res, TransactionStep.WAITING_FOR_MISSING_DATA) self._state_checker(fsm_res, 1, CfdpState.BUSY, TransactionStep.WAITING_FOR_MISSING_DATA) self.assertTrue(self.dest_handler.deferred_lost_segment_procedure_active) self.assertEqual(self.dest_handler.nak_activity_counter, 0) @@ -140,8 +139,7 @@ def test_immediate_missing_file_seg_handling_0(self): # All lost segments were delivered, regular transfer finish. fsm_res = self._generic_insert_eof_pdu(len(file_content), crc32) self._generic_eof_recv_indication_check(fsm_res) - self._generic_verify_eof_ack_packet(fsm_res) - fsm_res = self.dest_handler.state_machine() + self._generic_verify_eof_ack_packet(fsm_res, TransactionStep.WAITING_FOR_FINISHED_ACK) finished_pdu = self._generic_no_error_finished_pdu_check_acked(fsm_res) self._generic_verify_transfer_completion(fsm_res, file_content) self._generic_insert_finished_pdu_ack(finished_pdu) @@ -164,8 +162,7 @@ def test_immediate_missing_file_seg_handling_1(self): # All lost segments were delivered, regular transfer finish. fsm_res = self._generic_insert_eof_pdu(len(file_content), crc32) self._generic_eof_recv_indication_check(fsm_res) - self._generic_verify_eof_ack_packet(fsm_res) - fsm_res = self.dest_handler.state_machine() + self._generic_verify_eof_ack_packet(fsm_res, TransactionStep.WAITING_FOR_FINISHED_ACK) finished_pdu = self._generic_no_error_finished_pdu_check_acked(fsm_res) self._generic_verify_transfer_completion(fsm_res, file_content) self._generic_insert_finished_pdu_ack(finished_pdu) @@ -192,8 +189,7 @@ def test_immediate_multi_missing_segment_handling(self): # All lost segments were delivered, regular transfer finish. fsm_res = self._generic_insert_eof_pdu(len(file_content), crc32) self._generic_eof_recv_indication_check(fsm_res) - self._generic_verify_eof_ack_packet(fsm_res) - fsm_res = self.dest_handler.state_machine() + self._generic_verify_eof_ack_packet(fsm_res, TransactionStep.WAITING_FOR_FINISHED_ACK) finished_pdu = self._generic_no_error_finished_pdu_check_acked(fsm_res) self._generic_verify_transfer_completion(fsm_res, file_content) self._generic_insert_finished_pdu_ack(finished_pdu) @@ -214,9 +210,7 @@ def test_immediate_missing_segment_also_rerequested_after_eof(self): # All lost segments were delivered, regular transfer finish. fsm_res = self._generic_insert_eof_pdu(len(file_content), crc32) self._generic_eof_recv_indication_check(fsm_res) - self._generic_verify_eof_ack_packet(fsm_res) - - self.dest_handler.state_machine() + self._generic_verify_eof_ack_packet(fsm_res, TransactionStep.WAITING_FOR_MISSING_DATA) self._state_checker(fsm_res, 1, CfdpState.BUSY, TransactionStep.WAITING_FOR_MISSING_DATA) self.assertTrue(self.dest_handler.deferred_lost_segment_procedure_active) self.assertEqual(self.dest_handler.nak_activity_counter, 0) @@ -250,9 +244,7 @@ def test_multi_segment_missing_deferred_handling(self): fsm_res = self._generic_insert_eof_pdu(len(file_content), crc32) self._generic_eof_recv_indication_check(fsm_res) - self._generic_verify_eof_ack_packet(fsm_res) - - self.dest_handler.state_machine() + self._generic_verify_eof_ack_packet(fsm_res, TransactionStep.WAITING_FOR_MISSING_DATA) self._state_checker(fsm_res, 1, CfdpState.BUSY, TransactionStep.WAITING_FOR_MISSING_DATA) self.assertTrue(self.dest_handler.deferred_lost_segment_procedure_active) self.assertEqual(self.dest_handler.nak_activity_counter, 0) @@ -318,8 +310,7 @@ def test_missing_metadata_pdu(self): # All lost segments were delivered, regular transfer finish. fsm_res = self._generic_insert_eof_pdu(len(file_content), crc32) self._generic_eof_recv_indication_check(fsm_res) - self._generic_verify_eof_ack_packet(fsm_res) - fsm_res = self.dest_handler.state_machine() + self._generic_verify_eof_ack_packet(fsm_res, TransactionStep.WAITING_FOR_FINISHED_ACK) finished_pdu = self._generic_no_error_finished_pdu_check_acked(fsm_res) self._generic_verify_transfer_completion(fsm_res, file_content) self._generic_insert_finished_pdu_ack(finished_pdu) @@ -327,15 +318,16 @@ def test_missing_metadata_pdu(self): def test_metadata_eof_only_missing_metadata(self): fsm_res = self._generic_insert_eof_pdu(0, NULL_CHECKSUM_U32) self._generic_eof_recv_indication_check(fsm_res) - self._generic_verify_eof_ack_packet(fsm_res) - self.dest_handler.state_machine() + self._generic_verify_eof_ack_packet(fsm_res, TransactionStep.WAITING_FOR_METADATA) self._state_checker(fsm_res, 1, CfdpState.BUSY, TransactionStep.WAITING_FOR_METADATA) self._generic_verify_missing_segment_requested(0, 0, [(0, 0)]) + # The EOF has a file size of 0, so the file size parameter is already set. fsm_res = self._generic_transfer_init( 0, expected_init_packets=0, expected_init_state=CfdpState.BUSY, expected_init_step=TransactionStep.WAITING_FOR_METADATA, + expected_file_size=0, ) self._state_checker(fsm_res, 1, CfdpState.BUSY, TransactionStep.WAITING_FOR_FINISHED_ACK) finished_pdu = self._generic_no_error_finished_pdu_check_acked(fsm_res) @@ -360,8 +352,7 @@ def _generic_deferred_lost_segment_handling_with_timeout(self, file_content: byt # This should trigger deferred EOF handling. fsm_res = self._generic_insert_eof_pdu(len(file_content), crc32) self._generic_eof_recv_indication_check(fsm_res) - self._generic_verify_eof_ack_packet(fsm_res) - self.dest_handler.state_machine() + self._generic_verify_eof_ack_packet(fsm_res, TransactionStep.WAITING_FOR_MISSING_DATA) self._state_checker(fsm_res, 1, CfdpState.BUSY, TransactionStep.WAITING_FOR_MISSING_DATA) self.assertTrue(self.dest_handler.deferred_lost_segment_procedure_active) self.assertEqual(self.dest_handler.nak_activity_counter, 0) @@ -424,8 +415,7 @@ def test_positive_ack_procedure_finished_pdu(self): self._generic_regular_transfer_init(0) fsm_res = self._generic_insert_eof_pdu(0, NULL_CHECKSUM_U32) self._generic_eof_recv_indication_check(fsm_res) - self._generic_verify_eof_ack_packet(fsm_res) - fsm_res = self.dest_handler.state_machine() + self._generic_verify_eof_ack_packet(fsm_res, TransactionStep.WAITING_FOR_FINISHED_ACK) self._generic_no_error_finished_pdu_check_acked(fsm_res) self._generic_verify_transfer_completion(fsm_res, b"") time.sleep(self.timeout_positive_ack_procedure_seconds * 1.1) @@ -495,6 +485,7 @@ def _generic_finished_pdu_with_error_check( self._state_checker(fsm_res, 1, CfdpState.BUSY, TransactionStep.WAITING_FOR_FINISHED_ACK) next_pdu = self.dest_handler.get_next_packet() self.assertIsNotNone(next_pdu) + assert next_pdu is not None self.assertEqual(next_pdu.pdu_type, PduType.FILE_DIRECTIVE) self.assertEqual(next_pdu.pdu_directive_type, DirectiveType.FINISHED_PDU) finished_pdu = next_pdu.to_finished_pdu() @@ -520,13 +511,14 @@ def _generic_verify_missing_segment_requested( def _generic_verify_eof_ack_packet( self, fsm_res: FsmResult, + step_after_eof_ack: TransactionStep, condition_code_of_acked_pdu: ConditionCode = ConditionCode.NO_ERROR, ): self._state_checker( fsm_res, - 1, + 2, CfdpState.BUSY, - TransactionStep.SENDING_EOF_ACK_PDU, + step_after_eof_ack, ) next_pdu = self.dest_handler.get_next_packet() assert next_pdu is not None diff --git a/tests/test_dest_handler_naked.py b/tests/test_dest_handler_naked.py index edad388..53e6215 100644 --- a/tests/test_dest_handler_naked.py +++ b/tests/test_dest_handler_naked.py @@ -291,7 +291,7 @@ def test_file_data_pdu_before_metadata_is_discarded(self): fsm_res = self.dest_handler.state_machine(eof_pdu) self.cfdp_user.transaction_finished_indication.assert_called_once() finished_args = cast( - TransactionFinishedParams, + "TransactionFinishedParams", self.cfdp_user.transaction_finished_indication.call_args.args[0], ) # At least one segment was stored diff --git a/tests/test_src_handler.py b/tests/test_src_handler.py index b2a8aa0..3455dd7 100644 --- a/tests/test_src_handler.py +++ b/tests/test_src_handler.py @@ -4,7 +4,7 @@ import tempfile from dataclasses import dataclass from pathlib import Path -from typing import cast +from typing import TYPE_CHECKING, cast from unittest.mock import MagicMock from crcmod.predefined import PredefinedCrc @@ -33,12 +33,14 @@ from cfdppy.handler import FsmResult, SourceHandler from cfdppy.handler.source import TransactionStep from cfdppy.request import PutRequest -from cfdppy.user import TransactionFinishedParams, TransactionParams from .cfdp_fault_handler_mock import FaultHandler from .cfdp_user_mock import CfdpUser from .common import CheckTimerProviderForTest +if TYPE_CHECKING: + from cfdppy.user import TransactionFinishedParams, TransactionParams + @dataclass class TransactionStartParams: @@ -325,7 +327,7 @@ def _verify_transaction_indication(self, expected_originating_id: TransactionId self.cfdp_user.transaction_indication.assert_called_once() self.assertEqual(self.cfdp_user.transaction_indication.call_count, 1) transaction_params = cast( - TransactionParams, + "TransactionParams", self.cfdp_user.transaction_indication.call_args.args[0], ) self.assertEqual(transaction_params.transaction_id, self.source_handler.transaction_id) @@ -377,7 +379,7 @@ def _verify_transaction_finished_indication( self.cfdp_user.transaction_finished_indication.assert_called_once() self.assertEqual(self.cfdp_user.transaction_finished_indication.call_count, 1) transaction_finished_params = cast( - TransactionFinishedParams, + "TransactionFinishedParams", self.cfdp_user.transaction_finished_indication.call_args.args[0], ) self.assertEqual(transaction_finished_params.transaction_id, expected_id)