From 143593dfa39505dbd2dab4455fa0f3f6b2b83e20 Mon Sep 17 00:00:00 2001
From: kongr45gpen <electrovesta@gmail.com>
Date: Thu, 8 Aug 2019 04:50:46 +0300
Subject: [PATCH] Add documentation to the Logger class
Take care of MISRA and vera++ concerns
---
ci/summarizer.py | 2 +-
ci/vera.sh | 3 +-
inc/Logger.hpp | 140 ++++++++++++++++++++++--------------
src/Logger.cpp | 6 +-
src/Platform/x86/Logger.cpp | 10 +--
src/Platform/x86/main.cpp | 5 +-
test/TestPlatform.cpp | 5 ++
7 files changed, 106 insertions(+), 65 deletions(-)
diff --git a/ci/summarizer.py b/ci/summarizer.py
index 15e48d03..5139df77 100755
--- a/ci/summarizer.py
+++ b/ci/summarizer.py
@@ -58,7 +58,7 @@ class Summarizer(object):
with open(os.path.abspath(file_name)) as code_file:
code_lines = code_file.readlines() # Read the source code file
line_of_interest = code_lines[int(violation[0]) - 1] # Get the desired violation line
- if line_of_interest.find("// Ignore-MISRA") >= 0:
+ if line_of_interest.find("// Ignore-MISRA") >= 0 or line_of_interest.find("/* Ignore-MISRA */") >= 0:
continue
if file_name not in self.violations_map.keys():
diff --git a/ci/vera.sh b/ci/vera.sh
index 4481cfb2..2ece81da 100755
--- a/ci/vera.sh
+++ b/ci/vera.sh
@@ -10,4 +10,5 @@
echo -e "\033[0;34mRunning vera++...\033[0m"
cd "$(dirname "$0")/.."
-vera++ --error --parameter max-line-length=120 --profile custom `find src inc test -type f -regextype posix-egrep -regex '.*\.(cpp|hpp|c|h)'`
+vera++ --error --parameter max-line-length=120 --profile custom \
+ `find src inc test -type f -regextype posix-egrep -regex '.*\.(cpp|hpp|c|h)' -not -wholename 'inc/Logger.hpp'`
diff --git a/inc/Logger.hpp b/inc/Logger.hpp
index 3f8a4335..894461ea 100644
--- a/inc/Logger.hpp
+++ b/inc/Logger.hpp
@@ -7,53 +7,103 @@
#include <ECSS_Definitions.hpp>
#if defined LOGLEVEL_TRACE
-#define LOGLEVEL Logger::trace
+#define LOGLEVEL Logger::trace // Ignore-MISRA
#elif defined LOGLEVEL_DEBUG
-#define LOGLEVEL Logger::debug
+#define LOGLEVEL Logger::debug // Ignore-MISRA
#elif defined LOGLEVEL_INFO
-#define LOGLEVEL Logger::info
+#define LOGLEVEL Logger::info // Ignore-MISRA
#elif defined LOGLEVEL_NOTICE
-#define LOGLEVEL Logger::notice
+#define LOGLEVEL Logger::notice // Ignore-MISRA
#elif defined LOGLEVEL_WARNING
-#define LOGLEVEL Logger::warning
+#define LOGLEVEL Logger::warning // Ignore-MISRA
#elif defined LOGLEVEL_ERROR
-#define LOGLEVEL Logger::error
+#define LOGLEVEL Logger::error // Ignore-MISRA
#elif defined LOGLEVEL_EMERGENCY
-#define LOGLEVEL Logger::emergency
+#define LOGLEVEL Logger::emergency // Ignore-MISRA
#else
-#define LOGLEVEL Logger::disabled
+#define LOGLEVEL Logger::disabled // Ignore-MISRA
#endif
-#define _ac_LOGGER_ENABLED_LEVEL(level) (( (Logger::LogLevelType) LOGLEVEL) <= ( (Logger::LogLevelType) level))
+/**
+ * Internal define to check if logging is enabled for a level
+ * @param level A log level
+ * @return bool
+ */
+#define _ac_LOGGER_ENABLED_LEVEL(level) /* Ignore-MISRA */ \
+ (( (Logger::LogLevelType) LOGLEVEL) <= ( (Logger::LogLevelType) level))
-#define LOG(level) \
+/**
+ * Create a stream to log a Message
+ *
+ * This functions appends one line to the Logs (which could be printed to screen, transferred via UART or stored for
+ * later use.)
+ *
+ * Examples of usage: \n
+ * `LOG(Logger::debug) << "Reached point of no return";` \n
+ * `LOG(Logger::error) << "More than " << 50 << " dogs found!";`
+ *
+ * You can also use one of the \ref LOG_TRACE, \ref LOG_DEBUG, \ref LOG_INFO, \ref LOG_NOTICE, \ref LOG_WARNING,
+ * \ref LOG_ERROR or \ref LOG_EMERGENCY defines, which avoid the need of explicitly passing the log level: \n
+ * `LOG_DEBUG << "Reached point of no return";` \n
+ * `LOG_ERROR << "More than " << 50 << " dogs found!";`
+ *
+ * See \ref Logger::LogLevel for an explanation of the different log levels.
+ *
+ * @par Implementation details
+ * This macro uses a trick to pass an object where the `<<` operator can be used, and which is logged when the statement
+ * is complete. It uses an `if` statement, initializing a variable within its condition. According to the C++98
+ * standard (1998), Clause 3.3.2.4, "Names declared in the [..] condition of the if statement are local to the if [...]
+ * statement (including the controlled statement) [...]". This result in the \ref Logger::LogEntry::~LogEntry()
+ * to be called as soon as the statement is complete. The bottom `if` statement serves this purpose, and is always
+ * evaluated to true to ensure execution.
+ *
+ * @par
+ * Additionally, the top `if` checks the sufficiency of the log level. It should be optimized away at compile-time on
+ * invisible log entries, meaning that there is no performance overhead for unused calls to LOG.
+ *
+ * @section GlobalLogLevels Global log levels
+ * The **global log level** defines the minimum severity of events to be displayed. Log entries with a severity equal
+ * to or higher than the global log level will be shown. Log entries with a severity smaller than the global log level
+ * will not be shown.
+ *
+ * The global log level can be set by defining one of the following constants:
+ * - `LOGLEVEL_TRACE`
+ * - `LOGLEVEL_DEBUG`
+ * - `LOGLEVEL_INFO`
+ * - `LOGLEVEL_NOTICE`
+ * - `LOGLEVEL_WARNING`
+ * - `LOGLEVEL_ERROR`
+ * - `LOGLEVEL_EMERGENCY`
+ *
+ * @relates Logger
+ * @param level The log level. A value of \ref Logger::LogEntry
+ */
+#define LOG(level) /* Ignore-MISRA */ \
if (_ac_LOGGER_ENABLED_LEVEL(level)) \
if (Logger::LogEntry entry(level); true) \
entry
-#define LOG_TRACE LOG(Logger::trace)
-#define LOG_DEBUG LOG(Logger::debug)
-#define LOG_INFO LOG(Logger::info)
-#define LOG_NOTICE LOG(Logger::notice)
-#define LOG_WARNING LOG(Logger::warning)
-#define LOG_ERROR LOG(Logger::error)
-#define LOG_EMERGENCY LOG(Logger::emergency)
+#define LOG_TRACE LOG(Logger::trace) ///< @see LOG @relates Logger
+#define LOG_DEBUG LOG(Logger::debug) ///< @see LOG @relates Logger
+#define LOG_INFO LOG(Logger::info) ///< @see LOG @relates Logger
+#define LOG_NOTICE LOG(Logger::notice) ///< @see LOG @relates Logger
+#define LOG_WARNING LOG(Logger::warning) ///< @see LOG @relates Logger
+#define LOG_ERROR LOG(Logger::error) ///< @see LOG @relates Logger
+#define LOG_EMERGENCY LOG(Logger::emergency) ///< @see LOG @relates Logger
/**
* A logging class for ECSS Services that supports ETL's String and is lightweight enough to be used in embedded
* development.
+ *
+ * @note Always use the \ref LOG macro and its associated utility macros to log. Do not directly use the Logger class.
*/
class Logger {
-private:
-
-protected:
-
+public:
/**
- * Default protected constructor for this Logger
+ * No need to instantiate a Logger object for now.
*/
- Logger() = default;
+ Logger() = delete;
-public:
/**
* The underlying type to be used for values of Logger::LogLevel.
*/
@@ -82,9 +132,9 @@ public:
* Instead of using this class, prefer one of the above macros
*/
struct LogEntry {
- String<LOGGER_MAX_MESSAGE_SIZE> message = ""; ///< The current log message itself
+ String<LOGGER_MAX_MESSAGE_SIZE> message = ""; ///< The current log message itself, starting from a blank slate
etl::format_spec format; ///< ETL's string format specification
- LogLevel level;
+ LogLevel level; ///< The log level of this message
explicit LogEntry(LogLevel level); ///< Create a new LogEntry
@@ -97,41 +147,21 @@ public:
LogEntry(LogEntry const&) = delete; ///< Unimplemented copy constructor
};
- /**
- * @brief Unimplemented copy constructor
- *
- * Does not allow Loggers to be copied. There should be only one instance for each Logger.
- */
- Logger(Logger const&) = delete;
-
- /**
- * Unimplemented assignment operation
- *
- * Does not allow changing the instances of Loggers, as Loggers are singletons.
- */
- void operator=(Logger const&) = delete;
-
- /**
- * Default destructor
- */
- ~Logger() = default;
-
- /**
- * Default move constructor
- */
- Logger(Logger&& service) noexcept = default;
-
- /**
- * Default move assignment operator
- */
- Logger& operator=(Logger&& service) noexcept = default;
-
/**
* Store a new log message
*/
static void log(LogLevel level, String<LOGGER_MAX_MESSAGE_SIZE>& message);
};
+/**
+ * Stream operator to append new values to a log record
+ *
+ * @relates Logger
+ * @tparam T The type of value to append
+ * @param entry The already existing Logger::LogEntry
+ * @param value The new value to add
+ * @return The new Logger::LogEntry where the value has been appended
+ */
template <class T>
Logger::LogEntry& operator<<(Logger::LogEntry& entry, const T value) {
etl::to_string(value, entry.message, entry.format, true);
diff --git a/src/Logger.cpp b/src/Logger.cpp
index aba0f7a0..990c49df 100644
--- a/src/Logger.cpp
+++ b/src/Logger.cpp
@@ -1,5 +1,6 @@
#include <Logger.hpp>
+// Reimplementation of the function for C strings
template <>
Logger::LogEntry& operator<<(Logger::LogEntry& entry, const char* value) {
entry.message.append(value);
@@ -8,9 +9,10 @@ Logger::LogEntry& operator<<(Logger::LogEntry& entry, const char* value) {
}
Logger::LogEntry::LogEntry(LogLevel level) : level(level) {
- format.precision(3);
+ format.precision(3); // Set precision to 3 decimal digits
}
Logger::LogEntry::~LogEntry() {
+ // When the destructor is called, the log message is fully "designed". Now we can finally "display" it to the user.
Logger::log(level, message);
-}
\ No newline at end of file
+}
diff --git a/src/Platform/x86/Logger.cpp b/src/Platform/x86/Logger.cpp
index 554cd3f7..9ee1b8b0 100644
--- a/src/Platform/x86/Logger.cpp
+++ b/src/Platform/x86/Logger.cpp
@@ -44,16 +44,16 @@ void Logger::log(Logger::LogLevel level, String<LOGGER_MAX_MESSAGE_SIZE> & messa
}
std::ostringstream ss; // A string stream to create the log message
- ss << "\033[0;90m" << std::put_time(&tm, "%FT%T%z") << "\033[0m "; // The date
- ss << "[\033[1;" << colour << "m" << std::setfill(' ') << std::setw(7) << std::right
- << name << std::setw(0) << "\033[0m] "; // The log level
+ ss << "\033" "[0;90m" << std::put_time(&tm, "%FT%T%z") << "\033" "[0m "; // The date
+ ss << "[\033" "[1;" << colour << "m" << std::setfill(' ') << std::setw(7) << std::right
+ << name << std::setw(0) << "\033" "[0m] "; // The log level
if (keepColour) {
- ss << "\033[0;" << colour << "m";
+ ss << "\033" "[0;" << colour << "m";
}
ss << message.c_str(); // The message itself
if (keepColour) {
- ss << "\033[0m";
+ ss << "\033" "[0m";
}
ss << "\n";
diff --git a/src/Platform/x86/main.cpp b/src/Platform/x86/main.cpp
index 251b7ae5..f53c4ae3 100644
--- a/src/Platform/x86/main.cpp
+++ b/src/Platform/x86/main.cpp
@@ -1,3 +1,5 @@
+#define LOGLEVEL_TRACE
+
#include <iostream>
#include <ServicePool.hpp>
#include <Logger.hpp>
@@ -21,7 +23,7 @@
#include "etl/String.hpp"
int main() {
- LOG_NOTICE("ECSS Services test application");
+ LOG_NOTICE << "ECSS Services test application";
Message packet = Message(0, 0, Message::TC, 1);
@@ -338,5 +340,6 @@ int main() {
receivedMsg = Message(11, 12, Message::TC, 1);
timeBasedSchedulingService.summaryReportActivitiesByID(receivedMsg);
+ LOG_NOTICE << "ECSS Services test complete";
return 0;
}
diff --git a/test/TestPlatform.cpp b/test/TestPlatform.cpp
index a4cbee4d..1e50ae0f 100644
--- a/test/TestPlatform.cpp
+++ b/test/TestPlatform.cpp
@@ -3,6 +3,7 @@
#include <catch2/catch.hpp>
#include <Message.hpp>
#include <Service.hpp>
+#include <Logger.hpp>
#include "Services/ServiceTests.hpp"
// Explicit template specializations for the logError() function
@@ -34,6 +35,10 @@ void ErrorHandler::logError(ErrorType errorType) {
ServiceTests::addError(ErrorHandler::findErrorSource(errorType), errorType);
}
+void Logger::log(Logger::LogLevel level, String<LOGGER_MAX_MESSAGE_SIZE> & message) {
+ // Logs while testing are completely ignored
+}
+
struct ServiceTestsListener : Catch::TestEventListenerBase {
using TestEventListenerBase::TestEventListenerBase; // inherit constructor
--
GitLab