Merge pull request #25 from sgaisser/spelling_and_documentation

Spelling and documentation

docs/api/cfdp.handler.rst

@@ -25,7 +25,7 @@ Destination Handler Module
    :undoc-members:
    :show-inheritance:
 
-Defintions Module
+Definitions Module
 -------------------------------------
 
 .. automodule:: cfdppy.handler.defs

docs/examples.rst

@@ -14,5 +14,5 @@ which cross-tests the `cfdp-py` CFDP implementation with the
 
 Finally, you can see a more complex example also featuring more features of the CFDP state machines
 `here <https://github.com/us-irs/cfdp-py/tree/main/examples/cfdp-cli-udp>`_. This example
-uses UDP servers for communication and explicitely separates the local and remote entity
+uses UDP servers for communication and explicitly separates the local and remote entity
 application.

docs/introduction.rst

@@ -3,12 +3,12 @@ The CCSDS File Delivery Protocol (CFDP)
 =========================================
 
 The basic idea of CFDP is to convert files of any size into a stream of packets called packet
-data units (PDU). CFPD has an unacknowledged and acknowledged mode, with the option to request
+data units (PDU). CFDP has an unacknowledged and acknowledged mode, with the option to request
 a transaction closure for the unacknowledged mode. Using the unacknowledged mode with no
 transaction closure is applicable for simplex communication paths, while the unacknowledged mode
 with closure is the easiest way to get a confirmation of a successful file transfer, including a
 CRC check on the remote side to verify file integrity. The acknowledged mode is the most complex
-mode which includes multiple mechanism to ensure succesfull packet transaction even for unreliable
+mode which includes multiple mechanism to ensure successful packet transaction even for unreliable
 connections, including lost segment detection. As such, it can be compared to a specialized TCP
 for file transfers with remote systems.
 
@@ -23,7 +23,7 @@ The :py:class:`cfdppy.handler.source.SourceHandler` converts a
 :py:class:`cfdppy.request.PutRequest` into all packet data units (PDUs) which need to be
 sent to a remote CFDP entity to perform a File Copy operation to a remote entity.
 
-The source entity allows freedom of communcation by only generating the packets required to be
+The source entity allows freedom of communication by only generating the packets required to be
 sent, and leaving the actual transmission of those packets to the user. The packets are returned
 to the user using the :py:meth:`cfdppy.handler.source.SourceHandler.get_next_packet` call.
 It should be noted that for regular file transfers, each state machine call will map to one
@@ -64,7 +64,7 @@ example generated by the :py:class:`spacepackets.cfdp.pdu.file_data.FileDataPdu`
 into the destination handler and will be assembled into a file.
 
 A destination entity might still generate packets which need to be sent back to the source entity
-of the file transfer. However, it allows freedom of communcation like the source entity by leaving
+of the file transfer. However, it allows freedom of communication like the source entity by leaving
 the actual transmission of generated packets to the user. The packets are returned to the user
 using the :py:meth:`cfdppy.handler.dest.DestHandler.get_next_packet` call.
 

examples/cfdp-simple/file-copy-example.py

@@ -1,5 +1,5 @@
 #!/usr/bin/env python3
