From 7f7c18bc007c2324728ce1cbb7e3d5ae314bb7e6 Mon Sep 17 00:00:00 2001
From: Grigoris Pavlakis <grigpavl@ece.auth.gr>
Date: Sun, 18 Aug 2019 20:33:14 +0300
Subject: [PATCH] Implement preliminary flag feature

Other changes:
Tweaked addNewParameter() in order for the IDs to be arbitrary (insertion
of already existing ID test incoming) according to suggestion by @kongr45gpen
---
 inc/Services/Parameter.hpp         | 17 +++++++------
 inc/Services/ParameterService.hpp  |  2 +-
 src/Services/Parameter.cpp         | 14 ++++++++---
 src/Services/ParameterService.cpp  |  9 +++----
 test/Services/ParameterService.cpp | 39 +++++++++++++++++++++---------
 5 files changed, 52 insertions(+), 29 deletions(-)

diff --git a/inc/Services/Parameter.hpp b/inc/Services/Parameter.hpp
index 36b94b43..a63d846b 100644
--- a/inc/Services/Parameter.hpp
+++ b/inc/Services/Parameter.hpp
@@ -4,7 +4,7 @@
 #include "etl/bitset.h"
 
 // Number of binary flags in every parameter. Final number TBD.
-#define NUM_OF_FLAGS 5
+#define NUM_OF_FLAGS 3
 
 /**
  * Implementation of a Parameter field, as specified in ECSS-E-ST-70-41C.
@@ -24,6 +24,7 @@
 typedef uint16_t ParamId;
 typedef uint32_t ValueType;
 typedef void(*UpdatePtr)(ValueType*);
+typedef etl::bitset<NUM_OF_FLAGS> Flags;
 
 /**
  * Parameter class - Breakdown of fields
@@ -36,18 +37,20 @@ typedef void(*UpdatePtr)(ValueType*);
  * @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.
+ * @private flags: Various binary flags (number and meaning TBD).
+ * Current flag meanings (starting from LSB, big-endian):
+ * Index 0: update with priority
+ * Index 1: manual update available
+ * Index 2: automatic update available
  *
  *
  * 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 setCurrentValue(): Changes the current value of the parameter
  * @public getCurrentValue(): Gets the current value of the parameter
  * @public getPTC(), getPFC(): Returns the PFC and PTC of the parameter
  */
@@ -55,14 +58,14 @@ class Parameter {
 	uint8_t ptc;
 	uint8_t pfc;
 	UpdatePtr ptr;
-	etl::bitset<NUM_OF_FLAGS> flags;
+	Flags flags;
 	ValueType currentValue = 0;
 
 	public:
 		Parameter(uint8_t newPtc, uint8_t newPfc, uint32_t initialValue = 0, UpdatePtr newPtr = nullptr);
 
 		void setCurrentValue(ValueType newVal);
-		void setFlag(etl::bitset<NUM_OF_FLAGS> flags);
+		void setFlag(const char* flags);
 
 		ValueType getCurrentValue();
 		uint8_t getPTC();
diff --git a/inc/Services/ParameterService.hpp b/inc/Services/ParameterService.hpp
index 747703d0..db4ed28d 100644
--- a/inc/Services/ParameterService.hpp
+++ b/inc/Services/ParameterService.hpp
@@ -47,7 +47,7 @@ public:
 	 * exists in it) then returns false.
 	 * The parameter IDs are given sequentially, starting from 0.
 	 */
