From ab8fcc6b1d5366d196d4794b890a26bb500b1c2b Mon Sep 17 00:00:00 2001
From: Grigoris Pavlakis <grigpavl@ece.auth.gr>
Date: Mon, 12 Aug 2019 00:15:58 +0300
Subject: [PATCH] Move the Parameter class implementation to its own file

Need to fix linker errors. Also updated documentation.
---
 inc/Services/Parameter.hpp        | 84 ++++++++++++++++++++-----------
 inc/Services/ParameterService.hpp |  2 +-
 src/Services/Parameter.cpp        | 27 ++++++++++
 src/Services/ParameterService.cpp | 11 ++--
 4 files changed, 87 insertions(+), 37 deletions(-)
 create mode 100644 src/Services/Parameter.cpp

diff --git a/inc/Services/Parameter.hpp b/inc/Services/Parameter.hpp
index b7d584d3..99bb6f15 100644
--- a/inc/Services/Parameter.hpp
+++ b/inc/Services/Parameter.hpp
@@ -7,44 +7,68 @@
 #define NUM_OF_FLAGS 5
 
 /**
- * Generic parameter structure
- * PTC and PFC for each parameter shall be specified as in
- * ECSS-E-ST-70-41C, chapter 7.3
+ * Implementation of a Parameter field, as specified in ECSS-E-ST-70-41C.
+ * Fully compliant with the standards requirements, while adding some small,
+ * but useful extensions to its contents.
+ *
+ * @author Grigoris Pavlakis <grigpavl@ece.auth.gr>
  */
 
-typedef uint16_t ParamId;             // parameter IDs are given sequentially
-typedef void(*UpdatePtr)(uint32_t*);  // pointer to the update function of this parameter
-// (argument is a pointer to the variable where the value will be returned, in
-// this case currentValue)
+/**
+ * Useful type definitions
+ *
+ * @typedef ParamId: the unique ID of a parameter, used for searching
+ * @typedef ValueType: the type of the parameter's value (changing types is WIP)
+ * @typedef UpdatePtr: pointer to a void function, with a single ValueType* argument (return address)
+ */
+typedef uint16_t ParamId;
 typedef uint32_t ValueType;
+typedef void(*UpdatePtr)(ValueType*);
 
+/**
+ * Parameter class - Breakdown of fields
+ *
+ * @private ptc: The Packet field type code (PTC) as defined in ECSS-E-ST-70-41C, chapter 7.3.
+ * @private pfc: The Packet field format code (PfC) as defined in the same standard
+ * @private ptr: Pointer of the function that will update the parameter
+ * @private currentValue: The current (as in last good) value of the parameter
+ *
+ * @todo: Find a way to store arbitrary types in currentValue
+ *
+ * Additional features (not included in standard):
+ * @private flags: Various binary flags (number and meaning TBD). Ideas:
+ * update with priority, do not poll, do not allow manual manipulation etc.
+ *
+ *
+ * Methods:
+ * @public Parameter(): default constructor, do not use.
+ * @public Parameter(uint8_t newPtc, uint8_t newPfc, uint32_t initialValue = 0, UpdatePtr newPtr = nullptr):
+ * Create a new Parameter object with newPtc PTC, newPfc PFC, initialValue as its starting value and newPtr
+ * as its update function pointer. Arguments initialValue and newPtr are optional, and have default values of
+ * 0 and nullptr respectively.
+ *
+ * @public setCurrentValue(): Changes the current value of the parameter (todo: if the respective flag is not set)
+ * @public getCurrentValue(): Gets the current value of the parameter
+ * @public getPTC(), getPFC(): Returns the PFC and PTC of the parameter
+ */
 class Parameter {
-	uint8_t ptc;                                        // Packet field type code (PTC)
-	uint8_t pfc;                                        // Packet field format code (PFC)
-	UpdatePtr ptr;                                      // Function pointer used for updates
+	uint8_t ptc;
+	uint8_t pfc;
+	UpdatePtr ptr;
 	etl::bitset<NUM_OF_FLAGS> flags = {false};
-	// Various flags (TBD which. Ideas: update with priority, do not poll, etc.)
+	ValueType currentValue = 0;
 
 	public:
-		ValueType currentValue = 0; // Last good value of the parameter. TODO: Find a way to store arbitrary types
-
-		Parameter() {
-			ptc = 0;
-			pfc = 0;
-			ptr = nullptr;
-		}
-		Parameter(uint8_t new_ptc, uint8_t new_pfc, uint32_t initialValue = 0, UpdatePtr new_ptr = nullptr) {
-			ptc = new_ptc;
-			pfc = new_pfc;
-			ptr = new_ptr;
-
-			if (ptr != nullptr) {
-				(*ptr)(&currentValue);  // call the update function for the initial value
-			}
-			else {
-				currentValue = initialValue;
-			}
-		}
+		Parameter();
+		Parameter(uint8_t newPtc, uint8_t newPfc, uint32_t initialValue = 0, UpdatePtr newPtr = nullptr);
+
+		void setCurrentValue(ValueType newVal);
+		//void setFlag();
+
+		ValueType getCurrentValue();
+		uint8_t getPTC();
+		uint8_t getPFC();
+
 };
 
 
