fnished PUS TC creator unittests

src/fsfw/tmtcpacket/ccsds/PacketId.h

@@ -19,6 +19,10 @@ struct PacketId : public SerializeIF {
   PacketId(ccsds::PacketType packetType_, bool secHeaderFlag_, uint16_t apid_)
       : packetType(packetType_), secHeaderFlag(secHeaderFlag_), apid(apid_) {}
 
+  bool operator==(const PacketId &other) const {
+    return packetType == other.packetType and secHeaderFlag == other.secHeaderFlag and
+           apid == other.apid;
+  }
   /**
    * NOTE: If the APID has an invalid value, the invalid bits will be cut off
    * @return

src/fsfw/tmtcpacket/ccsds/SpacePacketCreator.cpp

@@ -79,3 +79,7 @@ void SpacePacketCreator::checkFieldValidity() {
 void SpacePacketCreator::setParams(SpacePacketParams params_) { params = std::move(params_); }
 
 SpacePacketParams &SpacePacketCreator::getParams() { return params; }
+
+void SpacePacketCreator::setPacketType(ccsds::PacketType type) {
+  params.packetId.packetType = type;
+}

src/fsfw/tmtcpacket/ccsds/SpacePacketCreator.h

@@ -35,6 +35,7 @@ class SpacePacketCreator : public SpacePacketIF, public SerializeIF {
 
   SpacePacketParams &getParams();
   void setParams(SpacePacketParams params);
+  void setPacketType(ccsds::PacketType type);
   void setApid(uint16_t apid);
   void setSeqCount(uint16_t seqCount);
   void setSeqFlags(ccsds::SequenceFlags flags);

src/fsfw/tmtcpacket/pus/PusIF.h

@@ -9,8 +9,10 @@
 class PusIF : public SpacePacketIF {
  public:
   static constexpr uint8_t INTERFACE_ID = CLASS_ID::PUS_IF;
-  static constexpr ReturnValue_t INVALID_CRC_16 =
+  static constexpr ReturnValue_t INVALID_PUS_VERSION =
       HasReturnvaluesIF::makeReturnCode(INTERFACE_ID, 0);
+  static constexpr ReturnValue_t INVALID_CRC_16 =
+      HasReturnvaluesIF::makeReturnCode(INTERFACE_ID, 1);
 
   ~PusIF() override = default;
   /**

src/fsfw/tmtcpacket/pus/defs.h

@@ -2,6 +2,7 @@
 #define FSFW_SRC_FSFW_TMTCPACKET_PUS_TM_DEFINITIONS_H_
 
 #include <cstdint>
+#include <utility>
 
 #include "fsfw/serialize/SerializeIF.h"
 
@@ -27,6 +28,7 @@ union DataUnion {
 struct DataWrapper {
   DataTypes type;
   DataUnion dataUnion;
+  using BufPairT = std::pair<uint8_t*, size_t>;
 
   [[nodiscard]] size_t getLength() const {
     if (type == DataTypes::RAW) {
@@ -36,6 +38,17 @@ struct DataWrapper {
     }
     return 0;
   }
+
+  void setRawData(BufPairT bufPair) {
+    type = DataTypes::RAW;
+    dataUnion.raw.data = bufPair.first;
+    dataUnion.raw.len = bufPair.second;
+  }
+
+  void setSerializable(SerializeIF* serializable) {
+    type = DataTypes::SERIALIZABLE;
+    dataUnion.serializable = serializable;
+  }
 };
 
 /**

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

@@ -8,29 +8,31 @@
 
 PusTcCreator::PusTcCreator(SpacePacketParams spParams, PusTcParams pusParams)
     : spCreator(std::move(spParams)), pusParams(pusParams) {
+  spCreator.setPacketType(ccsds::PacketType::TC);
   updateSpLengthField();
 }
 
 ReturnValue_t PusTcCreator::serialize(uint8_t **buffer, size_t *size, size_t maxSize,
                                       SerializeIF::Endianness streamEndianness) const {
+  const uint8_t *start = *buffer;
   size_t userDataLen = pusParams.dataWrapper.getLength();
-  if (*size + PusTcIF::MIN_SIZE + userDataLen > maxSize) {
+  if (*size + getFullPacketLen() > maxSize) {
     return SerializeIF::BUFFER_TOO_SHORT;
   }
+  if (pusParams.pusVersion != ecss::PusVersion::PUS_C) {
+    return PusIF::INVALID_PUS_VERSION;
+  }
   ReturnValue_t result = spCreator.serialize(buffer, size, maxSize, streamEndianness);
   if (result != HasReturnvaluesIF::RETURN_OK) {
     return result;
   }
-  if (pusParams.pusVersion != ecss::PusVersion::PUS_C) {
-    // TODO: Dedicated returnvalue
-    return HasReturnvaluesIF::RETURN_FAILED;
-  }
   **buffer = pusParams.pusVersion << 4 | pusParams.ackFlags;
   *buffer += 1;
   **buffer = pusParams.service;
   *buffer += 1;
   **buffer = pusParams.subservice;
   *buffer += 1;
+  *size += 3;
   result =
       SerializeAdapter::serialize(&pusParams.sourceId, buffer, size, maxSize, streamEndianness);
   if (result != HasReturnvaluesIF::RETURN_OK) {
@@ -52,7 +54,7 @@ ReturnValue_t PusTcCreator::serialize(uint8_t **buffer, size_t *size, size_t max
     }
   }
 
-  uint16_t crc16 = CRC::crc16ccitt(*buffer, getFullPacketLen() - 2);
+  uint16_t crc16 = CRC::crc16ccitt(start, getFullPacketLen() - 2);
   return SerializeAdapter::serialize(&crc16, buffer, size, maxSize, streamEndianness);
 }
 
@@ -89,3 +91,7 @@ ecss::DataWrapper &PusTcCreator::getDataWrapper() { return pusParams.dataWrapper
 PusTcParams &PusTcCreator::getPusParams() { return pusParams; }
 
 SpacePacketParams &PusTcCreator::getSpParams() { return spCreator.getParams(); }
+
+ReturnValue_t PusTcCreator::serialize(uint8_t **buffer, size_t *size, size_t maxSize) {
+  return serialize(buffer, size, maxSize, SerializeIF::Endianness::NETWORK);
+}

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

@@ -21,13 +21,19 @@ struct PusTcParams {
 
 class PusTcCreator : public PusTcIF, public SerializeIF, public CreatorDataIF {
  public:
-  PusTcCreator(SpacePacketParams spParams, PusTcParams pusParams);
+  PusTcCreator(SpacePacketParams initSpParams, PusTcParams initPusParams);
 
+  /**
+   * If the parameter structure is changed in a way which changes the resulting serialized packet
+   * size, this function should be called to set the data length field in the space packet
+   * header. This fields is the primary source of information for length information.
+   *
+   * The only case for a telecommand where this size changes would be if user data is set.
+   */
   void updateSpLengthField();
   PusTcParams &getPusParams();
   SpacePacketParams &getSpParams();
-  ReturnValue_t serialize(uint8_t **buffer, size_t *size, size_t maxSize,
+  ReturnValue_t serialize(uint8_t **buffer, size_t *size, size_t maxSize);
-                          Endianness streamEndianness) const override;
   [[nodiscard]] size_t getSerializedSize() const override;
   ReturnValue_t deSerialize(const uint8_t **buffer, size_t *size,
                             Endianness streamEndianness) override;
@@ -42,6 +48,8 @@ class PusTcCreator : public PusTcIF, public SerializeIF, public CreatorDataIF {
   ecss::DataWrapper &getDataWrapper() override;
 
  private:
+  ReturnValue_t serialize(uint8_t **buffer, size_t *size, size_t maxSize,
+                          Endianness streamEndianness) const override;
   SpacePacketCreator spCreator;
   PusTcParams pusParams;
 };

unittests/devicehandler/CMakeLists.txt

@@ -1,8 +1,4 @@
 target_sources(${FSFW_TEST_TGT} PRIVATE
-	CookieIFMock.cpp
-	ComIFMock.cpp
 	DeviceHandlerCommander.cpp
-	DeviceHandlerMock.cpp
-	DeviceFdirMock.cpp
 	TestDeviceHandlerBase.cpp
 )

unittests/devicehandler/TestDeviceHandlerBase.cpp

@@ -1,10 +1,10 @@
 #include <catch2/catch_test_macros.hpp>
 
-#include "ComIFMock.h"
-#include "DeviceFdirMock.h"
 #include "DeviceHandlerCommander.h"
-#include "DeviceHandlerMock.h"
+#include "mocks/ComIFMock.h"
-#include "devicehandler/CookieIFMock.h"
+#include "mocks/CookieIFMock.h"
+#include "mocks/DeviceFdirMock.h"
+#include "mocks/DeviceHandlerMock.h"
 #include "objects/systemObjectList.h"
 
 TEST_CASE("Device Handler Base", "[DeviceHandlerBase]") {

unittests/mocks/CMakeLists.txt

@@ -1,3 +1,7 @@
 target_sources(${FSFW_TEST_TGT} PRIVATE
 	PowerSwitcherMock.cpp
+	DeviceHandlerMock.cpp
+	DeviceFdirMock.cpp
+	CookieIFMock.cpp
+	ComIFMock.cpp
 )

unittests/mocks/DeviceFdirMock.cpp

@@ -1,11 +1,9 @@
 #include "DeviceFdirMock.h"
 
-#include "devicehandler/DeviceFdirMock.h"
-
 DeviceFdirMock::DeviceFdirMock(object_id_t owner, object_id_t parent)
     : DeviceHandlerFailureIsolation(owner, parent) {}
 
-DeviceFdirMock::~DeviceFdirMock() {}
+DeviceFdirMock::~DeviceFdirMock() = default;
 
 uint32_t DeviceFdirMock::getMissedReplyCount() {
   ParameterWrapper parameterWrapper;

unittests/mocks/SimpleSerializable.h

@@ -0,0 +1,35 @@
+#ifndef FSFW_TESTS_SIMPLESERIALIZABLE_H
+#define FSFW_TESTS_SIMPLESERIALIZABLE_H
+
+#include "fsfw/serialize.h"
+
+class SimpleSerializable : public SerializeIF {
+ public:
+  ReturnValue_t serialize(uint8_t **buffer, size_t *size, size_t maxSize,
+                          Endianness streamEndianness) const override {
+    if (*size + getSerializedSize() > maxSize) {
+      return SerializeIF::BUFFER_TOO_SHORT;
+    }
+    **buffer = someU8;
+    *buffer += 1;
+    *size += 1;
+    return SerializeAdapter::serialize(&someU16, buffer, size, maxSize, streamEndianness);
+  }
+
+  [[nodiscard]] size_t getSerializedSize() const override { return 3; }
+  ReturnValue_t deSerialize(const uint8_t **buffer, size_t *size,
+                            Endianness streamEndianness) override {
+    if (*size < getSerializedSize()) {
+      return SerializeIF::STREAM_TOO_SHORT;
+    }
+    someU8 = **buffer;
+    *size -= 1;
+    return SerializeAdapter::deSerialize(&someU16, buffer, size, streamEndianness);
+  }
+
+ private:
+  uint8_t someU8 = 1;
+  uint16_t someU16 = 0x0203;
+};
+
+#endif  // FSFW_TESTS_SIMPLESERIALIZABLE_H

unittests/tmtcpacket/CMakeLists.txt

@@ -1,4 +1,5 @@
 target_sources(${FSFW_TEST_TGT} PRIVATE
   testCcsdsCreator.cpp
   testCcsdsReader.cpp
+  testPusTcCreator.cpp
 )

unittests/tmtcpacket/testCcsdsReader.cpp

@@ -4,26 +4,22 @@
 #include "fsfw/tmtcpacket/ccsds/SpacePacketCreator.h"
 #include "fsfw/tmtcpacket/ccsds/SpacePacketReader.h"
 
+#define FULL_PACKET_LEN 29
+
 TEST_CASE("CCSDS Reader", "[ccsds-reader]") {
   auto params = SpacePacketParams(PacketId(ccsds::PacketType::TC, true, 0x02),
                                   PacketSeqCtrl(ccsds::SequenceFlags::FIRST_SEGMENT, 0x34), 0x16);
   SpacePacketCreator base = SpacePacketCreator(params);
   // This is enough to hold 0x16 (22) bytes + 6 (SP header length) + 1 as defined as the full packet
   // length derived from the length field
-  std::array<uint8_t, 32> buf{};
+  std::array<uint8_t, FULL_PACKET_LEN> buf{};
   uint8_t* bufPtr = buf.data();
   size_t serLen = 0;
   SpacePacketReader reader;
-  SECTION("Empty Reader") {
-    REQUIRE(reader.isNull());
-    REQUIRE(reader.checkSize() == HasReturnvaluesIF::RETURN_FAILED);
-  }
 
-  SECTION("Basic Read") {
+  auto checkReader = [&](SpacePacketReader& reader) {
-    REQUIRE(base.serialize(&bufPtr, &serLen, buf.size(), SerializeIF::Endianness::NETWORK) ==
-            HasReturnvaluesIF::RETURN_OK);
-    reader.setReadOnlyData(buf.data(), SpacePacketIF::getHeaderLen());
     REQUIRE(reader.getPacketDataLen() == 0x16);
+    REQUIRE(reader.getBufSize() == SpacePacketIF::getHeaderLen());
     REQUIRE(reader.getFullData() == buf.data());
     REQUIRE(reader.getFullPacketLen() == 0x16 + SpacePacketReader::getHeaderLen() + 1);
     REQUIRE(reader.getPacketIdRaw() == 0x1802);
@@ -35,12 +31,33 @@ TEST_CASE("CCSDS Reader", "[ccsds-reader]") {
     REQUIRE(not reader.isNull());
     // We only serialized the 6 bytes of the header, so the packer data should be invalid
     REQUIRE(reader.getPacketData() == nullptr);
+  };
+
+  SECTION("Empty Reader") {
+    REQUIRE(SpacePacketIF::getHeaderLen() == 6);
+    REQUIRE(reader.isNull());
+    REQUIRE(reader.checkSize() == HasReturnvaluesIF::RETURN_FAILED);
+  }
+
+  SECTION("Basic Read") {
+    REQUIRE(base.serialize(&bufPtr, &serLen, buf.size(), SerializeIF::Endianness::NETWORK) ==
+            HasReturnvaluesIF::RETURN_OK);
+    SECTION("Setter") {
+      reader.setReadOnlyData(buf.data(), SpacePacketIF::getHeaderLen());
+      checkReader(reader);
+    }
+    SECTION("Direct Construction") {
+      SpacePacketReader secondReader(buf.data(), serLen);
+      checkReader(secondReader);
+    }
   }
 
   SECTION("Read with additional data") {
     REQUIRE(base.serialize(&bufPtr, &serLen, buf.size(), SerializeIF::Endianness::NETWORK) ==
             HasReturnvaluesIF::RETURN_OK);
     REQUIRE(reader.setReadOnlyData(buf.data(), buf.size()) == HasReturnvaluesIF::RETURN_OK);
+    REQUIRE(reader.getBufSize() == buf.size());
+    REQUIRE(reader.getFullPacketLen() == FULL_PACKET_LEN);
     REQUIRE(reader.getPacketData() == buf.data() + SpacePacketIF::getHeaderLen());
   }
 

unittests/tmtcpacket/testPusTcCreator.cpp

@@ -0,0 +1,120 @@
+#include <array>
+#include <catch2/catch_test_macros.hpp>
+
+#include "fsfw/globalfunctions/CRC.h"
+#include "fsfw/tmtcpacket/pus/tc.h"
+#include "mocks/SimpleSerializable.h"
+
+TEST_CASE("PUS TC Creator", "[pus-tc-creator]") {
+  auto packetId = PacketId(ccsds::PacketType::TC, true, 0x02);
+  auto spParams =
+      SpacePacketParams(packetId, PacketSeqCtrl(ccsds::SequenceFlags::UNSEGMENTED, 0x34), 0x00);
+  auto pusParams = PusTcParams(17, 1);
+  PusTcCreator creator(spParams, pusParams);
+  std::array<uint8_t, 32> buf{};
+  uint8_t* dataPtr = buf.data();
+  size_t serLen = 0;
+
+  SECTION("State") {
+    REQUIRE(creator.getService() == 17);
+    REQUIRE(creator.getSubService() == 1);
+    REQUIRE(creator.getApid() == 0x02);
+    REQUIRE(creator.getPusVersion() == 2);
+    REQUIRE(creator.getAcknowledgeFlags() == 0b1111);
+    REQUIRE(creator.getSourceId() == 0x00);
+    REQUIRE(creator.getPacketSeqCtrlRaw() == 0xc034);
+    // bytes CCSDS header, 5 bytes secondary header, 2 bytes CRC, 3 bytes app data
+    REQUIRE(creator.getFullPacketLen() == 13);
+    // The data length field is the full packet length minus the primary header minus 1
+    REQUIRE(creator.getPacketDataLen() == 6);
+    auto& paramsLocal = creator.getSpParams();
+    REQUIRE(paramsLocal.packetId == packetId);
+  }
+
+  SECTION("Serialized") {
+    REQUIRE(creator.serialize(&dataPtr, &serLen, buf.size()) == HasReturnvaluesIF::RETURN_OK);
+    REQUIRE(serLen == 13);
+    REQUIRE(buf[0] == 0x18);
+    REQUIRE(buf[1] == 0x02);
+    // Unsegmented, first 2 bits 11
+    REQUIRE(buf[2] == 0xc0);
+    // Packet Sequence count only occupies lower byte of packet sequence control
+    REQUIRE(buf[3] == 0x34);
+    // Data length packed big endian
+    REQUIRE(buf[4] == 0x00);
+    REQUIRE(buf[5] == 0x06);
+    // PUS Version C (2)
+    REQUIRE(((buf[6] >> 4) & 0b1111) == 2);
+    // All Ack Fields is default
+    REQUIRE((buf[6] & 0b1111) == 0b1111);
+    // Service and subservice
+    REQUIRE(buf[7] == 17);
+    REQUIRE(buf[8] == 1);
+    // Source ID is 0
+    REQUIRE(((buf[9] << 8) | buf[10]) == 0);
+    // CRC16 check
+    REQUIRE(CRC::crc16ccitt(buf.data(), serLen) == 0);
+  }
+
+  SECTION("Custom Source ID") {
+    auto& params = creator.getPusParams();
+    params.sourceId = 0x5ff;
+    REQUIRE(creator.getSourceId() == 0x5ff);
+    REQUIRE(creator.serialize(&dataPtr, &serLen, buf.size()) == HasReturnvaluesIF::RETURN_OK);
+    REQUIRE(((buf[9] << 8) | buf[10]) == 0x5ff);
+  }
+
+  SECTION("Test with Application Data Raw") {
+    auto& params = creator.getPusParams();
+    std::array<uint8_t, 3> data{1, 2, 3};
+    params.dataWrapper.setRawData({data.data(), data.size()});
+    // To get correct size information, the SP length field needs to be updated automatically
+    REQUIRE(creator.getSerializedSize() == 13);
+    creator.updateSpLengthField();
+    REQUIRE(creator.getSerializedSize() == 16);
+    REQUIRE(creator.serialize(&dataPtr, &serLen, buf.size()) == HasReturnvaluesIF::RETURN_OK);
+    REQUIRE(serLen == 16);
+    REQUIRE(buf[11] == 1);
+    REQUIRE(buf[12] == 2);
+    REQUIRE(buf[13] == 3);
+  }
+
+  SECTION("Test with Application Data Serializable") {
+    auto& params = creator.getPusParams();
+    auto simpleSer = SimpleSerializable();
+    params.dataWrapper.setSerializable(&simpleSer);
+    auto& dataWrapper = creator.getDataWrapper();
+    REQUIRE(dataWrapper.type == ecss::DataTypes::SERIALIZABLE);
+    REQUIRE(dataWrapper.dataUnion.serializable == &simpleSer);
+    REQUIRE(creator.getSerializedSize() == 13);
+    creator.updateSpLengthField();
+    REQUIRE(creator.getSerializedSize() == 16);
+    REQUIRE(creator.serialize(&dataPtr, &serLen, buf.size()) == HasReturnvaluesIF::RETURN_OK);
+    REQUIRE(serLen == 16);
+    REQUIRE(buf[11] == 1);
+    REQUIRE(buf[12] == 2);
+    REQUIRE(buf[13] == 3);
+  }
+
+  SECTION("Deserialization Fails") {
+    size_t deserLen = buf.size();
+    const uint8_t* roPtr = buf.data();
+    REQUIRE(creator.deSerialize(&roPtr, &deserLen, SerializeIF::Endianness::NETWORK) ==
+            HasReturnvaluesIF::RETURN_FAILED);
+  }
+
+  SECTION("Serialize with invalid buffer length") {
+    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);
+    }
+  }
+
+  SECTION("Invalid PUS Version") {
+    auto& params = creator.getPusParams();
+    params.pusVersion = 0;
+    REQUIRE(creator.serialize(&dataPtr, &serLen, buf.size()) == PusIF::INVALID_PUS_VERSION);
+  }
+}