Why dont my interrupts work :(

I cant get my interrupt to work, and I dont know why…
The stop button works if i dont start the else in the loop where the digitalwrites happens…

#include <LCD16x2.h>
#include <Wire.h>

LCD16x2 lcd;
int buttons;

boolean start = true;                  //Code for the stopbutton
volatile boolean e_stop = false;
boolean BUTTONSTATE;

volatile unsigned long isrCounter;  //Code for the AMOUNT counter
unsigned long pulseCount;
unsigned long stopCount = 1400;

char project[] = {"Flaskfyllaren"};
const char volym[] = {"Volym"};
const char unit[] = {"ml"};
const char stop[] = {"STOPPAD"};
const char stop1[] = {"Starta om fyllaren"};

const int OUTPIN1 =  5;      // Gas inlet
const int OUTPIN2 = 4;       // Gas outlet
const int OUTPIN3 = 6;       // Liquid inlet
float AMOUNT;

int BUTTON = 0;						//code for peumatic
boolean off=true;
const int OUTPIN4 = 7;





void setup() {
Serial.begin(9600);
  Wire.begin();
    delay(100);
      attachInterrupt(1, e_stop_ISR, RISING);
      attachInterrupt(0, countP, RISING);
 
     lcd.lcdClear(); 
     lcd.lcdSetBlacklight(255);
     lcd.lcdGoToXY(2,1);
     lcd.lcdWrite(project);
  
  // initialize Outpins as output:
  pinMode(OUTPIN1, OUTPUT);
  pinMode(OUTPIN2, OUTPUT);
  pinMode(OUTPIN3, OUTPUT);
 
}

void loop(){
if(start == true){
    
if(e_stop == false){
buttons = lcd.readButtons();
 
 
 
if(buttons & 0x08) {
if(off==true)
BUTTON=LOW;
  digitalWrite(OUTPIN4, BUTTON);
  }
   else {
   if(off==false) {
   off=true;
   BUTTON=LOW;}
   else{
   off=false;
   BUTTON=HIGH;
   digitalWrite(OUTPIN4, BUTTON);
   }
 }




 if(buttons & 0x01){
      //Sets all outpins as LOW
      digitalWrite(OUTPIN1, LOW);
      digitalWrite(OUTPIN2, LOW);
      digitalWrite(OUTPIN3, LOW);
} 
  
else {
    
      digitalWrite(OUTPIN1, HIGH); //Opens gas inlet
    delay(2000);
    digitalWrite(OUTPIN1, LOW);  //Closes gas inlet
     delay(1000);
   
    digitalWrite (OUTPIN2, HIGH); //Opens gas outlet
    delay(2000);
    digitalWrite (OUTPIN2, LOW);  //Closes gas outlet
     delay(1000);
   
    digitalWrite(OUTPIN1, HIGH);  //Opens gas inlet
    delay(2000);
    digitalWrite(OUTPIN1, LOW);  //Closes gas outlet
     delay(1000);
    
   digitalWrite(OUTPIN3, HIGH);  //Opens liquid inlet.
   
 //   Serial.print("Volym :");
 //   Serial.print(AMOUNT);
 //   Serial.println("ml");

lcd.lcdClear();
     lcd.lcdGoToXY(2,1);
     //lcd.lcdWrite(volym);
     lcd.lcdWrite((char *) volym);
     lcd.lcdGoToXY(9,1);
     lcd.lcdWrite((float)AMOUNT);
     lcd.lcdGoToXY(12,1);
    // lcd.lcdWrite(unit);
     lcd.lcdWrite((char *) unit);
   
    noInterrupts();
    long pulseCount = isrCounter;
    interrupts();
    if (pulseCount > stopCount)
      
       digitalWrite(OUTPIN3, LOW);  // Closes liquid outlet
          
          while(true) {}  // endless loop here to stop program

    AMOUNT = (pulseCount * 2.25);
}
   }
else{
 lcd.lcdGoToXY(4,1);
   lcd.lcdWrite((char*) stop);
 lcd.lcdGoToXY(2,2);
 lcd.lcdWrite((char*) stop1);
  start = false;
}

}
}
void e_stop_ISR(void){
  detachInterrupt(1);
  e_stop = !e_stop;
}
void countP() 
{
  
  isrCounter++;
}

Please help :wink:

What do you have connected to pins 2 and 3 and how?

I’m assuming UNO since you failed to mention which type of Arduino you have. If you have a different flavor then instead of pins 2 and 3 I’m asking about the pins for interrupts 0 and 1.

Please fix your indentation. I have done it for you once so other people can read it. Use a standard indentation to clearly show the code blocks. Never put more than one statement per line. Place any brackets by themselves on a separate line. Use blank lines sparingly, no more than one at a time. Before posting the code, use Ctrl-T in the IDE to reformat the code in a standard format, which makes it easier for us to read.

#include <LCD16x2.h>
#include <Wire.h>

LCD16x2 lcd;
int buttons;

boolean start = true;                  //Code for the stopbutton
volatile boolean e_stop = false;
boolean BUTTONSTATE;

volatile unsigned long isrCounter;  //Code for the AMOUNT counter
unsigned long pulseCount;
unsigned long stopCount = 1400;

char project[] = {"Flaskfyllaren"};
const char volym[] = {"Volym"};
const char unit[] = {"ml"};
const char stop[] = {"STOPPAD"};
const char stop1[] = {"Starta om fyllaren"};

const int OUTPIN1 =  5;      // Gas inlet
const int OUTPIN2 = 4;       // Gas outlet
const int OUTPIN3 = 6;       // Liquid inlet
float AMOUNT;

int BUTTON = 0;            //code for peumatic
boolean off = true;
const int OUTPIN4 = 7;

void setup() {
  Serial.begin(9600);
  Wire.begin();
  delay(100);
  attachInterrupt(1, e_stop_ISR, RISING);
  attachInterrupt(0, countP, RISING);

  lcd.lcdClear();
  lcd.lcdSetBlacklight(255);
  lcd.lcdGoToXY(2, 1);
  lcd.lcdWrite(project);

  // initialize Outpins as output:
  pinMode(OUTPIN1, OUTPUT);
  pinMode(OUTPIN2, OUTPUT);
  pinMode(OUTPIN3, OUTPUT);
}

void loop() {
  if (start == true)
  {
    if (e_stop == false)
    {
      buttons = lcd.readButtons();
      if (buttons & 0x08)
      {
        if (off == true)
          BUTTON = LOW;
        digitalWrite(OUTPIN4, BUTTON);
      }
      else
      {
        if (off == false)
        {
          off = true;
          BUTTON = LOW;
        }
        else
        {
          off = false;
          BUTTON = HIGH;
          digitalWrite(OUTPIN4, BUTTON);
        }
      }

      if (buttons & 0x01) {
        //Sets all outpins as LOW
        digitalWrite(OUTPIN1, LOW);
        digitalWrite(OUTPIN2, LOW);
        digitalWrite(OUTPIN3, LOW);
      }
      else
      {
        digitalWrite(OUTPIN1, HIGH); //Opens gas inlet
        delay(2000);
        digitalWrite(OUTPIN1, LOW);  //Closes gas inlet
        delay(1000);

        digitalWrite (OUTPIN2, HIGH); //Opens gas outlet
        delay(2000);
        digitalWrite (OUTPIN2, LOW);  //Closes gas outlet
        delay(1000);

        digitalWrite(OUTPIN1, HIGH);  //Opens gas inlet
        delay(2000);
        digitalWrite(OUTPIN1, LOW);  //Closes gas outlet
        delay(1000);

        digitalWrite(OUTPIN3, HIGH);  //Opens liquid inlet.

        lcd.lcdClear();
        lcd.lcdGoToXY(2, 1);
        lcd.lcdWrite((char *) volym);
        lcd.lcdGoToXY(9, 1);
        lcd.lcdWrite((float)AMOUNT);
        lcd.lcdGoToXY(12, 1);
        lcd.lcdWrite((char *) unit);

        noInterrupts();
        long pulseCount = isrCounter;
        interrupts();
        if (pulseCount > stopCount)
          digitalWrite(OUTPIN3, LOW);  // Closes liquid outlet

        while (true) {} // endless loop here to stop program
        AMOUNT = (pulseCount * 2.25);
      }
    }
    else
    {
      lcd.lcdGoToXY(4, 1);
      lcd.lcdWrite((char*) stop);
      lcd.lcdGoToXY(2, 2);
      lcd.lcdWrite((char*) stop1);
      start = false;
    }
  }
}

void e_stop_ISR(void) {
  detachInterrupt(1);
  e_stop = !e_stop;
}

void countP()
{
  isrCounter++;
}

One obvious thing, the line after a forever loop will never execute:

        while (true) {} // endless loop here to stop program
        AMOUNT = (pulseCount * 2.25);

Sorry, missed that small bit what kind of board I'm using. =P
Yes it is an uno and interrup 0 is connected to pin 2 and interrupt 1 is connected to pin 3.
To interrupt 0 I have connected a flow meter, and i have it connected to ground and 5+. Signalwire from the flowmeter goes to pin 2.
For the stop button i have it connected with one leg to pin 3 and via a recistor to ground as a pulldown and the other leg from the switch goes to 5+.

Hope you understand what I mean =P

        while (true) {} // endless loop here to stop program
        AMOUNT = (pulseCount * 2.25);

You realize the the line after the infinite loop can never be reached right?

You also realize the the infinite loop isn't associated with that if statement checking if your pulseCount is greater than stopCount. Since that is the only place you ever check the pulse count I am thinking you probably don't want to freeze the code unconditionally there. I'm guessing that you left out some braces.

This is why braces on if statements are always a good idea even when not strictly needed.

So how about addressing what aarg brought up? Once you hit that while loop, you're done. You have provided no way out, so your program will never move from that spot ever again.

It would be easier to read your program if you would move some of the comments into the code to make it self documenting, for example with

       digitalWrite(OUTPIN3, HIGH);  //Opens liquid inlet.

If you have

#define OPEN HIGH
#define CLOSED LOW

const int liquid_inlet = 6;       // Liquid inlet valve

then you can say

       digitalWrite(liquid_inlet, OPEN);  //Here you can talk about why you're doing it...

I'm new to this thing.
I took some code from an example code to trying to get my counter to work, mayby i put it in all wrong.

// flow pulse counter
// Uses pin 2 interupt to measure flow of material signal input
// retrolefty 2/5/11

  
volatile unsigned long isrCounter;
unsigned long pulseCount;
unsigned long stopCount = 1400;

void setup() {

 Serial.begin(57600);
 attachInterrupt(0, countP, RISING);

 }  // End of setup


void loop() {
 delay(100); 
 noInterrupts();
 long pulseCount = isrCounter;
 interrupts();
 if (pulseCount > stopCount)
     { 
       digitalWrite(OUTPIN3, LOW); 	// issue a stop command here for the pump
       					// some kind of digitalWrite(pumpPin, LOW);
          
          while(true) {}  // endless loop here to stop program
     }

}  // end of loop

void countP() 
{
  isrCounter++;
}

Hitting ctrl T i will do for my following codes, didnt know how to fix indentation but now I can use the standard indentation that the IDE fixes for me. =)

