Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Smaller bugfixes and tweaks #9

Merged
merged 2 commits into from
Jul 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 14 additions & 2 deletions src/cfdppy/filestore.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -98,7 +102,7 @@ def list_directory(
return FilestoreResponseStatusCode.NOT_PERFORMED


class HostFilestore(VirtualFilestore):
class NativeFilestore(VirtualFilestore):
def __init__(self):
pass

Expand All @@ -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:
Expand All @@ -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)
Expand Down Expand Up @@ -255,3 +264,6 @@ def list_directory(
os.system(f"{cmd} >> {target_file}")
os.chdir(curr_path)
return FilestoreResponseStatusCode.SUCCESS


HostFilestore = NativeFilestore
18 changes: 9 additions & 9 deletions src/cfdppy/handler/source.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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:
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/cfdppy/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)

Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions tests/test_checksum.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions tests/test_filestore.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
Loading