writing ISR for UART

Generally speaking, most compilers have an _interrupt_ keyword or similar that tell the compiler that it must restore the status register and perhaps other registers when returning from an interrupt.

"case" is safe inside an ISR.

Most of the classic families of bugs with ISRs have to do with shared RAM or shared memory. This falls under the category of IPC problems in computer science, i.e.:

I wish I had the time to enumerate everything you can do wrong with an ISR (regrettably, I have a day job and my fingers are old), but the most common error is to fail to realize that the interrupt may occur at any time. You need to have a critical section protocol to prevent interrupts when modifying data structures or hardware that are shared. The most common technique is to disable interrupts for a short period of time.

For example, in the code below, if the interrupt occurs between approximately points /* 1 */ and /* 2 */ there may be serious logical trouble. I'll leave it to you to figure out why.

struct { int putpoint; int getpoint; int nchars; unsigned char body[QSIZE]; } queue;

_interrupt_ void rx_isr(void) { unsigned char c;

/* Get c from hardware */

if (queue.nchars < QSIZE) { queue.body[queue.putpoint] = c; queue.nchars++; queue.putpoint++; if (queue.putpoint >= QSIZE) queue.putpoint = 0; } }

unsigned char get_a_char_from_queue(void) { unsigned char return_value = 0;

/* 1 */ if (queue.nchars) { queue.nchars--; return_value = queue.body[queue.getpoint]; queue.getpoint++; if (queue.getpoint >= QSIZE) queue.getpoint = 0; } /* 2 */

return(return_value); }

Other links that may be helpful.

There are many ways to hang yourself. I'm too lazy to type much more.

--
David T. Ashley              (dta@e3ft.com)
http://www.e3ft.com          (Consulting Home Page)
http://www.dtashley.com      (Personal Home Page)
http://gpl.e3ft.com          (GPL Publications and Projects)

dear David, thanks for a nice reply. actually in my case problem is that i am no expert on C coding. and my idea about my work is segregated. i can' compile all due to lack of expertise on coding. i become confused how t complete. Let me tell you about my intension, it's like following:

i want to execute individual primitives of Zigbee protocol. let's take tw primitives like MLME-ASSOCIATE.request and MLME-ASSOCIATE.indication.i'l use them as two different "case". Actually my main intention is to measur the power consumption for each primitives of PHY,MAC,NWK and APL.

I am thinking to do it like this.as i don't find any complete Zigbe device i choose CC2420 Zigbee node connected with PICDEM Z boar containing a micro-controller PIC18f4620. usually i use the MPLAB IDE t run the project and MPLAB ICD 2 to debug the program that contain complete Zigbee stack command. instead of running the complete stack m intension is to run the individual command and to observe the power chang in the CC2420. to avoide the multiple debugging my plan is like this: i'l add all the command in the LabVIEW simulator and the each command wil written as a 'case' in C.by using the MPLAB and ICD 2 the program will b debugged in the PIC18f4620.this program should contain an ISR so that onc i debug the program i just select the command by clicking a button in th LabVIEW simulator connected to the PICDEM Z by a serial port (UART) according to the command it will directed to a interrupt table tha trigger an ISR. and accordingly the case will be selected and the registe value of the CC2420 will change. then i'll record the power change.

With a help from a colleague i sketch my program could be like this :

The interrupt service routine for the UART is something like th following:

at the beginning of main

- open the serial setting the right baud rate

- enable IRQ

---------------------------

:::few coding line though i am not sure whether it works:::::

#include #include

