SimoCross
SimoCross

Reputation: 1

How to get out of a USART ISR with an atmega?

I have a problem with an atmega 1284p, I wrote this ISR to receive commands via serial. If I send an CR or LF to the end of the command, the program works correctly, but if I don't send either of them the program stays in the ISR blocking my program.

Since the ISR disables me all interrupts I don't know how to get out of it!

Can someone help me?

void USART_init(void)
{   
UCSR0B |= (1<<RXEN0) | (1<<TXEN0);
UCSR0C &= ~(1<<USBS0);                  //Stop bits 1
UCSR0C &= ~((1<<UPM00) | (1<<UPM01));   //Parity check disabled

UCSR0C |= (1<<UCSZ00) | (1<<UCSZ01);    //8 bit data
UCSR0B &= ~(1<<UCSZ02);             //8 bit data continue

UCSR0B |= (1 << RXCIE0);

UBRR0H = 0;
UBRR0L = 64; //9600 baud for a 16MHz Clock.
}


unsigned char USART_receive(void)
{
while(!(UCSR0A & (1<<RXC0)));
return UDR0;
}


ISR(USART0_RX_vect)
{
clean_variables();

do {
    cmd[inc] = USART_receive();
    inc++;
} while ((cmd[inc - 1] != '\n') && (command[inc - 1] != '\r'));
inc = 0;                            
comd = 1;
split();
}

Upvotes: 0

Views: 1336

Answers (3)

Rev
Rev

Reputation: 6122

Good practice would be to limit the code in the ISR to a minimum (and non-blocking!). You only need to read the new char from UDR0 into a buffer. Do all the other processing in the main-loop.

ISR(USART0_RX_vect) {
    // no need to check RXC0, the byte is available when the ISR is called
    cmd[inc] = UDR0;
    inc++;        
}

main() {
    if ((cmd[inc - 1] == '\n') || (cmd[inc - 1] == '\r')) {
        // process...
    }
}

A few thinks to point out

  • There may be syntax errors, I just wrote this down
  • You should handle a potential buffer overflow when inc gets to large
  • Declare inc and cmd as volatile as those are accessed from ISR and main context
  • To avoid potential race conditions you would need to make this "thread safe". For example by disabling interrupts (for as short as possible) while accessing cmd and inc in the main loop. Something like disable->copy values->re-enable->work with copies->repeat.

Upvotes: 0

Lundin
Lundin

Reputation: 214770

This program defeats the very purpose of using interrupts. You trigger the interrupt upon the first character received and then poll from there on. Instead you should trigger the interrupt once per character and then leave the ISR as soon as possible.

A common technique for interrupt-based UART reception is to use a ring buffer, which you fill from the ISR and empty from the caller. Also keep re-entrance in mind! Always, for every single interrupt you write, no exceptions.

Upvotes: 2

ChrisBD
ChrisBD

Reputation: 9209

You're issue is that you will keep checking the UART for data, even if there isn't any. May I suggest that you alter USART_receive so that it returns a success/failure flag and writes data into your buffer using a pointer that you pass it.

Then in your interrupt handler you not only check for your termination characters, but also only run your while loop whilst USART_receive is returning true.

You need to allow for the fact that the interrupt may trigger before your full message has been received i.e. no newline or carriage return present.

Upvotes: 0

Related Questions