more simplfications

src/fsfw/devicehandlers/DeviceHandlerBase.cpp

@@ -8,6 +8,7 @@
 #include "fsfw/ipc/MessageQueueMessage.h"
 #include "fsfw/ipc/QueueFactory.h"
 #include "fsfw/objectmanager/ObjectManager.h"
+#include "fsfw/serialize/SerialBufferAdapter.h"
 #include "fsfw/serviceinterface/ServiceInterface.h"
 #include "fsfw/storagemanager/StorageManagerIF.h"
 #include "fsfw/subsystem/SubsystemBase.h"
@@ -1269,40 +1270,33 @@ ReturnValue_t DeviceHandlerBase::letChildHandleMessage(CommandMessage* message)
   return returnvalue::FAILED;
 }
 
-void DeviceHandlerBase::handleDeviceTm(util::DataWrapper dataWrapper, DeviceCommandId_t replyId,
+void DeviceHandlerBase::handleDeviceTm(const uint8_t* rawData, size_t rawDataLen,
-                                       bool forceDirectTm) {
+                                       DeviceCommandId_t replyId, bool forceDirectTm) {
-  if (dataWrapper.isNull()) {
+  SerialBufferAdapter bufferWrapper(rawData, rawDataLen);
-    return;
+  handleDeviceTm(bufferWrapper, replyId, forceDirectTm);
 }
 
+void DeviceHandlerBase::handleDeviceTm(const SerializeIF& dataSet, DeviceCommandId_t replyId,
+                                       bool forceDirectTm) {
   auto iter = deviceReplyMap.find(replyId);
   if (iter == deviceReplyMap.end()) {
     triggerEvent(DEVICE_UNKNOWN_REPLY, replyId);
     return;
   }
 
-  auto reportData = [&](MessageQueueId_t queueId) {
-    if (dataWrapper.type == util::DataTypes::SERIALIZABLE) {
-      return actionHelper.reportData(queueId, replyId, dataWrapper.dataUnion.serializable);
-    } else if (dataWrapper.type == util::DataTypes::RAW) {
-      return actionHelper.reportData(queueId, replyId, dataWrapper.dataUnion.raw.data,
-                                     dataWrapper.dataUnion.raw.len);
-    }
-    return returnvalue::FAILED;
-  };
-
   // Regular replies to a command
   if (iter->second.command != deviceCommandMap.end()) {
     MessageQueueId_t queueId = iter->second.command->second.sendReplyTo;
 
     // This may fail, but we'll ignore the fault.
     if (queueId != NO_COMMANDER) {
-      reportData(queueId);
+      // This may fail, but we'll ignore the fault.
+      actionHelper.reportData(queueId, replyId, const_cast<SerializeIF*>(&dataSet));
     }
 
     // This check should make sure we get any TM but don't get anything doubled.
     if (wiretappingMode == TM && (requestedRawTraffic != queueId)) {
-      DeviceTmReportingWrapper wrapper(getObjectId(), replyId, dataWrapper);
+      DeviceTmReportingWrapper wrapper(getObjectId(), replyId, dataSet);
       actionHelper.reportData(requestedRawTraffic, replyId, &wrapper);
     }
 
@@ -1311,12 +1305,13 @@ void DeviceHandlerBase::handleDeviceTm(util::DataWrapper dataWrapper, DeviceComm
       // hiding of sender needed so the service will handle it as
       // unexpected Data, no matter what state (progress or completed)
       // it is in
-      reportData(defaultRawReceiver);
+      actionHelper.reportData(defaultRawReceiver, replyId, const_cast<SerializeIF*>(&dataSet),
+                              true);
     }
   }
   // Unrequested or aperiodic replies
   else {
-    DeviceTmReportingWrapper wrapper(getObjectId(), replyId, dataWrapper);
+    DeviceTmReportingWrapper wrapper(getObjectId(), replyId, dataSet);
     if (wiretappingMode == TM) {
       actionHelper.reportData(requestedRawTraffic, replyId, &wrapper);
     }
@@ -1336,18 +1331,22 @@ ReturnValue_t DeviceHandlerBase::executeAction(ActionId_t actionId, MessageQueue
     return result;
   }
   DeviceCommandMap::iterator iter = deviceCommandMap.find(actionId);
+  MessageQueueId_t prevRecipient = MessageQueueIF::NO_QUEUE;
   if (iter == deviceCommandMap.end()) {
     result = COMMAND_NOT_SUPPORTED;
   } else if (iter->second.isExecuting) {
     result = COMMAND_ALREADY_SENT;
   } else {
+    prevRecipient = iter->second.sendReplyTo;
+    iter->second.sendReplyTo = commandedBy;
     result = buildCommandFromCommand(actionId, data, size);
   }
   if (result == returnvalue::OK) {
-    iter->second.sendReplyTo = commandedBy;
     iter->second.isExecuting = true;
     cookieInfo.pendingCommand = iter;
     cookieInfo.state = COOKIE_WRITE_READY;
+  } else {
+    iter->second.sendReplyTo = prevRecipient;
   }
   return result;
 }

src/fsfw/devicehandlers/DeviceHandlerBase.h

@@ -1070,7 +1070,9 @@ class DeviceHandlerBase : public DeviceHandlerIF,
 
   bool isAwaitingReply();
 
-  void handleDeviceTm(util::DataWrapper dataWrapper, DeviceCommandId_t replyId,
+  void handleDeviceTm(const uint8_t *rawData, size_t rawDataLen, DeviceCommandId_t replyId,
+                      bool forceDirectTm = false);
+  void handleDeviceTm(const SerializeIF &dataSet, DeviceCommandId_t replyId,
                       bool forceDirectTm = false);
 
   virtual ReturnValue_t checkModeCommand(Mode_t mode, Submode_t submode,

src/fsfw/devicehandlers/DeviceTmReportingWrapper.cpp

@@ -3,8 +3,8 @@
 #include "fsfw/serialize/SerializeAdapter.h"
 
 DeviceTmReportingWrapper::DeviceTmReportingWrapper(object_id_t objectId, ActionId_t actionId,
-                                                   util::DataWrapper data)
+                                                   const SerializeIF& data)
-    : objectId(objectId), actionId(actionId), dataWrapper(data) {}
+    : objectId(objectId), actionId(actionId), data(data) {}
 
 DeviceTmReportingWrapper::~DeviceTmReportingWrapper() = default;
 
@@ -19,24 +19,11 @@ ReturnValue_t DeviceTmReportingWrapper::serialize(uint8_t** buffer, size_t* size
   if (result != returnvalue::OK) {
     return result;
   }
-  if (dataWrapper.isNull()) {
+  return data.serialize(buffer, size, maxSize, streamEndianness);
-    return returnvalue::FAILED;
-  }
-  if (dataWrapper.type == util::DataTypes::SERIALIZABLE) {
-    return dataWrapper.dataUnion.serializable->serialize(buffer, size, maxSize, streamEndianness);
-  } else if (dataWrapper.type == util::DataTypes::RAW) {
-    if (*size + dataWrapper.dataUnion.raw.len > maxSize) {
-      return SerializeIF::BUFFER_TOO_SHORT;
-    }
-    std::memcpy(*buffer, dataWrapper.dataUnion.raw.data, dataWrapper.dataUnion.raw.len);
-    *buffer += dataWrapper.dataUnion.raw.len;
-    *size += dataWrapper.dataUnion.raw.len;
-  }
-  return returnvalue::OK;
 }
 
 size_t DeviceTmReportingWrapper::getSerializedSize() const {
-  return sizeof(objectId) + sizeof(ActionId_t) + dataWrapper.getLength();
+  return sizeof(objectId) + sizeof(ActionId_t) + data.getSerializedSize();
 }
 
 ReturnValue_t DeviceTmReportingWrapper::deSerialize(const uint8_t** buffer, size_t* size,

src/fsfw/devicehandlers/DeviceTmReportingWrapper.h

@@ -8,7 +8,7 @@
 
 class DeviceTmReportingWrapper : public SerializeIF {
  public:
-  DeviceTmReportingWrapper(object_id_t objectId, ActionId_t actionId, util::DataWrapper data);
+  DeviceTmReportingWrapper(object_id_t objectId, ActionId_t actionId, const SerializeIF& data);
   ~DeviceTmReportingWrapper() override;
 
   ReturnValue_t serialize(uint8_t** buffer, size_t* size, size_t maxSize,
@@ -19,7 +19,7 @@ class DeviceTmReportingWrapper : public SerializeIF {
  private:
   object_id_t objectId;
   ActionId_t actionId;
-  util::DataWrapper dataWrapper;
+  const SerializeIF& data;
 
   // Deserialization forbidden
   ReturnValue_t deSerialize(const uint8_t** buffer, size_t* size,

src/fsfw/osal/common/TcpTmTcServer.cpp

@@ -161,7 +161,7 @@ void TcpTmTcServer::handleServerOperation(socket_t& connSocket) {
 
   while (true) {
     ssize_t retval = recv(connSocket, reinterpret_cast<char*>(receptionBuffer.data()),
-                          receptionBuffer.capacity(), tcpConfig.tcpFlags);
+                          receptionBuffer.size(), tcpConfig.tcpFlags);
     if (retval == 0) {
       size_t availableReadData = ringBuffer.getAvailableReadData();
       if (availableReadData > lastRingBufferSize) {
@@ -335,31 +335,27 @@ ReturnValue_t TcpTmTcServer::handleTcRingBufferData(size_t availableReadData) {
   }
   ringBuffer.readData(receptionBuffer.data(), readAmount, true);
   const uint8_t* bufPtr = receptionBuffer.data();
-  const uint8_t** bufPtrPtr = &bufPtr;
+  SpacePacketParser::FoundPacketInfo info;
-  size_t startIdx = 0;
-  size_t foundSize = 0;
-  size_t readLen = 0;
-  while (readLen < readAmount) {
   if (spacePacketParser == nullptr) {
     return returnvalue::FAILED;
   }
-    result =
+  spacePacketParser->reset();
-        spacePacketParser->parseSpacePackets(bufPtrPtr, readAmount, startIdx, foundSize, readLen);
+  while (spacePacketParser->getAmountRead() < readAmount) {
+    result = spacePacketParser->parseSpacePackets(&bufPtr, readAmount, info);
     switch (result) {
       case (SpacePacketParser::NO_PACKET_FOUND):
       case (SpacePacketParser::SPLIT_PACKET): {
         break;
       }
       case (returnvalue::OK): {
-        result = handleTcReception(receptionBuffer.data() + startIdx, foundSize);
+        result = handleTcReception(receptionBuffer.data() + info.startIdx, info.sizeFound);
         if (result != returnvalue::OK) {
           status = result;
         }
       }
     }
-    ringBuffer.deleteData(foundSize);
+    ringBuffer.deleteData(info.sizeFound);
     lastRingBufferSize = ringBuffer.getAvailableReadData();
-    std::memset(receptionBuffer.data() + startIdx, 0, foundSize);
   }
   return status;
 }

src/fsfw/tcdistribution/PusDistributor.cpp

@@ -146,7 +146,7 @@ ReturnValue_t PusDistributor::initialize() {
 
 void PusDistributor::checkerFailurePrinter() const {
 #if FSFW_VERBOSE_LEVEL >= 1
-  const char* keyword = "unnamed error";
+  const char* keyword = "unnamed";
   if (tcStatus == tcdistrib::INCORRECT_CHECKSUM) {
     keyword = "checksum";
   } else if (tcStatus == tcdistrib::INCORRECT_PRIMARY_HEADER) {
@@ -157,6 +157,8 @@ void PusDistributor::checkerFailurePrinter() const {
     keyword = "incorrect secondary header";
   } else if (tcStatus == tcdistrib::INCOMPLETE_PACKET) {
     keyword = "incomplete packet";
+  } else if (tcStatus == tcdistrib::INVALID_SEC_HEADER_FIELD) {
+    keyword = "invalid secondary header field";
   }
 #if FSFW_CPP_OSTREAM_ENABLED == 1
   sif::warning << "PUSDistributor::handlePacket: Packet format invalid, " << keyword << " error"

src/fsfw/tmtcservices/SpacePacketParser.cpp

@@ -6,17 +6,9 @@
 SpacePacketParser::SpacePacketParser(std::vector<uint16_t> validPacketIds)
     : validPacketIds(validPacketIds) {}
 
-ReturnValue_t SpacePacketParser::parseSpacePackets(const uint8_t* buffer, const size_t maxSize,
-                                                   size_t& startIndex, size_t& foundSize) {
-  const uint8_t** tempPtr = &buffer;
-  size_t readLen = 0;
-  return parseSpacePackets(tempPtr, maxSize, startIndex, foundSize, readLen);
-}
-
 ReturnValue_t SpacePacketParser::parseSpacePackets(const uint8_t** buffer, const size_t maxSize,
-                                                   size_t& startIndex, size_t& foundSize,
+                                                   FoundPacketInfo& packetInfo) {
-                                                   size_t& readLen) {
+  if (buffer == nullptr or nextStartIdx > maxSize) {
-  if (buffer == nullptr or maxSize < 5) {
 #if FSFW_CPP_OSTREAM_ENABLED == 1
     sif::warning << "SpacePacketParser::parseSpacePackets: Frame invalid" << std::endl;
 #else
@@ -26,35 +18,32 @@ ReturnValue_t SpacePacketParser::parseSpacePackets(const uint8_t** buffer, const
   }
   const uint8_t* bufPtr = *buffer;
 
-  auto verifyLengthField = [&](size_t idx) {
+  auto verifyLengthField = [&](size_t localIdx) {
-    uint16_t lengthField = bufPtr[idx + 4] << 8 | bufPtr[idx + 5];
+    uint16_t lengthField = (bufPtr[localIdx + 4] << 8) | bufPtr[localIdx + 5];
     size_t packetSize = lengthField + 7;
-    startIndex = idx;
     ReturnValue_t result = returnvalue::OK;
-    if (lengthField == 0) {
+    if (packetSize + localIdx + amountRead > maxSize) {
-      // Skip whole header for now
-      foundSize = 6;
-      result = NO_PACKET_FOUND;
-    } else if (packetSize + idx > maxSize) {
       // Don't increment buffer and read length here, user has to decide what to do
-      foundSize = packetSize;
+      packetInfo.sizeFound = packetSize;
       return SPLIT_PACKET;
     } else {
-      foundSize = packetSize;
+      packetInfo.sizeFound = packetSize;
     }
-    *buffer += foundSize;
+    *buffer += packetInfo.sizeFound;
-    readLen += idx + foundSize;
+    packetInfo.startIdx = localIdx + amountRead;
+    nextStartIdx = localIdx + amountRead + packetInfo.sizeFound;
+    amountRead = nextStartIdx;
     return result;
   };
 
   size_t idx = 0;
   // Space packet ID as start marker
   if (validPacketIds.size() > 0) {
-    while (idx < maxSize - 5) {
+    while (idx + amountRead < maxSize - 5) {
-      uint16_t currentPacketId = bufPtr[idx] << 8 | bufPtr[idx + 1];
+      uint16_t currentPacketId = (bufPtr[idx] << 8) | bufPtr[idx + 1];
       if (std::find(validPacketIds.begin(), validPacketIds.end(), currentPacketId) !=
           validPacketIds.end()) {
-        if (idx + 5 >= maxSize) {
+        if (idx + amountRead >= maxSize - 5) {
           return SPLIT_PACKET;
         }
         return verifyLengthField(idx);
@@ -62,10 +51,10 @@ ReturnValue_t SpacePacketParser::parseSpacePackets(const uint8_t** buffer, const
         idx++;
       }
     }
-    startIndex = 0;
+    nextStartIdx = maxSize;
-    foundSize = maxSize;
+    packetInfo.sizeFound = maxSize;
-    *buffer += foundSize;
+    amountRead = maxSize;
-    readLen += foundSize;
+    *buffer += maxSize;
     return NO_PACKET_FOUND;
   }
   // Assume that the user verified a valid start of a space packet

src/fsfw/tmtcservices/SpacePacketParser.h

@@ -17,9 +17,11 @@
  */
 class SpacePacketParser {
  public:
-  //! The first entry is the index inside the buffer while the second index
+
-  //! is the size of the PUS packet starting at that index.
+  struct FoundPacketInfo {
-  using IndexSizePair = std::pair<size_t, size_t>;
+    size_t startIdx = 0;
+    size_t sizeFound = 0;
+  };
 
   static constexpr uint8_t INTERFACE_ID = CLASS_ID::SPACE_PACKET_PARSER;
   static constexpr ReturnValue_t NO_PACKET_FOUND = MAKE_RETURN_CODE(0x00);
@@ -36,44 +38,32 @@ class SpacePacketParser {
   SpacePacketParser(std::vector<uint16_t> validPacketIds);
 
   /**
-   * Parse a given frame for space packets but also increment the given buffer and assign the
+   * Parse a given frame for space packets but also increments the given buffer.
-   * total number of bytes read so far
    * @param buffer        Parser will look for space packets in this buffer
    * @param maxSize       Maximum size of the buffer
-   * @param startIndex    Start index of a found space packet
+   * @param packetInfo    Information about packets found.
-   * @param foundSize     Found size of the space packet
+   *  -@c NO_PACKET_FOUND if no packet was found in the given buffer
-   * @param readLen       Length read so far. This value is incremented by the number of parsed
+   *  -@c SPLIT_PACKET if a packet was found but the detected size exceeds maxSize. packetInfo
-   *                      bytes which also includes the size of a found packet
+   *      will contain the detected packet size and start index.
-   *  -@c NO_PACKET_FOUND if no packet was found in the given buffer or the length field is
+   *  -@c returnvalue::OK if a packet was found. Packet size and start index will be set in
-   *      invalid. foundSize will be set to the size of the space packet header. buffer and
+   *      packetInfo
-   *      readLen will be incremented accordingly.
-   *  -@c SPLIT_PACKET if a packet was found but the detected size exceeds maxSize. foundSize
-   *      will be set to the detected packet size and startIndex will be set to the start of the
-   *      detected packet. buffer and read length will not be incremented but the found length
-   *      will be assigned.
-   *  -@c returnvalue::OK if a packet was found
    */
-  ReturnValue_t parseSpacePackets(const uint8_t** buffer, const size_t maxSize, size_t& startIndex,
+  ReturnValue_t parseSpacePackets(const uint8_t** buffer, const size_t maxSize,
-                                  size_t& foundSize, size_t& readLen);
+                                  FoundPacketInfo& packetInfo);
 
-  /**
+  size_t getAmountRead() {
-   * Parse a given frame for space packets
+    return amountRead;
-   * @param buffer        Parser will look for space packets in this buffer
+  }
-   * @param maxSize       Maximum size of the buffer
+
-   * @param startIndex    Start index of a found space packet
+  void reset() {
-   * @param foundSize     Found size of the space packet
+    nextStartIdx = 0;
-   *  -@c NO_PACKET_FOUND if no packet was found in the given buffer or the length field is
+    amountRead = 0;
-   *      invalid. foundSize will be set to the size of the space packet header
+  }
-   *  -@c SPLIT_PACKET if a packet was found but the detected size exceeds maxSize. foundSize
-   *      will be set to the detected packet size and startIndex will be set to the start of the
-   *      detected packet
-   *  -@c returnvalue::OK if a packet was found
-   */
-  ReturnValue_t parseSpacePackets(const uint8_t* buffer, const size_t maxSize, size_t& startIndex,
-                                  size_t& foundSize);
 
  private:
   std::vector<uint16_t> validPacketIds;
+  size_t nextStartIdx = 0;
+  size_t amountRead = 0;
 };
 
 #endif /* FRAMEWORK_TMTCSERVICES_PUSPARSER_H_ */

src/fsfw/util/dataWrapper.h

@@ -25,16 +25,11 @@ union DataUnion {
 };
 
 struct DataWrapper {
-
   DataWrapper() = default;
 
-  DataWrapper(const uint8_t* data, size_t size): type(DataTypes::RAW) {
+  DataWrapper(const uint8_t* data, size_t size) : type(DataTypes::RAW) { setRawData({data, size}); }
-    setRawData({data, size});
-  }
 
-  explicit DataWrapper(BufPair raw): type(DataTypes::RAW) {
+  explicit DataWrapper(BufPair raw) : type(DataTypes::RAW) { setRawData(raw); }
-    setRawData(raw);
-  }
 
   explicit DataWrapper(SerializeIF& serializable) : type(DataTypes::SERIALIZABLE) {
     setSerializable(serializable);

unittests/util/testDataWrapper.cpp

@@ -16,9 +16,7 @@ TEST_CASE("Data Wrapper", "[util]") {
     bool deleteInst = false;
     REQUIRE(wrapper.isNull());
     std::array<uint8_t, 4> data = {1, 2, 3, 4};
-    SECTION("Setter") {
+    SECTION("Setter") { wrapper.setRawData({data.data(), data.size()}); }
-      wrapper.setRawData({data.data(), data.size()});
-    }
     SECTION("Direct Construction Pair") {
       instance = new util::DataWrapper(util::BufPair(data.data(), data.size()));
       deleteInst = true;
@@ -41,9 +39,7 @@ TEST_CASE("Data Wrapper", "[util]") {
     bool deleteInst = false;
     REQUIRE(instance->isNull());
     SimpleSerializable serializable;
-    SECTION("Setter") {
+    SECTION("Setter") { wrapper.setSerializable(serializable); }
-      wrapper.setSerializable(serializable);
-    }
     SECTION("Direct Construction") {
       instance = new util::DataWrapper(serializable);
       deleteInst = true;