From 63510064fa1fe66ce36c3806184456029438011a Mon Sep 17 00:00:00 2001
From: Grigoris Pavlakis <grigpavl@ece.auth.gr>
Date: Tue, 20 Nov 2018 01:19:56 +0200
Subject: [PATCH] Fix a bug with determining the correct Parameter struct for a
 given ID

---
 inc/Services/ParameterService.hpp | 18 ++++++-----
 src/Services/ParameterService.cpp | 51 +++++++++++++++++--------------
 src/main.cpp                      |  4 +--
 3 files changed, 41 insertions(+), 32 deletions(-)

diff --git a/inc/Services/ParameterService.hpp b/inc/Services/ParameterService.hpp
index 1d4ea05d..5d8d5176 100644
--- a/inc/Services/ParameterService.hpp
+++ b/inc/Services/ParameterService.hpp
@@ -17,23 +17,27 @@
  */
 struct Parameter {
 
-	uint8_t ptc;            //Packet field type code (PTC)
-	uint8_t pfc;            //Packet field format code (PFC)
-	uint16_t paramId;       //Unique ID of the parameter
-
-	uint32_t settingData;   //Actual data defining the operation of a peripheral or subsystem.
-	//Peripheral-dependent normally (memory address according to spec). Dummy AF for now.
+	uint8_t ptc;            // Packet field type code (PTC)
+	uint8_t pfc;            // Packet field format code (PFC)
+	uint16_t paramId;       // Unique ID of the parameter
+
+	uint32_t settingData;
+	// Actual data defining the operation of a peripheral or subsystem.
+	// Peripheral-dependent normally (void* maybe?) (it's a memory address according to spec).
+	// Dummy int for now.
 };
 
 /**
  * Parameter manager
  * Holds the list with the parameters and provides functions
  * for parameter reporting and modification.
+ *
+ * @todo Ensure that the parameter list is sorted by ID
  */
 
 class ParameterService : public Service {
 
-	Parameter paramsList[CONFIGLENGTH];   //5 is just a dummy number
+	Parameter paramsList[CONFIGLENGTH];   // CONFIGLENGTH is just a dummy number for now, this should be statically set
 
 	public:
 		ParameterService();
diff --git a/src/Services/ParameterService.cpp b/src/Services/ParameterService.cpp
index 731301d9..08b775ff 100644
--- a/src/Services/ParameterService.cpp
+++ b/src/Services/ParameterService.cpp
@@ -1,14 +1,18 @@
 #include "Services/ParameterService.hpp"
+#define DEMOMODE
+
+#ifdef DEMOMODE
 #include <ctime>
+#endif
 
 ParameterService::ParameterService() {
-
+#ifdef DEMOMODE
 	/**
 	 * Initializes the parameter list with some dummy values for now.
 	 * This normally will be initialized with actual values on boot.
 	 */
 
-	for (int i = 0; i < 5; i++) {
+	for (int i = 0; i < 5; i++) {   // hack just to shut CLion up (warning about range-based for loops)
 
 		paramsList[i].paramId = 0;
 		paramsList[i].settingData = 0;
@@ -16,41 +20,42 @@ ParameterService::ParameterService() {
 		paramsList[i].ptc = 1;
 	}
 
-	//Test code, setting up one of the parameter fields
+	// Test code, setting up one of the parameter fields
 
-	time_t currTime = time(NULL);
+	time_t currTime = time(nullptr);
 	struct tm* today = localtime(&currTime);
 
-	paramsList[2].paramId = 10;  //random parameter ID
-	paramsList[2].settingData = today -> tm_year;   //the current year
-	paramsList[2].ptc = 3;  //unsigned int
-	paramsList[2].pfc = 14;  //32 bits
+	paramsList[2].paramId = 341;                   // random parameter ID
+	paramsList[2].settingData = today -> tm_min;   // the minute of the current hour
+	paramsList[2].ptc = 3;                         // unsigned int
+	paramsList[2].pfc = 14;                        // 32 bits
+#endif
 }
 
 Message ParameterService::reportParameter(Message paramId) {
 
 	/**
 	 * This function receives a TC[20, 1] packet and returns a TM[20, 2] packet
-	 * containing the current configuration.
+	 * containing the current configuration. No error checking for now, just whether
+	 * the package is of the correct type (in which case it returns an empty message)
 	 *
-	 * @param Message paramSpecifier: a valid TC[20, 1] packet carrying the requested parameter ID
+	 * @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
 	 */
 
-	Message reqParam(20, 2, Message::TM, 1);    //empty TM[20, 2] parameter report message
-	//TODO: Try to have the parameter list always sorted so binary search can be a thing
-
-	if (paramId.packetType == Message::TC) {
+	Message reqParam(20, 2, Message::TM, 1);    // empty TM[20, 2] parameter report message
+	uint16_t reqParamId = paramId.readHalfword(); // parameter ID must be accessed only once
 
-		if (paramId.serviceType == 20 && paramId.messageType == 1) {
+	if (paramId.packetType == Message::TC && paramId.serviceType == 20 && paramId.messageType == 1) {
 
-			//if TC is of the wrong type return an empty message for now
-			for (int i = 0; i < CONFIGLENGTH; i++) {    //5 is a dummy, as always
+		for (int i = 0; i < 5; i++) {
 
-				if (paramsList[i].paramId == *paramId.data) {
+			if (paramsList[i].paramId == reqParamId) {
 
-					reqParam.appendHalfword(paramsList[i].paramId);
-					reqParam.appendWord(paramsList[i].settingData);
-				}
+				reqParam.appendHalfword(paramsList[i].paramId);
+				reqParam.appendWord(paramsList[i].settingData);
+				break;
 			}
 		}
 	}
@@ -58,6 +63,6 @@ Message ParameterService::reportParameter(Message paramId) {
 	return reqParam;
 }
 
-void ParameterService::setParamData(Message paramId) {
+/*void ParameterService::setParamData(Message paramId) {
 
-}
\ No newline at end of file
+}*/
\ No newline at end of file
diff --git a/src/main.cpp b/src/main.cpp
index 81cd1382..62e5b60f 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -31,9 +31,9 @@ int main() {
 	//ST[20] test
 	ParameterService paramService;
 	Message sentPacket = Message(20, 1, Message::TC, 1);  //application id is a dummy number (1)
-	sentPacket.appendByte(10);  //the packet sent contains the ID of the desired parameter
+	sentPacket.appendHalfword(341);  //the packet sent contains the ID of the desired parameter
 	Message returnedPacket = paramService.reportParameter(sentPacket);
-	std::cout << "Year: " << *returnedPacket.data << std::endl;
+	std::cout << "Parameter ID: " << returnedPacket.readHalfword() << "Parameter value: " << returnedPacket.readHalfword() << std::endl;
 
 	return 0;
 }
-- 
GitLab