Interrupts skipping most of incoming pulses

Hi!

I'm trying to get arduino read pulses coming from my cnc machine steppers, which are driven by usb motion board.

Interrupts are counting all pulses when I'm not zeroing pulse counters (x and y), but when in the code there is line x=0; and y=0; then interrupts are not working. it only counts about 4 pulses instead of 2000..

here is code ask more if needed. Arduino board is uno copy from china. I had this working at somepoint. not sure why it's not working anymore..

When I uncomment those lines at middle of code the interrupts no more count all but those 4 pulses or less..

const byte IX =2;
const byte IY =3;
byte ntila = 0;
int rpm = 12000;
byte laser =80;
int nvar = 1000;
//näyttö sketsi


byte PWM_out = 6;  //Pwm out
volatile int x = 0;  // X-ax pulses
volatile int y = 0;  // Y-ax pulses
float PWM = 0; // PWM strength       
int ON = 10;
int var= 0;
float PWM_nop = 0;
byte koe =0;
//laser sketsi


#include <Key.h>
#include <Keypad.h>


const byte ROWS = 2; //four rows
const byte COLS = 2; //three columns
byte keys[ROWS][COLS] = {
  {'1','3'},
  {'2','4'},
};
byte rowPins[ROWS] = {5, 4,}; //connect to the row pinouts of the keypad
byte colPins[COLS] = {8, 7,}; //connect to the column pinouts of the keypad

Keypad keypad = Keypad( makeKeymap(keys), rowPins, colPins, ROWS, COLS );



#include <Wire.h>
#include <LiquidCrystal_I2C.h>

LiquidCrystal_I2C lcd(0x27, 2, 1, 0, 4, 5, 6, 7, 3, POSITIVE);










void setup() {
lcd.begin(16,2);

ntila=2;

lcd.home ();
  pinMode(11, OUTPUT);
  pinMode(IX, INPUT_PULLUP);
  pinMode(IY, INPUT_PULLUP);
  pinMode(ON, INPUT_PULLUP);
  pinMode(PWM_out, OUTPUT);       // pwm ulostulo pinni 6
  attachInterrupt(digitalPinToInterrupt(IX), XA, FALLING); // X akseli pinni 2   kun menee high->low
  attachInterrupt(digitalPinToInterrupt(IY), YA, FALLING); // Y akseli pinni 3
  Serial.begin(9600);             //
}
//
void loop() {


 PWM = sqrt(sq((x))+sq(y));
   //Serial.println(PWM*6.1, DEC); // matka kuljettu

 

  
Serial.print("x = ");
Serial.print(x);
Serial.print("    y = ");
Serial.println(y);
  delay(100); //updates pwm signal 

//koe = digitalRead(ON);
//if (koe == LOW && ntila==1) {
//  Serial.print("toimi1 jyrsin");
//  digitalWrite ( 11, LOW);
//  x = 0;
//  y = 0;
//  analogWrite(PWM_out, PWM*0);
//  
//  }
//  
//else if (koe==LOW && ntila==2) {
//  Serial.print("toimi1 laser");
//  digitalWrite ( 11,HIGH);
//  analogWrite(PWM_out, (PWM/PWM_nop));
//  x = 0;
//  y = 0;
//}
//
//else {
//  analogWrite(PWM_out, PWM*0);
//  digitalWrite ( 11,HIGH);
//  x=0;
//  y=0;
//  }



 byte key = keypad.getKey();

  if (key == 49 && rpm<11900 ){
    rpm=rpm+nvar;
    ntila=1;
    Serial.println(key);
    lcd.clear();
    lcd.setCursor(0,0);
    lcd.print("Jyrsinta  RPM");
   
     lcd.setCursor(9,1);
      lcd.print(rpm);
  }
  
  if (key == 50 && laser<100 ){
   
    laser++;
    ntila=2;
    Serial.println(key);
    lcd.clear();
    lcd.setCursor(0,0);
    lcd.print("Laser   Teho");
   
     lcd.setCursor(9,1);
      lcd.print(laser);
  }
  
  if (key == 51 && rpm>0 ){
    rpm=rpm-nvar;
    ntila=1;
    Serial.println(key);
    lcd.clear();
    lcd.setCursor(0,0);
    lcd.print("Jyrsinta  RPM");
   
     lcd.setCursor(9,1);
      lcd.print(rpm);
  }
  
  if (key == 52 && laser>0 ){
     laser--;
     ntila=2;
    Serial.println(key);
    lcd.clear();
    lcd.setCursor(0,0);
    lcd.print("Laser   Teho");
   
     lcd.setCursor(9,1);
      lcd.print(laser);
  }




}

// Interrupti rutiinit
void YA() {
  y++;  }
    
