From d02c38318648ed684fc6e4099f65b93831c8e300 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 28 Jun 2024 10:43:42 +0200 Subject: [PATCH 1/2] internal name improvement --- src/cfdppy/handler/source.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cfdppy/handler/source.py b/src/cfdppy/handler/source.py index d39caac..9f72d34 100644 --- a/src/cfdppy/handler/source.py +++ b/src/cfdppy/handler/source.py @@ -338,7 +338,7 @@ def put_request(self, request: PutRequest): self._params.dest_id = request.destination_id self.states._num_packets_ready = 0 self.states.state = CfdpState.BUSY - self._setup_transmission_mode() + self._setup_transmission_params() if self._params.transmission_mode == TransmissionMode.UNACKNOWLEDGED: _LOGGER.debug("Starting Put Request handling in NAK mode") elif self._params.transmission_mode == TransmissionMode.ACKNOWLEDGED: @@ -869,7 +869,7 @@ def _start_positive_ack_procedure(self): ) self._params.positive_ack_params.ack_counter = 0 - def _setup_transmission_mode(self): + def _setup_transmission_params(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 From ad28fc7ece462cf7406d371c0181459b003d2186 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 28 Jun 2024 11:08:01 +0200 Subject: [PATCH 2/2] Smaller bugfixes and tweaks --- CHANGELOG.md | 11 +++++++++++ src/cfdppy/filestore.py | 16 ++++++++++++++-- src/cfdppy/handler/source.py | 14 +++++++------- src/cfdppy/user.py | 4 ++-- tests/test_checksum.py | 4 ++-- tests/test_filestore.py | 6 +++--- 6 files changed, 39 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c95b59..ab83f3b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,17 @@ and this project adheres to [Semantic Versioning](http://semver.org/). # [unreleased] +# [v0.2.0] 2024-06-28 + +## Fixed + +- The large file flag was not set properly in the source handler for large file transfers. + +## Changed + +- Added `file_size` abstract method to `VirtualFilestore` +- Renamed `HostFilestore` to `NativeFilestore`, but keep old name alias for backwards compatibility. + # [v0.1.2] 2024-06-04 Updated documentation configuration to include a `spacepackets` docs mapping. This diff --git a/src/cfdppy/filestore.py b/src/cfdppy/filestore.py index 9ade588..ab47f2d 100644 --- a/src/cfdppy/filestore.py +++ b/src/cfdppy/filestore.py @@ -42,6 +42,10 @@ def file_exists(self, path: Path) -> bool: def truncate_file(self, file: Path): pass + @abc.abstractmethod + def file_size(self, file: Path) -> int: + pass + @abc.abstractmethod def write_data(self, file: Path, data: bytes, offset: Optional[int]): """This is not used as part of a filestore request, it is used to build up the received @@ -98,7 +102,7 @@ def list_directory( return FilestoreResponseStatusCode.NOT_PERFORMED -class HostFilestore(VirtualFilestore): +class NativeFilestore(VirtualFilestore): def __init__(self): pass @@ -107,7 +111,7 @@ def read_data( ) -> bytes: if not file.exists(): raise FileNotFoundError(file) - file_size = file.stat().st_size + file_size = self.file_size(file) if read_len is None: read_len = file_size if offset is None: @@ -116,6 +120,11 @@ def read_data( rf.seek(offset) return rf.read(read_len) + def file_size(self, file: Path) -> int: + if not file.exists(): + raise FileNotFoundError(file) + return file.stat().st_size + def read_from_opened_file(self, bytes_io: BinaryIO, offset: int, read_len: int): bytes_io.seek(offset) return bytes_io.read(read_len) @@ -255,3 +264,6 @@ def list_directory( os.system(f"{cmd} >> {target_file}") os.chdir(curr_path) return FilestoreResponseStatusCode.SUCCESS + + +HostFilestore = NativeFilestore diff --git a/src/cfdppy/handler/source.py b/src/cfdppy/handler/source.py index 9f72d34..5004b14 100644 --- a/src/cfdppy/handler/source.py +++ b/src/cfdppy/handler/source.py @@ -527,10 +527,9 @@ def _fsm_non_idle(self): self._notice_of_completion() def _transaction_start(self): - file_size = 0 originating_transaction_id = self._check_for_originating_id() self._prepare_file_params() - self._prepare_pdu_conf(file_size) + self._prepare_pdu_conf(self._params.fp.file_size) self._get_next_transfer_seq_num() self._calculate_max_file_seg_len() self._params.transaction_id = TransactionId( @@ -577,7 +576,7 @@ def _prepare_file_params(self): if not self._put_req.source_file.exists(): # TODO: Handle this exception in the handler, reset CFDP state machine raise SourceFileDoesNotExist(self._put_req.source_file) - file_size = self._put_req.source_file.stat().st_size + file_size = self.user.vfs.file_size(self._put_req.source_file) if file_size == 0: self._params.fp.metadata_only = True else: @@ -588,10 +587,11 @@ def _prepare_pdu_conf(self, file_size: int): # a previous step. assert self._put_req is not None assert self._params.remote_cfg is not None - if file_size > pow(2, 32) - 1: - self._params.pdu_conf.file_flag = LargeFileFlag.LARGE - else: - self._params.pdu_conf.file_flag = LargeFileFlag.NORMAL + if not self._params.fp.metadata_only: + if file_size > pow(2, 32) - 1: + self._params.pdu_conf.file_flag = LargeFileFlag.LARGE + else: + self._params.pdu_conf.file_flag = LargeFileFlag.NORMAL if self._put_req.seg_ctrl is not None: self._params.pdu_conf.seg_ctrl = self._put_req.seg_ctrl # Both the source entity and destination entity ID field must have the same size. diff --git a/src/cfdppy/user.py b/src/cfdppy/user.py index e3b5f19..4fe8698 100644 --- a/src/cfdppy/user.py +++ b/src/cfdppy/user.py @@ -8,7 +8,7 @@ from spacepackets.cfdp.pdu.file_data import SegmentMetadata from spacepackets.cfdp.pdu.finished import FinishedParams from spacepackets.util import UnsignedByteField -from cfdppy.filestore import VirtualFilestore, HostFilestore +from cfdppy.filestore import VirtualFilestore, NativeFilestore _LOGGER = logging.getLogger(__name__) @@ -64,7 +64,7 @@ class CfdpUserBase(ABC): def __init__(self, vfs: Optional[VirtualFilestore] = None): if vfs is None: - vfs = HostFilestore() + vfs = NativeFilestore() self.vfs = vfs @abstractmethod diff --git a/tests/test_checksum.py b/tests/test_checksum.py index 3a8db1c..42fae0c 100644 --- a/tests/test_checksum.py +++ b/tests/test_checksum.py @@ -4,7 +4,7 @@ from tempfile import gettempdir from pathlib import Path from cfdppy.handler.crc import CrcHelper, calc_modular_checksum -from cfdppy.user import HostFilestore +from cfdppy.user import NativeFilestore from spacepackets.cfdp import ChecksumType @@ -32,7 +32,7 @@ class TestChecksumHelper(TestCase): def setUp(self): self.setUpPyfakefs() - self.crc_helper = CrcHelper(ChecksumType.NULL_CHECKSUM, HostFilestore()) + self.crc_helper = CrcHelper(ChecksumType.NULL_CHECKSUM, NativeFilestore()) self.file_path = Path(f"{gettempdir()}/crc_file") with open(self.file_path, "wb") as file: file.write(EXAMPLE_DATA_CFDP) diff --git a/tests/test_filestore.py b/tests/test_filestore.py index cabf1a5..c2f97a6 100644 --- a/tests/test_filestore.py +++ b/tests/test_filestore.py @@ -3,7 +3,7 @@ import tempfile from pyfakefs.fake_filesystem_unittest import TestCase -from cfdppy.filestore import HostFilestore, FilestoreResult +from cfdppy.filestore import NativeFilestore, FilestoreResult class TestCfdpHostFilestore(TestCase): @@ -15,7 +15,7 @@ def setUp(self): self.test_dir_name_0 = Path(f"{self.temp_dir}/cfdp_test_folder0") self.test_dir_name_1 = Path(f"{self.temp_dir}/cfdp_test_folder1") self.test_list_dir_name = Path(f"{self.temp_dir}/list-dir-test.txt") - self.filestore = HostFilestore() + self.filestore = NativeFilestore() def test_creation(self): res = self.filestore.create_file(self.test_file_name_0) @@ -83,7 +83,7 @@ def test_replace_file(self): self.assertEqual(self.filestore.read_data(self.test_file_name_1, 0), file_data) def test_list_dir(self): - filestore = HostFilestore() + filestore = NativeFilestore() tempdir = Path(tempfile.gettempdir()) if os.path.exists(self.test_list_dir_name): os.remove(self.test_list_dir_name)