Merge branch 'development' into mueller/DeviceHandler-LocalPools

CHANGELOG

@@ -15,6 +15,20 @@ a C file without issues
 - The same is possible for the event reporting service (PUS5)
 - PUS Health Service added, which allows to command and retrieve health via PUS packets
 
+### Parameter Service
+
+- The API of the parameter service has been changed to prevent inconsistencies
+between documentation and actual code and to clarify usage.
+- The parameter ID now consists of:
+	1. Domain ID (1 byte)
+	2. Unique Identifier (1 byte)
+	3. Linear Index (2 bytes)
+The linear index can be used for arrays as well as matrices.
+The parameter load command now explicitely expects the ECSS PTC and PFC
+information as well as  the rows and column number. Rows and column will
+default to one, which is equivalent to one scalar parameter (the most
+important use-case)
+   
 ### File System Interface
 
 - A new interfaces specifies the functions for a software object which exposes the file system of a given hardware to use message based file handling (e.g. PUS commanding)

parameters/HasParametersIF.h

@@ -1,12 +1,16 @@
 #ifndef FSFW_PARAMETERS_HASPARAMETERSIF_H_
 #define FSFW_PARAMETERS_HASPARAMETERSIF_H_
 
-#include "../parameters/ParameterWrapper.h"
+#include "ParameterWrapper.h"
 #include "../returnvalues/HasReturnvaluesIF.h"
-#include <stdint.h>
+#include <cstdint>
 
-/** Each parameter is identified with a unique parameter ID */
+/**
-typedef uint32_t ParameterId_t;
+ * Each parameter is identified with a unique parameter ID
+ * The first byte of the parameter ID will denote the domain ID.
+ * The second and third byte will be the unique identifier number.
+ */
+using ParameterId_t = uint32_t;
 
 /**
  * @brief	This interface is used by components which have modifiable
@@ -16,16 +20,15 @@ typedef uint32_t ParameterId_t;
  * ID is the domain ID which can be used to identify unqiue spacecraft domains
  * (e.g. control and sensor domain in the AOCS controller).
  *
- * The second and third byte represent the matrix ID, which can represent
+ * The second byte is a unique identfier ID.
- * a 8-bit row and column number and the last byte...
  *
- * Yeah, it it matrix ID oder parameter ID now and is index a 16 bit number
+ * The third and  fourth byte can be used as a linear index for matrix or array
- * of a 8 bit number now?
+ * parameter entries.
  */
 class HasParametersIF {
 public:
 	static const uint8_t INTERFACE_ID = CLASS_ID::HAS_PARAMETERS_IF;
-	static const ReturnValue_t INVALID_MATRIX_ID = MAKE_RETURN_CODE(0x01);
+	static const ReturnValue_t INVALID_IDENTIFIER_ID = MAKE_RETURN_CODE(0x01);
 	static const ReturnValue_t INVALID_DOMAIN_ID = MAKE_RETURN_CODE(0x02);
 	static const ReturnValue_t INVALID_VALUE = MAKE_RETURN_CODE(0x03);
 	static const ReturnValue_t READ_ONLY = MAKE_RETURN_CODE(0x05);
@@ -34,33 +37,45 @@ public:
 		return id >> 24;
 	}
 
