diff --git a/CMakeLists.txt b/CMakeLists.txt index fdfe6916d74e3510a0872ff0a7751a5fbf22acf9..50c416bbd0ee88fef12adb1af819d0675a4e5d51 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -31,7 +31,7 @@ add_library(common OBJECT src/Helpers/CRCHelper.cpp src/Helpers/TimeAndDate.cpp src/Helpers/TimeHelper.cpp - src/Parameters/Parameters.cpp + src/Platform/x86/Parameters/SystemParameters.cpp src/Services/EventReportService.cpp src/Services/MemoryManagementService.cpp src/Services/ParameterService.cpp diff --git a/inc/ECSS_Definitions.hpp b/inc/ECSS_Definitions.hpp index 8c3df99f433f6b04feeb3b4d820912f570754dfc..40131723d05152ffa5278fe197378ad03d06027b 100644 --- a/inc/ECSS_Definitions.hpp +++ b/inc/ECSS_Definitions.hpp @@ -144,11 +144,5 @@ /** * @brief Size of the map holding the Parameter objects for the ST[20] parameter service */ -#define ECSS_ST_20_MAX_PARAMETERS 5 - -/** - * @brief Maximum etl::string output length in bytes for each \ref Parameter of ST[20] - */ - -#define ECSS_ST_20_MAX_STRING_LENGTH 5 +#define ECSS_PARAMETER_COUNT 3 #endif // ECSS_SERVICES_ECSS_DEFINITIONS_H diff --git a/inc/ErrorHandler.hpp b/inc/ErrorHandler.hpp index b07148822fb61f38ee42bf264a76c99de9e0ad03..407a6bf1b05707d58331f38c9809405386185125 100644 --- a/inc/ErrorHandler.hpp +++ b/inc/ErrorHandler.hpp @@ -73,18 +73,10 @@ public: * Attempt to insert new element in a full map ST[08] */ MapFull = 10, - /** - * Attempt to access a non existing parameter (ST[20]) - */ - GetNonExistingParameter = 11, - /** - * Attempt to change the value of a non existing parameter (ST[20]) - */ - SetNonExistingParameter = 12, /** * A Message that is included within another message is too large */ - NestedMessageTooLarge = 13 + NestedMessageTooLarge = 11 }; /** @@ -111,6 +103,14 @@ 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, }; /** diff --git a/inc/Parameters/Parameters.hpp b/inc/Parameters/Parameters.hpp deleted file mode 100644 index f8651abb9a699a721fff9d174eeabecdf3378fe7..0000000000000000000000000000000000000000 --- a/inc/Parameters/Parameters.hpp +++ /dev/null @@ -1,17 +0,0 @@ -#include "Services/Parameter.hpp" -#include "etl/vector.h" - -class SystemParameters { -public: - Parameter<uint8_t> parameter1 = Parameter<uint8_t>(5); - Parameter<uint16_t> parameter2 = Parameter<uint16_t>(5); - Parameter<uint32_t> parameter3 = Parameter<uint32_t>(5); - etl::vector<std::reference_wrapper<ParameterBase>, ECSS_ST_20_MAX_PARAMETERS> parametersArray; - SystemParameters() { - parametersArray[0] = parameter1; - parametersArray[1] = parameter2; - parametersArray[2] = parameter3; - } -}; - -extern SystemParameters systemParameters; diff --git a/inc/Platform/x86/Parameters/SystemParameters.hpp b/inc/Platform/x86/Parameters/SystemParameters.hpp new file mode 100644 index 0000000000000000000000000000000000000000..c0b21c2cf200712d0e361dc32fc1deef054e330f --- /dev/null +++ b/inc/Platform/x86/Parameters/SystemParameters.hpp @@ -0,0 +1,27 @@ +#include "Services/Parameter.hpp" +#include "etl/vector.h" +/** + * @author Athanasios Theocharis <athatheoc@gmail.com> + */ + +/** + * This class was created for the purpose of initializing and storing explicitly + * parameters (that are instances of the \ref Parameter class). It stores all the parameters + * of the specific application. Different subsystems should have their own implementations of this class. + * The position of the parameter in the vector is also called the parameter ID. + * + * It is initialised statically. + * + * The parameters here are under the responsibility of \ref ParameterService. + */ +class SystemParameters { +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); + etl::array<std::reference_wrapper<ParameterBase>, ECSS_PARAMETER_COUNT> parametersArray = { + parameter1, parameter2, parameter3}; + SystemParameters() = default; +}; + +extern SystemParameters systemParameters; diff --git a/inc/Services/Parameter.hpp b/inc/Services/Parameter.hpp index 947e574dc191d27d58975c504a1b1d946686c287..0d47847a44cacc356618ae031e2d86e5ab482e5b 100644 --- a/inc/Services/Parameter.hpp +++ b/inc/Services/Parameter.hpp @@ -30,16 +30,17 @@ * would exclude from consideration constructs like etl::variant due to limitations on * the number of supported distinct types, a custom solution was needed. * Furthermore, the \ref ParameterService should provide ID-based access to parameters. - * */ - class ParameterBase { public: virtual void appendValueToMessage(Message& message) = 0; virtual void setValueFromMessage(Message& message) = 0; }; - +/** + * Implementation of a parameter containing its value. See \ref ParameterBase for more information. + * @tparam DataType + */ template <typename DataType> class Parameter : public ParameterBase { private: @@ -54,7 +55,7 @@ public: currentValue = value; } - inline DataType getValue() { + DataType getValue() { return currentValue; } diff --git a/inc/Services/ParameterService.hpp b/inc/Services/ParameterService.hpp index f6da64c90490bcc0fd3a6fabc9c386b1e637f74b..61286e031282a30cce9c73a3bafaa563273168c7 100644 --- a/inc/Services/ParameterService.hpp +++ b/inc/Services/ParameterService.hpp @@ -6,7 +6,7 @@ #include "ErrorHandler.hpp" #include "Parameter.hpp" #include "etl/vector.h" -#include "Parameters/Parameters.hpp" +#include "Parameters/SystemParameters.hpp" /** * Implementation of the ST[20] parameter management service, @@ -18,17 +18,13 @@ /** * Parameter manager - ST[20] - * Holds the list with the parameters and provides functions - * for parameter reporting and modification. * - * The parameter list is stored in a map with the parameter IDs as keys and values - * corresponding Parameter classes containing the parameter's value. + * The purpose of this class is to handle functions regarding the access and modification + * of the various parameters of the CubeSat. + * The parameters to be managed are initialized and kept in \ref SystemParameters. */ class ParameterService : public Service { public: - /** - * @brief Fills the parameter map. - */ ParameterService() = default; /** @@ -36,29 +32,17 @@ public: * containing the current configuration * **for the parameters specified in the carried valid IDs**. * - * The packet is checked for errors in service and message type, as well as for the - * validity of the IDs contained. For every invalid ID an ExecutionStartErrorType::UnknownExecutionStartError - * is raised. - * If the packet has an incorrect header and service type, an InternalError::UnacceptableMessage is raised. - * If no IDs are correct, the returned message shall be empty. - * * @param paramId: a TC[20, 1] packet carrying the requested parameter IDs * @return None (messages are stored using storeMessage()) - * - * Everything apart from the setting data is uint16 (setting data are uint32 for now) */ void reportParameters(Message& paramIds); /** * This function receives a TC[20, 3] message and after checking whether its type is correct, * iterates over all contained parameter IDs and replaces the settings for each valid parameter, - * while ignoring all invalid IDs. If the manual update flag is not set, the parameter's value should - * not change. + * while ignoring all invalid IDs. * * @param newParamValues: a valid TC[20, 3] message carrying parameter ID and replacement value - * @return None - * - * @todo Use pointers for changing and storing addresses to comply with the standard */ void setParameters(Message& newParamValues); diff --git a/src/Parameters/Parameters.cpp b/src/Parameters/Parameters.cpp deleted file mode 100644 index 6505b00bd96e54c818e8d1ec269efe300ff5075a..0000000000000000000000000000000000000000 --- a/src/Parameters/Parameters.cpp +++ /dev/null @@ -1,3 +0,0 @@ -#include "Parameters/Parameters.hpp" - -SystemParameters systemParameters; diff --git a/src/Platform/x86/Parameters/SystemParameters.cpp b/src/Platform/x86/Parameters/SystemParameters.cpp new file mode 100644 index 0000000000000000000000000000000000000000..b5f68de3991e7aea68d3a6de2578aa48a4783b85 --- /dev/null +++ b/src/Platform/x86/Parameters/SystemParameters.cpp @@ -0,0 +1,3 @@ +#include "Parameters/SystemParameters.hpp" + +SystemParameters systemParameters; diff --git a/src/Services/ParameterService.cpp b/src/Services/ParameterService.cpp index 264b8da5d13ea95853bd9f404f72088abe162554..a9a395eb257037e95fb0111dbbbc52fb7a06cd2c 100644 --- a/src/Services/ParameterService.cpp +++ b/src/Services/ParameterService.cpp @@ -6,9 +6,8 @@ void ParameterService::reportParameters(Message& paramIds) { // TM[20,2] - Message reqParam(20, 2, Message::TM, 1); + Message parameterReport(20, 2, Message::TM, 1); - paramIds.resetRead(); ErrorHandler::assertRequest(paramIds.packetType == Message::TC, paramIds, ErrorHandler::AcceptanceErrorType::UnacceptableMessage); ErrorHandler::assertRequest(paramIds.messageType == 1, paramIds, @@ -17,21 +16,26 @@ void ParameterService::reportParameters(Message& paramIds) { ErrorHandler::AcceptanceErrorType::UnacceptableMessage); uint16_t numOfIds = paramIds.readUint16(); - uint16_t validIds = 0; - reqParam.appendUint16(validIds); // Shouldn't you count first the actually fucking valid ids? - + uint16_t numberOfValidIds = 0; + for (uint16_t counter = 0; counter < numOfIds; counter++) { + 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(); if (currId < systemParameters.parametersArray.size()) { - reqParam.appendUint16(currId); - systemParameters.parametersArray[currId].get().appendValueToMessage(reqParam); - validIds++; + parameterReport.appendUint16(currId); + systemParameters.parametersArray[currId].get().appendValueToMessage(parameterReport); } else { - ErrorHandler::reportInternalError(ErrorHandler::GetNonExistingParameter); + ErrorHandler::reportError(paramIds, ErrorHandler::GetNonExistingParameter); } } - storeMessage(reqParam); + storeMessage(parameterReport); } void ParameterService::setParameters(Message& newParamValues) { @@ -50,7 +54,8 @@ void ParameterService::setParameters(Message& newParamValues) { if (currId < systemParameters.parametersArray.size()) { systemParameters.parametersArray[currId].get().setValueFromMessage(newParamValues); } else { - ErrorHandler::reportInternalError(ErrorHandler::SetNonExistingParameter); + ErrorHandler::reportError(newParamValues, ErrorHandler::InvalidParameterId); + break; } } } diff --git a/test/Services/Parameter.cpp b/test/Services/Parameter.cpp new file mode 100644 index 0000000000000000000000000000000000000000..12938fa0e65ba43c2c0256da9a007438b7e5bcc3 --- /dev/null +++ b/test/Services/Parameter.cpp @@ -0,0 +1,10 @@ +#include "catch2/catch.hpp" +#include "Services/ParameterService.hpp" +#include "Services/Parameter.hpp" +#include "Message.hpp" +#include "ServiceTests.hpp" + + +TEST_CASE("Parameter Functions") { + +} \ No newline at end of file diff --git a/test/Services/ParameterService.cpp b/test/Services/ParameterService.cpp index b31c3418d49b65dfcca984de05d85135a9c61948..71aa55ed57632209193202d9c586587d269bff06 100644 --- a/test/Services/ParameterService.cpp +++ b/test/Services/ParameterService.cpp @@ -1,10 +1,7 @@ #include "catch2/catch.hpp" -#include "Services/ParameterService.hpp" #include "Message.hpp" #include "ServiceTests.hpp" -ParameterService& pserv = Services.parameterManagement; - TEST_CASE("Parameter Report Subservice") { SECTION("All requested parameters invalid") { Message request = Message(20, 1, Message::TC, 1); @@ -15,11 +12,11 @@ TEST_CASE("Parameter Report Subservice") { MessageParser::execute(request); CHECK(ServiceTests::get(0).serviceType == 1); - CHECK(ServiceTests::get(0).messageType == 4); + CHECK(ServiceTests::get(0).messageType == 2); CHECK(ServiceTests::get(1).serviceType == 1); - CHECK(ServiceTests::get(1).messageType == 4); + CHECK(ServiceTests::get(1).messageType == 2); CHECK(ServiceTests::get(2).serviceType == 1); - CHECK(ServiceTests::get(2).messageType == 4); + CHECK(ServiceTests::get(2).messageType == 2); Message report = ServiceTests::get(3); CHECK(report.serviceType == 20); @@ -30,42 +27,52 @@ TEST_CASE("Parameter Report Subservice") { Services.reset(); } - SECTION("Faulty instruction handling") { - auto param0 = Parameter<uint16_t>(1); - auto param1 = Parameter<uint16_t>(12); - auto param2 = Parameter<uint16_t>(3); - systemParameters.parametersArray.push_back(param0); - systemParameters.parametersArray.push_back(param1); - systemParameters.parametersArray.push_back(param2); - - Message request(20, 1, Message::TC, 1); - request.appendUint16(2); // number of requested IDs - request.appendUint16(65535); // invalid ID in this context - request.appendUint16(1); // valid + SECTION("Some requested parameters invalid") { + Message request = Message(20, 1, Message::TC, 1); + request.appendUint16(3); + request.appendUint16(1); + request.appendUint16(2); + request.appendUint16(10000); MessageParser::execute(request); - Message report = ServiceTests::get(1); - - CHECK(ServiceTests::get(0).messageType == 4); CHECK(ServiceTests::get(0).serviceType == 1); - // check for an ST[1,4] message caused by the faulty ID - CHECK((ServiceTests::thrownError(ErrorHandler::ExecutionStartErrorType::UnknownExecutionStartError))); - // check for the thrown UnknownExecutionStartError + CHECK(ServiceTests::get(0).messageType == 2); - CHECK(report.messageType == 2); + Message report = ServiceTests::get(1); CHECK(report.serviceType == 20); - // check for an ST[20,2] message (the one that contains the settings) + CHECK(report.messageType == 2); + CHECK(report.readUint16() == 2); + CHECK(report.readUint16() == 1); + CHECK(report.readUint16() == 7); + CHECK(report.readUint16() == 2); + CHECK(report.readUint32() == 10); + + ServiceTests::reset(); + Services.reset(); + } + + SECTION("Parameters are of different types") { + Message request = Message(20, 1, Message::TC, 1); + request.appendUint16(3); + request.appendUint16(0); + request.appendUint16(1); + request.appendUint16(2); - CHECK(report.readUint16() == 1); // only one parameter shall be contained + MessageParser::execute(request); - CHECK(report.readUint16() == 1); // check for parameter ID - uint8_t data[ECSS_ST_20_MAX_STRING_LENGTH]; - report.readString(data, ECSS_ST_20_MAX_STRING_LENGTH); - String<ECSS_ST_20_MAX_STRING_LENGTH> str = String<ECSS_ST_20_MAX_STRING_LENGTH>(data); - CHECK(str.compare("12")); // check for value as string (defined when adding parameters) + Message report = ServiceTests::get(0); + CHECK(report.serviceType == 20); + CHECK(report.messageType == 2); + CHECK(report.readUint16() == 3); + CHECK(report.readUint16() == 0); + CHECK(report.readUint8() == 3); + CHECK(report.readUint16() == 1); + CHECK(report.readUint16() == 7); + CHECK(report.readUint16() == 2); + CHECK(report.readUint32() == 10); - ServiceTests::reset(); // clear all errors - Services.reset(); // reset the services + ServiceTests::reset(); + Services.reset(); } SECTION("Wrong Message Type Handling Test") { @@ -80,68 +87,109 @@ TEST_CASE("Parameter Report Subservice") { } TEST_CASE("Parameter Setting Subservice") { - SECTION("Faulty Instruction Handling Test") { - auto param0 = Parameter<uint16_t>(1); - auto param1 = Parameter<uint16_t>(12); - auto param2 = Parameter<uint16_t>(3); - systemParameters.parametersArray.push_back(param0); - systemParameters.parametersArray.push_back(param1); - systemParameters.parametersArray.push_back(param2); - - Message setRequest(20, 3, Message::TC, 1); - setRequest.appendUint16(2); // total number of IDs - setRequest.appendUint16(1); // correct ID in this context - setRequest.appendUint32(3735928559); // 0xDEADBEEF in hex (new setting) - setRequest.appendUint16(65535); // faulty ID - setRequest.appendUint32(3131746989); // 0xBAAAAAAD (this shouldn't be found in the report) - - MessageParser::execute(setRequest); + SECTION("All parameter ids are invalid") { + Message request = Message(20, 3, Message::TC, 1); + request.appendUint16(3); + request.appendUint16(54432); + request.appendUint16(1); + request.appendUint16(60000); + request.appendUint16(1); + request.appendUint16(65534); + request.appendUint16(1); + MessageParser::execute(request); CHECK(ServiceTests::get(0).serviceType == 1); - CHECK(ServiceTests::get(0).messageType == 4); + CHECK(ServiceTests::get(0).messageType == 2); // Just one error, because it should break after on invalid param - Message reportRequest(20, 1, Message::TC, 1); - reportRequest.appendUint16(1); - reportRequest.appendUint16(1); // the changed parameter has ID 1 + CHECK(systemParameters.parameter1.getValue() == 3); + CHECK(systemParameters.parameter2.getValue() == 7); + CHECK(systemParameters.parameter3.getValue() == 10); - MessageParser::execute(reportRequest); - Message report = ServiceTests::get(1); - CHECK(report.serviceType == 20); - CHECK(report.messageType == 2); - CHECK(report.readUint16() == 1); // only 1 ID contained - CHECK(report.readUint16() == 1); // contained ID should be ID 1 + ServiceTests::reset(); + Services.reset(); + } + + SECTION("The last parameter ids are invalid") { + Message request = Message(20, 3, Message::TC, 1); + request.appendUint16(3); + request.appendUint16(0); + request.appendUint8(1); + request.appendUint16(1); + request.appendUint16(2); + request.appendUint16(65534); + request.appendUint16(1); + + MessageParser::execute(request); + CHECK(ServiceTests::get(0).serviceType == 1); + CHECK(ServiceTests::get(0).messageType == 2); + + CHECK(systemParameters.parameter1.getValue() == 1); + CHECK(systemParameters.parameter2.getValue() == 2); + CHECK(systemParameters.parameter3.getValue() == 10); - char data[ECSS_ST_20_MAX_STRING_LENGTH]; - report.readString(data, ECSS_ST_20_MAX_STRING_LENGTH); - String<ECSS_ST_20_MAX_STRING_LENGTH> str = String<ECSS_ST_20_MAX_STRING_LENGTH>(data); - CHECK(str.compare("3735928559")); // whose value is the string 0xDEADBEEF + systemParameters.parameter1.setValue(3); + systemParameters.parameter2.setValue(7); + systemParameters.parameter3.setValue(10); ServiceTests::reset(); Services.reset(); } - SECTION("Attempt to set parameter with no manual update availability") { - auto param1 = Parameter<uint16_t>(12); - systemParameters.parametersArray.push_back(param1); + SECTION("The last parameter ids are invalid") { + Message request = Message(20, 3, Message::TC, 1); + request.appendUint16(3); + request.appendUint16(0); + request.appendUint8(1); + request.appendUint16(65534); + request.appendUint16(1); + request.appendUint16(2); + request.appendUint16(3); - Message setRequest = Message(20, 3, Message::TC, 1); - setRequest.appendUint16(1); - setRequest.appendUint16(1); - setRequest.appendUint32(0xBAAAAAAD); + MessageParser::execute(request); + CHECK(ServiceTests::get(0).serviceType == 1); + CHECK(ServiceTests::get(0).messageType == 2); - MessageParser::execute(setRequest); + CHECK(systemParameters.parameter1.getValue() == 1); + CHECK(systemParameters.parameter2.getValue() == 7); + CHECK(systemParameters.parameter3.getValue() == 10); - Message infoRequest = Message(20, 1, Message::TC, 1); - infoRequest.appendUint16(1); - infoRequest.appendUint16(1); + systemParameters.parameter1.setValue(3); + systemParameters.parameter2.setValue(7); + systemParameters.parameter3.setValue(10); - MessageParser::execute(infoRequest); + ServiceTests::reset(); + Services.reset(); + } - Message report = ServiceTests::get(0); + SECTION("All ids are valid") { + Message request = Message(20, 3, Message::TC, 1); + request.appendUint16(3); + request.appendUint16(0); + request.appendUint8(1); + request.appendUint16(1); + request.appendUint16(2); + request.appendUint16(2); + request.appendUint32(3); - CHECK(report.readUint16() == 1); - CHECK(report.readUint16() == 1); - CHECK_FALSE(report.readUint32() == 0xBAAAAAAD); + MessageParser::execute(request); + + CHECK(systemParameters.parameter1.getValue() == 1); + CHECK(systemParameters.parameter2.getValue() == 2); + CHECK(systemParameters.parameter3.getValue() == 3); + + systemParameters.parameter1.setValue(3); + systemParameters.parameter2.setValue(7); + systemParameters.parameter3.setValue(10); + + 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();