diff --git a/inc/Services/ParameterService.hpp b/inc/Services/ParameterService.hpp
index 83e0b61e..19b3d89f 100644
--- a/inc/Services/ParameterService.hpp
+++ b/inc/Services/ParameterService.hpp
@@ -41,7 +41,7 @@ public:
 	 * Adds a new parameter. If the parameter has not been added (either because the map is full or because it already
 	 * exists in it) then returns false.
 	 */
-	bool addNewParameter(uint8_t ptc, uint8_t pfc, uint32_t initial_value = 0, UpdatePtr ptr = nullptr);
+	bool addNewParameter(uint8_t ptc, uint8_t pfc, uint32_t initialValue = 0, UpdatePtr ptr = nullptr);
 
 	/**
 	 * This function receives a TC[20, 1] packet and returns a TM[20, 2] packet
diff --git a/src/Services/Parameter.cpp b/src/Services/Parameter.cpp
new file mode 100644
index 00000000..4c15b2f7
--- /dev/null
+++ b/src/Services/Parameter.cpp
@@ -0,0 +1,27 @@
+#include "Services/Parameter.hpp"
+
+Parameter::Parameter() {
+	ptc = 0;
+	pfc = 0;
+	ptr = nullptr;
+}
+
+Parameter::Parameter(uint8_t newPtc, uint8_t newPfc, uint32_t initialValue, UpdatePtr newPtr) {
+	ptc = newPtc;
+	pfc = newPfc;
+	ptr = newPtr;
+
+	if (ptr != nullptr) {
+		(*ptr)(&currentValue);  // call the update function for the initial value
+	} else {
+		currentValue = initialValue;
+	}
+}
+
+void Parameter::setCurrentValue(ValueType newVal) {
+	currentValue = newVal;
+}
+
+ValueType Parameter::getCurrentValue() {
+	return currentValue;
+}
\ No newline at end of file
diff --git a/src/Services/ParameterService.cpp b/src/Services/ParameterService.cpp
index e09b1a07..762af1bb 100644
--- a/src/Services/ParameterService.cpp
+++ b/src/Services/ParameterService.cpp
@@ -1,6 +1,5 @@
 #include "Services/ParameterService.hpp"
-
-
+#include "Services/Parameter.hpp"
 ParameterService::ParameterService() {
 
 	addNewParameter(3, 14);
@@ -33,8 +32,8 @@ ParameterService::ParameterService() {
 //#endif
 }
 
-bool ParameterService::addNewParameter(uint8_t ptc, uint8_t pfc, uint32_t initial_value, UpdatePtr ptr) {
-	Parameter param = Parameter(ptc, pfc, initial_value, ptr);
+bool ParameterService::addNewParameter(uint8_t ptc, uint8_t pfc, uint32_t initialValue, UpdatePtr ptr) {
+	Parameter param = Parameter(ptc, pfc, initialValue, ptr);
 	return paramsList.insert(std::make_pair(paramsList.size() + 1, param)).second;
 	// second element of the returned std::pair is whether the given item was inserted or not
 }
@@ -63,7 +62,7 @@ void ParameterService::reportParameterIds(Message& paramIds) {
 
 		if (paramsList.find(currId) != paramsList.end()) {
 			reqParam.appendUint16(currId);
-			reqParam.appendUint32(paramsList[currId].currentValue);
+			reqParam.appendUint32(paramsList[currId].getCurrentValue());
 		} else {
 			ErrorHandler::reportError(paramIds, ErrorHandler::ExecutionStartErrorType::UnknownExecutionStartError);
 			continue; // generate failed start of execution notification & ignore
@@ -92,7 +91,7 @@ void ParameterService::setParameterIds(Message& newParamValues) {
 		uint16_t currId = newParamValues.readUint16();
 
 		if (paramsList.find(currId) != paramsList.end()) {
-			paramsList[currId].currentValue = newParamValues.readUint32(); // TODO: add a check here with the new
+			paramsList[currId].setCurrentValue(newParamValues.readUint32()); // TODO: add a check here with the new
 			// flag functionality
 		} else {
 			ErrorHandler::reportError(newParamValues,
-- 
GitLab