From 0dfaba81f9a3868d64f5f88726ea91bfdb22af73 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 21 Jul 2022 19:10:15 +0200 Subject: [PATCH] finished basic TC unittests --- src/fsfw/osal/host/Clock.cpp | 2 +- src/fsfw/tmtcpacket/pus/tc/PusTcCreator.cpp | 12 ++- src/fsfw/tmtcpacket/pus/tc/PusTcCreator.h | 3 + src/fsfw/tmtcpacket/pus/tc/PusTcReader.cpp | 38 ++++------ src/fsfw/tmtcpacket/pus/tc/PusTcReader.h | 3 +- src/fsfw/tmtcpacket/pus/tm/PusTmCreator.cpp | 2 +- src/fsfw/tmtcpacket/pus/tm/PusTmReader.cpp | 4 - unittests/tmtcpacket/CMakeLists.txt | 1 + unittests/tmtcpacket/testPusTcCreator.cpp | 6 +- unittests/tmtcpacket/testPusTcReader.cpp | 81 +++++++++++++++++++++ unittests/tmtcpacket/testPusTm.cpp | 1 - 11 files changed, 118 insertions(+), 35 deletions(-) create mode 100644 unittests/tmtcpacket/testPusTcReader.cpp delete mode 100644 unittests/tmtcpacket/testPusTm.cpp diff --git a/src/fsfw/osal/host/Clock.cpp b/src/fsfw/osal/host/Clock.cpp index 2dc9d472..2f7ede89 100644 --- a/src/fsfw/osal/host/Clock.cpp +++ b/src/fsfw/osal/host/Clock.cpp @@ -146,7 +146,7 @@ ReturnValue_t Clock::getDateAndTime(TimeOfDay_t* time) { } ReturnValue_t Clock::convertTimeOfDayToTimeval(const TimeOfDay_t* from, timeval* to) { - struct tm time_tm{}; + struct tm time_tm {}; time_tm.tm_year = from->year - 1900; time_tm.tm_mon = from->month - 1; diff --git a/src/fsfw/tmtcpacket/pus/tc/PusTcCreator.cpp b/src/fsfw/tmtcpacket/pus/tc/PusTcCreator.cpp index 2ff82d25..240e7718 100644 --- a/src/fsfw/tmtcpacket/pus/tc/PusTcCreator.cpp +++ b/src/fsfw/tmtcpacket/pus/tc/PusTcCreator.cpp @@ -54,7 +54,7 @@ ReturnValue_t PusTcCreator::serialize(uint8_t **buffer, size_t *size, size_t max } } - uint16_t crc16 = CRC::crc16ccitt(start, getFullPacketLen() - 2); + uint16_t crc16 = CRC::crc16ccitt(start, getFullPacketLen() - sizeof(ecss::PusChecksumT)); return SerializeAdapter::serialize(&crc16, buffer, size, maxSize, streamEndianness); } @@ -95,3 +95,13 @@ 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); } + +void PusTcCreator::setRawAppData(ecss::DataWrapper::BufPairT bufPair) { + pusParams.dataWrapper.setRawData(bufPair); + updateSpLengthField(); +} + +void PusTcCreator::setSerializableAppData(SerializeIF *serAppData) { + pusParams.dataWrapper.setSerializable(serAppData); + updateSpLengthField(); +} diff --git a/src/fsfw/tmtcpacket/pus/tc/PusTcCreator.h b/src/fsfw/tmtcpacket/pus/tc/PusTcCreator.h index c09a7238..9e6b6e6f 100644 --- a/src/fsfw/tmtcpacket/pus/tc/PusTcCreator.h +++ b/src/fsfw/tmtcpacket/pus/tc/PusTcCreator.h @@ -33,6 +33,9 @@ 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; ReturnValue_t deSerialize(const uint8_t **buffer, size_t *size, diff --git a/src/fsfw/tmtcpacket/pus/tc/PusTcReader.cpp b/src/fsfw/tmtcpacket/pus/tc/PusTcReader.cpp index bac6d543..f5dd143a 100644 --- a/src/fsfw/tmtcpacket/pus/tc/PusTcReader.cpp +++ b/src/fsfw/tmtcpacket/pus/tc/PusTcReader.cpp @@ -23,20 +23,18 @@ ReturnValue_t PusTcReader::parseData(bool withCrc) { if (result != HasReturnvaluesIF::RETURN_OK) { return result; } - // We already have access to the space packet fields here, so we can perform a sanity check - // on the length field - if (spReader.getFullPacketLen() < spReader.getBufSize()) { - return SerializeIF::STREAM_TOO_SHORT; - } - size_t currentOffset = SpacePacketReader::getHeaderLen(); pointers.secHeaderStart = pointers.spHeaderStart + currentOffset; // Might become variable sized field in the future // TODO: No support for spare bytes yet currentOffset += ecss::PusTcDataFieldHeader::MIN_SIZE; - pointers.userDataStart = pointers.spHeaderStart + currentOffset; appDataSize = spReader.getFullPacketLen() - currentOffset - sizeof(ecss::PusChecksumT); - pointers.crcStart = pointers.userDataStart + appDataSize; + pointers.userDataStart = nullptr; + if (appDataSize > 0) { + pointers.userDataStart = pointers.spHeaderStart + currentOffset; + } + currentOffset += appDataSize; + pointers.crcStart = pointers.spHeaderStart + currentOffset; if (withCrc) { uint16_t crc16 = CRC::crc16ccitt(spReader.getFullData(), getFullPacketLen()); if (crc16 != 0) { @@ -47,25 +45,28 @@ ReturnValue_t PusTcReader::parseData(bool withCrc) { return HasReturnvaluesIF::RETURN_OK; } -uint8_t PusTcReader::getAcknowledgeFlags() const { - return (pointers.secHeaderStart[0] >> 4) & 0b1111; -} +uint8_t PusTcReader::getPusVersion() const { return (pointers.secHeaderStart[0] >> 4) & 0b1111; } + +uint8_t PusTcReader::getAcknowledgeFlags() const { return pointers.secHeaderStart[0] & 0b1111; } uint8_t PusTcReader::getService() const { return pointers.secHeaderStart[1]; } uint8_t PusTcReader::getSubService() const { return pointers.secHeaderStart[2]; } + uint16_t PusTcReader::getSourceId() const { return (pointers.secHeaderStart[3] << 8) | pointers.secHeaderStart[4]; } uint16_t PusTcReader::getErrorControl() const { - return pointers.crcStart[0] << 8 | pointers.crcStart[1]; + return (pointers.crcStart[0] << 8) | pointers.crcStart[1]; } uint16_t PusTcReader::getPacketIdRaw() const { return spReader.getPacketIdRaw(); } + uint16_t PusTcReader::getPacketSeqCtrlRaw() const { return spReader.getPacketSeqCtrlRaw(); } + uint16_t PusTcReader::getPacketDataLen() const { return spReader.getPacketDataLen(); } -uint8_t PusTcReader::getPusVersion() const { return spReader.getVersion(); } + const uint8_t* PusTcReader::getFullData() { return pointers.spHeaderStart; } ReturnValue_t PusTcReader::setData(uint8_t* pData, size_t size_, void* args) { @@ -80,13 +81,4 @@ ReturnValue_t PusTcReader::setReadOnlyData(const uint8_t* data, size_t size_) { const uint8_t* PusTcReader::getUserData() const { return pointers.userDataStart; } size_t PusTcReader::getUserDataLen() const { return appDataSize; } -/* -void PusTcReader::print() { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::info << "TcPacketBase::print:" << std::endl; -#else - sif::printInfo("TcPacketBase::print:\n"); -#endif - arrayprinter::print(getWholeData(), getFullSize()); -} -*/ \ No newline at end of file +bool PusTcReader::isNull() const { return spReader.isNull(); } diff --git a/src/fsfw/tmtcpacket/pus/tc/PusTcReader.h b/src/fsfw/tmtcpacket/pus/tc/PusTcReader.h index 528e0ca5..3be2e5e6 100644 --- a/src/fsfw/tmtcpacket/pus/tc/PusTcReader.h +++ b/src/fsfw/tmtcpacket/pus/tc/PusTcReader.h @@ -34,6 +34,7 @@ class PusTcReader : public PusTcIF, */ PusTcReader(const uint8_t* setData, size_t size); + [[nodiscard]] bool isNull() const; ReturnValue_t parseDataWithCrcCheck(); ReturnValue_t parseDataWithoutCrcCheck(); @@ -60,7 +61,7 @@ class PusTcReader : public PusTcIF, ReturnValue_t setReadOnlyData(const uint8_t* data, size_t size); [[nodiscard]] const uint8_t* getUserData() const override; - size_t getUserDataLen() const override; + [[nodiscard]] size_t getUserDataLen() const override; protected: /** diff --git a/src/fsfw/tmtcpacket/pus/tm/PusTmCreator.cpp b/src/fsfw/tmtcpacket/pus/tm/PusTmCreator.cpp index 927ec072..22c998c8 100644 --- a/src/fsfw/tmtcpacket/pus/tm/PusTmCreator.cpp +++ b/src/fsfw/tmtcpacket/pus/tm/PusTmCreator.cpp @@ -74,7 +74,7 @@ ReturnValue_t PusTmCreator::serialize(uint8_t** buffer, size_t* size, size_t max return result; } } - uint16_t crc16 = CRC::crc16ccitt(*buffer, getFullPacketLen() - 2); + uint16_t crc16 = CRC::crc16ccitt(*buffer, getFullPacketLen() - sizeof(ecss::PusChecksumT)); return SerializeAdapter::serialize(&crc16, buffer, size, maxSize, streamEndianness); } diff --git a/src/fsfw/tmtcpacket/pus/tm/PusTmReader.cpp b/src/fsfw/tmtcpacket/pus/tm/PusTmReader.cpp index 807dd8e0..e700a428 100644 --- a/src/fsfw/tmtcpacket/pus/tm/PusTmReader.cpp +++ b/src/fsfw/tmtcpacket/pus/tm/PusTmReader.cpp @@ -57,10 +57,6 @@ ReturnValue_t PusTmReader::parseData(bool crcCheck) { if (result != HasReturnvaluesIF::RETURN_OK) { return result; } - if (spReader.getFullPacketLen() < spReader.getBufSize()) { - return SerializeIF::STREAM_TOO_SHORT; - } - size_t currentOffset = SpacePacketReader::getHeaderLen(); pointers.secHeaderStart = pointers.spHeaderStart + currentOffset; currentOffset += PusTmIF::MIN_SEC_HEADER_LEN; diff --git a/unittests/tmtcpacket/CMakeLists.txt b/unittests/tmtcpacket/CMakeLists.txt index 67d8858c..673c5244 100644 --- a/unittests/tmtcpacket/CMakeLists.txt +++ b/unittests/tmtcpacket/CMakeLists.txt @@ -2,4 +2,5 @@ target_sources(${FSFW_TEST_TGT} PRIVATE testCcsdsCreator.cpp testCcsdsReader.cpp testPusTcCreator.cpp + testPusTcReader.cpp ) diff --git a/unittests/tmtcpacket/testPusTcCreator.cpp b/unittests/tmtcpacket/testPusTcCreator.cpp index f6c15b78..ca934121 100644 --- a/unittests/tmtcpacket/testPusTcCreator.cpp +++ b/unittests/tmtcpacket/testPusTcCreator.cpp @@ -54,6 +54,8 @@ TEST_CASE("PUS TC Creator", "[pus-tc-creator]") { REQUIRE(((buf[9] << 8) | buf[10]) == 0); // CRC16 check REQUIRE(CRC::crc16ccitt(buf.data(), serLen) == 0); + REQUIRE(buf[11] == 0xee); + REQUIRE(buf[12] == 0x63); } SECTION("Custom Source ID") { @@ -82,12 +84,10 @@ TEST_CASE("PUS TC Creator", "[pus-tc-creator]") { SECTION("Test with Application Data Serializable") { auto& params = creator.getPusParams(); auto simpleSer = SimpleSerializable(); - params.dataWrapper.setSerializable(&simpleSer); + creator.setSerializableAppData(&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); diff --git a/unittests/tmtcpacket/testPusTcReader.cpp b/unittests/tmtcpacket/testPusTcReader.cpp new file mode 100644 index 00000000..1c870f78 --- /dev/null +++ b/unittests/tmtcpacket/testPusTcReader.cpp @@ -0,0 +1,81 @@ +#include +#include + +#include "fsfw/tmtcpacket/pus/tc/PusTcCreator.h" +#include "fsfw/tmtcpacket/pus/tc/PusTcReader.h" + +TEST_CASE("PUS TC Reader", "[pus-tc-reader]") { + 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 buf{}; + uint8_t* dataPtr = buf.data(); + 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()); + + SECTION("Setter") { + REQUIRE(reader.setReadOnlyData(buf.data(), serLen) == HasReturnvaluesIF::RETURN_OK); + REQUIRE(reader.parseDataWithCrcCheck() == HasReturnvaluesIF::RETURN_OK); + checkReaderFields(reader); + } + SECTION("Directly Constructed") { + PusTcReader secondReader(buf.data(), serLen); + REQUIRE(not secondReader.isNull()); + REQUIRE(secondReader.parseDataWithCrcCheck() == HasReturnvaluesIF::RETURN_OK); + checkReaderFields(secondReader); + } + } + + SECTION("Invalid CRC") { + REQUIRE(creator.serialize(&dataPtr, &serLen, buf.size()) == HasReturnvaluesIF::RETURN_OK); + REQUIRE(reader.setReadOnlyData(buf.data(), serLen) == HasReturnvaluesIF::RETURN_OK); + buf[11] = 0x00; + REQUIRE(reader.parseDataWithCrcCheck() == PusIF::INVALID_CRC_16); + } + + SECTION("Invalid CRC but no check") { + REQUIRE(creator.serialize(&dataPtr, &serLen, buf.size()) == HasReturnvaluesIF::RETURN_OK); + REQUIRE(reader.setReadOnlyData(buf.data(), serLen) == HasReturnvaluesIF::RETURN_OK); + buf[11] = 0x00; + REQUIRE(reader.parseDataWithoutCrcCheck() == HasReturnvaluesIF::RETURN_OK); + } + + SECTION("With application data") { + auto& params = creator.getPusParams(); + std::array data{1, 2, 3}; + creator.setRawAppData({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); + const uint8_t* userDataPtr = reader.getUserData(); + REQUIRE(userDataPtr != nullptr); + REQUIRE(reader.getUserDataLen() == 3); + REQUIRE(userDataPtr[0] == 1); + REQUIRE(userDataPtr[1] == 2); + REQUIRE(userDataPtr[2] == 3); + } +} \ No newline at end of file diff --git a/unittests/tmtcpacket/testPusTm.cpp b/unittests/tmtcpacket/testPusTm.cpp deleted file mode 100644 index 8b137891..00000000 --- a/unittests/tmtcpacket/testPusTm.cpp +++ /dev/null @@ -1 +0,0 @@ -