From be01b831ae24563d810c2a856fcf32d66161f88d Mon Sep 17 00:00:00 2001 From: kongr45gpen <electrovesta@gmail.com> Date: Mon, 17 Oct 2022 20:34:07 +0000 Subject: [PATCH] Make clang-tidy work again --- .gitlab-ci.yml | 8 +- ci/.clang-tidy | 18 ++-- inc/ECSS_Definitions.hpp | 4 +- inc/ErrorHandler.hpp | 9 +- inc/Helpers/AllMessageTypes.hpp | 2 +- inc/Helpers/HousekeepingStructure.hpp | 5 +- inc/Helpers/PMONBase.hpp | 8 +- inc/Helpers/Statistic.hpp | 4 +- inc/Message.hpp | 17 ++-- inc/MessageParser.hpp | 2 +- inc/Service.hpp | 2 +- inc/ServicePool.hpp | 2 +- inc/Services/EventActionService.hpp | 5 +- inc/Services/EventReportService.hpp | 30 +++---- inc/Services/FunctionManagementService.hpp | 2 +- inc/Services/LargePacketTransferService.hpp | 6 +- inc/Services/MemoryManagementService.hpp | 33 ++++--- inc/Services/ParameterService.hpp | 2 +- inc/Services/ParameterStatisticsService.hpp | 4 +- inc/Services/TimeBasedSchedulingService.hpp | 16 ++-- inc/Time/TimeStamp.hpp | 8 +- inc/Time/UTCTimestamp.hpp | 8 +- inc/etl_profile.h | 1 - inc/macros.hpp | 2 + src/Helpers/AllMessageTypes.cpp | 74 ++++++++-------- src/Helpers/PMONBase.cpp | 6 +- src/Helpers/Statistic.cpp | 6 +- src/Platform/x86/main.cpp | 70 +++++++-------- src/ServicePool.cpp | 4 +- src/Services/FunctionManagementService.cpp | 7 +- src/Services/MemoryManagementService.cpp | 85 ++++++++++--------- src/Services/ParameterService.cpp | 2 +- .../RealTimeForwardingControlService.cpp | 15 ++-- src/Services/StorageAndRetrievalService.cpp | 36 ++++---- test/MessageTests.cpp | 8 +- test/Services/RealTimeForwardingControl.cpp | 18 ++-- 36 files changed, 261 insertions(+), 268 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index f5227898..324fa793 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -40,17 +40,19 @@ cppcheck: - ./cppcheck-html-report clang-tidy: - image: spacedot/clang-tools:13.0.0-html-1.3.7 + image: spacedot/clang-tools:13.0.0-html-1.4.1 stage: analyze script: - cd $CI_PROJECT_DIR - cmake -B ./build -DCMAKE_EXPORT_COMPILE_COMMANDS=ON - - clang-tidy -p $CI_PROJECT_DIR/build/compile_commands.json --checks=* `find $CI_PROJECT_DIR/src -type f -regextype posix-egrep -regex '.*\.(cpp|hpp|c|h)'` >> clang-tidy-output.log + - clang-tidy -p $CI_PROJECT_DIR/build/compile_commands.json --config-file=$CI_PROJECT_DIR/ci/.clang-tidy --use-color `find $CI_PROJECT_DIR/src $CI_PROJECT_DIR/inc -not -path "*/Platform/*" -type f -regextype posix-egrep -regex '.*\.(cpp|hpp|c|h)'` | tee clang-tidy-output.log after_script: + - sed -e 's/\x1b\[[0-9;]*m//g' -i clang-tidy-output.log - mkdir clang-tidy-html-report - clang-tidy-html clang-tidy-output.log - mv clang.html clang-tidy-html-report artifacts: + when: always paths: - ./clang-tidy-html-report @@ -126,8 +128,10 @@ pages: - find ./.public - mv .public public after_script: + - "echo Artifacts for this build: ${CI_JOB_URL}/artifacts/browse" - "echo Base page for this branch: ${CI_PAGES_URL}" artifacts: + expose_as: 'build artifacts and documentation' paths: - public # Upload the resulting website # only: diff --git a/ci/.clang-tidy b/ci/.clang-tidy index 478e0527..e86d3dac 100644 --- a/ci/.clang-tidy +++ b/ci/.clang-tidy @@ -1,5 +1,5 @@ --- -Checks: > +Checks: > -clang-diagnostic-error, clang-analyzer-*, bugprone-*, @@ -7,20 +7,26 @@ Checks: > cppcoreguidelines-*,-cppcoreguidelines-pro-bounds-array-to-pointer-decay,-cppcoreguidelines-pro-bounds-pointer-arithmetic,-cppcoreguidelines-pro-type-reinterpret-cast,-cppcoreguidelines-pro-bounds-constant-array-index,-cppcoreguidelines-avoid-magic-numbers,-cppcoreguidelines-avoid-c-arrays, misc-*,-misc-non-private-member-variables-in-classes, fuchsia-multiple-inheritance, - google-*,-google-readability-todo, + google-*,-google-readability-todo,-google-build-using-namespace,-google-runtime-int, fuchsia-statically-constructed-objects, hicpp-exception-baseclass, hicpp-signed-bitwise, - llvm-*,-llvm-include-order,-llvm-header-guard, + llvm-*,-llvm-include-order,-llvm-header-guard,-llvm-else-after-return, modernize-use-auto, modernize-use-equals-default, misc-*, -misc-non-private-member-variables-in-classes, performance-*, - readability-*,-readability-magic-numbers, + readability-*,-readability-magic-numbers,-readability-else-after-return, zircon-* -WarningsAsErrors: '*,-misc-unused-parameters,-llvm-header-guard,-cppcoreguidelines-pro-type-member-init,-google-runtime-references,-clang-diagnostic-tautological-constant-out-of-range-compare,-readability-redundant-declaration,-modernize-use-equals-default,-fuchsia-statically-constructed-objects,-hicpp-signed-bitwise,-cert-err58-cpp,-clang-diagnostic-error,-misc-noexcept-move-constructor' -HeaderFilterRegex: 'ecss-services\/((?!lib\/).)*$' +WarningsAsErrors: '*,-bugprone-easily-swappable-parameters,-misc-unused-parameters,-llvm-header-guard, +-cppcoreguidelines-pro-type-member-init,-cppcoreguidelines-non-private-member-variables-in-classes, +-google-runtime-references,-clang-diagnostic-tautological-constant-out-of-range-compare, +-readability-redundant-declaration,-modernize-use-equals-default,-fuchsia-statically-constructed-objects, +-hicpp-signed-bitwise,-cert-err58-cpp,-clang-diagnostic-error,-misc-noexcept-move-constructor, +-performance-no-int-to-ptr' +#HeaderFilterRegex: '^ecss-services\/((?!lib\/).)*$' +#HeaderFilterRegex: '^((?!/lib/).)*$' AnalyzeTemporaryDtors: false ... diff --git a/inc/ECSS_Definitions.hpp b/inc/ECSS_Definitions.hpp index 310997bb..0434ced3 100644 --- a/inc/ECSS_Definitions.hpp +++ b/inc/ECSS_Definitions.hpp @@ -43,7 +43,7 @@ inline const uint16_t ECSSSecondaryTCHeaderSize = 5U; /** * The maximum size of a regular ECSS message, plus its headers and trailing data, in bytes */ -inline const uint16_t CCSDSMaxMessageSize = ECSSMaxMessageSize + CCSDSPrimaryHeaderSize + ECSSSecondaryTMHeaderSize + 2u; +inline const uint16_t CCSDSMaxMessageSize = ECSSMaxMessageSize + CCSDSPrimaryHeaderSize + ECSSSecondaryTMHeaderSize + 2U; /** * The maximum size of a string to be read or appended to a Message, in bytes @@ -238,7 +238,7 @@ inline const uint8_t ECSSMaxEventDefinitionIDs = 15; /** * Limits noting the minimum and maximum valid Virtual Channels used by the Storage and Retrieval subservice */ -inline struct { +inline const struct { uint8_t min = 1; uint8_t max = 10; } VirtualChannelLimits; diff --git a/inc/ErrorHandler.hpp b/inc/ErrorHandler.hpp index 55a37bd0..b1c8d736 100644 --- a/inc/ErrorHandler.hpp +++ b/inc/ErrorHandler.hpp @@ -60,7 +60,6 @@ public: * Asked a Message type that doesn't exist */ UnknownMessageType = 7, - /** * Asked to append unnecessary spare bits */ @@ -93,10 +92,14 @@ public: * Invalid TimeStamp parameters at creation */ InvalidTimeStampInput = 15, + /** + * A requested element is not found + */ + ElementNotInArray = 16, /** * Timestamp out of bounds to be stored or converted */ - TimeStampOutOfBounds = 16, + TimeStampOutOfBounds = 17, }; /** @@ -494,7 +497,7 @@ public: * @return The corresponding ErrorSource */ template <typename ErrorType> - inline static ErrorSource findErrorSource(ErrorType error) { + inline static ErrorSource findErrorSource(ErrorType errorType) { // Static type checking ErrorSource source = Internal; diff --git a/inc/Helpers/AllMessageTypes.hpp b/inc/Helpers/AllMessageTypes.hpp index fb5c2c48..7e4ffd5a 100644 --- a/inc/Helpers/AllMessageTypes.hpp +++ b/inc/Helpers/AllMessageTypes.hpp @@ -13,7 +13,7 @@ namespace AllMessageTypes { * Map containing all the message types, per service. The key is the ServiceType and the value, * an etl vector containing the message types. */ - extern etl::map<uint8_t, etl::vector<uint8_t, ECSSMaxReportTypeDefinitions>, ECSSMaxServiceTypeDefinitions> messagesOfService; + extern const etl::map<uint8_t, etl::vector<uint8_t, ECSSMaxReportTypeDefinitions>, ECSSMaxServiceTypeDefinitions> MessagesOfService; } // namespace AllMessageTypes diff --git a/inc/Helpers/HousekeepingStructure.hpp b/inc/Helpers/HousekeepingStructure.hpp index 98271aa3..34256837 100644 --- a/inc/Helpers/HousekeepingStructure.hpp +++ b/inc/Helpers/HousekeepingStructure.hpp @@ -12,8 +12,7 @@ * * @author Petridis Konstantinos <petridkon@gmail.com> */ -class HousekeepingStructure { -public: +struct HousekeepingStructure { uint8_t structureId; /** @@ -34,4 +33,4 @@ public: HousekeepingStructure() = default; }; -#endif \ No newline at end of file +#endif diff --git a/inc/Helpers/PMONBase.hpp b/inc/Helpers/PMONBase.hpp index 66c00e58..0a56357a 100644 --- a/inc/Helpers/PMONBase.hpp +++ b/inc/Helpers/PMONBase.hpp @@ -1,13 +1,13 @@ #ifndef ECSS_SERVICES_PMONBASE_HPP #define ECSS_SERVICES_PMONBASE_HPP #include <cstdint> +#include "ECSS_Definitions.hpp" +#include "Helpers/Parameter.hpp" #include "Message.hpp" -#include "etl/array.h" #include "Service.hpp" -#include "Helpers/Parameter.hpp" -#include "etl/map.h" -#include "ECSS_Definitions.hpp" +#include "etl/array.h" #include "etl/list.h" +#include "etl/map.h" /** * Base class for Parameter Monitoring definitions. Contains the common variables of all check types. diff --git a/inc/Helpers/Statistic.hpp b/inc/Helpers/Statistic.hpp index 63fa8d64..cbf122db 100644 --- a/inc/Helpers/Statistic.hpp +++ b/inc/Helpers/Statistic.hpp @@ -39,7 +39,7 @@ public: * Appends itself to the received Message * message. */ - void appendStatisticsToMessage(Message& report); + void appendStatisticsToMessage(Message& report) const; /** * Setter function @@ -49,7 +49,7 @@ public: /** * Check if all the statistics are initialized */ - bool statisticsAreInitialized(); + bool statisticsAreInitialized() const; }; #endif diff --git a/inc/Message.hpp b/inc/Message.hpp index dfb142f7..327a9de4 100644 --- a/inc/Message.hpp +++ b/inc/Message.hpp @@ -243,7 +243,6 @@ public: */ void readCString(char* string, uint16_t size); -public: Message(uint8_t serviceType, uint8_t messageType, PacketType packetType, uint16_t applicationId); Message(uint8_t serviceType, uint8_t messageType, Message::PacketType packetType); @@ -665,7 +664,7 @@ public: * * @return True if the message is of correct type, false if not */ - bool assertType(Message::PacketType expectedPacketType, uint8_t expectedServiceType, uint8_t expectedMessageType) { + bool assertType(Message::PacketType expectedPacketType, uint8_t expectedServiceType, uint8_t expectedMessageType) const { bool status = true; if ((packetType != expectedPacketType) || (serviceType != expectedServiceType) || @@ -681,7 +680,7 @@ public: * Alias for Message::assertType(Message::TC, \p expectedServiceType, \p * expectedMessageType) */ - bool assertTC(uint8_t expectedServiceType, uint8_t expectedMessageType) { + bool assertTC(uint8_t expectedServiceType, uint8_t expectedMessageType) const { return assertType(TC, expectedServiceType, expectedMessageType); } @@ -689,7 +688,7 @@ public: * Alias for Message::assertType(Message::TM, \p expectedServiceType, \p * expectedMessageType) */ - bool assertTM(uint8_t expectedServiceType, uint8_t expectedMessageType) { + bool assertTM(uint8_t expectedServiceType, uint8_t expectedMessageType) const { return assertType(TM, expectedServiceType, expectedMessageType); } }; @@ -741,8 +740,8 @@ inline void Message::append(const double& value) { appendDouble(value); } template <> -inline void Message::append(const Time::DefaultCUC& timeCUC) { - appendDefaultCUCTimeStamp(timeCUC); +inline void Message::append(const Time::DefaultCUC& value) { + appendDefaultCUCTimeStamp(value); } template <> inline void Message::append(const Time::RelativeTime& value) { @@ -793,14 +792,12 @@ template <> inline bool Message::read<bool>() { return readBoolean(); } -template <> -inline char Message::read() { - return readByte(); -} + template <> inline float Message::read() { return readFloat(); } + template <> inline double Message::read() { return readDouble(); diff --git a/inc/MessageParser.hpp b/inc/MessageParser.hpp index 031a6fc5..825548d9 100644 --- a/inc/MessageParser.hpp +++ b/inc/MessageParser.hpp @@ -75,7 +75,7 @@ public: * error. Messages smaller than \p size are padded with zeros. When `size = 0`, there is no size limit. * @return A String class containing the parsed Message */ - static String<CCSDSMaxMessageSize> composeECSS(const Message& message, uint16_t size = 0u); // Ignore-MISRA + static String<CCSDSMaxMessageSize> composeECSS(const Message& message, uint16_t size = 0U); // Ignore-MISRA /** * @brief Converts a TC or TM message to a packet string, appending the ECSS and then the CCSDS header diff --git a/inc/Service.hpp b/inc/Service.hpp index b7ffbab7..76385897 100644 --- a/inc/Service.hpp +++ b/inc/Service.hpp @@ -34,7 +34,7 @@ protected: * the TC[17,3] message has `messageType = 3`. * @todo See if the Message must be returned by reference */ - Message createTM(uint8_t messageType) { + Message createTM(uint8_t messageType) const { return Message(serviceType, messageType, Message::TM); } diff --git a/inc/ServicePool.hpp b/inc/ServicePool.hpp index 080a3971..23067cae 100644 --- a/inc/ServicePool.hpp +++ b/inc/ServicePool.hpp @@ -138,6 +138,6 @@ public: /** * A global variable that defines the basic pool where services can be fetched from */ -extern ServicePool Services; +extern ServicePool Services; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) #endif // ECSS_SERVICES_SERVICEPOOL_HPP diff --git a/inc/Services/EventActionService.hpp b/inc/Services/EventActionService.hpp index b9c33870..f5ab24d0 100644 --- a/inc/Services/EventActionService.hpp +++ b/inc/Services/EventActionService.hpp @@ -30,7 +30,7 @@ private: /** * Event-action function status */ - bool eventActionFunctionStatus; + bool eventActionFunctionStatus = true; /** * Custom function that is called right after an event takes place, to initiate @@ -69,7 +69,6 @@ public: EventActionService() { serviceType = ServiceType; - eventActionFunctionStatus = true; } /** @@ -132,7 +131,7 @@ public: * Getter for event-action function status * @return eventActionFunctionStatus */ - bool getEventActionFunctionStatus() { + bool getEventActionFunctionStatus() const { return eventActionFunctionStatus; } diff --git a/inc/Services/EventReportService.hpp b/inc/Services/EventReportService.hpp index f58dbeae..a5e13105 100644 --- a/inc/Services/EventReportService.hpp +++ b/inc/Services/EventReportService.hpp @@ -34,34 +34,24 @@ public: }; // Variables that count the event reports per severity level - uint16_t lowSeverityReportCount; - uint16_t mediumSeverityReportCount; - uint16_t highSeverityReportCount; + uint16_t lowSeverityReportCount = 0; + uint16_t mediumSeverityReportCount = 0; + uint16_t highSeverityReportCount = 0; // Variables that count the event occurences per severity level - uint16_t lowSeverityEventCount; - uint16_t mediumSeverityEventCount; - uint16_t highSeverityEventCount; + uint16_t lowSeverityEventCount = 0; + uint16_t mediumSeverityEventCount = 0; + uint16_t highSeverityEventCount = 0; - uint16_t disabledEventsCount; + uint16_t disabledEventsCount = 0; - uint16_t lastLowSeverityReportID; - uint16_t lastMediumSeverityReportID; - uint16_t lastHighSeverityReportID; + uint16_t lastLowSeverityReportID = 65535; + uint16_t lastMediumSeverityReportID = 65535; + uint16_t lastHighSeverityReportID = 65535; EventReportService() { stateOfEvents.set(); serviceType = ServiceType; - lowSeverityReportCount = 0; - mediumSeverityReportCount = 0; - highSeverityReportCount = 0; - disabledEventsCount = 0; - lowSeverityEventCount = 0; - mediumSeverityEventCount = 0; - highSeverityEventCount = 0; - lastLowSeverityReportID = 65535; - lastMediumSeverityReportID = 65535; - lastHighSeverityReportID = 65535; } /** diff --git a/inc/Services/FunctionManagementService.hpp b/inc/Services/FunctionManagementService.hpp index 422f98dd..78c77104 100644 --- a/inc/Services/FunctionManagementService.hpp +++ b/inc/Services/FunctionManagementService.hpp @@ -89,7 +89,7 @@ public: */ void include(String<ECSSFunctionNameLength> funcName, void (*ptr)(String<ECSSFunctionMaxArgLength>)); - int getMapSize() { + size_t getMapSize() { return funcPtrIndex.size(); } diff --git a/inc/Services/LargePacketTransferService.hpp b/inc/Services/LargePacketTransferService.hpp index 3f14695f..154f7c6d 100644 --- a/inc/Services/LargePacketTransferService.hpp +++ b/inc/Services/LargePacketTransferService.hpp @@ -66,20 +66,20 @@ public: * TC[13,9] Function that handles the first part of the uplink request * @param string This will change when these function will be modified */ - String<ECSSMaxFixedOctetStringSize> firstUplinkPart(const String<ECSSMaxFixedOctetStringSize>& string); + static String<ECSSMaxFixedOctetStringSize> firstUplinkPart(const String<ECSSMaxFixedOctetStringSize>& string); /** * TC[13,10] Function that handles the n-2 parts of the n-part uplink request * @param string This will change when these function will be modified */ - String<ECSSMaxFixedOctetStringSize> + static String<ECSSMaxFixedOctetStringSize> intermediateUplinkPart(const String<ECSSMaxFixedOctetStringSize>& string); /** * TC[13,11] Function that handles the last part of the uplink request * @param string This will change when these function will be modified */ - String<ECSSMaxFixedOctetStringSize> lastUplinkPart(const String<ECSSMaxFixedOctetStringSize>& string); + static String<ECSSMaxFixedOctetStringSize> lastUplinkPart(const String<ECSSMaxFixedOctetStringSize>& string); /** * Function that splits large messages diff --git a/inc/Services/MemoryManagementService.hpp b/inc/Services/MemoryManagementService.hpp index 6439bfa6..18348800 100644 --- a/inc/Services/MemoryManagementService.hpp +++ b/inc/Services/MemoryManagementService.hpp @@ -2,17 +2,16 @@ #define ECSS_SERVICES_MEMMANGSERVICE_HPP #include <memory> -#include "Service.hpp" -#include "Helpers/CRCHelper.hpp" #include "ErrorHandler.hpp" +#include "Helpers/CRCHelper.hpp" #include "Platform/STM32F7/MemoryAddressLimits.hpp" +#include "Service.hpp" /** * @ingroup Services */ class MemoryManagementService : public Service { public: - inline static const uint8_t ServiceType = 6; enum MessageType : uint8_t { @@ -50,16 +49,6 @@ public: public: explicit RawDataMemoryManagement(MemoryManagementService& parent); - /** - * TC[6,2] load raw values to memory - * - * @details This function loads new values to memory data areas - * specified in the request - * @param request Provide the received message as a parameter - * @todo Only allow aligned memory address to be start addresses - */ - void loadRawData(Message& request); - /** * TC[6,5] read raw memory values * @@ -82,9 +71,19 @@ public: * different memory types * @todo Only allow aligned memory address to be start addresses */ - void checkRawData(Message &request); + void checkRawData(Message& request); } rawDataMemorySubservice; + /** + * TC[6,2] load raw values to memory + * + * @details This function loads new values to memory data areas + * specified in the request + * @param request Provide the received message as a parameter + * @todo Only allow aligned memory address to be start addresses + */ + static void loadRawData(Message& request); + /** * It is responsible to call the suitable function that executes a telecommand packet. The source of that packet * is the ground station. @@ -101,20 +100,20 @@ private: * @param memId The ID of the memory to check is passed * @param address Takes the address to be checked for validity */ - bool addressValidator(MemoryManagementService::MemoryID memId, uint64_t address); + static bool addressValidator(MemoryManagementService::MemoryID memId, uint64_t address); /** * Check if the provided memory ID is valid * * @param memId The memory ID for validation */ - bool memoryIdValidator(MemoryManagementService::MemoryID memId); + static bool memoryIdValidator(MemoryManagementService::MemoryID memId); /** * Validate the data according to checksum calculation * */ - bool dataValidator(const uint8_t* data, uint16_t checksum, uint16_t length); + static bool dataValidator(const uint8_t* data, uint16_t checksum, uint16_t length); }; #endif // ECSS_SERVICES_MEMMANGSERVICE_HPP diff --git a/inc/Services/ParameterService.hpp b/inc/Services/ParameterService.hpp index 9ee75040..87180150 100644 --- a/inc/Services/ParameterService.hpp +++ b/inc/Services/ParameterService.hpp @@ -105,7 +105,7 @@ public: * * @param newParamValues: a valid TC[20, 3] message carrying parameter ID and replacement value */ - void setParameters(Message& newParamValues); + void setParameters(Message& newParamValues) const; /** * It is responsible to call the suitable function that executes a telecommand packet. The source of that packet diff --git a/inc/Services/ParameterStatisticsService.hpp b/inc/Services/ParameterStatisticsService.hpp index c686558e..d04a12af 100644 --- a/inc/Services/ParameterStatisticsService.hpp +++ b/inc/Services/ParameterStatisticsService.hpp @@ -72,7 +72,7 @@ public: /** * Returns the periodic statistics reporting status */ - inline bool getPeriodicReportingStatus() { + inline bool getPeriodicReportingStatus() const { return periodicStatisticsReportingStatus; } @@ -86,7 +86,7 @@ public: /** * Returns the periodic statistics reporting status */ - inline const uint16_t getReportingIntervalMs() { + inline uint16_t getReportingIntervalMs() const { return reportingIntervalMs; } diff --git a/inc/Services/TimeBasedSchedulingService.hpp b/inc/Services/TimeBasedSchedulingService.hpp index a3816071..ad68b795 100644 --- a/inc/Services/TimeBasedSchedulingService.hpp +++ b/inc/Services/TimeBasedSchedulingService.hpp @@ -10,18 +10,20 @@ // Include platform specific files #include "Helpers/TimeGetter.hpp" + +/** + * @def GROUPS_ENABLED + * @brief Indicates whether scheduling groups are enabled + */ +#define GROUPS_ENABLED 0 // NOLINT(cppcoreguidelines-macro-usage) + /** * @def SUB_SCHEDULES_ENABLED * @brief Indicates whether sub-schedules are supported * * @details Sub-schedules are currently not implemented so this has no effect */ -/** - * @def GROUPS_ENABLED - * @brief Indicates whether scheduling groups are enabled - */ -#define GROUPS_ENABLED 0 -#define SUB_SCHEDULES_ENABLED 0 +#define SUB_SCHEDULES_ENABLED 0 // NOLINT(cppcoreguidelines-macro-usage) /** * @brief Namespace to access private members during test @@ -98,7 +100,7 @@ private: * @details The ECSS standard requires that the activities are sorted in the TM message * response. Also it is better to have the activities sorted. */ - inline void + inline static void sortActivitiesReleaseTime(etl::list<ScheduledActivity, ECSSMaxNumberOfTimeSchedActivities>& schedActivities) { schedActivities.sort([](ScheduledActivity const& leftSide, ScheduledActivity const& rightSide) { // cppcheck-suppress diff --git a/inc/Time/TimeStamp.hpp b/inc/Time/TimeStamp.hpp index dbdefaf1..5bdc1c37 100644 --- a/inc/Time/TimeStamp.hpp +++ b/inc/Time/TimeStamp.hpp @@ -42,7 +42,7 @@ * @author Konstantinos Kanavouras * @see [CCSDS 301.0-B-4](https://public.ccsds.org/Pubs/301x0b4e1.pdf) */ -template <uint8_t BaseBytes, uint8_t FractionBytes = 0, int Num = 1, int Denom = 1> +template <uint8_t BaseBytes = 4, uint8_t FractionBytes = 0, int Num = 1, int Denom = 1> class TimeStamp { public: /** @@ -155,7 +155,7 @@ public: * @note Internally uses double-precision floating point to allow for arbitrary ratios */ template <uint8_t BaseBytesIn, uint8_t FractionBytesIn, int NumIn = 1, int DenomIn = 1> - explicit TimeStamp(TimeStamp<BaseBytesIn, FractionBytesIn, NumIn, DenomIn>); + explicit TimeStamp(TimeStamp<BaseBytesIn, FractionBytesIn, NumIn, DenomIn> input); /** * Convert an [std::chrono::duration](https://en.cppreference.com/w/cpp/chrono/duration) representing seconds from @ref Time::Epoch @@ -278,8 +278,8 @@ public: */ template < uint8_t BaseBytesIn, uint8_t FractionBytesIn, int NumIn = 1, int DenomIn = 1, // Template parameters of the 2nd timestamp - class Duration = std::chrono::duration< // Create a new Duration based on our RawDuration... - typename std::make_signed<typename RawDuration::rep>::type, // the Duration base type is equal to the RawDuration, but converted to signed from unsigned + class Duration = std::chrono::duration< // Create a new Duration based on our RawDuration... + typename std::make_signed<typename RawDuration::rep>::type, // the Duration base type is equal to the RawDuration, but converted to signed from unsigned typename RawDuration::period>> Duration operator-(const TimeStamp<BaseBytesIn, FractionBytesIn, NumIn, DenomIn>& operand) const { Duration myDuration = asDuration<Duration>(); diff --git a/inc/Time/UTCTimestamp.hpp b/inc/Time/UTCTimestamp.hpp index f8ceca62..a2fe7bf6 100644 --- a/inc/Time/UTCTimestamp.hpp +++ b/inc/Time/UTCTimestamp.hpp @@ -60,13 +60,13 @@ public: uint64_t seconds = duration_cast<duration<uint64_t>>(in).count(); - while (seconds >= (isLeapYear(year) ? 366 : 365) * SecondsPerDay) { - seconds -= (isLeapYear(year) ? 366 : 365) * SecondsPerDay; + while (seconds >= (isLeapYear(year) ? 366L : 365L) * SecondsPerDay) { + seconds -= (isLeapYear(year) ? 366L : 365L) * SecondsPerDay; year++; } - while (seconds >= (daysOfMonth() * SecondsPerDay)) { - seconds -= daysOfMonth() * SecondsPerDay; + while (seconds >= (daysOfMonth() * uint64_t{SecondsPerDay})) { + seconds -= daysOfMonth() * uint64_t{SecondsPerDay}; month++; if (month > MonthsPerYear) { diff --git a/inc/etl_profile.h b/inc/etl_profile.h index 14e6e6b8..8e82b7b1 100644 --- a/inc/etl_profile.h +++ b/inc/etl_profile.h @@ -5,7 +5,6 @@ #ifndef ECSS_SERVICES_ETL_PROFILE_H #define ECSS_SERVICES_ETL_PROFILE_H -#define ETL_THROW_EXCEPTIONS #define ETL_VERBOSE_ERRORS #define ETL_CHECK_PUSH_POP diff --git a/inc/macros.hpp b/inc/macros.hpp index 12cce6c0..47310486 100644 --- a/inc/macros.hpp +++ b/inc/macros.hpp @@ -6,6 +6,7 @@ * * @todo Actually hold program execution or throw an exception here */ +// NOLINTNEXTLINE(cppcoreguidelines-macro-usage) #define ASSERT_INTERNAL(cond, error) (ErrorHandler::assertInternal((cond), (error))) /** @@ -13,6 +14,7 @@ * * Only to be used within the Message class. */ +// NOLINTNEXTLINE(cppcoreguidelines-macro-usage) #define ASSERT_REQUEST(cond, error) (ErrorHandler::assertRequest((cond), *this, (error))) #endif // ECSS_SERVICES_MACROS_HPP diff --git a/src/Helpers/AllMessageTypes.cpp b/src/Helpers/AllMessageTypes.cpp index c67e09b3..0ee958f3 100644 --- a/src/Helpers/AllMessageTypes.cpp +++ b/src/Helpers/AllMessageTypes.cpp @@ -12,17 +12,17 @@ #include "Services/TimeBasedSchedulingService.hpp" namespace AllMessageTypes { - etl::vector<uint8_t, ECSSMaxReportTypeDefinitions> st01Messages = {RequestVerificationService::MessageType::FailedAcceptanceReport, - RequestVerificationService::MessageType::FailedCompletionOfExecution, - RequestVerificationService::MessageType::FailedProgressOfExecution, - RequestVerificationService::MessageType::FailedRoutingReport, - RequestVerificationService::MessageType::FailedStartOfExecution, - RequestVerificationService::MessageType::SuccessfulAcceptanceReport, - RequestVerificationService::MessageType::SuccessfulCompletionOfExecution, - RequestVerificationService::MessageType::SuccessfulProgressOfExecution, - RequestVerificationService::MessageType::SuccessfulStartOfExecution}; + const etl::vector<uint8_t, ECSSMaxReportTypeDefinitions> ST01Messages = {RequestVerificationService::MessageType::FailedAcceptanceReport, + RequestVerificationService::MessageType::FailedCompletionOfExecution, + RequestVerificationService::MessageType::FailedProgressOfExecution, + RequestVerificationService::MessageType::FailedRoutingReport, + RequestVerificationService::MessageType::FailedStartOfExecution, + RequestVerificationService::MessageType::SuccessfulAcceptanceReport, + RequestVerificationService::MessageType::SuccessfulCompletionOfExecution, + RequestVerificationService::MessageType::SuccessfulProgressOfExecution, + RequestVerificationService::MessageType::SuccessfulStartOfExecution}; - etl::vector<uint8_t, ECSSMaxReportTypeDefinitions> st03Messages = { + const etl::vector<uint8_t, ECSSMaxReportTypeDefinitions> ST03Messages = { HousekeepingService::MessageType::DisablePeriodicHousekeepingParametersReport, HousekeepingService::MessageType::EnablePeriodicHousekeepingParametersReport, HousekeepingService::MessageType::GenerateOneShotHousekeepingReport, @@ -30,43 +30,43 @@ namespace AllMessageTypes { HousekeepingService::MessageType::HousekeepingPeriodicPropertiesReport, HousekeepingService::MessageType::HousekeepingStructuresReport}; - etl::vector<uint8_t, ECSSMaxReportTypeDefinitions> st04Messages = { + const etl::vector<uint8_t, ECSSMaxReportTypeDefinitions> ST04Messages = { ParameterStatisticsService::MessageType::ParameterStatisticsDefinitionsReport, ParameterStatisticsService::MessageType::ParameterStatisticsReport, }; - etl::vector<uint8_t, ECSSMaxReportTypeDefinitions> st05Messages = {EventReportService::MessageType::HighSeverityAnomalyReport, - EventReportService::MessageType::DisabledListEventReport, - EventReportService::MessageType::InformativeEventReport, - EventReportService::MessageType::LowSeverityAnomalyReport, - EventReportService::MessageType::MediumSeverityAnomalyReport}; + const etl::vector<uint8_t, ECSSMaxReportTypeDefinitions> ST05Messages = {EventReportService::MessageType::HighSeverityAnomalyReport, + EventReportService::MessageType::DisabledListEventReport, + EventReportService::MessageType::InformativeEventReport, + EventReportService::MessageType::LowSeverityAnomalyReport, + EventReportService::MessageType::MediumSeverityAnomalyReport}; - etl::vector<uint8_t, ECSSMaxReportTypeDefinitions> st06Messages = {MemoryManagementService::MessageType::CheckRawMemoryDataReport, - MemoryManagementService::MessageType::DumpRawMemoryDataReport}; + const etl::vector<uint8_t, ECSSMaxReportTypeDefinitions> ST06Messages = {MemoryManagementService::MessageType::CheckRawMemoryDataReport, + MemoryManagementService::MessageType::DumpRawMemoryDataReport}; - etl::vector<uint8_t, ECSSMaxReportTypeDefinitions> st11Messages = {TimeBasedSchedulingService::MessageType::TimeBasedScheduledSummaryReport}; + const etl::vector<uint8_t, ECSSMaxReportTypeDefinitions> ST11Messages = {TimeBasedSchedulingService::MessageType::TimeBasedScheduledSummaryReport}; - etl::vector<uint8_t, ECSSMaxReportTypeDefinitions> st13Messages = {LargePacketTransferService::MessageType::FirstDownlinkPartReport, - LargePacketTransferService::MessageType::InternalDownlinkPartReport, - LargePacketTransferService::MessageType::LastDownlinkPartReport}; + const etl::vector<uint8_t, ECSSMaxReportTypeDefinitions> ST13Messages = {LargePacketTransferService::MessageType::FirstDownlinkPartReport, + LargePacketTransferService::MessageType::InternalDownlinkPartReport, + LargePacketTransferService::MessageType::LastDownlinkPartReport}; - etl::vector<uint8_t, ECSSMaxReportTypeDefinitions> st17Messages = {TestService::MessageType::AreYouAliveTestReport, - TestService::MessageType::OnBoardConnectionTestReport}; + const etl::vector<uint8_t, ECSSMaxReportTypeDefinitions> ST17Messages = {TestService::MessageType::AreYouAliveTestReport, + TestService::MessageType::OnBoardConnectionTestReport}; - etl::vector<uint8_t, ECSSMaxReportTypeDefinitions> st19Messages = {EventActionService::MessageType::EventActionStatusReport}; + const etl::vector<uint8_t, ECSSMaxReportTypeDefinitions> ST19Messages = {EventActionService::MessageType::EventActionStatusReport}; - etl::vector<uint8_t, ECSSMaxReportTypeDefinitions> st20Messages = {ParameterService::MessageType::ParameterValuesReport}; + const etl::vector<uint8_t, ECSSMaxReportTypeDefinitions> ST20Messages = {ParameterService::MessageType::ParameterValuesReport}; - etl::map<uint8_t, etl::vector<uint8_t, ECSSMaxReportTypeDefinitions>, ECSSMaxServiceTypeDefinitions> messagesOfService = { - {RequestVerificationService::ServiceType, st01Messages}, - {HousekeepingService::ServiceType, st03Messages}, - {ParameterStatisticsService::ServiceType, st04Messages}, - {EventReportService::ServiceType, st05Messages}, - {MemoryManagementService::ServiceType, st06Messages}, - {TimeBasedSchedulingService::ServiceType, st11Messages}, - {LargePacketTransferService::ServiceType, st13Messages}, - {TestService::ServiceType, st17Messages}, - {EventActionService::ServiceType, st19Messages}, - {ParameterService::ServiceType, st20Messages}}; + const etl::map<uint8_t, etl::vector<uint8_t, ECSSMaxReportTypeDefinitions>, ECSSMaxServiceTypeDefinitions> MessagesOfService = { + {RequestVerificationService::ServiceType, ST01Messages}, + {HousekeepingService::ServiceType, ST03Messages}, + {ParameterStatisticsService::ServiceType, ST04Messages}, + {EventReportService::ServiceType, ST05Messages}, + {MemoryManagementService::ServiceType, ST06Messages}, + {TimeBasedSchedulingService::ServiceType, ST11Messages}, + {LargePacketTransferService::ServiceType, ST13Messages}, + {TestService::ServiceType, ST17Messages}, + {EventActionService::ServiceType, ST19Messages}, + {ParameterService::ServiceType, ST20Messages}}; } // namespace AllMessageTypes diff --git a/src/Helpers/PMONBase.cpp b/src/Helpers/PMONBase.cpp index 85f163fd..2e64cb34 100644 --- a/src/Helpers/PMONBase.cpp +++ b/src/Helpers/PMONBase.cpp @@ -3,7 +3,5 @@ PMONBase::PMONBase(uint16_t monitoredParameterId, uint16_t repetitionNumber) - : monitoredParameter(monitoredParameter), monitoredParameterId(monitoredParameterId), - repetitionNumber(repetitionNumber) { - monitoredParameter = Services.parameterManagement.getParameter(monitoredParameterId)->get(); -} \ No newline at end of file + : monitoredParameter(Services.parameterManagement.getParameter(monitoredParameterId)->get()), monitoredParameterId(monitoredParameterId), + repetitionNumber(repetitionNumber) {} \ No newline at end of file diff --git a/src/Helpers/Statistic.cpp b/src/Helpers/Statistic.cpp index 672f4671..024c586b 100644 --- a/src/Helpers/Statistic.cpp +++ b/src/Helpers/Statistic.cpp @@ -17,14 +17,14 @@ void Statistic::updateStatistics(double value) { sampleCounter++; } -void Statistic::appendStatisticsToMessage(Message& report) { +void Statistic::appendStatisticsToMessage(Message& report) const { report.appendFloat(static_cast<float>(max)); report.append(timeOfMaxValue); report.appendFloat(static_cast<float>(min)); report.append(timeOfMinValue); report.appendFloat(static_cast<float>(mean)); - if (SupportsStandardDeviation) { + if constexpr (SupportsStandardDeviation) { double standardDeviation = 0; if (sampleCounter == 0) { standardDeviation = 0; @@ -50,7 +50,7 @@ void Statistic::resetStatistics() { sampleCounter = 0; } -bool Statistic::statisticsAreInitialized() { +bool Statistic::statisticsAreInitialized() const { return (sampleCounter == 0 and mean == 0 and sumOfSquares == 0 and timeOfMaxValue == Time::DefaultCUC(0) and timeOfMinValue == Time::DefaultCUC(0) and max == -std::numeric_limits<double>::infinity() and min == std::numeric_limits<double>::infinity()); diff --git a/src/Platform/x86/main.cpp b/src/Platform/x86/main.cpp index d8be71a1..5d472ea1 100644 --- a/src/Platform/x86/main.cpp +++ b/src/Platform/x86/main.cpp @@ -1,25 +1,25 @@ -#include <iostream> #include <Logger.hpp> -#include <Time/UTCTimestamp.hpp> #include <Platform/x86/Helpers/UTCTimestamp.hpp> +#include <Time/UTCTimestamp.hpp> +#include <ctime> +#include <iostream> +#include "ErrorHandler.hpp" #include "Helpers/CRCHelper.hpp" -#include "Services/TestService.hpp" -#include "Services/ParameterService.hpp" -#include "Services/RequestVerificationService.hpp" -#include "Services/MemoryManagementService.hpp" +#include "Helpers/Statistic.hpp" +#include "Message.hpp" +#include "MessageParser.hpp" +#include "ServicePool.hpp" +#include "Services/EventActionService.hpp" #include "Services/EventReportService.hpp" #include "Services/FunctionManagementService.hpp" -#include "Services/EventActionService.hpp" #include "Services/LargePacketTransferService.hpp" -#include "Services/TimeBasedSchedulingService.hpp" +#include "Services/MemoryManagementService.hpp" +#include "Services/ParameterService.hpp" #include "Services/ParameterStatisticsService.hpp" -#include "Helpers/Statistic.hpp" -#include "Message.hpp" -#include "MessageParser.hpp" -#include "ErrorHandler.hpp" +#include "Services/RequestVerificationService.hpp" +#include "Services/TestService.hpp" +#include "Services/TimeBasedSchedulingService.hpp" #include "etl/String.hpp" -#include "ServicePool.hpp" -#include <ctime> int main() { LOG_NOTICE << "ECSS Services test application"; @@ -55,19 +55,19 @@ int main() { // Test code for reportParameter Message sentPacket = Message(ParameterService::ServiceType, ParameterService::MessageType::ReportParameterValues, Message::TC, 1); // application id is a dummy number (1) - sentPacket.appendUint16(2); // number of contained IDs - sentPacket.appendUint16(0); // first ID - sentPacket.appendUint16(1); // second ID + sentPacket.appendUint16(2); // number of contained IDs + sentPacket.appendUint16(0); // first ID + sentPacket.appendUint16(1); // second ID paramService.reportParameters(sentPacket); // Test code for setParameter Message sentPacket2 = Message(ParameterService::ServiceType, ParameterService::MessageType::SetParameterValues, Message::TC, 1); // application id is a dummy number (1) - sentPacket2.appendUint16(2); // number of contained IDs - sentPacket2.appendUint16(0); // first parameter ID - sentPacket2.appendUint32(63238); // settings for first parameter - sentPacket2.appendUint16(1); // 2nd parameter ID - sentPacket2.appendUint32(45823); // settings for 2nd parameter + sentPacket2.appendUint16(2); // number of contained IDs + sentPacket2.appendUint16(0); // first parameter ID + sentPacket2.appendUint32(63238); // settings for first parameter + sentPacket2.appendUint16(1); // 2nd parameter ID + sentPacket2.appendUint32(45823); // settings for 2nd parameter paramService.setParameters(sentPacket2); paramService.reportParameters(sentPacket); @@ -84,9 +84,9 @@ int main() { Message rcvPack = Message(MemoryManagementService::ServiceType, MemoryManagementService::MessageType::DumpRawMemoryData, Message::TC, 1); rcvPack.appendEnum8(MemoryManagementService::MemoryID::EXTERNAL); // Memory ID - rcvPack.appendUint16(3); // Iteration count - rcvPack.appendUint64(reinterpret_cast<uint64_t>(string)); // Start address - rcvPack.appendUint16(sizeof(string) / sizeof(string[0])); // Data read length + rcvPack.appendUint16(3); // Iteration count + rcvPack.appendUint64(reinterpret_cast<uint64_t>(string)); // Start address + rcvPack.appendUint16(sizeof(string) / sizeof(string[0])); // Data read length rcvPack.appendUint64(reinterpret_cast<uint64_t>(anotherStr)); rcvPack.appendUint16(sizeof(anotherStr) / sizeof(anotherStr[0])); @@ -100,21 +100,21 @@ int main() { uint8_t data[2] = {'h', 'R'}; rcvPack.appendEnum8(MemoryManagementService::MemoryID::EXTERNAL); // Memory ID - rcvPack.appendUint16(2); // Iteration count - rcvPack.appendUint64(reinterpret_cast<uint64_t>(pStr)); // Start address + rcvPack.appendUint16(2); // Iteration count + rcvPack.appendUint64(reinterpret_cast<uint64_t>(pStr)); // Start address rcvPack.appendOctetString(String<2>(data, 2)); - rcvPack.appendBits(16, CRCHelper::calculateCRC(data, 2)); // Append the CRC value + rcvPack.appendBits(16, CRCHelper::calculateCRC(data, 2)); // Append the CRC value rcvPack.appendUint64(reinterpret_cast<uint64_t>(pStr + 1)); // Start address rcvPack.appendOctetString(String<1>(data, 1)); rcvPack.appendBits(16, CRCHelper::calculateCRC(data, 1)); // Append the CRC value - memMangService.rawDataMemorySubservice.loadRawData(rcvPack); + memMangService.loadRawData(rcvPack); rcvPack = Message(MemoryManagementService::ServiceType, MemoryManagementService::MessageType::CheckRawMemoryData, Message::TC, 1); rcvPack.appendEnum8(MemoryManagementService::MemoryID::EXTERNAL); // Memory ID - rcvPack.appendUint16(2); // Iteration count - rcvPack.appendUint64(reinterpret_cast<uint64_t>(data)); // Start address + rcvPack.appendUint16(2); // Iteration count + rcvPack.appendUint64(reinterpret_cast<uint64_t>(data)); // Start address rcvPack.appendUint16(2); rcvPack.appendUint64(reinterpret_cast<uint64_t>(data + 1)); // Start address rcvPack.appendUint16(1); @@ -245,7 +245,7 @@ int main() { eventActionService.addEventActionDefinitions(eventActionDefinition7); std::cout << "Status should be 000:"; - for (auto& element : eventActionService.eventActionDefinitionMap) { + for (auto& element: eventActionService.eventActionDefinitionMap) { std::cout << element.second.enabled; } @@ -264,7 +264,7 @@ int main() { eventActionService.enableEventActionDefinitions(eventActionDefinition5); std::cout << "\nStatus should be 111:"; - for (auto& element : eventActionService.eventActionDefinitionMap) { + for (auto& element: eventActionService.eventActionDefinitionMap) { std::cout << element.second.enabled; } @@ -282,7 +282,7 @@ int main() { eventActionDefinition3.appendUint16(4); eventActionService.disableEventActionDefinitions(eventActionDefinition3); std::cout << "Status should be 000:"; - for (auto& element : eventActionService.eventActionDefinitionMap) { + for (auto& element: eventActionService.eventActionDefinitionMap) { std::cout << element.second.enabled; } @@ -327,7 +327,7 @@ int main() { TimeBasedSchedulingService::MessageType::EnableTimeBasedScheduleExecutionFunction, Message::TC, 1); Message testMessage1(6, 5, Message::TC, 1); Message testMessage2(4, 5, Message::TC, 1); - testMessage1.appendUint16(4253); // Append dummy data + testMessage1.appendUint16(4253); // Append dummy data testMessage2.appendUint16(45667); // Append dummy data timeBasedSchedulingService.enableScheduleExecution(receivedMsg); // Enable the schedule diff --git a/src/ServicePool.cpp b/src/ServicePool.cpp index b8e7c963..e3698947 100644 --- a/src/ServicePool.cpp +++ b/src/ServicePool.cpp @@ -1,6 +1,6 @@ #include "ServicePool.hpp" -ServicePool Services = ServicePool(); +ServicePool Services = ServicePool(); // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) void ServicePool::reset() { // Call the destructor @@ -15,7 +15,7 @@ void ServicePool::reset() { uint16_t ServicePool::getAndUpdateMessageTypeCounter(uint8_t serviceType, uint8_t messageType) { uint16_t key = (serviceType << 8U) | messageType; // Create the key of the map - return (messageTypeCounter[key])++; // Fetch and increase the value + return (messageTypeCounter[key])++; // Fetch and increase the value } uint16_t ServicePool::getAndUpdatePacketSequenceCounter() { diff --git a/src/Services/FunctionManagementService.cpp b/src/Services/FunctionManagementService.cpp index 15361ff6..703c7187 100644 --- a/src/Services/FunctionManagementService.cpp +++ b/src/Services/FunctionManagementService.cpp @@ -28,15 +28,14 @@ void FunctionManagementService::call(Message& msg) { // locate the appropriate function pointer String<ECSSFunctionNameLength> name(funcName); FunctionMap::iterator iter = funcPtrIndex.find(name); - void (*selected)(String<ECSSFunctionMaxArgLength>); - if (iter != funcPtrIndex.end()) { - selected = *iter->second; - } else { + if (iter == funcPtrIndex.end()) { ErrorHandler::reportError(msg, ErrorHandler::ExecutionStartErrorType::UnknownExecutionStartError); return; } + auto selected = *iter->second; + // execute the function if there are no obvious flaws (defined in the standard, pg.158) selected(funcArgs); } diff --git a/src/Services/MemoryManagementService.cpp b/src/Services/MemoryManagementService.cpp index c754aebf..ce0d5f9e 100644 --- a/src/Services/MemoryManagementService.cpp +++ b/src/Services/MemoryManagementService.cpp @@ -12,7 +12,7 @@ MemoryManagementService::MemoryManagementService() : rawDataMemorySubservice(*th MemoryManagementService::RawDataMemoryManagement::RawDataMemoryManagement(MemoryManagementService& parent) : mainService(parent) {} -void MemoryManagementService::RawDataMemoryManagement::loadRawData(Message& request) { +void MemoryManagementService::loadRawData(Message& request) { /** * Bear in mind that there is currently no error checking for invalid parameters. * A future version will include error checking and the corresponding error report/notification, @@ -24,42 +24,45 @@ void MemoryManagementService::RawDataMemoryManagement::loadRawData(Message& requ request.assertTC(MemoryManagementService::ServiceType, MemoryManagementService::MessageType::LoadRawMemoryDataAreas); auto memoryID = MemoryManagementService::MemoryID(request.readEnum8()); - if (mainService.memoryIdValidator(MemoryManagementService::MemoryID(memoryID))) { - uint8_t readData[ECSSMaxStringSize]; - uint16_t iterationCount = request.readUint16(); + if (!memoryIdValidator(MemoryManagementService::MemoryID(memoryID))) { + // TODO: Send a failed start of execution + return; + } - if (memoryID == MemoryManagementService::MemoryID::FLASH) { - // TODO: Define FLASH specific access code when we transfer to embedded - } else { - for (std::size_t j = 0; j < iterationCount; j++) { - uint64_t startAddress = request.readUint64(); - uint16_t dataLength = request.readOctetString(readData); - uint16_t checksum = request.readBits(16); - - if (mainService.dataValidator(readData, checksum, dataLength)) { - if (mainService.addressValidator(memoryID, startAddress) && - mainService.addressValidator(memoryID, startAddress + dataLength)) { - for (std::size_t i = 0; i < dataLength; i++) { - *(reinterpret_cast<uint8_t*>(startAddress) + i) = readData[i]; - } - - for (std::size_t i = 0; i < dataLength; i++) { - readData[i] = *(reinterpret_cast<uint8_t*>(startAddress) + i); - } - if (checksum != CRCHelper::calculateCRC(readData, dataLength)) { - ErrorHandler::reportError(request, ErrorHandler::ChecksumFailed); - } - } else { - ErrorHandler::reportError(request, ErrorHandler::ChecksumFailed); - } - } else { - ErrorHandler::reportError(request, ErrorHandler::ChecksumFailed); - continue; - } + uint8_t readData[ECSSMaxStringSize]; + uint16_t iterationCount = request.readUint16(); + + if (memoryID == MemoryManagementService::MemoryID::FLASH) { + // TODO: Define FLASH specific access code when we transfer to embedded + } else { + for (std::size_t j = 0; j < iterationCount; j++) { + uint64_t startAddress = request.readUint64(); + uint16_t dataLength = request.readOctetString(readData); + uint16_t checksum = request.readBits(16); + + if (!dataValidator(readData, checksum, dataLength)) { + ErrorHandler::reportError(request, ErrorHandler::ChecksumFailed); + continue; + } + + if (!addressValidator(memoryID, startAddress) || + !addressValidator(memoryID, startAddress + dataLength)) { + ErrorHandler::reportError(request, ErrorHandler::ChecksumFailed); + continue; + } + + for (std::size_t i = 0; i < dataLength; i++) { + *(reinterpret_cast<uint8_t*>(startAddress) + i) = readData[i]; + } + + for (std::size_t i = 0; i < dataLength; i++) { + readData[i] = *(reinterpret_cast<uint8_t*>(startAddress) + i); + } + + if (checksum != CRCHelper::calculateCRC(readData, dataLength)) { + ErrorHandler::reportError(request, ErrorHandler::ChecksumFailed); } } - } else { - // TODO: Send a failed start of execution } } @@ -69,7 +72,7 @@ void MemoryManagementService::RawDataMemoryManagement::dumpRawData(Message& requ Message report = mainService.createTM(MemoryManagementService::MessageType::DumpRawMemoryDataReport); uint8_t memoryID = request.readEnum8(); - if (mainService.memoryIdValidator(MemoryManagementService::MemoryID(memoryID))) { + if (memoryIdValidator(MemoryManagementService::MemoryID(memoryID))) { uint8_t readData[ECSSMaxStringSize]; uint16_t iterationCount = request.readUint16(); @@ -80,8 +83,8 @@ void MemoryManagementService::RawDataMemoryManagement::dumpRawData(Message& requ uint64_t startAddress = request.readUint64(); uint16_t readLength = request.readUint16(); - if (mainService.addressValidator(MemoryManagementService::MemoryID(memoryID), startAddress) && - mainService.addressValidator(MemoryManagementService::MemoryID(memoryID), startAddress + readLength)) { + if (addressValidator(MemoryManagementService::MemoryID(memoryID), startAddress) && + addressValidator(MemoryManagementService::MemoryID(memoryID), startAddress + readLength)) { for (std::size_t i = 0; i < readLength; i++) { readData[i] = *(reinterpret_cast<uint8_t*>(startAddress) + i); } @@ -107,7 +110,7 @@ void MemoryManagementService::RawDataMemoryManagement::checkRawData(Message& req Message report = mainService.createTM(MemoryManagementService::MessageType::CheckRawMemoryDataReport); uint8_t memoryID = request.readEnum8(); - if (mainService.memoryIdValidator(MemoryManagementService::MemoryID(memoryID))) { + if (memoryIdValidator(MemoryManagementService::MemoryID(memoryID))) { uint8_t readData[ECSSMaxStringSize]; uint16_t iterationCount = request.readUint16(); @@ -118,8 +121,8 @@ void MemoryManagementService::RawDataMemoryManagement::checkRawData(Message& req uint64_t startAddress = request.readUint64(); uint16_t readLength = request.readUint16(); - if (mainService.addressValidator(MemoryManagementService::MemoryID(memoryID), startAddress) && - mainService.addressValidator(MemoryManagementService::MemoryID(memoryID), startAddress + readLength)) { + if (addressValidator(MemoryManagementService::MemoryID(memoryID), startAddress) && + addressValidator(MemoryManagementService::MemoryID(memoryID), startAddress + readLength)) { for (std::size_t i = 0; i < readLength; i++) { readData[i] = *(reinterpret_cast<uint8_t*>(startAddress) + i); } @@ -199,7 +202,7 @@ inline bool MemoryManagementService::dataValidator(const uint8_t* data, uint16_t void MemoryManagementService::execute(Message& message) { switch (message.messageType) { case LoadRawMemoryDataAreas: - rawDataMemorySubservice.loadRawData(message); + loadRawData(message); break; case DumpRawMemoryData: rawDataMemorySubservice.dumpRawData(message); diff --git a/src/Services/ParameterService.cpp b/src/Services/ParameterService.cpp index 140eca27..552c309e 100644 --- a/src/Services/ParameterService.cpp +++ b/src/Services/ParameterService.cpp @@ -34,7 +34,7 @@ void ParameterService::reportParameters(Message& paramIds) { storeMessage(parameterReport); } -void ParameterService::setParameters(Message& newParamValues) { +void ParameterService::setParameters(Message& newParamValues) const { newParamValues.assertTC(ServiceType, MessageType::SetParameterValues); uint16_t numOfIds = newParamValues.readUint16(); diff --git a/src/Services/RealTimeForwardingControlService.cpp b/src/Services/RealTimeForwardingControlService.cpp index 315bfbe3..af7c3a04 100644 --- a/src/Services/RealTimeForwardingControlService.cpp +++ b/src/Services/RealTimeForwardingControlService.cpp @@ -2,14 +2,14 @@ #include <iostream> void RealTimeForwardingControlService::addAllReportsOfApplication(uint8_t applicationID) { - for (auto& service: AllMessageTypes::messagesOfService) { + for (const auto& service: AllMessageTypes::MessagesOfService) { uint8_t serviceType = service.first; addAllReportsOfService(applicationID, serviceType); } } void RealTimeForwardingControlService::addAllReportsOfService(uint8_t applicationID, uint8_t serviceType) { - for (auto& messageType: AllMessageTypes::messagesOfService[serviceType]) { + for (const auto& messageType: AllMessageTypes::MessagesOfService.at(serviceType)) { auto appServicePair = std::make_pair(applicationID, serviceType); applicationProcessConfiguration.definitions[appServicePair].push_back(messageType); } @@ -18,7 +18,7 @@ void RealTimeForwardingControlService::addAllReportsOfService(uint8_t applicatio uint8_t RealTimeForwardingControlService::countServicesOfApplication(uint8_t applicationID) { uint8_t serviceCounter = 0; for (auto& definition: applicationProcessConfiguration.definitions) { - auto& pair = definition.first; + const auto& pair = definition.first; if (pair.first == applicationID) { serviceCounter++; } @@ -79,7 +79,7 @@ bool RealTimeForwardingControlService::checkService(Message& request, uint8_t ap bool RealTimeForwardingControlService::maxReportTypesReached(Message& request, uint8_t applicationID, uint8_t serviceType) { - if (countReportsOfService(applicationID, serviceType) >= AllMessageTypes::messagesOfService[serviceType].size()) { + if (countReportsOfService(applicationID, serviceType) >= AllMessageTypes::MessagesOfService.at(serviceType).size()) { ErrorHandler::reportError(request, ErrorHandler::ExecutionStartErrorType::MaxReportTypesReached); return true; } @@ -88,11 +88,8 @@ bool RealTimeForwardingControlService::maxReportTypesReached(Message& request, u bool RealTimeForwardingControlService::checkMessage(Message& request, uint8_t applicationID, uint8_t serviceType, uint8_t messageType) { - if (maxReportTypesReached(request, applicationID, serviceType) or - reportExistsInAppProcessConfiguration(applicationID, serviceType, messageType)) { - return false; - } - return true; + return !maxReportTypesReached(request, applicationID, serviceType) and + !reportExistsInAppProcessConfiguration(applicationID, serviceType, messageType); } bool RealTimeForwardingControlService::reportExistsInAppProcessConfiguration(uint8_t applicationID, uint8_t serviceType, diff --git a/src/Services/StorageAndRetrievalService.cpp b/src/Services/StorageAndRetrievalService.cpp index d2eee2a3..b218ad84 100644 --- a/src/Services/StorageAndRetrievalService.cpp +++ b/src/Services/StorageAndRetrievalService.cpp @@ -25,7 +25,7 @@ void StorageAndRetrievalService::copyFromTagToTag(Message& request) { return; } - for (auto& packet : packetStores[fromPacketStoreId].storedTelemetryPackets) { + for (auto& packet: packetStores[fromPacketStoreId].storedTelemetryPackets) { if (packet.first < startTime) { continue; } @@ -46,7 +46,7 @@ void StorageAndRetrievalService::copyAfterTimeTag(Message& request) { return; } - for (auto& packet : packetStores[fromPacketStoreId].storedTelemetryPackets) { + for (auto& packet: packetStores[fromPacketStoreId].storedTelemetryPackets) { if (packet.first < startTime) { continue; } @@ -64,7 +64,7 @@ void StorageAndRetrievalService::copyBeforeTimeTag(Message& request) { return; } - for (auto& packet : packetStores[fromPacketStoreId].storedTelemetryPackets) { + for (auto& packet: packetStores[fromPacketStoreId].storedTelemetryPackets) { if (packet.first > endTime) { break; } @@ -159,7 +159,7 @@ void StorageAndRetrievalService::createContentSummary(Message& report, report.appendUint32(packetStores[packetStoreId].openRetrievalStartTimeTag); - auto filledPercentage1 = static_cast<uint16_t>(packetStores[packetStoreId].storedTelemetryPackets.size() * 100.0f / + auto filledPercentage1 = static_cast<uint16_t>(static_cast<float>(packetStores[packetStoreId].storedTelemetryPackets.size()) * 100 / ECSSMaxPacketStoreSize); report.appendUint16(filledPercentage1); @@ -169,7 +169,7 @@ void StorageAndRetrievalService::createContentSummary(Message& report, std::end(packetStores[packetStoreId].storedTelemetryPackets), [this, &packetStoreId](auto packet) { return packet.first >= packetStores[packetStoreId].openRetrievalStartTimeTag; }); - auto filledPercentage2 = static_cast<uint16_t>(numOfPacketsToBeTransferred * 100.0f / ECSSMaxPacketStoreSize); + auto filledPercentage2 = static_cast<uint16_t>(static_cast<float>(numOfPacketsToBeTransferred) * 100 / ECSSMaxPacketStoreSize); report.appendUint16(filledPercentage2); } @@ -217,9 +217,7 @@ uint16_t StorageAndRetrievalService::currentNumberOfPacketStores() { PacketStore& StorageAndRetrievalService::getPacketStore(const String<ECSSPacketStoreIdSize>& packetStoreId) { auto packetStore = packetStores.find(packetStoreId); - if (packetStore == packetStores.end()) { - assert(ErrorHandler::ExecutionStartErrorType::NonExistingPacketStore); - } + ASSERT_INTERNAL(packetStore != packetStores.end(), ErrorHandler::InternalErrorType::ElementNotInArray); return packetStore->second; } @@ -231,7 +229,7 @@ void StorageAndRetrievalService::executeOnPacketStores(Message& request, const std::function<void(PacketStore&)>& function) { uint16_t numOfPacketStores = request.readUint16(); if (numOfPacketStores == 0) { - for (auto& packetStore : packetStores) { + for (auto& packetStore: packetStores) { function(packetStore.second); } return; @@ -296,7 +294,7 @@ void StorageAndRetrievalService::deletePacketStoreContent(Message& request) { uint16_t numOfPacketStores = request.readUint16(); if (numOfPacketStores == 0) { - for (auto& packetStore : packetStores) { + for (auto& packetStore: packetStores) { if (packetStore.second.byTimeRangeRetrievalStatus) { ErrorHandler::reportError( request, ErrorHandler::ExecutionStartErrorType::SetPacketStoreWithByTimeRangeRetrieval); @@ -339,7 +337,7 @@ void StorageAndRetrievalService::packetStoreContentSummaryReport(Message& reques if (numOfPacketStores == 0) { report.appendUint16(packetStores.size()); - for (auto& packetStore : packetStores) { + for (auto& packetStore: packetStores) { auto packetStoreId = packetStore.first; report.appendString(packetStoreId); createContentSummary(report, packetStoreId); @@ -379,7 +377,7 @@ void StorageAndRetrievalService::changeOpenRetrievalStartTimeTag(Message& reques */ uint16_t numOfPacketStores = request.readUint16(); if (numOfPacketStores == 0) { - for (auto& packetStore : packetStores) { + for (auto& packetStore: packetStores) { if (packetStore.second.openRetrievalStatus == PacketStore::InProgress) { ErrorHandler::reportError( request, ErrorHandler::ExecutionStartErrorType::SetPacketStoreWithOpenRetrievalInProgress); @@ -410,7 +408,7 @@ void StorageAndRetrievalService::resumeOpenRetrievalOfPacketStores(Message& requ uint16_t numOfPacketStores = request.readUint16(); if (numOfPacketStores == 0) { - for (auto& packetStore : packetStores) { + for (auto& packetStore: packetStores) { if (packetStore.second.byTimeRangeRetrievalStatus) { ErrorHandler::reportError( request, ErrorHandler::ExecutionStartErrorType::SetPacketStoreWithByTimeRangeRetrieval); @@ -441,7 +439,7 @@ void StorageAndRetrievalService::suspendOpenRetrievalOfPacketStores(Message& req uint16_t numOfPacketStores = request.readUint16(); if (numOfPacketStores == 0) { - for (auto& packetStore : packetStores) { + for (auto& packetStore: packetStores) { packetStore.second.openRetrievalStatus = PacketStore::Suspended; } return; @@ -461,7 +459,7 @@ void StorageAndRetrievalService::abortByTimeRangeRetrieval(Message& request) { uint16_t numOfPacketStores = request.readUint16(); if (numOfPacketStores == 0) { - for (auto& packetStore : packetStores) { + for (auto& packetStore: packetStores) { packetStore.second.byTimeRangeRetrievalStatus = false; } return; @@ -481,7 +479,7 @@ void StorageAndRetrievalService::packetStoresStatusReport(Message& request) { Message report = createTM(PacketStoresStatusReport); report.appendUint16(packetStores.size()); - for (auto& packetStore : packetStores) { + for (auto& packetStore: packetStores) { auto packetStoreId = packetStore.first; report.appendString(packetStoreId); report.appendBoolean(packetStore.second.storageStatus); @@ -533,9 +531,9 @@ void StorageAndRetrievalService::deletePacketStores(Message& request) { uint16_t numOfPacketStores = request.readUint16(); if (numOfPacketStores == 0) { - int numOfPacketStoresToDelete = 0; + uint16_t numOfPacketStoresToDelete = 0; etl::string<ECSSPacketStoreIdSize> packetStoresToDelete[ECSSMaxPacketStores]; - for (auto& packetStore : packetStores) { + for (auto& packetStore: packetStores) { if (packetStore.second.storageStatus) { ErrorHandler::reportError( request, ErrorHandler::ExecutionStartErrorType::DeletionOfPacketStoreWithStorageStatusEnabled); @@ -596,7 +594,7 @@ void StorageAndRetrievalService::packetStoreConfigurationReport(Message& request Message report = createTM(PacketStoreConfigurationReport); report.appendUint16(packetStores.size()); - for (auto& packetStore : packetStores) { + for (auto& packetStore: packetStores) { auto packetStoreId = packetStore.first; report.appendString(packetStoreId); report.appendUint16(packetStore.second.sizeInBytes); diff --git a/test/MessageTests.cpp b/test/MessageTests.cpp index 27ff3886..f8d8c488 100644 --- a/test/MessageTests.cpp +++ b/test/MessageTests.cpp @@ -94,15 +94,13 @@ TEST_CASE("Requirement 7.3.3 (Enumerated)", "[message][ecss]") { TEST_CASE("Requirement 7.3.4 (Unsigned integer)", "[message][ecss]") { Message message(0, 0, Message::TC, 0); - message.append<char>(110); message.append<uint8_t>(230); message.append<uint16_t>(15933); message.append<uint32_t>(2000001); message.append<uint64_t>(12446744073709551615ULL); - REQUIRE(message.dataSize == 1 + 1 + 2 + 4 + 8); + REQUIRE(message.dataSize == 1 + 2 + 4 + 8); - CHECK(message.read<char>() == 110); CHECK(message.read<uint8_t>() == 230); CHECK(message.read<uint16_t>() == 15933); CHECK(message.read<uint32_t>() == 2000001); @@ -115,8 +113,8 @@ TEST_CASE("Requirement 7.3.4 (Unsigned integer)", "[message][ecss]") { * processors store data in little endian format. As a result, special care needs to be * taken for compliance. */ - CHECK(message.data[2] == 0x3e); - CHECK(message.data[3] == 0x3d); + CHECK(message.data[1] == 0x3e); + CHECK(message.data[2] == 0x3d); } } diff --git a/test/Services/RealTimeForwardingControl.cpp b/test/Services/RealTimeForwardingControl.cpp index e98ae59d..15b11ce5 100644 --- a/test/Services/RealTimeForwardingControl.cpp +++ b/test/Services/RealTimeForwardingControl.cpp @@ -5,7 +5,7 @@ #include "Services/RealTimeForwardingControlService.hpp" #include "catch2/catch_all.hpp" -RealTimeForwardingControlService& realTimeForwarding = Services.realTimeForwarding; +static RealTimeForwardingControlService& realTimeForwarding = Services.realTimeForwarding; uint8_t applications[] = {1}; uint8_t services[] = {3, 5}; @@ -307,7 +307,7 @@ TEST_CASE("Add report types to the Application Process Configuration") { realTimeForwarding.controlledApplications.push_back(applicationID); validReportTypes(request); - for (auto message: AllMessageTypes::messagesOfService[serviceType]) { + for (auto& message: AllMessageTypes::MessagesOfService.at(serviceType)) { realTimeForwarding.applicationProcessConfiguration.definitions[std::make_pair(applicationID, serviceType)] .push_back(message); } @@ -319,7 +319,7 @@ TEST_CASE("Add report types to the Application Process Configuration") { 2); REQUIRE( realTimeForwarding.applicationProcessConfiguration.definitions[std::make_pair(applicationID, serviceType)] - .size() == AllMessageTypes::messagesOfService[serviceType].size()); + .size() == AllMessageTypes::MessagesOfService.at(serviceType).size()); resetAppProcessConfiguration(); ServiceTests::reset(); @@ -345,8 +345,8 @@ TEST_CASE("Add report types to the Application Process Configuration") { REQUIRE(applicationProcessConfig.definitions[appServicePair1].empty()); REQUIRE(applicationProcessConfig.definitions[appServicePair2].empty()); - auto numOfMessages1 = AllMessageTypes::messagesOfService[serviceType1].size(); - auto numOfMessages2 = AllMessageTypes::messagesOfService[serviceType2].size(); + auto numOfMessages1 = AllMessageTypes::MessagesOfService.at(serviceType1).size(); + auto numOfMessages2 = AllMessageTypes::MessagesOfService.at(serviceType2).size(); for (uint8_t i = 0; i < numOfMessages1 - 1; i++) { applicationProcessConfig.definitions[appServicePair1].push_back(i); @@ -450,7 +450,7 @@ TEST_CASE("Add report types to the Application Process Configuration") { auto& applicationProcesses = realTimeForwarding.applicationProcessConfiguration.definitions; for (auto serviceType: services) { REQUIRE(applicationProcesses[std::make_pair(applicationID1, serviceType)].size() == - AllMessageTypes::messagesOfService[serviceType].size()); + AllMessageTypes::MessagesOfService.at(serviceType).size()); } resetAppProcessConfiguration(); @@ -491,11 +491,11 @@ TEST_CASE("Add report types to the Application Process Configuration") { for (auto& serviceType: allServices) { REQUIRE(definitions[std::make_pair(applicationID1, serviceType)].size() == - AllMessageTypes::messagesOfService[serviceType].size()); + AllMessageTypes::MessagesOfService.at(serviceType).size()); } for (auto& serviceType: services) { REQUIRE(definitions[std::make_pair(applicationID2, serviceType)].size() == - AllMessageTypes::messagesOfService[serviceType].size()); + AllMessageTypes::MessagesOfService.at(serviceType).size()); } resetAppProcessConfiguration(); @@ -520,7 +520,7 @@ TEST_CASE("Add report types to the Application Process Configuration") { for (auto serviceType: allServices) { REQUIRE(std::equal(definitions[std::make_pair(applicationID1, serviceType)].begin(), definitions[std::make_pair(applicationID1, serviceType)].end(), - AllMessageTypes::messagesOfService[serviceType].begin())); + AllMessageTypes::MessagesOfService.at(serviceType).begin())); } resetAppProcessConfiguration(); -- GitLab