void XA() {
  x++;  }

Please don't use single character variable names. How am I supposed to find all instances of a variable called 'y' in your program? Use meaningful variable names - for example yPulseCount

You need to temporarily disable interrupts when reading a multi-byte variable that is updated by an interrupt - for example

noInterrupts();
  copyOfYPulseCount = yPulseCount;
interrupts();

and then use the copy in your calculations. If you don't do that the variable might be changed by interrupt while you are reading its value.

...R

The simplest approach is to never zero the pulse counters, but record them at both start and end, then take the difference.

Then no synchronization is needed with the interrupt handler other than to guard the reading of the variables in a critical section:

  noInterrupts() ;
  int start_count = count ;
  interrupts() ;

  ...

  noInterrupts() ;
  int end_count = count ;
  interrupts() ;

  Serial.println (end_count - start_count) ;

This is just like using micros() or millis() for timing things, and the subtraction handles the wrap-around nicely in the same way.

hautamak:
Interrupts are counting all pulses when I'm not zeroing pulse counters (x and y), but when in the code there is line x=0; and y=0; then interrupts are not working. it only counts about 4 pulses instead of 2000..

How do you know that the interrupts are not working? All they do is increment x and y. It makes perfect sense that if you set x and y to zero in the main program, the count (however you read it) will be zero or show only a few counts.

Your code has several different ways of addressing a pin. These, too, are confounded by the use of opaque variable names and lack of comments. You should find the best way and stick with it through the entire sketch. You have:

const byte IX = 2;
const byte IY = 3;
[...]
byte PWM_out = 6;  //Pwm out
[...]
int ON = 10;

That's 3 different ways to declare a pin number! Not only did you randomly pick a different data type for ON, but there is no indication that it is a pin number, either in the name, or the comments. Later, you eschew pin number variables entirely:

 //  digitalWrite ( 11, LOW);

The only place where you can clearly see what is what, is in the pinMode setup:

 pinMode(11, OUTPUT);
  pinMode(IX, INPUT_PULLUP);
  pinMode(IY, INPUT_PULLUP);
  pinMode(ON, INPUT_PULLUP);
  pinMode(PWM_out, OUTPUT);       // pwm ulostulo pinni 6

By the way there is a convention that uppercase names are constants, e.g. "LOW". You can choose to be different but it's another source of confusion. Your inconsistent style makes it harder to write, read, edit, and troubleshoot (especially for other people when you have to call for help).

Sorry guys for making unnecessary trouble..

Here is tidier code with comments.

And I know the interrupts are not working because when I set delay to 1000ms and send 400 step to arduino, those all steps should have counted in that 1000ms. If I dont set x and y to zero those 400 steps blinks in just about 10ms..

How that no zero idea works? If variable goes over certain limit (30 000 something?) it goes negative and then that approach won't work..

appreciate your help!

edit: it's still bit of mess..
Forgot to mention but commented part of code is unnecessary.

byte ntila = 0; //variable for lcd screen mode
int rpm = 12000; // spindle rpm variable
byte laser =80; // laser power variable

//variables for lcd screen^

byte ix =2; //input 2 = interrupt 1
byte iy =3;  //input 2 = interrupt 1
byte pwm_out = 6;  //Pwm out pin 6
volatile byte x_count = 0;  // X-ax pulses count
volatile byte y_count = 0;  // Y-ax pulses count
float pwm = 0; // PWM strength  var      
byte on = 10;  //pin for turning laser/spindle on or off. signal comes from usb motion board
byte spindle_relay =11; // turning spindle relay off. pin 11
float pwm_nop = 0; //how fast laser is moving x and y planes combined
byte koe =0;       // variable for storing digital input from pin 10


//variables and inputs for laser and spindle^





#include <Wire.h>
#include <LiquidCrystal_I2C.h>

LiquidCrystal_I2C lcd(0x27, 2, 1, 0, 4, 5, 6, 7, 3, POSITIVE);










