From 0d26a0f54bdb00eabdffa7b6964377c6d0ba1b77 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 9 Aug 2022 13:04:23 +0200 Subject: [PATCH] fnish CFDP distributor unittests --- src/fsfw/cfdp/CfdpDistributor.h | 8 ++++ .../storagemanager/ConstStorageAccessor.h | 4 +- src/fsfw/storagemanager/LocalPool.cpp | 3 +- src/fsfw/storagemanager/PoolManager.h | 8 ++-- src/fsfw/storagemanager/StorageAccessor.h | 4 +- src/fsfw/storagemanager/StorageManagerIF.h | 3 +- unittests/cfdp/testDistributor.cpp | 40 ++++++++++++++++++- unittests/mocks/StorageManagerMock.cpp | 16 ++------ unittests/mocks/StorageManagerMock.h | 9 +++-- 9 files changed, 66 insertions(+), 29 deletions(-) diff --git a/src/fsfw/cfdp/CfdpDistributor.h b/src/fsfw/cfdp/CfdpDistributor.h index 8fece981..64c5ca9f 100644 --- a/src/fsfw/cfdp/CfdpDistributor.h +++ b/src/fsfw/cfdp/CfdpDistributor.h @@ -42,6 +42,14 @@ class CfdpDistributor : public TcDistributorBase, public AcceptsTelecommandsIF { [[nodiscard]] const char* getName() const override; [[nodiscard]] uint32_t getIdentifier() const override; [[nodiscard]] MessageQueueId_t getRequestQueue() const override; + + /** + * Register a new CFDP entity which can receive PDUs. + * @param address + * @param tcDest + * @return + * - @c RETURN_FAILED: Entry already exists for the given address + */ ReturnValue_t registerTcDestination(const cfdp::EntityId& address, AcceptsTelecommandsIF& tcDest); protected: diff --git a/src/fsfw/storagemanager/ConstStorageAccessor.h b/src/fsfw/storagemanager/ConstStorageAccessor.h index e70d3f40..a935e467 100644 --- a/src/fsfw/storagemanager/ConstStorageAccessor.h +++ b/src/fsfw/storagemanager/ConstStorageAccessor.h @@ -80,8 +80,8 @@ class ConstStorageAccessor { * @param * @return */ - ConstStorageAccessor& operator=(ConstStorageAccessor&&) noexcept ; - ConstStorageAccessor(ConstStorageAccessor&&) noexcept ; + ConstStorageAccessor& operator=(ConstStorageAccessor&&) noexcept; + ConstStorageAccessor(ConstStorageAccessor&&) noexcept; //! The copy ctor and copy assignemnt should be deleted implicitely //! according to https://foonathan.net/2019/02/special-member-functions/ diff --git a/src/fsfw/storagemanager/LocalPool.cpp b/src/fsfw/storagemanager/LocalPool.cpp index e40315ed..bcb1bea6 100644 --- a/src/fsfw/storagemanager/LocalPool.cpp +++ b/src/fsfw/storagemanager/LocalPool.cpp @@ -167,8 +167,7 @@ void LocalPool::clearStore() { } } -ReturnValue_t LocalPool::reserveSpace(size_t size, store_address_t* storeId, - bool ignoreFault) { +ReturnValue_t LocalPool::reserveSpace(size_t size, store_address_t* storeId, bool ignoreFault) { ReturnValue_t status = getSubPoolIndex(size, &storeId->poolIndex); if (status != RETURN_OK) { #if FSFW_CPP_OSTREAM_ENABLED == 1 diff --git a/src/fsfw/storagemanager/PoolManager.h b/src/fsfw/storagemanager/PoolManager.h index 0951a518..eaa978ef 100644 --- a/src/fsfw/storagemanager/PoolManager.h +++ b/src/fsfw/storagemanager/PoolManager.h @@ -27,7 +27,7 @@ class PoolManager : public LocalPool { * @brief In the PoolManager's destructor all allocated memory * is freed. */ - virtual ~PoolManager(); + ~PoolManager() override; /** * Set the default mutex timeout for internal calls. @@ -40,8 +40,7 @@ class PoolManager : public LocalPool { * which wraps LocalPool calls with a mutex protection. */ ReturnValue_t deleteData(store_address_t) override; - ReturnValue_t deleteData(uint8_t* buffer, size_t size, - store_address_t* storeId = nullptr) override; + ReturnValue_t deleteData(uint8_t* buffer, size_t size, store_address_t* storeId) override; /** * The developer is allowed to lock the mutex in case the lock needs @@ -58,8 +57,7 @@ class PoolManager : public LocalPool { //! Default mutex timeout value to prevent permanent blocking. uint32_t mutexTimeoutMs = 20; - ReturnValue_t reserveSpace(const size_t size, store_address_t* address, - bool ignoreFault) override; + ReturnValue_t reserveSpace(size_t size, store_address_t* address, bool ignoreFault) override; /** * @brief The mutex is created in the constructor and makes diff --git a/src/fsfw/storagemanager/StorageAccessor.h b/src/fsfw/storagemanager/StorageAccessor.h index f51ba7e6..b6c5dc14 100644 --- a/src/fsfw/storagemanager/StorageAccessor.h +++ b/src/fsfw/storagemanager/StorageAccessor.h @@ -26,8 +26,8 @@ class StorageAccessor : public ConstStorageAccessor { * @param * @return */ - StorageAccessor& operator=(StorageAccessor&&) noexcept ; - StorageAccessor(StorageAccessor&&) noexcept ; + StorageAccessor& operator=(StorageAccessor&&) noexcept; + StorageAccessor(StorageAccessor&&) noexcept; ReturnValue_t write(uint8_t* data, size_t size, uint16_t offset = 0); uint8_t* data(); diff --git a/src/fsfw/storagemanager/StorageManagerIF.h b/src/fsfw/storagemanager/StorageManagerIF.h index 36ad5370..502348de 100644 --- a/src/fsfw/storagemanager/StorageManagerIF.h +++ b/src/fsfw/storagemanager/StorageManagerIF.h @@ -92,8 +92,7 @@ class StorageManagerIF : public HasReturnvaluesIF { * @return @li RETURN_OK on success. * @li failure code if deletion did not work */ - virtual ReturnValue_t deleteData(uint8_t* buffer, size_t size, - store_address_t* storeId) = 0; + virtual ReturnValue_t deleteData(uint8_t* buffer, size_t size, store_address_t* storeId) = 0; virtual ReturnValue_t deleteData(uint8_t* buffer, size_t size) { return deleteData(buffer, size, nullptr); } diff --git a/unittests/cfdp/testDistributor.cpp b/unittests/cfdp/testDistributor.cpp index c5ed9948..2e94a55e 100644 --- a/unittests/cfdp/testDistributor.cpp +++ b/unittests/cfdp/testDistributor.cpp @@ -3,6 +3,7 @@ #include "fsfw/cfdp/CfdpDistributor.h" #include "fsfw/cfdp/pdu/MetadataPduCreator.h" #include "fsfw/storagemanager/LocalPool.h" +#include "fsfw/tcdistribution/definitions.h" #include "mocks/AcceptsTcMock.h" #include "mocks/MessageQueueMock.h" #include "mocks/StorageManagerMock.h" @@ -17,6 +18,8 @@ TEST_CASE("CFDP Distributor", "[cfdp][distributor]") { auto groundEntityId = cfdp::EntityId(UnsignedByteField(1)); MessageQueueId_t receiverQueueId = 3; auto tcAcceptor = AcceptsTcMock("CFDP Receiver", 0, receiverQueueId); + + // Set up Metadata PDU for generate test data. cfdp::FileSize fileSize(12); const cfdp::EntityId& sourceId(groundEntityId); const cfdp::EntityId& destId(obswEntityId); @@ -43,7 +46,8 @@ TEST_CASE("CFDP Distributor", "[cfdp][distributor]") { CHECK(distributor.registerTcDestination(obswEntityId, tcAcceptor) == result::OK); size_t serLen = 0; store_address_t storeId; - CHECK(pool.LocalPool::getFreeElement(&storeId, creator.getSerializedSize(), &dataPtr) == result::OK); + CHECK(pool.LocalPool::getFreeElement(&storeId, creator.getSerializedSize(), &dataPtr) == + result::OK); REQUIRE(creator.SerializeIF::serializeBe(dataPtr, serLen, creator.getSerializedSize()) == result::OK); TmTcMessage msg(storeId); @@ -57,4 +61,38 @@ TEST_CASE("CFDP Distributor", "[cfdp][distributor]") { CHECK(queue.getNextSentMessage(receiverQueueId, sentMsg) == result::OK); CHECK(sentMsg.getStorageId() == storeId); } + + SECTION("No Destination found") { + CHECK(distributor.initialize() == result::OK); + size_t serLen = 0; + store_address_t storeId; + CHECK(pool.LocalPool::getFreeElement(&storeId, creator.getSerializedSize(), &dataPtr) == + result::OK); + REQUIRE(creator.SerializeIF::serializeBe(dataPtr, serLen, creator.getSerializedSize()) == + result::OK); + TmTcMessage msg(storeId); + queue.addReceivedMessage(msg); + CHECK(distributor.performOperation(0) == tmtcdistrib::NO_DESTINATION_FOUND); + } + + SECTION("Getting data fails") { + pool.nextModifyDataCallFails.first = true; + pool.nextModifyDataCallFails.second = StorageManagerIF::DATA_DOES_NOT_EXIST; + size_t serLen = 0; + store_address_t storeId; + CHECK(distributor.registerTcDestination(obswEntityId, tcAcceptor) == result::OK); + CHECK(pool.LocalPool::getFreeElement(&storeId, creator.getSerializedSize(), &dataPtr) == + result::OK); + REQUIRE(creator.SerializeIF::serializeBe(dataPtr, serLen, creator.getSerializedSize()) == + result::OK); + TmTcMessage msg(storeId); + queue.addReceivedMessage(msg); + CHECK(distributor.performOperation(0) == StorageManagerIF::DATA_DOES_NOT_EXIST); + } + + SECTION("Duplicate registration") { + CHECK(distributor.initialize() == result::OK); + CHECK(distributor.registerTcDestination(obswEntityId, tcAcceptor) == result::OK); + CHECK(distributor.registerTcDestination(obswEntityId, tcAcceptor) == result::FAILED); + } } \ No newline at end of file diff --git a/unittests/mocks/StorageManagerMock.cpp b/unittests/mocks/StorageManagerMock.cpp index 13f513e6..387b5b17 100644 --- a/unittests/mocks/StorageManagerMock.cpp +++ b/unittests/mocks/StorageManagerMock.cpp @@ -8,7 +8,7 @@ ReturnValue_t StorageManagerMock::addData(store_address_t *storageId, const uint return LocalPool::addData(storageId, data, size, ignoreFault); } ReturnValue_t StorageManagerMock::deleteData(store_address_t packet_id) { - if(nextDeleteDataCallFails.first) { + if (nextDeleteDataCallFails.first) { return nextDeleteDataCallFails.second; } return LocalPool::deleteData(packet_id); @@ -16,7 +16,7 @@ ReturnValue_t StorageManagerMock::deleteData(store_address_t packet_id) { ReturnValue_t StorageManagerMock::deleteData(uint8_t *buffer, size_t size, store_address_t *storeId) { - if(nextDeleteDataCallFails.first) { + if (nextDeleteDataCallFails.first) { return nextDeleteDataCallFails.second; } return LocalPool::deleteData(buffer, size, storeId); @@ -24,9 +24,6 @@ ReturnValue_t StorageManagerMock::deleteData(uint8_t *buffer, size_t size, ReturnValue_t StorageManagerMock::getData(store_address_t packet_id, const uint8_t **packet_ptr, size_t *size) { - if (nextGetDataCallFails.first) { - return nextGetDataCallFails.second; - } return LocalPool::getData(packet_id, packet_ptr, size); } @@ -49,9 +46,7 @@ ReturnValue_t StorageManagerMock::getFreeElement(store_address_t *storageId, siz bool StorageManagerMock::hasDataAtId(store_address_t storeId) const { return LocalPool::hasDataAtId(storeId); } -void StorageManagerMock::clearStore() { - return LocalPool::clearStore(); -} +void StorageManagerMock::clearStore() { return LocalPool::clearStore(); } void StorageManagerMock::clearSubPool(uint8_t poolIndex) { return LocalPool::clearSubPool(poolIndex); @@ -75,8 +70,6 @@ void StorageManagerMock::reset() { nextAddDataCallFails.second = result::OK; nextModifyDataCallFails.first = false; nextModifyDataCallFails.second = result::OK; - nextGetDataCallFails.first = false; - nextGetDataCallFails.second = result::OK; nextDeleteDataCallFails.first = false; nextDeleteDataCallFails.second = result::OK; nextFreeElementCallFails.first = false; @@ -85,5 +78,4 @@ void StorageManagerMock::reset() { StorageManagerMock::StorageManagerMock(object_id_t setObjectId, const LocalPool::LocalPoolConfig &poolConfig) - : LocalPool(setObjectId, poolConfig) { -} + : LocalPool(setObjectId, poolConfig) {} diff --git a/unittests/mocks/StorageManagerMock.h b/unittests/mocks/StorageManagerMock.h index be2b7982..f33ba19b 100644 --- a/unittests/mocks/StorageManagerMock.h +++ b/unittests/mocks/StorageManagerMock.h @@ -4,9 +4,9 @@ #include "fsfw/storagemanager/LocalPool.h" #include "fsfw/storagemanager/StorageManagerIF.h" -class StorageManagerMock: public LocalPool { +class StorageManagerMock : public LocalPool { public: - StorageManagerMock(object_id_t setObjectId, const LocalPoolConfig& poolConfig); + StorageManagerMock(object_id_t setObjectId, const LocalPoolConfig &poolConfig); ReturnValue_t addData(store_address_t *storageId, const uint8_t *data, size_t size, bool ignoreFault) override; @@ -25,8 +25,11 @@ class StorageManagerMock: public LocalPool { [[nodiscard]] max_subpools_t getNumberOfSubPools() const override; std::pair nextAddDataCallFails; + /** + * This can be used to make both the modify and get API call fail. This is because generally, + * the pool implementation get functions will use the modify API internally. + */ std::pair nextModifyDataCallFails; - std::pair nextGetDataCallFails; std::pair nextDeleteDataCallFails; std::pair nextFreeElementCallFails; void reset();