From 0ce67de8c8eb8f4eb532e4445e3a1e3765ff07f2 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Fri, 10 Jan 2020 00:57:09 +0100 Subject: [PATCH] Changes to pool access classes 1. PoolRawAccessHelper is more robust now and has better error handling 2. PoolRawAccess: Removed an unneeded constructor value, moved serialize further to the top. Added new returnvalues and more precise error handling for read() call 3. DataSet: Made MAX Number of data pool entries public so it can be used by pool raw access error handling --- datapool/DataSet.h | 71 +++++++------- datapool/PoolRawAccess.cpp | 72 +++++++++------ datapool/PoolRawAccess.h | 153 ++++++++++++++++--------------- datapool/PoolRawAccessHelper.cpp | 51 ++++++++--- datapool/PoolRawAccessHelper.h | 2 + 5 files changed, 199 insertions(+), 150 deletions(-) diff --git a/datapool/DataSet.h b/datapool/DataSet.h index deee8ca6..e8e1a597 100644 --- a/datapool/DataSet.h +++ b/datapool/DataSet.h @@ -37,44 +37,11 @@ * \ingroup data_pool */ class DataSet: public DataSetIF, public HasReturnvaluesIF, public SerializeIF { -private: + +public: //SHOULDDO we could use a linked list of datapool variables static const uint8_t DATA_SET_MAX_SIZE = 63; //!< This definition sets the maximum number of variables to register in one DataSet. - /** - * \brief This array represents all pool variables registered in this set. - * \details It has a maximum size of DATA_SET_MAX_SIZE. - */ - PoolVariableIF* registeredVariables[DATA_SET_MAX_SIZE]; - /** - * \brief The fill_count attribute ensures that the variables register in the correct array - * position and that the maximum number of variables is not exceeded. - */ - uint16_t fill_count; - /** - * States of the seet. - */ - enum States { - DATA_SET_UNINITIALISED, //!< DATA_SET_UNINITIALISED - DATA_SET_WAS_READ //!< DATA_SET_WAS_READ - }; - /** - * \brief state manages the internal state of the data set, which is important e.g. for the - * behavior on destruction. - */ - States state; - /** - * \brief This is a small helper function to facilitate locking the global data pool. - * \details It makes use of the lockDataPool method offered by the DataPool class. - */ - uint8_t lockDataPool(); - /** - * \brief This is a small helper function to facilitate unlocking the global data pool. - * \details It makes use of the freeDataPoolLock method offered by the DataPool class. - */ - uint8_t freeDataPoolLock(); - -public: static const uint8_t INTERFACE_ID = CLASS_ID::DATA_SET_CLASS; static const ReturnValue_t INVALID_PARAMETER_DEFINITION = MAKE_RETURN_CODE( 0x01 ); @@ -165,6 +132,40 @@ public: ReturnValue_t serializeRawFromIdBuffer(uint8_t ** buffer, uint32_t * size, const uint32_t max_size, bool bigEndian, uint32_t * poolIdBuffer, uint32_t poolIdSize); +private: + /** + * \brief This array represents all pool variables registered in this set. + * \details It has a maximum size of DATA_SET_MAX_SIZE. + */ + PoolVariableIF* registeredVariables[DATA_SET_MAX_SIZE]; + /** + * \brief The fill_count attribute ensures that the variables register in the correct array + * position and that the maximum number of variables is not exceeded. + */ + uint16_t fill_count; + /** + * States of the seet. + */ + enum States { + DATA_SET_UNINITIALISED, //!< DATA_SET_UNINITIALISED + DATA_SET_WAS_READ //!< DATA_SET_WAS_READ + }; + /** + * \brief state manages the internal state of the data set, which is important e.g. for the + * behavior on destruction. + */ + States state; + /** + * \brief This is a small helper function to facilitate locking the global data pool. + * \details It makes use of the lockDataPool method offered by the DataPool class. + */ + uint8_t lockDataPool(); + /** + * \brief This is a small helper function to facilitate unlocking the global data pool. + * \details It makes use of the freeDataPoolLock method offered by the DataPool class. + */ + uint8_t freeDataPoolLock(); + }; #endif /* DATASET_H_ */ diff --git a/datapool/PoolRawAccess.cpp b/datapool/PoolRawAccess.cpp index 7cc61852..c4e327b1 100644 --- a/datapool/PoolRawAccess.cpp +++ b/datapool/PoolRawAccess.cpp @@ -5,8 +5,7 @@ #include PoolRawAccess::PoolRawAccess(uint32_t set_id, uint8_t setArrayEntry, - DataSetIF* data_set, ReadWriteMode_t setReadWriteMode, - bool registerVectors) : + DataSetIF* data_set, ReadWriteMode_t setReadWriteMode) : dataPoolId(set_id), arrayEntry(setArrayEntry), valid(false), type(Type::UNKNOWN_TYPE), typeSize( 0), arraySize(0), sizeTillEnd(0), readWriteMode(setReadWriteMode) { memset(value, 0, sizeof(value)); @@ -20,6 +19,7 @@ PoolRawAccess::~PoolRawAccess() { } ReturnValue_t PoolRawAccess::read() { + ReturnValue_t result = RETURN_FAILED; PoolEntryIF* read_out = ::dataPool.getRawData(dataPoolId); if (read_out != NULL) { valid = read_out->getValid(); @@ -35,21 +35,30 @@ ReturnValue_t PoolRawAccess::read() { memcpy(value, ptr, typeSize); return HasReturnvaluesIF::RETURN_OK; } else { - //Error value type too large. + result = READ_TYPE_TOO_LARGE; } } else { - //Error index requested too large + result = READ_INDEX_TOO_LARGE; } } else { - //Error entry does not exist. + result = READ_ENTRY_NON_EXISTENT; } error << "PoolRawAccess: read of DP Variable 0x" << std::hex << dataPoolId - << std::dec << " failed." << std::endl; + << std::dec << " failed, "; + if(result == READ_TYPE_TOO_LARGE) { + error << "type too large." << std::endl; + } + else if(result == READ_INDEX_TOO_LARGE) { + error << "index too large." << std::endl; + } + else { + error << "entry does not exist." << std::endl; + } valid = INVALID; typeSize = 0; sizeTillEnd = 0; memset(value, 0, sizeof(value)); - return HasReturnvaluesIF::RETURN_FAILED; + return result; } ReturnValue_t PoolRawAccess::commit() { @@ -90,6 +99,32 @@ ReturnValue_t PoolRawAccess::getEntryEndianSafe(uint8_t* buffer, return HasReturnvaluesIF::RETURN_OK; } + +ReturnValue_t PoolRawAccess::serialize(uint8_t** buffer, uint32_t* size, + const uint32_t max_size, bool bigEndian) const { + if (typeSize + *size <= max_size) { + if (bigEndian) { +#ifndef BYTE_ORDER_SYSTEM +#error BYTE_ORDER_SYSTEM not defined +#elif BYTE_ORDER_SYSTEM == LITTLE_ENDIAN + for (uint8_t count = 0; count < typeSize; count++) { + (*buffer)[count] = value[typeSize - count - 1]; + } +#elif BYTE_ORDER_SYSTEM == BIG_ENDIAN + memcpy(*buffer, value, typeSize); +#endif + } else { + memcpy(*buffer, value, typeSize); + } + *size += typeSize; + (*buffer) += typeSize; + return HasReturnvaluesIF::RETURN_OK; + } else { + return SerializeIF::BUFFER_TOO_SHORT; + } +} + + Type PoolRawAccess::getType() { return type; } @@ -146,29 +181,6 @@ uint16_t PoolRawAccess::getSizeTillEnd() const { return sizeTillEnd; } -ReturnValue_t PoolRawAccess::serialize(uint8_t** buffer, uint32_t* size, - const uint32_t max_size, bool bigEndian) const { - if (typeSize + *size <= max_size) { - if (bigEndian) { -#ifndef BYTE_ORDER_SYSTEM -#error BYTE_ORDER_SYSTEM not defined -#elif BYTE_ORDER_SYSTEM == LITTLE_ENDIAN - for (uint8_t count = 0; count < typeSize; count++) { - (*buffer)[count] = value[typeSize - count - 1]; - } -#elif BYTE_ORDER_SYSTEM == BIG_ENDIAN - memcpy(*buffer, value, typeSize); -#endif - } else { - memcpy(*buffer, value, typeSize); - } - *size += typeSize; - (*buffer) += typeSize; - return HasReturnvaluesIF::RETURN_OK; - } else { - return SerializeIF::BUFFER_TOO_SHORT; - } -} uint32_t PoolRawAccess::getSerializedSize() const { return typeSize; diff --git a/datapool/PoolRawAccess.h b/datapool/PoolRawAccess.h index 603bae23..432d9ad9 100644 --- a/datapool/PoolRawAccess.h +++ b/datapool/PoolRawAccess.h @@ -15,70 +15,17 @@ * data pool access. * @ingroup data_pool */ -class PoolRawAccess: public PoolVariableIF { -private: - /** - * \brief To access the correct data pool entry on read and commit calls, the data pool id - * is stored. - */ - uint32_t dataPoolId; - /** - * \brief The array entry that is fetched from the data pool. - */ - uint8_t arrayEntry; - /** - * \brief The valid information as it was stored in the data pool is copied to this attribute. - */ - uint8_t valid; - /** - * \brief This value contains the type of the data pool entry. - */ - Type type; - /** - * \brief This value contains the size of the data pool entry type in bytes. - */ - uint8_t typeSize; - /** - * The size of the DP array (single values return 1) - */ - uint8_t arraySize; - /** - * The size (in bytes) from the selected entry till the end of this DataPool variable. - */ - uint16_t sizeTillEnd; - /** - * \brief The information whether the class is read-write or read-only is stored here. - */ - ReadWriteMode_t readWriteMode; - static const uint8_t RAW_MAX_SIZE = sizeof(double); -protected: - /** - * \brief This is a call to read the value from the global data pool. - * \details When executed, this operation tries to fetch the pool entry with matching - * data pool id from the global data pool and copies the value and the valid - * information to its local attributes. In case of a failure (wrong type or - * pool id not found), the variable is set to zero and invalid. - * The operation does NOT provide any mutual exclusive protection by itself ! - * If reading from the data pool without information about the type is desired, - * initialize the raw pool access by supplying a data set and using the data set - * read function, which calls this read function. - */ - ReturnValue_t read(); - /** - * \brief The commit call writes back the variable's value to the data pool. - * \details It checks type and size, as well as if the variable is writable. If so, - * the value is copied and the valid flag is automatically set to "valid". - * The operation does NOT provide any mutual exclusive protection by itself. - * - */ - ReturnValue_t commit(); +class PoolRawAccess: public PoolVariableIF, HasReturnvaluesIF { public: - static const uint8_t INTERFACE_ID = CLASS_ID::POOL_RAW_ACCESS_CLASS; static const ReturnValue_t INCORRECT_SIZE = MAKE_RETURN_CODE(0x01); static const ReturnValue_t DATA_POOL_ACCESS_FAILED = MAKE_RETURN_CODE(0x02); + static const ReturnValue_t READ_TYPE_TOO_LARGE = MAKE_RETURN_CODE(0x03); + static const ReturnValue_t READ_INDEX_TOO_LARGE = MAKE_RETURN_CODE(0x04); + static const ReturnValue_t READ_ENTRY_NON_EXISTENT = MAKE_RETURN_CODE(0x05); + static const uint8_t RAW_MAX_SIZE = sizeof(double); uint8_t value[RAW_MAX_SIZE]; - //PoolRawAccess(); + /** * This constructor is used to access a data pool entry with a * given ID if the target type is not known. A DataSet object is supplied @@ -96,25 +43,13 @@ public: */ PoolRawAccess(uint32_t data_pool_id, uint8_t arrayEntry, DataSetIF* data_set, ReadWriteMode_t setReadWriteMode = - PoolVariableIF::VAR_READ,bool registerVectors = false); + PoolVariableIF::VAR_READ); /** * \brief The classes destructor is empty. If commit() was not called, the local value is * discarded and not written back to the data pool. */ ~PoolRawAccess(); - /** - * @brief Serialize raw pool entry into provided buffer directly - * @param buffer Provided buffer. Raw pool data will be copied here - * @param size [out] Increment provided size value by serialized size - * @param max_size Maximum allowed serialization size - * @param bigEndian Specify endianess - * @return - @c RETURN_OK if serialization was successfull - * - @c SerializeIF::BUFFER_TOO_SHORT if range check failed - */ - ReturnValue_t serialize(uint8_t** buffer, uint32_t* size, - const uint32_t max_size, bool bigEndian) const; - /** * \brief This operation returns a pointer to the entry fetched. * \details Return pointer to the buffer containing the raw data @@ -136,6 +71,19 @@ public: */ ReturnValue_t getEntryEndianSafe(uint8_t* buffer, uint32_t* size, uint32_t max_size); + + /** + * @brief Serialize raw pool entry into provided buffer directly + * @param buffer Provided buffer. Raw pool data will be copied here + * @param size [out] Increment provided size value by serialized size + * @param max_size Maximum allowed serialization size + * @param bigEndian Specify endianess + * @return - @c RETURN_OK if serialization was successfull + * - @c SerializeIF::BUFFER_TOO_SHORT if range check failed + */ + ReturnValue_t serialize(uint8_t** buffer, uint32_t* size, + const uint32_t max_size, bool bigEndian) const; + /** * With this method, the content can be set from a big endian buffer safely. * @param buffer Pointer to the data to set @@ -181,6 +129,67 @@ public: ReturnValue_t deSerialize(const uint8_t** buffer, int32_t* size, bool bigEndian); + +protected: + /** + * \brief This is a call to read the value from the global data pool. + * \details When executed, this operation tries to fetch the pool entry with matching + * data pool id from the global data pool and copies the value and the valid + * information to its local attributes. In case of a failure (wrong type or + * pool id not found), the variable is set to zero and invalid. + * The operation does NOT provide any mutual exclusive protection by itself ! + * If reading from the data pool without information about the type is desired, + * initialize the raw pool access by supplying a data set and using the data set + * read function, which calls this read function. + * @return -@c RETURN_OK Read successfull + * -@c READ_TYPE_TOO_LARGE + * -@c READ_INDEX_TOO_LARGE + * -@c READ_ENTRY_NON_EXISTENT + */ + ReturnValue_t read(); + /** + * \brief The commit call writes back the variable's value to the data pool. + * \details It checks type and size, as well as if the variable is writable. If so, + * the value is copied and the valid flag is automatically set to "valid". + * The operation does NOT provide any mutual exclusive protection by itself. + * + */ + ReturnValue_t commit(); + +private: + /** + * \brief To access the correct data pool entry on read and commit calls, the data pool id + * is stored. + */ + uint32_t dataPoolId; + /** + * \brief The array entry that is fetched from the data pool. + */ + uint8_t arrayEntry; + /** + * \brief The valid information as it was stored in the data pool is copied to this attribute. + */ + uint8_t valid; + /** + * \brief This value contains the type of the data pool entry. + */ + Type type; + /** + * \brief This value contains the size of the data pool entry type in bytes. + */ + uint8_t typeSize; + /** + * The size of the DP array (single values return 1) + */ + uint8_t arraySize; + /** + * The size (in bytes) from the selected entry till the end of this DataPool variable. + */ + uint16_t sizeTillEnd; + /** + * \brief The information whether the class is read-write or read-only is stored here. + */ + ReadWriteMode_t readWriteMode; }; #endif /* POOLRAWACCESS_H_ */ diff --git a/datapool/PoolRawAccessHelper.cpp b/datapool/PoolRawAccessHelper.cpp index e1ae56bf..8c750270 100644 --- a/datapool/PoolRawAccessHelper.cpp +++ b/datapool/PoolRawAccessHelper.cpp @@ -78,45 +78,70 @@ ReturnValue_t PoolRawAccessHelper::serializeCurrentPoolEntryIntoBuffer(Serializa ReturnValue_t PoolRawAccessHelper::handlePoolEntrySerialization(uint32_t currentPoolId,SerializationArgs argStruct, bool withValidMask, uint8_t * validityMask) { - ReturnValue_t result; + ReturnValue_t result = RETURN_FAILED; uint8_t arrayPosition = 0; + uint8_t counter = 0; bool poolEntrySerialized = false; - //info << "Pool Raw Access Helper: Handling Pool ID: " << std::hex << currentPoolId << std::endl; + info << "Pool Raw Access Helper: Handling Pool ID: " << std::hex << currentPoolId << std::endl; while(not poolEntrySerialized) { + + if(counter > DataSet::DATA_SET_MAX_SIZE) { + error << "Pool Raw Access Helper: Config error, max. number of possible data set variables exceeded" << std::endl; + return result; + } + counter ++; + DataSet currentDataSet = DataSet(); PoolRawAccess currentPoolRawAccess(currentPoolId,arrayPosition,¤tDataSet,PoolVariableIF::VAR_READ); + result = currentDataSet.read(); if (result != RETURN_OK) { - debug << std::hex << "Pool Raw Access Helper: Error reading raw dataset" << std::dec << std::endl; + debug << std::hex << "Pool Raw Access Helper: Error reading raw dataset with returncode " + << result << std::dec << std::endl; return result; } - uint8_t remainingSize = currentPoolRawAccess.getSizeTillEnd() - currentPoolRawAccess.getSizeOfType(); - if(remainingSize == 0) { - poolEntrySerialized = true; - } - else if(remainingSize > 0) { - arrayPosition += currentPoolRawAccess.getSizeOfType() / 8; - } - else { - error << "Pool Raw Access Helper: Configuration Error. Size till end smaller than 0" << std::endl; + + result = checkRemainingSize(¤tPoolRawAccess, &poolEntrySerialized, &arrayPosition); + if(result != RETURN_OK) { + error << "Pool Raw Access Helper: Configuration Error at pool ID " << std::hex << currentPoolId + << ". Size till end smaller than 0" << std::dec << std::endl; return result; } + // set valid mask bit if necessary if(withValidMask) { if(currentPoolRawAccess.isValid()) { handleMaskModification(validityMask); } } + result = currentDataSet.serialize(argStruct.buffer, argStruct.size, argStruct.max_size, argStruct.bigEndian); if (result != RETURN_OK) { - debug << "Pool Raw Access Helper: Error serializing pool data into send buffer" << std::endl; + debug << "Pool Raw Access Helper: Error serializing pool data with ID 0x" << std::hex << + currentPoolId << " into send buffer with return code " << result << std::dec << std::endl; return result; } + } return result; } +ReturnValue_t PoolRawAccessHelper::checkRemainingSize(PoolRawAccess * currentPoolRawAccess, + bool * isSerialized, uint8_t * arrayPosition) { + int8_t remainingSize = currentPoolRawAccess->getSizeTillEnd() - currentPoolRawAccess->getSizeOfType(); + if(remainingSize == 0) { + *isSerialized = true; + } + else if(remainingSize > 0) { + *arrayPosition += currentPoolRawAccess->getSizeOfType(); + } + else { + return RETURN_FAILED; + } + return RETURN_OK; +} + void PoolRawAccessHelper::handleMaskModification(uint8_t * validityMask) { validityMask[validBufferIndex] = bitSetter(validityMask[validBufferIndex], validBufferIndexBit, true); diff --git a/datapool/PoolRawAccessHelper.h b/datapool/PoolRawAccessHelper.h index e9e9c8ec..72f2324c 100644 --- a/datapool/PoolRawAccessHelper.h +++ b/datapool/PoolRawAccessHelper.h @@ -90,6 +90,8 @@ private: ReturnValue_t handlePoolEntrySerialization(uint32_t currentPoolId,SerializationArgs argStruct, bool withValidMask = false, uint8_t * validityMask = NULL); + ReturnValue_t checkRemainingSize(PoolRawAccess * currentPoolRawAccess, + bool * isSerialized, uint8_t * arrayPosition); void handleMaskModification(uint8_t * validityMask); /** * Sets specific bit of a byte