From 3bacc8ec53b1370ce6829af7e3c69ae7a0b21b5d Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Thu, 11 Mar 2021 12:04:54 +0100 Subject: [PATCH] more tests and bugfixes --- datapoollocal/LocalDataPoolManager.cpp | 20 ++++++-- datapoollocal/LocalPoolDataSetBase.cpp | 8 ++- datapoollocal/LocalPoolDataSetBase.h | 5 +- .../internal/LocalPoolDataSetAttorney.h | 5 +- housekeeping/PeriodicHousekeepingHelper.cpp | 11 ++-- housekeeping/PeriodicHousekeepingHelper.h | 7 ++- .../datapoollocal/LocalPoolManagerTest.cpp | 51 ++++++++++++++++++- .../datapoollocal/LocalPoolOwnerBase.cpp | 5 ++ .../tests/datapoollocal/LocalPoolOwnerBase.h | 15 ++---- unittest/tests/mocks/MessageQueueMockBase.h | 11 ++++ 10 files changed, 103 insertions(+), 35 deletions(-) diff --git a/datapoollocal/LocalDataPoolManager.cpp b/datapoollocal/LocalDataPoolManager.cpp index 83e30e824..cad54094e 100644 --- a/datapoollocal/LocalDataPoolManager.cpp +++ b/datapoollocal/LocalDataPoolManager.cpp @@ -38,7 +38,11 @@ LocalDataPoolManager::LocalDataPoolManager(HasLocalDataPoolIF* owner, MessageQue hkQueue = queueToUse; } -LocalDataPoolManager::~LocalDataPoolManager() {} +LocalDataPoolManager::~LocalDataPoolManager() { + if(mutex != nullptr) { + MutexFactory::instance()->deleteMutex(mutex); + } +} ReturnValue_t LocalDataPoolManager::initialize(MessageQueueIF* queueToUse) { if(queueToUse == nullptr) { @@ -375,7 +379,7 @@ ReturnValue_t LocalDataPoolManager::subscribeForPeriodicPacket(sid_t sid, LocalPoolDataSetAttorney::setReportingEnabled(*dataSet, enableReporting); LocalPoolDataSetAttorney::setDiagnostic(*dataSet, isDiagnostics); LocalPoolDataSetAttorney::initializePeriodicHelper(*dataSet, collectionInterval, - owner->getPeriodicOperationFrequency(), isDiagnostics); + owner->getPeriodicOperationFrequency()); } hkReceivers.push_back(hkReceiver); @@ -518,11 +522,19 @@ ReturnValue_t LocalDataPoolManager::handleHousekeepingMessage( } case(HousekeepingMessage::REPORT_DIAGNOSTICS_REPORT_STRUCTURES): { - return generateSetStructurePacket(sid, true); + result = generateSetStructurePacket(sid, true); + if(result == HasReturnvaluesIF::RETURN_OK) { + return result; + } + break; } case(HousekeepingMessage::REPORT_HK_REPORT_STRUCTURES): { - return generateSetStructurePacket(sid, false); + result = generateSetStructurePacket(sid, false); + if(result == HasReturnvaluesIF::RETURN_OK) { + return result; + } + break; } case(HousekeepingMessage::MODIFY_DIAGNOSTICS_REPORT_COLLECTION_INTERVAL): case(HousekeepingMessage::MODIFY_PARAMETER_REPORT_COLLECTION_INTERVAL): { diff --git a/datapoollocal/LocalPoolDataSetBase.cpp b/datapoollocal/LocalPoolDataSetBase.cpp index d93b926d2..2a2444c48 100644 --- a/datapoollocal/LocalPoolDataSetBase.cpp +++ b/datapoollocal/LocalPoolDataSetBase.cpp @@ -264,11 +264,9 @@ bool LocalPoolDataSetBase::getReportingEnabled() const { return reportingEnabled; } -void LocalPoolDataSetBase::initializePeriodicHelper( - float collectionInterval, dur_millis_t minimumPeriodicInterval, - bool isDiagnostics, uint8_t nonDiagIntervalFactor) { - periodicHelper->initialize(collectionInterval, minimumPeriodicInterval, - isDiagnostics, nonDiagIntervalFactor); +void LocalPoolDataSetBase::initializePeriodicHelper(float collectionInterval, + dur_millis_t minimumPeriodicInterval, uint8_t nonDiagIntervalFactor) { + periodicHelper->initialize(collectionInterval, minimumPeriodicInterval, nonDiagIntervalFactor); } void LocalPoolDataSetBase::setChanged(bool changed) { diff --git a/datapoollocal/LocalPoolDataSetBase.h b/datapoollocal/LocalPoolDataSetBase.h index 39ea9f1ac..ab67dc3f9 100644 --- a/datapoollocal/LocalPoolDataSetBase.h +++ b/datapoollocal/LocalPoolDataSetBase.h @@ -191,9 +191,8 @@ protected: bool reportingEnabled = false; void setReportingEnabled(bool enabled); - void initializePeriodicHelper(float collectionInterval, - dur_millis_t minimumPeriodicInterval, - bool isDiagnostics, uint8_t nonDiagIntervalFactor = 5); + void initializePeriodicHelper(float collectionInterval, dur_millis_t minimumPeriodicInterval, + uint8_t nonDiagIntervalFactor = 5); /** * If the valid state of a dataset is always relevant to the whole diff --git a/datapoollocal/internal/LocalPoolDataSetAttorney.h b/datapoollocal/internal/LocalPoolDataSetAttorney.h index 7a34e9c8e..f81428cd1 100644 --- a/datapoollocal/internal/LocalPoolDataSetAttorney.h +++ b/datapoollocal/internal/LocalPoolDataSetAttorney.h @@ -14,9 +14,8 @@ private: } static void initializePeriodicHelper(LocalPoolDataSetBase& set, float collectionInterval, - uint32_t minimumPeriodicIntervalMs, - bool isDiagnostics, uint8_t nonDiagIntervalFactor = 5) { - set.initializePeriodicHelper(collectionInterval, minimumPeriodicIntervalMs, isDiagnostics, + uint32_t minimumPeriodicIntervalMs, uint8_t nonDiagIntervalFactor = 5) { + set.initializePeriodicHelper(collectionInterval, minimumPeriodicIntervalMs, nonDiagIntervalFactor); } diff --git a/housekeeping/PeriodicHousekeepingHelper.cpp b/housekeeping/PeriodicHousekeepingHelper.cpp index 5152706fa..892eb354a 100644 --- a/housekeeping/PeriodicHousekeepingHelper.cpp +++ b/housekeeping/PeriodicHousekeepingHelper.cpp @@ -6,11 +6,8 @@ PeriodicHousekeepingHelper::PeriodicHousekeepingHelper( LocalPoolDataSetBase* owner): owner(owner) {} - void PeriodicHousekeepingHelper::initialize(float collectionInterval, - dur_millis_t minimumPeriodicInterval, bool isDiagnostics, - uint8_t nonDiagIntervalFactor) { - this->isDiagnostics = isDiagnostics; + dur_millis_t minimumPeriodicInterval, uint8_t nonDiagIntervalFactor) { this->minimumPeriodicInterval = minimumPeriodicInterval; this->nonDiagIntervalFactor = nonDiagIntervalFactor; collectionIntervalTicks = intervalSecondsToIntervalTicks(collectionInterval); @@ -34,6 +31,11 @@ bool PeriodicHousekeepingHelper::checkOpNecessary() { uint32_t PeriodicHousekeepingHelper::intervalSecondsToIntervalTicks( float collectionIntervalSeconds) { + if(owner == nullptr) { + return 0; + } + bool isDiagnostics = owner->isDiagnostics(); + /* Avoid division by zero */ if(minimumPeriodicInterval == 0) { if(isDiagnostics) { @@ -85,3 +87,4 @@ void PeriodicHousekeepingHelper::changeCollectionInterval( float newIntervalSeconds) { collectionIntervalTicks = intervalSecondsToIntervalTicks(newIntervalSeconds); } + diff --git a/housekeeping/PeriodicHousekeepingHelper.h b/housekeeping/PeriodicHousekeepingHelper.h index 3e8d030e6..3256fbffa 100644 --- a/housekeeping/PeriodicHousekeepingHelper.h +++ b/housekeeping/PeriodicHousekeepingHelper.h @@ -10,16 +10,15 @@ class PeriodicHousekeepingHelper { public: PeriodicHousekeepingHelper(LocalPoolDataSetBase* owner); - void initialize(float collectionInterval, - dur_millis_t minimumPeriodicInterval, bool isDiagnostics, - uint8_t nonDiagIntervalFactor); + void initialize(float collectionInterval, dur_millis_t minimumPeriodicInterval, + uint8_t nonDiagIntervalFactor); void changeCollectionInterval(float newInterval); float getCollectionIntervalInSeconds() const; bool checkOpNecessary(); + private: LocalPoolDataSetBase* owner = nullptr; - bool isDiagnostics = true; uint8_t nonDiagIntervalFactor = 0; uint32_t intervalSecondsToIntervalTicks(float collectionIntervalSeconds); diff --git a/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp b/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp index 2d7294efa..c152078d9 100644 --- a/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp +++ b/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp @@ -229,6 +229,7 @@ TEST_CASE("LocalPoolManagerTest" , "[LocManTest]") { /* Now HK packet should be sent as message immediately. */ REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); CHECK(messagesSent == 1); + CHECK(mqMock->popMessage() == retval::CATCH_OK); LocalPoolDataSetBase* setHandle = poolOwner->getDataSetHandle(lpool::testSid); REQUIRE(setHandle != nullptr); @@ -236,6 +237,7 @@ TEST_CASE("LocalPoolManagerTest" , "[LocManTest]") { setHandle, false) == retval::CATCH_OK); REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); CHECK(messagesSent == 1); + CHECK(mqMock->popMessage() == retval::CATCH_OK); CHECK(setHandle->getReportingEnabled() == true); CommandMessage hkCmd; @@ -244,17 +246,19 @@ TEST_CASE("LocalPoolManagerTest" , "[LocManTest]") { CHECK(setHandle->getReportingEnabled() == false); REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); CHECK(messagesSent == 1); + CHECK(mqMock->popMessage() == retval::CATCH_OK); + HousekeepingMessage::setToggleReportingCommand(&hkCmd, lpool::testSid, true, false); CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == retval::CATCH_OK); CHECK(setHandle->getReportingEnabled() == true); REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); - CHECK(messagesSent == 1); + CHECK(mqMock->popMessage() == retval::CATCH_OK); HousekeepingMessage::setToggleReportingCommand(&hkCmd, lpool::testSid, false, false); CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == retval::CATCH_OK); CHECK(setHandle->getReportingEnabled() == false); REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); - CHECK(messagesSent == 1); + CHECK(mqMock->popMessage() == retval::CATCH_OK); HousekeepingMessage::setCollectionIntervalModificationCommand(&hkCmd, lpool::testSid, 0.4, false); @@ -264,6 +268,7 @@ TEST_CASE("LocalPoolManagerTest" , "[LocManTest]") { CHECK(poolOwner->dataset.getCollectionInterval() == 1.0); REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); CHECK(messagesSent == 1); + CHECK(mqMock->popMessage() == retval::CATCH_OK); HousekeepingMessage::setStructureReportingCommand(&hkCmd, lpool::testSid, false); REQUIRE(poolOwner->poolManager.performHkOperation() == retval::CATCH_OK); @@ -271,11 +276,13 @@ TEST_CASE("LocalPoolManagerTest" , "[LocManTest]") { /* Now HK packet should be sent as message. */ REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); CHECK(messagesSent == 1); + CHECK(mqMock->popMessage() == retval::CATCH_OK); HousekeepingMessage::setOneShotReportCommand(&hkCmd, lpool::testSid, false); CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == retval::CATCH_OK); REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); CHECK(messagesSent == 1); + CHECK(mqMock->popMessage() == retval::CATCH_OK); HousekeepingMessage::setUpdateNotificationSetCommand(&hkCmd, lpool::testSid); sid_t sidToCheck; @@ -283,6 +290,46 @@ TEST_CASE("LocalPoolManagerTest" , "[LocManTest]") { CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == retval::CATCH_OK); CHECK(poolOwner->changedDataSetCallbackWasCalled(sidToCheck, storeId) == true); CHECK(sidToCheck == lpool::testSid); + + /* Now we test the handling is the dataset is set to diagnostic */ + poolOwner->dataset.setDiagnostic(true); + + HousekeepingMessage::setStructureReportingCommand(&hkCmd, lpool::testSid, false); + CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == + static_cast(LocalDataPoolManager::WRONG_HK_PACKET_TYPE)); + /* We still expect a failure message being sent */ + REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); + CHECK(messagesSent == 1); + CHECK(mqMock->popMessage() == retval::CATCH_OK); + + HousekeepingMessage::setCollectionIntervalModificationCommand(&hkCmd, + lpool::testSid, 0.4, false); + CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == + static_cast(LocalDataPoolManager::WRONG_HK_PACKET_TYPE)); + REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); + CHECK(messagesSent == 1); + CHECK(mqMock->popMessage() == retval::CATCH_OK); + + HousekeepingMessage::setStructureReportingCommand(&hkCmd, lpool::testSid, false); + CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == + static_cast(LocalDataPoolManager::WRONG_HK_PACKET_TYPE)); + REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); + CHECK(messagesSent == 1); + CHECK(mqMock->popMessage() == retval::CATCH_OK); + + HousekeepingMessage::setStructureReportingCommand(&hkCmd, lpool::testSid, true); + CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == retval::CATCH_OK); + REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); + CHECK(messagesSent == 1); + CHECK(mqMock->popMessage() == retval::CATCH_OK); + + HousekeepingMessage::setCollectionIntervalModificationCommand(&hkCmd, lpool::testSid, 0.4, + true); + CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == retval::CATCH_OK); + REQUIRE(mqMock->wasMessageSent(&messagesSent) == true); + CHECK(messagesSent == 1); + CHECK(mqMock->popMessage() == retval::CATCH_OK); + } /* we need to reset the subscription list because the pool owner diff --git a/unittest/tests/datapoollocal/LocalPoolOwnerBase.cpp b/unittest/tests/datapoollocal/LocalPoolOwnerBase.cpp index 0e2703a77..7ccceb063 100644 --- a/unittest/tests/datapoollocal/LocalPoolOwnerBase.cpp +++ b/unittest/tests/datapoollocal/LocalPoolOwnerBase.cpp @@ -6,6 +6,10 @@ LocalPoolOwnerBase::LocalPoolOwnerBase(object_id_t objectId): messageQueue = new MessageQueueMockBase(); } +LocalPoolOwnerBase::~LocalPoolOwnerBase() { + QueueFactory::instance()->deleteMessageQueue(messageQueue); +} + ReturnValue_t LocalPoolOwnerBase::initializeHkManager() { if(not initialized) { initialized = true; @@ -132,3 +136,4 @@ void LocalPoolOwnerBase::handleChangedPoolVariable(gp_id_t globPoolId, store_add this->changedPoolVariableGpid = globPoolId; this->storeIdForChangedVariable = storeId; } + diff --git a/unittest/tests/datapoollocal/LocalPoolOwnerBase.h b/unittest/tests/datapoollocal/LocalPoolOwnerBase.h index f08b9eb94..c045b6d39 100644 --- a/unittest/tests/datapoollocal/LocalPoolOwnerBase.h +++ b/unittest/tests/datapoollocal/LocalPoolOwnerBase.h @@ -54,13 +54,7 @@ private: class LocalPoolTestDataSet: public LocalDataSet { public: LocalPoolTestDataSet(): - LocalDataSet(lpool::testSid, lpool::dataSetMaxVariables), - localPoolVarUint8(lpool::uint8VarGpid, this), - localPoolVarFloat(lpool::floatVarGpid, this), - localPoolUint16Vec(lpool::uint16Vec3Gpid, this) - - { - } + LocalDataSet(lpool::testSid, lpool::dataSetMaxVariables) {} LocalPoolTestDataSet(HasLocalDataPoolIF* owner, uint32_t setId): LocalDataSet(owner, setId, lpool::dataSetMaxVariables) { @@ -70,6 +64,9 @@ public: lp_var_t localPoolVarFloat = lp_var_t(lpool::floatVarGpid, this); lp_vec_t localPoolUint16Vec = lp_vec_t(lpool::uint16Vec3Gpid, this); + void setDiagnostic(bool isDiagnostic) { + LocalPoolDataSetBase::setDiagnostic(isDiagnostic); + } private: }; @@ -78,9 +75,7 @@ class LocalPoolOwnerBase: public SystemObject, public HasLocalDataPoolIF { public: LocalPoolOwnerBase(object_id_t objectId = objects::TEST_LOCAL_POOL_OWNER_BASE); - ~LocalPoolOwnerBase() { - QueueFactory::instance()->deleteMessageQueue(messageQueue); - } + ~LocalPoolOwnerBase(); object_id_t getObjectId() const override { return SystemObject::getObjectId(); diff --git a/unittest/tests/mocks/MessageQueueMockBase.h b/unittest/tests/mocks/MessageQueueMockBase.h index 31146d34a..3000f7fb5 100644 --- a/unittest/tests/mocks/MessageQueueMockBase.h +++ b/unittest/tests/mocks/MessageQueueMockBase.h @@ -29,6 +29,16 @@ public: return tempMessageSent; } + /** + * Pop a message, clearing it in the process. + * @return + */ + ReturnValue_t popMessage() { + CommandMessage message; + message.clear(); + return receiveMessage(&message); + } + virtual ReturnValue_t reply( MessageQueueMessageIF* message ) { return sendMessage(myQueueId, message); }; @@ -36,6 +46,7 @@ public: MessageQueueId_t *receivedFrom) { return receiveMessage(message); } + virtual ReturnValue_t receiveMessage(MessageQueueMessageIF* message) { if(messagesSentQueue.empty()) { return MessageQueueIF::EMPTY;