From 6c244c3a7fc1317f8aa65e1f24ab63e517110491 Mon Sep 17 00:00:00 2001
From: kongr45gpen <electrovesta@gmail.com>
Date: Sat, 23 Mar 2019 20:28:44 +0200
Subject: [PATCH] Use the new assertions on services

---
 src/Services/EventActionService.cpp         | 197 +++++++++-----------
 src/Services/EventReportService.cpp         |  60 +++---
 src/Services/MemoryManagementService.cpp    |   9 +-
 src/Services/ParameterService.cpp           |   2 +
 src/Services/RequestVerificationService.cpp |   3 +-
 src/Services/TestService.cpp                |   2 +
 src/Services/TimeManagementService.cpp      |   3 +-
 7 files changed, 133 insertions(+), 143 deletions(-)

diff --git a/src/Services/EventActionService.cpp b/src/Services/EventActionService.cpp
index e33ff714..2ace42dd 100644
--- a/src/Services/EventActionService.cpp
+++ b/src/Services/EventActionService.cpp
@@ -8,145 +8,136 @@
  */
 void EventActionService::addEventActionDefinitions(Message message) {
 	// TC[19,1]
+	message.assertTC(19, 1);
 
-	if (message.messageType == 1 && message.packetType == Message::TC && message.serviceType ==
-	                                                                     19) {
-		uint16_t index;
-		uint16_t applicationID = message.readEnum16();
-		uint16_t eventDefinitionID = message.readEnum16();
-		bool accepted = true;
+	uint16_t index;
+	uint16_t applicationID = message.readEnum16();
+	uint16_t eventDefinitionID = message.readEnum16();
+	bool accepted = true;
+	for (index = 0; index < ECSS_EVENT_ACTION_STRUCT_ARRAY_SIZE; index++) {
+		if (eventActionDefinitionArray[index].applicationId == applicationID &&
+		    eventActionDefinitionArray[index].eventDefinitionID == eventDefinitionID &&
+		    eventActionDefinitionArray[index].enabled) {
+			// @todo: throw a failed start of execution error
+			accepted = false;
+		}
+	}
+	if (accepted){
 		for (index = 0; index < ECSS_EVENT_ACTION_STRUCT_ARRAY_SIZE; index++) {
-			if (eventActionDefinitionArray[index].applicationId == applicationID &&
-			    eventActionDefinitionArray[index].eventDefinitionID == eventDefinitionID &&
-			    eventActionDefinitionArray[index].enabled) {
-				// @todo: throw a failed start of execution error
-				accepted = false;
+			// @todo: throw an error if it's full
+			if (eventActionDefinitionArray[index].empty) {
+				break;
 			}
 		}
-		if (accepted){
-			for (index = 0; index < ECSS_EVENT_ACTION_STRUCT_ARRAY_SIZE; index++) {
-				// @todo: throw an error if it's full
-				if (eventActionDefinitionArray[index].empty) {
-					break;
-				}
-			}
-			if (index < ECSS_EVENT_ACTION_STRUCT_ARRAY_SIZE) {
-				eventActionDefinitionArray[index].empty = false;
-				eventActionDefinitionArray[index].enabled = true;
-				eventActionDefinitionArray[index].applicationId = applicationID;
-				eventActionDefinitionArray[index].eventDefinitionID = eventDefinitionID;
-				if (message.dataSize - 4 > ECSS_EVENT_SERVICE_STRING_SIZE) {
-					ErrorHandler::reportInternalError(ErrorHandler::InternalErrorType::MessageTooLarge);
-				} else {
-					char data[ECSS_EVENT_SERVICE_STRING_SIZE];
-					message.readString(data, message.dataSize - 4);
-					eventActionDefinitionArray[index].request = String<ECSS_EVENT_SERVICE_STRING_SIZE>(
-						data);
-				}
+		if (index < ECSS_EVENT_ACTION_STRUCT_ARRAY_SIZE) {
+			eventActionDefinitionArray[index].empty = false;
+			eventActionDefinitionArray[index].enabled = true;
+			eventActionDefinitionArray[index].applicationId = applicationID;
+			eventActionDefinitionArray[index].eventDefinitionID = eventDefinitionID;
+			if (message.dataSize - 4 > ECSS_EVENT_SERVICE_STRING_SIZE) {
+				ErrorHandler::reportInternalError(ErrorHandler::InternalErrorType::MessageTooLarge);
+			} else {
+				char data[ECSS_EVENT_SERVICE_STRING_SIZE];
+				message.readString(data, message.dataSize - 4);
+				eventActionDefinitionArray[index].request = String<ECSS_EVENT_SERVICE_STRING_SIZE>(
+					data);
 			}
 		}
 	}
 }
 
 void EventActionService::deleteEventActionDefinitions(Message message) {
-	// TC[19,2]
-	if (message.messageType == 2 && message.packetType == Message::TC && message.serviceType
-	                                                                     == 19) {
-		uint16_t numberOfEventActionDefinitions = message.readUint16();
-		for (uint16_t i = 0; i < numberOfEventActionDefinitions; i++) {
-			uint16_t applicationID = message.readEnum16();
-			uint16_t eventDefinitionID = message.readEnum16();
-			for (uint16_t index = 0; index < ECSS_EVENT_ACTION_STRUCT_ARRAY_SIZE; index++) {
-				if (eventActionDefinitionArray[index].applicationId == applicationID &&
-				    eventActionDefinitionArray[index].eventDefinitionID == eventDefinitionID &&
-				    eventActionDefinitionArray[index].enabled) {
-					eventActionDefinitionArray[index].empty = true;
-					eventActionDefinitionArray[index].eventDefinitionID = 65535;
-					eventActionDefinitionArray[index].request = "";
-					eventActionDefinitionArray[index].applicationId = 0;
-					eventActionDefinitionArray[index].enabled = false;
-				}
-			}
-		}
+	message.assertTC(19, 2);
 
-	}
-}
-
-void EventActionService::deleteAllEventActionDefinitions(Message message) {
-	// TC[19,3]
-	if (message.messageType == 3 && message.packetType == Message::TC && message.serviceType
-	                                                                     == 19) {
-		setEventActionFunctionStatus(false);
+	uint16_t numberOfEventActionDefinitions = message.readUint16();
+	for (uint16_t i = 0; i < numberOfEventActionDefinitions; i++) {
+		uint16_t applicationID = message.readEnum16();
+		uint16_t eventDefinitionID = message.readEnum16();
 		for (uint16_t index = 0; index < ECSS_EVENT_ACTION_STRUCT_ARRAY_SIZE; index++) {
-			if (not eventActionDefinitionArray[index].empty) {
+			if (eventActionDefinitionArray[index].applicationId == applicationID &&
+			    eventActionDefinitionArray[index].eventDefinitionID == eventDefinitionID &&
+			    eventActionDefinitionArray[index].enabled) {
 				eventActionDefinitionArray[index].empty = true;
-				eventActionDefinitionArray[index].enabled = false;
 				eventActionDefinitionArray[index].eventDefinitionID = 65535;
 				eventActionDefinitionArray[index].request = "";
 				eventActionDefinitionArray[index].applicationId = 0;
+				eventActionDefinitionArray[index].enabled = false;
 			}
 		}
 	}
 }
 
+void EventActionService::deleteAllEventActionDefinitions(Message message) {
+	// TC[19,3]
+	message.assertTC(19, 3);
+
+	setEventActionFunctionStatus(false);
+	for (uint16_t index = 0; index < ECSS_EVENT_ACTION_STRUCT_ARRAY_SIZE; index++) {
+		if (not eventActionDefinitionArray[index].empty) {
+			eventActionDefinitionArray[index].empty = true;
+			eventActionDefinitionArray[index].enabled = false;
+			eventActionDefinitionArray[index].eventDefinitionID = 65535;
+			eventActionDefinitionArray[index].request = "";
+			eventActionDefinitionArray[index].applicationId = 0;
+		}
+	}
+}
+
 void EventActionService::enableEventActionDefinitions(Message message) {
 	// TC[19,4]
-	if (message.messageType == 4 && message.packetType == Message::TC && message.serviceType
-	                                                                     == 19) {
-		uint16_t numberOfEventActionDefinitions = message.readUint16();
-		if (numberOfEventActionDefinitions != 0){
-			for (uint16_t i = 0; i < numberOfEventActionDefinitions; i++) {
-				uint16_t applicationID = message.readEnum16();
-				uint16_t eventDefinitionID = message.readEnum16();
-				for (uint16_t index = 0; index < ECSS_EVENT_ACTION_STRUCT_ARRAY_SIZE; index++) {
-					if (eventActionDefinitionArray[index].applicationId == applicationID &&
-					    eventActionDefinitionArray[index].eventDefinitionID == eventDefinitionID) {
-						eventActionDefinitionArray[index].enabled = true;
-					}
-				}
-			}
-		} else {
+	message.assertTC(19, 4);
+
+	uint16_t numberOfEventActionDefinitions = message.readUint16();
+	if (numberOfEventActionDefinitions != 0){
+		for (uint16_t i = 0; i < numberOfEventActionDefinitions; i++) {
+			uint16_t applicationID = message.readEnum16();
+			uint16_t eventDefinitionID = message.readEnum16();
 			for (uint16_t index = 0; index < ECSS_EVENT_ACTION_STRUCT_ARRAY_SIZE; index++) {
-				if (not eventActionDefinitionArray[index].empty){
+				if (eventActionDefinitionArray[index].applicationId == applicationID &&
+				    eventActionDefinitionArray[index].eventDefinitionID == eventDefinitionID) {
 					eventActionDefinitionArray[index].enabled = true;
 				}
 			}
 		}
+	} else {
+		for (uint16_t index = 0; index < ECSS_EVENT_ACTION_STRUCT_ARRAY_SIZE; index++) {
+			if (not eventActionDefinitionArray[index].empty){
+				eventActionDefinitionArray[index].enabled = true;
+			}
+		}
 	}
 }
 
 void EventActionService::disableEventActionDefinitions(Message message) {
 	// TC[19,5]
-	if (message.messageType == 5 && message.packetType == Message::TC && message.serviceType
-	                                                                     == 19) {
-		uint16_t numberOfEventActionDefinitions = message.readUint16();
-		if (numberOfEventActionDefinitions != 0){
-			for (uint16_t i = 0; i < numberOfEventActionDefinitions; i++) {
-				uint16_t applicationID = message.readEnum16();
-				uint16_t eventDefinitionID = message.readEnum16();
-				for (uint16_t index = 0; index < ECSS_EVENT_ACTION_STRUCT_ARRAY_SIZE; index++) {
-					if (eventActionDefinitionArray[index].applicationId == applicationID &&
-					    eventActionDefinitionArray[index].eventDefinitionID == eventDefinitionID) {
-						eventActionDefinitionArray[index].enabled = false;
-					}
-				}
-			}
-		} else {
+	message.assertTC(19, 5);
+
+	uint16_t numberOfEventActionDefinitions = message.readUint16();
+	if (numberOfEventActionDefinitions != 0){
+		for (uint16_t i = 0; i < numberOfEventActionDefinitions; i++) {
+			uint16_t applicationID = message.readEnum16();
+			uint16_t eventDefinitionID = message.readEnum16();
 			for (uint16_t index = 0; index < ECSS_EVENT_ACTION_STRUCT_ARRAY_SIZE; index++) {
-				if (not eventActionDefinitionArray[index].empty){
+				if (eventActionDefinitionArray[index].applicationId == applicationID &&
+				    eventActionDefinitionArray[index].eventDefinitionID == eventDefinitionID) {
 					eventActionDefinitionArray[index].enabled = false;
 				}
 			}
 		}
+	} else {
+		for (uint16_t index = 0; index < ECSS_EVENT_ACTION_STRUCT_ARRAY_SIZE; index++) {
+			if (not eventActionDefinitionArray[index].empty){
+				eventActionDefinitionArray[index].enabled = false;
+			}
+		}
 	}
 }
 
 void EventActionService::requestEventActionDefinitionStatus(Message message) {
 	// TC[19,6]
-	if (message.messageType == 6 && message.packetType == Message::TC && message.serviceType
-	                                                                     == 19) {
-		eventActionStatusReport();
-	}
+	message.assertTC(19, 6);
+
+	eventActionStatusReport();
 }
 
 void EventActionService::eventActionStatusReport() {
@@ -171,18 +162,16 @@ void EventActionService::eventActionStatusReport() {
 
 void EventActionService::enableEventActionFunction(Message message) {
 	// TC[19,8]
-	if (message.messageType == 8 && message.packetType == Message::TC && message.serviceType
-	                                                                     == 19) {
-		setEventActionFunctionStatus(true);
-	}
+	message.assertTC(19, 8);
+
+	setEventActionFunctionStatus(true);
 }
 
 void EventActionService::disableEventActionFunction(Message message) {
 	// TC[19,9]
-	if (message.messageType == 9 && message.packetType == Message::TC && message.serviceType
-	                                                                     == 19) {
-		setEventActionFunctionStatus(false);
-	}
+	message.assertTC(19, 9);
+
+	setEventActionFunctionStatus(false);
 }
 
 // TODO: Should I use applicationID too?
diff --git a/src/Services/EventReportService.cpp b/src/Services/EventReportService.cpp
index b86a017c..f537f329 100644
--- a/src/Services/EventReportService.cpp
+++ b/src/Services/EventReportService.cpp
@@ -71,51 +71,49 @@ EventReportService::highSeverityAnomalyReport(Event eventID, const String<64> &
 
 void EventReportService::enableReportGeneration(Message message) {
 	// TC[5,5]
-	if (message.serviceType == 5 && message.packetType == Message::TC && message.messageType == 5) {
-		/**
-		* @todo: Report an error if length > numberOfEvents
-		*/
-		uint16_t length = message.readUint16();
-		Event eventID[length];
+	message.assertTC(5, 5);
+
+	/**
+	* @todo: Report an error if length > numberOfEvents
+	*/
+	uint16_t length = message.readUint16();
+	Event eventID[length];
+	for (uint16_t i = 0; i < length; i++) {
+		eventID[i] = static_cast<Event >(message.readEnum16());
+	}
+	if (length <= numberOfEvents) {
 		for (uint16_t i = 0; i < length; i++) {
-			eventID[i] = static_cast<Event >(message.readEnum16());
+			stateOfEvents[static_cast<uint16_t> (eventID[i])] = true;
 		}
-		if (length <= numberOfEvents) {
-			for (uint16_t i = 0; i < length; i++) {
-				stateOfEvents[static_cast<uint16_t> (eventID[i])] = true;
-			}
-		}
-		disabledEventsCount = stateOfEvents.size() - stateOfEvents.count();
 	}
+	disabledEventsCount = stateOfEvents.size() - stateOfEvents.count();
 }
 
 void EventReportService::disableReportGeneration(Message message) {
 	// TC[5,6]
-	if (message.serviceType == 5 && message.packetType == Message::TC && message.messageType
-	                                                                     == 6) {
-		/**
-		* @todo: Report an error if length > numberOfEvents
-		*/
-		uint16_t length = message.readUint16();
-		Event eventID[length];
+	message.assertTC(5, 6);
+
+	/**
+	* @todo: Report an error if length > numberOfEvents
+	*/
+	uint16_t length = message.readUint16();
+	Event eventID[length];
+	for (uint16_t i = 0; i < length; i++) {
+		eventID[i] = static_cast<Event >(message.readEnum16());
+	}
+	if (length <= numberOfEvents) {
 		for (uint16_t i = 0; i < length; i++) {
-			eventID[i] = static_cast<Event >(message.readEnum16());
-		}
-		if (length <= numberOfEvents) {
-			for (uint16_t i = 0; i < length; i++) {
-				stateOfEvents[static_cast<uint16_t> (eventID[i])] = false;
-			}
+			stateOfEvents[static_cast<uint16_t> (eventID[i])] = false;
 		}
-		disabledEventsCount = stateOfEvents.size() - stateOfEvents.count();
 	}
+	disabledEventsCount = stateOfEvents.size() - stateOfEvents.count();
 }
 
 void EventReportService::requestListOfDisabledEvents(Message message) {
 	// TC[5,7]
-	// I think this is all that is needed here.
-	if (message.serviceType == 5 && message.packetType == Message::TC && message.messageType == 7) {
-		listOfDisabledEventsReport();
-	}
+	message.assertTC(5, 7);
+
+	listOfDisabledEventsReport();
 }
 
 void EventReportService::listOfDisabledEventsReport() {
diff --git a/src/Services/MemoryManagementService.cpp b/src/Services/MemoryManagementService.cpp
index ebb58a11..c29c8fc1 100644
--- a/src/Services/MemoryManagementService.cpp
+++ b/src/Services/MemoryManagementService.cpp
@@ -23,8 +23,7 @@ void MemoryManagementService::RawDataMemoryManagement::loadRawData(Message &requ
 	 * @todo Add failure reporting
 	 */
 	// Check if we have the correct packet
-	assert(request.serviceType == 6);
-	assert(request.messageType == 2);
+	request.assertTC(6, 2);
 
 	// Read the memory ID from the request
 	auto memoryID = MemoryManagementService::MemoryID(request.readEnum8());
@@ -74,8 +73,7 @@ void MemoryManagementService::RawDataMemoryManagement::loadRawData(Message &requ
 
 void MemoryManagementService::RawDataMemoryManagement::dumpRawData(Message &request) {
 	// Check if we have the correct packet
-	assert(request.serviceType == 6);
-	assert(request.messageType == 5);
+	request.assertTC(6, 5);
 
 	// Create the report message object of telemetry message subtype 6
 	Message report = mainService.createTM(6);
@@ -124,8 +122,7 @@ void MemoryManagementService::RawDataMemoryManagement::dumpRawData(Message &requ
 
 void MemoryManagementService::RawDataMemoryManagement::checkRawData(Message &request) {
 	// Check if we have the correct packet
-	assert(request.serviceType == 6);
-	assert(request.messageType == 9);
+	request.assertTC(6, 9);
 
 	// Create the report message object of telemetry message subtype 10
 	Message report = mainService.createTM(10);
diff --git a/src/Services/ParameterService.cpp b/src/Services/ParameterService.cpp
index dd0f4c74..b32fa5b5 100644
--- a/src/Services/ParameterService.cpp
+++ b/src/Services/ParameterService.cpp
@@ -35,6 +35,7 @@ ParameterService::ParameterService() {
 }
 
 void ParameterService::reportParameterIds(Message& paramIds) {
+	paramIds.assertTC(20, 1);
 	Message reqParam(20, 2, Message::TM, 1);    // empty TM[20, 2] parameter report message
 
 	paramIds.resetRead();  // since we're passing a reference, the reading position shall be reset
@@ -69,6 +70,7 @@ void ParameterService::reportParameterIds(Message& paramIds) {
 }
 
 void ParameterService::setParameterIds(Message& newParamValues) {
+	newParamValues.assertTC(20, 3);
 
 	// assertion: correct message, packet and service type (at failure throws an
 	// InternalError::UnacceptablePacket which gets logged)
diff --git a/src/Services/RequestVerificationService.cpp b/src/Services/RequestVerificationService.cpp
index 768971da..465b1c56 100644
--- a/src/Services/RequestVerificationService.cpp
+++ b/src/Services/RequestVerificationService.cpp
@@ -152,7 +152,8 @@ RequestVerificationService::failRoutingVerification(const Message &request,
 	storeMessage(report);
 }
 
-
+// TODO: This function should not be here. These are TM messages, but `execute` should accept a
+// TC message.
 void RequestVerificationService::execute(const Message &message) {
 	switch (message.messageType) {
 		case 1:
diff --git a/src/Services/TestService.cpp b/src/Services/TestService.cpp
index 2584c75c..224c2614 100644
--- a/src/Services/TestService.cpp
+++ b/src/Services/TestService.cpp
@@ -1,6 +1,7 @@
 #include "Services/TestService.hpp"
 
 void TestService::areYouAlive(Message &request) {
+	request.assertTC(17, 1);
 	// TM[17,2] are-you-alive connection test report
 	Message report = createTM(2);
 
@@ -8,6 +9,7 @@ void TestService::areYouAlive(Message &request) {
 }
 
 void TestService::onBoardConnection(Message &request) {
+	request.assertTC(17, 3);
 	// TM[17,4] on-board connection test report
 	Message report = createTM(4);
 
diff --git a/src/Services/TimeManagementService.cpp b/src/Services/TimeManagementService.cpp
index 70c89dba..c6f92295 100644
--- a/src/Services/TimeManagementService.cpp
+++ b/src/Services/TimeManagementService.cpp
@@ -14,7 +14,8 @@ void TimeManagementService::cdsTimeReport(TimeAndDate &TimeInfo) {
 }
 
 TimeAndDate TimeManagementService::cdsTimeRequest(Message &message) {
-	// TC{9,128] CDS time request
+	// TC[9,128] CDS time request
+	message.assertTC(9, 128);
 
 	// check if we have the correct size of the data. The size should be 6 (48 bits)
 	ErrorHandler::assertRequest(message.dataSize == 6, message,
-- 
GitLab