From b1d2d3f12335ef4714b867263940da888a61c329 Mon Sep 17 00:00:00 2001
From: athatheo <athatheoc@gmail.com>
Date: Sun, 2 Oct 2022 23:14:33 +0000
Subject: [PATCH] Fix Housekeeping service bug, that would report even if
reporting was disabled
---
inc/ECSS_Definitions.hpp | 4 +-
src/Services/HousekeepingService.cpp | 8 ++-
test/Services/HousekeepingServiceTests.cpp | 21 +++++--
test/TestPlatform.cpp | 70 ++++++++++++++++++----
4 files changed, 82 insertions(+), 21 deletions(-)
diff --git a/inc/ECSS_Definitions.hpp b/inc/ECSS_Definitions.hpp
index fd05e87b..310997bb 100644
--- a/inc/ECSS_Definitions.hpp
+++ b/inc/ECSS_Definitions.hpp
@@ -1,8 +1,8 @@
#ifndef ECSS_SERVICES_ECSS_DEFINITIONS_H
#define ECSS_SERVICES_ECSS_DEFINITIONS_H
-#include <cstdint>
#include <chrono>
+#include <cstdint>
/**
* @defgroup ECSSDefinitions ECSS Defined Constants
*
@@ -131,7 +131,7 @@ inline const uint8_t ECSSMaxDeltaOfReleaseTime = 60;
/**
* The max number of simply commutated parameters per housekeeping structure in ST[03]
*/
-inline const uint16_t ECSSMaxSimplyCommutatedParameters = 10;
+inline const uint16_t ECSSMaxSimplyCommutatedParameters = 30;
/**
* The number of functions supported by the \ref FunctionManagementService
diff --git a/src/Services/HousekeepingService.cpp b/src/Services/HousekeepingService.cpp
index ac616781..0f4bca42 100644
--- a/src/Services/HousekeepingService.cpp
+++ b/src/Services/HousekeepingService.cpp
@@ -116,10 +116,13 @@ void HousekeepingService::housekeepingParametersReport(uint8_t structureId) {
ErrorHandler::reportInternalError(ErrorHandler::InternalErrorType::NonExistentHousekeeping);
return;
}
+
+ HousekeepingStructure& housekeepingStructure = housekeepingStructures.at(structureId);
+
Message housekeepingReport = createTM(MessageType::HousekeepingParametersReport);
housekeepingReport.appendUint8(structureId);
- for (auto id: housekeepingStructures.at(structureId).simplyCommutatedParameterIds) {
+ for (auto id: housekeepingStructure.simplyCommutatedParameterIds) {
if (auto parameter = Services.parameterManagement.getParameter(id)) {
parameter->get().appendValueToMessage(housekeepingReport);
}
@@ -265,6 +268,9 @@ HousekeepingService::reportPendingStructures(uint32_t currentTime, uint32_t prev
uint32_t nextCollection = std::numeric_limits<uint32_t>::max();
for (auto& housekeepingStructure: housekeepingStructures) {
+ if (!housekeepingStructure.second.periodicGenerationActionStatus) {
+ continue;
+ }
if (housekeepingStructure.second.collectionInterval == 0) {
housekeepingParametersReport(housekeepingStructure.second.structureId);
nextCollection = 0;
diff --git a/test/Services/HousekeepingServiceTests.cpp b/test/Services/HousekeepingServiceTests.cpp
index 711a3035..b5882c84 100644
--- a/test/Services/HousekeepingServiceTests.cpp
+++ b/test/Services/HousekeepingServiceTests.cpp
@@ -1,7 +1,7 @@
-#include "Services/HousekeepingService.hpp"
#include <iostream>
#include "Message.hpp"
#include "ServiceTests.hpp"
+#include "Services/HousekeepingService.hpp"
#include "catch2/catch_all.hpp"
#include "etl/algorithm.h"
@@ -532,8 +532,12 @@ TEST_CASE("Append parameters in housekeeping report structure") {
Message request(HousekeepingService::ServiceType,
HousekeepingService::MessageType::AppendParametersToHousekeepingStructure, Message::TC, 1);
- uint16_t numOfSimplyCommutatedParams = 13;
- etl::vector<uint16_t, 13> simplyCommutatedIds = {0, 1, 2, 3, 6, 7, 8, 9, 12, 13, 14, 15, 16};
+ uint16_t numOfSimplyCommutatedParams = 34;
+
+ etl::vector<uint16_t, 34> simplyCommutatedIds;
+ for (uint16_t i = 0; i < 34; i++) {
+ simplyCommutatedIds.push_back(i);
+ }
request.appendUint8(structId);
request.appendUint16(numOfSimplyCommutatedParams);
@@ -546,11 +550,11 @@ TEST_CASE("Append parameters in housekeeping report structure") {
MessageParser::execute(request);
- REQUIRE(housekeepingService.housekeepingStructures[structId].simplyCommutatedParameterIds.size() == 10);
- CHECK(ServiceTests::count() == 2);
+ REQUIRE(housekeepingService.housekeepingStructures[structId].simplyCommutatedParameterIds.size() == 30);
+ CHECK(ServiceTests::count() == 4);
CHECK(ServiceTests::countThrownErrors(
ErrorHandler::ExecutionStartErrorType::ExceededMaxNumberOfSimplyCommutatedParameters) == 1);
- CHECK(ServiceTests::countThrownErrors(ErrorHandler::ExecutionStartErrorType::AlreadyExistingParameter) == 1);
+ CHECK(ServiceTests::countThrownErrors(ErrorHandler::ExecutionStartErrorType::AlreadyExistingParameter) == 3);
ServiceTests::reset();
Services.reset();
@@ -644,6 +648,10 @@ TEST_CASE("Periodically reporting Housekeeping Structures") {
housekeepingService.housekeepingStructures.at(0).collectionInterval = 900;
housekeepingService.housekeepingStructures.at(4).collectionInterval = 1000;
housekeepingService.housekeepingStructures.at(6).collectionInterval = 2700;
+ housekeepingService.housekeepingStructures.at(0).periodicGenerationActionStatus = true;
+ housekeepingService.housekeepingStructures.at(4).periodicGenerationActionStatus = true;
+ housekeepingService.housekeepingStructures.at(6).periodicGenerationActionStatus = true;
+
nextCollection = housekeepingService.reportPendingStructures(currentTime, previousTime, nextCollection);
previousTime = currentTime;
currentTime += nextCollection;
@@ -679,6 +687,7 @@ TEST_CASE("Periodically reporting Housekeeping Structures") {
}
SECTION("Collection Intervals set to 0") {
for (auto& housekeepingStructure: housekeepingService.housekeepingStructures) {
+ housekeepingStructure.second.periodicGenerationActionStatus = true;
housekeepingStructure.second.collectionInterval = 0;
}
nextCollection = housekeepingService.reportPendingStructures(currentTime, previousTime, nextCollection);
diff --git a/test/TestPlatform.cpp b/test/TestPlatform.cpp
index ddb8aad9..d0f491f1 100644
--- a/test/TestPlatform.cpp
+++ b/test/TestPlatform.cpp
@@ -103,24 +103,70 @@ namespace PlatformParameters {
inline Parameter<uint32_t> parameter10(43);
inline Parameter<uint32_t> parameter11(91);
inline Parameter<uint8_t> parameter12(1);
+ inline Parameter<uint8_t> parameter13(1);
+ inline Parameter<uint8_t> parameter14(1);
+ inline Parameter<uint8_t> parameter15(1);
+ inline Parameter<uint8_t> parameter16(1);
+ inline Parameter<uint8_t> parameter17(1);
+ inline Parameter<uint8_t> parameter18(1);
+ inline Parameter<uint8_t> parameter19(1);
+ inline Parameter<uint8_t> parameter20(1);
+ inline Parameter<uint8_t> parameter21(1);
+ inline Parameter<uint8_t> parameter22(1);
+ inline Parameter<uint8_t> parameter23(1);
+ inline Parameter<uint8_t> parameter24(1);
+ inline Parameter<uint8_t> parameter25(1);
+ inline Parameter<uint8_t> parameter26(1);
+ inline Parameter<uint8_t> parameter27(1);
+ inline Parameter<uint8_t> parameter28(1);
+ inline Parameter<uint8_t> parameter29(1);
+ inline Parameter<uint8_t> parameter30(1);
+ inline Parameter<uint8_t> parameter31(1);
+ inline Parameter<uint8_t> parameter32(1);
+ inline Parameter<uint8_t> parameter33(1);
+ inline Parameter<uint8_t> parameter34(1);
+
} // namespace PlatformParameters
/**
* Specific definition for \ref ParameterService's initialize function, for testing purposes.
*/
void ParameterService::initializeParameterMap() {
- parameters = {{static_cast<uint16_t>(0), PlatformParameters::parameter1},
- {static_cast<uint16_t>(1), PlatformParameters::parameter2},
- {static_cast<uint16_t>(2), PlatformParameters::parameter3},
- {static_cast<uint16_t>(3), PlatformParameters::parameter4},
- {static_cast<uint16_t>(4), PlatformParameters::parameter5},
- {static_cast<uint16_t>(5), PlatformParameters::parameter6},
- {static_cast<uint16_t>(6), PlatformParameters::parameter7},
- {static_cast<uint16_t>(7), PlatformParameters::parameter8},
- {static_cast<uint16_t>(8), PlatformParameters::parameter9},
- {static_cast<uint16_t>(9), PlatformParameters::parameter10},
- {static_cast<uint16_t>(10), PlatformParameters::parameter11},
- {static_cast<uint16_t>(11), PlatformParameters::parameter12}};
+ parameters = {
+ {uint16_t{0}, PlatformParameters::parameter1},
+ {uint16_t{1}, PlatformParameters::parameter2},
+ {uint16_t{2}, PlatformParameters::parameter3},
+ {uint16_t{3}, PlatformParameters::parameter4},
+ {uint16_t{4}, PlatformParameters::parameter5},
+ {uint16_t{5}, PlatformParameters::parameter6},
+ {uint16_t{6}, PlatformParameters::parameter7},
+ {uint16_t{7}, PlatformParameters::parameter8},
+ {uint16_t{8}, PlatformParameters::parameter9},
+ {uint16_t{9}, PlatformParameters::parameter10},
+ {uint16_t{10}, PlatformParameters::parameter11},
+ {uint16_t{11}, PlatformParameters::parameter12},
+ {uint16_t{12}, PlatformParameters::parameter13},
+ {uint16_t{13}, PlatformParameters::parameter14},
+ {uint16_t{14}, PlatformParameters::parameter15},
+ {uint16_t{15}, PlatformParameters::parameter16},
+ {uint16_t{16}, PlatformParameters::parameter17},
+ {uint16_t{17}, PlatformParameters::parameter18},
+ {uint16_t{18}, PlatformParameters::parameter19},
+ {uint16_t{19}, PlatformParameters::parameter20},
+ {uint16_t{20}, PlatformParameters::parameter21},
+ {uint16_t{21}, PlatformParameters::parameter22},
+ {uint16_t{22}, PlatformParameters::parameter23},
+ {uint16_t{23}, PlatformParameters::parameter24},
+ {uint16_t{24}, PlatformParameters::parameter25},
+ {uint16_t{25}, PlatformParameters::parameter26},
+ {uint16_t{26}, PlatformParameters::parameter27},
+ {uint16_t{27}, PlatformParameters::parameter28},
+ {uint16_t{28}, PlatformParameters::parameter29},
+ {uint16_t{29}, PlatformParameters::parameter30},
+ {uint16_t{30}, PlatformParameters::parameter31},
+ {uint16_t{31}, PlatformParameters::parameter32},
+ {uint16_t{32}, PlatformParameters::parameter33},
+ {uint16_t{33}, PlatformParameters::parameter34}};
}
void TimeBasedSchedulingService::notifyNewActivityAddition() {}
--
GitLab