several bugfixes

datapoollocal/HasLocalDataPoolIF.h

@@ -65,7 +65,7 @@ public:
      * usually be the period the pool owner performs its periodic operation.
      * @return
      */
-    virtual uint32_t getPeriodicOperationFrequency() const = 0;
+    virtual dur_millis_t getPeriodicOperationFrequency() const = 0;
 
     /**
      * @brief   This function will be called by the manager if an update

datapoollocal/LocalDataPoolManager.cpp

@@ -770,7 +770,7 @@ ReturnValue_t LocalDataPoolManager::changeCollectionInterval(sid_t sid,
 			LocalPoolDataSetAttorney::getPeriodicHelper(*dataSet);
 
 	if(periodicHelper == nullptr) {
-		// config error
+		/* Configuration error, set might not have a corresponding pool manager */
 		return PERIODIC_HELPER_INVALID;
 	}
 

datapoollocal/LocalPoolDataSetBase.cpp

@@ -308,3 +308,12 @@ void LocalPoolDataSetBase::setAllVariablesReadOnly() {
         registeredVariables[idx]->setReadWriteMode(pool_rwm_t::VAR_READ);
     }
 }
+
+float LocalPoolDataSetBase::getCollectionInterval() const {
+    if(periodicHelper != nullptr) {
+        return periodicHelper->getCollectionIntervalInSeconds();
+    }
+    else {
+        return 0.0;
+    }
+}

datapoollocal/LocalPoolDataSetBase.h

@@ -168,6 +168,14 @@ public:
 
     bool getReportingEnabled() const;
 
+    /**
+     * Returns the current periodic HK generation interval this set
+     * belongs to a HK manager and the interval is not 0. Otherwise,
+     * returns 0.0
+     * @return
+     */
+    float getCollectionInterval() const;
+
 protected:
     sid_t sid;
     //! This mutex is used if the data is created by one object only.

housekeeping/HousekeepingMessage.cpp

@@ -84,15 +84,21 @@ void HousekeepingMessage::setCollectionIntervalModificationCommand(
 	else {
 		command->setCommand(MODIFY_PARAMETER_REPORT_COLLECTION_INTERVAL);
 	}
-	command->setParameter3(collectionInterval);
+
+	/* Raw storage of the float in the message. Do not use setParameter3, does
+	implicit conversion to integer type! */
+	std::memcpy(command->getData() + 2 * sizeof(uint32_t), &collectionInterval,
+	        sizeof(collectionInterval));
 
 	setSid(command, sid);
 }
 
 sid_t HousekeepingMessage::getCollectionIntervalModificationCommand(
 		const CommandMessage* command, float* newCollectionInterval) {
+
 	if(newCollectionInterval != nullptr) {
-		*newCollectionInterval = command->getParameter3();
+	    std::memcpy(newCollectionInterval, command->getData() + 2 * sizeof(uint32_t),
+	            sizeof(*newCollectionInterval));
 	}
 
 	return getSid(command);

housekeeping/PeriodicHousekeepingHelper.cpp

@@ -4,49 +4,85 @@
 #include <cmath>
 
 PeriodicHousekeepingHelper::PeriodicHousekeepingHelper(
-		LocalPoolDataSetBase* owner): owner(owner) {}
+        LocalPoolDataSetBase* owner): owner(owner) {}
 
 
 void PeriodicHousekeepingHelper::initialize(float collectionInterval,
-		dur_millis_t minimumPeriodicInterval, bool isDiagnostics,
-		uint8_t nonDiagIntervalFactor) {
-	this->minimumPeriodicInterval = minimumPeriodicInterval;
-	if(not isDiagnostics) {
-		this->minimumPeriodicInterval = this->minimumPeriodicInterval *
-				nonDiagIntervalFactor;
-	}
-	collectionIntervalTicks = intervalSecondsToInterval(collectionInterval);
-	/* This will cause a checkOpNecessary call to be true immediately. I think it's okay
+        dur_millis_t minimumPeriodicInterval, bool isDiagnostics,
+        uint8_t nonDiagIntervalFactor) {
+    this->isDiagnostics = isDiagnostics;
+    this->minimumPeriodicInterval = minimumPeriodicInterval;
+    this->nonDiagIntervalFactor = nonDiagIntervalFactor;
+    collectionIntervalTicks = intervalSecondsToIntervalTicks(collectionInterval);
+    /* This will cause a checkOpNecessary call to be true immediately. I think it's okay
 	if a HK packet is generated immediately instead of waiting one generation cycle. */
-	internalTickCounter = collectionIntervalTicks;
+    internalTickCounter = collectionIntervalTicks;
 }
 
