Merge pull request #9 from us-irs/smaller-bugfixes-and-tweaks

Smaller bugfixes and tweaks

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

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

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:
@@ -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.
@@ -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

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

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)

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)