interrupt-driven USART reception: I'm stumped.

Hi all,
I'm trying to program the arduino to receive DMX-512, which runs at a 250Kbit baud rate. I can receive the channel information, but I get intermittent bad values, which I think is because of other interrupts running and disrupting the serial reception.

However, there is an interrupt on the Atmega168 that indicates a new byte has been received by the USART, USART_RXC_vect. So I thought, if I did the time-critical work of reading the incoming values inside the serial receive ISR, it would prevent that work from being screwed up by other interrupts, and also allow me to do other things besides poll the serial receive all the time.

I'm having trouble getting the ISR to work. Here's my test code:

#include <avr/io.h>
#include <avr/interrupt.h>

int receiveroutputenable = 3;  // receiver output enable (pin2) on the max485.  will be left low to set the max485 to receive data.
int driveroutputenable = 4;    // driver output enable (pin3) on the max485.  will left low to disable driver output.
int rxpin = 0;                // serial receive pin on the Arduino, which takes the incoming data from the MAX485.

volatile int index = 0;  //a variable incremented in the ISR and printed in the main routine.


void setup() {
  pinMode(receiveroutputenable, OUTPUT);
  pinMode(driveroutputenable, OUTPUT);
  digitalWrite(receiveroutputenable, LOW);
  digitalWrite(driveroutputenable, LOW);    //sets pins 3 and 4 to low to enable reciever mode on the MAX485.

  pinMode(rxpin, INPUT);  //sets serial pin to receive data

  Serial.begin(250000);  //the baud rate for DMX is 250000bps (since each bit is 4 microseconds).

  UCSR0B |= (1 << RXCIE0);  //sets RXCIE0 in register UCSR0B to 1, to enable USART reception interrupt.

  sei();  //Enable global interrupts
  
}  //end setup()

ISR(USART_RXC_vect){  //the USART receive Interrupt Service Routine
  index++;  //increment index
}

void loop()  {
  Serial.println(index);  //print index.  If the ISR is running, this should increment (although not in lockstep with the ISR).
}

