diff --git a/.github/actions/spelling/expect.txt b/.github/actions/spelling/expect.txt index d9fe23d0..14e4982f 100644 --- a/.github/actions/spelling/expect.txt +++ b/.github/actions/spelling/expect.txt @@ -572,6 +572,7 @@ Smath SNDHWM socketserver sorttable +spam sphinxcontrib splitext squashify diff --git a/src/fprime_gds/common/files/downlinker.py b/src/fprime_gds/common/files/downlinker.py index 452114a0..45beae47 100644 --- a/src/fprime_gds/common/files/downlinker.py +++ b/src/fprime_gds/common/files/downlinker.py @@ -52,7 +52,6 @@ def __init__(self, directory, timeout=20.0, log_dir=None): self.sequence = 0 # Keeps track of what the current sequence ID should be self.timer = fprime_gds.common.files.helpers.Timeout() self.timer.setup(self.timeout, timeout) - os.makedirs(self.__directory, exist_ok=True) def data_callback(self, data, sender=None): """ @@ -96,7 +95,15 @@ def handle_start(self, data): size, self.__log_dir, ) - self.active.open(TransmitFileState.WRITE) + try: + self.active.open(TransmitFileState.WRITE) + except PermissionError as exc: + self.state = FileStates.ERROR + LOGGER.warning( + "Unable to open file for writing. Discarding subsequent downlink packets. " + + str(exc) + ) + return LOGGER.addHandler(self.active.log_handler) message = "Received START packet with metadata:\n" message += "\tSize: %d\n" @@ -116,7 +123,10 @@ def handle_data(self, data): # Initialize all relevant DATA packet attributes into variables from file_data offset = data.offset if self.state != FileStates.RUNNING: - LOGGER.warning("Received unexpected data packet for offset: %d", offset) + # ERROR state means GDS is not ready to receive data packets (permission error) + # No need to log this, as it is already logged in handle_start and would only spam logs + if self.state != FileStates.ERROR: + LOGGER.warning("Received unexpected data packet for offset: %d", offset) else: if data.seqID != self.sequence: LOGGER.warning( @@ -156,9 +166,10 @@ def handle_end(self, data): """ # Initialize all relevant END packet attributes into variables from file_data # hashValue attribute is not relevant right now, but might be in the future - if self.state != FileStates.RUNNING: - LOGGER.warning("Received unexpected END packet") - else: + if self.state == FileStates.ERROR: + LOGGER.info("Received END packet for discarded downlink") + self.finish() + elif self.state == FileStates.RUNNING: if data.seqID != self.sequence: LOGGER.warning( "End packet has unexpected sequence id. Expected: %d got %d", @@ -167,6 +178,8 @@ def handle_end(self, data): ) LOGGER.info("Received END packet, finishing downlink") self.finish() + else: + LOGGER.warning("Received unexpected END packet") def timeout(self): """Timeout the current downlink""" diff --git a/src/fprime_gds/common/files/helpers.py b/src/fprime_gds/common/files/helpers.py index ee983615..1b588fc2 100644 --- a/src/fprime_gds/common/files/helpers.py +++ b/src/fprime_gds/common/files/helpers.py @@ -77,6 +77,7 @@ class FileStates(enum.Enum): RUNNING = 1 CANCELED = 2 END_WAIT = 3 # Waiting for the handshake for CANCEL or END packet + ERROR = 4 class CFDPChecksum: diff --git a/src/fprime_gds/common/pipeline/files.py b/src/fprime_gds/common/pipeline/files.py index 43542192..f88b475c 100644 --- a/src/fprime_gds/common/pipeline/files.py +++ b/src/fprime_gds/common/pipeline/files.py @@ -7,6 +7,7 @@ @author mstarch """ +import os import fprime_gds.common.files.downlinker import fprime_gds.common.files.uplinker @@ -27,7 +28,8 @@ def setup_file_handling( self, down_store, file_encoder, file_decoder, distributor, log_dir ): """ - Sets up the file handling (uplink and downlink) from a pair of encoders and decoders + Sets up the file handling (uplink and downlink) from a pair of encoders and decoders. + Raises a PermissionError if the down_store is not writable. :param down_store: downlink storage directory :param file_encoder: file encoder for uplink @@ -41,6 +43,11 @@ def setup_file_handling( ) file_decoder.register(self.__downlinker) distributor.register("FW_PACKET_HAND", self.__uplinker) + if not os.access(down_store, os.W_OK): + raise PermissionError( + f"{down_store} is not writable. Downlinker not be able to save files. " + "Fix permissions or change storage directory with --file-storage-directory." + ) @property def uplinker(self):