From b8de761d7ccebfb0a783a72981cc40adb4af2601 Mon Sep 17 00:00:00 2001
From: Dimitrios Sourlantzis <jem2010@windowslive.com>
Date: Mon, 6 Dec 2021 19:16:13 +0000
Subject: [PATCH] Magic numbers fix v3

---
 inc/ServicePool.hpp                         |  1 +
 inc/Services/EventReportService.hpp         |  2 +-
 src/MessageParser.cpp                       | 30 ++++++++--------
 src/Platform/x86/main.cpp                   |  2 +-
 src/Services/EventActionService.cpp         | 33 ++++++++---------
 src/Services/EventReportService.cpp         | 11 +++---
 src/Services/FunctionManagementService.cpp  |  2 +-
 src/Services/MemoryManagementService.cpp    | 12 +++----
 src/Services/ParameterService.cpp           |  8 ++---
 src/Services/ParameterStatisticsService.cpp | 14 ++++----
 src/Services/TestService.cpp                |  4 +--
 src/Services/TimeBasedSchedulingService.cpp | 40 ++++++++++-----------
 test/Services/EventReportService.cpp        |  3 +-
 13 files changed, 84 insertions(+), 78 deletions(-)

diff --git a/inc/ServicePool.hpp b/inc/ServicePool.hpp
index 753dca4d..7666d416 100644
--- a/inc/ServicePool.hpp
+++ b/inc/ServicePool.hpp
@@ -26,6 +26,7 @@ class ServicePool {
 	 * the number of the service, while the least significant 8 bits are the number of the Message. The value is the
 	 * counter of each MessageType.
 	 */
+
 	etl::map<uint16_t, uint16_t, ECSSTotalMessageTypes> messageTypeCounter;
 
 	/**
diff --git a/inc/Services/EventReportService.hpp b/inc/Services/EventReportService.hpp
index 0776159d..dc24b92c 100644
--- a/inc/Services/EventReportService.hpp
+++ b/inc/Services/EventReportService.hpp
@@ -30,7 +30,7 @@ public:
 		HighSeverityAnomalyReport = 4,
 		EnableReportGenerationOfEvents = 5,
 		DisableReportGenerationOfEvents = 6,
-		ReportListOfDisabledEvent = 7,
+		ReportListOfDisabledEvents = 7,
 		DisabledListEventReport = 8,
 	};
 
diff --git a/src/MessageParser.cpp b/src/MessageParser.cpp
index b7e81089..f0938627 100644
--- a/src/MessageParser.cpp
+++ b/src/MessageParser.cpp
@@ -8,50 +8,50 @@
 void MessageParser::execute(Message& message) {
 	switch (message.serviceType) {
 #ifdef SERVICE_PARAMETERSTATISTICS
-		case 4:
+		case ParameterStatisticsService::ServiceType:
 			Services.parameterStatistics.execute(message);
 			break;
 #endif
 
 #ifdef SERVICE_EVENTREPORT
-		case 5:
-			Services.eventReport.execute(message); // ST[05]
+		case EventReportService::ServiceType:
+			Services.eventReport.execute(message);
 			break;
 #endif
 
 #ifdef SERVICE_MEMORY
-		case 6:
-			Services.memoryManagement.execute(message); // ST[06]
+		case MemoryManagementService::ServiceType:
+			Services.memoryManagement.execute(message);
 			break;
 #endif
 
 #ifdef SERVICE_FUNCTION
-		case 8:
-			Services.functionManagement.execute(message); // ST[08]
+		case FunctionManagementService::ServiceType:
+			Services.functionManagement.execute(message);
 			break;
 #endif
 
 #ifdef SERVICE_TIMESCHEDULING
-		case 11:
-			Services.timeBasedScheduling.execute(message); // ST[11]
+		case TimeBasedSchedulingService::ServiceType:
+			Services.timeBasedScheduling.execute(message);
 			break;
 #endif
 
 #ifdef SERVICE_TEST
-		case 17:
-			Services.testService.execute(message); // ST[17]
+		case TestService::ServiceType:
+			Services.testService.execute(message);
 			break;
 #endif
 
 #ifdef SERVICE_EVENTACTION
-		case 19:
-			Services.eventAction.execute(message); // ST[19]
+		case EventActionService::ServiceType:
+			Services.eventAction.execute(message);
 			break;
 #endif
 
 #ifdef SERVICE_PARAMETER
-		case 20:
-			Services.parameterManagement.execute(message); // ST[20]
+		case ParameterService::ServiceType:
+			Services.parameterManagement.execute(message);
 			break;
 #endif
 
diff --git a/src/Platform/x86/main.cpp b/src/Platform/x86/main.cpp
index a9060ed1..52cdd073 100644
--- a/src/Platform/x86/main.cpp
+++ b/src/Platform/x86/main.cpp
@@ -195,7 +195,7 @@ int main() {
 	eventMessage2.appendUint16(1);
 	eventMessage2.appendEnum16(eventIDs2[0]);
 
-	Message eventMessage3(EventReportService::ServiceType, EventReportService::MessageType::ReportListOfDisabledEvent,
+	Message eventMessage3(EventReportService::ServiceType, EventReportService::MessageType::ReportListOfDisabledEvents,
 	                      Message::TC, 1);
 	eventReportService.disableReportGeneration(eventMessage);
 	eventReportService.listOfDisabledEventsReport();
diff --git a/src/Services/EventActionService.cpp b/src/Services/EventActionService.cpp
index 97546642..0963cf94 100644
--- a/src/Services/EventActionService.cpp
+++ b/src/Services/EventActionService.cpp
@@ -22,6 +22,7 @@ void EventActionService::addEventActionDefinitions(Message& message) {
 			}
 		}
 	}
+
 	if ((message.dataSize - 6) > ECSSTCRequestStringSize) {
 		canBeAdded = false;
 		ErrorHandler::reportInternalError(ErrorHandler::MessageTooLarge);
@@ -196,29 +197,29 @@ void EventActionService::executeAction(uint16_t eventID) {
 
 void EventActionService::execute(Message& message) {
 	switch (message.messageType) {
-		case 1:
-			addEventActionDefinitions(message); // TC[19,1]
+		case AddEventAction:
+			addEventActionDefinitions(message);
 			break;
-		case 2:
-			deleteEventActionDefinitions(message); // TC[19,2]
+		case DeleteEventAction:
+			deleteEventActionDefinitions(message);
 			break;
-		case 3:
-			deleteAllEventActionDefinitions(message); // TC[19,3]
+		case DeleteAllEventAction:
+			deleteAllEventActionDefinitions(message);
 			break;
-		case 4:
-			enableEventActionDefinitions(message); // TC[19,4]
+		case EnableEventAction:
+			enableEventActionDefinitions(message);
 			break;
-		case 5:
-			disableEventActionDefinitions(message); // TC[19,5]
+		case DisableEventAction:
+			disableEventActionDefinitions(message);
 			break;
-		case 6:
-			requestEventActionDefinitionStatus(message); // TC[19,6]
+		case ReportStatusOfEachEventAction:
+			requestEventActionDefinitionStatus(message);
 			break;
-		case 8:
-			enableEventActionFunction(message); // TC[19,8]
+		case EnableEventActionFunction :
+			enableEventActionFunction(message);
 			break;
-		case 9:
-			disableEventActionFunction(message); // TC[19,9]
+		case DisableEventActionFunction:
+			disableEventActionFunction(message);
 			break;
 		default:
 			ErrorHandler::reportInternalError(ErrorHandler::OtherMessageType);
diff --git a/src/Services/EventReportService.cpp b/src/Services/EventReportService.cpp
index a5257283..53dc18fb 100644
--- a/src/Services/EventReportService.cpp
+++ b/src/Services/EventReportService.cpp
@@ -115,7 +115,7 @@ void EventReportService::disableReportGeneration(Message message) {
 
 void EventReportService::requestListOfDisabledEvents(Message message) {
 	// TC[5,7]
-	message.assertTC(EventReportService::ServiceType, EventReportService::MessageType::ReportListOfDisabledEvent);
+	message.assertTC(EventReportService::ServiceType, EventReportService::MessageType::ReportListOfDisabledEvents);
 
 	listOfDisabledEventsReport();
 }
@@ -137,11 +137,14 @@ void EventReportService::listOfDisabledEventsReport() {
 
 void EventReportService::execute(Message& message) {
 	switch (message.messageType) {
-		case 5: enableReportGeneration(message); // TC[5,5]
+		case EnableReportGenerationOfEvents:
+			enableReportGeneration(message);
 			break;
-		case 6: disableReportGeneration(message); // TC[5,6]
+		case DisableReportGenerationOfEvents:
+			disableReportGeneration(message);
 			break;
-		case 7: requestListOfDisabledEvents(message); // TC[5,7]
+		case ReportListOfDisabledEvents:
+			requestListOfDisabledEvents(message);
 			break;
 		default:
 			ErrorHandler::reportInternalError(ErrorHandler::OtherMessageType);
diff --git a/src/Services/FunctionManagementService.cpp b/src/Services/FunctionManagementService.cpp
index c01260a9..15361ff6 100644
--- a/src/Services/FunctionManagementService.cpp
+++ b/src/Services/FunctionManagementService.cpp
@@ -54,7 +54,7 @@ void FunctionManagementService::include(String<ECSSFunctionNameLength> funcName,
 
 void FunctionManagementService::execute(Message& message) {
 	switch (message.messageType) {
-		case 1:
+		case PerformFunction:
 			call(message); // TC[8,1]
 			break;
 		default:
diff --git a/src/Services/MemoryManagementService.cpp b/src/Services/MemoryManagementService.cpp
index 870b84cd..db821cbf 100644
--- a/src/Services/MemoryManagementService.cpp
+++ b/src/Services/MemoryManagementService.cpp
@@ -225,14 +225,14 @@ inline bool MemoryManagementService::dataValidator(const uint8_t* data, uint16_t
 
 void MemoryManagementService::execute(Message& message) {
 	switch (message.messageType) {
-		case 2:
-			rawDataMemorySubservice.loadRawData(message); // TC[6,2]
+		case LoadRawMemoryDataAreas:
+			rawDataMemorySubservice.loadRawData(message);
 			break;
-		case 5:
-			rawDataMemorySubservice.dumpRawData(message); // TC[6,5]
+		case DumpRawMemoryData:
+			rawDataMemorySubservice.dumpRawData(message);
 			break;
-		case 9:
-			rawDataMemorySubservice.checkRawData(message); // TC[6,9]
+		case CheckRawMemoryData:
+			rawDataMemorySubservice.checkRawData(message);
 			break;
 		default:
 			ErrorHandler::reportInternalError(ErrorHandler::OtherMessageType);
diff --git a/src/Services/ParameterService.cpp b/src/Services/ParameterService.cpp
index ee308a69..de478627 100644
--- a/src/Services/ParameterService.cpp
+++ b/src/Services/ParameterService.cpp
@@ -64,11 +64,11 @@ void ParameterService::setParameters(Message& newParamValues) {
 
 void ParameterService::execute(Message& message) {
 	switch (message.messageType) {
-		case 1:
-			reportParameters(message); // TC[20,1]
+		case ReportParameterValues:
+			reportParameters(message);
 			break;
-		case 3:
-			setParameters(message); // TC[20,3]
+		case SetParameterValues:
+			setParameters(message);
 			break;
 		default:
 			ErrorHandler::reportInternalError(ErrorHandler::OtherMessageType);
diff --git a/src/Services/ParameterStatisticsService.cpp b/src/Services/ParameterStatisticsService.cpp
index 0cb1c86f..1567f917 100644
--- a/src/Services/ParameterStatisticsService.cpp
+++ b/src/Services/ParameterStatisticsService.cpp
@@ -178,25 +178,25 @@ void ParameterStatisticsService::statisticsDefinitionsReport() {
 
 void ParameterStatisticsService::execute(Message& message) {
 	switch (message.messageType) {
-		case 1:
+		case ReportParameterStatistics:
 			reportParameterStatistics(message);
 			break;
-		case 3:
+		case ResetParameterStatistics:
 			resetParameterStatistics(message);
 			break;
-		case 4:
+		case EnablePeriodicParameterReporting:
 			enablePeriodicStatisticsReporting(message);
 			break;
-		case 5:
+		case DisablePeriodicParameterReporting:
 			disablePeriodicStatisticsReporting(message);
 			break;
-		case 6:
+		case AddOrUpdateParameterStatisticsDefinitions:
 			addOrUpdateStatisticsDefinitions(message);
 			break;
-		case 7:
+		case DeleteParameterStatisticsDefinitions:
 			deleteStatisticsDefinitions(message);
 			break;
-		case 8:
+		case ReportParameterStatisticsDefinitions:
 			reportStatisticsDefinitions(message);
 			break;
 		default:
diff --git a/src/Services/TestService.cpp b/src/Services/TestService.cpp
index f7f3966e..93c1ebee 100644
--- a/src/Services/TestService.cpp
+++ b/src/Services/TestService.cpp
@@ -23,10 +23,10 @@ void TestService::onBoardConnection(Message& request) {
 
 void TestService::execute(Message& message) {
 	switch (message.messageType) {
-		case 1:
+		case AreYouAliveTest:
 			areYouAlive(message);
 			break;
-		case 3:
+		case OnBoardConnectionTest:
 			onBoardConnection(message);
 			break;
 		default:
diff --git a/src/Services/TimeBasedSchedulingService.cpp b/src/Services/TimeBasedSchedulingService.cpp
index 61f51e02..b88f46a4 100644
--- a/src/Services/TimeBasedSchedulingService.cpp
+++ b/src/Services/TimeBasedSchedulingService.cpp
@@ -262,35 +262,35 @@ void TimeBasedSchedulingService::summaryReportActivitiesByID(Message& request) {
 
 void TimeBasedSchedulingService::execute(Message& message) {
 	switch (message.messageType) {
-		case 1:
-			enableScheduleExecution(message); // TC[11,1]
+		case EnableTimeBasedScheduleExecutionFunction:
+			enableScheduleExecution(message);
 			break;
-		case 2:
-			disableScheduleExecution(message); // TC[11,2]
+		case DisableTimeBasedScheduleExecutionFunction:
+			disableScheduleExecution(message);
 			break;
-		case 3:
-			resetSchedule(message); // TC[11,3]
+		case ResetTimeBasedSchedule:
+			resetSchedule(message);
 			break;
-		case 4:
-			insertActivities(message); // TC[11,4]
+		case InsertActivities:
+			insertActivities(message);
 			break;
-		case 5:
-			deleteActivitiesByID(message); // TC[11,5]
+		case DeleteActivitiesById:
+			deleteActivitiesByID(message);
 			break;
-		case 7:
-			timeShiftActivitiesByID(message); // TC[11,7]
+		case TimeShiftActivitiesById:
+			timeShiftActivitiesByID(message);
 			break;
-		case 9:
-			detailReportActivitiesByID(message); // TC[11,9]
+		case DetailReportActivitiesById:
+			detailReportActivitiesByID(message);
 			break;
-		case 12:
-			summaryReportActivitiesByID(message); // TC[11,12]
+		case ActivitiesSummaryReportById:
+			summaryReportActivitiesByID(message);
 			break;
-		case 15:
-			timeShiftAllActivities(message); // TC[11,15]
+		case TimeShiftALlScheduledActivities:
+			timeShiftAllActivities(message);
 			break;
-		case 16:
-			detailReportAllActivities(message); // TC[11,16]
+		case DetailReportAllScheduledActivities:
+			detailReportAllActivities(message);
 			break;
 		default:
 			ErrorHandler::reportInternalError(ErrorHandler::OtherMessageType);
diff --git a/test/Services/EventReportService.cpp b/test/Services/EventReportService.cpp
index 6d1e1ebe..25a6b54f 100644
--- a/test/Services/EventReportService.cpp
+++ b/test/Services/EventReportService.cpp
@@ -108,7 +108,8 @@ TEST_CASE("Disable Report Generation TC[5,6]", "[service][st05]") {
 }
 
 TEST_CASE("Request list of disabled events TC[5,7]", "[service][st05]") {
-	Message message(EventReportService::ServiceType, EventReportService::MessageType::ReportListOfDisabledEvent, Message::TC, 1);
+	Message message(EventReportService::ServiceType, EventReportService::MessageType::ReportListOfDisabledEvents,
+	                Message::TC, 1);
 	MessageParser::execute(message);
 	REQUIRE(ServiceTests::hasOneMessage());
 
-- 
GitLab