From 902a4bfa9ce3228c6d14d96c00cd0f410a47141d Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 1 Aug 2022 17:16:37 +0200 Subject: [PATCH] unittests for TC and CCSDS distributor --- CMakeLists.txt | 4 +- src/fsfw/datapoollocal/HasLocalDataPoolIF.h | 2 +- src/fsfw/events/fwSubsystemIdRanges.h | 1 + .../serviceinterface/ServiceInterfaceBuffer.h | 1 - .../serviceinterface/ServiceInterfaceStream.h | 3 +- src/fsfw/tcdistribution/CcsdsDistributor.cpp | 69 ++++++----- src/fsfw/tcdistribution/CcsdsDistributor.h | 3 +- src/fsfw/tcdistribution/CcsdsDistributorIF.h | 4 +- src/fsfw/tcdistribution/CcsdsUnpacker.cpp | 21 ++-- src/fsfw/tcdistribution/CcsdsUnpacker.h | 3 +- src/fsfw/tcdistribution/CfdpDistributor.cpp | 2 + src/fsfw/tcdistribution/PusDistributor.cpp | 3 +- src/fsfw/tcdistribution/TcDistributorBase.cpp | 25 +++- src/fsfw/tcdistribution/TcDistributorBase.h | 8 +- .../tmtcpacket/ccsds/SpacePacketReader.cpp | 5 +- unittests/CMakeLists.txt | 1 + unittests/CatchDefinitions.cpp | 3 +- unittests/CatchDefinitions.h | 3 +- unittests/CatchSetup.cpp | 2 +- unittests/mocks/AcceptsTcMock.cpp | 8 ++ unittests/mocks/AcceptsTcMock.h | 20 ++++ unittests/mocks/CMakeLists.txt | 2 + unittests/mocks/CcsdsCheckerMock.cpp | 10 ++ unittests/mocks/CcsdsCheckerMock.h | 16 +++ unittests/mocks/PusDistributorMock.h | 4 +- unittests/tcdistributor/CMakeLists.txt | 3 + .../tcdistributor/testCcsdsDistributor.cpp | 113 ++++++++++++++++++ .../PollingSequenceFactory.cpp | 2 +- unittests/tmtcpacket/testCcsdsReader.cpp | 2 +- 29 files changed, 277 insertions(+), 66 deletions(-) create mode 100644 unittests/mocks/AcceptsTcMock.cpp create mode 100644 unittests/mocks/AcceptsTcMock.h create mode 100644 unittests/mocks/CcsdsCheckerMock.cpp create mode 100644 unittests/mocks/CcsdsCheckerMock.h create mode 100644 unittests/tcdistributor/CMakeLists.txt create mode 100644 unittests/tcdistributor/testCcsdsDistributor.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index cfae2acb..c9c267f2 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -104,7 +104,8 @@ if(FSFW_GENERATE_SECTIONS) option(FSFW_REMOVE_UNUSED_CODE "Remove unused code" ON) endif() -option(FSFW_BUILD_TESTS "Build unittest binary in addition to static library" +option(FSFW_BUILD_TESTS + "Build unittest binary in addition to static library. Requires Catch2" OFF) option(FSFW_CICD_BUILD "Build for CI/CD. This can disable problematic test" OFF) option(FSFW_BUILD_DOCS "Build documentation with Sphinx and Doxygen" OFF) @@ -115,7 +116,6 @@ endif() option(FSFW_WARNING_SHADOW_LOCAL_GCC "Enable -Wshadow=local warning in GCC" ON) # Options to exclude parts of the FSFW from compilation. option(FSFW_ADD_INTERNAL_TESTS "Add internal unit tests" ON) -option(FSFW_ADD_UNITTESTS "Add regular unittests. Requires Catch2" OFF) option(FSFW_ADD_HAL "Add Hardware Abstraction Layer" ON) # Optional sources diff --git a/src/fsfw/datapoollocal/HasLocalDataPoolIF.h b/src/fsfw/datapoollocal/HasLocalDataPoolIF.h index b987e760..a0f8b03a 100644 --- a/src/fsfw/datapoollocal/HasLocalDataPoolIF.h +++ b/src/fsfw/datapoollocal/HasLocalDataPoolIF.h @@ -3,11 +3,11 @@ #include +#include "LocalDataPoolManager.h" #include "fsfw/datapool/PoolEntryIF.h" #include "fsfw/housekeeping/HousekeepingMessage.h" #include "fsfw/ipc/MessageQueueSenderIF.h" #include "fsfw/serviceinterface.h" -#include "LocalDataPoolManager.h" #include "localPoolDefinitions.h" class AccessPoolManagerIF; diff --git a/src/fsfw/events/fwSubsystemIdRanges.h b/src/fsfw/events/fwSubsystemIdRanges.h index fa4351e9..a09c935c 100644 --- a/src/fsfw/events/fwSubsystemIdRanges.h +++ b/src/fsfw/events/fwSubsystemIdRanges.h @@ -19,6 +19,7 @@ enum : uint8_t { HK = 73, SYSTEM_MANAGER = 74, SYSTEM_MANAGER_1 = 75, + TC_DISTRIBUTION = 76, SYSTEM_1 = 79, PUS_SERVICE_1 = 80, PUS_SERVICE_2 = 82, diff --git a/src/fsfw/serviceinterface/ServiceInterfaceBuffer.h b/src/fsfw/serviceinterface/ServiceInterfaceBuffer.h index 6832e809..9e692a05 100644 --- a/src/fsfw/serviceinterface/ServiceInterfaceBuffer.h +++ b/src/fsfw/serviceinterface/ServiceInterfaceBuffer.h @@ -2,7 +2,6 @@ #define FRAMEWORK_SERVICEINTERFACE_SERVICEINTERFACEBUFFER_H_ #include "fsfw/FSFW.h" - #include "fsfw/returnvalues/HasReturnvaluesIF.h" #if FSFW_CPP_OSTREAM_ENABLED == 1 diff --git a/src/fsfw/serviceinterface/ServiceInterfaceStream.h b/src/fsfw/serviceinterface/ServiceInterfaceStream.h index bf3424a6..ca746e7d 100644 --- a/src/fsfw/serviceinterface/ServiceInterfaceStream.h +++ b/src/fsfw/serviceinterface/ServiceInterfaceStream.h @@ -1,9 +1,8 @@ #ifndef FRAMEWORK_SERVICEINTERFACE_SERVICEINTERFACESTREAM_H_ #define FRAMEWORK_SERVICEINTERFACE_SERVICEINTERFACESTREAM_H_ -#include "fsfw/FSFW.h" - #include "ServiceInterfaceBuffer.h" +#include "fsfw/FSFW.h" #if FSFW_CPP_OSTREAM_ENABLED == 1 diff --git a/src/fsfw/tcdistribution/CcsdsDistributor.cpp b/src/fsfw/tcdistribution/CcsdsDistributor.cpp index 816d3037..f168e20a 100644 --- a/src/fsfw/tcdistribution/CcsdsDistributor.cpp +++ b/src/fsfw/tcdistribution/CcsdsDistributor.cpp @@ -1,7 +1,7 @@ -#include "fsfw/FSFW.h" #include "fsfw/tcdistribution/CcsdsDistributor.h" #include "definitions.h" +#include "fsfw/FSFW.h" #include "fsfw/objectmanager/ObjectManager.h" #include "fsfw/serviceinterface/ServiceInterface.h" #include "fsfw/tmtcpacket/ccsds/SpacePacketReader.h" @@ -9,9 +9,11 @@ #define CCSDS_DISTRIBUTOR_DEBUGGING 0 CcsdsDistributor::CcsdsDistributor(uint16_t setDefaultApid, object_id_t setObjectId, + StorageManagerIF* tcStore, MessageQueueIF* queue, CcsdsPacketCheckIF* packetChecker) - : TcDistributorBase(setObjectId), + : TcDistributorBase(setObjectId, queue), defaultApid(setDefaultApid), + tcStore(tcStore), packetChecker(packetChecker) {} CcsdsDistributor::~CcsdsDistributor() = default; @@ -28,9 +30,8 @@ ReturnValue_t CcsdsDistributor::selectDestination(MessageQueueId_t& destId) { currentMessage.getStorageId().packetIndex); #endif #endif - const uint8_t* packet = nullptr; - size_t size = 0; - ReturnValue_t result = tcStore->getData(currentMessage.getStorageId(), &packet, &size); + auto accessorPair = tcStore->getData(currentMessage.getStorageId()); + ReturnValue_t result = accessorPair.first; if (result != HasReturnvaluesIF::RETURN_OK) { #if FSFW_VERBOSE_LEVEL >= 1 #if FSFW_CPP_OSTREAM_ENABLED == 1 @@ -45,8 +46,11 @@ ReturnValue_t CcsdsDistributor::selectDestination(MessageQueueId_t& destId) { #endif return result; } - SpacePacketReader currentPacket(packet, size); - result = packetChecker->checkPacket(currentPacket, size); + if (accessorPair.second.size() < ccsds::HEADER_LEN) { + return SerializeIF::STREAM_TOO_SHORT; + } + SpacePacketReader currentPacket(accessorPair.second.data(), accessorPair.second.size()); + result = packetChecker->checkPacket(currentPacket, accessorPair.second.size()); if (result != HasReturnvaluesIF::RETURN_OK) { handlePacketCheckFailure(result); return result; @@ -59,11 +63,12 @@ ReturnValue_t CcsdsDistributor::selectDestination(MessageQueueId_t& destId) { if (iter != receiverMap.end()) { destId = iter->second.destId; if (iter->second.removeHeader) { - handleCcsdsHeaderRemoval(); + // Do not call accessor release method here to ensure the old packet gets deleted. + return handleCcsdsHeaderRemoval(accessorPair.second); } } else { // The APID was not found. Forward packet to main SW-APID anyway to - // create acceptance failure report. + // create acceptance failure report. iter = receiverMap.find(defaultApid); if (iter != receiverMap.end()) { destId = iter->second.destId; @@ -71,6 +76,7 @@ ReturnValue_t CcsdsDistributor::selectDestination(MessageQueueId_t& destId) { return DESTINATION_NOT_FOUND; } } + accessorPair.second.release(); return HasReturnvaluesIF::RETURN_OK; } @@ -111,23 +117,28 @@ ReturnValue_t CcsdsDistributor::initialize() { if (packetChecker == nullptr) { packetChecker = new CcsdsPacketChecker(ccsds::PacketType::TC); } - ReturnValue_t status = this->TcDistributorBase::initialize(); - this->tcStore = ObjectManager::instance()->get(objects::TC_STORE); - if (this->tcStore == nullptr) { + ReturnValue_t result = TcDistributorBase::initialize(); + if (result != HasReturnvaluesIF::RETURN_OK) { + return result; + } + if (tcStore == nullptr) { + tcStore = ObjectManager::instance()->get(objects::TC_STORE); + if (tcStore == nullptr) { #if FSFW_VERBOSE_LEVEL >= 1 #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "CCSDSDistributor::initialize: Could not initialize" - " TC store!" - << std::endl; + sif::error << "CCSDSDistributor::initialize: Could not initialize" + " TC store!" + << std::endl; #else - sif::printError( - "CCSDSDistributor::initialize: Could not initialize" - " TC store!\n"); + sif::printError( + "CCSDSDistributor::initialize: Could not initialize" + " TC store!\n"); #endif #endif - status = RETURN_FAILED; + return ObjectManagerIF::CHILD_INIT_FAILED; + } } - return status; + return result; } ReturnValue_t CcsdsDistributor::callbackAfterSending(ReturnValue_t queueStatus) { @@ -151,15 +162,19 @@ void CcsdsDistributor::print() { const char* CcsdsDistributor::getName() const { return "CCSDS Distributor"; } -ReturnValue_t CcsdsDistributor::handleCcsdsHeaderRemoval() { - currentMessage; - auto accessorPair = tcStore->getData(currentMessage.getStorageId()); - if(accessorPair.first != HasReturnvaluesIF::RETURN_OK) { +ReturnValue_t CcsdsDistributor::handleCcsdsHeaderRemoval(ConstStorageAccessor& accessor) { + store_address_t newStoreId; + ReturnValue_t result = tcStore->addData(&newStoreId, accessor.data() + ccsds::HEADER_LEN, + accessor.size() - ccsds::HEADER_LEN); + if (result != HasReturnvaluesIF::RETURN_OK) { #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << __func__ << ": Getting TC data failed" << std::endl; + sif::error << __func__ << ": TC store full" << std::endl; #else - sif::printError("%s: Getting TC data failed\n", __func__); + sif::printError("%s: TC store full\n", __func__); #endif - return accessorPair.first; + return result; } + currentMessage.setStorageId(newStoreId); + // The const accessor will delete the old data automatically + return HasReturnvaluesIF::RETURN_OK; } diff --git a/src/fsfw/tcdistribution/CcsdsDistributor.h b/src/fsfw/tcdistribution/CcsdsDistributor.h index 542625eb..c2ada02d 100644 --- a/src/fsfw/tcdistribution/CcsdsDistributor.h +++ b/src/fsfw/tcdistribution/CcsdsDistributor.h @@ -31,6 +31,7 @@ class CcsdsDistributor : public TcDistributorBase, * destination are sent to. */ CcsdsDistributor(uint16_t unknownApid, object_id_t setObjectId, + StorageManagerIF* tcStore = nullptr, MessageQueueIF* msgQueue = nullptr, CcsdsPacketCheckIF* packetChecker = nullptr); /** * The destructor is empty. @@ -63,7 +64,7 @@ class CcsdsDistributor : public TcDistributorBase, static void handlePacketCheckFailure(ReturnValue_t result); - ReturnValue_t handleCcsdsHeaderRemoval(); + ReturnValue_t handleCcsdsHeaderRemoval(ConstStorageAccessor& accessor); void print(); /** * The default APID, where packets with unknown APID are sent to. diff --git a/src/fsfw/tcdistribution/CcsdsDistributorIF.h b/src/fsfw/tcdistribution/CcsdsDistributorIF.h index 7e7a586d..6deb1c67 100644 --- a/src/fsfw/tcdistribution/CcsdsDistributorIF.h +++ b/src/fsfw/tcdistribution/CcsdsDistributorIF.h @@ -17,8 +17,8 @@ class CcsdsDistributorIF { struct DestInfo { DestInfo(const char* name, uint16_t apid, MessageQueueId_t destId, bool removeHeader) : name(name), apid(apid), destId(destId), removeHeader(removeHeader) {} - DestInfo(const char* name, const AcceptsTelecommandsIF& ccsdsReceiver, bool removeHeader_) - : name(name) { + DestInfo(const AcceptsTelecommandsIF& ccsdsReceiver, bool removeHeader_) + : name(ccsdsReceiver.getName()) { apid = ccsdsReceiver.getIdentifier(); destId = ccsdsReceiver.getRequestQueue(); removeHeader = removeHeader_; diff --git a/src/fsfw/tcdistribution/CcsdsUnpacker.cpp b/src/fsfw/tcdistribution/CcsdsUnpacker.cpp index 42a7cd28..9b212d2e 100644 --- a/src/fsfw/tcdistribution/CcsdsUnpacker.cpp +++ b/src/fsfw/tcdistribution/CcsdsUnpacker.cpp @@ -4,29 +4,28 @@ CcsdsUnpacker::CcsdsUnpacker(MessageQueueIF& msgQueue, AcceptsTelecommandsIF& receiver, StorageManagerIF& sourceStore) - : sourceStore(sourceStore), msgQueue(msgQueue), receiver(receiver) { + : sourceStore(sourceStore), msgQueue(msgQueue), receiver(receiver) { msgQueue.setDefaultDestination(receiver.getRequestQueue()); } ReturnValue_t CcsdsUnpacker::performOperation(uint8_t operationCode) { TmTcMessage msg; ReturnValue_t result; - for (result = msgQueue.receiveMessage(&msg); result == HasReturnvaluesIF::RETURN_OK; + for (result = msgQueue.receiveMessage(&msg); result == HasReturnvaluesIF::RETURN_OK; result = msgQueue.receiveMessage(&msg)) { auto resultPair = sourceStore.getData(msg.getStorageId()); - if(resultPair.first != HasReturnvaluesIF::RETURN_OK) { + if (resultPair.first != HasReturnvaluesIF::RETURN_OK) { continue; } - if(resultPair.second.size() < 6) { + if (resultPair.second.size() < 6) { // TODO: This is a config error. Does it make sense to forward the message? result = msgQueue.sendToDefault(&msg); - if(result != HasReturnvaluesIF::RETURN_OK) { - + if (result != HasReturnvaluesIF::RETURN_OK) { } continue; } StorageManagerIF* tgtStore; - if(targetStore != nullptr) { + if (targetStore != nullptr) { tgtStore = targetStore; } else { tgtStore = &sourceStore; @@ -34,24 +33,22 @@ ReturnValue_t CcsdsUnpacker::performOperation(uint8_t operationCode) { store_address_t newId; uint8_t* ptr; result = tgtStore->getFreeElement(&newId, resultPair.second.size(), &ptr); - if(result != HasReturnvaluesIF::RETURN_OK) { + if (result != HasReturnvaluesIF::RETURN_OK) { // TODO: Implement error handling } std::memcpy(ptr, resultPair.second.data() + 6, resultPair.second.size() - 6); result = sourceStore.deleteData(msg.getStorageId()); - if(result != HasReturnvaluesIF::RETURN_OK) { + if (result != HasReturnvaluesIF::RETURN_OK) { // TODO: Implement error handling (though this really should not happen) } TmTcMessage newMsg(newId); result = msgQueue.sendToDefault(&newMsg); - if(result != HasReturnvaluesIF::RETURN_OK) { - + if (result != HasReturnvaluesIF::RETURN_OK) { } } return result; } - void CcsdsUnpacker::setDifferentTargetStore(StorageManagerIF& otherTargetStore) { targetStore = &otherTargetStore; } diff --git a/src/fsfw/tcdistribution/CcsdsUnpacker.h b/src/fsfw/tcdistribution/CcsdsUnpacker.h index d8c5e8b5..267b8e64 100644 --- a/src/fsfw/tcdistribution/CcsdsUnpacker.h +++ b/src/fsfw/tcdistribution/CcsdsUnpacker.h @@ -7,7 +7,8 @@ class CcsdsUnpacker : public ExecutableObjectIF, public AcceptsTelecommandsIF { public: - CcsdsUnpacker(MessageQueueIF& msgQueue, AcceptsTelecommandsIF& receiver, StorageManagerIF& sourceStore); + CcsdsUnpacker(MessageQueueIF& msgQueue, AcceptsTelecommandsIF& receiver, + StorageManagerIF& sourceStore); void setDifferentTargetStore(StorageManagerIF& otherTargetStore); ReturnValue_t performOperation(uint8_t operationCode) override; diff --git a/src/fsfw/tcdistribution/CfdpDistributor.cpp b/src/fsfw/tcdistribution/CfdpDistributor.cpp index abbad748..3b1d1538 100644 --- a/src/fsfw/tcdistribution/CfdpDistributor.cpp +++ b/src/fsfw/tcdistribution/CfdpDistributor.cpp @@ -68,6 +68,7 @@ ReturnValue_t CfdpDistributor::selectDestination(MessageQueueId_t& destId) { // } else { // return queueMapIt; // } + return HasReturnvaluesIF::RETURN_OK; } ReturnValue_t CfdpDistributor::registerHandler(AcceptsTelecommandsIF* handler) { @@ -141,4 +142,5 @@ ReturnValue_t CfdpDistributor::initialize() { // return RETURN_FAILED; // } // return ccsdsDistributor->registerApplication(this); + return HasReturnvaluesIF::RETURN_OK; } diff --git a/src/fsfw/tcdistribution/PusDistributor.cpp b/src/fsfw/tcdistribution/PusDistributor.cpp index e85a6047..7be0810d 100644 --- a/src/fsfw/tcdistribution/PusDistributor.cpp +++ b/src/fsfw/tcdistribution/PusDistributor.cpp @@ -139,8 +139,7 @@ ReturnValue_t PusDistributor::initialize() { return ObjectManagerIF::CHILD_INIT_FAILED; } } - return ccsdsDistributor->registerApplication( - CcsdsDistributorIF::DestInfo(getName(), *this, false)); + return ccsdsDistributor->registerApplication(CcsdsDistributorIF::DestInfo(*this, false)); } void PusDistributor::checkerFailurePrinter() const { diff --git a/src/fsfw/tcdistribution/TcDistributorBase.cpp b/src/fsfw/tcdistribution/TcDistributorBase.cpp index 70388b99..33c779eb 100644 --- a/src/fsfw/tcdistribution/TcDistributorBase.cpp +++ b/src/fsfw/tcdistribution/TcDistributorBase.cpp @@ -3,22 +3,35 @@ #include "fsfw/ipc/QueueFactory.h" #include "fsfw/tmtcservices/TmTcMessage.h" -TcDistributorBase::TcDistributorBase(object_id_t objectId) : SystemObject(objectId) { - tcQueue = QueueFactory::instance()->createMessageQueue(DISTRIBUTER_MAX_PACKETS); +TcDistributorBase::TcDistributorBase(object_id_t objectId, MessageQueueIF* tcQueue_) + : SystemObject(objectId), tcQueue(tcQueue_) { + if (tcQueue == nullptr) { + ownedQueue = true; + tcQueue = QueueFactory::instance()->createMessageQueue(DISTRIBUTER_MAX_PACKETS); + } } -TcDistributorBase::~TcDistributorBase() { QueueFactory::instance()->deleteMessageQueue(tcQueue); } +TcDistributorBase::~TcDistributorBase() { + if (ownedQueue) { + QueueFactory::instance()->deleteMessageQueue(tcQueue); + } +} ReturnValue_t TcDistributorBase::performOperation(uint8_t opCode) { ReturnValue_t status; + ReturnValue_t result = HasReturnvaluesIF::RETURN_OK; for (status = tcQueue->receiveMessage(¤tMessage); status == RETURN_OK; status = tcQueue->receiveMessage(¤tMessage)) { - status = handlePacket(); + ReturnValue_t packetResult = handlePacket(); + if (packetResult != HasReturnvaluesIF::RETURN_OK) { + result = packetResult; + triggerEvent(HANDLE_PACKET_FAILED, packetResult, __LINE__); + } } if (status == MessageQueueIF::EMPTY) { - return RETURN_OK; + return result; } - return status; + return result; } ReturnValue_t TcDistributorBase::handlePacket() { diff --git a/src/fsfw/tcdistribution/TcDistributorBase.h b/src/fsfw/tcdistribution/TcDistributorBase.h index 08b72823..c2bee0b9 100644 --- a/src/fsfw/tcdistribution/TcDistributorBase.h +++ b/src/fsfw/tcdistribution/TcDistributorBase.h @@ -3,6 +3,7 @@ #include +#include "fsfw/events/Event.h" #include "fsfw/ipc/MessageQueueIF.h" #include "fsfw/objectmanager/ObjectManagerIF.h" #include "fsfw/objectmanager/SystemObject.h" @@ -33,6 +34,10 @@ class TcDistributorBase : public SystemObject, public ExecutableObjectIF, public static constexpr ReturnValue_t PACKET_LOST = MAKE_RETURN_CODE(1); static constexpr ReturnValue_t DESTINATION_NOT_FOUND = MAKE_RETURN_CODE(2); static constexpr ReturnValue_t SERVICE_ID_ALREADY_EXISTS = MAKE_RETURN_CODE(3); + + static constexpr uint8_t SUBSYSTEM_ID = SUBSYSTEM_ID::TC_DISTRIBUTION; + //! P1: Returnvalue, P2: Line number + static constexpr Event HANDLE_PACKET_FAILED = event::makeEvent(SUBSYSTEM_ID, 0, severity::LOW); /** * Within the default constructor, the SystemObject id is set and the * message queue is initialized. @@ -40,7 +45,7 @@ class TcDistributorBase : public SystemObject, public ExecutableObjectIF, public * @param set_object_id This id is assigned to the distributor * implementation. */ - explicit TcDistributorBase(object_id_t objectId); + explicit TcDistributorBase(object_id_t objectId, MessageQueueIF* tcQueue = nullptr); /** * The destructor is empty, the message queues are not in the vicinity of * this class. @@ -56,6 +61,7 @@ class TcDistributorBase : public SystemObject, public ExecutableObjectIF, public ReturnValue_t performOperation(uint8_t opCode) override; protected: + bool ownedQueue = false; /** * This is the receiving queue for incoming Telecommands. * The child classes must make its queue id public. diff --git a/src/fsfw/tmtcpacket/ccsds/SpacePacketReader.cpp b/src/fsfw/tmtcpacket/ccsds/SpacePacketReader.cpp index b6f03de9..4a140dac 100644 --- a/src/fsfw/tmtcpacket/ccsds/SpacePacketReader.cpp +++ b/src/fsfw/tmtcpacket/ccsds/SpacePacketReader.cpp @@ -35,8 +35,11 @@ uint16_t SpacePacketReader::getPacketDataLen() const { return ccsds::getPacketLe ReturnValue_t SpacePacketReader::setInternalFields(const uint8_t* data, size_t maxSize_) { bufSize = maxSize_; + if (maxSize_ < ccsds::HEADER_LEN) { + return SerializeIF::STREAM_TOO_SHORT; + } spHeader = reinterpret_cast(data); - if (maxSize_ > 6) { + if (maxSize_ > ccsds::HEADER_LEN) { packetDataField = data + ccsds::HEADER_LEN; } return checkSize(); diff --git a/unittests/CMakeLists.txt b/unittests/CMakeLists.txt index fc03a728..595d9553 100644 --- a/unittests/CMakeLists.txt +++ b/unittests/CMakeLists.txt @@ -13,6 +13,7 @@ target_sources(${FSFW_TEST_TGT} PRIVATE add_subdirectory(testcfg) add_subdirectory(mocks) +add_subdirectory(tcdistributor) add_subdirectory(action) add_subdirectory(power) add_subdirectory(util) diff --git a/unittests/CatchDefinitions.cpp b/unittests/CatchDefinitions.cpp index 15ff1de9..3c76fd88 100644 --- a/unittests/CatchDefinitions.cpp +++ b/unittests/CatchDefinitions.cpp @@ -1,9 +1,10 @@ -#include "fsfw/FSFW.h" #include "CatchDefinitions.h" #include #include +#include "fsfw/FSFW.h" + StorageManagerIF* tglob::getIpcStoreHandle() { if (ObjectManager::instance() != nullptr) { return ObjectManager::instance()->get(objects::IPC_STORE); diff --git a/unittests/CatchDefinitions.h b/unittests/CatchDefinitions.h index be49e6e8..7ffad55a 100644 --- a/unittests/CatchDefinitions.h +++ b/unittests/CatchDefinitions.h @@ -1,11 +1,12 @@ #ifndef FSFW_UNITTEST_CORE_CATCHDEFINITIONS_H_ #define FSFW_UNITTEST_CORE_CATCHDEFINITIONS_H_ -#include "fsfw/FSFW.h" #include #include #include +#include "fsfw/FSFW.h" + namespace tconst { static constexpr MessageQueueId_t testQueueId = 42; } diff --git a/unittests/CatchSetup.cpp b/unittests/CatchSetup.cpp index 848af5e7..4f2a4a54 100644 --- a/unittests/CatchSetup.cpp +++ b/unittests/CatchSetup.cpp @@ -1,6 +1,6 @@ -#include "fsfw/FSFW.h" #include "CatchDefinitions.h" #include "CatchFactory.h" +#include "fsfw/FSFW.h" #ifdef GCOV #include diff --git a/unittests/mocks/AcceptsTcMock.cpp b/unittests/mocks/AcceptsTcMock.cpp new file mode 100644 index 00000000..a9afafd9 --- /dev/null +++ b/unittests/mocks/AcceptsTcMock.cpp @@ -0,0 +1,8 @@ +#include "AcceptsTcMock.h" + +AcceptsTcMock::AcceptsTcMock(const char* name, uint32_t id, MessageQueueId_t queueId) + : name(name), id(id), queueId(queueId) {} + +const char* AcceptsTcMock::getName() const { return name; } +uint32_t AcceptsTcMock::getIdentifier() const { return id; } +MessageQueueId_t AcceptsTcMock::getRequestQueue() const { return queueId; } diff --git a/unittests/mocks/AcceptsTcMock.h b/unittests/mocks/AcceptsTcMock.h new file mode 100644 index 00000000..028b68cd --- /dev/null +++ b/unittests/mocks/AcceptsTcMock.h @@ -0,0 +1,20 @@ +#ifndef FSFW_TESTS_ACCEPTSTCMOCK_H +#define FSFW_TESTS_ACCEPTSTCMOCK_H + +#include "fsfw/tmtcservices/AcceptsTelecommandsIF.h" + +class AcceptsTcMock : public AcceptsTelecommandsIF { + public: + AcceptsTcMock(const char* name, uint32_t id, MessageQueueId_t queueId); + [[nodiscard]] const char* getName() const override; + [[nodiscard]] uint32_t getIdentifier() const override; + [[nodiscard]] MessageQueueId_t getRequestQueue() const override; + + const char* name; + uint32_t id; + MessageQueueId_t queueId; + + private: +}; + +#endif // FSFW_TESTS_ACCEPTSTCMOCK_H diff --git a/unittests/mocks/CMakeLists.txt b/unittests/mocks/CMakeLists.txt index f3b50f62..3a8c8d18 100644 --- a/unittests/mocks/CMakeLists.txt +++ b/unittests/mocks/CMakeLists.txt @@ -11,4 +11,6 @@ target_sources(${FSFW_TEST_TGT} PRIVATE PusServiceBaseMock.cpp AcceptsTmMock.cpp PusDistributorMock.cpp + CcsdsCheckerMock.cpp + AcceptsTcMock.cpp ) diff --git a/unittests/mocks/CcsdsCheckerMock.cpp b/unittests/mocks/CcsdsCheckerMock.cpp new file mode 100644 index 00000000..18c5fb4e --- /dev/null +++ b/unittests/mocks/CcsdsCheckerMock.cpp @@ -0,0 +1,10 @@ +#include "CcsdsCheckerMock.h" + +CcsdsCheckerMock::CcsdsCheckerMock() = default; + +ReturnValue_t CcsdsCheckerMock::checkPacket(const SpacePacketReader& currentPacket, + size_t packetLen) { + checkCallCount++; + checkedPacketLen = packetLen; + return nextResult; +} diff --git a/unittests/mocks/CcsdsCheckerMock.h b/unittests/mocks/CcsdsCheckerMock.h new file mode 100644 index 00000000..a9388ae5 --- /dev/null +++ b/unittests/mocks/CcsdsCheckerMock.h @@ -0,0 +1,16 @@ +#ifndef FSFW_TESTS_CCSDSCHECKERMOCK_H +#define FSFW_TESTS_CCSDSCHECKERMOCK_H + +#include "fsfw/tcdistribution/CcsdsPacketCheckIF.h" +class CcsdsCheckerMock : public CcsdsPacketCheckIF { + public: + CcsdsCheckerMock(); + unsigned int checkCallCount = 0; + size_t checkedPacketLen = 0; + ReturnValue_t nextResult = HasReturnvaluesIF::RETURN_OK; + ReturnValue_t checkPacket(const SpacePacketReader& currentPacket, size_t packetLen) override; + + private: +}; + +#endif // FSFW_TESTS_CCSDSCHECKERMOCK_H diff --git a/unittests/mocks/PusDistributorMock.h b/unittests/mocks/PusDistributorMock.h index d85030c9..417ae448 100644 --- a/unittests/mocks/PusDistributorMock.h +++ b/unittests/mocks/PusDistributorMock.h @@ -1,11 +1,11 @@ #ifndef FSFW_TESTS_PUSDISTRIBUTORMOCK_H #define FSFW_TESTS_PUSDISTRIBUTORMOCK_H +#include + #include "fsfw/objectmanager/SystemObject.h" #include "fsfw/tcdistribution/PusDistributorIF.h" -#include - class PusDistributorMock : public SystemObject, public PusDistributorIF { public: PusDistributorMock(); diff --git a/unittests/tcdistributor/CMakeLists.txt b/unittests/tcdistributor/CMakeLists.txt new file mode 100644 index 00000000..d4182326 --- /dev/null +++ b/unittests/tcdistributor/CMakeLists.txt @@ -0,0 +1,3 @@ +target_sources(${FSFW_TEST_TGT} PRIVATE + testCcsdsDistributor.cpp +) diff --git a/unittests/tcdistributor/testCcsdsDistributor.cpp b/unittests/tcdistributor/testCcsdsDistributor.cpp new file mode 100644 index 00000000..8aa248fa --- /dev/null +++ b/unittests/tcdistributor/testCcsdsDistributor.cpp @@ -0,0 +1,113 @@ +#include +#include + +#include "fsfw/storagemanager/LocalPool.h" +#include "fsfw/tcdistribution/CcsdsDistributor.h" +#include "fsfw/tmtcpacket/ccsds/SpacePacketCreator.h" +#include "mocks/AcceptsTcMock.h" +#include "mocks/CcsdsCheckerMock.h" +#include "mocks/MessageQueueMock.h" + +TEST_CASE("CCSDS Distributor", "[ccsds-distrib]") { + LocalPool::LocalPoolConfig cfg = {{5, 32}, {2, 64}}; + LocalPool pool(objects::NO_OBJECT, cfg); + auto queue = MessageQueueMock(1); + auto checkerMock = CcsdsCheckerMock(); + uint16_t unregisteredApid = 0; + uint16_t defaultApid = 4; + MessageQueueId_t defaultQueueId = 5; + auto ccsdsDistrib = CcsdsDistributor(defaultApid, 1, &pool, &queue, &checkerMock); + uint32_t tcAcceptorApid = 1; + MessageQueueId_t tcAcceptorQueueId = 3; + + auto tcAcceptorMock = AcceptsTcMock("TC Receiver Dummy", tcAcceptorApid, tcAcceptorQueueId); + auto defReceiverMock = AcceptsTcMock("Default Receiver Dummy", defaultApid, defaultQueueId); + auto packetId = PacketId(ccsds::PacketType::TC, true, 0); + auto psc = PacketSeqCtrl(ccsds::SequenceFlags::FIRST_SEGMENT, 0x34); + auto spParams = SpacePacketParams(packetId, psc, 0x16); + SpacePacketCreator spCreator(spParams); + std::array buf{}; + + auto createSpacePacket = [&](uint16_t apid, TmTcMessage& msg) { + store_address_t storeId{}; + spCreator.setApid(tcAcceptorApid); + uint8_t* dataPtr; + REQUIRE(pool.getFreeElement(&storeId, spCreator.getSerializedSize(), &dataPtr) == result::OK); + size_t serLen = 0; + REQUIRE(spCreator.SerializeIF::serializeBe(dataPtr, serLen, ccsds::HEADER_LEN) == result::OK); + REQUIRE(spCreator.SerializeIF::serializeBe(buf.data(), serLen, ccsds::HEADER_LEN) == + result::OK); + msg.setStorageId(storeId); + }; + + SECTION("State") { + CHECK(ccsdsDistrib.initialize() == result::OK); + CHECK(ccsdsDistrib.getRequestQueue() == 1); + CHECK(ccsdsDistrib.getIdentifier() == 0); + CHECK(ccsdsDistrib.getObjectId() == 1); + REQUIRE(ccsdsDistrib.getName() != nullptr); + CHECK(std::strcmp(ccsdsDistrib.getName(), "CCSDS Distributor") == 0); + } + + SECTION("Basic Forwarding") { + CcsdsDistributor::DestInfo info(tcAcceptorMock, false); + REQUIRE(ccsdsDistrib.registerApplication(info) == result::OK); + TmTcMessage message; + createSpacePacket(tcAcceptorApid, message); + store_address_t storeId = message.getStorageId(); + queue.addReceivedMessage(message); + REQUIRE(ccsdsDistrib.performOperation(0) == result::OK); + CHECK(checkerMock.checkedPacketLen == 6); + CHECK(checkerMock.checkCallCount == 1); + CHECK(queue.wasMessageSent()); + CHECK(queue.numberOfSentMessages() == 1); + // The packet is forwarded, with no need to delete the data + CHECK(pool.hasDataAtId(storeId)); + TmTcMessage sentMsg; + CHECK(queue.getNextSentMessage(tcAcceptorQueueId, sentMsg) == result::OK); + CHECK(sentMsg.getStorageId() == storeId); + auto accessor = pool.getData(storeId); + CHECK(accessor.first == result::OK); + CHECK(accessor.second.size() == ccsds::HEADER_LEN); + for (size_t i = 0; i < ccsds::HEADER_LEN; i++) { + CHECK(accessor.second.data()[i] == buf[i]); + } + } + + SECTION("Forwarding to Default Destination, but not registered") { + TmTcMessage message; + createSpacePacket(unregisteredApid, message); + store_address_t storeId = message.getStorageId(); + message.setStorageId(storeId); + queue.addReceivedMessage(message); + REQUIRE(ccsdsDistrib.performOperation(0) == TcDistributorBase::DESTINATION_NOT_FOUND); + } + + SECTION("Forward to Default Handler") { + CcsdsDistributor::DestInfo info(defReceiverMock, false); + ccsdsDistrib.registerApplication(info); + TmTcMessage message; + createSpacePacket(defaultApid, message); + store_address_t storeId = message.getStorageId(); + message.setStorageId(storeId); + queue.addReceivedMessage(message); + REQUIRE(ccsdsDistrib.performOperation(0) == result::OK); + CHECK(checkerMock.checkedPacketLen == 6); + CHECK(checkerMock.checkCallCount == 1); + CHECK(queue.wasMessageSent()); + CHECK(queue.numberOfSentMessages() == 1); + // The packet is forwarded, with no need to delete the data + CHECK(pool.hasDataAtId(storeId)); + TmTcMessage sentMsg; + CHECK(queue.getNextSentMessage(defaultQueueId, sentMsg) == result::OK); + CHECK(sentMsg.getStorageId() == storeId); + auto accessor = pool.getData(storeId); + CHECK(accessor.first == result::OK); + CHECK(accessor.second.size() == ccsds::HEADER_LEN); + for (size_t i = 0; i < ccsds::HEADER_LEN; i++) { + CHECK(accessor.second.data()[i] == buf[i]); + } + } + + SECTION("Remove CCSDS header") {} +} \ No newline at end of file diff --git a/unittests/testcfg/pollingsequence/PollingSequenceFactory.cpp b/unittests/testcfg/pollingsequence/PollingSequenceFactory.cpp index 34a8cfbd..cdf40d89 100644 --- a/unittests/testcfg/pollingsequence/PollingSequenceFactory.cpp +++ b/unittests/testcfg/pollingsequence/PollingSequenceFactory.cpp @@ -1,10 +1,10 @@ -#include "fsfw/FSFW.h" #include "PollingSequenceFactory.h" #include #include #include +#include "fsfw/FSFW.h" #include "tests/TestsConfig.h" ReturnValue_t pst::pollingSequenceInitDefault(FixedTimeslotTaskIF *thisSequence) { diff --git a/unittests/tmtcpacket/testCcsdsReader.cpp b/unittests/tmtcpacket/testCcsdsReader.cpp index 75b4af87..61c4dd55 100644 --- a/unittests/tmtcpacket/testCcsdsReader.cpp +++ b/unittests/tmtcpacket/testCcsdsReader.cpp @@ -64,7 +64,7 @@ TEST_CASE("CCSDS Reader", "[ccsds-reader]") { SECTION("Invalid Size") { for (size_t i = 0; i < 5; i++) { REQUIRE(reader.setReadOnlyData(buf.data(), i) == SerializeIF::STREAM_TOO_SHORT); - REQUIRE(not reader.isNull()); + REQUIRE(reader.isNull()); REQUIRE(reader.getPacketData() == nullptr); } }