diff --git a/inc/Services/EventActionService.hpp b/inc/Services/EventActionService.hpp index b7e9b6d358f1f698b800fb384862e53986fa90c1..f774a1c43c003880dafa8144a0eeaa1b73449b95 100644 --- a/inc/Services/EventActionService.hpp +++ b/inc/Services/EventActionService.hpp @@ -12,7 +12,7 @@ * * ECSS 8.19 && 6.19 * - * Note: Make sure the check the note in the addEventActionDefinition() + * Note: Make sure to check the note in the addEventActionDefinition() * Note: A third variable was added, the eventActionDefinitionID. This was added for the purpose of identifying * various eventActionDefinitions that correspond to the same eventDefinitionID. The goal is to have multiple actions * be executed when one event takes place. This defies the standard. diff --git a/src/Services/EventActionService.cpp b/src/Services/EventActionService.cpp index bdcad7f00bd0935b7ab9ee73c49422a545548485..9aee53f1784d6d27ea6bf4dc36f6065b21918d14 100644 --- a/src/Services/EventActionService.cpp +++ b/src/Services/EventActionService.cpp @@ -6,25 +6,24 @@ void EventActionService::addEventActionDefinitions(Message& message) { // TC[19,1] message.assertTC(19, 1); - message.resetRead(); uint16_t applicationID = message.readEnum16(); uint16_t eventDefinitionID = message.readEnum16(); uint16_t eventActionDefinitionID = message.readEnum16(); - bool flag = true; // This variable checks if the message can be added or not. Couldn't think of a better name + bool canBeAdded = true; if (eventActionDefinitionMap.find(eventDefinitionID) != eventActionDefinitionMap.end()) { auto range = eventActionDefinitionMap.equal_range(eventDefinitionID); for (auto& element = range.first; element != range.second; ++element) { if (element->second.eventActionDefinitionID == eventActionDefinitionID) { - flag = false; + canBeAdded = false; ErrorHandler::reportError(message, ErrorHandler::EventActionEventActionDefinitionIDExistsError); } } } if (message.dataSize - 6 > ECSS_TC_REQUEST_STRING_SIZE) { - flag = false; + canBeAdded = false; ErrorHandler::reportInternalError(ErrorHandler::MessageTooLarge); } - if (flag){ + if (canBeAdded) { char data[ECSS_TC_REQUEST_STRING_SIZE]; message.readString(data, message.dataSize - 6); EventActionDefinition temp; @@ -39,9 +38,8 @@ void EventActionService::addEventActionDefinitions(Message& message) { void EventActionService::deleteEventActionDefinitions(Message& message) { message.assertTC(19, 2); - message.resetRead(); uint16_t numberOfEventActionDefinitions = message.readUint16(); - bool eventActionDefinitionIDexists = false; + bool definitionIDexists = false; for (uint16_t i = 0; i < numberOfEventActionDefinitions; i++) { message.skipBytes(2); uint16_t eventDefinitionID = message.readEnum16(); @@ -49,16 +47,16 @@ void EventActionService::deleteEventActionDefinitions(Message& message) { if (eventActionDefinitionMap.find(eventDefinitionID) != eventActionDefinitionMap.end()) { auto range = eventActionDefinitionMap.equal_range(eventDefinitionID); for (auto& element = range.first; element != range.second; ++element) { - if (element->second.eventActionDefinitionID == eventActionDefinitionID){ - eventActionDefinitionIDexists = true; - if (element->second.enabled == true){ + if (element->second.eventActionDefinitionID == eventActionDefinitionID) { + definitionIDexists = true; + if (element->second.enabled) { ErrorHandler::reportError(message, ErrorHandler::EventActionDeleteEnabledDefinitionError); } else { eventActionDefinitionMap.erase(element); } } } - if (eventActionDefinitionIDexists == false){ + if (not definitionIDexists) { ErrorHandler::reportError(message, ErrorHandler::EventActionUnknownEventActionDefinitionIDError); } } else { @@ -78,23 +76,23 @@ void EventActionService::deleteAllEventActionDefinitions(Message& message) { void EventActionService::enableEventActionDefinitions(Message& message) { // TC[19,4] message.assertTC(19, 4); - message.resetRead(); uint16_t numberOfEventActionDefinitions = message.readUint16(); if (numberOfEventActionDefinitions != 0) { for (uint16_t i = 0; i < numberOfEventActionDefinitions; i++) { - message.skipBytes(2); + message.skipBytes(2); // Skips reading the application ID uint16_t eventDefinitionID = message.readEnum16(); uint16_t eventActionDefinitionID = message.readEnum16(); if (eventActionDefinitionMap.find(eventDefinitionID) != eventActionDefinitionMap.end()) { - bool eventActionDefinitionIDexists = false; + bool definitionIDexists = false; auto range = eventActionDefinitionMap.equal_range(eventDefinitionID); for (auto& element = range.first; element != range.second; ++element) { - if (element->second.eventActionDefinitionID == eventActionDefinitionID){ + if (element->second.eventActionDefinitionID == eventActionDefinitionID) { element->second.enabled = true; - eventActionDefinitionIDexists = true; + definitionIDexists = true; + break; } } - if (eventActionDefinitionIDexists == false){ + if (not definitionIDexists) { ErrorHandler::reportError(message, ErrorHandler::EventActionUnknownEventActionDefinitionIDError); } } else { @@ -111,24 +109,22 @@ void EventActionService::enableEventActionDefinitions(Message& message) { void EventActionService::disableEventActionDefinitions(Message& message) { // TC[19,5] message.assertTC(19, 5); - message.resetRead(); - uint16_t numberOfEventActionDefinitions = message.readUint16(); if (numberOfEventActionDefinitions != 0) { for (uint16_t i = 0; i < numberOfEventActionDefinitions; i++) { - message.skipBytes(2); + message.skipBytes(2); // Skips reading applicationID uint16_t eventDefinitionID = message.readEnum16(); uint16_t eventActionDefinitionID = message.readEnum16(); if (eventActionDefinitionMap.find(eventDefinitionID) != eventActionDefinitionMap.end()) { - bool eventActionDefinitionIDexists = false; + bool definitionIDexists = false; auto range = eventActionDefinitionMap.equal_range(eventDefinitionID); for (auto& element = range.first; element != range.second; ++element) { - if (element->second.eventActionDefinitionID == eventActionDefinitionID){ + if (element->second.eventActionDefinitionID == eventActionDefinitionID) { element->second.enabled = false; - eventActionDefinitionIDexists = true; + definitionIDexists = true; } } - if (eventActionDefinitionIDexists == false){ + if (not definitionIDexists) { ErrorHandler::reportError(message, ErrorHandler::EventActionUnknownEventActionDefinitionIDError); } } else { @@ -181,14 +177,12 @@ void EventActionService::disableEventActionFunction(Message& message) { void EventActionService::executeAction(uint16_t eventID) { // Custom function if (eventActionFunctionStatus) { - if (eventActionDefinitionMap.find(eventID) != eventActionDefinitionMap.end()) { - auto range = eventActionDefinitionMap.equal_range(eventID); - for (auto& element = range.first; element != range.second; ++element){ - if (element->second.enabled == true){ - MessageParser messageParser; - Message message = messageParser.parseRequestTC(element->second.request); - messageParser.execute(message); - } + auto range = eventActionDefinitionMap.equal_range(eventID); + for (auto& element = range.first; element != range.second; ++element) { + if (element->second.enabled) { + MessageParser messageParser; + Message message = messageParser.parseRequestTC(element->second.request); + messageParser.execute(message); } } } diff --git a/test/Services/EventActionService.cpp b/test/Services/EventActionService.cpp index df396eea301ce1da299d603ec57e8d04ed3e8b7b..1ba23a7c65ccf245700ea2888bf19615d3274f2d 100644 --- a/test/Services/EventActionService.cpp +++ b/test/Services/EventActionService.cpp @@ -53,7 +53,13 @@ TEST_CASE("Add event-action definitions TC[19,1]", "[service][st19]") { CHECK(eventActionService.eventActionDefinitionMap.lower_bound(3)->second.request.compare(data) == 0); // Adding the same message to check for error - eventActionService.addEventActionDefinitions(message2); + Message message3(19, 1, Message::TC, 0); + message3.appendEnum16(1); + message3.appendEnum16(3); + message3.appendEnum16(1); + data = "456"; + message3.appendString(data); + eventActionService.addEventActionDefinitions(message3); CHECK(ServiceTests::thrownError(ErrorHandler::EventActionEventActionDefinitionIDExistsError)); CHECK(ServiceTests::countErrors() == 2); }