diff --git a/src/fsfw/tmtcpacket/pus/CreatorDataIF.h b/src/fsfw/tmtcpacket/pus/CreatorDataIF.h deleted file mode 100644 index f66ca567..00000000 --- a/src/fsfw/tmtcpacket/pus/CreatorDataIF.h +++ /dev/null @@ -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 diff --git a/src/fsfw/tmtcpacket/pus/CustomUserDataIF.h b/src/fsfw/tmtcpacket/pus/CustomUserDataIF.h new file mode 100644 index 00000000..ddbce9f1 --- /dev/null +++ b/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 diff --git a/src/fsfw/tmtcpacket/pus/defs.h b/src/fsfw/tmtcpacket/pus/defs.h index 83502ed3..72a154ff 100644 --- a/src/fsfw/tmtcpacket/pus/defs.h +++ b/src/fsfw/tmtcpacket/pus/defs.h @@ -28,7 +28,7 @@ union DataUnion { struct DataWrapper { DataTypes type; DataUnion dataUnion; - using BufPairT = std::pair; + using BufPairT = std::pair; [[nodiscard]] size_t getLength() const { if (type == DataTypes::RAW) { diff --git a/src/fsfw/tmtcpacket/pus/tc/PusTcCreator.cpp b/src/fsfw/tmtcpacket/pus/tc/PusTcCreator.cpp index 240e7718..9ffd6eb4 100644 --- a/src/fsfw/tmtcpacket/pus/tc/PusTcCreator.cpp +++ b/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) { - pusParams.dataWrapper.setRawData(bufPair); +ReturnValue_t PusTcCreator::setRawUserData(const uint8_t *data, size_t len) { + // TODO: Check length field? + pusParams.dataWrapper.setRawData({data, len}); updateSpLengthField(); + return HasReturnvaluesIF::RETURN_OK; } -void PusTcCreator::setSerializableAppData(SerializeIF *serAppData) { - pusParams.dataWrapper.setSerializable(serAppData); +ReturnValue_t PusTcCreator::setSerializableUserData(SerializeIF *serializable) { + // TODO: Check length field? + pusParams.dataWrapper.setSerializable(serializable); updateSpLengthField(); + return HasReturnvaluesIF::RETURN_OK; } diff --git a/src/fsfw/tmtcpacket/pus/tc/PusTcCreator.h b/src/fsfw/tmtcpacket/pus/tc/PusTcCreator.h index 9e6b6e6f..4e5951a5 100644 --- a/src/fsfw/tmtcpacket/pus/tc/PusTcCreator.h +++ b/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, diff --git a/src/fsfw/tmtcpacket/pus/tm/PusTmCreator.cpp b/src/fsfw/tmtcpacket/pus/tm/PusTmCreator.cpp index 9cbdeb5c..b8ef0852 100644 --- a/src/fsfw/tmtcpacket/pus/tm/PusTmCreator.cpp +++ b/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) { - pusParams.dataWrapper.type = ecss::DataTypes::RAW; - pusParams.dataWrapper.dataUnion.raw.data = bufPair.first; - pusParams.dataWrapper.dataUnion.raw.len = bufPair.second; +ReturnValue_t PusTmCreator::setRawUserData(const uint8_t* data, size_t len) { + pusParams.dataWrapper.setRawData({data, len}); updateSpLengthField(); + return HasReturnvaluesIF::RETURN_OK; } - -void PusTmCreator::setSerializableSourceData(SerializeIF* serializable) { - pusParams.dataWrapper.type = ecss::DataTypes::SERIALIZABLE; - pusParams.dataWrapper.dataUnion.serializable = serializable; +ReturnValue_t PusTmCreator::setSerializableUserData(SerializeIF* serializable) { + pusParams.dataWrapper.setSerializable(serializable); updateSpLengthField(); + return HasReturnvaluesIF::RETURN_OK; } diff --git a/src/fsfw/tmtcpacket/pus/tm/PusTmCreator.h b/src/fsfw/tmtcpacket/pus/tm/PusTmCreator.h index c0406dc0..9981c39c 100644 --- a/src/fsfw/tmtcpacket/pus/tm/PusTmCreator.h +++ b/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; - - private: - ecss::DataWrapper& getDataWrapper() override; + ReturnValue_t setRawUserData(const uint8_t* data, size_t len) override; + ReturnValue_t setSerializableUserData(SerializeIF* serializable) override; private: void setup(); diff --git a/unittests/tmtcpacket/testPusTcCreator.cpp b/unittests/tmtcpacket/testPusTcCreator.cpp index 28c7e555..978833cd 100644 --- a/unittests/tmtcpacket/testPusTcCreator.cpp +++ b/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); - auto& dataWrapper = creator.getDataWrapper(); - REQUIRE(dataWrapper.type == ecss::DataTypes::SERIALIZABLE); - REQUIRE(dataWrapper.dataUnion.serializable == &simpleSer); + creator.setSerializableUserData(&simpleSer); REQUIRE(creator.getSerializedSize() == 16); REQUIRE(creator.serialize(&dataPtr, &serLen, buf.size()) == HasReturnvaluesIF::RETURN_OK); REQUIRE(serLen == 16); diff --git a/unittests/tmtcpacket/testPusTcReader.cpp b/unittests/tmtcpacket/testPusTcReader.cpp index 1c870f78..82a88542 100644 --- a/unittests/tmtcpacket/testPusTcReader.cpp +++ b/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); - REQUIRE(reader.parseDataWithCrcCheck() == HasReturnvaluesIF::RETURN_OK); - checkReaderFields(reader); + readerPtr = &reader; + REQUIRE(readerPtr->setReadOnlyData(buf.data(), serLen) == HasReturnvaluesIF::RETURN_OK); + REQUIRE(readerPtr->parseDataWithCrcCheck() == HasReturnvaluesIF::RETURN_OK); } SECTION("Directly Constructed") { - PusTcReader secondReader(buf.data(), serLen); - REQUIRE(not secondReader.isNull()); - REQUIRE(secondReader.parseDataWithCrcCheck() == HasReturnvaluesIF::RETURN_OK); - checkReaderFields(secondReader); + callDelete = true; + readerPtr = new PusTcReader(buf.data(), serLen); + REQUIRE(not readerPtr->isNull()); + 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 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); diff --git a/unittests/tmtcpacket/testPusTmCreator.cpp b/unittests/tmtcpacket/testPusTmCreator.cpp index 0cb1c093..2b60515f 100644 --- a/unittests/tmtcpacket/testPusTmCreator.cpp +++ b/unittests/tmtcpacket/testPusTmCreator.cpp @@ -1,9 +1,10 @@ #include +#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 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); + } } } \ No newline at end of file