diff --git a/CHANGELOG b/CHANGELOG index 362979cea..e07976ff6 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -19,6 +19,20 @@ a C file without issues - New base class for a controller which also implements HasActionsIF and HasLocalDataPoolIF +### 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) diff --git a/parameters/HasParametersIF.h b/parameters/HasParametersIF.h index 005403fa5..19c12d81f 100644 --- a/parameters/HasParametersIF.h +++ b/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 +#include -/** 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 - * a 8-bit row and column number and the last byte... + * The second byte is a unique identfier ID. * - * Yeah, it it matrix ID oder parameter ID now and is index a 16 bit number - * of a 8 bit number now? + * The third and fourth byte can be used as a linear index for matrix or array + * 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) { - return id >> 8; + static uint8_t getUniqueIdentifierId(ParameterId_t id) { + 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, - uint8_t index) { - return (domainId << 24) + (parameterId << 8) + index; + static uint32_t getFullParameterId(uint8_t domainId, + uint8_t uniqueIdentifier, uint16_t linearIndex) { + 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, - ParameterWrapper *parameterWrapper, + virtual ReturnValue_t getParameter(uint8_t domainId, + uint16_t uniqueIdentifier, ParameterWrapper *parameterWrapper, const ParameterWrapper *newValues, uint16_t startAtIndex) = 0; }; diff --git a/parameters/ParameterHelper.cpp b/parameters/ParameterHelper.cpp index 659b00def..23d1a1f35 100644 --- a/parameters/ParameterHelper.cpp +++ b/parameters/ParameterHelper.cpp @@ -2,131 +2,133 @@ #include "ParameterMessage.h" #include "../objectmanager/ObjectManagerIF.h" -ParameterHelper::ParameterHelper(ReceivesParameterMessagesIF* owner) : - owner(owner) {} +ParameterHelper::ParameterHelper(ReceivesParameterMessagesIF* owner): + owner(owner) {} ParameterHelper::~ParameterHelper() { } ReturnValue_t ParameterHelper::handleParameterMessage(CommandMessage *message) { - 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( - ParameterMessage::getParameterId(message)); - result = owner->getParameter(domain, parameterId, - &description, &description, 0); - if (result == HasReturnvaluesIF::RETURN_OK) { - result = sendParameter(message->getSender(), - ParameterMessage::getParameterId(message), &description); - } - } - break; - case ParameterMessage::CMD_PARAMETER_LOAD: { - uint8_t domain = HasParametersIF::getDomain( - ParameterMessage::getParameterId(message)); - uint16_t parameterId = HasParametersIF::getMatrixId( - ParameterMessage::getParameterId(message)); - uint8_t index = HasParametersIF::getIndex( - ParameterMessage::getParameterId(message)); + if(storage == nullptr) { + // ParameterHelper was not initialized + return HasReturnvaluesIF::RETURN_FAILED; + } - const uint8_t *storedStream = nullptr; - size_t storedStreamSize = 0; - result = storage->getData( - ParameterMessage::getStoreId(message), &storedStream, - &storedStreamSize); - if (result != HasReturnvaluesIF::RETURN_OK) { - sif::error << "ParameterHelper::handleParameterMessage: Getting" - " store data failed for load command." << std::endl; - break; - } + ReturnValue_t result = HasReturnvaluesIF::RETURN_FAILED; + switch (message->getCommand()) { + case ParameterMessage::CMD_PARAMETER_DUMP: { + ParameterWrapper description; + uint8_t domain = HasParametersIF::getDomain( + ParameterMessage::getParameterId(message)); + uint8_t uniqueIdentifier = HasParametersIF::getUniqueIdentifierId( + ParameterMessage::getParameterId(message)); + result = owner->getParameter(domain, uniqueIdentifier, + &description, &description, 0); + if (result == HasReturnvaluesIF::RETURN_OK) { + result = sendParameter(message->getSender(), + ParameterMessage::getParameterId(message), &description); + } + } + break; + case ParameterMessage::CMD_PARAMETER_LOAD: { + ParameterId_t parameterId = 0; + uint8_t ptc = 0; + uint8_t pfc = 0; + uint8_t rows = 0; + uint8_t columns = 0; + store_address_t storeId = ParameterMessage::getParameterLoadCommand( + message, ¶meterId, &ptc, &pfc, &rows, &columns); + Type type(Type::getActualType(ptc, pfc)); - ParameterWrapper streamWrapper; - result = streamWrapper.set(storedStream, storedStreamSize); - if (result != HasReturnvaluesIF::RETURN_OK) { - storage->deleteData(ParameterMessage::getStoreId(message)); - break; - } + uint8_t domain = HasParametersIF::getDomain(parameterId); + uint8_t uniqueIdentifier = HasParametersIF::getUniqueIdentifierId( + parameterId); + uint16_t linearIndex = HasParametersIF::getIndex(parameterId); - ParameterWrapper ownerWrapper; - result = owner->getParameter(domain, parameterId, &ownerWrapper, - &streamWrapper, index); - if (result != HasReturnvaluesIF::RETURN_OK) { - storage->deleteData(ParameterMessage::getStoreId(message)); - break; - } + 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; + break; + } - result = ownerWrapper.copyFrom(&streamWrapper, index); + ParameterWrapper streamWrapper; + result = streamWrapper.set(type, rows, columns, accessor.data(), + accessor.size()); + if(result != HasReturnvaluesIF::RETURN_OK) { + return result; + } - storage->deleteData(ParameterMessage::getStoreId(message)); + ParameterWrapper ownerWrapper; + result = owner->getParameter(domain, uniqueIdentifier, &ownerWrapper, + &streamWrapper, linearIndex); - if (result == HasReturnvaluesIF::RETURN_OK) { - result = sendParameter(message->getSender(), - ParameterMessage::getParameterId(message), &ownerWrapper); - } - } - break; - default: - return HasReturnvaluesIF::RETURN_FAILED; - } + result = ownerWrapper.copyFrom(&streamWrapper, linearIndex); + if (result != HasReturnvaluesIF::RETURN_OK) { + return result; + } - if (result != HasReturnvaluesIF::RETURN_OK) { - rejectCommand(message->getSender(), result, message->getCommand()); - } + result = sendParameter(message->getSender(), + ParameterMessage::getParameterId(message), &ownerWrapper); + break; + } + default: + return HasReturnvaluesIF::RETURN_FAILED; + } - return HasReturnvaluesIF::RETURN_OK; + if (result != HasReturnvaluesIF::RETURN_OK) { + rejectCommand(message->getSender(), result, message->getCommand()); + } + + return HasReturnvaluesIF::RETURN_OK; } ReturnValue_t ParameterHelper::sendParameter(MessageQueueId_t to, uint32_t id, - const ParameterWrapper* description) { - size_t serializedSize = description->getSerializedSize(); + const ParameterWrapper* description) { + size_t serializedSize = description->getSerializedSize(); - uint8_t *storeElement; - store_address_t address; + uint8_t *storeElement; + store_address_t address; - ReturnValue_t result = storage->getFreeElement(&address, serializedSize, - &storeElement); - if (result != HasReturnvaluesIF::RETURN_OK) { - return result; - } + ReturnValue_t result = storage->getFreeElement(&address, serializedSize, + &storeElement); + if (result != HasReturnvaluesIF::RETURN_OK) { + return result; + } - size_t storeElementSize = 0; + size_t storeElementSize = 0; - result = description->serialize(&storeElement, &storeElementSize, - serializedSize, SerializeIF::Endianness::BIG); + result = description->serialize(&storeElement, &storeElementSize, + serializedSize, SerializeIF::Endianness::BIG); - if (result != HasReturnvaluesIF::RETURN_OK) { - storage->deleteData(address); - return result; - } + if (result != HasReturnvaluesIF::RETURN_OK) { + storage->deleteData(address); + return result; + } - CommandMessage reply; + CommandMessage reply; - ParameterMessage::setParameterDumpReply(&reply, id, address); + ParameterMessage::setParameterDumpReply(&reply, id, address); - MessageQueueSenderIF::sendMessage(to, &reply, ownerQueueId); + MessageQueueSenderIF::sendMessage(to, &reply, ownerQueueId); - return HasReturnvaluesIF::RETURN_OK; + return HasReturnvaluesIF::RETURN_OK; } ReturnValue_t ParameterHelper::initialize() { - ownerQueueId = owner->getCommandQueue(); + ownerQueueId = owner->getCommandQueue(); - - storage = objectManager->get(objects::IPC_STORE); - if (storage == NULL) { - return HasReturnvaluesIF::RETURN_FAILED; - } else { - return HasReturnvaluesIF::RETURN_OK; - } + storage = objectManager->get(objects::IPC_STORE); + if (storage == nullptr) { + return ObjectManagerIF::CHILD_INIT_FAILED; + } + return HasReturnvaluesIF::RETURN_OK; } void ParameterHelper::rejectCommand(MessageQueueId_t to, ReturnValue_t reason, - Command_t initialCommand) { - CommandMessage reply; - reply.setReplyRejected(reason, initialCommand); - MessageQueueSenderIF::sendMessage(to, &reply, ownerQueueId); + Command_t initialCommand) { + CommandMessage reply; + reply.setReplyRejected(reason, initialCommand); + MessageQueueSenderIF::sendMessage(to, &reply, ownerQueueId); } diff --git a/parameters/ParameterMessage.cpp b/parameters/ParameterMessage.cpp index e9f3191fc..af1f9ce83 100644 --- a/parameters/ParameterMessage.cpp +++ b/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) { diff --git a/parameters/ParameterMessage.h b/parameters/ParameterMessage.h index 31d7fe0bc..fd4481eec 100644 --- a/parameters/ParameterMessage.h +++ b/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); }; diff --git a/parameters/ParameterWrapper.cpp b/parameters/ParameterWrapper.cpp index 6adc6857c..d9f1ef685 100644 --- a/parameters/ParameterWrapper.cpp +++ b/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(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) - + (((startingRow + fromRow) * columns) + startingColumn); + uint16_t offset = (((startingRow + fromRow) * + static_cast(columns)) + startingColumn); + T *dataWithDataType = static_cast(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 startingColumn = startWritingAtIndex % columns; + uint8_t startingRow = 0; + 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(data); for (uint8_t fromRow = 0; fromRow < from->rows; fromRow++) { - size_t offset = (((startingRow + fromRow) * columns) + - startingColumn) * typeSize; + size_t offset = (((startingRow + fromRow) * static_cast( + 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; +} diff --git a/parameters/ParameterWrapper.h b/parameters/ParameterWrapper.h index f07205d4b..168a535f2 100644 --- a/parameters/ParameterWrapper.h +++ b/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 void set(const T *readonlyData, uint8_t rows, uint8_t columns) { - this->data = NULL; + this->data = nullptr; this->readonlyData = readonlyData; this->type = PodTypeConversion::type; this->rows = rows; @@ -97,14 +98,19 @@ public: } template 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 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(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(readonlyData);