High speed interrupt driven UART code not working

Go To Last Post
24 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hey guys,

 

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 (no relation to the start of the program of after a few loops), 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 & (1 << UDRE0)) {} //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
	}
}

 

Attachment(s): 

This topic has a solution.
Last Edited: Tue. May 15, 2018 - 01:55 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
	while (!(UCSR0A & 0b00100000)) {} //Wait for register empty

Now you are going to make me have to download the datasheet!....

 

 

OK so bit 5 is UDRE0. But then I have to get my glasses out so I can count the 0's to the left of the 1 in

0b00100000

Well, OK, that looks like the 1 is in the right place. But how much easier (and self documenting) would it have been if you'd simply written:

while (!(UCSR0A & (1 << UDRE0))) {} //Wait for register empty

BTW is the "missing 'H'" only at the very start or, after each _delay_ms(1000) does it happen each time?

 

It's also pretty bad design to rely on the _delay_ms(1000) anyway - if it came out of that before the full transmission had occurred then the code would be resetting transmit_index before it was ready. I'd have a flag set in the ISR when you finally disable the interrupt then just block on that flag rather than using the delay at all.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You enable interrupt in your main (in a loop), and disable in your ISR.

Why do not you keep the TX interrupt enabled?

 

BTW (it is off topic) what would happen if you change your Xal, change the baudrate (9600 is not a high speed : 115200 can be achieved)? magic numbers should be avoided.... if you want your program to become more complex or if you want to reread it within 3 - 6 months

 

"transmit_index" begins with 1 (and c arrays begin with c[0], c is inferior to Fortran, Pascal and R in this respect): it would be better, if you want to transmit "H", to make transmit_index beginning with 0 (and the test in the ISR might get simpler, too)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Good point, I changed the <register empty> checking.

For the missing 'H', it happens anywhere in the program.

As for the delay, the code is definitely not finished and this is only a module of the finished program. So the delay is there to make the serial output readable.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:
BTW is the "missing 'H'" ...

OP is introducing a race, as the UDRE interrupt will start firing continuously when the interrupt is enabled.  I don't know how the race will always turn out.  As for the missing first character:  "transmit_index" is set to 1, and that is past the H.

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The reason I don't keep the interrupt on (which would be the regular logic), is that it is necessary to write UDR0 to clear the interrupt source. If the register is not written, the ISR will start all over the moment it ends. Dummy data can't be written because the action of writing UDR0 starts a send of the data which I don't want. The other option is disabling the interrupt until it is needed again.

 

I can't change my crystal, I'm using a development board...

 

I know 9600 baud is rather low, in fact I intend on using 2 mega baud in the end, but I like to use a serial io program that can't handle 2 mega baud(So that makes it easier to test).

 

The 'H' should be transmitted in the main() code, which triggers the <USART_UDRE_vect> interrupt that transmits the remaining characters... I tought that would make sense.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Well the main triggers the first transmission, which does the 'H'. The following characters are handled by the ISR, so that makes it position 1 in the array of data.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

MrBertonio wrote:

Well the main triggers the first transmission, which does the 'H'. The following characters are handled by the ISR

Nope. The first interrupt is triggered as soon as you enable it. Then main tries to get the 'H' somewhere "in between" the already ongoing interrupt driven transmission.  That does not work and the H is missing.

Stefan Ernst

This reply has been marked as the solution. 
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

theusch wrote:
As for the missing first character: "transmit_index" is set to 1, and that is past the H.
Yeah but (to quote someone?), he does this:

		UDR0 = str[0]; //send first byte

so it's this "special" write that is supposed to send the 'H' separate from everything else. It's also the consequence of this write (UDRE set once it heads off down the line) that is supposed to trigger the back to back subsequent UDREs that send all the rest.

 

So the one thing I would not expect to see is the H missing.

 

However, like you say, the default condition is likely that UDR is empty so as soon as

	UCSR0B |= 0b00100000; //enable interrupt

is set it will trigger UDRE and start to send [1] onwards before it even reaches the line for:

	UDR0 = str[0]; //send first byte

Presumably a "fix" is to make that first write of UDR0 and then enable the interrupt.

 

All this reminds me why I generally stick to sync-Tx and just async-Rx ;-)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:
Yeah but (to quote someone?), he does this:

But that isn't reached until sometime later, and the ISR fires and the index is 1 and marches along.  It >>might<< come out later.  As I said, I don't know how the race will always come out.  Don't really care; race must be eliminated.

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I just tried the solution to move the interrupt enable call and now it works like a charm! Thanks a lot

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

I wouldn't "make this up as you go along". I'd google some well engineered existing solutions and see how they approach this. Something that "happens to work" is not necessarily the greatest solution.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

MrBertonio wrote:
send large amounts of data at high speeds with the absolute minimal amount of cpu overhead

So, surely, a chip with DMA is what you really need here ...

 

 

clawson wrote:
Something that "happens to work" is not necessarily the greatest solution

Especially when just "happens" to stop working!

 

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

MrBertonio wrote:
send large amounts of data

but an M328 only has 2k of ram!  Why the hurry?

 

Jim

 

Mission: Improving the readiness of hams world wide : flinthillsradioinc.com

Interests: Ham Radio, Solar power, futures & currency trading - whats yours?

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

FYI...if sending to PC (at high speed) you can adjust the uart buffering in the PC hardware settings...this can make a difference if chars are dropping on the PC side.

When in the dark remember-the future looks brighter than ever.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

MrBertonio wrote:
to send large amounts of data at high speeds with the absolute minimal amount of cpu overhead.

This sentence is meaningless. Give us real numbers.

What baudrate (could derive from your code).

How much data?

 

