1

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];
  • I don't see an ISR here. Your code is for UART Rx polling. Do you have a Uart RX ISR in parallel, which reads from UDR0? This may cause problems. Or is this function called inside the ISR? And hopefully your *x points to a reserved array big enough to hold your string. Otherwise you will overwrite other data in memory. – mic Dec 1 '14 at 8:24
  • Edited my question. Included the actual ISR now. – tsf144 Dec 1 '14 at 15:53
  • Just implement some timeout functionality, so the the while loop will break if no new data is transmitted withing the specified time. – Gerben Dec 1 '14 at 17:07
  • Do you know how can I do that? like put one of the timers into an ISR? Will it work if it's already stuck in this ISR? – tsf144 Dec 1 '14 at 17:19
  • 5
    Receiving multiple characters within an ISR is a big waste of time. Keep your ISR short. Interrupt when a char is received, get the single character and put it in a buffer, then exit the ISR. Let main() do the rest. – Tut Dec 1 '14 at 18:17
1

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.

| improve this 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.

| improve this answer | |
0

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.
| improve this answer | |
  • 3
    If you do this, make sure you stay in the loop for at least a full character time at the lowest baud-rate you expect to use. This can work if the entire string is sent at once, but may fail if there are gaps between characters such as can occur with typing using terminal programs or when using VCP adapters such as USB to serial. Failure may be avoided if you save your received characters if the string is incomplete. A better solution is to receive one character per interrupt and not waste time waiting for another (see my comments above). – Tut Dec 2 '14 at 11:17
0

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.

| improve this answer | |

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.