void setup() {
lcd.begin(16,2);

ntila=2;

lcd.home ();
  pinMode(spindle_relay, OUTPUT);
  pinMode(ix, INPUT_PULLUP);
  pinMode(iy, INPUT_PULLUP);
  pinMode(on, INPUT_PULLUP);
  pinMode(pwm_out, OUTPUT);       // pwm out pin 6
  attachInterrupt(digitalPinToInterrupt(ix), xa, FALLING); // x axis interrupt at pin 2. When goes high to low, counts as one pulse
  attachInterrupt(digitalPinToInterrupt(ix), ya, FALLING); // y axis interrupt at pin 3. When goes high to low, counts as one pulse
  Serial.begin(9600);             //
}
//
void loop() {


 pwm = sqrt(sq((x_count))+sq(y_count));  // Distance travelled
   //Serial.println(PWM*6.1, DEC); // 

 

  
Serial.print("x = ");
Serial.print(x_count);
Serial.print("    y = ");
Serial.println(y_count);
  delay(100); //updates pwm signal. how many pulses interrupts counts in 100ms 
  
  

//koe = digitalRead(ON);
//if (koe == LOW && ntila==1) {
//  Serial.print("toimi1 jyrsin");
//  digitalWrite ( 11, LOW);
//  x = 0;
//  y = 0;
//  analogWrite(PWM_out, PWM*0);
//  
//  }
//  
//else if (koe==LOW && ntila==2) {
//  Serial.print("toimi1 laser");
//  digitalWrite ( 11,HIGH);
//  analogWrite(PWM_out, (PWM/PWM_nop));
//  x = 0;
//  y = 0;
//}
//
//else {
//  analogWrite(PWM_out, PWM*0);
//  digitalWrite ( 11,HIGH);
//  x=0;
//  y=0;
//  }



//
//  if (key == 49 && rpm<11900 ){
//    rpm=rpm+1000;
//    ntila=1;
//    Serial.println(key);
//    lcd.clear();
//    lcd.setCursor(0,0);
//    lcd.print("Jyrsinta  RPM");
//   
//     lcd.setCursor(9,1);
//      lcd.print(rpm);
//  }
//  
//  if (key == 50 && laser<100 ){
//   
//    laser++;
//    ntila=2;
//    Serial.println(key);
//    lcd.clear();
//    lcd.setCursor(0,0);
//    lcd.print("Laser   Teho");
//   
//     lcd.setCursor(9,1);
//      lcd.print(laser);
//  }
//  
//  if (key == 51 && rpm>0 ){
//    rpm=rpm-1000;
//    ntila=1;
//    Serial.println(key);
//    lcd.clear();
//    lcd.setCursor(0,0);
//    lcd.print("Jyrsinta  RPM");
//   
//     lcd.setCursor(9,1);
//      lcd.print(rpm);
//  }
//  
//  if (key == 52 && laser>0 ){
//     laser--;
//     ntila=2;
//    Serial.println(key);
//    lcd.clear();
//    lcd.setCursor(0,0);
//    lcd.print("Laser   Teho");
//   
//     lcd.setCursor(9,1);
//      lcd.print(laser);
//  }
//



}

// Interrupti rutiinit
void ya() {
  y_count++;  }
    
void xa() {
  x_count++;  }

What the heck is that? Almost your entire program is commented out! If it's unnecessary, what is left over?

 attachInterrupt(digitalPinToInterrupt(ix), ya, FALLING); // y axis interrupt at pin 3. When goes high to low, counts as one pulse

Shouldn't it be (iy) ?

aarg:
What the heck is that? Almost your entire program is commented out! If it's unnecessary, what is left over?

 attachInterrupt(digitalPinToInterrupt(ix), ya, FALLING); // y axis interrupt at pin 3. When goes high to low, counts as one pulse

Shouldn't it be (iy) ?

Yes, good notice.

Well, I just try to get pulse counting with interrupts to work, so that is why I commented all other out. when I get it running I can start to add code and test it.

This way it's way easier to troubleshoot the code.

Agreed, but it's better to remove it entirely for us. Would you agree that this code should do what you want?

byte ix = 2; //input 2 = interrupt 1
byte iy = 3; //input 2 = interrupt 1
volatile byte x_count = 0;  // X-ax pulses count
volatile byte y_count = 0;  // Y-ax pulses count

void setup() {
  pinMode(ix, INPUT_PULLUP);
  pinMode(iy, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(ix), xa, FALLING); // x axis interrupt at pin 2. When goes high to low, counts as one pulse
  attachInterrupt(digitalPinToInterrupt(iy), ya, FALLING); // y axis interrupt at pin 3. When goes high to low, counts as one pulse
  Serial.begin(9600);             //
}

void loop() {
  Serial.print("x = ");
  Serial.print(x_count);
  Serial.print("    y = ");
  Serial.println(y_count);
  delay(100); //updates pwm signal. how many pulses interrupts counts in 100ms
}

// Interrupti rutiinit
void ya() {
  y_count++;
}

void xa() {
  x_count++;
}

Notice also that some of your comments need updating...

Woops, I kinda rushed those comments.

Anyway now it works like charm. no lost pulses thanks for markT. Also thanks for other for bringing up different ways of doing this.

here is working code.

It stores starting amount of pulses counted and then waits 50ms and then stores ending amount of pulses and then subtracts start pulses from end pulses.

Thanks again!

