Rotary Encoder Readings

Hi,

I’m Vinesh and currently I’m working on a project whose basic purpose is to check the Center value of total total pulses given by encoder while rotating.

In my Project I’m using a OMRON (E6C2-CWZ6C) 200 PPR resolution encoder to check to rotation, a 16*2 LED for display purpose, 2 Push buttons for resetting current values store in pulses and searching for the center and most importantly I’m using Arduino UNO.

But currently I’m trapped in a problem. Readings given by encoder and stored in PULSES variable works properly when encoder is rotated slowly but its Value i.e. PULSES get stuck when any jerk comes in rotation.
For resetting PULSES value, i have to reset my ARDUINO from ARDUINO reset button available on its button.

Please check the encoder’s pulses reading logic and FYI i have used interrupts for reading pulses.

Please suggest what can be the possible cause and countermeasure for this problem.

Encoder wiring circuit is also attached for reference.

Encoder LOgic.txt (7.65 KB)

Code posted properly below. Please read the “How to use this forum” post and follow the directions.

I don’t know what you mean by “PULSES get stuck when any jerk comes in rotation”, but

NEVER, EVER PRINT WITHIN AN INTERRUPT.

///button constants
const int startbuttonPin = 8;
const int searchbuttonPin = 9;
int startbuttonState;             // the current reading from the input pin
int laststartButtonState = LOW;   // the previous reading from the input pin
int searchbuttonState;             // the current reading from the input pin
int lastsearchButtonState = LOW;   // the previous reading from the input pin
// the following variables are unsigned long's because the time, measured in miliseconds,
// will quickly become a bigger number than can be stored in an int.
unsigned long laststartDebounceTime = 0;  // the last time the output pin was toggled
unsigned long lastsearchDebounceTime = 0;  // the last time the output pin was toggled
unsigned long debounceDelay = 50;    // the debounce time; increase if the output flickers

/////encoder constants
int pulses, A_SIG = 0, B_SIG = 1;

/////centring constants
int count;
int highrange;
int lowrange;
int i = 0;
int range = 0;
int led = 13;
int search = 0;
int sub = 0;
int dsub = 0;


//////lcd display
// include the library code:
#include <LiquidCrystal.h>

LiquidCrystal lcd(12, 11, 7, 6, 5, 4);



void setup()
{

  pinMode(led, OUTPUT);

  //////button setup
  pinMode(startbuttonPin, INPUT);
  pinMode(searchbuttonPin, INPUT);


  /////encoder setup
  attachInterrupt(0, A_RISE, RISING);
  attachInterrupt(1, B_RISE, RISING);
  Serial.begin(115200);

  //////lcd setup
  // set up the LCD's number of columns and rows:
  lcd.begin(16, 2);
  lcd.setCursor(0, 0);
  lcd.print("Vinesh Saini -AF");
  lcd.display();
  delay(1500);
  lcd.noDisplay();
  delay(500);

  lcd.setCursor(0, 1);
  lcd.print("Centring Project");
  lcd.display();
  delay(2000);


  lcd.clear();
  lcd.setCursor(0, 0);
  lcd.print("Vinesh's Project" );
  lcd.display();
  delay(1000);




}



