Attaching interrupt in loop

I'm having an issue with my sketch and I think the problem lies in the cut down example shown below to illustrate it;
I want to disable then re-enable an interrupt later in "loop", but it doesn't work. The Interrupts doesn't seem to be re-enabled.
What am I doing something wrong/illegal/stupid in the way this code is structured ?

Its on a 328 processor running 8Mhz external clock btw

//example:


const int IRQ1 = 2; //int0
const int IRQ2 = 3;  //int1
//FLAGS:
volatile bool X1 = 0; 
volatile bool X2 = 0;

void setup() {
  pinMode(IRQ1, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(IRQ1), Input1, LOW);

  pinMode(IRQ2, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(IRQ2), Input2, LOW);
  X1=0;
  X2=0;
}

void loop()
{
  if ( (X1) && (X2) == 1)
  {
    //  do something clever then....
    attachInterrupt(digitalPinToInterrupt(IRQ2), Input2, LOW);
    attachInterrupt(digitalPinToInterrupt(IRQ1), Input1, LOW);
     X1=0;
   X2=0;
  }
//**********************************************************
  //interrupt routines:
  void Input1()
  {
    X1 = 1;
    detachInterrupt(digitalPinToInterrupt(IRQ1));
  }

  void Input2()
  {
    X2 = 1;
    detachInterrupt(digitalPinToInterrupt(IRQ2));
  }

Why ?

Why not just ignore it if it occurs ?

These local variables are not used anywhere. Perhaps you wanted to reset the global variables of the same names?

    X1=0;
    X2=0;

You forgot to protect your access to volatile variables:

//example:
const int IRQ1 = 2; //int0
const int IRQ2 = 3;  //int1

//FLAGS:
volatile bool X1 = false;
volatile bool X2 = false;

void setup()
{
  pinMode(IRQ1, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(IRQ1), Input1, LOW);

  pinMode(IRQ2, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(IRQ2), Input2, LOW);

  // Only access volatile variables with interrupts disabled
  noInterrupts();
  X1 = false;
  X2 = false;
  interrupts();
}

void loop()
{
  bool localX1, localX2;

  // Only access volatile variables with interrupts disabled
  noInterrupts();
  localX1 = X1;
  localX2 = X2;
  // If both are set, clear both.
  if (X1 && X2)
  {
    X1 = false;
    X2 = false;
  }
  interrupts();

  // If both were set
  if (localX1 && localX2)
  {
    //  do something clever then....
    attachInterrupt(digitalPinToInterrupt(IRQ2), Input2, LOW);
    attachInterrupt(digitalPinToInterrupt(IRQ1), Input1, LOW);
  }
}

//**********************************************************
//interrupt routines:
void Input1()
{
  X1 = true;
  detachInterrupt(digitalPinToInterrupt(IRQ1));
}

void Input2()
{
  X2 = true;
  detachInterrupt(digitalPinToInterrupt(IRQ2));
}

Right. Just have a volatile, bool, flag to indicate whether the interrupt needs processing or not. If it's false, the ISR can just return immediately. If true, do whatever processing is required.

no need to keep both copies since he only requires the combined state

Ok thx

  • couple of things to consider there and have a play with - got stuck zoned in and needed some ways out
    The code I listed is made up just to demonstrate and I mistakenly redeclared X1andX2 in loop - corrected that but now ( not that it matters to your suggestions so far )

side notes:

just for the beauty of the language, I suggest you use true and false with your booleans instead of 1 and 0


also, as a boolean is a truth value, there is no need to compare to true (1 in your case) and the extra parenthesis are not needed when you write this:

if ( (X1) && (X2) == 1) ...

we usually would write

if (X1 && X2) ...

of course great variable names would help with self documenting code

this would be easier to read for me

if (irq1FlagRaised && irq2FlagRaised) ...

1. To my knowledge, in Arduino Platform, LOW refers to 0V to 0.9V and HIGH refers to 3V to 5V. So, the declarations could be like:

volatile int X1 = LOW;//0; 
volatile int X2 = LOW;//0;

===>

X1 = HIGH;
X2 = HIGH;

2. If using bool data type, then in line with post#7 @J-M-L---

volatile bool X1 = false;//0; 
volatile bool X2 = false;//0;

===>

X1 = true;
X2 = true;

Thanks all now sorted- The main thing was that is it was ok to do what I was doing with the Interrupts (starting in loop) and I was able to look elsewhere for the problems.
Some good tips I've used to tidy things up ( such as using local copied values of X1, X2) which makes it more bullett proof.
The actual issue was elsewhere in my code ( I had to use negative logic in one part and got that wrong, causing the code to hang inside a particular loop) and masking what I thought was a disabled interrupt.

glad it's all solved :wink:

have fun

1 Like

So what state would a pin at a voltage of between 1.0V and 2.9V be in ?

Aaaah ... that would be lowish, not quite true or 0.5 lol

It is Forbidden zone (Fig-1) and the user/manufacturer must ensure that the circuit/chip does not operate in this zone.
forbiddenZone
Figure-1:

I wish utility to have been impregnated with beauty. :grinning: That true and false have added utility to the language?

Ohi/o.

I don't know where that image was copied from or who produced it but they don't seem to be able to spell the word Table correctly

It adds type safety.

2 Likes

Still looking for the original visio version; else, I have to re-make it.

VIL propagates through buffer-G1 and transmission path and probably picks up road noise and assumes VOL which must be below the forbidden zone to activate the next driven gate.

You have to cross the transition zone to get from one logic state to another, it's not so much forbidden as undefined. This is why most CPU logic is synchronous (clocked), and has setup and hold times.

In Fig-1 of post #13, the logic level is not switching from LOW-to-HIGH or HIGH-to-LOW. It is the same logic (here LOW) that is being propagated through a buffer gate (unity gain) and transmission path/wire. The figure intends to illustrate the meanings of VIL, VOL, VIH, and VOH.