Interrupt continues firing after being handled

robws · Aug 22, 2023

I am having an issue with correct interrupt behaviour on a Teensy 4.1. I have boiled it down to a small piece of code.

Basically I am attaching an interrupt to a RISING edge of a pin. Using a function generator I send a block of 10 pulses to the pin. I have confirmed with an oscilloscope that the pin sees 10 pulses. However, the interrupt routine is called 16 times!

Any ideas or help would be appreciated.

Here is a screen shot of the input signal, blue signal is the input pin on the teensy 4.1

Here is the code

Code:

#include <arduino.h>
#include <elapsedMillis.h>

#define ENCA  11  
#define ENCB  12
#define LED   13


bool lastEncA;
bool lastEncB;
long countC;
void setup() {
  // put your setup code here, to run once:
  Serial.begin(57600);
  Serial.printf("hello world\n\r");

  pinMode(ENCA, INPUT);
  pinMode(ENCB, INPUT);
  pinMode(LED,OUTPUT);

  attachInterrupt(digitalPinToInterrupt(ENCA), encoder_shaft_moved, RISING);


}

const int cntDir = 1;
volatile long rCount = 0;
volatile bool currDir = 0;
 
void loop() {
  static elapsedMillis timeSinceLastChange=0;
  static elapsedMillis timeSinceLastDisplay=0;  
  static elapsedMillis  ledTimer=0;
  static bool lastLed=0;
  static bool sentOne = false;
  static long lastDisplayCnt = 0;

  if (ledTimer > 300) {
    if (lastLed == 1) lastLed = 0;
    else lastLed = 1;
    digitalWrite(LED,lastLed);
    ledTimer = 0;
  }

  if (rCount != lastDisplayCnt)
    timeSinceLastChange = 0;

  if ((((long)timeSinceLastChange) > 10) || (((long)timeSinceLastDisplay) > 2000)) {
    if ((lastDisplayCnt != rCount) || (((long)timeSinceLastDisplay) > 2000)) {
      if ((sentOne== false) || (((long)timeSinceLastDisplay) > 2000)) 
         Serial.printf("%lu: %ld %s\n\r",millis(), rCount,currDir == 1 ?"In":"out"); 
         sentOne = true;
         timeSinceLastDisplay = 0;
         lastDisplayCnt = rCount;
      }
      else sentOne = false;
  } 
}


void encoder_shaft_moved() {
  elapsedMillis timeSinceLastChange=0;
    bool newA, newB;
  // put your main code here, to run repeatedly:
  newA = digitalRead(ENCA);
  newB = digitalRead(ENCB);

  rCount++;

  return;
}

joepasquariello · Aug 22, 2023

Not sure about the 16 versus 10, but are you missing brackets after the statement with "if (SentOne == false)"? Perhaps that has an effect.

When you post code, please use the '#' button in the forum UI, else it's very hard to read.

Code:

#include <arduino.h>
#include <elapsedMillis.h>

#define ENCA 11
#define ENCB 12
#define LED 13


bool lastEncA;
bool lastEncB;
long countC;

void setup() {
  // put your setup code here, to run once:
  Serial.begin(57600);
  Serial.printf("hello world\n\r");

  pinMode(ENCA, INPUT);
  pinMode(ENCB, INPUT);
  pinMode(LED,OUTPUT);

  attachInterrupt(digitalPinToInterrupt(ENCA), encoder_shaft_moved, RISING);
}

const int cntDir = 1;
volatile long rCount = 0;
volatile bool currDir = 0;

void loop() {
  static elapsedMillis timeSinceLastChange=0;
  static elapsedMillis timeSinceLastDisplay=0;
  static elapsedMillis ledTimer=0;
  static bool lastLed=0;
  static bool sentOne = false;
  static long lastDisplayCnt = 0;

  if (ledTimer > 300) {
    if (lastLed == 1) lastLed = 0;
    else lastLed = 1;
    digitalWrite(LED,lastLed);
    ledTimer = 0;
  }

  if (rCount != lastDisplayCnt)
    timeSinceLastChange = 0;

  if ((((long)timeSinceLastChange) > 10) || (((long)timeSinceLastDisplay) > 2000)) {
    if ((lastDisplayCnt != rCount) || (((long)timeSinceLastDisplay) > 2000)) {
      if ((sentOne== false) || (((long)timeSinceLastDisplay) > 2000))
        Serial.printf("%lu: %ld %s\n\r",millis(), rCount,currDir == 1 ?"In":"out");
      sentOne = true;
      timeSinceLastDisplay = 0;
      lastDisplayCnt = rCount;
    }
    else sentOne = false;
  }
}

void encoder_shaft_moved() {
  elapsedMillis timeSinceLastChange=0;
  bool newA, newB;
  // put your main code here, to run repeatedly:
  newA = digitalRead(ENCA);
  newB = digitalRead(ENCB);

  rCount++;
  return;
}

defragster · Aug 22, 2023

For short _ISR routines they can complete before the interrupt is marked as serviced.

Try with this added "DSB":

Code:

void encoder_shaft_moved() {
  elapsedMillis timeSinceLastChange=0;
    bool newA, newB;
  // put your main code here, to run repeatedly:
  newA = digitalRead(ENCA);
  newB = digitalRead(ENCB);

  rCount++;
  [B]asm volatile ("dsb":::"memory");[/B]
  return;
}

Note these two "timeSinceLastChange" are not the same variable - both are local to the function. Are they meant to be the same? If so, a single global definition is needed.

Code:

void loop() {
  static elapsedMillis timeSinceLastChange=0;

and

Code:

void encoder_shaft_moved() {
  elapsedMillis timeSinceLastChange=0;

BriComp · Aug 22, 2023

Would you care to describe the various parameters (their effect) in asm volatile ("dsb":::"memory");.

defragster · Aug 22, 2023

BriComp said:
Would you care to describe the various parameters (their effect) in asm volatile ("dsb":::"memory");.

If I give a link can I say no?

stackoverflow.com/questions/15491751/real-life-use-cases-of-barriers-dsb-dmb-isb-in-arm

Code:

If you are mainly a device driver developer, then examples are fairly straightforward to find - 
whenever there is a [B][U]dependency in your code on a previous access having had an effect (cleared an interrupt source[/U][/B],
 written a DMA descriptor) before some other access is performed (re-enabling interrupts, initiating the DMA transaction).

The IO half of the 1062 AFAIK doesn't run at same 600 MHz processor half. This can result in the interrupt service completing before the bit saying "service interrupt" is cleared and the _ISR gets called again. The DSB says code has to wait until the memory indicating 'call _ISR' is cleared.

manitou · Aug 22, 2023

I don't think attachInterrupt ISR requires asm volatile ("dsb");. Teensy 4 core interrupt.c spins testing the ISR clear flag.

I think the missing brackets { } (post #2) is the problem.

Also, the declaration of timeSinceLastChange in encoder_shaft_moved() won't be seen outside that function. Probably should be declared externally and with volatile ... needs work

defragster · Aug 22, 2023

manitou said:
I don't think attachInterrupt ISR requires asm volatile ("dsb");. Teensy 4 core interrupt.c spins testing the ISR clear flag.

I think the missing brackets { } (post #2) is the problem.

Also, the declaration of timeSinceLastChange in encoder_shaft_moved() won't be seen outside that function. Probably should be declared externally and with volatile ... needs work

p#3 "DSB" was just a default guess for double/extra triggering - not seeing the missing brackets.

The two LOCAL function copies of timeSinceLastChange was noted in p#3 - also now seeing local copies of " bool newA, newB;" would be unusable if they were needed in code outside the _ISR. And they would need to be declared 'volatile' if referenced outside.

MarkT · Aug 23, 2023

I dot't think you don't need a DSB barrier for communicating between ISR and main program since they are both running on the processor using the same cache as the T4 is a single processor architecture.

The barrier is needed if communicating with a DMA channel though, since DMA doesn't use any cache and goes straight to DMA memory direct. DMA channels are effectively different processors.

You do need volatile for an ISR's statically declared variables.

jmarsh · Aug 23, 2023

MarkT said:
I dot't think you don't need a DSB barrier for communicating between ISR and main program since they are both running on the processor using the same cache as the T4 is a single processor architecture.

You do. It's not about cache usage, it's to ensure any instructions that clear hardware interrupt registers have actually completed (and clear the pending interrupt) before the CPU exits the interrupt routine. It's even listed in the known errata.

defragster · Aug 23, 2023

jmarsh said:
You do. It's not about cache usage, it's to ensure any instructions that clear hardware interrupt registers have actually completed (and clear the pending interrupt) before the CPU exits the interrupt routine. It's even listed in the known errata.

This discovery goes back to (June) 2019 Teensy-4-0-First-Beta-Test.

Various simple count/test _ISR's were double firing.

jmarsh · Aug 23, 2023

I thought the multi-gpio ISR handler in teensyduino (it's called gpio6789 or similar) had a dsb instruction at the end of it to handle this, am I imagining that?

MarkT · Aug 23, 2023

defragster said:
This discovery goes back to (June) 2019 Teensy-4-0-First-Beta-Test.

Various simple count/test _ISR's were double firing.

Ah, and looking at some PWM ISR I wrote a couple of years back I see I used it myself - easy to forget something like this when you switch between processors.

defragster · Aug 23, 2023

jmarsh said:
I thought the multi-gpio ISR handler in teensyduino (it's called gpio6789 or similar) had a dsb instruction at the end of it to handle this, am I imagining that?

Not seeing that if this is the right spot : github.com/PaulStoffregen/cores/blob/master/teensy4/interrupt.c#L31

Code:

FASTRUN static inline __attribute__((always_inline))
inline void irq_anyport(volatile uint32_t *gpio, voidFuncPtr *table)
{
	uint32_t status = gpio[ISR_INDEX] & gpio[IMR_INDEX];
	if (status) {
		gpio[ISR_INDEX] = status;
		while (status) {
			uint32_t index = __builtin_ctz(status);
			table[index]();
			status = status & ~(1 << index);
			//status = status & (status - 1);
		}
	}
}

FASTRUN
void irq_gpio6789(void)
{
	irq_anyport(&GPIO6_DR, isr_table_gpio1);
	irq_anyport(&GPIO7_DR, isr_table_gpio2);
	irq_anyport(&GPIO8_DR, isr_table_gpio3);
	irq_anyport(&GPIO9_DR, isr_table_gpio4);
}

jmarsh · Aug 24, 2023

Right, dsb should be at the end of irq_gpio6789() - I thought I'd checked before and it was, but I guess I imagined that.

(It's more efficient to have it in one place rather than relying on every custom gpio handler to have it...)

Interrupt continues firing after being handled

robws

Member

joepasquariello

Well-known member

defragster

Senior Member+

BriComp

Well-known member

defragster

Senior Member+

manitou

Senior Member+

defragster

Senior Member+

MarkT

Well-known member

jmarsh

Well-known member

defragster

Senior Member+

jmarsh

Well-known member

MarkT

Well-known member

defragster

Senior Member+

jmarsh

Well-known member