creativerobotics/commander

Arduino Command Line Utility

commander's Issues

typo in Commander/src/Commander.h => not compiling

#include "utilities\CommandHelpTags.h"

is containing a \ thus not compiling on my linux box for version 3.1.0 of Commander

README copy/paste error

Last paragraph has some extra text at the end:

Written by Bill Bigge. MIT license, all text above must be included in any redistributionle via USB and Bluetooth Serial. It also allows different command lists to be dynamically loaded so multiple command lists, and multiple Commander objects can be combined to produce hierarchical command structures.

Compile Errors with current Arduino ESP8266 on PlatformIO

Apparently several of the internal functions have unused values and at least one doesn't guarantee a return value for a non-void function, which is now treated as an error in PlatformIO so the library won't compile.
If it helps, here's the relevant errors from pio run

Compiling .pio/build/nodemcu/lib8e8/Commander/Commander.cpp.o
Compiling .pio/build/nodemcu/lib8e8/Commander/utilities/CommandHelpTags.cpp.o
.pio/libdeps/nodemcu/Commander/src/utilities/CommandHelpTags.cpp: In function 'bool getCommandArgCode(char*, cmdArgs_t)':
.pio/libdeps/nodemcu/Commander/src/utilities/CommandHelpTags.cpp:11:25: warning: NULL used in arithmetic [-Wpointer-arith]
   11 |  while(helpText[idx] != NULL){
      |                         ^~~~
.pio/libdeps/nodemcu/Commander/src/Commander.cpp: In constructor 'Commander::Commander()':
.pio/libdeps/nodemcu/Commander/src/Commander.cpp:10:34: warning: list-initializer for non-class type must not be parenthesized
   10 |                         "errors"}){
      |                                  ^
.pio/libdeps/nodemcu/Commander/src/Commander.cpp: In constructor 'Commander::Commander(uint16_t)':
.pio/libdeps/nodemcu/Commander/src/Commander.cpp:24:46: warning: list-initializer for non-class type must not be parenthesized
   24 |                                     "errors"}){
      |                                              ^
.pio/libdeps/nodemcu/Commander/src/Commander.cpp: In member function 'uint8_t Commander::countItems()':
.pio/libdeps/nodemcu/Commander/src/Commander.cpp:527:7: warning: unused variable 'state' [-Wunused-variable]
  527 |  bool state = 0;
      |       ^~~~~
.pio/libdeps/nodemcu/Commander/src/Commander.cpp: In member function 'uint8_t Commander::getLength(uint8_t)':
.pio/libdeps/nodemcu/Commander/src/Commander.cpp:658:44: warning: NULL used in arithmetic [-Wpointer-arith]
  658 |   if(commandList[indx].commandString[n] != NULL) length++;
      |                                            ^~~~
.pio/libdeps/nodemcu/Commander/src/Commander.cpp: In member function 'bool Commander::checkCommand(uint16_t)':
.pio/libdeps/nodemcu/Commander/src/Commander.cpp:940:27: warning: comparison of integer expressions of different signedness: 'unsigned int' and 'int' [-Wsign-compare]
  940 |  if(bufferString.length() < commandLengths[cmdIdx]+1) return false; //no match if the buffer is shorter than the command+1 (buffer will have the end of line char)
      |     ~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
.pio/libdeps/nodemcu/Commander/src/Commander.cpp: In member function 'bool Commander::itemToNextDelim()':
.pio/libdeps/nodemcu/Commander/src/Commander.cpp:1243:7: warning: unused variable 'endOnQuote' [-Wunused-variable]
 1243 |  bool endOnQuote = false;
      |       ^~~~~~~~~~
.pio/libdeps/nodemcu/Commander/src/Commander.cpp: In member function 'bool Commander::isItem(char)':
.pio/libdeps/nodemcu/Commander/src/Commander.cpp:1282:11: warning: NULL used in arithmetic [-Wpointer-arith]
 1282 |  if(ch == NULL) return false;
      |           ^~~~
.pio/libdeps/nodemcu/Commander/src/Commander.cpp: In member function 'uint8_t Commander::getInternalCmdLength(const char*)':
.pio/libdeps/nodemcu/Commander/src/Commander.cpp:1461:19: warning: NULL used in arithmetic [-Wpointer-arith]
 1461 |   if(intCmd[n] == NULL) return n;
      |                   ^~~~
.pio/libdeps/nodemcu/Commander/src/Commander.cpp: In member function 'uint8_t Commander::getLength(uint8_t)':
.pio/libdeps/nodemcu/Commander/src/Commander.cpp:661:1: error: control reaches end of non-void function [-Werror=return-type]

So, can you make sure that non-void member functions always have a return value?

endOfLineChar(CR) should also disable stripCR()?

My terminal uses CR as the default EOL character, and I've not had a problem with that setting for years, so I didn't feel like changing it to LF when trying out Commander. I found endOfLineChar() but it didn't work when I changed to CR, and it took some digging to find the stripCR() method.

I think it's probably a common use case to want to change the EOL character to CR, would it be reasonable to automatically disable the stripCR setting if EOL is set to CR?

startStreaming() using Stream class

I'm trying to add YMODEM receive to my project using this library (my fork which allows for setting the Stream port instead of using hardcoded Serial). I can't use the existing streaming options in Commander directly, as the library depends on receiving data via a Stream. I don't really want to modify the library just to work with Commander. I imagine there are more libraries using Stream that could be included in future prefabs, so I'd rather find a good solution to this problem than hack together support for one library to solve my immediate problem.

I see two options here:

1. Make a mode where Commander gives up control of inPort, and lets other code use it until stopStreaming() is called

• This wouldn't work with !dataStreamMode as the other code could read from the Stream before Commander gets a chance to look for 0x04

2. Use PairedPipedStream from this library (my fork where I modified it to optionally output to a Stream on one end automatically). Commander can inspect each byte, and load it into one end of a PairedPipedStream. The other code can read from the other end as if it were reading directly from inPort (though Commander decides when to stop streaming data), and whatever it writes gets automatically written to outPort.

• This uses more memory as the PairedPipedStream has a fixed buffer, but with a little extra code we could dynamically resize the buffer.

3. I guess there's a third option where there's no modification to Commander, and I just don't call cmd.update() to make option 1 work inside of my sketch, or I take care of option 2 inside a special handler in my sketch/prefab.

I only thought of the much simpler option 1 after doing all the work upgrading PairedPipedStream and prototyping option 2 and getting YMODEM receive to work. At this point I think the remaining work for option 1 and 2 are about the same.

LMK if you have some thoughts on these options, or want more details on anything

Commander seems to be missing a way to handle CR+LF as a valid line terminator

It seems to handle either 0x0d OR 0x0a, But not BOTH.

[RFC] help command not executed

When trying the getting started example
The command doesn't seems to get executed

Once connected to the serial port:

Hello: Type 'help' to get help
CMD>help

The cursor return at the beginning of the line and nothing happens after hitting enter

if I keep entering characters:

aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa#ERR: Buffer Overflow

Hardware: SAMD21G18 Microcontroller

library

arduino-cli lib list
Commander         4.0.1    -           user     - 

serial communication

cu -l /dev/ttyACM0 -s 115200

NOTE: same behavior with minicom

String in BasicCommands

I've tried several times to use the string as output in BasicCommands for an if function. First, I was told that the variable myString was not defined in the code, so I initialized String myString=""; as with float and int. However, I wasn't able to print myString and the if function wasn't working, so the code wasn't able to show me whether the string was set to the right value. I made a thread about this on the arduino forum and b707 helped me. It seems that initializing the string in BasicCommands is the correct way to use it in this code, but it has already been initialized in the Master, which would block the string from being modified in BasicCommands. The solution was to comment out the initialization in the Master, and it worked!
https://forum.arduino.cc/t/commander-library-set-string/1188811/12

`/*Commander example - basic

• Demonstrating commands to get and set an int and a float

#include <Commander.h>
Commander cmd;
//Variables we can set or get
int myInt = 0;
float myFloat = 0.0;
String myString="";

String deviceInfo = "#\tCommander basic commands example\n#\thttps://github.com/CreativeRobotics/Commander";
//SETUP ---------------------------------------------------------------------------
void setup() {
Serial.begin(115200);
initialiseCommander();
while(!Serial){;}
cmd.printUserString();
cmd.println();
Serial.println("Type 'help' to get help");
cmd.printCommandPrompt();
}

//MAIN LOOP ---------------------------------------------------------------------------
void loop() {

//command=cmd.getPayloadString();

if(myString =="stop"){
Serial.println("Succes string");
}
else if(myString=="go") {
Serial.println("Succes go");
}
cmd.update();
}
//Serial.println(myInt);
`

`//All commands for 'master'
//COMMAND ARRAY ------------------------------------------------------------------------------
const commandList_t masterCommands[] = {
{"hello", helloHandler, "hello"},
{"get int", getIntHandler, "get an int"},
{"set int", setIntHandler, "set an int"},
{"get float", getFloatHandler, "get a float"},
{"set float", setFloatHandler, "set a float"},
{"hidden1", hiddenHandler, "-Command hidden from help"},
{"myint", setIntHandler, "try myint=23"},
{"myfloat", setFloatHandler, "try myfloat=23.5"},
{"set ints", setIntsHandler, "set up to four ints"},
{"set floats", setFloatsHandler, "set up to four floats"},
{"set strings", setStringsHandler,"set up to four Strings"},
{"hidden2", hiddenHandler, "-Command hidden from help"},
};

/* Command handler template
bool myFunc(Commander &Cmdr){
//put your command handler code here
return 0;
}
*/
void initialiseCommander(){
cmd.begin(&Serial, masterCommands, sizeof(masterCommands));
cmd.commandPrompt(ON); //enable the command prompt
cmd.echo(true); //Echo incoming characters to theoutput port
cmd.errorMessages(ON); //error messages are enabled - it will tell us if we issue any unrecognised commands
cmd.autoChain(ON);
cmd.setUserString(deviceInfo);
}

//These are the command handlers, there needs to be one for each command in the command array myCommands[]
//The command array can have multiple commands strings that all call the same function
bool helloHandler(Commander &Cmdr){
Cmdr.print("Hello! this is ");
Cmdr.println(Cmdr.commanderName);
Cmdr.print("This is my buffer: ");
Cmdr.print(Cmdr.bufferString);
//Cmdr.printDiagnostics();
return 0;
}
bool getIntHandler(Commander &Cmdr){
Cmdr.print("myInt = ");
Cmdr.println(myInt);
//Cmdr.printDiagnostics();
return 0;
}

bool setIntHandler(Commander &Cmdr){
if(Cmdr.getInt(myInt)){
Cmdr.print("myInt set to ");
Cmdr.println(myInt);
}
//Cmdr.printDiagnostics();
return 0;
}
bool getFloatHandler(Commander &Cmdr){
Cmdr.print("myFloat = ");
Cmdr.println(myFloat);

//Cmdr.printDiagnostics();
return 0;
}

bool setFloatHandler(Commander &Cmdr){
if(Cmdr.getFloat(myFloat)){
Cmdr.print("myFloat set to ");
Cmdr.println(myFloat, 4); //print with 4 decimal places
}
//Cmdr.printDiagnostics();
return 0;
}

bool setIntsHandler(Commander &Cmdr){
//create an array to store any values we find
int values[4] = {0,0,0,0};

int itms = Cmdr.countItems();
Cmdr.print("There are ");
Cmdr.print(itms);
Cmdr.println(" items in the payload");

for(int n = 0; n < 4; n++){
//try and unpack an int, if it fails there are no more left so exit the loop
if(Cmdr.getInt(values[n])){
Cmdr.print("unpacked ");
Cmdr.println(values[n]);
}else break;
}
//print it out
Cmdr.println("Array contents after processing:");
for(int n = 0; n < 4; n++){
Cmdr.print(n);
Cmdr.print(" = ");
Cmdr.println(values[n]);
}
//Cmdr.chain();
//Cmdr.printDiagnostics();
return 0;
}

bool setFloatsHandler(Commander &Cmdr){
float values[4] = {0.0,0.0,0.0,0.0};
int itms = Cmdr.countItems();
Cmdr.print("There are ");
Cmdr.print(itms);
Cmdr.println(" items in the payload");

for(int n = 0; n < 4; n++){
if(Cmdr.getFloat(values[n])){
Cmdr.print("unpacked ");
Cmdr.println(values[n]);
}else break;
}
Cmdr.println("Array contents after processing:");
for(int n = 0; n < 4; n++){
Cmdr.print(n);
Cmdr.print(" = ");
Cmdr.println(values[n]);
}
//Cmdr.chain();
//Cmdr.printDiagnostics();
return 0;
}

bool setStringsHandler(Commander &Cmdr){
//String myString = "";
int itms = Cmdr.countItems();
Cmdr.print("There are ");
Cmdr.print(itms);
Cmdr.println(" items in the payload");
for(int n = 0; n < itms; n++){
if(Cmdr.getString(myString)){
Cmdr.print("String ");
Cmdr.print(n);
Cmdr.print(" = ");
Cmdr.println(myString);
}else Cmdr.println("Operation failed");
}
//Cmdr.chain();
//Cmdr.printDiagnostics();
return 0;
}

bool hiddenHandler(Commander &Cmdr){
Cmdr.println("This command is hidden from the help system");
//Cmdr.printDiagnostics();
return 0;
}`

Idea to change sketch structure to avoid Arduino-specific magic and externs

I noticed you're using multiple .ino files for several examples to work around some circular referencing (probably not the right terminology) issues. It seems that's allowed by the Arduino IDE, but not common practice. Even so, you also have to use externs to work around the same issues. I tried to figure out why this is needed, and it seems to come from needing to have a command list containing a function reference while the function also needs a reference to the list, and both definitions are needed at compile time. I couldn't find a good way to avoid using .INOs+externs without dramatically restructuring the code, and also making it more complex. The functions don't need to know the command list and size at compile time though, it can be set during setup(), making the problem easier to solve. I prototyped a solution to get your feedback.

embedded-creations@5547ef6

If it's not obvious to you why this is an improvement, let me know and I'll make a case for why I like it and think it should at least be an option. I'm too tired to write anything more right now ;-)

Arduino Library Manager latest release doesn't compile

Hi, I just found your very impressive library by searching "CLI" in Arduino Library Manager, but when I tried to compile an example it failed because setUserString() was missing. I see from the commit history that the 4.1.0 tag was made for the the commit before setUserString() was added. I believe you need to update library.properties with a new number, commit, and tag that commit to get a working copy of the latest code into Arduino Library Manager.

I was able to checkout the latest from GitHub and compile so I'm not stuck.

Unable to use library with Putty.

I flashed the 'BasicCommands' example to my ESP32 board. From the Arduino Serial Monitor, I'm able to use the library wonderfully. However, if I try to connect to the serial port using Putty, or the Visual Studio Code "Serial Monitor", entered commands aren't registered.