-	bool addNewParameter(uint8_t ptc, uint8_t pfc, uint32_t initialValue = 0, UpdatePtr ptr = nullptr);
+	bool addNewParameter(uint8_t id, Parameter param, const char* flags = "110");
 
 	/**
 	 * 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
index 8bfab448..7c825324 100644
--- a/src/Services/Parameter.cpp
+++ b/src/Services/Parameter.cpp
@@ -1,10 +1,13 @@
 #include "Services/Parameter.hpp"
 
-Parameter::Parameter(uint8_t newPtc, uint8_t newPfc, uint32_t initialValue, UpdatePtr newPtr) {
+Parameter::Parameter(uint8_t newPtc, uint8_t newPfc, ValueType initialValue, UpdatePtr newPtr) {
 	ptc = newPtc;
 	pfc = newPfc;
 	ptr = newPtr;
 
+	// see Parameter.hpp for explanation on flags
+	// by default: no update priority, manual and automatic update available
+
 	if (ptr != nullptr) {
 		(*ptr)(&currentValue);  // call the update function for the initial value
 	} else {
@@ -13,7 +16,10 @@ Parameter::Parameter(uint8_t newPtc, uint8_t newPfc, uint32_t initialValue, Upda
 }
 
 void Parameter::setCurrentValue(ValueType newVal) {
-	currentValue = newVal;
+	// set the value only if the parameter can be updated manually
+	if (flags[2]) {
+		currentValue = newVal;
+	}
 }
 
 ValueType Parameter::getCurrentValue() {
@@ -28,6 +34,6 @@ uint8_t Parameter::getPFC() {
 	return pfc;
 }
 
-void Parameter::setFlag(etl::bitset<NUM_OF_FLAGS> flags) {
-	this->flags |= flags;
+void Parameter::setFlag(const char* flags) {
+	this->flags = Flags(flags);
 }
diff --git a/src/Services/ParameterService.cpp b/src/Services/ParameterService.cpp
index 880c8c3d..f79d99b3 100644
--- a/src/Services/ParameterService.cpp
+++ b/src/Services/ParameterService.cpp
@@ -7,11 +7,11 @@ ParameterService::ParameterService() {
 //	addNewParameter(3, 14);
 }
 
-bool ParameterService::addNewParameter(uint8_t ptc, uint8_t pfc, uint32_t initialValue, UpdatePtr ptr) {
-	Parameter param = Parameter(ptc, pfc, initialValue, ptr);
+bool ParameterService::addNewParameter(uint8_t id, Parameter param, const char* flags) {
 	try {
 		// second element of the returned std::pair is whether the given item was inserted or not
-		paramsList.insert(std::make_pair(paramsList.size(), param));
+		param.setFlag(flags);
+		paramsList.insert(std::make_pair(id, param));
 		return true;
 	}
 	catch (etl::map_full &mapFull) {
@@ -82,8 +82,7 @@ void ParameterService::setParameterIds(Message& newParamValues) {
 
 	for (uint16_t i = 0; i < numOfIds; i++) {
 		uint16_t currId = newParamValues.readUint16();
-
-		// TODO: add a check here with the new flag functionality
+		// the parameter is checked for read-only status and manual update availability
 		try {
 			paramsList.at(currId).setCurrentValue(newParamValues.readUint32());
 		}
diff --git a/test/Services/ParameterService.cpp b/test/Services/ParameterService.cpp
index 857ed5c5..57e5a1c2 100644
--- a/test/Services/ParameterService.cpp
+++ b/test/Services/ParameterService.cpp
@@ -17,13 +17,22 @@ void foo(ValueType* bar) {  // sample function
 
 TEST_CASE("Parameter Service - General") {
 	SECTION("Addition to full map") {
-		pserv.addNewParameter(3, 14);  // this one has ID 0
-		pserv.addNewParameter(1, 7, 12);  // this one has 1
-		pserv.addNewParameter(4, 12, 3, nullptr);  // this one has 2
-		pserv.addNewParameter(12, 3, 6, &foo); // this one has 3
-		pserv.addNewParameter(15, 7, 3, &foo); //and this one 4
 
-		REQUIRE_FALSE(pserv.addNewParameter(15, 5, 4));  // addNewParameter should return false
+		Parameter param0 = Parameter(3, 14);
+		Parameter param1 = Parameter(1, 7, 12);
+		Parameter param2 = Parameter(4, 12, 3, nullptr);
+		Parameter param3 = Parameter(12, 3, 6, &foo);
+		Parameter param4 = Parameter(15, 7, 3, &foo);
+
+		Parameter param5 = Parameter(15, 5, 4);
+
+		pserv.addNewParameter(0, param0);
+		pserv.addNewParameter(1, param1);
+		pserv.addNewParameter(2, param2);
+		pserv.addNewParameter(3, param3);
+		pserv.addNewParameter(4, param4);
+
+		REQUIRE_FALSE(pserv.addNewParameter(5, param5));  // addNewParameter should return false
 		Services.reset();  // reset all services
 	}
 
@@ -41,9 +50,12 @@ TEST_CASE("Parameter Report Subservice") {
 //	}
 
 	SECTION("Faulty instruction handling") {
-		pserv.addNewParameter(3, 14);  // ID 0
-		pserv.addNewParameter(1, 7, 12);  // ID 1
-		pserv.addNewParameter(4, 12, 3, nullptr);  // ID 2
+		Parameter param0 = Parameter(3, 14);
+		Parameter param1 = Parameter(1, 7, 12);
+		Parameter param2 = Parameter(4, 12, 3, nullptr);
+		pserv.addNewParameter(0, param0);
+		pserv.addNewParameter(1, param1);
+		pserv.addNewParameter(2, param2);
 
 		Message request(20, 1, Message::TC, 1);
 		request.appendUint16(2); // number of requested IDs
@@ -86,9 +98,12 @@ TEST_CASE("Parameter Report Subservice") {
 TEST_CASE("Parameter Setting Subservice") {
 
 	SECTION("Faulty Instruction Handling Test") {
-		pserv.addNewParameter(3, 14);  // ID 0
-		pserv.addNewParameter(1, 7, 12);  // ID 1
-		pserv.addNewParameter(4, 12, 3, nullptr);  // ID 2
+		Parameter param0 = Parameter(3, 14);
+		Parameter param1 = Parameter(1, 7, 12);
+		Parameter param2 = Parameter(4, 12, 3, nullptr);
+		pserv.addNewParameter(0, param0);
+		pserv.addNewParameter(1, param1);
+		pserv.addNewParameter(2, param2);
 
 		Message setRequest(20, 3, Message::TC, 1);
 		setRequest.appendUint16(2); // total number of IDs
-- 
GitLab