From d7da8a764f89de3321ff6a931d083ac61007bd39 Mon Sep 17 00:00:00 2001 From: Sergey Zalesskiy Date: Tue, 7 Apr 2020 14:31:12 +0300 Subject: [PATCH 1/5] * Made command buffer local for getOutCmd - saved 65 bytes --- CommandHandler.cpp | 2 +- CommandHandler.h | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/CommandHandler.cpp b/CommandHandler.cpp index 624c5ff..48b2de7 100644 --- a/CommandHandler.cpp +++ b/CommandHandler.cpp @@ -502,8 +502,8 @@ void CommandHandler::addCmdString(const char *value) { char* CommandHandler::getOutCmd() { + char command[COMMANDHANDLER_BUFFER + 1]; commandString.toCharArray(command, COMMANDHANDLER_BUFFER + 1); - return command; } diff --git a/CommandHandler.h b/CommandHandler.h index dda71ea..8992341 100644 --- a/CommandHandler.h +++ b/CommandHandler.h @@ -135,7 +135,6 @@ class CommandHandler { char remains[COMMANDHANDLER_BUFFER + 1]; // Buffer of stored characters to pass to a relay function - char command[COMMANDHANDLER_BUFFER + 1]; String commandString; // Out Command String commandHeader; // header for out command byte commandDecimal; From 812bb1a1fe191f976787a7a32e4dc84461064dac Mon Sep 17 00:00:00 2001 From: Sergey Zalesskiy Date: Tue, 7 Apr 2020 19:39:27 +0300 Subject: [PATCH 2/5] * Added extra debug logging & free RAM reporting --- CommandHandler.cpp | 17 +++++++++++++++++ CommandHandler.h | 6 +++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/CommandHandler.cpp b/CommandHandler.cpp index 48b2de7..e4d4eac 100644 --- a/CommandHandler.cpp +++ b/CommandHandler.cpp @@ -183,6 +183,13 @@ void CommandHandler::processChar(char inChar) { // Execute the stored handler function for the command (*commandList[i].function)(); + + #ifdef COMMANDHANDLER_DEBUG + int free_mem = (int) AVR_STACK_POINTER_REG - (int) __brkval; + Serial.print("Free SRAM: "); + Serial.println(free_mem); + #endif + matched = true; break; } @@ -206,6 +213,12 @@ void CommandHandler::processChar(char inChar) { // Execute the stored handler function for the command (*relayList[i].function)(remaining(), relayList[i].pt2Object); + + #ifdef COMMANDHANDLER_DEBUG + int free_mem = (int) AVR_STACK_POINTER_REG - (int) __brkval; + Serial.print("Free SRAM: "); + Serial.println(free_mem); + #endif matched = true; break; } @@ -225,6 +238,10 @@ void CommandHandler::processChar(char inChar) { buffer[bufPos] = inChar; // Put character into buffer buffer[bufPos+1] = STRING_NULL_TERM; // Null terminate bufPos++; + #ifdef COMMANDHANDLER_DEBUG + Serial.print("Current buffer: "); + Serial.println(buffer); + #endif } else { #ifdef COMMANDHANDLER_DEBUG Serial.println("Line buffer is full - increase COMMANDHANDLER_BUFFER"); diff --git a/CommandHandler.h b/CommandHandler.h index 8992341..f4aef3d 100644 --- a/CommandHandler.h +++ b/CommandHandler.h @@ -46,7 +46,11 @@ #define STRING_NULL_TERM '\0' // Uncomment the next line to run the library in debug mode (verbose messages) -// #define COMMANDHANDLER_DEBUG +//#define COMMANDHANDLER_DEBUG + +#ifdef COMMANDHANDLER_DEBUG +extern void *__brkval; +#endif class CommandHandler { From 02f6f4bda97ead81fbc9bd94c9c07370d8e71f85 Mon Sep 17 00:00:00 2001 From: Sergey Zalesskiy Date: Tue, 7 Apr 2020 19:40:45 +0300 Subject: [PATCH 3/5] * Replaced remaining() with inline dynamic buffer inside processChar() --- CommandHandler.cpp | 46 ++++++++++++++-------------------------------- CommandHandler.h | 2 -- 2 files changed, 14 insertions(+), 34 deletions(-) diff --git a/CommandHandler.cpp b/CommandHandler.cpp index e4d4eac..a9f7945 100644 --- a/CommandHandler.cpp +++ b/CommandHandler.cpp @@ -211,14 +211,25 @@ void CommandHandler::processChar(char inChar) { Serial.println(command); #endif + // Get remaining part of the command buffer + // Max length minus command (at least 1 byte) + // And delimiter (at least 1 byte) + char remaining[COMMANDHANDLER_BUFFER-2]; + strcpy(remaining, last); + // Terminate it with terminator and null byte + char eol[2] = {term, STRING_NULL_TERM}; + strcat(remaining, eol); + // Clear buffer so that relay command can use it if needed + clearBuffer(); // Execute the stored handler function for the command - (*relayList[i].function)(remaining(), relayList[i].pt2Object); + (*relayList[i].function)(remaining, relayList[i].pt2Object); #ifdef COMMANDHANDLER_DEBUG int free_mem = (int) AVR_STACK_POINTER_REG - (int) __brkval; Serial.print("Free SRAM: "); Serial.println(free_mem); #endif + matched = true; break; } @@ -266,37 +277,6 @@ char *CommandHandler::next() { return strtok_r(NULL, delim, &last); } -/** - * Returns char* of the remaining of the command buffer (for getting arguments to commands). - * Returns NULL if no more tokens exist. - */ -char *CommandHandler::remaining() { - - //reinit the remains char - remains[0] = STRING_NULL_TERM; - - char str_term[2]; - str_term[0] = term; - str_term[1] = STRING_NULL_TERM; - - // Search for the remaining up to next term - char *command = strtok_r(NULL, str_term, &last); - - // forge term in string format - strcpy(remains, command); - strcat(remains, str_term); - - // clear the buffer now, we emptied the current command - // the remaining is might be given to another handler - // or it might used by the same commandHandler instance - // hence the buffer should be emptied now - clearBuffer(); - - return remains; -} - - - /***************************************** * Helpers to read args and cast them into specific type, strongly inspired by CmdMessenger *****************************************/ @@ -534,4 +514,6 @@ void CommandHandler::sendCmdSerial() { void CommandHandler::sendCmdSerial(Stream &outStream) { outStream.print(commandString); + // FIXME string not set to null, potential memory leak here + // if string construction is not started from initCmd() } diff --git a/CommandHandler.h b/CommandHandler.h index f4aef3d..4d7071c 100644 --- a/CommandHandler.h +++ b/CommandHandler.h @@ -137,8 +137,6 @@ class CommandHandler { byte bufPos; // Current position in the buffer char *last; // State variable used by strtok_r during processing - char remains[COMMANDHANDLER_BUFFER + 1]; // Buffer of stored characters to pass to a relay function - String commandString; // Out Command String commandHeader; // header for out command byte commandDecimal; From b993344c4ddf3572d2c21b0bd33c588fef8bdf2d Mon Sep 17 00:00:00 2001 From: Sergey Zalesskiy Date: Tue, 7 Apr 2020 19:41:07 +0300 Subject: [PATCH 4/5] * Updated example sketch to use remaining() functionality through relay functions. --- examples/Demo/Demo.ino | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/examples/Demo/Demo.ino b/examples/Demo/Demo.ino index 12788d5..2d691d5 100644 --- a/examples/Demo/Demo.ino +++ b/examples/Demo/Demo.ino @@ -18,8 +18,8 @@ void setup() { // Setup callbacks for SerialCommand commands cmdHdl.addCommand("HELLO", sayHello); // Echos the string argument back - cmdHdl.addCommand("FWD", forwardRemaining);// Fwd the remaining of the command to the cmdHdl - cmdHdl.addCommand("P", processCommand); // Converts two arguments, first to double and echos them back + cmdHdl.addCommand("P", processCommand); // Converts two arguments, first to double and echos them back + cmdHdl.addRelay("FWD", fwdRelay); // An example of relay function that gets the whole substring as an arguments cmdHdl.addCommand("GUESS", guessMyName); // A game for guessing my name, used to test compareStringArg cmdHdl.addCommand("PING", pongMesssage); // A function that use the packet forging tool to send a random ping time cmdHdl.setDefaultHandler(unrecognized); // Handler for command that isn't matched (says "What?") @@ -86,12 +86,7 @@ void loop() { // Imagine your main program receiving "M1,P,2000;" command throught Serial // The main program could redirect the "P,2000;" string command to the a device called "M1" // The M1 device can then use its own commandHandler to handle the sub command "P,2000;" - // To extract the remaining of a command, you can use the function remaining() - // This is illustrated in the forwardRemaining() function - // that is associated to the FWD command in this example - // Send "FWD,P,2000;" to the board, it should forward the "P,2000;" to itself - // (thanks to the processString() function) - // As "P" is assocviated with the processCommand() function, it will run it + // This is achieved by the use of relay functions (see example below) // Below are a bunch of serial command you can try sending // Try predicting what it will do @@ -114,16 +109,12 @@ void sayHello() { } } -void forwardRemaining() { - char *remaining; - remaining = cmdHdl.remaining(); // Get the next argument from the SerialCommand object buffer - if (remaining != NULL) { // As long as it existed, take it - Serial.print("Forwarding "); - Serial.println(remaining); - cmdHdl.processString(remaining); - } else { - Serial.print("Nothing to forward"); - } +void fwdRelay(char* remaining){ + Serial.print("Relay function, got substring - "); + Serial.println(remaining); + // Here we can do whatever we want with the remaining substring. + // For example, we can extract another command and process it: + cmdHdl.processString(remaining); } void processCommand() { From 064499518e9cb82f57da5d81c060813b323b78b7 Mon Sep 17 00:00:00 2001 From: Sergey Zalesskiy Date: Thu, 9 Apr 2020 17:41:23 +0300 Subject: [PATCH 5/5] * Added terminator after debug memory printout so that it confuses Python side less during debugging --- CommandHandler.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CommandHandler.cpp b/CommandHandler.cpp index a9f7945..1a8babe 100644 --- a/CommandHandler.cpp +++ b/CommandHandler.cpp @@ -227,8 +227,8 @@ void CommandHandler::processChar(char inChar) { #ifdef COMMANDHANDLER_DEBUG int free_mem = (int) AVR_STACK_POINTER_REG - (int) __brkval; Serial.print("Free SRAM: "); - Serial.println(free_mem); - #endif + Serial.print(free_mem); + Serial.println(term); matched = true; break;