-"""This example shows a end-to-end transfer of a small file using the CFDP high level
+"""This example shows an end-to-end transfer of a small file using the CFDP high level
 components provided by the tmtccmd package."""
 
 import argparse
@@ -105,7 +105,7 @@ def abandoned_cb(
         self, transaction_id: TransactionId, cond: ConditionCode, progress: int
     ) -> None:
         _LOGGER.warning(
-            f"Received Abanadoned Fault for transaction {transaction_id!r} with condition "
+            f"Received Abandoned Fault for transaction {transaction_id!r} with condition "
             f"code {cond!r}. Progress: {progress}"
         )
 
@@ -283,10 +283,8 @@ def main() -> None:
     source_thread.join()
     dest_thread.join()
 
-    src_file_content = None
     with open(SOURCE_FILE) as file:
         src_file_content = file.read()
-    dest_file_content = None
     with open(DEST_FILE) as file:
         dest_file_content = file.read()
     assert src_file_content == dest_file_content

src/cfdppy/exceptions.py

@@ -80,7 +80,7 @@ class InvalidTransactionSeqNum(Exception):
     def __init__(self, expected: UnsignedByteField, received: UnsignedByteField):
         self.expected = expected
         self.received = received
-        super().__init__(f"expected sequence number {expected}, reiceved {self.received}")
+        super().__init__(f"expected sequence number {expected}, received {self.received}")
 
 
 class BusyError(Exception):
@@ -98,7 +98,7 @@ class PduIgnoredForSourceReason(enum.IntEnum):
     ACK_MODE_PACKET_INVALID_MODE = 0
     # Received a Finished PDU, but source handler is currently not expecting one.
     NOT_WAITING_FOR_FINISHED_PDU = 1
-    # Received a ACK PDU, but source handler is currently not expecting one.
+    # Received an ACK PDU, but source handler is currently not expecting one.
     NOT_WAITING_FOR_ACK = 2
 
 

src/cfdppy/filestore.py

@@ -1,11 +1,13 @@
+"""Contains the Filestore Interface and a Native Filestore implementation."""
+
 from __future__ import annotations  # Python 3.9 compatibility for | syntax
 
 import abc
 import logging
 import os
 import platform
 import shutil
-from typing import TYPE_CHECKING, BinaryIO, NoReturn
+from typing import TYPE_CHECKING, BinaryIO
 
 from crcmod.predefined import PredefinedCrc
 from spacepackets.cfdp.defs import NULL_CHECKSUM_U32, ChecksumType
@@ -23,45 +25,87 @@
 
 
 class VirtualFilestore(abc.ABC):
+    """Interface for a virtual filestore implementation."""
+
     @abc.abstractmethod
     def read_data(self, file: Path, offset: int | None, read_len: int) -> bytes:
-        """This is not used as part of a filestore request, it is used to read a file, for example
-        to send it"""
-        raise NotImplementedError("Reading file not implemented in virtual filestore")
+        """Read data from a provided Path at a specific offset.
+
+        This is not used as part of a filestore request.
+        It is used to read a file, for example to send it.
+
+        :param file: File to read
+        :param offset: Offset to read from
+        :param read_len: Number of bytes to read
+        :return: The read data
+        :raises PermissionError: In case the file is not readable
+        :raises FileNotFoundError: In case the file does not exist.
+        """
 
     @abc.abstractmethod
-    def read_from_opened_file(self, bytes_io: BinaryIO, offset: int, read_len: int) -> NoReturn:
-        raise NotImplementedError("Reading from opened file not implemented in virtual filestore")
+    def read_from_opened_file(self, bytes_io: BinaryIO, offset: int, read_len: int) -> bytes:
+        """Read data from an already opened file object.
+
+        :param bytes_io: File object
+        :param offset: Offset to read from
+        :param read_len: Number of bytes to read
+        :return: The read data
+        """
 
     @abc.abstractmethod
     def is_directory(self, path: Path) -> bool:
-        pass
+        """Check if a given path is a directory.
+
+        :param path: Path to check
+        :return: True if the path is a directory
+        """
 
     @abc.abstractmethod
     def filename_from_full_path(self, path: Path) -> str | None:
-        pass
+        """Get the filename from a full path.
+
+        :param path: Full path
+        :return: Filename
+        """
 
     @abc.abstractmethod
     def file_exists(self, path: Path) -> bool:
-        pass
+        """Check if a file exists at the given path.
+
+        :param path: Path to check
+        :return: True if the file exists
+        """
 
     @abc.abstractmethod
     def truncate_file(self, file: Path) -> None:
-        pass
+        """Truncate a file to zero bytes.
+
+        :param file: File to truncate
+        :raises FileNotFoundError: In case the file does not exist
+        """
 
     @abc.abstractmethod
     def file_size(self, file: Path) -> int:
-        pass
+        """Get the size of a file.
+
+        :param file: File to get the size of as number of bytes
+        :return: Size of the file in bytes
+        :raises FileNotFoundError: In case the file does not exist
+        """
 
     @abc.abstractmethod
-    def write_data(self, file: Path, data: bytes, offset: int | None) -> NoReturn:
+    def write_data(self, file: Path, data: bytes, offset: int | None) -> None:
         """This is not used as part of a filestore request, it is used to build up the received
         file.
 
