motor encoder

int val;
int encoder0PinA = 3;//interrupt pin
int encoder0PinB = 4;
int encoder0Pos = 0;
int encoder0PinALast = LOW;
int n= LOW; // on Interrupt
int m = LOW;

void setup() {
n = digitalRead(encoder0PinA);
m = digitalRead(encoder0PinB);
pinMode (encoder0PinA,INPUT);
pinMode (encoder0PinB,INPUT);
attachInterrupt(1, doEncoder, CHANGE); // encoder pin on interrupt 1
Serial.begin (9600);
}

void loop() {}

void doEncoder() {

if (((m == HIGH) && (n == FALLING)) || ((n== RISING) && (m==LOW))) {
encoder0Pos--;}
else{ (((n== RISING) && (m==HIGH)) || ((n== FALLING) && (m==LOW)));
encoder0Pos++;
}
delay(10);
Serial.println (encoder0Pos);

encoder0PinALast = n;
}

i have pulse encoder in my motor, so theoretically this piece of code should give me the reading of clockwise and anticlockwise motor turning value as int....but i mm getting only 1, 2 and then nothing.

wuld really appreciate your suggestons :slight_smile:

One thing jumps out:
else{ (((n== RISING) && (m==HIGH)) || ((n== FALLING) && (m==LOW)));
Get rid of the ;

Do you want else or else if?

Hi
You LOOP has nothing in it so the program apart from setup does nothing.

Tom… :slight_smile:

i wanted a else if @LarryD, thanks

@TomGeorge if I take the serial print in the void loo() then it prints only 0 0 0 0 0 ....

int val;
int encoder0PinA = 3;
int encoder0PinB = 4;
int encoder0Pos = 0;
int encoder0PinALast = LOW;
int n= LOW; // on Interrupt
int m = LOW;

void setup() {
n = digitalRead(encoder0PinA);
m = digitalRead(encoder0PinB);
pinMode (encoder0PinA,INPUT);
pinMode (encoder0PinB,INPUT);
attachInterrupt(1, doEncoder, CHANGE); // encoder pin on interrupt 0 -

Serial.begin (9600);
}

void loop() {
delay(10);
Serial.println (encoder0Pos, DEC);
}

void doEncoder() {

if (((m == HIGH) && (n == (FALLING))) || ((n== RISING) && (m==LOW))) {
encoder0Pos--;}
else if (((n== RISING) && (m==HIGH)) || ((n== FALLING) && (m==LOW))) {
encoder0Pos++;
}

encoder0PinALast = n;
}

any suggestion pls?

HI

 void loop() {
 delay(10);
     Serial.println (encoder0Pos, DEC);
 }

Yes but you need to put the doEncoder function call in the loop as well before you serial.print.

Tom... :slight_smile:

TomGeorge:
HI

 void loop() {

delay(10);
    Serial.println (encoder0Pos, DEC);
}




Yes but you need to put the doEncoder function call in the loop as well before you serial.print.

http://www.arduino.cc/en/Reference/FunctionDeclaration

Tom... :)

No, doEncoder is an interrupt handler. Calling it in loop will completely defeat its purpose.

The biggest problem, which nobody has yet pointed out, is that he never actually READS the encoder inputs, except one time in setup(). doEncoder needs to do a digitalRead of both encoder inputs, or there is no point having an interrupt handler at all.

Regards,
Ray L.

Hi
Sorry you are right.

dasa-hs can you go back and edit your first post and place your code in code tags please.

Please use code tags.. See section 7 http://forum.arduino.cc/index.php/topic,148850.0.html

Tom.... :slight_smile:

RayLivingston:
No, doEncoder is an interrupt handler. Calling it in loop will completely defeat its purpose.

The biggest problem, which nobody has yet pointed out, is that he never actually READS the encoder inputs, except one time in setup(). doEncoder needs to do a digitalRead of both encoder inputs, or there is no point having an interrupt handler at all.

Regards,
Ray L.

int val;
int encoder0PinA = 3;//interrupt pin
int encoder0PinB = 4;
double encoder0Pos = 0;
int encoder0PinALast = LOW;
int n= LOW; // on Interrupt
int m = LOW;

void setup() {
n = digitalRead(encoder0PinA);
m = digitalRead(encoder0PinB);
pinMode (encoder0PinA,INPUT);
pinMode (encoder0PinB,INPUT);
attachInterrupt(1, doEncoder, CHANGE); // encoder pin on interrupt 1
Serial.begin (9600);
}

void doEncoder() {
n = digitalRead(encoder0PinA);
m = digitalRead(encoder0PinB);
if (((m == HIGH) && (n == FALLING)) || ((n== RISING) && (m==LOW))) {
encoder0Pos++;}
else {//if (((n== RISING) && (m==HIGH)) || ((n== FALLING) && (m==LOW))) { //if part is cmmented
encoder0Pos–;
}

encoder0PinALast = n;
}

void loop() {
delay(1000);
Serial.println (encoder0Pos);

}

but it gives me now only some incremental negative values. i still aint getting whats wrong.

digitalRead returns either HIGH or LOW, nothing else. RISING and FALLING have no meanng when comparing to the result of a digitalRead. If you want to know if an input has risen or fallen since the last read, you must determine that yourself, by comparing the new value to the previous value.

Regards,
Ray L.

Here's the approach I use, this sketch ought to work (compiles, untested, but its
based on working examples in various projects of mine).

#define enc_A  2
#define enc_B  3

