AVR UART message sync and decoding

Question

I have an ISR putting in bytes from the UART on an AVR XMEGA into a circular buffer. Every byte received, also increments RX_COUNT. I copy the circular buffer into a local buffer if it is at least two bytes for a potential "#A" command.

The protocol I am using is #A, #B, #C, etc. The problem is I am searching for the sync char, '#' and they seeing what the next byte is, and checking if it is a valid command. The issue is, sometimes if I send a string like aaaaaaa#A it doesn't execute the command. I'm simulating garbage data to see if it will ever sync to the byte. How can I make sure that I will eventually sync to the start of the message?

Periodically, I have from my main function, check how much data is available in the circular buffer. If there is at least 2 bytes available, I copy the circular buffer into a local buffer:

USART_RXBufferData_Available() returns true or false is there is any data in the circular buffer.

Another question is, do I even have to copy the circular buffer into a local buffer? What is generally the best practice to decode a serial packet similar to this?

    #define MIN_MSG_SIZE 2 // Command is #A, #B, #C, etc...
    bool dataAvailable = false; 

    ATOMIC_BLOCK(ATOMIC_RESTORESTATE)
    {

        // Copy Circular buffer into local buffer

        if (USART_RXBufferData_Available(&UARTCom1))
        {
            if (UARTCom1.buffer.RX_Count >= MIN_MSG_SIZE)
            {
                for (int i = 0; i < MIN_MSG_SIZE; i++)
                {
                    localBuff[i] = USART_RXBuffer_GetByte(&UARTCom1);

                    UARTCom1.buffer.RX_Count--; // decrement RX count
                }

                dataAvailable = true;
            }
        }   

        else

        {

              dataAvailable = false;

        }
   }

if ((dataAvailable) && localBuff[0] == '#') // Check if there is a new Message AND Check Sync Byte
{   
        uint8_t data = localBuff[1];
        PORTR_DIR = 0b00000011;

        char led0Msg[] = "Turning ON LED0\r\n";
        char led1Msg[] = "Turning ON LED1\r\n";
        char ledoffMsg[] = "Turning OFF LEDS\r\n";

        switch (data)
        {
            case 'A':
            UART->transmit(led0Msg, strlen(led0Msg));
            PORTR_OUT = ~(PIN0_bm); // Turn on LED0
            break;

            case 'B':
            UART->transmit(led1Msg, strlen(led1Msg));
            PORTR_OUT = ~(PIN1_bm); // Turn on LED1
            break;

            default:
            UART->transmit(ledoffMsg, strlen(ledoffMsg));
            PORTR_OUT = (PIN1_bm | PIN0_bm); // Turn off BOTH LEDS

        }

        memset(localBuff, 0, sizeof(localBuff)); // Clear the buffer
        return true;

    }

Answer 1

Creating your own protocol and processing messages can sometimes be tricky (unexpected results), and you won't notice the details until you start testing and debugging. The best thing you can do is define what your message should look like, and try to make the processing as robust as possible. You can also use things like a CRC or checksum, and/or acknowledgements. This is a quick/short example and does not use any of that.

Normally, it's good to have a start byte and an end byte, so it's easy to detect the beginning of a message and then know when the message is complete/was sent successfully. In your case since you only have a single byte for the command/message the ending byte isn't really necessary.

However, you say minimum message size, so if it's going to end up being longer I'd definitely add an ending byte. I'm going to show it with an ending byte. You can remove it if you want, and adjust the code accordingly.

So let's define your message: #cmd!, where '#' is your start byte, '!' is the end byte, cmd is what you're interested in, and the length is 3.

#define MIN_MSG_SIZE 3 
#define START_BYTE 0x23 //'#' 
#define END_BYTE 0x21   //'!'

Also, to make it easy, use a few global variables to control you processing. Like this:

bool bMsgStart = false;
bool bExpectingEnd = false; // helpful when you know the message should be ending
uint8_t cmd = '\0';         // This could also be a buffer of known message length if your are interested in more than one byte for the command (e.g. command and parameter)

Then for processing the received bytes, there are many ways you can do this depending on your needs, but try something like this:

while (USART_RXBufferData_Available(&UARTCom1))             /* Loop until empty */
{
    uint8_t byte = '\0';                                    /* temp storage for the received byte */

    if (UARTCom1.buffer.RX_Count > 0)
    {
        byte = USART_RXBuffer_GetByte(&UARTCom1);           /* Get the byte */

        if(bMsgStart)
        {
            if(bExpectingEnd && (byte == END_BYTE))         /* End of message */  
                processMessage(cmd);                        /* Process the message now */
            else if(!bExpectingEnd)                         /* Message buffer isn't full but the message could be complete */
            {
                cmd = byte;                                 /* Since you know the message has started and hasn't ended, this is the byte you are interested in */
                /* If you were interested in more than one byte for the command/message this is where you'd add it to the buffer and increment the index until you've received everything in between the start byte and end byte */
                bExpectingEnd = true;                       /* The next byte should be the end byte -> Set the message end flag */  
            }
            else if((bExpectingEnd && (byte != END_BYTE))   /* Something went wrong or you received a bad message -> start over */
                 MsgBytesInit();                            /* Reset */ 
            else
                MsgBytesInit();                             /* Shouldn't get here but just in case -> Start over */
        }
        else if(byte == START_BYTE)
            bMsgStart = true;                               /* Set the start of message flag */    
        else
            MsgBytesInit();                                 /* Not what we are looking for or isn't the start of a message -> Reset */
    }
}


void MsgBytesInit()
{
    bMsgStart = false;          /* Reset the message start flag */
    bExpectingEnd = false;      /* Reset the message end flag */
    cmd = '\0';                 /* Reset the command (or message buffer)*/                  
}


void processMessage(uint8_t cmd)
{
     PORTR_DIR = 0b00000011;

    char led0Msg[] = "Turning ON LED0\r\n";
    char led1Msg[] = "Turning ON LED1\r\n";
    char ledoffMsg[] = "Turning OFF LEDS\r\n";

    switch(cmd)
    {
         case 'A':
         {
            UART->transmit(led0Msg, strlen(led0Msg));
            PORTR_OUT = ~(PIN0_bm); // Turn on LED0
         }
         break;

        case 'B':
        {
            UART->transmit(led1Msg, strlen(led1Msg));
            PORTR_OUT = ~(PIN1_bm); // Turn on LED1
        }
        break;

        default:
        {
            UART->transmit(ledoffMsg, strlen(ledoffMsg));
            PORTR_OUT = (PIN1_bm | PIN0_bm); // Turn off BOTH LEDS
        }
        break;
    }

    /* Done with the message */
    MsgBytesInit(); /* Reset */             
}