Command Interpreter

mdkendall wrote on Wednesday, February 15, 2012:

I was very pleased recently to discover the Command Interpreter in Demo/Common/Utils. One often has to add some sort of command interface to products and having a standard framework to build from is a big time saver. I like the architecture of this one; it seems well thought-out.

Having used it I have a couple of suggestions and comments:

First, the semantics of the return value of xCmdIntProcessCommand surprised me. I was expecting the value to indicate “I did something” or “I did nothing”, so one would use it thus:

while (xCmdIntProcessCommand(line, buf, size)) {

Instead, the return value indicates whether there is anything further to do, hence:

do {
    more = xCmdIntProcessCommand(line, buf, size);
} while (more);

which seems less elegant.

Second, the pcCmdIntGetParameter() function is handy but I was surprised to see that when requesting parameter N I actually get a string that extends from the beginning of the Nth parameter all the way to the end of the line. I see that the pxParameterStringLength will tell me how many characters actually constitute the parameter. Interpreters I have used previously have had the tokeniser replace the delimiting characters in the command string with nulls, cutting the string in to chunks, so that when later fetching a particular parameter one gets a sub-string containing just that particular parameter. That seems a little more convenient.

Lastly, and somewhat trivially, the long, mixed-case filename CommandInterpreter.c does not really match the style of the other FreeRTOS filenames. Can I suggest cmdint.c (since CmdInt seems to be used as the “module prefix” for functions)?

Anyway, overall I’m very happy. Thanks for making this available.

rtel wrote on Wednesday, February 15, 2012:

The command interpreter is hiding in the code, but not yet formally announced or documented (other than a news snippet to say it is there).  It will be formally announced in a couple of weeks - and at the same time separated out of its current location to a separate download.  It is now fully documented and has some example applications, and these will appear on the web site at the same time.

Thanks for your feedback points:

+ I can see the semantics may seem odd, to me they were ok and it didn’t occur to me until you pointed it out.  The idea is that you keep calling the function until there is nothing left to do - the concept being to minimise RAM usage - and the semantics work from that perspective.  I will consider that.

+ The behaviour you are seeing for pcCmdIntGetParameter() is not what it should do, and not behaviour I have ever seen myself.  The string should be tokenised in exactly the way you are saying.  That relies on the string being in writable memory though, were you passing the string in RAM?

+ The file name has changed for the V1 release (the one in a couple of weeks), as have the names of the functions it defines.  They are documented now.  They won’t change again after the formal release though.


mdkendall wrote on Wednesday, February 15, 2012:

I am definitely seeing the behaviour I described for pcCmdIntGetParameter(). I register a command “foo” with 2 parameters. On the command line I enter “foo bar baz”. The callback gets called. Within it I have p = pcCmdIntGetParameter(pcCommandString, 1, &len) and the result is that p is a pointer in to the original string, starting at the first ‘b’, i.e. *p is “bar baz\0”. The space between the first and second params has not been replaced with a null. I see no code in xCmdIntProcessCommand() or pcCmdIntGetParameter() that would make such a substitution.

I am using the version of CommandInterpreter.c from the 7.10 distribution. Perhaps there has been a change since then?

rtel wrote on Wednesday, February 15, 2012:

My apologies.  It used to be done that the command was terminated, but to prevent the command string being altered as a side effect of calling the API function the API function was changed to instead return the length of the parameter string.  You are quite correct.

I just checked one of the demos and it looks like this:

	/* Obtain the name of the first parameter, and the length of its name. */
	pcParameter1 = ( int8_t * ) FreeRTOS_CLIGetParameter( pcCommandString, uxFirstParameter, &xParameter1StringLength );
	/* Terminate parameter string. */
	pcParameter1[ xParameter1StringLength ] = 0x00;

This means the application terminates the string if it wants to, which was considered safer.

Sorry for the confusion.