Problems using interrupts

wbanz:
I detach them in the ISR so that after I leave the ISR they are detached until I am ready to proceed more input from that sensor.

Doesn't work that way. The hardware will enable the interrupts on exit (I feel like I'm repeating myself). Read the datasheet.

When an interrupt occurs, the Global Interrupt Enable I-bit is cleared and all interrupts are disabled. The
user software can write logic one to the I-bit to enable nested interrupts. All enabled interrupts can then
interrupt the current interrupt routine. The I-bit is automatically set when a Return from Interrupt
instruction – RETI – is executed.

11.7 in the datasheet.

DKWatson:
All enabled interrupts can then interrupt the current interrupt routine.

Which enabled interrupt can interrupt the current on-going interrupt? The interrupt that has higher priority than the current on-going interrupt. For example: ISRINT0 is an on-going ISR. Interrupt request due to INT1 will not interrupt the ISRINT0 process as the INT0 has the higher priority than INT1 interrupt. The MCU will finish the ISRINT0 routine, and then it will begin the ISRINT1 process. Interrupt with lower vector value has the higher priority.

GolamMostafa:
Which enabled interrupt can interrupt the current on-going interrupt? The interrupt that has higher priority than the current on-going interrupt. For example: ISRINT0 is an on-going ISR. Interrupt request due to INT1 will not interrupt the ISRINT0 process as the INT0 has the higher priority than INT1 interrupt. The MCU will finish the ISRINT0 routine, and then it will begin the ISRINT1 process.

Priority is only used when the processor has to decide which interrupt to handle. Once an ISR is in progress, the specific interrupt flag is cleared and the MCU has no idea which one it is handling and hence you can interrupt even the highest priority interrupt with a low priority interrupt.

This obviously requires enabling interrupts again as DKWatson indicated.

sterretje:
Priority is only used when the processor has to decide which interrupt to handle. Once an ISR is in progress, the specific interrupt flag is cleared and the MCU has no idea which one it is handling and hence you can interrupt even the highest priority interrupt with a low priority interrupt.

This obviously requires enabling interrupts again as DKWatson indicated.

No, the specific interrupt flag ( Local interrupt enable bit~~)~~ is not cleared. It remains enabled. The global interrupt enable bit (I-bit of SREG Register) is cleared to prevent the MCU from responding to all kinds of further interrupts. If the user has an impending interrupt of higher priority than the on-going ISR to which the MCU should respond, then he must activate the global interrupt enable bit (putting LH at the I-bit) by executing interrupts() instruction just after arriving at the ISR. Edit: In view of Post#15.

The MCU has interrupt priority logic inside its interrupt module to resolve the priority issue based on the following vector table. Interrupt with lower vector value has the higher priority. This is a hard-cored rule like the 8086 which when receives an interrupt vector, the vector is multiplied by 4 to get the beginning address of the interrupt vector table which holds the 20-bit (SEG:OFF format) physical address of the corresponding ISR.

I'm not talking about the interrupt enable bit; for each interrupt there is a bit in a register that indicates that the interrupt occurred; for the external interrupts, look at the EIFR register description

Bit 1 – INTF1: External Interrupt Flag 1
When an edge or logic change on the INT1 pin triggers an interrupt request, INTF1 becomes set (one). If the Ibit
in SREG and the INT1 bit in EIMSK are set (one), the MCU will jump to the corresponding Interrupt Vector.
The flag is cleared when the interrupt routine is executed. Alternatively, the flag can be cleared by writing a
logical one to it. This flag is always cleared when INT1 is configured as a level interrupt.

I've added the emphasis.

Further

When an interrupt occurs, the Global Interrupt Enable I-bit is cleared and all interrupts are disabled. The user software can write logic one to the I-bit to enable nested interrupts. All enabled interrupts can then interrupt the current interrupt routine. The I-bit is automatically set when a Return from Interrupt instruction – RETI – is executed.

Emphasis added.

Both quotes are taken from ATmega48A/PA/88A/PA/168A/PA/328/P [DATASHEET], Atmel-8271J-AVR- ATmega-Datasheet_11/2015.

Yes! The interrupt flag is automatically cleared when the MCU vectors at the corresponding ISR. This is the flag that actually triggers the interrupt logic; therefore, it has to be cleared so that the same interrupt source can again trigger the interrupt logic once the MCU returns to the Main Line Program (MLP). I have edited my Post#14 based on your information of Post#15.

OP: Your technique of constantly attaching / detaching interrupts is too much work. And, it’s inelegant / ugly. Much better to just ignore the extra interrupts. Something like this (untested, but it compiles):

enum states {waitingInt1 = 0, waitingInt2, resultReady, resetDelay};
volatile uint8_t currentState = waitingInt1;

const uint8_t vibPin1 = 2;
const uint8_t vibPin2 = 3;
const uint8_t ledPin = 13;

volatile uint32_t int1Time;
volatile uint32_t int2Time;
volatile uint32_t stateTimer;
uint32_t timeInterval;
const uint32_t maxWaitTime = 1 * 1000UL;
const uint32_t resetTime = 5 * 1000UL;


