diff --git a/inc/ECSS_Definitions.hpp b/inc/ECSS_Definitions.hpp index 40131723d05152ffa5278fe197378ad03d06027b..a8523462496cad6652af9cb1c8eeb05d02fee2f4 100644 --- a/inc/ECSS_Definitions.hpp +++ b/inc/ECSS_Definitions.hpp @@ -142,7 +142,7 @@ #define LOGGER_MAX_MESSAGE_SIZE 512 /** - * @brief Size of the map holding the Parameter objects for the ST[20] parameter service + * @brief Size of the array holding the Parameter objects for the ST[20] parameter service */ #define ECSS_PARAMETER_COUNT 3 #endif // ECSS_SERVICES_ECSS_DEFINITIONS_H diff --git a/inc/ErrorHandler.hpp b/inc/ErrorHandler.hpp index 407a6bf1b05707d58331f38c9809405386185125..0c07fe079c4f98002cbcc7d17c56a17e93e521d2 100644 --- a/inc/ErrorHandler.hpp +++ b/inc/ErrorHandler.hpp @@ -102,15 +102,7 @@ public: /** * Cannot parse a Message, because there is an error in its secondary header */ - UnacceptableMessage = 5, - /** - * Attempt to change the value of a non existing parameter (ST[20]) - */ - InvalidParameterId = 6, - /** - * Attempt to access a non existing parameter (ST[20]) - */ - GetNonExistingParameter = 7, + UnacceptableMessage = 5 }; /** @@ -143,7 +135,14 @@ public: EventActionUnknownEventActionDefinitionIDError = 4, SubServiceExecutionStartError = 5, InstructionExecutionStartError = 6, - + /** + * Attempt to change the value of a non existing parameter (ST[20]) + */ + SetNonExistingParameter = 7, + /** + * Attempt to access a non existing parameter (ST[20]) + */ + GetNonExistingParameter = 8 }; /** diff --git a/inc/Platform/x86/Parameters/SystemParameters.hpp b/inc/Platform/x86/Parameters/SystemParameters.hpp index c0b21c2cf200712d0e361dc32fc1deef054e330f..1e58687e46c5d39b4d374fcc68cb4d2aa8b8f564 100644 --- a/inc/Platform/x86/Parameters/SystemParameters.hpp +++ b/inc/Platform/x86/Parameters/SystemParameters.hpp @@ -19,8 +19,13 @@ public: Parameter<uint8_t> parameter1 = Parameter<uint8_t>(3); Parameter<uint16_t> parameter2 = Parameter<uint16_t>(7); Parameter<uint32_t> parameter3 = Parameter<uint32_t>(10); + /** + * The key of the array is the ID of the parameter as specified in PUS + */ etl::array<std::reference_wrapper<ParameterBase>, ECSS_PARAMETER_COUNT> parametersArray = { - parameter1, parameter2, parameter3}; + parameter1, parameter2, parameter3 + }; + SystemParameters() = default; }; diff --git a/inc/Services/Parameter.hpp b/inc/Services/Parameter.hpp index 0d47847a44cacc356618ae031e2d86e5ab482e5b..932d450f5389fb6cb67c9979f52106aef00d9b4c 100644 --- a/inc/Services/Parameter.hpp +++ b/inc/Services/Parameter.hpp @@ -19,8 +19,7 @@ * common data type used to create any pointers to \ref Parameter objects, as well as * virtual functions for accessing the parameter's data part, and * 2) a templated \ref Parameter used to store any type-specific parameter information, - * such as the actual data field where the parameter's value will be stored and any pointers - * to suitable functions that will be responsible for updating the parameter's value. + * such as the actual data field where the parameter's value will be stored. * * @section Architecture Rationale * The ST[20] Parameter service is implemented with the need of arbitrary type storage @@ -39,7 +38,8 @@ public: /** * Implementation of a parameter containing its value. See \ref ParameterBase for more information. - * @tparam DataType + * @tparam DataType The type of the Parameter value. This is the type used for transmission and reception + * as per the PUS. */ template <typename DataType> class Parameter : public ParameterBase { @@ -65,14 +65,14 @@ public: }; template<> inline void Parameter<uint8_t>::setValueFromMessage(Message& message) { - this->currentValue = message.readUint8(); + currentValue = message.readUint8(); } template<> inline void Parameter<uint16_t>::setValueFromMessage(Message& message) { - this->currentValue = message.readUint16(); + currentValue = message.readUint16(); } template<> inline void Parameter<uint32_t>::setValueFromMessage(Message& message) { - this->currentValue = message.readUint32(); + currentValue = message.readUint32(); } template<> inline void Parameter<uint8_t>::appendValueToMessage(Message& message) { diff --git a/inc/Services/ParameterService.hpp b/inc/Services/ParameterService.hpp index 61286e031282a30cce9c73a3bafaa563273168c7..f347f651b1dda1e1a8e7073f5d19528ffeba7f2e 100644 --- a/inc/Services/ParameterService.hpp +++ b/inc/Services/ParameterService.hpp @@ -5,7 +5,6 @@ #include "Service.hpp" #include "ErrorHandler.hpp" #include "Parameter.hpp" -#include "etl/vector.h" #include "Parameters/SystemParameters.hpp" /** diff --git a/src/Services/ParameterService.cpp b/src/Services/ParameterService.cpp index a9a395eb257037e95fb0111dbbbc52fb7a06cd2c..3557f0ed7c66133dd50d075d6760649d97a4c7ef 100644 --- a/src/Services/ParameterService.cpp +++ b/src/Services/ParameterService.cpp @@ -17,13 +17,14 @@ void ParameterService::reportParameters(Message& paramIds) { uint16_t numOfIds = paramIds.readUint16(); uint16_t numberOfValidIds = 0; - for (uint16_t counter = 0; counter < numOfIds; counter++) { + for (uint16_t i = 0; i < numOfIds; i++) { if (paramIds.readUint16() < systemParameters.parametersArray.size()) { numberOfValidIds++; } } parameterReport.appendUint16(numberOfValidIds); paramIds.resetRead(); + numOfIds = paramIds.readUint16(); for (uint16_t i = 0; i < numOfIds; i++) { uint16_t currId = paramIds.readUint16(); @@ -54,8 +55,8 @@ void ParameterService::setParameters(Message& newParamValues) { if (currId < systemParameters.parametersArray.size()) { systemParameters.parametersArray[currId].get().setValueFromMessage(newParamValues); } else { - ErrorHandler::reportError(newParamValues, ErrorHandler::InvalidParameterId); - break; + ErrorHandler::reportError(newParamValues, ErrorHandler::SetNonExistingParameter); + break; // Setting next parameters is impossible, since the size of value to be read is unknown } } } diff --git a/test/Services/Parameter.cpp b/test/Services/Parameter.cpp index 12938fa0e65ba43c2c0256da9a007438b7e5bcc3..6cf629ee627eb1ced39be89518618663425e0240 100644 --- a/test/Services/Parameter.cpp +++ b/test/Services/Parameter.cpp @@ -1,10 +1,41 @@ #include "catch2/catch.hpp" -#include "Services/ParameterService.hpp" #include "Services/Parameter.hpp" #include "Message.hpp" -#include "ServiceTests.hpp" +TEST_CASE("Parameter Append") { + SECTION("Check correct appending") { + Message request = Message(20, 1, Message::TC, 1); + Parameter<uint8_t> parameter1 = Parameter<uint8_t>(1); + Parameter<uint16_t> parameter2 = Parameter<uint16_t>(500); + Parameter<uint32_t> parameter3 = Parameter<uint32_t>(70000); -TEST_CASE("Parameter Functions") { + parameter1.appendValueToMessage(request); + parameter2.appendValueToMessage(request); + parameter3.appendValueToMessage(request); -} \ No newline at end of file + CHECK(request.readUint8() == 1); + CHECK(request.readUint16() == 500); + CHECK(request.readUint32() == 70000); + } +} + +TEST_CASE("Parameter Set") { + SECTION("Check correct setting") { + Message request = Message(20, 1, Message::TC, 1); + Parameter<uint8_t> parameter1 = Parameter<uint8_t>(1); + Parameter<uint16_t> parameter2 = Parameter<uint16_t>(500); + Parameter<uint32_t> parameter3 = Parameter<uint32_t>(70000); + + request.appendUint8(10); + request.appendUint16(1000); + request.appendUint32(70001); + + parameter1.setValueFromMessage(request); + parameter2.setValueFromMessage(request); + parameter3.setValueFromMessage(request); + + CHECK(parameter1.getValue() == 10); + CHECK(parameter2.getValue() == 1000); + CHECK(parameter3.getValue() == 70001); + } +} diff --git a/test/Services/ParameterService.cpp b/test/Services/ParameterService.cpp index 71aa55ed57632209193202d9c586587d269bff06..f29143633e2b692fb55226330627831f094044bc 100644 --- a/test/Services/ParameterService.cpp +++ b/test/Services/ParameterService.cpp @@ -2,6 +2,12 @@ #include "Message.hpp" #include "ServiceTests.hpp" +static void resetParameterValues() { + systemParameters.parameter1.setValue(3); + systemParameters.parameter2.setValue(7); + systemParameters.parameter3.setValue(10); +}; + TEST_CASE("Parameter Report Subservice") { SECTION("All requested parameters invalid") { Message request = Message(20, 1, Message::TC, 1); @@ -11,12 +17,8 @@ TEST_CASE("Parameter Report Subservice") { request.appendUint16(65535); MessageParser::execute(request); - CHECK(ServiceTests::get(0).serviceType == 1); - CHECK(ServiceTests::get(0).messageType == 2); - CHECK(ServiceTests::get(1).serviceType == 1); - CHECK(ServiceTests::get(1).messageType == 2); - CHECK(ServiceTests::get(2).serviceType == 1); - CHECK(ServiceTests::get(2).messageType == 2); + CHECK(ServiceTests::countThrownErrors(ErrorHandler::GetNonExistingParameter) == 3); + CHECK(ServiceTests::count() == 4); Message report = ServiceTests::get(3); CHECK(report.serviceType == 20); @@ -31,12 +33,12 @@ TEST_CASE("Parameter Report Subservice") { Message request = Message(20, 1, Message::TC, 1); request.appendUint16(3); request.appendUint16(1); - request.appendUint16(2); request.appendUint16(10000); + request.appendUint16(2); MessageParser::execute(request); - CHECK(ServiceTests::get(0).serviceType == 1); - CHECK(ServiceTests::get(0).messageType == 2); + CHECK(ServiceTests::countThrownErrors(ErrorHandler::GetNonExistingParameter) == 1); + CHECK(ServiceTests::count() == 2); Message report = ServiceTests::get(1); CHECK(report.serviceType == 20); @@ -74,20 +76,10 @@ TEST_CASE("Parameter Report Subservice") { ServiceTests::reset(); Services.reset(); } - - SECTION("Wrong Message Type Handling Test") { - Message falseRequest(62, 3, Message::TM, 1); // a totally wrong message - - MessageParser::execute(falseRequest); - CHECK(ServiceTests::thrownError(ErrorHandler::InternalErrorType::OtherMessageType)); - - ServiceTests::reset(); - Services.reset(); - } } TEST_CASE("Parameter Setting Subservice") { - SECTION("All parameter ids are invalid") { + SECTION("All parameter IDs are invalid") { Message request = Message(20, 3, Message::TC, 1); request.appendUint16(3); request.appendUint16(54432); @@ -98,8 +90,8 @@ TEST_CASE("Parameter Setting Subservice") { request.appendUint16(1); MessageParser::execute(request); - CHECK(ServiceTests::get(0).serviceType == 1); - CHECK(ServiceTests::get(0).messageType == 2); // Just one error, because it should break after on invalid param + CHECK(ServiceTests::countThrownErrors(ErrorHandler::SetNonExistingParameter) == 1); + CHECK(ServiceTests::count() == 1); CHECK(systemParameters.parameter1.getValue() == 3); CHECK(systemParameters.parameter2.getValue() == 7); @@ -109,7 +101,7 @@ TEST_CASE("Parameter Setting Subservice") { Services.reset(); } - SECTION("The last parameter ids are invalid") { + SECTION("The last parameter ID is invalid") { Message request = Message(20, 3, Message::TC, 1); request.appendUint16(3); request.appendUint16(0); @@ -120,22 +112,20 @@ TEST_CASE("Parameter Setting Subservice") { request.appendUint16(1); MessageParser::execute(request); - CHECK(ServiceTests::get(0).serviceType == 1); - CHECK(ServiceTests::get(0).messageType == 2); + CHECK(ServiceTests::countThrownErrors(ErrorHandler::SetNonExistingParameter) == 1); + CHECK(ServiceTests::count() == 1); CHECK(systemParameters.parameter1.getValue() == 1); CHECK(systemParameters.parameter2.getValue() == 2); CHECK(systemParameters.parameter3.getValue() == 10); - systemParameters.parameter1.setValue(3); - systemParameters.parameter2.setValue(7); - systemParameters.parameter3.setValue(10); + resetParameterValues(); ServiceTests::reset(); Services.reset(); } - SECTION("The last parameter ids are invalid") { + SECTION("The middle parameter ID is invalid") { Message request = Message(20, 3, Message::TC, 1); request.appendUint16(3); request.appendUint16(0); @@ -146,22 +136,20 @@ TEST_CASE("Parameter Setting Subservice") { request.appendUint16(3); MessageParser::execute(request); - CHECK(ServiceTests::get(0).serviceType == 1); - CHECK(ServiceTests::get(0).messageType == 2); + CHECK(ServiceTests::countThrownErrors(ErrorHandler::SetNonExistingParameter) == 1); + CHECK(ServiceTests::count() == 1); CHECK(systemParameters.parameter1.getValue() == 1); CHECK(systemParameters.parameter2.getValue() == 7); CHECK(systemParameters.parameter3.getValue() == 10); - systemParameters.parameter1.setValue(3); - systemParameters.parameter2.setValue(7); - systemParameters.parameter3.setValue(10); + resetParameterValues(); ServiceTests::reset(); Services.reset(); } - SECTION("All ids are valid") { + SECTION("All IDs are valid") { Message request = Message(20, 3, Message::TC, 1); request.appendUint16(3); request.appendUint16(0); @@ -177,16 +165,37 @@ TEST_CASE("Parameter Setting Subservice") { CHECK(systemParameters.parameter2.getValue() == 2); CHECK(systemParameters.parameter3.getValue() == 3); - systemParameters.parameter1.setValue(3); - systemParameters.parameter2.setValue(7); - systemParameters.parameter3.setValue(10); + resetParameterValues(); + + ServiceTests::reset(); + Services.reset(); + } +} + +TEST_CASE("Wrong Messages") { + + SECTION("Wrong Service Type Handling Test for Report") { + Message falseRequest(62, 1, Message::TM, 1); + + MessageParser::execute(falseRequest); + CHECK(ServiceTests::thrownError(ErrorHandler::InternalErrorType::OtherMessageType)); + + ServiceTests::reset(); + Services.reset(); + } + + SECTION("Wrong Service Type Handling Test for Set") { + Message falseRequest(62, 3, Message::TM, 1); + + MessageParser::execute(falseRequest); + CHECK(ServiceTests::thrownError(ErrorHandler::InternalErrorType::OtherMessageType)); ServiceTests::reset(); Services.reset(); } - SECTION("Wrong Message Type Handling Test") { - Message falseRequest(62, 3, Message::TM, 1); // a totally wrong message + SECTION("Wrong Message Type") { + Message falseRequest(20, 127, Message::TM, 1); MessageParser::execute(falseRequest); CHECK(ServiceTests::thrownError(ErrorHandler::InternalErrorType::OtherMessageType)); diff --git a/test/Services/ServiceTests.hpp b/test/Services/ServiceTests.hpp index df769167d28843854d9d6919a7730c5015d5b2f7..d5b76b882b1f788b7da4267f5ab4d3f4841f5168 100644 --- a/test/Services/ServiceTests.hpp +++ b/test/Services/ServiceTests.hpp @@ -123,7 +123,7 @@ public: } /** - * Find if an error + * Find if an error exists * @tparam ErrorType An enumeration of ErrorHandler * @param errorType The error code of the Error, corresponding to the correct type as * specified in ErrorHandler @@ -136,6 +136,20 @@ public: return thrownErrors.find(std::make_pair(errorSource, errorType)) != thrownErrors.end(); } + /** + * Find the number of times that an error exists + * @tparam ErrorType An enumeration of ErrorHandler + * @param errorType The error code of the Error, corresponding to the correct type as + * specified in ErrorHandler + */ + template <typename ErrorType> + static int countThrownErrors(ErrorType errorType) { + ErrorHandler::ErrorSource errorSource = ErrorHandler::findErrorSource(errorType); + + expectingErrors = true; + + return thrownErrors.count(std::make_pair(errorSource, errorType)); + } }; #endif // ECSS_SERVICES_TESTS_SERVICES_SERVICETESTS_HPP