From e252a5b795147d11371514b9defad39d91a596ed Mon Sep 17 00:00:00 2001 From: "jakob.meier" Date: Thu, 26 Mar 2020 19:20:16 +0100 Subject: [PATCH 01/45] file system support --- memory/FileSystemMessage.cpp | 28 ++++++++++++++++++++++++++++ memory/FileSystemMessage.h | 30 ++++++++++++++++++++++++++++++ memory/HasFileSystemIF.h | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 94 insertions(+) create mode 100644 memory/FileSystemMessage.cpp create mode 100644 memory/FileSystemMessage.h create mode 100644 memory/HasFileSystemIF.h diff --git a/memory/FileSystemMessage.cpp b/memory/FileSystemMessage.cpp new file mode 100644 index 00000000..efbc8009 --- /dev/null +++ b/memory/FileSystemMessage.cpp @@ -0,0 +1,28 @@ +/* + * FileSystemMessage.cpp + * + * Created on: 19.01.2020 + * Author: Jakob Meier + */ + +#include "FileSystemMessage.h" +#include + +ReturnValue_t FileSystemMessage::setWriteToFileCommand(CommandMessage* message, + MessageQueueId_t replyQueueId, store_address_t storageID) { + message->setCommand(WRITE_TO_FILE); + message->setParameter(replyQueueId); + message->setParameter2(storageID.raw); + return HasReturnvaluesIF::RETURN_OK; +} + +store_address_t FileSystemMessage::getStoreID(const CommandMessage* message) { + store_address_t temp; + temp.raw = message->getParameter2(); + return temp; +} + +MessageQueueId_t FileSystemMessage::getReplyQueueId(const CommandMessage* message){ + return message->getParameter(); +} + diff --git a/memory/FileSystemMessage.h b/memory/FileSystemMessage.h new file mode 100644 index 00000000..b6fd7309 --- /dev/null +++ b/memory/FileSystemMessage.h @@ -0,0 +1,30 @@ +/* + * FileSystemMessage.h + * + * Created on: 19.01.2020 + * Author: Jakob Meier + */ + +#ifndef FRAMEWORK_MEMORY_FILESYSTEMMESSAGE_H_ +#define FRAMEWORK_MEMORY_FILESYSTEMMESSAGE_H_ + +#include +#include +#include + +class FileSystemMessage { +private: + FileSystemMessage(); //A private ctor inhibits instantiation +public: + static const uint8_t MESSAGE_ID = MESSAGE_TYPE::FILE_SYSTEM_MESSAGE; + static const Command_t CREATE_FILE = MAKE_COMMAND_ID( 0x01 ); + static const Command_t DELETE_FILE = MAKE_COMMAND_ID( 0x02 ); + static const Command_t WRITE_TO_FILE = MAKE_COMMAND_ID( 0x80 ); + + static ReturnValue_t setWriteToFileCommand(CommandMessage* message, MessageQueueId_t replyToQueue, store_address_t storageID ); + static store_address_t getStoreID( const CommandMessage* message ); + static MessageQueueId_t getReplyQueueId(const CommandMessage* message); + +}; + +#endif /* FRAMEWORK_MEMORY_FILESYSTEMMESSAGE_H_ */ diff --git a/memory/HasFileSystemIF.h b/memory/HasFileSystemIF.h new file mode 100644 index 00000000..8b66f88b --- /dev/null +++ b/memory/HasFileSystemIF.h @@ -0,0 +1,36 @@ +/* + * HasFileSystemIF.h + * + * Created on: 19.01.2020 + * Author: Jakob Meier + */ + +#ifndef FRAMEWORK_MEMORY_HASFILESYSTEMIF_H_ +#define FRAMEWORK_MEMORY_HASFILESYSTEMIF_H_ + +#include + +class HasFileSystemIF { +public: + + virtual ~HasFileSystemIF() {} + /** + * Function to get the MessageQueueId_t of the implementing object + * @return MessageQueueId_t of the object + */ + virtual MessageQueueId_t getCommandQueue() const = 0; + /** + * Function to write to a file + * @param dirname Directory of the file + * @param filename The filename of the file + * @param data The data to write to the file + * @param size The size of the data to write + * @param packetNumber Counts the number of packets. For large files the write procedure must be split in multiple calls to writeToFile + */ + virtual ReturnValue_t writeToFile(const char* dirname, char* filename, const uint8_t* data, uint32_t size, uint16_t packetNumber) = 0; + virtual ReturnValue_t createFile(const char* dirname, const char* filename, const uint8_t* data, uint32_t size) = 0; + virtual ReturnValue_t deleteFile(const char* dirname, const char* filename) = 0; +}; + + +#endif /* FRAMEWORK_MEMORY_HASFILESYSTEMIF_H_ */ From 0e56a094d305a35b80bdb20cc0c3cdcd533bd888 Mon Sep 17 00:00:00 2001 From: "jakob.meier" Date: Thu, 26 Mar 2020 19:43:38 +0100 Subject: [PATCH 02/45] merged FwMessageTypes --- ipc/FwMessageTypes.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ipc/FwMessageTypes.h b/ipc/FwMessageTypes.h index ec1c9aa2..07e3d245 100644 --- a/ipc/FwMessageTypes.h +++ b/ipc/FwMessageTypes.h @@ -14,7 +14,8 @@ enum FW_MESSAGE_TYPE { MONITORING, MEMORY, PARAMETER, - FW_MESSAGES_COUNT + FW_MESSAGES_COUNT, + FILE_SYSTEM_MESSAGE }; } From 58008c8db5c2310806833997f7fd5b354e0d433a Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 6 Apr 2020 11:19:05 +0200 Subject: [PATCH 03/45] all fixed slot sequence improvenements, freeRTOS fix --- devicehandlers/FixedSequenceSlot.h | 19 +++--- devicehandlers/FixedSlotSequence.cpp | 87 ++++++++++++++-------------- devicehandlers/FixedSlotSequence.h | 63 ++++++++++++-------- osal/FreeRTOS/FixedTimeslotTask.cpp | 23 +++++--- osal/FreeRTOS/FixedTimeslotTask.h | 1 + 5 files changed, 112 insertions(+), 81 deletions(-) diff --git a/devicehandlers/FixedSequenceSlot.h b/devicehandlers/FixedSequenceSlot.h index 5aff0f4f..b3bac08f 100644 --- a/devicehandlers/FixedSequenceSlot.h +++ b/devicehandlers/FixedSequenceSlot.h @@ -19,29 +19,34 @@ class PeriodicTaskIF; */ class FixedSequenceSlot { public: - FixedSequenceSlot( object_id_t handlerId, uint32_t setTimeMs, int8_t setSequenceId, PeriodicTaskIF* executingTask ); + FixedSequenceSlot( object_id_t handlerId, uint32_t setTimeMs, + int8_t setSequenceId, PeriodicTaskIF* executingTask ); virtual ~FixedSequenceSlot(); /** - * \brief \c handler identifies which device handler object is executed in this slot. + * @brief Handler identifies which device handler object is executed in this slot. */ ExecutableObjectIF* handler; /** - * \brief This attribute defines when a device handler object is executed. + * @brief This attribute defines when a device handler object is executed. * - * \details The pollingTime attribute identifies the time the handler is executed in ms. It must be - * smaller than the period length of the polling sequence, what is ensured by automated calculation - * from a database. + * @details The pollingTime attribute identifies the time the handler is executed in ms. + * It must be smaller than the period length of the polling sequence. */ uint32_t pollingTimeMs; /** * \brief This value defines the type of device communication. * - * \details The state of this value decides what communication routine is called in the PST executable or the device handler object. + * \details The state of this value decides what communication routine is + * called in the PST executable or the device handler object. */ uint8_t opcode; + + bool operator <(const FixedSequenceSlot & fixedSequenceSlot) const { + return pollingTimeMs < fixedSequenceSlot.pollingTimeMs; + } }; diff --git a/devicehandlers/FixedSlotSequence.cpp b/devicehandlers/FixedSlotSequence.cpp index a65dd929..23bb0e78 100644 --- a/devicehandlers/FixedSlotSequence.cpp +++ b/devicehandlers/FixedSlotSequence.cpp @@ -2,22 +2,23 @@ #include FixedSlotSequence::FixedSlotSequence(uint32_t setLengthMs) : - lengthMs(setLengthMs) { + slotLengthMs(setLengthMs) { current = slotList.begin(); } FixedSlotSequence::~FixedSlotSequence() { - std::list::iterator slotIt; - //Iterate through slotList and delete all entries. - slotIt = this->slotList.begin(); - while (slotIt != this->slotList.end()) { - delete (*slotIt); - slotIt++; - } + // This should call the destructor on each list entry. + slotList.clear(); +// SlotListIter slotListIter = this->slotList.begin(); +// //Iterate through slotList and delete all entries. +// while (slotListIter != this->slotList.end()) { +// delete (*slotIt); +// slotIt++; +// } } void FixedSlotSequence::executeAndAdvance() { - (*this->current)->handler->performOperation((*this->current)->opcode); + current->handler->performOperation(current->opcode); // if (returnValue != RETURN_OK) { // this->sendErrorMessage( returnValue ); // } @@ -31,53 +32,50 @@ void FixedSlotSequence::executeAndAdvance() { uint32_t FixedSlotSequence::getIntervalToNextSlotMs() { uint32_t oldTime; - std::list::iterator it; - it = current; + SlotListIter slotListIter = current; // Get the pollingTimeMs of the current slot object. - oldTime = (*it)->pollingTimeMs; + oldTime = slotListIter->pollingTimeMs; // Advance to the next object. - it++; + slotListIter++; // Find the next interval which is not 0. - while (it != slotList.end()) { - if (oldTime != (*it)->pollingTimeMs) { - return (*it)->pollingTimeMs - oldTime; + while (slotListIter != slotList.end()) { + if (oldTime != slotListIter->pollingTimeMs) { + return slotListIter->pollingTimeMs - oldTime; } else { - it++; + slotListIter++; } } // If the list end is reached (this is definitely an interval != 0), // the interval is calculated by subtracting the remaining time of the PST // and adding the start time of the first handler in the list. - it = slotList.begin(); - return lengthMs - oldTime + (*it)->pollingTimeMs; + slotListIter = slotList.begin(); + return slotLengthMs - oldTime + slotListIter->pollingTimeMs; } uint32_t FixedSlotSequence::getIntervalToPreviousSlotMs() { uint32_t currentTime; - std::list::iterator it; - it = current; + SlotListIter slotListIter = current; // Get the pollingTimeMs of the current slot object. - currentTime = (*it)->pollingTimeMs; + currentTime = slotListIter->pollingTimeMs; //if it is the first slot, calculate difference to last slot - if (it == slotList.begin()){ - return lengthMs - (*(--slotList.end()))->pollingTimeMs + currentTime; + if (slotListIter == slotList.begin()){ + return slotLengthMs - (--slotList.end())->pollingTimeMs + currentTime; } // get previous slot - it--; + slotListIter--; - return currentTime - (*it)->pollingTimeMs; + return currentTime - slotListIter->pollingTimeMs; } bool FixedSlotSequence::slotFollowsImmediately() { - uint32_t currentTime = (*current)->pollingTimeMs; - std::list::iterator it; - it = this->current; + uint32_t currentTime = current->pollingTimeMs; + SlotListIter fixedSequenceIter = this->current; // Get the pollingTimeMs of the current slot object. - if (it == slotList.begin()) + if (fixedSequenceIter == slotList.begin()) return false; - it--; - if ((*it)->pollingTimeMs == currentTime) { + fixedSequenceIter--; + if (fixedSequenceIter->pollingTimeMs == currentTime) { return true; } else { return false; @@ -85,31 +83,35 @@ bool FixedSlotSequence::slotFollowsImmediately() { } uint32_t FixedSlotSequence::getLengthMs() const { - return this->lengthMs; + return this->slotLengthMs; } ReturnValue_t FixedSlotSequence::checkSequence() const { - //Iterate through slotList and check successful creation. Checks if timing is ok (must be ascending) and if all handlers were found. + // Iterate through slotList and check successful creation. + // Checks if timing is ok (must be ascending) and if all handlers were found. auto slotIt = slotList.begin(); uint32_t count = 0; uint32_t time = 0; while (slotIt != slotList.end()) { - if ((*slotIt)->handler == NULL) { + if (slotIt->handler == NULL) { error << "FixedSlotSequene::initialize: ObjectId does not exist!" << std::endl; count++; - } else if ((*slotIt)->pollingTimeMs < time) { + } else if (slotIt->pollingTimeMs < time) { error << "FixedSlotSequence::initialize: Time: " - << (*slotIt)->pollingTimeMs + << slotIt->pollingTimeMs << " is smaller than previous with " << time << std::endl; count++; } else { - //All ok, print slot. -// (*slotIt)->print(); + // All ok, print slot. + //info << "Current slot polling time: " << std::endl; + //info << std::dec << slotIt->pollingTimeMs << std::endl; } - time = (*slotIt)->pollingTimeMs; + time = slotIt->pollingTimeMs; slotIt++; } + //info << "Number of elements in slot list: " + // << slotList.size() << std::endl; if (count > 0) { return HasReturnvaluesIF::RETURN_FAILED; } @@ -118,8 +120,7 @@ ReturnValue_t FixedSlotSequence::checkSequence() const { void FixedSlotSequence::addSlot(object_id_t componentId, uint32_t slotTimeMs, int8_t executionStep, PeriodicTaskIF* executingTask) { - this->slotList.push_back( - new FixedSequenceSlot(componentId, slotTimeMs, executionStep, - executingTask)); + this->slotList.insert(FixedSequenceSlot(componentId, slotTimeMs, executionStep, + executingTask)); this->current = slotList.begin(); } diff --git a/devicehandlers/FixedSlotSequence.h b/devicehandlers/FixedSlotSequence.h index f8fd9a36..813d2fce 100644 --- a/devicehandlers/FixedSlotSequence.h +++ b/devicehandlers/FixedSlotSequence.h @@ -4,11 +4,15 @@ #include #include #include +#include + +using SlotList = std::multiset; +using SlotListIter = std::multiset::iterator; /** - * \brief This class is the representation of a Polling Sequence Table in software. + * @brief This class is the representation of a Polling Sequence Table in software. * - * \details The FixedSlotSequence object maintains the dynamic execution of device handler objects. + * @details The FixedSlotSequence object maintains the dynamic execution of device handler objects. * The main idea is to create a list of device handlers, to announce all handlers to the * polling sequence and to maintain a list of polling slot objects. This slot list represents the * Polling Sequence Table in software. Each polling slot contains information to indicate when and @@ -19,29 +23,37 @@ class FixedSlotSequence { public: + /** - * \brief The constructor of the FixedSlotSequence object. + * @brief The constructor of the FixedSlotSequence object. * - * \details The constructor takes two arguments, the period length and the init function. + * @details The constructor takes two arguments, the period length and the init function. * - * \param setLength The period length, expressed in ms. + * @param setLength The period length, expressed in ms. */ FixedSlotSequence(uint32_t setLengthMs); /** - * \brief The destructor of the FixedSlotSequence object. + * @brief The destructor of the FixedSlotSequence object. * - * \details The destructor frees all allocated memory by iterating through the slotList + * @details The destructor frees all allocated memory by iterating through the slotList * and deleting all allocated resources. */ virtual ~FixedSlotSequence(); /** - * \brief This is a method to add an PollingSlot object to slotList. + * @brief This is a method to add an PollingSlot object to slotList. * - * \details Here, a polling slot object is added to the slot list. It is appended + * @details Here, a polling slot object is added to the slot list. It is appended * to the end of the list. The list is currently NOT reordered. * Afterwards, the iterator current is set to the beginning of the list. + * @param Object ID of the object to add + * @param setTime Value between (0 to 1) * slotLengthMs, when a FixedTimeslotTask + * will be called inside the slot period. + * @param setSequenceId ID which can be used to distinguish + * different task operations + * @param + * @param */ void addSlot(object_id_t handlerId, uint32_t setTime, int8_t setSequenceId, PeriodicTaskIF* executingTask); @@ -57,11 +69,14 @@ public: /** * \brief This method returns the time until the next software component is invoked. * - * \details This method is vitally important for the operation of the PST. By fetching the polling time - * of the current slot and that of the next one (or the first one, if the list end is reached) - * it calculates and returns the interval in milliseconds within which the handler execution - * shall take place. If the next slot has the same time as the current one, it is ignored until - * a slot with different time or the end of the PST is found. + * \details + * This method is vitally important for the operation of the PST. + * By fetching the polling time of the current slot and that of the + * next one (or the first one, if the list end is reached) + * it calculates and returns the interval in milliseconds within + * which the handler execution shall take place. + * If the next slot has the same time as the current one, it is ignored + * until a slot with different time or the end of the PST is found. */ uint32_t getIntervalToNextSlotMs(); @@ -75,12 +90,12 @@ public: uint32_t getIntervalToPreviousSlotMs(); /** - * \brief This method returns the length of this FixedSlotSequence instance. + * @brief This method returns the length of this FixedSlotSequence instance. */ uint32_t getLengthMs() const; /** - * \brief The method to execute the device handler entered in the current OPUSPollingSlot object. + * \brief The method to execute the device handler entered in the current PollingSlot object. * * \details Within this method the device handler object to be executed is chosen by looking up the * handler address of the current slot in the handlerMap. Either the device handler's @@ -91,27 +106,27 @@ public: void executeAndAdvance(); /** - * \brief An iterator that indicates the current polling slot to execute. + * @brief An iterator that indicates the current polling slot to execute. * - * \details This is an iterator for slotList and always points to the polling slot which is executed next. + * @details This is an iterator for slotList and always points to the polling slot which is executed next. */ - std::list::iterator current; + SlotListIter current; - ReturnValue_t checkSequence() const; + virtual ReturnValue_t checkSequence() const; protected: /** - * \brief This list contains all OPUSPollingSlot objects, defining order and execution time of the + * @brief This list contains all PollingSlot objects, defining order and execution time of the * device handler objects. * - * \details The slot list is a std:list object that contains all created OPUSPollingSlot instances. + * @details The slot list is a std:list object that contains all created PollingSlot instances. * They are NOT ordered automatically, so by adding entries, the correct order needs to be ensured. * By iterating through this list the polling sequence is executed. Two entries with identical * polling times are executed immediately one after another. */ - std::list slotList; + SlotList slotList; - uint32_t lengthMs; + uint32_t slotLengthMs; }; #endif /* FIXEDSLOTSEQUENCE_H_ */ diff --git a/osal/FreeRTOS/FixedTimeslotTask.cpp b/osal/FreeRTOS/FixedTimeslotTask.cpp index 00fd73d3..b1c97420 100644 --- a/osal/FreeRTOS/FixedTimeslotTask.cpp +++ b/osal/FreeRTOS/FixedTimeslotTask.cpp @@ -19,8 +19,7 @@ FixedTimeslotTask::~FixedTimeslotTask() { void FixedTimeslotTask::taskEntryPoint(void* argument) { //The argument is re-interpreted as FixedTimeslotTask. The Task object is global, so it is found from any place. - FixedTimeslotTask *originalTask( - reinterpret_cast(argument)); + FixedTimeslotTask *originalTask(reinterpret_cast(argument)); // Task should not start until explicitly requested // in FreeRTOS, tasks start as soon as they are created if the scheduler is running // but not if the scheduler is not running. @@ -58,8 +57,19 @@ ReturnValue_t FixedTimeslotTask::startTask() { ReturnValue_t FixedTimeslotTask::addSlot(object_id_t componentId, uint32_t slotTimeMs, int8_t executionStep) { - pst.addSlot(componentId, slotTimeMs, executionStep, this); - return HasReturnvaluesIF::RETURN_OK; + if (objectManager->get(componentId) != NULL) { + if(slotTimeMs == 0) { + // TODO: FreeRTOS throws errors for zero values. + // maybe there is a better solution than this. + slotTimeMs = 1; + } + pst.addSlot(componentId, slotTimeMs, executionStep, this); + return HasReturnvaluesIF::RETURN_OK; + } + + error << "Component " << std::hex << componentId + << " not found, not adding it to pst" << std::endl; + return HasReturnvaluesIF::RETURN_FAILED; } uint32_t FixedTimeslotTask::getPeriodMs() const { @@ -72,10 +82,10 @@ ReturnValue_t FixedTimeslotTask::checkSequence() const { void FixedTimeslotTask::taskFunctionality() { // A local iterator for the Polling Sequence Table is created to find the start time for the first entry. - std::list::iterator it = pst.current; + SlotListIter slotListIter = pst.current; //The start time for the first entry is read. - uint32_t intervalMs = (*it)->pollingTimeMs; + uint32_t intervalMs = slotListIter->pollingTimeMs; TickType_t interval = pdMS_TO_TICKS(intervalMs); TickType_t xLastWakeTime; @@ -110,4 +120,3 @@ ReturnValue_t FixedTimeslotTask::sleepFor(uint32_t ms) { vTaskDelay(pdMS_TO_TICKS(ms)); return HasReturnvaluesIF::RETURN_OK; } - diff --git a/osal/FreeRTOS/FixedTimeslotTask.h b/osal/FreeRTOS/FixedTimeslotTask.h index bed13051..1ab8724f 100644 --- a/osal/FreeRTOS/FixedTimeslotTask.h +++ b/osal/FreeRTOS/FixedTimeslotTask.h @@ -51,6 +51,7 @@ public: ReturnValue_t checkSequence() const; ReturnValue_t sleepFor(uint32_t ms); + protected: bool started; TaskHandle_t handle; From cee8b4dc37eb897fa2bedd44c07fd26781cc0c38 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Sun, 19 Apr 2020 21:37:55 +0200 Subject: [PATCH 04/45] name slotLengthMs reverted to lengthMs --- devicehandlers/FixedSlotSequence.cpp | 8 ++++---- devicehandlers/FixedSlotSequence.h | 3 +-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/devicehandlers/FixedSlotSequence.cpp b/devicehandlers/FixedSlotSequence.cpp index 23bb0e78..07439e5d 100644 --- a/devicehandlers/FixedSlotSequence.cpp +++ b/devicehandlers/FixedSlotSequence.cpp @@ -2,7 +2,7 @@ #include FixedSlotSequence::FixedSlotSequence(uint32_t setLengthMs) : - slotLengthMs(setLengthMs) { + lengthMs(setLengthMs) { current = slotList.begin(); } @@ -49,7 +49,7 @@ uint32_t FixedSlotSequence::getIntervalToNextSlotMs() { // the interval is calculated by subtracting the remaining time of the PST // and adding the start time of the first handler in the list. slotListIter = slotList.begin(); - return slotLengthMs - oldTime + slotListIter->pollingTimeMs; + return lengthMs - oldTime + slotListIter->pollingTimeMs; } uint32_t FixedSlotSequence::getIntervalToPreviousSlotMs() { @@ -60,7 +60,7 @@ uint32_t FixedSlotSequence::getIntervalToPreviousSlotMs() { //if it is the first slot, calculate difference to last slot if (slotListIter == slotList.begin()){ - return slotLengthMs - (--slotList.end())->pollingTimeMs + currentTime; + return lengthMs - (--slotList.end())->pollingTimeMs + currentTime; } // get previous slot slotListIter--; @@ -83,7 +83,7 @@ bool FixedSlotSequence::slotFollowsImmediately() { } uint32_t FixedSlotSequence::getLengthMs() const { - return this->slotLengthMs; + return this->lengthMs; } ReturnValue_t FixedSlotSequence::checkSequence() const { diff --git a/devicehandlers/FixedSlotSequence.h b/devicehandlers/FixedSlotSequence.h index 813d2fce..a198421f 100644 --- a/devicehandlers/FixedSlotSequence.h +++ b/devicehandlers/FixedSlotSequence.h @@ -23,7 +23,6 @@ using SlotListIter = std::multiset::iterator; class FixedSlotSequence { public: - /** * @brief The constructor of the FixedSlotSequence object. * @@ -126,7 +125,7 @@ protected: */ SlotList slotList; - uint32_t slotLengthMs; + uint32_t lengthMs; }; #endif /* FIXEDSLOTSEQUENCE_H_ */ From 88f229cef924fcb45c46b8d6e002c3d8142f60d8 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Thu, 23 Apr 2020 15:03:55 +0200 Subject: [PATCH 05/45] fifo enhancement --- container/FIFO.h | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/container/FIFO.h b/container/FIFO.h index 134da9b8..889d2ade 100644 --- a/container/FIFO.h +++ b/container/FIFO.h @@ -3,6 +3,11 @@ #include +/** + * @brief Simple First-In-First-Out data structure + * @tparam T Entry Type + * @tparam capacity Maximum capacity + */ template class FIFO { private: @@ -54,6 +59,26 @@ public: return HasReturnvaluesIF::RETURN_OK; } } + + ReturnValue_t peek(T * value) { + if(empty()) { + return EMPTY; + } else { + *value = data[readIndex]; + return HasReturnvaluesIF::RETURN_OK; + } + } + + ReturnValue_t pop() { + if(empty()) { + return EMPTY; + } else { + readIndex = next(readIndex); + --currentSize; + return HasReturnvaluesIF::RETURN_OK; + } + } + static const uint8_t INTERFACE_ID = CLASS_ID::FIFO_CLASS; static const ReturnValue_t FULL = MAKE_RETURN_CODE(1); static const ReturnValue_t EMPTY = MAKE_RETURN_CODE(2); From 4b65d6e84739ea13e82441768917c530154d656f Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 4 May 2020 12:33:57 +0200 Subject: [PATCH 06/45] local pool bugfix --- storagemanager/LocalPool.h | 5 +++-- storagemanager/PoolManager.h | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/storagemanager/LocalPool.h b/storagemanager/LocalPool.h index 7e934c40..112f0111 100644 --- a/storagemanager/LocalPool.h +++ b/storagemanager/LocalPool.h @@ -125,7 +125,8 @@ protected: * @return - #RETURN_OK on success, * - the return codes of #getPoolIndex or #findEmpty otherwise. */ - virtual ReturnValue_t reserveSpace(const uint32_t size, store_address_t* address, bool ignoreFault); + virtual ReturnValue_t reserveSpace(const uint32_t size, + store_address_t* address, bool ignoreFault); InternalErrorReporterIF *internalErrorReporter; private: @@ -292,7 +293,7 @@ inline ReturnValue_t LocalPool::reserveSpace( size_list[address->pool_index][address->packet_index] = size; } else { - if (!ignoreFault) { + if (!ignoreFault and internalErrorReporter != nullptr) { internalErrorReporter->storeFull(); } sif::error << "LocalPool( " << std::hex << getObjectId() << std::dec diff --git a/storagemanager/PoolManager.h b/storagemanager/PoolManager.h index 68a7addc..329235d5 100644 --- a/storagemanager/PoolManager.h +++ b/storagemanager/PoolManager.h @@ -57,7 +57,8 @@ inline ReturnValue_t PoolManager::reserveSpace(const uint32_t s template inline PoolManager::PoolManager(object_id_t setObjectId, const uint16_t element_sizes[NUMBER_OF_POOLS], - const uint16_t n_elements[NUMBER_OF_POOLS]) : LocalPool(setObjectId, element_sizes, n_elements, true) { + const uint16_t n_elements[NUMBER_OF_POOLS]) : + LocalPool(setObjectId, element_sizes, n_elements, true) { mutex = MutexFactory::instance()->createMutex(); } From b947253ac3107e05ccb85760bb0413d28c46b5c5 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 4 May 2020 16:49:15 +0200 Subject: [PATCH 07/45] local pool neat --- storagemanager/LocalPool.h | 313 ++---------------------------- storagemanager/LocalPool.tpp | 260 +++++++++++++++++++++++++ storagemanager/StorageManagerIF.h | 21 +- 3 files changed, 287 insertions(+), 307 deletions(-) create mode 100644 storagemanager/LocalPool.tpp diff --git a/storagemanager/LocalPool.h b/storagemanager/LocalPool.h index 112f0111..a6334458 100644 --- a/storagemanager/LocalPool.h +++ b/storagemanager/LocalPool.h @@ -1,15 +1,6 @@ #ifndef FRAMEWORK_STORAGEMANAGER_LOCALPOOL_H_ #define FRAMEWORK_STORAGEMANAGER_LOCALPOOL_H_ -/** - * @file LocalPool - * - * @date 02.02.2012 - * @author Bastian Baetz - * - * @brief This file contains the definition of the LocalPool class. - */ - #include #include #include @@ -70,53 +61,21 @@ public: virtual ~LocalPool(void); /** - * Add data to local data pool, performs range check - * @param storageId [out] Store ID in which the data will be stored - * @param data - * @param size - * @param ignoreFault - * @return @c RETURN_OK if write was successful + * Documentation: See StorageManagerIF.h */ ReturnValue_t addData(store_address_t* storageId, const uint8_t * data, - uint32_t size, bool ignoreFault = false); - - /** - * With this helper method, a free element of \c size is reserved. - * @param storageId [out] storeID of the free element - * @param size The minimum packet size that shall be reserved. - * @param p_data [out] pointer to the pointer of free element - * @param ignoreFault - * @return Returns the storage identifier within the storage or - * StorageManagerIF::INVALID_ADDRESS (in raw). - */ - ReturnValue_t getFreeElement(store_address_t* storageId, - const uint32_t size, uint8_t** p_data, bool ignoreFault = false); - - /** - * Retrieve data from local pool - * @param packet_id - * @param packet_ptr - * @param size [out] Size of retrieved data - * @return @c RETURN_OK if data retrieval was successfull - */ + size_t size, bool ignoreFault = false) override; + ReturnValue_t getFreeElement(store_address_t* storageId,const size_t size, + uint8_t** p_data, bool ignoreFault = false) override; ReturnValue_t getData(store_address_t packet_id, const uint8_t** packet_ptr, - size_t * size); - - /** - * Modify data by supplying a packet pointer and using that packet pointer - * to access and modify the pool entry (via *pointer call) - * @param packet_id Store ID of data to modify - * @param packet_ptr [out] pointer to the pool entry to modify - * @param size [out] size of pool entry - * @return - */ + size_t * size) override; ReturnValue_t modifyData(store_address_t packet_id, uint8_t** packet_ptr, - size_t * size); - virtual ReturnValue_t deleteData(store_address_t); - virtual ReturnValue_t deleteData(uint8_t* ptr, uint32_t size, - store_address_t* storeId = NULL); - void clearStore(); - ReturnValue_t initialize(); + size_t * size) override; + virtual ReturnValue_t deleteData(store_address_t) override; + virtual ReturnValue_t deleteData(uint8_t* ptr, size_t size, + store_address_t* storeId = NULL) override; + void clearStore() override; + ReturnValue_t initialize() override; protected: /** * With this helper method, a free element of \c size is reserved. @@ -167,7 +126,7 @@ private: * @param data The data to be stored. * @param size The size of the data to be stored. */ - void write(store_address_t packet_id, const uint8_t* data, uint32_t size); + void write(store_address_t packet_id, const uint8_t* data, size_t size); /** * @brief A helper method to read the element size of a certain pool. * @param pool_index The pool in which to look. @@ -190,7 +149,8 @@ private: * @return - #RETURN_OK on success, * - #DATA_TOO_LARGE otherwise. */ - ReturnValue_t getPoolIndex(uint32_t packet_size, uint16_t* poolIndex, uint16_t startAtIndex = 0); + ReturnValue_t getPoolIndex(size_t packet_size, uint16_t* poolIndex, + uint16_t startAtIndex = 0); /** * @brief This helper method calculates the true array position in store * of a given packet id. @@ -212,249 +172,6 @@ private: ReturnValue_t findEmpty(uint16_t pool_index, uint16_t* element); }; -template -inline ReturnValue_t LocalPool::findEmpty(uint16_t pool_index, - uint16_t* element) { - ReturnValue_t status = DATA_STORAGE_FULL; - for (uint16_t foundElement = 0; foundElement < n_elements[pool_index]; - foundElement++) { - if (size_list[pool_index][foundElement] == STORAGE_FREE) { - *element = foundElement; - status = RETURN_OK; - break; - } - } - return status; -} - -template -inline void LocalPool::write(store_address_t packet_id, - const uint8_t* data, uint32_t size) { - uint8_t* ptr; - uint32_t packet_position = getRawPosition(packet_id); - - //check size? -> Not necessary, because size is checked before calling this function. - ptr = &store[packet_id.pool_index][packet_position]; - memcpy(ptr, data, size); - size_list[packet_id.pool_index][packet_id.packet_index] = size; -} - -//Returns page size of 0 in case store_index is illegal -template -inline uint32_t LocalPool::getPageSize(uint16_t pool_index) { - if (pool_index < NUMBER_OF_POOLS) { - return element_sizes[pool_index]; - } else { - return 0; - } -} - -template -inline ReturnValue_t LocalPool::getPoolIndex( - uint32_t packet_size, uint16_t* poolIndex, uint16_t startAtIndex) { - for (uint16_t n = startAtIndex; n < NUMBER_OF_POOLS; n++) { -// debug << "LocalPool " << getObjectId() << "::getPoolIndex: Pool: " << n << ", Element Size: " << element_sizes[n] << std::endl; - if (element_sizes[n] >= packet_size) { - *poolIndex = n; - return RETURN_OK; - } - } - return DATA_TOO_LARGE; -} - -template -inline uint32_t LocalPool::getRawPosition( - store_address_t packet_id) { - return packet_id.packet_index * element_sizes[packet_id.pool_index]; -} - -template -inline ReturnValue_t LocalPool::reserveSpace( - const uint32_t size, store_address_t* address, bool ignoreFault) { - ReturnValue_t status = getPoolIndex(size, &address->pool_index); - if (status != RETURN_OK) { - sif::error << "LocalPool( " << std::hex << getObjectId() << std::dec - << " )::reserveSpace: Packet too large." << std::endl; - return status; - } - status = findEmpty(address->pool_index, &address->packet_index); - while (status != RETURN_OK && spillsToHigherPools) { - status = getPoolIndex(size, &address->pool_index, address->pool_index + 1); - if (status != RETURN_OK) { - //We don't find any fitting pool anymore. - break; - } - status = findEmpty(address->pool_index, &address->packet_index); - } - if (status == RETURN_OK) { -// if (getObjectId() == objects::IPC_STORE && address->pool_index >= 3) { -// debug << "Reserve: Pool: " << std::dec << address->pool_index << " Index: " << address->packet_index << std::endl; -// } - - size_list[address->pool_index][address->packet_index] = size; - } else { - if (!ignoreFault and internalErrorReporter != nullptr) { - internalErrorReporter->storeFull(); - } - sif::error << "LocalPool( " << std::hex << getObjectId() << std::dec - << " )::reserveSpace: Packet store is full." << std::endl; - } - return status; -} - -template -inline LocalPool::LocalPool(object_id_t setObjectId, - const uint16_t element_sizes[NUMBER_OF_POOLS], - const uint16_t n_elements[NUMBER_OF_POOLS], bool registered, bool spillsToHigherPools) : - SystemObject(setObjectId, registered), internalErrorReporter(NULL), spillsToHigherPools(spillsToHigherPools){ - for (uint16_t n = 0; n < NUMBER_OF_POOLS; n++) { - this->element_sizes[n] = element_sizes[n]; - this->n_elements[n] = n_elements[n]; - store[n] = new uint8_t[n_elements[n] * element_sizes[n]]; - size_list[n] = new uint32_t[n_elements[n]]; - memset(store[n], 0x00, (n_elements[n] * element_sizes[n])); - memset(size_list[n], STORAGE_FREE, (n_elements[n] * sizeof(**size_list))); //TODO checkme - } -} - -template -inline LocalPool::~LocalPool(void) { - for (uint16_t n = 0; n < NUMBER_OF_POOLS; n++) { - delete[] store[n]; - delete[] size_list[n]; - } -} - -template -inline ReturnValue_t LocalPool::addData( - store_address_t* storageId, const uint8_t* data, uint32_t size, bool ignoreFault) { - ReturnValue_t status = reserveSpace(size, storageId, ignoreFault); - if (status == RETURN_OK) { - write(*storageId, data, size); - } - return status; -} - -template -inline ReturnValue_t LocalPool::getFreeElement( - store_address_t* storageId, const uint32_t size, uint8_t** p_data, bool ignoreFault) { - ReturnValue_t status = reserveSpace(size, storageId, ignoreFault); - if (status == RETURN_OK) { - *p_data = &store[storageId->pool_index][getRawPosition(*storageId)]; - } else { - *p_data = NULL; - } - return status; -} - -template -inline ReturnValue_t LocalPool::getData( - store_address_t packet_id, const uint8_t** packet_ptr, size_t * size) { - uint8_t* tempData = NULL; - ReturnValue_t status = modifyData(packet_id, &tempData, size); - *packet_ptr = tempData; - return status; -} - -template -inline ReturnValue_t LocalPool::modifyData(store_address_t packet_id, - uint8_t** packet_ptr, size_t * size) { - ReturnValue_t status = RETURN_FAILED; - if (packet_id.pool_index >= NUMBER_OF_POOLS) { - return ILLEGAL_STORAGE_ID; - } - if ((packet_id.packet_index >= n_elements[packet_id.pool_index])) { - return ILLEGAL_STORAGE_ID; - } - if (size_list[packet_id.pool_index][packet_id.packet_index] - != STORAGE_FREE) { - uint32_t packet_position = getRawPosition(packet_id); - *packet_ptr = &store[packet_id.pool_index][packet_position]; - *size = size_list[packet_id.pool_index][packet_id.packet_index]; - status = RETURN_OK; - } else { - status = DATA_DOES_NOT_EXIST; - } - return status; -} - -template -inline ReturnValue_t LocalPool::deleteData( - store_address_t packet_id) { - -// if (getObjectId() == objects::IPC_STORE && packet_id.pool_index >= 3) { -// debug << "Delete: Pool: " << std::dec << packet_id.pool_index << " Index: " << packet_id.packet_index << std::endl; -// } - ReturnValue_t status = RETURN_OK; - uint32_t page_size = getPageSize(packet_id.pool_index); - if ((page_size != 0) - && (packet_id.packet_index < n_elements[packet_id.pool_index])) { - uint16_t packet_position = getRawPosition(packet_id); - uint8_t* ptr = &store[packet_id.pool_index][packet_position]; - memset(ptr, 0, page_size); - //Set free list - size_list[packet_id.pool_index][packet_id.packet_index] = STORAGE_FREE; - } else { - //pool_index or packet_index is too large - sif::error << "LocalPool:deleteData failed." << std::endl; - status = ILLEGAL_STORAGE_ID; - } - return status; -} - -template -inline void LocalPool::clearStore() { - for (uint16_t n = 0; n < NUMBER_OF_POOLS; n++) { - memset(size_list[n], STORAGE_FREE, (n_elements[n] * sizeof(**size_list)));//TODO checkme - } -} - -template -inline ReturnValue_t LocalPool::deleteData(uint8_t* ptr, - uint32_t size, store_address_t* storeId) { - store_address_t localId; - ReturnValue_t result = ILLEGAL_ADDRESS; - for (uint16_t n = 0; n < NUMBER_OF_POOLS; n++) { - //Not sure if new allocates all stores in order. so better be careful. - if ((store[n] <= ptr) && (&store[n][n_elements[n]*element_sizes[n]]) > ptr) { - localId.pool_index = n; - uint32_t deltaAddress = ptr - store[n]; - //Getting any data from the right "block" is ok. This is necessary, as IF's sometimes don't point to the first element of an object. - localId.packet_index = deltaAddress / element_sizes[n]; - result = deleteData(localId); -// if (deltaAddress % element_sizes[n] != 0) { -// error << "Pool::deleteData: address not aligned!" << std::endl; -// } - break; - } - } - if (storeId != NULL) { - *storeId = localId; - } - return result; -} - -template -inline ReturnValue_t LocalPool::initialize() { - ReturnValue_t result = SystemObject::initialize(); - if (result != RETURN_OK) { - return result; - } - internalErrorReporter = objectManager->get(objects::INTERNAL_ERROR_REPORTER); - if (internalErrorReporter == NULL){ - return RETURN_FAILED; - } - - //Check if any pool size is large than the maximum allowed. - for (uint8_t count = 0; count < NUMBER_OF_POOLS; count++) { - if (element_sizes[count] >= STORAGE_FREE) { - sif::error - << "LocalPool::initialize: Pool is too large! Max. allowed size is: " - << (STORAGE_FREE - 1) << std::endl; - return RETURN_FAILED; - } - } - return RETURN_OK; -} +#include #endif /* FRAMEWORK_STORAGEMANAGER_LOCALPOOL_H_ */ diff --git a/storagemanager/LocalPool.tpp b/storagemanager/LocalPool.tpp new file mode 100644 index 00000000..5cead166 --- /dev/null +++ b/storagemanager/LocalPool.tpp @@ -0,0 +1,260 @@ +#ifndef LOCALPOOL_TPP +#define LOCALPOOL_TPP + +template +inline LocalPool::LocalPool(object_id_t setObjectId, + const uint16_t element_sizes[NUMBER_OF_POOLS], + const uint16_t n_elements[NUMBER_OF_POOLS], bool registered, + bool spillsToHigherPools) : + SystemObject(setObjectId, registered), internalErrorReporter(nullptr), + spillsToHigherPools(spillsToHigherPools) +{ + for (uint16_t n = 0; n < NUMBER_OF_POOLS; n++) { + this->element_sizes[n] = element_sizes[n]; + this->n_elements[n] = n_elements[n]; + store[n] = new uint8_t[n_elements[n] * element_sizes[n]]; + size_list[n] = new uint32_t[n_elements[n]]; + memset(store[n], 0x00, (n_elements[n] * element_sizes[n])); + //TODO checkme + memset(size_list[n], STORAGE_FREE, (n_elements[n] * sizeof(**size_list))); + } +} + + +template +inline ReturnValue_t LocalPool::findEmpty(uint16_t pool_index, + uint16_t* element) { + ReturnValue_t status = DATA_STORAGE_FULL; + for (uint16_t foundElement = 0; foundElement < n_elements[pool_index]; + foundElement++) { + if (size_list[pool_index][foundElement] == STORAGE_FREE) { + *element = foundElement; + status = RETURN_OK; + break; + } + } + return status; +} + +template +inline void LocalPool::write(store_address_t packet_id, + const uint8_t* data, size_t size) { + uint8_t* ptr; + uint32_t packet_position = getRawPosition(packet_id); + + //check size? -> Not necessary, because size is checked before calling this function. + ptr = &store[packet_id.pool_index][packet_position]; + memcpy(ptr, data, size); + size_list[packet_id.pool_index][packet_id.packet_index] = size; +} + +//Returns page size of 0 in case store_index is illegal +template +inline uint32_t LocalPool::getPageSize(uint16_t pool_index) { + if (pool_index < NUMBER_OF_POOLS) { + return element_sizes[pool_index]; + } else { + return 0; + } +} + +template +inline ReturnValue_t LocalPool::getPoolIndex( + size_t packet_size, uint16_t* poolIndex, uint16_t startAtIndex) { + for (uint16_t n = startAtIndex; n < NUMBER_OF_POOLS; n++) { + //debug << "LocalPool " << getObjectId() << "::getPoolIndex: Pool: " << + // n << ", Element Size: " << element_sizes[n] << std::endl; + if (element_sizes[n] >= packet_size) { + *poolIndex = n; + return RETURN_OK; + } + } + return DATA_TOO_LARGE; +} + +template +inline uint32_t LocalPool::getRawPosition( + store_address_t packet_id) { + return packet_id.packet_index * element_sizes[packet_id.pool_index]; +} + +template +inline ReturnValue_t LocalPool::reserveSpace( + const uint32_t size, store_address_t* address, bool ignoreFault) { + ReturnValue_t status = getPoolIndex(size, &address->pool_index); + if (status != RETURN_OK) { + sif::error << "LocalPool( " << std::hex << getObjectId() << std::dec + << " )::reserveSpace: Packet too large." << std::endl; + return status; + } + status = findEmpty(address->pool_index, &address->packet_index); + while (status != RETURN_OK && spillsToHigherPools) { + status = getPoolIndex(size, &address->pool_index, address->pool_index + 1); + if (status != RETURN_OK) { + //We don't find any fitting pool anymore. + break; + } + status = findEmpty(address->pool_index, &address->packet_index); + } + if (status == RETURN_OK) { + // if (getObjectId() == objects::IPC_STORE && address->pool_index >= 3) { + // debug << "Reserve: Pool: " << std::dec << address->pool_index << + // " Index: " << address->packet_index << std::endl; + // } + + size_list[address->pool_index][address->packet_index] = size; + } else { + if (!ignoreFault and internalErrorReporter != nullptr) { + internalErrorReporter->storeFull(); + } + // error << "LocalPool( " << std::hex << getObjectId() << std::dec + // << " )::reserveSpace: Packet store is full." << std::endl; + } + return status; +} + +template +inline LocalPool::~LocalPool(void) { + for (uint16_t n = 0; n < NUMBER_OF_POOLS; n++) { + delete[] store[n]; + delete[] size_list[n]; + } +} + +template +inline ReturnValue_t LocalPool::addData(store_address_t* storageId, + const uint8_t* data, size_t size, bool ignoreFault) { + ReturnValue_t status = reserveSpace(size, storageId, ignoreFault); + if (status == RETURN_OK) { + write(*storageId, data, size); + } + return status; +} + +template +inline ReturnValue_t LocalPool::getFreeElement( + store_address_t* storageId, const size_t size, + uint8_t** p_data, bool ignoreFault) { + ReturnValue_t status = reserveSpace(size, storageId, ignoreFault); + if (status == RETURN_OK) { + *p_data = &store[storageId->pool_index][getRawPosition(*storageId)]; + } else { + *p_data = NULL; + } + return status; +} + +template +inline ReturnValue_t LocalPool::getData( + store_address_t packet_id, const uint8_t** packet_ptr, size_t* size) { + uint8_t* tempData = NULL; + ReturnValue_t status = modifyData(packet_id, &tempData, size); + *packet_ptr = tempData; + return status; +} + +template +inline ReturnValue_t LocalPool::modifyData( + store_address_t packet_id, uint8_t** packet_ptr, size_t* size) { + ReturnValue_t status = RETURN_FAILED; + if (packet_id.pool_index >= NUMBER_OF_POOLS) { + return ILLEGAL_STORAGE_ID; + } + if ((packet_id.packet_index >= n_elements[packet_id.pool_index])) { + return ILLEGAL_STORAGE_ID; + } + if (size_list[packet_id.pool_index][packet_id.packet_index] + != STORAGE_FREE) { + uint32_t packet_position = getRawPosition(packet_id); + *packet_ptr = &store[packet_id.pool_index][packet_position]; + *size = size_list[packet_id.pool_index][packet_id.packet_index]; + status = RETURN_OK; + } else { + status = DATA_DOES_NOT_EXIST; + } + return status; +} + +template +inline ReturnValue_t LocalPool::deleteData( + store_address_t packet_id) { + //if (getObjectId() == objects::IPC_STORE && packet_id.pool_index >= 3) { + // debug << "Delete: Pool: " << std::dec << packet_id.pool_index << " Index: " + // << packet_id.packet_index << std::endl; + //} + ReturnValue_t status = RETURN_OK; + uint32_t page_size = getPageSize(packet_id.pool_index); + if ((page_size != 0) + && (packet_id.packet_index < n_elements[packet_id.pool_index])) { + uint16_t packet_position = getRawPosition(packet_id); + uint8_t* ptr = &store[packet_id.pool_index][packet_position]; + memset(ptr, 0, page_size); + //Set free list + size_list[packet_id.pool_index][packet_id.packet_index] = STORAGE_FREE; + } else { + //pool_index or packet_index is too large + sif::error << "LocalPool:deleteData failed." << std::endl; + status = ILLEGAL_STORAGE_ID; + } + return status; +} + +template +inline void LocalPool::clearStore() { + for (uint16_t n = 0; n < NUMBER_OF_POOLS; n++) { + //TODO checkme + memset(size_list[n], STORAGE_FREE, (n_elements[n] * sizeof(**size_list))); + } +} + +template +inline ReturnValue_t LocalPool::deleteData(uint8_t* ptr, + size_t size, store_address_t* storeId) { + store_address_t localId; + ReturnValue_t result = ILLEGAL_ADDRESS; + for (uint16_t n = 0; n < NUMBER_OF_POOLS; n++) { + //Not sure if new allocates all stores in order. so better be careful. + if ((store[n] <= ptr) && (&store[n][n_elements[n]*element_sizes[n]]) > ptr) { + localId.pool_index = n; + uint32_t deltaAddress = ptr - store[n]; + // Getting any data from the right "block" is ok. + // This is necessary, as IF's sometimes don't point to the first + // element of an object. + localId.packet_index = deltaAddress / element_sizes[n]; + result = deleteData(localId); + //if (deltaAddress % element_sizes[n] != 0) { + // error << "Pool::deleteData: address not aligned!" << std::endl; + //} + break; + } + } + if (storeId != NULL) { + *storeId = localId; + } + return result; +} + +template +inline ReturnValue_t LocalPool::initialize() { + ReturnValue_t result = SystemObject::initialize(); + if (result != RETURN_OK) { + return result; + } + internalErrorReporter = objectManager->get( + objects::INTERNAL_ERROR_REPORTER); + if (internalErrorReporter == NULL){ + return RETURN_FAILED; + } + + //Check if any pool size is large than the maximum allowed. + for (uint8_t count = 0; count < NUMBER_OF_POOLS; count++) { + if (element_sizes[count] >= STORAGE_FREE) { + sif::error << "LocalPool::initialize: Pool is too large! " + "Max. allowed size is: " << (STORAGE_FREE - 1) << std::endl; + return RETURN_FAILED; + } + } + return RETURN_OK; +} + +#endif diff --git a/storagemanager/StorageManagerIF.h b/storagemanager/StorageManagerIF.h index bb88931b..d85fe86f 100644 --- a/storagemanager/StorageManagerIF.h +++ b/storagemanager/StorageManagerIF.h @@ -6,9 +6,9 @@ #include /** - * This union defines the type that identifies where a data packet is stored in the store. - * It comprises of a raw part to read it as raw value and a structured part to use it in - * pool-like stores. + * This union defines the type that identifies where a data packet is + * stored in the store. It comprises of a raw part to read it as raw value and + * a structured part to use it in pool-like stores. */ union store_address_t { /** @@ -94,7 +94,8 @@ public: * @li RETURN_FAILED if data could not be added. * storageId is unchanged then. */ - virtual ReturnValue_t addData(store_address_t* storageId, const uint8_t * data, uint32_t size, bool ignoreFault = false) = 0; + virtual ReturnValue_t addData(store_address_t* storageId, + const uint8_t * data, size_t size, bool ignoreFault = false) = 0; /** * @brief With deleteData, the storageManager frees the memory region * identified by packet_id. @@ -105,14 +106,16 @@ public: */ virtual ReturnValue_t deleteData(store_address_t packet_id) = 0; /** - * @brief Another deleteData which uses the pointer and size of the stored data to delete the content. + * @brief Another deleteData which uses the pointer and size of the + * stored data to delete the content. * @param buffer Pointer to the data. * @param size Size of data to be stored. * @param storeId Store id of the deleted element (optional) * @return @li RETURN_OK on success. * @li failure code if deletion did not work */ - virtual ReturnValue_t deleteData(uint8_t* buffer, uint32_t size, store_address_t* storeId = NULL) = 0; + virtual ReturnValue_t deleteData(uint8_t* buffer, size_t size, + store_address_t* storeId = nullptr) = 0; /** * @brief getData returns an address to data and the size of the data * for a given packet_id. @@ -125,12 +128,12 @@ public: * (e.g. an illegal packet_id was passed). */ virtual ReturnValue_t getData(store_address_t packet_id, - const uint8_t** packet_ptr, size_t * size) = 0; + const uint8_t** packet_ptr, size_t* size) = 0; /** * Same as above, but not const and therefore modifiable. */ virtual ReturnValue_t modifyData(store_address_t packet_id, - uint8_t** packet_ptr, size_t * size) = 0; + uint8_t** packet_ptr, size_t* size) = 0; /** * This method reserves an element of \c size. * @@ -145,7 +148,7 @@ public: * storageId is unchanged then. */ virtual ReturnValue_t getFreeElement(store_address_t* storageId, - const uint32_t size, uint8_t** p_data, bool ignoreFault = false ) = 0; + const size_t size, uint8_t** p_data, bool ignoreFault = false ) = 0; /** * Clears the whole store. * Use with care! From 6817aa4d03f542939742ef9ccdadd578faaf428a Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 4 May 2020 16:57:08 +0200 Subject: [PATCH 08/45] pool manager refactoring --- storagemanager/PoolManager.h | 85 +++++++++------------------------- storagemanager/PoolManager.tpp | 50 ++++++++++++++++++++ 2 files changed, 72 insertions(+), 63 deletions(-) create mode 100644 storagemanager/PoolManager.tpp diff --git a/storagemanager/PoolManager.h b/storagemanager/PoolManager.h index 329235d5..6e6c7613 100644 --- a/storagemanager/PoolManager.h +++ b/storagemanager/PoolManager.h @@ -1,12 +1,3 @@ -/** - * @file PoolManager - * - * @date 02.02.2012 - * @author Bastian Baetz - * - * @brief This file contains the definition of the PoolManager class. - */ - #ifndef POOLMANAGER_H_ #define POOLMANAGER_H_ @@ -17,71 +8,39 @@ /** * @brief The PoolManager class provides an intermediate data storage with * a fixed pool size policy for inter-process communication. - * \details Uses local pool, but is thread-safe. + * @details Uses local pool calls but is thread safe by protecting the call + * with a lock. */ template class PoolManager : public LocalPool { -protected: - /** - * Overwritten for thread safety. - * Locks during execution. - */ - virtual ReturnValue_t reserveSpace(const uint32_t size, store_address_t* address, bool ignoreFault); - - /** - * \brief The mutex is created in the constructor and makes access mutual exclusive. - * \details Locking and unlocking is done during searching for free slots and deleting existing slots. - */ - MutexIF* mutex; public: - PoolManager( object_id_t setObjectId, const uint16_t element_sizes[NUMBER_OF_POOLS], const uint16_t n_elements[NUMBER_OF_POOLS] ); + PoolManager( object_id_t setObjectId, const uint16_t element_sizes[NUMBER_OF_POOLS], + const uint16_t n_elements[NUMBER_OF_POOLS] ); /** * @brief In the PoolManager's destructor all allocated memory is freed. */ - virtual ~PoolManager( void ); + virtual ~PoolManager(); + + ReturnValue_t deleteData(store_address_t) override; + ReturnValue_t deleteData(uint8_t* buffer, size_t size, + store_address_t* storeId = NULL) override; + + ReturnValue_t modifyData(store_address_t packet_id, uint8_t** packet_ptr, + size_t* size) override; +protected: + ReturnValue_t reserveSpace(const uint32_t size, store_address_t* address, + bool ignoreFault) override; + /** - * Overwritten for thread safety. + * @brief The mutex is created in the constructor and makes + * access mutual exclusive. + * @details Locking and unlocking is done during searching for free slots + * and deleting existing slots. */ - virtual ReturnValue_t deleteData(store_address_t); - virtual ReturnValue_t deleteData(uint8_t* buffer, uint32_t size, store_address_t* storeId = NULL); + MutexIF* mutex; }; -template -inline ReturnValue_t PoolManager::reserveSpace(const uint32_t size, store_address_t* address, bool ignoreFault) { - MutexHelper mutexHelper(mutex,MutexIF::NO_TIMEOUT); - ReturnValue_t status = LocalPool::reserveSpace(size,address,ignoreFault); - return status; -} - -template -inline PoolManager::PoolManager(object_id_t setObjectId, - const uint16_t element_sizes[NUMBER_OF_POOLS], - const uint16_t n_elements[NUMBER_OF_POOLS]) : - LocalPool(setObjectId, element_sizes, n_elements, true) { - mutex = MutexFactory::instance()->createMutex(); -} - -template -inline PoolManager::~PoolManager(void) { - MutexFactory::instance()->deleteMutex(mutex); -} - -template -inline ReturnValue_t PoolManager::deleteData( - store_address_t packet_id) { - // debug << "PoolManager( " << translateObject(getObjectId()) << " )::deleteData from store " << packet_id.pool_index << ". id is " << packet_id.packet_index << std::endl; - MutexHelper mutexHelper(mutex,MutexIF::NO_TIMEOUT); - ReturnValue_t status = LocalPool::deleteData(packet_id); - return status; -} - -template -inline ReturnValue_t PoolManager::deleteData(uint8_t* buffer, uint32_t size, - store_address_t* storeId) { - MutexHelper mutexHelper(mutex,MutexIF::NO_TIMEOUT); - ReturnValue_t status = LocalPool::deleteData(buffer, size, storeId); - return status; -} +#include "PoolManager.tpp" #endif /* POOLMANAGER_H_ */ diff --git a/storagemanager/PoolManager.tpp b/storagemanager/PoolManager.tpp new file mode 100644 index 00000000..ed340b91 --- /dev/null +++ b/storagemanager/PoolManager.tpp @@ -0,0 +1,50 @@ +template +inline PoolManager::PoolManager(object_id_t setObjectId, + const uint16_t element_sizes[NUMBER_OF_POOLS], + const uint16_t n_elements[NUMBER_OF_POOLS]) : + LocalPool(setObjectId, element_sizes, n_elements, true) { + mutex = MutexFactory::instance()->createMutex(); +} + +template +inline PoolManager::~PoolManager(void) { + MutexFactory::instance()->deleteMutex(mutex); +} + +template +inline ReturnValue_t PoolManager::reserveSpace( + const uint32_t size, store_address_t* address, bool ignoreFault) { + MutexHelper mutexHelper(mutex,MutexIF::NO_TIMEOUT); + ReturnValue_t status = LocalPool::reserveSpace(size, + address,ignoreFault); + return status; +} + +template +inline ReturnValue_t PoolManager::deleteData( + store_address_t packet_id) { + // debug << "PoolManager( " << translateObject(getObjectId()) << + // " )::deleteData from store " << packet_id.pool_index << + // ". id is "<< packet_id.packet_index << std::endl; + MutexHelper mutexHelper(mutex,MutexIF::NO_TIMEOUT); + ReturnValue_t status = LocalPool::deleteData(packet_id); + return status; +} + +template +inline ReturnValue_t PoolManager::deleteData(uint8_t* buffer, + size_t size, store_address_t* storeId) { + MutexHelper mutexHelper(mutex,MutexIF::NO_TIMEOUT); + ReturnValue_t status = LocalPool::deleteData(buffer, + size, storeId); + return status; +} + +template +inline ReturnValue_t PoolManager::modifyData( + store_address_t packet_id, uint8_t** packet_ptr, size_t* size) { + MutexHelper mutexHelper(mutex,MutexIF::NO_TIMEOUT); + ReturnValue_t status = LocalPool::modifyData(packet_id, + packet_ptr, size); + return status; +} From bc17b5a907ca3fe2ead9829a1a582bedc84389b1 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 5 May 2020 18:43:28 +0200 Subject: [PATCH 09/45] resolved conflict --- osal/linux/FixedTimeslotTask.cpp | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/osal/linux/FixedTimeslotTask.cpp b/osal/linux/FixedTimeslotTask.cpp index 2775e4f6..098753a7 100644 --- a/osal/linux/FixedTimeslotTask.cpp +++ b/osal/linux/FixedTimeslotTask.cpp @@ -42,25 +42,14 @@ uint32_t FixedTimeslotTask::getPeriodMs() const { ReturnValue_t FixedTimeslotTask::addSlot(object_id_t componentId, uint32_t slotTimeMs, int8_t executionStep) { -<<<<<<< HEAD - if (!objectManager->get(componentId)) { - sif::error << "Component " << std::hex << componentId - << " not found, not adding it to pst" << std::endl; - return HasReturnvaluesIF::RETURN_FAILED; - } - - pst.addSlot(componentId, slotTimeMs, executionStep, this); - return HasReturnvaluesIF::RETURN_OK; -======= if (objectManager->get(componentId) != nullptr) { pst.addSlot(componentId, slotTimeMs, executionStep, this); return HasReturnvaluesIF::RETURN_OK; } - error << "Component " << std::hex << componentId << + sif::error << "Component " << std::hex << componentId << " not found, not adding it to pst" << std::endl; return HasReturnvaluesIF::RETURN_FAILED; ->>>>>>> luz_FixedTimeslotTask_ExistenceCheck } ReturnValue_t FixedTimeslotTask::checkSequence() const { From 160a09790e112fdd6cc40cbb75b502c9ca057fd8 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 5 May 2020 18:53:25 +0200 Subject: [PATCH 10/45] removed c omment for now --- devicehandlers/FixedSlotSequence.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/devicehandlers/FixedSlotSequence.cpp b/devicehandlers/FixedSlotSequence.cpp index 466d846f..aa0df9cb 100644 --- a/devicehandlers/FixedSlotSequence.cpp +++ b/devicehandlers/FixedSlotSequence.cpp @@ -90,10 +90,6 @@ uint32_t FixedSlotSequence::getLengthMs() const { ReturnValue_t FixedSlotSequence::checkSequence() const { if(slotList.empty()) { sif::error << "Fixed Slot Sequence: Slot list is empty!" << std::endl; - // does check sequence have to be const? - // if I want to check a class, I need the ability to set - // internal class states. - //isEmpty = true; std::exit(0); } // Iterate through slotList and check successful creation. From df9e66834e5ccb7d7d81a07b199baaebd4f6826c Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 5 May 2020 19:07:11 +0200 Subject: [PATCH 11/45] pop() better --- container/FIFO.h | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/container/FIFO.h b/container/FIFO.h index 889d2ade..f70c78b0 100644 --- a/container/FIFO.h +++ b/container/FIFO.h @@ -70,13 +70,8 @@ public: } ReturnValue_t pop() { - if(empty()) { - return EMPTY; - } else { - readIndex = next(readIndex); - --currentSize; - return HasReturnvaluesIF::RETURN_OK; - } + T value; + return this->retrieve(&value); } static const uint8_t INTERFACE_ID = CLASS_ID::FIFO_CLASS; From f09836a9eb4aeb561ae1e6fa13e20c0aa0fc01cf Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 5 May 2020 19:30:03 +0200 Subject: [PATCH 12/45] removed exit for empty psremoved exit for empty pstt --- devicehandlers/FixedSlotSequence.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/devicehandlers/FixedSlotSequence.cpp b/devicehandlers/FixedSlotSequence.cpp index aa0df9cb..00020d68 100644 --- a/devicehandlers/FixedSlotSequence.cpp +++ b/devicehandlers/FixedSlotSequence.cpp @@ -90,7 +90,6 @@ uint32_t FixedSlotSequence::getLengthMs() const { ReturnValue_t FixedSlotSequence::checkSequence() const { if(slotList.empty()) { sif::error << "Fixed Slot Sequence: Slot list is empty!" << std::endl; - std::exit(0); } // Iterate through slotList and check successful creation. // Checks if timing is ok (must be ascending) and if all handlers were found. From cc0469fef6031ebde23583f958fcf1c0b1e5ef3e Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 5 May 2020 20:09:42 +0200 Subject: [PATCH 13/45] return failed insteead of exiting --- devicehandlers/FixedSlotSequence.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/devicehandlers/FixedSlotSequence.cpp b/devicehandlers/FixedSlotSequence.cpp index 00020d68..860a643b 100644 --- a/devicehandlers/FixedSlotSequence.cpp +++ b/devicehandlers/FixedSlotSequence.cpp @@ -90,6 +90,7 @@ uint32_t FixedSlotSequence::getLengthMs() const { ReturnValue_t FixedSlotSequence::checkSequence() const { if(slotList.empty()) { sif::error << "Fixed Slot Sequence: Slot list is empty!" << std::endl; + return HasReturnvaluesIF::RETURN_FAILED; } // Iterate through slotList and check successful creation. // Checks if timing is ok (must be ascending) and if all handlers were found. From e950051b4ab7edcc6919af226f3c7a1eae63602e Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 6 May 2020 14:35:30 +0200 Subject: [PATCH 14/45] some object managerIF security measures objectmanager get function checks whether global object manager was initialized now. New returnvalues, which are also used for local pool init --- devicehandlers/FixedSlotSequence.cpp | 8 -------- devicehandlers/FixedSlotSequence.h | 16 +-------------- objectmanager/ObjectManagerIF.h | 29 ++++++++++++++++++---------- storagemanager/LocalPool.tpp | 6 +++--- storagemanager/StorageManagerIF.h | 1 + 5 files changed, 24 insertions(+), 36 deletions(-) diff --git a/devicehandlers/FixedSlotSequence.cpp b/devicehandlers/FixedSlotSequence.cpp index 6db7ee43..8974510e 100644 --- a/devicehandlers/FixedSlotSequence.cpp +++ b/devicehandlers/FixedSlotSequence.cpp @@ -89,17 +89,9 @@ uint32_t FixedSlotSequence::getLengthMs() const { ReturnValue_t FixedSlotSequence::checkSequence() const { if(slotList.empty()) { -<<<<<<< HEAD sif::error << "Fixed Slot Sequence: Slot list is empty!" << std::endl; return HasReturnvaluesIF::RETURN_FAILED; } - // Iterate through slotList and check successful creation. - // Checks if timing is ok (must be ascending) and if all handlers were found. -======= - error << "Fixed Slot Sequence: Slot list is empty!" << std::endl; - return HasReturnvaluesIF::RETURN_FAILED; - } ->>>>>>> master auto slotIt = slotList.begin(); uint32_t count = 0; uint32_t time = 0; diff --git a/devicehandlers/FixedSlotSequence.h b/devicehandlers/FixedSlotSequence.h index 65ad3451..dd9636c1 100644 --- a/devicehandlers/FixedSlotSequence.h +++ b/devicehandlers/FixedSlotSequence.h @@ -12,16 +12,6 @@ using SlotListIter = std::multiset::iterator; /** * @brief This class is the representation of a Polling Sequence Table in software. - * -<<<<<<< HEAD - * @details The FixedSlotSequence object maintains the dynamic execution of device handler objects. - * The main idea is to create a list of device handlers, to announce all handlers to the - * polling sequence and to maintain a list of polling slot objects. This slot list represents the - * Polling Sequence Table in software. Each polling slot contains information to indicate when and - * which device handler shall be executed within a given polling period. - * The sequence is then executed by iterating through this slot list. - * Handlers are invoking by calling a certain function stored in the handler list. -======= * @details * The FixedSlotSequence object maintains the dynamic execution of * device handler objects. @@ -35,7 +25,6 @@ using SlotListIter = std::multiset::iterator; * which device handler shall be executed within a given polling period. * The sequence is then executed by iterating through this slot list. * Handlers are invoking by calling a certain function stored in the handler list. ->>>>>>> master */ class FixedSlotSequence { public: @@ -128,16 +117,13 @@ public: */ SlotListIter current; -<<<<<<< HEAD - virtual ReturnValue_t checkSequence() const; -======= /** * Iterate through slotList and check successful creation. * Checks if timing is ok (must be ascending) and if all handlers were found. * @return */ ReturnValue_t checkSequence() const; ->>>>>>> master + protected: /** diff --git a/objectmanager/ObjectManagerIF.h b/objectmanager/ObjectManagerIF.h index 1d7674c6..3a23e339 100644 --- a/objectmanager/ObjectManagerIF.h +++ b/objectmanager/ObjectManagerIF.h @@ -12,6 +12,7 @@ #include #include #include +#include /** * @brief This class provides an interface to the global object manager. @@ -24,9 +25,12 @@ */ class ObjectManagerIF : public HasReturnvaluesIF { public: - static const uint8_t INTERFACE_ID = CLASS_ID::OBJECT_MANAGER_IF; - static const ReturnValue_t INSERTION_FAILED = MAKE_RETURN_CODE( 1 ); - static const ReturnValue_t NOT_FOUND = MAKE_RETURN_CODE( 2 ); + static constexpr uint8_t INTERFACE_ID = CLASS_ID::OBJECT_MANAGER_IF; + static constexpr ReturnValue_t INSERTION_FAILED = MAKE_RETURN_CODE( 1 ); + static constexpr ReturnValue_t NOT_FOUND = MAKE_RETURN_CODE( 2 ); + static constexpr ReturnValue_t CHILD_INIT_FAILED = MAKE_RETURN_CODE( 3 ); + static constexpr ReturnValue_t INTERNAL_ERR_REPORTER_UNINIT = MAKE_RETURN_CODE( 4 ); + protected: /** * @brief This method is used to hide the template-based get call from @@ -79,16 +83,21 @@ public: }; -/*Documentation can be found in the class method declaration above.*/ -template -T* ObjectManagerIF::get( object_id_t id ) { - SystemObjectIF* temp = this->getSystemObject(id); - return dynamic_cast(temp); -} - /** * @brief This is the forward declaration of the global objectManager instance. */ extern ObjectManagerIF *objectManager; +/*Documentation can be found in the class method declaration above.*/ +template +T* ObjectManagerIF::get( object_id_t id ) { + if(objectManager == nullptr) { + sif::error << "ObjectManagerIF: Global object manager has not " + "been initialized yet!" << std::endl; + std::exit(0); + } + SystemObjectIF* temp = this->getSystemObject(id); + return dynamic_cast(temp); +} + #endif /* OBJECTMANAGERIF_H_ */ diff --git a/storagemanager/LocalPool.tpp b/storagemanager/LocalPool.tpp index 5cead166..c46e1a8d 100644 --- a/storagemanager/LocalPool.tpp +++ b/storagemanager/LocalPool.tpp @@ -242,8 +242,8 @@ inline ReturnValue_t LocalPool::initialize() { } internalErrorReporter = objectManager->get( objects::INTERNAL_ERROR_REPORTER); - if (internalErrorReporter == NULL){ - return RETURN_FAILED; + if (internalErrorReporter == nullptr){ + return ObjectManagerIF::INTERNAL_ERR_REPORTER_UNINIT; } //Check if any pool size is large than the maximum allowed. @@ -251,7 +251,7 @@ inline ReturnValue_t LocalPool::initialize() { if (element_sizes[count] >= STORAGE_FREE) { sif::error << "LocalPool::initialize: Pool is too large! " "Max. allowed size is: " << (STORAGE_FREE - 1) << std::endl; - return RETURN_FAILED; + return StorageManagerIF::POOL_TOO_LARGE; } } return RETURN_OK; diff --git a/storagemanager/StorageManagerIF.h b/storagemanager/StorageManagerIF.h index d85fe86f..d1f9cde5 100644 --- a/storagemanager/StorageManagerIF.h +++ b/storagemanager/StorageManagerIF.h @@ -71,6 +71,7 @@ public: static const ReturnValue_t ILLEGAL_STORAGE_ID = MAKE_RETURN_CODE(3); //!< This return code indicates that data was requested with an illegal storage ID. static const ReturnValue_t DATA_DOES_NOT_EXIST = MAKE_RETURN_CODE(4); //!< This return code indicates that the requested ID was valid, but no data is stored there. static const ReturnValue_t ILLEGAL_ADDRESS = MAKE_RETURN_CODE(5); + static const ReturnValue_t POOL_TOO_LARGE = MAKE_RETURN_CODE(6); //!< Pool size too large on initialization. static const uint8_t SUBSYSTEM_ID = SUBSYSTEM_ID::OBSW; static const Event GET_DATA_FAILED = MAKE_EVENT(0, SEVERITY::LOW); From b016f2995a00a3a5183794bb576820d17b6f078b Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 6 May 2020 16:34:43 +0200 Subject: [PATCH 15/45] added default vlaue for init function --- action/ActionHelper.cpp | 10 ++++++---- action/ActionHelper.h | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/action/ActionHelper.cpp b/action/ActionHelper.cpp index ef9f06a9..19a84d82 100644 --- a/action/ActionHelper.cpp +++ b/action/ActionHelper.cpp @@ -1,9 +1,9 @@ #include #include #include + ActionHelper::ActionHelper(HasActionsIF* setOwner, MessageQueueIF* useThisQueue) : - owner(setOwner), queueToUse(useThisQueue), ipcStore( - NULL) { + owner(setOwner), queueToUse(useThisQueue), ipcStore(nullptr) { } ActionHelper::~ActionHelper() { @@ -22,10 +22,12 @@ ReturnValue_t ActionHelper::handleActionMessage(CommandMessage* command) { ReturnValue_t ActionHelper::initialize(MessageQueueIF* queueToUse_) { ipcStore = objectManager->get(objects::IPC_STORE); - if (ipcStore == NULL) { + if (ipcStore == nullptr) { return HasReturnvaluesIF::RETURN_FAILED; } - setQueueToUse(queueToUse_); + if(queueToUse_ != nullptr) { + setQueueToUse(queueToUse_); + } return HasReturnvaluesIF::RETURN_OK; } diff --git a/action/ActionHelper.h b/action/ActionHelper.h index 6ba6dd89..3d8351d6 100644 --- a/action/ActionHelper.h +++ b/action/ActionHelper.h @@ -38,7 +38,7 @@ public: * @param queueToUse_ Pointer to the messageQueue to be used * @return Returns RETURN_OK if successful */ - ReturnValue_t initialize(MessageQueueIF* queueToUse_); + ReturnValue_t initialize(MessageQueueIF* queueToUse_ = nullptr); /** * Function to be called from the owner to send a step message. Success or failure will be determined by the result value. * From fe9aa46cf8d33ccb93b570fe77631abb08555829 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Thu, 7 May 2020 12:22:17 +0200 Subject: [PATCH 16/45] removed system object list include. makes it difficult for multiple configurations, because the wrong header might be included --- objectmanager/ObjectManagerIF.h | 1 - 1 file changed, 1 deletion(-) diff --git a/objectmanager/ObjectManagerIF.h b/objectmanager/ObjectManagerIF.h index 3a23e339..3575fc16 100644 --- a/objectmanager/ObjectManagerIF.h +++ b/objectmanager/ObjectManagerIF.h @@ -9,7 +9,6 @@ #define OBJECTMANAGERIF_H_ #include -#include #include #include #include From 9489b7abc1bee2a2b164e81f178222706a8c2b8e Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Thu, 7 May 2020 19:23:56 +0200 Subject: [PATCH 17/45] modifyData override deleted is not really thread-safe anyway --- devicehandlers/CommunicationMessage.h | 8 ++------ storagemanager/PoolManager.h | 3 --- storagemanager/PoolManager.tpp | 8 -------- 3 files changed, 2 insertions(+), 17 deletions(-) diff --git a/devicehandlers/CommunicationMessage.h b/devicehandlers/CommunicationMessage.h index a92c4b5c..95266319 100644 --- a/devicehandlers/CommunicationMessage.h +++ b/devicehandlers/CommunicationMessage.h @@ -13,15 +13,11 @@ #include /** - * @brief Used to pass communication information between tasks + * @brief Message type to send larger messages * * @details * Can be used to pass information like data pointers and - * data sizes between communication tasks - * like the Device Handler Comm Interfaces and Polling Tasks. - * - * Can also be used to exchange actual data if its not too large - * (e.g. Sensor values). + * data sizes between communication tasks. * */ class CommunicationMessage: public MessageQueueMessage { diff --git a/storagemanager/PoolManager.h b/storagemanager/PoolManager.h index 6e6c7613..41325273 100644 --- a/storagemanager/PoolManager.h +++ b/storagemanager/PoolManager.h @@ -25,9 +25,6 @@ public: ReturnValue_t deleteData(store_address_t) override; ReturnValue_t deleteData(uint8_t* buffer, size_t size, store_address_t* storeId = NULL) override; - - ReturnValue_t modifyData(store_address_t packet_id, uint8_t** packet_ptr, - size_t* size) override; protected: ReturnValue_t reserveSpace(const uint32_t size, store_address_t* address, bool ignoreFault) override; diff --git a/storagemanager/PoolManager.tpp b/storagemanager/PoolManager.tpp index ed340b91..0408d55b 100644 --- a/storagemanager/PoolManager.tpp +++ b/storagemanager/PoolManager.tpp @@ -40,11 +40,3 @@ inline ReturnValue_t PoolManager::deleteData(uint8_t* buffer, return status; } -template -inline ReturnValue_t PoolManager::modifyData( - store_address_t packet_id, uint8_t** packet_ptr, size_t* size) { - MutexHelper mutexHelper(mutex,MutexIF::NO_TIMEOUT); - ReturnValue_t status = LocalPool::modifyData(packet_id, - packet_ptr, size); - return status; -} From 0f286461d065f25ddda7b63f4f3ae45d2e6b5db3 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Sat, 9 May 2020 18:10:26 +0200 Subject: [PATCH 18/45] added new storage raw accessor --- storagemanager/StorageAccessor.cpp | 154 +++++++++++++++++++++++++ storagemanager/StorageAccessor.h | 174 +++++++++++++++++++++++++++++ 2 files changed, 328 insertions(+) create mode 100644 storagemanager/StorageAccessor.cpp create mode 100644 storagemanager/StorageAccessor.h diff --git a/storagemanager/StorageAccessor.cpp b/storagemanager/StorageAccessor.cpp new file mode 100644 index 00000000..50e503cf --- /dev/null +++ b/storagemanager/StorageAccessor.cpp @@ -0,0 +1,154 @@ +#include + +ConstStorageAccessor::ConstStorageAccessor(store_address_t storeId): storeId(storeId) {} + +ConstStorageAccessor::~ConstStorageAccessor() { + if(deleteData and store != nullptr) { + sif::debug << "deleting store data" << std::endl; + store->deleteDataNonLocking(storeId); + } + if(mutexLock != nullptr) { + sif::debug << "unlocking mutex lock" << std::endl; + mutexLock.reset(); + } +} + +ConstStorageAccessor& ConstStorageAccessor::operator=( + ConstStorageAccessor&& other) { + constDataPointer = other.constDataPointer; + storeId = other.storeId; + store = other.store; + size_ = other.size_; + deleteData = other.deleteData; + this->store = other.store; + // Transfer ownership of the lock. + mutexLock = std::move(other.mutexLock); + // This prevents double deletion of the resource + other.mutexLock = nullptr; + // This prevent premature deletion + other.store = nullptr; + return *this; +} + +StorageAccessor::StorageAccessor(store_address_t storeId): + ConstStorageAccessor(storeId) { +} + +StorageAccessor& StorageAccessor::operator =( + StorageAccessor&& other) { + // Call the parent move assignment and also assign own member. + dataPointer = other.dataPointer; + StorageAccessor::operator=(std::move(other)); + return * this; +} + +// Call the parent move ctor and also transfer own member. +StorageAccessor::StorageAccessor(StorageAccessor&& other): + ConstStorageAccessor(std::move(other)), dataPointer(other.dataPointer) { +} + +ConstStorageAccessor::ConstStorageAccessor(ConstStorageAccessor&& other): + constDataPointer(other.constDataPointer), storeId(other.storeId), + size_(other.size_), store(other.store), deleteData(other.deleteData), + internalState(other.internalState) { + // Transfer ownership of the lock. + mutexLock = std::move(other.mutexLock); + // This prevents double deletion of the resource. Not strictly necessary, + // from the testing I have conducted so far but I am not familiar enough + // with move semantics so I will just set the other lock to nullptr for now. + other.mutexLock = nullptr; + // This prevent premature deletion + other.store = nullptr; +} + +const uint8_t* ConstStorageAccessor::data() const { + return constDataPointer; +} + +size_t ConstStorageAccessor::size() const { + if(internalState == AccessState::UNINIT) { + sif::warning << "StorageAccessor: Not initialized!" << std::endl; + } + return size_; +} + +void ConstStorageAccessor::getDataCopy(uint8_t *pointer) { + if(internalState == AccessState::UNINIT) { + sif::warning << "StorageAccessor: Not initialized!" << std::endl; + return; + } + std::copy(constDataPointer, constDataPointer + size_, pointer); +} + +void ConstStorageAccessor::release() { + deleteData = false; +} + +ReturnValue_t ConstStorageAccessor::lock(MutexIF* mutex, uint32_t mutexTimeout) { + if(mutexLock == nullptr) { + mutexLock = std::unique_ptr(new MutexHelper(mutex, mutexTimeout)); + return mutexLock.get()->getResult(); + } + else { + sif::warning << "StorageAccessor: Attempted to lock twice. Check code!" << std::endl; + return HasReturnvaluesIF::RETURN_FAILED; + } +} + +void ConstStorageAccessor::unlock() { + if(mutexLock != nullptr) { + mutexLock.reset(); + } +} + +store_address_t ConstStorageAccessor::getId() const { + return storeId; +} + +void ConstStorageAccessor::print() const { + if(internalState == AccessState::UNINIT) { + sif::warning << "StorageAccessor: Not initialized!" << std::endl; + return; + } + sif::info << "StorageAccessor: Printing data: ["; + for(uint16_t iPool = 0; iPool < size_; iPool++) { + sif::info << std::hex << (int)constDataPointer[iPool]; + if(iPool < size_ - 1){ + sif::info << " , "; + } + } + sif::info << " ] " << std::endl; +} + +void ConstStorageAccessor::assignStore(StorageManagerIF* store) { + internalState = AccessState::READ; + this->store = store; +} + + +uint8_t* StorageAccessor::data() { + if(internalState == AccessState::UNINIT) { + sif::warning << "StorageAccessor: Not initialized!" << std::endl; + } + return dataPointer; +} + +ReturnValue_t StorageAccessor::write(uint8_t *data, size_t size, + uint16_t offset) { + if(internalState == AccessState::UNINIT) { + sif::warning << "StorageAccessor: Not initialized!" << std::endl; + return HasReturnvaluesIF::RETURN_FAILED; + } + if(offset + size > size_) { + sif::error << "StorageAccessor: Data too large for pool entry!" << std::endl; + return HasReturnvaluesIF::RETURN_FAILED; + } + std::copy(data, data + size, dataPointer); + return HasReturnvaluesIF::RETURN_OK; +} + +void StorageAccessor::assignConstPointer() { + constDataPointer = dataPointer; +} + + diff --git a/storagemanager/StorageAccessor.h b/storagemanager/StorageAccessor.h new file mode 100644 index 00000000..f861a2cc --- /dev/null +++ b/storagemanager/StorageAccessor.h @@ -0,0 +1,174 @@ +/** + * @brief Helper classes to facilitate safe access to storages which is also + * conforming to RAII principles + * @details These helper can be used together with the + * StorageManager classes to manage access to a storage. + * It can take care of thread-safety while also providing + * mechanisms to automatically clear storage data and unlocking the + * pool. + */ +#ifndef TEST_PROTOTYPES_STORAGEACCESSOR_H_ +#define TEST_PROTOTYPES_STORAGEACCESSOR_H_ + +#include +#include +#include + + +/** + * @brief Accessor class which can be returned by pool managers + * or passed and set by pool managers to have safe access to the pool + * resources. + */ +class ConstStorageAccessor { + //! StorageManager classes have exclusive access to private variables. + template + friend class PoolManager; + template + friend class LocalPool; +public: + /** + * @brief Simple constructor which takes the store ID of the storage + * entry to access. + * @param storeId + */ + ConstStorageAccessor(store_address_t storeId); + + /** + * @brief Move ctor and move assignment allow returning accessors as + * a returnvalue. They prevent resource being free prematurely. + * Refer to: https://github.com/MicrosoftDocs/cpp-docs/blob/master/docs/cpp/ + * move-constructors-and-move-assignment-operators-cpp.md + * @param + * @return + */ + ConstStorageAccessor& operator= (ConstStorageAccessor&&); + ConstStorageAccessor (ConstStorageAccessor&&); + + //! The copy ctor and copy assignemnt should be deleted implicitely + //! according to https://foonathan.net/2019/02/special-member-functions/ + //! but I still deleted them to make it more explicit. (remember rule of 5). + ConstStorageAccessor& operator= (ConstStorageAccessor&) = delete; + ConstStorageAccessor (ConstStorageAccessor&) = delete; + + /** + * @brief The destructor in default configuration takes care of + * deleting the accessed pool entry and unlocking the mutex + */ + virtual ~ConstStorageAccessor(); + + /** + * @brief Returns a pointer to the read-only data + * @return + */ + const uint8_t* data() const; + + /** + * @brief Copies the read-only data to the supplied pointer + * @param pointer + */ + void getDataCopy(uint8_t *pointer); + + /** + * @brief Calling this will prevent the Accessor from deleting the data + * when the destructor is called. + */ + void release(); + /** + * @brief Locks the supplied mutex. + * @details + * The mutex will be unlocked automatically + * when this class is destroyed (for example when exiting the scope). + * Only one mutex can be locked at a time! + * @param mutex + * @param mutexTimeout + * @return + */ + ReturnValue_t lock(MutexIF* mutex, + uint32_t mutexTimeout = MutexIF::NO_TIMEOUT); + /** + * @brief Unlocks the mutex (if one has been locked previously). + * Unless this function is called, the mutex is unlocked + * when the class exits the scope. + */ + void unlock(); + + + /** + * Get the size of the data + * @return + */ + size_t size() const; + + /** + * Get the storage ID. + * @return + */ + store_address_t getId() const; + + void print() const; +protected: + const uint8_t* constDataPointer = nullptr; + store_address_t storeId; + size_t size_ = 0; + //! Managing pool, has to assign itself. + StorageManagerIF* store = nullptr; + //! Unique pointer to the mutex lock instance. Is initialized by + //! the pool manager. + std::unique_ptr mutexLock = nullptr; + bool deleteData = true; + + enum class AccessState { + UNINIT, + READ + }; + //! Internal state for safety reasons. + AccessState internalState = AccessState::UNINIT; + /** + * Used by the pool manager instances to assign themselves to the + * accessor. This is necessary to delete the data when the acessor + * exits the scope ! The internal state will be considered read + * when this function is called, so take care all data is set properly as + * well. + * @param + */ + void assignStore(StorageManagerIF*); + +}; + + +/** + * @brief Child class for modifyable data. Also has a normal pointer member. + */ +class StorageAccessor: public ConstStorageAccessor { + //! StorageManager classes have exclusive access to private variables. + template + friend class PoolManager; + template + friend class LocalPool; +public: + StorageAccessor(store_address_t storeId); + /** + * @brief Move ctor and move assignment allow returning accessors as + * a returnvalue. They prevent resource being free prematurely. + * Refer to: https://github.com/MicrosoftDocs/cpp-docs/blob/master/docs/cpp/ + * move-constructors-and-move-assignment-operators-cpp.md + * @param + * @return + */ + StorageAccessor& operator= (StorageAccessor&&); + StorageAccessor (StorageAccessor&&); + + ReturnValue_t write(uint8_t *data, size_t size, + uint16_t offset); + uint8_t* data(); + +private: + //! Non-const pointer for modifyable data. + uint8_t* dataPointer = nullptr; + //! For modifyable data, the const pointer is assigned to the normal + //! pointer by the pool manager so both access functions can be used safely + void assignConstPointer(); +}; + +#endif /* TEST_PROTOTYPES_STORAGEACCESSOR_H_ */ From 2f58c3a3052506e922ba1758e9a7fa4f1c642c9f Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Sun, 10 May 2020 00:14:00 +0200 Subject: [PATCH 19/45] commented out storage accessor --- storagemanager/StorageAccessor.cpp | 308 ++++++++++++------------- storagemanager/StorageAccessor.h | 348 ++++++++++++++--------------- 2 files changed, 328 insertions(+), 328 deletions(-) diff --git a/storagemanager/StorageAccessor.cpp b/storagemanager/StorageAccessor.cpp index 50e503cf..a068d169 100644 --- a/storagemanager/StorageAccessor.cpp +++ b/storagemanager/StorageAccessor.cpp @@ -1,154 +1,154 @@ -#include - -ConstStorageAccessor::ConstStorageAccessor(store_address_t storeId): storeId(storeId) {} - -ConstStorageAccessor::~ConstStorageAccessor() { - if(deleteData and store != nullptr) { - sif::debug << "deleting store data" << std::endl; - store->deleteDataNonLocking(storeId); - } - if(mutexLock != nullptr) { - sif::debug << "unlocking mutex lock" << std::endl; - mutexLock.reset(); - } -} - -ConstStorageAccessor& ConstStorageAccessor::operator=( - ConstStorageAccessor&& other) { - constDataPointer = other.constDataPointer; - storeId = other.storeId; - store = other.store; - size_ = other.size_; - deleteData = other.deleteData; - this->store = other.store; - // Transfer ownership of the lock. - mutexLock = std::move(other.mutexLock); - // This prevents double deletion of the resource - other.mutexLock = nullptr; - // This prevent premature deletion - other.store = nullptr; - return *this; -} - -StorageAccessor::StorageAccessor(store_address_t storeId): - ConstStorageAccessor(storeId) { -} - -StorageAccessor& StorageAccessor::operator =( - StorageAccessor&& other) { - // Call the parent move assignment and also assign own member. - dataPointer = other.dataPointer; - StorageAccessor::operator=(std::move(other)); - return * this; -} - -// Call the parent move ctor and also transfer own member. -StorageAccessor::StorageAccessor(StorageAccessor&& other): - ConstStorageAccessor(std::move(other)), dataPointer(other.dataPointer) { -} - -ConstStorageAccessor::ConstStorageAccessor(ConstStorageAccessor&& other): - constDataPointer(other.constDataPointer), storeId(other.storeId), - size_(other.size_), store(other.store), deleteData(other.deleteData), - internalState(other.internalState) { - // Transfer ownership of the lock. - mutexLock = std::move(other.mutexLock); - // This prevents double deletion of the resource. Not strictly necessary, - // from the testing I have conducted so far but I am not familiar enough - // with move semantics so I will just set the other lock to nullptr for now. - other.mutexLock = nullptr; - // This prevent premature deletion - other.store = nullptr; -} - -const uint8_t* ConstStorageAccessor::data() const { - return constDataPointer; -} - -size_t ConstStorageAccessor::size() const { - if(internalState == AccessState::UNINIT) { - sif::warning << "StorageAccessor: Not initialized!" << std::endl; - } - return size_; -} - -void ConstStorageAccessor::getDataCopy(uint8_t *pointer) { - if(internalState == AccessState::UNINIT) { - sif::warning << "StorageAccessor: Not initialized!" << std::endl; - return; - } - std::copy(constDataPointer, constDataPointer + size_, pointer); -} - -void ConstStorageAccessor::release() { - deleteData = false; -} - -ReturnValue_t ConstStorageAccessor::lock(MutexIF* mutex, uint32_t mutexTimeout) { - if(mutexLock == nullptr) { - mutexLock = std::unique_ptr(new MutexHelper(mutex, mutexTimeout)); - return mutexLock.get()->getResult(); - } - else { - sif::warning << "StorageAccessor: Attempted to lock twice. Check code!" << std::endl; - return HasReturnvaluesIF::RETURN_FAILED; - } -} - -void ConstStorageAccessor::unlock() { - if(mutexLock != nullptr) { - mutexLock.reset(); - } -} - -store_address_t ConstStorageAccessor::getId() const { - return storeId; -} - -void ConstStorageAccessor::print() const { - if(internalState == AccessState::UNINIT) { - sif::warning << "StorageAccessor: Not initialized!" << std::endl; - return; - } - sif::info << "StorageAccessor: Printing data: ["; - for(uint16_t iPool = 0; iPool < size_; iPool++) { - sif::info << std::hex << (int)constDataPointer[iPool]; - if(iPool < size_ - 1){ - sif::info << " , "; - } - } - sif::info << " ] " << std::endl; -} - -void ConstStorageAccessor::assignStore(StorageManagerIF* store) { - internalState = AccessState::READ; - this->store = store; -} - - -uint8_t* StorageAccessor::data() { - if(internalState == AccessState::UNINIT) { - sif::warning << "StorageAccessor: Not initialized!" << std::endl; - } - return dataPointer; -} - -ReturnValue_t StorageAccessor::write(uint8_t *data, size_t size, - uint16_t offset) { - if(internalState == AccessState::UNINIT) { - sif::warning << "StorageAccessor: Not initialized!" << std::endl; - return HasReturnvaluesIF::RETURN_FAILED; - } - if(offset + size > size_) { - sif::error << "StorageAccessor: Data too large for pool entry!" << std::endl; - return HasReturnvaluesIF::RETURN_FAILED; - } - std::copy(data, data + size, dataPointer); - return HasReturnvaluesIF::RETURN_OK; -} - -void StorageAccessor::assignConstPointer() { - constDataPointer = dataPointer; -} - - +//#include +// +//ConstStorageAccessor::ConstStorageAccessor(store_address_t storeId): storeId(storeId) {} +// +//ConstStorageAccessor::~ConstStorageAccessor() { +// if(deleteData and store != nullptr) { +// sif::debug << "deleting store data" << std::endl; +// store->deleteDataNonLocking(storeId); +// } +// if(mutexLock != nullptr) { +// sif::debug << "unlocking mutex lock" << std::endl; +// mutexLock.reset(); +// } +//} +// +//ConstStorageAccessor& ConstStorageAccessor::operator=( +// ConstStorageAccessor&& other) { +// constDataPointer = other.constDataPointer; +// storeId = other.storeId; +// store = other.store; +// size_ = other.size_; +// deleteData = other.deleteData; +// this->store = other.store; +// // Transfer ownership of the lock. +// mutexLock = std::move(other.mutexLock); +// // This prevents double deletion of the resource +// other.mutexLock = nullptr; +// // This prevent premature deletion +// other.store = nullptr; +// return *this; +//} +// +//StorageAccessor::StorageAccessor(store_address_t storeId): +// ConstStorageAccessor(storeId) { +//} +// +//StorageAccessor& StorageAccessor::operator =( +// StorageAccessor&& other) { +// // Call the parent move assignment and also assign own member. +// dataPointer = other.dataPointer; +// StorageAccessor::operator=(std::move(other)); +// return * this; +//} +// +//// Call the parent move ctor and also transfer own member. +//StorageAccessor::StorageAccessor(StorageAccessor&& other): +// ConstStorageAccessor(std::move(other)), dataPointer(other.dataPointer) { +//} +// +//ConstStorageAccessor::ConstStorageAccessor(ConstStorageAccessor&& other): +// constDataPointer(other.constDataPointer), storeId(other.storeId), +// size_(other.size_), store(other.store), deleteData(other.deleteData), +// internalState(other.internalState) { +// // Transfer ownership of the lock. +// mutexLock = std::move(other.mutexLock); +// // This prevents double deletion of the resource. Not strictly necessary, +// // from the testing I have conducted so far but I am not familiar enough +// // with move semantics so I will just set the other lock to nullptr for now. +// other.mutexLock = nullptr; +// // This prevent premature deletion +// other.store = nullptr; +//} +// +//const uint8_t* ConstStorageAccessor::data() const { +// return constDataPointer; +//} +// +//size_t ConstStorageAccessor::size() const { +// if(internalState == AccessState::UNINIT) { +// sif::warning << "StorageAccessor: Not initialized!" << std::endl; +// } +// return size_; +//} +// +//void ConstStorageAccessor::getDataCopy(uint8_t *pointer) { +// if(internalState == AccessState::UNINIT) { +// sif::warning << "StorageAccessor: Not initialized!" << std::endl; +// return; +// } +// std::copy(constDataPointer, constDataPointer + size_, pointer); +//} +// +//void ConstStorageAccessor::release() { +// deleteData = false; +//} +// +//ReturnValue_t ConstStorageAccessor::lock(MutexIF* mutex, uint32_t mutexTimeout) { +// if(mutexLock == nullptr) { +// mutexLock = std::unique_ptr(new MutexHelper(mutex, mutexTimeout)); +// return mutexLock.get()->getResult(); +// } +// else { +// sif::warning << "StorageAccessor: Attempted to lock twice. Check code!" << std::endl; +// return HasReturnvaluesIF::RETURN_FAILED; +// } +//} +// +//void ConstStorageAccessor::unlock() { +// if(mutexLock != nullptr) { +// mutexLock.reset(); +// } +//} +// +//store_address_t ConstStorageAccessor::getId() const { +// return storeId; +//} +// +//void ConstStorageAccessor::print() const { +// if(internalState == AccessState::UNINIT) { +// sif::warning << "StorageAccessor: Not initialized!" << std::endl; +// return; +// } +// sif::info << "StorageAccessor: Printing data: ["; +// for(uint16_t iPool = 0; iPool < size_; iPool++) { +// sif::info << std::hex << (int)constDataPointer[iPool]; +// if(iPool < size_ - 1){ +// sif::info << " , "; +// } +// } +// sif::info << " ] " << std::endl; +//} +// +//void ConstStorageAccessor::assignStore(StorageManagerIF* store) { +// internalState = AccessState::READ; +// this->store = store; +//} +// +// +//uint8_t* StorageAccessor::data() { +// if(internalState == AccessState::UNINIT) { +// sif::warning << "StorageAccessor: Not initialized!" << std::endl; +// } +// return dataPointer; +//} +// +//ReturnValue_t StorageAccessor::write(uint8_t *data, size_t size, +// uint16_t offset) { +// if(internalState == AccessState::UNINIT) { +// sif::warning << "StorageAccessor: Not initialized!" << std::endl; +// return HasReturnvaluesIF::RETURN_FAILED; +// } +// if(offset + size > size_) { +// sif::error << "StorageAccessor: Data too large for pool entry!" << std::endl; +// return HasReturnvaluesIF::RETURN_FAILED; +// } +// std::copy(data, data + size, dataPointer); +// return HasReturnvaluesIF::RETURN_OK; +//} +// +//void StorageAccessor::assignConstPointer() { +// constDataPointer = dataPointer; +//} +// +// diff --git a/storagemanager/StorageAccessor.h b/storagemanager/StorageAccessor.h index f861a2cc..d391c5c2 100644 --- a/storagemanager/StorageAccessor.h +++ b/storagemanager/StorageAccessor.h @@ -1,174 +1,174 @@ -/** - * @brief Helper classes to facilitate safe access to storages which is also - * conforming to RAII principles - * @details These helper can be used together with the - * StorageManager classes to manage access to a storage. - * It can take care of thread-safety while also providing - * mechanisms to automatically clear storage data and unlocking the - * pool. - */ -#ifndef TEST_PROTOTYPES_STORAGEACCESSOR_H_ -#define TEST_PROTOTYPES_STORAGEACCESSOR_H_ - -#include -#include -#include - - -/** - * @brief Accessor class which can be returned by pool managers - * or passed and set by pool managers to have safe access to the pool - * resources. - */ -class ConstStorageAccessor { - //! StorageManager classes have exclusive access to private variables. - template - friend class PoolManager; - template - friend class LocalPool; -public: - /** - * @brief Simple constructor which takes the store ID of the storage - * entry to access. - * @param storeId - */ - ConstStorageAccessor(store_address_t storeId); - - /** - * @brief Move ctor and move assignment allow returning accessors as - * a returnvalue. They prevent resource being free prematurely. - * Refer to: https://github.com/MicrosoftDocs/cpp-docs/blob/master/docs/cpp/ - * move-constructors-and-move-assignment-operators-cpp.md - * @param - * @return - */ - ConstStorageAccessor& operator= (ConstStorageAccessor&&); - ConstStorageAccessor (ConstStorageAccessor&&); - - //! The copy ctor and copy assignemnt should be deleted implicitely - //! according to https://foonathan.net/2019/02/special-member-functions/ - //! but I still deleted them to make it more explicit. (remember rule of 5). - ConstStorageAccessor& operator= (ConstStorageAccessor&) = delete; - ConstStorageAccessor (ConstStorageAccessor&) = delete; - - /** - * @brief The destructor in default configuration takes care of - * deleting the accessed pool entry and unlocking the mutex - */ - virtual ~ConstStorageAccessor(); - - /** - * @brief Returns a pointer to the read-only data - * @return - */ - const uint8_t* data() const; - - /** - * @brief Copies the read-only data to the supplied pointer - * @param pointer - */ - void getDataCopy(uint8_t *pointer); - - /** - * @brief Calling this will prevent the Accessor from deleting the data - * when the destructor is called. - */ - void release(); - /** - * @brief Locks the supplied mutex. - * @details - * The mutex will be unlocked automatically - * when this class is destroyed (for example when exiting the scope). - * Only one mutex can be locked at a time! - * @param mutex - * @param mutexTimeout - * @return - */ - ReturnValue_t lock(MutexIF* mutex, - uint32_t mutexTimeout = MutexIF::NO_TIMEOUT); - /** - * @brief Unlocks the mutex (if one has been locked previously). - * Unless this function is called, the mutex is unlocked - * when the class exits the scope. - */ - void unlock(); - - - /** - * Get the size of the data - * @return - */ - size_t size() const; - - /** - * Get the storage ID. - * @return - */ - store_address_t getId() const; - - void print() const; -protected: - const uint8_t* constDataPointer = nullptr; - store_address_t storeId; - size_t size_ = 0; - //! Managing pool, has to assign itself. - StorageManagerIF* store = nullptr; - //! Unique pointer to the mutex lock instance. Is initialized by - //! the pool manager. - std::unique_ptr mutexLock = nullptr; - bool deleteData = true; - - enum class AccessState { - UNINIT, - READ - }; - //! Internal state for safety reasons. - AccessState internalState = AccessState::UNINIT; - /** - * Used by the pool manager instances to assign themselves to the - * accessor. This is necessary to delete the data when the acessor - * exits the scope ! The internal state will be considered read - * when this function is called, so take care all data is set properly as - * well. - * @param - */ - void assignStore(StorageManagerIF*); - -}; - - -/** - * @brief Child class for modifyable data. Also has a normal pointer member. - */ -class StorageAccessor: public ConstStorageAccessor { - //! StorageManager classes have exclusive access to private variables. - template - friend class PoolManager; - template - friend class LocalPool; -public: - StorageAccessor(store_address_t storeId); - /** - * @brief Move ctor and move assignment allow returning accessors as - * a returnvalue. They prevent resource being free prematurely. - * Refer to: https://github.com/MicrosoftDocs/cpp-docs/blob/master/docs/cpp/ - * move-constructors-and-move-assignment-operators-cpp.md - * @param - * @return - */ - StorageAccessor& operator= (StorageAccessor&&); - StorageAccessor (StorageAccessor&&); - - ReturnValue_t write(uint8_t *data, size_t size, - uint16_t offset); - uint8_t* data(); - -private: - //! Non-const pointer for modifyable data. - uint8_t* dataPointer = nullptr; - //! For modifyable data, the const pointer is assigned to the normal - //! pointer by the pool manager so both access functions can be used safely - void assignConstPointer(); -}; - -#endif /* TEST_PROTOTYPES_STORAGEACCESSOR_H_ */ +///** +// * @brief Helper classes to facilitate safe access to storages which is also +// * conforming to RAII principles +// * @details These helper can be used together with the +// * StorageManager classes to manage access to a storage. +// * It can take care of thread-safety while also providing +// * mechanisms to automatically clear storage data and unlocking the +// * pool. +// */ +//#ifndef TEST_PROTOTYPES_STORAGEACCESSOR_H_ +//#define TEST_PROTOTYPES_STORAGEACCESSOR_H_ +// +//#include +//#include +//#include +// +// +///** +// * @brief Accessor class which can be returned by pool managers +// * or passed and set by pool managers to have safe access to the pool +// * resources. +// */ +//class ConstStorageAccessor { +// //! StorageManager classes have exclusive access to private variables. +// template +// friend class PoolManager; +// template +// friend class LocalPool; +//public: +// /** +// * @brief Simple constructor which takes the store ID of the storage +// * entry to access. +// * @param storeId +// */ +// ConstStorageAccessor(store_address_t storeId); +// +// /** +// * @brief Move ctor and move assignment allow returning accessors as +// * a returnvalue. They prevent resource being free prematurely. +// * Refer to: https://github.com/MicrosoftDocs/cpp-docs/blob/master/docs/cpp/ +// * move-constructors-and-move-assignment-operators-cpp.md +// * @param +// * @return +// */ +// ConstStorageAccessor& operator= (ConstStorageAccessor&&); +// ConstStorageAccessor (ConstStorageAccessor&&); +// +// //! The copy ctor and copy assignemnt should be deleted implicitely +// //! according to https://foonathan.net/2019/02/special-member-functions/ +// //! but I still deleted them to make it more explicit. (remember rule of 5). +// ConstStorageAccessor& operator= (ConstStorageAccessor&) = delete; +// ConstStorageAccessor (ConstStorageAccessor&) = delete; +// +// /** +// * @brief The destructor in default configuration takes care of +// * deleting the accessed pool entry and unlocking the mutex +// */ +// virtual ~ConstStorageAccessor(); +// +// /** +// * @brief Returns a pointer to the read-only data +// * @return +// */ +// const uint8_t* data() const; +// +// /** +// * @brief Copies the read-only data to the supplied pointer +// * @param pointer +// */ +// void getDataCopy(uint8_t *pointer); +// +// /** +// * @brief Calling this will prevent the Accessor from deleting the data +// * when the destructor is called. +// */ +// void release(); +// /** +// * @brief Locks the supplied mutex. +// * @details +// * The mutex will be unlocked automatically +// * when this class is destroyed (for example when exiting the scope). +// * Only one mutex can be locked at a time! +// * @param mutex +// * @param mutexTimeout +// * @return +// */ +// ReturnValue_t lock(MutexIF* mutex, +// uint32_t mutexTimeout = MutexIF::NO_TIMEOUT); +// /** +// * @brief Unlocks the mutex (if one has been locked previously). +// * Unless this function is called, the mutex is unlocked +// * when the class exits the scope. +// */ +// void unlock(); +// +// +// /** +// * Get the size of the data +// * @return +// */ +// size_t size() const; +// +// /** +// * Get the storage ID. +// * @return +// */ +// store_address_t getId() const; +// +// void print() const; +//protected: +// const uint8_t* constDataPointer = nullptr; +// store_address_t storeId; +// size_t size_ = 0; +// //! Managing pool, has to assign itself. +// StorageManagerIF* store = nullptr; +// //! Unique pointer to the mutex lock instance. Is initialized by +// //! the pool manager. +// std::unique_ptr mutexLock = nullptr; +// bool deleteData = true; +// +// enum class AccessState { +// UNINIT, +// READ +// }; +// //! Internal state for safety reasons. +// AccessState internalState = AccessState::UNINIT; +// /** +// * Used by the pool manager instances to assign themselves to the +// * accessor. This is necessary to delete the data when the acessor +// * exits the scope ! The internal state will be considered read +// * when this function is called, so take care all data is set properly as +// * well. +// * @param +// */ +// void assignStore(StorageManagerIF*); +// +//}; +// +// +///** +// * @brief Child class for modifyable data. Also has a normal pointer member. +// */ +//class StorageAccessor: public ConstStorageAccessor { +// //! StorageManager classes have exclusive access to private variables. +// template +// friend class PoolManager; +// template +// friend class LocalPool; +//public: +// StorageAccessor(store_address_t storeId); +// /** +// * @brief Move ctor and move assignment allow returning accessors as +// * a returnvalue. They prevent resource being free prematurely. +// * Refer to: https://github.com/MicrosoftDocs/cpp-docs/blob/master/docs/cpp/ +// * move-constructors-and-move-assignment-operators-cpp.md +// * @param +// * @return +// */ +// StorageAccessor& operator= (StorageAccessor&&); +// StorageAccessor (StorageAccessor&&); +// +// ReturnValue_t write(uint8_t *data, size_t size, +// uint16_t offset); +// uint8_t* data(); +// +//private: +// //! Non-const pointer for modifyable data. +// uint8_t* dataPointer = nullptr; +// //! For modifyable data, the const pointer is assigned to the normal +// //! pointer by the pool manager so both access functions can be used safely +// void assignConstPointer(); +//}; +// +//#endif /* TEST_PROTOTYPES_STORAGEACCESSOR_H_ */ From 43d3ca7e379cf1dd20cd0372cf4883eb468d030a Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Sun, 10 May 2020 21:56:21 +0200 Subject: [PATCH 20/45] memory message: no retval CSB: retval --- memory/MemoryMessage.cpp | 3 +-- memory/MemoryMessage.h | 2 +- tmtcservices/CommandingServiceBase.h | 2 ++ 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/memory/MemoryMessage.cpp b/memory/MemoryMessage.cpp index c1116e08..f978ed4e 100644 --- a/memory/MemoryMessage.cpp +++ b/memory/MemoryMessage.cpp @@ -31,12 +31,11 @@ ReturnValue_t MemoryMessage::setMemoryDumpReply(CommandMessage* message, store_a return HasReturnvaluesIF::RETURN_OK; } -ReturnValue_t MemoryMessage::setMemoryLoadCommand(CommandMessage* message, +void MemoryMessage::setMemoryLoadCommand(CommandMessage* message, uint32_t address, store_address_t storageID) { message->setCommand(CMD_MEMORY_LOAD); message->setParameter( address ); message->setParameter2( storageID.raw ); - return HasReturnvaluesIF::RETURN_OK; } ReturnValue_t MemoryMessage::getErrorCode(const CommandMessage* message) { diff --git a/memory/MemoryMessage.h b/memory/MemoryMessage.h index 124fad08..bcc262e9 100644 --- a/memory/MemoryMessage.h +++ b/memory/MemoryMessage.h @@ -24,7 +24,7 @@ public: static ReturnValue_t getErrorCode( const CommandMessage* message ); static ReturnValue_t setMemoryDumpCommand( CommandMessage* message, uint32_t address, uint32_t length ); static ReturnValue_t setMemoryDumpReply( CommandMessage* message, store_address_t storageID ); - static ReturnValue_t setMemoryLoadCommand( CommandMessage* message, uint32_t address, store_address_t storageID ); + static void setMemoryLoadCommand( CommandMessage* message, uint32_t address, store_address_t storageID ); static ReturnValue_t setMemoryCheckCommand( CommandMessage* message, uint32_t address, uint32_t length ); static ReturnValue_t setMemoryCheckReply( CommandMessage* message, uint16_t crc ); static ReturnValue_t setMemoryReplyFailed( CommandMessage* message, ReturnValue_t errorCode, Command_t initialCommand ); diff --git a/tmtcservices/CommandingServiceBase.h b/tmtcservices/CommandingServiceBase.h index 6b76bdab..259ed1a3 100644 --- a/tmtcservices/CommandingServiceBase.h +++ b/tmtcservices/CommandingServiceBase.h @@ -212,6 +212,8 @@ protected: */ PeriodicTaskIF* executingTask; + // todo: why do these functions not have returnvalues? the caller should be + // able to check whether the send operations actually work. /** * Send TM data from pointer to data. If a header is supplied it is added before data * @param subservice Number of subservice From effac0e9b7c46913283c45f483b37df3335e8042 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 11 May 2020 12:22:06 +0200 Subject: [PATCH 21/45] global printer function --- globalfunctions/printer.cpp | 13 +++++++++++++ globalfunctions/printer.h | 12 ++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 globalfunctions/printer.cpp create mode 100644 globalfunctions/printer.h diff --git a/globalfunctions/printer.cpp b/globalfunctions/printer.cpp new file mode 100644 index 00000000..d6ede0a5 --- /dev/null +++ b/globalfunctions/printer.cpp @@ -0,0 +1,13 @@ +#include +#include + +void printer::print(uint8_t *data, size_t size) { + sif::info << "StorageAccessor: Printing data: ["; + for(size_t i = 0; i < size; i++) { + sif::info << std::hex << (int)data[i]; + if(i < size - 1){ + sif::info << " , "; + } + } + sif::info << " ] " << std::endl; +} diff --git a/globalfunctions/printer.h b/globalfunctions/printer.h new file mode 100644 index 00000000..d4a573e9 --- /dev/null +++ b/globalfunctions/printer.h @@ -0,0 +1,12 @@ +#ifndef FRAMEWORK_GLOBALFUNCTIONS_PRINTER_H_ +#define FRAMEWORK_GLOBALFUNCTIONS_PRINTER_H_ +#include +#include + +namespace printer { +void print(uint8_t *data, size_t size); +} + + + +#endif /* FRAMEWORK_GLOBALFUNCTIONS_PRINTER_H_ */ From 3d8a152496cb05b1675f5f1bf4ee1a8e579e18c0 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 11 May 2020 17:14:12 +0200 Subject: [PATCH 22/45] removed newline --- osal/FreeRTOS/FixedTimeslotTask.h | 1 - 1 file changed, 1 deletion(-) diff --git a/osal/FreeRTOS/FixedTimeslotTask.h b/osal/FreeRTOS/FixedTimeslotTask.h index 1ab8724f..bed13051 100644 --- a/osal/FreeRTOS/FixedTimeslotTask.h +++ b/osal/FreeRTOS/FixedTimeslotTask.h @@ -51,7 +51,6 @@ public: ReturnValue_t checkSequence() const; ReturnValue_t sleepFor(uint32_t ms); - protected: bool started; TaskHandle_t handle; From eb376318c3ad8a415649fd8062db35506635b73a Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 11 May 2020 17:20:10 +0200 Subject: [PATCH 23/45] comment format --- devicehandlers/FixedSlotSequence.h | 50 ++++++++++++++++++------------ 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/devicehandlers/FixedSlotSequence.h b/devicehandlers/FixedSlotSequence.h index 2fd866ac..fa4c4aeb 100644 --- a/devicehandlers/FixedSlotSequence.h +++ b/devicehandlers/FixedSlotSequence.h @@ -3,7 +3,6 @@ #include #include -#include #include using SlotList = std::multiset; @@ -72,9 +71,10 @@ public: bool slotFollowsImmediately(); /** - * \brief This method returns the time until the next software component is invoked. + * @brief This method returns the time until the next software + * component is invoked. * - * \details + * @details * This method is vitally important for the operation of the PST. * By fetching the polling time of the current slot and that of the * next one (or the first one, if the list end is reached) @@ -86,11 +86,15 @@ public: uint32_t getIntervalToNextSlotMs(); /** - * \brief This method returns the time difference between the current slot and the previous slot + * @brief This method returns the time difference between the current + * slot and the previous slot * - * \details This method is vitally important for the operation of the PST. By fetching the polling time - * of the current slot and that of the prevous one (or the last one, if the slot is the first one) - * it calculates and returns the interval in milliseconds that the handler execution shall be delayed. + * @details + * This method is vitally important for the operation of the PST. + * By fetching the polling time of the current slot and that of the previous + * one (or the last one, if the slot is the first one) it calculates and + * returns the interval in milliseconds that the handler execution shall + * be delayed. */ uint32_t getIntervalToPreviousSlotMs(); @@ -100,20 +104,24 @@ public: uint32_t getLengthMs() const; /** - * \brief The method to execute the device handler entered in the current PollingSlot object. + * @brief The method to execute the device handler entered in the current + * PollingSlot object. * - * \details Within this method the device handler object to be executed is chosen by looking up the - * handler address of the current slot in the handlerMap. Either the device handler's - * talkToInterface or its listenToInterface method is invoked, depending on the isTalking flag - * of the polling slot. After execution the iterator current is increased or, by reaching the - * end of slotList, reset to the beginning. + * @details + * Within this method the device handler object to be executed is chosen by + * looking up the handler address of the current slot in the handlerMap. + * Either the device handler's talkToInterface or its listenToInterface + * method is invoked, depending on the isTalking flag of the polling slot. + * After execution the iterator current is increased or, by reaching the + * end of slotList, reset to the beginning. */ void executeAndAdvance(); /** * @brief An iterator that indicates the current polling slot to execute. * - * @details This is an iterator for slotList and always points to the polling slot which is executed next. + * @details This is an iterator for slotList and always points to the + * polling slot which is executed next. */ SlotListIter current; @@ -127,13 +135,15 @@ public: protected: /** - * @brief This list contains all PollingSlot objects, defining order and execution time of the - * device handler objects. + * @brief This list contains all PollingSlot objects, defining order and + * execution time of the device handler objects. * - * @details The slot list is a std:list object that contains all created PollingSlot instances. - * They are NOT ordered automatically, so by adding entries, the correct order needs to be ensured. - * By iterating through this list the polling sequence is executed. Two entries with identical - * polling times are executed immediately one after another. + * @details + * The slot list is a std:list object that contains all created + * PollingSlot instances. They are NOT ordered automatically, so by + * adding entries, the correct order needs to be ensured. By iterating + * through this list the polling sequence is executed. Two entries with + * identical polling times are executed immediately one after another. */ SlotList slotList; From 26103aa8ccd15ca347ee885cd34c98cebdeeaa10 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 11 May 2020 17:21:24 +0200 Subject: [PATCH 24/45] fixed sequence slot comments --- devicehandlers/FixedSequenceSlot.cpp | 3 +-- devicehandlers/FixedSequenceSlot.h | 9 +++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/devicehandlers/FixedSequenceSlot.cpp b/devicehandlers/FixedSequenceSlot.cpp index 29e7ee6a..bb97a8e5 100644 --- a/devicehandlers/FixedSequenceSlot.cpp +++ b/devicehandlers/FixedSequenceSlot.cpp @@ -16,6 +16,5 @@ FixedSequenceSlot::FixedSequenceSlot(object_id_t handlerId, uint32_t setTime, handler->setTaskIF(executingTask); } -FixedSequenceSlot::~FixedSequenceSlot() { -} +FixedSequenceSlot::~FixedSequenceSlot() {} diff --git a/devicehandlers/FixedSequenceSlot.h b/devicehandlers/FixedSequenceSlot.h index b3bac08f..b18d2768 100644 --- a/devicehandlers/FixedSequenceSlot.h +++ b/devicehandlers/FixedSequenceSlot.h @@ -13,9 +13,10 @@ class PeriodicTaskIF; /** - * \brief This class is the representation of a single polling sequence table entry. + * @brief This class is the representation of a single polling sequence table entry. * - * \details The PollingSlot class is the representation of a single polling sequence table entry. + * @details The PollingSlot class is the representation of a single polling + * sequence table entry. */ class FixedSequenceSlot { public: @@ -37,9 +38,9 @@ public: uint32_t pollingTimeMs; /** - * \brief This value defines the type of device communication. + * @brief This value defines the type of device communication. * - * \details The state of this value decides what communication routine is + * @details The state of this value decides what communication routine is * called in the PST executable or the device handler object. */ uint8_t opcode; From 77565c741245a936cbbb7504aa8d432bd7b5d71e Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 11 May 2020 19:20:21 +0200 Subject: [PATCH 25/45] additional comment for oprator overload --- devicehandlers/FixedSequenceSlot.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/devicehandlers/FixedSequenceSlot.h b/devicehandlers/FixedSequenceSlot.h index b18d2768..7868cce3 100644 --- a/devicehandlers/FixedSequenceSlot.h +++ b/devicehandlers/FixedSequenceSlot.h @@ -45,6 +45,12 @@ public: */ uint8_t opcode; + /** + * @brief Operator overload for the comparison operator to + * allow sorting by polling time. + * @param fixedSequenceSlot + * @return + */ bool operator <(const FixedSequenceSlot & fixedSequenceSlot) const { return pollingTimeMs < fixedSequenceSlot.pollingTimeMs; } From 1946af64af5afb479bda4604b33d712eef99e43a Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 12 May 2020 14:11:00 +0200 Subject: [PATCH 26/45] storage accessor mutex lock removed --- storagemanager/StorageAccessor.cpp | 284 ++++++++++++------------- storagemanager/StorageAccessor.h | 326 ++++++++++++++--------------- 2 files changed, 282 insertions(+), 328 deletions(-) diff --git a/storagemanager/StorageAccessor.cpp b/storagemanager/StorageAccessor.cpp index a068d169..51b93f11 100644 --- a/storagemanager/StorageAccessor.cpp +++ b/storagemanager/StorageAccessor.cpp @@ -1,154 +1,130 @@ -//#include -// -//ConstStorageAccessor::ConstStorageAccessor(store_address_t storeId): storeId(storeId) {} -// -//ConstStorageAccessor::~ConstStorageAccessor() { -// if(deleteData and store != nullptr) { -// sif::debug << "deleting store data" << std::endl; -// store->deleteDataNonLocking(storeId); -// } -// if(mutexLock != nullptr) { -// sif::debug << "unlocking mutex lock" << std::endl; -// mutexLock.reset(); -// } -//} -// -//ConstStorageAccessor& ConstStorageAccessor::operator=( -// ConstStorageAccessor&& other) { -// constDataPointer = other.constDataPointer; -// storeId = other.storeId; -// store = other.store; -// size_ = other.size_; -// deleteData = other.deleteData; -// this->store = other.store; -// // Transfer ownership of the lock. -// mutexLock = std::move(other.mutexLock); -// // This prevents double deletion of the resource -// other.mutexLock = nullptr; -// // This prevent premature deletion -// other.store = nullptr; -// return *this; -//} -// -//StorageAccessor::StorageAccessor(store_address_t storeId): -// ConstStorageAccessor(storeId) { -//} -// -//StorageAccessor& StorageAccessor::operator =( -// StorageAccessor&& other) { -// // Call the parent move assignment and also assign own member. -// dataPointer = other.dataPointer; -// StorageAccessor::operator=(std::move(other)); -// return * this; -//} -// -//// Call the parent move ctor and also transfer own member. -//StorageAccessor::StorageAccessor(StorageAccessor&& other): -// ConstStorageAccessor(std::move(other)), dataPointer(other.dataPointer) { -//} -// -//ConstStorageAccessor::ConstStorageAccessor(ConstStorageAccessor&& other): -// constDataPointer(other.constDataPointer), storeId(other.storeId), -// size_(other.size_), store(other.store), deleteData(other.deleteData), -// internalState(other.internalState) { -// // Transfer ownership of the lock. -// mutexLock = std::move(other.mutexLock); -// // This prevents double deletion of the resource. Not strictly necessary, -// // from the testing I have conducted so far but I am not familiar enough -// // with move semantics so I will just set the other lock to nullptr for now. -// other.mutexLock = nullptr; -// // This prevent premature deletion -// other.store = nullptr; -//} -// -//const uint8_t* ConstStorageAccessor::data() const { -// return constDataPointer; -//} -// -//size_t ConstStorageAccessor::size() const { -// if(internalState == AccessState::UNINIT) { -// sif::warning << "StorageAccessor: Not initialized!" << std::endl; -// } -// return size_; -//} -// -//void ConstStorageAccessor::getDataCopy(uint8_t *pointer) { -// if(internalState == AccessState::UNINIT) { -// sif::warning << "StorageAccessor: Not initialized!" << std::endl; -// return; -// } -// std::copy(constDataPointer, constDataPointer + size_, pointer); -//} -// -//void ConstStorageAccessor::release() { -// deleteData = false; -//} -// -//ReturnValue_t ConstStorageAccessor::lock(MutexIF* mutex, uint32_t mutexTimeout) { -// if(mutexLock == nullptr) { -// mutexLock = std::unique_ptr(new MutexHelper(mutex, mutexTimeout)); -// return mutexLock.get()->getResult(); -// } -// else { -// sif::warning << "StorageAccessor: Attempted to lock twice. Check code!" << std::endl; -// return HasReturnvaluesIF::RETURN_FAILED; -// } -//} -// -//void ConstStorageAccessor::unlock() { -// if(mutexLock != nullptr) { -// mutexLock.reset(); -// } -//} -// -//store_address_t ConstStorageAccessor::getId() const { -// return storeId; -//} -// -//void ConstStorageAccessor::print() const { -// if(internalState == AccessState::UNINIT) { -// sif::warning << "StorageAccessor: Not initialized!" << std::endl; -// return; -// } -// sif::info << "StorageAccessor: Printing data: ["; -// for(uint16_t iPool = 0; iPool < size_; iPool++) { -// sif::info << std::hex << (int)constDataPointer[iPool]; -// if(iPool < size_ - 1){ -// sif::info << " , "; -// } -// } -// sif::info << " ] " << std::endl; -//} -// -//void ConstStorageAccessor::assignStore(StorageManagerIF* store) { -// internalState = AccessState::READ; -// this->store = store; -//} -// -// -//uint8_t* StorageAccessor::data() { -// if(internalState == AccessState::UNINIT) { -// sif::warning << "StorageAccessor: Not initialized!" << std::endl; -// } -// return dataPointer; -//} -// -//ReturnValue_t StorageAccessor::write(uint8_t *data, size_t size, -// uint16_t offset) { -// if(internalState == AccessState::UNINIT) { -// sif::warning << "StorageAccessor: Not initialized!" << std::endl; -// return HasReturnvaluesIF::RETURN_FAILED; -// } -// if(offset + size > size_) { -// sif::error << "StorageAccessor: Data too large for pool entry!" << std::endl; -// return HasReturnvaluesIF::RETURN_FAILED; -// } -// std::copy(data, data + size, dataPointer); -// return HasReturnvaluesIF::RETURN_OK; -//} -// -//void StorageAccessor::assignConstPointer() { -// constDataPointer = dataPointer; -//} -// -// +#include + +ConstStorageAccessor::ConstStorageAccessor(store_address_t storeId): + storeId(storeId) {} + +ConstStorageAccessor::~ConstStorageAccessor() { + if(deleteData and store != nullptr) { + sif::debug << "deleting store data" << std::endl; + store->deleteData(storeId); + } +} + +ConstStorageAccessor& ConstStorageAccessor::operator=( + ConstStorageAccessor&& other) { + constDataPointer = other.constDataPointer; + storeId = other.storeId; + store = other.store; + size_ = other.size_; + deleteData = other.deleteData; + this->store = other.store; + // This prevents premature deletion + other.store = nullptr; + return *this; +} + +StorageAccessor::StorageAccessor(store_address_t storeId): + ConstStorageAccessor(storeId) { +} + +StorageAccessor& StorageAccessor::operator =( + StorageAccessor&& other) { + // Call the parent move assignment and also assign own member. + dataPointer = other.dataPointer; + StorageAccessor::operator=(std::move(other)); + return * this; +} + +// Call the parent move ctor and also transfer own member. +StorageAccessor::StorageAccessor(StorageAccessor&& other): + ConstStorageAccessor(std::move(other)), dataPointer(other.dataPointer) { +} + +ConstStorageAccessor::ConstStorageAccessor(ConstStorageAccessor&& other): + constDataPointer(other.constDataPointer), storeId(other.storeId), + size_(other.size_), store(other.store), deleteData(other.deleteData), + internalState(other.internalState) { + // This prevent premature deletion + other.store = nullptr; +} + +const uint8_t* ConstStorageAccessor::data() const { + return constDataPointer; +} + +size_t ConstStorageAccessor::size() const { + if(internalState == AccessState::UNINIT) { + sif::warning << "StorageAccessor: Not initialized!" << std::endl; + } + return size_; +} + +ReturnValue_t ConstStorageAccessor::getDataCopy(uint8_t *pointer, + size_t maxSize) { + if(internalState == AccessState::UNINIT) { + sif::warning << "StorageAccessor: Not initialized!" << std::endl; + return HasReturnvaluesIF::RETURN_FAILED; + } + if(size_ > maxSize) { + sif::error << "StorageAccessor: Supplied buffer not large enough" << std::endl; + return HasReturnvaluesIF::RETURN_FAILED; + } + std::copy(constDataPointer, constDataPointer + size_, pointer); + return HasReturnvaluesIF::RETURN_OK; +} + +void ConstStorageAccessor::release() { + deleteData = false; +} + +store_address_t ConstStorageAccessor::getId() const { + return storeId; +} + +void ConstStorageAccessor::print() const { + if(internalState == AccessState::UNINIT) { + sif::warning << "StorageAccessor: Not initialized!" << std::endl; + return; + } + sif::info << "StorageAccessor: Printing data: ["; + for(uint16_t iPool = 0; iPool < size_; iPool++) { + sif::info << std::hex << (int)constDataPointer[iPool]; + if(iPool < size_ - 1){ + sif::info << " , "; + } + } + sif::info << " ] " << std::endl; +} + +void ConstStorageAccessor::assignStore(StorageManagerIF* store) { + internalState = AccessState::READ; + this->store = store; +} + + +uint8_t* StorageAccessor::data() { + if(internalState == AccessState::UNINIT) { + sif::warning << "StorageAccessor: Not initialized!" << std::endl; + } + return dataPointer; +} + +ReturnValue_t StorageAccessor::write(uint8_t *data, size_t size, + uint16_t offset) { + if(internalState == AccessState::UNINIT) { + sif::warning << "StorageAccessor: Not initialized!" << std::endl; + return HasReturnvaluesIF::RETURN_FAILED; + } + if(offset + size > size_) { + sif::error << "StorageAccessor: Data too large for pool entry!" << std::endl; + return HasReturnvaluesIF::RETURN_FAILED; + } + std::copy(data, data + size, dataPointer); + return HasReturnvaluesIF::RETURN_OK; +} + +void StorageAccessor::assignConstPointer() { + constDataPointer = dataPointer; +} + + diff --git a/storagemanager/StorageAccessor.h b/storagemanager/StorageAccessor.h index d391c5c2..2e1d44bd 100644 --- a/storagemanager/StorageAccessor.h +++ b/storagemanager/StorageAccessor.h @@ -1,174 +1,152 @@ -///** -// * @brief Helper classes to facilitate safe access to storages which is also -// * conforming to RAII principles -// * @details These helper can be used together with the -// * StorageManager classes to manage access to a storage. -// * It can take care of thread-safety while also providing -// * mechanisms to automatically clear storage data and unlocking the -// * pool. -// */ -//#ifndef TEST_PROTOTYPES_STORAGEACCESSOR_H_ -//#define TEST_PROTOTYPES_STORAGEACCESSOR_H_ -// -//#include -//#include -//#include -// -// -///** -// * @brief Accessor class which can be returned by pool managers -// * or passed and set by pool managers to have safe access to the pool -// * resources. -// */ -//class ConstStorageAccessor { -// //! StorageManager classes have exclusive access to private variables. -// template -// friend class PoolManager; -// template -// friend class LocalPool; -//public: -// /** -// * @brief Simple constructor which takes the store ID of the storage -// * entry to access. -// * @param storeId -// */ -// ConstStorageAccessor(store_address_t storeId); -// -// /** -// * @brief Move ctor and move assignment allow returning accessors as -// * a returnvalue. They prevent resource being free prematurely. -// * Refer to: https://github.com/MicrosoftDocs/cpp-docs/blob/master/docs/cpp/ -// * move-constructors-and-move-assignment-operators-cpp.md -// * @param -// * @return -// */ -// ConstStorageAccessor& operator= (ConstStorageAccessor&&); -// ConstStorageAccessor (ConstStorageAccessor&&); -// -// //! The copy ctor and copy assignemnt should be deleted implicitely -// //! according to https://foonathan.net/2019/02/special-member-functions/ -// //! but I still deleted them to make it more explicit. (remember rule of 5). -// ConstStorageAccessor& operator= (ConstStorageAccessor&) = delete; -// ConstStorageAccessor (ConstStorageAccessor&) = delete; -// -// /** -// * @brief The destructor in default configuration takes care of -// * deleting the accessed pool entry and unlocking the mutex -// */ -// virtual ~ConstStorageAccessor(); -// -// /** -// * @brief Returns a pointer to the read-only data -// * @return -// */ -// const uint8_t* data() const; -// -// /** -// * @brief Copies the read-only data to the supplied pointer -// * @param pointer -// */ -// void getDataCopy(uint8_t *pointer); -// -// /** -// * @brief Calling this will prevent the Accessor from deleting the data -// * when the destructor is called. -// */ -// void release(); -// /** -// * @brief Locks the supplied mutex. -// * @details -// * The mutex will be unlocked automatically -// * when this class is destroyed (for example when exiting the scope). -// * Only one mutex can be locked at a time! -// * @param mutex -// * @param mutexTimeout -// * @return -// */ -// ReturnValue_t lock(MutexIF* mutex, -// uint32_t mutexTimeout = MutexIF::NO_TIMEOUT); -// /** -// * @brief Unlocks the mutex (if one has been locked previously). -// * Unless this function is called, the mutex is unlocked -// * when the class exits the scope. -// */ -// void unlock(); -// -// -// /** -// * Get the size of the data -// * @return -// */ -// size_t size() const; -// -// /** -// * Get the storage ID. -// * @return -// */ -// store_address_t getId() const; -// -// void print() const; -//protected: -// const uint8_t* constDataPointer = nullptr; -// store_address_t storeId; -// size_t size_ = 0; -// //! Managing pool, has to assign itself. -// StorageManagerIF* store = nullptr; -// //! Unique pointer to the mutex lock instance. Is initialized by -// //! the pool manager. -// std::unique_ptr mutexLock = nullptr; -// bool deleteData = true; -// -// enum class AccessState { -// UNINIT, -// READ -// }; -// //! Internal state for safety reasons. -// AccessState internalState = AccessState::UNINIT; -// /** -// * Used by the pool manager instances to assign themselves to the -// * accessor. This is necessary to delete the data when the acessor -// * exits the scope ! The internal state will be considered read -// * when this function is called, so take care all data is set properly as -// * well. -// * @param -// */ -// void assignStore(StorageManagerIF*); -// -//}; -// -// -///** -// * @brief Child class for modifyable data. Also has a normal pointer member. -// */ -//class StorageAccessor: public ConstStorageAccessor { -// //! StorageManager classes have exclusive access to private variables. -// template -// friend class PoolManager; -// template -// friend class LocalPool; -//public: -// StorageAccessor(store_address_t storeId); -// /** -// * @brief Move ctor and move assignment allow returning accessors as -// * a returnvalue. They prevent resource being free prematurely. -// * Refer to: https://github.com/MicrosoftDocs/cpp-docs/blob/master/docs/cpp/ -// * move-constructors-and-move-assignment-operators-cpp.md -// * @param -// * @return -// */ -// StorageAccessor& operator= (StorageAccessor&&); -// StorageAccessor (StorageAccessor&&); -// -// ReturnValue_t write(uint8_t *data, size_t size, -// uint16_t offset); -// uint8_t* data(); -// -//private: -// //! Non-const pointer for modifyable data. -// uint8_t* dataPointer = nullptr; -// //! For modifyable data, the const pointer is assigned to the normal -// //! pointer by the pool manager so both access functions can be used safely -// void assignConstPointer(); -//}; -// -//#endif /* TEST_PROTOTYPES_STORAGEACCESSOR_H_ */ +/** + * @brief Helper classes to facilitate safe access to storages which is also + * conforming to RAII principles + * @details These helper can be used together with the + * StorageManager classes to manage access to a storage. + * It can take care of thread-safety while also providing + * mechanisms to automatically clear storage data and unlocking the + * pool. + */ +#ifndef TEST_PROTOTYPES_STORAGEACCESSOR_H_ +#define TEST_PROTOTYPES_STORAGEACCESSOR_H_ + +#include +#include +#include + + +/** + * @brief Accessor class which can be returned by pool managers + * or passed and set by pool managers to have safe access to the pool + * resources. + */ +class ConstStorageAccessor { + //! StorageManager classes have exclusive access to private variables. + template + friend class PoolManager; + template + friend class LocalPool; +public: + /** + * @brief Simple constructor which takes the store ID of the storage + * entry to access. + * @param storeId + */ + ConstStorageAccessor(store_address_t storeId); + + /** + * @brief Move ctor and move assignment allow returning accessors as + * a returnvalue. They prevent resource being free prematurely. + * Refer to: https://github.com/MicrosoftDocs/cpp-docs/blob/master/docs/cpp/ + * move-constructors-and-move-assignment-operators-cpp.md + * @param + * @return + */ + ConstStorageAccessor& operator= (ConstStorageAccessor&&); + ConstStorageAccessor (ConstStorageAccessor&&); + + //! The copy ctor and copy assignemnt should be deleted implicitely + //! according to https://foonathan.net/2019/02/special-member-functions/ + //! but I still deleted them to make it more explicit. (remember rule of 5). + ConstStorageAccessor& operator= (ConstStorageAccessor&) = delete; + ConstStorageAccessor (ConstStorageAccessor&) = delete; + + /** + * @brief The destructor in default configuration takes care of + * deleting the accessed pool entry and unlocking the mutex + */ + virtual ~ConstStorageAccessor(); + + /** + * @brief Returns a pointer to the read-only data + * @return + */ + const uint8_t* data() const; + + /** + * @brief Copies the read-only data to the supplied pointer + * @param pointer + */ + ReturnValue_t getDataCopy(uint8_t *pointer, size_t maxSize); + + /** + * @brief Calling this will prevent the Accessor from deleting the data + * when the destructor is called. + */ + void release(); + + /** + * Get the size of the data + * @return + */ + size_t size() const; + + /** + * Get the storage ID. + * @return + */ + store_address_t getId() const; + + void print() const; +protected: + const uint8_t* constDataPointer = nullptr; + store_address_t storeId; + size_t size_ = 0; + //! Managing pool, has to assign itself. + StorageManagerIF* store = nullptr; + bool deleteData = true; + + enum class AccessState { + UNINIT, + READ + }; + //! Internal state for safety reasons. + AccessState internalState = AccessState::UNINIT; + /** + * Used by the pool manager instances to assign themselves to the + * accessor. This is necessary to delete the data when the acessor + * exits the scope ! The internal state will be considered read + * when this function is called, so take care all data is set properly as + * well. + * @param + */ + void assignStore(StorageManagerIF*); + +}; + + +/** + * @brief Child class for modifyable data. Also has a normal pointer member. + */ +class StorageAccessor: public ConstStorageAccessor { + //! StorageManager classes have exclusive access to private variables. + template + friend class PoolManager; + template + friend class LocalPool; +public: + StorageAccessor(store_address_t storeId); + /** + * @brief Move ctor and move assignment allow returning accessors as + * a returnvalue. They prevent resource being free prematurely. + * Refer to: https://github.com/MicrosoftDocs/cpp-docs/blob/master/docs/cpp/ + * move-constructors-and-move-assignment-operators-cpp.md + * @param + * @return + */ + StorageAccessor& operator= (StorageAccessor&&); + StorageAccessor (StorageAccessor&&); + + ReturnValue_t write(uint8_t *data, size_t size, + uint16_t offset); + uint8_t* data(); + +private: + //! Non-const pointer for modifyable data. + uint8_t* dataPointer = nullptr; + //! For modifyable data, the const pointer is assigned to the normal + //! pointer by the pool manager so both access functions can be used safely + void assignConstPointer(); +}; + +#endif /* TEST_PROTOTYPES_STORAGEACCESSOR_H_ */ From 5af0c15dfc6906eb16675e53416c0fe2f95b34b0 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 12 May 2020 14:12:39 +0200 Subject: [PATCH 27/45] simplified storage accessor --- ipc/MutexHelper.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ipc/MutexHelper.h b/ipc/MutexHelper.h index f76ccec4..671cd5a6 100644 --- a/ipc/MutexHelper.h +++ b/ipc/MutexHelper.h @@ -10,11 +10,11 @@ public: internalMutex(mutex) { ReturnValue_t status = mutex->lockMutex(timeoutMs); if(status != HasReturnvaluesIF::RETURN_OK){ - sif::error << "MutexHelper: Lock of Mutex failed " << status << std::endl; + sif::error << "MutexHelper: Lock of Mutex failed " << + status << std::endl; } } - ~MutexHelper() { internalMutex->unlockMutex(); } From 6c70abfe1621a83074f21476d680bc808a52576f Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 12 May 2020 16:32:01 +0200 Subject: [PATCH 28/45] moved pool accessor fuctions to local pool --- storagemanager/LocalPool.h | 10 ++++++- storagemanager/LocalPool.tpp | 43 +++++++++++++++++++++++++++++-- storagemanager/PoolManager.h | 19 +++++++------- storagemanager/PoolManager.tpp | 5 ++++ storagemanager/StorageAccessor.h | 4 +-- storagemanager/StorageManagerIF.h | 17 +++++++++++- 6 files changed, 82 insertions(+), 16 deletions(-) diff --git a/storagemanager/LocalPool.h b/storagemanager/LocalPool.h index a6334458..6892b881 100644 --- a/storagemanager/LocalPool.h +++ b/storagemanager/LocalPool.h @@ -6,7 +6,8 @@ #include #include #include -#include +#include +#include /** * @brief The LocalPool class provides an intermediate data storage with @@ -67,10 +68,17 @@ public: size_t size, bool ignoreFault = false) override; ReturnValue_t getFreeElement(store_address_t* storageId,const size_t size, uint8_t** p_data, bool ignoreFault = false) override; + + ConstAccessorPair getData(store_address_t packet_id) override; + ReturnValue_t getData(store_address_t packet_id, ConstStorageAccessor&) override; ReturnValue_t getData(store_address_t packet_id, const uint8_t** packet_ptr, size_t * size) override; + + AccessorPair modifyData(store_address_t packet_id) override; + ReturnValue_t modifyData(store_address_t packet_id, StorageAccessor&) override; ReturnValue_t modifyData(store_address_t packet_id, uint8_t** packet_ptr, size_t * size) override; + virtual ReturnValue_t deleteData(store_address_t) override; virtual ReturnValue_t deleteData(uint8_t* ptr, size_t size, store_address_t* storeId = NULL) override; diff --git a/storagemanager/LocalPool.tpp b/storagemanager/LocalPool.tpp index c46e1a8d..31034974 100644 --- a/storagemanager/LocalPool.tpp +++ b/storagemanager/LocalPool.tpp @@ -121,8 +121,8 @@ inline LocalPool::~LocalPool(void) { } } -template -inline ReturnValue_t LocalPool::addData(store_address_t* storageId, +template inline +ReturnValue_t LocalPool::addData(store_address_t* storageId, const uint8_t* data, size_t size, bool ignoreFault) { ReturnValue_t status = reserveSpace(size, storageId, ignoreFault); if (status == RETURN_OK) { @@ -144,6 +144,25 @@ inline ReturnValue_t LocalPool::getFreeElement( return status; } +template +inline ConstAccessorPair LocalPool::getData( + store_address_t storeId) { + uint8_t* tempData = nullptr; + ConstStorageAccessor constAccessor(storeId); + ReturnValue_t status = modifyData(storeId, &tempData, &constAccessor.size_); + constAccessor.constDataPointer = tempData; + return ConstAccessorPair(status, std::move(constAccessor)); +} + +template +inline ReturnValue_t LocalPool::getData(store_address_t storeId, + ConstStorageAccessor& storeAccessor) { + uint8_t* tempData = nullptr; + ReturnValue_t status = modifyData(storeId, &tempData, &storeAccessor.size_); + storeAccessor.constDataPointer = tempData; + return status; +} + template inline ReturnValue_t LocalPool::getData( store_address_t packet_id, const uint8_t** packet_ptr, size_t* size) { @@ -153,6 +172,26 @@ inline ReturnValue_t LocalPool::getData( return status; } +template +inline AccessorPair LocalPool::modifyData( + store_address_t storeId) { + uint8_t* tempData = nullptr; + StorageAccessor accessor(storeId); + ReturnValue_t status = modifyData(storeId, &tempData, &accessor.size_); + accessor.constDataPointer = tempData; + accessor.assignConstPointer(); + return AccessorPair(status, std::move(accessor)); +} + +template +inline ReturnValue_t LocalPool::modifyData( + store_address_t storeId, StorageAccessor& storeAccessor) { + ReturnValue_t status = modifyData(storeId, &storeAccessor.dataPointer, + &storeAccessor.size_); + storeAccessor.assignConstPointer(); + return status; +} + template inline ReturnValue_t LocalPool::modifyData( store_address_t packet_id, uint8_t** packet_ptr, size_t* size) { diff --git a/storagemanager/PoolManager.h b/storagemanager/PoolManager.h index 41325273..87670a82 100644 --- a/storagemanager/PoolManager.h +++ b/storagemanager/PoolManager.h @@ -1,9 +1,9 @@ #ifndef POOLMANAGER_H_ #define POOLMANAGER_H_ - #include #include +#include /** * @brief The PoolManager class provides an intermediate data storage with @@ -11,20 +11,21 @@ * @details Uses local pool calls but is thread safe by protecting the call * with a lock. */ - -template +template class PoolManager : public LocalPool { public: - PoolManager( object_id_t setObjectId, const uint16_t element_sizes[NUMBER_OF_POOLS], - const uint16_t n_elements[NUMBER_OF_POOLS] ); - /** - * @brief In the PoolManager's destructor all allocated memory is freed. - */ + PoolManager(object_id_t setObjectId, + const uint16_t element_sizes[NUMBER_OF_POOLS], + const uint16_t n_elements[NUMBER_OF_POOLS]); + + //! @brief In the PoolManager's destructor all allocated memory is freed. virtual ~PoolManager(); + //! @brief LocalPool overrides for thread-safety. Decorator function which + //! wraps LocalPool calls with a mutex protection. ReturnValue_t deleteData(store_address_t) override; ReturnValue_t deleteData(uint8_t* buffer, size_t size, - store_address_t* storeId = NULL) override; + store_address_t* storeId = nullptr) override; protected: ReturnValue_t reserveSpace(const uint32_t size, store_address_t* address, bool ignoreFault) override; diff --git a/storagemanager/PoolManager.tpp b/storagemanager/PoolManager.tpp index 0408d55b..7025d795 100644 --- a/storagemanager/PoolManager.tpp +++ b/storagemanager/PoolManager.tpp @@ -1,3 +1,6 @@ +#ifndef POOLMANAGER_TPP_ +#define POOLMANAGER_TPP_ + template inline PoolManager::PoolManager(object_id_t setObjectId, const uint16_t element_sizes[NUMBER_OF_POOLS], @@ -40,3 +43,5 @@ inline ReturnValue_t PoolManager::deleteData(uint8_t* buffer, return status; } +#endif + diff --git a/storagemanager/StorageAccessor.h b/storagemanager/StorageAccessor.h index 2e1d44bd..da319a97 100644 --- a/storagemanager/StorageAccessor.h +++ b/storagemanager/StorageAccessor.h @@ -4,8 +4,7 @@ * @details These helper can be used together with the * StorageManager classes to manage access to a storage. * It can take care of thread-safety while also providing - * mechanisms to automatically clear storage data and unlocking the - * pool. + * mechanisms to automatically clear storage data. */ #ifndef TEST_PROTOTYPES_STORAGEACCESSOR_H_ #define TEST_PROTOTYPES_STORAGEACCESSOR_H_ @@ -14,7 +13,6 @@ #include #include - /** * @brief Accessor class which can be returned by pool managers * or passed and set by pool managers to have safe access to the pool diff --git a/storagemanager/StorageManagerIF.h b/storagemanager/StorageManagerIF.h index d1f9cde5..27448ef7 100644 --- a/storagemanager/StorageManagerIF.h +++ b/storagemanager/StorageManagerIF.h @@ -3,7 +3,14 @@ #include #include -#include +#include +#include + +class StorageAccessor; +class ConstStorageAccessor; + +using AccessorPair = std::pair; +using ConstAccessorPair = std::pair; /** * This union defines the type that identifies where a data packet is @@ -150,6 +157,14 @@ public: */ virtual ReturnValue_t getFreeElement(store_address_t* storageId, const size_t size, uint8_t** p_data, bool ignoreFault = false ) = 0; + + virtual AccessorPair modifyData(store_address_t storeId) = 0; + virtual ConstAccessorPair getData(store_address_t storeId) = 0; + virtual ReturnValue_t modifyData(store_address_t storeId, + StorageAccessor&) = 0; + virtual ReturnValue_t getData(store_address_t storeId, + ConstStorageAccessor&) = 0; + /** * Clears the whole store. * Use with care! From d873fcbf8ece518e1da89500ef8195da4384c1a3 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 12 May 2020 16:47:47 +0200 Subject: [PATCH 29/45] added documentation for storage manager IF --- storagemanager/StorageManagerIF.h | 53 +++++++++++++++++++++++++------ 1 file changed, 44 insertions(+), 9 deletions(-) diff --git a/storagemanager/StorageManagerIF.h b/storagemanager/StorageManagerIF.h index 27448ef7..b5508da8 100644 --- a/storagemanager/StorageManagerIF.h +++ b/storagemanager/StorageManagerIF.h @@ -124,6 +124,27 @@ public: */ virtual ReturnValue_t deleteData(uint8_t* buffer, size_t size, store_address_t* storeId = nullptr) = 0; + + /** + * @brief Access the data by supplying a store ID. + * @details + * A pair consisting of the retrieval result and an instance of a + * ConstStorageAccessor class is returned + * @param storeId + * @return Pair of return value and a ConstStorageAccessor instance + */ + virtual ConstAccessorPair getData(store_address_t storeId) = 0; + + /** + * @brief Access the data by supplying a store ID and a helper + * instance + * @param storeId + * @param constAccessor Wrapper function to access store data. + * @return + */ + virtual ReturnValue_t getData(store_address_t storeId, + ConstStorageAccessor& constAccessor) = 0; + /** * @brief getData returns an address to data and the size of the data * for a given packet_id. @@ -137,8 +158,30 @@ public: */ virtual ReturnValue_t getData(store_address_t packet_id, const uint8_t** packet_ptr, size_t* size) = 0; + + /** - * Same as above, but not const and therefore modifiable. + * Modify data by supplying a store ID + * @param storeId + * @return Pair of return value and StorageAccessor helper + */ + virtual AccessorPair modifyData(store_address_t storeId) = 0; + + /** + * Modify data by supplying a store ID and a StorageAccessor helper instance. + * @param storeId + * @param accessor Helper class to access the modifiable data. + * @return + */ + virtual ReturnValue_t modifyData(store_address_t storeId, + StorageAccessor& accessor) = 0; + + /** + * Get pointer and size of modifiable data by supplying the storeId + * @param packet_id + * @param packet_ptr [out] Pointer to pointer of data to set + * @param size [out] Pointer to size to set + * @return */ virtual ReturnValue_t modifyData(store_address_t packet_id, uint8_t** packet_ptr, size_t* size) = 0; @@ -158,19 +201,11 @@ public: virtual ReturnValue_t getFreeElement(store_address_t* storageId, const size_t size, uint8_t** p_data, bool ignoreFault = false ) = 0; - virtual AccessorPair modifyData(store_address_t storeId) = 0; - virtual ConstAccessorPair getData(store_address_t storeId) = 0; - virtual ReturnValue_t modifyData(store_address_t storeId, - StorageAccessor&) = 0; - virtual ReturnValue_t getData(store_address_t storeId, - ConstStorageAccessor&) = 0; - /** * Clears the whole store. * Use with care! */ virtual void clearStore() = 0; - }; #endif /* STORAGEMANAGERIF_H_ */ From 291710f257745e60e8417b2fa3648dcbfa09ed5d Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 12 May 2020 17:57:37 +0200 Subject: [PATCH 30/45] new ctor and bugfixes --- storagemanager/LocalPool.tpp | 6 +++-- storagemanager/StorageAccessor.cpp | 13 +++++++++- storagemanager/StorageAccessor.h | 39 +++++++++++++++--------------- storagemanager/StorageManagerIF.h | 4 +++ 4 files changed, 40 insertions(+), 22 deletions(-) diff --git a/storagemanager/LocalPool.tpp b/storagemanager/LocalPool.tpp index 31034974..98343ea7 100644 --- a/storagemanager/LocalPool.tpp +++ b/storagemanager/LocalPool.tpp @@ -148,7 +148,7 @@ template inline ConstAccessorPair LocalPool::getData( store_address_t storeId) { uint8_t* tempData = nullptr; - ConstStorageAccessor constAccessor(storeId); + ConstStorageAccessor constAccessor(storeId, this); ReturnValue_t status = modifyData(storeId, &tempData, &constAccessor.size_); constAccessor.constDataPointer = tempData; return ConstAccessorPair(status, std::move(constAccessor)); @@ -159,6 +159,7 @@ inline ReturnValue_t LocalPool::getData(store_address_t storeId ConstStorageAccessor& storeAccessor) { uint8_t* tempData = nullptr; ReturnValue_t status = modifyData(storeId, &tempData, &storeAccessor.size_); + storeAccessor.assignStore(this); storeAccessor.constDataPointer = tempData; return status; } @@ -176,7 +177,7 @@ template inline AccessorPair LocalPool::modifyData( store_address_t storeId) { uint8_t* tempData = nullptr; - StorageAccessor accessor(storeId); + StorageAccessor accessor(storeId, this); ReturnValue_t status = modifyData(storeId, &tempData, &accessor.size_); accessor.constDataPointer = tempData; accessor.assignConstPointer(); @@ -188,6 +189,7 @@ inline ReturnValue_t LocalPool::modifyData( store_address_t storeId, StorageAccessor& storeAccessor) { ReturnValue_t status = modifyData(storeId, &storeAccessor.dataPointer, &storeAccessor.size_); + storeAccessor.assignStore(this); storeAccessor.assignConstPointer(); return status; } diff --git a/storagemanager/StorageAccessor.cpp b/storagemanager/StorageAccessor.cpp index 51b93f11..67693054 100644 --- a/storagemanager/StorageAccessor.cpp +++ b/storagemanager/StorageAccessor.cpp @@ -3,6 +3,12 @@ ConstStorageAccessor::ConstStorageAccessor(store_address_t storeId): storeId(storeId) {} +ConstStorageAccessor::ConstStorageAccessor(store_address_t storeId, + StorageManagerIF* store): + storeId(storeId), store(store) { + internalState = AccessState::ASSIGNED; +} + ConstStorageAccessor::~ConstStorageAccessor() { if(deleteData and store != nullptr) { sif::debug << "deleting store data" << std::endl; @@ -27,6 +33,11 @@ StorageAccessor::StorageAccessor(store_address_t storeId): ConstStorageAccessor(storeId) { } +StorageAccessor::StorageAccessor(store_address_t storeId, + StorageManagerIF* store): + ConstStorageAccessor(storeId, store) { +} + StorageAccessor& StorageAccessor::operator =( StorageAccessor&& other) { // Call the parent move assignment and also assign own member. @@ -97,7 +108,7 @@ void ConstStorageAccessor::print() const { } void ConstStorageAccessor::assignStore(StorageManagerIF* store) { - internalState = AccessState::READ; + internalState = AccessState::ASSIGNED; this->store = store; } diff --git a/storagemanager/StorageAccessor.h b/storagemanager/StorageAccessor.h index da319a97..2be80601 100644 --- a/storagemanager/StorageAccessor.h +++ b/storagemanager/StorageAccessor.h @@ -31,23 +31,7 @@ public: * @param storeId */ ConstStorageAccessor(store_address_t storeId); - - /** - * @brief Move ctor and move assignment allow returning accessors as - * a returnvalue. They prevent resource being free prematurely. - * Refer to: https://github.com/MicrosoftDocs/cpp-docs/blob/master/docs/cpp/ - * move-constructors-and-move-assignment-operators-cpp.md - * @param - * @return - */ - ConstStorageAccessor& operator= (ConstStorageAccessor&&); - ConstStorageAccessor (ConstStorageAccessor&&); - - //! The copy ctor and copy assignemnt should be deleted implicitely - //! according to https://foonathan.net/2019/02/special-member-functions/ - //! but I still deleted them to make it more explicit. (remember rule of 5). - ConstStorageAccessor& operator= (ConstStorageAccessor&) = delete; - ConstStorageAccessor (ConstStorageAccessor&) = delete; + ConstStorageAccessor(store_address_t storeId, StorageManagerIF* store); /** * @brief The destructor in default configuration takes care of @@ -86,6 +70,23 @@ public: store_address_t getId() const; void print() const; + + /** + * @brief Move ctor and move assignment allow returning accessors as + * a returnvalue. They prevent resource being free prematurely. + * Refer to: https://github.com/MicrosoftDocs/cpp-docs/blob/master/docs/cpp/ + * move-constructors-and-move-assignment-operators-cpp.md + * @param + * @return + */ + ConstStorageAccessor& operator= (ConstStorageAccessor&&); + ConstStorageAccessor (ConstStorageAccessor&&); + + //! The copy ctor and copy assignemnt should be deleted implicitely + //! according to https://foonathan.net/2019/02/special-member-functions/ + //! but I still deleted them to make it more explicit. (remember rule of 5). + ConstStorageAccessor& operator= (ConstStorageAccessor&) = delete; + ConstStorageAccessor (ConstStorageAccessor&) = delete; protected: const uint8_t* constDataPointer = nullptr; store_address_t storeId; @@ -96,7 +97,7 @@ protected: enum class AccessState { UNINIT, - READ + ASSIGNED }; //! Internal state for safety reasons. AccessState internalState = AccessState::UNINIT; @@ -109,7 +110,6 @@ protected: * @param */ void assignStore(StorageManagerIF*); - }; @@ -124,6 +124,7 @@ class StorageAccessor: public ConstStorageAccessor { friend class LocalPool; public: StorageAccessor(store_address_t storeId); + StorageAccessor(store_address_t storeId, StorageManagerIF* store); /** * @brief Move ctor and move assignment allow returning accessors as * a returnvalue. They prevent resource being free prematurely. diff --git a/storagemanager/StorageManagerIF.h b/storagemanager/StorageManagerIF.h index b5508da8..559c4b9a 100644 --- a/storagemanager/StorageManagerIF.h +++ b/storagemanager/StorageManagerIF.h @@ -55,6 +55,10 @@ union store_address_t { * Alternative access to the raw value. */ uint32_t raw; + + bool operator==(const store_address_t& other) const { + return raw == other.raw; + } }; /** From 3122f62b0a1e81268ad5f7ac8fa249a5458dc906 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 12 May 2020 19:02:59 +0200 Subject: [PATCH 31/45] bugfixes for write() call --- storagemanager/LocalPool.tpp | 7 ++- storagemanager/StorageAccessor.cpp | 75 ++++++++++++++++++------------ storagemanager/StorageAccessor.h | 5 +- 3 files changed, 50 insertions(+), 37 deletions(-) diff --git a/storagemanager/LocalPool.tpp b/storagemanager/LocalPool.tpp index 98343ea7..59d06ab8 100644 --- a/storagemanager/LocalPool.tpp +++ b/storagemanager/LocalPool.tpp @@ -176,10 +176,9 @@ inline ReturnValue_t LocalPool::getData( template inline AccessorPair LocalPool::modifyData( store_address_t storeId) { - uint8_t* tempData = nullptr; StorageAccessor accessor(storeId, this); - ReturnValue_t status = modifyData(storeId, &tempData, &accessor.size_); - accessor.constDataPointer = tempData; + ReturnValue_t status = modifyData(storeId, &accessor.dataPointer, + &accessor.size_); accessor.assignConstPointer(); return AccessorPair(status, std::move(accessor)); } @@ -187,9 +186,9 @@ inline AccessorPair LocalPool::modifyData( template inline ReturnValue_t LocalPool::modifyData( store_address_t storeId, StorageAccessor& storeAccessor) { + storeAccessor.assignStore(this); ReturnValue_t status = modifyData(storeId, &storeAccessor.dataPointer, &storeAccessor.size_); - storeAccessor.assignStore(this); storeAccessor.assignConstPointer(); return status; } diff --git a/storagemanager/StorageAccessor.cpp b/storagemanager/StorageAccessor.cpp index 67693054..cc292d6c 100644 --- a/storagemanager/StorageAccessor.cpp +++ b/storagemanager/StorageAccessor.cpp @@ -16,6 +16,14 @@ ConstStorageAccessor::~ConstStorageAccessor() { } } +ConstStorageAccessor::ConstStorageAccessor(ConstStorageAccessor&& other): + constDataPointer(other.constDataPointer), storeId(other.storeId), + size_(other.size_), store(other.store), deleteData(other.deleteData), + internalState(other.internalState) { + // This prevent premature deletion + other.store = nullptr; +} + ConstStorageAccessor& ConstStorageAccessor::operator=( ConstStorageAccessor&& other) { constDataPointer = other.constDataPointer; @@ -29,36 +37,6 @@ ConstStorageAccessor& ConstStorageAccessor::operator=( return *this; } -StorageAccessor::StorageAccessor(store_address_t storeId): - ConstStorageAccessor(storeId) { -} - -StorageAccessor::StorageAccessor(store_address_t storeId, - StorageManagerIF* store): - ConstStorageAccessor(storeId, store) { -} - -StorageAccessor& StorageAccessor::operator =( - StorageAccessor&& other) { - // Call the parent move assignment and also assign own member. - dataPointer = other.dataPointer; - StorageAccessor::operator=(std::move(other)); - return * this; -} - -// Call the parent move ctor and also transfer own member. -StorageAccessor::StorageAccessor(StorageAccessor&& other): - ConstStorageAccessor(std::move(other)), dataPointer(other.dataPointer) { -} - -ConstStorageAccessor::ConstStorageAccessor(ConstStorageAccessor&& other): - constDataPointer(other.constDataPointer), storeId(other.storeId), - size_(other.size_), store(other.store), deleteData(other.deleteData), - internalState(other.internalState) { - // This prevent premature deletion - other.store = nullptr; -} - const uint8_t* ConstStorageAccessor::data() const { return constDataPointer; } @@ -113,6 +91,41 @@ void ConstStorageAccessor::assignStore(StorageManagerIF* store) { } +StorageAccessor::StorageAccessor(store_address_t storeId): + ConstStorageAccessor(storeId) { +} + +StorageAccessor::StorageAccessor(store_address_t storeId, + StorageManagerIF* store): + ConstStorageAccessor(storeId, store) { +} + +StorageAccessor& StorageAccessor::operator =( + StorageAccessor&& other) { + // Call the parent move assignment and also assign own member. + dataPointer = other.dataPointer; + StorageAccessor::operator=(std::move(other)); + return * this; +} + +// Call the parent move ctor and also transfer own member. +StorageAccessor::StorageAccessor(StorageAccessor&& other): + ConstStorageAccessor(std::move(other)), dataPointer(other.dataPointer) { +} + +ReturnValue_t StorageAccessor::getDataCopy(uint8_t *pointer, size_t maxSize) { + if(internalState == AccessState::UNINIT) { + sif::warning << "StorageAccessor: Not initialized!" << std::endl; + return HasReturnvaluesIF::RETURN_FAILED; + } + if(size_ > maxSize) { + sif::error << "StorageAccessor: Supplied buffer not large enough" << std::endl; + return HasReturnvaluesIF::RETURN_FAILED; + } + std::copy(dataPointer, dataPointer + size_, pointer); + return HasReturnvaluesIF::RETURN_OK; +} + uint8_t* StorageAccessor::data() { if(internalState == AccessState::UNINIT) { sif::warning << "StorageAccessor: Not initialized!" << std::endl; @@ -130,7 +143,7 @@ ReturnValue_t StorageAccessor::write(uint8_t *data, size_t size, sif::error << "StorageAccessor: Data too large for pool entry!" << std::endl; return HasReturnvaluesIF::RETURN_FAILED; } - std::copy(data, data + size, dataPointer); + std::copy(data, data + size, dataPointer + offset); return HasReturnvaluesIF::RETURN_OK; } diff --git a/storagemanager/StorageAccessor.h b/storagemanager/StorageAccessor.h index 2be80601..8949642c 100644 --- a/storagemanager/StorageAccessor.h +++ b/storagemanager/StorageAccessor.h @@ -49,7 +49,7 @@ public: * @brief Copies the read-only data to the supplied pointer * @param pointer */ - ReturnValue_t getDataCopy(uint8_t *pointer, size_t maxSize); + virtual ReturnValue_t getDataCopy(uint8_t *pointer, size_t maxSize); /** * @brief Calling this will prevent the Accessor from deleting the data @@ -137,8 +137,9 @@ public: StorageAccessor (StorageAccessor&&); ReturnValue_t write(uint8_t *data, size_t size, - uint16_t offset); + uint16_t offset = 0); uint8_t* data(); + ReturnValue_t getDataCopy(uint8_t *pointer, size_t maxSize) override; private: //! Non-const pointer for modifyable data. From 50a1b5170aa472c143c8a1a349e04316fb399225 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 13 May 2020 18:03:09 +0200 Subject: [PATCH 32/45] formatting --- devicehandlers/DeviceHandlerBase.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/devicehandlers/DeviceHandlerBase.h b/devicehandlers/DeviceHandlerBase.h index 729eac0b..25ec75b9 100644 --- a/devicehandlers/DeviceHandlerBase.h +++ b/devicehandlers/DeviceHandlerBase.h @@ -327,7 +327,7 @@ protected: * @param len length of remaining buffer to be scanned * @param[out] foundId the id of the data found in the buffer. * @param[out] foundLen length of the data found. Is to be set in function, - * buffer is scanned at previous position + foundLen. + * buffer is scanned at previous position + foundLen. * @return * - @c RETURN_OK a valid packet was found at @c start, @c foundLen is valid * - @c RETURN_FAILED no reply could be found starting at @c start, From 49fa2fe32c6bfc4fba6fc91e8cbf564eb7532658 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Thu, 14 May 2020 16:12:01 +0200 Subject: [PATCH 33/45] changed void* cast to QueueHandle_t cast --- osal/FreeRTOS/MessageQueue.cpp | 4 ++-- osal/FreeRTOS/MessageQueue.h | 7 +++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/osal/FreeRTOS/MessageQueue.cpp b/osal/FreeRTOS/MessageQueue.cpp index e7de799c..aa4b93b7 100644 --- a/osal/FreeRTOS/MessageQueue.cpp +++ b/osal/FreeRTOS/MessageQueue.cpp @@ -120,7 +120,7 @@ ReturnValue_t MessageQueue::sendMessageFromMessageQueue(MessageQueueId_t sendTo, message->setSender(sentFrom); BaseType_t result; if(callContext == CallContext::task) { - result = xQueueSendToBack(reinterpret_cast(sendTo), + result = xQueueSendToBack(reinterpret_cast(sendTo), reinterpret_cast(message->getBuffer()), 0); } else { @@ -128,7 +128,7 @@ ReturnValue_t MessageQueue::sendMessageFromMessageQueue(MessageQueueId_t sendTo, // request a context switch if a higher priority task // was blocked by the interrupt. BaseType_t xHigherPriorityTaskWoken = pdFALSE; - result = xQueueSendFromISR(reinterpret_cast(sendTo), + result = xQueueSendFromISR(reinterpret_cast(sendTo), reinterpret_cast(message->getBuffer()), &xHigherPriorityTaskWoken); if(xHigherPriorityTaskWoken == pdTRUE) { diff --git a/osal/FreeRTOS/MessageQueue.h b/osal/FreeRTOS/MessageQueue.h index 27076869..d8a307ea 100644 --- a/osal/FreeRTOS/MessageQueue.h +++ b/osal/FreeRTOS/MessageQueue.h @@ -6,8 +6,11 @@ #include #include -#include -#include "queue.h" +extern "C" { +#include "FreeRTOS.h" +#include "freertos/queue.h" +} + //TODO this class assumes that MessageQueueId_t is the same size as void* (the FreeRTOS handle type), compiler will catch this but it might be nice to have something checking or even an always working solution From 69237bc2e87fba9d5b3c8dfc91d11e328e2bdfc8 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Thu, 14 May 2020 21:26:04 +0200 Subject: [PATCH 34/45] freertos includes adapted --- osal/FreeRTOS/MessageQueue.cpp | 4 ++-- osal/FreeRTOS/MessageQueue.h | 4 ++-- osal/FreeRTOS/Mutex.h | 6 ++++-- osal/FreeRTOS/TaskManagement.h | 4 ++-- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/osal/FreeRTOS/MessageQueue.cpp b/osal/FreeRTOS/MessageQueue.cpp index aa4b93b7..aedf4831 100644 --- a/osal/FreeRTOS/MessageQueue.cpp +++ b/osal/FreeRTOS/MessageQueue.cpp @@ -121,7 +121,7 @@ ReturnValue_t MessageQueue::sendMessageFromMessageQueue(MessageQueueId_t sendTo, BaseType_t result; if(callContext == CallContext::task) { result = xQueueSendToBack(reinterpret_cast(sendTo), - reinterpret_cast(message->getBuffer()), 0); + static_cast(message->getBuffer()), 0); } else { // If the call context is from an interrupt, @@ -129,7 +129,7 @@ ReturnValue_t MessageQueue::sendMessageFromMessageQueue(MessageQueueId_t sendTo, // was blocked by the interrupt. BaseType_t xHigherPriorityTaskWoken = pdFALSE; result = xQueueSendFromISR(reinterpret_cast(sendTo), - reinterpret_cast(message->getBuffer()), + static_cast(message->getBuffer()), &xHigherPriorityTaskWoken); if(xHigherPriorityTaskWoken == pdTRUE) { TaskManagement::requestContextSwitch(callContext); diff --git a/osal/FreeRTOS/MessageQueue.h b/osal/FreeRTOS/MessageQueue.h index d8a307ea..ce6ed46b 100644 --- a/osal/FreeRTOS/MessageQueue.h +++ b/osal/FreeRTOS/MessageQueue.h @@ -7,8 +7,8 @@ #include extern "C" { -#include "FreeRTOS.h" -#include "freertos/queue.h" +#include +#include } diff --git a/osal/FreeRTOS/Mutex.h b/osal/FreeRTOS/Mutex.h index 604511b5..ef93fd34 100644 --- a/osal/FreeRTOS/Mutex.h +++ b/osal/FreeRTOS/Mutex.h @@ -3,9 +3,11 @@ #include +extern "C" { +#include +#include +} -#include -#include "semphr.h" /** * @brief OS component to implement MUTual EXclusion diff --git a/osal/FreeRTOS/TaskManagement.h b/osal/FreeRTOS/TaskManagement.h index da0cce2a..eaaf97e2 100644 --- a/osal/FreeRTOS/TaskManagement.h +++ b/osal/FreeRTOS/TaskManagement.h @@ -4,8 +4,8 @@ #include extern "C" { -#include "FreeRTOS.h" -#include "task.h" +#include +#include } #include From 684da2b8d5400ad68a59a59579bbb9244a1be5c3 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Fri, 15 May 2020 15:30:29 +0200 Subject: [PATCH 35/45] set application data function added --- tmtcpacket/SpacePacketBase.cpp | 3 ++- tmtcpacket/pus/TcPacketBase.cpp | 13 ++++++++++--- tmtcpacket/pus/TcPacketBase.h | 9 ++++++++- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/tmtcpacket/SpacePacketBase.cpp b/tmtcpacket/SpacePacketBase.cpp index 13c062d8..a06c3a33 100644 --- a/tmtcpacket/SpacePacketBase.cpp +++ b/tmtcpacket/SpacePacketBase.cpp @@ -14,7 +14,8 @@ uint8_t SpacePacketBase::getPacketVersionNumber( void ) { return (this->data->header.packet_id_h & 0b11100000) >> 5; } -void SpacePacketBase::initSpacePacketHeader(bool isTelecommand, bool hasSecondaryHeader, uint16_t apid, uint16_t sequenceCount) { +void SpacePacketBase::initSpacePacketHeader(bool isTelecommand, + bool hasSecondaryHeader, uint16_t apid, uint16_t sequenceCount) { //reset header to zero: memset(data,0, sizeof(this->data->header) ); //Set TC/TM bit. diff --git a/tmtcpacket/pus/TcPacketBase.cpp b/tmtcpacket/pus/TcPacketBase.cpp index d7f6636d..cdeef7f8 100644 --- a/tmtcpacket/pus/TcPacketBase.cpp +++ b/tmtcpacket/pus/TcPacketBase.cpp @@ -46,9 +46,16 @@ void TcPacketBase::setErrorControl() { (&tcData->data)[size + 1] = (crc) & 0X00FF; // CRCL } -void TcPacketBase::setData(const uint8_t* p_Data) { - SpacePacketBase::setData(p_Data); - tcData = (TcPacketPointer*) p_Data; +void TcPacketBase::setData(const uint8_t* pData) { + SpacePacketBase::setData(pData); + tcData = (TcPacketPointer*) pData; +} + +void TcPacketBase::setApplicationData(const uint8_t * pData, size_t dataLen) { + SpacePacketBase::setData(pData); + tcData = (TcPacketPointer*) pData; + SpacePacketBase::setPacketDataLength(dataLen + + sizeof(PUSTcDataFieldHeader) + TcPacketBase::CRC_SIZE-1); } uint8_t TcPacketBase::getSecondaryHeaderFlag() { diff --git a/tmtcpacket/pus/TcPacketBase.h b/tmtcpacket/pus/TcPacketBase.h index d84d6c45..a7e823fc 100644 --- a/tmtcpacket/pus/TcPacketBase.h +++ b/tmtcpacket/pus/TcPacketBase.h @@ -2,6 +2,7 @@ #define TCPACKETBASE_H_ #include +#include /** * This struct defines a byte-wise structured PUS TC Data Field Header. @@ -175,7 +176,13 @@ public: * * @param p_data A pointer to another PUS Telecommand Packet. */ - void setData( const uint8_t* p_data ); + void setData( const uint8_t* pData ); + /** + * Set application data and corresponding length field. + * @param pData + * @param dataLen + */ + void setApplicationData(const uint8_t * pData, size_t dataLen); /** * This is a debugging helper method that prints the whole packet content * to the screen. From 237dd4a256598ed805442a71bef58a4ddc9aabb4 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Fri, 15 May 2020 18:23:57 +0200 Subject: [PATCH 36/45] added additonal functions for tc packet base --- tmtcpacket/pus/TcPacketBase.cpp | 5 +++++ tmtcpacket/pus/TcPacketBase.h | 9 ++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/tmtcpacket/pus/TcPacketBase.cpp b/tmtcpacket/pus/TcPacketBase.cpp index cdeef7f8..54ad92be 100644 --- a/tmtcpacket/pus/TcPacketBase.cpp +++ b/tmtcpacket/pus/TcPacketBase.cpp @@ -87,3 +87,8 @@ void TcPacketBase::initializeTcPacket(uint16_t apid, uint16_t sequenceCount, tcData->data_field.service_type = service; tcData->data_field.service_subtype = subservice; } + +size_t TcPacketBase::calculateFullPacketLength(size_t appDataLen) { + return sizeof(CCSDSPrimaryHeader) + sizeof(PUSTcDataFieldHeader) + + appDataLen + TcPacketBase::CRC_SIZE; +} diff --git a/tmtcpacket/pus/TcPacketBase.h b/tmtcpacket/pus/TcPacketBase.h index a7e823fc..69c64b87 100644 --- a/tmtcpacket/pus/TcPacketBase.h +++ b/tmtcpacket/pus/TcPacketBase.h @@ -100,7 +100,8 @@ public: * @param service PUS Service * @param subservice PUS Subservice */ - void initializeTcPacket(uint16_t apid, uint16_t sequenceCount, uint8_t ack, uint8_t service, uint8_t subservice); + void initializeTcPacket(uint16_t apid, uint16_t sequenceCount, uint8_t ack, + uint8_t service, uint8_t subservice); /** * This command returns the CCSDS Secondary Header Flag. * It shall always be zero for PUS Packets. This is the @@ -188,6 +189,12 @@ public: * to the screen. */ void print(); + /** + * Calculate full packet length from application data length. + * @param appDataLen + * @return + */ + static size_t calculateFullPacketLength(size_t appDataLen); }; From 6e1bb16d4e22f5e4b0b3d9010a5662ffe7f1b16e Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Fri, 15 May 2020 18:46:58 +0200 Subject: [PATCH 37/45] printers continued, possible bugfix in tc packet base --- globalfunctions/printer.cpp | 25 +++++++++++++++++++++++-- globalfunctions/printer.h | 12 +++++++++--- tmtcpacket/pus/TcPacketBase.cpp | 2 +- 3 files changed, 33 insertions(+), 6 deletions(-) diff --git a/globalfunctions/printer.cpp b/globalfunctions/printer.cpp index d6ede0a5..d7af30a5 100644 --- a/globalfunctions/printer.cpp +++ b/globalfunctions/printer.cpp @@ -1,13 +1,34 @@ #include #include -void printer::print(uint8_t *data, size_t size) { +void printer::print(uint8_t *data, size_t size, OutputType type) { sif::info << "StorageAccessor: Printing data: ["; + if(type == OutputType::HEX) { + printer::printHex(data, size); + } + else { + printer::printDec(data, size); + } +} + +void printer::printHex(uint8_t *data, size_t size) { + sif::info << std::hex; for(size_t i = 0; i < size; i++) { - sif::info << std::hex << (int)data[i]; + sif::info << "0x" << static_cast(data[i]); if(i < size - 1){ sif::info << " , "; } } + sif::info << std::dec; sif::info << " ] " << std::endl; } + +void printer::printDec(uint8_t *data, size_t size) { + for(size_t i = 0; i < size; i++) { + sif::info << "0x" << static_cast(data[i]); + if(i < size - 1){ + sif::info << " , "; + } + } +} + diff --git a/globalfunctions/printer.h b/globalfunctions/printer.h index d4a573e9..3c9d6aa0 100644 --- a/globalfunctions/printer.h +++ b/globalfunctions/printer.h @@ -4,9 +4,15 @@ #include namespace printer { -void print(uint8_t *data, size_t size); + +enum class OutputType { + DEC, + HEX +}; + +void print(uint8_t* data, size_t size, OutputType type = OutputType::HEX); +void printHex(uint8_t* data, size_t size); +void printDec(uint8_t* data, size_t size); } - - #endif /* FRAMEWORK_GLOBALFUNCTIONS_PRINTER_H_ */ diff --git a/tmtcpacket/pus/TcPacketBase.cpp b/tmtcpacket/pus/TcPacketBase.cpp index 54ad92be..b98cf136 100644 --- a/tmtcpacket/pus/TcPacketBase.cpp +++ b/tmtcpacket/pus/TcPacketBase.cpp @@ -79,7 +79,7 @@ void TcPacketBase::initializeTcPacket(uint16_t apid, uint16_t sequenceCount, uint8_t ack, uint8_t service, uint8_t subservice) { initSpacePacketHeader(true, true, apid, sequenceCount); memset(&tcData->data_field, 0, sizeof(tcData->data_field)); - setPacketDataLength(sizeof(tcData->data_field) + CRC_SIZE); + setPacketDataLength(sizeof(PUSTcDataFieldHeader) + CRC_SIZE - 1); //Data Field Header: //Set CCSDS_secondary_header_flag to 0, version number to 001 and ack to 0000 tcData->data_field.version_type_ack = 0b00010000; From e92c324c7ef26eef4877041df94fa2d8fa16f8db Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Fri, 15 May 2020 18:47:46 +0200 Subject: [PATCH 38/45] updated printer --- globalfunctions/printer.cpp | 25 +++++++++++++++++++++++-- globalfunctions/printer.h | 12 +++++++++--- 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/globalfunctions/printer.cpp b/globalfunctions/printer.cpp index d6ede0a5..d7af30a5 100644 --- a/globalfunctions/printer.cpp +++ b/globalfunctions/printer.cpp @@ -1,13 +1,34 @@ #include #include -void printer::print(uint8_t *data, size_t size) { +void printer::print(uint8_t *data, size_t size, OutputType type) { sif::info << "StorageAccessor: Printing data: ["; + if(type == OutputType::HEX) { + printer::printHex(data, size); + } + else { + printer::printDec(data, size); + } +} + +void printer::printHex(uint8_t *data, size_t size) { + sif::info << std::hex; for(size_t i = 0; i < size; i++) { - sif::info << std::hex << (int)data[i]; + sif::info << "0x" << static_cast(data[i]); if(i < size - 1){ sif::info << " , "; } } + sif::info << std::dec; sif::info << " ] " << std::endl; } + +void printer::printDec(uint8_t *data, size_t size) { + for(size_t i = 0; i < size; i++) { + sif::info << "0x" << static_cast(data[i]); + if(i < size - 1){ + sif::info << " , "; + } + } +} + diff --git a/globalfunctions/printer.h b/globalfunctions/printer.h index d4a573e9..3c9d6aa0 100644 --- a/globalfunctions/printer.h +++ b/globalfunctions/printer.h @@ -4,9 +4,15 @@ #include namespace printer { -void print(uint8_t *data, size_t size); + +enum class OutputType { + DEC, + HEX +}; + +void print(uint8_t* data, size_t size, OutputType type = OutputType::HEX); +void printHex(uint8_t* data, size_t size); +void printDec(uint8_t* data, size_t size); } - - #endif /* FRAMEWORK_GLOBALFUNCTIONS_PRINTER_H_ */ From 1b093d96b012cf3ee12ff02ba384734af8a3853c Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Fri, 15 May 2020 18:51:15 +0200 Subject: [PATCH 39/45] additional size output --- globalfunctions/printer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/globalfunctions/printer.cpp b/globalfunctions/printer.cpp index d7af30a5..df98568e 100644 --- a/globalfunctions/printer.cpp +++ b/globalfunctions/printer.cpp @@ -2,7 +2,7 @@ #include void printer::print(uint8_t *data, size_t size, OutputType type) { - sif::info << "StorageAccessor: Printing data: ["; + sif::info << "StorageAccessor: Printing data with size " << size << ": ["; if(type == OutputType::HEX) { printer::printHex(data, size); } From 80cee2742964485910faa168cb9a1c96e2c0dec3 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Fri, 15 May 2020 18:53:54 +0200 Subject: [PATCH 40/45] printer fixes --- globalfunctions/printer.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/globalfunctions/printer.cpp b/globalfunctions/printer.cpp index df98568e..5af4346b 100644 --- a/globalfunctions/printer.cpp +++ b/globalfunctions/printer.cpp @@ -20,15 +20,17 @@ void printer::printHex(uint8_t *data, size_t size) { } } sif::info << std::dec; - sif::info << " ] " << std::endl; + sif::info << "]" << std::endl; } void printer::printDec(uint8_t *data, size_t size) { + sif::info << std::dec; for(size_t i = 0; i < size; i++) { sif::info << "0x" << static_cast(data[i]); if(i < size - 1){ sif::info << " , "; } } + sif::info << "]" << std::endl; } From 24bfbfbd33e93d35dabd716d72d67eefc9512b11 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Fri, 15 May 2020 18:56:06 +0200 Subject: [PATCH 41/45] removed StorageAccessoremoved StorageAccessorr --- globalfunctions/printer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/globalfunctions/printer.cpp b/globalfunctions/printer.cpp index 5af4346b..bf4c5848 100644 --- a/globalfunctions/printer.cpp +++ b/globalfunctions/printer.cpp @@ -2,7 +2,7 @@ #include void printer::print(uint8_t *data, size_t size, OutputType type) { - sif::info << "StorageAccessor: Printing data with size " << size << ": ["; + sif::info << "Printing data with size " << size << ": ["; if(type == OutputType::HEX) { printer::printHex(data, size); } From 3f71babfa995965ef39299f5285ad5990efe6921 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Fri, 15 May 2020 19:50:51 +0200 Subject: [PATCH 42/45] app data len uint16_t, full length size_t --- tmtcpacket/SpacePacketBase.cpp | 2 +- tmtcpacket/SpacePacketBase.h | 5 +++-- tmtcpacket/pus/TcPacketBase.cpp | 2 +- tmtcpacket/pus/TcPacketBase.h | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/tmtcpacket/SpacePacketBase.cpp b/tmtcpacket/SpacePacketBase.cpp index a06c3a33..b9ae0684 100644 --- a/tmtcpacket/SpacePacketBase.cpp +++ b/tmtcpacket/SpacePacketBase.cpp @@ -82,7 +82,7 @@ void SpacePacketBase::setPacketDataLength( uint16_t new_length) { this->data->header.packet_length_l = ( new_length & 0x00FF ); } -uint32_t SpacePacketBase::getFullSize() { +size_t SpacePacketBase::getFullSize() { //+1 is done because size in packet data length field is: size of data field -1 return this->getPacketDataLength() + sizeof(this->data->header) + 1; } diff --git a/tmtcpacket/SpacePacketBase.h b/tmtcpacket/SpacePacketBase.h index cc68f714..57a5df9c 100644 --- a/tmtcpacket/SpacePacketBase.h +++ b/tmtcpacket/SpacePacketBase.h @@ -2,9 +2,10 @@ #define SPACEPACKETBASE_H_ #include +#include /** - * \defgroup tmtcpackets Space Packets + * @defgroup tmtcpackets Space Packets * This is the group, where all classes associated with Telecommand and * Telemetry packets belong to. * The class hierarchy resembles the dependency between the different standards @@ -167,7 +168,7 @@ public: * This method returns the full raw packet size. * @return The full size of the packet in bytes. */ - uint32_t getFullSize(); + size_t getFullSize(); uint32_t getApidAndSequenceCount() const; diff --git a/tmtcpacket/pus/TcPacketBase.cpp b/tmtcpacket/pus/TcPacketBase.cpp index b98cf136..4ef50c0d 100644 --- a/tmtcpacket/pus/TcPacketBase.cpp +++ b/tmtcpacket/pus/TcPacketBase.cpp @@ -51,7 +51,7 @@ void TcPacketBase::setData(const uint8_t* pData) { tcData = (TcPacketPointer*) pData; } -void TcPacketBase::setApplicationData(const uint8_t * pData, size_t dataLen) { +void TcPacketBase::setApplicationData(const uint8_t * pData, uint16_t dataLen) { SpacePacketBase::setData(pData); tcData = (TcPacketPointer*) pData; SpacePacketBase::setPacketDataLength(dataLen + diff --git a/tmtcpacket/pus/TcPacketBase.h b/tmtcpacket/pus/TcPacketBase.h index 69c64b87..c8342896 100644 --- a/tmtcpacket/pus/TcPacketBase.h +++ b/tmtcpacket/pus/TcPacketBase.h @@ -183,7 +183,7 @@ public: * @param pData * @param dataLen */ - void setApplicationData(const uint8_t * pData, size_t dataLen); + void setApplicationData(const uint8_t * pData, uint16_t dataLen); /** * This is a debugging helper method that prints the whole packet content * to the screen. From c77ec9e7fcbe038a4622b3faf3cf311d6d3cc901 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Fri, 15 May 2020 19:56:54 +0200 Subject: [PATCH 43/45] uint16_t correction --- tmtcpacket/pus/TcPacketBase.cpp | 2 +- tmtcpacket/pus/TcPacketBase.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tmtcpacket/pus/TcPacketBase.cpp b/tmtcpacket/pus/TcPacketBase.cpp index 4ef50c0d..f8dba79f 100644 --- a/tmtcpacket/pus/TcPacketBase.cpp +++ b/tmtcpacket/pus/TcPacketBase.cpp @@ -28,7 +28,7 @@ const uint8_t* TcPacketBase::getApplicationData() const { return &tcData->data; } -size_t TcPacketBase::getApplicationDataSize() { +uint16_t TcPacketBase::getApplicationDataSize() { return getPacketDataLength() - sizeof(tcData->data_field) - CRC_SIZE + 1; } diff --git a/tmtcpacket/pus/TcPacketBase.h b/tmtcpacket/pus/TcPacketBase.h index c8342896..136aaa0b 100644 --- a/tmtcpacket/pus/TcPacketBase.h +++ b/tmtcpacket/pus/TcPacketBase.h @@ -153,7 +153,7 @@ public: * @return The size of the PUS Application Data (without Error Control * field) */ - size_t getApplicationDataSize(); + uint16_t getApplicationDataSize(); /** * This getter returns the Error Control Field of the packet. * From 5b41f2a6bd4e91956874fff43774865d9887dab5 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Fri, 15 May 2020 19:58:58 +0200 Subject: [PATCH 44/45] optimization --- tmtcpacket/pus/TcPacketBase.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tmtcpacket/pus/TcPacketBase.cpp b/tmtcpacket/pus/TcPacketBase.cpp index f8dba79f..3e2b34c2 100644 --- a/tmtcpacket/pus/TcPacketBase.cpp +++ b/tmtcpacket/pus/TcPacketBase.cpp @@ -52,8 +52,7 @@ void TcPacketBase::setData(const uint8_t* pData) { } void TcPacketBase::setApplicationData(const uint8_t * pData, uint16_t dataLen) { - SpacePacketBase::setData(pData); - tcData = (TcPacketPointer*) pData; + setData(pData); SpacePacketBase::setPacketDataLength(dataLen + sizeof(PUSTcDataFieldHeader) + TcPacketBase::CRC_SIZE-1); } From 0e2438416d0af245d086175868a5496c679b94ef Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Fri, 15 May 2020 20:00:43 +0200 Subject: [PATCH 45/45] added comment --- tmtcpacket/pus/TcPacketBase.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tmtcpacket/pus/TcPacketBase.cpp b/tmtcpacket/pus/TcPacketBase.cpp index 3e2b34c2..0972ffff 100644 --- a/tmtcpacket/pus/TcPacketBase.cpp +++ b/tmtcpacket/pus/TcPacketBase.cpp @@ -53,8 +53,9 @@ void TcPacketBase::setData(const uint8_t* pData) { void TcPacketBase::setApplicationData(const uint8_t * pData, uint16_t dataLen) { setData(pData); + // packet data length is actual size of data field minus 1 SpacePacketBase::setPacketDataLength(dataLen + - sizeof(PUSTcDataFieldHeader) + TcPacketBase::CRC_SIZE-1); + sizeof(PUSTcDataFieldHeader) + TcPacketBase::CRC_SIZE - 1); } uint8_t TcPacketBase::getSecondaryHeaderFlag() {