change layering

mission/com/TmStoreTaskBase.cpp

@@ -14,6 +14,7 @@ TmStoreTaskBase::TmStoreTaskBase(object_id_t objectId, StorageManagerIF& ipcStor
     : SystemObject(objectId),
       modeHelper(this),
       ipcStore(ipcStore),
+      tmReader(&timeReader),
       channel(channel),
       sdcMan(sdcMan) {
   requestQueue = QueueFactory::instance()->createMessageQueue(10);
@@ -34,7 +35,7 @@ bool TmStoreTaskBase::handleOneStore(PersistentTmStoreWithTmQueue& store,
   if (result == returnvalue::OK) {
     tmToStoreReceived = true;
   }
-  // Dump TMs when applicable and allowed (mode is on)
+  // Dump TMs
   if (store.getState() == PersistentTmStore::State::DUMPING) {
     if (handleOneDump(store, dumpContext, dumpPerformed) != returnvalue::OK) {
       return result;
@@ -93,35 +94,13 @@ ReturnValue_t TmStoreTaskBase::handleOneDump(PersistentTmStoreWithTmQueue& store
   ReturnValue_t result = returnvalue::OK;
   // The PTME might have been reset an transmitter state change, so there is no point in continuing
   // the dump.
+  // TODO: Will be solved in a cleaner way, this is kind of a hack.
   if (not channel.isTxOn()) {
     cancelDump(dumpContext, store, false);
     return returnvalue::FAILED;
   }
-  size_t dumpedLen = 0;
   if (not channel.isBusy()) {
-    // Dump the next packet into the PTME.
+    performDump(store, dumpContext, dumpPerformed);
-    dumpContext.ptmeBusyCounter = 0;
-    tmSinkBusyCd.resetTimer();
-    result = store.dumpNextPacket(channel, dumpedLen, fileHasSwapped);
-    if (result == DirectTmSinkIF::IS_BUSY) {
-      sif::warning << "Unexpected PAPB busy" << std::endl;
-    }
-    if ((result == PersistentTmStore::DUMP_DONE or result == returnvalue::OK)) {
-      dumpPerformed = true;
-      if (dumpedLen > 0) {
-        dumpContext.dumpedBytes += dumpedLen;
-        dumpContext.numberOfDumpedPackets += 1;
-        dumpContext.packetWasDumped = true;
-      }
-    }
-    if (result == PersistentTmStore::DUMP_DONE) {
-      uint32_t startTime;
-      uint32_t endTime;
-      store.getStartAndEndTimeCurrentOrLastDump(startTime, endTime);
-      triggerEvent(dumpContext.eventIfDone, dumpContext.numberOfDumpedPackets,
-                   dumpContext.dumpedBytes);
-      dumpContext.reset();
-    }
   } else {
     // The PTME might be at full load, so it might sense to delay for a bit to let it do
     // its work until some more bandwidth is available. Set a flag here so the upper layer can
@@ -140,6 +119,55 @@ ReturnValue_t TmStoreTaskBase::handleOneDump(PersistentTmStoreWithTmQueue& store
   return result;
 }
 
+ReturnValue_t TmStoreTaskBase::performDump(PersistentTmStoreWithTmQueue& store,
+                                           DumpContext& dumpContext, bool& dumpPerformed) {
+  size_t dumpedLen = 0;
+
+  auto dumpDoneHandler = [&]() {
+    uint32_t startTime;
+    uint32_t endTime;
+    store.getStartAndEndTimeCurrentOrLastDump(startTime, endTime);
+    triggerEvent(dumpContext.eventIfDone, dumpContext.numberOfDumpedPackets,
+                 dumpContext.dumpedBytes);
+    dumpContext.reset();
+  };
+  // Dump the next packet into the PTME.
+  dumpContext.ptmeBusyCounter = 0;
+  tmSinkBusyCd.resetTimer();
+  ReturnValue_t result = store.getNextDumpPacket(tmReader, fileHasSwapped);
+  if (result != returnvalue::OK) {
+    sif::error << "PersistentTmStore: Getting next dump packet failed" << std::endl;
+  } else if (fileHasSwapped or result == PersistentTmStore::DUMP_DONE) {
+    // This can happen if a file is corrupted and the next file swap completes the dump.
+    dumpDoneHandler();
+    return returnvalue::OK;
+  }
+  // Only write to VC if mode is on, but always confirm the dump.
+  // If the mode is OFF, it is assumed the PTME is not usable and is not allowed to be written
+  // (e.g. to confirm a reset or the transmitter is off anyway).
+  if (mode == MODE_ON) {
+    result = channel.write(tmReader.getFullData(), tmReader.getFullPacketLen());
+    if (result == DirectTmSinkIF::IS_BUSY) {
+      sif::warning << "PersistentTmStore: Unexpected VC channel busy" << std::endl;
+    } else if (result != returnvalue::OK) {
+      sif::warning << "PersistentTmStore: Unexpected VC channel write failure" << std::endl;
+    }
+  }
+  result = store.confirmDump(tmReader, fileHasSwapped);
+  if ((result == PersistentTmStore::DUMP_DONE or result == returnvalue::OK)) {
+    dumpPerformed = true;
+    if (dumpedLen > 0) {
+      dumpContext.dumpedBytes += dumpedLen;
+      dumpContext.numberOfDumpedPackets += 1;
+      dumpContext.packetWasDumped = true;
+    }
+  }
+  if (result == PersistentTmStore::DUMP_DONE) {
+    dumpDoneHandler();
+  }
+  return returnvalue::OK;
+}
+
 ReturnValue_t TmStoreTaskBase::initialize() {
   modeHelper.initialize();
   return returnvalue::OK;

mission/com/TmStoreTaskBase.h

@@ -45,16 +45,20 @@ class TmStoreTaskBase : public SystemObject,
   ReturnValue_t connectModeTreeParent(HasModeTreeChildrenIF& parent) override;
 
  protected:
-  MessageQueueIF* requestQueue;
   ModeHelper modeHelper;
+  MessageQueueIF* requestQueue;
   StorageManagerIF& ipcStore;
+  PusTmReader tmReader;
+  CdsShortTimeStamper timeReader;
+  VirtualChannel& channel;
+
   Mode_t mode = HasModesIF::MODE_OFF;
   Countdown sdCardCheckCd = Countdown(800);
   // 20 minutes are allowed as maximum dump time.
   Countdown cancelDumpCd = Countdown(60 * 20 * 1000);
   // If the TM sink is busy for 1 minute for whatever reason, cancel the dump.
   Countdown tmSinkBusyCd = Countdown(60 * 1000);
-  VirtualChannel& channel;
+
   bool storesInitialized = false;
   bool fileHasSwapped = false;
   SdCardMountedIF& sdcMan;
@@ -75,6 +79,8 @@ class TmStoreTaskBase : public SystemObject,
 
   ReturnValue_t handleOneDump(PersistentTmStoreWithTmQueue& store, DumpContext& dumpContext,
                               bool& dumpPerformed);
+  ReturnValue_t performDump(PersistentTmStoreWithTmQueue& store, DumpContext& dumpContext,
+                            bool& dumpPerformed);
 
   /**
    * Occasionally check whether SD card is okay to be used. If not, poll whether it is ready to

mission/tmtc/PersistentTmStore.cpp

@@ -251,41 +251,37 @@ ReturnValue_t PersistentTmStore::loadNextDumpFile() {
   return DUMP_DONE;
 }
 
-ReturnValue_t PersistentTmStore::dumpNextPacket(DirectTmSinkIF& tmSink, size_t& dumpedLen,
+ReturnValue_t PersistentTmStore::getNextDumpPacket(PusTmReader& reader, bool& fileHasSwapped) {
-                                                bool& fileHasSwapped) {
+  if (state == State::IDLE or dumpParams.pendingPacketDump) {
-  if (state == State::IDLE) {
     return returnvalue::FAILED;
   }
-  PusTmReader reader(&timeReader, fileBuf.data() + dumpParams.currentSize,
+  reader.setReadOnlyData(fileBuf.data() + dumpParams.currentSize,
-                     fileBuf.size() - dumpParams.currentSize);
+                         fileBuf.size() - dumpParams.currentSize);
   // CRC check to fully ensure this is a valid TM
   ReturnValue_t result = reader.parseDataWithCrcCheck();
-  if (result == returnvalue::OK) {
+  if (result != returnvalue::OK) {
-    result = tmSink.write(fileBuf.data() + dumpParams.currentSize, reader.getFullPacketLen());
-    if (result == DirectTmSinkIF::IS_BUSY) {
-      return result;
-    } else if (result != returnvalue::OK) {
-      // TODO: Event?
-      sif::error << "PersistentTmStore: Writing to TM sink failed" << std::endl;
-      return result;
-    }
-    dumpParams.currentSize += reader.getFullPacketLen();
-    dumpedLen = reader.getFullPacketLen();
-    if (dumpParams.currentSize >= dumpParams.fileSize) {
-      fileHasSwapped = true;
-      return loadNextDumpFile();
-    }
-  } else {
     sif::error << "PersistentTmStore: Parsing of PUS TM failed with code " << result << std::endl;
     triggerEvent(persTmStore::POSSIBLE_FILE_CORRUPTION, result, dumpParams.currentFileUnixStamp);
     // Delete the file and load next. Could use better algorithm to partially
     // restore the file dump, but for now do not trust the file.
-    dumpedLen = 0;
     std::error_code e;
     std::filesystem::remove(dumpParams.dirEntry.path().c_str(), e);
     fileHasSwapped = true;
     return loadNextDumpFile();
   }
+  fileHasSwapped = false;
+  dumpParams.pendingPacketDump = true;
+  return returnvalue::OK;
+}
+
+ReturnValue_t PersistentTmStore::confirmDump(const PusTmReader& reader, bool& fileHasSwapped) {
+  dumpParams.pendingPacketDump = false;
+  dumpParams.currentSize += reader.getFullPacketLen();
+  if (dumpParams.currentSize >= dumpParams.fileSize) {
+    fileHasSwapped = true;
+    return loadNextDumpFile();
+  }
+  fileHasSwapped = false;
   return returnvalue::OK;
 }
 

mission/tmtc/PersistentTmStore.h

@@ -4,12 +4,10 @@
 #include <fsfw/ipc/CommandMessageIF.h>
 #include <fsfw/objectmanager/SystemObject.h>
 #include <fsfw/storagemanager/StorageManagerIF.h>
-#include <fsfw/timemanager/CdsShortTimeStamper.h>
 #include <fsfw/tmstorage/TmStoreFrontendSimpleIF.h>
 #include <fsfw/tmtcpacket/pus/tm/PusTmReader.h>
 #include <fsfw/tmtcservices/AcceptsTelemetryIF.h>
 #include <mission/memory/SdCardMountedIF.h>
-#include <mission/tmtc/DirectTmSinkIF.h>
 
 #include <filesystem>
 
@@ -57,13 +55,25 @@ class PersistentTmStore : public TmStoreFrontendSimpleIF, public SystemObject {
   ReturnValue_t startDumpFromUpTo(uint32_t fromUnixSeconds, uint32_t upToUnixSeconds);
   /**
    *
-   * @param tmSink
+   * @param tmReader: Next packet will be loaded into the PUS TM reader. A CRC check will be
-   * @param dumpedLen
+   *    performed on the packet. If that check fails, the file is considered corrupted and will
-   * @param fileHasSwapped
+   *    be deleted for now.
-   * @return DUMP_DONE if dump is finished, returnvalue::OK if dump of next packet was a success,
+   * @param fileHasSwapped: If the CRC check fails, the file will be deleted and a new one has to
-   *   and DirectTmSinkIF::IS_BUSY is TM sink is busy.
+   *    be loaded. The dump can reach completion during that process. If a file is swapped, this
+   *    boolean is set to true
+   * @return DUMP_DONE if dump is finished, returnvalue::OK if the next packet was loaded into the
+   *    TM reader, and the returnvalue of the file swap operation if the CRC check failed and
+   *    a new file was loaded.
    */
-  ReturnValue_t dumpNextPacket(DirectTmSinkIF& tmSink, size_t& dumpedLen, bool& fileHasSwapped);
+  ReturnValue_t getNextDumpPacket(PusTmReader& tmReader, bool& fileHasSwapped);
+  /**
+   * Confirm the dump to advance the dump state machine.
+   * @param tmReader
+   * @param fileHasSwapped: If the confirmed dumps completes the current file, a new file will
+   *    be loaded and this parameter will be set to true.
+   * @return If a file is swapped, the retrunvalue of the file swap operation.
+   */
+  ReturnValue_t confirmDump(const PusTmReader& tmReader, bool& fileHasSwapped);
 
   void getStartAndEndTimeCurrentOrLastDump(uint32_t& startTime, uint32_t& endTime) const;
   ReturnValue_t storePacket(PusTmReader& reader);
@@ -83,8 +93,6 @@ class PersistentTmStore : public TmStoreFrontendSimpleIF, public SystemObject {
 
   MessageQueueIF* tcQueue;
   State state = State::IDLE;
-  // PacketFilter filter;
-  CdsShortTimeStamper timeReader;
   bool baseDirUninitialized = true;
   const char* baseDir;
   std::string baseName;
@@ -96,6 +104,7 @@ class PersistentTmStore : public TmStoreFrontendSimpleIF, public SystemObject {
   timeval activeFileTv{};
 
   struct ActiveDumpParams {
+    bool pendingPacketDump = false;
     uint32_t fromUnixTime = 0;
     uint32_t untilUnixTime = 0;
     uint32_t currentFileUnixStamp = 0;