From d12baec37511179492aa08628b8781d24d881ffe Mon Sep 17 00:00:00 2001
From: Dimitrios Stoupis <dimitris.apple@gmail.com>
Date: Wed, 20 Mar 2019 17:33:21 +0000
Subject: [PATCH] Remove currentNumberOfActivities
- A potential bug
---
inc/Services/TimeBasedSchedulingService.hpp | 5 -----
src/Services/TimeBasedSchedulingService.cpp | 7 ++-----
test/Services/TimeBasedSchedulingService.cpp | 19 +------------------
3 files changed, 3 insertions(+), 28 deletions(-)
diff --git a/inc/Services/TimeBasedSchedulingService.hpp b/inc/Services/TimeBasedSchedulingService.hpp
index 917e48bf..3e61db27 100644
--- a/inc/Services/TimeBasedSchedulingService.hpp
+++ b/inc/Services/TimeBasedSchedulingService.hpp
@@ -55,11 +55,6 @@ private:
*/
bool executionFunctionStatus = false; // True indicates "enabled" and False "disabled" state
- /**
- * @brief Number of activities currently in the schedule
- * @todo Define the maximum allowed number of activities in the schedule
- */
- uint8_t currentNumberOfActivities = 0; // Keep track of the number of activities
MessageParser msgParser; // Parse TC packets
/**
diff --git a/src/Services/TimeBasedSchedulingService.cpp b/src/Services/TimeBasedSchedulingService.cpp
index 34ab4073..bbcba14d 100644
--- a/src/Services/TimeBasedSchedulingService.cpp
+++ b/src/Services/TimeBasedSchedulingService.cpp
@@ -31,7 +31,6 @@ void TimeBasedSchedulingService::resetSchedule(Message &request) {
executionFunctionStatus = false; // Disable the service
scheduledActivities.clear(); // Delete all scheduled activities
- currentNumberOfActivities = 0;
// todo: Add resetting for sub-schedules and groups, if defined
}
@@ -48,7 +47,7 @@ void TimeBasedSchedulingService::insertActivities(Message &request) {
uint32_t currentTime = TimeGetter::getUnixSeconds(); // Get the current system time
uint32_t releaseTime = request.readUint32(); // Get the specified release time
- if ((currentNumberOfActivities >= ECSS_MAX_NUMBER_OF_TIME_SCHED_ACTIVITIES) ||
+ if ((scheduledActivities.size() >= ECSS_MAX_NUMBER_OF_TIME_SCHED_ACTIVITIES) ||
(releaseTime < (currentTime + ECSS_TIME_MARGIN_FOR_ACTIVATION))) {
ErrorHandler::reportError(request, ErrorHandler::InstructionExecutionStartError);
request.readPosition += ECSS_TC_REQUEST_STRING_SIZE;
@@ -77,7 +76,6 @@ void TimeBasedSchedulingService::insertActivities(Message &request) {
// Add activities ordered by release time as per the standard requirement
scheduledActivities.insert(releaseTimeOrder, newActivity);
- currentNumberOfActivities++;
}
}
}
@@ -190,7 +188,6 @@ void TimeBasedSchedulingService::deleteActivitiesByID(Message &request) {
if (requestIDMatch != scheduledActivities.end()) {
scheduledActivities.erase(requestIDMatch); // Delete activity from the schedule
- currentNumberOfActivities--;
} else {
ErrorHandler::reportError(request, ErrorHandler::InstructionExecutionStartError);
}
@@ -205,7 +202,7 @@ void TimeBasedSchedulingService::detailReportAllActivities(Message &request) {
// Create the report message object of telemetry message subtype 10 for each activity
Message report = createTM(10);
- report.appendUint16(currentNumberOfActivities);
+ report.appendUint16(static_cast<uint16_t >(scheduledActivities.size()));
for (auto &activity : scheduledActivities) {
// todo: append sub-schedule and group ID if they are defined
diff --git a/test/Services/TimeBasedSchedulingService.cpp b/test/Services/TimeBasedSchedulingService.cpp
index d02a55ed..d8db6ad9 100644
--- a/test/Services/TimeBasedSchedulingService.cpp
+++ b/test/Services/TimeBasedSchedulingService.cpp
@@ -12,10 +12,6 @@ namespace unit_test {
return tmService.executionFunctionStatus;
}
- static uint8_t *currentNumberOfActivities(TimeBasedSchedulingService &tmService) {
- return &tmService.currentNumberOfActivities;
- }
-
static auto scheduledActivities(TimeBasedSchedulingService &tmService) {
return &tmService.scheduledActivities;
}
@@ -104,9 +100,7 @@ TEST_CASE("TC[11,4] Activity Insertion", "[service][st11]") {
TimeBasedSchedulingService timeService;
auto scheduledActivities = activityInsertion(timeService);
- CHECK(scheduledActivities->size() == 4);
-
- REQUIRE(*unit_test::Tester::currentNumberOfActivities(timeService) == 4);
+ REQUIRE(scheduledActivities->size() == 4);
REQUIRE(scheduledActivities->at(0).requestReleaseTime == currentTime + 1556435);
REQUIRE(scheduledActivities->at(1).requestReleaseTime == currentTime + 1726435);
REQUIRE(scheduledActivities->at(2).requestReleaseTime == currentTime + 1957232);
@@ -126,7 +120,6 @@ TEST_CASE("TC[11,15] Time shift all scheduled activities (Positive shift)", "[se
receivedMessage.appendSint32(timeShift);
CHECK(scheduledActivities->size() == 4);
- REQUIRE(*unit_test::Tester::currentNumberOfActivities(timeService) == 4);
timeService.timeShiftAllActivities(receivedMessage);
REQUIRE(scheduledActivities->at(0).requestReleaseTime == currentTime + 1556435 + timeShift);
@@ -144,7 +137,6 @@ TEST_CASE("TC[11,15] Time shift all scheduled activities (Negative shift)", "[se
receivedMessage.appendSint32(-timeShift);
CHECK(scheduledActivities->size() == 4);
- REQUIRE(*unit_test::Tester::currentNumberOfActivities(timeService) == 4);
timeService.timeShiftAllActivities(receivedMessage);
REQUIRE(scheduledActivities->at(0).requestReleaseTime == currentTime + 1556435 - timeShift);
@@ -161,7 +153,6 @@ TEST_CASE("TC[11,7] Time shift activities by ID (Positive Shift)", "[service][st
// Verify that everything is in place
CHECK(scheduledActivities->size() == 4);
- REQUIRE(*unit_test::Tester::currentNumberOfActivities(timeService) == 4);
scheduledActivities->at(2).requestID.applicationID = 4; // Append a dummy application ID
Message receivedMessage(11, 7, Message::TC, 1);
@@ -185,7 +176,6 @@ TEST_CASE("TC[11,7] Time shift activities by ID (Negative Shift)", "[service][st
// Verify that everything is in place
CHECK(scheduledActivities->size() == 4);
- REQUIRE(*unit_test::Tester::currentNumberOfActivities(timeService) == 4);
scheduledActivities->at(2).requestID.applicationID = 4; // Append a dummy application ID
Message receivedMessage(11, 7, Message::TC, 1);
@@ -209,7 +199,6 @@ TEST_CASE("TC[11,9] Detail report scheduled activities by ID", "[service][st11]"
// Verify that everything is in place
CHECK(scheduledActivities->size() == 4);
- REQUIRE(*unit_test::Tester::currentNumberOfActivities(timeService) == 4);
scheduledActivities->at(0).requestID.applicationID = 8; // Append a dummy application ID
scheduledActivities->at(2).requestID.applicationID = 4; // Append a dummy application ID
@@ -255,7 +244,6 @@ TEST_CASE("TC[11,12] Summary report scheduled activities by ID", "[service][st11
// Verify that everything is in place
CHECK(scheduledActivities->size() == 4);
- REQUIRE(*unit_test::Tester::currentNumberOfActivities(timeService) == 4);
scheduledActivities->at(0).requestID.applicationID = 8; // Append a dummy application ID
scheduledActivities->at(2).requestID.applicationID = 4; // Append a dummy application ID
@@ -311,7 +299,6 @@ TEST_CASE("TC[11,16] Detail report all scheduled activities", "[service][st11]")
uint16_t iterationCount = response.readUint16();
REQUIRE(iterationCount == scheduledActivities->size());
- REQUIRE(iterationCount == *unit_test::Tester::currentNumberOfActivities(timeService));
for (uint16_t i = 0; i < iterationCount; i++) {
uint32_t receivedReleaseTime = response.readUint32();
@@ -332,7 +319,6 @@ TEST_CASE("TC[11,5] Activity deletion by ID", "[service][st11]") {
// Verify that everything is in place
CHECK(scheduledActivities->size() == 4);
- REQUIRE(*unit_test::Tester::currentNumberOfActivities(timeService) == 4);
scheduledActivities->at(2).requestID.applicationID = 4; // Append a dummy application ID
Message receivedMessage(11, 5, Message::TC, 1);
@@ -342,10 +328,8 @@ TEST_CASE("TC[11,5] Activity deletion by ID", "[service][st11]") {
receivedMessage.appendUint16(0); // todo: Remove the dummy sequence count
CHECK(scheduledActivities->size() == 4);
- REQUIRE(*unit_test::Tester::currentNumberOfActivities(timeService) == 4);
timeService.deleteActivitiesByID(receivedMessage);
- CHECK(*unit_test::Tester::currentNumberOfActivities(timeService) == 3);
REQUIRE(scheduledActivities->size() == 3);
REQUIRE(scheduledActivities->at(2).requestReleaseTime == currentTime + 17248435);
REQUIRE(scheduledActivities->at(2).request == testMessage4);
@@ -359,6 +343,5 @@ TEST_CASE("TC[11,3] Reset schedule", "[service][st11]") {
timeService.resetSchedule(receivedMessage);
REQUIRE(scheduledActivities->empty());
- REQUIRE(*unit_test::Tester::currentNumberOfActivities(timeService) == 0);
REQUIRE(not unit_test::Tester::executionFunctionStatus(timeService));
}
--
GitLab