Atmega168 breaking out of ISR

Question

I wrote a code to handle an ISR triggered by UART input. I'm looking for 2 characters or more. I can handle extra characters and garbage values, but problem arises when I get less than 2 chars, specially when an empty string is sent to the atmel. The UART_receive() on the atmel gets stuck at: while ( !(UCSR0A & (1<<RXC0)) );

Is there any way I can break out of this? The code is posted below:

unsigned char USART_Receive_string(unsigned char *x, unsigned char size)
{
unsigned char i = 0;

if (size == 0) return 0;            // return 0 if no space

while (i < size - 1) {              // check space is available (including additional null char at end)
    unsigned char c;
    while ( !(UCSR0A & (1<<RXC0)) );  // wait for another char - WARNING this will wait forever if nothing is received
    c = UDR0;
    if (c == '\0') break;           // break on NULL character
    x[i] = c;                       // write into the supplied buffer
    i++;
}
x[i] = 0;                           // ensure string is null terminated

return i + 1;                       // return number of characters written
}

The actual ISR in question is posted below. It reads the value, sets a flag, and exits as soon as possible.

ISR(USART_RX_vect)
{
    len = USART_Receive_string(uart_string,4); //makes space for 2 character and "\0" 
    UART_flag = 1;
} 

The the buffer is set up in main()

unsigned char uart_string[20];

Answer 1

First of all, you shouldn't share a simple variable between ISRs and the main loop: unsigned char uart_string[20]; should be volatile unsigned char uart_string[20];. Moreover, you must define it out of any scope.

Also, using possibly blocking loop in an ISR is strongly not recommended. In fact, whenever your microcontroller enter an ISR, everything else is stopped: thus, an ISR should last as short as possible. Plus, the USART_RX_vect triggers whenever a single character is received: this means that you'll have to wait for the next one. Thus, you mustn't use ISR like that: I would just use a global volatile variable to monitor the amount of received characters. Then, in the main loop, I would analyze the input characters whenever this counter is greater or equal than 2 . Finally, in your ISR, you try to read four input characters: '/0' is only one character, in the ASCII table it's called NULL, so you should only read up to three characters.

Answer 2

As a fundamental principle of design you should not be expecting to receive multiple characters in the ISR.

The interrupt fires when a character has been received, so that is all your program should count on finding. You can if you really want to poll the UART when you are done reading the first character and see if another has arrived, but there would generally not be time for that to have happened and you definitely should not be waiting for it in the ISR. Rather, if the ultimate consuming code needs multiple characters, then that should hold off executing dependent code until multiple invocations of the ISR have resulted in the collection of the needed number of characters.

Things can get a little more complicated on parts with multi-stage UART FIFOs, but that is not the case here.

Answer 3

I think I may have a very simple solution. Completely overlooked this while I was messing with the timers. I took the hint from a I2C routine in some threads in the the forum here.

This works with empty strings as well as large strings with garbage values. It doesn't seem to hold the system anymore, so it works for my purpose.

Anything I should add here?

int count_for_break = 0;
while ( !(UCSR0A & (1<<RXC0)) )     // wait for another char - Waits for 200 cycles.
        {
            count_for_break++;
            if(count_for_break > 200)
                break;
        }
//then reset the counter upon break or char receive.

Answer 4

I would suggest using code like this:

//Above your program, global variable.
char uartRXbuffer[100];//Adjust size!
int  uartRXbufferIndex = 0;
//bool uartRXmessage = false;

ISR(USART_RXC_vect){
   uartRXbuffer[uartRXbufferIndex++] = UDR;//read UART register into buffer. And increase index by 1.
   if(uartRXbufferIndex > sizeof(uartRXbuffer){
      //uartRXmessage = true;
      uartRXbufferIndex = 0;
   }//else if(uartRXbuffer[uartRXbufferIndex-1] == "\n"){
   //   uartRXmessage = true;
   //}
}

void loop(){
   HandleBuffer();
}

void HandleBuffer(){
   //Do something with your buffer values.
   //if(uartRXmessage){
      if(strstr(uartRXbuffer,"Command1")){
        //Code for when "Command1" is found in your buffer.  
      }
   //}
//uartRXmessage = false;
}

I havent tested this code, but the idea is correct I believe. You can un-comment the uartRXmessage stuff if you only want to handle the buffer when there is an end-of-line character or when the buffer is full. Alternatively you can implement a ring buffer.