From 639b210666b6e38201a82f2b19689989bb67728e Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 7 Nov 2024 13:18:47 +0100 Subject: [PATCH] continue fixing tests --- CMakeLists.txt | 3 +- src/fsfw/datapool/SharedSetBase.cpp | 16 +++-------- src/fsfw/datapool/SharedSetBase.h | 7 ++--- src/fsfw/datapool/definitions.h | 7 +++-- src/fsfw/housekeeping/PeriodicHkHelper.cpp | 28 +++++++++---------- .../internalerror/InternalErrorReporter.cpp | 4 +-- src/fsfw/serialize/SerializableList.h | 6 ++++ src/fsfw/serialize/SerializeAdapter.h | 4 +-- unittests/datapoollocal/testDataSet.cpp | 17 ++++++----- .../datapoollocal/testLocalPoolManager.cpp | 8 +++--- .../datapoollocal/testLocalPoolVector.cpp | 3 +- unittests/mock/TestPoolOwner.cpp | 17 ++++++++++- unittests/mock/TestPoolOwner.h | 9 +++--- unittests/mock/poolDefinitions.h | 28 +++++++++++++------ 14 files changed, 90 insertions(+), 67 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 4c7332e7..2255826a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -123,8 +123,7 @@ option(FSFW_ADD_SGP4_PROPAGATOR "Add SGP4 propagator code" OFF) set(FSFW_TEST_TGT fsfw-tests) set(FSFW_DUMMY_TGT fsfw-dummy) -add_library(${LIB_FSFW_NAME} src/fsfw/housekeeping/PeriodicHkHelperIF.h - src/fsfw/housekeeping/definitions.h) +add_library(${LIB_FSFW_NAME}) if(IPO_SUPPORTED AND FSFW_ENABLE_IPO) set_property(TARGET ${LIB_FSFW_NAME} PROPERTY INTERPROCEDURAL_OPTIMIZATION diff --git a/src/fsfw/datapool/SharedSetBase.cpp b/src/fsfw/datapool/SharedSetBase.cpp index 0dad8b89..36482ec3 100644 --- a/src/fsfw/datapool/SharedSetBase.cpp +++ b/src/fsfw/datapool/SharedSetBase.cpp @@ -68,13 +68,9 @@ ReturnValue_t SharedSetBase::unlockDataPool() { } ReturnValue_t SharedSetBase::serializeLocalPoolIds(uint8_t **buffer, size_t *size, size_t maxSize, - SerializeIF::Endianness streamEndianness, - bool serializeFillCount) const { + SerializeIF::Endianness streamEndianness) const { /* Serialize fill count as uint8_t */ auto fillCount = this->getFillCount(); - if (serializeFillCount) { - SerializeAdapter::serialize(&fillCount, buffer, size, maxSize, streamEndianness); - } for (uint16_t count = 0; count < fillCount; count++) { id_t currentPoolId = base.registeredVariables[count]->getDataPoolId(); auto result = @@ -94,12 +90,8 @@ ReturnValue_t SharedSetBase::serializeLocalPoolIds(uint8_t **buffer, size_t *siz return returnvalue::OK; } -size_t SharedSetBase::getLocalPoolIdsSerializedSize(bool serializeFillCount) const { - if (serializeFillCount) { - return base.getFillCount() * sizeof(id_t) + sizeof(u8_t); - } else { - return base.getFillCount() * sizeof(id_t); - } +size_t SharedSetBase::getLocalPoolIdsSerializedSize() const { + return base.getFillCount() * sizeof(id_t); } size_t SharedSetBase::getSerializedSize() const { return base.getSerializedSize(); } @@ -126,7 +118,7 @@ void SharedSetBase::setReportingEnabled(bool reportingEnabled) { bool SharedSetBase::getReportingEnabled() const { return reportingEnabled; } -structure_id_t SharedSetBase::getSid() const { return sid; } +structure_id_t SharedSetBase::getStructureId() const { return sid; } object_id_t SharedSetBase::getCreatorObjectId() { if (sharedPool != nullptr) { diff --git a/src/fsfw/datapool/SharedSetBase.h b/src/fsfw/datapool/SharedSetBase.h index c9ef5af9..34ebd0f1 100644 --- a/src/fsfw/datapool/SharedSetBase.h +++ b/src/fsfw/datapool/SharedSetBase.h @@ -118,14 +118,13 @@ class SharedSetBase : public SerializeIF, public PoolDataSetIF { ReturnValue_t deSerialize(const uint8_t** buffer, size_t* size, Endianness streamEndianness) override; - [[nodiscard]] dp::sid_t getSid() const; + [[nodiscard]] dp::sid_t getStructureId() const; [[nodiscard]] size_t getSerializedSize() const override; ReturnValue_t serializeLocalPoolIds(uint8_t** buffer, size_t* size, size_t maxSize, - SerializeIF::Endianness streamEndianness, - bool serializeFillCount = true) const; - [[nodiscard]] size_t getLocalPoolIdsSerializedSize(bool serializeFillCount = true) const; + SerializeIF::Endianness streamEndianness) const; + [[nodiscard]] size_t getLocalPoolIdsSerializedSize() const; object_id_t getCreatorObjectId(); diff --git a/src/fsfw/datapool/definitions.h b/src/fsfw/datapool/definitions.h index 9d3ce082..cee79e80 100644 --- a/src/fsfw/datapool/definitions.h +++ b/src/fsfw/datapool/definitions.h @@ -29,13 +29,16 @@ using DataPool = std::map; * Used as a unique identifier for data sets. Consists of 4 byte object ID and 4 byte set ID. */ union structure_id_t { + static constexpr size_t SIZE = sizeof(object_id_t) + sizeof(uint32_t); + static constexpr uint64_t INVALID_SID = -1; static constexpr uint32_t INVALID_OBJECT_ID = objects::NO_OBJECT; static constexpr uint32_t INVALID_SET_ID = -1; structure_id_t() : raw(INVALID_SID) {} - structure_id_t(object_id_t objectId, uint32_t setId) : objectId(objectId), ownerSetId(setId) {} + constexpr structure_id_t(object_id_t objectId, uint32_t setId) + : objectId(objectId), ownerSetId(setId) {} struct { object_id_t objectId; @@ -51,7 +54,7 @@ union structure_id_t { */ uint64_t raw; - bool notSet() const { return raw == INVALID_SID; } + [[nodiscard]] bool notSet() const { return raw == INVALID_SID; } bool operator==(const structure_id_t& other) const { return raw == other.raw; } diff --git a/src/fsfw/housekeeping/PeriodicHkHelper.cpp b/src/fsfw/housekeeping/PeriodicHkHelper.cpp index 65a1c9ad..7298ab8a 100644 --- a/src/fsfw/housekeeping/PeriodicHkHelper.cpp +++ b/src/fsfw/housekeeping/PeriodicHkHelper.cpp @@ -67,6 +67,9 @@ ReturnValue_t PeriodicHelper::performHkOperation() { // Periodic packets shall only be generated from datasets continue; } + if (not setSpec.periodicCollectionEnabled) { + continue; + } performPeriodicHkGeneration(setSpec, now); break; } @@ -141,20 +144,24 @@ ReturnValue_t PeriodicHelper::generateHousekeepingPacket(const dp::sid_t sid, } auto& setSpec = optSetSpec.value().get(); uint8_t* dataPtr = nullptr; - const size_t maxSize = setSpec.size; + const size_t maxSize = setSpec.size + dp::structure_id_t::SIZE; ReturnValue_t result = ipcStore->getFreeElement(&storeId, maxSize, &dataPtr); if (result != returnvalue::OK) { return result; } - result = owner->serializeHkDataset(sid, dataPtr, maxSize); + size_t serSize = 0; + result = SerializeAdapter::serialize(&sid.objectId, &dataPtr, &serSize, maxSize, + SerializeIF::Endianness::NETWORK); if (result != returnvalue::OK) { return result; } - HousekeepingPacketDownlink hkPacket(sid, dataPtr, maxSize); - - size_t serializedSize = 0; - result = hkPacket.serialize(&dataPtr, &serializedSize, maxSize, SerializeIF::Endianness::NETWORK); - if (result != returnvalue::OK or serializedSize == 0) { + result = SerializeAdapter::serialize(&sid.ownerSetId, &dataPtr, &serSize, maxSize, + SerializeIF::Endianness::NETWORK); + if (result != returnvalue::OK) { + return result; + } + result = owner->serializeHkDataset(sid, dataPtr, maxSize - 8); + if (result != returnvalue::OK) { return result; } @@ -182,13 +189,6 @@ ReturnValue_t PeriodicHelper::generateHousekeepingPacket(const dp::sid_t sid, } void PeriodicHelper::performPeriodicHkGeneration(SetSpecification& setSpec, timeval& now) { - if (not setSpec.periodicCollectionEnabled) { - return; - } - - // timeval now{}; - Clock::getClockMonotonic(&now); - dp::sid_t sid = setSpec.dataId.sid; timeval diff = now - setSpec.lastGenerated; diff --git a/src/fsfw/internalerror/InternalErrorReporter.cpp b/src/fsfw/internalerror/InternalErrorReporter.cpp index 98916148..f27ecb63 100644 --- a/src/fsfw/internalerror/InternalErrorReporter.cpp +++ b/src/fsfw/internalerror/InternalErrorReporter.cpp @@ -154,7 +154,7 @@ void InternalErrorReporter::setMutexTimeout(MutexIF::TimeoutType timeoutType, ui ReturnValue_t InternalErrorReporter::serializeHkDataset(dp::structure_id_t structureId, uint8_t *buf, size_t maxSize) { - if (structureId == internalErrorDataset.getSid()) { + if (structureId == internalErrorDataset.getStructureId()) { size_t serSize = 0; return internalErrorDataset.serialize(buf, serSize, maxSize, SerializeIF::Endianness::NETWORK); } @@ -163,7 +163,7 @@ ReturnValue_t InternalErrorReporter::serializeHkDataset(dp::structure_id_t struc ReturnValue_t InternalErrorReporter::specifyHkDatasets( std::vector &setSpecification) { - setSpecification.emplace_back(internalErrorDataset.getSid(), + setSpecification.emplace_back(internalErrorDataset.getStructureId(), internalErrorDataset.getSerializedSize(), generationFrequency); return returnvalue::OK; } diff --git a/src/fsfw/serialize/SerializableList.h b/src/fsfw/serialize/SerializableList.h index 04410814..3e78dcfc 100644 --- a/src/fsfw/serialize/SerializableList.h +++ b/src/fsfw/serialize/SerializableList.h @@ -22,6 +22,12 @@ class List : public SerializeIF { } return result; } + + [[nodiscard]] ReturnValue_t serialize(uint8_t *buffer, size_t &serSize, size_t maxSize, + Endianness streamEndianness) const override { + return SerializeIF::serialize(buffer, serSize, maxSize, streamEndianness); + } + [[nodiscard]] size_t getSerializedSize() const override { size_t size = 0; for (auto &serializable : serializables) { diff --git a/src/fsfw/serialize/SerializeAdapter.h b/src/fsfw/serialize/SerializeAdapter.h index 3c478271..d154edd8 100644 --- a/src/fsfw/serialize/SerializeAdapter.h +++ b/src/fsfw/serialize/SerializeAdapter.h @@ -68,8 +68,8 @@ class SerializeAdapter { * - @c returnvalue::OK Successful serialization */ template - static ReturnValue_t serialize(const T *object, uint8_t *const buffer, size_t *serSize, - size_t maxSize, SerializeIF::Endianness streamEndianness) { + static ReturnValue_t serialize(const T *object, uint8_t *buffer, size_t *serSize, size_t maxSize, + SerializeIF::Endianness streamEndianness) { if (object == nullptr or buffer == nullptr) { return returnvalue::FAILED; } diff --git a/unittests/datapoollocal/testDataSet.cpp b/unittests/datapoollocal/testDataSet.cpp index 34a10637..93e761b3 100644 --- a/unittests/datapoollocal/testDataSet.cpp +++ b/unittests/datapoollocal/testDataSet.cpp @@ -18,19 +18,18 @@ TEST_CASE("DataSetTest", "[DataSetTest]") { auto queue = MessageQueueMock(1); TestPoolOwner poolOwner(queue, objects::TEST_LOCAL_POOL_OWNER_BASE); poolOwner.initialize(); - StaticTestDataSet localSet; + StaticTestDataset localSet; SECTION("BasicTest") { /* Test some basic functions */ CHECK(localSet.getReportingEnabled() == false); - CHECK(localSet.getLocalPoolIdsSerializedSize(false) == 3 * sizeof(dp::id_t)); - CHECK(localSet.getLocalPoolIdsSerializedSize(true) == 3 * sizeof(dp::id_t) + sizeof(uint8_t)); - CHECK(localSet.getSid() == lpool::testSid0); + CHECK(localSet.getLocalPoolIdsSerializedSize() == 3 * sizeof(dp::id_t)); + CHECK(localSet.getStructureId() == lpool::testSid1); CHECK(localSet.getCreatorObjectId() == objects::TEST_LOCAL_POOL_OWNER_BASE); - size_t maxSize = localSet.getLocalPoolIdsSerializedSize(true); + size_t maxSize = localSet.getLocalPoolIdsSerializedSize(); uint8_t localPoolIdBuff[maxSize]; // Skip size field - auto* lpIds = reinterpret_cast(localPoolIdBuff + 1); + auto* lpIds = reinterpret_cast(localPoolIdBuff); size_t serSize = 0; auto* localPoolIdBuffPtr = reinterpret_cast(localPoolIdBuff); @@ -38,7 +37,7 @@ TEST_CASE("DataSetTest", "[DataSetTest]") { CHECK(localSet.serializeLocalPoolIds(&localPoolIdBuffPtr, &serSize, maxSize, SerializeIF::Endianness::MACHINE) == returnvalue::OK); CHECK(serSize == maxSize); - CHECK(localPoolIdBuff[0] == 3); + // CHECK(localPoolIdBuff[0] == 3); CHECK(lpIds[0] == localSet.localPoolVarUint8.getDataPoolId()); CHECK(lpIds[1] == localSet.localPoolVarFloat.getDataPoolId()); CHECK(lpIds[2] == localSet.localPoolUint16Vec.getDataPoolId()); @@ -47,8 +46,8 @@ TEST_CASE("DataSetTest", "[DataSetTest]") { localPoolIdBuffPtr = localPoolIdBuff; serSize = 0; CHECK(localSet.serializeLocalPoolIds(&localPoolIdBuffPtr, &serSize, maxSize, - SerializeIF::Endianness::MACHINE, false) == OK); - CHECK(serSize == maxSize - sizeof(uint8_t)); + SerializeIF::Endianness::MACHINE) == OK); + CHECK(serSize == maxSize); CHECK(lpIds[0] == localSet.localPoolVarUint8.getDataPoolId()); CHECK(lpIds[1] == localSet.localPoolVarFloat.getDataPoolId()); CHECK(lpIds[2] == localSet.localPoolUint16Vec.getDataPoolId()); diff --git a/unittests/datapoollocal/testLocalPoolManager.cpp b/unittests/datapoollocal/testLocalPoolManager.cpp index e465bb17..89c69956 100644 --- a/unittests/datapoollocal/testLocalPoolManager.cpp +++ b/unittests/datapoollocal/testLocalPoolManager.cpp @@ -15,9 +15,9 @@ using namespace lpool; -TEST_CASE("Local Pool Manager Tests", "[LocManTest]") { - const MessageQueueId_t defaultDestId = 1; - const MessageQueueId_t hkDest = defaultDestId; +TEST_CASE("Pool Owner and Periodic HK", "[datapool]") { + constexpr MessageQueueId_t defaultDestId = 1; + constexpr MessageQueueId_t hkDest = defaultDestId; const MessageQueueId_t subscriberId = 2; auto hkReceiver = HkReceiverMock(hkDest); auto queue = MessageQueueMock(3); @@ -47,7 +47,7 @@ TEST_CASE("Local Pool Manager Tests", "[LocManTest]") { // the temporal behaviour correctly the HK manager should generate a HK packet // immediately and the periodic helper depends on HK op function calls anyway instead of // using the clock, so we could also just call performHkOperation multiple times - REQUIRE(poolOwner.subscribePeriodicHk(true) == returnvalue::OK); + REQUIRE(poolOwner.enablePeriodicHk(lpool::testSid0, 30) == returnvalue::OK); REQUIRE(poolOwner.hkHelper.performHkOperation() == returnvalue::OK); // Now HK packet should be sent as message immediately. REQUIRE(poolOwnerMock.wasMessageSent()); diff --git a/unittests/datapoollocal/testLocalPoolVector.cpp b/unittests/datapoollocal/testLocalPoolVector.cpp index 30bb51d5..47629b5a 100644 --- a/unittests/datapoollocal/testLocalPoolVector.cpp +++ b/unittests/datapoollocal/testLocalPoolVector.cpp @@ -17,8 +17,7 @@ TEST_CASE("LocalPoolVector", "[LocPoolVecTest]") { SECTION("BasicTest") { // very basic test. - vec_t testVector = - vec_t(objects::TEST_LOCAL_POOL_OWNER_BASE, lpool::uint16Vec3Id); + auto testVector = vec_t(objects::TEST_LOCAL_POOL_OWNER_BASE, lpool::uint16Vec3Id); REQUIRE(testVector.read() == returnvalue::OK); testVector.value[0] = 5; testVector.value[1] = 232; diff --git a/unittests/mock/TestPoolOwner.cpp b/unittests/mock/TestPoolOwner.cpp index cd10568e..804fad4d 100644 --- a/unittests/mock/TestPoolOwner.cpp +++ b/unittests/mock/TestPoolOwner.cpp @@ -31,10 +31,25 @@ SharedPool *TestPoolOwner::getOptionalSharedPool() { return &sharedPool; } ReturnValue_t TestPoolOwner::serializeHkDataset(structure_id_t structureId, uint8_t *buf, size_t maxSize) { - return returnvalue::OK; + size_t dummy = 0; + if (structureId == testSid0) { + return set0.serialize(buf, dummy, maxSize, SerializeIF::Endianness::NETWORK); + } + if (structureId == testSid1) { + return set1.serialize(buf, dummy, maxSize, SerializeIF::Endianness::NETWORK); + } + if (structureId == testSid2) { + return set2.serialize(buf, dummy, maxSize, SerializeIF::Endianness::NETWORK); + } + return returnvalue::FAILED; } ReturnValue_t TestPoolOwner::specifyHkDatasets(std::vector &setList) { + // For the first set, we explicitely associate a set with an ID ourselves. + setList.emplace_back(testSid0, set0.getSerializedSize(), 50); + // For the other sets, we can use getter functions of the same structure. + setList.emplace_back(set1.getStructureId(), set1.getSerializedSize(), 50); + setList.emplace_back(set2.getStructureId(), set2.getSerializedSize(), 50); return returnvalue::OK; } diff --git a/unittests/mock/TestPoolOwner.h b/unittests/mock/TestPoolOwner.h index b1a2b9f7..9d90d316 100644 --- a/unittests/mock/TestPoolOwner.h +++ b/unittests/mock/TestPoolOwner.h @@ -32,16 +32,17 @@ class TestPoolOwner : public SystemObject, public hk::GeneratesPeriodicHkIF { return dynamic_cast(queue); } - ReturnValue_t subscribePeriodicHk(bool enableReporting) { - return hkHelper.enablePeriodicPacket(lpool::testSid0, 50); + ReturnValue_t enablePeriodicHk(dp::structure_id_t structureId, dur_millis_t frequencyMs) { + return hkHelper.enablePeriodicPacket(structureId, frequencyMs); } ReturnValue_t reset(); hk::PeriodicHelper hkHelper; SharedPool sharedPool; - LocalPoolTestDataSet set1; - StaticTestDataSet set2; + Dataset set0; + StaticTestDataset set1; + TestDataset set2; private: PoolEntry u8PoolEntry = PoolEntry({0}); diff --git a/unittests/mock/poolDefinitions.h b/unittests/mock/poolDefinitions.h index 6d750235..81794eb3 100644 --- a/unittests/mock/poolDefinitions.h +++ b/unittests/mock/poolDefinitions.h @@ -3,6 +3,8 @@ #include #include #include +#include +#include #include "fsfw/datapool/PoolEntry.h" #include "mock/MessageQueueMock.h" @@ -11,6 +13,7 @@ namespace lpool { using namespace dp; +using namespace serialize; static constexpr id_t uint8VarId = 0; static constexpr id_t floatVarId = 1; @@ -23,9 +26,9 @@ static constexpr uint32_t testSetId1 = 1; static constexpr uint32_t testSetId2 = 2; static constexpr uint8_t dataSetMaxVariables = 10; -static const auto testSid0 = structure_id_t(objects::TEST_LOCAL_POOL_OWNER_BASE, testSetId0); -static const auto testSid1 = structure_id_t(objects::TEST_LOCAL_POOL_OWNER_BASE, testSetId1); -static const auto testSid2 = structure_id_t(objects::TEST_LOCAL_POOL_OWNER_BASE, testSetId2); +static constexpr auto testSid0 = structure_id_t(objects::TEST_LOCAL_POOL_OWNER_BASE, testSetId0); +static constexpr auto testSid1 = structure_id_t(objects::TEST_LOCAL_POOL_OWNER_BASE, testSetId1); +static constexpr auto testSid2 = structure_id_t(objects::TEST_LOCAL_POOL_OWNER_BASE, testSetId2); static const g_id_t uint8VarGpid = g_id_t(objects::TEST_LOCAL_POOL_OWNER_BASE, uint8VarId); static const g_id_t floatVarGpid = g_id_t(objects::TEST_LOCAL_POOL_OWNER_BASE, floatVarId); @@ -33,11 +36,18 @@ static const g_id_t uint32Gpid = g_id_t(objects::TEST_LOCAL_POOL_OWNER_BASE, uin static const g_id_t uint16Vec3Gpid = g_id_t(objects::TEST_LOCAL_POOL_OWNER_BASE, uint16Vec3Id); static const g_id_t uint64Vec2Id = g_id_t(objects::TEST_LOCAL_POOL_OWNER_BASE, int64Vec2Id); -class StaticTestDataSet : public StaticSharedSet<3> { +class Dataset : public List { public: - StaticTestDataSet() : StaticSharedSet(lpool::testSid1) {} + LE u8Element{*this}; + LE u16Element{*this}; + LAE floatVec{*this}; +}; - StaticTestDataSet(SharedPool& sharedPool, uint32_t setId) : StaticSharedSet(sharedPool, setId) {} +class StaticTestDataset : public StaticSharedSet<3> { + public: + StaticTestDataset() : StaticSharedSet(lpool::testSid1) {} + + StaticTestDataset(SharedPool& sharedPool, uint32_t setId) : StaticSharedSet(sharedPool, setId) {} u8_t localPoolVarUint8{lpool::uint8VarGpid, this}; f32_t localPoolVarFloat{lpool::floatVarGpid, this}; @@ -46,11 +56,11 @@ class StaticTestDataSet : public StaticSharedSet<3> { private: }; -class LocalPoolTestDataSet : public SharedSet { +class TestDataset : public SharedSet { public: - LocalPoolTestDataSet() : SharedSet(lpool::testSid2, lpool::dataSetMaxVariables) {} + TestDataset() : SharedSet(lpool::testSid2, lpool::dataSetMaxVariables) {} - LocalPoolTestDataSet(SharedPool& sharedPool, uint32_t setId) + TestDataset(SharedPool& sharedPool, uint32_t setId) : SharedSet(sharedPool, setId, lpool::dataSetMaxVariables) {} u8_t localPoolVarUint8{lpool::uint8VarGpid, this};