I have commentted away the

 //while(true) { }  // endless loop here to stop program

and now I can use the stop button, but not exactly as I wanted, I want it to be like an emergancy stop,
but now if i press it the else bit where i do all digitalwrite continues until it is done and then it pops up that it have been stopped.

Changed some in the code by the tips of aarg, hopes it makes it a little mer readable

#include <LCD16x2.h>
#include <Wire.h>

LCD16x2 lcd;
int buttons;

#define OPEN HIGH
#define CLOSED LOW

boolean start = true;                  //Code for the stopbutton
volatile boolean e_stop = false;
boolean BUTTONSTATE;

volatile unsigned long isrCounter;  //Code for the AMOUNT counter
unsigned long pulseCount;
unsigned long stopCount = 1400;

char project[] = {"Flaskfyllaren"};
const char volym[] = {"Volym"};
const char unit[] = {"ml"};
const char stop[] = {"STOPPAD"};
const char stop1[] = {"Starta om fyllaren"};

const int gas_inlet =  5;      // Gas inlet
const int gas_outlet = 4;       // Gas outlet
const int liquid_inlet = 6;       // Liquid inlet
float AMOUNT;

int BUTTON = 0;						//code for peumatic
boolean off = true;
const int OUTPIN4 = 7;





void setup() {
  Serial.begin(9600);
  Wire.begin();
  delay(100);
  attachInterrupt(1, e_stop_ISR, RISING);
  attachInterrupt(0, countP, RISING);

  lcd.lcdClear();
  lcd.lcdSetBlacklight(255);
  lcd.lcdGoToXY(2, 1);
  lcd.lcdWrite(project);

  // initialize Outpins as output:
  pinMode(gas_inlet, OUTPUT);
  pinMode(gas_outlet, OUTPUT);
  pinMode(liquid_inlet, OUTPUT);

}

