more tests and bugfixes

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): {

datapoollocal/LocalPoolDataSetBase.cpp

@@ -264,11 +264,9 @@ bool LocalPoolDataSetBase::getReportingEnabled() const {
     return reportingEnabled;
 }
 
-void LocalPoolDataSetBase::initializePeriodicHelper(
+void LocalPoolDataSetBase::initializePeriodicHelper(float collectionInterval,
-        float collectionInterval, dur_millis_t minimumPeriodicInterval,
+        dur_millis_t minimumPeriodicInterval, uint8_t nonDiagIntervalFactor) {
-        bool isDiagnostics, uint8_t nonDiagIntervalFactor) {
+    periodicHelper->initialize(collectionInterval, minimumPeriodicInterval, nonDiagIntervalFactor);
-    periodicHelper->initialize(collectionInterval, minimumPeriodicInterval,
-            isDiagnostics, nonDiagIntervalFactor);
 }
 
 void LocalPoolDataSetBase::setChanged(bool changed) {

datapoollocal/LocalPoolDataSetBase.h

@@ -191,9 +191,8 @@ protected:
     bool reportingEnabled = false;
     void setReportingEnabled(bool enabled);
 
-    void initializePeriodicHelper(float collectionInterval,
+    void initializePeriodicHelper(float collectionInterval, dur_millis_t minimumPeriodicInterval,
-            dur_millis_t minimumPeriodicInterval,
+            uint8_t nonDiagIntervalFactor = 5);
-            bool isDiagnostics, uint8_t nonDiagIntervalFactor = 5);
 
     /**
      * If the valid state of a dataset is always relevant to the whole

datapoollocal/internal/LocalPoolDataSetAttorney.h

@@ -14,9 +14,8 @@ private:
     }
 
     static void initializePeriodicHelper(LocalPoolDataSetBase& set, float collectionInterval,
-            uint32_t minimumPeriodicIntervalMs,
+            uint32_t minimumPeriodicIntervalMs, uint8_t nonDiagIntervalFactor = 5) {
-            bool isDiagnostics, uint8_t nonDiagIntervalFactor = 5) {
+        set.initializePeriodicHelper(collectionInterval, minimumPeriodicIntervalMs,
-        set.initializePeriodicHelper(collectionInterval, minimumPeriodicIntervalMs, isDiagnostics,
                 nonDiagIntervalFactor);
     }
 

housekeeping/PeriodicHousekeepingHelper.cpp

@@ -6,11 +6,8 @@
 PeriodicHousekeepingHelper::PeriodicHousekeepingHelper(
         LocalPoolDataSetBase* owner): owner(owner) {}
 
-
 void PeriodicHousekeepingHelper::initialize(float collectionInterval,
-        dur_millis_t minimumPeriodicInterval, bool isDiagnostics,
+        dur_millis_t minimumPeriodicInterval, uint8_t nonDiagIntervalFactor) {
-        uint8_t nonDiagIntervalFactor) {
-    this->isDiagnostics = isDiagnostics;
     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);
 }
+

housekeeping/PeriodicHousekeepingHelper.h

@@ -10,16 +10,15 @@ class PeriodicHousekeepingHelper {
 public:
 	PeriodicHousekeepingHelper(LocalPoolDataSetBase* owner);
 
-	void initialize(float collectionInterval,
+	void initialize(float collectionInterval, dur_millis_t minimumPeriodicInterval,
-			dur_millis_t minimumPeriodicInterval, bool isDiagnostics,
+	        uint8_t nonDiagIntervalFactor);
-			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);

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<int>(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<int>(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<int>(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

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;
 }
+

unittest/tests/datapoollocal/LocalPoolOwnerBase.h

@@ -54,13 +54,7 @@ private:
 class LocalPoolTestDataSet: public LocalDataSet {
 public:
     LocalPoolTestDataSet():
-        LocalDataSet(lpool::testSid, lpool::dataSetMaxVariables),
+        LocalDataSet(lpool::testSid, lpool::dataSetMaxVariables) {}
-        localPoolVarUint8(lpool::uint8VarGpid, this),
-        localPoolVarFloat(lpool::floatVarGpid, this),
-        localPoolUint16Vec(lpool::uint16Vec3Gpid, this)
-
-    {
-    }
 
     LocalPoolTestDataSet(HasLocalDataPoolIF* owner, uint32_t setId):
         LocalDataSet(owner, setId, lpool::dataSetMaxVariables) {
@@ -70,6 +64,9 @@ public:
     lp_var_t<float> localPoolVarFloat = lp_var_t<float>(lpool::floatVarGpid, this);
     lp_vec_t<uint16_t, 3> localPoolUint16Vec = lp_vec_t<uint16_t, 3>(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() {
+    ~LocalPoolOwnerBase();
-        QueueFactory::instance()->deleteMessageQueue(messageQueue);
-    }
 
     object_id_t getObjectId() const override {
         return SystemObject::getObjectId();

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;