views:

68

answers:

3

I'd like to interpret a command string, recieved by a microcontroller (PIC16f877A if that makes any difference) via serial.

The strings have a pretty simple and straight-foward formatting: $AABBCCDDEE (5 "blocks" of 2 chracters+'$' for 11 characters in total) where: $AA= the actual name of the command (could be letters, numbers, both; mandatory); BB-EE= parameters (numbers; optional);

I'd like to write the code in C/C++.

I figure I could just grab the string via serial, hack it up into blocks, switch () {case} and memcmp the command block ($AA). Then I could have a binary decision tree to make use of the BB CC DD and EE blocks.

I'd like to know if that's the right way to do it (It kinda' seems ugly to me, surely there must be a less tedious way to do this!).

+2  A: 

It depends on how fancy you want to get, how many different commands there are, and whether new commands are likely to be frequently added.

You could create a data structure that associates each valid command string with a corresponding function pointer - a sorted list accessed with bsearch() is probably fine, although a hash table is an alternative which may have better performance (since the set of valid commands is known beforehand, you could construct a perfect hash with a tool like gperf).

The bsearch() approach might look something like this:

void func_aa(char args[11]);
void func_cc(char args[11]);
void func_xy(char args[11]);

struct command {
    char *name;
    void (*cmd_func)(char args[11]);
} command_tbl[] = {
    { "AA", func_aa },
    { "CC", func_cc },
    { "XY", func_xy }
};

#define N_CMDS (sizeof command_tbl / sizeof command_tbl[0])

static int comp_cmd(const void *c1, const void *c2)
{
    const struct command *cmd1 = c1, *cmd2 = c2;

    return memcmp(cmd1->name, cmd2->name, 2);
}

static struct command *get_cmd(char *name)
{
    struct command target = { name, NULL };

    return bsearch(&target, command_tbl, N_CMDS, sizeof command_tbl[0], comp_cmd);
}

Then if you have command_str pointing to a string from the serial port, you'd do this to dispatch the right function:

struct command *cmd = get_cmd(command_str + 1);

if (cmd)
    cmd->cmd_func(command_str);
caf
Thanks!I'll need to get reading on hash tables and hash functions :D
Pav
+3  A: 

Don't over design it ! It does not mean to go blindly coding, but once you have designed something that looks like it can do the job, you can start to implement it. Implementation will give you feedback about your architecture.

For example, when writing your switch case, you might see yourself rewriting code very similar to the one you just wrote for the preceding case. Actually writing down an algorithm will help you see some problem you did not think off, or some simplification you did not see.

Don't aim for the best code on the first try. Aim for

  • easy to read
  • easy to debug

Take litlle steps. You do not have to implement the whole thing in one go.

  • Grab the string from the serial port. Looks easy, right ? Well, let's do that first, just printing out the commands.
  • Separate the command from the parameters.
  • Extract the parameters. Will the extraction be the same for each command ? Can you design a data structure valid for every command ?

Once you have done it right, you can start to think of a better solution.

shodanex
Thanks!I was about to do just that..Start coding blindly, that is :D
Pav
A: 

ASCII interfaces are ugly by definition. Ideally you have some sort of frame structure, which maybe you have, the $ indicates the division between frames and you say they are 11 characters in length. If always 11 that is good, if only sometimes that is harder, hopefully there is a $ at the start and 0x0A and or 0x0D/0x0A at the end (CR/LF). Normally I have one module of code that simply extracts bytes from the serial port and puts them into a (circular) buffer. The buffering dating to the days when serial ports had very little of no buffer on board, but even today, esp with microcontrollers, that is still the case. Then another module of code that monitors the buffer searching for frames. Ideally this buffer is big enough to leave the frame there and have room for the next frame and not require another buffer for keeping copies of the frames received. using the circular buffer this second module can move (discarding if necessary as it goes) the head pointer to the beginning of frame marker and waits for a full frames worth of data. Once a full frame appears to be there it calls another function that processes that frame. That function may be the one you are asking about. And "just code it" may be the answer, you are in a microcontroller, so you cant use lazy high level desktop application on an operating system solutions. You will need some sort of strcmp function if created yourself or available to you through a library, or not depending on your solution. The brute force if(strncmp(&frame[1],"bob",3)==0) then, else if(strncmp(&frame[1],"ted",3) then, else if... Certainly works but you may chew up your rom with that kind of thing, or not. And the buffering required for this kind of approach can chew up a lot of ram. This aproach is very readable and maintainable, and portable though. May not be fast (maintainable normally conflicts with reliable and/or performance), but that may not be a concern, so long as you can process this one before the next one comes along, and or before unprocessed data falls out of the circular buffer. Depending on the task the frame checker routine may simply check that the frame is good, I normally put start and end markers, length and some sort of arithmetic checksum and if it is a bad frame it is discarded, this saves on a lot of code checking for bad/corrupt data. When the frame processing routine returns to the search for frame routine it moves the head pointer to purge the frame as it is no longer needed, good frame or bad. The frame checker may only validate a frame and hand it off to yet another function that does the parsing. Each lego block in this arrangement has a very simple task, and operates on the assumption that the lego block below it has performed its task properly. Modular, object oriented, whatever term you want to use makes the design, coding, maintenance, debugging much easier. (at the cost of peformance and resources). This approach works well for any serial type stream be it serial port in a microcontroller (with enough resources) as well as applications on a desktop looking at serial data from a serial port or TCP data which is also serial and NOT frame oriented.

if your micro doesnt have the resources for all that, then the state machine approach also works quite well. Each byte that arrives ticks the state machine one state. Start with idle waiting for the first byte, is the first byte a $? no discard it and go back to idle. if first byte is a $ then go to the next state. If you were looking for say the commands "and", "add", "or", and "xor", then the second state would compare with "a","o", and "x", if none of these then go to idle. if an a then go to a state that compares for n and d, if an o then go to a state that looks for the r. If the look for the r in or state does not see the r then go to idle, if it does then process the command and then go to idle. The code is readable in the sense that you can look at the state machine and see the words a,n,d, a,d,d, o,r, x,o,r, and where they ultimately lead to, but generally not considered readable code. This approach uses very little ram, leans on the rom a bit more but overall could use the least amount of rom as well compared to other parsing approaches. And here again is very portable, beyond microcontrollers, but outside a microcontroller folks might think you are insane with this kind of code (well not if this were verilog or vhdl of course). This approach is harder to maintain, harder to read, but is very fast and reliable and uses the least amount of resources.

To matter what approach once the command is interpreted you have to insure you can perform the command without losing any bytes on the serial port, either through deterministic performance of the code or interrupts or whatever.

Bottom line ascii interfaces are always ugly, the code for them, no matter how many layers of libraries you use to make the job easier, the resulting instructions that get executed are ugly. And one size fits no-one by definition. Just start coding, try a state machine and try the if-then-else-strncmp, and optimizations in between. You should see quickly which one performs best both with your coding style, the tools/processor, and the problem being solved.

dwelch
Thanks!Your and shodanex's answers have been by far the most useful for me.The finite state machine seems like the right choice, even though it's not as intuitive as the modular approach.And, yes, ASCII interfaces are ugly. Not my choice though :P
Pav