16MHz is not a good choice for "common uart" baudrate's.

MrBertonio wrote:
//9600 baud UBRR0L = 207; UBRR0H = 0;
???

This is a very low baudrate. (have not checked if that is ok with 16MHz crystal, See table's in the datasheet).

I have done 115k2 with an 1.8432MHz crystall. That is about

octave:2> 115200/9600*16/1.84 =  104.35 times the performance of what you do / want.

CPU load was considerable though, and I decided to specify 3.6864MHz as the minimum F_CPU for my 115k2 application.

Paul van der Hoeven.
Bunch of old projects with AVR's:
http://www.hoevendesign.com

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The UART TXD interrupt is different from all the other IRQs.  You only activate it when you need to use it.  All other times, the TxD IRQ is disabled.  The TXD interrupt is used when you have a buffer of multiple data bytes to be sent from the UART.  You load the buffer with the string of bytes that you want to send and turn on the TXD IRQ.  Since the TxD IRQ activates whenever the UART's TX data register is empty, it will immediately execute the IRQ when you enable it.  The TxD IRQ gets the next byte from the Tx buffer and puts it into the UART's TX shift register. Next it increments the out (tail) pointer.  If the buffer's in (or head) pointer is equal to the buffer's out (or tail) buffer, then there is no more data waiting to be sent from the buffer, and the IRQ code disables the IRQ itself.   If enabled, the TXD IRQ will execute when the TX shift register in the UART is empty.  So the IRQ will run over and over again if there is no data in the TX buffer.  The AVR will appear to have crashed.  It hasn't, it is just pumping meaningless data from the TX buffer to the UART.  

 

16 MHz works fine with 9600 baud.  It is one of the few "non UART" crystal values that divides down to 9600 baud with only about 1% error.  It also works fine with 31,250 baud MIDI.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

Wdll, thanx Simonetta and Mr Bettorio fo explaining how TX interrupts wotk; but if one wnts to send a known array of chars (bytes) is iesting for a counter (whar a loop would do; often arrays of byters are shorter than 255 ...) slower than saving/restoring registers (an ISR does, I bet?)

If what I fear is exact, a loop in the main would be faster than an TX - ISR driven software...

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

dbrion0606 wrote:
If what I fear is exact, a loop in the main would be faster than an TX - ISR driven software...
Do you mean because of the cycle overhead getting into/out of the ISR? I suppose it's true, but what's nice about interrupt Tx is that it is "fire and forget". Once the process is started you can leave it to it and get on with other things. In some situations that may outweigh the small additional cycle cost.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Well, I think the rentering/leaving ISR overhead might be higher than a classical loop. And, as main allows the Tx ISR, it will be blocked (perhaps other things would be "only" made way  slower) for a longer time , when sending an array, than the time it takes looping and sending chars.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

dbrion0606 wrote:
Well, I think the rentering/leaving ISR overhead might be higher than a classical loop.

I don't necessarily disagree.  However, OP's

MrBertonio wrote:
with the absolute minimal amount of cpu overhead.
could be interpreted as "leave as many cycles available for other work".  Sitting in a wait loop will waste [nearly] all of the CPU for the duration of the message activity.

 

OP would have to further explain.  Low clock speed and low baud rate seem to contradict the aims. 

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

theusch wrote:

dbrion0606 wrote:
Well, I think the rentering/leaving ISR overhead might be higher than a classical loop.

I don't necessarily disagree.  [In fact, with the typical fast clock and short packets when doing SPI it is generally hard to find a case for interrrupt-driven SPI, right?.]  However, OP's

MrBertonio wrote:
with the absolute minimal amount of cpu overhead.
could be interpreted as "leave as many cycles available for other work".  Sitting in a wait loop will waste [nearly] all of the CPU for the duration of the message activity.

 

OP would have to further explain.  Low clock speed and low baud rate seem to contradict the aims. 

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

OP does not have low clock speed, but very low baud rate (with a RPi, arduini can transmit up to 115.2k -thisis a RPi limit, maybe an absurd one).

 

Another thing makes me uneasy :

 

Simonetta wrote there is a 1% error on the baud rate, but I do not see which are the consequences on the error on chars (I suppose it is a systematic error, and that chars are resynchonized at every stop/start transmission: at the end of the character, bit is samples with a time offset of 1% ... but remains OK if there is not too much "noise"; but maybe my way of reading is wrong).

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

ISR overhead vs wait loop timing  has to be analyzed on a case by case basis.

 

On a m328 project @ 16MHz a couple of years ago I needed to transfer data @ 9600 baud.

The data was transmitted in packets twenty times each second. Each packet was 37 characters.

Due to the way the data was processed, all of the data became available essentially at the same time, so I could not just intermittently send data.

I tested how fast I could "dispose" of the data using various  transmit buffer sizes  and waiting when the buffer was full.

My results (for this specific case):

    No Buffer     36.4 ms    
       4-byte     32.3 ms
       8-byte     28.1 ms
      16-byte     19.8 ms
      32-byte      3.14 ms
      64-byte      0.12 ms
     128-byte      0.12 ms

Based on these results, I determined that the 32-byte buffer was sufficient and optimized the time available for other processing (during each 50ms cycle) while minimizing impact on RAM usage. For buffer sizes less than 16 bytes, I did not have enough processing time to handle all of the other tasks. At 16 bytes I could do everything, but had very little "extra" processing time in the remotely possible case of collision by multiple random external interrupts during a cycle.

 

YMMV (and probably will) wink

 

Edit: FYI the Transmit ISR required about 3us per byte...

 

 

David (aka frog_jr)

Last Edited: Wed. May 16, 2018 - 03:17 PM