diff --git a/inc/ErrorHandler.hpp b/inc/ErrorHandler.hpp index ce2b551c5aa6717634a23a66d5d835039b7e528c..b07148822fb61f38ee42bf264a76c99de9e0ad03 100644 --- a/inc/ErrorHandler.hpp +++ b/inc/ErrorHandler.hpp @@ -74,13 +74,17 @@ public: */ MapFull = 10, /** - * Attempt to overwrite an existing parameter (ST[20]) + * Attempt to access a non existing parameter (ST[20]) */ - ExistingParameterId = 11, + 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 = 12 + NestedMessageTooLarge = 13 }; /** diff --git a/inc/Helpers/TimeHelper.hpp b/inc/Helpers/TimeHelper.hpp index 027e078e817bf6748ac693821d7527a9e5c7c811..ecbad93d35d5893842e080e585c210f549bbce1d 100644 --- a/inc/Helpers/TimeHelper.hpp +++ b/inc/Helpers/TimeHelper.hpp @@ -66,7 +66,7 @@ public: * @todo check if we need to change the epoch to the recommended one from the standard, 1 * January 1958 */ - static uint32_t utcToSeconds(TimeAndDate& TimeInfo); + static uint32_t utcToSeconds(const TimeAndDate& TimeInfo); /** * Convert elapsed seconds since Unix epoch to UTC date. @@ -94,7 +94,7 @@ public: * @todo time security for critical time operations * @todo declare the implicit P-field */ - static uint64_t generateCDSTimeFormat(struct TimeAndDate& TimeInfo); + static uint64_t generateCDSTimeFormat(const struct TimeAndDate& TimeInfo); /** * Parse the CDS time format (3.3 in CCSDS 301.0-B-4 standard) @@ -124,7 +124,7 @@ public: * @todo time security for critical time operations * @todo declare the implicit P-field */ - static uint32_t generateCUCTimeFormat(struct TimeAndDate& TimeInfo); + static uint32_t generateCUCTimeFormat(const struct TimeAndDate& TimeInfo); /** * Parse the CUC time format (3.3 in CCSDS 301.0-B-4 standard) diff --git a/inc/Parameters/Parameters.hpp b/inc/Parameters/Parameters.hpp index d3a58827ddea7dadcbb0b310c820df0b12347d1c..f8651abb9a699a721fff9d174eeabecdf3378fe7 100644 --- a/inc/Parameters/Parameters.hpp +++ b/inc/Parameters/Parameters.hpp @@ -1,14 +1,17 @@ #include "Services/Parameter.hpp" #include "etl/vector.h" + class SystemParameters { public: Parameter<uint8_t> parameter1 = Parameter<uint8_t>(5); - Parameter<char> parameter2= Parameter<char>('a'); - Parameter<int> parameter3 = Parameter<int>('b'); - Parameter<uint8_t> parameter4 = Parameter<uint8_t>(5); - Parameter<uint8_t> parameter5 = 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() = default; + SystemParameters() { + parametersArray[0] = parameter1; + parametersArray[1] = parameter2; + parametersArray[2] = parameter3; + } }; extern SystemParameters systemParameters; diff --git a/inc/Services/Parameter.hpp b/inc/Services/Parameter.hpp index b59f5d0d9e50bf0232ee9ba139cf6005db84ef31..947e574dc191d27d58975c504a1b1d946686c287 100644 --- a/inc/Services/Parameter.hpp +++ b/inc/Services/Parameter.hpp @@ -7,73 +7,36 @@ /** * Implementation of a Parameter field, as specified in ECSS-E-ST-70-41C. - * Fully compliant with the standard's requirements. * * @author Grigoris Pavlakis <grigpavl@ece.auth.gr> - * @author Athanasios Theocharis <athatheo@csd.auth.gr> - * - * + * @author Athanasios Theocharis <athatheoc@gmail.com> + * * @section Introduction * The Parameter class implements a way of storing and updating system parameters * of arbitrary size and type, while avoiding std::any and dynamic memory allocation. - * - * It is split in two distinct parts: - * - * 1) an abstract \ref ParameterBase class which provides a - * common data type used to create any pointers to \ref Parameter objects, as well as + * It is split in two distinct parts: + * 1) an abstract \ref ParameterBase class which provides a + * 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 template \ref Parameter used to store any type-specific parameter information, + * 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. - * - * + * * @section Architecture Rationale * The ST[20] Parameter service is implemented with the need of arbitrary type storage * in mind, while avoiding any use of dynamic memory allocation, a requirement for use - * in embedded systems. Since lack of DMA precludes usage of stl::any and the need for - * truly arbitrary (even for template-based objects like etl::string) type storage + * in embedded systems. Since lack of Dynamic Memory Access precludes usage of stl::any + * and the need for truly arbitrary (even for template-based objects like etl::string) type storage * 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 both ID-based access to parameters, - * - * - * @section Methods - * @public getValueAsString() - returns a \ref ECSS_ST_20_MAX_STRING_LENGTH-lengthed - * \ref String containing the value of the Parameter - * @public - * - */ - -/* - * MILLION DOLLAR QUESTIONS - OLD IMPLEMENTATION: - * setCurrentValue is templated. Since Parameter (a template class) inherits ParameterBase - * (a class containing a template member), does a specialization of Parameter also specialize - * setCurrentValue? If not, we have a problem, since Parameter won't necessarily specialize - * setCurrentValue with the correct type => our setter is *not* typesafe. - * - * Answer: NO! After a discussion on ##C++-general@Freenode, it turns out that this specialization - * does not happen! What this means is that, while a Parameter can be specialized as e.g. int, there - * is no way to prevent the setter from being called with a non-int (e.g. string or float) argument, - * resulting in an attempt to write data inside the Parameter that have a different type from what the - * template is carrying. - * - * Proof of concept: - * Parameter<int> ircTest = Parameter<int>(1337); - * ircTest.setCurrentValue("Hello this is a problem speaking"); - * auto s = ircTest.getValueAsString(); - * - * This snippet will overwrite the 4 bytes of the Parameter field with garbage and most importantly, - * it will do so silently. - * + * Furthermore, the \ref ParameterService should provide ID-based access to parameters. * */ class ParameterBase { public: - virtual String<ECSS_ST_20_MAX_STRING_LENGTH> getValueAsString() = 0; - virtual void setValueFromMessage(Message message) = 0; + virtual void appendValueToMessage(Message& message) = 0; + virtual void setValueFromMessage(Message& message) = 0; }; @@ -81,27 +44,45 @@ template <typename DataType> class Parameter : public ParameterBase { private: DataType currentValue; - void (* updateFunction)(DataType*); public: - Parameter(DataType initialValue, void (* updateFunction)(DataType*) = nullptr) { - this->updateFunction = updateFunction; - - if (this->updateFunction != nullptr) { - (*updateFunction)(¤tValue); - } - else { - currentValue = initialValue; - } + Parameter(DataType initialValue) { + currentValue = initialValue; } - void setValueFromMessage(Message message) override { - // TODO: implement setValueForMessage + inline void setValue(DataType value) { + currentValue = value; } - String<ECSS_ST_20_MAX_STRING_LENGTH> getValueAsString() override { - // TODO: implement getValueAsString() - return String<ECSS_ST_20_MAX_STRING_LENGTH>("DUMMY STRING"); + inline DataType getValue() { + return currentValue; } + + inline void setValueFromMessage(Message& message) override; + + inline void appendValueToMessage(Message& message) override; }; + +template<> inline void Parameter<uint8_t>::setValueFromMessage(Message& message) { + this->currentValue = message.readUint8(); +} +template<> inline void Parameter<uint16_t>::setValueFromMessage(Message& message) { + this->currentValue = message.readUint16(); +} + +template<> inline void Parameter<uint32_t>::setValueFromMessage(Message& message) { + this->currentValue = message.readUint32(); +} + +template<> inline void Parameter<uint8_t>::appendValueToMessage(Message& message) { + message.appendUint8(this->currentValue); +} + +template<> inline void Parameter<uint16_t>::appendValueToMessage(Message& message) { + message.appendUint16(this->currentValue); +} + +template<> inline void Parameter<uint32_t>::appendValueToMessage(Message& message) { + message.appendUint32(this->currentValue); +} #endif //ECSS_SERVICES_PARAMETER_HPP diff --git a/inc/Services/ParameterService.hpp b/inc/Services/ParameterService.hpp index 228f6a59d3db512c962c2bd0e455343dfdca2fc4..f6da64c90490bcc0fd3a6fabc9c386b1e637f74b 100644 --- a/inc/Services/ParameterService.hpp +++ b/inc/Services/ParameterService.hpp @@ -27,24 +27,9 @@ class ParameterService : public Service { public: /** - * @brief Initializes the parameter list. + * @brief Fills the parameter map. */ - ParameterService() { - addToParameterArray(1, systemParameters.parameter1); - addToParameterArray(2, systemParameters.parameter2); - addToParameterArray(3, systemParameters.parameter3); - addToParameterArray(4, systemParameters.parameter4); - addToParameterArray(5, systemParameters.parameter5); - } - - /** - * @brief Adds a new parameter. Emits an InternalError::MapFull if an attempt is made to insert - * parameters in a full map or an InternalError::ExistingParameterId if the given parameter ID - * exists already. - * @param id: the desired ID for this parameter - * @param param: the parameter field to be included - */ - void addToParameterArray(uint16_t id, ParameterBase& param); + ParameterService() = default; /** * This function receives a TC[20, 1] packet and returns a TM[20, 2] packet diff --git a/src/Helpers/TimeHelper.cpp b/src/Helpers/TimeHelper.cpp index 04792af7c90b0be0153d8212aa3af4262673870b..b0e02d9b4debc90164b0b041ca16ee11bc3b0e28 100644 --- a/src/Helpers/TimeHelper.cpp +++ b/src/Helpers/TimeHelper.cpp @@ -10,7 +10,7 @@ bool TimeHelper::IsLeapYear(uint16_t year) { return (year % 400) == 0; } -uint32_t TimeHelper::utcToSeconds(TimeAndDate& TimeInfo) { +uint32_t TimeHelper::utcToSeconds(const TimeAndDate& TimeInfo) { // the date, that \p TimeInfo represents, should be greater than or equal to 1/1/2019 and the // date should be valid according to Gregorian calendar ASSERT_INTERNAL(TimeInfo.year >= 2019, ErrorHandler::InternalErrorType::InvalidDate); @@ -91,7 +91,7 @@ struct TimeAndDate TimeHelper::secondsToUTC(uint32_t seconds) { return TimeInfo; } -uint64_t TimeHelper::generateCDSTimeFormat(TimeAndDate& TimeInfo) { +uint64_t TimeHelper::generateCDSTimeFormat(const TimeAndDate& TimeInfo) { /** * Define the T-field. The total number of octets for the implementation of T-field is 6(2 for * the `DAY` and 4 for the `ms of day` @@ -126,7 +126,7 @@ TimeAndDate TimeHelper::parseCDStimeFormat(const uint8_t* data) { return secondsToUTC(seconds); } -uint32_t TimeHelper::generateCUCTimeFormat(struct TimeAndDate& TimeInfo) { +uint32_t TimeHelper::generateCUCTimeFormat(const struct TimeAndDate& TimeInfo) { return (utcToSeconds(TimeInfo) + LEAP_SECONDS); } diff --git a/src/Parameters/Parameters.cpp b/src/Parameters/Parameters.cpp index a67683703449d32acec673bd8cf02aea8e027203..6505b00bd96e54c818e8d1ec269efe300ff5075a 100644 --- a/src/Parameters/Parameters.cpp +++ b/src/Parameters/Parameters.cpp @@ -1,3 +1,3 @@ #include "Parameters/Parameters.hpp" -SystemParameters systemParameters; \ No newline at end of file +SystemParameters systemParameters; diff --git a/src/Services/EventActionService.cpp b/src/Services/EventActionService.cpp index f42079794019ef1d4fca34b4ed176e7d40e5c18c..b0f2f7e22f4157d7d9b239309b8095f108663d53 100644 --- a/src/Services/EventActionService.cpp +++ b/src/Services/EventActionService.cpp @@ -221,4 +221,4 @@ void EventActionService::execute(Message& message) { } } -#endif \ No newline at end of file +#endif diff --git a/src/Services/ParameterService.cpp b/src/Services/ParameterService.cpp index a43bb572ea086b0a6ee4296333e6d6d8f32eb81d..264b8da5d13ea95853bd9f404f72088abe162554 100644 --- a/src/Services/ParameterService.cpp +++ b/src/Services/ParameterService.cpp @@ -4,10 +4,6 @@ #include "Services/ParameterService.hpp" #include "Services/Parameter.hpp" -void ParameterService::addToParameterArray(uint16_t id, ParameterBase& param) { - systemParameters.parametersArray[id] = param; -} - void ParameterService::reportParameters(Message& paramIds) { // TM[20,2] Message reqParam(20, 2, Message::TM, 1); @@ -22,16 +18,18 @@ void ParameterService::reportParameters(Message& paramIds) { uint16_t numOfIds = paramIds.readUint16(); uint16_t validIds = 0; + reqParam.appendUint16(validIds); // Shouldn't you count first the actually fucking valid ids? for (uint16_t i = 0; i < numOfIds; i++) { uint16_t currId = paramIds.readUint16(); - if (currId < ECSS_ST_20_MAX_PARAMETERS) { + if (currId < systemParameters.parametersArray.size()) { reqParam.appendUint16(currId); - reqParam.appendString(systemParameters.parametersArray[currId].get().getValueAsString()); + systemParameters.parametersArray[currId].get().appendValueToMessage(reqParam); validIds++; + } else { + ErrorHandler::reportInternalError(ErrorHandler::GetNonExistingParameter); } } - reqParam.appendUint16(validIds); storeMessage(reqParam); } @@ -49,8 +47,10 @@ void ParameterService::setParameters(Message& newParamValues) { for (uint16_t i = 0; i < numOfIds; i++) { uint16_t currId = newParamValues.readUint16(); - if (currId < ECSS_ST_20_MAX_PARAMETERS) { + if (currId < systemParameters.parametersArray.size()) { systemParameters.parametersArray[currId].get().setValueFromMessage(newParamValues); + } else { + ErrorHandler::reportInternalError(ErrorHandler::SetNonExistingParameter); } } } diff --git a/test/Services/EventActionService.cpp b/test/Services/EventActionService.cpp index d87262dce4bf2fad920224dd25c306f40ae6eb60..1931fc56dcee2120c7b0db8f5c3ad23716490195 100644 --- a/test/Services/EventActionService.cpp +++ b/test/Services/EventActionService.cpp @@ -3,9 +3,7 @@ #include <Message.hpp> #include "ServiceTests.hpp" #include <etl/String.hpp> -#include <etl/map.h> #include <cstring> -#include <iostream> #include <ServicePool.hpp> EventActionService& eventActionService = Services.eventAction; diff --git a/test/Services/ParameterService.cpp b/test/Services/ParameterService.cpp index eee01ed47ac4bcc4a4be1d7d53dbe6a07adeacc7..b31c3418d49b65dfcca984de05d85135a9c61948 100644 --- a/test/Services/ParameterService.cpp +++ b/test/Services/ParameterService.cpp @@ -5,40 +5,6 @@ ParameterService& pserv = Services.parameterManagement; -TEST_CASE("Parameter Service - General") { - SECTION("Addition to full map") { - - Parameter<int> param0 = Parameter<int>(1); - Parameter<int> param1 = Parameter<int>(12); - Parameter<int> param2 = Parameter<int>(3, nullptr); - Parameter<int> param3 = Parameter<int>(6); - Parameter<int> param4 = Parameter<int>(3); - - Parameter<int> param5 = Parameter<int>(4); - - pserv.addNewParameter(0, static_cast<ParameterBase*>(¶m0)); - pserv.addNewParameter(1, static_cast<ParameterBase*>(¶m1)); - pserv.addNewParameter(2, static_cast<ParameterBase*>(¶m2)); - pserv.addNewParameter(3, static_cast<ParameterBase*>(¶m3)); - pserv.addNewParameter(4, static_cast<ParameterBase*>(¶m4)); - - pserv.addNewParameter(5, static_cast<ParameterBase*>(¶m5)); // addNewParameter should return false - CHECK(ServiceTests::thrownError(ErrorHandler::InternalErrorType::MapFull)); - ServiceTests::reset(); - Services.reset(); // reset all services - } - - SECTION("Addition of already existing parameter") { - Parameter<int> param0 = Parameter<int>(1); - pserv.addNewParameter(0, static_cast<ParameterBase*>(¶m0)); - - pserv.addNewParameter(0, static_cast<ParameterBase*>(¶m0)); - CHECK(ServiceTests::thrownError(ErrorHandler::InternalErrorType::ExistingParameterId)); - ServiceTests::reset(); - Services.reset(); - } -} - TEST_CASE("Parameter Report Subservice") { SECTION("All requested parameters invalid") { Message request = Message(20, 1, Message::TC, 1); @@ -65,12 +31,12 @@ TEST_CASE("Parameter Report Subservice") { } SECTION("Faulty instruction handling") { - Parameter<int> param0 = Parameter<int>(); - Parameter<int> param1 = Parameter<int>(12); - Parameter<int> param2 = Parameter<int>(3, nullptr); - pserv.addNewParameter(0, static_cast<ParameterBase*>(¶m0)); - pserv.addNewParameter(1, static_cast<ParameterBase*>(¶m1)); - pserv.addNewParameter(2, static_cast<ParameterBase*>(¶m2)); + 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 @@ -114,14 +80,13 @@ TEST_CASE("Parameter Report Subservice") { } TEST_CASE("Parameter Setting Subservice") { - SECTION("Faulty Instruction Handling Test") { - Parameter<int> param0 = Parameter<int>(); - Parameter<int> param1 = Parameter<int>(12); - Parameter<int> param2 = Parameter<int>(3, nullptr); - pserv.addNewParameter(0, static_cast<ParameterBase*>(¶m0)); - pserv.addNewParameter(1, static_cast<ParameterBase*>(¶m1)); - pserv.addNewParameter(2, static_cast<ParameterBase*>(¶m2)); + 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 @@ -132,7 +97,6 @@ TEST_CASE("Parameter Setting Subservice") { MessageParser::execute(setRequest); - CHECK(ServiceTests::get(0).serviceType == 1); CHECK(ServiceTests::get(0).messageType == 4); @@ -157,8 +121,8 @@ TEST_CASE("Parameter Setting Subservice") { } SECTION("Attempt to set parameter with no manual update availability") { - Parameter<int> param1 = Parameter<int>(12); - pserv.addNewParameter(1, static_cast<ParameterBase*>(¶m1)); + auto param1 = Parameter<uint16_t>(12); + systemParameters.parametersArray.push_back(param1); Message setRequest = Message(20, 3, Message::TC, 1); setRequest.appendUint16(1);