void main (void) {

// Configure all PORTB pins for output

TRISB = 0;

/ * open the serial setting the right baud rate

Open the USART configured as 8N1, 2400 baud, in polled mode

*/

OpenUSART (USART_TX_INT_OFF & USART_RX_INT_ON & USART_ASYNCH_MODE & USART_EIGHT_BIT & USART_CONT_RX & USART_BRGH_HIGH, 103);

TXSTA = 0x20; // setup USART transmit RCSTA = 0x90; // setup USART receive // Use high priority interrupt IPR1bits.TXIP = 1;

""Here i have to enable IRQ"""'

}

"""now i have to write the ISR. i got an example like following""" bu don't know how to write my own program.::::

/*********************************************************************

• Function: void UART_ISR(void)

*

• PreCondition: UART interrupt has occured

*

• Input: None

*

• Output: None

*

• Side Effects: None

*

• Overview: None

*

• Note: This function is supposed to be called in the ISR

• context.

********************************************************************/

void UART_ISR(void) { // NOTE: All local variables used here should be declared static static BYTE rxdata;

// Store a received byte, if pending, if possible if(PIR1bits.RCIF) { // Get the byte rxdata = RCREG;

// Clear the interrupt flag so we don't keep entering this ISR PIR1bits.RCIF = 0;

// YOU CAN USE THE rxdata HERE // OR PUT IN A CIRCULAR BUFFER TO BE USED IN THE WHILE(1) IN TH MAIN // IN THIS EXAMPLE // Copy the byte into the local FIFO, if it won't cause an overflow

if(RXHeadPtr != RXTailPtr - 1) { if((RXHeadPtr != vUARTRXFIFO + sizeof(vUARTRXFIFO)) | (RXTailPtr != vUARTRXFIFO)) { *RXHeadPtr++ = i; if(RXHeadPtr >= vUARTRXFIFO + sizeof(vUARTRXFIFO)) RXHeadPtr = vUARTRXFIFO; } } }

#IF 0 //TX CODE CAN BE SKIPPED

// Transmit a byte, if pending, if possible if(PIR1bits.TXIF) { if(TXHeadPtr != TXTailPtr) { TXREG = *TXTailPtr++; if(TXTailPtr >= vUARTTXFIFO + sizeof(vUARTTXFIFO)) TXTailPtr = vUARTTXFIFO; } else // Disable the TX interrupt if we are done so that we don't keep entering this ISR

{ PIE1bits.TXIE = 0; } }

#ENDIF

}

""""Moreover, in the main application, i need to declare the needed variables:""""""""

#define BAUD_RATE 19200

// Ring buffers for transfering data to and from the UART ISR: // - (Head pointer == Tail pointer) is defined as an empty FIFO // - (Head pointer == Tail pointer - 1), accounting for wraparound, // is defined as a completely full FIFO. As a result, the max data // in a FIFO is the buffer size - 1. static BYTE vUARTRXFIFO[65]; static BYTE vUARTTXFIFO[17]; static BYTE *RXHeadPtr = vUARTRXFIFO, *RXTailPtr = vUARTRXFIFO; static BYTE *TXHeadPtr = vUARTTXFIFO, *TXTailPtr = vUARTTXFIFO;

[...]

now the point is how i can make the program complete. could you please help me out. and another important question how to add my "case" in the program.

thanks to read the long writing.

thanks in advance and waiting for a kind guidance.

This looks a lot like code I've used successfully for more than a decade. Is there really a problem if q.nchars is accessed atomically and the q.nchars-- operation occurs in a single uninterruptible instruction? On the 68K machine where i run the code, q.nchars--; becomes subq.w #1, 4(A1)

so if the interrupt handler adds a character to the queue while the get_a_char_from_queue() function is executing there should be no problems. q.nchars is the only variable that is modified by both the interrupt handler and by the getchar routine.

I forsee a problem if queue.nchars was a 16-bit or

32-bit integer and the code was running on an 8-bit processor. nchars could get messed up if an interrupt occured between the mulitple instructions needed to decrement the value.

I think this discussion points out one of the reasons that it is a really good idea to understand the processor and the assembly language if you are going to write interrupt handlers.

**** INTERRUPT HAPPENS HERE ****

First of all, I just wanted to say _something_ to the OP ... I really can't generate high-quality examples when I don't have a lot of time to throw at it.

That being said, are you __SURE__ that is the only problem?

How about this scenario:

a)The queue is full.