void loop()
{
  /////lcd logic


  // Print a message to the LCD.

  lcd.setCursor(0, 0);
  lcd.print("Vinesh's Project" );



  if (i == 0)
  {
    lcd.setCursor(0, 1);
    lcd.print(pulses);


  }
  if (pulses < 1010 && pulses > 990)
  {
    lcd.clear();
    lcd.setCursor(0, 1);
    lcd.print(pulses);

  }

  if (pulses < 100 && pulses > 95)
  {
    lcd.clear();
    lcd.setCursor(0, 1);
    lcd.print(pulses);

  }


  if (pulses < 3 && pulses > -3 && pulses != 0)
  {
    lcd.clear();
    lcd.setCursor(0, 1);
    lcd.print(pulses);

  }

  if (pulses < -90 && pulses > -110)
  {
    lcd.clear();
    lcd.setCursor(0, 1);
    lcd.print(pulses);


  }

  if (pulses < -995 && pulses > -1010)
  {
    lcd.clear();
    lcd.setCursor(0, 1);
    lcd.print(pulses);

  }

  if (i == 2)
  {

    lcd.setCursor(5, 1);
    lcd.print(search);

    if (search < 1050 && search > 950)
    {
      lcd.clear();
      lcd.setCursor(0, 1);
      lcd.print(count);
      lcd.setCursor(5, 1);
      lcd.print(search);
    }

    if (search < 100 && search > 90)
    {
      lcd.clear();
      lcd.setCursor(0, 1);
      lcd.print(count);
      lcd.setCursor(5, 1);
      lcd.print(search);
    }


    if (search < 10 && search > -10)
    {
      lcd.clear();
      lcd.setCursor(0, 1);
      lcd.print(count);
      lcd.setCursor(5, 1);
      lcd.print(search);
    }

    if (search < -90 && search > -110)
    {
      lcd.clear();
      lcd.setCursor(0, 1);
      lcd.print(count);
      lcd.setCursor(5, 1);
      lcd.print(search);

    }

    if (search < -990 && search > -1100)
    {
      lcd.clear();
      lcd.setCursor(0, 1);
      lcd.print(count);
      lcd.setCursor(5, 1);
      lcd.print(search);

    }
  }

  ///////button debouncing logic


  // read the state of the switch into a local variable:
  int startreading = digitalRead(startbuttonPin);

  // 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 (startreading != laststartButtonState)
  {
    // reset the debouncing timer
    laststartDebounceTime = millis();
  }

  if ((millis() - laststartDebounceTime) > debounceDelay)
  {
    // whatever the reading is at, it's been there for longer
    // than the debounce delay, so take it as the actual current state:

    // if the button state has changed:
    if (startreading != startbuttonState)
    {
      startbuttonState = startreading;

      // only toggle the LED if the new button state is HIGH
      if (startbuttonState == HIGH)
      {
        count = 0;
        pulses = 0;
        i = 0;

        delay(500);
        int startbuttonState = LOW;
        lcd.clear();
      }
    }
  }


  ///////////////////////////

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


  /////////////////////////////////////////////////////////


  // read the state of the switch into a local variable:
  int searchreading = digitalRead(searchbuttonPin);

  // 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 (searchreading != lastsearchButtonState)
  {
    // reset the debouncing timer
    lastsearchDebounceTime = millis();
  }

  if ((millis() - lastsearchDebounceTime) > debounceDelay)
  {
    // whatever the reading is at, it's been there for longer
    // than the debounce delay, so take it as the actual current state:

    // if the button state has changed:
    if (searchreading != searchbuttonState)
    {
      searchbuttonState = searchreading;

      // only toggle the LED if the new button state is HIGH
      if (searchbuttonState == HIGH)
      {
        i = 1;
        count = pulses;
        delay(200);
        range = count / 2 ;
        highrange = range + 5;
        lowrange = range - 5;
        delay(500);
        i++;
        int searchbuttonState = LOW;


      }
    }

  }

  ///////////////////////////

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





  if (i == 2 && highrange >= pulses && lowrange <= pulses)
  {

    digitalWrite(led, HIGH);
    lcd.setCursor(0, 0);
    lcd.print("Vinesh's Project" );
    lcd.setCursor(0, 1);
    lcd.print(count);
    lcd.setCursor(10, 1);
    lcd.print("CENTRE");
    delay(1000);

  }
  digitalWrite(led, LOW);


  sub = count - pulses;
  dsub = sub * 2;
  search = count - dsub;




}







////////encoder's reading_logic

void A_RISE()
{
  detachInterrupt(0);
  A_SIG = 1;

  if (B_SIG == 0)
    pulses++;//moving forward
  if (B_SIG == 1)
    pulses--;//moving reverse
  Serial.println(pulses);
  attachInterrupt(0, A_FALL, FALLING);
}

void A_FALL()
{
  detachInterrupt(0);
  A_SIG = 0;

  if (B_SIG == 1)
    pulses++;//moving forward
  if (B_SIG == 0)
    pulses--;//moving reverse
  Serial.println(pulses);
  attachInterrupt(0, A_RISE, RISING);
}

void B_RISE()
{
  detachInterrupt(1);
  B_SIG = 1;

  if (A_SIG == 1)
    pulses++;//moving forward
  if (A_SIG == 0)
    pulses--;//moving reverse
  Serial.println(pulses);
  attachInterrupt(1, B_FALL, FALLING);
}

void B_FALL()
{
  detachInterrupt(1);
  B_SIG = 0;
  if (A_SIG == 0)
    pulses++;//moving forward
  if (A_SIG == 1)
    pulses--;//moving reverse
  Serial.println(pulses);
  attachInterrupt(1, B_RISE, RISING);
}

Please suggest what can be the possible cause and countermeasure for this problem.

I think that the continual detaching and reattaching interrupts to switch between RISING and FALLING triggers is a poor way to read an encoder. It will not be able to respond to very fast changes. Use CHANGE, and read the pin to see if it rose or fell.

