From 6ea933492368e31217851717f9d880c033cfce17 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Thu, 29 Oct 2020 17:57:27 +0100 Subject: [PATCH 1/7] parameter updates --- parameters/HasParametersIF.h | 42 ++++++++++------- parameters/ParameterHelper.cpp | 80 +++++++++++++++++---------------- parameters/ParameterMessage.cpp | 20 ++++++++- parameters/ParameterMessage.h | 35 ++++++++++++++- parameters/ParameterWrapper.cpp | 55 ++++++++++++++++++++--- parameters/ParameterWrapper.h | 23 +++++++--- 6 files changed, 186 insertions(+), 69 deletions(-) diff --git a/parameters/HasParametersIF.h b/parameters/HasParametersIF.h index 005403fa..c4fd81b3 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 "../returnvalues/HasReturnvaluesIF.h" -#include +#include "ParameterWrapper.h" +#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 @@ -19,13 +23,13 @@ typedef uint32_t ParameterId_t; * The second and third byte represent the matrix ID, which can represent * 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 + * Yeah, is it matrix ID or parameter ID now and is index a 16 bit number * of a 8 bit number now? */ 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,17 +38,24 @@ 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() {} @@ -56,11 +67,12 @@ public: * @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 659b00de..d2193b28 100644 --- a/parameters/ParameterHelper.cpp +++ b/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( - ParameterMessage::getParameterId(message)); - uint16_t parameterId = HasParametersIF::getMatrixId( - ParameterMessage::getParameterId(message)); - uint8_t index = HasParametersIF::getIndex( - ParameterMessage::getParameterId(message)); + 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)); - const uint8_t *storedStream = nullptr; - size_t storedStreamSize = 0; - result = storage->getData( - ParameterMessage::getStoreId(message), &storedStream, - &storedStreamSize); + uint8_t domain = HasParametersIF::getDomain(parameterId); + uint8_t uniqueIdentifier = HasParametersIF::getUniqueIdentifierId( + parameterId); + uint16_t linearIndex = HasParametersIF::getIndex(parameterId); + + 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); - if (result != HasReturnvaluesIF::RETURN_OK) { - storage->deleteData(ParameterMessage::getStoreId(message)); - break; + result = streamWrapper.set(type, rows, columns, accessor.data(), + accessor.size()); + if(result != HasReturnvaluesIF::RETURN_OK) { + return result; } ParameterWrapper ownerWrapper; - result = owner->getParameter(domain, parameterId, &ownerWrapper, - &streamWrapper, index); + result = owner->getParameter(domain, uniqueIdentifier, &ownerWrapper, + &streamWrapper, linearIndex); + + result = ownerWrapper.copyFrom(&streamWrapper, linearIndex); if (result != HasReturnvaluesIF::RETURN_OK) { - storage->deleteData(ParameterMessage::getStoreId(message)); - break; + return result; } - result = ownerWrapper.copyFrom(&streamWrapper, index); - - storage->deleteData(ParameterMessage::getStoreId(message)); - - if (result == HasReturnvaluesIF::RETURN_OK) { - result = sendParameter(message->getSender(), - ParameterMessage::getParameterId(message), &ownerWrapper); - } - } + result = sendParameter(message->getSender(), + ParameterMessage::getParameterId(message), &ownerWrapper); break; + } default: return HasReturnvaluesIF::RETURN_FAILED; } @@ -113,15 +117,13 @@ ReturnValue_t ParameterHelper::sendParameter(MessageQueueId_t to, uint32_t id, } 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, diff --git a/parameters/ParameterMessage.cpp b/parameters/ParameterMessage.cpp index e9f3191f..af1f9ce8 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 31d7fe0b..fd4481ee 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 6adc6857..23955516 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,8 @@ 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) * columns) + startingColumn); + T *dataWithDataType = static_cast(data) + offset; for (uint8_t fromColumn = 0; fromColumn < fromColumns; fromColumn++) { result = SerializeAdapter::deSerialize( @@ -159,6 +159,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 +219,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 +233,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))) { @@ -279,3 +305,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 f07205d4..168a535f 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); From 0304f61bdeb3035137bea029b145bacd34b5dd75 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 10 Nov 2020 11:00:13 +0100 Subject: [PATCH 2/7] has parametersIF doc improved --- parameters/HasParametersIF.h | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/parameters/HasParametersIF.h b/parameters/HasParametersIF.h index c4fd81b3..19c12d81 100644 --- a/parameters/HasParametersIF.h +++ b/parameters/HasParametersIF.h @@ -1,8 +1,8 @@ #ifndef FSFW_PARAMETERS_HASPARAMETERSIF_H_ #define FSFW_PARAMETERS_HASPARAMETERSIF_H_ -#include "../returnvalues/HasReturnvaluesIF.h" #include "ParameterWrapper.h" +#include "../returnvalues/HasReturnvaluesIF.h" #include /** @@ -20,11 +20,10 @@ using ParameterId_t = uint32_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, is it matrix ID or 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: @@ -61,6 +60,10 @@ public: 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 From 7c75c6e8cc22a4b8540f8866a5425f101bcfea6e Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 10 Nov 2020 11:14:07 +0100 Subject: [PATCH 3/7] parameter helper update --- parameters/ParameterHelper.cpp | 166 ++++++++++++++++----------------- 1 file changed, 83 insertions(+), 83 deletions(-) diff --git a/parameters/ParameterHelper.cpp b/parameters/ParameterHelper.cpp index d2193b28..22465ca3 100644 --- a/parameters/ParameterHelper.cpp +++ b/parameters/ParameterHelper.cpp @@ -3,7 +3,7 @@ #include "../objectmanager/ObjectManagerIF.h" ParameterHelper::ParameterHelper(ReceivesParameterMessagesIF* owner) : - owner(owner) {} +owner(owner) {} ParameterHelper::~ParameterHelper() { } @@ -14,106 +14,106 @@ ReturnValue_t ParameterHelper::handleParameterMessage(CommandMessage *message) { 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)); - 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)); + 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)); - uint8_t domain = HasParametersIF::getDomain(parameterId); - uint8_t uniqueIdentifier = HasParametersIF::getUniqueIdentifierId( - parameterId); - uint16_t linearIndex = HasParametersIF::getIndex(parameterId); + uint8_t domain = HasParametersIF::getDomain(parameterId); + uint8_t uniqueIdentifier = HasParametersIF::getUniqueIdentifierId( + parameterId); + uint16_t linearIndex = HasParametersIF::getIndex(parameterId); - 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; - } + 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; + } - ParameterWrapper streamWrapper; - result = streamWrapper.set(type, rows, columns, accessor.data(), - accessor.size()); - if(result != HasReturnvaluesIF::RETURN_OK) { - return result; - } + ParameterWrapper streamWrapper; + result = streamWrapper.set(type, rows, columns, accessor.data(), + accessor.size()); + if(result != HasReturnvaluesIF::RETURN_OK) { + return result; + } - ParameterWrapper ownerWrapper; - result = owner->getParameter(domain, uniqueIdentifier, &ownerWrapper, - &streamWrapper, linearIndex); + ParameterWrapper ownerWrapper; + result = owner->getParameter(domain, uniqueIdentifier, &ownerWrapper, + &streamWrapper, linearIndex); - result = ownerWrapper.copyFrom(&streamWrapper, linearIndex); - if (result != HasReturnvaluesIF::RETURN_OK) { - return result; - } + result = ownerWrapper.copyFrom(&streamWrapper, linearIndex); + if (result != HasReturnvaluesIF::RETURN_OK) { + return result; + } result = sendParameter(message->getSender(), ParameterMessage::getParameterId(message), &ownerWrapper); - break; - } - default: - return HasReturnvaluesIF::RETURN_FAILED; - } + break; + } + default: + return HasReturnvaluesIF::RETURN_FAILED; + } - if (result != HasReturnvaluesIF::RETURN_OK) { - rejectCommand(message->getSender(), result, message->getCommand()); - } + if (result != HasReturnvaluesIF::RETURN_OK) { + rejectCommand(message->getSender(), result, message->getCommand()); + } - return HasReturnvaluesIF::RETURN_OK; + 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() { @@ -127,8 +127,8 @@ ReturnValue_t ParameterHelper::initialize() { } 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); } From 5865e56c54209ffe3076fda86cf94e02a706df25 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 10 Nov 2020 11:15:42 +0100 Subject: [PATCH 4/7] indentation fixed --- parameters/ParameterHelper.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/parameters/ParameterHelper.cpp b/parameters/ParameterHelper.cpp index 22465ca3..23d1a1f3 100644 --- a/parameters/ParameterHelper.cpp +++ b/parameters/ParameterHelper.cpp @@ -2,8 +2,8 @@ #include "ParameterMessage.h" #include "../objectmanager/ObjectManagerIF.h" -ParameterHelper::ParameterHelper(ReceivesParameterMessagesIF* owner) : -owner(owner) {} +ParameterHelper::ParameterHelper(ReceivesParameterMessagesIF* owner): + owner(owner) {} ParameterHelper::~ParameterHelper() { } From 74ba8b76644c31405d67878421b2081306eab453 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 3 Dec 2020 13:10:33 +0100 Subject: [PATCH 5/7] changel og updated --- CHANGELOG | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index a6686478..62a6b0a9 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -11,3 +11,19 @@ a C file without issues - It is now possible to change the message queue depth for the telecommand verification service (PUS1) - 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 incosistencies +and clarify usage when inspecting the code. +- 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) + + From 3f9f062435061c32818c4c4ddb078cb328ea8f4d Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 3 Dec 2020 13:11:52 +0100 Subject: [PATCH 6/7] changelog update --- CHANGELOG | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 62a6b0a9..8341e138 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -14,8 +14,8 @@ a C file without issues ### Parameter Service -- The API of the parameter service has been changed to prevent incosistencies -and clarify usage when inspecting the code. +- 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) From a2823835dd8f6708fd004aef2d95d74547c7cdf7 Mon Sep 17 00:00:00 2001 From: Spacefish Date: Thu, 10 Dec 2020 17:05:41 +0100 Subject: [PATCH 7/7] explicit cast added --- parameters/ParameterWrapper.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/parameters/ParameterWrapper.cpp b/parameters/ParameterWrapper.cpp index 23955516..d9f1ef68 100644 --- a/parameters/ParameterWrapper.cpp +++ b/parameters/ParameterWrapper.cpp @@ -123,7 +123,8 @@ ReturnValue_t ParameterWrapper::deSerializeData(uint8_t startingRow, for (uint8_t fromRow = 0; fromRow < fromRows; fromRow++) { //get the start element of this row in data - uint16_t offset = (((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++) { @@ -296,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); }