b)The "get" code executes to the point marked above as "INTERRUPT HAPPENS HERE".

c)The interrupt runs and overwrites the character that was to be retrieved. The issue is that the count has been decremented before the character from the queue body has been copied out.

Perhaps. But it also points out that human beings aren't made to produce software and that any of us can screw up a trivial example.

It might also point out the fact that I'm never wrong (*).

--
David T. Ashley              (dta@e3ft.com)
http://www.e3ft.com          (Consulting Home Page)
http://www.dtashley.com      (Personal Home Page)
http://gpl.e3ft.com          (GPL Publications and Projects)

(*) In my own mind.

Good point. I looked at my code and I find that I don't decrement the queue length until AFTER I fetch the character. That's why I said your example looked a lot like my code---but I didn't say it was identical.

No matter how you write the code, you are going to have problems with data integrity if the queue becomes full. No amount of interrupt masking will save you from that problem!

Which is why it's always good to test your code with extreme cases before you release it to your customers. That won't eliminate all problems, but it can reduce their frequency.

The only programmers I know that have never produced a bug are those that haven't finished their first real application!

It's much easier to never be wrong than it is to always be right. You can achieve the former by doing nothing!

Thanks for the examples. Just because nobody has complained about my ISRs and queue handlers in the last 10 years doesn't mean the code is perfect. It's always a good idea to reexamine old code before you decide you can simply copy and paste the code into a new compiler with a different processor.

Mark Borgerson

Dear Mark Borgerson, can i have a look your program to test. it might give me some idea for m code.

thanks

Reza

The reason that your code (with the modifications you mentioned in another post, and a little care to avoid buffer overflows) is safe on the m68k is that the m68k has an atomic decrement instruction. For many embedded processors, that is not the case. First, it could be that the nchars variable is wider than the processor (16-bit int on an 8-bit processor, for example). This is generally due to the programmer not understanding their target - it's rare that you would need more than 256 bytes of buffer on an 8-bit micro, so the programmer has picked the wrong types when they used "int". Secondly, many modern processors are RISC load-store architectures, so that the decrement is done in three operations (load the old value, decrement it, store it again) and the interrupt can break into any of these.

If you *don't* have access to such an atomic decrement, you have two options - disable interrupts around the critical code to make the operation atomic, or use a better buffer structure! (Hint - do you really need nchars?)

Dear Mark Borgerson, can i have a look your program to test. it might give me some idea for m code.

thanks

Reza

I've seen and used queue code that compares the get and put pointers to detect whether a character is available. I avoided it for a couple of reasons:

• the pointer comparisons are not atomic on the 68K although that may not matter when checking for getptr != putptr

• there are times when it is handy to have a count of the characters in the buffer. With that count you can implement a faster loop to pull characters from the queue. It's easier to increment and decrement a counter at one instruction each, than it is to get the queue length by pointer arithmetic that accounts for queue wraparound.

I do use that technique in my serial handler for the AT91SAM7 processor. There, I use the DMA support in the Atmel chips, so there is not an interrupt for each character or FIFO. That leaves me with comparing the getptr with the DMA receive pointer to detect incoming characters.

One of the first uses for my serial I/O code on a new system is always as part of the monitor used to upload new code. If you start getting a lot of checksum errors when uploading new code, it's a good indication that you've missed something in the serial I/O driver.

The simple example code here also lacks a few elements:

1: there is no error reporting for queue full errors or queue empty errors. Without those error reports, the program can't distinguish between a valid 0x00 character and the result of a get operation on an empty queue.

1. There should be a ChAvailable() function that returns the number of characters available. This prevents either blocking on an empty queue or the queue empty 0x00 character problem.
2. Many UARTS have just one interrupt, which has to handle receive, transmit and error interrupts, so the ISR will be more complex than that shown. If the UART has a FIFO, it gets to be even more fun!