void loop() {
  if (start == true) {

    if (e_stop == false) {
      buttons = lcd.readButtons();



      if (buttons & 0x08) {
        if (off == true)
          BUTTON = LOW;
        digitalWrite(OUTPIN4, BUTTON);
      }
      else {
        if (off == false) {
          off = true;
          BUTTON = LOW;
        }
        else {
          off = false;
          BUTTON = HIGH;
          digitalWrite(OUTPIN4, BUTTON);
        }
      }




      if (buttons & 0x01) {
        //Sets all outpins as LOW
        digitalWrite(gas_inlet, LOW);
        digitalWrite(gas_outlet, LOW);
        digitalWrite(liquid_inlet, LOW);
      }

      else {

        digitalWrite(gas_inlet, OPEN); //Opens gas inlet
        delay(2000);
        digitalWrite(gas_inlet, CLOSED);  //Closes gas inlet
        delay(1000);

        digitalWrite (gas_outlet, OPEN); //Opens gas outlet
        delay(2000);
        digitalWrite (gas_outlet, CLOSED);  //Closes gas outlet
        delay(1000);

        digitalWrite(gas_inlet, OPEN);  //Opens gas inlet
        delay(2000);
        digitalWrite(gas_inlet, CLOSED);  //Closes gas outlet
        delay(1000);

        digitalWrite(liquid_inlet, OPEN);  //Opens liquid inlet.

        //   Serial.print("Volym :");
        //   Serial.print(AMOUNT);
        //   Serial.println("ml");

        lcd.lcdClear();
        lcd.lcdGoToXY(2, 1);
        //lcd.lcdWrite(volym);
        lcd.lcdWrite((char *) volym);
        lcd.lcdGoToXY(9, 1);
        lcd.lcdWrite((float)AMOUNT);
        lcd.lcdGoToXY(12, 1);
        // lcd.lcdWrite(unit);
        lcd.lcdWrite((char *) unit);
        AMOUNT = (pulseCount * 2.25);
        noInterrupts();
        long pulseCount = isrCounter;
        interrupts();
        if (pulseCount > stopCount)

          digitalWrite(liquid_inlet, CLOSED);  // Closes liquid outlet

        //while(true) { }  // endless loop here to stop program


      }
    }
    else {
      lcd.lcdClear();
      lcd.lcdGoToXY(4, 1);
      lcd.lcdWrite((char*) stop);
      lcd.lcdGoToXY(2, 2);
      lcd.lcdWrite((char*) stop1);
      start = false;
    }

  }
}
void e_stop_ISR(void) {
  detachInterrupt(1);
  e_stop = !e_stop;
}
void countP()
{

  isrCounter++;
}

