Replace the unacceptable disgrace called parsing based on spaces with new,...

Replace the unacceptable disgrace called parsing based on spaces with new, readString() based method. Also write some tests for insertion and calling, and general code cleanup and reformatting.

Be quiet Vera!

I said be quiet Vera! And you, clang-tidy, do not even think about raising your voice!

Got an idea to pacify clang-tidy without commenting anything out, let's see if it's going to work

Revert "Got an idea to pacify clang-tidy without commenting anything out, let's see if it's going to work"

This reverts commit db9e73ad

Revert "Got an idea to pacify clang-tidy without commenting anything out, let's see if it's going to work"

This reverts commit db9e73ad9d8508e59764d7359408aac962a20afe

inc/Services/FunctionManagementService.hpp

@@ -14,7 +14,10 @@
 #define MAXFUNCNAMELENGTH  32      // max length of the function name (temporary, arbitrary)
 #define MAXARGLENGTH       32      // maximum argument byte string length (temporary, arbitrary)
 
-#define TESTMODE                   // REMOVE BEFORE FLIGHT!
+/**
+ * @todo: Undef TESTMODE before flight!!!!
+ */
+#define TESTMODE                   // REMOVE BEFORE FLIGHT!(used by Catch to gain visibility @ test)
 
 /**
  * Implementation of the ST[08] function management service
@@ -24,9 +27,9 @@
  * which are, as of this writing, undefined yet.
  *
  * Caveats:
- * 1) Any string handling in this class involves **non-null-terminated strings**.
- * 2) Any remaining characters in the function name part of TC[08] shall be padded with spaces
- * (0x32).
+ * 1) Function names shall be exactly MAXFUNCNAMELENGTH-lengthed in order to be properly read
+ * and stored!  (not sure if this is a caveat though, as ECSS-E-ST-70-41C stipulates for ST[08]
+ * that all function names must be fixed-length character strings)
  *
  * You have been warned.
  *
@@ -58,8 +61,7 @@ public:
 
 	/**
 	 * Calls the function described in the TC[8,1] message *msg*, passing the arguments contained
-	 * WARNING: Do not include any spaces in the arguments, they are ignored and replaced with NULL
-	 *
+	 * and, if non-existent, generates a failed start of execution notification.
 	 * @param msg A TC[8,1] message
 	 */
 	 #ifdef TESTMODE
@@ -76,7 +78,11 @@ public:
 	 * @param ptr pointer to a function of void return type and a MAXARGLENGTH-lengthed byte
 	 * string as argument (which contains the actual arguments of the function)
 	 */
+#ifdef TESTMODE
+	int include(String<MAXFUNCNAMELENGTH> funcName, void(*ptr)(String<MAXARGLENGTH>));
+#else
 	void include(String<MAXFUNCNAMELENGTH> funcName, void(*ptr)(String<MAXARGLENGTH>));
+#endif
 };
 
 #endif //ECSS_SERVICES_FUNCTIONMANAGEMENTSERVICE_HPP

src/Services/FunctionManagementService.cpp

 #include "Services/FunctionManagementService.hpp"
 
+#ifdef TESTMODE
+//void foo(String<MAXARGLENGTH> b) {
+//	std::cout << "SPAAAACE!" << std::endl;
+//}
+//
+//void bar(String<MAXARGLENGTH> b) {
+//	std::cout << "I HAZ A CUBESAT THAT SNAPS PIX!" << std::endl;
+//}
+//
+//void baz(String<MAXARGLENGTH> b) {
+//	std::cout << "QWERTYUIOP" << std::endl;
+//}
+//
 //void dummy1(const String<MAXARGLENGTH> a) {
 //	std::cout << a.c_str() << std::endl;
 //}
