From f1225266471dfc86216c5832fa0483c24a741843 Mon Sep 17 00:00:00 2001
From: kongr45gpen <electrovesta@gmail.com>
Date: Fri, 9 Aug 2019 14:44:02 +0300
Subject: [PATCH] Make the function names and documentation of the
 MessageParser clearer

---
 inc/MessageParser.hpp                        | 41 ++++++++++++++------
 inc/Services/LargePacketTransferService.hpp  |  2 +-
 src/MessageParser.cpp                        | 18 ++++-----
 src/Platform/x86/main.cpp                    |  4 +-
 src/Services/EventActionService.cpp          |  2 +-
 src/Services/TimeBasedSchedulingService.cpp  |  6 +--
 test/Services/TimeBasedSchedulingService.cpp | 12 +++---
 7 files changed, 52 insertions(+), 33 deletions(-)

diff --git a/inc/MessageParser.hpp b/inc/MessageParser.hpp
index b3b1f0de..e7ebe863 100644
--- a/inc/MessageParser.hpp
+++ b/inc/MessageParser.hpp
@@ -7,12 +7,33 @@
 /**
  * A generic class responsible for the execution and the parsing of the incoming telemetry and telecommand
  * packets
+ *
+ * This class is responsible for converting Packets and Messages to and from the internal representation used in this
+ * project. The following hierarchy is used between the different layers on ecss-services:
+ *
+ * \code
+ *                                                       -------------------
+ *                                                       | User data field |
+ *                                                       -------------------
+ *                            ---------------------------                            Application Layer
+ *                            | Packet secondary header |
+ *                            |      (ECSS header)      |
+ *                            ---------------------------                    --------------------------------
+ *  -------------------------
+ *  | Packet primary header |
+ *  |     (CCSDS header)    |                                                          Network Layer
+ *  -------------------------
+ * \endcode
+ *
+ * The service data is encapsulated within the, **ECSS packet** which is encapsulated within the **CCSDS packet**.
+ * The MessageParser class is responsible for adding and processing both the ECSS and CCSDS headers. The target it uses
+ * for the internal representation of all received Telemetry (TM) and Telecommands (TC) is the \ref Message class.
  */
 
 class MessageParser {
 public:
 	/**
-     * This function takes as input TC packets and and calls the proper services' functions that have been
+     * This function takes as input TC packets and calls the proper services' functions that have been
 	 * implemented to handle TC packets.
 	 *
 	 * @param Message Contains the necessary parameters to call the suitable subservice
@@ -33,31 +54,29 @@ public:
 	Message parse(uint8_t* data, uint32_t length);
 
 	/**
-	 * @todo: elaborate on this comment
-	 * Create a message so that a string can be parsed
+	 * Parse data that contains the ECSS packet header, without the CCSDS space packet header
 	 *
 	 * Note: conversion of char* to unsigned char* should flow without any problems according to
 	 * this great analysis:
 	 * stackoverflow.com/questions/15078638/can-i-turn-unsigned-char-into-char-and-vice-versa
 	 */
-	Message parseRequestTC(String<ECSS_TC_REQUEST_STRING_SIZE> data);
+	Message parseECSSTC(String<ECSS_TC_REQUEST_STRING_SIZE> data);
 
 	/**
-	 * @brief Overloaded version
+	 * @brief Overloaded version of \ref MessageParser::parseECSSTC(String<ECSS_TC_REQUEST_STRING_SIZE> data)
 	 * @param data A uint8_t array of the TC packet data
 	 * @return Parsed message
 	 */
-	Message parseRequestTC(uint8_t* data);
+	Message parseECSSTC(uint8_t* data);
 
 	/**
-	 * @brief Converts a TC packet of type Message to a String
-	 * @details Convert a parsed TC message to a string in order to be used by the services
+	 * @brief Converts a TC message to a string, appending just the ECSS header
 	 * @param message The Message object to be parsed to a String
 	 * @return A String class containing the parsed TC request
 	 * @attention The returned String has a fixed size, therefore the message size is considered
 	 * fixed and equal to the ECSS_TC_REQUEST_STRING_SIZE definition.
 	 */
-	String<ECSS_TC_REQUEST_STRING_SIZE> convertTCToStr(Message& message);
+	String<ECSS_TC_REQUEST_STRING_SIZE> createECSSTC(Message& message);
 
 private:
 	/**
@@ -69,7 +88,7 @@ private:
 	 * @param length The size of the header
 	 * @param message The Message to modify based on the header
 	 */
-	void parseTC(const uint8_t* data, uint16_t length, Message& message);
+	void parseECSSTCHeader(const uint8_t* data, uint16_t length, Message& message);
 
 	/**
 	 * Parse the ECSS Telemetry packet secondary header
@@ -80,7 +99,7 @@ private:
 	 * @param length The size of the header
 	 * @param message The Message to modify based on the header
 	 */
-	void parseTM(const uint8_t* data, uint16_t length, Message& message);
+	void parseECSSTMHeader(const uint8_t* data, uint16_t length, Message& message);
 };
 
 #endif // ECSS_SERVICES_MESSAGEPARSER_HPP
diff --git a/inc/Services/LargePacketTransferService.hpp b/inc/Services/LargePacketTransferService.hpp
index e3219472..533a3ba0 100644
--- a/inc/Services/LargePacketTransferService.hpp
+++ b/inc/Services/LargePacketTransferService.hpp
@@ -49,7 +49,7 @@ public:
 	                            const String<ECSS_MAX_FIXED_OCTET_STRING_SIZE>& string);
 
 	// The three uplink functions should handle a TC request to "upload" data. Since there is not
-	// a createTC function ready, I just return the given string.
+	// a createECSSTC function ready, I just return the given string.
 	// @TODO: Modify these functions properly
 	/**
 	 * Function that handles the first part of the uplink request
diff --git a/src/MessageParser.cpp b/src/MessageParser.cpp
index 656e0756..b300816b 100644
--- a/src/MessageParser.cpp
+++ b/src/MessageParser.cpp
@@ -60,15 +60,15 @@ Message MessageParser::parse(uint8_t* data, uint32_t length) {
 	Message message(0, 0, packetType, APID);
 
 	if (packetType == Message::TC) {
-		parseTC(data + 6, packetDataLength, message);
+		parseECSSTCHeader(data + 6, packetDataLength, message);
 	} else {
-		parseTM(data + 6, packetDataLength, message);
+		parseECSSTMHeader(data + 6, packetDataLength, message);
 	}
 
 	return message;
 }
 
-void MessageParser::parseTC(const uint8_t* data, uint16_t length, Message& message) {
+void MessageParser::parseECSSTCHeader(const uint8_t* data, uint16_t length, Message& message) {
 	ErrorHandler::assertRequest(length >= 5, message, ErrorHandler::UnacceptableMessage);
 
 	// Individual fields of the TC header
@@ -91,22 +91,22 @@ void MessageParser::parseTC(const uint8_t* data, uint16_t length, Message& messa
 	message.dataSize = length;
 }
 
-Message MessageParser::parseRequestTC(String<ECSS_TC_REQUEST_STRING_SIZE> data) {
+Message MessageParser::parseECSSTC(String<ECSS_TC_REQUEST_STRING_SIZE> data) {
 	Message message;
 	auto* dataInt = reinterpret_cast<uint8_t*>(data.data());
 	message.packetType = Message::TC;
-	parseTC(dataInt, ECSS_TC_REQUEST_STRING_SIZE, message);
+	parseECSSTCHeader(dataInt, ECSS_TC_REQUEST_STRING_SIZE, message);
 	return message;
 }
 
-Message MessageParser::parseRequestTC(uint8_t* data) {
+Message MessageParser::parseECSSTC(uint8_t* data) {
 	Message message;
 	message.packetType = Message::TC;
-	parseTC(data, ECSS_TC_REQUEST_STRING_SIZE, message);
+	parseECSSTCHeader(data, ECSS_TC_REQUEST_STRING_SIZE, message);
 	return message;
 }
 
-String<ECSS_TC_REQUEST_STRING_SIZE> MessageParser::convertTCToStr(Message& message) {
+String<ECSS_TC_REQUEST_STRING_SIZE> MessageParser::createECSSTC(Message& message) {
 	uint8_t tempString[ECSS_TC_REQUEST_STRING_SIZE] = {0};
 
 	tempString[0] = ECSS_PUS_VERSION << 4; // Assign the pusVersion = 2
@@ -118,7 +118,7 @@ String<ECSS_TC_REQUEST_STRING_SIZE> MessageParser::convertTCToStr(Message& messa
 	return dataString;
 }
 
-void MessageParser::parseTM(const uint8_t* data, uint16_t length, Message& message) {
+void MessageParser::parseECSSTMHeader(const uint8_t* data, uint16_t length, Message& message) {
 	ErrorHandler::assertRequest(length >= 5, message, ErrorHandler::UnacceptableMessage);
 
 	// Individual fields of the TM header
diff --git a/src/Platform/x86/main.cpp b/src/Platform/x86/main.cpp
index 57ad74b6..8d69e83a 100644
--- a/src/Platform/x86/main.cpp
+++ b/src/Platform/x86/main.cpp
@@ -319,10 +319,10 @@ int main() {
 	receivedMsg.appendUint16(2); // Total number of requests
 
 	receivedMsg.appendUint32(currentTime + 1556435U);
-	receivedMsg.appendString(msgParser.convertTCToStr(testMessage1));
+	receivedMsg.appendString(msgParser.createECSSTC(testMessage1));
 
 	receivedMsg.appendUint32(currentTime + 1957232U);
-	receivedMsg.appendString(msgParser.convertTCToStr(testMessage2));
+	receivedMsg.appendString(msgParser.createECSSTC(testMessage2));
 	timeBasedSchedulingService.insertActivities(receivedMsg);
 
 	// Time shift activities
diff --git a/src/Services/EventActionService.cpp b/src/Services/EventActionService.cpp
index 9ed4f9b6..26e67d0d 100644
--- a/src/Services/EventActionService.cpp
+++ b/src/Services/EventActionService.cpp
@@ -181,7 +181,7 @@ void EventActionService::executeAction(uint16_t eventID) {
 		for (auto& element = range.first; element != range.second; ++element) {
 			if (element->second.enabled) {
 				MessageParser messageParser;
-				Message message = messageParser.parseRequestTC(element->second.request);
+				Message message = messageParser.parseECSSTC(element->second.request);
 				MessageParser::execute(message);
 			}
 		}
diff --git a/src/Services/TimeBasedSchedulingService.cpp b/src/Services/TimeBasedSchedulingService.cpp
index 170f9c93..ddd7e8ef 100644
--- a/src/Services/TimeBasedSchedulingService.cpp
+++ b/src/Services/TimeBasedSchedulingService.cpp
@@ -49,7 +49,7 @@ void TimeBasedSchedulingService::insertActivities(Message& request) {
 			// Get the TC packet request
 			uint8_t requestData[ECSS_TC_REQUEST_STRING_SIZE] = {0};
 			request.readString(requestData, ECSS_TC_REQUEST_STRING_SIZE);
-			Message receivedTCPacket = msgParser.parseRequestTC(requestData);
+			Message receivedTCPacket = msgParser.parseECSSTC(requestData);
 			ScheduledActivity newActivity; // Create the new activity
 
 			// Assign the attributes to the newly created activity
@@ -168,7 +168,7 @@ void TimeBasedSchedulingService::detailReportAllActivities(Message& request) {
 		// todo: append sub-schedule and group ID if they are defined
 
 		report.appendUint32(activity.requestReleaseTime);
-		report.appendString(msgParser.convertTCToStr(activity.request));
+		report.appendString(msgParser.createECSSTC(activity.request));
 	}
 	storeMessage(report); // Save the report
 }
@@ -209,7 +209,7 @@ void TimeBasedSchedulingService::detailReportActivitiesByID(Message& request) {
 	report.appendUint16(static_cast<uint16_t>(matchedActivities.size()));
 	for (auto& match : matchedActivities) {
 		report.appendUint32(match.requestReleaseTime); // todo: Replace with the time parser
-		report.appendString(msgParser.convertTCToStr(match.request));
+		report.appendString(msgParser.createECSSTC(match.request));
 	}
 	storeMessage(report); // Save the report
 }
diff --git a/test/Services/TimeBasedSchedulingService.cpp b/test/Services/TimeBasedSchedulingService.cpp
index b56a8bd3..5b72b171 100644
--- a/test/Services/TimeBasedSchedulingService.cpp
+++ b/test/Services/TimeBasedSchedulingService.cpp
@@ -71,19 +71,19 @@ auto activityInsertion(TimeBasedSchedulingService& timeService) {
 
 	// Test activity 1
 	receivedMessage.appendUint32(currentTime + 1556435);
-	receivedMessage.appendString(msgParser.convertTCToStr(testMessage1));
+	receivedMessage.appendString(msgParser.createECSSTC(testMessage1));
 
 	// Test activity 2
 	receivedMessage.appendUint32(currentTime + 1957232);
-	receivedMessage.appendString(msgParser.convertTCToStr(testMessage2));
+	receivedMessage.appendString(msgParser.createECSSTC(testMessage2));
 
 	// Test activity 3
 	receivedMessage.appendUint32(currentTime + 1726435);
-	receivedMessage.appendString(msgParser.convertTCToStr(testMessage3));
+	receivedMessage.appendString(msgParser.createECSSTC(testMessage3));
 
 	// Test activity 4
 	receivedMessage.appendUint32(currentTime + 17248435);
-	receivedMessage.appendString(msgParser.convertTCToStr(testMessage4));
+	receivedMessage.appendString(msgParser.createECSSTC(testMessage4));
 
 	// Insert activities in the schedule. They have to be inserted sorted
 	timeService.insertActivities(receivedMessage);
@@ -276,7 +276,7 @@ TEST_CASE("TC[11,9] Detail report scheduled activities by ID", "[service][st11]"
 			Message receivedTCPacket;
 			uint8_t receivedDataStr[ECSS_TC_REQUEST_STRING_SIZE];
 			response.readString(receivedDataStr, ECSS_TC_REQUEST_STRING_SIZE);
-			receivedTCPacket = msgParser.parseRequestTC(receivedDataStr);
+			receivedTCPacket = msgParser.parseECSSTC(receivedDataStr);
 
 			if (i == 0) {
 				REQUIRE(receivedReleaseTime == scheduledActivities.at(0)->requestReleaseTime);
@@ -380,7 +380,7 @@ TEST_CASE("TC[11,16] Detail report all scheduled activities", "[service][st11]")
 		Message receivedTCPacket;
 		uint8_t receivedDataStr[ECSS_TC_REQUEST_STRING_SIZE];
 		response.readString(receivedDataStr, ECSS_TC_REQUEST_STRING_SIZE);
-		receivedTCPacket = msgParser.parseRequestTC(receivedDataStr);
+		receivedTCPacket = msgParser.parseECSSTC(receivedDataStr);
 
 		REQUIRE(receivedReleaseTime == scheduledActivities.at(i)->requestReleaseTime);
 		REQUIRE(receivedTCPacket.bytesEqualWith(scheduledActivities.at(i)->request));
-- 
GitLab