Problems using interrupts

I am using two vibration sensors to trigger interrupts. Both sensors will react when a shock is received on a board but one will trigger before the other. The objective is to calculate the time difference between the sensors triggering and determine which sensor triggered first.
I am using an Arduino UNO and interrupts 0 and 1.
Because the vibrators may (will) result in more than one pulse per shock, after receiving the first pulse I disconnect the interrupt. Each interrupt captures micros() and after both interrupts have been processed I calculate the difference with a plus or minus result depending on which interrupt was processed first. Before re-attaching the interrupts I use code I borrowed from Nick Gammons interrupts tutorial.
My problem is that I still get extra interrupts triggering after the interrupt processors are re-attached.
Any advice will be gratefully received.

int vibPin1 = 2;
int vibPin2 = 3;
int ledPin = 13;
int CR = 13;
volatile long timeone = 0;
volatile long timetwo = 0;
volatile long timedelay = 0;
volatile int(sequence);

void setup() {
  Serial.begin(9699);
  pinMode(vibPin1,INPUT);
  pinMode(vibPin2,INPUT);
  pinMode(ledPin,OUTPUT);
  attachInterrupt(0, pin1_ISR, CHANGE);
  attachInterrupt(1, pin2_ISR, CHANGE);

}

void loop() {
  if (timeone * timetwo != 0) {
    digitalWrite(ledPin, HIGH);
    timedelay = timeone - timetwo;
    Serial.print(sequence); Serial.print("\n");
    Serial.print(timeone);Serial.print(" ");  Serial.print(timetwo);Serial.print(" ");Serial.print(timedelay);Serial.print(" delay\n");    
    timetwo = 0;
    timeone = 0;
    delay(5000);
    digitalWrite(ledPin, LOW);
    sequence = 0;
    EIFR = bit (INTF0);
    EIFR = bit (INTF1);
    attachInterrupt(0, pin1_ISR, CHANGE);
    attachInterrupt(1, pin2_ISR, CHANGE);

  }
}
void pin1_ISR(){
  detachInterrupt(0);
  if (timeone == 0)timeone = micros();
  sequence = 1;

}

void pin2_ISR(){
    detachInterrupt(1);
  if (timetwo == 0)timetwo = micros();
  sequence = 2;

}

vibratorInterupt2.ino (1.08 KB)

Your code is short enough to post in-line using code tags. It's all described in Item #6 here: Read this before posting a programming question ...

You did read that, didn't you?

Why go to all that bother? After you post your code, why not use a single character variable cleared to NULL. The first interrupt to trigger checks this variable and if NULL, knows that it is first so sets the variable to say A. After that, you can have as many interrupts as you want, but the first was still A. When you're ready, reset the variable to NULL and start all over again. Unless of course you actually need the time difference between the two in which case you use another pair to hold the timestamps.

Are you saying that you are getting vibrations detected even five seconds after the initial shock? Maybe you have to wait longer for these "vibration detectors" to settle down. Have you tried using an oscilloscope to see what kind of signal you are getting from your sensors?

It is recommended that you use the "digitalPinToInterrupt()" macro to get the interrupt number for a pin. That will take care of the case of switching between an UNO and a Leonardo where the interrupts on Pin 2 and Pin 3 are swapped: UNO Pin 2 -> Int 0, Pin 3 -> Int 1 Leonardo Pin 2 -> Int 2, Pin 3 -> Int 0

You also need to provide a link to the sensors you are using!

and a diagram to the way they are wired.

Mark

gfvalvo: Your code is short enough to post in-line using code tags. It's all described in Item #6 here: Read this before posting a programming question ...

You did read that, didn't you?

Yes, I did read that. I just chose the option to attach rather than include. I have gone back and included the code.

johnwasser: Are you saying that you are getting vibrations detected even five seconds after the initial shock? Maybe you have to wait longer for these "vibration detectors" to settle down. Have you tried using an oscilloscope to see what kind of signal you are getting from your sensors?

No, the five seconds delay is purely there to allow the physical environment to settle. Yes, I have scoped it and the 5 seconds is more than ample. The issue is that because I am using a vibration sensor I am getting multiple pulses. The code responds to the first pulse from a sensor then detaches that interrupt, then waits for a pulse from the second sensor and detaches that interrupt. However it seems that if a second pulse (or possibly more) from the vibrator sensor is detected before the interrupt is detached, then one of these interrupts is stacked. When the interrupt is attached again, even after 5 seconds, this stacked interrupt is processed. I understand all of this to be expected. So, to prevent the stacked interrupt from being actioned, I use Nick Gammons suggested code EIFR = bit (INTF0); EIFR = bit (INTF1); to reset the stored interrupts. However, this does not seem to work.

It is recommended that you use the "digitalPinToInterrupt()" macro to get the interrupt number for a pin. That will take care of the case of switching between an UNO and a Leonardo where the interrupts on Pin 2 and Pin 3 are swapped: UNO Pin 2 -> Int 0, Pin 3 -> Int 1 Leonardo Pin 2 -> Int 2, Pin 3 -> Int 0

Valid point, this point is noted for final code, thanks.

A) I don't see anywhere that you have engaged or disabled interrupts.

B) I don't see any interrupts being called.

C) I don't see any interrupt service routines.

D) If you are timing, and supposedly using micros, in each of your startTimer functions you have a print statement that, at 9600 baud, will take at least 14ms to complete. Not a very good delay in a timing loop.

Am I looking at the correct code?

DKWatson: A) I don't see anywhere that you have engaged or disabled interrupts.

B) I don't see any interrupts being called.

C) I don't see any interrupt service routines.

D) If you are timing, and supposedly using micros, in each of your startTimer functions you have a print statement that, at 9600 baud, will take at least 14ms to complete. Not a very good delay in a timing loop.

Am I looking at the correct code?

No, sorry, not sure how I did it but i uploaded the wrong code. I have corrected that.

FYI there is no need to detach interrupts when you enter ISR as they are disabled automatically by the hardware, enabled on exit.

DKWatson: FYI there is no need to detach interrupts when you enter ISR as they are disabled automatically by the hardware, enabled on exit.

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

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.