diff --git a/inc/Message.hpp b/inc/Message.hpp index b03bbbdc0609800b159a23395183fe8be482c99a..6cd0ef1d73f06c6a2dda9fad42d17c8f4c86f3e4 100644 --- a/inc/Message.hpp +++ b/inc/Message.hpp @@ -135,11 +135,30 @@ public: * need to use a function like Message::appendOctetString(), or have specified the size of the * string beforehand. Note that the standard does not support null-terminated strings. * + * This does not append the full size of the string, just its current size. Use + * Message::appendFixedString() to have a constant number of characters added. + * * @param string The string to insert */ template <const size_t SIZE> void appendString(const String<SIZE>& string); + /** + * Appends a number of bytes to the message + * + * Note that this doesn't append the number of bytes that the string contains. For this, you + * need to use a function like Message::appendOctetString(), or have specified the size of the + * string beforehand. Note that the standard does not support null-terminated strings. + * + * The number of bytes appended is equal to \p SIZE. To append variable-sized parameters, use + * Message::appendString() instead. Missing bytes are padded with zeros, until the length of SIZE + * is reached. + * + * @param string The string to insert + */ + template <const size_t SIZE> + void appendFixedString(const String<SIZE>& string); + /** * Reads the next \p numBits bits from the the message in a big-endian format * @param numBits @@ -166,19 +185,28 @@ public: * Reads the next \p size bytes from the message, and stores them into the allocated \p string * * NOTE: We assume that \p string is already allocated, and its size is at least - * ECSS_MAX_STRING_SIZE. This function does placs a \0 at the end of the created string. + * ECSS_MAX_STRING_SIZE. This function does NOT place a \0 at the end of the created string. */ - void readString(char* string, uint8_t size); + void readString(char* string, uint16_t size); /** * Reads the next \p size bytes from the message, and stores them into the allocated \p string * * NOTE: We assume that \p string is already allocated, and its size is at least - * ECSS_MAX_STRING_SIZE. This function does placs a \0 at the end of the created string + * ECSS_MAX_STRING_SIZE. This function does NOT place a \0 at the end of the created string * @todo Is uint16_t size too much or not enough? It has to be defined */ void readString(uint8_t* string, uint16_t size); + /** + * Reads the next \p size bytes from the message, and stores them into the allocated \p string + * + * NOTE: We assume that \p string is already allocated, and its size is at least + * ECSS_MAX_STRING_SIZE + 1. This function DOES place a \0 at the end of the created string, + * meaning that \p string should contain 1 more byte than the string stored in the message. + */ + void readCString(char *string, uint16_t size); + public: Message(uint8_t serviceType, uint8_t messageType, PacketType packetType, uint16_t applicationId); @@ -452,8 +480,8 @@ public: /** * Fetches a N-byte string from the current position in the message * - * @details In the current implementation we assume that a preallocated array of - * sufficient size is provided as the argument + * In the current implementation we assume that a preallocated array of sufficient size + * is provided as the argument. This does NOT append a trailing `\0` to \p byteString. * @todo Specify if the provided array size is too small or too large * * PTC = 7, PFC = 0 @@ -526,4 +554,14 @@ inline void Message::appendString(const String<SIZE>& string) { dataSize += string.size(); } +template <const size_t SIZE> +inline void Message::appendFixedString(const String<SIZE>& string) { + ASSERT_INTERNAL((dataSize + SIZE) < ECSS_MAX_MESSAGE_SIZE, ErrorHandler::MessageTooLarge); + + memcpy(data + dataSize, string.data(), string.size()); // Append the bytes with content + (void) memset(data + dataSize + string.size(), 0, SIZE - string.size()); // The rest of the bytes is set to 0 + + dataSize += SIZE; +} + #endif // ECSS_SERVICES_PACKET_H diff --git a/src/Message.cpp b/src/Message.cpp index 13667390dc5f652e3889c55a0c2e9a7aecd09283..efedb65c16b2388fc6a87326cb0ffbcd35f2fac9 100644 --- a/src/Message.cpp +++ b/src/Message.cpp @@ -129,12 +129,11 @@ uint32_t Message::readWord() { return value; } -void Message::readString(char* string, uint8_t size) { +void Message::readString(char* string, uint16_t size) { ASSERT_REQUEST((readPosition + size) <= ECSS_MAX_MESSAGE_SIZE, ErrorHandler::MessageTooShort); ASSERT_REQUEST(size < ECSS_MAX_STRING_SIZE, ErrorHandler::StringTooShort); memcpy(string, data + readPosition, size); - string[size] = 0; // todo: Use that for now to avoid problems. Later to be removed readPosition += size; } @@ -144,11 +143,15 @@ void Message::readString(uint8_t* string, uint16_t size) { ASSERT_REQUEST(size < ECSS_MAX_STRING_SIZE, ErrorHandler::StringTooShort); memcpy(string, data + readPosition, size); - string[size] = 0; // todo: Use that for now to avoid problems. Later to be removed readPosition += size; } +void Message::readCString(char *string, uint16_t size) { + readString(string, size); + string[size] = 0; +} + void Message::resetRead() { readPosition = 0; currentBit = 0; diff --git a/src/Services/EventActionService.cpp b/src/Services/EventActionService.cpp index 99f3ccce0ef7524a8f85afa9fb1831f6278ecd75..e3788002c83a8c7b175e8c150a91880919c8f4f7 100644 --- a/src/Services/EventActionService.cpp +++ b/src/Services/EventActionService.cpp @@ -24,7 +24,7 @@ void EventActionService::addEventActionDefinitions(Message& message) { ErrorHandler::reportInternalError(ErrorHandler::MessageTooLarge); } if (canBeAdded) { - char data[ECSS_TC_REQUEST_STRING_SIZE]; + char data[ECSS_TC_REQUEST_STRING_SIZE] = { 0 }; message.readString(data, message.dataSize - 6); EventActionDefinition temp; temp.enabled = false; diff --git a/src/Services/FunctionManagementService.cpp b/src/Services/FunctionManagementService.cpp index f83354ad355c6e5a6899a06b314f8eac95d64f62..c3831872f501acff1be628955cfb262499df2b62 100644 --- a/src/Services/FunctionManagementService.cpp +++ b/src/Services/FunctionManagementService.cpp @@ -7,8 +7,8 @@ void FunctionManagementService::call(Message& msg) { ErrorHandler::assertRequest(msg.messageType == 1, msg, ErrorHandler::AcceptanceErrorType::UnacceptableMessage); ErrorHandler::assertRequest(msg.serviceType == 8, msg, ErrorHandler::AcceptanceErrorType::UnacceptableMessage); - uint8_t funcName[FUNC_NAME_LENGTH]; // the function's name - uint8_t funcArgs[MAX_ARG_LENGTH]; // arguments for the function + uint8_t funcName[FUNC_NAME_LENGTH] = { 0 }; // the function's name + uint8_t funcArgs[MAX_ARG_LENGTH] = { 0 }; // arguments for the function msg.readString(funcName, FUNC_NAME_LENGTH); msg.readString(funcArgs, MAX_ARG_LENGTH); diff --git a/src/main.cpp b/src/main.cpp index b5afecc46859bb5c874be7b497a1bfd2ad58e3e6..aed43a835568e32cffa5c32ba04f5dcbcb8ce5a6 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -32,7 +32,7 @@ int main() { std::cout << std::hex << packet.data << std::endl; // packet data must be 'helloQQ' char string[6]; - packet.readString(string, 5); + packet.readCString(string, 5); std::cout << "Word: " << string << " " << packet.readBits(15) << packet.readBits(1) << std::endl; std::cout << packet.readFloat() << " " << std::dec << packet.readSint32() << std::endl; diff --git a/test/Message.cpp b/test/Message.cpp index 1caa5a2624377bc123c9dd7192e27104cd053afe..25f098322d48d4ec9b80f8fce62937653b22b9ac 100644 --- a/test/Message.cpp +++ b/test/Message.cpp @@ -160,7 +160,7 @@ TEST_CASE("Requirement 7.3.8 (Octet-string)", "[message][ecss]") { REQUIRE(message.dataSize == 4); char string[5]; - message.readString(string, 4); + message.readCString(string, 4); CHECK_THAT(string, Catch::Matchers::Equals("test")); } diff --git a/test/Services/EventReportService.cpp b/test/Services/EventReportService.cpp index e87f7e4098713f9ea507ad5fb0a223a8c9a3b994..ab3ae8aae4249a44656cdd7a6599c5ee5723727f 100644 --- a/test/Services/EventReportService.cpp +++ b/test/Services/EventReportService.cpp @@ -20,7 +20,7 @@ TEST_CASE("Informative Event Report TM[5,1]", "[service][st05]") { REQUIRE(report.dataSize == 12); // Check for the value that is stored in <<data>> array(data-member of object response) CHECK(report.readEnum16() == 0); - report.readString(checkString, 10); + report.readCString(checkString, 10); CHECK(strcmp(checkString, eventReportData) == 0); } @@ -38,7 +38,7 @@ TEST_CASE("Low Severity Anomaly Report TM[5,2]", "[service][st05]") { REQUIRE(report.dataSize == 12); // Check for the value that is stored in <<data>> array(data-member of object response) CHECK(report.readEnum16() == 4); - report.readString(checkString, 10); + report.readCString(checkString, 10); CHECK(strcmp(checkString, eventReportData) == 0); } @@ -56,7 +56,7 @@ TEST_CASE("Medium Severity Anomaly Report TM[5,3]", "[service][st05]") { REQUIRE(report.dataSize == 12); // Check for the value that is stored in <<data>> array(data-member of object response) CHECK(report.readEnum16() == 5); - report.readString(checkString, 10); + report.readCString(checkString, 10); CHECK(strcmp(checkString, eventReportData) == 0); } @@ -74,7 +74,7 @@ TEST_CASE("High Severity Anomaly Report TM[5,4]", "[service][st05]") { REQUIRE(report.dataSize == 12); // Check for the value that is stored in <<data>> array(data-member of object response) CHECK(report.readEnum16() == 6); - report.readString(checkString, 10); + report.readCString(checkString, 10); CHECK(strcmp(checkString, eventReportData) == 0); } diff --git a/test/Services/FunctionManagementService.cpp b/test/Services/FunctionManagementService.cpp index cb7e7e31accac7bf328e9cd5b00f81c07d033ff6..45088a8670351de5ca7efbe02057b739714565b8 100644 --- a/test/Services/FunctionManagementService.cpp +++ b/test/Services/FunctionManagementService.cpp @@ -6,31 +6,58 @@ FunctionManagementService& fms = Services.functionManagement; +uint8_t globalVariable = 10; + void test(String<MAX_ARG_LENGTH> a) { - std::cout << a.c_str() << std::endl; + globalVariable = a[0]; } TEST_CASE("ST[08] - Call Tests") { + SECTION("Function call") { + ServiceTests::reset(); + globalVariable = 10; + + fms.include(String<FUNC_NAME_LENGTH>("test"), &test); + Message msg(8, 1, Message::TC, 1); + + msg.appendFixedString(String<FUNC_NAME_LENGTH>("test")); + msg.appendByte(199); + MessageParser::execute(msg); + + CHECK(ServiceTests::hasNoErrors()); + CHECK(globalVariable == 199); + } + SECTION("Malformed name") { ServiceTests::reset(); + globalVariable = 10; + fms.include(String<FUNC_NAME_LENGTH>("test"), &test); Message msg(8, 1, Message::TC, 1); - msg.appendString(String<FUNC_NAME_LENGTH>("t3st")); + msg.appendFixedString(String<FUNC_NAME_LENGTH>("t3st")); MessageParser::execute(msg); + CHECK(ServiceTests::get(0).messageType == 4); CHECK(ServiceTests::get(0).serviceType == 1); + CHECK(ServiceTests::countErrors() == 1); + CHECK(globalVariable == 10); } SECTION("Too long message") { ServiceTests::reset(); + globalVariable = 10; + fms.include(String<FUNC_NAME_LENGTH>("test"), &test); Message msg(8, 1, Message::TC, 1); - msg.appendString(String<FUNC_NAME_LENGTH>("test")); + msg.appendFixedString(String<FUNC_NAME_LENGTH>("test")); msg.appendString(String<65> ("eqrhjweghjhwqgthjkrghthjkdsfhgsdfhjsdjsfdhgkjdfsghfjdgkdfsgdfgsgd")); MessageParser::execute(msg); + CHECK(ServiceTests::get(0).messageType == 4); CHECK(ServiceTests::get(0).serviceType == 1); + CHECK(ServiceTests::countErrors() == 2); + CHECK(globalVariable == 10); } }