Return to Answer

Don't use String is you are going to use functions like strtol() a lot. It is better to use Serial.readBytes() and read directly into a char array than to first read it into a String and then convert it. Or if you really want to use String, use the c_str() member function to get access to the underlying char array, like so: strtol(string.c_str(), NULL, 16).

Don't use String if you are going to use functions like strtol() a lot. It is better to use Serial.readBytes() and read directly into a char array than to first read it into a String and then convert it. Or if you really want to use String, use the c_str() member function to get access to the underlying char array, like so: strtol(string.c_str(), NULL, 16).

Avoid casting Strings to char arrays and back

Don't use String is you are going to use functions like strtol() a lot. It is better to use Serial.readBytes() and read directly into a char array than to first read it into a String and then convert it. Or if you really want to use String, use the c_str() member function to get access to the underlying char array, like so: strtol(string.c_str(), NULL, 16).

Avoid casting Strings to char arrays and back

Don't use String is you are going to use functions like strtol() a lot. It is better to use Serial.readBytes() and read directly into a char array than to first read it into a String and then convert it. Or if you really want to use String, use the c_str() member function to get access to the underlying char array, like so: strtol(string.c_str(), NULL, 16).

Don't forget to account for the NUL terminator

C strings must always be terminated by a NUL byte. When you convert a String to a char array, you must ensure the array has room for the NUL byte. So if you have a String of 2 characters, the char array must be at least 3 characters long. But, as mentioned above, just use c_str() and you will avoid this issue.

Avoid global variables

You can move the declaration of toWrite into loop(). Keep variable declarations close to where they are actually used.

Check that parsing of numbers actually succeeded.

You call strtol() but you don't check if the value was correctly parsed. What happens if the wrong command is sent to the Arduino, or if there is a glitch on the serial bus and the message is garbled? What damage might be done if strtol() returns 0? If safety is important, also consider adding a checksum to the message.

Avoid regular expressions

Some people, when confronted with a problem, think "I know, I'll use regular expressions." Now they have two problems. -- Jamie Zawinsky

Your problem is simple enough that you don't need any regular expressions to solve it. The input format is very simple: it's a single command with underscores separating the various parameters. The right tool for this job is to simply split the input using '_' as the separator. Since you are using the String class to hold strings, use indexOf() to find the position of the next separator. For example, to loop over all tokens:

String input = ...;
int start = 0;
int pos;
while ((pos = input.indexOf('_', start)) != -1) {
 String token = input.substring(start, pos);
 // do something with this token
 start = pos + 1;
}

Instead of a loop though, you probably want to write a class that will handle finding the next token for you, such that you can write something like:

String input = ...
Tokenizer tokenizer(input);
String type = tokenizer.get_next_token();
if (type != "MSv2")
 return false;
String shield = tokenizer.get_next_token();
...
String command = tokenizer.get_next_token();
if (command == "speed")
 // handle speed command
else if (command == "direction")
 // handle direction command
else ...

This code avoids the overhead of using the regular expression library, and avoids having to Match() multiple regular expressions against the same input string. Your code will be faster and consume less flash memory and RAM.

Avoid regular expressions

Some people, when confronted with a problem, think "I know, I'll use regular expressions." Now they have two problems. -- Jamie Zawinsky

Your problem is simple enough that you don't need any regular expressions to solve it. The input format is very simple: it's a single command with underscores separating the various parameters. The right tool for this job is to simply split the input using '_' as the separator. Since you are using the String class to hold strings, use indexOf() to find the position of the next separator. For example, to loop over all tokens:

String input = ...;
int start = 0;
int pos;
while ((pos = input.indexOf('_', start)) != -1) {
 String token = input.substring(start, pos);
 // do something with this token
 start = pos + 1;
}

Instead of a loop though, you probably want to write a class that will handle finding the next token for you, such that you can write something like:

String input = ...
Tokenizer tokenizer(input);
String type = tokenizer.get_next_token();
if (type != "MSv2")
 return false;
String shield = tokenizer.get_next_token();
...
String command = tokenizer.get_next_token();
if (command == "speed")
 // handle speed command
else if (command == "direction")
 // handle direction command
else ...

This code avoids the overhead of using the regular expression library, and avoids having to Match() multiple regular expressions against the same input string. Your code will be faster and consume less flash memory and RAM.

Avoid casting Strings to char arrays and back

Don't use String is you are going to use functions like strtol() a lot. It is better to use Serial.readBytes() and read directly into a char array than to first read it into a String and then convert it. Or if you really want to use String, use the c_str() member function to get access to the underlying char array, like so: strtol(string.c_str(), NULL, 16).