-float PeriodicHousekeepingHelper::getCollectionIntervalInSeconds() {
-	return intervalToIntervalSeconds(collectionIntervalTicks);
+float PeriodicHousekeepingHelper::getCollectionIntervalInSeconds() const {
+    return intervalTicksToSeconds(collectionIntervalTicks);
 }
 
 bool PeriodicHousekeepingHelper::checkOpNecessary() {
-	if(internalTickCounter >= collectionIntervalTicks) {
-		internalTickCounter = 1;
-		return true;
-	}
-	internalTickCounter++;
-	return false;
+    if(internalTickCounter >= collectionIntervalTicks) {
+        internalTickCounter = 1;
+        return true;
+    }
+    internalTickCounter++;
+    return false;
 }
 
-uint32_t PeriodicHousekeepingHelper::intervalSecondsToInterval(
-		float collectionIntervalSeconds) {
-        return std::ceil(collectionIntervalSeconds * 1000
-                / minimumPeriodicInterval);
+uint32_t PeriodicHousekeepingHelper::intervalSecondsToIntervalTicks(
+        float collectionIntervalSeconds) {
+    /* Avoid division by zero */
+    if(minimumPeriodicInterval == 0) {
+        if(isDiagnostics) {
+            /* Perform operation each cycle */
+            return 1;
+        }
+        else {
+            return nonDiagIntervalFactor;
+        }
+    }
+    else {
+        dur_millis_t intervalInMs = collectionIntervalSeconds * 1000;
+        uint32_t divisor = minimumPeriodicInterval;
+        if(not isDiagnostics) {
+            /* We need to multiply the divisor because non-diagnostics only
+            allow a multiple of the minimum periodic interval */
+            divisor *= nonDiagIntervalFactor;
+        }
+        uint32_t ticks = std::ceil(static_cast<float>(intervalInMs) / divisor);
+        if(not isDiagnostics) {
+            /* Now we need to multiply the calculated ticks with the factor as as well
+            because the minimum tick count to generate a non-diagnostic is
+            the factor.
+
+            Example calculation for non-diagnostic with
+            0.4 second interval and 0.2 second task interval.
+            Resultant tick count of 5 is equal to operation each second.
+
+            Examle calculation for non-diagnostic with  2.0 second interval and 0.2 second
+            task interval.
+            Resultant tick count of 10 is equal to operatin every 2 seconds.
+
+            Example calculation for diagnostic with 0.4 second interval and 0.3
+            second task interval. Resulting tick count of 2 is equal to operation
+            every 0.6 seconds. */
+            ticks *= nonDiagIntervalFactor;
+        }
+        return ticks;
+    }
 }
 
-float PeriodicHousekeepingHelper::intervalToIntervalSeconds(
-		uint32_t collectionInterval) {
-	return static_cast<float>(collectionInterval *
-			minimumPeriodicInterval);
+float PeriodicHousekeepingHelper::intervalTicksToSeconds(
+        uint32_t collectionInterval) const {
+    /* Number of ticks times the minimum interval is in milliseconds, so we divide by 1000 to get
+    the value in seconds */
+    return static_cast<float>(collectionInterval * minimumPeriodicInterval / 1000.0);
 }
 
 void PeriodicHousekeepingHelper::changeCollectionInterval(
-		float newIntervalSeconds) {
-	collectionIntervalTicks = intervalSecondsToInterval(newIntervalSeconds);
+        float newIntervalSeconds) {
+    collectionIntervalTicks = intervalSecondsToIntervalTicks(newIntervalSeconds);
 }

housekeeping/PeriodicHousekeepingHelper.h

@@ -15,13 +15,15 @@ public:
 			uint8_t nonDiagIntervalFactor);
 
 	void changeCollectionInterval(float newInterval);
-	float getCollectionIntervalInSeconds();
+	float getCollectionIntervalInSeconds() const;
 	bool checkOpNecessary();
 private:
 	LocalPoolDataSetBase* owner = nullptr;
+	bool isDiagnostics = true;
+	uint8_t nonDiagIntervalFactor = 0;
 
-	uint32_t intervalSecondsToInterval(float collectionIntervalSeconds);
-	float intervalToIntervalSeconds(uint32_t collectionInterval);
+	uint32_t intervalSecondsToIntervalTicks(float collectionIntervalSeconds);
+	float intervalTicksToSeconds(uint32_t collectionInterval) const;
 
 	dur_millis_t minimumPeriodicInterval = 0;
 	uint32_t internalTickCounter = 1;

unittest/tests/datapoollocal/LocalPoolManagerTest.cpp

@@ -245,6 +245,13 @@ TEST_CASE("LocalPoolManagerTest" , "[LocManTest]") {
         HousekeepingMessage::setToggleReportingCommand(&hkCmd, lpool::testSid, true, false);
         CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == retval::CATCH_OK);
         CHECK(setHandle->getReportingEnabled() == true);
+
+        HousekeepingMessage::setCollectionIntervalModificationCommand(&hkCmd,
+                lpool::testSid, 0.4, false);
+        CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == retval::CATCH_OK);
+        /* For non-diagnostics and a specified minimum frequency of 0.2 seconds, the
+        resulting collection interval should be 1.0 second */
+        CHECK(poolOwner->dataset.getCollectionInterval() == 1.0);
     }
 
     /* we need to reset the subscription list because the pool owner

unittest/tests/datapoollocal/LocalPoolOwnerBase.h

@@ -129,8 +129,8 @@ public:
         return &poolManager;
     }
 
-    uint32_t getPeriodicOperationFrequency() const override {
-        return 0.2;
+    dur_millis_t getPeriodicOperationFrequency() const override {
+        return 200;
     }
 
     /**