If the ISR(USART_RXC_vect) is firing, it should increment "index" every time. Then at some point the main code should print out the new value. However, the serial code just prints out 0's, so I conclude the ISR isn't firing (I'm using Putty to read the incoming values at 250000bps).

Can anyone help me get the ISR up and running? I'm new to assembly/reading the Atmega168 data sheet, so it's probably something pretty simple.

Thanks for the help!

The Serial library is already using the ISR for the UART_RXC vector. That is why your ISR is not being called.

When you get bad info, are you dropping data or getting corrupted data? Dropping COULD be caused by too many higher priority ISRs being called, but the only other one being used by default is the TIMER0 ISR, and its handler is pretty short.

Another cause of dropping data could be that you are overflowing the receive buffer. The size of the buffer is defined to be 128 bytes inside the arduino core. If it receives more than 128 bytes while you are busy dealing with the data you have already received, it will just start throwing out data because it has nowhere to put it.

If you are getting 'corrupted' data, it's time to pull out the oscilloscope and check for noise or slew-rate problems on your RX line.

Good luck,
-Chris

Thank you for your post, it was illuminating to me on a number of levels.

So it sounds like if I want to access the UART_RXC ISR, I'll need to configure/access the USART manually by setting the bits in the control register and reading the buffer register, rather than using the Arduino serial library. I can probably figure out how to do that.

I think that the problems receiving data I was having are due to buffer overrun, not noise. I'm using good twisted pair shielded cable that I've properly terminated, etc. per EI485 best practices. I'll check for slew rate on my o-scope, but I've used this controller with no problems before, so I'm sure it's fine.

I was assuming that ALL the interrupts were enabled by default.

As a first step, I was attempting to read and post to the serial port all 512 levels as they came in. But I'm actually only interested in a small subset of those values, so perhaps if I write that code first, the resulting routine will be fast enough that it won't overflow the buffer.

I'll post my results, thanks for your help so far.

Okay, I have some answers:

First of all, the serial receive interrupt is USART_RX_vect, not USART_RXC_vect as I had in my first post. Defining USART_RXC_vect is like defining NOTHINGWILLHAPPEN_vect, it's a valid name for an interrupt vector, but it will never execute, because the CPU will never call it.

When you define USART_RX_vect, however, the compiler throws you an error, because the arduino software defines that interrupt vector and includes it in your code, whether you reference Arduino serial functions or not. So you have to comment out some parts of wiring_serial.c

IMHO, and I'm new at this so please correct me if I'm wrong about this, including the serial functions even if they're not referenced is a bug, not a feature. It increases the uploaded sketch size, and reduces the flexibility of the programmer, with no upside. Anyway, in the next post is my workaround:

The following code configures the USART manually without use of the Arduino serial functions. The reason for this is that to get it to compile you have to comment out some of the wiring_serial.c file, and thus we can't use the Arduino serial commands.

Functionally, every time the receive interrupt is called it increments an index and then prints it to the serial buffer (again, manually).

If you run the code, and you have Putty or hyperterminal or whatever properly configured, you'll just get the ASCII chart over and over as the index overflows.

Here it is:

#ifndef cbi
#define cbi(sfr, bit) (_SFR_BYTE(sfr) &= ~_BV(bit))
#endif
#ifndef sbi
#define sbi(sfr, bit) (_SFR_BYTE(sfr) |= _BV(bit))
#endif
/* Definitions for setting and clearing register bits
*  See http://www.sigterm.de/projects/sens-o-nuts/Dokumentation/atmel/libc/group__avr__sfr.html */

int receiveroutputenable = 3;   
/* receiver output enable (pin2) on the max485.  
*  will be left low to set the max485 to receive data. */

int driveroutputenable = 4;     
/* driver output enable (pin3) on the max485.  
*  will left low to disable driver output. */

int rxpin = 0;                  // serial receive pin, which takes the incoming data from the MAX485.

unsigned long baud = 250000;    // The baud rate for DMX is 250000bps, since each bit = 4 microseconds.
unsigned long clockspeed = F_CPU; 
/* F_CPU is a predefined constant equal to the clock speed of the microcontroller.
*  for the Arduino duomilanove it is 16000000.
*  I'm going to dump it into clockspeed because I'm worried about it being a signed long,
*  which could screw up the bitwise math I'm going to do on baudprescale, maybe. */

unsigned long baudprescale;     
/* The baud prescale that will be loaded into the 
*  UBRR0H and UBRR0L registers to set the USART baud rate */


volatile int i = 0;           //dummy variable for for() loops, etc.
volatile byte ReceivedByte = 0; //holds the incoming byte from the receive buffer.

void setup() {
  pinMode(receiveroutputenable, OUTPUT);
  pinMode(driveroutputenable, OUTPUT);
  digitalWrite(receiveroutputenable, LOW);
  digitalWrite(driveroutputenable, LOW);    //sets pins 3 and 4 to low to enable reciever mode on the MAX485.

  pinMode(rxpin, INPUT);  //sets serial pin to receive data

  baudprescale = (((clockspeed / (baud * 16UL))) - 1); //

  cli();  //disable interrupts while initializing the USART

  sbi(UCSR0A, TXC0);
  cbi(UCSR0A, FE0);
  cbi(UCSR0A, DOR0);
  cbi(UCSR0A, UPE0);
  cbi(UCSR0A, U2X0); //
  cbi(UCSR0A, MPCM0);
  /* set transfer complete flag (TXC0 = 1).
  *  clear Frame Error flag (FE0 = 0).   
  *  clear Data overrun flag (DOR0 = 0).
  *  clear Parity overrun flag (UPE0 = 0).
  *  disable doubling of USART transmission speed (U2X0 = 0).  
  *  Disable Multi-Processor Communication Mode-- whatever that is. (MCPM0 = 0)  */
  
  sbi(UCSR0B, RXCIE0);
  cbi(UCSR0B, TXCIE0);
  cbi(UCSR0B, UDRIE0);
  sbi(UCSR0B, RXEN0);
  sbi(UCSR0B, TXEN0);
  cbi(UCSR0B, UCSZ02);
  /* Enable Receive Interrupt (RXCIE0 = 1).
  *  Disable Tranmission Interrupt (TXCIE = 0).
  *  Disable Data Register Empty interrupt (UDRIE0 = 0). 
  *  Enable reception (RXEN0 = 1).
  *  Enable transmission (TXEN0 = 1). 
  *  Set 8-bit character mode (UCSZ00, UCSZ01, and UCSZ02 together control this, 
  *  But UCSZ00, UCSZ01 are in Register UCSR0C). */
  
  cbi(UCSR0C, UMSEL00);
  cbi(UCSR0C, UMSEL01);
  cbi(UCSR0C, UPM00);
  cbi(UCSR0C, UPM01);
  sbi(UCSR0C, USBS0);
  sbi(UCSR0C, UCSZ00);
  sbi(UCSR0C, UCSZ01);
  cbi(UCSR0C, UCPOL0);
  /* USART Mode select -- UMSEL00 = 0 and UMSEL01 = 0 for asynchronous mode.
  *  disable parity mode -- UPM00 = 0 and UPM01 = 0. 
  *  Set USBS = 1 to configure to 2 stop bits per DMX standard.  The USART receiver ignores this 
  *  setting anyway, and will only set a frame error flag if the first stop bit is 0.  
  *  But, we have to set it to something.
  *  Finish configuring for 8 data bits by setting UCSZ00 and UCSZ01 to 1.  
  *  Set clock parity to 0 for asynchronous mode (UCPOL0 = 0). */
  
  UBRR0L = baudprescale; // Load lower 8-bits of the baud rate value into the low byte of the UBRR register 
  UBRR0H = (baudprescale >> 8); // Load upper 8-bits of the baud rate value into the high byte of the UBRR register 
  // ***note to self: at some point add a command to write UBRR0H high bits to 0 per datasheet for "future compatibility"

  sei(); // Enable the Global Interrupt Enable flag so that interrupts can be processed 
}  //end setup()

ISR(USART_RX_vect){
  ReceivedByte = UDR0;  
  /* The receive buffer (UDR0) must be read during the reception ISR, or the ISR will just 
  *  execute immediately upon exiting. */
  i++;
  UDR0 = i;
} // end ISR

void loop()  {
 
} // end loop()

If you have to do too much low level and interrupt coding you may be better of
just using avr-gcc and skipping the Arduino libraries.

For serial communication I like to use circular buffers. You add a character
at the tail and remove from the head. When the head pointer and tail pointer
are equal the buffer is empty.

The transmit interrupt is enabled when the TX buffer is not empty. An empty
TX buffer disables the interrupt.

The receive interrupt is enabled when the RX buffer is not full. A full RX buffer
disables the interrupt.

I use the queue code from Knuth TAOCP, Fundamental Algorithms (1973).

(* jcl *)

For a project that I was working on, I simply cut and pasted the SIGNAL(SIG_USART_RECV) handler from wiring_serial.c to my arduino sketch and added my own code within the handler

I can still use the the Serial functions provided by arduino, but would any problem be caused by this?

I take your point that there are some trade-offs between ease of use and configurability. But in this case I don't see the upside of automatically including the serial receive ISR in the uploaded code when there's no serial definitions in the code.

I'll look into that circular buffer algorithm if I can track down that book, it sounds interesting.

Cutting and pasting the ISR definition into the code is definitely an easier work-around than mine, thanks for the tip.

My copy of TAOCP is 25 years older but I would be surprised if the queues
implementation was removed. Every CS library will have a copy of TAOCP.

Art of Computer Programming, Volume 1: Fundamental Algorithms (3rd
Edition) (Art of Computer Programming Volume 1) (Hardcover)

Hardcover: 672 pages
Publisher: Addison-Wesley Professional; 3 edition (July 17, 1997)
Language: English
ISBN-10: 0201896834
ISBN-13: 978-0201896831

(* jcl *)

P.S. The first finder of any error in Knuth's books receives 0x$1.00 ($2.56), deposited to their account at the Bank of San Serriffe; significant suggestions are also worth 0x$0.20 ($0.32) each. If you are really a careful reader, you may be able to recoup more than the cost of the books this way.

See http://www-cs-faculty.stanford.edu/~uno/taocp.html

Thanks for the info, I love the hexadecimal reward system.

Hi i got the same issue,
This compile perfectly in rev 0012 but won't compile in 0013,

This is controling a light that move left and right and have a RGB output to make a light control by DMX.

Thing is that some PWM bug were fixed in 0013 but i can't use it!
is the fix only modifing the librairie the way to go???
why were the 0012 working and not the 0013

Thanks guys for good support as usual

here is my code,

#include <Servo.h>
#define myubbr (16000000L/16/250000-1)
Servo servo1; Servo servo2;

char oldDMX;
volatile unsigned char DMX[64];
volatile int DMXChannel=0;

ISR(USART_RX_vect)
{
  char temp,temp1;
  temp1 = UCSR0A;
  temp = UDR0&0xFF;
  if ((temp1 & (1<<FE0))||temp1 & (1<<DOR0)) //Break
  {
    DMXChannel = 0;
    return;
  }
  else if (DMXChannel<(char)25)
  {
    DMX[DMXChannel++]=temp&0xFF;
  }
}

void setup()
{
  pinMode(0,INPUT); //DMX Serial IN
  pinMode(6,OUTPUT); //BLUE
  pinMode(5,OUTPUT); //GREEN
  pinMode(11,OUTPUT); //RED
  pinMode(10,OUTPUT); // TILT
  pinMode(9,OUTPUT); // PAN

  servo1.attach(9); //analog pin 0
  servo2.attach(10); //analog pin 1
  delay(100);
  UBRR0H = (unsigned char)(myubbr>>8);
  UBRR0L = (unsigned char)myubbr;
  UCSR0B |= ((1<<RXEN0)|(1<<RXCIE0));//Enable Receiver and Interrupt RX
  UCSR0C |= (3<<UCSZ00);//N81
 
}
void loop()
{
    servo1.write(map(DMX[1],0,255,25,125));
    servo2.write(map(DMX[2],0,255,30,120));
    analogWrite(11,255-DMX[3]);
    analogWrite(5,255-DMX[4]);
    analogWrite(6,255-DMX[5]);
}

I did a test using the Exemple ASCIITable.
i added this:

ISR(USART_RX_vect)
{
}

And got the typical error. in both 0012 and 0013

did the same with BLINK.PDE
this time it only crash in 0013
even if i did not use any serial command???
any clue?
THanks

Well, it compiles fine on my computer, with my altered wiring_serial.c

Once I load it onto the Arduino, it doesn't do anything, though -- from poking at it it looks like it's not setting the values in DMX[] to anything, they just stay at their default value of 0. I haven't gone through it in detail, though.

Have you managed to receive DMX correctly with this code, under any version of the Arduino library?

If you want to compile it, here's what I did to wiring_serial.c (towards the end, there's a part that's commented out):

/*
  wiring_serial.c - serial functions.
  Part of Arduino - http://www.arduino.cc/

  Copyright (c) 2005-2006 David A. Mellis

  This library is free software; you can redistribute it and/or
  modify it under the terms of the GNU Lesser General Public
  License as published by the Free Software Foundation; either
  version 2.1 of the License, or (at your option) any later version.

  This library is distributed in the hope that it will be useful,
  but WITHOUT ANY WARRANTY; without even the implied warranty of
  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
  Lesser General Public License for more details.

  You should have received a copy of the GNU Lesser General
  Public License along with this library; if not, write to the
  Free Software Foundation, Inc., 59 Temple Place, Suite 330,
  Boston, MA  02111-1307  USA

  $Id: wiring.c 248 2007-02-03 15:36:30Z mellis $

  Modified on March 1, 2009 by Norbert Pozar
*/

#include "wiring_private.h"

// Define constants and variables for buffering incoming serial data.  We're
// using a ring buffer (I think), in which rx_buffer_head is the index of the
// location to which to write the next incoming character and rx_buffer_tail
// is the index of the location from which to read.

// use only powers of 2 for the buffer size to produce optimal code
// 8, 16, 32, 64, 128 or 256

// input buffer
#define RX_BUFFER_SIZE 128
// output buffer
// set to 0 to disable the output buffer altogether (saves space)
#define TX_BUFFER_SIZE 32

unsigned char rx_buffer[RX_BUFFER_SIZE];

unsigned char rx_buffer_head = 0;
unsigned char rx_buffer_tail = 0;


void beginSerial(long baud)
{

#if defined(__AVR_ATmega8__)
      UBRRH = ((F_CPU / 16 + baud / 2) / baud - 1) >> 8;
      UBRRL = ((F_CPU / 16 + baud / 2) / baud - 1);
      
      // enable rx and tx
      sbi(UCSRB, RXEN);
      sbi(UCSRB, TXEN);
      
      // enable interrupt on complete reception of a byte
      sbi(UCSRB, RXCIE);
#else
      UBRR0H = ((F_CPU / 16 + baud / 2) / baud - 1) >> 8;
      UBRR0L = ((F_CPU / 16 + baud / 2) / baud - 1);
      
      // enable rx and tx
      sbi(UCSR0B, RXEN0);
      sbi(UCSR0B, TXEN0);
      
      // enable interrupt on complete reception of a byte
      sbi(UCSR0B, RXCIE0);
#endif

      // defaults to 8-bit, no parity, 1 stop bit
}

// advanced function, saves ~200 bytes because it doesn't use the long division
// prescaler for the baud rate can be found using the formula
// prescaler = fClock / (16 * baud rate) - 1
// see beginSerial
void beginSerialWithPrescaler(unsigned int prescaler)
{

#if defined(__AVR_ATmega8__)
      UBRRH = prescaler >> 8;
      UBRRL = prescaler;
      
      // enable rx and tx
      sbi(UCSRB, RXEN);
      sbi(UCSRB, TXEN);
      
      // enable interrupt on complete reception of a byte
      sbi(UCSRB, RXCIE);
#else
      UBRR0H = prescaler >> 8;
      UBRR0L = prescaler;
      
      // enable rx and tx
      sbi(UCSR0B, RXEN0);
      sbi(UCSR0B, TXEN0);
      
      // enable interrupt on complete reception of a byte
      sbi(UCSR0B, RXCIE0);
#endif

      // defaults to 8-bit, no parity, 1 stop bit
}

int serialAvailable()
{
      unsigned char i = RX_BUFFER_SIZE + rx_buffer_head - rx_buffer_tail;
      i %= RX_BUFFER_SIZE;

      return i;
}

int serialRead()
{
      // if the head isn't ahead of the tail, we don't have any characters
      if (rx_buffer_head == rx_buffer_tail) {
            return -1;
      } else {
            unsigned char c = rx_buffer[rx_buffer_tail];
            rx_buffer_tail = rx_buffer_tail + 1;
            rx_buffer_tail %= RX_BUFFER_SIZE;
            return c;
      }
}

void serialFlush()
{
      // don't reverse this or there may be problems if the RX interrupt
      // occurs after reading the value of rx_buffer_head but before writing
      // the value to rx_buffer_tail; the previous value of rx_buffer_head
      // may be written to rx_buffer_tail, making it appear as if the buffer
      // were full, not empty.
      rx_buffer_head = rx_buffer_tail;
}

/* #if defined(__AVR_ATmega8__)  *********THIS PART COMMENTED OUT*********
SIGNAL(SIG_UART_RECV)
#else
SIGNAL(USART_RX_vect)
#endif
{
#if defined(__AVR_ATmega8__)
      unsigned char c = UDR;
#else
      unsigned char c = UDR0;
#endif

      unsigned char i = rx_buffer_head + 1;
      i %= RX_BUFFER_SIZE;

      // if we should be storing the received character into the location
      // just before the tail (meaning that the head would advance to the
      // current location of the tail), we're about to overflow the buffer
      // and so we don't write the character or advance the head.
      if (i != rx_buffer_tail) {
            rx_buffer[rx_buffer_head] = c;
            rx_buffer_head = i;
      }
} */

// buffered output
#if (TX_BUFFER_SIZE > 0 )

// TX is buffered
// ************************
// tested only for ATmega168
// ************************

unsigned char tx_buffer[TX_BUFFER_SIZE];

unsigned char tx_buffer_head = 0;
volatile unsigned char tx_buffer_tail = 0;


// interrupt called on Data Register Empty
#if defined(__AVR_ATmega8__)
SIGNAL(SIG_UART_DATA)
#else
SIGNAL(USART_UDRE_vect) 
#endif
{
  // temporary tx_buffer_tail 
  // (to optimize for volatile, there are no interrupts inside an interrupt routine)
  unsigned char tail = tx_buffer_tail;

  // get a byte from the buffer      
  unsigned char c = tx_buffer[tail];
  // send the byte
#if defined(__AVR_ATmega8__)
      UDR = c;
#else
      UDR0 = c;
#endif

  // update tail position
  tail ++;
  tail %= TX_BUFFER_SIZE;

  // if the buffer is empty,  disable the interrupt
  if (tail == tx_buffer_head) {
#if defined(__AVR_ATmega8__)
    UCSRB &=  ~(1 << UDRIE);
#else
    UCSR0B &=  ~(1 << UDRIE0);
#endif

  }

  tx_buffer_tail = tail;

}


void serialWrite(unsigned char c) {

#if defined(__AVR_ATmega8__)
       if ((!(UCSRA & (1 << UDRE)))
#else
      if ((!(UCSR0A & (1 << UDRE0)))
#endif 
  || (tx_buffer_head != tx_buffer_tail)) {
    // maybe checking if buffer is empty is not necessary, 
    // not sure if there can be a state when the data register empty flag is set
    // and read here without the interrupt being executed
    // well, it shouldn't happen, right?

    // data register is not empty, use the buffer
    unsigned char newhead = tx_buffer_head + 1;
    newhead %= TX_BUFFER_SIZE;

    // wait until there's a space in the buffer
    while (newhead == tx_buffer_tail) ;

    tx_buffer[tx_buffer_head] = c;
    tx_buffer_head = newhead;

    // enable the Data Register Empty Interrupt
#if defined(__AVR_ATmega8__)
    UCSRB |=  (1 << UDRIE);
#else
    UCSR0B |=  (1 << UDRIE0);
#endif

  } 
  else {
    // no need to wait
#if defined(__AVR_ATmega8__)
      UDR = c;
#else
      UDR0 = c;
#endif
  }
}

#else

// unbuffered write
void serialWrite(unsigned char c)
{
#if defined(__AVR_ATmega8__)
      while (!(UCSRA & (1 << UDRE)))
            ;

      UDR = c;
#else
      while (!(UCSR0A & (1 << UDRE0)))
            ;

      UDR0 = c;
#endif
}

#endif

Yes this code compile on version 0012. no colision with the interrupt vector.

Does this means that when i need serial in version 0013 i need to manually to modify wiring_serial.c back again to the original?

This code receive DMX sgnal, and move a Light PAN/TILT, and dim RGB LED individually. yes it works
Thanks

Does this means that when i need serial in version 0013 i need to manually to modify wiring_serial.c back again to the original?

It depends on whether you need Serial.flush(), which as near as I can tell is the only thing this change to wiring_serial.c would disable. I started this thread in hopes that someone would chime in with a better work-around, but at the moment this is the only one I've got.

yes it works

Interestingly, this code doesn't work with my controller (an Enttec USB-DMX Pro with Lightfactory software), even when I compile it in the 0012 version of the software, with an unmodified wiring_serial.c

What controller(s) have you used it with?

I'm currently trying to write some software that will work with my (or any) controller, so I'm interested to know. Thanks,

Max

Any more on the DMX front?

Please can you post some complete DMX projects so I can have a good starting point for making my own?

Thanks so much! :slight_smile:

Hi Dolby,
If you want to send DMX, you can check out the playground on this site. If you want to receive it, try my project:
http://blog.wingedvictorydesign.com/2009/03/20/receive-dmx-512-with-an-arduino/

both sites are the top results of a Google search, so you may want to start there.

Does anyone know if this bug has been fixed in the current version?