USART RX interrupt fires 3x after re-enabling

While digging into ways to receive DMX using Arduino (atmega328P) i came to the conclusion that if i disable the RX interrupt and re-enable it, it fires 3 times before it actually passes on data on the input port. Now in the sketch / method i was using this was accounted for, but i had somehow always assumed that this was because the DMX frame contained these dummy values, but now i have to conclude that the interrupt just gets fired 3x. Now i read nothing about this in the datasheet. Can someone please confirm that this is exactly what is happening ? or what is happening ?

Your description sounds like the UART receives 3 characters. Or it is something in your code. Who knows?

Maybe something simple like prefix characters, or the trailing CR & LF / check digit from the preceding ‘sentence’ ?

Yeah yeah i know i didn't post it.

Know that is definitely not true.
This is the code that uses the hwSerial class

#define PWM_LED_PIN 5
#define DMX_PING 13
#define MAX_DMX_CHANNELS 512
#define NUMBER_OF_CHANNELS 512

uint16_t dmxaddress = 1;
uint16_t i = 0;
uint8_t dmxreceived = 0;
uint16_t dmxcurrent = 0;
uint8_t dmxvalue[NUMBER_OF_CHANNELS];

uint8_t dmxnewvalue = 0;
volatile uint8_t zerocounter = 0;
volatile bool storeSerial = false;

void setup() {

  pinMode(PWM_LED_PIN, OUTPUT); // first dmxchannel LED value
  pinMode(DMX_PING, OUTPUT); //ping LED
  SetupTimerRegisters();
}

void loop()  {
  while (dmxnewvalue != 1) {
    if (Serial.available()) HandleSerial();
  }
  Action();
  EnableDMX();
}

void SetupTimerRegisters() {

  cli();
  //bitClear(UCSR0B, RXCIE0);
  bitClear(TCCR2A, COM2A1);
  bitClear(TCCR2A, COM2A0);
  bitClear(TCCR2A, COM2B1);
  bitClear(TCCR2A, COM2B0);
  bitSet(TCCR2A, WGM21);
  bitClear(TCCR2A, WGM20);
  bitClear(TCCR2B, FOC2A);
  bitClear(TCCR2B, FOC2B);
  bitClear(TCCR2B, WGM22);
  bitClear(TCCR2B, CS22);
  bitClear(TCCR2B, CS21);
  bitSet(TCCR2B, CS20);
  OCR2A = 64;
  bitClear(TIMSK2, OCIE2B);
  bitSet(TIMSK2, OCIE2A);
  bitClear(TIMSK2, TOIE2);
  sei();

  Serial.begin(250000, SERIAL_8N2);
}

void EnableDMX() {
  dmxnewvalue = 0;
  dmxcurrent = 0;
  zerocounter = 0;
  i = 0;
  while (Serial.available()) {
    uint8_t dummy = Serial.read();
  }
  bitSet(TIMSK2, OCIE2A);
}

void Action() {
  static bool pin = true;
  pin = !pin;
  analogWrite(5, dmxvalue[0]);
  //digitalWrite(13, pin);
  if (dmxvalue[0] > 200) digitalWrite(13, HIGH);
  else digitalWrite(13, LOW);
}


void HandleSerial() {
  if (!storeSerial) {
    uint8_t dummy = Serial.read();
    return;
  }
  dmxreceived = Serial.read();
  dmxcurrent++;
  if (dmxcurrent > dmxaddress) {
    dmxvalue[i] = dmxreceived;
    i++;
    if ((i == NUMBER_OF_CHANNELS) || (dmxcurrent >= MAX_DMX_CHANNELS))  {
      storeSerial = false;
      dmxnewvalue = 1;
    }
  }
}


ISR(TIMER2_COMPA_vect) {
  if (bitRead(PIND, PIND0)) {
    zerocounter = 0;
  }
  else {
    zerocounter++;
    if (zerocounter == 20)
    {
      bitClear(TIMSK2, OCIE2A);
      storeSerial = true;
    }
  }
} //end Timer2 ISR

and this is the code that defines the USART RX ISR

#define PWM_LED_PIN 5
#define DMX_PING 13
#define MAX_DMX_CHANNELS 512

#define NUMBER_OF_CHANNELS 510

#define BAUDDMX 250000UL