Mark Borgerson

You can find the source code at

Click on the button for the U4S and you'll see a link to the source code.

Mark Borgerson

You simply calculate "nchars = putptr - getptr", then adjust if there has been a wrap around the end of the buffer. It does not matter that this is not atomic - if a new character has come in during this operation, then the new putptr will be greater than the one used, and your nchars will be one character too small. But that's just the same as if the new character came in after your code had finished.

Thus you calculate "nchars = putptr - getptr" at the start of your code, and any new incoming characters are ignored until you next run the code

- just as if they arrived after the code was finished.

All this is correct - the code above is (hopefully!) just an example.

That sounds simple--but there are a few details to remember:

1. putptr can be smaller than getptr if the buffer has just wrapped around.
2. because of #1, you have to be careful about correcting for buffer wrap if you intend to actually use the character count in a loop and not just compare it with zero.
3. pointer arithmetic on 8 or 16-bit processors may involve quite a large number of instructions.

With 8-bit processors, it may be best to limit the buffer size to something less than 256 bytes. If you need maximum efficiency, you could also use the technique of making the buffer length equal to an even power of 2. Then you can handle buffer wrap with a single AND instruction. I don't think I've had to worry that much about performance since my 6502 and 6800 assembly coding days.

Yep. There's usually a lot more going on under the hood of a good serial driver than we've discussed in these examples. It gets to be even more fun if there are FIFOs or DMA and you really want to minimize time in the handlers in high-traffic situations. I went through all that in a data multiplexer/concentrator application a decade or two back. We didn't exactly have to count cycles, but with 4 input channels and no FIFOs, we did have to optimize the receive interrupt handlers as best we could. The input data could be pretty 'bursty' and we had to make sure we could handle four separate channels in less than one character time at 115KB. IIRC the result involved a separate handler for each channel and each handler used hard-coded register addresses rather than computing addresses from a channel index.

Mark Borgerson

I was just clowning around, naturally.

There was some famous discussion in a computer science journal (it had to do with early IPC work), where something like 6 people, all professional researchers, wrote in to correct the comments of the previous person who wrote in, and each one made a new mistake. These were people experienced in the field with IQ's way up there.

Human beings weren't made for software.

--
David T. Ashley              (dta@e3ft.com)
http://www.e3ft.com          (Consulting Home Page)
http://www.dtashley.com      (Personal Home Page)
http://gpl.e3ft.com          (GPL Publications and Projects)

Good eye!

You are correct! Holding too much state is a common cause of software defects.

Between getpoint, putpoint, and nchars : I think you can show that only two (any two) of the three are required.

I think though that if you eliminate nchars you can't fill the queue. If (getpoint == putpoint), the buffer is either empty or full, and you can't tell which without slightly more information.

However, if you have (putpoint and nchars) or (getpoint and nchars) I think it will work.

--
David T. Ashley              (dta@e3ft.com)
http://www.e3ft.com          (Consulting Home Page)
http://www.dtashley.com      (Personal Home Page)
http://gpl.e3ft.com          (GPL Publications and Projects)

I've always liked the following:

#define SIZE 64 // should be power of 2, and < 256

volatile unsigned char putptr; unsigned char getptr; unsigned char buffer[SIZE];

isr( ) { unsigned char c = GET_CHAR_FROM_HARDWARE();

if( (int) (putptr - getptr) < SIZE ) buffer[putptr++ % SIZE] = c; }

int uart_getchar() { if( putptr == getptr ) return -1; return buffer[getptr++ % SIZE]; }

SIZE should be a power of two, and less than 256 so that you can tell the difference between an empty and completely full buffer.

As long as the incremented value for 'putptr++' can be atomically written to memory, interrupts don't need to be disabled in the uart_getchar() routine.

Thanks. Your solution is _amazingly_ clever. It seems that essentially the extra required state to tell the difference between empty and full is stored in the upper bits of getptr and putptr.