And the liquid_inlet starts and stops directly, it dosent wait until it reaches stopCount.
And it newer starts counting =(

Do us a favor and put braces on ALL if and else statements, even the one liners where it isn't strictly necessary. That will help us to see clearly what you intend.

By braces do you mean the curly braces { } ?

Dont really understand where you want me to put them, when I look at the code I cant see where I dont have braces have in mind that I'm a total noob when it comes to programing =P

Here

if (buttons & 0x08) {
        if (off == true){
          BUTTON = LOW;
        digitalWrite(OUTPIN4, BUTTON);
     } }

and here

if (pulseCount > stopCount){

          digitalWrite(liquid_inlet, CLOSED);  // Closes liquid outlet
}

They where the only ones I found.

code now is

#include <LCD16x2.h>
#include <Wire.h>

LCD16x2 lcd;
int buttons;

#define OPEN HIGH
#define CLOSED LOW

boolean start = true;                  //Code for the stopbutton
volatile boolean e_stop = false;
boolean BUTTONSTATE;

volatile unsigned long isrCounter;  //Code for the AMOUNT counter
unsigned long pulseCount;
unsigned long stopCount = 1400;

char project[] = {"Flaskfyllaren"};
const char volym[] = {"Volym"};
const char unit[] = {"ml"};
const char stop[] = {"STOPPAD"};
const char stop1[] = {"Starta om fyllaren"};
const char gas_in[] = {"Gas in"};
const char gas_out[] = {"Gas ut"};

const int gas_inlet =  5;      // Gas inlet
const int gas_outlet = 4;       // Gas outlet
const int liquid_inlet = 6;       // Liquid inlet
float AMOUNT;

int BUTTON = 0;						//code for peumatic
boolean off = true;
const int OUTPIN4 = 7;





void setup() {
  Serial.begin(9600);
  Wire.begin();
  delay(100);
  attachInterrupt(1, e_stop_ISR, RISING);
  attachInterrupt(0, countP, RISING);

  lcd.lcdClear();
  lcd.lcdSetBlacklight(255);
  lcd.lcdGoToXY(2, 1);
  lcd.lcdWrite(project);

  // initialize Outpins as output:
  pinMode(gas_inlet, OUTPUT);
  pinMode(gas_outlet, OUTPUT);
  pinMode(liquid_inlet, OUTPUT);

}

void loop() {
  if (start == true) {

    if (e_stop == false) {
      buttons = lcd.readButtons();



      if (buttons & 0x08) {
        if (off == true){
          BUTTON = LOW;
        digitalWrite(OUTPIN4, BUTTON);
      }
      }
      else {
        if (off == false) {
          off = true;
          BUTTON = LOW;
        }
        else {
          off = false;
          BUTTON = HIGH;
          digitalWrite(OUTPIN4, BUTTON);
        }
      }




      if (buttons & 0x01) {
        //Sets all outpins as LOW
        digitalWrite(gas_inlet, CLOSED);
        digitalWrite(gas_outlet, CLOSED);
        digitalWrite(liquid_inlet, CLOSED);
      }

      else {
        lcd.lcdClear();
        lcd.lcdGoToXY(3, 1);
        lcd.lcdWrite((char *) gas_in);
        digitalWrite(gas_inlet, OPEN); //Opens gas inlet
        delay(2000);
        digitalWrite(gas_inlet, CLOSED);  //Closes gas inlet
        delay(1000);

        lcd.lcdClear();
        lcd.lcdGoToXY(3, 1);
        lcd.lcdWrite((char *) gas_out);
        digitalWrite (gas_outlet, OPEN); //Opens gas outlet
        delay(2000);
        digitalWrite (gas_outlet, CLOSED);  //Closes gas outlet
        delay(1000);

        lcd.lcdClear();
        lcd.lcdGoToXY(3, 1);
        lcd.lcdWrite((char *) gas_in);
        digitalWrite(gas_inlet, OPEN);  //Opens gas inlet
        delay(2000);
        digitalWrite(gas_inlet, CLOSED);  //Closes gas outlet
        delay(1000);

        digitalWrite(liquid_inlet, OPEN);  //Opens liquid inlet.

        //   Serial.print("Volym :");
        //   Serial.print(AMOUNT);
        //   Serial.println("ml");

        lcd.lcdClear();
        lcd.lcdGoToXY(2, 1);
        //lcd.lcdWrite(volym);
        lcd.lcdWrite((char *) volym);
        lcd.lcdGoToXY(9, 1);
        lcd.lcdWrite((float)AMOUNT);
        lcd.lcdGoToXY(12, 1);
        // lcd.lcdWrite(unit);
        lcd.lcdWrite((char *) unit);
        AMOUNT = (pulseCount * 2.25);
        noInterrupts();
        long pulseCount = isrCounter;
        interrupts();
        if (pulseCount > stopCount){

          digitalWrite(liquid_inlet, CLOSED);  // Closes liquid outlet
        }
        //while(true) { }  // endless loop here to stop program


      }
    }
    else {
      lcd.lcdClear();
      lcd.lcdGoToXY(4, 1);
      lcd.lcdWrite((char*) stop);
      lcd.lcdGoToXY(2, 2);
      lcd.lcdWrite((char*) stop1);
      start = false;
    }

  }
}
void e_stop_ISR(void) {
  detachInterrupt(1);
  e_stop = !e_stop;
}
void countP()
{

  isrCounter++;
}

Yes I mean the {}. Just look at every single if statement or else statement. If you see an if statement that doesn't have an opening brace right after it then put one and put a closing brace after the last line that should be run only if the if is true. Do the same for the else. Then autoformat and repost. Actually, retest first.

Delta_G:
Yes I mean the {}. Just look at every single if statement or else statement. If you see an if statement that doesn't have an opening brace right after it then put one and put a closing brace after the last line that should be run only if the if is true. Do the same for the else. Then autoformat and repost. Actually, retest first.

Done, in the post above. Can't find any more, and I have had the IDE auto fixed the indentation by pressing down ctrl T. I have also verified the code and it comes out clean, uploaded it and no difference from previous atempt. :frowning:

For readability closing } should always be on their own line. It's a matter of preference if you want the opening { on a new line or after e.g. the if condition. I prefer

     if (buttons & 0x01)
      {
        ...
        ...
      }
      else
      {
        ...
        ...
      }