#define RECEIVE_FORMAT 0x06 // 8N1 (could be 8N2 aswell.

#define UBDMX ((F_CPU / (8 * BAUDDMX)) - 1)



volatile uint16_t dmxaddress = 1;  
volatile uint16_t i = 0;
volatile uint8_t dmxreceived = 0;
volatile uint16_t dmxcurrent = 0;
volatile uint8_t dmxvalue[NUMBER_OF_CHANNELS];

volatile uint8_t dmxnewvalue = 0;
volatile uint8_t zerocounter = 0;

void setup() {

  pinMode(PWM_LED_PIN, OUTPUT); // first dmxchannel LED value
  pinMode(DMX_PING, OUTPUT); //ping LED
  init_uart();
  SetupTimerRegisters();
}

void loop()  { 
  ReceiveDMX(); 
  Processing();

}

void ReceiveDMX() {
  dmxnewvalue = 0;
  dmxcurrent = 0;
  zerocounter = 0;
  i = 0;  
  UCSR0C = RECEIVE_FORMAT; //0x06; // 8N1
  bitSet(TIMSK2, OCIE2A);
  while (dmxnewvalue != 1);
}

void Processing() {  // main processing
  static bool pin = true;
  pin = !pin;
  analogWrite(5, dmxvalue[0]);
  digitalWrite(13, pin);
  if (dmxvalue[0] > 200) {
    digitalWrite(13, HIGH);
  }
  else {
    digitalWrite(13, LOW);
  } 
}

void init_uart() {
  DDRD |=  (1 << PORTD1); // set TX pin  to output
  DDRD &= ~(1 << PORTD0); // set RX pin to input
  UCSR0A = 0x02; // 1<<U2X | 0<<MPCM;
  UCSR0B = 1 << RXCIE0 | 0 << TXCIE0 | 0 << UDRIE0 | 1 << RXEN0 | 1 << TXEN0 | 0 << UCSZ02; // Enable TX & RX, disable RX interrupt
  UCSR0C = RECEIVE_FORMAT; // 8N1
  UBRR0H = (UBDMX >> 8);  // set baud rate
  UBRR0L = (UBDMX & 0xFF); // HL register
}

void SetupTimerRegisters() {  // Sets up Timer2 to fire every 4us
  cli();
  bitClear(TCCR2A, COM2A1);
  bitClear(TCCR2A, COM2A0);
  bitClear(TCCR2A, COM2B1);
  bitClear(TCCR2A, COM2B0);
  bitSet(TCCR2A, WGM21);
  bitClear(TCCR2A, WGM20);
  bitClear(TCCR2B, FOC2A);
  bitClear(TCCR2B, FOC2B);
  bitClear(TCCR2B, WGM22);
  bitClear(TCCR2B, CS22);
  bitClear(TCCR2B, CS21);
  bitSet(TCCR2B, CS20);
  OCR2A = 64;
  bitClear(TIMSK2, OCIE2B);
  bitSet(TIMSK2, OCIE2A);
  bitClear(TIMSK2, TOIE2);
  sei();
}

ISR(TIMER2_COMPA_vect) {
  if (bitRead(PIND, PIND0)) {  // checks if the pin has gone HIGH
    zerocounter = 0;
  }
  else {
    zerocounter++;
    if (zerocounter == 20)  // if 80us have passed 'Break' has been detected
    {
      bitClear(TIMSK2, OCIE2A);  // disable Timer2 Interrupt
      bitSet(UCSR0B, RXCIE0);   // enable
    }
  }
} //end Timer2 ISR

ISR(USART_RX_vect) {
  /* The receive buffer (UDR0) must be read during the reception ISR, or the ISR will just
     execute again immediately upon exiting. */
  static uint8_t dummies = 3;  // somehow the ISR gets fired 3 times after re-enabling
  dmxreceived = UDR0;
  if (dummies) {
    dummies--;
    return;
  }
  
  dmxcurrent++;     //increment address counter starts at 0
            // this skips the start code
  if (dmxcurrent > dmxaddress) {        //check if the current address is the one we want.
    dmxvalue[i] = dmxreceived;
    i++;
    if ((i == NUMBER_OF_CHANNELS) || (dmxcurrent > MAX_DMX_CHANNELS - 2)) {
      bitClear(UCSR0B, RXCIE0);
      dummies = 3;                             // reset number of dummy values
      dmxnewvalue = 1;                        //set newvalue, so that the main code can be executed.
    }
  }
} // end ISR

