From 6c0972b2d5770d066f2b0666ad40a33caed364a8 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 10 Mar 2021 21:17:08 +0100 Subject: [PATCH] several bugfixes --- datapoollocal/HasLocalDataPoolIF.h | 2 +- datapoollocal/LocalDataPoolManager.cpp | 2 +- datapoollocal/LocalPoolDataSetBase.cpp | 9 ++ datapoollocal/LocalPoolDataSetBase.h | 8 ++ housekeeping/HousekeepingMessage.cpp | 10 +- housekeeping/PeriodicHousekeepingHelper.cpp | 94 +++++++++++++------ housekeeping/PeriodicHousekeepingHelper.h | 8 +- .../datapoollocal/LocalPoolManagerTest.cpp | 7 ++ .../tests/datapoollocal/LocalPoolOwnerBase.h | 4 +- 9 files changed, 106 insertions(+), 38 deletions(-) diff --git a/datapoollocal/HasLocalDataPoolIF.h b/datapoollocal/HasLocalDataPoolIF.h index d52c72b6..ec25f082 100644 --- a/datapoollocal/HasLocalDataPoolIF.h +++ b/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 diff --git a/datapoollocal/LocalDataPoolManager.cpp b/datapoollocal/LocalDataPoolManager.cpp index 9f0eb779..83e30e82 100644 --- a/datapoollocal/LocalDataPoolManager.cpp +++ b/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; } diff --git a/datapoollocal/LocalPoolDataSetBase.cpp b/datapoollocal/LocalPoolDataSetBase.cpp index 2d70712b..3abbf0dd 100644 --- a/datapoollocal/LocalPoolDataSetBase.cpp +++ b/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; + } +} diff --git a/datapoollocal/LocalPoolDataSetBase.h b/datapoollocal/LocalPoolDataSetBase.h index b9946aaf..39ea9f1a 100644 --- a/datapoollocal/LocalPoolDataSetBase.h +++ b/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. diff --git a/housekeeping/HousekeepingMessage.cpp b/housekeeping/HousekeepingMessage.cpp index d9803ef6..58f8551d 100644 --- a/housekeeping/HousekeepingMessage.cpp +++ b/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); diff --git a/housekeeping/PeriodicHousekeepingHelper.cpp b/housekeeping/PeriodicHousekeepingHelper.cpp index e7345677..b0dd8522 100644 --- a/housekeeping/PeriodicHousekeepingHelper.cpp +++ b/housekeeping/PeriodicHousekeepingHelper.cpp @@ -4,49 +4,85 @@ #include 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(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(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(collectionInterval * minimumPeriodicInterval / 1000.0); } void PeriodicHousekeepingHelper::changeCollectionInterval( - float newIntervalSeconds) { - collectionIntervalTicks = intervalSecondsToInterval(newIntervalSeconds); + float newIntervalSeconds) { + collectionIntervalTicks = intervalSecondsToIntervalTicks(newIntervalSeconds); } diff --git a/housekeeping/PeriodicHousekeepingHelper.h b/housekeeping/PeriodicHousekeepingHelper.h index d96eae1d..3e8d030e 100644 --- a/housekeeping/PeriodicHousekeepingHelper.h +++ b/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; diff --git a/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp b/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp index 685016ed..1ab87151 100644 --- a/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp +++ b/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 diff --git a/unittest/tests/datapoollocal/LocalPoolOwnerBase.h b/unittest/tests/datapoollocal/LocalPoolOwnerBase.h index f74c47b5..20c565ff 100644 --- a/unittest/tests/datapoollocal/LocalPoolOwnerBase.h +++ b/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; } /**