Correct use of interrupts

Hi, my name is Edoardo and thisis my first post in this forum. I bought 2 arduino + 2 ethernet shield , but i'm not so able to program it. My idea is start a function when pin2 rise and another one when it fall. I wrote the code but this interrupt doesn't seem to work properly. I put a led on pin 12 but when i close the switch on pin 2 nothing happens.

this is the part of code where i call the interrupts

   attachInterrupt(0, startinv, RISING);
   attachInterrupt(0, offinv, FALLING);

and this one the entire code. It is not complete but the part i tested should work in theory. It compiles normally

Thank you in advance for every reply!

#include <SD.h>

int temp1 = A0;    // temp1
int temp2 = A1;    // temp2
int temp3 = A2;    // temp3
int curr1 = A3;    // current1
int curr2 = A4;    // current2
int ledPin = 13;   // select the pin for the LED
int tmax = 0;      //max temperature between the three converted in 8 bit lenght
int valcurr1 = 0;  //reading of charging current
int valcurr2 = 0;  //reading of draining current
int invstat1;      //actual state of inverter
int fault1=0;      //fault indicator: 0 by default
int buttonState;             // the current reading from the input pin
int lastButtonState = LOW;
long lastDebounceTime = 0;  // the last time the output pin was toggled
long debounceDelay = 50;    // the debounce time; increase if the output flickers
const int chipSelect = 4;
const int partemp1 = 0;   //minimum temperature for fan start in 8 bit lenght
const int partemp2 = 0;   //direct law constant for pwm driving
const int buttonPin = 2;     // the number of the pushbutton pin


void maxtemp ();    //find out the highest temperature
void startinv (); //activation macro for inverter
void offinv ();
void statisticWrite ();  //statistics trought serial
void pwmset ();          //pwm driving for fan

void setup() 
{
  pinMode(buttonPin, INPUT);
  invstat1 = digitalRead(7);
  pinMode(ledPin, OUTPUT);  
  Serial.begin(9600);
  Serial.print("Initializing SD card...");
  pinMode(10, OUTPUT);
  if (!SD.begin(chipSelect)) {
    Serial.println("Card failed, or not present");
    return;
  }
  Serial.println("card initialized.");
  
   attachInterrupt(0, startinv, RISING);
   attachInterrupt(0, offinv, FALLING);
   attachInterrupt(1, statisticWrite, HIGH);
   pinMode(12, OUTPUT);
   pinMode(8, OUTPUT);
   pinMode(7, INPUT );
}

void loop() 
{
  int a,lastime=0;
  if (invstat1 == 1)
 { 
    maxtemp();
    if ( tmax < partemp1 )
    {
      
      pwmset();
 }
 interrupts();
 a = millis();
 if ( a >= (lastime+60000))
 {
 }
}
}

void maxtemp() 
{
  int a,b,c,i=0;
  tmax = 0;
  a = analogRead(temp1);
  b = analogRead(temp2);
  c = analogRead(temp3);
    if (a > tmax)
    {
     tmax = a;
    }
    if (b > tmax)
    {
     tmax = b;
    }
    if (c > tmax)
    {
     tmax = c;
    }
    
}

void startinv ()
{
  
  int a=0,i=0;
  // read the state of the switch into a local variable:
  int reading = digitalRead(buttonPin);

  // check to see if you just pressed the button 
  // (i.e. the input went from LOW to HIGH),  and you've waited 
  // long enough since the last press to ignore any noise:  

  // If the switch changed, due to noise or pressing:
  if (reading != lastButtonState) {
    // reset the debouncing timer
    lastDebounceTime = millis();
  } 
  
  if ((millis() - lastDebounceTime) > debounceDelay) {
    // whatever the reading is at, it's been there for longer
    // than the debounce delay, so take it as the actual current state:
    buttonState = reading;
  }

  // save the reading.  Next time through the loop,
  // it'll be the lastButtonState:
  lastButtonState = reading;

if ( reading == 1)
{
       while ( (a != 1)&&(i<4))
         {
         digitalWrite(12,HIGH);
         delay(100);
         digitalWrite(8,HIGH);
         delay (5000);
         a = digitalRead(7);
         i++;
         }
         if (  a == 1 )
         {
                invstat1 = 1;
         }
         if ( (a !=1)||(i>=4))
         {
                fault1 = 1;
         }
}
else
{
  return;
}
        
}

