What should be simple: reading a rotary encoder

No, my encoder doesn't have detents, as far as I can feel.

From previous experiments I know the encoder has about 100 steps/revolution.

Thanks to your debugging code I'm now sure there is something with the connection between the wiring and the encoder. I put them both in the breadboard, assuming that would work, but when I touch the wiring and encoder directly the code does something different. To be exact, it seems to work better.

I noted in an earlier post that it seemed to work better when I pushed the encoder... well I should have tried this earlier I guess.

I'm now holding the three wires with one hand and the encoder in the other hand, but it's kind of a hassle and I hope there is a more practicable solution close at hand... as I apparently can't trust the breadboard...

Anyway, since I can't really feel what is one step I'll just stick to moving the encoder slowly.
With a little (300) shorter delay that makes:

Which is, I think, quite a step in the right direction isn't it? That pattern of 0's and 1's seems familiar to me.

When I try cattledog's polling code again, and when I can hold my fingers steady... it gives this result:

Index:  20	Old-New AB Pattern:  1000
Index:  21	Old-New AB Pattern:  0010
Index:  20	Old-New AB Pattern:  1000
Index:  21	Old-New AB Pattern:  0010
Index:  20	Old-New AB Pattern:  1000
Index:  21	Old-New AB Pattern:  0010
Index:  22	Old-New AB Pattern:  1011
Index:  23	Old-New AB Pattern:  1101
Index:  22	Old-New AB Pattern:  0111
Index:  23	Old-New AB Pattern:  1101
Index:  24	Old-New AB Pattern:  0100
Index:  23	Old-New AB Pattern:  0001
Index:  24	Old-New AB Pattern:  0100
Index:  23	Old-New AB Pattern:  0001
Index:  24	Old-New AB Pattern:  0100
Index:  25	Old-New AB Pattern:  0010
Index:  26	Old-New AB Pattern:  1011
Index:  25	Old-New AB Pattern:  1110
Index:  26	Old-New AB Pattern:  1011
Index:  25	Old-New AB Pattern:  1110
Index:  26	Old-New AB Pattern:  1011

A lot better right?

(Do they often have such graphical forum updates? I just thought I knew where all the buttons were and they just changed them all haha)

Ok, one more time.
The encoder is producing Gray code and the result of the encoder will VARY depending how you read it.
If you insist on following the "not use interrupt on Arduino" fools and do you reading in loop() and insert other process in-between readouts , Serial prints or delay - good luck.

If you use interrupt and select PROPER / DESIRED trigger ( rising, change etc) you WILL get repeatable and reliable code from you encoder irregardless if it is slow, fast on Tuesday or Sunday.

As I said in PM , it is your call.
Cheers Vaclav

PS
did you find the ISR in the code mess? Sorry I do not have time nor desire to clean things up for public consumption.

Vaclav:
Ok, one more time.
The encoder is producing Gray code and the result of the encoder will VARY depending how you read it.

What do you mean, it will vary? Do you mean the different outputs in the serial monitor because of a different code?

PS
did you find the ISR in the code mess? Sorry I do not have time nor desire to clean things up for public consumption.

Sorry, which code? Did I miss something?

UKHeliBob:

I know my ISR's are the same, because I copy-pasted the first one so that that ISR can work for the second pin.

As they are both the same then why not trigger the same ISR from both pins ?

Uh, you mean to combine the ISR's into one? Yes, that sounds like a good idea. But to use both interrupt pins in one ISR, how would I go on that? attachinterrupt() when one of the pins CHANGE?

yes, this is the repeating pattern you should see

0 0
1 0
1 1
0 1
0 0
1 0
1 1
0 1
0 0

I now think that what you are seeing when you run the original code I supplied is the result of switch bounce.

