From e13942597562b9d18e8a7a0493c14ebec3db2da5 Mon Sep 17 00:00:00 2001
From: Grigoris Pavlakis <grigpavl@ece.auth.gr>
Date: Wed, 2 Jan 2019 23:35:56 +0200
Subject: [PATCH] Add a test case. More to come.

Fix a blunder with the test version of call()

Attempt to calm down clang-tidy and fix some stupid warnings in my CRC code (nothing much, just turned the counter int in a for-loop into a uint32_t to shut gcc up)

Comment out the empty constructor to mute clang-tidy's warning about trivial constructors
---
 CMakeLists.txt                              |   2 +-
 inc/Services/FunctionManagementService.hpp  |  32 +++++-
 src/Helpers/CRCHelper.cpp                   |   2 +-
 src/Services/FunctionManagementService.cpp  | 102 +++++++++++++++++---
 test/Services/FunctionManagementService.cpp |  23 +++++
 5 files changed, 141 insertions(+), 20 deletions(-)
 create mode 100644 test/Services/FunctionManagementService.cpp

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 6752c03c..e675cca2 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -44,6 +44,6 @@ IF (EXISTS "${PROJECT_SOURCE_DIR}/lib/Catch2/CMakeLists.txt")
     add_executable(tests
             $<TARGET_OBJECTS:common>
             ${test_main_SRC}
-            ${test_SRC})
+            ${test_SRC} test/Services/FunctionManagementService.cpp)
     target_link_libraries(tests Catch2::Catch2)
 ENDIF ()
diff --git a/inc/Services/FunctionManagementService.hpp b/inc/Services/FunctionManagementService.hpp
index add0b031..d2bba0ea 100644
--- a/inc/Services/FunctionManagementService.hpp
+++ b/inc/Services/FunctionManagementService.hpp
@@ -14,6 +14,8 @@
 #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!
+
 /**
  * Implementation of the ST[08] function management service
  *
@@ -23,6 +25,8 @@
  *
  * 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).
  *
  * You have been warned.
  *
@@ -30,29 +34,49 @@
  */
 
 typedef String<MAXFUNCNAMELENGTH> functionName;
-typedef etl::map<functionName, void(*)(String<MAXARGLENGTH>), (const size_t) FUNCMAPSIZE>
+typedef etl::map<functionName, void(*)(String<MAXARGLENGTH>), FUNCMAPSIZE>
 PointerMap;
 
 class FunctionManagementService {
 	/**
 	 * Map of the function names to their respective pointers. Size controlled by FUNCMAPSIZE
 	 */
-	 PointerMap funcPtrIndex;
+#ifdef TESTMODE
+public: PointerMap funcPtrIndex;
+#else
+	PointerMap funcPtrIndex;
+#endif
 
 public:
 	/**
 	 * Constructs the function pointer index with all the necessary functions at initialization time
-	 * These functions need to be in scope.
+	 * These functions need to be in scope. Uncomment when needed.
+	 *
 	 * @param None
 	 */
-	FunctionManagementService();
+	//FunctionManagementService();
 
 	/**
 	 * 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
+	 *
 	 * @param msg A TC[8,1] message
 	 */
+	 #ifdef TESTMODE
+	int call(Message msg);
+	 #else
 	void call(Message msg);
+	 #endif
+
+	/**
+	 * Includes a new function in the pointer map. This enables it to be called by way of a valid
+	 * TC [8,1] message.
+	 *
+	 * @param funcName the function's name. Max. length is MAXFUNCNAMELENGTH bytes.
+	 * @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)
+	 */
+	void include(String<MAXFUNCNAMELENGTH> funcName, void(*ptr)(String<MAXARGLENGTH>));
 };
 
 #endif //ECSS_SERVICES_FUNCTIONMANAGEMENTSERVICE_HPP