Well no, clearly not. There is no buffer, the interrupt gets disabled in the ISR and re-enabled only on detection of the 'break' Whatever comes after the 'break' is the startcode ('0') which gets discarded, but somehow the interrupt gets fired 3 times previous to registering the startcode. I am not saying it is not valid system, it may be fine to fire the ISR 3x before actually passing on received values, it's more that nothing is mentioned about it anywhere.

Is that the valid statement to exit an ISR?

what does that have to do with it ?
Would you prefer it like this

ISR(USART_RX_vect) {
  /* The receive buffer (UDR0) must be read during the reception ISR, or the ISR will just
     execute again immediately upon exiting. */
  static uint8_t dummies = 3;  // somehow the ISR gets fired 3 times after re-enabling
  dmxreceived = UDR0;
  if (dummies) {
    dummies--;
  }
  else {
    dmxcurrent++;     //increment address counter starts at 0
    // this skips the start code
    if (dmxcurrent > dmxaddress) {        //check if the current address is the one we want.
      dmxvalue[i] = dmxreceived;
      i++;
      if ((i == NUMBER_OF_CHANNELS) || (dmxcurrent > MAX_DMX_CHANNELS - 2)) {
        bitClear(UCSR0B, RXCIE0);
        dummies = 3;                             // reset number of dummy values
        dmxnewvalue = 1;                        //set newvalue, so that the main code can be executed.
      }
    }
  }
} // end ISR

Bad code yields unexpected results.

Seriously ? I mean i ask a question, you don't know the answer, and all you can come up with is that.
Just be honest and say that you don't know.

After all that I don't want to dig deeper into your code, honestly.

1 Like

Then don't reply to it to begin with. If you are just here (the forum) to feed your feeling of superiority but not to help, then refrain from getting involved into topics you do not have anything useful to contribute to. (all spelling errors are in this reply to see if you can find them)

There is a FIFO buffer in the UART hardware.

Thanks for your invaluable advice, I'll remember you.

This can easily be solved by reading the USART status register in the interrupt code.
Paul

The FIFO also can be flushed before interrupts are enabled, see Flushing the Receive Buffer in the datasheet.

Isn't that only 1 byte on an AVR ?

… answer that for yourself, you obviously know more than we do… we’d on,y be guessing.

(hint: if you are expecting help, show some respect for people that are offering suggestions to help you)

When an avr uart is “idle” it will store up to 3 characters - two bytes in the fifo, and one in the receive shift register. Each will cause an interrupt when interrupts are Re-enabled.

The fifo will hold the first two chars received after interrupts are disabled, and the shift register the last character received. So if you disable interrupts and send “test 1 2 3”, you should “receive” “te3”…

You can easily write a test program to demonstrate this.

1 Like

A more sensible answer might be: No, it is not a valid statement to exit an ISR.

Good luck with your project, and attitude!

What is it with you people. Look @westfw Just answers the question without any condescension. How hard can that be. There is something wrong with the attitude here.

I totally respected you until that comment. I do also request the code if i think i need it, and i was just hoping someone would just enlighten me like @westfw did. You were trying to be helpful, and as it turned out, even the code may have explained how the question came up.
@DrDiettrich, You probably actually know the answer (and if not, you know it now) But for some reason you started pointing at something completely different, which had nothing to do with the issue. I am confident that anyone that judged my attitude and complained about it, knew that. And to point out a different mistake with the use of a question as rather condescending in my opinion, which i am entitled to.

Anyway, the thing that i am a little confused about is that as far as i know,

  • either something gets sent if the next frame is transmitted while the ISR is disabled,
  • or nothing gets sent during the time that the UART is idle if it finds the break of next frame.
    but somehow the interrupt gets fired 3 times regardless.

@johnwasser I always really appreciate your input, in my topics, and other's topics that i am involved in. I really just thought it was just the 1 byte and had forgotten about the shift register.

I dunno, it seemed valid like statement to exit an ISR to me, but obviously it isn't. The compiler had no issues with it, and it works just fine like that, although.... Do you really think that has anything to do with the question ?