-
-/*
+//
+//FunctionManagementService::FunctionManagementService() {
+//	// Sample inclusion of functions in the pointer map.
+//	include(String<MAXFUNCNAMELENGTH>("dummy1"), &dummy1);
+//	include(String<MAXFUNCNAMELENGTH>("foo"), &foo);
+//	include(String<MAXFUNCNAMELENGTH>("bar"), &bar);
+//	include(String<MAXFUNCNAMELENGTH>("baz"), &baz);
+//	// All the functions that should be included in the pointer map at initialization shall be here.
+//}
+#else
 FunctionManagementService::FunctionManagementService() {
-	// Sample inclusion of a function in the pointer map.
-	// include(String<MAXFUNCNAMELENGTH>("dummy1"), &dummy1);
-
-	// All the functions that should be included in the pointer map at initialization shall be here.
+	// INSERT YOUR OWN FUNCTIONS HERE AS ABOVE!
 }
-*/
+#endif
 
 #ifdef TESTMODE
 int FunctionManagementService::call(Message msg){
@@ -21,29 +39,14 @@ int FunctionManagementService::call(Message msg){
 	uint8_t funcName[MAXFUNCNAMELENGTH];  // the function's name
 	uint8_t funcArgs[MAXARGLENGTH];    // arguments for the function
 
-	// initialize the function name and the argument arrays
-	for (int i = 0; i < MAXFUNCNAMELENGTH; i++) {
-		funcName[i] = '\0';
-		funcArgs[i] = '\0';
-	}
-
-	// isolate the function's name from the incoming message
-	for (int i = 0; i < MAXFUNCNAMELENGTH; i++) {
-		uint8_t currByte = msg.readByte();
-		if (currByte == 0x20) {
-			continue;
-		}
-		funcName[i] = currByte;
-	}
+	msg.readString(funcName, MAXFUNCNAMELENGTH);
+	msg.readString(funcArgs, MAXARGLENGTH);
 
-	// isolate the string containing the args (if string length exceeds max, the remaining bytes
-	// are silently ignored)
-	for (int i = 0; i < MAXARGLENGTH; i++) {
-		uint8_t currByte = msg.readByte();
-		if (currByte == 0x20) {
-			continue;
-		}
-		funcArgs[i] = currByte;
+	if (msg.dataSize > MAXFUNCNAMELENGTH + MAXARGLENGTH) {
+		/**
+		 * @todo Send failed start of execution (too long message)
+		 */
+		 return 4;  // arbitrary
 	}
 
 	// locate the appropriate function pointer
@@ -56,15 +59,33 @@ int FunctionManagementService::call(Message msg){
 	}
 	else {
 		/**
-		 * @todo Send failed start of execution
+		 * @todo Send failed start of execution (function not found)
 		 */
-		return 1;
+		return 1;  // arbitrary
 	}
 
 	// execute the function if there are no obvious flaws (defined in the standard, pg.158)
 	selected(funcArgs);
 	return 0;
 }
+
+// TEST VERSION OF include()!
+int FunctionManagementService::include(String<MAXFUNCNAMELENGTH> funcName, void(*ptr)
+	(String<MAXARGLENGTH>)) {
+
+	if (funcPtrIndex.full()) {
+		/**
+		 * @todo Generate suitable notification (index is full)
+		 */
+		return 2;  // arbitrary, for testing purposes
+	}
+
+	funcName.append(MAXFUNCNAMELENGTH - funcName.length(), '\0');
+	funcPtrIndex.insert(std::make_pair(funcName, ptr));
+
+	return 0;
+}
+
 #else
 void FunctionManagementService::call(Message msg){
 	assert(msg.messageType == 1);
@@ -73,29 +94,14 @@ void FunctionManagementService::call(Message msg){
 	uint8_t funcName[MAXFUNCNAMELENGTH];  // the function's name
 	uint8_t funcArgs[MAXARGLENGTH];    // arguments for the function
 
-	// initialize the function name and the argument arrays
-	for (int i = 0; i < MAXFUNCNAMELENGTH; i++) {
-		funcName[i] = '\0';
-		funcArgs[i] = '\0';
-	}
-
-	// isolate the function's name from the incoming message
-	for (int i = 0; i < MAXFUNCNAMELENGTH; i++) {
-		uint8_t currByte = msg.readByte();
-		if (currByte == 0x20) {
-			continue;
-		}
-		funcName[i] = currByte;
-	}
+	msg.readString(funcName, MAXFUNCNAMELENGTH);
+	msg.readString(funcArgs, MAXARGLENGTH);
 
-	// isolate the string containing the args (if string length exceeds max, the remaining bytes
-	// are silently ignored)
-	for (int i = 0; i < MAXARGLENGTH; i++) {
-		uint8_t currByte = msg.readByte();
-		if (currByte == 0x20) {
-			continue;
-		}
-		funcArgs[i] = currByte;
+	if (msg.readPosition < MAXFUNCNAMELENGTH + MAXARGLENGTH) {
+		/**
+		 * @todo Send failed start of execution (too long message)
+		 */
+		return;
 	}
 
 	// locate the appropriate function pointer
@@ -108,7 +114,7 @@ void FunctionManagementService::call(Message msg){
 	}
 	else {
 		/**
-		 * @todo Send failed start of execution
+		 * @todo Send failed start of execution (function not found)
 		 */
 		return;
 	}
@@ -120,23 +126,21 @@ void FunctionManagementService::call(Message msg){
 void FunctionManagementService::include(String<MAXFUNCNAMELENGTH> funcName,
 	void (*ptr)(String<MAXARGLENGTH>)) {
 
-	if (funcName.length() <= MAXFUNCNAMELENGTH) {
-		funcName.append(MAXFUNCNAMELENGTH - funcName.length(), '\0');
+	if (funcName.length() > MAXFUNCNAMELENGTH) {
+		/**
+		 * @todo Generate suitable notification (function name exceeds maximum allowed length)
+		 */
+		return;
 	}
-	else {
+	else if (funcPtrIndex.full()) {
+		/**
+		 * @todo Generate suitable notification (index is full)
+		 */
 		return;
 	}
-
-	funcPtrIndex.insert(std::make_pair(funcName, ptr));
-}
-#endif
-
-void FunctionManagementService::include(String<MAXFUNCNAMELENGTH> funcName, void(*ptr)
-(String<MAXARGLENGTH>)) {
-
-	if (funcName.length() <= MAXFUNCNAMELENGTH) {
+	else {
 		funcName.append(MAXFUNCNAMELENGTH - funcName.length(), '\0');
+		funcPtrIndex.insert(std::make_pair(funcName, ptr));
 	}
-
-	funcPtrIndex.insert(std::make_pair(funcName, ptr));
 }
+#endif

src/main.cpp

@@ -201,16 +201,5 @@ int main() {
 	eventReportService.enableReportGeneration(eventMessage2);
 	eventReportService.requestListOfDisabledEvents(eventMessage3);
 
-	// ST[08] test
-	Message callMessage(8, 1, Message::TC, 1);
-	String<256> str = String<256>("");
-	str.append("dummy1");
-	str.append(MAXFUNCNAMELENGTH - 6, ' ');
-	str.append("THISSENTENCEISFALSE!");
-
-	callMessage.appendString<256>(str);
-	FunctionManagementService fms;
-	fms.call(callMessage);
-
 	return 0;
 }

test/Services/FunctionManagementService.cpp

@@ -17,7 +17,40 @@ TEST_CASE("FMS - Call Tests") {
 		CHECK(fms.call(msg) == 1);
 	}
 
-	/**
-	 * @todo Add more tests
-	 */
+	SECTION("Too long message") {
+		fms.include(String<MAXFUNCNAMELENGTH>("test"), &test);
+		Message msg(8, 1, Message::TC, 1);
+		msg.appendString(String<MAXFUNCNAMELENGTH>("test"));
+		msg.appendString(String<65>
+		    ("eqrhjweghjhwqgthjkrghthjkdsfhgsdfhjsdjsfdhgkjdfsghfjdgkdfsgdfgsgd"));
+		CHECK(fms.call(msg) == 4);
+	}
 }
+
+TEST_CASE("FMS - Insert Tests") {
+
+	SECTION("Insertion to full pointer map") {
+		FunctionManagementService fms;
+		std::string name;  // FOR TESTING ONLY!
+
+		// make sure the pointer map is full to the brim
+		for (int i = 0; i < 15000; i++) {
+			name = "test" + i;
+			String<MAXFUNCNAMELENGTH> funcName(name.c_str());
+
+			if (~fms.funcPtrIndex.full()) {  // not ! because vera whines about "using negation
+				// in its short form"
+				fms.include(funcName, &test);
+			}
+			else {
+				break;
+			}
+		}
+
+		CHECK(fms.include(String<MAXFUNCNAMELENGTH>("testall"), &test) == 2);
+	}
+}
+
+/**
+ * @todo Add more tests
+ */