Merge pull request 'bugfix for virt channel: clear invalid state' (#827) from virt-channel-bugfix into main

Reviewed-on: #827
Reviewed-by: Marius Eggert <eggertm@irs.uni-stuttgart.de>

CHANGELOG.md

@@ -23,6 +23,10 @@ will consitute of a breaking change warranting a new major release:
 
 ## Fixed
 
+- Increase allowed time for PTME writers to finish partial transfers. A duration of 200 ms was
+  not sufficient for cases where 3 writers write concurrently.
+- Fixed state issue for PTME writer object where the writer was not reset properly after a timeout
+  of a partial transfer. This was a major bug blocking the whole VC if it occured.
 - STR config path was previously hardcoded to `/mnt/sd0/startracker/flight-config.json`.
   A new abstraction was introduces which now uses the active SD card to build the correct
   config path when initializing the star tracker.

linux/ipcore/PapbVcInterface.cpp

@@ -30,6 +30,7 @@ ReturnValue_t PapbVcInterface::initialize() {
 ReturnValue_t PapbVcInterface::write(const uint8_t* data, size_t size, size_t& writtenSize) {
   // There are no packets smaller than 4, this is considered a configuration error.
   if (size < 4) {
+    sif::warning << "PapbVcInterface::write: Passed packet smaller than 4 bytes" << std::endl;
     return returnvalue::FAILED;
   }
   // The user must call advance until completion before starting a new packet transfer.
@@ -83,6 +84,9 @@ ReturnValue_t PapbVcInterface::advanceWrite(size_t& writtenSize) {
     writtenSize++;
   }
   if (not pollReadyForOctet(MAX_BUSY_POLLS)) {
+    if (not pollReadyForPacket()) {
+      return PARTIALLY_WRITTEN;
+    }
     abortPacketTransfer();
     return returnvalue::FAILED;
   }

mission/com/LiveTmTask.cpp

@@ -54,8 +54,13 @@ ReturnValue_t LiveTmTask::performOperation(uint8_t opCode) {
           }
         }
       } else if (result != MessageQueueIF::EMPTY) {
-        sif::warning << "LiveTmTask: TM queue failure, returncode 0x" << std::hex << std::setw(4)
-                     << result << std::dec << std::endl;
+        const char* contextStr = "Regular TM queue";
+        if (isCfdp) {
+          contextStr = "CFDP TM queue";
+        }
+        sif::warning << "LiveTmTask: " << contextStr << " handling failure, returncode 0x"
+                     << std::setfill('0') << std::hex << std::setw(4) << result << std::dec
+                     << std::endl;
       }
     }
 
@@ -173,15 +178,16 @@ ReturnValue_t LiveTmTask::handleGenericTmQueue(MessageQueueIF& queue, bool isCfd
     size_t writtenSize = 0;
     result = channel.write(data, size, writtenSize);
     if (result == DirectTmSinkIF::PARTIALLY_WRITTEN) {
-      result = channel.handleWriteCompletionSynchronously(writtenSize, 200);
+      result = channel.handleWriteCompletionSynchronously(writtenSize, 400);
       if (result != returnvalue::OK) {
         // TODO: Event? Might lead to dangerous spam though..
         sif::warning << "LiveTmTask: Synchronous write of last segment failed with code 0x"
-                     << std::setw(4) << std::hex << result << std::dec << std::endl;
+                     << std::setfill('0') << std::setw(4) << std::hex << result << std::dec
+                     << std::endl;
       }
     } else if (result != returnvalue::OK) {
-      sif::error << "LiveTmTask: Channel write failed with code 0x" << std::hex << std::setw(4)
-                 << result << std::dec << std::endl;
+      sif::error << "LiveTmTask: Channel write failed with code 0x" << std::setfill('0') << std::hex
+                 << std::setw(4) << result << std::dec << std::endl;
     }
   }
   // Try delete in any case, ignore failures (which should not happen), it is more important to

mission/com/TmStoreTaskBase.cpp

@@ -141,11 +141,12 @@ ReturnValue_t TmStoreTaskBase::performDump(PersistentTmStoreWithTmQueue& store,
   size_t writtenSize = 0;
   result = channel.write(tmReader.getFullData(), dumpedLen, writtenSize);
   if (result == VirtualChannelIF::PARTIALLY_WRITTEN) {
-    result = channel.handleWriteCompletionSynchronously(writtenSize, 200);
+    result = channel.handleWriteCompletionSynchronously(writtenSize, 400);
     if (result != returnvalue::OK) {
       // TODO: Event? Might lead to dangerous spam though..
-      sif::warning << "PersistentTmStore: Synchronous write of last segment failed with code 0x"
-                   << std::setw(4) << std::hex << result << std::dec << std::endl;
+      sif::warning << "LiveTmTask: Synchronous write of last segment failed with code 0x"
+                   << std::setfill('0') << std::setw(4) << std::hex << result << std::dec
+                   << std::endl;
     }
   } else if (result == DirectTmSinkIF::IS_BUSY) {
     sif::warning << "PersistentTmStore: Unexpected VC channel busy" << std::endl;

mission/com/VirtualChannel.cpp

@@ -74,5 +74,7 @@ ReturnValue_t VirtualChannel::handleWriteCompletionSynchronously(size_t& written
       return result;
     }
   }
-  return returnvalue::FAILED;
+  // Timeout. Cancel the transfer
+  cancelTransfer();
+  return TIMEOUT;
 }

mission/com/VirtualChannelWithQueue.cpp

@@ -41,7 +41,7 @@ ReturnValue_t VirtualChannelWithQueue::handleNextTm(bool performWriteOp) {
   if (performWriteOp) {
     result = write(data, size, writtenSize);
     if (result == PARTIALLY_WRITTEN) {
-      result = handleWriteCompletionSynchronously(writtenSize, 200);
+      result = handleWriteCompletionSynchronously(writtenSize, 400);
       if (result != returnvalue::OK) {
         // TODO: Event? Might lead to dangerous spam though..
         sif::warning

mission/tmtc/DirectTmSinkIF.h

@@ -15,6 +15,7 @@ class DirectTmSinkIF {
   static constexpr ReturnValue_t IS_BUSY = returnvalue::makeCode(CLASS_ID, 0);
   static constexpr ReturnValue_t PARTIALLY_WRITTEN = returnvalue::makeCode(CLASS_ID, 1);
   static constexpr ReturnValue_t NO_WRITE_ACTIVE = returnvalue::makeCode(CLASS_ID, 2);
+  static constexpr ReturnValue_t TIMEOUT = returnvalue::makeCode(CLASS_ID, 3);
 
   /**
    * @brief   Implements the functionality to write to a TM sink directly.