Hi everyone,
I have to control an arduino with a PC, so I wrote the attached sketch, I would like feedback if there is a different way to do this, or if there are any fundamental flaws in my logic.
All input welcome.
char inCommand [255]; // Command line MAX length
String Commands [10]; // Number of 'words' in the command
String temp = "";
int inChar;
int Terminator = 0;
int location = 0;
int Space;
void setup()
{
Serial.begin(115200);
}
void loop()
{
if (Serial.available()>0)
{
DoCommands();
}
}
/*
*
* This gets the input from the Serial port and returns the complete command
* line. You can either use direct comparison to have commands, or you can
* strip the returned string and write an interpreter.
*
*/
String getCommandLine()
{
if(Serial.available() > 0)
{
getInput();
}
temp="";
if (Terminator == 1)
{
temp = inCommand; // convert inCommand to a usable format
temp.toUpperCase(); // and convert to uppercase
memset(inCommand, 0, sizeof(inCommand));
Terminator = 0; // Reset Terminator
location = 0; // Reset String Location Pointer
}
return(temp);
}
/*
*
* This gets the command line from the serial port
*
*
*/
void getInput()
{
while (Terminator == 0)
{
if(Serial.available()>0)
{
inChar = Serial.read();
if (inChar == '\n')
{
Terminator = 1;
break;
}
inCommand[location] = inChar;
location++;
}
}
}
void Stripcommand(String Command)
{
Space = Command.indexOf(" ");
Commands[1] = Command.substring(0,Space);
Command = Command.substring(Space+1,Command.length());
for(int here = 2; here < 11; here++)
{
Space = Command.indexOf(" ");
Commands[here] = Command.substring(0,Space);
Command = Command.substring(Space+1,Command.length());
if(Space<1)
{
Command[0] = here; // Pass back the command count
break;
}
}
}
void DoCommands()
{
String Command = getCommandLine();
if (Command != "")
{
Stripcommand(Command);
Command=""; // Clear the command
}
if (Commands[1] == "LED")
{
if (Commands[3] == "ON")
{
digitalWrite(Commands[2].toInt(),HIGH); // I don't test if the value is in range here!
}
if (Commands[3] == "OFF")
{
digitalWrite(Commands[2].toInt(),LOW);
}
}
if (Commands[1] == "DIGITALWRITE")
{
if (Commands[3]== "HIGH")
{
digitalWrite(Commands[2].toInt(),HIGH);
}
if (Commands[3]== "LOW")
{
digitalWrite(Commands[2].toInt(),LOW);
}
}
if (Commands[1] == "PINMODE")
{
if (Commands[3] == "OUTPUT")
{
pinMode(Commands[2].toInt(),OUTPUT);
}
if (Commands[3] == "INPUT")
{
pinMode(Commands[2].toInt(),INPUT);
}
}
if (Commands[1] == "ANALOGREAD")
{
{
Serial.print(analogRead(Commands[2].toInt()));
}
}
}
The way you collect the command line is not good, as it will block in waitInput until your terminator is received. If the terminator is not received, it will never return. You should take whatever characters are available, then return, WITHOUT waiting, but don't process the command line until the terminator is received. That allows the rest of the program to continue doing other things while waiting.
You should get rid of the String class, and use strtok, or at least strcmp, to parse the commands. Also, use strupr to force all the commands to upper case before parsing, so the user can enter lower case or upper case, and both will work. Case-sensitive user input is very 1970s....
@Ray: You raise an interesting point about blocking until the termination character is read. If a program cannot do anything until the input is read, what's the best way to collect the input? If there's nothing to do until all the input is read, blocking is less of an issue. If there is something that might happen while waiting (e.g., an ISR perhaps), then blocking becomes an issue. Your thoughts?
Not Ray but...
Call readSerial1 from loop and if it returns true you have a full line. If the sender is uncontrolled then you may need to buffer the line or stop calling readSerial1 until the line is processed as it will start overwriting the current line as soon as it's called again.
if (readSerial1()){ // Call serial1 read routine (it returns true if linefeed character found)
}
-------------------------------------------
boolean readSerial1() {
static byte index = 0; // Buffer array index counter
while (Serial1.available() > 0) { // Does serial1 have characters available?
char rc = Serial1.read(); // Read a character
if (rc == '\n') { // Is it newline?
charBuffer1[index] = 0; // Yes, so terminate char array with 0
index = 0; // Reset array index
return true; // Return true (we have a full line)
}
if (rc >= '0') { // Is value between 32 and 127?
charBuffer1[index++] = rc; // Yes, so store it and increment index
if (index > CHARBUFFERSIZE) { // Have we filled the buffer?
index--; // Yes, so decrement index
}
}
}
return false; // Return false (we don't have a full line)
}
char buffer[MAXCHARSEXPECTED};
int charsRead;
// perhaps some more code?
if (Serial.available() > 0) {
charsRead = Serial.readBytesUntil('\n', buffer, MAXCHARSEXPECTED - 1);
buffer[charsRead] = '\0';
// the packet is now a string...do whatever...
}
econjack: @Riva: Given what I see, why not just use:
char buffer[MAXCHARSEXPECTED};
int charsRead;
// perhaps some more code?
if (Serial.available() > 0) {
charsRead = Serial.readBytesUntil('\n', buffer, MAXCHARSEXPECTED - 1);
buffer[charsRead] = '\0';
// the packet is now a string...do whatever...
}
I had not included the full code, I had just lifted most relevant section from the sketch I wrote it for. The reason your example is no good is it's blocking. The code I posted you call periodically and it returns true when a full line has arrived else it returns false.
Programme Serveur WEB
Configuration Serial commande Line (cmdLine)
Log Serial :
Ouver le Moniteur série ou IDE Arduino, PUTTY, KUTTY, COM - Termite.exe ....
Commande disponible :
help
General commands :
help aide commande cmd Serial (man)
ram RAM Disponible
Configuration commands :
print print configuration courrante
defaults set default configuration values
mac set local MAC address (default: DE:AD:BE:EF:FE:ED
dhcp-on enable DHCP
dhcp-off disable DHCP
ip set local IP address Exemple : ip 192.168.0.66
subnet set Masque de sous réseau
gateway set Passerelle
EEPROM access commands:
reload load EEPROM avec la configuration sauvegarde
save save configuration en EEPROM
reset Restart Arduino et load la configuration enregistre en EEPROM
Pour changer l'IP de l'Arduino :
Commande ip xxx.xxx.xxx.xxx puis save et enfin reset
pour redemarrer l'arduino dans la nouvelle configuration
a good command parser will recognise reserved words (or abbreviated words), and their position within the command line.
If an invalid word is detected in any position, it should be flagged in the return code.
Similarly, when a complete command is parsed, it must be a valid combination of keywords for the current context, if not tell the user what’s wrong.
You can do this with a list of valid keywords for each position - maybe in an array....
e.g
SET, CLEAR, LOCK, UNLOCK — first word 0/1/2
MODE, LED, RELAY, ALL — second word 0/1/2
ON, OFF, DEFAULT, blank — third word 0/1/2
Develop a scoring algorithm* to make a best guess at misspelled words, or reject them.
Normalise the case if necessary to permit case insensitive typing if that’s what you want.
Finally, you have a value e.g. 2.2.1, which suggests LOCK RELAY OFF was typed.
Then you can use your choice of selector to run the functions you want.
While a little more complicated, scoring allows simple mistyping to get through as long as the command isn’t ambiguous... e.g. reLy cannot be confused with any other word in the second position, and almost matches RELAY without any ambiguity... so you can safely assume they neant to type reLay.
Advanced devs will allow auto completion once a keyword has been identified.