Fix your wiring, and get the bounce2 library here (https://github.com/thomasfredericks/Bounce-Arduino-Wiring/tree/master/Bounce2). Install the library, and debounce the digital reads.

PHPirates:

UKHeliBob:

I know my ISR's are the same, because I copy-pasted the first one so that that ISR can work for the second pin.

As they are both the same then why not trigger the same ISR from both pins ?

Uh, you mean to combine the ISR's into one? Yes, that sounds like a good idea. But to use both interrupt pins in one ISR, how would I go on that? attachinterrupt() when one of the pins CHANGE?

No. I meant attach the same ISR to both pins.

Like in my sample code.

// Rotary encoder example.
// Author: Nick Gammon
// Date:   24th May 2011

// Wiring: Connect common pin of encoder to ground.
// Connect pins A and B (the outer ones) to pins 2 and 3 (which can generate interrupts)

volatile boolean fired = false;
volatile long rotaryCount = 0;

// Interrupt Service Routine
void isr ()
{
  
static boolean ready;
static unsigned long lastFiredTime;
static byte pinA, pinB;  

// wait for main program to process it
  if (fired)
    return;
    
  byte newPinA = digitalRead (2);
  byte newPinB = digitalRead (3);
  
  // Forward is: LH/HH or HL/LL
  // Reverse is: HL/HH or LH/LL
  
  // so we only record a turn on both the same (HH or LL)
  
  if (newPinA == newPinB)
    {
    if (ready)
      {
      long increment = 1;
        
      // if they turn the encoder faster, make the count go up more
      // (use for humans, not for measuring ticks on a machine)
      unsigned long now = millis ();
      unsigned long interval = now - lastFiredTime;
      lastFiredTime = now;
      
      if (interval < 10)
        increment = 5;
      else if (interval < 20)
        increment = 3;
      else if (interval < 50)
        increment = 2;
         
      if (newPinA == HIGH)  // must be HH now
        {
        if (pinA == LOW)
          rotaryCount += increment;
        else
          rotaryCount -= increment;
        }
      else
        {                  // must be LL now
        if (pinA == LOW)  
          rotaryCount -= increment;
        else
          rotaryCount += increment;        
        }
      fired = true;
      ready = false;
      }  // end of being ready
    }  // end of completed click
  else
    ready = true;
    
  pinA = newPinA;
  pinB = newPinB;
}  // end of isr


void setup ()
{
  digitalWrite (2, HIGH);   // activate pull-up resistors
  digitalWrite (3, HIGH); 
  
  attachInterrupt (0, isr, CHANGE);   // pin 2
  attachInterrupt (1, isr, CHANGE);   // pin 3

  Serial.begin (115200);
}  // end of setup

void loop ()
{

  if (fired)
    {
    long currentCount;
    
    // protected access to the counter
    noInterrupts ();
    currentCount = rotaryCount;
    interrupts ();
    
    Serial.print ("Count = ");  
    Serial.println (currentCount);
    fired = false;
  }  // end if fired

}  // end of loop

Here is a version of the polled, full quadrature, code I previously posted which includes a debounce of 10ms. You may need more or less time depending upon your encoder.

//Based on code from: http://bildr.org/2012/08/rotary-encoder-arduino/
//uses quadrature bit pattern from current and previous reading

//Changes
//Polled rather than interrupts
//Added start up position check to make index +1/-1 from first move
//Add bounce2 and debounce of digitalReads

#define encoderPinA  3  
#define encoderPinB  2
#define buttonPin 5

#include <Bounce2.h>
// Instantiate three Bounce objects for all pins with digitalRead
Bounce debouncerA = Bounce(); 
Bounce debouncerB = Bounce(); 
Bounce debouncer5 = Bounce();

int lastEncoded = 0;
int encoderValue = 0;
int lastencoderValue = 0;

void setup() {
  Serial.begin (115200);

  pinMode(encoderPinA, INPUT_PULLUP); 
  pinMode(encoderPinB, INPUT_PULLUP);
  pinMode(buttonPin, INPUT_PULLUP);

  debouncerA.attach(encoderPinA);
  debouncerA.interval(10);
  debouncerB.attach(encoderPinB);
  debouncerB.interval(10);
  debouncer5.attach(buttonPin);
  debouncer5.interval(10);

  //get starting position
  debouncerA.update();
  debouncerB.update();

  int lastMSB = debouncerA.read(); 
  int lastLSB = debouncerB.read(); 

  Serial.print("Starting Position AB  " );
  Serial.print(lastMSB);
  Serial.println(lastLSB);

  //let start be lastEncoded so will index on first click
  lastEncoded = (lastMSB << 1) |lastLSB;

}

void loop(){ 

  debouncerA.update();
  debouncerB.update();

  int MSB = debouncerA.read();//MSB = most significant bit
  int LSB = debouncerB.read();//LSB = least significant bit

  int encoded = (MSB << 1) |LSB; //converting the 2 pin values to single number

  int sum  = (lastEncoded << 2) | encoded; //adding it to the previous encoded value

  //test against quadrature patterns CW and CCW
  if(sum == 0b1101 || sum == 0b0100 || sum == 0b0010 || sum == 0b1011) encoderValue ++;
  if(sum == 0b1110 || sum == 0b0111 || sum == 0b0001 || sum == 0b1000) encoderValue --;

  lastEncoded = encoded; //store this value for next time

  if(encoderValue != lastencoderValue){
    Serial.print("Index:  ");
    Serial.print(encoderValue);
    Serial.print('\t');

    Serial.print("Old-New AB Pattern:  ");

    for (int i = 3; i >= 0; i-- )
    {
      Serial.print((sum >> i) & 0X01);//shift and select first bit
    }

    Serial.println();

    lastencoderValue=encoderValue;
  }

  //reset index
  debouncer5.update();
  if(debouncer5.read()==LOW){
    encoderValue=0;
  }

}

PHPirates:

Vaclav:
Ok, one more time.
The encoder is producing Gray code and the result of the encoder will VARY depending how you read it.

What do you mean, it will vary? Do you mean the different outputs in the serial monitor because of a different code?

PS
did you find the ISR in the code mess? Sorry I do not have time nor desire to clean things up for public consumption.

Sorry, which code? Did I miss something?
Yes, I send you my encoder code via PM . I guess you did not get it. Not surprised since I just received PM notification which did not belong to me!!

You cannot read the encoder output at random in loop(). Unless you like to punish yourself and make sure you are REALLY reading NEW code. For example if your read output A and than do delay(2000); you just missed 2 seconds of encoder data before reading output B. Using ISR is the only way to appreciate Gray code properties ( only 1 bit changes ( error detect) and rotation direction can be detected) and to get reliable data from the encoder.

void doEncoderB() {

if (digitalRead(encoder0PinA) == digitalRead(encoder0PinB)) {
    encoder0Pos++;
  } else {
    encoder0Pos--;
  }
Serial.println ("pos:");                                 //interrupt fired
  Serial.println (encoder0Pos, DEC);
}

Don't do serial prints inside an ISR.
My code works, you know. Wired up exactly as you showed above. Middle pin to ground, outside pins to pins 2 and 3 of the Arduino. Pretty simple stuff.

Grrr. Copy and pasting doesn't work.

Don't do serial prints inside an ISR.

cattledog:
Fix your wiring, and get the bounce2 library here (https://github.com/thomasfredericks/Bounce-Arduino-Wiring/tree/master/Bounce2). Install the library, and debounce the digital reads.

Thanks for the link, I used the code of JimboZa (without interrupts), because that includes bounce2.h.
Result:

Setup
Setup done
CW
grossCounter: 1
nettCounter: 1
Nett position: 0 + 1
 
CW
grossCounter: 2
nettCounter: 2
Nett position: 0 + 2
 
CW
grossCounter: 3
nettCounter: 3
Nett position: 0 + 3
 
CW
grossCounter: 4
nettCounter: 4
Nett position: 0 + 4
 
CCW
grossCounter: 5
nettCounter: 3
Nett position: 0 + 3
 
CCW
grossCounter: 6
nettCounter: 2
Nett position: 0 + 2

Seems to work perfectly!

It's a good thing you have now supplied to me different kinds of code, so I can compare them and use the easiest one.

[quote author=Nick Gammon date=1413841234 link=msg=1928774]
Like in my sample code.[/quote]

Yes, you've given me your code already.
Your ISR is constructed very different from my original (wrong) ISR, so I didn't see directly that yours was similar to what UKHeliBob was directing me to.

When I try it though, the counter goes only down, no matter in which direction I turn. Sometimes, when I restart it, it goes only up no matter in which direction I turn.

Count = 121
Count = 120
Count = 119
Count = 118
Count = 117
Count = 116
Count = 115
Count = 114
Count = 113
Count = 108
Count = 107
Count = 105
Count = 100
Count = 99
Count = 98
Count = 97

cattledog:
Here is a version of the polled, full quadrature, code I previously posted which includes a debounce of 10ms. You may need more or less time depending upon your encoder.

Aha, so that's how that works. I will experiment with that a bit, as it's essentially a better version of your previous code.
The index goes now up and down, if I turn the encoder clockwise or counterclockwise. Quite a good result, I'd say.

Yes, I know your code works, but unfortunately my encoder hasn't really understood that...
At least this morning both the codes ( from JimboZa and Cattledog) with debouncing seem to work best for me.
Your code has similar results of the code of UKHeliBob, also without debouncing.

That may change, I haven't fully fixed the hardware connections yet.

Yes, I now know I can't use serial prints inside an ISR, that's where the topic started with after all.

UKHeliBob:
You are using Serial.print() in your ISR
Serial.print() depends on interrupts for some functionality
Interrupts are automatically disabled in ISRs

Can you see a potential problem ?

Ok, so now the next problem is how to control the stepper with the codes supplied above. Should I start a new topic for that?

Ok, so now the next problem is how to control the stepper with the codes supplied above. Should I start a new topic for that?

No.
What do you want to use the encoder data to do? What, exactly, does "control the stepper" mean?

To control the stepper means here:
When I turn the encoder X degrees, I want the stepper to turn also X degrees.

Given that the encoder is only turned slowly (by hand).

Accuracy is very much more important than speed: The stepper does't have to follow fast, but preferably it should turn exactly X degrees.

Eventually we want to try to also make it so, that when you turn the encoder back Y degrees when the stepper hasn't finished turning X degrees, the stepper will go to X-Y degrees anyway, in the ideal case immediately of course.
I hope I put it here understandable enough.

When I turn the encoder X degrees

It emits some number of pulses that you are reading. You need to translate the number of pulses to degrees, based on the number of pulses per revolution.

I want the stepper to turn also X degrees.

Which means that the stepper needs to step n times, based on the number of steps per revolution, and the number of revolutions it needs to make.
Since the encoder is being turned slowly, presumably slower than the stepper can step, it should be a simple matter of stepping n times for each encoder change.
Now, if n is not a whole number, things get a lot more complicated.

PaulS:

When I turn the encoder X degrees

It emits some number of pulses that you are reading. You need to translate the number of pulses to degrees, based on the number of pulses per revolution.

I want the stepper to turn also X degrees.

Which means that the stepper needs to step n times, based on the number of steps per revolution, and the number of revolutions it needs to make.
Since the encoder is being turned slowly, presumably slower than the stepper can step, it should be a simple matter of stepping n times for each encoder change.
Now, if n is not a whole number, things get a lot more complicated.

So it would come down to some translating back and forth, and then trying to make it as exact as possible. Yes, you're right, the stepper can step quite fast, I forgot that.
I'm sorry, I have no time to just now test the code I just made, but maybe you can already take a look at it? I edited JimoZa's code, and added stuff for the stepper.

#include <Bounce2.h>
#include <Stepper.h>

/* 
 DESCRIPTION
 ====================
 Reads the 2 switches in an encoder, 
 determines direction,
 updates counter.
 No interrupts.
 Switches debounced, didn't test first, just did it anyway.
 
 PHPirates' edit: turning the stepper
 */


const byte ENCODER_PINA= 2;
//const byte LED_PINA= 12;
const byte ENCODER_PINB= 3;
//const byte LED_PINB= 11;

const int StepsPerRev = 2048;
Stepper myStepper(StepsPerRev, 1,4,5,6);   //Make known to Arduino what the stepper is
int steps;  

int valueA; //debounced encoder switch reads
int valueB;

bool motionDetected = false;
int grossCounter = 0; // total steps
int nettCounter = 0;  // cw-ccw
int fullRevolutions = 0;
int surplusSteps = 0; //part revs
bool CW;

byte cyclesPerRev =100;  //check encoder datasheet

// Instantiate 2 Bounce object
Bounce debouncerA = Bounce(); 
Bounce debouncerB = Bounce(); 

// setup ********************************************
void setup() {
  Serial.begin(115200);
  Serial.println("Setup");
  // Setup the buttons
  pinMode(ENCODER_PINA,INPUT_PULLUP);
  pinMode(ENCODER_PINB,INPUT_PULLUP);


  // After setting up the button, setup debouncer
  debouncerA.attach(ENCODER_PINA);
  debouncerA.interval(5);
  debouncerB.attach(ENCODER_PINB);
  debouncerB.interval(5);

  //Setup the LED
 // pinMode(LED_PINA,OUTPUT);
 // pinMode(LED_PINB,OUTPUT);
  Serial.println("Setup done");
}

// loop *****************************************
void loop() {

  // Update the debouncers
  doDebounce();

  // Read the encoder switches
  doEncoderRead();

  // Update LEDs and serial print A, a, B, b
 // updateLEDs();

  //determine direction and update counter

  updateCounter();

 // make the encoder0Pos between 0 and 100 !:
  

  // encoder to stepper
steps = round(nettCounter * (2048.0/100.0)); //100 pulses/rev for the encoder, 2048 steps/rev for the stepper 
Serial.print("Steps: ");
    Serial.println(steps);
    delay(2000);
// Turn stepper
/*    
for (int i = 1; i <= steps; i++)
    {
    	myStepper.step(1);
    	delay(10);
    			
    }
*/     
    

} //loop

// my functions **************************************************
void doDebounce()
{
  debouncerA.update();
  debouncerB.update();
} //doDebounce

void doEncoderRead()
{
  valueA = debouncerA.read();
  valueB = debouncerB.read();
} //doEncoderRead

/*
void updateLEDs()
{

  if ( valueA == HIGH) {
    digitalWrite(LED_PINA, HIGH );
    //Serial.print("A");

  } 
  else {
    digitalWrite(LED_PINA, LOW );
    //Serial.print("a");
  }

  if ( valueB == HIGH) {
    digitalWrite(LED_PINB, HIGH );
    //Serial.println("B");
  } 
  else {
    digitalWrite(LED_PINB, LOW );
    //Serial.println("b");  
  }

} //updateLEDs
*/

void updateCounter()
{

  /*
  the possibilites are:
   
   AB: in a detent
   if just arrived, update counter, clear motiondetected
   otherwise do nothing
   Ab: start of CW or end of CCW
   if start, set CW bool and set motionDetected
   if at end (know becasue motionDetected already set), do nothing
   aB: start of CCW or end of CW
   if start, clear CW bool and set motionDetected
   if at end (know becasue motionDetected already set), do nothing
   ab: in middle of either CW or CCW, do nothing
   */

  if (valueA && valueB && motionDetected ) //in a detent and just arrived
  {
    if (CW)
    {
      grossCounter= grossCounter + 1;
      nettCounter= nettCounter + 1;
    }
    else //CCW
    {
      grossCounter= grossCounter + 1;
      nettCounter= nettCounter - 1;
    }
    motionDetected = false;
    //Serial.print("grossCounter: ");
    //Serial.println(grossCounter);
    //Serial.print("nettCounter: ");
    //Serial.println(nettCounter);
    
    fullRevolutions = nettCounter / cyclesPerRev; 
    surplusSteps = nettCounter % cyclesPerRev;
    
    Serial.print("Nett position: ");
    Serial.print(fullRevolutions);
    Serial.print(" + ");
    Serial.println(surplusSteps);
    Serial.println(" ");
    
    


  }

  if (valueA && !valueB && !motionDetected ) // just started CW
  {
    CW= true;
    motionDetected=true;
    Serial.println("CW");

  }

  if (!valueA && valueB && !motionDetected )  //just started CCW
  {
    CW= false;
    motionDetected=true;
    Serial.println("CCW");

  }
} //updateCounter

This is the datasheet of the stepper we use:

This is the breadboard layout:

The code I used is above.

Hmmm, I think I want to expriment just a bit more before I post this as a problem on the forum. Hang on.

PHPirates:
This is the breadboard layout:

I see a problem immediately!

You have indicated that your stepper and ULN2003 are taking power from the 5V terminal on the Arduino. You will be in for a world of pain!

You need a separate power supply for the stepper, one capable of providing the necessary current.


Vaclav:
You cannot read the encoder output at random in loop().

You certainly cannot. You have to code it properly.

Vaclav:
Unless you like to punish yourself and make sure you are REALLY reading NEW code.

Or in fact, writing proper code.

Vaclav:
For example if your read output A and than do delay(2000); you just missed 2 seconds of encoder data before reading output B.

And if you do that in an interrupt routine, you will probably lock up the processor, so that doesn't mean anything.

Vaclav:
Using ISR is the only way to appreciate Gray code properties ( only 1 bit changes ( error detect) and rotation direction can be detected) and to get reliable data from the encoder.

No, if you know how to code, you will have no problems. If you do not know how, no interrupts on earth will help you. :o

I do appreciate all of the comments, but somehow keep thinking about "you can leed a horse to the water but you cannot make him drink".
Now we are debouncing an optical encoder which generates Gray code , not to mention still doing Serial and delay in the loop.
I sure like to see the final outcome of this project.
Cheers Vaclav

Now we are debouncing an optical encoder

Wrong.

If you read the data sheet for this encoder posted early in the thread you will see that it is a mechanical encoder with well documented switch bounce. It is also manually operated.

Using polling with debounce is the best way to read this type of encoder.