From 2de972bb8a9a35b81824f5c6bc5d00802c32dadd Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 30 Jun 2020 15:51:19 +0200 Subject: [PATCH 01/34] const store accessor copy ctor fixx --- storagemanager/ConstStorageAccessor.h | 6 +++--- storagemanager/StorageAccessor.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/storagemanager/ConstStorageAccessor.h b/storagemanager/ConstStorageAccessor.h index cf8ca08f..020030a9 100644 --- a/storagemanager/ConstStorageAccessor.h +++ b/storagemanager/ConstStorageAccessor.h @@ -80,13 +80,13 @@ public: * @return */ ConstStorageAccessor& operator= (ConstStorageAccessor&&); - ConstStorageAccessor (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& operator=(const ConstStorageAccessor&) = delete; + ConstStorageAccessor(const ConstStorageAccessor&) = delete; protected: const uint8_t* constDataPointer = nullptr; store_address_t storeId; diff --git a/storagemanager/StorageAccessor.h b/storagemanager/StorageAccessor.h index c5e38306..475b2f4f 100644 --- a/storagemanager/StorageAccessor.h +++ b/storagemanager/StorageAccessor.h @@ -26,7 +26,7 @@ public: * @return */ StorageAccessor& operator= (StorageAccessor&&); - StorageAccessor (StorageAccessor&&); + StorageAccessor(StorageAccessor&&); ReturnValue_t write(uint8_t *data, size_t size, uint16_t offset = 0); From 69946d5276549dce7624540df1de75c94ffc5004 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Sat, 11 Jul 2020 11:52:01 +0200 Subject: [PATCH 02/34] FIFO hotfix --- container/DynamicFIFO.h | 21 ++++++++++++++++++--- container/FIFO.h | 12 ++++++++++-- container/FIFOBase.h | 8 ++++++-- container/FIFOBase.tpp | 8 +++++++- 4 files changed, 41 insertions(+), 8 deletions(-) diff --git a/container/DynamicFIFO.h b/container/DynamicFIFO.h index 3a5242c7..59adfb3a 100644 --- a/container/DynamicFIFO.h +++ b/container/DynamicFIFO.h @@ -17,11 +17,26 @@ template class DynamicFIFO: public FIFOBase { public: - DynamicFIFO(size_t maxCapacity): FIFOBase(values.data(), maxCapacity), - values(maxCapacity) {}; + DynamicFIFO(size_t maxCapacity): FIFOBase(nullptr, maxCapacity), + fifoVector(maxCapacity) { + // trying to pass the pointer of the uninitialized vector + // to the FIFOBase constructor directly lead to a super evil bug. + // So we do it like this now. + this->setData(fifoVector.data()); + }; + + /** + * @brief Custom copy constructor which prevents setting the + * underlying pointer wrong. + */ + DynamicFIFO(const DynamicFIFO& other): FIFOBase(other), + fifoVector(other.maxCapacity) { + this->setData(fifoVector.data()); + } + private: - std::vector values; + std::vector fifoVector; }; #endif /* FRAMEWORK_CONTAINER_DYNAMICFIFO_H_ */ diff --git a/container/FIFO.h b/container/FIFO.h index ecb218fd..2e332dc2 100644 --- a/container/FIFO.h +++ b/container/FIFO.h @@ -17,10 +17,18 @@ template class FIFO: public FIFOBase { public: - FIFO(): FIFOBase(values.data(), capacity) {}; + FIFO(): FIFOBase(fifoArray.data(), capacity) {}; + + /** + * @brief Custom copy constructor to set pointer correctly. + * @param other + */ + FIFO(const FIFO& other): FIFOBase(other) { + this->setData(fifoArray.data()); + } private: - std::array values; + std::array fifoArray; }; #endif /* FRAMEWORK_CONTAINERS_STATICFIFO_H_ */ diff --git a/container/FIFOBase.h b/container/FIFOBase.h index 8bdb333f..7f8bde96 100644 --- a/container/FIFOBase.h +++ b/container/FIFOBase.h @@ -3,6 +3,7 @@ #include #include +#include template class FIFOBase { @@ -43,12 +44,15 @@ public: bool full(); size_t size(); + size_t getMaxCapacity() const; -private: - T* values; +protected: + void setData(T* data); size_t maxCapacity = 0; + T* values; + size_t readIndex = 0; size_t writeIndex = 0; size_t currentSize = 0; diff --git a/container/FIFOBase.tpp b/container/FIFOBase.tpp index 48a060ff..c73e9d59 100644 --- a/container/FIFOBase.tpp +++ b/container/FIFOBase.tpp @@ -7,7 +7,7 @@ template inline FIFOBase::FIFOBase(T* values, const size_t maxCapacity): - values(values), maxCapacity(maxCapacity) {}; + maxCapacity(maxCapacity), values(values){}; template inline ReturnValue_t FIFOBase::insert(T value) { @@ -78,4 +78,10 @@ inline size_t FIFOBase::getMaxCapacity() const { return maxCapacity; } + +template +inline void FIFOBase::setData(T *data) { + this->values = data; +} + #endif From e179288c00ffedf8e160b399db15a0ec26395ea2 Mon Sep 17 00:00:00 2001 From: Steffen Gaisser Date: Thu, 25 Jun 2020 18:09:32 +0200 Subject: [PATCH 03/34] Fixed spelling mistake in HealthHelper --- devicehandlers/ChildHandlerBase.cpp | 2 +- devicehandlers/DeviceHandlerBase.cpp | 2 +- devicehandlers/HealthDevice.cpp | 2 +- health/HealthHelper.cpp | 4 ++-- health/HealthHelper.h | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/devicehandlers/ChildHandlerBase.cpp b/devicehandlers/ChildHandlerBase.cpp index bdc5fa65..2e4428e5 100644 --- a/devicehandlers/ChildHandlerBase.cpp +++ b/devicehandlers/ChildHandlerBase.cpp @@ -39,7 +39,7 @@ ReturnValue_t ChildHandlerBase::initialize() { parent->registerChild(getObjectId()); } - healthHelper.setParentQeueue(parentQueue); + healthHelper.setParentQueue(parentQueue); modeHelper.setParentQueue(parentQueue); diff --git a/devicehandlers/DeviceHandlerBase.cpp b/devicehandlers/DeviceHandlerBase.cpp index 282f20b2..9f4c7baf 100644 --- a/devicehandlers/DeviceHandlerBase.cpp +++ b/devicehandlers/DeviceHandlerBase.cpp @@ -1134,7 +1134,7 @@ ReturnValue_t DeviceHandlerBase::handleDeviceHandlerMessage( void DeviceHandlerBase::setParentQueue(MessageQueueId_t parentQueueId) { modeHelper.setParentQueue(parentQueueId); - healthHelper.setParentQeueue(parentQueueId); + healthHelper.setParentQueue(parentQueueId); } bool DeviceHandlerBase::isAwaitingReply() { diff --git a/devicehandlers/HealthDevice.cpp b/devicehandlers/HealthDevice.cpp index 0bce34ec..8367a6bc 100644 --- a/devicehandlers/HealthDevice.cpp +++ b/devicehandlers/HealthDevice.cpp @@ -38,7 +38,7 @@ MessageQueueId_t HealthDevice::getCommandQueue() const { } void HealthDevice::setParentQueue(MessageQueueId_t parentQueue) { - healthHelper.setParentQeueue(parentQueue); + healthHelper.setParentQueue(parentQueue); } bool HealthDevice::hasHealthChanged() { diff --git a/health/HealthHelper.cpp b/health/HealthHelper.cpp index a39305a3..ba8504c2 100644 --- a/health/HealthHelper.cpp +++ b/health/HealthHelper.cpp @@ -28,11 +28,11 @@ HasHealthIF::HealthState HealthHelper::getHealth() { } ReturnValue_t HealthHelper::initialize(MessageQueueId_t parentQueue) { - setParentQeueue(parentQueue); + setParentQueue(parentQueue); return initialize(); } -void HealthHelper::setParentQeueue(MessageQueueId_t parentQueue) { +void HealthHelper::setParentQueue(MessageQueueId_t parentQueue) { this->parentQueue = parentQueue; } diff --git a/health/HealthHelper.h b/health/HealthHelper.h index a17a69f3..e112ad2c 100644 --- a/health/HealthHelper.h +++ b/health/HealthHelper.h @@ -79,7 +79,7 @@ public: /** * @param parentQueue the Queue id of the parent object. Set to 0 if no parent present */ - void setParentQeueue(MessageQueueId_t parentQueue); + void setParentQueue(MessageQueueId_t parentQueue); /** * From 6425c0dd4c43ba0fa0711d5d894ae76a1858782f Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Sun, 26 Jul 2020 03:12:04 +0200 Subject: [PATCH 04/34] better init error output --- osal/FreeRTOS/Mutex.cpp | 2 +- osal/FreeRTOS/Mutex.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/osal/FreeRTOS/Mutex.cpp b/osal/FreeRTOS/Mutex.cpp index 6d90c3f6..8a5c444d 100644 --- a/osal/FreeRTOS/Mutex.cpp +++ b/osal/FreeRTOS/Mutex.cpp @@ -8,7 +8,7 @@ const uint32_t MutexIF::BLOCKING = portMAX_DELAY; Mutex::Mutex() { handle = xSemaphoreCreateMutex(); if(handle == nullptr) { - sif::error << "Mutex: Creation failure" << std::endl; + sif::error << "Mutex::Mutex(FreeRTOS): Creation failure" << std::endl; } } diff --git a/osal/FreeRTOS/Mutex.h b/osal/FreeRTOS/Mutex.h index d6e0aab9..37f1791c 100644 --- a/osal/FreeRTOS/Mutex.h +++ b/osal/FreeRTOS/Mutex.h @@ -20,6 +20,7 @@ public: ~Mutex(); ReturnValue_t lockMutex(uint32_t timeoutMs = MutexIF::BLOCKING) override; ReturnValue_t unlockMutex() override; + private: SemaphoreHandle_t handle; }; From afc16ef2d72dd6662ecb19c4a08a9aedc2119aab Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 28 Jul 2020 12:49:18 +0200 Subject: [PATCH 05/34] new servicei nterface buffer /stream --- serviceinterface/ServiceInterfaceBuffer.cpp | 12 ++++++------ serviceinterface/ServiceInterfaceBuffer.h | 2 +- serviceinterface/ServiceInterfaceStream.cpp | 2 +- serviceinterface/ServiceInterfaceStream.h | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/serviceinterface/ServiceInterfaceBuffer.cpp b/serviceinterface/ServiceInterfaceBuffer.cpp index 5c80862c..8c510eac 100644 --- a/serviceinterface/ServiceInterfaceBuffer.cpp +++ b/serviceinterface/ServiceInterfaceBuffer.cpp @@ -78,9 +78,9 @@ int ServiceInterfaceBuffer::sync(void) { } size_t preambleSize = 0; - auto preamble = getPreamble(&preambleSize); + std::string* preamble = getPreamble(&preambleSize); // Write logMessage and time - this->putChars(preamble.data(), preamble.data() + preambleSize); + this->putChars(preamble->data(), preamble->data() + preambleSize); // Handle output this->putChars(pbase(), pptr()); // This tells that buffer is empty again @@ -92,7 +92,7 @@ bool ServiceInterfaceBuffer::isBuffered() const { return buffered; } -std::string ServiceInterfaceBuffer::getPreamble(size_t * preambleSize) { +std::string* ServiceInterfaceBuffer::getPreamble(size_t * preambleSize) { Clock::TimeOfDay_t loggerTime; Clock::getDateAndTime(&loggerTime); size_t currentSize = 0; @@ -110,18 +110,18 @@ std::string ServiceInterfaceBuffer::getPreamble(size_t * preambleSize) { loggerTime.usecond /1000); if(charCount < 0) { printf("ServiceInterfaceBuffer: Failure parsing preamble\r\n"); - return ""; + return &preamble; } if(charCount > MAX_PREAMBLE_SIZE) { printf("ServiceInterfaceBuffer: Char count too large for maximum " "preamble size"); - return ""; + return &preamble; } currentSize += charCount; if(preambleSize != nullptr) { *preambleSize = currentSize; } - return preamble; + return &preamble; } diff --git a/serviceinterface/ServiceInterfaceBuffer.h b/serviceinterface/ServiceInterfaceBuffer.h index 39ea25c2..7a2ce2ee 100644 --- a/serviceinterface/ServiceInterfaceBuffer.h +++ b/serviceinterface/ServiceInterfaceBuffer.h @@ -60,7 +60,7 @@ private: //! In this function, the characters are parsed. void putChars(char const* begin, char const* end); - std::string getPreamble(size_t * preambleSize = nullptr); + std::string* getPreamble(size_t * preambleSize = nullptr); }; #endif diff --git a/serviceinterface/ServiceInterfaceStream.cpp b/serviceinterface/ServiceInterfaceStream.cpp index 76481ed1..31bc7c73 100644 --- a/serviceinterface/ServiceInterfaceStream.cpp +++ b/serviceinterface/ServiceInterfaceStream.cpp @@ -9,7 +9,7 @@ void ServiceInterfaceStream::setActive( bool myActive) { this->streambuf.isActive = myActive; } -std::string ServiceInterfaceStream::getPreamble() { +std::string* ServiceInterfaceStream::getPreamble() { return streambuf.getPreamble(); } diff --git a/serviceinterface/ServiceInterfaceStream.h b/serviceinterface/ServiceInterfaceStream.h index 9e19c228..dc111459 100644 --- a/serviceinterface/ServiceInterfaceStream.h +++ b/serviceinterface/ServiceInterfaceStream.h @@ -33,7 +33,7 @@ public: * the unbuffered mode. * @return Preamle consisting of log message and timestamp. */ - std::string getPreamble(); + std::string* getPreamble(); /** * This prints an error with a preamble. Useful if using the unbuffered From 69f9ff02f047917c2c6a622bcb6fc6244d60564f Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 28 Jul 2020 17:00:51 +0200 Subject: [PATCH 06/34] better returnvalue for failed comIF init --- devicehandlers/DeviceHandlerBase.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/devicehandlers/DeviceHandlerBase.cpp b/devicehandlers/DeviceHandlerBase.cpp index 9f4c7baf..1e7ebf04 100644 --- a/devicehandlers/DeviceHandlerBase.cpp +++ b/devicehandlers/DeviceHandlerBase.cpp @@ -128,7 +128,9 @@ ReturnValue_t DeviceHandlerBase::initialize() { result = communicationInterface->initializeInterface(comCookie); if (result != RETURN_OK) { - return result; + sif::error << "DeviceHandlerBase::initialize: Initializing " + "communication interface failed!" << std::endl; + return result; } IPCStore = objectManager->get(objects::IPC_STORE); From 9acf82cf510be6dfd0f7e94601cdf4795213e5f4 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 28 Jul 2020 18:12:10 +0200 Subject: [PATCH 07/34] doc improved --- devicehandlers/DeviceHandlerBase.h | 2 ++ devicehandlers/DeviceHandlerIF.h | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/devicehandlers/DeviceHandlerBase.h b/devicehandlers/DeviceHandlerBase.h index ddbf6041..632db5e0 100644 --- a/devicehandlers/DeviceHandlerBase.h +++ b/devicehandlers/DeviceHandlerBase.h @@ -379,6 +379,8 @@ protected: * @param deviceCommand Identifier of the command to add. * @param maxDelayCycles The maximum number of delay cycles the command * waits until it times out. + * @param replyLen Will be supplied to the requestReceiveMessage call of + * the communication interface. * @param periodic Indicates if the command is periodic (i.e. it is sent * by the device repeatedly without request) or not. Default is aperiodic (0) * @return - @c RETURN_OK when the command was successfully inserted, diff --git a/devicehandlers/DeviceHandlerIF.h b/devicehandlers/DeviceHandlerIF.h index cb190344..59302080 100644 --- a/devicehandlers/DeviceHandlerIF.h +++ b/devicehandlers/DeviceHandlerIF.h @@ -98,7 +98,7 @@ public: static const uint8_t INTERFACE_ID = CLASS_ID::DEVICE_HANDLER_IF; // Standard codes used when building commands. - static const ReturnValue_t NO_COMMAND_DATA = MAKE_RETURN_CODE(0xA0); //!< If the command size is 0. Checked in DHB + static const ReturnValue_t NO_COMMAND_DATA = MAKE_RETURN_CODE(0xA0); //!< If no command data was given when expected. static const ReturnValue_t COMMAND_NOT_SUPPORTED = MAKE_RETURN_CODE(0xA1); //!< Command ID not in commandMap. Checked in DHB static const ReturnValue_t COMMAND_ALREADY_SENT = MAKE_RETURN_CODE(0xA2); //!< Command was already executed. Checked in DHB static const ReturnValue_t COMMAND_WAS_NOT_SENT = MAKE_RETURN_CODE(0xA3); From 9fa9421ba58d4c6793aa19886ce6ee17e7f85d71 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 28 Jul 2020 20:57:05 +0200 Subject: [PATCH 08/34] important bugfix --- fdir/FailureIsolationBase.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fdir/FailureIsolationBase.cpp b/fdir/FailureIsolationBase.cpp index 083f5af0..4df56dcd 100644 --- a/fdir/FailureIsolationBase.cpp +++ b/fdir/FailureIsolationBase.cpp @@ -104,9 +104,9 @@ MessageQueueId_t FailureIsolationBase::getEventReceptionQueue() { ReturnValue_t FailureIsolationBase::sendConfirmationRequest(EventMessage* event, MessageQueueId_t destination) { event->setMessageId(EventMessage::CONFIRMATION_REQUEST); - if (destination != 0) { + if (destination != MessageQueueIF::NO_QUEUE) { return eventQueue->sendMessage(destination, event); - } else if (faultTreeParent != 0) { + } else if (faultTreeParent != objects::NO_OBJECT) { return eventQueue->sendToDefault(event); } return RETURN_FAILED; From c64aa9f7f55e4afbc5f21cf149bfc71a889a06e5 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 28 Jul 2020 21:00:11 +0200 Subject: [PATCH 09/34] another important bugfix --- fdir/FailureIsolationBase.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fdir/FailureIsolationBase.h b/fdir/FailureIsolationBase.h index dea149bd..6010baca 100644 --- a/fdir/FailureIsolationBase.h +++ b/fdir/FailureIsolationBase.h @@ -45,7 +45,7 @@ protected: virtual ReturnValue_t confirmFault(EventMessage* event); virtual void decrementFaultCounters() = 0; ReturnValue_t sendConfirmationRequest(EventMessage* event, - MessageQueueId_t destination = 0); + MessageQueueId_t destination = MessageQueueIF::NO_QUEUE); void throwFdirEvent(Event event, uint32_t parameter1 = 0, uint32_t parameter2 = 0); private: From 276c3b172e18df8b7f457e23fb43c22e9c332553 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 29 Jul 2020 15:32:36 +0200 Subject: [PATCH 10/34] better diagnostic warning for DHB --- devicehandlers/DeviceHandlerBase.cpp | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/devicehandlers/DeviceHandlerBase.cpp b/devicehandlers/DeviceHandlerBase.cpp index 1e7ebf04..a4346afb 100644 --- a/devicehandlers/DeviceHandlerBase.cpp +++ b/devicehandlers/DeviceHandlerBase.cpp @@ -680,16 +680,24 @@ void DeviceHandlerBase::parseReply(const uint8_t* receivedData, switch (result) { case RETURN_OK: handleReply(receivedData, foundId, foundLen); + if(foundLen == 0) { + sif::warning << "DeviceHandlerBase::parseReply: foundLen is 0!" + " Packet parsing will be stuck." << std::endl; + } break; case APERIODIC_REPLY: { result = interpretDeviceReply(foundId, receivedData); if (result != RETURN_OK) { - replyRawReplyIfnotWiretapped(receivedData, foundLen); - triggerEvent(DEVICE_INTERPRETING_REPLY_FAILED, result, - foundId); + replyRawReplyIfnotWiretapped(receivedData, foundLen); + triggerEvent(DEVICE_INTERPRETING_REPLY_FAILED, result, + foundId); } + if(foundLen == 0) { + sif::warning << "DeviceHandlerBase::parseReply: foundLen is 0!" + " Packet parsing will be stuck." << std::endl; + } + break; } - break; case IGNORE_REPLY_DATA: break; case IGNORE_FULL_PACKET: From 18899a4c828262258699d9c80beab12cd251b506 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 29 Jul 2020 20:07:24 +0200 Subject: [PATCH 11/34] hotfix for deadline checking --- osal/FreeRTOS/FixedTimeslotTask.cpp | 19 +++++++++---------- osal/FreeRTOS/PeriodicTask.cpp | 19 +++++++++---------- 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/osal/FreeRTOS/FixedTimeslotTask.cpp b/osal/FreeRTOS/FixedTimeslotTask.cpp index 630952d2..1dc45b1d 100644 --- a/osal/FreeRTOS/FixedTimeslotTask.cpp +++ b/osal/FreeRTOS/FixedTimeslotTask.cpp @@ -125,19 +125,18 @@ void FixedTimeslotTask::checkMissedDeadline(const TickType_t xLastWakeTime, * it. */ TickType_t currentTickCount = xTaskGetTickCount(); TickType_t timeToWake = xLastWakeTime + interval; - // Tick count has overflown - if(currentTickCount < xLastWakeTime) { - // Time to wake has overflown as well. If the tick count - // is larger than the time to wake, a deadline was missed. - if(timeToWake < xLastWakeTime and - currentTickCount > timeToWake) { + // Time to wake has not overflown. + if(timeToWake > xLastWakeTime) { + /* If the current time has overflown exclusively or the current + * tick count is simply larger than the time to wake, a deadline was + * missed */ + if((currentTickCount < xLastWakeTime) or (currentTickCount > timeToWake)) { handleMissedDeadline(); } } - // No tick count overflow. If the timeToWake has not overflown - // and the current tick count is larger than the time to wake, - // a deadline was missed. - else if(timeToWake > xLastWakeTime and currentTickCount > timeToWake) { + /* Time to wake has overflown. A deadline was missed if the current time + * is larger than the time to wake */ + else if((timeToWake < xLastWakeTime) and (currentTickCount > timeToWake)) { handleMissedDeadline(); } } diff --git a/osal/FreeRTOS/PeriodicTask.cpp b/osal/FreeRTOS/PeriodicTask.cpp index f960d8bb..817cb06f 100644 --- a/osal/FreeRTOS/PeriodicTask.cpp +++ b/osal/FreeRTOS/PeriodicTask.cpp @@ -109,19 +109,18 @@ void PeriodicTask::checkMissedDeadline(const TickType_t xLastWakeTime, * it. */ TickType_t currentTickCount = xTaskGetTickCount(); TickType_t timeToWake = xLastWakeTime + interval; - // Tick count has overflown - if(currentTickCount < xLastWakeTime) { - // Time to wake has overflown as well. If the tick count - // is larger than the time to wake, a deadline was missed. - if(timeToWake < xLastWakeTime and - currentTickCount > timeToWake) { + // Time to wake has not overflown. + if(timeToWake > xLastWakeTime) { + /* If the current time has overflown exclusively or the current + * tick count is simply larger than the time to wake, a deadline was + * missed */ + if((currentTickCount < xLastWakeTime) or (currentTickCount > timeToWake)) { handleMissedDeadline(); } } - // No tick count overflow. If the timeToWake has not overflown - // and the current tick count is larger than the time to wake, - // a deadline was missed. - else if(timeToWake > xLastWakeTime and currentTickCount > timeToWake) { + /* Time to wake has overflown. A deadline was missed if the current time + * is larger than the time to wake */ + else if((timeToWake < xLastWakeTime) and (currentTickCount > timeToWake)) { handleMissedDeadline(); } } From 58a4f4f8a1d9608b671cd6fab087afea5d086dc1 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Sat, 1 Aug 2020 16:39:17 +0200 Subject: [PATCH 12/34] command message bugfix, CSB improvement parameter helper diagnostic message --- ipc/CommandMessage.cpp | 1 + parameters/ParameterHelper.cpp | 7 ++++--- parameters/ParameterHelper.h | 6 ++++++ tmtcservices/CommandingServiceBase.cpp | 8 ++++---- 4 files changed, 15 insertions(+), 7 deletions(-) diff --git a/ipc/CommandMessage.cpp b/ipc/CommandMessage.cpp index 8c296abd..cc1185c7 100644 --- a/ipc/CommandMessage.cpp +++ b/ipc/CommandMessage.cpp @@ -88,6 +88,7 @@ void CommandMessage::setToUnknownCommand() { void CommandMessage::setReplyRejected(ReturnValue_t reason, Command_t initialCommand) { + setCommand(REPLY_REJECTED); setParameter(reason); setParameter2(initialCommand); } diff --git a/parameters/ParameterHelper.cpp b/parameters/ParameterHelper.cpp index e40ed675..53f1229f 100644 --- a/parameters/ParameterHelper.cpp +++ b/parameters/ParameterHelper.cpp @@ -26,7 +26,6 @@ ReturnValue_t ParameterHelper::handleParameterMessage(CommandMessage *message) { } break; case ParameterMessage::CMD_PARAMETER_LOAD: { - uint8_t domain = HasParametersIF::getDomain( ParameterMessage::getParameterId(message)); uint16_t parameterId = HasParametersIF::getMatrixId( @@ -34,12 +33,14 @@ ReturnValue_t ParameterHelper::handleParameterMessage(CommandMessage *message) { uint8_t index = HasParametersIF::getIndex( ParameterMessage::getParameterId(message)); - const uint8_t *storedStream; - size_t storedStreamSize; + const uint8_t *storedStream = nullptr; + size_t storedStreamSize = 0; result = storage->getData( ParameterMessage::getStoreId(message), &storedStream, &storedStreamSize); if (result != HasReturnvaluesIF::RETURN_OK) { + sif::error << "ParameterHelper::handleParameterMessage: Getting" + " store data failed for load command." << std::endl; break; } diff --git a/parameters/ParameterHelper.h b/parameters/ParameterHelper.h index 352cf0a9..538aaac6 100644 --- a/parameters/ParameterHelper.h +++ b/parameters/ParameterHelper.h @@ -5,6 +5,12 @@ #include #include +/** + * @brief Helper class to handle parameter messages + * @details + * This class simplfiies handling of parameter messages, which are sent + * to a class which implements ReceivesParameterMessagesIF. + */ class ParameterHelper { public: ParameterHelper(ReceivesParameterMessagesIF *owner); diff --git a/tmtcservices/CommandingServiceBase.cpp b/tmtcservices/CommandingServiceBase.cpp index 4c9b5375..80a28093 100644 --- a/tmtcservices/CommandingServiceBase.cpp +++ b/tmtcservices/CommandingServiceBase.cpp @@ -126,11 +126,11 @@ void CommandingServiceBase::handleCommandMessage(CommandMessage* reply) { &nextCommand, iter->objectId, &isStep); /* If the child implementation does not implement special handling for - * rejected replies (RETURN_FAILED is returned), a failure verification - * will be generated with the reason as the return code and the initial - * command as failure parameter 1 */ + * rejected replies (RETURN_FAILED or INVALID_REPLY is returned), a + * failure verification will be generated with the reason as the + * return code and the initial command as failure parameter 1 */ if(reply->getCommand() == CommandMessage::REPLY_REJECTED and - result == RETURN_FAILED) { + (result == RETURN_FAILED or result == INVALID_REPLY)) { result = reply->getReplyRejectedReason(); failureParameter1 = iter->command; } From 66eac57e3bb77c73a4030d1320a64b42e5841641 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Sat, 1 Aug 2020 16:54:54 +0200 Subject: [PATCH 13/34] csb update --- tmtcservices/CommandingServiceBase.cpp | 6 +++--- tmtcservices/CommandingServiceBase.h | 17 ++++++++--------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/tmtcservices/CommandingServiceBase.cpp b/tmtcservices/CommandingServiceBase.cpp index 80a28093..330fd00f 100644 --- a/tmtcservices/CommandingServiceBase.cpp +++ b/tmtcservices/CommandingServiceBase.cpp @@ -129,7 +129,7 @@ void CommandingServiceBase::handleCommandMessage(CommandMessage* reply) { * rejected replies (RETURN_FAILED or INVALID_REPLY is returned), a * failure verification will be generated with the reason as the * return code and the initial command as failure parameter 1 */ - if(reply->getCommand() == CommandMessage::REPLY_REJECTED and + if((reply->getCommand() == CommandMessage::REPLY_REJECTED) and (result == RETURN_FAILED or result == INVALID_REPLY)) { result = reply->getReplyRejectedReason(); failureParameter1 = iter->command; @@ -230,8 +230,8 @@ void CommandingServiceBase::handleRequestQueue() { address = message.getStorageId(); packet.setStoreAddress(address); - if (packet.getSubService() == 0 - or isValidSubservice(packet.getSubService()) != RETURN_OK) { + if ((packet.getSubService() == 0) + or (isValidSubservice(packet.getSubService()) != RETURN_OK)) { rejectPacket(TC_VERIFY::START_FAILURE, &packet, INVALID_SUBSERVICE); continue; } diff --git a/tmtcservices/CommandingServiceBase.h b/tmtcservices/CommandingServiceBase.h index 1bd18571..b3a15985 100644 --- a/tmtcservices/CommandingServiceBase.h +++ b/tmtcservices/CommandingServiceBase.h @@ -87,7 +87,7 @@ public: * @param opCode is unused here at the moment * @return RETURN_OK */ - virtual ReturnValue_t performOperation(uint8_t opCode); + virtual ReturnValue_t performOperation(uint8_t opCode) override; virtual uint16_t getIdentifier(); @@ -116,7 +116,7 @@ public: * Used to setup the reference of the task, that executes this component * @param task Pointer to the taskIF of this task */ - virtual void setTaskIF(PeriodicTaskIF* task); + virtual void setTaskIF(PeriodicTaskIF* task) override; protected: /** @@ -173,9 +173,7 @@ protected: * This function is implemented by child services to specify how replies * to a command from another software component are handled. * @param reply - * This is the reply which can be accessed via the command message - * interface. The internal message pointer can be passed to different - * command message implementations (see CommandMessageIF) + * This is the reply in form of a generic read-only command message. * @param previousCommand * Command_t of related command * @param state [out/in] @@ -189,10 +187,11 @@ protected: * - @c RETURN_OK, @c EXECUTION_COMPLETE or @c NO_STEP_MESSAGE to * generate TC verification success * - @c INVALID_REPLY Calls handleUnrequestedReply - * - Anything else triggers a TC verification failure. If RETURN_FAILED - * is returned and the command ID is CommandMessage::REPLY_REJECTED, - * a failure verification message with the reason as the error parameter - * and the initial command as failure parameter 1. + * - Anything else triggers a TC verification failure. If RETURN_FAILED or + * INVALID_REPLY is returned and the command ID is + * CommandMessage::REPLY_REJECTED, a failure verification message with + * the reason as the error parameter and the initial command as + * failure parameter 1 is generated. */ virtual ReturnValue_t handleReply(const CommandMessage* reply, Command_t previousCommand, uint32_t *state, From ebf5d41a7880f4c866fa1eb29113ecc960bd0efc Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Sat, 1 Aug 2020 19:03:03 +0200 Subject: [PATCH 14/34] parameters IF doc and improvement --- parameters/HasParametersIF.h | 19 +++++-- parameters/ParameterWrapper.cpp | 24 ++++----- parameters/ParameterWrapper.h | 91 +++++++++++++++++++++------------ 3 files changed, 84 insertions(+), 50 deletions(-) diff --git a/parameters/HasParametersIF.h b/parameters/HasParametersIF.h index fbb69445..02f36318 100644 --- a/parameters/HasParametersIF.h +++ b/parameters/HasParametersIF.h @@ -5,8 +5,23 @@ #include #include +/** Each parameter is identified with a unique parameter ID */ typedef uint32_t ParameterId_t; +/** + * @brief This interface is used by components which have modifiable + * parameters, e.g. atittude controllers + * @details + * Each parameter has a unique parameter ID. The first byte of the parameter + * ID is the domain ID which can be used to identify unqiue spacecraft domains + * (e.g. control and sensor domain in the AOCS controller). + * + * The second and third byte represent the matrix ID, which can represent + * a 8-bit row and column number and the last byte... + * + * Yeah, it it matrix ID oder parameter ID now and is index a 16 bit number + * of a 8 bit number now? + */ class HasParametersIF { public: static const uint8_t INTERFACE_ID = CLASS_ID::HAS_PARAMETERS_IF; @@ -32,13 +47,11 @@ public: return (domainId << 24) + (parameterId << 8) + index; } - virtual ~HasParametersIF() { - } + virtual ~HasParametersIF() {} /** * Always set parameter before checking newValues! * - * * @param domainId * @param parameterId * @param parameterWrapper diff --git a/parameters/ParameterWrapper.cpp b/parameters/ParameterWrapper.cpp index 7daf738c..521a6644 100644 --- a/parameters/ParameterWrapper.cpp +++ b/parameters/ParameterWrapper.cpp @@ -1,20 +1,19 @@ #include ParameterWrapper::ParameterWrapper() : - pointsToStream(false), type(Type::UNKNOWN_TYPE), rows(0), columns(0), data( - NULL), readonlyData(NULL) { + pointsToStream(false), type(Type::UNKNOWN_TYPE) { } ParameterWrapper::ParameterWrapper(Type type, uint8_t rows, uint8_t columns, void *data) : - pointsToStream(false), type(type), rows(rows), columns(columns), data( - data), readonlyData(data) { + pointsToStream(false), type(type), rows(rows), columns(columns), + data(data), readonlyData(data) { } ParameterWrapper::ParameterWrapper(Type type, uint8_t rows, uint8_t columns, const void *data) : - pointsToStream(false), type(type), rows(rows), columns(columns), data( - NULL), readonlyData(data) { + pointsToStream(false), type(type), rows(rows), columns(columns), + data(nullptr), readonlyData(data) { } ParameterWrapper::~ParameterWrapper() { @@ -266,15 +265,14 @@ ReturnValue_t ParameterWrapper::copyFrom(const ParameterWrapper *from, result = UNKNOW_DATATYPE; break; } - } else { + } + else { //need a type to do arithmetic - uint8_t *toDataWithType = (uint8_t*) data; + uint8_t* typedData = static_cast(data); for (uint8_t fromRow = 0; fromRow < from->rows; fromRow++) { - memcpy( - toDataWithType - + (((startingRow + fromRow) * columns) - + startingColumn) * typeSize, - from->readonlyData, typeSize * from->columns); + uint8_t offset = (((startingRow + fromRow) * columns) + startingColumn) * typeSize; + std::memcpy(typedData + offset, from->readonlyData, + typeSize * from->columns); } } diff --git a/parameters/ParameterWrapper.h b/parameters/ParameterWrapper.h index a00c997c..eec101f2 100644 --- a/parameters/ParameterWrapper.h +++ b/parameters/ParameterWrapper.h @@ -1,5 +1,5 @@ -#ifndef PARAMETERWRAPPER_H_ -#define PARAMETERWRAPPER_H_ +#ifndef FRAMEWORK_PARAMETERS_PARAMETERWRAPPER_H_ +#define FRAMEWORK_PARAMETERS_PARAMETERWRAPPER_H_ #include #include @@ -7,6 +7,10 @@ #include #include +/** + * @brief + * @details + */ class ParameterWrapper: public SerializeIF { friend class DataPoolParameterWrapper; public: @@ -36,32 +40,21 @@ public: virtual ReturnValue_t deSerialize(const uint8_t** buffer, size_t* size, Endianness streamEndianness, uint16_t startWritingAtIndex = 0); + /** + * Get a specific parameter value by supplying the row and the column. + * @tparam T Type of target data + * @param value [out] Pointer to storage location + * @param row + * @param column + * @return + * -@c RETURN_OK if element was retrieved successfully + * -@c NOT_SET data has not been set yet + * -@c DATATYPE_MISSMATCH Invalid supplied type + * -@c OUT_OF_BOUNDS Invalid row and/or column. + */ template - ReturnValue_t getElement(T *value, uint8_t row = 0, uint8_t column = 0) const { - if (readonlyData == NULL){ - return NOT_SET; - } - - if (PodTypeConversion::type != type) { - return DATATYPE_MISSMATCH; - } - - if ((row >= rows) || (column >= columns)) { - return OUT_OF_BOUNDS; - } - - if (pointsToStream) { - const uint8_t *streamWithtype = (const uint8_t *) readonlyData; - streamWithtype += (row * columns + column) * type.getSize(); - int32_t size = type.getSize(); - return SerializeAdapter::deSerialize(value, &streamWithtype, - &size, true); - } else { - const T *dataWithType = (const T *) readonlyData; - *value = dataWithType[row * columns + column]; - return HasReturnvaluesIF::RETURN_OK; - } - } + ReturnValue_t getElement(T *value, uint8_t row = 0, + uint8_t column = 0) const; template void set(T *data, uint8_t rows, uint8_t columns) { @@ -111,21 +104,22 @@ public: void setMatrix(const T& member) { this->set(member[0], sizeof(member)/sizeof(member[0]), sizeof(member[0])/sizeof(member[0][0])); } + ReturnValue_t set(const uint8_t *stream, size_t streamSize, - const uint8_t **remainingStream = NULL, size_t *remainingSize = - NULL); + const uint8_t **remainingStream = nullptr, + size_t *remainingSize = nullptr); ReturnValue_t copyFrom(const ParameterWrapper *from, uint16_t startWritingAtIndex); private: - bool pointsToStream; + bool pointsToStream = false; Type type; - uint8_t rows; - uint8_t columns; - void *data; - const void *readonlyData; + uint8_t rows = 0; + uint8_t columns = 0; + void *data = nullptr; + const void *readonlyData = nullptr; template ReturnValue_t serializeData(uint8_t** buffer, size_t* size, @@ -136,4 +130,33 @@ private: const void *from, uint8_t fromRows, uint8_t fromColumns); }; +template +inline ReturnValue_t ParameterWrapper::getElement(T *value, uint8_t row, + uint8_t column) const { + if (readonlyData == nullptr){ + return NOT_SET; + } + + if (PodTypeConversion::type != type) { + return DATATYPE_MISSMATCH; + } + + if ((row >= rows) or (column >= columns)) { + return OUT_OF_BOUNDS; + } + + if (pointsToStream) { + const uint8_t *streamWithType = static_cast(readonlyData); + streamWithType += (row * columns + column) * type.getSize(); + int32_t size = type.getSize(); + return SerializeAdapter::deSerialize(value, &streamWithType, + &size, true); + } + else { + const T *dataWithType = static_cast(readonlyData); + *value = dataWithType[row * columns + column]; + return HasReturnvaluesIF::RETURN_OK; + } +} + #endif /* PARAMETERWRAPPER_H_ */ From f240827bbdd9ad7542e21ca5810235e6acb20c96 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 4 Aug 2020 00:59:19 +0200 Subject: [PATCH 15/34] added getFreeElement --- container/SimpleRingBuffer.h | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/container/SimpleRingBuffer.h b/container/SimpleRingBuffer.h index cf78814f..9a548815 100644 --- a/container/SimpleRingBuffer.h +++ b/container/SimpleRingBuffer.h @@ -16,12 +16,17 @@ class SimpleRingBuffer: public RingBufferBase<> { public: /** * This constructor allocates a new internal buffer with the supplied size. + * * @param size - * @param overwriteOld - * If the ring buffer is overflowing at a write operartion, the oldest data - * will be overwritten. + * @param overwriteOld If the ring buffer is overflowing at a write + * operation, the oldest data will be overwritten. + * @param maxExcessBytes These additional bytes will be allocated in addtion + * to the specified size to accomodate contiguous write operations + * with getFreeElement. + * */ - SimpleRingBuffer(const size_t size, bool overwriteOld); + SimpleRingBuffer(const size_t size, bool overwriteOld, + size_t maxExcessBytes = 0); /** * This constructor takes an external buffer with the specified size. * @param buffer @@ -29,8 +34,13 @@ public: * @param overwriteOld * If the ring buffer is overflowing at a write operartion, the oldest data * will be overwritten. + * @param maxExcessBytes + * If the buffer can accomodate additional bytes for contigous write + * operations with getFreeElement, this is the maximum allowed additional + * size */ - SimpleRingBuffer(uint8_t* buffer, const size_t size, bool overwriteOld); + SimpleRingBuffer(uint8_t* buffer, const size_t size, bool overwriteOld, + size_t maxExcessBytes = 0); virtual ~SimpleRingBuffer(); @@ -43,6 +53,17 @@ public: */ ReturnValue_t writeData(const uint8_t* data, size_t amount); + /** + * Returns a pointer to a free element. If the remaining buffer is + * not large enough, the data will be written past the actual size + * and the amount of excess bytes will be cached. + * @param writePointer Pointer to a pointer which can be used to write + * contiguous blocks into the ring buffer + * @param amount + * @return + */ + ReturnValue_t getFreeElement(uint8_t** writePointer, size_t amount); + /** * Read from circular buffer at read pointer. * @param data @@ -82,6 +103,8 @@ public: private: static const uint8_t READ_PTR = 0; uint8_t* buffer = nullptr; + const size_t maxExcessBytes; + size_t excessBytes = 0; }; #endif /* FRAMEWORK_CONTAINER_SIMPLERINGBUFFER_H_ */ From 0ead44bea976bba70676fc40ed9716f2ae6b84d8 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 4 Aug 2020 01:07:59 +0200 Subject: [PATCH 16/34] getFreeElement implemented --- container/SimpleRingBuffer.cpp | 44 ++++++++++++++++++++++++++++++---- container/SimpleRingBuffer.h | 8 +++++++ 2 files changed, 47 insertions(+), 5 deletions(-) diff --git a/container/SimpleRingBuffer.cpp b/container/SimpleRingBuffer.cpp index 69d94143..3cc1ffdf 100644 --- a/container/SimpleRingBuffer.cpp +++ b/container/SimpleRingBuffer.cpp @@ -1,20 +1,43 @@ #include #include -SimpleRingBuffer::SimpleRingBuffer(const size_t size, bool overwriteOld) : - RingBufferBase<>(0, size, overwriteOld) { - buffer = new uint8_t[size]; +SimpleRingBuffer::SimpleRingBuffer(const size_t size, bool overwriteOld, + size_t maxExcessBytes) : + RingBufferBase<>(0, size, overwriteOld), + maxExcessBytes(maxExcessBytes) { + buffer = new uint8_t[size + maxExcessBytes]; } SimpleRingBuffer::SimpleRingBuffer(uint8_t *buffer, const size_t size, - bool overwriteOld): - RingBufferBase<>(0, size, overwriteOld), buffer(buffer) {} + bool overwriteOld, size_t maxExcessBytes): + RingBufferBase<>(0, size, overwriteOld), buffer(buffer), + maxExcessBytes(maxExcessBytes) {} SimpleRingBuffer::~SimpleRingBuffer() { delete[] buffer; } + +ReturnValue_t SimpleRingBuffer::getFreeElement(uint8_t **writePointer, + size_t amount) { + if (availableWriteSpace() >= amount or overwriteOld) { + size_t amountTillWrap = writeTillWrap(); + if (amountTillWrap < amount) { + if((amount - amountTillWrap + excessBytes) > maxExcessBytes) { + return HasReturnvaluesIF::RETURN_FAILED; + } + excessBytes += amount - amountTillWrap; + } + *writePointer = &buffer[write]; + incrementWrite(amount); + return HasReturnvaluesIF::RETURN_OK; + } + else { + return HasReturnvaluesIF::RETURN_FAILED; + } +} + ReturnValue_t SimpleRingBuffer::writeData(const uint8_t* data, size_t amount) { if (availableWriteSpace() >= amount or overwriteOld) { @@ -62,6 +85,17 @@ ReturnValue_t SimpleRingBuffer::readData(uint8_t* data, size_t amount, return HasReturnvaluesIF::RETURN_OK; } +size_t SimpleRingBuffer::getExcessBytes() const { + return excessBytes; +} + +void SimpleRingBuffer::moveExcessBytesToStart() { + if(excessBytes > 0) { + std::memcpy(buffer, &buffer[size], excessBytes); + excessBytes = 0; + } +} + ReturnValue_t SimpleRingBuffer::deleteData(size_t amount, bool deleteRemaining, size_t* trueAmount) { size_t availableData = availableReadData(READ_PTR); diff --git a/container/SimpleRingBuffer.h b/container/SimpleRingBuffer.h index 9a548815..4bb3a4e8 100644 --- a/container/SimpleRingBuffer.h +++ b/container/SimpleRingBuffer.h @@ -64,6 +64,14 @@ public: */ ReturnValue_t getFreeElement(uint8_t** writePointer, size_t amount); + size_t getExcessBytes() const; + /** + * Helper functions which moves any excess bytes to the start + * of the ring buffer. + * @return + */ + void moveExcessBytesToStart(); + /** * Read from circular buffer at read pointer. * @param data From 4e9e465360b2b4757699e79b265c12358ac5e8db Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 4 Aug 2020 02:00:00 +0200 Subject: [PATCH 17/34] shared ring buffer continued --- container/SimpleRingBuffer.cpp | 20 ++++++++++++++++---- container/SimpleRingBuffer.h | 2 +- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/container/SimpleRingBuffer.cpp b/container/SimpleRingBuffer.cpp index 3cc1ffdf..cad1a48e 100644 --- a/container/SimpleRingBuffer.cpp +++ b/container/SimpleRingBuffer.cpp @@ -2,16 +2,28 @@ #include SimpleRingBuffer::SimpleRingBuffer(const size_t size, bool overwriteOld, - size_t maxExcessBytes) : + size_t maxExcessBytes) : RingBufferBase<>(0, size, overwriteOld), maxExcessBytes(maxExcessBytes) { + if(maxExcessBytes > size) { + this->maxExcessBytes = size; + } + else { + this->maxExcessBytes = maxExcessBytes; + } buffer = new uint8_t[size + maxExcessBytes]; } SimpleRingBuffer::SimpleRingBuffer(uint8_t *buffer, const size_t size, - bool overwriteOld, size_t maxExcessBytes): - RingBufferBase<>(0, size, overwriteOld), buffer(buffer), - maxExcessBytes(maxExcessBytes) {} + bool overwriteOld, size_t maxExcessBytes): + RingBufferBase<>(0, size, overwriteOld), buffer(buffer) { + if(maxExcessBytes > size) { + this->maxExcessBytes = size; + } + else { + this->maxExcessBytes = maxExcessBytes; + } +} SimpleRingBuffer::~SimpleRingBuffer() { diff --git a/container/SimpleRingBuffer.h b/container/SimpleRingBuffer.h index 4bb3a4e8..2627541f 100644 --- a/container/SimpleRingBuffer.h +++ b/container/SimpleRingBuffer.h @@ -111,7 +111,7 @@ public: private: static const uint8_t READ_PTR = 0; uint8_t* buffer = nullptr; - const size_t maxExcessBytes; + size_t maxExcessBytes; size_t excessBytes = 0; }; From 6b475792a4a7c57a5006a010c554dda8b31d41b7 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 4 Aug 2020 02:25:10 +0200 Subject: [PATCH 18/34] shared ring buffer continued --- container/SharedRingBuffer.cpp | 19 +++++++++++++------ container/SharedRingBuffer.h | 9 +++++++-- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/container/SharedRingBuffer.cpp b/container/SharedRingBuffer.cpp index 64cbc669..86277a80 100644 --- a/container/SharedRingBuffer.cpp +++ b/container/SharedRingBuffer.cpp @@ -3,19 +3,26 @@ #include SharedRingBuffer::SharedRingBuffer(object_id_t objectId, const size_t size, - bool overwriteOld, dur_millis_t mutexTimeout): - SystemObject(objectId), SimpleRingBuffer(size, overwriteOld), - mutexTimeout(mutexTimeout) { + bool overwriteOld, size_t maxExcessBytes, dur_millis_t mutexTimeout): + SystemObject(objectId), SimpleRingBuffer(size, overwriteOld, + maxExcessBytes), mutexTimeout(mutexTimeout) { mutex = MutexFactory::instance()->createMutex(); } SharedRingBuffer::SharedRingBuffer(object_id_t objectId, uint8_t *buffer, - const size_t size, bool overwriteOld, dur_millis_t mutexTimeout): - SystemObject(objectId), SimpleRingBuffer(buffer, size, overwriteOld), - mutexTimeout(mutexTimeout) { + const size_t size, bool overwriteOld, size_t maxExcessBytes, + dur_millis_t mutexTimeout): + SystemObject(objectId), SimpleRingBuffer(buffer, size, overwriteOld, + maxExcessBytes), mutexTimeout(mutexTimeout) { mutex = MutexFactory::instance()->createMutex(); } +ReturnValue_t SharedRingBuffer::getFreeElementProtected(uint8_t** writePtr, + size_t amount) { + MutexHelper(mutex, mutexTimeout); + return SimpleRingBuffer::getFreeElement(writePtr,amount); +} + ReturnValue_t SharedRingBuffer::writeDataProtected(const uint8_t *data, size_t amount) { MutexHelper(mutex, mutexTimeout); diff --git a/container/SharedRingBuffer.h b/container/SharedRingBuffer.h index f7d3bc3c..7fed7b22 100644 --- a/container/SharedRingBuffer.h +++ b/container/SharedRingBuffer.h @@ -17,7 +17,8 @@ public: * will be overwritten. */ SharedRingBuffer(object_id_t objectId, const size_t size, - bool overwriteOld, dur_millis_t mutexTimeout = 10); + bool overwriteOld, size_t maxExcessBytes, + dur_millis_t mutexTimeout = 10); /** * This constructor takes an external buffer with the specified size. @@ -28,10 +29,14 @@ public: * will be overwritten. */ SharedRingBuffer(object_id_t objectId, uint8_t* buffer, const size_t size, - bool overwriteOld, dur_millis_t mutexTimeout = 10); + bool overwriteOld, size_t maxExcessBytes, + dur_millis_t mutexTimeout = 10); void setMutexTimeout(dur_millis_t newTimeout); + /** Performs mutex protected SimpleRingBuffer::getFreeElement call */ + ReturnValue_t getFreeElementProtected(uint8_t** writePtr, size_t amount); + /** Performs mutex protected SimpleRingBuffer::writeData call */ ReturnValue_t writeDataProtected(const uint8_t* data, size_t amount); From 4dcfa5125edf80f2bc6bde7861e1da394b4d7a4e Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 4 Aug 2020 11:47:47 +0200 Subject: [PATCH 19/34] added additional calls --- container/SharedRingBuffer.cpp | 10 ++++++++++ container/SharedRingBuffer.h | 8 ++++++++ container/SimpleRingBuffer.cpp | 2 +- container/SimpleRingBuffer.h | 4 ++-- 4 files changed, 21 insertions(+), 3 deletions(-) diff --git a/container/SharedRingBuffer.cpp b/container/SharedRingBuffer.cpp index 86277a80..2fde2851 100644 --- a/container/SharedRingBuffer.cpp +++ b/container/SharedRingBuffer.cpp @@ -43,6 +43,16 @@ ReturnValue_t SharedRingBuffer::deleteDataProtected(size_t amount, return SimpleRingBuffer::deleteData(amount, deleteRemaining, trueAmount); } +size_t SharedRingBuffer::getExcessBytes() const { + MutexHelper(mutex, mutexTimeout); + return SimpleRingBuffer::getExcessBytes(); +} + +void SharedRingBuffer::moveExcessBytesToStart() { + MutexHelper(mutex, mutexTimeout); + return SimpleRingBuffer::moveExcessBytesToStart(); +} + size_t SharedRingBuffer::getAvailableReadDataProtected(uint8_t n) const { MutexHelper(mutex, mutexTimeout); return ((write + size) - read[n]) % size; diff --git a/container/SharedRingBuffer.h b/container/SharedRingBuffer.h index 7fed7b22..8d68b967 100644 --- a/container/SharedRingBuffer.h +++ b/container/SharedRingBuffer.h @@ -34,6 +34,14 @@ public: void setMutexTimeout(dur_millis_t newTimeout); + virtual size_t getExcessBytes() const override; + /** + * Helper functions which moves any excess bytes to the start + * of the ring buffer. + * @return + */ + virtual void moveExcessBytesToStart() override; + /** Performs mutex protected SimpleRingBuffer::getFreeElement call */ ReturnValue_t getFreeElementProtected(uint8_t** writePtr, size_t amount); diff --git a/container/SimpleRingBuffer.cpp b/container/SimpleRingBuffer.cpp index cad1a48e..36880813 100644 --- a/container/SimpleRingBuffer.cpp +++ b/container/SimpleRingBuffer.cpp @@ -39,7 +39,7 @@ ReturnValue_t SimpleRingBuffer::getFreeElement(uint8_t **writePointer, if((amount - amountTillWrap + excessBytes) > maxExcessBytes) { return HasReturnvaluesIF::RETURN_FAILED; } - excessBytes += amount - amountTillWrap; + excessBytes = amount - amountTillWrap; } *writePointer = &buffer[write]; incrementWrite(amount); diff --git a/container/SimpleRingBuffer.h b/container/SimpleRingBuffer.h index 2627541f..0784e415 100644 --- a/container/SimpleRingBuffer.h +++ b/container/SimpleRingBuffer.h @@ -64,13 +64,13 @@ public: */ ReturnValue_t getFreeElement(uint8_t** writePointer, size_t amount); - size_t getExcessBytes() const; + virtual size_t getExcessBytes() const; /** * Helper functions which moves any excess bytes to the start * of the ring buffer. * @return */ - void moveExcessBytesToStart(); + virtual void moveExcessBytesToStart(); /** * Read from circular buffer at read pointer. From 2177877625aa1543c585504505b55b49fd1c2904 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 4 Aug 2020 15:16:09 +0200 Subject: [PATCH 20/34] updated framework subsystem ID ranges --- events/fwSubsystemIdRanges.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/events/fwSubsystemIdRanges.h b/events/fwSubsystemIdRanges.h index 4c261e57..a05652c2 100644 --- a/events/fwSubsystemIdRanges.h +++ b/events/fwSubsystemIdRanges.h @@ -18,6 +18,8 @@ enum { SYSTEM_MANAGER = 74, SYSTEM_MANAGER_1 = 75, SYSTEM_1 = 79, + PUS_SERVICE_1 = 80, + FW_SUBSYSTEM_ID_RANGE }; } From 4d3f0875c18a69147c18f99d401175e5b64c361f Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 4 Aug 2020 15:23:37 +0200 Subject: [PATCH 21/34] periodic task if adapted --- osal/FreeRTOS/PeriodicTask.cpp | 7 ++----- osal/FreeRTOS/PeriodicTask.h | 3 +-- osal/linux/PeriodicPosixTask.cpp | 11 +++-------- osal/linux/PeriodicPosixTask.h | 3 +-- tasks/PeriodicTaskIF.h | 3 +-- 5 files changed, 8 insertions(+), 19 deletions(-) diff --git a/osal/FreeRTOS/PeriodicTask.cpp b/osal/FreeRTOS/PeriodicTask.cpp index 817cb06f..cc29edae 100644 --- a/osal/FreeRTOS/PeriodicTask.cpp +++ b/osal/FreeRTOS/PeriodicTask.cpp @@ -90,12 +90,9 @@ ReturnValue_t PeriodicTask::addComponent(object_id_t object, bool setTaskIF) { return HasReturnvaluesIF::RETURN_FAILED; } objectList.push_back(newObject); + newObject->setTaskIF(this); - if(setTaskIF) { - newObject->setTaskIF(this); - } - ReturnValue_t result = newObject->initializeAfterTaskCreation(); - return result; + return newObject->initializeAfterTaskCreation(); } uint32_t PeriodicTask::getPeriodMs() const { diff --git a/osal/FreeRTOS/PeriodicTask.h b/osal/FreeRTOS/PeriodicTask.h index ca2c5324..2be2521c 100644 --- a/osal/FreeRTOS/PeriodicTask.h +++ b/osal/FreeRTOS/PeriodicTask.h @@ -65,8 +65,7 @@ public: * -@c RETURN_OK on success * -@c RETURN_FAILED if the object could not be added. */ - ReturnValue_t addComponent(object_id_t object, - bool setTaskIF = true) override; + ReturnValue_t addComponent(object_id_t object) override; uint32_t getPeriodMs() const override; diff --git a/osal/linux/PeriodicPosixTask.cpp b/osal/linux/PeriodicPosixTask.cpp index 4b0c77fc..cd487531 100644 --- a/osal/linux/PeriodicPosixTask.cpp +++ b/osal/linux/PeriodicPosixTask.cpp @@ -22,8 +22,7 @@ void* PeriodicPosixTask::taskEntryPoint(void* arg) { return NULL; } -ReturnValue_t PeriodicPosixTask::addComponent(object_id_t object, - bool setTaskIF) { +ReturnValue_t PeriodicPosixTask::addComponent(object_id_t object) { ExecutableObjectIF* newObject = objectManager->get( object); if (newObject == nullptr) { @@ -32,13 +31,9 @@ ReturnValue_t PeriodicPosixTask::addComponent(object_id_t object, return HasReturnvaluesIF::RETURN_FAILED; } objectList.push_back(newObject); + newObject->setTaskIF(this); - if(setTaskIF) { - newObject->setTaskIF(this); - } - - ReturnValue_t result = newObject->initializeAfterTaskCreation(); - return result; + return newObject->initializeAfterTaskCreation(); } ReturnValue_t PeriodicPosixTask::sleepFor(uint32_t ms) { diff --git a/osal/linux/PeriodicPosixTask.h b/osal/linux/PeriodicPosixTask.h index 9b477e36..0b2d51cd 100644 --- a/osal/linux/PeriodicPosixTask.h +++ b/osal/linux/PeriodicPosixTask.h @@ -39,8 +39,7 @@ public: * @param object Id of the object to add. * @return RETURN_OK on success, RETURN_FAILED if the object could not be added. */ - ReturnValue_t addComponent(object_id_t object, - bool setTaskIF = true) override; + ReturnValue_t addComponent(object_id_t object) override; uint32_t getPeriodMs() const override; diff --git a/tasks/PeriodicTaskIF.h b/tasks/PeriodicTaskIF.h index 6f490977..f54d6155 100644 --- a/tasks/PeriodicTaskIF.h +++ b/tasks/PeriodicTaskIF.h @@ -36,8 +36,7 @@ public: * to the component. * @return */ - virtual ReturnValue_t addComponent(object_id_t object, - bool setTaskIF = true) { + virtual ReturnValue_t addComponent(object_id_t object) { return HasReturnvaluesIF::RETURN_FAILED; }; From 3f87537c6106feb54ab0151247be0ad300a88580 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 4 Aug 2020 15:25:24 +0200 Subject: [PATCH 22/34] minor corrections --- osal/FreeRTOS/PeriodicTask.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osal/FreeRTOS/PeriodicTask.cpp b/osal/FreeRTOS/PeriodicTask.cpp index cc29edae..f2dcd093 100644 --- a/osal/FreeRTOS/PeriodicTask.cpp +++ b/osal/FreeRTOS/PeriodicTask.cpp @@ -81,7 +81,7 @@ void PeriodicTask::taskFunctionality() { } } -ReturnValue_t PeriodicTask::addComponent(object_id_t object, bool setTaskIF) { +ReturnValue_t PeriodicTask::addComponent(object_id_t object) { ExecutableObjectIF* newObject = objectManager->get( object); if (newObject == nullptr) { From c4ffb1acd5bff2a8214ddb9f4336d081b3f84636 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 4 Aug 2020 18:12:15 +0200 Subject: [PATCH 23/34] subservice number correction --- pus/servicepackets/Service1Packets.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pus/servicepackets/Service1Packets.h b/pus/servicepackets/Service1Packets.h index b01942c6..4a2c7cbc 100644 --- a/pus/servicepackets/Service1Packets.h +++ b/pus/servicepackets/Service1Packets.h @@ -16,7 +16,7 @@ * @brief Subservice 1, 3, 5, 7 * @ingroup spacepackets */ -class FailureReport: public SerializeIF { //!< [EXPORT] : [SUBSERVICE] 1, 3, 5, 7 +class FailureReport: public SerializeIF { //!< [EXPORT] : [SUBSERVICE] 2, 4, 6, 8 public: FailureReport(uint8_t failureSubtype_, uint16_t packetId_, uint16_t packetSequenceControl_, uint8_t stepNumber_, @@ -111,7 +111,7 @@ private: * @brief Subservices 2, 4, 6, 8 * @ingroup spacepackets */ -class SuccessReport: public SerializeIF { //!< [EXPORT] : [SUBSERVICE] 2, 4, 6, 8 +class SuccessReport: public SerializeIF { //!< [EXPORT] : [SUBSERVICE] 1, 3, 5, 7 public: SuccessReport(uint8_t subtype_, uint16_t packetId_, uint16_t packetSequenceControl_,uint8_t stepNumber_) : From 66039bd3feb553e913fe8440a48e09c1b8843166 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 4 Aug 2020 18:14:39 +0200 Subject: [PATCH 24/34] move group def down --- pus/servicepackets/Service1Packets.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pus/servicepackets/Service1Packets.h b/pus/servicepackets/Service1Packets.h index 4a2c7cbc..4ba493d0 100644 --- a/pus/servicepackets/Service1Packets.h +++ b/pus/servicepackets/Service1Packets.h @@ -1,3 +1,9 @@ +#ifndef MISSION_PUS_SERVICEPACKETS_SERVICE1PACKETS_H_ +#define MISSION_PUS_SERVICEPACKETS_SERVICE1PACKETS_H_ + +#include +#include + /** * @defgroup spacepackets PUS Packet Definitions * This group contains all implemented TM or TM packages that are sent to @@ -5,12 +11,6 @@ * packet structures in Mission Information Base (MIB). */ -#ifndef MISSION_PUS_SERVICEPACKETS_SERVICE1PACKETS_H_ -#define MISSION_PUS_SERVICEPACKETS_SERVICE1PACKETS_H_ - -#include -#include - /** * @brief FailureReport class to serialize a failure report * @brief Subservice 1, 3, 5, 7 From 1a118fe215350eb321184bae30777278095cb1c7 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Fri, 7 Aug 2020 19:21:47 +0200 Subject: [PATCH 25/34] added documentation to assembly base --- devicehandlers/AssemblyBase.cpp | 5 +- devicehandlers/AssemblyBase.h | 85 ++++++++++++++++++++++----------- subsystem/SubsystemBase.cpp | 10 ++-- 3 files changed, 65 insertions(+), 35 deletions(-) diff --git a/devicehandlers/AssemblyBase.cpp b/devicehandlers/AssemblyBase.cpp index bd85d1de..f5657327 100644 --- a/devicehandlers/AssemblyBase.cpp +++ b/devicehandlers/AssemblyBase.cpp @@ -165,9 +165,8 @@ ReturnValue_t AssemblyBase::checkChildrenState() { } ReturnValue_t AssemblyBase::checkChildrenStateOff() { - for (std::map::iterator iter = childrenMap.begin(); - iter != childrenMap.end(); iter++) { - if (checkChildOff(iter->first) != RETURN_OK) { + for (const auto& childIter: childrenMap) { + if (checkChildOff(childIter.first) != RETURN_OK) { return NOT_ENOUGH_CHILDREN_IN_CORRECT_STATE; } } diff --git a/devicehandlers/AssemblyBase.h b/devicehandlers/AssemblyBase.h index ee121032..d0dc868c 100644 --- a/devicehandlers/AssemblyBase.h +++ b/devicehandlers/AssemblyBase.h @@ -1,10 +1,30 @@ -#ifndef ASSEMBLYBASE_H_ -#define ASSEMBLYBASE_H_ +#ifndef FRAMEWORK_DEVICEHANDLERS_ASSEMBLYBASE_H_ +#define FRAMEWORK_DEVICEHANDLERS_ASSEMBLYBASE_H_ #include #include #include +/** + * @brief Base class to implement reconfiguration and failure handling for + * redundant devices by monitoring their modes health states. + * @details + * Documentation: Dissertation Baetz p.156, 157. + * + * This class reduces the complexity of controller components which would + * otherwise be needed for the handling of redundant devices. + * + * The template class monitors mode and health state of its children + * and checks availability of devices on every detected change. + * AssemblyBase does not implement any redundancy logic by itself, but provides + * adaptation points for implementations to do so. Since most monitoring + * activities rely on mode and health state only and are therefore + * generic, it is sufficient for subclasses to provide: + * + * 1. check logic when active-> checkChildrenStateOn + * 2. transition logic to change the mode -> commandChildren + * + */ class AssemblyBase: public SubsystemBase { public: static const uint8_t INTERFACE_ID = CLASS_ID::ASSEMBLY_BASE; @@ -16,10 +36,41 @@ public: static const ReturnValue_t NOT_ENOUGH_CHILDREN_IN_CORRECT_STATE = MAKE_RETURN_CODE(0xa1); - AssemblyBase(object_id_t objectId, object_id_t parentId, uint16_t commandQueueDepth = 8); + AssemblyBase(object_id_t objectId, object_id_t parentId, + uint16_t commandQueueDepth = 8); virtual ~AssemblyBase(); protected: + + // SHOULDDO: Change that OVERWRITE_HEALTH may be returned + // (or return internalState directly?) + /** + * Command children to reach [mode,submode] combination + * Can be done by setting #commandsOutstanding correctly, + * or using executeTable() + * @param mode + * @param submode + * @return + * - @c RETURN_OK if ok + * - @c NEED_SECOND_STEP if children need to be commanded again + */ + virtual ReturnValue_t commandChildren(Mode_t mode, Submode_t submode) = 0; + + /** + * Check whether desired assembly mode was achieved by checking the modes + * or/and health states of child device handlers. + * The assembly template class will also call this function if a health + * or mode change of a child device handler was detected. + * @param wantedMode + * @param wantedSubmode + * @return + */ + virtual ReturnValue_t checkChildrenStateOn(Mode_t wantedMode, + Submode_t wantedSubmode) = 0; + + virtual ReturnValue_t isModeCombinationValid(Mode_t mode, + Submode_t submode) = 0; + enum InternalState { STATE_NONE, STATE_OVERWRITE_HEALTH, @@ -36,6 +87,7 @@ protected: RECOVERY_WAIT } recoveryState; //!< Indicates if one of the children requested a recovery. ChildrenMap::iterator recoveringDevice; + /** * the mode the current transition is trying to achieve. * Can be different from the modehelper.commandedMode! @@ -61,8 +113,8 @@ protected: bool handleChildrenChanged(); /** - * This method is called if the children changed its mode in a way that the current - * mode can't be kept. + * This method is called if the children changed its mode in a way that + * the current mode can't be kept. * Default behavior is to go to MODE_OFF. * @param result The failure code which was returned by checkChildrenState. */ @@ -75,9 +127,6 @@ protected: ReturnValue_t checkModeCommand(Mode_t mode, Submode_t submode, uint32_t *msToReachTheMode); - virtual ReturnValue_t isModeCombinationValid(Mode_t mode, - Submode_t submode) = 0; - virtual void startTransition(Mode_t mode, Submode_t submode); virtual void doStartTransition(Mode_t mode, Submode_t submode); @@ -90,24 +139,6 @@ protected: void sendHealthCommand(MessageQueueId_t sendTo, HealthState health); - //SHOULDDO: Change that OVERWRITE_HEALTH may be returned (or return internalState directly?) - /** - * command children to reach mode,submode - * - * set #commandsOutstanding correctly, or use executeTable() - * - * @param mode - * @param submode - * @return - * - @c RETURN_OK if ok - * - @c NEED_SECOND_STEP if children need to be commanded again - */ - virtual ReturnValue_t commandChildren(Mode_t mode, Submode_t submode) = 0; - - //SHOULDDO: Remove wantedMode, wantedSubmode, as targetMode/submode is available? - virtual ReturnValue_t checkChildrenStateOn(Mode_t wantedMode, - Submode_t wantedSubmode) = 0; - virtual ReturnValue_t checkChildrenStateOff(); ReturnValue_t checkChildrenState(); @@ -129,4 +160,4 @@ protected: }; -#endif /* ASSEMBLYBASE_H_ */ +#endif /* FRAMEWORK_DEVICEHANDLERS_ASSEMBLYBASE_H_ */ diff --git a/subsystem/SubsystemBase.cpp b/subsystem/SubsystemBase.cpp index 737accef..51c35f1f 100644 --- a/subsystem/SubsystemBase.cpp +++ b/subsystem/SubsystemBase.cpp @@ -5,9 +5,9 @@ SubsystemBase::SubsystemBase(object_id_t setObjectId, object_id_t parent, Mode_t initialMode, uint16_t commandQueueDepth) : - SystemObject(setObjectId), mode(initialMode), submode(SUBMODE_NONE), childrenChangedMode( - false), commandsOutstanding(0), commandQueue(NULL), healthHelper(this, - setObjectId), modeHelper(this), parentId(parent) { + SystemObject(setObjectId), mode(initialMode), submode(SUBMODE_NONE), + childrenChangedMode(false), commandsOutstanding(0), commandQueue(NULL), + healthHelper(this, setObjectId), modeHelper(this), parentId(parent) { commandQueue = QueueFactory::instance()->createMessageQueue(commandQueueDepth, MessageQueueMessage::MAX_MESSAGE_SIZE); } @@ -23,8 +23,8 @@ ReturnValue_t SubsystemBase::registerChild(object_id_t objectId) { HasModesIF *child = objectManager->get(objectId); //This is a rather ugly hack to have the changedHealth info for all children available. (needed for FOGs). HasHealthIF* healthChild = objectManager->get(objectId); - if (child == NULL) { - if (healthChild == NULL) { + if (child == nullptr) { + if (healthChild == nullptr) { return CHILD_DOESNT_HAVE_MODES; } else { info.commandQueue = healthChild->getCommandQueue(); From bfecbfbd80a9f64f84b55246b742c07bf1beccf9 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Fri, 7 Aug 2020 20:36:37 +0200 Subject: [PATCH 26/34] implemented new mutex interface --- datapoolglob/GlobalDataPool.cpp | 3 ++- datapoollocal/LocalDataSet.cpp | 2 +- ipc/MutexHelper.h | 3 ++- ipc/MutexIF.h | 37 ++++++++++++++++++--------------- osal/FreeRTOS/Mutex.cpp | 15 +++++++------ osal/FreeRTOS/Mutex.h | 3 ++- osal/linux/Mutex.cpp | 12 ++++++++--- osal/linux/Mutex.h | 2 +- subsystem/SubsystemBase.cpp | 5 +++-- 9 files changed, 47 insertions(+), 35 deletions(-) diff --git a/datapoolglob/GlobalDataPool.cpp b/datapoolglob/GlobalDataPool.cpp index 25acaab3..9dedb4b8 100644 --- a/datapoolglob/GlobalDataPool.cpp +++ b/datapoolglob/GlobalDataPool.cpp @@ -54,7 +54,8 @@ ReturnValue_t GlobalDataPool::unlockDataPool() { } ReturnValue_t GlobalDataPool::lockDataPool(uint32_t timeoutMs) { - ReturnValue_t status = mutex->lockMutex(timeoutMs); + ReturnValue_t status = mutex->lockMutex(MutexIF::TimeoutType::WAITING, + timeoutMs); if(status != RETURN_OK) { sif::error << "DataPool::DataPool: lock of mutex failed " "with error code: " << status << std::endl; diff --git a/datapoollocal/LocalDataSet.cpp b/datapoollocal/LocalDataSet.cpp index 2a42cd38..8725f697 100644 --- a/datapoollocal/LocalDataSet.cpp +++ b/datapoollocal/LocalDataSet.cpp @@ -38,7 +38,7 @@ LocalDataSet::~LocalDataSet() { ReturnValue_t LocalDataSet::lockDataPool(uint32_t timeoutMs) { MutexIF* mutex = hkManager->getMutexHandle(); - return mutex->lockMutex(timeoutMs); + return mutex->lockMutex(MutexIF::TimeoutType::WAITING, timeoutMs); } ReturnValue_t LocalDataSet::serializeWithValidityBuffer(uint8_t **buffer, diff --git a/ipc/MutexHelper.h b/ipc/MutexHelper.h index c9579e9b..fc6e38bf 100644 --- a/ipc/MutexHelper.h +++ b/ipc/MutexHelper.h @@ -8,7 +8,8 @@ class MutexHelper { public: MutexHelper(MutexIF* mutex, uint32_t timeoutMs) : internalMutex(mutex) { - ReturnValue_t status = mutex->lockMutex(timeoutMs); + ReturnValue_t status = mutex->lockMutex(MutexIF::TimeoutType::WAITING, + timeoutMs); if(status == MutexIF::MUTEX_TIMEOUT) { sif::error << "MutexHelper: Lock of mutex failed with timeout of " << timeoutMs << " milliseconds!" << std::endl; diff --git a/ipc/MutexIF.h b/ipc/MutexIF.h index 29e59e58..b174fe0d 100644 --- a/ipc/MutexIF.h +++ b/ipc/MutexIF.h @@ -12,20 +12,25 @@ */ class MutexIF { public: - /** - * @brief Timeout value used for polling lock attempt. - * @details - * If the lock is not successfull, MUTEX_TIMEOUT will be returned - * immediately. Value needs to be defined in implementation. - */ - static const uint32_t POLLING; - /** - * @brief Timeout value used for permanent blocking lock attempt. - * @details - * The task will be blocked (indefinitely) until the mutex is unlocked. - * Value needs to be defined in implementation. - */ - static const uint32_t BLOCKING; + /** + * Different types of timeout for the mutex lock. + */ + enum TimeoutType { + POLLING, //!< If mutex is not available, return immediately + WAITING, //!< Wait a specified time for the mutex to become available + BLOCKING //!< Block indefinitely until the mutex becomes available. + }; + + /** + * Lock the mutex. + * @param timeoutType + * @param timeoutMs + * This value will only be used for TimeoutType::WAIT + * @return + */ + virtual ReturnValue_t lockMutex(TimeoutType timeoutType = + TimeoutType::BLOCKING, uint32_t timeoutMs = 0) = 0; + virtual ReturnValue_t unlockMutex() = 0; static const uint8_t INTERFACE_ID = CLASS_ID::MUTEX_IF; /** @@ -77,9 +82,7 @@ public: */ static const ReturnValue_t MUTEX_DESTROYED_WHILE_WAITING = MAKE_RETURN_CODE(12); - virtual ~MutexIF() {} - virtual ReturnValue_t lockMutex(uint32_t timeoutMs) = 0; - virtual ReturnValue_t unlockMutex() = 0; + virtual ~MutexIF() {} }; diff --git a/osal/FreeRTOS/Mutex.cpp b/osal/FreeRTOS/Mutex.cpp index 8a5c444d..764b0ae0 100644 --- a/osal/FreeRTOS/Mutex.cpp +++ b/osal/FreeRTOS/Mutex.cpp @@ -2,9 +2,6 @@ #include -const uint32_t MutexIF::POLLING = 0; -const uint32_t MutexIF::BLOCKING = portMAX_DELAY; - Mutex::Mutex() { handle = xSemaphoreCreateMutex(); if(handle == nullptr) { @@ -19,15 +16,17 @@ Mutex::~Mutex() { } -ReturnValue_t Mutex::lockMutex(uint32_t timeoutMs) { +ReturnValue_t Mutex::lockMutex(TimeoutType timeoutType, + uint32_t timeoutMs) { if (handle == nullptr) { return MutexIF::MUTEX_NOT_FOUND; } - TickType_t timeout = MutexIF::POLLING; - if(timeoutMs == MutexIF::BLOCKING) { - timeout = MutexIF::BLOCKING; + // If the timeout type is BLOCKING, this will be the correct value. + uint32_t timeout = portMAX_DELAY; + if(timeoutType == TimeoutType::POLLING) { + timeout = 0; } - else if(timeoutMs > MutexIF::POLLING){ + else if(timeoutType == TimeoutType::WAITING){ timeout = pdMS_TO_TICKS(timeoutMs); } diff --git a/osal/FreeRTOS/Mutex.h b/osal/FreeRTOS/Mutex.h index 37f1791c..2a4fae26 100644 --- a/osal/FreeRTOS/Mutex.h +++ b/osal/FreeRTOS/Mutex.h @@ -18,7 +18,8 @@ class Mutex : public MutexIF { public: Mutex(); ~Mutex(); - ReturnValue_t lockMutex(uint32_t timeoutMs = MutexIF::BLOCKING) override; + ReturnValue_t lockMutex(TimeoutType timeoutType, + uint32_t timeoutMs) override; ReturnValue_t unlockMutex() override; private: diff --git a/osal/linux/Mutex.cpp b/osal/linux/Mutex.cpp index bdcf3439..c99fe531 100644 --- a/osal/linux/Mutex.cpp +++ b/osal/linux/Mutex.cpp @@ -40,9 +40,13 @@ Mutex::~Mutex() { pthread_mutex_destroy(&mutex); } -ReturnValue_t Mutex::lockMutex(uint32_t timeoutMs) { +ReturnValue_t Mutex::lockMutex(TimeoutType timeoutType, uint32_t timeoutMs) { int status = 0; - if (timeoutMs != MutexIF::BLOCKING) { + + if(timeoutType == TimeoutType::POLLING) { + status = pthread_mutex_trylock(&mutex); + } + else if (timeoutType == TimeoutType::WAITING) { timespec timeOut; clock_gettime(CLOCK_REALTIME, &timeOut); uint64_t nseconds = timeOut.tv_sec * 1000000000 + timeOut.tv_nsec; @@ -50,9 +54,11 @@ ReturnValue_t Mutex::lockMutex(uint32_t timeoutMs) { timeOut.tv_sec = nseconds / 1000000000; timeOut.tv_nsec = nseconds - timeOut.tv_sec * 1000000000; status = pthread_mutex_timedlock(&mutex, &timeOut); - } else { + } + else if(timeoutType == TimeoutType::BLOCKING) { status = pthread_mutex_lock(&mutex); } + switch (status) { case EINVAL: //The mutex was created with the protocol attribute having the value PTHREAD_PRIO_PROTECT and the calling thread's priority is higher than the mutex's current priority ceiling. diff --git a/osal/linux/Mutex.h b/osal/linux/Mutex.h index 607337ae..26420c2e 100644 --- a/osal/linux/Mutex.h +++ b/osal/linux/Mutex.h @@ -12,7 +12,7 @@ class Mutex : public MutexIF { public: Mutex(); virtual ~Mutex(); - virtual ReturnValue_t lockMutex(uint32_t timeoutMs); + virtual ReturnValue_t lockMutex(TimeoutType timeoutType, uint32_t timeoutMs); virtual ReturnValue_t unlockMutex(); private: pthread_mutex_t mutex; diff --git a/subsystem/SubsystemBase.cpp b/subsystem/SubsystemBase.cpp index 51c35f1f..71fa81b7 100644 --- a/subsystem/SubsystemBase.cpp +++ b/subsystem/SubsystemBase.cpp @@ -76,14 +76,15 @@ ReturnValue_t SubsystemBase::checkStateAgainstTable( return RETURN_OK; } -void SubsystemBase::executeTable(HybridIterator tableIter, Submode_t targetSubmode) { +void SubsystemBase::executeTable(HybridIterator tableIter, + Submode_t targetSubmode) { CommandMessage command; std::map::iterator iter; commandsOutstanding = 0; - for (; tableIter.value != NULL; ++tableIter) { + for (; tableIter.value != nullptr; ++tableIter) { object_id_t object = tableIter.value->getObject(); if ((iter = childrenMap.find(object)) == childrenMap.end()) { //illegal table entry, should only happen due to misconfigured mode table From 37f2539d894b8f1e9b85fd29005032c9bd4ba12a Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Fri, 7 Aug 2020 20:44:40 +0200 Subject: [PATCH 27/34] empty line removed --- ipc/MutexIF.h | 1 - 1 file changed, 1 deletion(-) diff --git a/ipc/MutexIF.h b/ipc/MutexIF.h index b174fe0d..0d0cb444 100644 --- a/ipc/MutexIF.h +++ b/ipc/MutexIF.h @@ -5,7 +5,6 @@ /** * @brief Common interface for OS Mutex objects which provide MUTual EXclusion. - * * @details https://en.wikipedia.org/wiki/Lock_(computer_science) * @ingroup osal * @ingroup interface From e3d094b2dec8ddc838a0f70218b0200b8c1b1ce4 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Fri, 7 Aug 2020 22:02:23 +0200 Subject: [PATCH 28/34] linux: removed old static timeout values --- osal/linux/Mutex.cpp | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/osal/linux/Mutex.cpp b/osal/linux/Mutex.cpp index c99fe531..03789e62 100644 --- a/osal/linux/Mutex.cpp +++ b/osal/linux/Mutex.cpp @@ -2,8 +2,6 @@ #include #include -const uint32_t MutexIF::BLOCKING = 0xffffffff; -const uint32_t MutexIF::POLLING = 0; uint8_t Mutex::count = 0; @@ -61,22 +59,28 @@ ReturnValue_t Mutex::lockMutex(TimeoutType timeoutType, uint32_t timeoutMs) { switch (status) { case EINVAL: - //The mutex was created with the protocol attribute having the value PTHREAD_PRIO_PROTECT and the calling thread's priority is higher than the mutex's current priority ceiling. + // The mutex was created with the protocol attribute having the value + // PTHREAD_PRIO_PROTECT and the calling thread's priority is higher + // than the mutex's current priority ceiling. return WRONG_ATTRIBUTE_SETTING; - //The process or thread would have blocked, and the abs_timeout parameter specified a nanoseconds field value less than zero or greater than or equal to 1000 million. - //The value specified by mutex does not refer to an initialized mutex object. + // The process or thread would have blocked, and the abs_timeout + // parameter specified a nanoseconds field value less than zero or + // greater than or equal to 1000 million. + // The value specified by mutex does not refer to an initialized mutex object. //return MUTEX_NOT_FOUND; case EBUSY: - //The mutex could not be acquired because it was already locked. + // The mutex could not be acquired because it was already locked. return MUTEX_ALREADY_LOCKED; case ETIMEDOUT: - //The mutex could not be locked before the specified timeout expired. + // The mutex could not be locked before the specified timeout expired. return MUTEX_TIMEOUT; case EAGAIN: - //The mutex could not be acquired because the maximum number of recursive locks for mutex has been exceeded. + // The mutex could not be acquired because the maximum number of + // recursive locks for mutex has been exceeded. return MUTEX_MAX_LOCKS; case EDEADLK: - //A deadlock condition was detected or the current thread already owns the mutex. + // A deadlock condition was detected or the current thread + // already owns the mutex. return CURR_THREAD_ALREADY_OWNS_MUTEX; case 0: //Success From 44b70b45e216600bf5cfdca4899166f6bad0fd41 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Fri, 7 Aug 2020 22:06:20 +0200 Subject: [PATCH 29/34] little doc correction --- ipc/MutexIF.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ipc/MutexIF.h b/ipc/MutexIF.h index 0d0cb444..b0096a34 100644 --- a/ipc/MutexIF.h +++ b/ipc/MutexIF.h @@ -21,10 +21,10 @@ public: }; /** - * Lock the mutex. + * Lock the mutex. The timeout value will only be used for + * TimeoutType::WAITING * @param timeoutType * @param timeoutMs - * This value will only be used for TimeoutType::WAIT * @return */ virtual ReturnValue_t lockMutex(TimeoutType timeoutType = From 458e849f7d86ca95b0a3ae675d4d00895e26ede8 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Fri, 7 Aug 2020 22:11:13 +0200 Subject: [PATCH 30/34] adapting mutex helper to new interface --- ipc/MutexHelper.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ipc/MutexHelper.h b/ipc/MutexHelper.h index fc6e38bf..a8949737 100644 --- a/ipc/MutexHelper.h +++ b/ipc/MutexHelper.h @@ -6,9 +6,10 @@ class MutexHelper { public: - MutexHelper(MutexIF* mutex, uint32_t timeoutMs) : + MutexHelper(MutexIF* mutex, MutexIF::TimeoutType timeoutType = + MutexIF::TimeoutType::BLOCKING, uint32_t timeoutMs) : internalMutex(mutex) { - ReturnValue_t status = mutex->lockMutex(MutexIF::TimeoutType::WAITING, + ReturnValue_t status = mutex->lockMutex(timeoutType, timeoutMs); if(status == MutexIF::MUTEX_TIMEOUT) { sif::error << "MutexHelper: Lock of mutex failed with timeout of " From 979fea3400668661d817621766c64af1a62ed379 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Fri, 7 Aug 2020 22:15:03 +0200 Subject: [PATCH 31/34] mutex helper api change --- container/SharedRingBuffer.cpp | 14 +++++++------- datapoollocal/LocalPoolVariable.tpp | 6 ++++-- datapoollocal/LocalPoolVector.tpp | 6 ++++-- ipc/MutexHelper.h | 2 +- osal/linux/TmTcUnixUdpBridge.cpp | 2 +- storagemanager/PoolManager.tpp | 9 ++++++--- 6 files changed, 23 insertions(+), 16 deletions(-) diff --git a/container/SharedRingBuffer.cpp b/container/SharedRingBuffer.cpp index 2fde2851..a8b06b01 100644 --- a/container/SharedRingBuffer.cpp +++ b/container/SharedRingBuffer.cpp @@ -19,41 +19,41 @@ SharedRingBuffer::SharedRingBuffer(object_id_t objectId, uint8_t *buffer, ReturnValue_t SharedRingBuffer::getFreeElementProtected(uint8_t** writePtr, size_t amount) { - MutexHelper(mutex, mutexTimeout); + MutexHelper(mutex, MutexIF::TimeoutType::WAITING, mutexTimeout); return SimpleRingBuffer::getFreeElement(writePtr,amount); } ReturnValue_t SharedRingBuffer::writeDataProtected(const uint8_t *data, size_t amount) { - MutexHelper(mutex, mutexTimeout); + MutexHelper(mutex, MutexIF::TimeoutType::WAITING, mutexTimeout); return SimpleRingBuffer::writeData(data,amount); } ReturnValue_t SharedRingBuffer::readDataProtected(uint8_t *data, size_t amount, bool incrementReadPtr, bool readRemaining, size_t *trueAmount) { - MutexHelper(mutex, mutexTimeout); + MutexHelper(mutex, MutexIF::TimeoutType::WAITING, mutexTimeout); return SimpleRingBuffer::readData(data,amount, incrementReadPtr, readRemaining, trueAmount); } ReturnValue_t SharedRingBuffer::deleteDataProtected(size_t amount, bool deleteRemaining, size_t *trueAmount) { - MutexHelper(mutex, mutexTimeout); + MutexHelper(mutex, MutexIF::TimeoutType::WAITING, mutexTimeout); return SimpleRingBuffer::deleteData(amount, deleteRemaining, trueAmount); } size_t SharedRingBuffer::getExcessBytes() const { - MutexHelper(mutex, mutexTimeout); + MutexHelper(mutex, MutexIF::TimeoutType::WAITING, mutexTimeout); return SimpleRingBuffer::getExcessBytes(); } void SharedRingBuffer::moveExcessBytesToStart() { - MutexHelper(mutex, mutexTimeout); + MutexHelper(mutex, MutexIF::TimeoutType::WAITING, mutexTimeout); return SimpleRingBuffer::moveExcessBytesToStart(); } size_t SharedRingBuffer::getAvailableReadDataProtected(uint8_t n) const { - MutexHelper(mutex, mutexTimeout); + MutexHelper(mutex, MutexIF::TimeoutType::WAITING, mutexTimeout); return ((write + size) - read[n]) % size; } diff --git a/datapoollocal/LocalPoolVariable.tpp b/datapoollocal/LocalPoolVariable.tpp index 0b96bb4f..b4e09f2d 100644 --- a/datapoollocal/LocalPoolVariable.tpp +++ b/datapoollocal/LocalPoolVariable.tpp @@ -48,7 +48,8 @@ inline LocalPoolVar::LocalPoolVar(lp_id_t poolId, object_id_t poolOwner, template inline ReturnValue_t LocalPoolVar::read(dur_millis_t lockTimeout) { - MutexHelper(hkManager->getMutexHandle(), lockTimeout); + MutexHelper(hkManager->getMutexHandle(), MutexIF::TimeoutType::WAITING, + lockTimeout); return readWithoutLock(); } @@ -76,7 +77,8 @@ inline ReturnValue_t LocalPoolVar::readWithoutLock() { template inline ReturnValue_t LocalPoolVar::commit(dur_millis_t lockTimeout) { - MutexHelper(hkManager->getMutexHandle(), lockTimeout); + MutexHelper(hkManager->getMutexHandle(), MutexIF::TimeoutType::WAITING, + lockTimeout); return commitWithoutLock(); } diff --git a/datapoollocal/LocalPoolVector.tpp b/datapoollocal/LocalPoolVector.tpp index b7f2fe19..7c3d2c1e 100644 --- a/datapoollocal/LocalPoolVector.tpp +++ b/datapoollocal/LocalPoolVector.tpp @@ -44,7 +44,8 @@ inline LocalPoolVector::LocalPoolVector(lp_id_t poolId, template inline ReturnValue_t LocalPoolVector::read(uint32_t lockTimeout) { - MutexHelper(hkManager->getMutexHandle(), lockTimeout); + MutexHelper(hkManager->getMutexHandle(), MutexIF::TimeoutType::WAITING, + lockTimeout); return readWithoutLock(); } template @@ -74,7 +75,8 @@ inline ReturnValue_t LocalPoolVector::readWithoutLock() { template inline ReturnValue_t LocalPoolVector::commit( uint32_t lockTimeout) { - MutexHelper(hkManager->getMutexHandle(), lockTimeout); + MutexHelper(hkManager->getMutexHandle(), MutexIF::TimeoutType::WAITING, + lockTimeout); return commitWithoutLock(); } diff --git a/ipc/MutexHelper.h b/ipc/MutexHelper.h index a8949737..52ce51e3 100644 --- a/ipc/MutexHelper.h +++ b/ipc/MutexHelper.h @@ -7,7 +7,7 @@ class MutexHelper { public: MutexHelper(MutexIF* mutex, MutexIF::TimeoutType timeoutType = - MutexIF::TimeoutType::BLOCKING, uint32_t timeoutMs) : + MutexIF::TimeoutType::BLOCKING, uint32_t timeoutMs = 0) : internalMutex(mutex) { ReturnValue_t status = mutex->lockMutex(timeoutType, timeoutMs); diff --git a/osal/linux/TmTcUnixUdpBridge.cpp b/osal/linux/TmTcUnixUdpBridge.cpp index 92fbbdfa..2cdb93f3 100644 --- a/osal/linux/TmTcUnixUdpBridge.cpp +++ b/osal/linux/TmTcUnixUdpBridge.cpp @@ -85,7 +85,7 @@ ReturnValue_t TmTcUnixUdpBridge::sendTm(const uint8_t *data, size_t dataLen) { } void TmTcUnixUdpBridge::checkAndSetClientAddress(sockaddr_in newAddress) { - MutexHelper lock(mutex, 10); + MutexHelper lock(mutex, MutexIF::TimeoutType::WAITING, 10); // char ipAddress [15]; // sif::debug << "IP Address Sender: "<< inet_ntop(AF_INET, diff --git a/storagemanager/PoolManager.tpp b/storagemanager/PoolManager.tpp index 4ca22b85..0656e967 100644 --- a/storagemanager/PoolManager.tpp +++ b/storagemanager/PoolManager.tpp @@ -21,7 +21,8 @@ inline PoolManager::~PoolManager(void) { template inline ReturnValue_t PoolManager::reserveSpace( const uint32_t size, store_address_t* address, bool ignoreFault) { - MutexHelper mutexHelper(mutex, mutexTimeout); + MutexHelper mutexHelper(mutex, MutexIF::TimeoutType::WAITING, + mutexTimeout); ReturnValue_t status = LocalPool::reserveSpace(size, address,ignoreFault); return status; @@ -33,7 +34,8 @@ inline ReturnValue_t PoolManager::deleteData( // debug << "PoolManager( " << translateObject(getObjectId()) << // " )::deleteData from store " << packet_id.pool_index << // ". id is "<< packet_id.packet_index << std::endl; - MutexHelper mutexHelper(mutex, mutexTimeout); + MutexHelper mutexHelper(mutex, MutexIF::TimeoutType::WAITING, + mutexTimeout); ReturnValue_t status = LocalPool::deleteData(packet_id); return status; } @@ -41,7 +43,8 @@ inline ReturnValue_t PoolManager::deleteData( template inline ReturnValue_t PoolManager::deleteData(uint8_t* buffer, size_t size, store_address_t* storeId) { - MutexHelper mutexHelper(mutex, mutexTimeout); + MutexHelper mutexHelper(mutex, MutexIF::TimeoutType::WAITING, + mutexTimeout); ReturnValue_t status = LocalPool::deleteData(buffer, size, storeId); return status; From af11c8fcf96c242447baeffbbb9731c9807a6f8e Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Sat, 8 Aug 2020 12:38:39 +0200 Subject: [PATCH 32/34] freeRTOS corrections --- osal/FreeRTOS/FixedTimeslotTask.cpp | 5 +++-- osal/FreeRTOS/FixedTimeslotTask.h | 2 +- osal/FreeRTOS/PeriodicTask.cpp | 7 ++++--- osal/FreeRTOS/PeriodicTask.h | 4 ++-- osal/FreeRTOS/TaskFactory.cpp | 8 ++++---- tasks/Typedef.h | 1 - 6 files changed, 14 insertions(+), 13 deletions(-) diff --git a/osal/FreeRTOS/FixedTimeslotTask.cpp b/osal/FreeRTOS/FixedTimeslotTask.cpp index 1dc45b1d..2a7bff9c 100644 --- a/osal/FreeRTOS/FixedTimeslotTask.cpp +++ b/osal/FreeRTOS/FixedTimeslotTask.cpp @@ -5,11 +5,12 @@ uint32_t FixedTimeslotTask::deadlineMissedCount = 0; const size_t PeriodicTaskIF::MINIMUM_STACK_SIZE = configMINIMAL_STACK_SIZE; -FixedTimeslotTask::FixedTimeslotTask(const char *name, TaskPriority setPriority, +FixedTimeslotTask::FixedTimeslotTask(TaskName name, TaskPriority setPriority, TaskStackSize setStack, TaskPeriod overallPeriod, void (*setDeadlineMissedFunc)()) : started(false), handle(NULL), pst(overallPeriod * 1000) { - xTaskCreate(taskEntryPoint, name, setStack, this, setPriority, &handle); + configSTACK_DEPTH_TYPE stackSize = setStack / sizeof(configSTACK_DEPTH_TYPE); + xTaskCreate(taskEntryPoint, name, stackSize, this, setPriority, &handle); // All additional attributes are applied to the object. this->deadlineMissedFunc = setDeadlineMissedFunc; } diff --git a/osal/FreeRTOS/FixedTimeslotTask.h b/osal/FreeRTOS/FixedTimeslotTask.h index b63cb775..36950773 100644 --- a/osal/FreeRTOS/FixedTimeslotTask.h +++ b/osal/FreeRTOS/FixedTimeslotTask.h @@ -24,7 +24,7 @@ public: * @param setDeadlineMissedFunc Callback if a deadline was missed. * @return Pointer to the newly created task. */ - FixedTimeslotTask(const char *name, TaskPriority setPriority, + FixedTimeslotTask(TaskName name, TaskPriority setPriority, TaskStackSize setStack, TaskPeriod overallPeriod, void (*setDeadlineMissedFunc)()); diff --git a/osal/FreeRTOS/PeriodicTask.cpp b/osal/FreeRTOS/PeriodicTask.cpp index f2dcd093..c316d4fc 100644 --- a/osal/FreeRTOS/PeriodicTask.cpp +++ b/osal/FreeRTOS/PeriodicTask.cpp @@ -5,12 +5,13 @@ PeriodicTask::PeriodicTask(const char *name, TaskPriority setPriority, TaskStackSize setStack, TaskPeriod setPeriod, - void (*setDeadlineMissedFunc)()) : + TaskDeadlineMissedFunction deadlineMissedFunc) : started(false), handle(NULL), period(setPeriod), deadlineMissedFunc( - setDeadlineMissedFunc) + deadlineMissedFunc) { + configSTACK_DEPTH_TYPE stackSize = setStack / sizeof(configSTACK_DEPTH_TYPE); BaseType_t status = xTaskCreate(taskEntryPoint, name, - setStack, this, setPriority, &handle); + stackSize, this, setPriority, &handle); if(status != pdPASS){ sif::debug << "PeriodicTask Insufficient heap memory remaining. " "Status: " << status << std::endl; diff --git a/osal/FreeRTOS/PeriodicTask.h b/osal/FreeRTOS/PeriodicTask.h index 2be2521c..8030baad 100644 --- a/osal/FreeRTOS/PeriodicTask.h +++ b/osal/FreeRTOS/PeriodicTask.h @@ -40,9 +40,9 @@ public: * The function pointer to the deadline missed function that shall * be assigned. */ - PeriodicTask(const char *name, TaskPriority setPriority, + PeriodicTask(TaskName name, TaskPriority setPriority, TaskStackSize setStack, TaskPeriod setPeriod, - void (*setDeadlineMissedFunc)()); + TaskDeadlineMissedFunction deadlineMissedFunc); /** * @brief Currently, the executed object's lifetime is not coupled with * the task object's lifetime, so the destructor is empty. diff --git a/osal/FreeRTOS/TaskFactory.cpp b/osal/FreeRTOS/TaskFactory.cpp index 663b0531..68866f39 100644 --- a/osal/FreeRTOS/TaskFactory.cpp +++ b/osal/FreeRTOS/TaskFactory.cpp @@ -18,8 +18,8 @@ PeriodicTaskIF* TaskFactory::createPeriodicTask(TaskName name_, TaskPriority taskPriority_, TaskStackSize stackSize_, TaskPeriod period_, TaskDeadlineMissedFunction deadLineMissedFunction_) { - return (PeriodicTaskIF*) (new PeriodicTask(name_, taskPriority_, stackSize_, - period_, deadLineMissedFunction_)); + return dynamic_cast(new PeriodicTask(name_, taskPriority_, + stackSize_, period_, deadLineMissedFunction_)); } /** @@ -29,8 +29,8 @@ FixedTimeslotTaskIF* TaskFactory::createFixedTimeslotTask(TaskName name_, TaskPriority taskPriority_, TaskStackSize stackSize_, TaskPeriod period_, TaskDeadlineMissedFunction deadLineMissedFunction_) { - return (FixedTimeslotTaskIF*) (new FixedTimeslotTask(name_, taskPriority_, - stackSize_, period_, deadLineMissedFunction_)); + return dynamic_cast(new FixedTimeslotTask(name_, + taskPriority_,stackSize_, period_, deadLineMissedFunction_)); } ReturnValue_t TaskFactory::deleteTask(PeriodicTaskIF* task) { diff --git a/tasks/Typedef.h b/tasks/Typedef.h index b393ee1e..07d96b00 100644 --- a/tasks/Typedef.h +++ b/tasks/Typedef.h @@ -1,7 +1,6 @@ #ifndef FRAMEWORK_TASKS_TYPEDEF_H_ #define FRAMEWORK_TASKS_TYPEDEF_H_ -//TODO more generic? typedef const char* TaskName; typedef uint8_t TaskPriority; typedef size_t TaskStackSize; From 5c85d03c39af4e9ec31a03e238dd1994b02ea68a Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Sat, 8 Aug 2020 13:04:31 +0200 Subject: [PATCH 33/34] additional check for freeRTOS --- osal/FreeRTOS/MessageQueue.cpp | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/osal/FreeRTOS/MessageQueue.cpp b/osal/FreeRTOS/MessageQueue.cpp index 8eb669ed..e9f3871e 100644 --- a/osal/FreeRTOS/MessageQueue.cpp +++ b/osal/FreeRTOS/MessageQueue.cpp @@ -123,11 +123,20 @@ bool MessageQueue::isDefaultDestinationSet() const { ReturnValue_t MessageQueue::sendMessageFromMessageQueue(MessageQueueId_t sendTo, MessageQueueMessageIF* message, MessageQueueId_t sentFrom, bool ignoreFault, CallContext callContext) { + BaseType_t result = pdFALSE; + QueueHandle_t destination = nullptr; + + if(sendTo == MessageQueueIF::NO_QUEUE) { + return MessageQueueIF::DESTINVATION_INVALID; + } + else { + destination = reinterpret_cast(sendTo); + } message->setSender(sentFrom); - BaseType_t result; + if(callContext == CallContext::TASK) { - result = xQueueSendToBack(reinterpret_cast(sendTo), + result = xQueueSendToBack(destination, static_cast(message->getBuffer()), 0); } else { From 03cd5780f9b6f915566c07b07482a1b86b103eed Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Sat, 8 Aug 2020 13:29:38 +0200 Subject: [PATCH 34/34] null initializations in header --- fdir/FailureIsolationBase.cpp | 4 ++-- fdir/FailureIsolationBase.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/fdir/FailureIsolationBase.cpp b/fdir/FailureIsolationBase.cpp index 4df56dcd..ad4493ff 100644 --- a/fdir/FailureIsolationBase.cpp +++ b/fdir/FailureIsolationBase.cpp @@ -7,8 +7,8 @@ FailureIsolationBase::FailureIsolationBase(object_id_t owner, object_id_t parent, uint8_t messageDepth, uint8_t parameterDomainBase) : - eventQueue(NULL), ownerId(owner), owner(NULL), - faultTreeParent(parent), parameterDomainBase(parameterDomainBase) { + ownerId(owner), faultTreeParent(parent), + parameterDomainBase(parameterDomainBase) { eventQueue = QueueFactory::instance()->createMessageQueue(messageDepth, EventMessage::EVENT_MESSAGE_SIZE); } diff --git a/fdir/FailureIsolationBase.h b/fdir/FailureIsolationBase.h index 6010baca..c0fe6041 100644 --- a/fdir/FailureIsolationBase.h +++ b/fdir/FailureIsolationBase.h @@ -33,9 +33,9 @@ public: virtual void triggerEvent(Event event, uint32_t parameter1 = 0, uint32_t parameter2 = 0); protected: - MessageQueueIF* eventQueue; + MessageQueueIF* eventQueue = nullptr; object_id_t ownerId; - HasHealthIF* owner; + HasHealthIF* owner = nullptr; object_id_t faultTreeParent; uint8_t parameterDomainBase; void setOwnerHealth(HasHealthIF::HealthState health);