Do some research on quadrature encoder reading algorithms and come up with something better.
http://playground.arduino.cc/Main/RotaryEncoders

cattledog: I think that the continual detaching and reattaching interrupts to switch between RISING and FALLING triggers is a poor way to read an encoder.

Worse than that, its a completely broken way to sense an encoder - it will never be reliable because it loses pulses and generates false pulses.

The only reasonable use for detachInterrupt() I can think of is before entering sleep.

Thank you all for your useful suggestions.

I have done some changes in my logic but still pulses (encoder counts) are not reliable. They are fluctuating and readings not same in every rotation. They also varies with the speed of rotation.

I don’t find logic up to the mark for my application and type of encoder I’m using.
Details are mention here /

Encoder Spec:
OMRON : E6A2-CW3C
RESOLUTION : 200P/R

Aruino UNO

APPLICATION-

  1. to measure the degree of rotation(termed as pulses in logic) done over encoder shaft to cover distance between any two points
  2. display measurement(pulses) on 2*16 LCD
  3. find the mid value(range) of final measurement(count)
  4. rotate encoder in reverse direction to reach in middle value of both point(i.e. range)

CODE

///button constants
const int startbuttonPin = 8;
const int searchbuttonPin = 9;
int startbuttonState;             // the current reading from the input pin
int laststartButtonState = LOW;   // the previous reading from the input pin
int searchbuttonState;             // the current reading from the input pin
int lastsearchButtonState = LOW;   // the previous reading from the input pin
// the following variables are unsigned long's because the time, measured in miliseconds,
// will quickly become a bigger number than can be stored in an int.
unsigned long laststartDebounceTime = 0;  // the last time the output pin was toggled
unsigned long lastsearchDebounceTime = 0;  // the last time the output pin was toggled
unsigned long debounceDelay = 50;    // the debounce time; increase if the output flickers

/////encoder constants
int pulses, A_SIG=0, B_SIG=1;

/////centring constants
int count;
int highrange;
int lowrange;
int i = 0;
int range = 0;
int led = 13;

//////lcd display
// include the library code:
#include <LiquidCrystal.h>

LiquidCrystal lcd(12, 11, 7, 6, 5, 4);



void setup() 
{ 
  
  pinMode(led, OUTPUT);
  
  //////button setup
  pinMode(startbuttonPin, INPUT);
  pinMode(searchbuttonPin, INPUT);
  
  
  /////encoder setup
  attachInterrupt(0, A_RISE, RISING);
  attachInterrupt(1, B_RISE, RISING);
  Serial.begin(115200);
  
  //////lcd setup
  // set up the LCD's number of columns and rows:
  lcd.begin(16, 2);
   

}



void loop() 
{
  /////lcd logic
  

  // Print a message to the LCD.
  lcd.setCursor(0, 0);
  lcd.print("Vinesh's Project");
  lcd.setCursor(0, 1);
  lcd.print(pulses);
  

  
  
 ///////button debouncing logic
 
 
  // read the state of the switch into a local variable:
  int startreading = digitalRead(startbuttonPin);

  // 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 (startreading != laststartButtonState) 
  {
    // reset the debouncing timer
    laststartDebounceTime = millis();
  }
  
  if ((millis() - laststartDebounceTime) > debounceDelay) 
  {
    // whatever the reading is at, it's been there for longer
    // than the debounce delay, so take it as the actual current state:

    // if the button state has changed:
    if (startreading != startbuttonState) 
    {
      startbuttonState = startreading;

      // the start button state is HIGH
      if (startbuttonState == HIGH) 
      {
        count = 0;
        pulses = 0;
        i = 0;
        delay(500);
        int startbuttonState = LOW;
        lcd.clear();
      }
    }
  }


  ///////////////////////////

  // save the reading.  Next time through the loop,
  // it'll be the lastButtonState:
  laststartButtonState = startreading;
  
  
  /////////////////////////////////////////////////////////
  
  
  // read the state of the switch into a local variable:
  int searchreading = digitalRead(searchbuttonPin);

  // 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 (searchreading != lastsearchButtonState) 
  {
    // reset the debouncing timer
    lastsearchDebounceTime = millis();
  }
  
  if ((millis() - lastsearchDebounceTime) > debounceDelay) 
  {
    // whatever the reading is at, it's been there for longer
    // than the debounce delay, so take it as the actual current state:

    // if the button state has changed:
    if (searchreading != searchbuttonState) 
    {
      searchbuttonState = searchreading;

      // if search button state is HIGH
      if (searchbuttonState == HIGH) 
      {
        i = 1;
        count = pulses;
        delay(200);
        range = count / 2 ;
        highrange = range + 5;
        lowrange = range - 5;
        delay(500);
        i++;
        int searchbuttonState = LOW;
        lcd.clear();
        
      }
    }
  }

  ///////////////////////////

  // save the reading.  Next time through the loop,
  // it'll be the lastButtonState:
  lastsearchButtonState = searchreading;
  
  
  
  
  
   if(i==2 && highrange >= pulses && lowrange <= pulses)
   {
     digitalWrite(led, HIGH); 
     lcd.setCursor(10, 1);
       lcd.print("CENTRE");
       delay(1000);
       lcd.clear();
   }
    digitalWrite(led, LOW);
   
 }