-	static uint16_t getMatrixId(ParameterId_t id) {
+	static uint8_t getUniqueIdentifierId(ParameterId_t id) {
-		return id >> 8;
+		return id >> 16;
 	}
 
-	static uint8_t getIndex(ParameterId_t id) {
+	/**
+	 * Get the index of a parameter. Please note that the index is always a
+	 * linear index. For a vector, this is straightforward.
+	 * For a matrix, the linear indexing run from left to right, top to bottom.
+	 * @param id
+	 * @return
+	 */
+	static uint16_t getIndex(ParameterId_t id) {
 		return id;
 	}
 
-	static uint32_t getFullParameterId(uint8_t domainId, uint16_t parameterId,
+	static uint32_t getFullParameterId(uint8_t domainId,
-			uint8_t index) {
+	        uint8_t uniqueIdentifier, uint16_t linearIndex) {
-		return (domainId << 24) + (parameterId << 8) + index;
+		return (domainId << 24) + (uniqueIdentifier << 16) + linearIndex;
 	}
 
 	virtual ~HasParametersIF() {}
 
 	/**
+	 * This is the generic function overriden by child classes to set
+	 * parameters. To set a parameter, the parameter wrapper is used with
+	 * a variety of set functions. The provided values can be checked with
+	 * newValues.
 	 * Always set parameter before checking newValues!
 	 *
 	 * @param domainId
 	 * @param parameterId
 	 * @param parameterWrapper
 	 * @param newValues
-	 * @param startAtIndex
+	 * @param startAtIndex Linear index, runs left to right, top to bottom for
+	 * matrix indexes.
 	 * @return
 	 */
-	virtual ReturnValue_t getParameter(uint8_t domainId, uint16_t parameterId,
+	virtual ReturnValue_t getParameter(uint8_t domainId,
-			ParameterWrapper *parameterWrapper,
+	        uint16_t uniqueIdentifier, ParameterWrapper *parameterWrapper,
 			const ParameterWrapper *newValues, uint16_t startAtIndex) = 0;
 };
 

parameters/ParameterHelper.cpp

@@ -9,15 +9,20 @@ ParameterHelper::~ParameterHelper() {
 }
 
 ReturnValue_t ParameterHelper::handleParameterMessage(CommandMessage *message) {
+    if(storage == nullptr) {
+        // ParameterHelper was not initialized
+        return HasReturnvaluesIF::RETURN_FAILED;
+    }
+
     ReturnValue_t result = HasReturnvaluesIF::RETURN_FAILED;
     switch (message->getCommand()) {
     case ParameterMessage::CMD_PARAMETER_DUMP: {
         ParameterWrapper description;
         uint8_t domain = HasParametersIF::getDomain(
                 ParameterMessage::getParameterId(message));
-		uint16_t parameterId = HasParametersIF::getMatrixId(
+        uint8_t uniqueIdentifier = HasParametersIF::getUniqueIdentifierId(
                 ParameterMessage::getParameterId(message));
-		result = owner->getParameter(domain, parameterId,
+        result = owner->getParameter(domain, uniqueIdentifier,
                 &description, &description, 0);
         if (result == HasReturnvaluesIF::RETURN_OK) {
             result = sendParameter(message->getSender(),
@@ -26,49 +31,48 @@ ReturnValue_t ParameterHelper::handleParameterMessage(CommandMessage *message) {
     }
     break;
     case ParameterMessage::CMD_PARAMETER_LOAD: {
-		uint8_t domain = HasParametersIF::getDomain(
+        ParameterId_t parameterId = 0;
-				ParameterMessage::getParameterId(message));
+        uint8_t ptc = 0;
-		uint16_t parameterId = HasParametersIF::getMatrixId(
+        uint8_t pfc = 0;
-				ParameterMessage::getParameterId(message));
+        uint8_t rows = 0;
-		uint8_t index = HasParametersIF::getIndex(
+        uint8_t columns = 0;
-				ParameterMessage::getParameterId(message));
+        store_address_t storeId = ParameterMessage::getParameterLoadCommand(
+                message, &parameterId, &ptc, &pfc, &rows, &columns);
+        Type type(Type::getActualType(ptc, pfc));
 
-		const uint8_t *storedStream = nullptr;
+        uint8_t domain = HasParametersIF::getDomain(parameterId);
-		size_t storedStreamSize = 0;
+        uint8_t uniqueIdentifier = HasParametersIF::getUniqueIdentifierId(
-		result = storage->getData(
+                parameterId);
-				ParameterMessage::getStoreId(message), &storedStream,
+        uint16_t linearIndex = HasParametersIF::getIndex(parameterId);
-				&storedStreamSize);
+
+        ConstStorageAccessor accessor(storeId);
+        result = storage->getData(storeId, accessor);
         if (result != HasReturnvaluesIF::RETURN_OK) {
             sif::error << "ParameterHelper::handleParameterMessage: Getting"
-					" store data failed for load command." << std::endl;
+                    << " store data failed for load command." << std::endl;
             break;
         }
 
         ParameterWrapper streamWrapper;
-		result = streamWrapper.set(storedStream, storedStreamSize);
+        result = streamWrapper.set(type, rows, columns, accessor.data(),
+                accessor.size());
         if(result != HasReturnvaluesIF::RETURN_OK) {
-			storage->deleteData(ParameterMessage::getStoreId(message));
+            return result;
-			break;
         }
 
         ParameterWrapper ownerWrapper;
-		result = owner->getParameter(domain, parameterId, &ownerWrapper,
+        result = owner->getParameter(domain, uniqueIdentifier, &ownerWrapper,
-				&streamWrapper, index);
+                &streamWrapper, linearIndex);
+
+        result = ownerWrapper.copyFrom(&streamWrapper, linearIndex);
         if (result != HasReturnvaluesIF::RETURN_OK) {
-			storage->deleteData(ParameterMessage::getStoreId(message));
+            return result;
-			break;
         }
 
-		result = ownerWrapper.copyFrom(&streamWrapper, index);
-
-		storage->deleteData(ParameterMessage::getStoreId(message));
-
-		if (result == HasReturnvaluesIF::RETURN_OK) {
         result = sendParameter(message->getSender(),
                 ParameterMessage::getParameterId(message), &ownerWrapper);
-		}
-	}
         break;
+    }
     default:
         return HasReturnvaluesIF::RETURN_FAILED;
     }
@@ -115,13 +119,11 @@ ReturnValue_t ParameterHelper::sendParameter(MessageQueueId_t to, uint32_t id,
 ReturnValue_t ParameterHelper::initialize() {
     ownerQueueId = owner->getCommandQueue();
 
-
     storage = objectManager->get<StorageManagerIF>(objects::IPC_STORE);
-	if (storage == NULL) {
+    if (storage == nullptr) {
-		return HasReturnvaluesIF::RETURN_FAILED;
+        return ObjectManagerIF::CHILD_INIT_FAILED;
-	} else {
-		return HasReturnvaluesIF::RETURN_OK;
     }
+    return HasReturnvaluesIF::RETURN_OK;
 }
 
 void ParameterHelper::rejectCommand(MessageQueueId_t to, ReturnValue_t reason,

parameters/ParameterMessage.cpp

@@ -25,10 +25,26 @@ void ParameterMessage::setParameterDumpReply(CommandMessage* message,
 }
 
 void ParameterMessage::setParameterLoadCommand(CommandMessage* message,
-		ParameterId_t id, store_address_t storageID) {
+		ParameterId_t id, store_address_t storeId, uint8_t ptc, uint8_t pfc,
+		uint8_t rows = 1, uint8_t columns = 1) {
 	message->setCommand(CMD_PARAMETER_LOAD);
 	message->setParameter(id);
-	message->setParameter2(storageID.raw);
+	message->setParameter2(storeId.raw);
+	uint32_t packedParameterSettings = (ptc << 24) | (pfc << 16) |
+	        (rows << 8) | columns;
+	message->setParameter3(packedParameterSettings);
+}
+
+store_address_t ParameterMessage::getParameterLoadCommand(
+        const CommandMessage *message, ParameterId_t* parameterId, uint8_t *ptc,
+        uint8_t *pfc, uint8_t *rows, uint8_t *columns) {
+    *parameterId = message->getParameter2();
+    uint32_t packedParamSettings = message->getParameter3();
+    *ptc = packedParamSettings >> 24 & 0xff;
+    *pfc = packedParamSettings >> 16 & 0xff;
+    *rows = packedParamSettings >> 8 & 0xff;
+    *columns = packedParamSettings & 0xff;
+    return message->getParameter2();
 }
 
 void ParameterMessage::clear(CommandMessage* message) {

parameters/ParameterMessage.h

@@ -5,6 +5,20 @@
 #include "../ipc/CommandMessage.h"
 #include "../storagemanager/StorageManagerIF.h"
 
+/**
+ * @brief   ParameterMessage interface
+ * @details
+ * General structure of a parameter message:
+ *  1. 4-byte Object ID
+ *  2. 4-byte Parameter ID, first byte is Domain ID, second byte is unique
+ *     identifier, third and fourth byte is linear index to start from
+ *  3. 4-byte Parameter Settings. First byte and second byte are the PTC and PFC
+ *     ECSS type identifiers (see ECSS-E-ST-70-41C15 p.428 or Type class in
+ *     globalfunctions). Third byte is the number of rows and fourth byte
+ *     is the number of columns. For single variable parameters, this will
+ *     be [1, 1].
+ *
+ */
 class ParameterMessage {
 private:
 	ParameterMessage();
@@ -20,8 +34,27 @@ public:
 			ParameterId_t id);
 	static void setParameterDumpReply(CommandMessage* message,
 			ParameterId_t id,  store_address_t storageID);
+
+	/**
+	 * Command to set a load parameter message. The CCSDS / ECSS type in
+	 * form of a PTC and a PFC is expected. See ECSS-E-ST-70-41C15 p.428
+	 * for all types or the Type class in globalfunctions.
+	 * @param message
+	 * @param id
+	 * @param storeId
+	 * @param ptc Type information according to CCSDS/ECSS standards
+	 * @param pfc Type information according to CCSDS/ECSS standards
+	 * @param rows Set number of rows in parameter set, minimum one.
+	 * @param columns Set number of columns in parameter set, minimum one
+	 */
 	static void setParameterLoadCommand(CommandMessage* message,
-			ParameterId_t id, store_address_t storageID);
+	        ParameterId_t id, store_address_t storeId, uint8_t ptc,
+	        uint8_t pfc, uint8_t rows, uint8_t columns);
+
+	static store_address_t getParameterLoadCommand(
+	        const CommandMessage* message, ParameterId_t* parameterId,
+	        uint8_t* ptc, uint8_t* pfc, uint8_t* rows, uint8_t* columns) ;
+
 	static void clear(CommandMessage* message);
 
 };

parameters/ParameterWrapper.cpp

@@ -115,7 +115,7 @@ ReturnValue_t ParameterWrapper::deSerializeData(uint8_t startingRow,
 		uint8_t fromColumns) {
 
 	//treat from as a continuous Stream as we copy all of it
-	const uint8_t *fromAsStream = (const uint8_t*) from;
+	const uint8_t *fromAsStream = reinterpret_cast<const uint8_t*>(from);
 	size_t streamSize = fromRows * fromColumns * sizeof(T);
 
 	ReturnValue_t result = HasReturnvaluesIF::RETURN_OK;
@@ -123,8 +123,9 @@ ReturnValue_t ParameterWrapper::deSerializeData(uint8_t startingRow,
 	for (uint8_t fromRow = 0; fromRow < fromRows; fromRow++) {
 
 		//get the start element of this row in data
-		T *dataWithDataType = ((T*) data)
+	    uint16_t offset = (((startingRow + fromRow) *
-				+ (((startingRow + fromRow) * columns) + startingColumn);
+	    		static_cast<uint16_t>(columns)) + startingColumn);
+		T *dataWithDataType = static_cast<T*>(data) + offset;
 
 		for (uint8_t fromColumn = 0; fromColumn < fromColumns; fromColumn++) {
 			result = SerializeAdapter::deSerialize(
@@ -159,6 +160,23 @@ ReturnValue_t ParameterWrapper::deSerialize(const uint8_t **buffer,
 	return copyFrom(&streamDescription, startWritingAtIndex);
 }
 
+ReturnValue_t ParameterWrapper::set(Type type, uint8_t rows, uint8_t columns,
+        const void *data, size_t dataSize) {
+    this->type = type;
+    this->rows = rows;
+    this->columns = columns;
+
+    size_t expectedSize = type.getSize() * rows * columns;
+    if (expectedSize < dataSize) {
+        return SerializeIF::STREAM_TOO_SHORT;
+    }
+
+    this->data = nullptr;
+    this->readonlyData = data;
+    pointsToStream = true;
+    return HasReturnvaluesIF::RETURN_OK;
+}
+
 ReturnValue_t ParameterWrapper::set(const uint8_t *stream, size_t streamSize,
 		const uint8_t **remainingStream, size_t *remainingSize) {
 	ReturnValue_t result = SerializeAdapter::deSerialize(&type, &stream,
@@ -202,11 +220,13 @@ ReturnValue_t ParameterWrapper::set(const uint8_t *stream, size_t streamSize,
 
 ReturnValue_t ParameterWrapper::copyFrom(const ParameterWrapper *from,
 		uint16_t startWritingAtIndex) {
-	if (data == NULL) {
+    // TODO: Optional diagnostic output (which can be disabled in FSFWConfig)
+    // to determined faulty implementations and configuration errors quickly.
+	if (data == nullptr) {
 		return READONLY;
 	}
 
-	if (from->readonlyData == NULL) {
+	if (from->readonlyData == nullptr) {
 		return SOURCE_NOT_SET;
 	}
 
@@ -214,9 +234,16 @@ ReturnValue_t ParameterWrapper::copyFrom(const ParameterWrapper *from,
 		return DATATYPE_MISSMATCH;
 	}
 
+	// The smallest allowed value for rows and columns is one.
+	if(rows == 0 or columns == 0) {
+	    return COLUMN_OR_ROWS_ZERO;
+	}
+
 	//check if from fits into this
-	uint8_t startingRow = startWritingAtIndex / columns;
+	uint8_t startingRow = 0;
-	uint8_t startingColumn = startWritingAtIndex % columns;
+	uint8_t startingColumn = 0;
+	ParameterWrapper::convertLinearIndexToRowAndColumn(startWritingAtIndex,
+	        &startingRow, &startingColumn);
 
 	if ((from->rows > (rows - startingRow))
 			|| (from->columns > (columns - startingColumn))) {
@@ -270,8 +297,8 @@ ReturnValue_t ParameterWrapper::copyFrom(const ParameterWrapper *from,
 		//need a type to do arithmetic
 		uint8_t* typedData = static_cast<uint8_t*>(data);
 		for (uint8_t fromRow = 0; fromRow < from->rows; fromRow++) {
-			size_t offset = (((startingRow + fromRow) * columns) +
+			size_t offset = (((startingRow + fromRow) * static_cast<uint16_t>(
-					startingColumn) * typeSize;
+					columns)) + startingColumn) * typeSize;
 			std::memcpy(typedData +  offset, from->readonlyData,
 					typeSize * from->columns);
 		}
@@ -279,3 +306,18 @@ ReturnValue_t ParameterWrapper::copyFrom(const ParameterWrapper *from,
 
 	return result;
 }
+
+void ParameterWrapper::convertLinearIndexToRowAndColumn(uint16_t index,
+        uint8_t *row, uint8_t *column) {
+    if(row == nullptr or column == nullptr) {
+        return;
+    }
+    // Integer division.
+    *row = index / columns;
+    *column = index % columns;
+}
+
+uint16_t ParameterWrapper::convertRowAndColumnToLinearIndex(uint8_t row,
+        uint8_t column) {
+    return row * columns + column;
+}

parameters/ParameterWrapper.h

@@ -22,6 +22,7 @@ public:
 	static const ReturnValue_t SOURCE_NOT_SET = MAKE_RETURN_CODE(0x05);
 	static const ReturnValue_t OUT_OF_BOUNDS = MAKE_RETURN_CODE(0x06);
 	static const ReturnValue_t NOT_SET = MAKE_RETURN_CODE(0x07);
+	static const ReturnValue_t COLUMN_OR_ROWS_ZERO = MAKE_RETURN_CODE(0x08);
 
 	ParameterWrapper();
 	ParameterWrapper(Type type, uint8_t rows, uint8_t columns, void *data);
@@ -68,7 +69,7 @@ public:
 
 	template<typename T>
 	void set(const T *readonlyData, uint8_t rows, uint8_t columns) {
-		this->data = NULL;
+		this->data = nullptr;
 		this->readonlyData = readonlyData;
 		this->type = PodTypeConversion<T>::type;
 		this->rows = rows;
@@ -97,14 +98,19 @@ public:
 	}
 	template<typename T>
 	void setMatrix(T& member) {
-		this->set(member[0], sizeof(member)/sizeof(member[0]), sizeof(member[0])/sizeof(member[0][0]));
+		this->set(member[0], sizeof(member)/sizeof(member[0]),
+		        sizeof(member[0])/sizeof(member[0][0]));
 	}
 
 	template<typename T>
 	void setMatrix(const T& member) {
-		this->set(member[0], sizeof(member)/sizeof(member[0]), sizeof(member[0])/sizeof(member[0][0]));
+		this->set(member[0], sizeof(member)/sizeof(member[0]),
+		        sizeof(member[0])/sizeof(member[0][0]));
 	}
 
+	ReturnValue_t set(Type type, uint8_t rows, uint8_t columns,
+	        const void *data, size_t dataSize);
+
 	ReturnValue_t set(const uint8_t *stream, size_t streamSize,
 			const uint8_t **remainingStream = nullptr,
 			size_t *remainingSize = nullptr);
@@ -113,6 +119,13 @@ public:
 			uint16_t startWritingAtIndex);
 
 private:
+
+    void convertLinearIndexToRowAndColumn(uint16_t index,
+            uint8_t *row, uint8_t *column);
+
+    uint16_t convertRowAndColumnToLinearIndex(uint8_t row,
+            uint8_t column);
+
 	bool pointsToStream = false;
 
 	Type type;
@@ -148,9 +161,9 @@ inline ReturnValue_t ParameterWrapper::getElement(T *value, uint8_t row,
 	if (pointsToStream) {
 		const uint8_t *streamWithType = static_cast<const uint8_t*>(readonlyData);
 		streamWithType += (row * columns + column) * type.getSize();
-		int32_t size = type.getSize();
+		size_t size = type.getSize();
 		return SerializeAdapter::deSerialize(value, &streamWithType,
-				&size, true);
+				&size, SerializeIF::Endianness::BIG);
 	}
 	else {
 		const T *dataWithType = static_cast<const T*>(readonlyData);