Working attachInterrupt and pulseIn()

[yeehaw886]

Hi,

My friend and I are working on a project of detecting motor speed using Arduino UNO and Adafruit RGB LCD display.

The motor we have has a encoder which the pulses are 512/revolution. We wrote a script using pulseIn() function and get a good result.

// include the library code:
#include <Wire.h>
#include <Adafruit_RGBLCDShield.h>
#include <utility/Adafruit_MCP23017.h>

// define lcd instance
Adafruit_RGBLCDShield lcd = Adafruit_RGBLCDShield();

// These #defines make it easy to set the backlight color
#define RED 0x1
#define YELLOW 0x3
#define GREEN 0x2
#define TEAL 0x6
#define BLUE 0x4
#define VIOLET 0x5
#define WHITE 0x7

const int IntPin=7;

unsigned long onTime, offTime;
unsigned long Tdelt, Tnow, Tlast;
unsigned long rpm;

void setup() {
  lcd.begin(16, 2);
  lcd.setBacklight(TEAL);
  Tlast = millis();
  Serial.begin(9600);
}

// loop here
void loop(){
  Tnow = millis();
  Tdelt = Tnow - Tlast;
  if (Tdelt >= 1000) {
    lcd.clear();
    Tlast = Tnow;
    onTime = pulseIn(7, HIGH);
    offTime = pulseIn(7, LOW);
    rpm=1000000.0/((onTime+offTime)*512)*60;
    lcd.setCursor(0,1);
    lcd.print("RPM =");
    lcd.setCursor(6,1);
    lcd.print(rpm);
  }

  uint8_t buttons = lcd.readButtons();

  if (buttons) {
    lcd.clear();
    lcd.setCursor(0,0);
    if (buttons & BUTTON_UP) {
      lcd.print("UP ");
      lcd.setBacklight(RED);
    }
    if (buttons & BUTTON_DOWN) {
      lcd.print("DOWN ");
      lcd.setBacklight(YELLOW);
    }
    if (buttons & BUTTON_LEFT) {
      lcd.print("LEFT ");
      lcd.setBacklight(GREEN);
    }
    if (buttons & BUTTON_RIGHT) {
      lcd.print("RIGHT ");
      lcd.setBacklight(TEAL);
    }
    if (buttons & BUTTON_SELECT) {
      lcd.print("SELECT ");
      lcd.setBacklight(VIOLET);
    }
  }
  
}

Then, we came across the idea of attachInterrupt which should give us a much precise result. We just modified the existing script which is showed below

// include the library code:
#include <Wire.h>
#include <Adafruit_RGBLCDShield.h>
#include <utility/Adafruit_MCP23017.h>

// define lcd instance
Adafruit_RGBLCDShield lcd = Adafruit_RGBLCDShield();

// These #defines make it easy to set the backlight color
#define RED 0x1
#define YELLOW 0x3
#define GREEN 0x2
#define TEAL 0x6
#define BLUE 0x4
#define VIOLET 0x5
#define WHITE 0x7

const int IntPin=7;

volatile unsigned long cnt=0;
unsigned long oldcnt, nowcnt;
unsigned long Tdelt, Tnow, Tlast;
unsigned long rpm;

void setup() {
  lcd.begin(16, 2);
  lcd.setBacklight(TEAL);
  pinMode(IntPin, INPUT)
  attachInterrupt(digitalPinToInterrupt(IntPin), irq1, CHANGE);
  oldcnt = 0;
  Tlast = millis();
  Serial.begin(9600);
}


// if detects interrupt, execute this function
void irq1(){
  cnt++;
  Serial.println("hahah");
}

// loop here
void loop(){
  Tnow = millis();
  Tdelt = Tnow - Tlast;
  if (Tdelt >= 1000) {
    lcd.clear();
    nowcnt = cnt;
    Tlast = Tnow;
    rpm = round((nowcnt - oldcnt)*1000/Tdelt)/512 * 60; //takes counts and puts them in Hz (#/s). Encoder has 512 pulses per revolution
    oldcnt = nowcnt;
    lcd.setCursor(0,1);
    lcd.print("RPM =");
    lcd.setCursor(6,1);
    lcd.print(rpm);
  }
...
}

However, this doesn't really work. The number displayed in the LCD is just 0 the whole time. It seems to us that the irq1() has never been called. Unfortunately , we couldn't figure out what to fix this.

Could anyone help us out? Many thanks in advance!!

---

edit:
The pulse is a square wave

[cattledog]

using Arduino UNO

const int IntPin=7;
attachInterrupt(digitalPinToInterrupt(IntPin), irq1, CHANGE);

Only pins 2 and 3 are external interrupt pins on the Uno.
Do not print in the isr.

[Railroader]

Serial.print in an ISR is bad practise.
How does the the code "..." compile?..... Always post the entire code, never snippets.
Why is cnt never reset to 0? Things can get wrong when cnt wraps around from 0xffffffff to 0 "now and then".

This line looses precision and magnifies the error by 60.
rpm = round((nowcnt - oldcnt)*1000/Tdelt)/512 * 60

This might do a little better.
rpm = round(((nowcnt - oldcnt)*1000)*60)/Tdelt/512

round or truncate affects LSB only.

[yeehaw886]

Thank you so much!