External interrupt executes too many times and blocks Internal interrupts

Projects Programming
1

Hello everybody,

I am trying to synchronize two Leds controlled by two Arduino Unos. They both blink at the same frequency but have a phase shift which should get compensated over time throughout reacting to each others blinking.

I do this via external interrupts that trigger on a Low Signal on Pin 2. The respective ISR called 'raisePhase' increments a variable called 'phaseValue'. As soon as phaseValue reaches a critical level (I defined it as 5.0) the LED blinks and promptly sends a LOW Signal to the other Arduinos Pin 2 thus incrementing his phaseValue and so on. I am also constantly incrementing phaseValue via a Timer interrupt using the TimerOne library to make sure every LED is able to blink at all.

#include "TimerOne.h"                               

const float e = 2.71828;
const unsigned long t_increase = 15000;              
const int t_wait = 300;                             
const float crit_phaseValue = 5.0;                 
const float epsilon = 0.25;                         
const int tolerance = 2;
const float slope = 1.5;                            
volatile float phaseValue = 0.0;
volatile int potiValue_new = 0;
volatile int potiValue_old = 0;
volatile float timeScale = 0.0;



void sendPulse() {
  digitalWrite(3, LOW);
  delayMicroseconds(1);                           
  digitalWrite(3, HIGH);
}


void increase_phaseValue() {
  phaseValue = (1 / slope) * log(1 + (pow(e, slope) - 1) * timeScale);
  timeScale = timeScale + 0.5;
  Serial.println(phaseValue);
}


void raisePhase()
{
  phaseValue = phaseValue + epsilon;
  Serial.println("phase raised");
  //delay(t_wait);                    
}

void phaseShift() {
  potiValue_new = analogRead(A0);
  if ((potiValue_new >= (potiValue_old + tolerance)) || (potiValue_new <= (potiValue_old - tolerance))) {
    phaseValue = phaseValue + map(potiValue_new, 0, 1023, 0, 5);
    Serial.println("phaseValue manipulated");
  }
  potiValue_old = potiValue_new;
}

void setup()

{
  Serial.begin(9600);
  pinMode(A0, INPUT);
  pinMode(3, OUTPUT);
  pinMode(8, OUTPUT);
  pinMode(2, INPUT_PULLUP);                       

  Timer1.initialize(t_increase);                 
  Timer1.attachInterrupt(increase_phaseValue);    

  attachInterrupt(0, raisePhase, LOW);           

  digitalWrite(8, LOW);

  potiValue_old = analogRead(A0);
}



void loop()
{
  //Serial.println(potiValue_new);
  phaseShift();
  if (phaseValue >  crit_phaseValue) {
    digitalWrite(8, HIGH);
    sendPulse();
    delay(t_wait);
    digitalWrite(8, LOW);
    phaseValue = 0.0;
    timeScale = 0.0;
  }
}

The project is similar to this one: http://www.instructables.com/id/Synchronizing-Fireflies/?ALLSTEPS

My problem is that as soon as my code enters the ISR 'raisePhase' it gets stuck there and executes it for about 70 times before going back to normal (I am only setting Pin 2 on LOW for 1 microsecond). My Timer triggered ISR seems not to work anymore whereas void loop() mus somehow still be working since it resets phaseValue to zero and makes the LED blink.

I attached three Screenshots of my serial monitor leaving out the ~70 'phase raised' for better reading. Note that phaseValue gets reset at a value of 3.76 instead of 5.0

I am aware that external interrupts are stronger than timer interrupts which again have priority over the loop function. Anyway I don't understand why the external interrupt ISR gets executed so many times and how I can avoid this. For my algorithm to work I should apparently also include a delaytime in which one Arduino is not listening to the other right after blinking. Is there a good way to do this without using delay within my ISRs?

I am very grateful for any ideas on how to fix this.

Screenshot SerialMonitor.PNG

Screenshot2.PNG

Screenshot3.PNG

2

(1) This code would not compile:

void raisePhase()
{
  phaseValue = phaseValue + epsilon;
  Serial.println("phase raised");

void increase_phaseValue() {
  phaseValue = (1 / slope) * log(1 + (pow(e, slope) - 1) * timeScale); // This is basically modelling an RC Circuit Voltage Curve. TimeScale only represents a fixed step since the actual time does not matter and I don't want to work with millis()
  timeScale=timeScale+0.5;
  Serial.println(phaseValue);

as it appears to define a function within another function. It appears to be missing two curly brackets.

(2) Serial.print(...) and Serial.println(...) should not be used inside an ISR.

(3) If you post your code piecemeal, you are likely to get pieces of an answer. Context is important but is lost when you post this way.

To post code and/or error messages:

  1. Use CTRL-T in the Arduino IDE to autoformat your complete code.
  2. Paste the complete autoformatted code between code tags (the </> button)
    so that we can easily see and deal with your code.
  3. Paste the complete error message between code tags (the </> button)
    so that we can easily see and deal with your messages.
  4. If you already posted without code tags, you may add the code tags by
    editing your post. Do not change your existing posts in any other way.
    You may make additional posts as needed.

Before posting again, you should read the three locked topics at the top of the Programming Questions forum, and any links to which these posts point.

If your project involves wiring, please provide a schematic and/or a wiring diagram and/or a clear photograph of the wiring.

Good Luck!

3

Thanks for the answer so far. I edited my code example.
I did read the forum instructions before posting but I apparently misunderstood this point here:
'With coding problems, if possible post a "minimal" sketch that demonstrates the problem - not hundreds of lines of code.'

vaj4088:
(1) This code would not compile:

void raisePhase()

{
  phaseValue = phaseValue + epsilon;
  Serial.println("phase raised");

void increase_phaseValue() {
  phaseValue = (1 / slope) * log(1 + (pow(e, slope) - 1) * timeScale); // This is basically modelling an RC Circuit Voltage Curve. TimeScale only represents a fixed step since the actual time does not matter and I don't want to work with millis()
  timeScale=timeScale+0.5;
  Serial.println(phaseValue);




as it appears to define a function within another function. It appears to be missing two curly brackets.

That's right I messed it up when copying my code.

If I can't use Serial.println within ISRs, is there a good way to find out whether they have just run or not?

4

What happens when you trigger the interrupt FALLING instead of LOW?

If I can't use Serial.println within ISRs, is there a good way to find out whether they have just run or not?

Set a flag or a counter. Check for it in loop. You can also light up an led.

5

Using LOW for attachInterrupt means the ISR is called repeatedly while the pin is LOW.

This is (almost) never the right behaviour = use RISING, FALLING or CHANGE which self-cancel.

6

Bionic6:
Thanks for the answer so far. I edited my code example.
I did read the forum instructions before posting but I apparently misunderstood this point here:
'With coding problems, if possible post a "minimal" sketch that demonstrates the problem - not hundreds of lines of code.'

You should read that as meaning a minimal complete compilable sketch

If I can't use Serial.println within ISRs, is there a good way to find out whether they have just run or not?

In you ISR just use a variable as a flag - something like

interruptTriggered = true;

and then the regular code can watch out for that and set the variable back to false when it has noticed the ISR.

If you really are getting too many interrupts it is perfectly normal to use the first interrupt to switch off interrupts on that I/O pin. Then some other code can switch them on again when appropriate.

...R

7

Thanks for the good tips! I used FALLING now which worked just fine. My interrupts now use flags instead of Serial.println as well. Sometimes I just can't think of the simple way to do it :slight_smile: