From b143a37745ea8a0d1c8d4b131e9d2dc33aa6e330 Mon Sep 17 00:00:00 2001
From: Grigoris Pavlakis <grigpavl@ece.auth.gr>
Date: Sat, 17 Aug 2019 22:57:49 +0300
Subject: [PATCH] Finish revamping parameter report function

---
 inc/Services/ParameterService.hpp  | 15 +++++----------
 src/Services/ParameterService.cpp  | 11 ++++++-----
 test/Services/ParameterService.cpp |  2 +-
 3 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/inc/Services/ParameterService.hpp b/inc/Services/ParameterService.hpp
index 8725c17a..747703d0 100644
--- a/inc/Services/ParameterService.hpp
+++ b/inc/Services/ParameterService.hpp
@@ -35,7 +35,6 @@
 class ParameterService : public Service {
 private:
 	etl::map<ParamId, Parameter, MAX_PARAMS> paramsList;
-	uint16_t numOfValidIds(Message idMsg); // count the valid ids in a given TC[20, 1]
 
 public:
 	/**
@@ -55,19 +54,15 @@ public:
 	 * containing the current configuration
 	 * **for the parameters specified in the carried valid IDs**.
 	 *
-	 * No sophisticated error checking for now, just whether the packet is of the correct type
-	 * and whether the requested IDs are valid, ignoring the invalid ones.
-	 * If the packet has an incorrect header, an InternalError::UnacceptablePacket is raised.
+	 * The packet is checked for errors in service and message type, as well as for the
+	 * validity of the IDs contained. For every invalid ID an ExecutionStartErrorType::UnknownExecutionStartError
+	 * is raised.
+	 * If the packet has an incorrect header and service type, an InternalError::UnacceptableMessage is raised.
 	 * If no IDs are correct, the returned message shall be empty.
 	 *
-	 * @param paramId: a valid TC[20, 1] packet carrying the requested parameter IDs
+	 * @param paramId: a TC[20, 1] packet carrying the requested parameter IDs
 	 * @return None (messages are stored using storeMessage())
 	 *
-	 *
-	 * NOTES:
-	 * Method for valid ID counting is a hack (clones the message and figures out the number
-	 * separately, due to message access being non-random). Should be enough for now.
-	 *
 	 * Everything apart from the setting data is uint16 (setting data are uint32 for now)
 	 */
 	void reportParameterIds(Message& paramIds);
diff --git a/src/Services/ParameterService.cpp b/src/Services/ParameterService.cpp
index 00edb304..783b41ba 100644
--- a/src/Services/ParameterService.cpp
+++ b/src/Services/ParameterService.cpp
@@ -14,7 +14,7 @@ bool ParameterService::addNewParameter(uint8_t ptc, uint8_t pfc, uint32_t initia
 		paramsList.insert(std::make_pair(paramsList.size(), param));
 		return true;
 	}
-	catch(etl::map_full) {
+	catch (etl::map_full) {
 		return false;
 	}
 }
@@ -38,13 +38,14 @@ void ParameterService::reportParameterIds(Message& paramIds) {
 	ErrorHandler::assertRequest(paramIds.serviceType == 20, paramIds,
 	                            ErrorHandler::AcceptanceErrorType::UnacceptableMessage);
 
-	uint16_t numOfIds = paramIds.readUint16();  // number of parameter IDs carried in the message
-	uint16_t validIds = 0;
+	uint16_t numOfIds = paramIds.readUint16();  // total number of parameter IDs carried in the message
+	uint16_t validIds = 0;                      // number of valid IDs
 
 	for (uint16_t i = 0; i < numOfIds; i++) {
 		uint16_t currId = paramIds.readUint16();
 		try {
 			std::pair<ValueType, uint16_t> p = std::make_pair(i, paramsList.at(currId).getCurrentValue());
+			// pair containing the parameter's ID as first element and its current value as second
 			validParams.push_back(p);
 			validIds++;
 		}
@@ -61,7 +62,7 @@ void ParameterService::reportParameterIds(Message& paramIds) {
 		reqParam.appendUint32(i.second); // and its value
 	}
 
-	storeMessage(reqParam);
+	storeMessage(reqParam);  // then store the message
 }
 
 void ParameterService::setParameterIds(Message& newParamValues) {
@@ -104,4 +105,4 @@ void ParameterService::execute(Message& message) {
 		default:
 			ErrorHandler::reportInternalError(ErrorHandler::OtherMessageType);
 	}
-}
+}
\ No newline at end of file
diff --git a/test/Services/ParameterService.cpp b/test/Services/ParameterService.cpp
index 3a958be2..9f693718 100644
--- a/test/Services/ParameterService.cpp
+++ b/test/Services/ParameterService.cpp
@@ -11,7 +11,7 @@ void foo(ValueType* bar) {  // sample function
 
 /* test ideas:
 * parameter setting while flag is active
-*
+* requesting only invalid parameter IDs
 *
 */
 
-- 
GitLab