diff --git a/src/Helpers/CRCHelper.cpp b/src/Helpers/CRCHelper.cpp
index e0cd4ebf..83614550 100644
--- a/src/Helpers/CRCHelper.cpp
+++ b/src/Helpers/CRCHelper.cpp
@@ -9,7 +9,7 @@ uint16_t CRCHelper::calculateCRC(const uint8_t* message, uint32_t length) {
 	// CRC16-CCITT generator polynomial (as specified in standard)
 	uint16_t polynomial = 0x1021u;
 
-	for (int i = 0; i < length; i++) {
+	for (uint32_t i = 0; i < length; i++) {
 		// "copy" (XOR w/ existing contents) the current msg bits into the MSB of the shift register
 		shiftReg ^= (message[i] << 8u);
 
diff --git a/src/Services/FunctionManagementService.cpp b/src/Services/FunctionManagementService.cpp
index 99d749f9..757346ab 100644
--- a/src/Services/FunctionManagementService.cpp
+++ b/src/Services/FunctionManagementService.cpp
@@ -1,20 +1,20 @@
 #include "Services/FunctionManagementService.hpp"
 
-// Dummy functions which will populate the map.
-// This one prints whatever bytes are contained in the argument.
-void dummy1(const String<MAXARGLENGTH> a) {
-	std::cout << a.c_str() << std::endl;
-}
+//void dummy1(const String<MAXARGLENGTH> a) {
+//	std::cout << a.c_str() << std::endl;
+//}
 
+/*
 FunctionManagementService::FunctionManagementService() {
-	String<MAXFUNCNAMELENGTH> str("");
-	str.append("dummy1");
-	str.append(MAXFUNCNAMELENGTH - 6, '\0');
-	void(*dummyPtr)(String<MAXARGLENGTH>) = &dummy1;
-	funcPtrIndex.insert(std::make_pair(str, dummyPtr));
+	// 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.
 }
+*/
 
-void FunctionManagementService::call(Message msg){
+#ifdef TESTMODE
+int FunctionManagementService::call(Message msg){
 	assert(msg.messageType == 1);
 	assert(msg.serviceType == 8);
 
@@ -49,14 +49,64 @@ void FunctionManagementService::call(Message msg){
 	// locate the appropriate function pointer
 	String<MAXFUNCNAMELENGTH> name(funcName);
 	PointerMap::iterator iter = funcPtrIndex.find(name);
-	void(*selected)(String<MAXARGLENGTH>) = nullptr;
+	void(*selected)(String<MAXARGLENGTH>);
 
 	if (iter != funcPtrIndex.end()) {
 		selected = *iter->second;
 	}
+	else {
+		/**
+		 * @todo Send failed start of execution
+		 */
+		return 1;
+	}
+
+	// execute the function if there are no obvious flaws (defined in the standard, pg.158)
+	selected(funcArgs);
+	return 0;
+}
+#else
+void FunctionManagementService::call(Message msg){
+	assert(msg.messageType == 1);
+	assert(msg.serviceType == 8);
 
-	// send proper exec failure notification and terminate if name is incorrect
-	if (selected == nullptr) {
+	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;
+	}
+
+	// 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;
+	}
+
+	// locate the appropriate function pointer
+	String<MAXFUNCNAMELENGTH> name(funcName);
+	PointerMap::iterator iter = funcPtrIndex.find(name);
+	void(*selected)(String<MAXARGLENGTH>);
+
+	if (iter != funcPtrIndex.end()) {
+		selected = *iter->second;
+	}
+	else {
 		/**
 		 * @todo Send failed start of execution
 		 */
@@ -66,3 +116,27 @@ void FunctionManagementService::call(Message msg){
 	// execute the function if there are no obvious flaws (defined in the standard, pg.158)
 	selected(funcArgs);
 }
+
+void FunctionManagementService::include(String<MAXFUNCNAMELENGTH> funcName,
+	void (*ptr)(String<MAXARGLENGTH>)) {
+
+	if (funcName.length() <= MAXFUNCNAMELENGTH) {
+		funcName.append(MAXFUNCNAMELENGTH - funcName.length(), '\0');
+	}
+	else {
+		return;
+	}
+
+	funcPtrIndex.insert(std::make_pair(funcName, ptr));
+}
+#endif
+
+void FunctionManagementService::include(String<MAXFUNCNAMELENGTH> funcName, void(*ptr)
+(String<MAXARGLENGTH>)) {
+
+	if (funcName.length() <= MAXFUNCNAMELENGTH) {
+		funcName.append(MAXFUNCNAMELENGTH - funcName.length(), '\0');
+	}
+
+	funcPtrIndex.insert(std::make_pair(funcName, ptr));
+}
diff --git a/test/Services/FunctionManagementService.cpp b/test/Services/FunctionManagementService.cpp
new file mode 100644
index 00000000..e85bdd83
--- /dev/null
+++ b/test/Services/FunctionManagementService.cpp
@@ -0,0 +1,23 @@
+#include "catch2/catch.hpp"
+#include "Services/FunctionManagementService.hpp"
+
+#define CATCH_CONFIG_MAIN
+
+void test(String<MAXARGLENGTH> a) {
+	std::cout << a.c_str() << std::endl;
+}
+
+TEST_CASE("FMS - Call Tests") {
+	FunctionManagementService fms;
+
+	SECTION("Malformed name") {
+		fms.include(String<MAXFUNCNAMELENGTH>("test"), &test);
+		Message msg(8, 1, Message::TC, 1);
+		msg.appendString(String<MAXFUNCNAMELENGTH>("t3st"));
+		CHECK(fms.call(msg) == 1);
+	}
+
+	/**
+	 * @todo Add more tests
+	 */
+}
-- 
GitLab