/////////////////////////////encoder reading_logic////////////////////////////////////////

void A_RISE()
{
 detachInterrupt(0);
 A_SIG=1;
 
 if(B_SIG==0)
 pulses++;//moving forward
 if(B_SIG==1)
 pulses--;//moving reverse
 attachInterrupt(0, A_FALL, FALLING);
}

void A_FALL()
{
 detachInterrupt(0);
 A_SIG=0;
 
 if(B_SIG==1)
 pulses++;//moving forward
 if(B_SIG==0)
 pulses--;//moving reverse
 attachInterrupt(0, A_RISE, RISING);  
}

void B_RISE()
{
 detachInterrupt(1);
 B_SIG=1;
 
 if(A_SIG==1)
 pulses++;//moving forward
 if(A_SIG==0)
 pulses--;//moving reverse
 attachInterrupt(1, B_FALL, FALLING);
}

void B_FALL()
{
 detachInterrupt(1);
 B_SIG=0;
 if(A_SIG==0)
 pulses++;//moving forward
 if(A_SIG==1)
 pulses--;//moving reverse
 attachInterrupt(1, B_RISE, RISING);
}

You are having trouble because you are not reading the replies.

It is a [u]very bad idea[/u] to have all those attach and detachInterrupt calls. Just use CHANGE, in setup().

Thank you all for your useful suggestions. I have done some changes in my logic but still pulses (encoder counts) are not reliable. They are fluctuating and readings not same in every rotation. They also varies with the speed of rotation.

Yes, you have removed the serial printing from the interrupt, but have not addressed the major design problem of the encoder algorithm based on switching between RISING/FALLING and attaching/detaching the interrupt to make the change over.

Use CHANGE, and read the pins to determine the states. Pulses should be declared as "volatile" and it needs to be read in a protected fashion which prevents the value from changing while it is being read. See Nick Gammons interrupt tutorial at http://gammon.com.au/interrupts

Calling detachInterrupt WILL LOSE COUNTS. You cannot get away with this for an encoder, and there's seldom/never any reason to call detachInterrupt() at all.

Put the logic in the ISR, leave it installed as CHANGE.

I have changed the application of interrupt.
Now i have used CHANGE at the place of RISING/FALLING.
But still the problem is same.

///button constants
#define aSIG 2
#define bSIG 3

const int startbuttonPin = 8;
const int searchbuttonPin = 9;
int startbuttonState;             // the current reading from the input pin
int laststartButtonState = LOW;   // the previous reading from the input pin
int searchbuttonState;             // the current reading from the input pin
int lastsearchButtonState = LOW;   // the previous reading from the input pin
// the following variables are unsigned long's because the time, measured in miliseconds,
// will quickly become a bigger number than can be stored in an int.
unsigned long laststartDebounceTime = 0;  // the last time the output pin was toggled
unsigned long lastsearchDebounceTime = 0;  // the last time the output pin was toggled
unsigned long debounceDelay = 50;    // the debounce time; increase if the output flickers

/////encoder constants
volatile int pulses = 0;


/////centring constants
int count;
int highrange;
int lowrange;
int i = 0;
int range = 0;
int led = 13;

//////lcd display
// include the library code:
#include <LiquidCrystal.h>

LiquidCrystal lcd(12, 11, 7, 6, 5, 4);



void setup() 
{ 
  
  pinMode(led, OUTPUT);
  
  //////button setup
  pinMode(startbuttonPin, INPUT);
  pinMode(searchbuttonPin, INPUT);
  pinMode(aSIG, INPUT);
  pinMode(bSIG, INPUT);  
  /////encoder setup
  attachInterrupt(0, A_ENCODER, CHANGE);
  attachInterrupt(1, B_ENCODER, CHANGE);
  Serial.begin(9600);
  
  //////lcd setup
  // set up the LCD's number of columns and rows:
  lcd.begin(16, 2);
   

}






/////////////////////////////encoder reading_logic////////////////////////////////////////