void setup() {
  Serial.begin(115200);
  pinMode(vibPin1, INPUT);
  pinMode(vibPin2, INPUT);
  pinMode(ledPin, OUTPUT);
  attachInterrupt(digitalPinToInterrupt(vibPin1), pin1_ISR, RISING);  // Just use one edge from sensor. RISING or FALLING, not CHANGE
  attachInterrupt(digitalPinToInterrupt(vibPin2), pin2_ISR, RISING);  // Just use one edge from sensor. RISING or FALLING, not CHANGE
}

void loop() {
  uint32_t localInt1Time, localInt2Time, interval;

  uint32_t currentMillis = millis();
  noInterrupts();
  switch (currentState) {
    case resultReady:
      currentState = resetDelay;
      localInt1Time = int1Time;
      localInt2Time = int2Time;
      stateTimer = millis();
      interrupts();
      interval = localInt2Time - localInt1Time;
      Serial.print("Interrupt 1 Time = ");
      Serial.println(localInt1Time);
      Serial.print(", Interrupt 2 Time = ");
      Serial.println(localInt2Time);
      Serial.print(", Interval Time = ");
      Serial.println(interval);
      Serial.println();
      break;

    case waitingInt1:
      interrupts();
      break;

    case waitingInt2:
      // See if we've waited too long for second interrupt
      if ((currentMillis - stateTimer) >= maxWaitTime) {
        currentState = waitingInt1;
      }
      interrupts();
      break;

    case resetDelay:
      // See if reset delay has expired
      if ((currentMillis - stateTimer) >= resetTime) {
        currentState = waitingInt1;
      }
      interrupts();
      break;

    default:
      interrupts();
      break;
  }
}

void pin1_ISR() {
  if (currentState == waitingInt1) {
    int1Time = micros();
    stateTimer = millis();
    currentState = waitingInt2;
  }
}

void pin2_ISR() {
  if (currentState == waitingInt2) {
    int2Time = micros();
    currentState = resultReady;
  }
}

@gfvalvo (Post#17)

Is it essential to execute the following two pinMode() instructions when the attachInterrupt() automatically configures the DPin-2 and DPin-3 as interrupt lines?

pinMode(vibPin1, INPUT);
pinMode(vibPin2, INPUT);

GolamMostafa:
Is it essential to execute the following two pinMode() instructions when the attachInterrupt() automatically configures the DPin-2 and DPin-3 as interrupt lines?

pinMode(vibPin1, INPUT);

pinMode(vibPin2, INPUT);

Perhaps not. But, it doesn't hurt anything and is only executed ONCE in setup(). So, why would you care?

Plus, it makes what's going on explicitly clear.

gfvalvo:
So, why would you care?

A poster takes time for his readers, and he expects feedback?

By default all I/O pins are configured as INPUT (on AVR processors) and all global variables are initialized to zero/NULL. This may not be the case on other platforms with other compilers. As @gfvalvo rightly pointed out, it never hurts and is considered good programming style to initialize everything.

DKWatson:
As @gfvalvo rightly pointed out, it never hurts and is considered good programming style to initialize everything.

Doing so, I am afraid that we are undermining the tremendous efforts that the Arduino Team are employing to facilitate our Arduino Platform based programming jobs?

Of course, the point is not at all germane to the OP's question.

gfvalvo:
OP: Your technique of constantly attaching / detaching interrupts is too much work. And, it’s inelegant / ugly. Much better to just ignore the extra interrupts. Something like this (untested, but it compiles):

Many thanks for taking the time to provide a detailed suggestion to solve why problem. It is appreciated.
One of the problems I have is that I do not know which interrupt is going to occur first and using interrupt/nointerrupt it did not work as I had expected so I switched to turning off the specific interrupt which occurred first etc.
However, even using no interrupt I had the same problem, and that is that if other interrupts are received, one of them is saved when interrupts are turned off, and it is processed when interrupts are turned back on.

My problem is purging that saved interrupt - I need it to go away, which is what I thought EIFR = bit (INTF0
EIFR = bit (INTF1); should do, but it does not.

Again, you don't need to "purge" the extra interrupts, just IGNORE them. The code I posted does just that. Once a given interrupt occurs the ISR just ignores subsequent firings of the same interrupt until the two-interrupt sequence and reset delay is complete.

My code works under the assumption that vibPin1 always occurs first. Guess maybe I didn't read your original post carefully enough. Anyway the state machine technique I posted can easily be adapted to work when EITHER interrupt occurs first.

gfvalvo:
Again, you don't need to "purge" the extra interrupts, just IGNORE them. The code I posted does just that. Once a given interrupt occurs the ISR just ignores subsequent firings of the same interrupt until the two-interrupt sequence and reset delay is complete.

OK, I misunderstood what you meant by ignore but understand how you have implemented that.
Many thanks. This has been really helpful.

An alternative is to clear all interrupt flags before exiting the ISR as these will have been set if an interrupt occurred while global interrupts was disabled. This is how the processor knows there's a pending interrupt.

DKWatson:
An alternative is to clear all interrupt flags before exiting the ISR as these will have been set if an interrupt occurred while global interrupts was disabled. This is how the processor knows there's a pending interrupt.

I Guess that was my initial question - how do you clear all interrupt flags?

You have a copy of the datasheet?