void setup ()
{
  Serial.begin (115200) ;
  pinMode (enc_A, INPUT_PULLUP) ;
  pinMode (enc_B, INPUT_PULLUP) ;

  // setup interrupt handling.  Important to sample every change in both signals to avoid mis-accounting
  attachInterrupt (0, handle_enc, CHANGE) ;
  attachInterrupt (1, handle_enc, CHANGE) ;
  // alternatively can poll the handle_enc() function in loop() for slow encoders.
}


// read_pins () - sample the encoder pins and turn quadrature into phase value
// ie return 00, 01, 10, 11 as pins AB go 00, 01, 11, 10
// ideally this would read the two pins simultaneously (as via direct port access).
byte read_pins ()  
{
  return digitalRead (enc_A) ? 
    (digitalRead (enc_B) ? 2 : 3) : 
    (digitalRead (enc_B) ? 1 : 0) ;
}

volatile long phase = 0L ;  // The output counter
volatile int errors = 0 ;
volatile byte pin_state = read_pins () ;   // record previous pin state so can tell if it changed

void handle_enc ()
{
  byte new_pin_state = read_pins () ;
  if (new_pin_state == pin_state)   // this unlikely to happen unless noisy signals or handle_enc() polled
    return ;
  byte diff = (new_pin_state - pin_state) & 3 ;  // calculate phase difference, should be 0, +1 or -1
  pin_state = new_pin_state ;  // ready for next time

  if (diff == 1)
    phase -- ;
  else if (diff == 3)
    phase ++ ;
  else 
    errors ++ ;   // two pins changed together, count this as erroneous.
}

long read_phase ()
{
  noInterrupts () ;   // prevent ISR changing volatile phase while we read it.
  long my_phase = phase ;
  interrupts () ;
  return my_phase ;
}

int read_errors ()
{
  noInterrupts () ;   // prevent ISR changing volatile phase while we read it.
  int my_errors = errors ;
  interrupts () ;
  return my_errors ;
}

void loop ()
{
  Serial.print (read_phase ()) ; // report phase value every second for testing
  Serial.print (" ") ;
  Serial.println (read_errors ()) ;
  delay (1000) ;
}

return digitalRead (enc_A) ?
(digitalRead (enc_B) ? 2 : 3) :
(digitalRead (enc_B) ? 1 : 0) ;

I am not getting what does this "?" mean. moreover i am getting some odd characters after compiling.

It is the C/C++ "ternary operator"

some conditon ? code that happens if some condition is true : code that happens if some condition is false ;

The code posted in the previous reply is equivalent to

int val ;
if ( digitalRead(enc_A) == HIGH )
{
    if ( digitalRead( enc_B ) == HIGH )
    {
        val=2 ;
    }
    else
    {
        val = 3 '
    }
}
else
{
    if ( digitalRead( enc_B ) == HIGH )
    {
        val = 1 ;
     }
    else
    {
        val = 0 ;
     }
}
return val ;

As you can see, more concise.

There is probably some way of only including the actual code to call digitalRead(enc_B) once, instead of twice.

The name "ternary operator" I find bizarre - its not an operator, its a syntactic form.
The normal linquistic term for such things is "distfix" - distributed-fix as opposed to
prefix or postfix.

Anyway the a?b:c syntax is a condtional expression, the expression equivalent
of the if-statement, and very powerful - its easy to code !, && and || using
it:

  a ? false : true  // equivalent to !a
  a ? true : b  // equivalent to a || b
  a ? b : false  // equivalent to a && b

For some reason i find beginners shy away from this very handy and powerful
conditional expression, which yields unwieldy and clumsy nested if-statements.

To avoid the entire rising edge/falling edge hassle with incremental encoders, you could also go the hardware route with a couple flip flops as a quadrature decoder. Just an idea.

If you want correct counting you need a method that samples both signals on every change,
not just rising edges. Two flip flops cannot sample all the edges.

Not to hijack and turn a software thread into hardware talk but-
Assuming an ordinary 2-output incremental encoder, all you need in order to count correctly is basically to detect the leading edge of the trailing signal.

Not true.

[ Draw the state transition diagram if you don't believe me ]

Sure it is. In order to detect the 90 degree phase shift in the two otherwise identical digital signals, all you need to do is:

  1. wait for a pulse (high or low as applies to the particular circuit) on either line.
  2. look for the rising (or falling, if applicable) edge on the other line while the first one is still present.
    Two flip flops will do this admirably. It’s an ordinary bit of circuitry.

syntaxterror:
Sure it is. In order to detect the 90 degree phase shift in the two otherwise identical digital signals, all you need to do is:

  1. wait for a pulse (high or low as applies to the particular circuit) on either line.
  2. look for the rising (or falling, if applicable) edge on the other line while the first one is still present.
    Two flip flops will do this admirably. It's an ordinary bit of circuitry.

Which means you'll only be seeing half the total count - two counts per cycle, rather than four. To properly read a quadrature encoder, and get a count that is 4X the "line" rate, you must process ALL transitions, both rising and falling, on BOTH signals.

Regards,
Ray L.

Alright, let's just put this to bed and get back to the OP.

I get what you're saying. All I'm saying is quite simply that there's nothing incorrect in running a quadrature decoder with x1 multiplication. Sure, you can up your resolution by running the decoder in x2 or x4 mode instead (as per the proposed software solution). But is it necessary? Do you need the resolution? Do you want the extra interrupts? I can't say. I just proposed a simple alternative.

As for what's correct and incorrect - that's a matter of interpretation. If you have an encoder with 100 CPR, getting a count of 100 per full revolution is pretty much on the money, isn't it? Also, maybe you have an encoder with detents. Odds are it will produce a full quadrature cycle PER DETENT. So you NEED to decode x1.