void A_ENCODER() 
{
  // look for a low-to-high on channel A
  if (digitalRead(aSIG) == HIGH) 
  {
    if (digitalRead(bSIG) == LOW)     // check channel B to see which way encoder is turning
    {
      pulses=pulses + 1;        // CW
    }
    else 
    {
      pulses= pulses-1 ;       // CCW
    }
  }

  else   // must be a high-to-low edge on channel A
  {
    if (digitalRead(bSIG) == HIGH)  // check channel B to see which way encoder is turning 
    {
       pulses=pulses + 1;          // CW
    }
    else 
    {
       pulses=pulses - 1;          // CCW
    }
  }

}


/////////////////////////////////////

void B_ENCODER() 
{
  // look for a low-to-high on channel A
  if (digitalRead(bSIG) == HIGH) 
  {
    if (digitalRead(aSIG) == HIGH)     // check channel B to see which way encoder is turning
    {
       pulses=pulses + 1;        // CW
    }
    else 
    {
       pulses=pulses - 1;       // CCW
    }
  }

  else   // must be a high-to-low edge on channel A
  {
    if (digitalRead(aSIG) == LOW)  // check channel B to see which way encoder is turning 
    {
       pulses=pulses + 1;          // CW
    }
    else 
    {
       pulses=pulses - 1;          // CCW
    }
  }

}



void loop() 
{
  /////lcd logic
  

  // Print a message to the LCD.
  lcd.setCursor(0, 0);
  lcd.print("Vinesh's Project");
  lcd.setCursor(0, 1);
  lcd.print(pulses);
  

  
  
 ///////button debouncing logic
 
 
  // read the state of the switch into a local variable:
  int startreading = digitalRead(startbuttonPin);

  // 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 (startreading != laststartButtonState) 
  {
    // reset the debouncing timer
    laststartDebounceTime = millis();
  }
  
  if ((millis() - laststartDebounceTime) > debounceDelay) 
  {
    // whatever the reading is at, it's been there for longer
    // than the debounce delay, so take it as the actual current state:

    // if the button state has changed:
    if (startreading != startbuttonState) 
    {
      startbuttonState = startreading;

      // the start button state is HIGH
      if (startbuttonState == HIGH) 
      {
        count = 0;
        pulses = 0;
        i = 0;
        delay(500);
        int startbuttonState = LOW;
        lcd.clear();
      }
    }
  }


  ///////////////////////////

  // save the reading.  Next time through the loop,
  // it'll be the lastButtonState:
  laststartButtonState = startreading;
  
  
  /////////////////////////////////////////////////////////
  
  
  // read the state of the switch into a local variable:
  int searchreading = digitalRead(searchbuttonPin);

  // 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 (searchreading != lastsearchButtonState) 
  {
    // reset the debouncing timer
    lastsearchDebounceTime = millis();
  }
  
  if ((millis() - lastsearchDebounceTime) > debounceDelay) 
  {
    // whatever the reading is at, it's been there for longer
    // than the debounce delay, so take it as the actual current state:

    // if the button state has changed:
    if (searchreading != searchbuttonState) 
    {
      searchbuttonState = searchreading;

      // if search button state is HIGH
      if (searchbuttonState == HIGH) 
      {
        i = 1;
        count = pulses;
        delay(200);
        range = count / 2 ;
        highrange = range + 5;
        lowrange = range - 5;
        delay(500);
        i++;
        int searchbuttonState = LOW;
        lcd.clear();
        
      }
    }
  }

  ///////////////////////////

  // save the reading.  Next time through the loop,
  // it'll be the lastButtonState:
  lastsearchButtonState = searchreading;
  
  
  
  
  
   if(i==2 && highrange >= pulses && lowrange <= pulses)
   {
     digitalWrite(led, HIGH); 
     lcd.setCursor(10, 1);
       lcd.print("CENTRE");
       delay(1000);
       lcd.clear();
   }
    digitalWrite(led, LOW);
   
 }

With only one interrupt per encoder and all those useless delay() statements, you are guaranteed to miss encoder transitions.

Both interrupts are for the same encoder, and the logic is sound in the ISRs. However the reading of the pulses variable is not guarded with a critical section so it will sometimes fail.

Most importantly though the inputs are floating:

  pinMode(aSIG, INPUT);
  pinMode(bSIG, INPUT);

should be

  pinMode(aSIG, INPUT_PULLUP);
  pinMode(bSIG, INPUT_PULLUP);

And lose those resistors and particularly capacitors that are destroying the high speed performance and causing mayhem. The time constant is probably too slow and the slow edges will be causing grief.