From 589fe725e76977d12a9b19274b513f87e8d8bb13 Mon Sep 17 00:00:00 2001
From: Grigoris Pavlakis <grigpavl@ece.auth.gr>
Date: Wed, 21 Nov 2018 00:41:36 +0200
Subject: [PATCH] Fix parameter report to use multiple IDs for the same message
 (todo: write unit tests for both functions)

---
 inc/Services/ParameterService.hpp |  2 +-
 src/Services/ParameterService.cpp | 34 +++++++++++++++++++------------
 src/main.cpp                      | 20 ++++++++++++++----
 3 files changed, 38 insertions(+), 18 deletions(-)

diff --git a/inc/Services/ParameterService.hpp b/inc/Services/ParameterService.hpp
index 65f37e65..1c0cbbc3 100644
--- a/inc/Services/ParameterService.hpp
+++ b/inc/Services/ParameterService.hpp
@@ -45,7 +45,7 @@ class ParameterService : public Service {
 public:
 	ParameterService();
 
-	Message reportParameterId(Message paramId);
+	Message reportParameterIds(Message paramIds);
 
 	void setParamData(Message newParamValues);
 
diff --git a/src/Services/ParameterService.cpp b/src/Services/ParameterService.cpp
index c5f40d90..368dad2a 100644
--- a/src/Services/ParameterService.cpp
+++ b/src/Services/ParameterService.cpp
@@ -20,44 +20,52 @@ ParameterService::ParameterService() {
 	time_t currTime = time(nullptr);
 	struct tm *today = localtime(&currTime);
 
-	paramsList[0].paramId = 341;                   // random parameter ID
+	paramsList[0].paramId = 0;                     // random parameter ID
 	paramsList[0].settingData = today->tm_hour;    // the current hour
 	paramsList[0].ptc = 3;                         // unsigned int
 	paramsList[0].pfc = 14;                        // 32 bits
 
-	paramsList[1].paramId = 345;                   // random parameter ID
+	paramsList[1].paramId = 1;                     // random parameter ID
 	paramsList[1].settingData = today->tm_min;     // the current minute
 	paramsList[1].ptc = 3;                         // unsigned int
 	paramsList[1].pfc = 14;                        // 32 bits
 #endif
 }
 
-Message ParameterService::reportParameterId(Message paramId) {
+Message ParameterService::reportParameterIds(Message paramIds) {
 
 	/**
 	 * This function receives a TC[20, 1] packet and returns a TM[20, 2] packet
-	 * containing the current configuration **for the parameter specified in the carried ID**.
+	 * containing the current configuration **for the parameters specified in the carried IDs**.
 	 * No sophisticated error checking for now, just whether the package is of the correct type
 	 * (in which case it returns an empty message)
 	 *
 	 * @param paramId: a valid TC[20, 1] packet carrying the requested parameter ID
 	 * @return A TM[20, 2] packet containing the parameter ID
-	 * @todo Implement binary search for the lookup in order to be faster
+	 * @todo Implement binary search for the lookup in order to be faster when the number of
+	 * params inevitably rises
+	 *
+	 * NOTE: Everything apart from the setting data is uint16 (setting data are uint32 for now)
 	 */
 
 	Message reqParam(20, 2, Message::TM, 1);    // empty TM[20, 2] parameter report message
-	uint16_t reqParamId = paramId.readUint16(); // parameter ID must be accessed only once
 
-	if (paramId.packetType == Message::TC && paramId.serviceType == 20 &&
-	    paramId.messageType == 1) {
+	if (paramIds.packetType == Message::TC && paramIds.serviceType == 20 &&
+	    paramIds.messageType == 1) {
 
-		for (int i = 0; i < CONFIGLENGTH; i++) {
+		uint16_t ids = paramIds.readUint16();        // first 16bits of the packet are # of IDs
 
-			if (paramsList[i].paramId == reqParamId) {
+		reqParam.appendUint16(ids);                  //include the number of contained IDs
 
-				reqParam.appendUint16(paramsList[i].paramId);  // first 16 bits are the parameter ID
-				reqParam.appendUint32(paramsList[i].settingData); // rest 32 are the current setting
-				break;
+		for (int i = 0; i < ids; i++) {
+
+			uint16_t currId = paramIds.readUint16();      // current ID to be appended
+
+			if (currId < CONFIGLENGTH) {  // check to prevent out-of-bounds access due to invalid id
+
+				reqParam.appendUint16(currId);      // append it to the new packet
+				reqParam.appendUint32(paramsList[currId].settingData);
+				// right after that append the settings
 			}
 		}
 	}
diff --git a/src/main.cpp b/src/main.cpp
index 0cb6e00a..1262e909 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -34,11 +34,23 @@ int main() {
 
 	//Test code for reportParameter
 	Message sentPacket = Message(20, 1, Message::TC, 1);  //application id is a dummy number (1)
-	sentPacket.appendUint16(341);  //the packet sent contains the ID of the desired parameter
-	Message returnedPacket = paramService.reportParameter(sentPacket);
+	sentPacket.appendUint16(2);  //number of contained IDs
+	sentPacket.appendUint16(0);  //first ID
+	sentPacket.appendUint16(1);  //second ID
+	Message returnedPacket = paramService.reportParameterIds(sentPacket);
 
-	std::cout << "Parameter ID: " << std::dec << returnedPacket.readUint16() << std::endl
-	          << "Parameter value: " << std::dec << returnedPacket.readUint32() << std::endl;
+	uint16_t numOfIds = returnedPacket.readUint16();
+
+	std::cout << std::endl << "Number of contained configs: " << numOfIds << std::endl;
+
+	for (int i = 0; i < numOfIds; i++) {
+
+		std::cout << "Parameter ID: " << std::dec << returnedPacket.readUint16() << std::endl
+		          << "Parameter value: " << std::dec << returnedPacket.readUint32() << std::endl;
+
+	}
+
+	std::cout << std::endl << "(First value is hours, second is minutes)" << std::endl;
 
 	return 0;
 }
-- 
GitLab