From 47b588c429275b19836feaff1f476fa4c103c219 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Sat, 18 Jan 2020 22:53:38 +0100 Subject: [PATCH 01/21] PusServiceBase bufix + doc Bugfix for error parameter 2 (wrong type). Some formatting and doc improvements --- tmtcservices/PusServiceBase.cpp | 10 +++++----- tmtcservices/PusServiceBase.h | 22 +++++++++++++++------- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/tmtcservices/PusServiceBase.cpp b/tmtcservices/PusServiceBase.cpp index c8f6accf..6e105325 100644 --- a/tmtcservices/PusServiceBase.cpp +++ b/tmtcservices/PusServiceBase.cpp @@ -23,14 +23,14 @@ ReturnValue_t PusServiceBase::performOperation(uint8_t opCode) { TmTcMessage message; for (uint8_t count = 0; count < PUS_SERVICE_MAX_RECEPTION; count++) { ReturnValue_t status = this->requestQueue->receiveMessage(&message); - // debug << "PusServiceBase::performOperation: Receiving from MQ ID: " << std::hex << this->requestQueue.getId() << std::dec << " returned: " << status << std::endl; + // debug << "PusServiceBase::performOperation: Receiving from MQ ID: " << std::hex << this->requestQueue.getId() + // << std::dec << " returned: " << status << std::endl; if (status == RETURN_OK) { this->currentPacket.setStoreAddress(message.getStorageId()); -// info << "Service " << (uint16_t) this->serviceId << ": new packet!" -// << std::endl; + // info << "Service " << (uint16_t) this->serviceId << ": new packet!" << std::endl; ReturnValue_t return_code = this->handleRequest(); - // debug << "Service " << (uint16_t)this->serviceId << ": handleRequest returned: " << (int)return_code << std::endl; + // debug << "Service " << (uint16_t)this->serviceId << ": handleRequest returned: " << (int)return_code << std::endl; if (return_code == RETURN_OK) { this->verifyReporter.sendSuccessReport( TC_VERIFY::COMPLETION_SUCCESS, &this->currentPacket); @@ -44,7 +44,7 @@ ReturnValue_t PusServiceBase::performOperation(uint8_t opCode) { errorParameter2 = 0; } else if (status == MessageQueueIF::EMPTY) { status = RETURN_OK; - // debug << "PusService " << (uint16_t)this->serviceId << ": no new packet." << std::endl; + // debug << "PusService " << (uint16_t)this->serviceId << ": no new packet." << std::endl; break; } else { diff --git a/tmtcservices/PusServiceBase.h b/tmtcservices/PusServiceBase.h index 782d375f..6c8c5f61 100644 --- a/tmtcservices/PusServiceBase.h +++ b/tmtcservices/PusServiceBase.h @@ -46,13 +46,20 @@ public: */ virtual ~PusServiceBase(); /** - * The handleRequest method shall handle any kind of Telecommand Request immediately. + * @brief The handleRequest method shall handle any kind of Telecommand Request immediately. + * @details * Implemetations can take the Telecommand in currentPacket and perform any kind of operation. * They may send additional "Start Success (1,3)" messages with the verifyReporter, but Completion * Success or Failure Reports are generated automatically after execution of this method. - * If a Telecommand can not be executed within one call cycle, this Base class is not the right parent. - * The child class may add additional error information in #errorParameters which are attached to the generated - * verification message. + * + * If a Telecommand can not be executed within one call cycle, + * this Base class is not the right parent. + * + * The child class may add additional error information by setting #errorParameters which are + * attached to the generated verification message. + * + * Subservice checking should be implemented in this method. + * * @return The returned status_code is directly taken as main error code in the Verification Report. * On success, RETURN_OK shall be returned. */ @@ -68,8 +75,8 @@ public: * It checks for new requests, and, if found, calls handleRequest, sends completion verification messages and deletes * the TC requests afterwards. * performService is always executed afterwards. - * @return - \c RETURN_OK if the periodic performService was successful. - * - \c RETURN_FAILED else. + * @return \c RETURN_OK if the periodic performService was successful. + * \c RETURN_FAILED else. */ ReturnValue_t performOperation(uint8_t opCode); virtual uint16_t getIdentifier(); @@ -91,7 +98,8 @@ protected: /** * One of two error parameters for additional error information. */ - uint8_t errorParameter2; + // shouldn't this be uint32_t ? The PUS Verification Message structure param2 has the size 4 + uint32_t errorParameter2; /** * This is a complete instance of the Telecommand reception queue of the class. * It is initialized on construction of the class. From 782ba143a68c85f2317219b04a5d2b134c1bf011 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 20 Jan 2020 22:03:13 +0100 Subject: [PATCH 02/21] FixedTimeslotTask object check Before adding to PST. Prevents NULL exception. --- osal/FreeRTOS/FixedTimeslotTask.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/osal/FreeRTOS/FixedTimeslotTask.cpp b/osal/FreeRTOS/FixedTimeslotTask.cpp index 00fd73d3..4a30774a 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,6 +57,11 @@ ReturnValue_t FixedTimeslotTask::startTask() { ReturnValue_t FixedTimeslotTask::addSlot(object_id_t componentId, uint32_t slotTimeMs, int8_t executionStep) { + if (!objectManager->get(componentId)) { + 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; } From e7318995f49f1b22ac8ee2411954f9fe44864841 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 3 Feb 2020 18:03:10 +0100 Subject: [PATCH 03/21] FIFO typo fix --- container/FIFO.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/container/FIFO.h b/container/FIFO.h index 670a50d7..134da9b8 100644 --- a/container/FIFO.h +++ b/container/FIFO.h @@ -21,7 +21,7 @@ public: readIndex(0), writeIndex(0), currentSize(0) { } - bool emtpy() { + bool empty() { return (currentSize == 0); } @@ -45,7 +45,7 @@ public: } ReturnValue_t retrieve(T *value) { - if (emtpy()) { + if (empty()) { return EMPTY; } else { *value = data[readIndex]; From 11861c0dbc7595a57f7812603e6938b94bde895d Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 3 Feb 2020 22:27:44 +0100 Subject: [PATCH 04/21] Moved CSB abstract functions to top Interface functions closer at top --- tmtcservices/CommandingServiceBase.h | 79 ++++++++++++++++++++++------ 1 file changed, 63 insertions(+), 16 deletions(-) diff --git a/tmtcservices/CommandingServiceBase.h b/tmtcservices/CommandingServiceBase.h index ee59ffe4..a62b7397 100644 --- a/tmtcservices/CommandingServiceBase.h +++ b/tmtcservices/CommandingServiceBase.h @@ -64,7 +64,7 @@ public: virtual ~CommandingServiceBase(); /*** - * This is the periodic called function + * This is the periodically called function. * Handle request queue for external commands. * Handle command Queue for internal commands. * @param opCode is unused here at the moment @@ -104,6 +104,66 @@ public: }; protected: + /** + * Check the target subservice + * @param subservice + * @return + */ + virtual ReturnValue_t isValidSubservice(uint8_t subservice) = 0; + + /** + * Once a TC Request is valid, the existence of the destination and its target interface is checked and retrieved. + * The target message queue ID can then be acquired by using the target interface. + * @param subservice + * @param tcData Application Data of TC Packet + * @param tcDataLen + * @param id MessageQueue ID is stored here + * @param objectId Object ID is extracted and stored here + * @return - @c RETURN_OK on success + * - @c RETURN_FAILED + * - @c CSB or implementation specific return codes + */ + virtual ReturnValue_t getMessageQueueAndObject(uint8_t subservice, + const uint8_t *tcData, uint32_t tcDataLen, MessageQueueId_t *id, + object_id_t *objectId) = 0; + + /** + * After the Message Queue and Object ID are determined, + * the command is prepared by using an implementation specific CommandMessage type + * which is sent to the target object. + * It contains all necessary information for the device to execute telecommands. + * @param message + * @param subservice + * @param tcData + * @param tcDataLen + * @param state + * @param objectId Target object ID + * @return + */ + virtual ReturnValue_t prepareCommand(CommandMessage *message, + uint8_t subservice, const uint8_t *tcData, uint32_t tcDataLen, + uint32_t *state, object_id_t objectId) = 0; + + /** + * This function is responsible for the communication between the Command Service Base + * and the respective PUS Commanding Service once the execution has started. + * The PUS Commanding Service receives replies from the target device and forwards them by calling this function. + * There are different translations of these replies to specify how the Command Service proceeds. + * @param reply[out] Command Message which contains information about the command + * @param previousCommand [out] + * @param state + * @param optionalNextCommand + * @param objectId Source object ID + * @param isStep Flag value to mark steps of command execution + * @return - @c RETURN_OK, @c EXECUTION_COMPLETE or @c NO_STEP_MESSAGE to generate TC verification success + * - @c INVALID_REPLY can handle unrequested replies + * - Anything else triggers a TC verification failure + */ + virtual ReturnValue_t handleReply(const CommandMessage *reply, + Command_t previousCommand, uint32_t *state, + CommandMessage *optionalNextCommand, object_id_t objectId, + bool *isStep) = 0; + struct CommandInfo { struct tcInfo { uint8_t ackFlags; @@ -180,20 +240,6 @@ protected: */ void sendTmPacket(uint8_t subservice, SerializeIF* content, SerializeIF* header = NULL); - virtual ReturnValue_t isValidSubservice(uint8_t subservice) = 0; - - virtual ReturnValue_t prepareCommand(CommandMessage *message, - uint8_t subservice, const uint8_t *tcData, uint32_t tcDataLen, - uint32_t *state, object_id_t objectId) = 0; - - virtual ReturnValue_t handleReply(const CommandMessage *reply, - Command_t previousCommand, uint32_t *state, - CommandMessage *optionalNextCommand, object_id_t objectId, - bool *isStep) = 0; - - virtual ReturnValue_t getMessageQueueAndObject(uint8_t subservice, - const uint8_t *tcData, uint32_t tcDataLen, MessageQueueId_t *id, - object_id_t *objectId) = 0; virtual void handleUnrequestedReply(CommandMessage *reply); @@ -209,7 +255,8 @@ private: * It handles replies generated by the devices and relayed by the specific service implementation. * This means that it determines further course of action depending on the return values specified * in the service implementation. - * This includes the generation of TC verification messages: + * This includes the generation of TC verification messages. Note that + * the static framework object ID @c VerificationReporter::messageReceiver needs to be set. * - TM[1,5] Step Successs * - TM[1,6] Step Failure * - TM[1,7] Completion Success From c98f19b4c08873df8aa92d1c39d52f49a59dafbc Mon Sep 17 00:00:00 2001 From: gaisser Date: Tue, 11 Feb 2020 15:17:03 +0100 Subject: [PATCH 05/21] Update 'tmtcservices/PusServiceBase.h' Removed comment. --- tmtcservices/PusServiceBase.h | 1 - 1 file changed, 1 deletion(-) diff --git a/tmtcservices/PusServiceBase.h b/tmtcservices/PusServiceBase.h index 6c8c5f61..d318ff1e 100644 --- a/tmtcservices/PusServiceBase.h +++ b/tmtcservices/PusServiceBase.h @@ -98,7 +98,6 @@ protected: /** * One of two error parameters for additional error information. */ - // shouldn't this be uint32_t ? The PUS Verification Message structure param2 has the size 4 uint32_t errorParameter2; /** * This is a complete instance of the Telecommand reception queue of the class. From 3f2f76b8cf65089b9f1e0510d72f278c0d29ecfe Mon Sep 17 00:00:00 2001 From: gaisser Date: Tue, 11 Feb 2020 15:38:12 +0100 Subject: [PATCH 06/21] Update 'tmtcservices/CommandingServiceBase.h' Added a few comments --- tmtcservices/CommandingServiceBase.h | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/tmtcservices/CommandingServiceBase.h b/tmtcservices/CommandingServiceBase.h index a62b7397..bf2e8242 100644 --- a/tmtcservices/CommandingServiceBase.h +++ b/tmtcservices/CommandingServiceBase.h @@ -106,8 +106,9 @@ public: protected: /** * Check the target subservice - * @param subservice - * @return + * @param subservice[in] + * @return -@c RETURN_OK on success + * -@c INVALID_SUBSERVICE if service is not known */ virtual ReturnValue_t isValidSubservice(uint8_t subservice) = 0; @@ -132,13 +133,15 @@ protected: * the command is prepared by using an implementation specific CommandMessage type * which is sent to the target object. * It contains all necessary information for the device to execute telecommands. - * @param message - * @param subservice - * @param tcData - * @param tcDataLen - * @param state + * @param message[out] message to be sent to the object + * @param subservice[in] Subservice of the current communication + * @param tcData Additional data of the command + * @param tcDataLen Length of the additional data + * @param state[out] Setable state of the communication * @param objectId Target object ID - * @return + * @return - @c RETURN_OK on success + * - @c EXECUTION_COMPLETE if exectuin is finished + * - any other return code will be part of (1,4) start failure */ virtual ReturnValue_t prepareCommand(CommandMessage *message, uint8_t subservice, const uint8_t *tcData, uint32_t tcDataLen, @@ -149,10 +152,10 @@ protected: * and the respective PUS Commanding Service once the execution has started. * The PUS Commanding Service receives replies from the target device and forwards them by calling this function. * There are different translations of these replies to specify how the Command Service proceeds. - * @param reply[out] Command Message which contains information about the command - * @param previousCommand [out] - * @param state - * @param optionalNextCommand + * @param reply Command Message which contains information about the command + * @param previousCommand Command_t of last command + * @param state state of the communication + * @param optionalNextCommand[out] An optional next command which can be set in this function * @param objectId Source object ID * @param isStep Flag value to mark steps of command execution * @return - @c RETURN_OK, @c EXECUTION_COMPLETE or @c NO_STEP_MESSAGE to generate TC verification success From 413d65938194e945dc7e45038a0c37ef564c5f00 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sat, 15 Feb 2020 15:27:06 +0100 Subject: [PATCH 07/21] slight change --- osal/FreeRTOS/FixedTimeslotTask.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/osal/FreeRTOS/FixedTimeslotTask.cpp b/osal/FreeRTOS/FixedTimeslotTask.cpp index 4a30774a..413d7596 100644 --- a/osal/FreeRTOS/FixedTimeslotTask.cpp +++ b/osal/FreeRTOS/FixedTimeslotTask.cpp @@ -57,13 +57,13 @@ ReturnValue_t FixedTimeslotTask::startTask() { ReturnValue_t FixedTimeslotTask::addSlot(object_id_t componentId, uint32_t slotTimeMs, int8_t executionStep) { - if (!objectManager->get(componentId)) { - error << "Component " << std::hex << componentId << " not found, not adding it to pst" << std::endl; - return HasReturnvaluesIF::RETURN_FAILED; + if (objectManager->get(componentId) != NULL) { + pst.addSlot(componentId, slotTimeMs, executionStep, this); + return HasReturnvaluesIF::RETURN_OK; } - 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 { From a5692079c643fc0c8f32877f835a42a78d7c41a2 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sat, 15 Feb 2020 15:37:00 +0100 Subject: [PATCH 08/21] Small possilbe bugfix in MessageQueue.cpp And other small changes. Only assign last partner if message receiving was successful. Some formatting stuff, include in <> notation doc for task factory free RTOS, high priority means high number --- osal/FreeRTOS/MessageQueue.cpp | 25 ++++++++++++++----------- osal/FreeRTOS/QueueFactory.cpp | 2 +- osal/FreeRTOS/TaskFactory.cpp | 1 + 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/osal/FreeRTOS/MessageQueue.cpp b/osal/FreeRTOS/MessageQueue.cpp index f3aebcd1..887df392 100644 --- a/osal/FreeRTOS/MessageQueue.cpp +++ b/osal/FreeRTOS/MessageQueue.cpp @@ -38,7 +38,9 @@ ReturnValue_t MessageQueue::reply(MessageQueueMessage* message) { ReturnValue_t MessageQueue::receiveMessage(MessageQueueMessage* message, MessageQueueId_t* receivedFrom) { ReturnValue_t status = this->receiveMessage(message); - *receivedFrom = this->lastPartner; + if(status == HasReturnvaluesIF::RETURN_OK) { + *receivedFrom = this->lastPartner; + } return status; } @@ -89,23 +91,24 @@ MessageQueueId_t MessageQueue::getDefaultDestination() const { bool MessageQueue::isDefaultDestinationSet() const { return 0; } + ReturnValue_t MessageQueue::sendMessageFromMessageQueue(MessageQueueId_t sendTo, MessageQueueMessage *message, MessageQueueId_t sentFrom, bool ignoreFault) { message->setSender(sentFrom); - BaseType_t result = xQueueSendToBack(reinterpret_cast(sendTo),reinterpret_cast(message->getBuffer()), 0); - if (result != pdPASS) { - if (!ignoreFault) { - InternalErrorReporterIF* internalErrorReporter = objectManager->get( - objects::INTERNAL_ERROR_REPORTER); - if (internalErrorReporter != NULL) { - internalErrorReporter->queueMessageNotSent(); - } + BaseType_t result = xQueueSendToBack(reinterpret_cast(sendTo),reinterpret_cast(message->getBuffer()), 0); + if (result != pdPASS) { + if (!ignoreFault) { + InternalErrorReporterIF* internalErrorReporter = objectManager->get( + objects::INTERNAL_ERROR_REPORTER); + if (internalErrorReporter != NULL) { + internalErrorReporter->queueMessageNotSent(); } - return MessageQueueIF::FULL; } - return HasReturnvaluesIF::RETURN_OK; + return MessageQueueIF::FULL; + } + return HasReturnvaluesIF::RETURN_OK; } diff --git a/osal/FreeRTOS/QueueFactory.cpp b/osal/FreeRTOS/QueueFactory.cpp index 5c7087de..eaf245d3 100644 --- a/osal/FreeRTOS/QueueFactory.cpp +++ b/osal/FreeRTOS/QueueFactory.cpp @@ -1,6 +1,6 @@ #include -#include "../FreeRTOS/MessageQueue.h" +#include QueueFactory* QueueFactory::factoryInstance = NULL; diff --git a/osal/FreeRTOS/TaskFactory.cpp b/osal/FreeRTOS/TaskFactory.cpp index 7b2f70a3..753da60f 100644 --- a/osal/FreeRTOS/TaskFactory.cpp +++ b/osal/FreeRTOS/TaskFactory.cpp @@ -15,6 +15,7 @@ TaskFactory* TaskFactory::instance() { } /*** * Keep in Mind that you need to call before this vTaskStartScheduler()! + * High taskPriority_ number means high priority. */ PeriodicTaskIF* TaskFactory::createPeriodicTask(TaskName name_, TaskPriority taskPriority_, TaskStackSize stackSize_, From 90cba58dedae1d9cc65f5add9046e25f41e91311 Mon Sep 17 00:00:00 2001 From: Ulrich Mohr Date: Mon, 6 Apr 2020 12:50:57 +0200 Subject: [PATCH 09/21] Putting the Cyclic back into CRC added a parameter to the crc function to supply it with a starting value for the crc, so one can calculate a crc over mutiple separate parts. --- globalfunctions/crc_ccitt.cpp | 9 ++++----- globalfunctions/crc_ccitt.h | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/globalfunctions/crc_ccitt.cpp b/globalfunctions/crc_ccitt.cpp index 2fbebd07..74a93465 100644 --- a/globalfunctions/crc_ccitt.cpp +++ b/globalfunctions/crc_ccitt.cpp @@ -38,19 +38,18 @@ static const uint16_t crc_table[256] = { // CRC implementation -uint16_t Calculate_CRC(uint8_t const input[], uint32_t length) +uint16_t Calculate_CRC(uint8_t const input[], uint32_t length, uint16_t startingCrc) { - uint16_t crc = 0xFFFF; uint8_t *data = (uint8_t *)input; unsigned int tbl_idx; while (length--) { - tbl_idx = ((crc >> 8) ^ *data) & 0xff; - crc = (crc_table[tbl_idx] ^ (crc << 8)) & 0xffff; + tbl_idx = ((startingCrc >> 8) ^ *data) & 0xff; + startingCrc = (crc_table[tbl_idx] ^ (startingCrc << 8)) & 0xffff; data++; } - return crc & 0xffff; + return startingCrc & 0xffff; //The part below is not used! // bool temr[16]; diff --git a/globalfunctions/crc_ccitt.h b/globalfunctions/crc_ccitt.h index 6310f184..60077d0b 100644 --- a/globalfunctions/crc_ccitt.h +++ b/globalfunctions/crc_ccitt.h @@ -3,7 +3,7 @@ #include -uint16_t Calculate_CRC(uint8_t const input[], uint32_t length); +uint16_t Calculate_CRC(uint8_t const input[], uint32_t length, uint16_t startingCrc = 0xffff); #endif /* CRC_H_ */ From f28886e970e13fd20c65d852c4729473a71c4c99 Mon Sep 17 00:00:00 2001 From: Ulrich Mohr Date: Mon, 6 Apr 2020 13:22:42 +0200 Subject: [PATCH 10/21] Moved crc calculation into its own class, renamed function to show which crc is calculated. --- container/IndexedRingMemoryArray.h | 2 +- datalinklayer/DataLinkLayer.cpp | 4 ++-- datalinklayer/TcTransferFrameLocal.cpp | 6 +++--- devicehandlers/DeviceHandlerBase.cpp | 2 +- globalfunctions/{crc_ccitt.cpp => CRC.cpp} | 8 ++++---- globalfunctions/CRC.h | 16 ++++++++++++++++ globalfunctions/crc_ccitt.h | 9 --------- memory/MemoryHelper.cpp | 4 ++-- tcdistribution/TcPacketCheck.cpp | 4 ++-- tmtcpacket/pus/TcPacketBase.cpp | 4 ++-- tmtcpacket/pus/TmPacketBase.cpp | 4 ++-- 11 files changed, 35 insertions(+), 28 deletions(-) rename globalfunctions/{crc_ccitt.cpp => CRC.cpp} (94%) create mode 100644 globalfunctions/CRC.h delete mode 100644 globalfunctions/crc_ccitt.h diff --git a/container/IndexedRingMemoryArray.h b/container/IndexedRingMemoryArray.h index eacfe3e5..992b7489 100644 --- a/container/IndexedRingMemoryArray.h +++ b/container/IndexedRingMemoryArray.h @@ -2,10 +2,10 @@ #define FRAMEWORK_CONTAINER_INDEXEDRINGMEMORY_H_ #include +#include #include #include #include -#include #include template diff --git a/datalinklayer/DataLinkLayer.cpp b/datalinklayer/DataLinkLayer.cpp index 1ee546c1..70999403 100644 --- a/datalinklayer/DataLinkLayer.cpp +++ b/datalinklayer/DataLinkLayer.cpp @@ -1,5 +1,5 @@ #include -#include +#include #include DataLinkLayer::DataLinkLayer(uint8_t* set_frame_buffer, ClcwIF* setClcw, @@ -60,7 +60,7 @@ ReturnValue_t DataLinkLayer::frameValidationCheck() { } ReturnValue_t DataLinkLayer::frameCheckCRC() { - uint16_t checkValue = ::Calculate_CRC(this->currentFrame.getFullFrame(), + uint16_t checkValue = CRC::crc16ccitt(this->currentFrame.getFullFrame(), this->currentFrame.getFullSize()); if (checkValue == 0) { return RETURN_OK; diff --git a/datalinklayer/TcTransferFrameLocal.cpp b/datalinklayer/TcTransferFrameLocal.cpp index a53c5b28..7362a6ae 100644 --- a/datalinklayer/TcTransferFrameLocal.cpp +++ b/datalinklayer/TcTransferFrameLocal.cpp @@ -6,7 +6,7 @@ */ #include -#include +#include #include #include @@ -25,7 +25,7 @@ TcTransferFrameLocal::TcTransferFrameLocal(bool bypass, bool controlCommand, uin uint16_t totalSize = sizeof(TcTransferFramePrimaryHeader) + dataSize + FRAME_CRC_SIZE -1; frame->header.vcidAndLength_h |= (totalSize & 0x0300) >> 8; frame->header.length_l = (totalSize & 0x00FF); - uint16_t crc = ::Calculate_CRC(getFullFrame(), getFullSize() -2); + uint16_t crc = CRC::crc16ccitt(getFullFrame(), getFullSize() -2); this->getFullFrame()[getFullSize()-2] = (crc & 0xFF00) >> 8; this->getFullFrame()[getFullSize()-1] = (crc & 0x00FF); } else if (dataSize <= 1016) { @@ -33,7 +33,7 @@ TcTransferFrameLocal::TcTransferFrameLocal(bool bypass, bool controlCommand, uin uint16_t dataCrcSize = sizeof(TcTransferFramePrimaryHeader) + 1 + dataSize + FRAME_CRC_SIZE -1; frame->header.vcidAndLength_h |= (dataCrcSize & 0x0300) >> 8; frame->header.length_l = (dataCrcSize & 0x00FF); - uint16_t crc = ::Calculate_CRC(getFullFrame(), getFullSize() -2); + uint16_t crc = CRC::crc16ccitt(getFullFrame(), getFullSize() -2); this->getFullFrame()[getFullSize()-2] = (crc & 0xFF00) >> 8; this->getFullFrame()[getFullSize()-1] = (crc & 0x00FF); } else { diff --git a/devicehandlers/DeviceHandlerBase.cpp b/devicehandlers/DeviceHandlerBase.cpp index ae9199f2..22d49d37 100644 --- a/devicehandlers/DeviceHandlerBase.cpp +++ b/devicehandlers/DeviceHandlerBase.cpp @@ -4,7 +4,7 @@ #include #include #include -#include +#include #include #include #include diff --git a/globalfunctions/crc_ccitt.cpp b/globalfunctions/CRC.cpp similarity index 94% rename from globalfunctions/crc_ccitt.cpp rename to globalfunctions/CRC.cpp index 74a93465..ecd0b675 100644 --- a/globalfunctions/crc_ccitt.cpp +++ b/globalfunctions/CRC.cpp @@ -1,7 +1,7 @@ -#include +#include #include -static const uint16_t crc_table[256] = { +const uint16_t CRC::crc16ccitt_table[256] = { 0x0000, 0x1021, 0x2042, 0x3063, 0x4084, 0x50a5, 0x60c6, 0x70e7, 0x8108, 0x9129, 0xa14a, 0xb16b, 0xc18c, 0xd1ad, 0xe1ce, 0xf1ef, 0x1231, 0x0210, 0x3273, 0x2252, 0x52b5, 0x4294, 0x72f7, 0x62d6, @@ -38,14 +38,14 @@ static const uint16_t crc_table[256] = { // CRC implementation -uint16_t Calculate_CRC(uint8_t const input[], uint32_t length, uint16_t startingCrc) +uint16_t CRC::crc16ccitt(uint8_t const input[], uint32_t length, uint16_t startingCrc) { uint8_t *data = (uint8_t *)input; unsigned int tbl_idx; while (length--) { tbl_idx = ((startingCrc >> 8) ^ *data) & 0xff; - startingCrc = (crc_table[tbl_idx] ^ (startingCrc << 8)) & 0xffff; + startingCrc = (crc16ccitt_table[tbl_idx] ^ (startingCrc << 8)) & 0xffff; data++; } diff --git a/globalfunctions/CRC.h b/globalfunctions/CRC.h new file mode 100644 index 00000000..4200f94f --- /dev/null +++ b/globalfunctions/CRC.h @@ -0,0 +1,16 @@ +#ifndef CRC_CCITT_H_ +#define CRC_CCITT_H_ + +#include + +class CRC { +public: + static uint16_t crc16ccitt(uint8_t const input[], uint32_t length, + uint16_t startingCrc = 0xffff); +private: + CRC(); + + static const uint16_t crc16ccitt_table[256]; +}; + +#endif /* CRC_H_ */ diff --git a/globalfunctions/crc_ccitt.h b/globalfunctions/crc_ccitt.h deleted file mode 100644 index 60077d0b..00000000 --- a/globalfunctions/crc_ccitt.h +++ /dev/null @@ -1,9 +0,0 @@ -#ifndef CRC_CCITT_H_ -#define CRC_CCITT_H_ - -#include - -uint16_t Calculate_CRC(uint8_t const input[], uint32_t length, uint16_t startingCrc = 0xffff); - - -#endif /* CRC_H_ */ diff --git a/memory/MemoryHelper.cpp b/memory/MemoryHelper.cpp index 4905875e..69830084 100644 --- a/memory/MemoryHelper.cpp +++ b/memory/MemoryHelper.cpp @@ -1,4 +1,4 @@ -#include +#include #include #include #include @@ -98,7 +98,7 @@ void MemoryHelper::completeDump(ReturnValue_t errorCode, break; } case MemoryMessage::CMD_MEMORY_CHECK: { - uint16_t crc = ::Calculate_CRC(reservedSpaceInIPC, size); + uint16_t crc = CRC::crc16ccitt(reservedSpaceInIPC, size); //Delete data immediately, was temporary. ipcStore->deleteData(ipcAddress); MemoryMessage::setMemoryCheckReply(&reply, crc); diff --git a/tcdistribution/TcPacketCheck.cpp b/tcdistribution/TcPacketCheck.cpp index b942e6b6..90a1167f 100644 --- a/tcdistribution/TcPacketCheck.cpp +++ b/tcdistribution/TcPacketCheck.cpp @@ -1,4 +1,4 @@ -#include +#include #include #include #include @@ -8,7 +8,7 @@ TcPacketCheck::TcPacketCheck( uint16_t set_apid ) : apid(set_apid) { } ReturnValue_t TcPacketCheck::checkPacket( TcPacketStored* current_packet ) { - uint16_t calculated_crc = ::Calculate_CRC ( current_packet->getWholeData(), current_packet->getFullSize() ); + uint16_t calculated_crc = CRC::crc16ccitt( current_packet->getWholeData(), current_packet->getFullSize() ); if ( calculated_crc != 0 ) { return INCORRECT_CHECKSUM; } diff --git a/tmtcpacket/pus/TcPacketBase.cpp b/tmtcpacket/pus/TcPacketBase.cpp index 630394f8..c81e06b7 100644 --- a/tmtcpacket/pus/TcPacketBase.cpp +++ b/tmtcpacket/pus/TcPacketBase.cpp @@ -1,4 +1,4 @@ -#include +#include #include #include #include @@ -40,7 +40,7 @@ uint16_t TcPacketBase::getErrorControl() { void TcPacketBase::setErrorControl() { uint32_t full_size = getFullSize(); - uint16_t crc = ::Calculate_CRC(getWholeData(), full_size - CRC_SIZE); + uint16_t crc = CRC::crc16ccitt(getWholeData(), full_size - CRC_SIZE); uint32_t size = getApplicationDataSize(); (&tcData->data)[size] = (crc & 0XFF00) >> 8; // CRCH (&tcData->data)[size + 1] = (crc) & 0X00FF; // CRCL diff --git a/tmtcpacket/pus/TmPacketBase.cpp b/tmtcpacket/pus/TmPacketBase.cpp index 1599f79e..74bc9b8a 100644 --- a/tmtcpacket/pus/TmPacketBase.cpp +++ b/tmtcpacket/pus/TmPacketBase.cpp @@ -1,4 +1,4 @@ -#include +#include #include #include #include @@ -39,7 +39,7 @@ uint16_t TmPacketBase::getErrorControl() { void TmPacketBase::setErrorControl() { uint32_t full_size = getFullSize(); - uint16_t crc = ::Calculate_CRC(getWholeData(), full_size - CRC_SIZE); + uint16_t crc = CRC::crc16ccitt(getWholeData(), full_size - CRC_SIZE); uint32_t size = getSourceDataSize(); getSourceData()[size] = (crc & 0XFF00) >> 8; // CRCH getSourceData()[size + 1] = (crc) & 0X00FF; // CRCL From 504c8177e8b428818108b57e091b77ee1e0155a8 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Fri, 24 Apr 2020 17:05:34 +0200 Subject: [PATCH 11/21] uninitialized variable --- parameters/ParameterWrapper.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parameters/ParameterWrapper.cpp b/parameters/ParameterWrapper.cpp index 8f661bb3..1dac7075 100644 --- a/parameters/ParameterWrapper.cpp +++ b/parameters/ParameterWrapper.cpp @@ -91,7 +91,7 @@ template ReturnValue_t ParameterWrapper::serializeData(uint8_t** buffer, uint32_t* size, const uint32_t max_size, bool bigEndian) const { const T *element = (const T*) readonlyData; - ReturnValue_t result; + ReturnValue_t result = HasReturnvaluesIF::RETURN_OK; uint16_t dataSize = columns * rows; while (dataSize != 0) { result = SerializeAdapter::serialize(element, buffer, size, max_size, From 5b933dfe3d922e7f60ee0c3109b602a2b8331188 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 5 May 2020 18:28:50 +0200 Subject: [PATCH 12/21] also added additonal time check for freeRTOS --- osal/FreeRTOS/FixedTimeslotTask.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/osal/FreeRTOS/FixedTimeslotTask.cpp b/osal/FreeRTOS/FixedTimeslotTask.cpp index 413d7596..e621d312 100644 --- a/osal/FreeRTOS/FixedTimeslotTask.cpp +++ b/osal/FreeRTOS/FixedTimeslotTask.cpp @@ -57,12 +57,18 @@ ReturnValue_t FixedTimeslotTask::startTask() { ReturnValue_t FixedTimeslotTask::addSlot(object_id_t componentId, uint32_t slotTimeMs, int8_t executionStep) { - if (objectManager->get(componentId) != NULL) { + if (objectManager->get(componentId) != nullptr) { + 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; + error << "Component " << std::hex << componentId << + " not found, not adding it to pst" << std::endl; return HasReturnvaluesIF::RETURN_FAILED; } From 736a69795dd3aec0cc9441209c04a2acda770f2d Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 5 May 2020 18:32:50 +0200 Subject: [PATCH 13/21] change ported to linux and rtems --- osal/linux/FixedTimeslotTask.cpp | 10 ++++++++-- osal/rtems/PollingTask.cpp | 15 ++++++++++----- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/osal/linux/FixedTimeslotTask.cpp b/osal/linux/FixedTimeslotTask.cpp index 99cbf818..29eb37e7 100644 --- a/osal/linux/FixedTimeslotTask.cpp +++ b/osal/linux/FixedTimeslotTask.cpp @@ -40,8 +40,14 @@ uint32_t FixedTimeslotTask::getPeriodMs() const { 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) != nullptr) { + 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; } ReturnValue_t FixedTimeslotTask::checkSequence() const { diff --git a/osal/rtems/PollingTask.cpp b/osal/rtems/PollingTask.cpp index c2dc629b..7409a6b8 100644 --- a/osal/rtems/PollingTask.cpp +++ b/osal/rtems/PollingTask.cpp @@ -66,12 +66,17 @@ ReturnValue_t PollingTask::startTask() { } } -ReturnValue_t PollingTask::addSlot(object_id_t componentId, uint32_t slotTimeMs, - int8_t executionStep) { - pst.addSlot(componentId, slotTimeMs, executionStep, this); - return HasReturnvaluesIF::RETURN_OK; -} +ReturnValue_t Polling::addSlot(object_id_t componentId, + uint32_t slotTimeMs, int8_t executionStep) { + if (objectManager->get(componentId) != nullptr) { + 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 PollingTask::getPeriodMs() const { return pst.getLengthMs(); } From f3dca8044e8c4ee7bdd08a3bcd5034beb4e2d2a5 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 5 May 2020 18:34:04 +0200 Subject: [PATCH 14/21] typo --- osal/rtems/PollingTask.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osal/rtems/PollingTask.cpp b/osal/rtems/PollingTask.cpp index 7409a6b8..4a70a1ed 100644 --- a/osal/rtems/PollingTask.cpp +++ b/osal/rtems/PollingTask.cpp @@ -66,7 +66,7 @@ ReturnValue_t PollingTask::startTask() { } } -ReturnValue_t Polling::addSlot(object_id_t componentId, +ReturnValue_t PollingTask::addSlot(object_id_t componentId, uint32_t slotTimeMs, int8_t executionStep) { if (objectManager->get(componentId) != nullptr) { pst.addSlot(componentId, slotTimeMs, executionStep, this); From f1a0bb9dc3ece68f16a42eca68218c807efa1bd7 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 5 May 2020 18:36:18 +0200 Subject: [PATCH 15/21] whitespace --- osal/rtems/PollingTask.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/osal/rtems/PollingTask.cpp b/osal/rtems/PollingTask.cpp index 4a70a1ed..03ba0951 100644 --- a/osal/rtems/PollingTask.cpp +++ b/osal/rtems/PollingTask.cpp @@ -77,6 +77,7 @@ ReturnValue_t PollingTask::addSlot(object_id_t componentId, " not found, not adding it to pst" << std::endl; return HasReturnvaluesIF::RETURN_FAILED; } + uint32_t PollingTask::getPeriodMs() const { return pst.getLengthMs(); } From 75da7a4c500e343b559406ecb1877d7ec750fd29 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 5 May 2020 18:55:05 +0200 Subject: [PATCH 16/21] comment moved to header --- devicehandlers/FixedSlotSequence.cpp | 1 - devicehandlers/FixedSlotSequence.h | 5 +++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/devicehandlers/FixedSlotSequence.cpp b/devicehandlers/FixedSlotSequence.cpp index a65dd929..9ff5a140 100644 --- a/devicehandlers/FixedSlotSequence.cpp +++ b/devicehandlers/FixedSlotSequence.cpp @@ -89,7 +89,6 @@ uint32_t FixedSlotSequence::getLengthMs() const { } 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. auto slotIt = slotList.begin(); uint32_t count = 0; uint32_t time = 0; diff --git a/devicehandlers/FixedSlotSequence.h b/devicehandlers/FixedSlotSequence.h index f8fd9a36..1da9d350 100644 --- a/devicehandlers/FixedSlotSequence.h +++ b/devicehandlers/FixedSlotSequence.h @@ -97,6 +97,11 @@ public: */ std::list::iterator current; + /** + * 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; protected: From a6b3cee898a8be16658cda04153e11829a5db372 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 5 May 2020 18:56:45 +0200 Subject: [PATCH 17/21] class comment formatting --- devicehandlers/FixedSlotSequence.h | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/devicehandlers/FixedSlotSequence.h b/devicehandlers/FixedSlotSequence.h index 1da9d350..64c68aea 100644 --- a/devicehandlers/FixedSlotSequence.h +++ b/devicehandlers/FixedSlotSequence.h @@ -6,15 +6,21 @@ #include /** - * \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. - * 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. + * + * The main idea is to create a list of device handlers, to announce all + * handlers to thepolling 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. */ class FixedSlotSequence { public: From 432dbbd26eca201025d5d064425881f8b3f40222 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 5 May 2020 18:57:30 +0200 Subject: [PATCH 18/21] removed sif --- devicehandlers/FixedSlotSequence.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/devicehandlers/FixedSlotSequence.cpp b/devicehandlers/FixedSlotSequence.cpp index 9ff5a140..bf9ff4fd 100644 --- a/devicehandlers/FixedSlotSequence.cpp +++ b/devicehandlers/FixedSlotSequence.cpp @@ -89,6 +89,10 @@ uint32_t FixedSlotSequence::getLengthMs() const { } ReturnValue_t FixedSlotSequence::checkSequence() const { + if(slotList.empty()) { + error << "Fixed Slot Sequence: Slot list is empty!" << std::endl; + std::exit(0); + } auto slotIt = slotList.begin(); uint32_t count = 0; uint32_t time = 0; From 98e505c9ab3fc42513390e859bc98df201fe3cc0 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 5 May 2020 19:03:00 +0200 Subject: [PATCH 19/21] todo removed --- osal/FreeRTOS/FixedTimeslotTask.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osal/FreeRTOS/FixedTimeslotTask.cpp b/osal/FreeRTOS/FixedTimeslotTask.cpp index e621d312..74ce9b78 100644 --- a/osal/FreeRTOS/FixedTimeslotTask.cpp +++ b/osal/FreeRTOS/FixedTimeslotTask.cpp @@ -59,8 +59,8 @@ ReturnValue_t FixedTimeslotTask::addSlot(object_id_t componentId, uint32_t slotTimeMs, int8_t executionStep) { if (objectManager->get(componentId) != nullptr) { if(slotTimeMs == 0) { - // TODO: FreeRTOS throws errors for zero values. - // maybe there is a better solution than this. + // FreeRTOS throws a sanity error for zero values, so we set + // the time to one millisecond. slotTimeMs = 1; } pst.addSlot(componentId, slotTimeMs, executionStep, this); From c4486e79eca8fa258b744bf27cf364b5258fc2ba Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 5 May 2020 19:30:26 +0200 Subject: [PATCH 20/21] removed exit for empty pst --- devicehandlers/FixedSlotSequence.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/devicehandlers/FixedSlotSequence.cpp b/devicehandlers/FixedSlotSequence.cpp index bf9ff4fd..61959ac0 100644 --- a/devicehandlers/FixedSlotSequence.cpp +++ b/devicehandlers/FixedSlotSequence.cpp @@ -91,7 +91,6 @@ uint32_t FixedSlotSequence::getLengthMs() const { ReturnValue_t FixedSlotSequence::checkSequence() const { if(slotList.empty()) { error << "Fixed Slot Sequence: Slot list is empty!" << std::endl; - std::exit(0); } auto slotIt = slotList.begin(); uint32_t count = 0; From 119455f3fd7f65ebd4559c14e8e5a763a2870936 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 5 May 2020 19:33:06 +0200 Subject: [PATCH 21/21] replaced exit by returning failed --- devicehandlers/FixedSlotSequence.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/devicehandlers/FixedSlotSequence.cpp b/devicehandlers/FixedSlotSequence.cpp index 61959ac0..dae62bdd 100644 --- a/devicehandlers/FixedSlotSequence.cpp +++ b/devicehandlers/FixedSlotSequence.cpp @@ -91,6 +91,7 @@ uint32_t FixedSlotSequence::getLengthMs() const { ReturnValue_t FixedSlotSequence::checkSequence() const { if(slotList.empty()) { error << "Fixed Slot Sequence: Slot list is empty!" << std::endl; + return HasReturnvaluesIF::RETURN_FAILED; } auto slotIt = slotList.begin(); uint32_t count = 0;