From ea8fc901dbeaf68f0c7548f1cdbea9ea7a1103dd Mon Sep 17 00:00:00 2001
From: Grigoris Pavlakis <grigpavl@ece.auth.gr>
Date: Sat, 5 Jan 2019 20:04:32 +0200
Subject: [PATCH] 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 | 18 ++-
src/Services/FunctionManagementService.cpp | 140 ++++++++++----------
src/main.cpp | 11 --
test/Services/FunctionManagementService.cpp | 39 +++++-
4 files changed, 120 insertions(+), 88 deletions(-)
diff --git a/inc/Services/FunctionManagementService.hpp b/inc/Services/FunctionManagementService.hpp
index d2bba0ea..3060989f 100644
--- a/inc/Services/FunctionManagementService.hpp
+++ b/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
diff --git a/src/Services/FunctionManagementService.cpp b/src/Services/FunctionManagementService.cpp
index 757346ab..0ec37aaf 100644
--- a/src/Services/FunctionManagementService.cpp
+++ b/src/Services/FunctionManagementService.cpp
@@ -1,17 +1,35 @@
#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
diff --git a/src/main.cpp b/src/main.cpp
index 37188eb7..492dabf3 100644
--- a/src/main.cpp
+++ b/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;
}
diff --git a/test/Services/FunctionManagementService.cpp b/test/Services/FunctionManagementService.cpp
index e85bdd83..79d4b958 100644
--- a/test/Services/FunctionManagementService.cpp
+++ b/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
+ */
--
GitLab