diff --git a/.gitignore b/.gitignore
index d30fcd4b531e90eeb63a8246bfd7d4261ac3f697..051810d3f334d430727a431c6daba6ae1a37e196 100644
--- a/.gitignore
+++ b/.gitignore
@@ -3,6 +3,13 @@ build
cmake-build-debug
docs
+# Dump and continuous integration files
+*.dump
+__pycache__
+/ci/cppcheckdata.py
+/ci/misra.py
+/ci/report.msr
+
# Prerequisites
*.d
@@ -65,3 +72,6 @@ docs
.idea/**/uiDesigner.xml
.idea/**/dbnavigator.xml
.idea/**/markdown-*
+
+# IDEs
+.vscode
\ No newline at end of file
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index e1d4353524db193088abb502e90d4887ecffd6c1..91b67e1aa1e2dfe5497b050a23f6e0c4db384ade 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -38,11 +38,22 @@ tests:
cppcheck:
stage: build
before_script:
- - apt-get update -qq && apt-get install -y -qq cppcheck
+ - echo deb http://deb.debian.org/debian testing main > /etc/apt/sources.list
+ - apt-get update -qq && apt-get -t testing install -y cppcheck
- cppcheck --version
script:
- ci/cppcheck.sh
+cppcheck-misra:
+ stage: build
+ before_script:
+ # install cppcheck from the testing repos in order to get the latest version
+ - echo deb http://deb.debian.org/debian testing main > /etc/apt/sources.list
+ - apt-get update -qq && apt-get -t testing install -y cppcheck && apt-get -t testing install -y python3
+ - cppcheck --version
+ script:
+ - ci/cppcheck-misra.sh
+
vera:
stage: build
before_script:
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 37b151c03e16be403d2d367a27ca5daffbd2b092..d9b528b386c42c8e790916188b53db229c5e270b 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -11,6 +11,7 @@ add_custom_target(check
COMMAND ./cppcheck.sh
COMMAND ./vera.sh
COMMAND ./clang-tidy.sh
+ COMMAND ./cppcheck-misra.sh
WORKING_DIRECTORY "${PROJECT_SOURCE_DIR}/ci")
# Specify the .cpp files common across all targets
diff --git a/ci/cppcheck-misra.sh b/ci/cppcheck-misra.sh
new file mode 100755
index 0000000000000000000000000000000000000000..1389882e300451d43b623d1bc6fbad31392dadd7
--- /dev/null
+++ b/ci/cppcheck-misra.sh
@@ -0,0 +1,38 @@
+#!/usr/bin/env bash
+
+#
+# Static code analysis for MISRA C-2012 compliance using cppcheck
+#
+# Usage:
+# $ ci/cppcheck-misra.sh
+#
+
+# make sure we are in the correct directory, regardless of where the script was called from
+cd "$(dirname "$0")/.."
+
+echo -e "\u001b[34;1mGetting prerequisites...\u001b[0m"
+# grab the MISRA addon and the cppcheck addon interface from github
+curl https://raw.githubusercontent.com/danmar/cppcheck/f4b5b156d720c712f6ce99f6e01d8c1b3f800d52/addons/misra.py > ci/misra.py
+curl https://raw.githubusercontent.com/danmar/cppcheck/f4b5b156d720c712f6ce99f6e01d8c1b3f800d52/addons/cppcheckdata.py > ci/cppcheckdata.py
+
+# clean up old files
+echo -e "\u001b[34;1mRemoving old files...\u001b[0m"
+echo > ci/report.msr # clear the report file
+find inc/ src/ -type f -name "*.dump" | xargs rm
+
+# generate dump files (XML representations of AST etc.) for all headers, source files etc.
+echo -e "\u001b[34;1mGenerating dump files...\u001b[0m"
+find inc/ src/ -type f \( -iname "*.cpp" -or -iname "*.hpp" \) | xargs cppcheck --dump
+
+# run the MISRA checks against the dumps and send the results to a file
+echo -e "\u001b[34;1mRunning MISRA C(2012) rule compliance tests...\u001b[0m"
+find inc/ src/ -type f -name "*.dump" | xargs python3 ci/misra.py >> ci/report.msr 2>&1
+
+# pre-process the generated report to remove all useless strings
+echo -e "\u001b[34;1mPre-processing report...\u001b[0m"
+sed -i -r 's/(.*Script.*)|(.*Checking.*)|(.*MISRA.*)//gm; /(^$)/d; s/(\s\(.*\)\s)//gm; s/(\]|\[)//gm; s/(misra-c2012-)//gm' ci/report.msr
+
+# run the summarizer for a nice, clean summary of errors
+echo -e "\u001b[34;1mSummarizing results...\u001b[0m"
+python3 ci/summarizer.py --report ci/report.msr --suppress 3.1 5.1 5.2 5.3 12.3 14.4 15.5 16.3 18.8
+
diff --git a/ci/cppcheck.sh b/ci/cppcheck.sh
index d6bcded38887bd666573d5e012d057287e91b32e..527e10d7670bdb8b14326b2616fe5eb15253d6c8 100755
--- a/ci/cppcheck.sh
+++ b/ci/cppcheck.sh
@@ -1,13 +1,15 @@
#!/usr/bin/env bash
#
-# Static code analysis using cppchecl
+# Static code analysis using cppcheck
#
# Usage:
# $ ci/cppcheck.sh
#
-echo -e "\033[0;34mRunning cppcheck...\033[0m"
+echo -e "\u001b[34;1mStarting cppcheck...\u001b[0m"
+
+echo -e "\u001b[34;1mRunning cppcheck with default checklist...\u001b[0m"
cd "$(dirname "$0")/.."
cppcheck --enable=all --inline-suppr --suppress=unusedFunction --suppress=missingIncludeSystem \
diff --git a/ci/summarizer.py b/ci/summarizer.py
new file mode 100755
index 0000000000000000000000000000000000000000..15e48d03c61bffd4caeb8c43804393abbf00016e
--- /dev/null
+++ b/ci/summarizer.py
@@ -0,0 +1,131 @@
+#!/bin/env python3
+
+import os
+from argparse import ArgumentParser
+
+"""
+Naive parser and pretty printer for the MISRA reports by cppcheck.
+
+Usage:
+python3 summarizer.py --report <report-file-generated-from-cppcheck-misra.sh>
+--suppress <list-of-rule-numbers-to-be-suppressed> (as they are given on the
+MISRA handbook)
+
+e.g.: python3 summarizer.py --report ci/report.msr --suppress 5.2 10.4 15.5
+
+You can also mark lines you wish to be skipped by adding
+"// Ignore-MISRA" at a suitable position on the line you want to ignore
+from assessing (NOTE: the double slashes are part of the string!)
+
+(Credits to @dimst23 for the feature!)
+
+TODO: Keep track of line numbers and filenames of suppressed lines.
+"""
+
+
+class Summarizer(object):
+ def __init__(self, report_name, suppression_list):
+ with open(report_name, 'r') as f:
+ self.file_lines = f.readlines() # read the report file
+ f.close()
+
+ self.red = "\033[91m" # terminal colors
+ self.yellow = "\033[93m"
+ self.green = "\033[92m"
+ self.bold = "\033[1m"
+ self.end = "\033[0m"
+
+ self.violations_map = {} # dictionary containing filenames, rule violations and line where the violation
+ # occurred
+ self.suppression_list = suppression_list # list of rule numbers to be suppressed
+
+ def analyze(self):
+ """
+ A really dumb parser for the pre-processed report generated by cppcheck
+ """
+
+ lines_seen = set() # contains the unique lines from the file
+
+ for line in self.file_lines: # remove duplicate lines
+ if line not in lines_seen:
+ lines_seen.add(line)
+
+ line_contents = line.split(':')
+ file_name = line_contents[0] # first part is the filename (index 0)
+ violation = (line_contents[1], line_contents[2].strip(
+ '\n')) # index 1 is the line number, index 2 is the number of violated rule (both are strings)
+
+ 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:
+ continue
+
+ if file_name not in self.violations_map.keys():
+ self.violations_map[
+ file_name] = list() # create a new list for the new filename and append the tuple w/ line &
+ # rule no.
+ self.violations_map[file_name].append(violation)
+ else:
+ self.violations_map[file_name].append(violation) # do not create a key if it already exists
+
+ for e in self.suppression_list:
+ for file_name in self.violations_map.keys():
+ self.violations_map[file_name] = [x for x in self.violations_map[file_name] if x[1] != str(e)]
+ # replace the list of infractions with a new one, containing everything except the suppressed rules
+
+ self.violations_map = {k: v for (k, v) in self.violations_map.items() if len(v) != 0}
+ # "delete" all keys whose lists are empty
+
+
+ def pretty_print_violations(self):
+ """
+ Just a pretty printing function, no fancy logic here.
+ """
+ print(self.bold + self.red + "=========================================================\n" + self.end)
+ print(self.bold + self.red + " Static analysis results: Infraction summary \n" + self.end)
+ for file_name in self.violations_map:
+ print("")
+ for violation in sorted(self.violations_map[file_name], key=lambda x: int(x[0])):
+
+ name_string = f"{self.bold}{self.red}File {self.yellow}{file_name}{self.red}"
+ rule_violated_string = f"violates rule {self.yellow}#{violation[1]}{self.red} " \
+ f"of the MISRA C 2012 standard"
+ line_number_string = f"at line {self.yellow}{violation[0]}{self.end}"
+
+ print(f"{name_string.ljust(75)} {rule_violated_string} {line_number_string}")
+
+ print("")
+ print("")
+ print(self.bold + self.red + "=================================================" + self.end)
+
+ def suppression_info(self):
+ """
+ Pretty-prints the suppressed rule numbers.
+ """
+ if (len(self.suppression_list) != 0):
+ print("\n")
+ print(self.bold + self.yellow + "WARNING: Suppressed infractions of rules: ", end="")
+ print(f", ".join(self.suppression_list), end=".")
+ print("")
+ print("")
+ else:
+ print(self.bold + self.green + "All available rules enforced - no suppressions")
+
+
+if __name__ == "__main__":
+ cli = ArgumentParser()
+ cli.add_argument("--report", nargs=1, default="./report.msr")
+ cli.add_argument("--suppress", nargs="*", type=str, default="")
+
+ args = cli.parse_args()
+ s = Summarizer(str(args.report[0]), args.suppress)
+ s.analyze()
+ s.suppression_info()
+
+ if len(s.violations_map) != 0:
+ s.pretty_print_violations()
+ exit(127)
+ elif len(s.violations_map) == 0:
+ print(s.bold + s.green + "Static analysis for MISRA compliance complete. No infractions found." + s.end)
+ exit(0)
diff --git a/inc/ECSS_Definitions.hpp b/inc/ECSS_Definitions.hpp
index 047db0f70cda0a66d4069ffb7aacd3c780e45baf..e712e5327035f247581e3804413a338b449f815d 100644
--- a/inc/ECSS_Definitions.hpp
+++ b/inc/ECSS_Definitions.hpp
@@ -1,11 +1,11 @@
#ifndef ECSS_SERVICES_ECSS_DEFINITIONS_H
#define ECSS_SERVICES_ECSS_DEFINITIONS_H
-#define ECSS_MAX_MESSAGE_SIZE 1024
+#define ECSS_MAX_MESSAGE_SIZE 1024u
-#define ECSS_MAX_STRING_SIZE 256
+#define ECSS_MAX_STRING_SIZE 256u
-#define ECSS_MAX_FIXED_OCTET_STRING_SIZE 256 // For the ST13 large packet transfer service
+#define ECSS_MAX_FIXED_OCTET_STRING_SIZE 256u // For the ST13 large packet transfer service
// 7.4.1
#define CCSDS_PACKET_VERSION 0
diff --git a/inc/ErrorHandler.hpp b/inc/ErrorHandler.hpp
index 4f50837588ce77417bc2818c2093b675a9bfd267..74ba544e365fa1b38067915a5dd54171b48f3d03 100644
--- a/inc/ErrorHandler.hpp
+++ b/inc/ErrorHandler.hpp
@@ -2,12 +2,11 @@
#define PROJECT_ERRORHANDLER_HPP
#include <type_traits>
+#include <stdint.h> // for the uint_8t stepID
// Forward declaration of the class, since its header file depends on the ErrorHandler
class Message;
-#include <stdint.h> // for the uint_8t stepID
-
/**
* A class that handles unexpected software errors, including internal errors or errors due to
* invalid & incorrect input data.
@@ -236,22 +235,25 @@ public:
template<typename ErrorType>
inline static ErrorSource findErrorSource(ErrorType error) {
// Static type checking
+ ErrorSource source = Internal;
+
if (std::is_same<ErrorType, AcceptanceErrorType>()) {
- return Acceptance;
+ source = Acceptance;
}
if (std::is_same<ErrorType, ExecutionStartErrorType>()) {
- return ExecutionStart;
+ source = ExecutionStart;
}
if (std::is_same<ErrorType, ExecutionProgressErrorType>()) {
- return ExecutionProgress;
+ source = ExecutionProgress;
}
if (std::is_same<ErrorType, ExecutionCompletionErrorType>()) {
- return ExecutionCompletion;
+ source = ExecutionCompletion;
}
if (std::is_same<ErrorType, RoutingErrorType>()) {
- return Routing;
+ source = Routing;
}
- return Internal;
+
+ return source;
}
};
diff --git a/inc/Helpers/TimeHelper.hpp b/inc/Helpers/TimeHelper.hpp
index b9039cf346c410ae15563810f67e39685903716b..d145f43c89209022321fffa9e9b62e10bfa53c75 100644
--- a/inc/Helpers/TimeHelper.hpp
+++ b/inc/Helpers/TimeHelper.hpp
@@ -5,9 +5,9 @@
#include <Message.hpp>
#include "TimeAndDate.hpp"
-#define SECONDS_PER_MINUTE 60
-#define SECONDS_PER_HOUR 3600
-#define SECONDS_PER_DAY 86400
+#define SECONDS_PER_MINUTE 60u
+#define SECONDS_PER_HOUR 3600u
+#define SECONDS_PER_DAY 86400u
/**
* This class formats the spacecraft time and cooperates closely with the ST[09] time management.
diff --git a/inc/Message.hpp b/inc/Message.hpp
index ac1509f9ea39dc2c8d330e0ebd4bcc34978c382a..1bbfcd48bb3b8495de8b3712ae2bb360099a2ae2 100644
--- a/inc/Message.hpp
+++ b/inc/Message.hpp
@@ -1,10 +1,6 @@
#ifndef ECSS_SERVICES_PACKET_H
#define ECSS_SERVICES_PACKET_H
-
-// Forward declaration of the Message class, needed for the ErrorHandler
-class Message;
-
#include "ECSS_Definitions.hpp"
#include <cstdint>
#include <etl/String.hpp>
@@ -496,13 +492,15 @@ public:
*/
bool assertType(Message::PacketType expectedPacketType, uint8_t expectedServiceType,
uint8_t expectedMessageType) {
- if (packetType != expectedPacketType || serviceType != expectedServiceType ||
- messageType != expectedMessageType) {
+ bool status = true;
+
+ if ((packetType != expectedPacketType) || (serviceType != expectedServiceType) ||
+ (messageType != expectedMessageType)) {
ErrorHandler::reportInternalError(ErrorHandler::OtherMessageType);
- return false;
+ status = false;
}
- return true;
+ return status;
}
/**
diff --git a/inc/Services/TimeBasedSchedulingService.hpp b/inc/Services/TimeBasedSchedulingService.hpp
index 76343b5d76fa5aac07e9a59f29899ec00769e4bd..6f6ab971a1d32cfdfe194968e2c3b4a3b2a28308 100644
--- a/inc/Services/TimeBasedSchedulingService.hpp
+++ b/inc/Services/TimeBasedSchedulingService.hpp
@@ -109,7 +109,10 @@ private:
inline void sortActivitiesReleaseTime(etl::list<ScheduledActivity,
ECSS_MAX_NUMBER_OF_TIME_SCHED_ACTIVITIES> &schedActivities) {
schedActivities.sort([](ScheduledActivity const &leftSide, ScheduledActivity const
- &rightSide) { return leftSide.requestReleaseTime < rightSide.requestReleaseTime; });
+ &rightSide) {
+ // cppcheck-suppress
+ return leftSide.requestReleaseTime < rightSide.requestReleaseTime;
+ });
}
/**
diff --git a/lib/Catch2 b/lib/Catch2
index 62460fafe6b54c3173bc5cbc46d05a5f071017ff..d63307279412de3870cf97cc6802bae8ab36089e 160000
--- a/lib/Catch2
+++ b/lib/Catch2
@@ -1 +1 @@
-Subproject commit 62460fafe6b54c3173bc5cbc46d05a5f071017ff
+Subproject commit d63307279412de3870cf97cc6802bae8ab36089e
diff --git a/src/Helpers/CRCHelper.cpp b/src/Helpers/CRCHelper.cpp
index 8361455071673204dcaa88ab4c3f136615127fdf..63bcde7d90e908dbbc46c874239251522e4579c2 100644
--- a/src/Helpers/CRCHelper.cpp
+++ b/src/Helpers/CRCHelper.cpp
@@ -15,7 +15,7 @@ uint16_t CRCHelper::calculateCRC(const uint8_t* message, uint32_t length) {
for (int j = 0; j < 8; j++) {
// if the MSB is set, the bitwise AND gives 1
- if ((shiftReg & 0x8000u) != 0) {
+ if ((shiftReg & 0x8000u) != 0u) {
// toss out of the register the MSB and divide (XOR) its content with the generator
shiftReg = ((shiftReg << 1u) ^ polynomial);
}
diff --git a/src/Helpers/TimeAndDate.cpp b/src/Helpers/TimeAndDate.cpp
index 38280ecb745925f3ce989ad3547b9379bb888f67..5fd9f0faa347aa54597837a36c92bb086d136374 100644
--- a/src/Helpers/TimeAndDate.cpp
+++ b/src/Helpers/TimeAndDate.cpp
@@ -15,11 +15,11 @@ TimeAndDate::TimeAndDate(uint16_t year, uint8_t month, uint8_t day, uint8_t hour
uint8_t second) {
// check if the parameters make sense
ASSERT_INTERNAL(2019 <= year, ErrorHandler::InternalErrorType::InvalidDate);
- ASSERT_INTERNAL(1 <= month && month <= 12, ErrorHandler::InternalErrorType::InvalidDate);
- ASSERT_INTERNAL(1 <= day && day <= 31, ErrorHandler::InternalErrorType::InvalidDate);
- ASSERT_INTERNAL(0 <= hour && hour <= 24, ErrorHandler::InternalErrorType::InvalidDate);
- ASSERT_INTERNAL(0 <= minute && minute <= 60, ErrorHandler::InternalErrorType::InvalidDate);
- ASSERT_INTERNAL(0 <= second && second <= 60, ErrorHandler::InternalErrorType::InvalidDate);
+ ASSERT_INTERNAL((1 <= month) && (month <= 12), ErrorHandler::InternalErrorType::InvalidDate);
+ ASSERT_INTERNAL((1 <= day) && (day <= 31), ErrorHandler::InternalErrorType::InvalidDate);
+ ASSERT_INTERNAL(hour <= 24, ErrorHandler::InternalErrorType::InvalidDate);
+ ASSERT_INTERNAL(minute <= 60, ErrorHandler::InternalErrorType::InvalidDate);
+ ASSERT_INTERNAL(second <= 60, ErrorHandler::InternalErrorType::InvalidDate);
this->year = year;
this->month = month;
@@ -163,9 +163,9 @@ bool TimeAndDate::operator==(const TimeAndDate &Date) {
bool TimeAndDate::operator<=(const TimeAndDate &Date) {
- return (*this < Date || *this == Date);
+ return ((*this < Date) || (*this == Date));
}
bool TimeAndDate::operator>=(const TimeAndDate &Date) {
- return (*this > Date || *this == Date);
+ return ((*this > Date) || (*this == Date));
}
diff --git a/src/Helpers/TimeHelper.cpp b/src/Helpers/TimeHelper.cpp
index 4b6885dc9b842ade6bb33fd0c5a09270b4df11e4..84beb4ac6fb2280f9e5489852112c65f5616ba03 100644
--- a/src/Helpers/TimeHelper.cpp
+++ b/src/Helpers/TimeHelper.cpp
@@ -1,10 +1,10 @@
#include "Helpers/TimeHelper.hpp"
bool TimeHelper::IsLeapYear(uint16_t year) {
- if (year % 4 != 0) {
+ if ((year % 4) != 0) {
return false;
}
- if (year % 100 != 0) {
+ if ((year % 100) != 0) {
return true;
}
return (year % 400) == 0;
@@ -14,15 +14,15 @@ uint32_t TimeHelper::utcToSeconds(TimeAndDate &TimeInfo) {
// the date, that \p TimeInfo represents, should be greater than or equal to 1/1/2019 and the
// date should be valid according to Gregorian calendar
ASSERT_INTERNAL(TimeInfo.year >= 2019, ErrorHandler::InternalErrorType::InvalidDate);
- ASSERT_INTERNAL(1 <= TimeInfo.month && TimeInfo.month <= 12,
+ ASSERT_INTERNAL((1 <= TimeInfo.month) && (TimeInfo.month <= 12),
ErrorHandler::InternalErrorType::InvalidDate);
- ASSERT_INTERNAL(1 <= TimeInfo.day && TimeInfo.day <= 31,
+ ASSERT_INTERNAL((1 <= TimeInfo.day) && (TimeInfo.day <= 31),
ErrorHandler::InternalErrorType::InvalidDate);
- ASSERT_INTERNAL(0 <= TimeInfo.hour && TimeInfo.hour <= 24,
+ ASSERT_INTERNAL(TimeInfo.hour <= 24,
ErrorHandler::InternalErrorType::InvalidDate);
- ASSERT_INTERNAL(0 <= TimeInfo.minute && TimeInfo.minute <= 60,
+ ASSERT_INTERNAL(TimeInfo.minute <= 60,
ErrorHandler::InternalErrorType::InvalidDate);
- ASSERT_INTERNAL(0 <= TimeInfo.second && TimeInfo.second <= 60,
+ ASSERT_INTERNAL(TimeInfo.second <= 60,
ErrorHandler::InternalErrorType::InvalidDate);
uint32_t secs = 1546300800; // elapsed seconds from Unix epoch until 1/1/2019 00:00:00 (UTC)
@@ -30,8 +30,8 @@ uint32_t TimeHelper::utcToSeconds(TimeAndDate &TimeInfo) {
secs += (IsLeapYear(y) ? 366 : 365) * SECONDS_PER_DAY;
}
for (uint16_t m = 1; m < TimeInfo.month; ++m) {
- secs += DaysOfMonth[m - 1] * SECONDS_PER_DAY;
- if (m == 2 && IsLeapYear(TimeInfo.year)) {
+ secs += DaysOfMonth[m - 1u] * SECONDS_PER_DAY;
+ if ((m == 2u) && IsLeapYear(TimeInfo.year)) {
secs += SECONDS_PER_DAY;
}
}
@@ -67,7 +67,7 @@ struct TimeAndDate TimeHelper::secondsToUTC(uint32_t seconds) {
TimeInfo.month++;
seconds -= (DaysOfMonth[i] * SECONDS_PER_DAY);
i++;
- if (i == 1 && IsLeapYear(TimeInfo.year)) {
+ if ((i == 1u) && IsLeapYear(TimeInfo.year)) {
if (seconds <= (28 * SECONDS_PER_DAY)) {
break;
}
@@ -116,20 +116,20 @@ uint64_t TimeHelper::generateCDStimeFormat(TimeAndDate &TimeInfo) {
*/
auto msOfDay = static_cast<uint32_t >((seconds % SECONDS_PER_DAY) * 1000);
- uint64_t timeFormat = (static_cast<uint64_t>(elapsedDays) << 32 | msOfDay);
+ uint64_t timeFormat = (static_cast<uint64_t>(elapsedDays) << 32) | msOfDay;
return timeFormat;
}
TimeAndDate TimeHelper::parseCDStimeFormat(const uint8_t *data) {
- uint16_t elapsedDays = (static_cast<uint16_t >(data[0])) << 8 | static_cast<uint16_t >
- (data[1]);
- uint32_t msOfDay = (static_cast<uint32_t >(data[2])) << 24 |
- (static_cast<uint32_t >(data[3])) << 16 |
- (static_cast<uint32_t >(data[4])) << 8 |
- static_cast<uint32_t >(data[5]);
-
- uint32_t seconds = elapsedDays * SECONDS_PER_DAY + msOfDay / 1000;
+ uint16_t elapsedDays = ((static_cast<uint16_t >(data[0])) << 8) | (static_cast<uint16_t >
+ (data[1]));
+ uint32_t msOfDay = ((static_cast<uint32_t >(data[2])) << 24) |
+ ((static_cast<uint32_t >(data[3]))) << 16 |
+ ((static_cast<uint32_t >(data[4]))) << 8 |
+ (static_cast<uint32_t >(data[5]));
+
+ uint32_t seconds = (elapsedDays * SECONDS_PER_DAY) + (msOfDay / 1000u);
return secondsToUTC(seconds);
}
diff --git a/src/Message.cpp b/src/Message.cpp
index 0127ecf679d0bdf93600413653eea9878f20690e..dca275934730d3869af180f7c8b5337b2973fb13 100644
--- a/src/Message.cpp
+++ b/src/Message.cpp
@@ -15,7 +15,7 @@ void Message::appendBits(uint8_t numBits, uint16_t data) {
while (numBits > 0) { // For every sequence of 8 bits...
ASSERT_INTERNAL(dataSize < ECSS_MAX_MESSAGE_SIZE, ErrorHandler::MessageTooLarge);
- if (currentBit + numBits >= 8) {
+ if ((currentBit + numBits) >= 8) {
// Will have to shift the bits and insert the next ones later
auto bitsToAddNow = static_cast<uint8_t>(8 - currentBit);
@@ -54,7 +54,7 @@ void Message::appendByte(uint8_t value) {
}
void Message::appendHalfword(uint16_t value) {
- ASSERT_INTERNAL(dataSize + 2 <= ECSS_MAX_MESSAGE_SIZE, ErrorHandler::MessageTooLarge);
+ ASSERT_INTERNAL((dataSize + 2) <= ECSS_MAX_MESSAGE_SIZE, ErrorHandler::MessageTooLarge);
ASSERT_INTERNAL(currentBit == 0, ErrorHandler::ByteBetweenBits);
data[dataSize] = static_cast<uint8_t>((value >> 8) & 0xFF);
@@ -64,7 +64,7 @@ void Message::appendHalfword(uint16_t value) {
}
void Message::appendWord(uint32_t value) {
- ASSERT_INTERNAL(dataSize + 4 <= ECSS_MAX_MESSAGE_SIZE, ErrorHandler::MessageTooLarge);
+ ASSERT_INTERNAL((dataSize + 4) <= ECSS_MAX_MESSAGE_SIZE, ErrorHandler::MessageTooLarge);
ASSERT_INTERNAL(currentBit == 0, ErrorHandler::ByteBetweenBits);
data[dataSize] = static_cast<uint8_t>((value >> 24) & 0xFF);
@@ -83,10 +83,11 @@ uint16_t Message::readBits(uint8_t numBits) {
while (numBits > 0) {
ASSERT_REQUEST(readPosition < ECSS_MAX_MESSAGE_SIZE, ErrorHandler::MessageTooShort);
- if (currentBit + numBits >= 8) {
- auto bitsToAddNow = static_cast<uint8_t>(8 - currentBit);
+ if ((currentBit + numBits) >= 8) {
+ uint8_t bitsToAddNow = static_cast<uint8_t>(8 - currentBit);
- auto maskedData = static_cast<uint8_t>(data[readPosition] & ((1 << bitsToAddNow) - 1));
+ uint8_t mask = ((1u << bitsToAddNow) - 1u);
+ uint8_t maskedData = data[readPosition] & mask;
value |= maskedData << (numBits - bitsToAddNow);
numBits -= bitsToAddNow;
@@ -112,7 +113,7 @@ uint8_t Message::readByte() {
}
uint16_t Message::readHalfword() {
- ASSERT_REQUEST(readPosition + 2 <= ECSS_MAX_MESSAGE_SIZE, ErrorHandler::MessageTooShort);
+ ASSERT_REQUEST((readPosition + 2) <= ECSS_MAX_MESSAGE_SIZE, ErrorHandler::MessageTooShort);
uint16_t value = (data[readPosition] << 8) | data[readPosition + 1];
readPosition += 2;
@@ -121,7 +122,7 @@ uint16_t Message::readHalfword() {
}
uint32_t Message::readWord() {
- ASSERT_REQUEST(readPosition + 4 <= ECSS_MAX_MESSAGE_SIZE, ErrorHandler::MessageTooShort);
+ ASSERT_REQUEST((readPosition + 4) <= ECSS_MAX_MESSAGE_SIZE, ErrorHandler::MessageTooShort);
uint32_t value = (data[readPosition] << 24) | (data[readPosition + 1] << 16) |
(data[readPosition + 2] << 8) | data[readPosition + 3];
@@ -131,7 +132,7 @@ uint32_t Message::readWord() {
}
void Message::readString(char *string, uint8_t size) {
- ASSERT_REQUEST(readPosition + size <= ECSS_MAX_MESSAGE_SIZE, ErrorHandler::MessageTooShort);
+ ASSERT_REQUEST((readPosition + size) <= ECSS_MAX_MESSAGE_SIZE, ErrorHandler::MessageTooShort);
ASSERT_REQUEST(size < ECSS_MAX_STRING_SIZE, ErrorHandler::StringTooShort);
memcpy(string, data + readPosition, size);
@@ -141,7 +142,7 @@ void Message::readString(char *string, uint8_t size) {
}
void Message::readString(uint8_t *string, uint16_t size) {
- ASSERT_REQUEST(readPosition + size <= ECSS_MAX_MESSAGE_SIZE, ErrorHandler::MessageTooShort);
+ ASSERT_REQUEST((readPosition + size) <= ECSS_MAX_MESSAGE_SIZE, ErrorHandler::MessageTooShort);
ASSERT_REQUEST(size < ECSS_MAX_STRING_SIZE, ErrorHandler::StringTooShort);
memcpy(string, data + readPosition, size);
diff --git a/src/MessageParser.cpp b/src/MessageParser.cpp
index d382dd3432bc10b1dbe1b097db8afec2a56827ef..bf3d36e2e1c73cd91e3130c5ea92f2652e9e1073 100644
--- a/src/MessageParser.cpp
+++ b/src/MessageParser.cpp
@@ -36,10 +36,10 @@ Message MessageParser::parse(uint8_t *data, uint32_t length) {
auto sequenceFlags = static_cast<uint8_t>(packetSequenceControl >> 14);
// Returning an internal error, since the Message is not available yet
- ASSERT_INTERNAL(versionNumber == 0, ErrorHandler::UnacceptablePacket);
- ASSERT_INTERNAL(secondaryHeaderFlag == 1, ErrorHandler::UnacceptablePacket);
- ASSERT_INTERNAL(sequenceFlags == 0x3, ErrorHandler::UnacceptablePacket);
- ASSERT_INTERNAL(packetDataLength == length - 6, ErrorHandler::UnacceptablePacket);
+ ASSERT_INTERNAL(versionNumber == 0u, ErrorHandler::UnacceptablePacket);
+ ASSERT_INTERNAL(secondaryHeaderFlag == 1u, ErrorHandler::UnacceptablePacket);
+ ASSERT_INTERNAL(sequenceFlags == 0x3u, ErrorHandler::UnacceptablePacket);
+ ASSERT_INTERNAL(packetDataLength == (length - 6u), ErrorHandler::UnacceptablePacket);
Message message(0, 0, packetType, APID);
@@ -62,7 +62,7 @@ void MessageParser::parseTC(const uint8_t *data, uint16_t length, Message &messa
// todo: Fix this parsing function, because it assumes PUS header in data, which is not true
// with the current implementation
- ErrorHandler::assertRequest(pusVersion == 2, message, ErrorHandler::UnacceptableMessage);
+ ErrorHandler::assertRequest(pusVersion == 2u, message, ErrorHandler::UnacceptableMessage);
// Remove the length of the header
length -= 5;
@@ -110,7 +110,7 @@ void MessageParser::parseTM(const uint8_t *data, uint16_t length, Message &messa
uint8_t serviceType = data[1];
uint8_t messageType = data[2];
- ErrorHandler::assertRequest(pusVersion == 2, message, ErrorHandler::UnacceptableMessage);
+ ErrorHandler::assertRequest(pusVersion == 2u, message, ErrorHandler::UnacceptableMessage);
// Remove the length of the header
length -= 5;
diff --git a/src/Service.cpp b/src/Service.cpp
index 66fdd045bb9f777cea939f8c32fb344a995263a5..559315bd7ed54f3840139c3303b3b65eccb71e25 100644
--- a/src/Service.cpp
+++ b/src/Service.cpp
@@ -1 +1,3 @@
#include "Service.hpp"
+
+// Nothing exists here yet
diff --git a/src/Services/EventActionService.cpp b/src/Services/EventActionService.cpp
index 6ac29c0531184da62bbd617f94189fb240995cf6..ed44e1c697682fab5c941691c7eaa27c6183fe5b 100644
--- a/src/Services/EventActionService.cpp
+++ b/src/Services/EventActionService.cpp
@@ -15,8 +15,8 @@ void EventActionService::addEventActionDefinitions(Message message) {
uint16_t eventDefinitionID = message.readEnum16();
bool accepted = true;
for (index = 0; index < ECSS_EVENT_ACTION_STRUCT_ARRAY_SIZE; index++) {
- if (eventActionDefinitionArray[index].applicationId == applicationID &&
- eventActionDefinitionArray[index].eventDefinitionID == eventDefinitionID &&
+ if ((eventActionDefinitionArray[index].applicationId == applicationID) &&
+ (eventActionDefinitionArray[index].eventDefinitionID == eventDefinitionID) &&
eventActionDefinitionArray[index].enabled) {
// @todo: throw a failed start of execution error
accepted = false;
@@ -34,7 +34,7 @@ void EventActionService::addEventActionDefinitions(Message message) {
eventActionDefinitionArray[index].enabled = true;
eventActionDefinitionArray[index].applicationId = applicationID;
eventActionDefinitionArray[index].eventDefinitionID = eventDefinitionID;
- if (message.dataSize - 4 > ECSS_TC_REQUEST_STRING_SIZE) {
+ if ((message.dataSize - 4) > ECSS_TC_REQUEST_STRING_SIZE) {
ErrorHandler::reportInternalError(ErrorHandler::InternalErrorType::MessageTooLarge);
} else {
char data[ECSS_TC_REQUEST_STRING_SIZE];
@@ -54,8 +54,8 @@ void EventActionService::deleteEventActionDefinitions(Message message) {
uint16_t applicationID = message.readEnum16();
uint16_t eventDefinitionID = message.readEnum16();
for (uint16_t index = 0; index < ECSS_EVENT_ACTION_STRUCT_ARRAY_SIZE; index++) {
- if (eventActionDefinitionArray[index].applicationId == applicationID &&
- eventActionDefinitionArray[index].eventDefinitionID == eventDefinitionID &&
+ if ((eventActionDefinitionArray[index].applicationId == applicationID) &&
+ (eventActionDefinitionArray[index].eventDefinitionID == eventDefinitionID) &&
eventActionDefinitionArray[index].enabled) {
eventActionDefinitionArray[index].empty = true;
eventActionDefinitionArray[index].eventDefinitionID = 65535;
@@ -88,13 +88,13 @@ void EventActionService::enableEventActionDefinitions(Message message) {
message.assertTC(19, 4);
uint16_t numberOfEventActionDefinitions = message.readUint16();
- if (numberOfEventActionDefinitions != 0){
+ if (numberOfEventActionDefinitions != 0u){
for (uint16_t i = 0; i < numberOfEventActionDefinitions; i++) {
uint16_t applicationID = message.readEnum16();
uint16_t eventDefinitionID = message.readEnum16();
for (uint16_t index = 0; index < ECSS_EVENT_ACTION_STRUCT_ARRAY_SIZE; index++) {
- if (eventActionDefinitionArray[index].applicationId == applicationID &&
- eventActionDefinitionArray[index].eventDefinitionID == eventDefinitionID) {
+ if ((eventActionDefinitionArray[index].applicationId == applicationID) &&
+ (eventActionDefinitionArray[index].eventDefinitionID == eventDefinitionID)) {
eventActionDefinitionArray[index].enabled = true;
}
}
@@ -113,13 +113,13 @@ void EventActionService::disableEventActionDefinitions(Message message) {
message.assertTC(19, 5);
uint16_t numberOfEventActionDefinitions = message.readUint16();
- if (numberOfEventActionDefinitions != 0){
+ if (numberOfEventActionDefinitions != 0u) {
for (uint16_t i = 0; i < numberOfEventActionDefinitions; i++) {
uint16_t applicationID = message.readEnum16();
uint16_t eventDefinitionID = message.readEnum16();
for (uint16_t index = 0; index < ECSS_EVENT_ACTION_STRUCT_ARRAY_SIZE; index++) {
- if (eventActionDefinitionArray[index].applicationId == applicationID &&
- eventActionDefinitionArray[index].eventDefinitionID == eventDefinitionID) {
+ if ((eventActionDefinitionArray[index].applicationId == applicationID) &&
+ (eventActionDefinitionArray[index].eventDefinitionID == eventDefinitionID)) {
eventActionDefinitionArray[index].enabled = false;
}
}
diff --git a/src/Services/FunctionManagementService.cpp b/src/Services/FunctionManagementService.cpp
index 8dcc602792b45199d17bf95e53330905ebcfaab0..ed05573ba488384dceb382f331055253fbdf26de 100644
--- a/src/Services/FunctionManagementService.cpp
+++ b/src/Services/FunctionManagementService.cpp
@@ -15,7 +15,7 @@ void FunctionManagementService::call(Message& msg) {
msg.readString(funcName, FUNC_NAME_LENGTH);
msg.readString(funcArgs, MAX_ARG_LENGTH);
- if (msg.dataSize > FUNC_NAME_LENGTH + MAX_ARG_LENGTH) {
+ if (msg.dataSize > (FUNC_NAME_LENGTH + MAX_ARG_LENGTH)) {
ErrorHandler::reportError(msg,
ErrorHandler::ExecutionStartErrorType::UnknownExecutionStartError); // report failed
// start of execution as requested by the standard
diff --git a/src/Services/MemoryManagementService.cpp b/src/Services/MemoryManagementService.cpp
index c29c8fc1fc6a60efda6386a10d20c733d7ad78b2..0dd6f769d264ba40e9ac5e37b2d8179d6e4d9a2e 100644
--- a/src/Services/MemoryManagementService.cpp
+++ b/src/Services/MemoryManagementService.cpp
@@ -177,32 +177,32 @@ bool MemoryManagementService::addressValidator(
switch (memId) {
case MemoryManagementService::MemoryID::DTCMRAM:
- if (address >= DTCMRAM_LOWER_LIM && address <= DTCMRAM_UPPER_LIM) {
+ if ((address >= DTCMRAM_LOWER_LIM) && (address <= DTCMRAM_UPPER_LIM)) {
validIndicator = true;
}
break;
case MemoryManagementService::MemoryID::ITCMRAM:
- if (address >= ITCMRAM_LOWER_LIM && address <= ITCMRAM_UPPER_LIM) {
+ if ((address >= ITCMRAM_LOWER_LIM) && (address <= ITCMRAM_UPPER_LIM)) {
validIndicator = true;
}
break;
case MemoryManagementService::MemoryID::RAM_D1:
- if (address >= RAM_D1_LOWER_LIM && address <= RAM_D1_UPPER_LIM) {
+ if ((address >= RAM_D1_LOWER_LIM) && (address <= RAM_D1_UPPER_LIM)) {
validIndicator = true;
}
break;
case MemoryManagementService::MemoryID::RAM_D2:
- if (address >= RAM_D2_LOWER_LIM && address <= RAM_D2_UPPER_LIM) {
+ if ((address >= RAM_D2_LOWER_LIM) && (address <= RAM_D2_UPPER_LIM)) {
validIndicator = true;
}
break;
case MemoryManagementService::MemoryID::RAM_D3:
- if (address >= RAM_D3_LOWER_LIM && address <= RAM_D3_UPPER_LIM) {
+ if ((address >= RAM_D3_LOWER_LIM) && (address <= RAM_D3_UPPER_LIM)) {
validIndicator = true;
}
break;
case MemoryManagementService::MemoryID::FLASH:
- if (address >= FLASH_LOWER_LIM && address <= FLASH_UPPER_LIM) {
+ if ((address >= FLASH_LOWER_LIM) && (address <= FLASH_UPPER_LIM)) {
validIndicator = true;
}
break;
diff --git a/src/Services/ParameterService.cpp b/src/Services/ParameterService.cpp
index b32fa5b589adf76d9910c7fd3ca1c3aa33803f38..15b4122350da1b5f3e5f1ede35400fe2569f6ba1 100644
--- a/src/Services/ParameterService.cpp
+++ b/src/Services/ParameterService.cpp
@@ -53,7 +53,7 @@ void ParameterService::reportParameterIds(Message& paramIds) {
uint16_t ids = paramIds.readUint16();
reqParam.appendUint16(numOfValidIds(paramIds)); // include the number of valid IDs
- for (int i = 0; i < ids; i++) {
+ for (uint16_t i = 0; i < ids; i++) {
uint16_t currId = paramIds.readUint16(); // current ID to be appended
if (paramsList.find(currId) != paramsList.end()) {
@@ -84,7 +84,7 @@ void ParameterService::setParameterIds(Message& newParamValues) {
uint16_t ids = newParamValues.readUint16(); //get number of ID's
- for (int i = 0; i < ids; i++) {
+ for (uint16_t i = 0; i < ids; i++) {
uint16_t currId = newParamValues.readUint16();
if (paramsList.find(currId) != paramsList.end()) {
@@ -105,7 +105,7 @@ uint16_t ParameterService::numOfValidIds(Message idMsg) {
uint16_t ids = idMsg.readUint16(); // first 16bits of the packet are # of IDs
uint16_t validIds = 0;
- for (int i = 0; i < ids; i++) {
+ for (uint16_t i = 0; i < ids; i++) {
uint16_t currId = idMsg.readUint16();
if (idMsg.messageType == 3) {
diff --git a/src/main.cpp b/src/main.cpp
index 4e5a4f4652fe4463d4d20f1586d15eb48c922ec0..f9430a0e16e406b8465fcf0e85cf3dc6f81ced2f 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -322,10 +322,10 @@ int main() {
receivedMsg = Message(11, 4, Message::TC, 1);
receivedMsg.appendUint16(2); // Total number of requests
- receivedMsg.appendUint32(currentTime + 1556435);
+ receivedMsg.appendUint32(currentTime + 1556435u);
receivedMsg.appendString(msgParser.convertTCToStr(testMessage1));
- receivedMsg.appendUint32(currentTime + 1957232);
+ receivedMsg.appendUint32(currentTime + 1957232u);
receivedMsg.appendString(msgParser.convertTCToStr(testMessage2));
timeBasedSchedulingService.insertActivities(receivedMsg);
diff --git a/test/Services/MemoryManagementService.cpp b/test/Services/MemoryManagementService.cpp
index ec1ef2d5c0e4094d1905ad3ebfcc342d5adef396..781aa17f08cadff2971f32e55d3858ceb111ccc1 100644
--- a/test/Services/MemoryManagementService.cpp
+++ b/test/Services/MemoryManagementService.cpp
@@ -38,7 +38,6 @@ TEST_CASE("TM[6,5]", "[service][st06]") {
uint8_t checkString[ECSS_MAX_STRING_SIZE];
uint16_t readSize = 0, checksum = 0;
- MemoryManagementService memMangService;
Message receivedPacket = Message(6, 5, Message::TC, 1);
receivedPacket.appendEnum8(MemoryManagementService::MemoryID::EXTERNAL); // Memory ID
receivedPacket.appendUint16(3); // Iteration count (Equal to 3 test strings)
@@ -100,7 +99,6 @@ TEST_CASE("TM[6,9]", "[service][st06]") {
uint8_t testString_2[8] = "SecStrT";
uint16_t readSize = 0, checksum = 0;
- MemoryManagementService memMangService;
Message receivedPacket = Message(6, 9, Message::TC, 1);
receivedPacket.appendEnum8(MemoryManagementService::MemoryID::EXTERNAL); // Memory ID
receivedPacket.appendUint16(2); // Iteration count
diff --git a/test/Services/TimeBasedSchedulingService.cpp b/test/Services/TimeBasedSchedulingService.cpp
index bfa878325c513b2a28dd5f1c840cd5d414a7c7ba..daee588c5404096ee09166cc8b4fa3049357c150 100644
--- a/test/Services/TimeBasedSchedulingService.cpp
+++ b/test/Services/TimeBasedSchedulingService.cpp
@@ -23,9 +23,12 @@ namespace unit_test {
static auto scheduledActivities(TimeBasedSchedulingService &tmService) {
std::vector<TimeBasedSchedulingService::ScheduledActivity*>listElements;
- for (auto &element : tmService.scheduledActivities) {
- listElements.push_back(&element);
- }
+ std::transform(tmService.scheduledActivities.begin(),
+ tmService.scheduledActivities.end(),
+ std::back_inserter(listElements),
+ [](auto & activity) -> auto { return &activity; }
+ );
+
return listElements; // Return the list elements
}
};
@@ -427,12 +430,12 @@ TEST_CASE("TC[11,5] Activity deletion by ID", "[service][st11]") {
TEST_CASE("TC[11,3] Reset schedule", "[service][st11]") {
TimeBasedSchedulingService timeService;
- auto scheduledActivities = activityInsertion(timeService);
+ activityInsertion(timeService);
Message receivedMessage(11, 3, Message::TC, 1);
timeService.resetSchedule(receivedMessage);
- scheduledActivities = unit_test::Tester::scheduledActivities(timeService); // Get the new list
+ auto scheduledActivities = unit_test::Tester::scheduledActivities(timeService); // Get the new list
REQUIRE(scheduledActivities.empty());
REQUIRE(not unit_test::Tester::executionFunctionStatus(timeService));