diff --git a/inc/Services/EventActionService.hpp b/inc/Services/EventActionService.hpp index dab27267f60ba37d4c701d695f9cd7045b8c83b7..3964a1ef39ac56f1963d7cf1a247b788795bec5d 100644 --- a/inc/Services/EventActionService.hpp +++ b/inc/Services/EventActionService.hpp @@ -3,6 +3,8 @@ #define ECSS_EVENT_SERVICE_STRING_SIZE 64 +#define ECSS_EVENT_ACTION_STRUCT_ARRAY_SIZE 256 + #include "Service.hpp" #include "MessageParser.hpp" #include "etl/String.hpp" @@ -14,9 +16,6 @@ * * ECSS 8.19 && 6.19 * - * Note towards the reviewers: Please double-check the string sizes that I use, the string - * initialization or rather the lack of it. Pay attention especially in parts of the code that I - * use strings <3 . * * @todo: (Possible) Use a etl::map for eventActionDefinitionArray * @todo: check if executeAction should accept applicationID too @@ -47,7 +46,7 @@ public: // If the size is changed maybe then indexOfAvailableSlots as well as the initiating loop in the // constructor should be changed from uint16_t - EventActionDefinition eventActionDefinitionArray[256]; + EventActionDefinition eventActionDefinitionArray[ECSS_EVENT_ACTION_STRUCT_ARRAY_SIZE]; EventActionService() { serviceType = 19; diff --git a/src/Services/EventActionService.cpp b/src/Services/EventActionService.cpp index 5a53e9542ee42ec368168282ce21516aff1d1d41..607efaa4ff11246924dc952929bccaf953bea4a0 100644 --- a/src/Services/EventActionService.cpp +++ b/src/Services/EventActionService.cpp @@ -3,8 +3,7 @@ #include "MessageParser.hpp" /** - * @todo: Should a check be added for index to not exceed the size of eventActionDefinitionArray - * ? Also check if there a uint16_t is needed (in case of changing the size of + * @todo: Check if a uint16_t is needed (in case of changing the size of * eventActionDefinitionArray */ void EventActionService::addEventActionDefinitions(Message message) { @@ -14,7 +13,7 @@ void EventActionService::addEventActionDefinitions(Message message) { 19) { bool flag = true; uint16_t index; - for (index = 0; index < 256; index++) { + for (index = 0; index < ECSS_EVENT_ACTION_STRUCT_ARRAY_SIZE; index++) { if (eventActionDefinitionArray[index].empty == true) { flag = false; break; @@ -41,10 +40,9 @@ void EventActionService::deleteEventActionDefinitions(Message message) { == 19) { uint16_t N = message.readUint16(); for (uint16_t i = 0; i < N; i++) { - uint16_t index = 0; uint16_t applicationID = message.readEnum16(); uint16_t eventDefinitionID = message.readEnum16(); - while (index < 255) { + for (uint16_t index = 0; index< ECSS_EVENT_ACTION_STRUCT_ARRAY_SIZE; index++) { if (eventActionDefinitionArray[index].applicationId == applicationID && eventActionDefinitionArray[index].eventDefinitionID == eventDefinitionID) { eventActionDefinitionArray[index].empty = true; @@ -53,7 +51,6 @@ void EventActionService::deleteEventActionDefinitions(Message message) { eventActionDefinitionArray[index].applicationId = 0; eventActionDefinitionArray[index].enabled = false; } - index++; } } @@ -64,7 +61,7 @@ void EventActionService::deleteAllEventActionDefinitions(Message message) { // TC[19,3] if (message.messageType == 3 && message.packetType == Message::TC && message.serviceType == 19) { - for (uint16_t index = 0; index < 256; index++) { + for (uint16_t index = 0; index < ECSS_EVENT_ACTION_STRUCT_ARRAY_SIZE; index++) { if (eventActionDefinitionArray[index].empty == false) { eventActionDefinitionArray[index].empty = true; eventActionDefinitionArray[index].enabled = false; @@ -82,15 +79,13 @@ void EventActionService::enableEventActionDefinitions(Message message) { == 19) { uint16_t N = message.readUint16(); for (uint16_t i = 0; i < N; i++) { - uint16_t index = 0; uint16_t applicationID = message.readEnum16(); uint16_t eventDefinitionID = message.readEnum16(); - while (index < 255) { + for (uint16_t index = 0; index < ECSS_EVENT_ACTION_STRUCT_ARRAY_SIZE; index++) { if (eventActionDefinitionArray[index].applicationId == applicationID && eventActionDefinitionArray[index].eventDefinitionID == eventDefinitionID) { eventActionDefinitionArray[index].enabled = true; } - index++; } } } @@ -102,15 +97,13 @@ void EventActionService::disableEventActionDefinitions(Message message) { == 19) { uint16_t N = message.readUint16(); for (uint16_t i = 0; i < N; i++) { - uint16_t index = 0; uint16_t applicationID = message.readEnum16(); uint16_t eventDefinitionID = message.readEnum16(); - while (index < 256) { + for (uint16_t index = 0; index < ECSS_EVENT_ACTION_STRUCT_ARRAY_SIZE; index++) { if (eventActionDefinitionArray[index].applicationId == applicationID && eventActionDefinitionArray[index].eventDefinitionID == eventDefinitionID) { eventActionDefinitionArray[index].enabled = false; } - index++; } } } @@ -128,13 +121,13 @@ void EventActionService::eventActionStatusReport() { // TM[19,7] Message report = createTM(7); uint8_t count = 0; - for (uint16_t i = 0; i < 256; i++) { + for (uint16_t i = 0; i < ECSS_EVENT_ACTION_STRUCT_ARRAY_SIZE; i++) { if (eventActionDefinitionArray[i].empty == false) { count++; } } report.appendUint8(count); - for (uint16_t i = 0; i < 256; i++) { + for (uint16_t i = 0; i < ECSS_EVENT_ACTION_STRUCT_ARRAY_SIZE; i++) { if (eventActionDefinitionArray[i].empty == false) { report.appendEnum16(eventActionDefinitionArray[i].applicationId); report.appendEnum16(eventActionDefinitionArray[i].eventDefinitionID); @@ -165,8 +158,7 @@ void EventActionService::disableEventActionFunction(Message message) { void EventActionService::executeAction(uint16_t eventID) { // Custom function if (eventActionFunctionStatus) { - uint16_t i = 0; - while (i < 256) { + for (uint16_t i = 0; i < ECSS_EVENT_ACTION_STRUCT_ARRAY_SIZE; i++) { if (eventActionDefinitionArray[i].empty == false && eventActionDefinitionArray[i].enabled == true) { @@ -177,7 +169,6 @@ void EventActionService::executeAction(uint16_t eventID) { messageParser.execute(message); } } - i++; } } }