void offinv ()
{
  int a=0,i=0;
// read the state of the switch into a local variable:
  int reading = digitalRead(buttonPin);

  // check to see if you just pressed the button 
  // (i.e. the input went from LOW to HIGH),  and you've waited 
  // long enough since the last press to ignore any noise:  

  // If the switch changed, due to noise or pressing:
  if (reading != lastButtonState) {
    // reset the debouncing timer
    lastDebounceTime = millis();
  } 
  
  if ((millis() - lastDebounceTime) > debounceDelay) {
    // whatever the reading is at, it's been there for longer
    // than the debounce delay, so take it as the actual current state:
    buttonState = reading;
  }

  // save the reading.  Next time through the loop,
  // it'll be the lastButtonState:
  lastButtonState = reading;

if ( reading == 0)
{
  while ( (a != 1)&&(i<4))
         {
         digitalWrite(12,LOW);
         delay(500);
         digitalWrite(8,LOW);
         delay (1000);
         a = digitalRead(7);
         i++;
         }
         if (  a == 0 )
         {
                invstat1 = 0;
         }
         if ( (a !=0)||(i>=4))
         {
                fault1 = 2;
         }
  
}
}

void statisticWrite ()
{
    
}

void pwmset ()
{
  
}
   attachInterrupt(0, startinv, RISING);
   attachInterrupt(0, offinv, FALLING);

There is only ONE interrupt on each pin, that can be set to trigger on rising OR falling edges (or high/low.)
You don't get to give it a separate function for each edge.
(I didn't look for other things that might be wrong...)

thank you for help i will try soon :smiley:

Variables used in an interrupt must be declared volatile so the compiler will not optimize them ...

The problem with rising and falling can be handled as follows:

attachInterrupt(0, myirq, CHANGE);

void myirq()
{
  if (digitalRead(2) == LOW) offinv();
  else startinv()
}

You can speed up digitalRead(2) by direct port manipulation - Arduino Reference - Arduino Reference - to speed up the myirq code as the portnumber is fixed.

My idea is start a function when pin2 rise and another one when it fall. I wrote the code but this interrupt doesn't seem to work properly. I put a led on pin 12 but when i close the switch on pin 2 nothing happens.

You have software problems that have been pointed out. You have potential hardware problems with how the switch is wired that may be affecting why the interrupt is not being called. For an interrupt trigger, external pull-up or pull-down resistors are required.

You also have a very poor idea of what an interrupt handler is supposed to do.

       while ( (a != 1)&&(i<4))
         {
         digitalWrite(12,HIGH);
         delay(100);
         digitalWrite(8,HIGH);
         delay (5000);

Interrupt handlers are supposed to be fast. Even if delay() worked in a handler (it does not; it relies on interrupts which are disabled while your ISR is running), you should not be writing a handler that takes more than a few nanoseconds to complete. Serial printing, delays, while loops, etc. have no place in an ISR.

Set a flag, and get out. Make loop() do something when that flag is set.

You mean a few microseconds, not a few nanoseconds(!!)

thank you for help, main problem was the number of interrupts , and i also wrote a wrong (or better copied :D) debounce code. I choosed interrupts because i need sleep mode, not to speed up the code and I need a little delay because of filtering noise on button. Now my code works properly, i removed all debounce code and wrote new one simpler.

you should not be writing a handler that takes more than a few nanoseconds to complete.

The theory says that the duration of an interrupt must be smaller than the minimum time between interrupts otherwise an interrupt can be called before the previous is finished.
That gives all kind of problems which can be solved in some way but that is far from trivial IMHO. Furthermore there can be multiple different interrupt that need to be handled too, e.g. timer interrupts or other external interrupts. That is why IRQ's need to be as short as possible, so they don't get each other in the way..

Serial printing, delays, while loops, etc. have no place in an ISR.

Very true, should be printed in bold in the tutorial pages ..