1

I want to make a interrupt driven uart program, to send large amounts of data at high speeds with the absolute minimal amount of cpu overhead. I combined existing code and reading of the datasheet to make this code. It compiles without errors or warnings in Atmel Studio 7 on an atmega328p (Atmega328p Xplained Mini).

The problem that I'm having is that data is erratic, sometimes it sends 'ello!' sometimes nothing for a while. The 'H' is often skipped, I don't understand this since the ISR shouldn't execute before the 'H' has been copied from UDR0 to be sent.

Any help would be greatly appreciated!

Greetings,

Bert.

#define F_CPU 16000000

#include <avr/io.h>
#include <avr/interrupt.h>
#include <util/delay.h>
#include <string.h>

volatile uint8_t transmit_index = 0;
volatile char str[] = "Hello!\n";
volatile uint8_t len = 6;

int main(void){
    UCSR0A = 0b00000010;
    UCSR0B = 0b00111000;
    UCSR0C = 0b00000110;

//9600 baud
    UBRR0L = 207; 
    UBRR0H = 0;

    DDRD |= 0x02;

    sei();

    //Flash led
    DDRB |= 0b00100000;
    PORTB |= 0b00100000;
    _delay_ms(1000);
    PORTB &= ~0b00100000;
    _delay_ms(1000);

    while (1){  
        transmit_index = 1;

        //Enable udre interrupt
        UCSR0B |= 0b00100000; //enable interrupt

        //Send first byte in main()
        while (!(UCSR0A & 0b00100000)) {} //Wait for register empty
        UDR0 = str[0]; //send first byte

        _delay_ms(1000);
    }
}

ISR(USART_UDRE_vect) {
    //Buffer empty, ready for new data
    if (transmit_index < (len + 1)) {
        UDR0 = str[transmit_index];
        transmit_index++;
    } else {
        UCSR0B &= ~0b00100000; //disable interrupt
    }
}
  • What happens if you enable interrupts *after* you send the first byte? Or not send it in the loop at all, and just enable interrupts? And don't reset `transmit_index` first thing you do in the loop? – Some programmer dude May 15 '18 at 13:16
  • 2
    You've got some interactions between the main loop and the ISR (you're resetting `transmit_index` every time through but the ISR increments it and will likely trigger as soon as the interrupt is enabled causing issues). I think you'd want to wait for the ISR to be disabled, wait for the register to be empty, load in the data, enable the ISR and repeat. – John Szakmeister May 15 '18 at 13:23
  • @Someprogrammerdude Moving the enabling of the interrupt worked! I have no clue why but many thanks ^^ – Bert van Rossem May 15 '18 at 13:51

2 Answers2

4

per the datasheet:

"When the Data Register Empty Interrupt Enable (UDRIE) bit in UCSRnB is written to '1', the USART data register empty interrupt will be executed as long as UDRE is set"

As soon as you enable the interrupt, the ISR is triggered, thus skipping the "H". You have a couple of options. 1) Enable the interrupt after you send the H. 2) Just use the ISR to send the entire message, including the H (e.g. don't send anything in the main routine. 3) Use the Tramsmit Complete ((TXC) interrupt. If you use this, send the H in the main routine, and once it is transferred, the ISR will trigger and your ISR will send the rest of the message.

Lastly, change "transmit_index < (len + 1)" to transmit_index <= len. There is no need to waste instructions inside an ISR

Joe Thomas
  • 325
  • 1
  • 7
  • Just commenting the suggested optimization on eliminating the `+ 1`. It shouldn't be necessary, compilers can usually figure such out well to generate optimal code, so I say write what's more intuitive. A valid optimization for this ISR code could be ensuring that it is only enabled when there is at least 1 character to send, then within the ISR, having the proper comparison after the increment, whether the interrupt should be disabled. That would remove an unnecessary interrupt entry (for the emptied buffer), and the elimination of an `else` path also helps. – Jubatian May 16 '18 at 09:52
  • Good point, the compiler should be smart enough to figure it out. I typically error on the side of trying to optimize myself. – Joe Thomas May 16 '18 at 15:23
0

In your main loop, change this line :

transmit_index = 1;

for this line :

transmit_index = 0;

str[0] = 'H' but you start at index [1]...