diff --git a/tests/cfdp/test_src_handler.py b/tests/cfdp/test_src_handler.py index f9df1889..40dc2c8b 100644 --- a/tests/cfdp/test_src_handler.py +++ b/tests/cfdp/test_src_handler.py @@ -47,6 +47,7 @@ def common_setup( self.cfdp_user.transaction_indication = MagicMock() self.cfdp_user.transaction_finished_indication = MagicMock() self.seq_num_provider = SeqCountProvider(bit_width=8) + self.expected_cfdp_state = CfdpState.BUSY_CLASS_1_NACKED self.source_id = ByteFieldU16(1) self.dest_id = ByteFieldU16(2) self.file_path = Path(f"{tempfile.gettempdir()}/hello.txt") @@ -70,7 +71,7 @@ def common_setup( ) def _common_empty_file_test( - self, transmission_mode: Optional[TransmissionMode], expected_state: CfdpState + self, transmission_mode: Optional[TransmissionMode] ) -> EofPdu: dest_path = Path("/tmp/hello_copy.txt") dest_id = ByteFieldU16(2) @@ -83,11 +84,9 @@ def _common_empty_file_test( trans_mode=transmission_mode, closure_requested=None, ) - transaction_id = self._start_source_transaction( - dest_id, put_req, expected_state - ) + transaction_id = self._start_source_transaction(dest_id, put_req) fsm_res = self.source_handler.state_machine() - self._state_checker(fsm_res, True, expected_state, TransactionStep.SENDING_EOF) + self._state_checker(fsm_res, True, TransactionStep.SENDING_EOF) self.assertEqual(self.source_handler.transaction_seq_num.value, 3) next_packet = self.source_handler.get_next_packet() self.assertIsNotNone(next_packet) @@ -107,8 +106,11 @@ def _common_empty_file_test( return eof_pdu def _common_small_file_test( - self, closure_requested: bool, file_content: str, expected_state: CfdpState - ) -> TransactionId: + self, + transmission_mode: Optional[TransmissionMode], + closure_requested: bool, + file_content: str, + ) -> Tuple[EofPdu, TransactionId]: dest_path = Path("/tmp/hello_copy.txt") self.source_id = ByteFieldU32(1) self.dest_id = ByteFieldU32(2) @@ -119,7 +121,7 @@ def _common_small_file_test( source_file=self.file_path, dest_file=dest_path, # Let the transmission mode be auto-determined by the remote MIB - trans_mode=None, + trans_mode=transmission_mode, closure_requested=closure_requested, ) with open(self.file_path, "wb") as of: @@ -129,9 +131,7 @@ def _common_small_file_test( crc32 = crc32.digest() of.write(data) file_size = self.file_path.stat().st_size - transaction_id = self._start_source_transaction( - self.dest_id, put_req, expected_state - ) + transaction_id = self._start_source_transaction(self.dest_id, put_req) self.assertEqual(transaction_id.source_id, self.source_handler.source_id) self.assertEqual(transaction_id.seq_num.value, 2) self.assertEqual(self.source_handler.transaction_seq_num.value, 2) @@ -143,12 +143,10 @@ def _common_small_file_test( file_data_pdu = self._check_fsm_and_contained_file_data(fsm_res, next_packet) self.assertFalse(file_data_pdu.has_segment_metadata) self.assertEqual(file_data_pdu.transaction_seq_num.value, 2) - self.assertEqual(file_data_pdu.file_data, "Hello World\n".encode()) + self.assertEqual(file_data_pdu.file_data, file_content.encode()) self.assertEqual(file_data_pdu.offset, 0) fsm_res = self.source_handler.state_machine() - self._state_checker( - fsm_res, True, CfdpState.BUSY_CLASS_1_NACKED, TransactionStep.SENDING_EOF - ) + self._state_checker(fsm_res, True, TransactionStep.SENDING_EOF) next_packet = self.source_handler.get_next_packet() assert next_packet is not None self.assertEqual(next_packet.pdu_directive_type, DirectiveType.EOF_PDU) @@ -156,10 +154,12 @@ def _common_small_file_test( self.assertEqual(crc32, eof_pdu.file_checksum) self.assertEqual(eof_pdu.file_size, file_size) self.assertEqual(eof_pdu.condition_code, ConditionCode.NO_ERROR) - return transaction_id + fsm_res = self.source_handler.state_machine() + self._verify_eof_indication(transaction_id) + return eof_pdu, transaction_id def _transaction_with_file_data_wrapper( - self, dest_path: Path, data: bytes, expected_state: CfdpState + self, dest_path: Path, data: bytes ) -> Tuple[TransactionId, int, bytes]: put_req = PutRequest( destination_id=self.dest_id, @@ -176,9 +176,7 @@ def _transaction_with_file_data_wrapper( of.write(data) file_size = self.file_path.stat().st_size self.local_cfg.local_entity_id = self.source_id - transaction_id = self._start_source_transaction( - self.dest_id, put_req, expected_state - ) + transaction_id = self._start_source_transaction(self.dest_id, put_req) return transaction_id, file_size, crc32 def _first_file_segment_handling( @@ -203,7 +201,6 @@ def _check_fsm_and_contained_file_data( self._state_checker( fsm_res, False, - CfdpState.BUSY_CLASS_1_NACKED, TransactionStep.SENDING_FILE_DATA, ) self.assertFalse(pdu_holder.is_file_directive) @@ -213,7 +210,6 @@ def _start_source_transaction( self, dest_id: UnsignedByteField, put_request: PutRequest, - expected_state: CfdpState, ) -> TransactionId: self.remote_cfg.entity_id = dest_id self.source_handler.put_request(put_request, self.remote_cfg) @@ -221,7 +217,6 @@ def _start_source_transaction( self._state_checker( fsm_res, True, - expected_state, TransactionStep.SENDING_METADATA, ) transaction_id = self.source_handler.transaction_id @@ -256,16 +251,15 @@ def _state_checker( self, fsm_res: Optional[FsmResult], packet_ready: bool, - expected_state: CfdpState, expected_step: TransactionStep, ): if fsm_res is not None: - self.assertEqual(fsm_res.states.state, expected_state) + self.assertEqual(fsm_res.states.state, self.expected_cfdp_state) self.assertEqual(fsm_res.states.step, expected_step) - self.assertEqual(self.source_handler.states.state, expected_state) + self.assertEqual(self.source_handler.states.state, self.expected_cfdp_state) self.assertEqual(self.source_handler.states.step, expected_step) self.assertEqual(self.source_handler.states.packets_ready, packet_ready) - self.assertEqual(self.source_handler.state, expected_state) + self.assertEqual(self.source_handler.state, self.expected_cfdp_state) self.assertEqual(self.source_handler.step, expected_step) self.assertEqual(self.source_handler.packets_ready, packet_ready) diff --git a/tests/cfdp/test_src_handler_acked.py b/tests/cfdp/test_src_handler_acked.py index 7de2f186..fd34c075 100644 --- a/tests/cfdp/test_src_handler_acked.py +++ b/tests/cfdp/test_src_handler_acked.py @@ -8,6 +8,7 @@ from spacepackets.cfdp.pdu import ( AckPdu, DeliveryCode, + EofPdu, FileDeliveryStatus, FinishedPdu, TransactionStatus, @@ -22,18 +23,14 @@ class TestSourceHandlerAcked(TestCfdpSourceHandler): def setUp(self) -> None: self.common_setup(True, TransmissionMode.ACKNOWLEDGED) + self.expected_cfdp_state = CfdpState.BUSY_CLASS_2_ACKED - def test_empty_file_transfer(self): - eof_pdu = self._common_empty_file_test(None, CfdpState.BUSY_CLASS_2_ACKED) + def _generic_success_ack_handling(self, eof_pdu: EofPdu): self._state_checker( None, False, - CfdpState.BUSY_CLASS_2_ACKED, TransactionStep.WAITING_FOR_EOF_ACK, ) - # TODO: 1: Acknowledge EOF PDU by inserting ACK, 2: Insert Finished PDU, 3: Retrieve - # and check ACK PDU generated as a response to the Finished PDU. - # pdu_conf = PduConfig(eof_pdu.source_entity_id, eof_pdu.dest_entity_id, eof_pdu.transaction_seq_num, eof_pdu.transmission_mode) pdu_conf = eof_pdu.pdu_header.pdu_conf ack_pdu = AckPdu( pdu_conf, @@ -47,7 +44,6 @@ def test_empty_file_transfer(self): self._state_checker( None, False, - CfdpState.BUSY_CLASS_2_ACKED, TransactionStep.WAITING_FOR_FINISHED, ) finished_pdu = FinishedPdu( @@ -63,7 +59,6 @@ def test_empty_file_transfer(self): self._state_checker( None, True, - CfdpState.BUSY_CLASS_2_ACKED, TransactionStep.SENDING_ACK_OF_FINISHED, ) next_pdu = self.source_handler.get_next_packet() @@ -80,9 +75,21 @@ def test_empty_file_transfer(self): pdu_conf.direction = Direction.TOWARDS_RECEIVER self.assertEqual(ack_pdu.pdu_header.pdu_conf, pdu_conf) self.source_handler.state_machine() + self.expected_cfdp_state = CfdpState.IDLE self._state_checker( None, False, - CfdpState.IDLE, TransactionStep.IDLE, ) + + def test_empty_file_transfer(self): + eof_pdu = self._common_empty_file_test(None) + self._generic_success_ack_handling(eof_pdu) + + def test_small_file_transfer(self): + eof_pdu, _ = self._common_small_file_test( + TransmissionMode.ACKNOWLEDGED, + True, + "Hello World!", + ) + self._generic_success_ack_handling(eof_pdu) diff --git a/tests/cfdp/test_src_handler_nak_closure.py b/tests/cfdp/test_src_handler_nak_closure.py index 6fd2263a..ce43e727 100644 --- a/tests/cfdp/test_src_handler_nak_closure.py +++ b/tests/cfdp/test_src_handler_nak_closure.py @@ -30,39 +30,38 @@ def setUp(self) -> None: self.seq_num_provider.get_and_increment = MagicMock(return_value=2) def test_empty_file_pdu_generation_nacked_by_remote_cfg(self): - self._common_empty_file_test(None, CfdpState.BUSY_CLASS_1_NACKED) + self._common_empty_file_test(None) self._pass_simple_finish_pdu_to_source_handler() # Transaction should be finished fsm_res = self.source_handler.state_machine() - self._state_checker(fsm_res, False, CfdpState.IDLE, TransactionStep.IDLE) + self.expected_cfdp_state = CfdpState.IDLE + self._state_checker(fsm_res, False, TransactionStep.IDLE) def test_empty_file_pdu_generation_nacked_explicitely(self): self.remote_cfg.default_transmission_mode = TransmissionMode.ACKNOWLEDGED - self._common_empty_file_test( - TransmissionMode.UNACKNOWLEDGED, CfdpState.BUSY_CLASS_1_NACKED - ) + self._common_empty_file_test(TransmissionMode.UNACKNOWLEDGED) self._pass_simple_finish_pdu_to_source_handler() # Transaction should be finished fsm_res = self.source_handler.state_machine() - self._state_checker(fsm_res, False, CfdpState.IDLE, TransactionStep.IDLE) + self.expected_cfdp_state = CfdpState.IDLE + self._state_checker(fsm_res, False, TransactionStep.IDLE) def test_small_file_pdu_generation(self): file_content = "Hello World\n" - transaction_id = self._common_small_file_test( - True, file_content, CfdpState.BUSY_CLASS_1_NACKED + _, transaction_id = self._common_small_file_test( + TransmissionMode.UNACKNOWLEDGED, True, file_content ) self._verify_eof_indication(transaction_id) self.source_handler.state_machine() self._pass_simple_finish_pdu_to_source_handler() # Transaction should be finished fsm_res = self.source_handler.state_machine() - self._state_checker(fsm_res, False, CfdpState.IDLE, TransactionStep.IDLE) + self.expected_cfdp_state = CfdpState.IDLE + self._state_checker(fsm_res, False, TransactionStep.IDLE) def test_invalid_dir_pdu_passed(self): dest_id = ByteFieldU16(2) - self._start_source_transaction( - dest_id, self._prepare_dummy_put_req(dest_id), CfdpState.BUSY_CLASS_1_NACKED - ) + self._start_source_transaction(dest_id, self._prepare_dummy_put_req(dest_id)) finish_pdu = self._prepare_finish_pdu() finish_pdu.pdu_file_directive.pdu_header.direction = Direction.TOWARDS_RECEIVER with self.assertRaises(InvalidPduDirection): @@ -82,7 +81,6 @@ def test_cancelled_transaction(self): transaction_id, file_size, crc32 = self._transaction_with_file_data_wrapper( dest_path, rand_data[0 : self.file_segment_len], - CfdpState.BUSY_CLASS_1_NACKED, ) self._first_file_segment_handling(self.source_handler, rand_data) self.assertTrue(self.source_handler.cancel_request(transaction_id)) @@ -96,7 +94,8 @@ def test_cancelled_transaction(self): self.assertEqual(eof_pdu.file_size, self.file_segment_len) self.assertEqual(eof_pdu.file_size, file_size) fsm_res = self.source_handler.state_machine() - self._state_checker(fsm_res, False, CfdpState.IDLE, TransactionStep.IDLE) + self.expected_cfdp_state = CfdpState.IDLE + self._state_checker(fsm_res, False, TransactionStep.IDLE) def test_invalid_source_id_pdu_passed(self): dest_id = ByteFieldU16(2) @@ -119,9 +118,7 @@ def test_invalid_dest_id_pdu_passed(self): self.assertEqual(exception.expected_dest_id, ByteFieldU16(3)) def _regular_transaction_start(self, dest_id: UnsignedByteField) -> FinishedPdu: - self._start_source_transaction( - dest_id, self._prepare_dummy_put_req(dest_id), CfdpState.BUSY_CLASS_1_NACKED - ) + self._start_source_transaction(dest_id, self._prepare_dummy_put_req(dest_id)) finish_pdu = self._prepare_finish_pdu() self.assertEqual(finish_pdu.transaction_seq_num.value, 2) return finish_pdu @@ -140,7 +137,6 @@ def _pass_simple_finish_pdu_to_source_handler(self): self._state_checker( None, False, - CfdpState.BUSY_CLASS_1_NACKED, TransactionStep.WAITING_FOR_FINISHED, ) self.source_handler.insert_packet(self._prepare_finish_pdu()) diff --git a/tests/cfdp/test_src_handler_nak_no_closure.py b/tests/cfdp/test_src_handler_nak_no_closure.py index eb567c79..0f35ecb4 100644 --- a/tests/cfdp/test_src_handler_nak_no_closure.py +++ b/tests/cfdp/test_src_handler_nak_no_closure.py @@ -27,32 +27,31 @@ def setUp(self) -> None: def test_empty_file_nacked_by_def_config(self): self._common_empty_file_test( - transmission_mode=None, expected_state=CfdpState.BUSY_CLASS_1_NACKED + transmission_mode=None, ) fsm_res = self.source_handler.state_machine() self.cfdp_user.transaction_finished_indication.assert_called_once() self.assertEqual(self.cfdp_user.transaction_finished_indication.call_count, 1) self.source_handler.state_machine() - self.assertEqual(fsm_res.states.state, CfdpState.IDLE) - self.assertEqual(fsm_res.states.step, TransactionStep.IDLE) + self.expected_cfdp_state = CfdpState.IDLE + self._state_checker(fsm_res, False, TransactionStep.IDLE) def test_empty_file_explicit_nacked(self): self.remote_cfg.default_transmission_mode = TransmissionMode.ACKNOWLEDGED self._common_empty_file_test( transmission_mode=TransmissionMode.UNACKNOWLEDGED, - expected_state=CfdpState.BUSY_CLASS_1_NACKED, ) fsm_res = self.source_handler.state_machine() self.cfdp_user.transaction_finished_indication.assert_called_once() self.assertEqual(self.cfdp_user.transaction_finished_indication.call_count, 1) self.source_handler.state_machine() - self.assertEqual(fsm_res.states.state, CfdpState.IDLE) - self.assertEqual(fsm_res.states.step, TransactionStep.IDLE) + self.expected_cfdp_state = CfdpState.IDLE + self._state_checker(fsm_res, False, TransactionStep.IDLE) def test_small_file_pdu_generation(self): file_content = "Hello World\n" - transaction_id = self._common_small_file_test( - False, file_content, CfdpState.BUSY_CLASS_1_NACKED + eof_pdu, transaction_id = self._common_small_file_test( + None, False, file_content ) self._verify_eof_indication(transaction_id) self._test_transaction_completion() @@ -68,7 +67,7 @@ def test_perfectly_segmented_file_pdu_generation(self): self.source_handler.source_id = self.source_id dest_path = Path("/tmp/hello_two_segments_copy.txt") transaction_id, file_size, crc32 = self._transaction_with_file_data_wrapper( - dest_path, rand_data, CfdpState.BUSY_CLASS_1_NACKED + dest_path, rand_data ) self.assertEqual(transaction_id.source_id, self.source_id) self.assertEqual(transaction_id.seq_num.value, 0) @@ -97,7 +96,7 @@ def test_segmented_file_pdu_generation(self): self.source_handler.source_id = self.source_id dest_path = Path("/tmp/hello_two_segments_imperfect_copy.txt") transaction_id, file_size, crc32 = self._transaction_with_file_data_wrapper( - dest_path, rand_data, CfdpState.BUSY_CLASS_1_NACKED + dest_path, rand_data ) self.assertEqual(transaction_id.source_id, self.source_id) self.assertEqual(transaction_id.seq_num.value, 0) diff --git a/tmtccmd/cfdp/handler/source.py b/tmtccmd/cfdp/handler/source.py index 4606599b..e029e02c 100644 --- a/tmtccmd/cfdp/handler/source.py +++ b/tmtccmd/cfdp/handler/source.py @@ -1,7 +1,6 @@ from __future__ import annotations from collections import deque import enum -import copy import logging from dataclasses import dataclass from typing import Deque, Optional, Tuple @@ -701,17 +700,15 @@ def _setup_transmission_mode(self): assert self._put_req is not None assert self._params.remote_cfg is not None # Transmission mode settings in the put request override settings from the remote MIB - if self._put_req.trans_mode is not None: - trans_mode_to_set = self._put_req.trans_mode - else: + trans_mode_to_set = self._put_req.trans_mode + if trans_mode_to_set is None: trans_mode_to_set = self._params.remote_cfg.default_transmission_mode - self._params.transmission_mode = trans_mode_to_set - if self._put_req.closure_requested is not None: - closure_req_to_set = self._put_req.closure_requested - else: + closure_req_to_set = self._put_req.closure_requested + if closure_req_to_set is None: closure_req_to_set = self._params.remote_cfg.closure_requested - self._params.crc_helper.checksum_type = self._params.remote_cfg.crc_type + self._params.transmission_mode = trans_mode_to_set self._params.closure_requested = closure_req_to_set + self._params.crc_helper.checksum_type = self._params.remote_cfg.crc_type def _add_packet_to_be_sent(self, packet: GenericPduPacket): self._pdus_to_be_sent.append(PduHolder(packet))