From 368ef242ff59b6556e155ba4007630991c328cff Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Sat, 18 Jan 2020 18:49:55 +0100 Subject: [PATCH 01/59] CCSDSTime bugfix for atmel Possible good for other cases too? --- timemanager/CCSDSTime.cpp | 43 +++++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/timemanager/CCSDSTime.cpp b/timemanager/CCSDSTime.cpp index 320327161..2eb6c4b54 100644 --- a/timemanager/CCSDSTime.cpp +++ b/timemanager/CCSDSTime.cpp @@ -154,32 +154,41 @@ ReturnValue_t CCSDSTime::convertFromASCII(Clock::TimeOfDay_t* to, const uint8_t* if (length < 19) { return RETURN_FAILED; } - uint16_t year; - uint8_t month; - uint16_t day; - uint8_t hour; - uint8_t minute; - float second; -//try Code A (yyyy-mm-dd) - int count = sscanf((char *) from, "%4hi-%2hhi-%2hiT%2hhi:%2hhi:%fZ", &year, - &month, &day, &hour, &minute, &second); - if (count == 6) { + // In the size optimized nano library used by ATMEL, floating point conversion + // is not allowed. There is a linker flag to allow it apparently, but I + // could not manage to make it run (removing -specs=nano.specs in linker flags works though, + // but we propably should include this). Using floats with sscanf is also expensive. + // Furthermore, the stdio.c library by ATMEL can't resolve the %hhi specifiers + // Therefore, I adapted this function. + int year; + int month; + int day; + int hour; + int minute; + int second; + int usecond; + // try Code A (yyyy-mm-dd) + //int count = sscanf((char *) from, "%4hi-%2hi-%2hiT%2hi:%2hi:%fZ", &year, + // &month, &day, &hour, &minute, &second); + int count = sscanf((char *) from, "%4d-%2d-%2dT%2d:%2d:%2d.%dZ", &year, + &month, &day, &hour, &minute, &second, &usecond); + if (count == 7) { to->year = year; to->month = month; to->day = day; to->hour = hour; to->minute = minute; to->second = second; - to->usecond = (second - floor(second)) * 1000000; + to->usecond = usecond;//(second - floor(second)) * 1000000; return RETURN_OK; } - //try Code B (yyyy-ddd) - count = sscanf((char *) from, "%4hi-%3hiT%2hhi:%2hhi:%fZ", &year, &day, - &hour, &minute, &second); - if (count == 5) { + // try Code B (yyyy-ddd) + count = sscanf((char *) from, "%4i-%3iT%2i:%2i:%2i.%iZ", &year, &day, + &hour, &minute, &second, &usecond); + if (count == 6) { uint8_t tempDay; - ReturnValue_t result = CCSDSTime::convertDaysOfYear(day, year, &month, + ReturnValue_t result = CCSDSTime::convertDaysOfYear((uint16_t)day,(uint16_t) year,(uint8_t *) &month, &tempDay); if (result != RETURN_OK) { return RETURN_FAILED; @@ -190,7 +199,7 @@ ReturnValue_t CCSDSTime::convertFromASCII(Clock::TimeOfDay_t* to, const uint8_t* to->hour = hour; to->minute = minute; to->second = second; - to->usecond = (second - floor(second)) * 1000000; + to->usecond = usecond; return RETURN_OK; } From ea904642d1c8b4085b03fdd2837912ebe242c20c Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 21 Apr 2020 16:16:02 +0200 Subject: [PATCH 02/59] CCSDS time possible bugfix for sscanf() --- timemanager/CCSDSTime.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/timemanager/CCSDSTime.cpp b/timemanager/CCSDSTime.cpp index 320327161..8a0e65b78 100644 --- a/timemanager/CCSDSTime.cpp +++ b/timemanager/CCSDSTime.cpp @@ -1,5 +1,6 @@ #include #include +#include #include CCSDSTime::CCSDSTime() { @@ -160,9 +161,10 @@ ReturnValue_t CCSDSTime::convertFromASCII(Clock::TimeOfDay_t* to, const uint8_t* uint8_t hour; uint8_t minute; float second; -//try Code A (yyyy-mm-dd) - int count = sscanf((char *) from, "%4hi-%2hhi-%2hiT%2hhi:%2hhi:%fZ", &year, - &month, &day, &hour, &minute, &second); + //try Code A (yyyy-mm-dd) + int count = sscanf((char *) from, "%4" SCNu16 "-%2" SCNu8 "-%2" SCNu16 + "T%2" SCNu8 ":%2" SCNu8 ":%fZ", &year, &month, &day, + &hour, &minute, &second); if (count == 6) { to->year = year; to->month = month; @@ -175,8 +177,8 @@ ReturnValue_t CCSDSTime::convertFromASCII(Clock::TimeOfDay_t* to, const uint8_t* } //try Code B (yyyy-ddd) - count = sscanf((char *) from, "%4hi-%3hiT%2hhi:%2hhi:%fZ", &year, &day, - &hour, &minute, &second); + count = sscanf((char *) from, "%4" SCNu16 "-%3" SCNu16 "T%2" SCNu8 + ":%2" SCNu8 ":%fZ", &year, &day, &hour, &minute, &second); if (count == 5) { uint8_t tempDay; ReturnValue_t result = CCSDSTime::convertDaysOfYear(day, year, &month, @@ -193,7 +195,6 @@ ReturnValue_t CCSDSTime::convertFromASCII(Clock::TimeOfDay_t* to, const uint8_t* to->usecond = (second - floor(second)) * 1000000; return RETURN_OK; } - return UNSUPPORTED_TIME_FORMAT; } From c30cae3431d586304bf0d3fd1c5d1ff05d7f9691 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 21 Apr 2020 16:32:39 +0200 Subject: [PATCH 03/59] added back NoC99 io section --- timemanager/CCSDSTime.cpp | 46 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/timemanager/CCSDSTime.cpp b/timemanager/CCSDSTime.cpp index 8a0e65b78..e00beb27a 100644 --- a/timemanager/CCSDSTime.cpp +++ b/timemanager/CCSDSTime.cpp @@ -155,6 +155,51 @@ ReturnValue_t CCSDSTime::convertFromASCII(Clock::TimeOfDay_t* to, const uint8_t* if (length < 19) { return RETURN_FAILED; } + // Newlib nano can't parse uint8, see SCNu8 documentation and https://sourceware.org/newlib/README + // Suggestion: use uint16 all the time. This should work on all systems. +#ifdef NEWLIB_NANO_NO_C99_IO + uint16_t year; + uint16_t month; + uint16_t day; + uint16_t hour; + uint16_t minute; + float second; + int count = sscanf((char *) from, "%4" SCNu16 "-%2" SCNu16 "-%2" + SCNu16 "T%2" SCNu16 ":%2" SCNu16 ":%fZ", &year, &month, &day, &hour, + &minute, &second); + if (count == 6) { + to->year = year; + to->month = month; + to->day = day; + to->hour = hour; + to->minute = minute; + to->second = second; + to->usecond = (second - floor(second)) * 1000000; + return RETURN_OK; + } + + // try Code B (yyyy-ddd) + count = sscanf((char *) from, "%4" SCNu16 "-%3" SCNu16 "T%2" SCNu16 ":%2" + SCNu16 ":%fZ", &year, &day, &hour, &minute, &second); + if (count == 5) { + uint8_t tempDay; + ReturnValue_t result = CCSDSTime::convertDaysOfYear(day, year, + reinterpret_cast(&month), reinterpret_cast(&tempDay)); + if (result != RETURN_OK) { + return RETURN_FAILED; + } + to->year = year; + to->month = month; + to->day = tempDay; + to->hour = hour; + to->minute = minute; + to->second = second; + to->usecond = (second - floor(second)) * 1000000; + return RETURN_OK; + } + // Warning: Compiler/Linker fails ambiguously if library does not implement + // C99 I/O +#else uint16_t year; uint8_t month; uint16_t day; @@ -195,6 +240,7 @@ ReturnValue_t CCSDSTime::convertFromASCII(Clock::TimeOfDay_t* to, const uint8_t* to->usecond = (second - floor(second)) * 1000000; return RETURN_OK; } +#endif return UNSUPPORTED_TIME_FORMAT; } From e5cea3ead078302df24d17ce43895ceae5c5046c Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Fri, 29 May 2020 20:31:08 +0200 Subject: [PATCH 04/59] service interface stream enhancements --- serviceinterface/ServiceInterfaceBuffer.cpp | 28 +++++++++++++-------- serviceinterface/ServiceInterfaceBuffer.h | 28 ++++++++++++++++----- serviceinterface/ServiceInterfaceStream.cpp | 6 ++--- serviceinterface/ServiceInterfaceStream.h | 7 +++--- 4 files changed, 47 insertions(+), 22 deletions(-) diff --git a/serviceinterface/ServiceInterfaceBuffer.cpp b/serviceinterface/ServiceInterfaceBuffer.cpp index 58065994a..2a97ab90b 100644 --- a/serviceinterface/ServiceInterfaceBuffer.cpp +++ b/serviceinterface/ServiceInterfaceBuffer.cpp @@ -23,14 +23,16 @@ int ServiceInterfaceBuffer::sync(void) { if (this->isActive) { Clock::TimeOfDay_t loggerTime; Clock::getDateAndTime(&loggerTime); - char preamble[96] = { 0 }; - sprintf(preamble, "%s: | %lu:%02lu:%02lu.%03lu | ", - this->log_message.c_str(), (unsigned long) loggerTime.hour, - (unsigned long) loggerTime.minute, - (unsigned long) loggerTime.second, - (unsigned long) loggerTime.usecond /1000); + std::string preamble; + if(addCrToPreamble) { + preamble += "\r"; + } + preamble += log_message + ": | " + zero_padded(loggerTime.hour, 2) + + ":" + zero_padded(loggerTime.minute, 2) + ":" + + zero_padded(loggerTime.second, 2) + "." + + zero_padded(loggerTime.usecond/1000, 3) + " | "; // Write log_message and time - this->putChars(preamble, preamble + sizeof(preamble)); + this->putChars(preamble.c_str(), preamble.c_str() + preamble.size()); // Handle output this->putChars(pbase(), pptr()); } @@ -39,9 +41,13 @@ int ServiceInterfaceBuffer::sync(void) { return 0; } + + #ifndef UT699 -ServiceInterfaceBuffer::ServiceInterfaceBuffer(std::string set_message, uint16_t port) { +ServiceInterfaceBuffer::ServiceInterfaceBuffer(std::string set_message, + uint16_t port, bool addCrToPreamble) { + this->addCrToPreamble = addCrToPreamble; this->log_message = set_message; this->isActive = true; setp( buf, buf + BUF_SIZE ); @@ -55,17 +61,19 @@ void ServiceInterfaceBuffer::putChars(char const* begin, char const* end) { } memcpy(array, begin, length); - for( ; begin != end; begin++){ + for(; begin != end; begin++){ printChar(begin); } } #endif + #ifdef UT699 #include -ServiceInterfaceBuffer::ServiceInterfaceBuffer(std::string set_message, uint16_t port) { +ServiceInterfaceBuffer::ServiceInterfaceBuffer(std::string set_message, + uint16_t port) { this->log_message = set_message; this->isActive = true; setp( buf, buf + BUF_SIZE ); diff --git a/serviceinterface/ServiceInterfaceBuffer.h b/serviceinterface/ServiceInterfaceBuffer.h index b42c8a197..a2bc4f4b1 100644 --- a/serviceinterface/ServiceInterfaceBuffer.h +++ b/serviceinterface/ServiceInterfaceBuffer.h @@ -2,32 +2,36 @@ #define FRAMEWORK_SERVICEINTERFACE_SERVICEINTERFACEBUFFER_H_ #include -#include #include #include +#include #ifndef UT699 -class ServiceInterfaceBuffer: public std::basic_streambuf > { +class ServiceInterfaceBuffer: + public std::basic_streambuf> { friend class ServiceInterfaceStream; public: - ServiceInterfaceBuffer(std::string set_message, uint16_t port); + ServiceInterfaceBuffer(std::string set_message, uint16_t port, + bool addCrToPreamble); protected: bool isActive; // This is called when buffer becomes full. If // buffer is not used, then this is called every // time when characters are put to stream. - virtual int overflow(int c = Traits::eof()); + int overflow(int c = Traits::eof()) override; // This function is called when stream is flushed, // for example when std::endl is put to stream. - virtual int sync(void); + int sync(void) override; private: // For additional message information std::string log_message; // For EOF detection typedef std::char_traits Traits; + // This is useful for some terminal programs which do not have + // implicit carriage return with newline characters. + bool addCrToPreamble; // Work in buffer mode. It is also possible to work without buffer. static size_t const BUF_SIZE = 128; @@ -35,6 +39,18 @@ private: // In this function, the characters are parsed. void putChars(char const* begin, char const* end); + + template + std::string zero_padded(const T& num, uint8_t width) { + std::ostringstream string_to_pad; + string_to_pad << std::setw(width) << std::setfill('0') << num; + std::string result = string_to_pad.str(); + if (result.length() > width) + { + result.erase(0, result.length() - width); + } + return result; + } }; #endif diff --git a/serviceinterface/ServiceInterfaceStream.cpp b/serviceinterface/ServiceInterfaceStream.cpp index c2979f369..40f52f1fe 100644 --- a/serviceinterface/ServiceInterfaceStream.cpp +++ b/serviceinterface/ServiceInterfaceStream.cpp @@ -5,7 +5,7 @@ void ServiceInterfaceStream::setActive( bool myActive) { } ServiceInterfaceStream::ServiceInterfaceStream(std::string set_message, - uint16_t port) : - std::basic_ostream >(&buf), buf( - set_message, port) { + bool addCrToPreamble, uint16_t port) : + std::basic_ostream>(&buf), + buf(set_message, port, addCrToPreamble) { } diff --git a/serviceinterface/ServiceInterfaceStream.h b/serviceinterface/ServiceInterfaceStream.h index df736a1b6..a445dcedd 100644 --- a/serviceinterface/ServiceInterfaceStream.h +++ b/serviceinterface/ServiceInterfaceStream.h @@ -17,14 +17,15 @@ extern std::ostream error; } -class ServiceInterfaceStream : public std::basic_ostream< char, std::char_traits< char > > { +class ServiceInterfaceStream : + public std::basic_ostream> { protected: ServiceInterfaceBuffer buf; public: - ServiceInterfaceStream( std::string set_message, uint16_t port = 1234 ); + ServiceInterfaceStream( std::string set_message, + bool addCrToPreamble = false, uint16_t port = 1234); void setActive( bool ); }; - #endif /* FRAMEWORK_SERVICEINTERFACE_SERVICEINTERFACESTREAM_H_ */ From 7259a1356959ab9fb18472bfdeb45a2c88f864dd Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 3 Jun 2020 23:14:17 +0200 Subject: [PATCH 05/59] more improvements: 1. New optional flag to redirect print to stderr. THis can be useful on host environemtns (e.g linux) 2. non-buffered mode if this flag is true: the preamble msut be printed manually 2. Getter function for preamble for that case. 3. printChar function: specify whether to print to stderr or stdout --- serviceinterface/ServiceInterfaceBuffer.cpp | 94 ++++++++++++--------- serviceinterface/ServiceInterfaceBuffer.h | 50 ++++++++--- serviceinterface/ServiceInterfaceStream.cpp | 12 ++- serviceinterface/ServiceInterfaceStream.h | 37 ++++++-- 4 files changed, 131 insertions(+), 62 deletions(-) diff --git a/serviceinterface/ServiceInterfaceBuffer.cpp b/serviceinterface/ServiceInterfaceBuffer.cpp index 2a97ab90b..93de88b39 100644 --- a/serviceinterface/ServiceInterfaceBuffer.cpp +++ b/serviceinterface/ServiceInterfaceBuffer.cpp @@ -3,9 +3,42 @@ #include // to be implemented by bsp -extern "C" void printChar(const char*); +extern "C" void printChar(const char*, bool errStream); + +#ifndef UT699 + +ServiceInterfaceBuffer::ServiceInterfaceBuffer(std::string setMessage, + bool errStream, bool addCrToPreamble, uint16_t port): + isActive(true), logMessage(setMessage), + addCrToPreamble(addCrToPreamble), errStream(errStream) { + if(not errStream) { + // Set pointers if the stream is buffered. + setp( buf, buf + BUF_SIZE ); + } +} + +void ServiceInterfaceBuffer::putChars(char const* begin, char const* end) { + char array[BUF_SIZE]; + uint32_t length = end - begin; + if (length > sizeof(array)) { + length = sizeof(array); + } + memcpy(array, begin, length); + + for(; begin != end; begin++){ + printChar(begin, false); + } +} + +#endif int ServiceInterfaceBuffer::overflow(int c) { + if(errStream and this->isActive) { + if (c != Traits::eof()) { + printChar(reinterpret_cast(&c), true); + } + return 0; + } // Handle output putChars(pbase(), pptr()); if (c != Traits::eof()) { @@ -20,53 +53,38 @@ int ServiceInterfaceBuffer::overflow(int c) { } int ServiceInterfaceBuffer::sync(void) { - if (this->isActive) { - Clock::TimeOfDay_t loggerTime; - Clock::getDateAndTime(&loggerTime); - std::string preamble; - if(addCrToPreamble) { - preamble += "\r"; + if(not this->isActive or errStream) { + if(not errStream) { + setp(buf, buf + BUF_SIZE - 1); } - preamble += log_message + ": | " + zero_padded(loggerTime.hour, 2) - + ":" + zero_padded(loggerTime.minute, 2) + ":" - + zero_padded(loggerTime.second, 2) + "." - + zero_padded(loggerTime.usecond/1000, 3) + " | "; - // Write log_message and time - this->putChars(preamble.c_str(), preamble.c_str() + preamble.size()); - // Handle output - this->putChars(pbase(), pptr()); + return 0; } + + auto preamble = getPreamble(); + // Write logMessage and time + this->putChars(preamble.c_str(), preamble.c_str() + preamble.size()); + // Handle output + this->putChars(pbase(), pptr()); // This tells that buffer is empty again setp(buf, buf + BUF_SIZE - 1); return 0; } - -#ifndef UT699 - -ServiceInterfaceBuffer::ServiceInterfaceBuffer(std::string set_message, - uint16_t port, bool addCrToPreamble) { - this->addCrToPreamble = addCrToPreamble; - this->log_message = set_message; - this->isActive = true; - setp( buf, buf + BUF_SIZE ); +std::string ServiceInterfaceBuffer::getPreamble() { + Clock::TimeOfDay_t loggerTime; + Clock::getDateAndTime(&loggerTime); + std::string preamble; + if(addCrToPreamble) { + preamble += "\r"; + } + preamble += logMessage + ": | " + zero_padded(loggerTime.hour, 2) + + ":" + zero_padded(loggerTime.minute, 2) + ":" + + zero_padded(loggerTime.second, 2) + "." + + zero_padded(loggerTime.usecond/1000, 3) + " | "; + return preamble; } -void ServiceInterfaceBuffer::putChars(char const* begin, char const* end) { - char array[BUF_SIZE]; - uint32_t length = end - begin; - if (length > sizeof(array)) { - length = sizeof(array); - } - memcpy(array, begin, length); - - for(; begin != end; begin++){ - printChar(begin); - } - -} -#endif #ifdef UT699 diff --git a/serviceinterface/ServiceInterfaceBuffer.h b/serviceinterface/ServiceInterfaceBuffer.h index a2bc4f4b1..74facafe0 100644 --- a/serviceinterface/ServiceInterfaceBuffer.h +++ b/serviceinterface/ServiceInterfaceBuffer.h @@ -3,43 +3,64 @@ #include #include -#include #include #ifndef UT699 + +/** + * @brief This is the underlying stream buffer which implements the + * streambuf class and overloads the overflow() and sync() methods + * @details + * This class is used to modify the output of the stream, for example by adding. + * It also calls the char printing function which is implemented in the + * board supply package (BSP). + */ class ServiceInterfaceBuffer: - public std::basic_streambuf> { + public std::streambuf { friend class ServiceInterfaceStream; public: - ServiceInterfaceBuffer(std::string set_message, uint16_t port, - bool addCrToPreamble); + ServiceInterfaceBuffer(std::string setMessage, bool errStream, + bool addCrToPreamble, uint16_t port); + protected: bool isActive; - // This is called when buffer becomes full. If - // buffer is not used, then this is called every - // time when characters are put to stream. + //! This is called when buffer becomes full. If + //! buffer is not used, then this is called every + //! time when characters are put to stream. int overflow(int c = Traits::eof()) override; - // This function is called when stream is flushed, - // for example when std::endl is put to stream. + //! This function is called when stream is flushed, + //! for example when std::endl is put to stream. int sync(void) override; private: - // For additional message information - std::string log_message; + //! For additional message information + std::string logMessage; // For EOF detection typedef std::char_traits Traits; - // This is useful for some terminal programs which do not have - // implicit carriage return with newline characters. + //! This is useful for some terminal programs which do not have + //! implicit carriage return with newline characters. bool addCrToPreamble; + //! This specifies to print to stderr and work in unbuffered mode. + bool errStream; // Work in buffer mode. It is also possible to work without buffer. static size_t const BUF_SIZE = 128; char buf[BUF_SIZE]; - // In this function, the characters are parsed. + //! In this function, the characters are parsed. void putChars(char const* begin, char const* end); + std::string getPreamble(); + + /** + * This helper function returns the zero padded string version of a number. + * The type is deduced automatically. + * @tparam T + * @param num + * @param width + * @return + */ template std::string zero_padded(const T& num, uint8_t width) { std::ostringstream string_to_pad; @@ -52,6 +73,7 @@ private: return result; } }; + #endif diff --git a/serviceinterface/ServiceInterfaceStream.cpp b/serviceinterface/ServiceInterfaceStream.cpp index 40f52f1fe..783715484 100644 --- a/serviceinterface/ServiceInterfaceStream.cpp +++ b/serviceinterface/ServiceInterfaceStream.cpp @@ -1,11 +1,15 @@ #include +ServiceInterfaceStream::ServiceInterfaceStream(std::string setMessage, + bool errStream, bool addCrToPreamble, uint16_t port) : + std::ostream(&buf), + buf(setMessage, errStream, addCrToPreamble, port) { +} + void ServiceInterfaceStream::setActive( bool myActive) { this->buf.isActive = myActive; } -ServiceInterfaceStream::ServiceInterfaceStream(std::string set_message, - bool addCrToPreamble, uint16_t port) : - std::basic_ostream>(&buf), - buf(set_message, port, addCrToPreamble) { +std::string ServiceInterfaceStream::getPreamble() { + return buf.getPreamble(); } diff --git a/serviceinterface/ServiceInterfaceStream.h b/serviceinterface/ServiceInterfaceStream.h index a445dcedd..aecd45c37 100644 --- a/serviceinterface/ServiceInterfaceStream.h +++ b/serviceinterface/ServiceInterfaceStream.h @@ -3,11 +3,9 @@ #include #include -#include -#include #include -// Unfortunately, there must be a forward declaration of log_fe +// Unfortunately, there must be a forward declaration of the log front end // (MUST be defined in main), to let the system know where to write to. namespace sif { extern std::ostream debug; @@ -18,14 +16,41 @@ extern std::ostream error; class ServiceInterfaceStream : - public std::basic_ostream> { + public std::ostream { protected: ServiceInterfaceBuffer buf; public: - ServiceInterfaceStream( std::string set_message, - bool addCrToPreamble = false, uint16_t port = 1234); + /** + * This constructor is used by specifying the preamble message. + * Optionally, the output can be directed to stderr and a CR character + * can be prepended to the preamble. + * @param set_message + * @param errStream + * @param addCrToPreamble + * @param port + */ + ServiceInterfaceStream(std::string setMessage, + bool errStream = false, bool addCrToPreamble = false, + uint16_t port = 1234); + + //! An inactive stream will not print anything. void setActive( bool ); + + /** + * This can be used to retrieve the preamble in case it should be printed in + * the unbuffered mode. + * @return Preamle consisting of log message and timestamp. + */ + std::string getPreamble(); }; +// Forward declaration of interface streams. These are needed so the public +// functions can be used by including this header +namespace sif { +extern ServiceInterfaceStream debugStream; +extern ServiceInterfaceStream infoStream; +extern ServiceInterfaceStream warningStream; +extern ServiceInterfaceStream errorStream; +} #endif /* FRAMEWORK_SERVICEINTERFACE_SERVICEINTERFACESTREAM_H_ */ From 9361568b4513645fb42a571659e8e060b3d374c7 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 3 Jun 2020 23:28:31 +0200 Subject: [PATCH 06/59] clarifying commnet --- serviceinterface/ServiceInterfaceStream.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/serviceinterface/ServiceInterfaceStream.h b/serviceinterface/ServiceInterfaceStream.h index aecd45c37..375a18759 100644 --- a/serviceinterface/ServiceInterfaceStream.h +++ b/serviceinterface/ServiceInterfaceStream.h @@ -5,8 +5,11 @@ #include #include -// Unfortunately, there must be a forward declaration of the log front end -// (MUST be defined in main), to let the system know where to write to. +/* Unfortunately, there must be a forward declaration of the log front end + * (MUST be defined in main), to let the system know where to write to. + * The ServiceInterfaceStream instances, which are declared below and + * can instantaited somewhere else can be passed to these front ends by passing + * their underlying buffers with .rdbuf() */ namespace sif { extern std::ostream debug; extern std::ostream info; From 764608005b0934c02faedf9913ae7b3153e11ba3 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Thu, 4 Jun 2020 13:26:35 +0200 Subject: [PATCH 07/59] buf renamed to streambuf --- osal/FreeRTOS/MessageQueue.cpp | 3 ++- serviceinterface/ServiceInterfaceStream.cpp | 8 ++++---- serviceinterface/ServiceInterfaceStream.h | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/osal/FreeRTOS/MessageQueue.cpp b/osal/FreeRTOS/MessageQueue.cpp index e5da04427..18e7aa3d0 100644 --- a/osal/FreeRTOS/MessageQueue.cpp +++ b/osal/FreeRTOS/MessageQueue.cpp @@ -97,7 +97,8 @@ ReturnValue_t MessageQueue::sendMessageFromMessageQueue(MessageQueueId_t sendTo, bool ignoreFault) { message->setSender(sentFrom); - BaseType_t result = xQueueSendToBack(reinterpret_cast(sendTo),reinterpret_cast(message->getBuffer()), 0); + BaseType_t result = xQueueSendToBack(reinterpret_cast(sendTo), + reinterpret_cast(message->getBuffer()), 0); if (result != pdPASS) { if (!ignoreFault) { InternalErrorReporterIF* internalErrorReporter = objectManager->get( diff --git a/serviceinterface/ServiceInterfaceStream.cpp b/serviceinterface/ServiceInterfaceStream.cpp index 783715484..d9bc980a7 100644 --- a/serviceinterface/ServiceInterfaceStream.cpp +++ b/serviceinterface/ServiceInterfaceStream.cpp @@ -2,14 +2,14 @@ ServiceInterfaceStream::ServiceInterfaceStream(std::string setMessage, bool errStream, bool addCrToPreamble, uint16_t port) : - std::ostream(&buf), - buf(setMessage, errStream, addCrToPreamble, port) { + std::ostream(&streambuf), + streambuf(setMessage, errStream, addCrToPreamble, port) { } void ServiceInterfaceStream::setActive( bool myActive) { - this->buf.isActive = myActive; + this->streambuf.isActive = myActive; } std::string ServiceInterfaceStream::getPreamble() { - return buf.getPreamble(); + return streambuf.getPreamble(); } diff --git a/serviceinterface/ServiceInterfaceStream.h b/serviceinterface/ServiceInterfaceStream.h index 375a18759..c44fe03a0 100644 --- a/serviceinterface/ServiceInterfaceStream.h +++ b/serviceinterface/ServiceInterfaceStream.h @@ -21,7 +21,7 @@ extern std::ostream error; class ServiceInterfaceStream : public std::ostream { protected: - ServiceInterfaceBuffer buf; + ServiceInterfaceBuffer streambuf; public: /** * This constructor is used by specifying the preamble message. From f8fb370ae7698c8b27c86428ab01129042cedfa3 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Thu, 4 Jun 2020 14:08:26 +0200 Subject: [PATCH 08/59] preamble changes started --- osal/FreeRTOS/FixedTimeslotTask.cpp | 2 +- serviceinterface/ServiceInterfaceBuffer.cpp | 65 +++++++++++++++------ serviceinterface/ServiceInterfaceBuffer.h | 8 ++- serviceinterface/ServiceInterfaceStream.cpp | 28 +++++++-- serviceinterface/ServiceInterfaceStream.h | 53 ++++++++--------- 5 files changed, 104 insertions(+), 52 deletions(-) diff --git a/osal/FreeRTOS/FixedTimeslotTask.cpp b/osal/FreeRTOS/FixedTimeslotTask.cpp index fb3c3b03e..549ffbf17 100644 --- a/osal/FreeRTOS/FixedTimeslotTask.cpp +++ b/osal/FreeRTOS/FixedTimeslotTask.cpp @@ -68,7 +68,7 @@ ReturnValue_t FixedTimeslotTask::addSlot(object_id_t componentId, } sif::error << "Component " << std::hex << componentId << - " not found, not adding it to pst" << std::endl; + " not found, not adding it to pst\r" << std::endl; return HasReturnvaluesIF::RETURN_FAILED; } diff --git a/serviceinterface/ServiceInterfaceBuffer.cpp b/serviceinterface/ServiceInterfaceBuffer.cpp index 93de88b39..321b1d3ca 100644 --- a/serviceinterface/ServiceInterfaceBuffer.cpp +++ b/serviceinterface/ServiceInterfaceBuffer.cpp @@ -8,13 +8,15 @@ extern "C" void printChar(const char*, bool errStream); #ifndef UT699 ServiceInterfaceBuffer::ServiceInterfaceBuffer(std::string setMessage, - bool errStream, bool addCrToPreamble, uint16_t port): + bool addCrToPreamble, bool buffered , bool errStream, uint16_t port): isActive(true), logMessage(setMessage), - addCrToPreamble(addCrToPreamble), errStream(errStream) { - if(not errStream) { + addCrToPreamble(addCrToPreamble), buffered(buffered), + errStream(errStream) { + if(buffered) { // Set pointers if the stream is buffered. setp( buf, buf + BUF_SIZE ); } + preamble.reserve(96); } void ServiceInterfaceBuffer::putChars(char const* begin, char const* end) { @@ -26,16 +28,27 @@ void ServiceInterfaceBuffer::putChars(char const* begin, char const* end) { memcpy(array, begin, length); for(; begin != end; begin++){ - printChar(begin, false); + if(errStream) { + printChar(begin, true); + } + else { + printChar(begin, false); + } + } } #endif int ServiceInterfaceBuffer::overflow(int c) { - if(errStream and this->isActive) { + if(not buffered and this->isActive) { if (c != Traits::eof()) { - printChar(reinterpret_cast(&c), true); + if(errStream) { + printChar(reinterpret_cast(&c), true); + } + else { + printChar(reinterpret_cast(&c), false); + } } return 0; } @@ -53,16 +66,17 @@ int ServiceInterfaceBuffer::overflow(int c) { } int ServiceInterfaceBuffer::sync(void) { - if(not this->isActive or errStream) { - if(not errStream) { + if(not this->isActive) { + if(not buffered) { setp(buf, buf + BUF_SIZE - 1); } return 0; } - auto preamble = getPreamble(); + size_t preambleSize = 0; + auto preamble = getPreamble(&preambleSize); // Write logMessage and time - this->putChars(preamble.c_str(), preamble.c_str() + preamble.size()); + this->putChars(preamble.c_str(), preamble.c_str() + preambleSize); // Handle output this->putChars(pbase(), pptr()); // This tells that buffer is empty again @@ -71,17 +85,34 @@ int ServiceInterfaceBuffer::sync(void) { } -std::string ServiceInterfaceBuffer::getPreamble() { +std::string ServiceInterfaceBuffer::getPreamble(size_t * preambleSize) { Clock::TimeOfDay_t loggerTime; Clock::getDateAndTime(&loggerTime); - std::string preamble; + //preamble.clear(); + //std::string preamble; + size_t currentSize = 0; if(addCrToPreamble) { - preamble += "\r"; + preamble[0] = '\r'; + currentSize += 1; } - preamble += logMessage + ": | " + zero_padded(loggerTime.hour, 2) - + ":" + zero_padded(loggerTime.minute, 2) + ":" - + zero_padded(loggerTime.second, 2) + "." - + zero_padded(loggerTime.usecond/1000, 3) + " | "; + logMessage.copy(preamble.data() + 1, logMessage.size()); + currentSize += logMessage.size(); + preamble[++currentSize] = ':'; + preamble[++currentSize] = ' '; + preamble[++currentSize] = '|'; + zero_padded(loggerTime.hour, 2).copy(preamble.data() + ) + //preamble.c_str() + 1 = logMessage; +// +// + ": | " + zero_padded(loggerTime.hour, 2) +// + ":" + zero_padded(loggerTime.minute, 2) + ":" +// + zero_padded(loggerTime.second, 2) + "." +// + zero_padded(loggerTime.usecond/1000, 3) + " | "; + currentSize += logMessage.size(); //+ 4 +2 +1 +2 +1 +2 +1 + 3 + 3; + preamble[currentSize] = '\0'; + printf("%s", preamble.c_str()); + uint8_t debugArray[96]; + memcpy(debugArray, preamble.data(), currentSize); + *preambleSize = currentSize; return preamble; } diff --git a/serviceinterface/ServiceInterfaceBuffer.h b/serviceinterface/ServiceInterfaceBuffer.h index 74facafe0..ab5d8c901 100644 --- a/serviceinterface/ServiceInterfaceBuffer.h +++ b/serviceinterface/ServiceInterfaceBuffer.h @@ -19,8 +19,8 @@ class ServiceInterfaceBuffer: public std::streambuf { friend class ServiceInterfaceStream; public: - ServiceInterfaceBuffer(std::string setMessage, bool errStream, - bool addCrToPreamble, uint16_t port); + ServiceInterfaceBuffer(std::string setMessage, bool addCrToPreamble, + bool buffered, bool errStream, uint16_t port); protected: bool isActive; @@ -36,11 +36,13 @@ protected: private: //! For additional message information std::string logMessage; + std::string preamble; // For EOF detection typedef std::char_traits Traits; //! This is useful for some terminal programs which do not have //! implicit carriage return with newline characters. bool addCrToPreamble; + bool buffered; //! This specifies to print to stderr and work in unbuffered mode. bool errStream; @@ -51,7 +53,7 @@ private: //! In this function, the characters are parsed. void putChars(char const* begin, char const* end); - std::string getPreamble(); + std::string getPreamble(size_t * preambleSize = nullptr); /** * This helper function returns the zero padded string version of a number. diff --git a/serviceinterface/ServiceInterfaceStream.cpp b/serviceinterface/ServiceInterfaceStream.cpp index d9bc980a7..bef8136e1 100644 --- a/serviceinterface/ServiceInterfaceStream.cpp +++ b/serviceinterface/ServiceInterfaceStream.cpp @@ -1,15 +1,33 @@ #include ServiceInterfaceStream::ServiceInterfaceStream(std::string setMessage, - bool errStream, bool addCrToPreamble, uint16_t port) : - std::ostream(&streambuf), - streambuf(setMessage, errStream, addCrToPreamble, port) { + bool addCrToPreamble, bool buffered, bool errStream, uint16_t port) : + std::ostream(&buf), + buf(setMessage, addCrToPreamble, buffered, errStream, port) { } void ServiceInterfaceStream::setActive( bool myActive) { - this->streambuf.isActive = myActive; + this->buf.isActive = myActive; } std::string ServiceInterfaceStream::getPreamble() { - return streambuf.getPreamble(); + return buf.getPreamble(); +} + +void ServiceInterfaceStream::print(std::string error, + bool withPreamble, bool withNewline, bool flush) { + if(not buffered and withPreamble) { + *this << getPreamble() << error; + } + else { + *this << error; + } + + if(withNewline) { + *this << "\n"; + } + // if mode is non-buffered, no need to flush. + if(flush and buffered) { + this->flush(); + } } diff --git a/serviceinterface/ServiceInterfaceStream.h b/serviceinterface/ServiceInterfaceStream.h index c44fe03a0..9b8b5b6ed 100644 --- a/serviceinterface/ServiceInterfaceStream.h +++ b/serviceinterface/ServiceInterfaceStream.h @@ -5,36 +5,28 @@ #include #include -/* Unfortunately, there must be a forward declaration of the log front end - * (MUST be defined in main), to let the system know where to write to. - * The ServiceInterfaceStream instances, which are declared below and - * can instantaited somewhere else can be passed to these front ends by passing - * their underlying buffers with .rdbuf() */ -namespace sif { -extern std::ostream debug; -extern std::ostream info; -extern std::ostream warning; -extern std::ostream error; -} - - +/** + * Generic service interface stream which can be used like std::cout or + * std::cerr but has additional capability. Add preamble and timestamp + * to output. Can be run in buffered or unbuffered mode. + */ class ServiceInterfaceStream : public std::ostream { protected: - ServiceInterfaceBuffer streambuf; + ServiceInterfaceBuffer buf; public: /** * This constructor is used by specifying the preamble message. * Optionally, the output can be directed to stderr and a CR character * can be prepended to the preamble. - * @param set_message - * @param errStream - * @param addCrToPreamble - * @param port + * @param setMessage message of preamble. + * @param addCrToPreamble Useful for applications like Puttty. + * @param buffered specify whether to use buffered mode. + * @param errStream specify which output stream to use (stderr or stdout). */ ServiceInterfaceStream(std::string setMessage, - bool errStream = false, bool addCrToPreamble = false, - uint16_t port = 1234); + bool addCrToPreamble = false, bool buffered = true, + bool errStream = false, uint16_t port = 1234); //! An inactive stream will not print anything. void setActive( bool ); @@ -45,15 +37,24 @@ public: * @return Preamle consisting of log message and timestamp. */ std::string getPreamble(); + + /** + * This prints an error with a preamble. Useful if using the unbuffered + * mode. Flushes in default mode (prints immediately). + */ + void print(std::string error, bool withPreamble = true, + bool withNewline = true, bool flush = true); + + bool buffered = false; }; -// Forward declaration of interface streams. These are needed so the public -// functions can be used by including this header +// Forward declaration of interface streams. These should be instantiated in +// main. They can then be used like std::cout or std::cerr. namespace sif { -extern ServiceInterfaceStream debugStream; -extern ServiceInterfaceStream infoStream; -extern ServiceInterfaceStream warningStream; -extern ServiceInterfaceStream errorStream; +extern ServiceInterfaceStream debug; +extern ServiceInterfaceStream info; +extern ServiceInterfaceStream warning; +extern ServiceInterfaceStream error; } #endif /* FRAMEWORK_SERVICEINTERFACE_SERVICEINTERFACESTREAM_H_ */ From 966c9c3993df460de712823a43c5fef04213dd53 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Thu, 4 Jun 2020 14:22:04 +0200 Subject: [PATCH 09/59] buffer changes --- serviceinterface/ServiceInterfaceBuffer.cpp | 52 ++++++++++++++------- 1 file changed, 34 insertions(+), 18 deletions(-) diff --git a/serviceinterface/ServiceInterfaceBuffer.cpp b/serviceinterface/ServiceInterfaceBuffer.cpp index 321b1d3ca..3c365f177 100644 --- a/serviceinterface/ServiceInterfaceBuffer.cpp +++ b/serviceinterface/ServiceInterfaceBuffer.cpp @@ -95,24 +95,40 @@ std::string ServiceInterfaceBuffer::getPreamble(size_t * preambleSize) { preamble[0] = '\r'; currentSize += 1; } - logMessage.copy(preamble.data() + 1, logMessage.size()); - currentSize += logMessage.size(); - preamble[++currentSize] = ':'; - preamble[++currentSize] = ' '; - preamble[++currentSize] = '|'; - zero_padded(loggerTime.hour, 2).copy(preamble.data() + ) - //preamble.c_str() + 1 = logMessage; -// -// + ": | " + zero_padded(loggerTime.hour, 2) -// + ":" + zero_padded(loggerTime.minute, 2) + ":" -// + zero_padded(loggerTime.second, 2) + "." -// + zero_padded(loggerTime.usecond/1000, 3) + " | "; - currentSize += logMessage.size(); //+ 4 +2 +1 +2 +1 +2 +1 + 3 + 3; - preamble[currentSize] = '\0'; - printf("%s", preamble.c_str()); - uint8_t debugArray[96]; - memcpy(debugArray, preamble.data(), currentSize); - *preambleSize = currentSize; + int32_t charCount = sprintf(preamble.data() + currentSize, + "%s: | %lu:%02lu:%02lu.%03lu | ", + this->logMessage.c_str(), (unsigned long) loggerTime.hour, + (unsigned long) loggerTime.minute, + (unsigned long) loggerTime.second, + (unsigned long) loggerTime.usecond /1000); + if(charCount < 0) { + printf("ServiceInterfaceBuffer: Failure parsing preamble"); + return ""; + } + currentSize += charCount; +// size_t currentSize = 0; +// if(addCrToPreamble) { +// preamble[0] = '\r'; +// currentSize += 1; +// } +// logMessage.copy(preamble.data() + 1, logMessage.size()); +// currentSize += logMessage.size(); +// preamble[++currentSize] = ':'; +// preamble[++currentSize] = ' '; +// preamble[++currentSize] = '|'; +// zero_padded(loggerTime.hour, 2).copy(preamble.data() + ) +// //preamble.c_str() + 1 = logMessage; +//// +//// + ": | " + zero_padded(loggerTime.hour, 2) +//// + ":" + zero_padded(loggerTime.minute, 2) + ":" +//// + zero_padded(loggerTime.second, 2) + "." +//// + zero_padded(loggerTime.usecond/1000, 3) + " | "; +// currentSize += logMessage.size(); //+ 4 +2 +1 +2 +1 +2 +1 + 3 + 3; +// preamble[currentSize] = '\0'; +// printf("%s", preamble.c_str()); +// uint8_t debugArray[96]; +// memcpy(debugArray, preamble.data(), currentSize); +// *preambleSize = currentSize; return preamble; } From 1cb241ca0cb3ab729b4bf67167a3e36f5dc2bcb3 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Thu, 4 Jun 2020 17:30:09 +0200 Subject: [PATCH 10/59] zero padded not using dyn mem alloc now --- serviceinterface/ServiceInterfaceBuffer.h | 31 ++++++++++++++++------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/serviceinterface/ServiceInterfaceBuffer.h b/serviceinterface/ServiceInterfaceBuffer.h index ab5d8c901..a032691ce 100644 --- a/serviceinterface/ServiceInterfaceBuffer.h +++ b/serviceinterface/ServiceInterfaceBuffer.h @@ -1,6 +1,7 @@ #ifndef FRAMEWORK_SERVICEINTERFACE_SERVICEINTERFACEBUFFER_H_ #define FRAMEWORK_SERVICEINTERFACE_SERVICEINTERFACEBUFFER_H_ +#include #include #include #include @@ -58,21 +59,33 @@ private: /** * This helper function returns the zero padded string version of a number. * The type is deduced automatically. + * TODO: This uses dynamic memory allocation, so we should provide + * a custom streambuf class to use it (which takes maxSize as argument) + * Then we would propably * @tparam T * @param num * @param width * @return */ template - std::string zero_padded(const T& num, uint8_t width) { - std::ostringstream string_to_pad; - string_to_pad << std::setw(width) << std::setfill('0') << num; - std::string result = string_to_pad.str(); - if (result.length() > width) - { - result.erase(0, result.length() - width); - } - return result; + ReturnValue_t zeroPadded(std::string stringToFill, const T& num, + uint8_t width) { + auto numString = std::to_string(num); + uint8_t i = 0; + for(i = 0; i < width; i++) { + stringToFill[i] = '0'; + } + numString.copy(stringToFill.data() + i, numString.size()); +// std::streambuf streambuf; +// std::ostringstream string_to_pad; +// string_to_pad << std::setw(width) << std::setfill('0') << num; +// std::string result = string_to_pad.str(); +// if (result.length() > width) +// { +// result.erase(0, result.length() - width); +// } +// return result; + //std::string dest = std::string( number_of_zeros, '0').append( original_string); } }; From d466921aa0f5ac1adc8fe8b95f83d8b518eb7653 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Thu, 4 Jun 2020 17:58:22 +0200 Subject: [PATCH 11/59] some more experiments --- serviceinterface/ServiceInterfaceBuffer.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/serviceinterface/ServiceInterfaceBuffer.cpp b/serviceinterface/ServiceInterfaceBuffer.cpp index 3c365f177..7cf75a36f 100644 --- a/serviceinterface/ServiceInterfaceBuffer.cpp +++ b/serviceinterface/ServiceInterfaceBuffer.cpp @@ -75,6 +75,8 @@ int ServiceInterfaceBuffer::sync(void) { size_t preambleSize = 0; auto preamble = getPreamble(&preambleSize); + uint8_t debugArray[96]; + memcpy(debugArray, preamble.data(), preambleSize); // Write logMessage and time this->putChars(preamble.c_str(), preamble.c_str() + preambleSize); // Handle output @@ -124,12 +126,11 @@ std::string ServiceInterfaceBuffer::getPreamble(size_t * preambleSize) { //// + zero_padded(loggerTime.second, 2) + "." //// + zero_padded(loggerTime.usecond/1000, 3) + " | "; // currentSize += logMessage.size(); //+ 4 +2 +1 +2 +1 +2 +1 + 3 + 3; -// preamble[currentSize] = '\0'; -// printf("%s", preamble.c_str()); -// uint8_t debugArray[96]; -// memcpy(debugArray, preamble.data(), currentSize); -// *preambleSize = currentSize; - return preamble; + preamble[++currentSize] = '\0'; + uint8_t debugArray[96]; + memcpy(debugArray, preamble.data(), currentSize); + *preambleSize = currentSize; + return std::move(preamble); } From c0808e71d99324fd40082bbbbb16d4cbbff9a7ab Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Thu, 4 Jun 2020 19:07:04 +0200 Subject: [PATCH 12/59] no dyn memory allocation, print seems to work --- objectmanager/ObjectManagerIF.h | 1 - serviceinterface/ServiceInterfaceBuffer.cpp | 48 +++++++-------------- serviceinterface/ServiceInterfaceBuffer.h | 18 +------- 3 files changed, 17 insertions(+), 50 deletions(-) diff --git a/objectmanager/ObjectManagerIF.h b/objectmanager/ObjectManagerIF.h index aca24a24a..830aa4006 100644 --- a/objectmanager/ObjectManagerIF.h +++ b/objectmanager/ObjectManagerIF.h @@ -9,7 +9,6 @@ #define OBJECTMANAGERIF_H_ #include -#include #include #include diff --git a/serviceinterface/ServiceInterfaceBuffer.cpp b/serviceinterface/ServiceInterfaceBuffer.cpp index 7cf75a36f..30149ce0b 100644 --- a/serviceinterface/ServiceInterfaceBuffer.cpp +++ b/serviceinterface/ServiceInterfaceBuffer.cpp @@ -1,6 +1,7 @@ #include #include #include +#include // to be implemented by bsp extern "C" void printChar(const char*, bool errStream); @@ -17,6 +18,7 @@ ServiceInterfaceBuffer::ServiceInterfaceBuffer(std::string setMessage, setp( buf, buf + BUF_SIZE ); } preamble.reserve(96); + preamble.resize(96); } void ServiceInterfaceBuffer::putChars(char const* begin, char const* end) { @@ -78,7 +80,7 @@ int ServiceInterfaceBuffer::sync(void) { uint8_t debugArray[96]; memcpy(debugArray, preamble.data(), preambleSize); // Write logMessage and time - this->putChars(preamble.c_str(), preamble.c_str() + preambleSize); + this->putChars(preamble.data(), preamble.data() + preambleSize); // Handle output this->putChars(pbase(), pptr()); // This tells that buffer is empty again @@ -90,47 +92,29 @@ int ServiceInterfaceBuffer::sync(void) { std::string ServiceInterfaceBuffer::getPreamble(size_t * preambleSize) { Clock::TimeOfDay_t loggerTime; Clock::getDateAndTime(&loggerTime); - //preamble.clear(); - //std::string preamble; + std::string preamble (96, 0); size_t currentSize = 0; + char* parsePosition = &preamble[0]; if(addCrToPreamble) { preamble[0] = '\r'; currentSize += 1; + parsePosition += 1; } - int32_t charCount = sprintf(preamble.data() + currentSize, - "%s: | %lu:%02lu:%02lu.%03lu | ", - this->logMessage.c_str(), (unsigned long) loggerTime.hour, - (unsigned long) loggerTime.minute, - (unsigned long) loggerTime.second, - (unsigned long) loggerTime.usecond /1000); + int32_t charCount = sprintf(parsePosition, + "%s: | %02" SCNu32 ":%02" SCNu32 ":%02" SCNu32 ".%03" SCNu32 " | ", + this->logMessage.c_str(), loggerTime.hour, + loggerTime.minute, + loggerTime.second, + loggerTime.usecond /1000); if(charCount < 0) { printf("ServiceInterfaceBuffer: Failure parsing preamble"); return ""; } currentSize += charCount; -// size_t currentSize = 0; -// if(addCrToPreamble) { -// preamble[0] = '\r'; -// currentSize += 1; -// } -// logMessage.copy(preamble.data() + 1, logMessage.size()); -// currentSize += logMessage.size(); -// preamble[++currentSize] = ':'; -// preamble[++currentSize] = ' '; -// preamble[++currentSize] = '|'; -// zero_padded(loggerTime.hour, 2).copy(preamble.data() + ) -// //preamble.c_str() + 1 = logMessage; -//// -//// + ": | " + zero_padded(loggerTime.hour, 2) -//// + ":" + zero_padded(loggerTime.minute, 2) + ":" -//// + zero_padded(loggerTime.second, 2) + "." -//// + zero_padded(loggerTime.usecond/1000, 3) + " | "; -// currentSize += logMessage.size(); //+ 4 +2 +1 +2 +1 +2 +1 + 3 + 3; - preamble[++currentSize] = '\0'; - uint8_t debugArray[96]; - memcpy(debugArray, preamble.data(), currentSize); - *preambleSize = currentSize; - return std::move(preamble); + if(preambleSize != nullptr) { + *preambleSize = currentSize; + } + return preamble; } diff --git a/serviceinterface/ServiceInterfaceBuffer.h b/serviceinterface/ServiceInterfaceBuffer.h index a032691ce..9a4ae4798 100644 --- a/serviceinterface/ServiceInterfaceBuffer.h +++ b/serviceinterface/ServiceInterfaceBuffer.h @@ -76,29 +76,13 @@ private: stringToFill[i] = '0'; } numString.copy(stringToFill.data() + i, numString.size()); -// std::streambuf streambuf; -// std::ostringstream string_to_pad; -// string_to_pad << std::setw(width) << std::setfill('0') << num; -// std::string result = string_to_pad.str(); -// if (result.length() > width) -// { -// result.erase(0, result.length() - width); -// } -// return result; - //std::string dest = std::string( number_of_zeros, '0').append( original_string); + return HasReturnvaluesIF::RETURN_OK; } }; #endif - - - - - - - #ifdef UT699 class ServiceInterfaceBuffer: public std::basic_streambuf > { From 3a573c1b4c460cb19163d8bb9f319b501657f1e8 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Thu, 4 Jun 2020 19:37:33 +0200 Subject: [PATCH 13/59] no run-time dyn memory allocation now --- serviceinterface/ServiceInterfaceBuffer.cpp | 8 +++++- serviceinterface/ServiceInterfaceBuffer.h | 28 ++---------------- serviceinterface/ServiceInterfaceStream.cpp | 13 ++++----- serviceinterface/ServiceInterfaceStream.h | 32 ++++++++++----------- 4 files changed, 31 insertions(+), 50 deletions(-) diff --git a/serviceinterface/ServiceInterfaceBuffer.cpp b/serviceinterface/ServiceInterfaceBuffer.cpp index 30149ce0b..f1332637c 100644 --- a/serviceinterface/ServiceInterfaceBuffer.cpp +++ b/serviceinterface/ServiceInterfaceBuffer.cpp @@ -68,12 +68,15 @@ int ServiceInterfaceBuffer::overflow(int c) { } int ServiceInterfaceBuffer::sync(void) { - if(not this->isActive) { + if(not this->isActive and not buffered) { if(not buffered) { setp(buf, buf + BUF_SIZE - 1); } return 0; } + if(not buffered) { + return 0; + } size_t preambleSize = 0; auto preamble = getPreamble(&preambleSize); @@ -88,6 +91,9 @@ int ServiceInterfaceBuffer::sync(void) { return 0; } +bool ServiceInterfaceBuffer::isBuffered() const { + return buffered; +} std::string ServiceInterfaceBuffer::getPreamble(size_t * preambleSize) { Clock::TimeOfDay_t loggerTime; diff --git a/serviceinterface/ServiceInterfaceBuffer.h b/serviceinterface/ServiceInterfaceBuffer.h index 9a4ae4798..00a8e74a3 100644 --- a/serviceinterface/ServiceInterfaceBuffer.h +++ b/serviceinterface/ServiceInterfaceBuffer.h @@ -34,6 +34,7 @@ protected: //! for example when std::endl is put to stream. int sync(void) override; + bool isBuffered() const; private: //! For additional message information std::string logMessage; @@ -43,10 +44,10 @@ private: //! This is useful for some terminal programs which do not have //! implicit carriage return with newline characters. bool addCrToPreamble; - bool buffered; + //! This specifies to print to stderr and work in unbuffered mode. bool errStream; - + bool buffered; // Work in buffer mode. It is also possible to work without buffer. static size_t const BUF_SIZE = 128; char buf[BUF_SIZE]; @@ -55,29 +56,6 @@ private: void putChars(char const* begin, char const* end); std::string getPreamble(size_t * preambleSize = nullptr); - - /** - * This helper function returns the zero padded string version of a number. - * The type is deduced automatically. - * TODO: This uses dynamic memory allocation, so we should provide - * a custom streambuf class to use it (which takes maxSize as argument) - * Then we would propably - * @tparam T - * @param num - * @param width - * @return - */ - template - ReturnValue_t zeroPadded(std::string stringToFill, const T& num, - uint8_t width) { - auto numString = std::to_string(num); - uint8_t i = 0; - for(i = 0; i < width; i++) { - stringToFill[i] = '0'; - } - numString.copy(stringToFill.data() + i, numString.size()); - return HasReturnvaluesIF::RETURN_OK; - } }; #endif diff --git a/serviceinterface/ServiceInterfaceStream.cpp b/serviceinterface/ServiceInterfaceStream.cpp index bef8136e1..76481ed12 100644 --- a/serviceinterface/ServiceInterfaceStream.cpp +++ b/serviceinterface/ServiceInterfaceStream.cpp @@ -2,21 +2,20 @@ ServiceInterfaceStream::ServiceInterfaceStream(std::string setMessage, bool addCrToPreamble, bool buffered, bool errStream, uint16_t port) : - std::ostream(&buf), - buf(setMessage, addCrToPreamble, buffered, errStream, port) { -} + std::ostream(&streambuf), + streambuf(setMessage, addCrToPreamble, buffered, errStream, port) {} void ServiceInterfaceStream::setActive( bool myActive) { - this->buf.isActive = myActive; + this->streambuf.isActive = myActive; } std::string ServiceInterfaceStream::getPreamble() { - return buf.getPreamble(); + return streambuf.getPreamble(); } void ServiceInterfaceStream::print(std::string error, bool withPreamble, bool withNewline, bool flush) { - if(not buffered and withPreamble) { + if(not streambuf.isBuffered() and withPreamble) { *this << getPreamble() << error; } else { @@ -27,7 +26,7 @@ void ServiceInterfaceStream::print(std::string error, *this << "\n"; } // if mode is non-buffered, no need to flush. - if(flush and buffered) { + if(flush and streambuf.isBuffered()) { this->flush(); } } diff --git a/serviceinterface/ServiceInterfaceStream.h b/serviceinterface/ServiceInterfaceStream.h index 9b8b5b6ed..9e19c228c 100644 --- a/serviceinterface/ServiceInterfaceStream.h +++ b/serviceinterface/ServiceInterfaceStream.h @@ -10,25 +10,22 @@ * std::cerr but has additional capability. Add preamble and timestamp * to output. Can be run in buffered or unbuffered mode. */ -class ServiceInterfaceStream : - public std::ostream { -protected: - ServiceInterfaceBuffer buf; +class ServiceInterfaceStream : public std::ostream { public: - /** - * This constructor is used by specifying the preamble message. - * Optionally, the output can be directed to stderr and a CR character - * can be prepended to the preamble. - * @param setMessage message of preamble. - * @param addCrToPreamble Useful for applications like Puttty. - * @param buffered specify whether to use buffered mode. - * @param errStream specify which output stream to use (stderr or stdout). - */ - ServiceInterfaceStream(std::string setMessage, - bool addCrToPreamble = false, bool buffered = true, + /** + * This constructor is used by specifying the preamble message. + * Optionally, the output can be directed to stderr and a CR character + * can be prepended to the preamble. + * @param setMessage message of preamble. + * @param addCrToPreamble Useful for applications like Puttty. + * @param buffered specify whether to use buffered mode. + * @param errStream specify which output stream to use (stderr or stdout). + */ + ServiceInterfaceStream(std::string setMessage, + bool addCrToPreamble = false, bool buffered = true, bool errStream = false, uint16_t port = 1234); - //! An inactive stream will not print anything. + //! An inactive stream will not print anything. void setActive( bool ); /** @@ -45,7 +42,8 @@ public: void print(std::string error, bool withPreamble = true, bool withNewline = true, bool flush = true); - bool buffered = false; +protected: + ServiceInterfaceBuffer streambuf; }; // Forward declaration of interface streams. These should be instantiated in From 7014833c1c89938d0475e88a7a57b628933051e5 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Thu, 4 Jun 2020 19:50:56 +0200 Subject: [PATCH 14/59] improvements and fixes --- serviceinterface/ServiceInterfaceBuffer.cpp | 15 ++++++++------- serviceinterface/ServiceInterfaceBuffer.h | 9 +++++++-- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/serviceinterface/ServiceInterfaceBuffer.cpp b/serviceinterface/ServiceInterfaceBuffer.cpp index f1332637c..5c80862c2 100644 --- a/serviceinterface/ServiceInterfaceBuffer.cpp +++ b/serviceinterface/ServiceInterfaceBuffer.cpp @@ -17,8 +17,8 @@ ServiceInterfaceBuffer::ServiceInterfaceBuffer(std::string setMessage, // Set pointers if the stream is buffered. setp( buf, buf + BUF_SIZE ); } - preamble.reserve(96); - preamble.resize(96); + preamble.reserve(MAX_PREAMBLE_SIZE); + preamble.resize(MAX_PREAMBLE_SIZE); } void ServiceInterfaceBuffer::putChars(char const* begin, char const* end) { @@ -36,7 +36,6 @@ void ServiceInterfaceBuffer::putChars(char const* begin, char const* end) { else { printChar(begin, false); } - } } @@ -80,8 +79,6 @@ int ServiceInterfaceBuffer::sync(void) { size_t preambleSize = 0; auto preamble = getPreamble(&preambleSize); - uint8_t debugArray[96]; - memcpy(debugArray, preamble.data(), preambleSize); // Write logMessage and time this->putChars(preamble.data(), preamble.data() + preambleSize); // Handle output @@ -98,7 +95,6 @@ bool ServiceInterfaceBuffer::isBuffered() const { std::string ServiceInterfaceBuffer::getPreamble(size_t * preambleSize) { Clock::TimeOfDay_t loggerTime; Clock::getDateAndTime(&loggerTime); - std::string preamble (96, 0); size_t currentSize = 0; char* parsePosition = &preamble[0]; if(addCrToPreamble) { @@ -113,7 +109,12 @@ std::string ServiceInterfaceBuffer::getPreamble(size_t * preambleSize) { loggerTime.second, loggerTime.usecond /1000); if(charCount < 0) { - printf("ServiceInterfaceBuffer: Failure parsing preamble"); + printf("ServiceInterfaceBuffer: Failure parsing preamble\r\n"); + return ""; + } + if(charCount > MAX_PREAMBLE_SIZE) { + printf("ServiceInterfaceBuffer: Char count too large for maximum " + "preamble size"); return ""; } currentSize += charCount; diff --git a/serviceinterface/ServiceInterfaceBuffer.h b/serviceinterface/ServiceInterfaceBuffer.h index 00a8e74a3..39ea25c2d 100644 --- a/serviceinterface/ServiceInterfaceBuffer.h +++ b/serviceinterface/ServiceInterfaceBuffer.h @@ -20,6 +20,8 @@ class ServiceInterfaceBuffer: public std::streambuf { friend class ServiceInterfaceStream; public: + static constexpr uint8_t MAX_PREAMBLE_SIZE = 40; + ServiceInterfaceBuffer(std::string setMessage, bool addCrToPreamble, bool buffered, bool errStream, uint16_t port); @@ -41,14 +43,17 @@ private: std::string preamble; // For EOF detection typedef std::char_traits Traits; + //! This is useful for some terminal programs which do not have //! implicit carriage return with newline characters. bool addCrToPreamble; + //! Specifies whether the stream operates in buffered or unbuffered mode. + bool buffered; //! This specifies to print to stderr and work in unbuffered mode. bool errStream; - bool buffered; - // Work in buffer mode. It is also possible to work without buffer. + + //! Needed for buffered mode. static size_t const BUF_SIZE = 128; char buf[BUF_SIZE]; From 639b517edaffc63094870cd1bcdcdb9bed9229ca Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Thu, 4 Jun 2020 19:57:25 +0200 Subject: [PATCH 15/59] removed unnecessary change --- osal/FreeRTOS/FixedTimeslotTask.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osal/FreeRTOS/FixedTimeslotTask.cpp b/osal/FreeRTOS/FixedTimeslotTask.cpp index 549ffbf17..fb3c3b03e 100644 --- a/osal/FreeRTOS/FixedTimeslotTask.cpp +++ b/osal/FreeRTOS/FixedTimeslotTask.cpp @@ -68,7 +68,7 @@ ReturnValue_t FixedTimeslotTask::addSlot(object_id_t componentId, } sif::error << "Component " << std::hex << componentId << - " not found, not adding it to pst\r" << std::endl; + " not found, not adding it to pst" << std::endl; return HasReturnvaluesIF::RETURN_FAILED; } From 860cdba94d4cb6d1de1ba59f4a0f1dd68f761549 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 10 Jun 2020 20:28:44 +0200 Subject: [PATCH 16/59] subservicve passed to handleRequest() --- tmtcservices/PusServiceBase.cpp | 3 ++- tmtcservices/PusServiceBase.h | 7 +++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/tmtcservices/PusServiceBase.cpp b/tmtcservices/PusServiceBase.cpp index b1d972933..c27defe61 100644 --- a/tmtcservices/PusServiceBase.cpp +++ b/tmtcservices/PusServiceBase.cpp @@ -29,7 +29,8 @@ ReturnValue_t PusServiceBase::performOperation(uint8_t opCode) { this->currentPacket.setStoreAddress(message.getStorageId()); // info << "Service " << (uint16_t) this->serviceId << ": new packet!" << std::endl; - ReturnValue_t return_code = this->handleRequest(); + ReturnValue_t return_code = this->handleRequest(currentPacket.getSubService()); + // debug << "Service " << (uint16_t)this->serviceId << ": handleRequest returned: " << (int)return_code << std::endl; if (return_code == RETURN_OK) { this->verifyReporter.sendSuccessReport( diff --git a/tmtcservices/PusServiceBase.h b/tmtcservices/PusServiceBase.h index d318ff1e0..dbb1754ce 100644 --- a/tmtcservices/PusServiceBase.h +++ b/tmtcservices/PusServiceBase.h @@ -30,7 +30,10 @@ void setStaticFrameworkObjectIds(); * All PUS Services are System Objects, so an Object ID needs to be specified on construction. * \ingroup pus_services */ -class PusServiceBase : public ExecutableObjectIF, public AcceptsTelecommandsIF, public SystemObject, public HasReturnvaluesIF { +class PusServiceBase : public ExecutableObjectIF, + public AcceptsTelecommandsIF, + public SystemObject, + public HasReturnvaluesIF { friend void (Factory::setStaticFrameworkObjectIds)(); public: /** @@ -63,7 +66,7 @@ public: * @return The returned status_code is directly taken as main error code in the Verification Report. * On success, RETURN_OK shall be returned. */ - virtual ReturnValue_t handleRequest() = 0; + virtual ReturnValue_t handleRequest(uint8_t subservice) = 0; /** * In performService, implementations can handle periodic, non-TC-triggered activities. * The performService method is always called. From ed7b4e2a3a88142ff6ff1aed74e3f0cb009c85ea Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 10 Jun 2020 20:49:30 +0200 Subject: [PATCH 17/59] PSB improvements --- objectmanager/ObjectManager.cpp | 1 + tmtcservices/PusServiceBase.cpp | 74 +++++++++++++++++++-------------- tmtcservices/PusServiceBase.h | 8 ++-- 3 files changed, 49 insertions(+), 34 deletions(-) diff --git a/objectmanager/ObjectManager.cpp b/objectmanager/ObjectManager.cpp index cfdf65621..1c54355a3 100644 --- a/objectmanager/ObjectManager.cpp +++ b/objectmanager/ObjectManager.cpp @@ -1,5 +1,6 @@ #include #include +#include #include ObjectManager::ObjectManager( void (*setProducer)() ): diff --git a/tmtcservices/PusServiceBase.cpp b/tmtcservices/PusServiceBase.cpp index c27defe61..f5d1e30bd 100644 --- a/tmtcservices/PusServiceBase.cpp +++ b/tmtcservices/PusServiceBase.cpp @@ -9,10 +9,11 @@ object_id_t PusServiceBase::packetSource = 0; object_id_t PusServiceBase::packetDestination = 0; -PusServiceBase::PusServiceBase(object_id_t setObjectId, uint16_t setApid, uint8_t setServiceId) : - SystemObject(setObjectId), apid(setApid), serviceId(setServiceId), errorParameter1( - 0), errorParameter2(0), requestQueue(NULL) { - requestQueue = QueueFactory::instance()->createMessageQueue(PUS_SERVICE_MAX_RECEPTION); +PusServiceBase::PusServiceBase(object_id_t setObjectId, uint16_t setApid, + uint8_t setServiceId) : + SystemObject(setObjectId), apid(setApid), serviceId(setServiceId) { + requestQueue = QueueFactory::instance()-> + createMessageQueue(PUS_SERVICE_MAX_RECEPTION); } PusServiceBase::~PusServiceBase() { @@ -20,51 +21,60 @@ PusServiceBase::~PusServiceBase() { } ReturnValue_t PusServiceBase::performOperation(uint8_t opCode) { + handleRequestQueue(); + ReturnValue_t result = this->performService(); + if (result != RETURN_OK) { + sif::error << "PusService " << (uint16_t) this->serviceId + << ": performService returned with " << (int16_t) result + << std::endl; + return RETURN_FAILED; + } + return RETURN_OK; +} + +void PusServiceBase::handleRequestQueue() { TmTcMessage message; + ReturnValue_t result = RETURN_FAILED; 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(currentPacket.getSubService()); + result = this->handleRequest(currentPacket.getSubService()); - // debug << "Service " << (uint16_t)this->serviceId << ": handleRequest returned: " << (int)return_code << std::endl; - if (return_code == RETURN_OK) { + // debug << "Service " << (uint16_t)this->serviceId << + // ": handleRequest returned: " << (int)return_code << std::endl; + if (result == RETURN_OK) { this->verifyReporter.sendSuccessReport( TC_VERIFY::COMPLETION_SUCCESS, &this->currentPacket); - } else { + } + else { this->verifyReporter.sendFailureReport( TC_VERIFY::COMPLETION_FAILURE, &this->currentPacket, - return_code, 0, errorParameter1, errorParameter2); + result, 0, errorParameter1, errorParameter2); } this->currentPacket.deletePacket(); errorParameter1 = 0; errorParameter2 = 0; - } else if (status == MessageQueueIF::EMPTY) { + } + 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 { - + } + else { sif::error << "PusServiceBase::performOperation: Service " << (uint16_t) this->serviceId << ": Error receiving packet. Code: " << std::hex << status << std::dec << std::endl; } } - ReturnValue_t return_code = this->performService(); - if (return_code == RETURN_OK) { - return RETURN_OK; - } else { - - sif::error << "PusService " << (uint16_t) this->serviceId - << ": performService returned with " << (int16_t) return_code - << std::endl; - return RETURN_FAILED; - } } uint16_t PusServiceBase::getIdentifier() { @@ -80,19 +90,21 @@ ReturnValue_t PusServiceBase::initialize() { if (result != RETURN_OK) { return result; } - AcceptsTelemetryIF* dest_service = objectManager->get( + AcceptsTelemetryIF* destService = objectManager->get( packetDestination); PUSDistributorIF* distributor = objectManager->get( packetSource); - if ((dest_service != NULL) && (distributor != NULL)) { + if ((destService != nullptr) && (distributor != nullptr)) { this->requestQueue->setDefaultDestination( - dest_service->getReportReceptionQueue()); + destService->getReportReceptionQueue()); distributor->registerService(this); return RETURN_OK; - } else { + } + else { sif::error << "PusServiceBase::PusServiceBase: Service " << (uint32_t) this->serviceId << ": Configuration error." - << " Make sure packetSource and packetDestination are defined correctly" << std::endl; + << " Make sure packetSource and packetDestination are defined " + "correctly" << std::endl; return RETURN_FAILED; } } diff --git a/tmtcservices/PusServiceBase.h b/tmtcservices/PusServiceBase.h index dbb1754ce..5f741410a 100644 --- a/tmtcservices/PusServiceBase.h +++ b/tmtcservices/PusServiceBase.h @@ -97,16 +97,16 @@ protected: /** * One of two error parameters for additional error information. */ - uint32_t errorParameter1; + uint32_t errorParameter1 = 0; /** * One of two error parameters for additional error information. */ - uint32_t errorParameter2; + uint32_t errorParameter2 = 0; /** * This is a complete instance of the Telecommand reception queue of the class. * It is initialized on construction of the class. */ - MessageQueueIF* requestQueue; + MessageQueueIF* requestQueue = nullptr; /** * An instance of the VerificationReporter class, that simplifies sending any kind of * Verification Message to the TC Verification Service. @@ -127,6 +127,8 @@ private: * Remember that one packet must be completely handled in one #handleRequest call. */ static const uint8_t PUS_SERVICE_MAX_RECEPTION = 10; + + void handleRequestQueue(); }; #endif /* PUSSERVICEBASE_H_ */ From 39d5fe34bb3ee1ddda0d8724ec4beb3459eed783 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 10 Jun 2020 21:36:21 +0200 Subject: [PATCH 18/59] better include guard, doc form improvement --- tmtcservices/PusServiceBase.h | 80 +++++++++++++++++++++-------------- 1 file changed, 48 insertions(+), 32 deletions(-) diff --git a/tmtcservices/PusServiceBase.h b/tmtcservices/PusServiceBase.h index 5f741410a..39600c413 100644 --- a/tmtcservices/PusServiceBase.h +++ b/tmtcservices/PusServiceBase.h @@ -1,5 +1,5 @@ -#ifndef PUSSERVICEBASE_H_ -#define PUSSERVICEBASE_H_ +#ifndef FRAMEWORK_TMTCSERVICES_PUSSERVICEBASE_H_ +#define FRAMEWORK_TMTCSERVICES_PUSSERVICEBASE_H_ #include #include @@ -9,7 +9,6 @@ #include #include #include -#include #include namespace Factory{ @@ -17,18 +16,22 @@ void setStaticFrameworkObjectIds(); } /** - * \defgroup pus_services PUS Service Framework + * @defgroup pus_services PUS Service Framework * These group contains all implementations of PUS Services in the OBSW. * Most of the Services are directly taken from the ECSS PUS Standard. */ /** - * \brief This class is the basis for all PUS Services, which can immediately process Telecommand Packets. - * It manages Telecommand reception and the generation of Verification Reports. Every class that inherits - * from this abstract class has to implement handleRequest and performService. Services that are created with this + * @brief This class is the basis for all PUS Services, + * which can immediately process Telecommand Packets. + * @details + * It manages Telecommand reception and the generation of Verification Reports. + * Every class that inherits from this abstract class has to implement + * handleRequest and performService. Services that are created with this * Base class have to handle any kind of request immediately on reception. - * All PUS Services are System Objects, so an Object ID needs to be specified on construction. - * \ingroup pus_services + * All PUS Services are System Objects, so an Object ID needs to be specified + * on construction. + * @ingroup pus_services */ class PusServiceBase : public ExecutableObjectIF, public AcceptsTelecommandsIF, @@ -37,49 +40,61 @@ class PusServiceBase : public ExecutableObjectIF, friend void (Factory::setStaticFrameworkObjectIds)(); public: /** - * The constructor for the class. - * The passed values are set, but inter-object initialization is done in the initialize method. - * @param setObjectId The system object identifier of this Service instance. - * @param set_apid The APID the Service is instantiated for. - * @param set_service_id The Service Identifier as specified in ECSS PUS. + * @brief The passed values are set, but inter-object initialization is + * done in the initialize method. + * @param setObjectId + * The system object identifier of this Service instance. + * @param setApid + * The APID the Service is instantiated for. + * @param setServiceId + * The Service Identifier as specified in ECSS PUS. */ - PusServiceBase( object_id_t setObjectId, uint16_t setApid, uint8_t setServiceId); + PusServiceBase( object_id_t setObjectId, uint16_t setApid, + uint8_t setServiceId); /** * The destructor is empty. */ virtual ~PusServiceBase(); /** - * @brief 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. + * 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 by setting #errorParameters which are - * attached to the generated verification message. + * The child class may add additional error information by setting + * #errorParameters which aren 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. + * @return The returned status_code is directly taken as main error code + * in the Verification Report. * On success, RETURN_OK shall be returned. */ virtual ReturnValue_t handleRequest(uint8_t subservice) = 0; /** - * In performService, implementations can handle periodic, non-TC-triggered activities. + * In performService, implementations can handle periodic, + * non-TC-triggered activities. * The performService method is always called. - * @return A success or failure code that does not trigger any kind of verification message. + * @return Currently, everything other that RETURN_OK only triggers + * diagnostic output. */ virtual ReturnValue_t performService() = 0; /** * This method implements the typical activity of a simple PUS Service. - * It checks for new requests, and, if found, calls handleRequest, sends completion verification messages and deletes + * 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(); @@ -103,13 +118,13 @@ protected: */ uint32_t errorParameter2 = 0; /** - * This is a complete instance of the Telecommand reception queue of the class. - * It is initialized on construction of the class. + * This is a complete instance of the telecommand reception queue + * of the class. It is initialized on construction of the class. */ MessageQueueIF* requestQueue = nullptr; /** - * An instance of the VerificationReporter class, that simplifies sending any kind of - * Verification Message to the TC Verification Service. + * An instance of the VerificationReporter class, that simplifies + * sending any kind of verification message to the TC Verification Service. */ VerificationReporter verifyReporter; /** @@ -124,7 +139,8 @@ protected: private: /** * This constant sets the maximum number of packets accepted per call. - * Remember that one packet must be completely handled in one #handleRequest call. + * Remember that one packet must be completely handled in one + * #handleRequest call. */ static const uint8_t PUS_SERVICE_MAX_RECEPTION = 10; From 5dc2133c3a4218f5027955fc77c3a505f0e948f4 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 10 Jun 2020 21:41:48 +0200 Subject: [PATCH 19/59] CSB improvements --- tmtcservices/CommandingServiceBase.cpp | 56 +++++++++++++------------- tmtcservices/CommandingServiceBase.h | 48 +++++++++++----------- 2 files changed, 54 insertions(+), 50 deletions(-) diff --git a/tmtcservices/CommandingServiceBase.cpp b/tmtcservices/CommandingServiceBase.cpp index d70b90428..adc9400fa 100644 --- a/tmtcservices/CommandingServiceBase.cpp +++ b/tmtcservices/CommandingServiceBase.cpp @@ -218,9 +218,9 @@ void CommandingServiceBase::handleRequestQueue() { } -void CommandingServiceBase::sendTmPacket(uint8_t subservice, - const uint8_t* data, uint32_t dataLen, const uint8_t* headerData, - uint32_t headerSize) { +ReturnValue_t CommandingServiceBase::sendTmPacket(uint8_t subservice, + const uint8_t* data, size_t dataLen, const uint8_t* headerData, + size_t headerSize) { TmPacketStored tmPacketStored(this->apid, this->service, subservice, this->tmPacketCounter, data, dataLen, headerData, headerSize); ReturnValue_t result = tmPacketStored.sendPacket( @@ -228,36 +228,38 @@ void CommandingServiceBase::sendTmPacket(uint8_t subservice, if (result == HasReturnvaluesIF::RETURN_OK) { this->tmPacketCounter++; } + return result; } -void CommandingServiceBase::sendTmPacket(uint8_t subservice, - object_id_t objectId, const uint8_t *data, uint32_t dataLen) { - uint8_t buffer[sizeof(object_id_t)]; - uint8_t* pBuffer = buffer; - uint32_t size = 0; - SerializeAdapter::serialize(&objectId, &pBuffer, &size, - sizeof(object_id_t), true); - TmPacketStored tmPacketStored(this->apid, this->service, subservice, - this->tmPacketCounter, data, dataLen, buffer, size); - ReturnValue_t result = tmPacketStored.sendPacket( - requestQueue->getDefaultDestination(), requestQueue->getId()); - if (result == HasReturnvaluesIF::RETURN_OK) { - this->tmPacketCounter++; - } - +ReturnValue_t CommandingServiceBase::sendTmPacket(uint8_t subservice, + object_id_t objectId, const uint8_t *data, size_t dataLen) { + uint8_t buffer[sizeof(object_id_t)]; + uint8_t* pBuffer = buffer; + size_t size = 0; + SerializeAdapter::serialize(&objectId, &pBuffer, &size, + sizeof(object_id_t), true); + TmPacketStored tmPacketStored(this->apid, this->service, subservice, + this->tmPacketCounter, data, dataLen, buffer, size); + ReturnValue_t result = tmPacketStored.sendPacket( + requestQueue->getDefaultDestination(), requestQueue->getId()); + if (result == HasReturnvaluesIF::RETURN_OK) { + this->tmPacketCounter++; + } + return result; } -void CommandingServiceBase::sendTmPacket(uint8_t subservice, - SerializeIF* content, SerializeIF* header) { - TmPacketStored tmPacketStored(this->apid, this->service, subservice, - this->tmPacketCounter, content, header); - ReturnValue_t result = tmPacketStored.sendPacket( - requestQueue->getDefaultDestination(), requestQueue->getId()); - if (result == HasReturnvaluesIF::RETURN_OK) { - this->tmPacketCounter++; - } +ReturnValue_t CommandingServiceBase::sendTmPacket(uint8_t subservice, + SerializeIF* content, SerializeIF* header) { + TmPacketStored tmPacketStored(this->apid, this->service, subservice, + this->tmPacketCounter, content, header); + ReturnValue_t result = tmPacketStored.sendPacket( + requestQueue->getDefaultDestination(), requestQueue->getId()); + if (result == HasReturnvaluesIF::RETURN_OK) { + this->tmPacketCounter++; + } + return result; } diff --git a/tmtcservices/CommandingServiceBase.h b/tmtcservices/CommandingServiceBase.h index bf2e82423..e79e806de 100644 --- a/tmtcservices/CommandingServiceBase.h +++ b/tmtcservices/CommandingServiceBase.h @@ -125,7 +125,7 @@ protected: * - @c CSB or implementation specific return codes */ virtual ReturnValue_t getMessageQueueAndObject(uint8_t subservice, - const uint8_t *tcData, uint32_t tcDataLen, MessageQueueId_t *id, + const uint8_t *tcData, size_t tcDataLen, MessageQueueId_t *id, object_id_t *objectId) = 0; /** @@ -133,18 +133,16 @@ 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[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 message + * @param subservice + * @param tcData + * @param tcDataLen + * @param state * @param objectId Target object ID - * @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 + * @return */ virtual ReturnValue_t prepareCommand(CommandMessage *message, - uint8_t subservice, const uint8_t *tcData, uint32_t tcDataLen, + uint8_t subservice, const uint8_t *tcData, size_t tcDataLen, uint32_t *state, object_id_t objectId) = 0; /** @@ -152,10 +150,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 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 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 @@ -215,34 +213,38 @@ protected: PeriodicTaskIF* executingTask; /** - * Send TM data from pointer to data. If a header is supplied it is added before data + * @brief Send TM data from pointer to data. + * If a header is supplied it is added before data * @param subservice Number of subservice * @param data Pointer to the data in the Packet * @param dataLen Lenght of data in the Packet * @param headerData HeaderData will be placed before data * @param headerSize Size of HeaderData */ - void sendTmPacket(uint8_t subservice, const uint8_t *data, uint32_t dataLen, - const uint8_t* headerData = NULL, uint32_t headerSize = 0); + ReturnValue_t sendTmPacket(uint8_t subservice, const uint8_t *data, + size_t dataLen, const uint8_t* headerData = nullptr, + size_t headerSize = 0); /** - * To send TM packets of objects that still need to be serialized and consist of an object ID with appended data + * @brief To send TM packets of objects that still need to be serialized + * and consist of an object ID with appended data. * @param subservice Number of subservice * @param objectId ObjectId is placed before data * @param data Data to append to the packet * @param dataLen Length of Data */ - void sendTmPacket(uint8_t subservice, object_id_t objectId, - const uint8_t *data, uint32_t dataLen); + ReturnValue_t sendTmPacket(uint8_t subservice, object_id_t objectId, + const uint8_t *data, size_t dataLen); /** - * To send packets has data which is in form of a SerializeIF or Adapters implementing it + * @brief To send packets which are contained inside a class implementing + * SerializeIF. * @param subservice Number of subservice * @param content This is a pointer to the serialized packet * @param header Serialize IF header which will be placed before content */ - void sendTmPacket(uint8_t subservice, SerializeIF* content, - SerializeIF* header = NULL); + ReturnValue_t sendTmPacket(uint8_t subservice, SerializeIF* content, + SerializeIF* header = nullptr); virtual void handleUnrequestedReply(CommandMessage *reply); From 482aedfaf23cca466dcd9bb060b6238ac63f185c Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 10 Jun 2020 22:13:49 +0200 Subject: [PATCH 20/59] cleaned up includes, improved doc --- tmtcservices/CommandingServiceBase.cpp | 27 +++--- tmtcservices/CommandingServiceBase.h | 114 ++++++++++++++----------- 2 files changed, 76 insertions(+), 65 deletions(-) diff --git a/tmtcservices/CommandingServiceBase.cpp b/tmtcservices/CommandingServiceBase.cpp index adc9400fa..361baf3b6 100644 --- a/tmtcservices/CommandingServiceBase.cpp +++ b/tmtcservices/CommandingServiceBase.cpp @@ -1,22 +1,21 @@ -/* - * CommandingServiceBase.cpp - * - * Created on: 28.08.2019 - * Author: gaisser - */ +#include +#include +#include #include +#include +#include +#include +#include CommandingServiceBase::CommandingServiceBase(object_id_t setObjectId, uint16_t apid, uint8_t service, uint8_t numberOfParallelCommands, - uint16_t commandTimeout_seconds, object_id_t setPacketSource, + uint16_t commandTimeoutSeconds, object_id_t setPacketSource, object_id_t setPacketDestination, size_t queueDepth) : - SystemObject(setObjectId), apid(apid), service(service), timeout_seconds( - commandTimeout_seconds), tmPacketCounter(0), IPCStore(NULL), TCStore( - NULL), commandQueue(NULL), requestQueue(NULL), commandMap( - numberOfParallelCommands), failureParameter1(0), failureParameter2( - 0), packetSource(setPacketSource), packetDestination( - setPacketDestination),executingTask(NULL) { + SystemObject(setObjectId), apid(apid), service(service), + timeoutSeconds(commandTimeoutSeconds), + commandMap(numberOfParallelCommands), packetSource(setPacketSource), + packetDestination(setPacketDestination) { commandQueue = QueueFactory::instance()->createMessageQueue(queueDepth); requestQueue = QueueFactory::instance()->createMessageQueue(queueDepth); } @@ -369,7 +368,7 @@ void CommandingServiceBase::checkTimeout() { typename FixedMap::Iterator iter; for (iter = commandMap.begin(); iter != commandMap.end(); ++iter) { - if ((iter->uptimeOfStart + (timeout_seconds * 1000)) < uptime) { + if ((iter->uptimeOfStart + (timeoutSeconds * 1000)) < uptime) { verificationReporter.sendFailureReport( TC_VERIFY::COMPLETION_FAILURE, iter->tcInfo.ackFlags, iter->tcInfo.tcPacketId, iter->tcInfo.tcSequenceControl, diff --git a/tmtcservices/CommandingServiceBase.h b/tmtcservices/CommandingServiceBase.h index e79e806de..37f6891c4 100644 --- a/tmtcservices/CommandingServiceBase.h +++ b/tmtcservices/CommandingServiceBase.h @@ -1,35 +1,33 @@ -#ifndef COMMANDINGSERVICEBASE_H_ -#define COMMANDINGSERVICEBASE_H_ +#ifndef FRAMEWORK_TMTCSERVICES_COMMANDINGSERVICEBASE_H_ +#define FRAMEWORK_TMTCSERVICES_COMMANDINGSERVICEBASE_H_ -#include -#include -#include -#include #include -#include #include #include -#include -#include -#include +#include #include -#include -#include + #include -#include -#include -#include +#include +#include +#include +#include + +class TcPacketStored; /** - * \brief This class is the basis for all PUS Services, which have to relay Telecommands to software bus. + * @brief This class is the basis for all PUS Services, which have to + * relay Telecommands to software bus. * - * It manages Telecommand reception and the generation of Verification Reports like PUSServiceBase. - * Every class that inherits from this abstract class has to implement four adaption points: + * It manages Telecommand reception and the generation of Verification Reports + * similar to PusServiceBase. This class is used if a telecommand can't be + * handled immediately and must be relayed to the internal software bus. * - isValidSubservice * - getMessageQueueAndObject * - prepareCommand * - handleReply - * \ingroup pus_services + * @author gaisser + * @ingroup pus_services */ class CommandingServiceBase: public SystemObject, public AcceptsTelecommandsIF, @@ -59,7 +57,7 @@ public: */ CommandingServiceBase(object_id_t setObjectId, uint16_t apid, uint8_t service, uint8_t numberOfParallelCommands, - uint16_t commandTimeout_seconds, object_id_t setPacketSource, + uint16_t commandTimeoutSeconds, object_id_t setPacketSource, object_id_t setPacketDestination, size_t queueDepth = 20); virtual ~CommandingServiceBase(); @@ -113,8 +111,9 @@ protected: 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. + * 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 @@ -129,10 +128,10 @@ protected: 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. + * 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 @@ -146,25 +145,40 @@ protected: 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 + * 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 in form of a command message. + * @param previousCommand + * Command_t of related command + * @param state [out/in] + * Additional parameter which can be used to pass state information. + * 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 - * - @c INVALID_REPLY can handle unrequested replies - * - Anything else triggers a TC verification failure + * @return + * - @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 */ virtual ReturnValue_t handleReply(const CommandMessage *reply, Command_t previousCommand, uint32_t *state, CommandMessage *optionalNextCommand, object_id_t objectId, bool *isStep) = 0; + /** + * This function can be overidden to handle unrequested reply, + * when the reply sender ID is unknown or is not found is the command map. + * @param reply + */ + virtual void handleUnrequestedReply(CommandMessage *reply); + + virtual void doPeriodicOperation(); + + struct CommandInfo { struct tcInfo { uint8_t ackFlags; @@ -184,33 +198,35 @@ protected: const uint8_t service; - const uint16_t timeout_seconds; + const uint16_t timeoutSeconds; - uint8_t tmPacketCounter; + uint8_t tmPacketCounter = 0; - StorageManagerIF *IPCStore; + StorageManagerIF *IPCStore = nullptr; - StorageManagerIF *TCStore; + StorageManagerIF *TCStore = nullptr; - MessageQueueIF* commandQueue; + MessageQueueIF* commandQueue = nullptr; - MessageQueueIF* requestQueue; + MessageQueueIF* requestQueue = nullptr; VerificationReporter verificationReporter; FixedMap commandMap; - uint32_t failureParameter1; //!< May be set be children to return a more precise failure condition. - uint32_t failureParameter2; //!< May be set be children to return a more precise failure condition. + /* May be set be children to return a more precise failure condition. */ + uint32_t failureParameter1 = 0; + uint32_t failureParameter2 = 0; object_id_t packetSource; object_id_t packetDestination; /** - * Pointer to the task which executes this component, is invalid before setTaskIF was called. + * Pointer to the task which executes this component, + * is invalid before setTaskIF was called. */ - PeriodicTaskIF* executingTask; + PeriodicTaskIF* executingTask = nullptr; /** * @brief Send TM data from pointer to data. @@ -246,10 +262,6 @@ protected: ReturnValue_t sendTmPacket(uint8_t subservice, SerializeIF* content, SerializeIF* header = nullptr); - virtual void handleUnrequestedReply(CommandMessage *reply); - - virtual void doPeriodicOperation(); - void checkAndExecuteFifo( typename FixedMap::Iterator *iter); From 534fddd2c6443f52bfedbe3532cd4263470ee18c Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 10 Jun 2020 22:19:08 +0200 Subject: [PATCH 21/59] added back comment removed for unknown reasons --- tmtcservices/CommandingServiceBase.h | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/tmtcservices/CommandingServiceBase.h b/tmtcservices/CommandingServiceBase.h index 37f6891c4..f97eabb0e 100644 --- a/tmtcservices/CommandingServiceBase.h +++ b/tmtcservices/CommandingServiceBase.h @@ -132,11 +132,12 @@ protected: * 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 which can be set and is sent to the object + * @param subservice Subservice of the current communication + * @param tcData Application data of command + * @param tcDataLen Application data length + * @param state [out/in] Setable state of the communication. + * communication * @param objectId Target object ID * @return */ From 72f3b16c2474eadbf1569edd60b06b0296ae6bf6 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 10 Jun 2020 22:53:24 +0200 Subject: [PATCH 22/59] split up huge member function for readability --- tmtcservices/CommandingServiceBase.cpp | 193 ++++++++++++++----------- tmtcservices/CommandingServiceBase.h | 7 + 2 files changed, 113 insertions(+), 87 deletions(-) diff --git a/tmtcservices/CommandingServiceBase.cpp b/tmtcservices/CommandingServiceBase.cpp index 361baf3b6..4c557f82d 100644 --- a/tmtcservices/CommandingServiceBase.cpp +++ b/tmtcservices/CommandingServiceBase.cpp @@ -56,7 +56,7 @@ ReturnValue_t CommandingServiceBase::initialize() { objectManager->get(packetDestination); PUSDistributorIF* distributor = objectManager->get( packetSource); - if ((packetForwarding == NULL) && (distributor == NULL)) { + if (packetForwarding == nullptr or distributor == nullptr) { return RETURN_FAILED; } @@ -67,7 +67,7 @@ ReturnValue_t CommandingServiceBase::initialize() { IPCStore = objectManager->get(objects::IPC_STORE); TCStore = objectManager->get(objects::TC_STORE); - if ((IPCStore == NULL) || (TCStore == NULL)) { + if (IPCStore == nullptr or TCStore == nullptr) { return RETURN_FAILED; } @@ -76,97 +76,116 @@ ReturnValue_t CommandingServiceBase::initialize() { } void CommandingServiceBase::handleCommandQueue() { - CommandMessage reply, nextCommand; - ReturnValue_t result, sendResult = RETURN_OK; - bool isStep = false; + CommandMessage reply; + ReturnValue_t result = RETURN_FAILED; for (result = commandQueue->receiveMessage(&reply); result == RETURN_OK; result = commandQueue->receiveMessage(&reply)) { - isStep = false; - typename FixedMap::Iterator iter; - if (reply.getSender() == MessageQueueIF::NO_QUEUE) { - handleUnrequestedReply(&reply); - continue; - } - if ((iter = commandMap.find(reply.getSender())) == commandMap.end()) { - handleUnrequestedReply(&reply); - continue; - } - nextCommand.setCommand(CommandMessage::CMD_NONE); - result = handleReply(&reply, iter->command, &iter->state, &nextCommand, - iter->objectId, &isStep); - switch (result) { - case EXECUTION_COMPLETE: - case RETURN_OK: - case NO_STEP_MESSAGE: - iter->command = nextCommand.getCommand(); - if (nextCommand.getCommand() != CommandMessage::CMD_NONE) { - sendResult = commandQueue->sendMessage(reply.getSender(), - &nextCommand); - } - if (sendResult == RETURN_OK) { - if (isStep) { - if (result != NO_STEP_MESSAGE) { - verificationReporter.sendSuccessReport( - TC_VERIFY::PROGRESS_SUCCESS, - iter->tcInfo.ackFlags, iter->tcInfo.tcPacketId, - iter->tcInfo.tcSequenceControl, ++iter->step); - } - } else { - verificationReporter.sendSuccessReport( - TC_VERIFY::COMPLETION_SUCCESS, - iter->tcInfo.ackFlags, iter->tcInfo.tcPacketId, - iter->tcInfo.tcSequenceControl, 0); - checkAndExecuteFifo(&iter); - } - } else { - if (isStep) { - nextCommand.clearCommandMessage(); - verificationReporter.sendFailureReport( - TC_VERIFY::PROGRESS_FAILURE, iter->tcInfo.ackFlags, - iter->tcInfo.tcPacketId, - iter->tcInfo.tcSequenceControl, sendResult, - ++iter->step, failureParameter1, failureParameter2); - } else { - nextCommand.clearCommandMessage(); - verificationReporter.sendFailureReport( - TC_VERIFY::COMPLETION_FAILURE, - iter->tcInfo.ackFlags, iter->tcInfo.tcPacketId, - iter->tcInfo.tcSequenceControl, sendResult, 0, - failureParameter1, failureParameter2); - } - failureParameter1 = 0; - failureParameter2 = 0; - checkAndExecuteFifo(&iter); - } - break; - case INVALID_REPLY: - //might be just an unrequested reply at a bad moment - handleUnrequestedReply(&reply); - break; - default: - if (isStep) { - verificationReporter.sendFailureReport( - TC_VERIFY::PROGRESS_FAILURE, iter->tcInfo.ackFlags, - iter->tcInfo.tcPacketId, iter->tcInfo.tcSequenceControl, - result, ++iter->step, failureParameter1, - failureParameter2); - } else { - verificationReporter.sendFailureReport( - TC_VERIFY::COMPLETION_FAILURE, iter->tcInfo.ackFlags, - iter->tcInfo.tcPacketId, iter->tcInfo.tcSequenceControl, - result, 0, failureParameter1, failureParameter2); - } - failureParameter1 = 0; - failureParameter2 = 0; - checkAndExecuteFifo(&iter); - break; - } - + handleCommandMessage(reply); } } +void CommandingServiceBase::handleCommandMessage(CommandMessage& reply) { + bool isStep = false; + CommandMessage nextCommand; + CommandMapIter iter; + if (reply.getSender() == MessageQueueIF::NO_QUEUE) { + handleUnrequestedReply(&reply); + return; + } + if ((iter = commandMap.find(reply.getSender())) == commandMap.end()) { + handleUnrequestedReply(&reply); + return; + } + nextCommand.setCommand(CommandMessage::CMD_NONE); + + // Implemented by child class, specifies what to do with reply. + ReturnValue_t result = handleReply(&reply, iter->command, &iter->state, + &nextCommand, iter->objectId, &isStep); + + switch (result) { + case EXECUTION_COMPLETE: + case RETURN_OK: + case NO_STEP_MESSAGE: + // handle result of reply handler implemented by developer. + handleReplyHandlerResult(result, iter, nextCommand, reply, isStep); + break; + case INVALID_REPLY: + //might be just an unrequested reply at a bad moment + handleUnrequestedReply(&reply); + break; + default: + if (isStep) { + verificationReporter.sendFailureReport( + TC_VERIFY::PROGRESS_FAILURE, iter->tcInfo.ackFlags, + iter->tcInfo.tcPacketId, iter->tcInfo.tcSequenceControl, + result, ++iter->step, failureParameter1, + failureParameter2); + } else { + verificationReporter.sendFailureReport( + TC_VERIFY::COMPLETION_FAILURE, iter->tcInfo.ackFlags, + iter->tcInfo.tcPacketId, iter->tcInfo.tcSequenceControl, + result, 0, failureParameter1, failureParameter2); + } + failureParameter1 = 0; + failureParameter2 = 0; + checkAndExecuteFifo(&iter); + break; + } + +} + +void CommandingServiceBase::handleReplyHandlerResult(ReturnValue_t result, + CommandMapIter iter, CommandMessage& nextCommand, CommandMessage& reply, + bool& isStep) { + iter->command = nextCommand.getCommand(); + + // In case a new command is to be sent immediately, this is performed here. + // If no new command is sent, only analyse reply result by initializing + // sendResult as RETURN_OK + ReturnValue_t sendResult = RETURN_OK; + if (nextCommand.getCommand() != CommandMessage::CMD_NONE) { + sendResult = commandQueue->sendMessage(reply.getSender(), + &nextCommand); + } + + if (sendResult == RETURN_OK) { + if (isStep and result != NO_STEP_MESSAGE) { + verificationReporter.sendSuccessReport( + TC_VERIFY::PROGRESS_SUCCESS, + iter->tcInfo.ackFlags, iter->tcInfo.tcPacketId, + iter->tcInfo.tcSequenceControl, ++iter->step); + } + else { + verificationReporter.sendSuccessReport( + TC_VERIFY::COMPLETION_SUCCESS, + iter->tcInfo.ackFlags, iter->tcInfo.tcPacketId, + iter->tcInfo.tcSequenceControl, 0); + checkAndExecuteFifo(&iter); + } + } + else { + if (isStep) { + nextCommand.clearCommandMessage(); + verificationReporter.sendFailureReport( + TC_VERIFY::PROGRESS_FAILURE, iter->tcInfo.ackFlags, + iter->tcInfo.tcPacketId, + iter->tcInfo.tcSequenceControl, sendResult, + ++iter->step, failureParameter1, failureParameter2); + } else { + nextCommand.clearCommandMessage(); + verificationReporter.sendFailureReport( + TC_VERIFY::COMPLETION_FAILURE, + iter->tcInfo.ackFlags, iter->tcInfo.tcPacketId, + iter->tcInfo.tcSequenceControl, sendResult, 0, + failureParameter1, failureParameter2); + } + failureParameter1 = 0; + failureParameter2 = 0; + checkAndExecuteFifo(&iter); + } +} + void CommandingServiceBase::handleRequestQueue() { TmTcMessage message; ReturnValue_t result; diff --git a/tmtcservices/CommandingServiceBase.h b/tmtcservices/CommandingServiceBase.h index f97eabb0e..9a68a2d3c 100644 --- a/tmtcservices/CommandingServiceBase.h +++ b/tmtcservices/CommandingServiceBase.h @@ -267,6 +267,9 @@ protected: typename FixedMap::Iterator *iter); private: + using CommandMapIter = FixedMap::Iterator; + /** * This method handles internal execution of a command, * once it has been started by @sa{startExecution()} in the Request Queue handler. @@ -298,6 +301,10 @@ private: void startExecution(TcPacketStored *storedPacket, typename FixedMap::Iterator *iter); + void handleCommandMessage(CommandMessage& reply); + void handleReplyHandlerResult(ReturnValue_t result, CommandMapIter iter, + CommandMessage& nextCommand,CommandMessage& reply, bool& isStep); + void checkTimeout(); }; From 0c4552254089de1b5626a4bca31114acb39a0290 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 17 Jun 2020 20:57:35 +0200 Subject: [PATCH 23/59] hybrid iterator fix and improvement --- container/HybridIterator.h | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/container/HybridIterator.h b/container/HybridIterator.h index b34fdfd01..db1ce4bc8 100644 --- a/container/HybridIterator.h +++ b/container/HybridIterator.h @@ -8,13 +8,11 @@ template class HybridIterator: public LinkedElement::Iterator, public ArrayList::Iterator { public: - HybridIterator() : - value(NULL), linked(NULL), end(NULL) { - } + HybridIterator() {} HybridIterator(typename LinkedElement::Iterator *iter) : - LinkedElement::Iterator(*iter), value( - iter->value), linked(true), end(NULL) { + LinkedElement::Iterator(*iter), value(iter->value), + linked(true) { } @@ -66,11 +64,11 @@ public: return tmp; } - bool operator==(HybridIterator other) { - return value == other->value; + bool operator==(const HybridIterator& other) { + return value == other.value; } - bool operator!=(HybridIterator other) { + bool operator!=(const HybridIterator& other) { return !(*this == other); } @@ -82,11 +80,11 @@ public: return value; } - T* value; + T* value = nullptr; private: - bool linked; - T *end; + bool linked = false; + T *end = nullptr; }; #endif /* HYBRIDITERATOR_H_ */ From d8e9e34ad9fc33b365f98ef3497fdc84fa698c8a Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Fri, 19 Jun 2020 14:36:49 +0200 Subject: [PATCH 24/59] framework submakefile improvements --- framework.mk | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/framework.mk b/framework.mk index a5a653641..c88229814 100644 --- a/framework.mk +++ b/framework.mk @@ -1,5 +1,5 @@ -# This file needs FRAMEWORK_PATH and API set correctly -# Valid API settings: rtems, linux, freeRTOS +# This file needs FRAMEWORK_PATH and OS_FSFW set correctly by another Makefile. +# Valid API settings: rtems, linux, freeRTOS, host CXXSRC += $(wildcard $(FRAMEWORK_PATH)/action/*.cpp) CXXSRC += $(wildcard $(FRAMEWORK_PATH)/container/*.cpp) @@ -8,11 +8,13 @@ CXXSRC += $(wildcard $(FRAMEWORK_PATH)/controller/*.cpp) CXXSRC += $(wildcard $(FRAMEWORK_PATH)/coordinates/*.cpp) CXXSRC += $(wildcard $(FRAMEWORK_PATH)/datalinklayer/*.cpp) CXXSRC += $(wildcard $(FRAMEWORK_PATH)/datapool/*.cpp) +CXXSRC += $(wildcard $(FRAMEWORK_PATH)/datapoolglob/*.cpp) +CXXSRC += $(wildcard $(FRAMEWORK_PATH)/datapoollocal/*.cpp) +CXXSRC += $(wildcard $(FRAMEWORK_PATH)/housekeeping/*.cpp) CXXSRC += $(wildcard $(FRAMEWORK_PATH)/devicehandlers/*.cpp) CXXSRC += $(wildcard $(FRAMEWORK_PATH)/events/*.cpp) CXXSRC += $(wildcard $(FRAMEWORK_PATH)/events/eventmatching/*.cpp) CXXSRC += $(wildcard $(FRAMEWORK_PATH)/fdir/*.cpp) -CXXSRC += $(wildcard $(FRAMEWORK_PATH)/framework.mk/*.cpp) CXXSRC += $(wildcard $(FRAMEWORK_PATH)/globalfunctions/*.cpp) CXXSRC += $(wildcard $(FRAMEWORK_PATH)/globalfunctions/matching/*.cpp) CXXSRC += $(wildcard $(FRAMEWORK_PATH)/globalfunctions/math/*.cpp) @@ -26,14 +28,16 @@ CXXSRC += $(wildcard $(FRAMEWORK_PATH)/objectmanager/*.cpp) CXXSRC += $(wildcard $(FRAMEWORK_PATH)/osal/*.cpp) # select the OS -ifeq ($(OS),rtems) +ifeq ($(OS_FSFW),rtems) CXXSRC += $(wildcard $(FRAMEWORK_PATH)/osal/rtems/*.cpp) -else ifeq ($(OS),linux) +else ifeq ($(OS_FSFW),linux) CXXSRC += $(wildcard $(FRAMEWORK_PATH)/osal/linux/*.cpp) -else ifeq ($(OS),freeRTOS) +else ifeq ($(OS_FSFW),freeRTOS) CXXSRC += $(wildcard $(FRAMEWORK_PATH)/osal/FreeRTOS/*.cpp) +else ifeq ($(OS_FSFW),host) +CXXSRC += $(wildcard $(FRAMEWORK_PATH)/osal/host/*.cpp) else -$(error invalid OS specified, valid OS are rtems, linux, freeRTOS) +$(error invalid OS specified, valid OS are rtems, linux, freeRTOS, host) endif CXXSRC += $(wildcard $(FRAMEWORK_PATH)/parameters/*.cpp) @@ -54,3 +58,4 @@ CXXSRC += $(wildcard $(FRAMEWORK_PATH)/tmtcpacket/*.cpp) CXXSRC += $(wildcard $(FRAMEWORK_PATH)/tmtcpacket/packetmatcher/*.cpp) CXXSRC += $(wildcard $(FRAMEWORK_PATH)/tmtcpacket/pus/*.cpp) CXXSRC += $(wildcard $(FRAMEWORK_PATH)/tmtcservices/*.cpp) +CXXSRC += $(wildcard $(FRAMEWORK_PATH)/test/*.cpp) \ No newline at end of file From 31e557776373eb5334430ae6f1e68616aba4a617 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Fri, 19 Jun 2020 14:45:29 +0200 Subject: [PATCH 25/59] added deadline missed check --- osal/FreeRTOS/PeriodicTask.cpp | 55 ++++++++++++------- osal/FreeRTOS/PeriodicTask.h | 96 +++++++++++++++++++--------------- 2 files changed, 90 insertions(+), 61 deletions(-) diff --git a/osal/FreeRTOS/PeriodicTask.cpp b/osal/FreeRTOS/PeriodicTask.cpp index deab2dc15..318676393 100644 --- a/osal/FreeRTOS/PeriodicTask.cpp +++ b/osal/FreeRTOS/PeriodicTask.cpp @@ -1,17 +1,19 @@ +#include "PeriodicTask.h" + #include #include -#include "PeriodicTask.h" PeriodicTask::PeriodicTask(const char *name, TaskPriority setPriority, TaskStackSize setStack, TaskPeriod setPeriod, void (*setDeadlineMissedFunc)()) : started(false), handle(NULL), period(setPeriod), deadlineMissedFunc( - setDeadlineMissedFunc) { - - BaseType_t status = xTaskCreate(taskEntryPoint, name, setStack, this, setPriority, &handle); + setDeadlineMissedFunc) +{ + BaseType_t status = xTaskCreate(taskEntryPoint, name, + setStack, this, setPriority, &handle); if(status != pdPASS){ - sif::debug << "PeriodicTask Insufficient heap memory remaining. Status: " - << status << std::endl; + sif::debug << "PeriodicTask Insufficient heap memory remaining. " + "Status: " << status << std::endl; } } @@ -21,16 +23,19 @@ PeriodicTask::~PeriodicTask(void) { } void PeriodicTask::taskEntryPoint(void* argument) { - //The argument is re-interpreted as PeriodicTask. The Task object is global, so it is found from any place. + // The argument is re-interpreted as PeriodicTask. The Task object is + // global, so it is found from any place. PeriodicTask *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. - // to be able to accommodate both cases we check a member which is set in #startTask() - // if it is not set and we get here, the scheduler was started before #startTask() was called and we need to suspend - // if it is set, the scheduler was not running before #startTask() was called and we can continue + /* Task should not start until explicitly requested, + * but in FreeRTOS, tasks start as soon as they are created if the scheduler + * is running but not if the scheduler is not running. + * To be able to accommodate both cases we check a member which is set in + * #startTask(). If it is not set and we get here, the scheduler was started + * before #startTask() was called and we need to suspend if it is set, + * the scheduler was not running before #startTask() was called and we + * can continue */ - if (!originalTask->started) { + if (not originalTask->started) { vTaskSuspend(NULL); } @@ -59,9 +64,9 @@ void PeriodicTask::taskFunctionality() { TickType_t xLastWakeTime; const TickType_t xPeriod = pdMS_TO_TICKS(this->period * 1000.); /* The xLastWakeTime variable needs to be initialized with the current tick - count. Note that this is the only time the variable is written to explicitly. - After this assignment, xLastWakeTime is updated automatically internally within - vTaskDelayUntil(). */ + count. Note that this is the only time the variable is written to + explicitly. After this assignment, xLastWakeTime is updated automatically + internally within vTaskDelayUntil(). */ xLastWakeTime = xTaskGetTickCount(); /* Enter the loop that defines the task behavior. */ for (;;) { @@ -69,8 +74,22 @@ void PeriodicTask::taskFunctionality() { it != objectList.end(); ++it) { (*it)->performOperation(); } - //TODO deadline missed check + + /* If all operations are finished and the difference of the + * current time minus the last wake time is larger than the + * wait period, a deadline was missed. */ + if(xTaskGetTickCount() - xLastWakeTime >= xPeriod) { +#ifdef DEBUG + sif::warning << "PeriodicTask: " << pcTaskGetName(NULL) << + " missed deadline!\n" << std::flush; +#endif + if(deadlineMissedFunc != nullptr) { + this->deadlineMissedFunc(); + } + } + vTaskDelayUntil(&xLastWakeTime, xPeriod); + } } diff --git a/osal/FreeRTOS/PeriodicTask.h b/osal/FreeRTOS/PeriodicTask.h index e7ba8dd26..b9930a73b 100644 --- a/osal/FreeRTOS/PeriodicTask.h +++ b/osal/FreeRTOS/PeriodicTask.h @@ -1,48 +1,49 @@ -#ifndef MULTIOBJECTTASK_H_ -#define MULTIOBJECTTASK_H_ +#ifndef FRAMEWORK_OSAL_FREERTOS_PERIODICTASK_H_ +#define FRAMEWORK_OSAL_FREERTOS_PERIODICTASK_H_ #include #include #include -#include -#include "task.h" +#include +#include #include class ExecutableObjectIF; /** - * @brief This class represents a specialized task for periodic activities of multiple objects. - * - * @details MultiObjectTask is an extension to ObjectTask in the way that it is able to execute - * multiple objects that implement the ExecutableObjectIF interface. The objects must be - * added prior to starting the task. - * + * @brief This class represents a specialized task for + * periodic activities of multiple objects. * @ingroup task_handling */ class PeriodicTask: public PeriodicTaskIF { public: /** - * @brief Standard constructor of the class. - * @details The class is initialized without allocated objects. These need to be added - * with #addObject. - * In the underlying TaskBase class, a new operating system task is created. - * In addition to the TaskBase parameters, the period, the pointer to the - * aforementioned initialization function and an optional "deadline-missed" - * function pointer is passed. - * @param priority Sets the priority of a task. Values range from a low 0 to a high 99. - * @param stack_size The stack size reserved by the operating system for the task. - * @param setPeriod The length of the period with which the task's functionality will be - * executed. It is expressed in clock ticks. - * @param setDeadlineMissedFunc The function pointer to the deadline missed function - * that shall be assigned. + * Keep in Mind that you need to call before this vTaskStartScheduler()! + * A lot of task parameters are set in "FreeRTOSConfig.h". + * TODO: why does this need to be called before vTaskStartScheduler? + * @details + * The class is initialized without allocated objects. + * These need to be added with #addComponent. + * @param priority + * Sets the priority of a task. Values depend on freeRTOS configuration, + * high number means high priority. + * @param stack_size + * The stack size reserved by the operating system for the task. + * @param setPeriod + * The length of the period with which the task's + * functionality will be executed. It is expressed in clock ticks. + * @param setDeadlineMissedFunc + * The function pointer to the deadline missed function that shall + * be assigned. */ - PeriodicTask(const char *name, TaskPriority setPriority, TaskStackSize setStack, TaskPeriod setPeriod, - void (*setDeadlineMissedFunc)()); + PeriodicTask(const char *name, TaskPriority setPriority, + TaskStackSize setStack, TaskPeriod setPeriod, + void (*setDeadlineMissedFunc)()); /** - * @brief Currently, the executed object's lifetime is not coupled with the task object's - * lifetime, so the destructor is empty. + * @brief Currently, the executed object's lifetime is not coupled with + * the task object's lifetime, so the destructor is empty. */ virtual ~PeriodicTask(void); @@ -58,7 +59,9 @@ public: * Adds an object to the list of objects to be executed. * The objects are executed in the order added. * @param object Id of the object to add. - * @return RETURN_OK on success, RETURN_FAILED if the object could not be added. + * @return + * -@c RETURN_OK on success + * -@c RETURN_FAILED if the object could not be added. */ ReturnValue_t addComponent(object_id_t object); @@ -69,42 +72,49 @@ protected: bool started; TaskHandle_t handle; - typedef std::vector ObjectList; //!< Typedef for the List of objects. + //! Typedef for the List of objects. + typedef std::vector ObjectList; /** * @brief This attribute holds a list of objects to be executed. */ ObjectList objectList; /** * @brief The period of the task. - * @details The period determines the frequency of the task's execution. It is expressed in clock ticks. + * @details + * The period determines the frequency of the task's execution. + * It is expressed in clock ticks. */ TaskPeriod period; /** * @brief The pointer to the deadline-missed function. - * @details This pointer stores the function that is executed if the task's deadline is missed. - * So, each may react individually on a timing failure. The pointer may be NULL, - * then nothing happens on missing the deadline. The deadline is equal to the next execution - * of the periodic task. + * @details + * This pointer stores the function that is executed if the task's deadline + * is missed so each may react individually on a timing failure. + * The pointer may be NULL, then nothing happens on missing the deadline. + * The deadline is equal to the next execution of the periodic task. */ void (*deadlineMissedFunc)(void); /** * @brief This is the function executed in the new task's context. - * @details It converts the argument back to the thread object type and copies the class instance - * to the task context. The taskFunctionality method is called afterwards. + * @details + * It converts the argument back to the thread object type and copies the + * class instance to the task context. The taskFunctionality method is + * called afterwards. * @param A pointer to the task object itself is passed as argument. */ static void taskEntryPoint(void* argument); /** * @brief The function containing the actual functionality of the task. - * @details The method sets and starts - * the task's period, then enters a loop that is repeated as long as the isRunning - * attribute is true. Within the loop, all performOperation methods of the added - * objects are called. Afterwards the checkAndRestartPeriod system call blocks the task - * until the next period. - * On missing the deadline, the deadlineMissedFunction is executed. + * @details + * The method sets and starts the task's period, then enters a loop that is + * repeated as long as the isRunning attribute is true. Within the loop, + * all performOperation methods of the added objects are called. + * Afterwards the checkAndRestartPeriod system call blocks the task until + * the next period. + * On missing the deadline, the deadlineMissedFunction is executed. */ void taskFunctionality(void); }; -#endif /* MULTIOBJECTTASK_H_ */ +#endif /* PERIODICTASK_H_ */ From 56aaa299854f4a7e0cf02499cdb2cb11db0899b9 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Fri, 19 Jun 2020 14:47:01 +0200 Subject: [PATCH 26/59] added deadline missed check --- osal/FreeRTOS/FixedTimeslotTask.cpp | 64 ++++++++++++++++---------- osal/FreeRTOS/FixedTimeslotTask.h | 69 ++++++++++++++++------------- 2 files changed, 77 insertions(+), 56 deletions(-) diff --git a/osal/FreeRTOS/FixedTimeslotTask.cpp b/osal/FreeRTOS/FixedTimeslotTask.cpp index fb3c3b03e..ea324d35e 100644 --- a/osal/FreeRTOS/FixedTimeslotTask.cpp +++ b/osal/FreeRTOS/FixedTimeslotTask.cpp @@ -1,6 +1,7 @@ -#include #include "FixedTimeslotTask.h" +#include + uint32_t FixedTimeslotTask::deadlineMissedCount = 0; const size_t PeriodicTaskIF::MINIMUM_STACK_SIZE = configMINIMAL_STACK_SIZE; @@ -18,16 +19,19 @@ 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. + // The argument is re-interpreted as FixedTimeslotTask. The Task object is + // global, so it is found from any place. 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. - // to be able to accommodate both cases we check a member which is set in #startTask() - // if it is not set and we get here, the scheduler was started before #startTask() was called and we need to suspend - // if it is set, the scheduler was not running before #startTask() was called and we can continue + /* Task should not start until explicitly requested, + * but in FreeRTOS, tasks start as soon as they are created if the scheduler + * is running but not if the scheduler is not running. + * To be able to accommodate both cases we check a member which is set in + * #startTask(). If it is not set and we get here, the scheduler was started + * before #startTask() was called and we need to suspend if it is set, + * the scheduler was not running before #startTask() was called and we + * can continue */ - if (!originalTask->started) { + if (not originalTask->started) { vTaskSuspend(NULL); } @@ -81,11 +85,12 @@ ReturnValue_t FixedTimeslotTask::checkSequence() const { } void FixedTimeslotTask::taskFunctionality() { - // A local iterator for the Polling Sequence Table is created to find the start time for the first entry. - std::list::iterator it = pst.current; + // A local iterator for the Polling Sequence Table is created to find the + // start time for the first entry. + SlotListIter slotListIter = pst.current; //The start time for the first entry is read. - uint32_t intervalMs = (*it)->pollingTimeMs; + uint32_t intervalMs = slotListIter->pollingTimeMs; TickType_t interval = pdMS_TO_TICKS(intervalMs); TickType_t xLastWakeTime; @@ -101,18 +106,30 @@ void FixedTimeslotTask::taskFunctionality() { /* Enter the loop that defines the task behavior. */ for (;;) { //The component for this slot is executed and the next one is chosen. - this->pst.executeAndAdvance(); - if (pst.slotFollowsImmediately()) { - //Do nothing - } else { - // we need to wait before executing the current slot - //this gives us the time to wait: - intervalMs = this->pst.getIntervalToPreviousSlotMs(); - interval = pdMS_TO_TICKS(intervalMs); - vTaskDelayUntil(&xLastWakeTime, interval); - //TODO deadline missed check - } + this->pst.executeAndAdvance(); + if (not pst.slotFollowsImmediately()) { + /* If all operations are finished and the difference of the + * current time minus the last wake time is larger than the + * expected wait period, a deadline was missed. */ + if(xTaskGetTickCount() - xLastWakeTime >= + pdMS_TO_TICKS(this->pst.getIntervalToPreviousSlotMs())) { +#ifdef DEBUG + sif::warning << "FixedTimeslotTask: " << pcTaskGetName(NULL) << + " missed deadline!\n" << std::flush; +#endif + if(deadlineMissedFunc != nullptr) { + this->deadlineMissedFunc(); + } + // Continue immediately, no need to wait. + break; + } + // we need to wait before executing the current slot + //this gives us the time to wait: + intervalMs = this->pst.getIntervalToPreviousSlotMs(); + interval = pdMS_TO_TICKS(intervalMs); + vTaskDelayUntil(&xLastWakeTime, interval); + } } } @@ -120,4 +137,3 @@ ReturnValue_t FixedTimeslotTask::sleepFor(uint32_t ms) { vTaskDelay(pdMS_TO_TICKS(ms)); return HasReturnvaluesIF::RETURN_OK; } - diff --git a/osal/FreeRTOS/FixedTimeslotTask.h b/osal/FreeRTOS/FixedTimeslotTask.h index bed130511..39db667cb 100644 --- a/osal/FreeRTOS/FixedTimeslotTask.h +++ b/osal/FreeRTOS/FixedTimeslotTask.h @@ -1,24 +1,27 @@ -#ifndef POLLINGTASK_H_ -#define POLLINGTASK_H_ +#ifndef FRAMEWORK_OSAL_FREERTOS_FIXEDTIMESLOTTASK_H_ +#define FRAMEWORK_OSAL_FREERTOS_FIXEDTIMESLOTTASK_H_ #include #include #include -#include -#include "task.h" +#include +#include class FixedTimeslotTask: public FixedTimeslotTaskIF { public: + /** - * @brief The standard constructor of the class. - * - * @details This is the general constructor of the class. In addition to the TaskBase parameters, - * the following variables are passed: - * - * @param (*setDeadlineMissedFunc)() The function pointer to the deadline missed function that shall be assigned. - * - * @param getPst The object id of the completely initialized polling sequence. + * Keep in Mind that you need to call before this vTaskStartScheduler()! + * A lot of task parameters are set in "FreeRTOSConfig.h". + * @param name Name of the task, lenght limited by configMAX_TASK_NAME_LEN + * @param setPriority Number of priorities specified by + * configMAX_PRIORITIES. High taskPriority_ number means high priority. + * @param setStack Stack size in words (not bytes!). + * Lower limit specified by configMINIMAL_STACK_SIZE + * @param overallPeriod Period in seconds. + * @param setDeadlineMissedFunc Callback if a deadline was missed. + * @return Pointer to the newly created task. */ FixedTimeslotTask(const char *name, TaskPriority setPriority, TaskStackSize setStack, TaskPeriod overallPeriod, @@ -26,16 +29,18 @@ public: /** * @brief The destructor of the class. - * - * @details The destructor frees all heap memory that was allocated on thread initialization for the PST and - * the device handlers. This is done by calling the PST's destructor. + * @details + * The destructor frees all heap memory that was allocated on thread + * initialization for the PST and the device handlers. This is done by + * calling the PST's destructor. */ virtual ~FixedTimeslotTask(void); ReturnValue_t startTask(void); /** * This static function can be used as #deadlineMissedFunc. - * It counts missedDeadlines and prints the number of missed deadlines every 10th time. + * It counts missedDeadlines and prints the number of missed deadlines + * every 10th time. */ static void missedDeadlineCounter(); /** @@ -51,6 +56,7 @@ public: ReturnValue_t checkSequence() const; ReturnValue_t sleepFor(uint32_t ms); + protected: bool started; TaskHandle_t handle; @@ -58,30 +64,29 @@ protected: FixedSlotSequence pst; /** - * @brief This attribute holds a function pointer that is executed when a deadline was missed. - * - * @details Another function may be announced to determine the actions to perform when a deadline was missed. - * Currently, only one function for missing any deadline is allowed. - * If not used, it shall be declared NULL. + * @brief This attribute holds a function pointer that is executed when + * a deadline was missed. + * @details + * Another function may be announced to determine the actions to perform + * when a deadline was missed. Currently, only one function for missing + * any deadline is allowed. If not used, it shall be declared NULL. */ void (*deadlineMissedFunc)(void); /** - * @brief This is the entry point in a new polling thread. - * - * @details This method, that is the generalOSAL::checkAndRestartPeriod( this->periodId, interval ); entry point in the new thread, is here set to generate - * and link the Polling Sequence Table to the thread object and start taskFunctionality() - * on success. If operation of the task is ended for some reason, - * the destructor is called to free allocated memory. + * @brief This is the entry point for a new task. + * @details + * This method starts the task by calling taskFunctionality(), as soon as + * all requirements (task scheduler has started and startTask() + * has been called) are met. */ static void taskEntryPoint(void* argument); /** * @brief This function holds the main functionality of the thread. - * - * - * @details Holding the main functionality of the task, this method is most important. - * It links the functionalities provided by FixedSlotSequence with the OS's System Calls - * to keep the timing of the periods. + * @details + * Core function holding the main functionality of the task + * It links the functionalities provided by FixedSlotSequence with the + * OS's System Calls to keep the timing of the periods. */ void taskFunctionality(void); }; From ffe2a7bffefbb744d1149e4c4d53807649c0570e Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 22 Jun 2020 19:00:02 +0200 Subject: [PATCH 27/59] fix for fixed timeslot task improvement --- osal/FreeRTOS/FixedTimeslotTask.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/osal/FreeRTOS/FixedTimeslotTask.cpp b/osal/FreeRTOS/FixedTimeslotTask.cpp index ea324d35e..a4acc420d 100644 --- a/osal/FreeRTOS/FixedTimeslotTask.cpp +++ b/osal/FreeRTOS/FixedTimeslotTask.cpp @@ -87,10 +87,10 @@ ReturnValue_t FixedTimeslotTask::checkSequence() const { void FixedTimeslotTask::taskFunctionality() { // A local iterator for the Polling Sequence Table is created to find the // start time for the first entry. - SlotListIter slotListIter = pst.current; + std::list::iterator slotListIter = pst.current; //The start time for the first entry is read. - uint32_t intervalMs = slotListIter->pollingTimeMs; + uint32_t intervalMs = (*slotListIter)->pollingTimeMs; TickType_t interval = pdMS_TO_TICKS(intervalMs); TickType_t xLastWakeTime; @@ -121,7 +121,7 @@ void FixedTimeslotTask::taskFunctionality() { this->deadlineMissedFunc(); } // Continue immediately, no need to wait. - break; + continue; } // we need to wait before executing the current slot From 2cada2df4a611daf5d8c9e6bfa1e8026dc98e09b Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 22 Jun 2020 20:21:11 +0200 Subject: [PATCH 28/59] some fixes and improvements --- osal/FreeRTOS/FixedTimeslotTask.cpp | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/osal/FreeRTOS/FixedTimeslotTask.cpp b/osal/FreeRTOS/FixedTimeslotTask.cpp index a4acc420d..edb7f835d 100644 --- a/osal/FreeRTOS/FixedTimeslotTask.cpp +++ b/osal/FreeRTOS/FixedTimeslotTask.cpp @@ -62,11 +62,6 @@ ReturnValue_t FixedTimeslotTask::startTask() { ReturnValue_t FixedTimeslotTask::addSlot(object_id_t componentId, uint32_t slotTimeMs, int8_t executionStep) { if (objectManager->get(componentId) != nullptr) { - if(slotTimeMs == 0) { - // FreeRTOS throws a sanity error for zero values, so we set - // the time to one millisecond. - slotTimeMs = 1; - } pst.addSlot(componentId, slotTimeMs, executionStep, this); return HasReturnvaluesIF::RETURN_OK; } @@ -87,7 +82,7 @@ ReturnValue_t FixedTimeslotTask::checkSequence() const { void FixedTimeslotTask::taskFunctionality() { // A local iterator for the Polling Sequence Table is created to find the // start time for the first entry. - std::list::iterator slotListIter = pst.current; + auto slotListIter = pst.current; //The start time for the first entry is read. uint32_t intervalMs = (*slotListIter)->pollingTimeMs; @@ -101,18 +96,23 @@ void FixedTimeslotTask::taskFunctionality() { xLastWakeTime = xTaskGetTickCount(); // wait for first entry's start time - vTaskDelayUntil(&xLastWakeTime, interval); + if(interval > 0) { + vTaskDelayUntil(&xLastWakeTime, interval); + } /* Enter the loop that defines the task behavior. */ for (;;) { //The component for this slot is executed and the next one is chosen. this->pst.executeAndAdvance(); if (not pst.slotFollowsImmediately()) { + // Get the interval till execution of the next slot. + intervalMs = this->pst.getIntervalToPreviousSlotMs(); + interval = pdMS_TO_TICKS(intervalMs); + /* If all operations are finished and the difference of the * current time minus the last wake time is larger than the * expected wait period, a deadline was missed. */ - if(xTaskGetTickCount() - xLastWakeTime >= - pdMS_TO_TICKS(this->pst.getIntervalToPreviousSlotMs())) { + if(xTaskGetTickCount() - xLastWakeTime >= interval) { #ifdef DEBUG sif::warning << "FixedTimeslotTask: " << pcTaskGetName(NULL) << " missed deadline!\n" << std::flush; @@ -120,14 +120,9 @@ void FixedTimeslotTask::taskFunctionality() { if(deadlineMissedFunc != nullptr) { this->deadlineMissedFunc(); } - // Continue immediately, no need to wait. - continue; } - - // we need to wait before executing the current slot - //this gives us the time to wait: - intervalMs = this->pst.getIntervalToPreviousSlotMs(); - interval = pdMS_TO_TICKS(intervalMs); + // Wait for the interval. This exits immediately if a deadline was + // missed while also updating the last wake time. vTaskDelayUntil(&xLastWakeTime, interval); } } @@ -137,3 +132,4 @@ ReturnValue_t FixedTimeslotTask::sleepFor(uint32_t ms) { vTaskDelay(pdMS_TO_TICKS(ms)); return HasReturnvaluesIF::RETURN_OK; } + From 3c7ac60dbe2e2e623f6aea1d746418335a2a6dbe Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 22 Jun 2020 20:39:36 +0200 Subject: [PATCH 29/59] added overflow checking --- osal/FreeRTOS/FixedTimeslotTask.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/osal/FreeRTOS/FixedTimeslotTask.cpp b/osal/FreeRTOS/FixedTimeslotTask.cpp index edb7f835d..d92474c79 100644 --- a/osal/FreeRTOS/FixedTimeslotTask.cpp +++ b/osal/FreeRTOS/FixedTimeslotTask.cpp @@ -111,8 +111,15 @@ void FixedTimeslotTask::taskFunctionality() { /* If all operations are finished and the difference of the * current time minus the last wake time is larger than the - * expected wait period, a deadline was missed. */ - if(xTaskGetTickCount() - xLastWakeTime >= interval) { + * expected wait period, a deadline was missed. + * We also check whether an overflow has occured first. + * In this special case, we will ignore missed deadlines for now. */ + const TickType_t constTickCount = xTaskGetTickCount(); + if(constTickCount < xLastWakeTime or + constTickCount + interval < xLastWakeTime) { + // don't do anything for now. + } + else if(xTaskGetTickCount() - xLastWakeTime >= interval) { #ifdef DEBUG sif::warning << "FixedTimeslotTask: " << pcTaskGetName(NULL) << " missed deadline!\n" << std::flush; From bf63ba15fedfacd7ae19de5298d49a0a05154aae Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 22 Jun 2020 23:30:17 +0200 Subject: [PATCH 30/59] finished overflow checking (hopefully) --- osal/FreeRTOS/FixedTimeslotTask.cpp | 55 +++++++++++++++++++---------- osal/FreeRTOS/FixedTimeslotTask.h | 4 +++ 2 files changed, 40 insertions(+), 19 deletions(-) diff --git a/osal/FreeRTOS/FixedTimeslotTask.cpp b/osal/FreeRTOS/FixedTimeslotTask.cpp index d92474c79..d72354aef 100644 --- a/osal/FreeRTOS/FixedTimeslotTask.cpp +++ b/osal/FreeRTOS/FixedTimeslotTask.cpp @@ -109,25 +109,8 @@ void FixedTimeslotTask::taskFunctionality() { intervalMs = this->pst.getIntervalToPreviousSlotMs(); interval = pdMS_TO_TICKS(intervalMs); - /* If all operations are finished and the difference of the - * current time minus the last wake time is larger than the - * expected wait period, a deadline was missed. - * We also check whether an overflow has occured first. - * In this special case, we will ignore missed deadlines for now. */ - const TickType_t constTickCount = xTaskGetTickCount(); - if(constTickCount < xLastWakeTime or - constTickCount + interval < xLastWakeTime) { - // don't do anything for now. - } - else if(xTaskGetTickCount() - xLastWakeTime >= interval) { -#ifdef DEBUG - sif::warning << "FixedTimeslotTask: " << pcTaskGetName(NULL) << - " missed deadline!\n" << std::flush; -#endif - if(deadlineMissedFunc != nullptr) { - this->deadlineMissedFunc(); - } - } + checkMissedDeadline(xLastWakeTime, interval); + // Wait for the interval. This exits immediately if a deadline was // missed while also updating the last wake time. vTaskDelayUntil(&xLastWakeTime, interval); @@ -135,6 +118,40 @@ void FixedTimeslotTask::taskFunctionality() { } } +void FixedTimeslotTask::checkMissedDeadline(const TickType_t xLastWakeTime, + const TickType_t interval) { + /* Check whether deadline was missed while also taking overflows + * into account. Drawing this on paper with a timeline helps to understand + * 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) { + 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) { + handleMissedDeadline(); + } +} + +void FixedTimeslotTask::handleMissedDeadline() { +#ifdef DEBUG + sif::warning << "FixedTimeslotTask: " << pcTaskGetName(NULL) << + " missed deadline!\n" << std::flush; +#endif + if(deadlineMissedFunc != nullptr) { + this->deadlineMissedFunc(); + } +} + ReturnValue_t FixedTimeslotTask::sleepFor(uint32_t ms) { vTaskDelay(pdMS_TO_TICKS(ms)); return HasReturnvaluesIF::RETURN_OK; diff --git a/osal/FreeRTOS/FixedTimeslotTask.h b/osal/FreeRTOS/FixedTimeslotTask.h index 39db667cb..cddd5ecdc 100644 --- a/osal/FreeRTOS/FixedTimeslotTask.h +++ b/osal/FreeRTOS/FixedTimeslotTask.h @@ -89,6 +89,10 @@ protected: * OS's System Calls to keep the timing of the periods. */ void taskFunctionality(void); + + void checkMissedDeadline(const TickType_t xLastWakeTime, + const TickType_t interval); + void handleMissedDeadline(); }; #endif /* POLLINGTASK_H_ */ From 0c9c9c581ba1aa05acc9de1e6af9528c99b1a4fd Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 22 Jun 2020 23:31:27 +0200 Subject: [PATCH 31/59] minor formatting --- osal/FreeRTOS/FixedTimeslotTask.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/osal/FreeRTOS/FixedTimeslotTask.cpp b/osal/FreeRTOS/FixedTimeslotTask.cpp index d72354aef..871b79222 100644 --- a/osal/FreeRTOS/FixedTimeslotTask.cpp +++ b/osal/FreeRTOS/FixedTimeslotTask.cpp @@ -90,9 +90,9 @@ void FixedTimeslotTask::taskFunctionality() { TickType_t xLastWakeTime; /* The xLastWakeTime variable needs to be initialized with the current tick - count. Note that this is the only time the variable is written to explicitly. - After this assignment, xLastWakeTime is updated automatically internally within - vTaskDelayUntil(). */ + count. Note that this is the only time the variable is written to + explicitly. After this assignment, xLastWakeTime is updated automatically + internally within vTaskDelayUntil(). */ xLastWakeTime = xTaskGetTickCount(); // wait for first entry's start time From 1cc50639c7ca8d99377d6bc753f9c6815469b659 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 22 Jun 2020 23:49:31 +0200 Subject: [PATCH 32/59] improvements integrated --- osal/FreeRTOS/FixedTimeslotTask.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/osal/FreeRTOS/FixedTimeslotTask.h b/osal/FreeRTOS/FixedTimeslotTask.h index cddd5ecdc..08a9c3b37 100644 --- a/osal/FreeRTOS/FixedTimeslotTask.h +++ b/osal/FreeRTOS/FixedTimeslotTask.h @@ -12,7 +12,7 @@ class FixedTimeslotTask: public FixedTimeslotTaskIF { public: /** - * Keep in Mind that you need to call before this vTaskStartScheduler()! + * Keep in mind that you need to call before vTaskStartScheduler()! * A lot of task parameters are set in "FreeRTOSConfig.h". * @param name Name of the task, lenght limited by configMAX_TASK_NAME_LEN * @param setPriority Number of priorities specified by @@ -49,13 +49,13 @@ public: static uint32_t deadlineMissedCount; ReturnValue_t addSlot(object_id_t componentId, uint32_t slotTimeMs, - int8_t executionStep); + int8_t executionStep) override; - uint32_t getPeriodMs() const; + uint32_t getPeriodMs() const override; - ReturnValue_t checkSequence() const; + ReturnValue_t checkSequence() const override; - ReturnValue_t sleepFor(uint32_t ms); + ReturnValue_t sleepFor(uint32_t ms) override; protected: bool started; From 4507bdfb69f9a8239b8af6c51d1f9c1a32338ae8 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 23 Jun 2020 01:14:28 +0200 Subject: [PATCH 33/59] added setting task IF --- osal/FreeRTOS/PeriodicTask.cpp | 62 +++++++++++++++++++++++--------- osal/FreeRTOS/PeriodicTask.h | 13 ++++--- osal/linux/PeriodicPosixTask.cpp | 9 +++-- osal/linux/PeriodicPosixTask.h | 7 ++-- tasks/PeriodicTaskIF.h | 26 +++++++++++--- 5 files changed, 87 insertions(+), 30 deletions(-) diff --git a/osal/FreeRTOS/PeriodicTask.cpp b/osal/FreeRTOS/PeriodicTask.cpp index 318676393..170eb0a48 100644 --- a/osal/FreeRTOS/PeriodicTask.cpp +++ b/osal/FreeRTOS/PeriodicTask.cpp @@ -70,39 +70,67 @@ void PeriodicTask::taskFunctionality() { xLastWakeTime = xTaskGetTickCount(); /* Enter the loop that defines the task behavior. */ for (;;) { - for (ObjectList::iterator it = objectList.begin(); - it != objectList.end(); ++it) { - (*it)->performOperation(); + for (auto const& object: objectList) { + object->performOperation(); } - /* If all operations are finished and the difference of the - * current time minus the last wake time is larger than the - * wait period, a deadline was missed. */ - if(xTaskGetTickCount() - xLastWakeTime >= xPeriod) { -#ifdef DEBUG - sif::warning << "PeriodicTask: " << pcTaskGetName(NULL) << - " missed deadline!\n" << std::flush; -#endif - if(deadlineMissedFunc != nullptr) { - this->deadlineMissedFunc(); - } - } + checkMissedDeadline(xLastWakeTime, xPeriod); vTaskDelayUntil(&xLastWakeTime, xPeriod); } } -ReturnValue_t PeriodicTask::addComponent(object_id_t object) { +ReturnValue_t PeriodicTask::addComponent(object_id_t object, bool setTaskIF) { ExecutableObjectIF* newObject = objectManager->get( object); - if (newObject == NULL) { + if (newObject == nullptr) { + sif::error << "PeriodicTask::addComponent: Invalid object. Make sure" + "it implement ExecutableObjectIF" << std::endl; return HasReturnvaluesIF::RETURN_FAILED; } objectList.push_back(newObject); + + if(setTaskIF) { + newObject->setTaskIF(this); + } return HasReturnvaluesIF::RETURN_OK; } uint32_t PeriodicTask::getPeriodMs() const { return period * 1000; } + +void PeriodicTask::checkMissedDeadline(const TickType_t xLastWakeTime, + const TickType_t interval) { + /* Check whether deadline was missed while also taking overflows + * into account. Drawing this on paper with a timeline helps to understand + * 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) { + 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) { + handleMissedDeadline(); + } +} + +void PeriodicTask::handleMissedDeadline() { +#ifdef DEBUG + sif::warning << "PeriodicTask: " << pcTaskGetName(NULL) << + " missed deadline!\n" << std::flush; +#endif + if(deadlineMissedFunc != nullptr) { + this->deadlineMissedFunc(); + } +} diff --git a/osal/FreeRTOS/PeriodicTask.h b/osal/FreeRTOS/PeriodicTask.h index b9930a73b..09aa6bc7d 100644 --- a/osal/FreeRTOS/PeriodicTask.h +++ b/osal/FreeRTOS/PeriodicTask.h @@ -54,7 +54,7 @@ public: * The address of the task object is passed as an argument * to the system call. */ - ReturnValue_t startTask(void); + ReturnValue_t startTask() override; /** * Adds an object to the list of objects to be executed. * The objects are executed in the order added. @@ -63,11 +63,12 @@ public: * -@c RETURN_OK on success * -@c RETURN_FAILED if the object could not be added. */ - ReturnValue_t addComponent(object_id_t object); + ReturnValue_t addComponent(object_id_t object, + bool setTaskIF = true) override; - uint32_t getPeriodMs() const; + uint32_t getPeriodMs() const override; - ReturnValue_t sleepFor(uint32_t ms); + ReturnValue_t sleepFor(uint32_t ms) override; protected: bool started; TaskHandle_t handle; @@ -115,6 +116,10 @@ protected: * On missing the deadline, the deadlineMissedFunction is executed. */ void taskFunctionality(void); + + void checkMissedDeadline(const TickType_t xLastWakeTime, + const TickType_t interval); + void handleMissedDeadline(); }; #endif /* PERIODICTASK_H_ */ diff --git a/osal/linux/PeriodicPosixTask.cpp b/osal/linux/PeriodicPosixTask.cpp index b754c3f44..1a06a8bf5 100644 --- a/osal/linux/PeriodicPosixTask.cpp +++ b/osal/linux/PeriodicPosixTask.cpp @@ -20,13 +20,18 @@ void* PeriodicPosixTask::taskEntryPoint(void* arg) { return NULL; } -ReturnValue_t PeriodicPosixTask::addComponent(object_id_t object) { +ReturnValue_t PeriodicPosixTask::addComponent(object_id_t object, + bool addTaskIF) { ExecutableObjectIF* newObject = objectManager->get( object); - if (newObject == NULL) { + if (newObject == nullptr) { return HasReturnvaluesIF::RETURN_FAILED; } objectList.push_back(newObject); + + if(setTaskIF) { + newObject->setTaskIF(this); + } return HasReturnvaluesIF::RETURN_OK; } diff --git a/osal/linux/PeriodicPosixTask.h b/osal/linux/PeriodicPosixTask.h index 43647eda1..e994898ab 100644 --- a/osal/linux/PeriodicPosixTask.h +++ b/osal/linux/PeriodicPosixTask.h @@ -26,11 +26,12 @@ 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); + ReturnValue_t addComponent(object_id_t object, + bool addTaskIF = true) override; - uint32_t getPeriodMs() const; + uint32_t getPeriodMs() const override; - ReturnValue_t sleepFor(uint32_t ms); + ReturnValue_t sleepFor(uint32_t ms) override; private: typedef std::vector ObjectList; //!< Typedef for the List of objects. diff --git a/tasks/PeriodicTaskIF.h b/tasks/PeriodicTaskIF.h index 304a7de60..6f4909771 100644 --- a/tasks/PeriodicTaskIF.h +++ b/tasks/PeriodicTaskIF.h @@ -1,9 +1,11 @@ -#ifndef PERIODICTASKIF_H_ -#define PERIODICTASKIF_H_ +#ifndef FRAMEWORK_TASK_PERIODICTASKIF_H_ +#define FRAMEWORK_TASK_PERIODICTASKIF_H_ #include +#include #include class ExecutableObjectIF; + /** * New version of TaskIF * Follows RAII principles, i.e. there's no create or delete method. @@ -17,11 +19,27 @@ public: */ virtual ~PeriodicTaskIF() { } /** - * @brief With the startTask method, a created task can be started for the first time. + * @brief With the startTask method, a created task can be started + * for the first time. */ virtual ReturnValue_t startTask() = 0; - virtual ReturnValue_t addComponent(object_id_t object) {return HasReturnvaluesIF::RETURN_FAILED;}; + /** + * Add a component (object) to a periodic task. The pointer to the + * task can be set optionally + * @param object + * Add an object to the task. The most important case is to add an + * executable object with a function which will be called regularly + * (see ExecutableObjectIF) + * @param setTaskIF + * Can be used to specify whether the task object pointer is passed + * to the component. + * @return + */ + virtual ReturnValue_t addComponent(object_id_t object, + bool setTaskIF = true) { + return HasReturnvaluesIF::RETURN_FAILED; + }; virtual ReturnValue_t sleepFor(uint32_t ms) = 0; From f6d2549534e8c4baa59e718781d3384c1d7922eb Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 24 Jun 2020 11:35:07 +0200 Subject: [PATCH 34/59] requested changed implemented --- container/HybridIterator.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/container/HybridIterator.h b/container/HybridIterator.h index db1ce4bc8..79f0485f6 100644 --- a/container/HybridIterator.h +++ b/container/HybridIterator.h @@ -17,8 +17,8 @@ public: } HybridIterator(LinkedElement *start) : - LinkedElement::Iterator(start), value( - start->value), linked(true), end(NULL) { + LinkedElement::Iterator(start), value(start->value), + linked(true) { } @@ -64,11 +64,11 @@ public: return tmp; } - bool operator==(const HybridIterator& other) { + bool operator==(const HybridIterator& other) const { return value == other.value; } - bool operator!=(const HybridIterator& other) { + bool operator!=(const HybridIterator& other) const { return !(*this == other); } From a5a53e7f9b67bc6fd9e8df90f354a5502dace4df Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 24 Jun 2020 11:36:10 +0200 Subject: [PATCH 35/59] include guard updated --- container/HybridIterator.h | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/container/HybridIterator.h b/container/HybridIterator.h index 79f0485f6..3dbd47a85 100644 --- a/container/HybridIterator.h +++ b/container/HybridIterator.h @@ -1,5 +1,5 @@ -#ifndef HYBRIDITERATOR_H_ -#define HYBRIDITERATOR_H_ +#ifndef FRAMEWORK_CONTAINER_HYBRIDITERATOR_H_ +#define FRAMEWORK_CONTAINER_HYBRIDITERATOR_H_ #include #include @@ -24,16 +24,16 @@ public: HybridIterator(typename ArrayList::Iterator start, typename ArrayList::Iterator end) : - ArrayList::Iterator(start), value(start.value), linked( - false), end(end.value) { + ArrayList::Iterator(start), value(start.value), + linked(false), end(end.value) { if (value == this->end) { value = NULL; } } HybridIterator(T *firstElement, T *lastElement) : - ArrayList::Iterator(firstElement), value(firstElement), linked( - false), end(++lastElement) { + ArrayList::Iterator(firstElement), value(firstElement), + linked(false), end(++lastElement) { if (value == end) { value = NULL; } @@ -42,17 +42,17 @@ public: HybridIterator& operator++() { if (linked) { LinkedElement::Iterator::operator++(); - if (LinkedElement::Iterator::value != NULL) { + if (LinkedElement::Iterator::value != nullptr) { value = LinkedElement::Iterator::value->value; } else { - value = NULL; + value = nullptr; } } else { ArrayList::Iterator::operator++(); value = ArrayList::Iterator::value; if (value == end) { - value = NULL; + value = nullptr; } } return *this; From 19cbac923f885f94f40876aa41e3f163874003e6 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 24 Jun 2020 12:06:08 +0200 Subject: [PATCH 36/59] typo fix --- 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 170eb0a48..44a7b8010 100644 --- a/osal/FreeRTOS/PeriodicTask.cpp +++ b/osal/FreeRTOS/PeriodicTask.cpp @@ -86,7 +86,7 @@ ReturnValue_t PeriodicTask::addComponent(object_id_t object, bool setTaskIF) { object); if (newObject == nullptr) { sif::error << "PeriodicTask::addComponent: Invalid object. Make sure" - "it implement ExecutableObjectIF" << std::endl; + "it implements ExecutableObjectIF!" << std::endl; return HasReturnvaluesIF::RETURN_FAILED; } objectList.push_back(newObject); From 8a56964dab62de52a67563c5446fd7986100db06 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 24 Jun 2020 16:01:17 +0200 Subject: [PATCH 37/59] doc fix, various improvements --- tmtcservices/CommandingServiceBase.cpp | 140 +++++++++++++------------ tmtcservices/CommandingServiceBase.h | 53 ++++++---- 2 files changed, 102 insertions(+), 91 deletions(-) diff --git a/tmtcservices/CommandingServiceBase.cpp b/tmtcservices/CommandingServiceBase.cpp index 4c557f82d..acf663893 100644 --- a/tmtcservices/CommandingServiceBase.cpp +++ b/tmtcservices/CommandingServiceBase.cpp @@ -80,39 +80,49 @@ void CommandingServiceBase::handleCommandQueue() { ReturnValue_t result = RETURN_FAILED; for (result = commandQueue->receiveMessage(&reply); result == RETURN_OK; result = commandQueue->receiveMessage(&reply)) { - handleCommandMessage(reply); + handleCommandMessage(&reply); } } -void CommandingServiceBase::handleCommandMessage(CommandMessage& reply) { +void CommandingServiceBase::handleCommandMessage(CommandMessage* reply) { bool isStep = false; CommandMessage nextCommand; - CommandMapIter iter; - if (reply.getSender() == MessageQueueIF::NO_QUEUE) { - handleUnrequestedReply(&reply); - return; - } - if ((iter = commandMap.find(reply.getSender())) == commandMap.end()) { - handleUnrequestedReply(&reply); + CommandMapIter iter = commandMap.find(reply->getSender()); + + // handle unrequested reply first + if (reply->getSender() == MessageQueueIF::NO_QUEUE or + iter == commandMap.end()) { + handleUnrequestedReply(reply); return; } nextCommand.setCommand(CommandMessage::CMD_NONE); + // Implemented by child class, specifies what to do with reply. - ReturnValue_t result = handleReply(&reply, iter->command, &iter->state, + ReturnValue_t result = handleReply(reply, iter->command, &iter->state, &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 */ + if(reply->getCommand() == CommandMessage::REPLY_REJECTED and + result == RETURN_FAILED) { + result = reply->getReplyRejectedReason( + reinterpret_cast(&failureParameter1)); + } + switch (result) { case EXECUTION_COMPLETE: case RETURN_OK: case NO_STEP_MESSAGE: // handle result of reply handler implemented by developer. - handleReplyHandlerResult(result, iter, nextCommand, reply, isStep); + handleReplyHandlerResult(result, iter, &nextCommand, reply, isStep); break; case INVALID_REPLY: //might be just an unrequested reply at a bad moment - handleUnrequestedReply(&reply); + handleUnrequestedReply(reply); break; default: if (isStep) { @@ -129,24 +139,24 @@ void CommandingServiceBase::handleCommandMessage(CommandMessage& reply) { } failureParameter1 = 0; failureParameter2 = 0; - checkAndExecuteFifo(&iter); + checkAndExecuteFifo(iter); break; } } void CommandingServiceBase::handleReplyHandlerResult(ReturnValue_t result, - CommandMapIter iter, CommandMessage& nextCommand, CommandMessage& reply, - bool& isStep) { - iter->command = nextCommand.getCommand(); + CommandMapIter iter, CommandMessage* nextCommand, + CommandMessage* reply, bool& isStep) { + iter->command = nextCommand->getCommand(); // In case a new command is to be sent immediately, this is performed here. // If no new command is sent, only analyse reply result by initializing // sendResult as RETURN_OK ReturnValue_t sendResult = RETURN_OK; - if (nextCommand.getCommand() != CommandMessage::CMD_NONE) { - sendResult = commandQueue->sendMessage(reply.getSender(), - &nextCommand); + if (nextCommand->getCommand() != CommandMessage::CMD_NONE) { + sendResult = commandQueue->sendMessage(reply->getSender(), + nextCommand); } if (sendResult == RETURN_OK) { @@ -161,19 +171,19 @@ void CommandingServiceBase::handleReplyHandlerResult(ReturnValue_t result, TC_VERIFY::COMPLETION_SUCCESS, iter->tcInfo.ackFlags, iter->tcInfo.tcPacketId, iter->tcInfo.tcSequenceControl, 0); - checkAndExecuteFifo(&iter); + checkAndExecuteFifo(iter); } } else { if (isStep) { - nextCommand.clearCommandMessage(); + nextCommand->clear(); verificationReporter.sendFailureReport( TC_VERIFY::PROGRESS_FAILURE, iter->tcInfo.ackFlags, iter->tcInfo.tcPacketId, iter->tcInfo.tcSequenceControl, sendResult, ++iter->step, failureParameter1, failureParameter2); } else { - nextCommand.clearCommandMessage(); + nextCommand->clear(); verificationReporter.sendFailureReport( TC_VERIFY::COMPLETION_FAILURE, iter->tcInfo.ackFlags, iter->tcInfo.tcPacketId, @@ -182,7 +192,7 @@ void CommandingServiceBase::handleReplyHandlerResult(ReturnValue_t result, } failureParameter1 = 0; failureParameter2 = 0; - checkAndExecuteFifo(&iter); + checkAndExecuteFifo(iter); } } @@ -198,8 +208,8 @@ void CommandingServiceBase::handleRequestQueue() { address = message.getStorageId(); packet.setStoreAddress(address); - if ((packet.getSubService() == 0) - || (isValidSubservice(packet.getSubService()) != RETURN_OK)) { + if (packet.getSubService() == 0 + or isValidSubservice(packet.getSubService()) != RETURN_OK) { rejectPacket(TC_VERIFY::START_FAILURE, &packet, INVALID_SUBSERVICE); continue; } @@ -212,8 +222,7 @@ void CommandingServiceBase::handleRequestQueue() { } //Is a command already active for the target object? - typename FixedMap::Iterator iter; + CommandMapIter iter; iter = commandMap.find(queue); if (iter != commandMap.end()) { @@ -228,7 +237,7 @@ void CommandingServiceBase::handleRequestQueue() { if (result != RETURN_OK) { rejectPacket(TC_VERIFY::START_FAILURE, &packet, BUSY); } else { - startExecution(&packet, &iter); + startExecution(&packet, iter); } } @@ -281,46 +290,44 @@ ReturnValue_t CommandingServiceBase::sendTmPacket(uint8_t subservice, } -void CommandingServiceBase::startExecution( - TcPacketStored *storedPacket, - typename FixedMap::Iterator *iter) { - ReturnValue_t result, sendResult = RETURN_OK; - CommandMessage message; - (*iter)->subservice = storedPacket->getSubService(); - result = prepareCommand(&message, (*iter)->subservice, - storedPacket->getApplicationData(), - storedPacket->getApplicationDataSize(), &(*iter)->state, - (*iter)->objectId); +void CommandingServiceBase::startExecution(TcPacketStored *storedPacket, + CommandMapIter iter) { + ReturnValue_t result = RETURN_OK; + CommandMessage command; + iter->subservice = storedPacket->getSubService(); + result = prepareCommand(&command, iter->subservice, + storedPacket->getApplicationData(), + storedPacket->getApplicationDataSize(), &iter->state, + iter->objectId); + ReturnValue_t sendResult = RETURN_OK; switch (result) { case RETURN_OK: - if (message.getCommand() != CommandMessage::CMD_NONE) { - sendResult = commandQueue->sendMessage((*iter).value->first, - &message); + if (command.getCommand() != CommandMessage::CMD_NONE) { + sendResult = commandQueue->sendMessage(iter.value->first, + &command); } if (sendResult == RETURN_OK) { - Clock::getUptime(&(*iter)->uptimeOfStart); - (*iter)->step = 0; -// (*iter)->state = 0; - (*iter)->subservice = storedPacket->getSubService(); - (*iter)->command = message.getCommand(); - (*iter)->tcInfo.ackFlags = storedPacket->getAcknowledgeFlags(); - (*iter)->tcInfo.tcPacketId = storedPacket->getPacketId(); - (*iter)->tcInfo.tcSequenceControl = + Clock::getUptime(&iter->uptimeOfStart); + iter->step = 0; + iter->subservice = storedPacket->getSubService(); + iter->command = command.getCommand(); + iter->tcInfo.ackFlags = storedPacket->getAcknowledgeFlags(); + iter->tcInfo.tcPacketId = storedPacket->getPacketId(); + iter->tcInfo.tcSequenceControl = storedPacket->getPacketSequenceControl(); acceptPacket(TC_VERIFY::START_SUCCESS, storedPacket); } else { - message.clearCommandMessage(); + command.clear(); rejectPacket(TC_VERIFY::START_FAILURE, storedPacket, sendResult); checkAndExecuteFifo(iter); } break; case EXECUTION_COMPLETE: - if (message.getCommand() != CommandMessage::CMD_NONE) { + if (command.getCommand() != CommandMessage::CMD_NONE) { //Fire-and-forget command. - sendResult = commandQueue->sendMessage((*iter).value->first, - &message); + sendResult = commandQueue->sendMessage(iter.value->first, + &command); } if (sendResult == RETURN_OK) { verificationReporter.sendSuccessReport(TC_VERIFY::START_SUCCESS, @@ -328,7 +335,7 @@ void CommandingServiceBase::startExecution( acceptPacket(TC_VERIFY::COMPLETION_SUCCESS, storedPacket); checkAndExecuteFifo(iter); } else { - message.clearCommandMessage(); + command.clear(); rejectPacket(TC_VERIFY::START_FAILURE, storedPacket, sendResult); checkAndExecuteFifo(iter); } @@ -355,12 +362,10 @@ void CommandingServiceBase::acceptPacket(uint8_t reportId, } -void CommandingServiceBase::checkAndExecuteFifo( - typename FixedMap::Iterator *iter) { +void CommandingServiceBase::checkAndExecuteFifo(CommandMapIter iter) { store_address_t address; - if ((*iter)->fifo.retrieve(&address) != RETURN_OK) { - commandMap.erase(iter); + if (iter->fifo.retrieve(&address) != RETURN_OK) { + commandMap.erase(&iter); } else { TcPacketStored newPacket(address); startExecution(&newPacket, iter); @@ -368,9 +373,8 @@ void CommandingServiceBase::checkAndExecuteFifo( } -void CommandingServiceBase::handleUnrequestedReply( - CommandMessage* reply) { - reply->clearCommandMessage(); +void CommandingServiceBase::handleUnrequestedReply(CommandMessage* reply) { + reply->clear(); } @@ -384,18 +388,18 @@ MessageQueueId_t CommandingServiceBase::getCommandQueue() { void CommandingServiceBase::checkTimeout() { uint32_t uptime; Clock::getUptime(&uptime); - typename FixedMap::Iterator iter; + CommandMapIter iter; for (iter = commandMap.begin(); iter != commandMap.end(); ++iter) { if ((iter->uptimeOfStart + (timeoutSeconds * 1000)) < uptime) { verificationReporter.sendFailureReport( TC_VERIFY::COMPLETION_FAILURE, iter->tcInfo.ackFlags, iter->tcInfo.tcPacketId, iter->tcInfo.tcSequenceControl, TIMEOUT); - checkAndExecuteFifo(&iter); + checkAndExecuteFifo(iter); } } } - - +void CommandingServiceBase::setTaskIF(PeriodicTaskIF* task_) { + executingTask = task_; +} diff --git a/tmtcservices/CommandingServiceBase.h b/tmtcservices/CommandingServiceBase.h index 9a68a2d3c..277e5589e 100644 --- a/tmtcservices/CommandingServiceBase.h +++ b/tmtcservices/CommandingServiceBase.h @@ -97,9 +97,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_){ - executingTask = task_; - }; + virtual void setTaskIF(PeriodicTaskIF* task_); protected: /** @@ -141,15 +139,15 @@ protected: * @param objectId Target object ID * @return */ - virtual ReturnValue_t prepareCommand(CommandMessage *message, + virtual ReturnValue_t prepareCommand(CommandMessage* message, uint8_t subservice, const uint8_t *tcData, size_t tcDataLen, uint32_t *state, object_id_t objectId) = 0; /** * This function is implemented by child services to specify how replies - * to a command from another software component are handled + * to a command from another software component are handled. * @param reply - * This is the reply in form of a command message. + * This is the reply in form of a generic read-only command message. * @param previousCommand * Command_t of related command * @param state [out/in] @@ -163,19 +161,26 @@ 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 + * - 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. */ - virtual ReturnValue_t handleReply(const CommandMessage *reply, + virtual ReturnValue_t handleReply(const CommandMessage* reply, Command_t previousCommand, uint32_t *state, - CommandMessage *optionalNextCommand, object_id_t objectId, + CommandMessage* optionalNextCommand, object_id_t objectId, bool *isStep) = 0; /** * This function can be overidden to handle unrequested reply, * when the reply sender ID is unknown or is not found is the command map. + * The default implementation will clear the command message and all + * its contents. * @param reply + * Reply which is non-const so the default implementation can clear the + * message. */ - virtual void handleUnrequestedReply(CommandMessage *reply); + virtual void handleUnrequestedReply(CommandMessage* reply); virtual void doPeriodicOperation(); @@ -195,6 +200,9 @@ protected: FIFO fifo; }; + using CommandMapIter = FixedMap::Iterator; + const uint16_t apid; const uint8_t service; @@ -263,21 +271,21 @@ protected: ReturnValue_t sendTmPacket(uint8_t subservice, SerializeIF* content, SerializeIF* header = nullptr); - void checkAndExecuteFifo( - typename FixedMap::Iterator *iter); + void checkAndExecuteFifo(CommandMapIter iter); private: - using CommandMapIter = FixedMap::Iterator; /** * This method handles internal execution of a command, - * once it has been started by @sa{startExecution()} in the Request Queue handler. - * 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. + * once it has been started by @sa{startExecution()} in the request + * queue handler. + * 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. Note that - * the static framework object ID @c VerificationReporter::messageReceiver needs to be set. + * 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 @@ -298,12 +306,11 @@ private: void acceptPacket(uint8_t reportId, TcPacketStored* packet); - void startExecution(TcPacketStored *storedPacket, - typename FixedMap::Iterator *iter); + void startExecution(TcPacketStored *storedPacket, CommandMapIter iter); - void handleCommandMessage(CommandMessage& reply); + void handleCommandMessage(CommandMessage* reply); void handleReplyHandlerResult(ReturnValue_t result, CommandMapIter iter, - CommandMessage& nextCommand,CommandMessage& reply, bool& isStep); + CommandMessage* nextCommand, CommandMessage* reply, bool& isStep); void checkTimeout(); }; From 644896245ff2eca019ebb0a5e9e0840f2b1dcc22 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 24 Jun 2020 16:03:54 +0200 Subject: [PATCH 38/59] better returnvalues for CSB init --- objectmanager/ObjectManagerIF.h | 3 ++- tmtcservices/CommandingServiceBase.cpp | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/objectmanager/ObjectManagerIF.h b/objectmanager/ObjectManagerIF.h index 90a1ce3be..dcef81cd6 100644 --- a/objectmanager/ObjectManagerIF.h +++ b/objectmanager/ObjectManagerIF.h @@ -21,7 +21,8 @@ public: static constexpr uint8_t INTERFACE_ID = CLASS_ID::OBJECT_MANAGER_IF; static constexpr ReturnValue_t INSERTION_FAILED = MAKE_RETURN_CODE( 1 ); static constexpr ReturnValue_t NOT_FOUND = MAKE_RETURN_CODE( 2 ); - static constexpr ReturnValue_t CHILD_INIT_FAILED = MAKE_RETURN_CODE( 3 ); + + static constexpr ReturnValue_t CHILD_INIT_FAILED = MAKE_RETURN_CODE( 3 ); //!< Can be used if the initialization of a SystemObject failed. static constexpr ReturnValue_t INTERNAL_ERR_REPORTER_UNINIT = MAKE_RETURN_CODE( 4 ); protected: diff --git a/tmtcservices/CommandingServiceBase.cpp b/tmtcservices/CommandingServiceBase.cpp index acf663893..abf5b0edb 100644 --- a/tmtcservices/CommandingServiceBase.cpp +++ b/tmtcservices/CommandingServiceBase.cpp @@ -57,7 +57,7 @@ ReturnValue_t CommandingServiceBase::initialize() { PUSDistributorIF* distributor = objectManager->get( packetSource); if (packetForwarding == nullptr or distributor == nullptr) { - return RETURN_FAILED; + return ObjectManagerIF::CHILD_INIT_FAILED; } distributor->registerService(this); @@ -68,7 +68,7 @@ ReturnValue_t CommandingServiceBase::initialize() { TCStore = objectManager->get(objects::TC_STORE); if (IPCStore == nullptr or TCStore == nullptr) { - return RETURN_FAILED; + return ObjectManagerIF::CHILD_INIT_FAILED; } return RETURN_OK; From fc0d42e3e06e6d7c632f8a60ea0e5fe0e0972c7c Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 24 Jun 2020 16:07:02 +0200 Subject: [PATCH 39/59] error output for CSB init failure --- tmtcservices/CommandingServiceBase.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tmtcservices/CommandingServiceBase.cpp b/tmtcservices/CommandingServiceBase.cpp index abf5b0edb..f370cc6f7 100644 --- a/tmtcservices/CommandingServiceBase.cpp +++ b/tmtcservices/CommandingServiceBase.cpp @@ -57,6 +57,8 @@ ReturnValue_t CommandingServiceBase::initialize() { PUSDistributorIF* distributor = objectManager->get( packetSource); if (packetForwarding == nullptr or distributor == nullptr) { + sif::error << "CommandingServiceBase::intialize: Packet source or " + "packet destination invalid!" << std::endl; return ObjectManagerIF::CHILD_INIT_FAILED; } @@ -68,6 +70,8 @@ ReturnValue_t CommandingServiceBase::initialize() { TCStore = objectManager->get(objects::TC_STORE); if (IPCStore == nullptr or TCStore == nullptr) { + sif::error << "CommandingServiceBase::intialize: IPC store or TC store " + "not initialized yet!" << std::endl; return ObjectManagerIF::CHILD_INIT_FAILED; } From ce3e4a1176afd5170fb9dce62c2737c88d7d9786 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 24 Jun 2020 16:24:16 +0200 Subject: [PATCH 40/59] added getter for reject reply --- ipc/CommandMessage.cpp | 11 +++++++++++ ipc/CommandMessage.h | 2 ++ tmtcservices/CommandingServiceBase.cpp | 12 ++++++------ 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/ipc/CommandMessage.cpp b/ipc/CommandMessage.cpp index fa41c6531..991c9df87 100644 --- a/ipc/CommandMessage.cpp +++ b/ipc/CommandMessage.cpp @@ -120,3 +120,14 @@ void CommandMessage::setReplyRejected(ReturnValue_t reason, setParameter(reason); setParameter2(initialCommand); } + +ReturnValue_t CommandMessage::getReplyRejectedReason( + Command_t *initialCommand) const { + ReturnValue_t reason = HasReturnvaluesIF::RETURN_FAILED; + std::memcpy(&reason, getData(), sizeof(reason)); + if(initialCommand != nullptr) { + std::memcpy(initialCommand, getData() + sizeof(reason), + sizeof(Command_t)); + } + return reason; +} diff --git a/ipc/CommandMessage.h b/ipc/CommandMessage.h index 2d966063a..b800b9d21 100644 --- a/ipc/CommandMessage.h +++ b/ipc/CommandMessage.h @@ -124,6 +124,8 @@ public: */ void setToUnknownCommand(); void setReplyRejected(ReturnValue_t reason, Command_t initialCommand = CMD_NONE); + ReturnValue_t getReplyRejectedReason(Command_t *initialCommand) const; + size_t getMinimumMessageSize() const; }; diff --git a/tmtcservices/CommandingServiceBase.cpp b/tmtcservices/CommandingServiceBase.cpp index f370cc6f7..e3ecada17 100644 --- a/tmtcservices/CommandingServiceBase.cpp +++ b/tmtcservices/CommandingServiceBase.cpp @@ -180,14 +180,14 @@ void CommandingServiceBase::handleReplyHandlerResult(ReturnValue_t result, } else { if (isStep) { - nextCommand->clear(); + nextCommand->clearCommandMessage(); verificationReporter.sendFailureReport( TC_VERIFY::PROGRESS_FAILURE, iter->tcInfo.ackFlags, iter->tcInfo.tcPacketId, iter->tcInfo.tcSequenceControl, sendResult, ++iter->step, failureParameter1, failureParameter2); } else { - nextCommand->clear(); + nextCommand->clearCommandMessage(); verificationReporter.sendFailureReport( TC_VERIFY::COMPLETION_FAILURE, iter->tcInfo.ackFlags, iter->tcInfo.tcPacketId, @@ -267,7 +267,7 @@ ReturnValue_t CommandingServiceBase::sendTmPacket(uint8_t subservice, object_id_t objectId, const uint8_t *data, size_t dataLen) { uint8_t buffer[sizeof(object_id_t)]; uint8_t* pBuffer = buffer; - size_t size = 0; + uint32_t size = 0; SerializeAdapter::serialize(&objectId, &pBuffer, &size, sizeof(object_id_t), true); TmPacketStored tmPacketStored(this->apid, this->service, subservice, @@ -322,7 +322,7 @@ void CommandingServiceBase::startExecution(TcPacketStored *storedPacket, storedPacket->getPacketSequenceControl(); acceptPacket(TC_VERIFY::START_SUCCESS, storedPacket); } else { - command.clear(); + command.clearCommandMessage(); rejectPacket(TC_VERIFY::START_FAILURE, storedPacket, sendResult); checkAndExecuteFifo(iter); } @@ -339,7 +339,7 @@ void CommandingServiceBase::startExecution(TcPacketStored *storedPacket, acceptPacket(TC_VERIFY::COMPLETION_SUCCESS, storedPacket); checkAndExecuteFifo(iter); } else { - command.clear(); + command.clearCommandMessage(); rejectPacket(TC_VERIFY::START_FAILURE, storedPacket, sendResult); checkAndExecuteFifo(iter); } @@ -378,7 +378,7 @@ void CommandingServiceBase::checkAndExecuteFifo(CommandMapIter iter) { void CommandingServiceBase::handleUnrequestedReply(CommandMessage* reply) { - reply->clear(); + reply->clearCommandMessage(); } From 1ed5da3a122c98b01b9d59cac646bc9064cd33f5 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 24 Jun 2020 16:26:44 +0200 Subject: [PATCH 41/59] getter function bugfix --- ipc/CommandMessage.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/ipc/CommandMessage.cpp b/ipc/CommandMessage.cpp index 991c9df87..679793e3f 100644 --- a/ipc/CommandMessage.cpp +++ b/ipc/CommandMessage.cpp @@ -123,11 +123,9 @@ void CommandMessage::setReplyRejected(ReturnValue_t reason, ReturnValue_t CommandMessage::getReplyRejectedReason( Command_t *initialCommand) const { - ReturnValue_t reason = HasReturnvaluesIF::RETURN_FAILED; - std::memcpy(&reason, getData(), sizeof(reason)); + ReturnValue_t reason = getParameter(); if(initialCommand != nullptr) { - std::memcpy(initialCommand, getData() + sizeof(reason), - sizeof(Command_t)); + *initialCommand = getParameter2(); } return reason; } From 053b47215786d38b0f604755e0b712300908296a Mon Sep 17 00:00:00 2001 From: Steffen Gaisser Date: Thu, 25 Jun 2020 18:09:32 +0200 Subject: [PATCH 42/59] 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 50a5c07e5..430b24e0e 100644 --- a/devicehandlers/ChildHandlerBase.cpp +++ b/devicehandlers/ChildHandlerBase.cpp @@ -34,7 +34,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 90875838e..2e24e0d5a 100644 --- a/devicehandlers/DeviceHandlerBase.cpp +++ b/devicehandlers/DeviceHandlerBase.cpp @@ -1072,7 +1072,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 ea0b99ffa..61a824215 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 ed97f3d3d..7b8b3a53f 100644 --- a/health/HealthHelper.cpp +++ b/health/HealthHelper.cpp @@ -29,11 +29,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 a1ca2c525..471bc7e9f 100644 --- a/health/HealthHelper.h +++ b/health/HealthHelper.h @@ -78,7 +78,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 bfd49caab4dd4432a575cc13738a818167d62c43 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Fri, 26 Jun 2020 12:52:03 +0200 Subject: [PATCH 43/59] fixed include guard comment --- container/HybridIterator.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/container/HybridIterator.h b/container/HybridIterator.h index 3dbd47a85..f2fd6b288 100644 --- a/container/HybridIterator.h +++ b/container/HybridIterator.h @@ -87,4 +87,4 @@ private: T *end = nullptr; }; -#endif /* HYBRIDITERATOR_H_ */ +#endif /* FRAMEWORK_CONTAINER_HYBRIDITERATOR_H_ */ From 5400e38126d89730af33177f60963051659a985f Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 29 Jun 2020 16:53:32 +0200 Subject: [PATCH 44/59] slight change --- tmtcservices/CommandingServiceBase.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tmtcservices/CommandingServiceBase.cpp b/tmtcservices/CommandingServiceBase.cpp index e3ecada17..d69a0f03f 100644 --- a/tmtcservices/CommandingServiceBase.cpp +++ b/tmtcservices/CommandingServiceBase.cpp @@ -113,8 +113,8 @@ void CommandingServiceBase::handleCommandMessage(CommandMessage* reply) { * command as failure parameter 1 */ if(reply->getCommand() == CommandMessage::REPLY_REJECTED and result == RETURN_FAILED) { - result = reply->getReplyRejectedReason( - reinterpret_cast(&failureParameter1)); + result = reply->getReplyRejectedReason(); + failureParameter1 = iter->command; } switch (result) { From d311c499981bbd247466e1da6bf5f84999931bad Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 29 Jun 2020 16:57:00 +0200 Subject: [PATCH 45/59] setTaskIF implemented --- tmtcservices/PusServiceBase.cpp | 11 +++++++++++ tmtcservices/PusServiceBase.h | 17 +++++++++++++---- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/tmtcservices/PusServiceBase.cpp b/tmtcservices/PusServiceBase.cpp index f5d1e30bd..82e5ff5cc 100644 --- a/tmtcservices/PusServiceBase.cpp +++ b/tmtcservices/PusServiceBase.cpp @@ -32,6 +32,10 @@ ReturnValue_t PusServiceBase::performOperation(uint8_t opCode) { return RETURN_OK; } +void PusServiceBase::setTaskIF(PeriodicTaskIF* taskHandle) { + this->taskHandle = taskHandle; +} + void PusServiceBase::handleRequestQueue() { TmTcMessage message; ReturnValue_t result = RETURN_FAILED; @@ -108,3 +112,10 @@ ReturnValue_t PusServiceBase::initialize() { return RETURN_FAILED; } } + +ReturnValue_t PusServiceBase::initializeAfterTaskCreation() { + // If task parameters, for example task frequency are required, this + // function should be overriden and the system object task IF can + // be used to get those parameters. + return HasReturnvaluesIF::RETURN_OK; +} diff --git a/tmtcservices/PusServiceBase.h b/tmtcservices/PusServiceBase.h index 39600c413..6d3d9bac6 100644 --- a/tmtcservices/PusServiceBase.h +++ b/tmtcservices/PusServiceBase.h @@ -96,11 +96,20 @@ public: * @return @c RETURN_OK if the periodic performService was successful. * @c RETURN_FAILED else. */ - ReturnValue_t performOperation(uint8_t opCode); - virtual uint16_t getIdentifier(); - MessageQueueId_t getRequestQueue(); - virtual ReturnValue_t initialize(); + ReturnValue_t performOperation(uint8_t opCode) override; + virtual uint16_t getIdentifier() override; + MessageQueueId_t getRequestQueue() override; + virtual ReturnValue_t initialize() override; + + virtual void setTaskIF(PeriodicTaskIF* taskHandle) override; + virtual ReturnValue_t initializeAfterTaskCreation() override; protected: + /** + * @brief Handle to the underlying task + * @details + * Will be set by setTaskIF(), which is called on task creation. + */ + PeriodicTaskIF* taskHandle = nullptr; /** * The APID of this instance of the Service. */ From 6a0a2675b1bba3f69e81224d935ac93166306c97 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 6 Jul 2020 12:36:01 +0200 Subject: [PATCH 46/59] typedef instead of auto used now --- 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 223cf4bd4..c062101d9 100644 --- a/osal/FreeRTOS/FixedTimeslotTask.cpp +++ b/osal/FreeRTOS/FixedTimeslotTask.cpp @@ -82,10 +82,10 @@ ReturnValue_t FixedTimeslotTask::checkSequence() const { void FixedTimeslotTask::taskFunctionality() { // A local iterator for the Polling Sequence Table is created to find the // start time for the first entry. - auto slotListIter = pst.current; + FixedSlotSequence::SlotListIter slotListIter = pst.current; //The start time for the first entry is read. - uint32_t intervalMs = (*slotListIter)->pollingTimeMs; + uint32_t intervalMs = slotListIter->pollingTimeMs; TickType_t interval = pdMS_TO_TICKS(intervalMs); TickType_t xLastWakeTime; From 133ed9586b2ecceccdf92316f25d7cbf2c551c0d Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Thu, 9 Jul 2020 19:31:28 +0200 Subject: [PATCH 47/59] tmtc bridge improvements --- tmtcservices/TmTcBridge.cpp | 58 +++++++++++++++++++++++-------------- tmtcservices/TmTcBridge.h | 53 +++++++++++++++++---------------- 2 files changed, 63 insertions(+), 48 deletions(-) diff --git a/tmtcservices/TmTcBridge.cpp b/tmtcservices/TmTcBridge.cpp index f9a7d3bc5..8c44f7de0 100644 --- a/tmtcservices/TmTcBridge.cpp +++ b/tmtcservices/TmTcBridge.cpp @@ -5,11 +5,13 @@ #include #include -TmTcBridge::TmTcBridge(object_id_t objectId_, - object_id_t ccsdsPacketDistributor_): SystemObject(objectId_), - ccsdsPacketDistributor(ccsdsPacketDistributor_) +TmTcBridge::TmTcBridge(object_id_t objectId, object_id_t tcDestination, + object_id_t tmStoreId, object_id_t tcStoreId): + SystemObject(objectId),tmStoreId(tmStoreId), tcStoreId(tcStoreId), + tcDestination(tcDestination) + { - TmTcReceptionQueue = QueueFactory::instance()-> + tmTcReceptionQueue = QueueFactory::instance()-> createMessageQueue(TMTC_RECEPTION_QUEUE_DEPTH); } @@ -42,20 +44,27 @@ ReturnValue_t TmTcBridge::setMaxNumberOfPacketsStored( } ReturnValue_t TmTcBridge::initialize() { - tcStore = objectManager->get(objects::TC_STORE); - if (tcStore == NULL) { - return RETURN_FAILED; + tcStore = objectManager->get(tcStoreId); + if (tcStore == nullptr) { + sif::error << "TmTcBridge::initialize: TC store invalid. Make sure" + "it is created and set up properly." << std::endl; + return ObjectManagerIF::CHILD_INIT_FAILED; } - tmStore = objectManager->get(objects::TM_STORE); - if (tmStore == NULL) { - return RETURN_FAILED; + tmStore = objectManager->get(tmStoreId); + if (tmStore == nullptr) { + sif::error << "TmTcBridge::initialize: TM store invalid. Make sure" + "it is created and set up properly." << std::endl; + return ObjectManagerIF::CHILD_INIT_FAILED; } AcceptsTelecommandsIF* tcDistributor = - objectManager->get(ccsdsPacketDistributor); - if (tcDistributor == NULL) { - return RETURN_FAILED; + objectManager->get(tcDestination); + if (tcDistributor == nullptr) { + sif::error << "TmTcBridge::initialize: TC Distributor invalid" + << std::endl; + return ObjectManagerIF::CHILD_INIT_FAILED; } - TmTcReceptionQueue->setDefaultDestination(tcDistributor->getRequestQueue()); + + tmTcReceptionQueue->setDefaultDestination(tcDistributor->getRequestQueue()); return RETURN_OK; } @@ -73,10 +82,7 @@ ReturnValue_t TmTcBridge::performOperation(uint8_t operationCode) { } ReturnValue_t TmTcBridge::handleTc() { - uint8_t * recvBuffer = nullptr; - size_t recvLen = 0; - ReturnValue_t result = receiveTc(&recvBuffer, &recvLen); - return result; + return HasReturnvaluesIF::RETURN_OK; } ReturnValue_t TmTcBridge::handleTm() { @@ -97,8 +103,8 @@ ReturnValue_t TmTcBridge::handleTmQueue() { TmTcMessage message; const uint8_t* data = nullptr; size_t size = 0; - for (ReturnValue_t result = TmTcReceptionQueue->receiveMessage(&message); - result == RETURN_OK; result = TmTcReceptionQueue->receiveMessage(&message)) + for (ReturnValue_t result = tmTcReceptionQueue->receiveMessage(&message); + result == RETURN_OK; result = tmTcReceptionQueue->receiveMessage(&message)) { if(communicationLinkUp == false) { result = storeDownlinkData(&message); @@ -183,10 +189,20 @@ void TmTcBridge::registerCommDisconnect() { } MessageQueueId_t TmTcBridge::getReportReceptionQueue(uint8_t virtualChannel) { - return TmTcReceptionQueue->getId(); + return tmTcReceptionQueue->getId(); } void TmTcBridge::printData(uint8_t * data, size_t dataLen) { arrayprinter::print(data, dataLen); } + +uint16_t TmTcBridge::getIdentifier() { + // This is no PUS service, so we just return 0 + return 0; +} + +MessageQueueId_t TmTcBridge::getRequestQueue() { + // Default implementation: Relay TC messages to TC distributor directly. + return tmTcReceptionQueue->getDefaultDestination(); +} diff --git a/tmtcservices/TmTcBridge.h b/tmtcservices/TmTcBridge.h index 3e0432d8a..3e3d1b714 100644 --- a/tmtcservices/TmTcBridge.h +++ b/tmtcservices/TmTcBridge.h @@ -1,16 +1,18 @@ #ifndef FRAMEWORK_TMTCSERVICES_TMTCBRIDGE_H_ #define FRAMEWORK_TMTCSERVICES_TMTCBRIDGE_H_ +#include #include #include #include #include -#include +#include -#include #include +#include class TmTcBridge : public AcceptsTelemetryIF, + public AcceptsTelecommandsIF, public ExecutableObjectIF, public HasReturnvaluesIF, public SystemObject { @@ -22,7 +24,8 @@ public: static constexpr uint8_t DEFAULT_STORED_DATA_SENT_PER_CYCLE = 5; static constexpr uint8_t DEFAULT_DOWNLINK_PACKETS_STORED = 10; - TmTcBridge(object_id_t objectId_, object_id_t ccsdsPacketDistributor_); + TmTcBridge(object_id_t objectId, object_id_t tcDestination, + object_id_t tmStoreId, object_id_t tcStoreId); virtual ~TmTcBridge(); /** @@ -57,45 +60,41 @@ public: */ virtual ReturnValue_t performOperation(uint8_t operationCode = 0) override; - /** - * Return TMTC Reception Queue - * @param virtualChannel - * @return - */ - MessageQueueId_t getReportReceptionQueue( + + /** AcceptsTelemetryIF override */ + virtual MessageQueueId_t getReportReceptionQueue( uint8_t virtualChannel = 0) override; + + /** AcceptsTelecommandsIF override */ + virtual uint16_t getIdentifier() override; + virtual MessageQueueId_t getRequestQueue() override; + protected: + //! Cached for initialize function. + object_id_t tmStoreId = objects::NO_OBJECT; + object_id_t tcStoreId = objects::NO_OBJECT; + object_id_t tcDestination = objects::NO_OBJECT; + //! Used to send and receive TMTC messages. //! TmTcMessage is used to transport messages between tasks. - MessageQueueIF* TmTcReceptionQueue = nullptr; - StorageManagerIF* tcStore = nullptr; + MessageQueueIF* tmTcReceptionQueue = nullptr; + StorageManagerIF* tmStore = nullptr; - object_id_t ccsdsPacketDistributor = 0; - //! Used to specify whether communication link is up - bool communicationLinkUp = false; + StorageManagerIF* tcStore = nullptr; + + //! Used to specify whether communication link is up by default. + bool communicationLinkUp = true; bool tmStored = false; /** * @brief Handle TC reception * @details * Default implementation provided, but is empty. - * Child handler should override this in most cases orsend TC to the - * TC distributor directly with the address of the reception queue by - * calling getReportRecptionQueue() + * In most cases, TC reception will be handled in a separate task anyway. * @return */ virtual ReturnValue_t handleTc(); - /** - * Implemented by child class. Perform receiving of Telecommand, - * for example by implementing specific drivers or wrappers, - * e.g. UART Communication or an ethernet stack - * @param recvBuffer [out] Received data - * @param size [out] Size of received data - * @return - */ - virtual ReturnValue_t receiveTc(uint8_t ** recvBuffer, size_t * size) = 0; - /** * Handle Telemetry. Default implementation provided. * Calls sendTm() From 90e299977bcb0d32672d520ecf94fee666913923 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Thu, 9 Jul 2020 19:41:16 +0200 Subject: [PATCH 48/59] minor formatting improvements --- tmtcservices/TmTcBridge.cpp | 27 +++++++++++++++------------ tmtcservices/TmTcBridge.h | 11 +++++++---- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/tmtcservices/TmTcBridge.cpp b/tmtcservices/TmTcBridge.cpp index 8c44f7de0..b55803cbc 100644 --- a/tmtcservices/TmTcBridge.cpp +++ b/tmtcservices/TmTcBridge.cpp @@ -24,8 +24,9 @@ ReturnValue_t TmTcBridge::setNumberOfSentPacketsPerCycle( return RETURN_OK; } else { - sif::warning << "TmTcBridge: Number of packets sent per cycle " - "exceeds limits. Keeping default value." << std::endl; + sif::warning << "TmTcBridge::setNumberOfSentPacketsPerCycle: Number of " + << "packets sent per cycle exceeds limits. " + << "Keeping default value." << std::endl; return RETURN_FAILED; } } @@ -37,8 +38,9 @@ ReturnValue_t TmTcBridge::setMaxNumberOfPacketsStored( return RETURN_OK; } else { - sif::warning << "TmTcBridge: Number of packets stored " - "exceeds limits. Keeping default value." << std::endl; + sif::warning << "TmTcBridge::setMaxNumberOfPacketsStored: Number of " + << "packets stored exceeds limits. " + << "Keeping default value." << std::endl; return RETURN_FAILED; } } @@ -72,11 +74,13 @@ ReturnValue_t TmTcBridge::performOperation(uint8_t operationCode) { ReturnValue_t result; result = handleTc(); if(result != RETURN_OK) { - sif::error << "TMTC Bridge: Error handling TCs" << std::endl; + sif::debug << "TmTcBridge::performOperation: " + << "Error handling TCs" << std::endl; } result = handleTm(); if (result != RETURN_OK) { - sif::error << "TMTC Bridge: Error handling TMs" << std::endl; + sif::debug << "TmTcBridge::performOperation: " + << "Error handling TMs" << std::endl; } return result; } @@ -88,7 +92,7 @@ ReturnValue_t TmTcBridge::handleTc() { ReturnValue_t TmTcBridge::handleTm() { ReturnValue_t result = handleTmQueue(); if(result != RETURN_OK) { - sif::error << "TMTC Bridge: Reading TM Queue failed" << std::endl; + sif::warning << "TmTcBridge: Reading TM Queue failed" << std::endl; return RETURN_FAILED; } @@ -118,7 +122,7 @@ ReturnValue_t TmTcBridge::handleTmQueue() { result = sendTm(data, size); if (result != RETURN_OK) { - sif::error << "TMTC Bridge: Could not send TM packet"<< std::endl; + sif::warning << "TmTcBridge: Could not send TM packet" << std::endl; tmStore->deleteData(message.getStorageId()); return result; @@ -129,13 +133,12 @@ ReturnValue_t TmTcBridge::handleTmQueue() { } ReturnValue_t TmTcBridge::storeDownlinkData(TmTcMessage *message) { - //debug << "TMTC Bridge: Comm Link down. " - // "Saving packet ID to be sent later\r\n" << std::flush; store_address_t storeId = 0; if(tmFifo.full()) { - sif::error << "TMTC Bridge: TM downlink max. number of stored packet IDs " - "reached! Overwriting old data" << std::endl; + sif::error << "TmTcBridge::storeDownlinkData: TM downlink max. number " + << "of stored packet IDs reached! " + << "Overwriting old data" << std::endl; tmFifo.retrieve(&storeId); tmStore->deleteData(storeId); } diff --git a/tmtcservices/TmTcBridge.h b/tmtcservices/TmTcBridge.h index 3e3d1b714..993cd5b99 100644 --- a/tmtcservices/TmTcBridge.h +++ b/tmtcservices/TmTcBridge.h @@ -76,13 +76,14 @@ protected: object_id_t tcDestination = objects::NO_OBJECT; //! Used to send and receive TMTC messages. - //! TmTcMessage is used to transport messages between tasks. + //! The TmTcMessage class is used to transport messages between tasks. MessageQueueIF* tmTcReceptionQueue = nullptr; StorageManagerIF* tmStore = nullptr; StorageManagerIF* tcStore = nullptr; - //! Used to specify whether communication link is up by default. + //! Used to specify whether communication link is up. Will be true + //! by default, so telemetry will be handled immediately. bool communicationLinkUp = true; bool tmStored = false; @@ -103,7 +104,8 @@ protected: virtual ReturnValue_t handleTm(); /** - * Read the TM Queue and send TM if necessary. Default implementation provided + * Read the TM Queue and send TM if necessary. + * Default implementation provided * @return */ virtual ReturnValue_t handleTmQueue(); @@ -116,7 +118,8 @@ protected: /** * Implemented by child class. Perform sending of Telemetry by implementing - * communication drivers or wrappers, e.g. UART communication or lwIP stack. + * communication drivers or wrappers, e.g. serial communication or a socket + * call. * @param data * @param dataLen * @return From 83484237ae67298190275965c64e83c4268fd4e1 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Thu, 9 Jul 2020 19:53:17 +0200 Subject: [PATCH 49/59] default argument for getter function --- ipc/CommandMessage.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ipc/CommandMessage.h b/ipc/CommandMessage.h index b800b9d21..b75d94b15 100644 --- a/ipc/CommandMessage.h +++ b/ipc/CommandMessage.h @@ -124,7 +124,8 @@ public: */ void setToUnknownCommand(); void setReplyRejected(ReturnValue_t reason, Command_t initialCommand = CMD_NONE); - ReturnValue_t getReplyRejectedReason(Command_t *initialCommand) const; + ReturnValue_t getReplyRejectedReason( + Command_t *initialCommand = nullptr) const; size_t getMinimumMessageSize() const; }; From d1e922eecf6b24bdd01286864a075b63080d4aa0 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Thu, 9 Jul 2020 20:08:40 +0200 Subject: [PATCH 50/59] new intiialize after task creation function --- tasks/ExecutableObjectIF.h | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/tasks/ExecutableObjectIF.h b/tasks/ExecutableObjectIF.h index 5c5955c1c..d716cdfbb 100644 --- a/tasks/ExecutableObjectIF.h +++ b/tasks/ExecutableObjectIF.h @@ -1,15 +1,5 @@ -/** - * @file ExecutableObjectIF.h - * - * @brief This file contains the definition for the ExecutableObjectIF interface. - * - * @author Bastian Baetz - * - * @date 12.03.2012 - */ - -#ifndef EXECUTABLEOBJECTIF_H_ -#define EXECUTABLEOBJECTIF_H_ +#ifndef FRAMEWORK_TASKS_EXECUTABLEOBJECTIF_H_ +#define FRAMEWORK_TASKS_EXECUTABLEOBJECTIF_H_ class PeriodicTaskIF; @@ -20,6 +10,7 @@ class PeriodicTaskIF; * @brief The interface provides a method to execute objects within a task. * @details The performOperation method, that is required by the interface is * executed cyclically within a task context. + * @author Bastian Baetz */ class ExecutableObjectIF { public: @@ -37,13 +28,26 @@ public: /** * @brief Function called during setup assignment of object to task - * @details Has to be called from the function that assigns the object to a task and - * enables the object implementation to overwrite this function and get a reference to the executing task + * @details + * Has to be called from the function that assigns the object to a task and + * enables the object implementation to overwrite this function and get + * a reference to the executing task * @param task_ Pointer to the taskIF of this task */ - virtual void setTaskIF(PeriodicTaskIF* task_) { + virtual void setTaskIF(PeriodicTaskIF* task_) {}; + /** + * This function should be called after the object was assigned to a + * specific task. + * + * Example: Can be used to get task execution frequency. + * The task is created after initialize() and the object ctors have been + * called so the execution frequency can't be cached in initialize() + * @return + */ + virtual ReturnValue_t initializeAfterTaskCreation() { + return HasReturnvaluesIF::RETURN_OK; } }; -#endif /* EXECUTABLEOBJECTIF_H_ */ +#endif /* FRAMEWORK_TASKS_EXECUTABLEOBJECTIF_H_ */ From 72c9ef1089ddc3108c999b9b5aec01bf8bb26376 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 13 Jul 2020 02:12:11 +0200 Subject: [PATCH 51/59] size_t replacement --- health/HealthTableIF.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/health/HealthTableIF.h b/health/HealthTableIF.h index 6fdfc2c04..a850a8048 100644 --- a/health/HealthTableIF.h +++ b/health/HealthTableIF.h @@ -17,7 +17,7 @@ public: HasHealthIF::HealthState initilialState = HasHealthIF::HEALTHY) = 0; virtual uint32_t getPrintSize() = 0; - virtual void printAll(uint8_t *pointer, uint32_t maxSize) = 0; + virtual void printAll(uint8_t *pointer, size_t maxSize) = 0; protected: virtual ReturnValue_t iterate(std::pair *value, bool reset = false) = 0; From 14f86422e366390583a8070f685b390c2eccb04f Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 13 Jul 2020 19:47:31 +0200 Subject: [PATCH 52/59] additional size_t replacement --- tmtcservices/CommandingServiceBase.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tmtcservices/CommandingServiceBase.cpp b/tmtcservices/CommandingServiceBase.cpp index 4277de6fb..0c5f5eab1 100644 --- a/tmtcservices/CommandingServiceBase.cpp +++ b/tmtcservices/CommandingServiceBase.cpp @@ -267,7 +267,7 @@ ReturnValue_t CommandingServiceBase::sendTmPacket(uint8_t subservice, object_id_t objectId, const uint8_t *data, size_t dataLen) { uint8_t buffer[sizeof(object_id_t)]; uint8_t* pBuffer = buffer; - uint32_t size = 0; + size_t size = 0; SerializeAdapter::serialize(&objectId, &pBuffer, &size, sizeof(object_id_t), SerializeIF::Endianness::BIG); TmPacketStored tmPacketStored(this->apid, this->service, subservice, From ef2a44c683fe0d236dfa55ad935c09d8f7472f2e Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 13 Jul 2020 22:15:38 +0200 Subject: [PATCH 53/59] added back inttypes.h for cleaner code --- timemanager/CCSDSTime.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/timemanager/CCSDSTime.cpp b/timemanager/CCSDSTime.cpp index 47cab911d..71b2539df 100644 --- a/timemanager/CCSDSTime.cpp +++ b/timemanager/CCSDSTime.cpp @@ -205,9 +205,10 @@ ReturnValue_t CCSDSTime::convertFromASCII(Clock::TimeOfDay_t* to, const uint8_t* uint8_t hour; uint8_t minute; float second; - //try Code A (yyyy-mm-dd) - int count = sscanf((char *) from, "%4hi-%2hhi-%2hiT%2hhi:%2hhi:%fZ", &year, - &month, &day, &hour, &minute, &second); + //try Code A (yyyy-mm-dd) + int count = sscanf((char *) from, "%4" SCNu16 "-%2" SCNu8 "-%2" SCNu16 + "T%2" SCNu8 ":%2" SCNu8 ":%fZ", &year, &month, &day, + &hour, &minute, &second); if (count == 6) { to->year = year; to->month = month; @@ -219,9 +220,9 @@ ReturnValue_t CCSDSTime::convertFromASCII(Clock::TimeOfDay_t* to, const uint8_t* return RETURN_OK; } - //try Code B (yyyy-ddd) - count = sscanf((char *) from, "%4hi-%3hiT%2hhi:%2hhi:%fZ", &year, &day, - &hour, &minute, &second); + //try Code B (yyyy-ddd) + count = sscanf((char *) from, "%4" SCNu16 "-%3" SCNu16 "T%2" SCNu8 + ":%2" SCNu8 ":%fZ", &year, &day, &hour, &minute, &second); if (count == 5) { uint8_t tempDay; ReturnValue_t result = CCSDSTime::convertDaysOfYear(day, year, &month, From be9d0a61f47b7e630f8d3ba94b36dd4ee25d0910 Mon Sep 17 00:00:00 2001 From: Steffen Gaisser Date: Tue, 21 Jul 2020 15:15:53 +0200 Subject: [PATCH 54/59] Fixes #149 --- 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 5c80862c2..8c510eac4 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 39ea25c2d..7a2ce2ee7 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 76481ed12..31bc7c730 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 9e19c228c..dc111459a 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 06e7f286d6592b454823a2027377cfde10be798f Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Sat, 25 Jul 2020 10:55:28 +0200 Subject: [PATCH 55/59] added explicit brackets --- tmtcservices/CommandingServiceBase.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tmtcservices/CommandingServiceBase.cpp b/tmtcservices/CommandingServiceBase.cpp index 0c5f5eab1..331bc7c8a 100644 --- a/tmtcservices/CommandingServiceBase.cpp +++ b/tmtcservices/CommandingServiceBase.cpp @@ -212,8 +212,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; } From 0defc6a7d867ec1d966b2e6078e224c5a94f4686 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 28 Jul 2020 12:36:22 +0200 Subject: [PATCH 56/59] removed folders --- framework.mk | 4 ---- 1 file changed, 4 deletions(-) diff --git a/framework.mk b/framework.mk index c88229814..ee0a7ba4a 100644 --- a/framework.mk +++ b/framework.mk @@ -8,9 +8,6 @@ CXXSRC += $(wildcard $(FRAMEWORK_PATH)/controller/*.cpp) CXXSRC += $(wildcard $(FRAMEWORK_PATH)/coordinates/*.cpp) CXXSRC += $(wildcard $(FRAMEWORK_PATH)/datalinklayer/*.cpp) CXXSRC += $(wildcard $(FRAMEWORK_PATH)/datapool/*.cpp) -CXXSRC += $(wildcard $(FRAMEWORK_PATH)/datapoolglob/*.cpp) -CXXSRC += $(wildcard $(FRAMEWORK_PATH)/datapoollocal/*.cpp) -CXXSRC += $(wildcard $(FRAMEWORK_PATH)/housekeeping/*.cpp) CXXSRC += $(wildcard $(FRAMEWORK_PATH)/devicehandlers/*.cpp) CXXSRC += $(wildcard $(FRAMEWORK_PATH)/events/*.cpp) CXXSRC += $(wildcard $(FRAMEWORK_PATH)/events/eventmatching/*.cpp) @@ -58,4 +55,3 @@ CXXSRC += $(wildcard $(FRAMEWORK_PATH)/tmtcpacket/*.cpp) CXXSRC += $(wildcard $(FRAMEWORK_PATH)/tmtcpacket/packetmatcher/*.cpp) CXXSRC += $(wildcard $(FRAMEWORK_PATH)/tmtcpacket/pus/*.cpp) CXXSRC += $(wildcard $(FRAMEWORK_PATH)/tmtcservices/*.cpp) -CXXSRC += $(wildcard $(FRAMEWORK_PATH)/test/*.cpp) \ No newline at end of file From 036a887ea31124fe1ae0492343c811afceeb7eef Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 28 Jul 2020 13:02:43 +0200 Subject: [PATCH 57/59] bugfix --- serialize/SerialArrayListAdapter.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/serialize/SerialArrayListAdapter.h b/serialize/SerialArrayListAdapter.h index bcd998181..56fb3452d 100644 --- a/serialize/SerialArrayListAdapter.h +++ b/serialize/SerialArrayListAdapter.h @@ -63,6 +63,9 @@ public: count_t tempSize = 0; ReturnValue_t result = SerializeAdapter::deSerialize(&tempSize, buffer, size, streamEndianness); + if(result != HasReturnvaluesIF::RETURN_OK) { + return result; + } if (tempSize > list->maxSize()) { return SerializeIF::TOO_MANY_ELEMENTS; } From 4bffcf17fb2a70373c8d3765796cc8e9f4aac277 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 28 Jul 2020 13:04:58 +0200 Subject: [PATCH 58/59] some formatting stuff --- serialize/SerialArrayListAdapter.h | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/serialize/SerialArrayListAdapter.h b/serialize/SerialArrayListAdapter.h index 56fb3452d..21c6951d9 100644 --- a/serialize/SerialArrayListAdapter.h +++ b/serialize/SerialArrayListAdapter.h @@ -1,18 +1,13 @@ -/** - * @file SerialArrayListAdapter.h - * @brief This file defines the SerialArrayListAdapter class. - * @date 22.07.2014 - * @author baetz - */ -#ifndef SERIALARRAYLISTADAPTER_H_ -#define SERIALARRAYLISTADAPTER_H_ +#ifndef FRAMEWORK_SERIALIZE_SERIALARRAYLISTADAPTER_H_ +#define FRAMEWORK_SERIALIZE_SERIALARRAYLISTADAPTER_H_ #include #include #include /** - * \ingroup serialize + * @ingroup serialize + * @author baetz */ template class SerialArrayListAdapter : public SerializeIF { @@ -25,14 +20,15 @@ public: return serialize(adaptee, buffer, size, maxSize, streamEndianness); } - static ReturnValue_t serialize(const ArrayList* list, uint8_t** buffer, size_t* size, - size_t maxSize, Endianness streamEndianness) { + static ReturnValue_t serialize(const ArrayList* list, + uint8_t** buffer, size_t* size, size_t maxSize, + Endianness streamEndianness) { ReturnValue_t result = SerializeAdapter::serialize(&list->size, buffer, size, maxSize, streamEndianness); count_t i = 0; while ((result == HasReturnvaluesIF::RETURN_OK) && (i < list->size)) { - result = SerializeAdapter::serialize(&list->entries[i], buffer, size, - maxSize, streamEndianness); + result = SerializeAdapter::serialize(&list->entries[i], buffer, + size, maxSize, streamEndianness); ++i; } return result; @@ -58,7 +54,8 @@ public: return deSerialize(adaptee, buffer, size, streamEndianness); } - static ReturnValue_t deSerialize(ArrayList* list, const uint8_t** buffer, size_t* size, + static ReturnValue_t deSerialize(ArrayList* list, + const uint8_t** buffer, size_t* size, Endianness streamEndianness) { count_t tempSize = 0; ReturnValue_t result = SerializeAdapter::deSerialize(&tempSize, @@ -85,4 +82,4 @@ private: -#endif /* SERIALARRAYLISTADAPTER_H_ */ +#endif /* FRAMEWORK_SERIALIZE_SERIALARRAYLISTADAPTER_H_ */ From 652c60c362f2b9b57ef6b54b6d3ca0b3051e6a3b Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 29 Jul 2020 20:02:04 +0200 Subject: [PATCH 59/59] important bugfix I checked all 5 cases for overflows when checking for missed deadlines (there is current time, timeToWake and lastWakeTime, with various combinations of overflows) This should be the correct implementation now --- 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 c062101d9..17804d908 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 44a7b8010..911353753 100644 --- a/osal/FreeRTOS/PeriodicTask.cpp +++ b/osal/FreeRTOS/PeriodicTask.cpp @@ -108,19 +108,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(); } }