byte ntila = 0; //variable for lcd screen mode
int rpm = 12000; // spindle rpm variable
byte laser =80; // laser power variable

//variables for lcd screen^
int start_countx=0;
int start_county =0;
int end_countx=0;
int end_county =0;
byte ix =2; //input 2 = interrupt 1
byte iy =3;  //input 2 = interrupt 1
byte pwm_out = 6;  //Pwm out pin 6
volatile int x_count = 0;  // X-ax pulses count
volatile int y_count = 0;  // Y-ax pulses count
float pwm = 0; // PWM strength  var      
byte on = 10;  //pin for turning laser/spindle on or off. signal comes from usb motion board
byte spindle_relay =11; // turning spindle relay off. pin 11
float pwm_nop = 0; //how fast laser is moving x and y planes combined
byte koe =0;       // variable for storing digital input from pin 10


//variables and inputs for laser and spindle^





#include <Wire.h>
#include <LiquidCrystal_I2C.h>

LiquidCrystal_I2C lcd(0x27, 2, 1, 0, 4, 5, 6, 7, 3, POSITIVE);










void setup() {
lcd.begin(16,2);

ntila=2;

lcd.home ();
  pinMode(spindle_relay, OUTPUT);
  pinMode(ix, INPUT_PULLUP);
  pinMode(iy, INPUT_PULLUP);
  pinMode(on, INPUT_PULLUP);
  pinMode(pwm_out, OUTPUT);       // pwm out pin 6
  attachInterrupt(digitalPinToInterrupt(ix), xa, FALLING); // x axis interrupt at pin 2. When goes high to low, counts as one pulse
  attachInterrupt(digitalPinToInterrupt(iy), ya, FALLING); // y axis interrupt at pin 3. When goes high to low, counts as one pulse
  Serial.begin(9600);             //
}
//
void loop() {


 pwm = sqrt(sq((x_count))+sq(y_count));  // Distance travelled
   //Serial.println(PWM*6.1, DEC); // 

noInterrupts() ;
int start_countx = x_count ;
int start_county = y_count ;
interrupts() ;

 delay(50); //updates pwm signal. how many pulses interrupts counts in 100ms 
  
noInterrupts() ;
int end_countx = x_count ;
int end_county = y_count ;
interrupts() ;
  
Serial.print("x = ");
Serial.print(end_countx-start_countx);
Serial.print("   xend =  ");
Serial.print(end_countx);
Serial.print("    xstart = ");
Serial.print(start_countx);

Serial.print("    y = ");
Serial.println(end_county-start_county);
  


//koe = digitalRead(ON);
//if (koe == LOW && ntila==1) {
//  Serial.print("toimi1 jyrsin");
//  digitalWrite ( 11, LOW);
//  x = 0;
//  y = 0;
//  analogWrite(PWM_out, PWM*0);
//  
//  }
//  
//else if (koe==LOW && ntila==2) {
//  Serial.print("toimi1 laser");
//  digitalWrite ( 11,HIGH);
//  analogWrite(PWM_out, (PWM/PWM_nop));
//  x = 0;
//  y = 0;
//}
//
//else {
//  analogWrite(PWM_out, PWM*0);
//  digitalWrite ( 11,HIGH);
//  x=0;
//  y=0;
//  }



//
//  if (key == 49 && rpm<11900 ){
//    rpm=rpm+1000;
//    ntila=1;
//    Serial.println(key);
//    lcd.clear();
//    lcd.setCursor(0,0);
//    lcd.print("Jyrsinta  RPM");
//   
//     lcd.setCursor(9,1);
//      lcd.print(rpm);
//  }
//  
//  if (key == 50 && laser<100 ){
//   
//    laser++;
//    ntila=2;
//    Serial.println(key);
//    lcd.clear();
//    lcd.setCursor(0,0);
//    lcd.print("Laser   Teho");
//   
//     lcd.setCursor(9,1);
//      lcd.print(laser);
//  }
//  
//  if (key == 51 && rpm>0 ){
//    rpm=rpm-1000;
//    ntila=1;
//    Serial.println(key);
//    lcd.clear();
//    lcd.setCursor(0,0);
//    lcd.print("Jyrsinta  RPM");
//   
//     lcd.setCursor(9,1);
//      lcd.print(rpm);
//  }
//  
//  if (key == 52 && laser>0 ){
//     laser--;
//     ntila=2;
//    Serial.println(key);
//    lcd.clear();
//    lcd.setCursor(0,0);
//    lcd.print("Laser   Teho");
//   
//     lcd.setCursor(9,1);
//      lcd.print(laser);
//  }
//



}

// Interrupti rutiinit
void ya() {
  y_count++;  }
    
void xa() {
  x_count++;  }