I think your solution is a special case of a more general problem that occurs in computer systems, but I'm not a good enough mathematician to state the problem.

The problem is essentially how to arrange the state vector in a way that maps to the desired result under the machine instructions available. Your solution has the property that the C and Z flags from a subtraction give the desired information.

One can arrange the state vector any way that one wants, but the question is how efficiently the machine instructions "equivalence class" it.

Let me give a related example. In implementing state machines, one may see that the statement:

if ((state == X) || (state == Y) || (state == Z))

occurs frequently. Clearly, testing against 3 different values isn't efficient. But if we choose X as 7, Y as 15, and Z as 31, then:

if (state & 0x3)

may do the trick.

The critical question is the machine instructions available.

It just so happens that with your choice of state vector (for the queue), the result of a subtraction equivalence classes in the way desired.

Your queue implementation is related to vertical counters and to the problem stated above; but like I said, I'm not a good enough mathematician to make a general statement.

--
David T. Ashley              (dta@e3ft.com)
http://www.e3ft.com          (Consulting Home Page)
http://www.dtashley.com      (Personal Home Page)
http://gpl.e3ft.com          (GPL Publications and Projects)

Not really. nchars=0 and nchars=256 are just as indistinguishable (assuming the canonical case of keeping everything in 8-bit variables) as the two cases 'completely full' and 'completely empty' in the implementation using only pointers.

A simpler solution is to step back a bit and re-consider how many bytes have to be in the buffer for it to be considered "full". If you keep the head and tail pointer separated by at least one position at all times, it becomes easier to implement the ring buffer. Squeezing out that one last byte of size is not always worth the effort.

As I said above, "You simply calculate "nchars = putptr - getptr", then adjust if there has been a wrap around the end of the buffer."

It's not exactly hard to do:

sint8_t nchars = putptr - getptr; if (nchars < 0) nchars += lengthOfBuffer;

You might want to think about different cases if your buffer sizes are close to your counter type sizes (i.e., a buffer of more than 128 bytes when using 8-bit counters).

If your buffers are a power of two in length, the an AND mask is even smaller and faster.

That's true, but irrelevant. We are working here with indexes into an array, not pointers - that way we can use minimal sized types, and happily do arithmetic, masking, and comparisons on them. Actually putting data into or taking data out of the buffer uses array access - it's the same whether or not you try to track nchars independently!

That's always the case when working with small micros - think about your types, and choose appropriate ones. It is unlikely, but not impossible, that you'll need a buffer of more than 256 bytes (255 actually - you can never have the buffer entirely full as it will be indistinguishable from entirely empty). The way to make your code flexible, efficient and portable is to typedef a "qcounter_t" type which is used throughout the implementation, and which is typedefed somewhere at the start of the module depending on the size of the buffers you need. Ideally you should typedef it to "uint_fast8_t" rather than just "uint8_t" - that will produce good code on everything from an 8-bit AVR to a ColdFire (which works faster with 32-bit data than 8-bit data).

Holding too much state can be useful for efficiency, if you can be sure that the extra data is consistent. In this particular case, keeping consistency requires atomic operations or interrupt disabling, which is too expensive for the potential gains from knowing all three numbers.

Yes - the number of characters in the buffer at any time is the number of characters put in the buffer, minus the number of characters removed.

That is correct. Without nchars, you can't fill the last byte of the buffer. But generally that is a cost worth paying for the advantages of the getpoint/putpoint method.

Note that even with nchars, you come across the same situation if you have a 256 byte buffer and are keeping your variables as 8-bit data for efficiency.

True, but then you lose your independence of the variables. The point of using putpoint and getpoint is that the interrupt routine only ever modifies putpoint (it *reads* getpoint, but that's no problem), and the extraction routine only ever modifies getpoint (it *reads* putpoint - that's no problem if it can be read atomically, and if not, you can work around the issue without having to disable interrupts). If you track nchars, then the two functions both have to modify the same variable.