-        :raises PermissionError:
-        :raises FileNotFoundError:
+        The file needs to exist before writing to it.
+
+        :param file: File to write to
+        :param data: Data to write
+        :param offset: Offset to write, may be None for no offset
+        :raises PermissionError: In case the file is not writable
+        :raises FileNotFoundError: In case the file does not exist
         """
-        raise NotImplementedError("Writing to data not implemented in virtual filestore")
 
     @abc.abstractmethod
     def create_file(self, file: Path) -> FilestoreResponseStatusCode:

src/cfdppy/handler/dest.py

@@ -122,7 +122,7 @@ class TransactionStep(enum.Enum):
     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 succesfully."""
+    note that this does not necessarily mean that the file transfer was completed successfully."""
     SENDING_FINISHED_PDU = 8
     WAITING_FOR_FINISHED_ACK = 9
 
@@ -498,7 +498,7 @@ def _reset_internal(self, clear_packet_queue: bool) -> None:
             self._pdus_to_be_sent.clear()
 
     def reset(self) -> None:
-        """This function is public to allow completely resetting the handler, but it is explicitely
+        """This function is public to allow completely resetting the handler, but it is explicitly
         discouraged to do this. CFDP generally has mechanism to detect issues and errors on itself.
         """
         self._reset_internal(False)
@@ -782,7 +782,7 @@ def _handle_waiting_for_finished_ack(self, packet_holder: PduHolder) -> None:
         ):
             ack_pdu = packet_holder.to_ack_pdu()
             if ack_pdu.directive_code_of_acked_pdu != DirectiveType.FINISHED_PDU:
-                _LOGGER.warn(
+                _LOGGER.warning(
                     f"received ACK PDU with invalid ACKed directive code "
                     f" {ack_pdu.directive_code_of_acked_pdu}"
                 )
@@ -801,7 +801,7 @@ def _handle_positive_ack_procedures(self) -> FsmResult | None:
             ):
                 self._declare_fault(ConditionCode.POSITIVE_ACK_LIMIT_REACHED)
                 # This is a bit of a hack: We want the transfer completion and the corresponding
-                # re-send of the Finished PDU to happen in the same FSM cycle. However, the call
+                # Finished PDU to be re-sent in the same FSM cycle. However, the call
                 # order in the FSM prevents this from happening, so we just call the state machine
                 # again manually.
                 if self._params.completion_disposition == CompletionDisposition.CANCELED:
@@ -999,7 +999,7 @@ def _handle_no_error_eof(self) -> bool:
             )
         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..
