Display previously received UART values

Question

This should be easy to answer to anyone familiar with C. I want to display the previous values of a variable (receive register of a UART (RS-232) on a microcontroller) on an LCD. This is my current implementation and it works ok. But I'd like to know if there is a way to spend less time in my interrupt routine. Currently, the peripheral is configured to jump to the interrupt routine as soon as it receives one new character in the UART feed. Suggestions anyone?

//Initialization
char U1RX_data = '\0';
char p0_U1RX_data = '\0';
char p1_U1RX_data = '\0';
char p2_U1RX_data = '\0';
char p3_U1RX_data = '\0';
char p4_U1RX_data = '\0';
char p5_U1RX_data = '\0';
char p6_U1RX_data = '\0';
char p7_U1RX_data = '\0';

char U1buf[] = {p7_U1RX_data, p6_U1RX_data, p5_U1RX_data,
     p4_U1RX_data, p3_U1RX_data, p2_U1RX_data,
    p1_U1RX_data, p0_U1RX_data, U1RX_data, '\0'};
disp_string(-61, 17, 1, U1buf); //X, Y, mode, string

void _U1RXInterrupt(void){
    p7_U1RX_data = p6_U1RX_data;
    p6_U1RX_data = p5_U1RX_data;
    p5_U1RX_data = p4_U1RX_data;
    p4_U1RX_data = p3_U1RX_data;
    p3_U1RX_data = p2_U1RX_data;
    p2_U1RX_data = p1_U1RX_data;
    p1_U1RX_data = p0_U1RX_data;
    p0_U1RX_data = U1RX_data;

    U1RX_data = U1RXREG;
    IFS0bits.U1RXIF = 0; 
}

Answer 1

You can use memmove in your interrupt, like this:

void _U1RXInterrupt(void)
{
    memmove(&U1Buf[0], &U1Buf[1], 7);
    U1Buf[7] = U1RX_data;
    ...
}

That replaces the assignments that you're currently performing manually and is a bit more idiomatic.

I'm hoping I understood you properly; the main point is to use memmove to shift the buffer down by one byte. As an aside, it's important to use memmove instead of memcpy when the destination and source buffers overlap, as in this example.

Answer 2

As Emerick noted, memmove() will work nicely if you have access to it. If not, just grab a simple implementation of it from Google, it shouldn't take up too much instruction memory.

What's your clock frequency on the microcontroller? Another thing to think about is that if your clock speed is significantly higher than your baud rate a lot of these things become non-issues. For example, if your clock speed is 16 MHz, you really don't have to worry about creating the world's shortest ISR as long as your not doing anything insanely computationally intensive inside it. Also, if you're clocking the system significantly faster than the baud rate, polling is also an option.

EDIT: Another option I just thought of, using interrupts.

You could store your buffer as an array of chars and then keep a global index to the next empty slot, like this:

#define UART_BUF_SIZE 16
char uart_buf[UART_BUF_SIZE] = {0, 0, 0, 0, 0, 0, 0, 0
                                0, 0, 0, 0, 0, 0, 0, 0};
char uart_buf_index = 0;

Then in your ISR, all you need to do is dump the new byte into the bucket that index points at and increment the index. This will automatically overwrite the oldest character in the buffer as the start position rotates back around the buffer.

void my_isr()
{
    uart_buf[uart_buf_index] = get_uart_byte();
    uart_buf_index = (uart_buf_index + 1) % UART_BUF_SIZE;
}

Basically at this point you've got a buffer with a rotating starting position, but it saves you from having to move 16 bytes of memory every ISR. The trick comes in reading it back out, because you have to account for the wrap-around.

char i;
for (i = uart_buf_index; i < UART_BUF_SIZE; i++)
{
    lcd_write_byte(uart_buf[i]);
}
for (i = 0; i < uart_buf_index; i++)
{
    lcd_write_byte(uart_buf[i]);
}

Answer 3

I would create an array of previous values, and treat it as a circular buffer. The interrupt routine then simply records the new value in the next slot, overwriting the last value, and incrementing the index.

#define DIM(x)  (sizeof(x)/sizeof(*(x)))
static int  index = 0;
static char uart[8];

void _U1RXInterrupt(void){
    if (++index >= DIM(uart))
        index = 0;
    uart[index] = U1RXREG;
    IFS0bits.U1RXIF = 0;        
}

int uart_value(unsigned n)
{
    int i = index + DIM(uart) - (n % DIM(uart));
    if (i > DIM(uart))
        i -= DIM(uart);
    return(uart[i]);
}

I'm assuming synchronous, non-threaded operation; if you have to deal with multi-threaded, then there's work to protect the index variable from concurrent access. It also returns zeroes for the last but one reading before the buffer is full. Etc. If you are confident of your coding, you can remove the modulo operation, too.

Answer 4

You could always make a fixed length circular buffer

char buffer[8] = { '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0',);
char position = 0;


/* useable if you have a version of disp_string that takes a "number of chars"*/
char buffer_nprint()
{
    /* print from position to the end of the buffer*/
    disp_string(-61, 17, 1, &buffer[position], 8 - position);
    if (position > 0)
    {
        /* now print from start of buffer to position */
        disp_string(-61, 17, 1, buffer, position);
    }
}

/* if you _don't_ have a version of disp_string that takes a "number of chars"
   and are able to do stack allocations*/
char buffer_print()
{
    char temp[9];
    temp[8] = '/0';
    memcpy(temp, &buffer[position], 8 - position);
    memcpy(temp, buffer, position);
    temp[8] = '/0';
    disp_string(-61, 17, 1, temp);
}

char buffer_add(char new_data)
{
    char old_data = buffer[position];
    buffer[position] = new_data;
    position = ((position + 1) & 8);
}

void _U1RXInterrupt(void)
{
    buffer_add(U1RXREG);
    IFS0bits.U1RXIF = 0;
}