For readability, I would also use #defines for the possible buttons (e.g. #define RESET_INLETS 0x01)

Compared to the code that you borrowed, the while statement that you commented out is in the wrong place.

Original (reply #7)

 if (pulseCount > stopCount)
  {
    digitalWrite(OUTPIN3, LOW);   // issue a stop command here for the pump
    // some kind of digitalWrite(pumpPin, LOW);

    while (true) {} // endless loop here to stop program
  }

Your original (code in opening post)

   if (pulseCount > stopCount)
     
       digitalWrite(OUTPIN3, LOW);  // Closes liquid outlet
         
          while(true) {}  // endless loop here to stop program

Your updated code (reply #12; compare this against the original, where is the while located?)

       if (pulseCount > stopCount)
        {
          digitalWrite(liquid_inlet, CLOSED);  // Closes liquid outlet
        }
        //while(true) { }  // endless loop here to stop program

If your liquid_inlet closes immediately, you have one of two problems as far as I can see
1)
Your counter goes faster than you think. For this I would write a simple test sketch based the original that you borrowed and in loop() display the pulseCount on the serial port to check. Also check that there are no pulses if there is no flow. Does your original work properly?
2)
You have a hardware problem; opening the liquit_inlet draws so much current that the Arduino resets. Are there any symptoms of that? How are your valves wired to the Arduino? How are they powered? Can you draw a diagram how everything is connected and scan it or take a photo and post it here? This is however a little less likely as I would expect that to be the case for all valves (if they are the same).

Dont think the liquid_inlet draws to much current. There is nothing hooked up but a simple led to show that the code puts out a high state, so that i can use the signal for to switch a relay later on when I get the code to work.

As for the counter I dont let the liquid flow through until the led marks the liquid_inlet as open. I cant see the counter register any pulses.
I guess I should mayby look for a mailfunktion in the hardware now before i dig in more in the code.

The while i commented away since I dont want to get stuck in a infinit loop. And when the the pulsecount reaches stopcount I can start the loop again with the same butten without resetting the arduino.

Hope my english makes sense, it isnt the language I speak daily :wink: