Review command processing restructure PR (#99)

* Review suggestions

* Remove command processing from USB library

---------

Co-authored-by: mikee47 <mike@silyhouse.net>

Sming/Components/CommandProcessing/component.mk

@@ -4,9 +4,6 @@ COMPONENT_SRCDIRS := \
 
 COMPONENT_INCDIRS := $(COMPONENT_SRCDIRS)
 
-
-COMPONENT_DEPENDS := FlashString
-
 COMPONENT_DOXYGEN_INPUT := src
 
 COMPONENT_DOCFILES := \

Sming/Components/CommandProcessing/samples/Arducam/app/ArduCamCommand.cpp

@@ -23,9 +23,9 @@ void ArduCamCommand::initCommand()
 void ArduCamCommand::showSettings(ReadWriteStream& commandOutput)
 {
 	// review settings
-	commandOutput.printf("ArduCam Settings:\n");
-	commandOutput.printf("    img Type: [%s]\n", getImageType());
-	commandOutput.printf("    img Size: [%s]\n", getImageSize());
+	commandOutput << _F("ArduCam Settings:") << endl
+				  << _F("    img Type: [") << getImageType() << ']' << endl
+				  << _F("    img Size: [") << getImageSize() << ']' << endl;
 };
 
 void ArduCamCommand::processSetCommands(String commandLine, ReadWriteStream& commandOutput)
@@ -61,7 +61,6 @@ void ArduCamCommand::processSetCommands(String commandLine, ReadWriteStream& com
 		showSettings(commandOutput);
 	}
 
-	// handle command ->   settings
 	else if(commandToken[1] == "size") {
 		if(numToken == 3) {
 			if(commandToken[2] == "160") {
@@ -109,7 +108,7 @@ void ArduCamCommand::processSetCommands(String commandLine, ReadWriteStream& com
 	}
 }
 
-void ArduCamCommand::setSize(String size)
+void ArduCamCommand::setSize(const String& size)
 {
 	if(size == "160x120") {
 		imgSize = OV2640_160x120;
@@ -151,26 +150,9 @@ void ArduCamCommand::setSize(String size)
 	}
 }
 
-void ArduCamCommand::setType(String type)
+void ArduCamCommand::setType(const String& type)
 {
-	if(type == "BMP") {
-		myCAM.set_format(BMP);
-		if(imgType != BMP) {
-			// reset the cam
-			myCAM.InitCAM();
-			imgType = BMP;
-			imgSize = OV2640_320x240;
-		}
-	} else {
-		myCAM.set_format(JPEG);
-		// reset the cam
-		if(imgType != JPEG) {
-			// reset the cam
-			myCAM.InitCAM();
-			myCAM.OV2640_set_JPEG_size(imgSize);
-			imgType = JPEG;
-		}
-	}
+	setFormat(type == "BMP" ? BMP : JPEG);
 }
 
 void ArduCamCommand::setFormat(uint8 type)

Sming/Components/CommandProcessing/samples/Arducam/app/application.cpp

@@ -1,7 +1,6 @@
 #include <SmingCore.h>
 #include <CommandProcessing/Utils.h>
 
-//#include "CamSettings.h"
 #include <ArduCamCommand.h>
 
 #include <Libraries/ArduCAM/ArduCAM.h>
@@ -42,7 +41,6 @@ HttpServer server;
 
 CommandProcessing::Handler commandHandler;
 
-HexDump hdump;
 ArduCAM myCAM(OV2640, CAM_CS);
 ArduCamCommand arduCamCommand(myCAM, commandHandler);
 

Sming/Components/CommandProcessing/samples/Arducam/include/ArduCamCommand.h

@@ -16,8 +16,8 @@ class ArduCamCommand
 	virtual ~ArduCamCommand();
 	void initCommand();
 	const char* getContentType();
-	void setSize(String size);
-	void setType(String type);
+	void setSize(const String& size);
+	void setType(const String& type);
 
 private:
 	bool status = true;

Sming/Components/CommandProcessing/samples/CommandLine/README.rst

@@ -1,4 +1,4 @@
 CommandLine
 ===========
 
-Demonstrates Sming's CommandProcessing capability via HTTP, FTP and serial interfaces.
+Demonstrates Sming's CommandProcessing capability via serial interface.

Sming/Components/CommandProcessing/samples/CommandLine/app/application.cpp

@@ -14,22 +14,22 @@ void processExampleCommand(String commandLine, ReadWriteStream& commandOutput)
 	Vector<String> commandToken;
 	int numToken = splitString(commandLine, ' ', commandToken);
 
+	// First token is "example"
 	if(numToken == 1) {
-		commandOutput.printf("Example Commands available : \r\n");
-		commandOutput.printf("on   : Set example status ON\r\n");
-		commandOutput.printf("off  : Set example status OFF\r\n");
-		commandOutput.printf("status : Show example status\r\n");
+		commandOutput << _F("Example Commands available :") << endl;
+		commandOutput << _F("on   : Set example status ON") << endl;
+		commandOutput << _F("off  : Set example status OFF") << endl;
+		commandOutput << _F("status : Show example status") << endl;
+	} else if(commandToken[1] == "on") {
+		exampleStatus = true;
+		commandOutput << _F("Status ON") << endl;
+	} else if(commandToken[1] == "off") {
+		exampleStatus = false;
+		commandOutput << _F("Status OFF") << endl;
+	} else if(commandToken[1] == "status") {
+		commandOutput << _F("Example Status is ") << (exampleStatus ? "ON" : "OFF") << endl;
 	} else {
-		if(commandToken[1] == "on") {
-			exampleStatus = true;
-			commandOutput.printf("Status ON\r\n");
-		} else if(commandToken[1] == "off") {
-			exampleStatus = false;
-			commandOutput.printf("Status OFF\r\n");
-		} else if(commandToken[1] == "status") {
-			String tempString = exampleStatus ? "ON" : "OFF";
-			commandOutput.printf("Example Status is %s\r\n", tempString.c_str());
-		};
+		commandOutput << _F("Bad command") << endl;
 	}
 }
 

Sming/Components/CommandProcessing/src/CommandProcessing/Handler.cpp

@@ -96,18 +96,15 @@ void Handler::processCommandLine(const String& cmdString)
 void Handler::registerSystemCommands()
 {
 	String system = F("system");
-	registerCommand(Command(F("status"), F("Displays System Information"), system,
-							Command::Callback(&Handler::procesStatusCommand, this)));
-	registerCommand(Command(F("echo"), F("Displays command entered"), system,
-							Command::Callback(&Handler::procesEchoCommand, this)));
-	registerCommand(Command(F("help"), F("Displays all available commands"), system,
-							Command::Callback(&Handler::procesHelpCommand, this)));
-	registerCommand(Command(F("debugon"), F("Set Serial debug on"), system,
-							Command::Callback(&Handler::procesDebugOnCommand, this)));
-	registerCommand(Command(F("debugoff"), F("Set Serial debug off"), system,
-							Command::Callback(&Handler::procesDebugOffCommand, this)));
-	registerCommand(Command(F("command"), F("Use verbose/silent/prompt as command options"), system,
-							Command::Callback(&Handler::processCommandOptions, this)));
+	registerCommand({F("status"), F("Displays System Information"), system, {&Handler::procesStatusCommand, this}});
+	registerCommand({F("echo"), F("Displays command entered"), system, {&Handler::procesEchoCommand, this}});
+	registerCommand({F("help"), F("Displays all available commands"), system, {&Handler::procesHelpCommand, this}});
+	registerCommand({F("debugon"), F("Set Serial debug on"), system, {&Handler::procesDebugOnCommand, this}});
+	registerCommand({F("debugoff"), F("Set Serial debug off"), system, {&Handler::procesDebugOffCommand, this}});
+	registerCommand({F("command"),
+					 F("Use verbose/silent/prompt as command options"),
+					 system,
+					 {&Handler::processCommandOptions, this}});
 }
 
 Command Handler::getCommandDelegate(const String& commandString)
@@ -150,35 +147,24 @@ void Handler::procesHelpCommand(String commandLine, ReadWriteStream& outputStrea
 {
 	debug_d("HelpCommand entered");
 	outputStream.println(_F("Commands available are :"));
-	for(unsigned idx = 0; idx < registeredCommands.count(); idx++) {
-		outputStream.print(registeredCommands.valueAt(idx).name);
-		outputStream.print(" | ");
-		outputStream.print(registeredCommands.valueAt(idx).group);
-		outputStream.print(" | ");
-		outputStream.print(registeredCommands.valueAt(idx).description);
-		outputStream.print("\r\n");
+	for(auto cmd : registeredCommands) {
+		outputStream << cmd->name << " | " << cmd->group << " | " << cmd->description << endl;
 	}
 }
 
 void Handler::procesStatusCommand(String commandLine, ReadWriteStream& outputStream)
 {
 	debug_d("StatusCommand entered");
-	outputStream.println(_F("Sming Framework Version : " SMING_VERSION));
-	outputStream.print(_F("ESP SDK version : "));
-	outputStream.print(system_get_sdk_version());
-	outputStream.println();
-	outputStream.print(_F("Time = "));
-	outputStream.print(SystemClock.getSystemTimeString());
-	outputStream.println();
-	outputStream.printf(_F("System Start Reason : %d\r\n"), system_get_rst_info()->reason);
+	outputStream << _F("Sming Framework Version : " SMING_VERSION) << endl;
+	outputStream << _F("ESP SDK version : ") << system_get_sdk_version() << endl;
+	outputStream << _F("Time = ") << SystemClock.getSystemTimeString() << endl;
+	outputStream << _F("System Start Reason : ") << system_get_rst_info()->reason << endl;
 }
 
 void Handler::procesEchoCommand(String commandLine, ReadWriteStream& outputStream)
 {
 	debug_d("HelpCommand entered");
-	outputStream.print(_F("You entered : '"));
-	outputStream.print(commandLine);
-	outputStream.println('\'');
+	outputStream << _F("You entered : '") << commandLine << '\'' << endl;
 }
 
 void Handler::procesDebugOnCommand(String commandLine, ReadWriteStream& outputStream)
@@ -223,23 +209,19 @@ void Handler::processCommandOptions(String commandLine, ReadWriteStream& outputS
 			break;
 		}
 		setCommandPrompt(commandToken[2]);
-		outputStream.print(_F("Prompt set to : "));
-		outputStream.print(commandToken[2]);
-		outputStream.println();
+		outputStream << _F("Prompt set to : ") << commandToken[2] << endl;
 		break;
 	default:
 		errorCommand = true;
 	}
 	if(errorCommand) {
-		outputStream.print(_F("Unknown command : "));
-		outputStream.print(commandLine);
-		outputStream.println();
+		outputStream << _F("Unknown command : ") << commandLine << endl;
 	}
 	if(printUsage) {
-		outputStream.println(_F("command usage : \r\n"));
-		outputStream.println(_F("command verbose : Set verbose mode"));
-		outputStream.println(_F("command silent : Set silent mode"));
-		outputStream.println(_F("command prompt 'new prompt' : Set prompt to use"));
+		outputStream << _F("command usage :") << endl
+					 << _F("command verbose : Set verbose mode") << endl
+					 << _F("command silent : Set silent mode") << endl
+					 << _F("command prompt 'new prompt' : Set prompt to use") << endl;
 	}
 }
 

Sming/Components/CommandProcessing/src/CommandProcessing/Handler.h

@@ -132,7 +132,7 @@ class Handler
 	/** @brief  Get the verbose mode
 	 *  @retval VerboseMode Verbose mode
 	 */
-	bool isVerbose()
+	bool isVerbose() const
 	{
 		return verboseMode;
 	}
@@ -150,7 +150,7 @@ class Handler
 	 *  @note   This is what is shown on the command line before user input
 	 *          Default is Sming>
 	 */
-	String getCommandPrompt()
+	const String& getCommandPrompt() const
 	{
 		return currentPrompt;
 	}
@@ -169,7 +169,7 @@ class Handler
 	 *  @retval char The EOL character
 	 *  @note   Only supports one EOL, unlike Windows
 	 */
-	char getCommandEOL()
+	char getCommandEOL() const
 	{
 		return currentEOL;
 	}
@@ -187,7 +187,7 @@ class Handler
 	 *  @retval String The welcome message that is shown when clients connect
 	 *  @note   Only if verbose mode is enabled
 	 */
-	String getCommandWelcomeMessage()
+	const String& getCommandWelcomeMessage() const
 	{
 		return currentWelcomeMessage;
 	}

Sming/Components/CommandProcessing/src/CommandProcessing/Utils.cpp

@@ -0,0 +1,15 @@
+#include "Utils.h"
+
+namespace CommandProcessing
+{
+void enable(Handler& commandHandler, HardwareSerial& serial)
+{
+	commandHandler.setOutputStream(&serial, false);
+	Serial.onDataReceived([&commandHandler](Stream& source, char arrivedChar, uint16_t availableCharsCount) {
+		while(availableCharsCount--) {
+			commandHandler.process(source.read());
+		}
+	});
+}
+
+} // namespace CommandProcessing

Sming/Components/CommandProcessing/src/CommandProcessing/Utils.h

@@ -5,14 +5,6 @@
 
 namespace CommandProcessing
 {
-void enable(Handler& commandHandler, HardwareSerial& serial)
-{
-	commandHandler.setOutputStream(&serial, false);
-	Serial.onDataReceived([&commandHandler](Stream& source, char arrivedChar, uint16_t availableCharsCount) {
-		while(availableCharsCount--) {
-			commandHandler.process(source.read());
-		}
-	});
-}
+void enable(Handler& commandHandler, HardwareSerial& serial);
 
 } // namespace CommandProcessing

Sming/Core/HardwareSerial.h

@@ -114,10 +114,6 @@ class HardwareSerial : public ReadWriteStream
 	{
 	}
 
-	~HardwareSerial()
-	{
-	}
-
 	void setPort(int uartPort)
 	{
 		end();