+            # error for now.
             _LOGGER.warning("missmatch of EOF file size and Metadata File Size for success EOF")
         if (
             self.transmission_mode == TransmissionMode.UNACKNOWLEDGED
@@ -1163,6 +1163,6 @@ def _notice_of_suspension(self) -> None:
         pass
 
     def _abandon_transaction(self) -> None:
-        # I guess an abandoned transaction just stops whatever it is doing.. The implementation
+        # I guess an abandoned transaction just stops whatever it is doing. The implementation
         # for this is quite easy.
         self.reset()

src/cfdppy/handler/source.py

@@ -211,7 +211,7 @@ class SourceHandler:
     source-to-destination file transfer. This class does not send the CFDP PDU packets directly
     to allow for greater flexibility. For example, a user might want to wrap the CFDP packet
     entities into a CCSDS space packet or into a special frame type. The handler can handle
-    both unacknowledged (class 1) and acknowledged (class 2) file tranfers.
+    both unacknowledged (class 1) and acknowledged (class 2) file transfers.
 
     The following core functions are the primary interface:
 
@@ -308,10 +308,10 @@ def num_packets_ready(self) -> int:
     def put_request(self, request: PutRequest) -> bool:
         """This function is used to pass a put request to the source handler, which is
         also used to start a file copy operation. As such, this function models the Put.request
-        CFDP primtiive.
+        CFDP primitive.
 
         Please note that the source handler can also process one put request at a time.
-        The caller is responsible of creating a new source handler, one handler can only handle
+        The caller is responsible for creating a new source handler, one handler can only handle
         one file copy request at a time.
 
 
@@ -328,7 +328,7 @@ def put_request(self, request: PutRequest) -> bool:
         Returns
         --------
 
-        False if the handler is busy. True if the handling of the request was successfull.
+        False if the handler is busy. True if the handling of the request was finished successfully.
         """
         if self.states.state != CfdpState.IDLE:
             _LOGGER.debug("CFDP source handler is busy, can't process put request")
@@ -483,7 +483,7 @@ def transaction_id(self) -> TransactionId | None:
         return self._params.transaction_id
 
     def _reset_internal(self, clear_packet_queue: bool) -> None:
-        """This function is public to allow completely resetting the handler, but it is explicitely
+        """This function is public to allow completely resetting the handler, but it is explicitly
         discouraged to do this. CFDP generally has mechanism to detect issues and errors on itself.
         """
         self.states.step = TransactionStep.IDLE
@@ -493,7 +493,7 @@ def _reset_internal(self, clear_packet_queue: bool) -> None:
         self._params.reset()
 
     def reset(self) -> None:
-        """This function is public to allow completely resetting the handler, but it is explicitely
+        """This function is public to allow completely resetting the handler, but it is explicitly
         discouraged to do this. CFDP generally has mechanism to detect issues and errors on itself.
         """
         self._reset_internal(True)
@@ -544,7 +544,7 @@ def _transaction_start(self) -> None:
     def _check_for_originating_id(self) -> TransactionId | None:
         """This function only returns an originating ID for if not proxy put response is
         contained in the message to user list. This special logic is in place to avoid permanent
-        loop which would occur when the user uses the orignating ID to register active proxy put
+        loop which would occur when the user uses the originating ID to register active proxy put
         request, and this ID would also be generated for proxy put responses."""
         contains_proxy_put_response = False
         contains_originating_id = False
@@ -952,7 +952,7 @@ def _declare_fault(self, cond: ConditionCode) -> None:
         self.cfg.default_fault_handlers.report_fault(transaction_id, cond, progress)
 
     def _notice_of_cancellation(self, condition_code: ConditionCode) -> bool:
-        """Returns whether the fault declaration handler can returns prematurely."""
+        """Returns whether the fault declaration handler can return prematurely."""
         # CFDP standard 4.11.2.2.3: Any fault declared in the course of transferring
         # the EOF (cancel) PDU must result in abandonment of the transaction.
         if (
@@ -980,7 +980,7 @@ def _notice_of_suspension(self) -> None:
         pass
 
     def _abandon_transaction(self) -> None:
-        # I guess an abandoned transaction just stops whatever it is doing.. The implementation
+        # I guess an abandoned transaction just stops whatever it is doing. The implementation
         # for this is quite easy.
         self.reset()
 

src/cfdppy/mib.py

@@ -181,7 +181,7 @@ class RemoteEntityCfg:
     **Notes on Positive Acknowledgment Procedures**
 
     The ``positive_ack_timer_interval_seconds`` and ``positive_ack_timer_expiration_limit`` will
-    be used for positive acknowledgement procedures as specified in CFDP chapter 4.7. The sending
+    be used for positive acknowledgment procedures as specified in CFDP chapter 4.7. The sending
     entity will start the timer for any PDUs where an acknowledgment is required (e.g. EOF PDU).
     Once the expected ACK response has not been received for that interval, as counter will be
     incremented and the timer will be reset. Once the counter exceeds the