PUS TM creator tests done

src/fsfw/tmtcpacket/pus/CreatorDataIF.h

@@ -1,10 +0,0 @@
-#ifndef FSFW_TMTCPACKET_CREATORDATAIF_H
-#define FSFW_TMTCPACKET_CREATORDATAIF_H
-
-#include "defs.h"
-
-class CreatorDataIF {
- public:
-  virtual ecss::DataWrapper& getDataWrapper() = 0;
-};
-#endif  // FSFW_TMTCPACKET_CREATORDATAIF_H

src/fsfw/tmtcpacket/pus/CustomUserDataIF.h

@@ -0,0 +1,12 @@
+#ifndef FSFW_TMTCPACKET_CREATORDATAIF_H
+#define FSFW_TMTCPACKET_CREATORDATAIF_H
+
+#include "defs.h"
+
+class CustomUserDataIF {
+ public:
+  virtual ~CustomUserDataIF() = default;
+  virtual ReturnValue_t setRawUserData(const uint8_t* data, size_t len) = 0;
+  virtual ReturnValue_t setSerializableUserData(SerializeIF* serializable) = 0;
+};
+#endif  // FSFW_TMTCPACKET_CREATORDATAIF_H

src/fsfw/tmtcpacket/pus/defs.h

@@ -28,7 +28,7 @@ union DataUnion {
 struct DataWrapper {
   DataTypes type;
   DataUnion dataUnion;
-  using BufPairT = std::pair<uint8_t*, size_t>;
+  using BufPairT = std::pair<const uint8_t*, size_t>;
 
   [[nodiscard]] size_t getLength() const {
     if (type == DataTypes::RAW) {

src/fsfw/tmtcpacket/pus/tc/PusTcCreator.cpp

@@ -16,7 +16,7 @@ ReturnValue_t PusTcCreator::serialize(uint8_t **buffer, size_t *size, size_t max
                                       SerializeIF::Endianness streamEndianness) const {
   const uint8_t *start = *buffer;
   size_t userDataLen = pusParams.dataWrapper.getLength();
-  if (*size + getFullPacketLen() > maxSize) {
+  if (*size + getSerializedSize() > maxSize) {
     return SerializeIF::BUFFER_TOO_SHORT;
   }
   if (pusParams.pusVersion != ecss::PusVersion::PUS_C) {
@@ -86,8 +86,6 @@ uint8_t PusTcCreator::getSubService() const { return pusParams.subservice; }
 
 uint16_t PusTcCreator::getSourceId() const { return pusParams.sourceId; }
 
-ecss::DataWrapper &PusTcCreator::getDataWrapper() { return pusParams.dataWrapper; }
-
 PusTcParams &PusTcCreator::getPusParams() { return pusParams; }
 
 SpacePacketParams &PusTcCreator::getSpParams() { return spCreator.getParams(); }
@@ -96,12 +94,16 @@ ReturnValue_t PusTcCreator::serialize(uint8_t **buffer, size_t *size, size_t max
   return serialize(buffer, size, maxSize, SerializeIF::Endianness::NETWORK);
 }
 
-void PusTcCreator::setRawAppData(ecss::DataWrapper::BufPairT bufPair) {
+ReturnValue_t PusTcCreator::setRawUserData(const uint8_t *data, size_t len) {
-  pusParams.dataWrapper.setRawData(bufPair);
+  // TODO: Check length field?
+  pusParams.dataWrapper.setRawData({data, len});
   updateSpLengthField();
+  return HasReturnvaluesIF::RETURN_OK;
 }
 
-void PusTcCreator::setSerializableAppData(SerializeIF *serAppData) {
+ReturnValue_t PusTcCreator::setSerializableUserData(SerializeIF *serializable) {
-  pusParams.dataWrapper.setSerializable(serAppData);
+  // TODO: Check length field?
+  pusParams.dataWrapper.setSerializable(serializable);
   updateSpLengthField();
+  return HasReturnvaluesIF::RETURN_OK;
 }

src/fsfw/tmtcpacket/pus/tc/PusTcCreator.h

@@ -4,7 +4,7 @@
 #include "fsfw/tmtcpacket/RedirectableDataPointerIF.h"
 #include "fsfw/tmtcpacket/ccsds/SpacePacketCreator.h"
 #include "fsfw/tmtcpacket/ccsds/SpacePacketIF.h"
-#include "fsfw/tmtcpacket/pus/CreatorDataIF.h"
+#include "fsfw/tmtcpacket/pus/CustomUserDataIF.h"
 #include "fsfw/tmtcpacket/pus/defs.h"
 #include "fsfw/tmtcpacket/pus/tc/PusTcIF.h"
 
@@ -19,7 +19,7 @@ struct PusTcParams {
   uint8_t pusVersion = ecss::PusVersion::PUS_C;
 };
 
-class PusTcCreator : public PusTcIF, public SerializeIF, public CreatorDataIF {
+class PusTcCreator : public PusTcIF, public SerializeIF, public CustomUserDataIF {
  public:
   PusTcCreator(SpacePacketParams initSpParams, PusTcParams initPusParams);
 
@@ -33,8 +33,6 @@ class PusTcCreator : public PusTcIF, public SerializeIF, public CreatorDataIF {
   void updateSpLengthField();
   PusTcParams &getPusParams();
   SpacePacketParams &getSpParams();
-  void setRawAppData(ecss::DataWrapper::BufPairT bufPair);
-  void setSerializableAppData(SerializeIF *serAppData);
 
   ReturnValue_t serialize(uint8_t **buffer, size_t *size, size_t maxSize);
   [[nodiscard]] size_t getSerializedSize() const override;
@@ -48,7 +46,8 @@ class PusTcCreator : public PusTcIF, public SerializeIF, public CreatorDataIF {
   [[nodiscard]] uint8_t getService() const override;
   [[nodiscard]] uint8_t getSubService() const override;
   [[nodiscard]] uint16_t getSourceId() const override;
-  ecss::DataWrapper &getDataWrapper() override;
+  ReturnValue_t setRawUserData(const uint8_t *data, size_t len) override;
+  ReturnValue_t setSerializableUserData(SerializeIF *serializable) override;
 
  private:
   ReturnValue_t serialize(uint8_t **buffer, size_t *size, size_t maxSize,

src/fsfw/tmtcpacket/pus/tm/PusTmCreator.cpp

@@ -28,6 +28,7 @@ PusTmParams& PusTmCreator::getParams() { return pusParams; }
 
 void PusTmCreator::setTimeStamper(TimeStamperIF* timeStamper_) {
   pusParams.secHeader.timeStamper = timeStamper_;
+  updateSpLengthField();
 }
 
 uint8_t PusTmCreator::getScTimeRefStatus() { return pusParams.secHeader.scTimeRefStatus; }
@@ -92,8 +93,6 @@ ReturnValue_t PusTmCreator::deSerialize(const uint8_t** buffer, size_t* size,
   return HasReturnvaluesIF::RETURN_FAILED;
 }
 
-ecss::DataWrapper& PusTmCreator::getDataWrapper() { return pusParams.dataWrapper; }
-
 TimeStamperIF* PusTmCreator::getTimestamper() const { return pusParams.secHeader.timeStamper; }
 
 SpacePacketParams& PusTmCreator::getSpParams() { return spCreator.getParams(); }
@@ -124,15 +123,13 @@ void PusTmCreator::setMessageTypeCounter(uint16_t messageTypeCounter) {
 
 void PusTmCreator::setDestId(uint16_t destId) { pusParams.secHeader.destId = destId; }
 
-void PusTmCreator::setRawSourceData(ecss::DataWrapper::BufPairT bufPair) {
+ReturnValue_t PusTmCreator::setRawUserData(const uint8_t* data, size_t len) {
-  pusParams.dataWrapper.type = ecss::DataTypes::RAW;
+  pusParams.dataWrapper.setRawData({data, len});
-  pusParams.dataWrapper.dataUnion.raw.data = bufPair.first;
-  pusParams.dataWrapper.dataUnion.raw.len = bufPair.second;
   updateSpLengthField();
+  return HasReturnvaluesIF::RETURN_OK;
 }
-
+ReturnValue_t PusTmCreator::setSerializableUserData(SerializeIF* serializable) {
-void PusTmCreator::setSerializableSourceData(SerializeIF* serializable) {
+  pusParams.dataWrapper.setSerializable(serializable);
-  pusParams.dataWrapper.type = ecss::DataTypes::SERIALIZABLE;
-  pusParams.dataWrapper.dataUnion.serializable = serializable;
   updateSpLengthField();
+  return HasReturnvaluesIF::RETURN_OK;
 }

src/fsfw/tmtcpacket/pus/tm/PusTmCreator.h

@@ -3,7 +3,7 @@
 
 #include "PusTmIF.h"
 #include "fsfw/tmtcpacket/ccsds/SpacePacketCreator.h"
-#include "fsfw/tmtcpacket/pus/CreatorDataIF.h"
+#include "fsfw/tmtcpacket/pus/CustomUserDataIF.h"
 
 struct PusTmSecHeader {
   PusTmSecHeader() = default;
@@ -39,7 +39,7 @@ struct PusTmParams {
 
 class TimeStamperIF;
 
-class PusTmCreator : public SerializeIF, public PusTmIF, public CreatorDataIF {
+class PusTmCreator : public SerializeIF, public PusTmIF, public CustomUserDataIF {
  public:
   PusTmCreator();
   PusTmCreator(SpacePacketParams initSpParams, PusTmParams initPusParams);
@@ -47,8 +47,6 @@ class PusTmCreator : public SerializeIF, public PusTmIF, public CreatorDataIF {
 
   void setTimeStamper(TimeStamperIF* timeStamper);
   SpacePacketParams& getSpParams();
-  void setRawSourceData(ecss::DataWrapper::BufPairT bufPair);
-  void setSerializableSourceData(SerializeIF* serializable);
   void setApid(uint16_t apid);
   void setDestId(uint16_t destId);
   void setMessageTypeCounter(uint16_t messageTypeCounter);
@@ -70,9 +68,8 @@ class PusTmCreator : public SerializeIF, public PusTmIF, public CreatorDataIF {
   ReturnValue_t deSerialize(const uint8_t** buffer, size_t* size,
                             Endianness streamEndianness) override;
   [[nodiscard]] TimeStamperIF* getTimestamper() const;
-
+  ReturnValue_t setRawUserData(const uint8_t* data, size_t len) override;
- private:
+  ReturnValue_t setSerializableUserData(SerializeIF* serializable) override;
-  ecss::DataWrapper& getDataWrapper() override;
 
  private:
   void setup();

unittests/tmtcpacket/testPusTcCreator.cpp

@@ -85,10 +85,7 @@ TEST_CASE("PUS TC Creator", "[pus-tc-creator]") {
   SECTION("Test with Application Data Serializable") {
     auto& params = creator.getPusParams();
     auto simpleSer = SimpleSerializable();
-    creator.setSerializableAppData(&simpleSer);
+    creator.setSerializableUserData(&simpleSer);
-    auto& dataWrapper = creator.getDataWrapper();
-    REQUIRE(dataWrapper.type == ecss::DataTypes::SERIALIZABLE);
-    REQUIRE(dataWrapper.dataUnion.serializable == &simpleSer);
     REQUIRE(creator.getSerializedSize() == 16);
     REQUIRE(creator.serialize(&dataPtr, &serLen, buf.size()) == HasReturnvaluesIF::RETURN_OK);
     REQUIRE(serLen == 16);

unittests/tmtcpacket/testPusTcReader.cpp

@@ -15,38 +15,43 @@ TEST_CASE("PUS TC Reader", "[pus-tc-reader]") {
   size_t serLen = 0;
   PusTcReader reader;
   auto checkReaderFields = [&](PusTcReader& reader) {
-    REQUIRE(not reader.isNull());
+
-    REQUIRE(reader.getPacketType() == ccsds::PacketType::TC);
-    REQUIRE(reader.getApid() == 0x02);
-    REQUIRE(reader.getService() == 17);
-    REQUIRE(reader.getSubService() == 1);
-    REQUIRE(reader.getFullPacketLen() == 13);
-    REQUIRE(reader.getPacketDataLen() == 6);
-    REQUIRE(reader.getPusVersion() == 2);
-    REQUIRE(reader.getSequenceCount() == 0x34);
-    REQUIRE(reader.getUserData() == nullptr);
-    REQUIRE(reader.getUserDataLen() == 0);
-    REQUIRE(reader.getFullData() == buf.data());
-    REQUIRE(reader.getSourceId() == 0x00);
-    REQUIRE(reader.getAcknowledgeFlags() == 0b1111);
-    // This value was verified to be correct
-    REQUIRE(reader.getErrorControl() == 0xee63);
   };
 
   SECTION("State") {
     REQUIRE(creator.serialize(&dataPtr, &serLen, buf.size()) == HasReturnvaluesIF::RETURN_OK);
     REQUIRE(reader.isNull());
-
+    PusTcReader* readerPtr = nullptr;
+    bool callDelete = false;
     SECTION("Setter") {
-      REQUIRE(reader.setReadOnlyData(buf.data(), serLen) == HasReturnvaluesIF::RETURN_OK);
+      readerPtr = &reader;
-      REQUIRE(reader.parseDataWithCrcCheck() == HasReturnvaluesIF::RETURN_OK);
+      REQUIRE(readerPtr->setReadOnlyData(buf.data(), serLen) == HasReturnvaluesIF::RETURN_OK);
-      checkReaderFields(reader);
+      REQUIRE(readerPtr->parseDataWithCrcCheck() == HasReturnvaluesIF::RETURN_OK);
     }
     SECTION("Directly Constructed") {
-      PusTcReader secondReader(buf.data(), serLen);
+      callDelete = true;
-      REQUIRE(not secondReader.isNull());
+      readerPtr = new PusTcReader(buf.data(), serLen);
-      REQUIRE(secondReader.parseDataWithCrcCheck() == HasReturnvaluesIF::RETURN_OK);
+      REQUIRE(not readerPtr->isNull());
-      checkReaderFields(secondReader);
+      REQUIRE(readerPtr->parseDataWithCrcCheck() == HasReturnvaluesIF::RETURN_OK);
+    }
+    REQUIRE(not readerPtr->isNull());
+    REQUIRE(readerPtr->getPacketType() == ccsds::PacketType::TC);
+    REQUIRE(readerPtr->getApid() == 0x02);
+    REQUIRE(readerPtr->getService() == 17);
+    REQUIRE(readerPtr->getSubService() == 1);
+    REQUIRE(readerPtr->getFullPacketLen() == 13);
+    REQUIRE(readerPtr->getPacketDataLen() == 6);
+    REQUIRE(readerPtr->getPusVersion() == 2);
+    REQUIRE(readerPtr->getSequenceCount() == 0x34);
+    REQUIRE(readerPtr->getUserData() == nullptr);
+    REQUIRE(readerPtr->getUserDataLen() == 0);
+    REQUIRE(readerPtr->getFullData() == buf.data());
+    REQUIRE(readerPtr->getSourceId() == 0x00);
+    REQUIRE(readerPtr->getAcknowledgeFlags() == 0b1111);
+    // This value was verified to be correct
+    REQUIRE(readerPtr->getErrorControl() == 0xee63);
+    if (callDelete) {
+      delete readerPtr;
     }
   }
 
@@ -67,7 +72,7 @@ TEST_CASE("PUS TC Reader", "[pus-tc-reader]") {
   SECTION("With application data") {
     auto& params = creator.getPusParams();
     std::array<uint8_t, 3> data{1, 2, 3};
-    creator.setRawAppData({data.data(), data.size()});
+    creator.setRawUserData(data.data(), data.size());
     REQUIRE(creator.serialize(&dataPtr, &serLen, buf.size()) == HasReturnvaluesIF::RETURN_OK);
     REQUIRE(reader.setReadOnlyData(buf.data(), serLen) == HasReturnvaluesIF::RETURN_OK);
     REQUIRE(reader.parseDataWithCrcCheck() == HasReturnvaluesIF::RETURN_OK);

unittests/tmtcpacket/testPusTmCreator.cpp

@@ -1,9 +1,10 @@
 #include <catch2/catch_test_macros.hpp>
 
+#include "fsfw/globalfunctions/CRC.h"
 #include "fsfw/globalfunctions/arrayprinter.h"
 #include "fsfw/tmtcpacket/pus/tm.h"
-#include "fsfw/globalfunctions/CRC.h"
 #include "mocks/CdsShortTimestamperMock.h"
+#include "mocks/SimpleSerializable.h"
 
 TEST_CASE("PUS TM Creator", "[pus-tm-creator]") {
   auto packetId = PacketId(ccsds::PacketType::TC, true, 0xef);
@@ -20,6 +21,8 @@ TEST_CASE("PUS TM Creator", "[pus-tm-creator]") {
   SECTION("State") {
     REQUIRE(creator.isTm());
     REQUIRE(creator.getApid() == 0xef);
+    REQUIRE(creator.getPusVersion() == 2);
+    REQUIRE(creator.getScTimeRefStatus() == 0);
     REQUIRE(creator.getService() == 17);
     REQUIRE(creator.getSubService() == 2);
     REQUIRE(creator.getTimestamper() == &timeStamper);
@@ -34,6 +37,12 @@ TEST_CASE("PUS TM Creator", "[pus-tm-creator]") {
     REQUIRE(timeStamper.getSizeCallCount == 1);
   }
 
+  SECTION("SP Params") {
+    auto& spParamsRef = creator.getSpParams();
+    REQUIRE(spParamsRef.dataLen == 15);
+    REQUIRE(spParamsRef.packetId.apid == 0xef);
+  }
+
   SECTION("Serialization") {
     REQUIRE(creator.serialize(&dataPtr, &serLen, buf.size()) == HasReturnvaluesIF::RETURN_OK);
     REQUIRE(buf[0] == 0x08);
@@ -54,7 +63,7 @@ TEST_CASE("PUS TM Creator", "[pus-tm-creator]") {
     // Destination ID
     REQUIRE(((buf[11] << 8) | buf[12]) == 0);
     // Custom timestamp
-    for(size_t i = 1; i < 8; i++) {
+    for (size_t i = 1; i < 8; i++) {
       REQUIRE(buf[12 + i] == i);
     }
     REQUIRE(serLen == 22);
@@ -87,12 +96,13 @@ TEST_CASE("PUS TM Creator", "[pus-tm-creator]") {
 
   SECTION("Deserialization fails") {
     const uint8_t* roDataPtr = nullptr;
-    REQUIRE(creator.deSerialize(&roDataPtr, &serLen, SerializeIF::Endianness::NETWORK) == HasReturnvaluesIF::RETURN_FAILED);
+    REQUIRE(creator.deSerialize(&roDataPtr, &serLen, SerializeIF::Endianness::NETWORK) ==
+            HasReturnvaluesIF::RETURN_FAILED);
   }
 
   SECTION("Serialize with Raw Data") {
     std::array<uint8_t, 3> data{1, 2, 3};
-    creator.setRawSourceData({data.data(), data.size()});
+    creator.setRawUserData(data.data(), data.size());
     REQUIRE(creator.getFullPacketLen() == 25);
     REQUIRE(creator.serialize(&dataPtr, &serLen, buf.size()) == HasReturnvaluesIF::RETURN_OK);
     REQUIRE(buf[20] == 1);
@@ -101,6 +111,34 @@ TEST_CASE("PUS TM Creator", "[pus-tm-creator]") {
   }
 
   SECTION("Serialize with Serializable") {
+    auto simpleSer = SimpleSerializable();
+    creator.setSerializableUserData(&simpleSer);
+    REQUIRE(creator.getFullPacketLen() == 25);
+    REQUIRE(creator.serialize(&dataPtr, &serLen, buf.size()) == HasReturnvaluesIF::RETURN_OK);
+    REQUIRE(buf[20] == 1);
+    REQUIRE(buf[21] == 2);
+    REQUIRE(buf[22] == 3);
+  }
 
+  SECTION("Empty Ctor") {
+    PusTmCreator creatorFromEmptyCtor;
+    // 6 bytes CCSDS header, 7 bytes secondary header, no timestamp (IF is null),
+    // 0 bytes application data, 2 bytes CRC
+    REQUIRE(creatorFromEmptyCtor.getFullPacketLen() == 15);
+    // As specified in standard, the data length fields is the total size of the packet without
+    // the primary header minus 1
+    REQUIRE(creatorFromEmptyCtor.getPacketDataLen() == 8);
+    creatorFromEmptyCtor.setTimeStamper(&timeStamper);
+    REQUIRE(creatorFromEmptyCtor.getFullPacketLen() == 22);
+    REQUIRE(creatorFromEmptyCtor.getPacketDataLen() == 15);
+  }
+
+  SECTION("Invalid Buffer Sizes") {
+    size_t reqSize = creator.getSerializedSize();
+    for (size_t maxSize = 0; maxSize < reqSize; maxSize++) {
+      dataPtr = buf.data();
+      serLen = 0;
+      REQUIRE(creator.serialize(&dataPtr, &serLen, maxSize) == SerializeIF::BUFFER_TOO_SHORT);
+    }
   }
 }