RPM meter for rotary encoder

Hi everyone
iv'e been trying to make an RPM meter for a rotary encoder (YUMO A6B2-CWZ3E-1024), basically by looping for the length of a given time and dividing the number of interrupts within that time frame by the P/R of the encoder
It doesn't seem to work, however.

Here's the code-

unsigned long start;
//const byte encoderPinA = 2;//A pin 
//const byte encoderPinB = 4;//B pin 
volatile long pulse;
volatile bool pinB, pinA, dir;
const byte ppr = 2048, upDatesPerSec = 2;
const int fin = 1000 / upDatesPerSec;
const float konstant = 60.0 * upDatesPerSec / (ppr * 2);
int rpm;


void setup() {
  Serial.begin(1000000);
  attachInterrupt(0, readEncoder, CHANGE);
  //pinMode(encoderPinA,INPUT);
  //pinMode(encoderPinB,INPUT); 
}

void loop() {
  if(millis() - start > fin)
  {
    start = millis();
    rpm = pulse * konstant;
   Serial.println(pulse);
    Serial.println(rpm);

  }

}

void readEncoder()
{
  pinA = PIND&(0x04);
  pinB = PIND&(0x08);
  pulse += (pinA == pinB) ? +1 : -1 ;
}

I deliberately left in unused parts that are used when not doing direct port manipulation. It's based on a sketch i found in the forums, and seems to work for other people.
I have tried the suggested solutions in said thread, and searched for other ones as well, but did not manage to get it to work

Any help or ideas will be much appreciated, thanks

I moded your code based using a code from here:

try it out and let us know. hope it helps!

#define encoder0PinA  2
#define encoder0PinB  3

const byte ppr = 2048, upDatesPerSec = 2;
const unsigned long fin = 1000 / upDatesPerSec;
const float konstant = 60.0 * upDatesPerSec / (ppr * 2);

unsigned long oldtime, rpm;
unsigned int encoder0Pos_old = 0;
volatile unsigned int encoder0Pos = 0;

void setup() {
  pinMode(encoder0PinA, INPUT);
  pinMode(encoder0PinB, INPUT);

  // encoder pin on interrupt 0 (pin 2)
  attachInterrupt(0, doEncoderA, CHANGE);

  // encoder pin on interrupt 1 (pin 3)
  attachInterrupt(1, doEncoderB, CHANGE);

  Serial.begin (9600);

  oldtime = millis();
}

void loop() {
  unsigned int temp_pos;

  if (millis() - oldtime > fin)
  {
    oldtime = millis();
    temp_pos = encoder0Pos;
    rpm = (temp_pos - encoder0Pos_old) * konstant;
    Serial.print("pulse count: ");
    Serial.print(temp_pos - encoder0Pos_old, DEC);
    Serial.print(", rpm: ");
    Serial.println(rpm);
    encoder0Pos_old = temp_pos;
  }
}

void doEncoderA() {
  // look for a low-to-high on channel A
  if (digitalRead(encoder0PinA) == HIGH) {

    // check channel B to see which way encoder is turning
    if (digitalRead(encoder0PinB) == LOW) {
      encoder0Pos = encoder0Pos + 1;         // CW
    }
    else {
      encoder0Pos = encoder0Pos - 1;         // CCW
    }
  }

  else   // must be a high-to-low edge on channel A
  {
    // check channel B to see which way encoder is turning
    if (digitalRead(encoder0PinB) == HIGH) {
      encoder0Pos = encoder0Pos + 1;          // CW
    }
    else {
      encoder0Pos = encoder0Pos - 1;          // CCW
    }
  }
  Serial.println (encoder0Pos, DEC);
  // use for debugging - remember to comment out
}

void doEncoderB() {
  // look for a low-to-high on channel B
  if (digitalRead(encoder0PinB) == HIGH) {

    // check channel A to see which way encoder is turning
    if (digitalRead(encoder0PinA) == HIGH) {
      encoder0Pos = encoder0Pos + 1;         // CW
    }
    else {
      encoder0Pos = encoder0Pos - 1;         // CCW
    }
  }

  // Look for a high-to-low on channel B

  else {
    // check channel B to see which way encoder is turning
    if (digitalRead(encoder0PinA) == LOW) {
      encoder0Pos = encoder0Pos + 1;          // CW
    }
    else {
      encoder0Pos = encoder0Pos - 1;          // CCW
    }
  }
}

Nice one sherzaad (that doesn't mean I've tested your work).

I had a look to see if I could help as I'm currently working with quadrature signals, but I gave up when I saw this line:

pulse += (pinA == pinB) ? +1 : -1 ;

I'm sure I could learn something if I decoded that. But maybe just how to write obscure code :slight_smile:

More important, we don't know if Pika007 needs to know the direction of spin. If not we can do something much simpler (and easier to understand).

sherzaad:
I moded your code based using a code from here:

try it out and let us know. hope it helps!

Thank you for your help, that is some work there
Unfortunately, it does not seem to work. I'm trying to debug it myself, stays on zeros the entire time.
One thing that might be a hint is the warning when i try to upload it-

warning: division by zero [-Wdiv-by-zero]

const float konstant = 60.0 * upDatesPerSec / (ppr * 2);

It does upload , just doesn't work.

jimmer-
i should've figured it might cause confusion, i used a ternary operator, it's a good way to operate a "yes or no" situation in a single line without an if
In this case- "pulse=pulse plus [if pin a= pin b, +1. Otherwise, -1]"

I need just a single direction, and it's ok to even count only the rising edges to get 1024 pings/revolution, i really don't need 2048, it's overaccurate

strange indeed, I would have expected at LEAST the pulse count to return you some value at the encoder is rotated. I have used the actual encoder code from that link before

just noticed something from your OP

"const byte ppr = 2048, upDatesPerSec = 2;"

max value a "byte" variable can only be 255.

try changing ppr it to uint16_t

That seems to fix the problem, Thanks!
but something new popped up-
for some reason, i'm getting more 3000 pings for one revolution, probably 4000, something is being counted twice. Trying to find what right now, will report when (if) i find it.

After that i'll change the code to use direct port manipulation and share it, this encoder is supposed to be spinning up to 3000rpm, so reading FAST is important....

I guessed it was some kind of compressed if then else statement :slight_smile:

I think the problem with your original is that that method (comparing A== B ) needs a reversed answer depending on which channel has changed. You could do that by two separate interrupts or adding an if then else.

I've been using a different method that's not as neat as yours, gonna have a play with it now.

iv'e been trying to make an RPM meter for a rotary encoder (YUMO A6B2-CWZ3E-1024)

For an rpm meter the resolution of this encoder is not required. According to the part number, there should be a Z signal output once per revolution. Why not use it?

I agree 1024ppr is a lot of resolution for an RPM meter. 1ppr might be too low though.

I've had a play with the code, the below works for me. I did not check the maths on the RPM calc.

Given the resolution you probably need, I would change to RISING instead of CHANGE.

This is still proper quadrature with direction. You could use only 1 channel. (maybe read both and compare counts as a fault check).

#include <util/atomic.h>

unsigned long start;

volatile long pulse, snap_pulse;
volatile bool pinB, pinA, dir;
const int ppr = 96;
const int upDatesPerSec = 4;
const int fin = 1000/upDatesPerSec;
const float konstant = (60 * upDatesPerSec) / (ppr * 2);
int rpm;

void setup() {
Serial.begin(1000000);
pinMode(0, INPUT_PULLUP);
pinMode(1, INPUT_PULLUP);
attachInterrupt(digitalPinToInterrupt(0), readEncoderA, CHANGE);
attachInterrupt(digitalPinToInterrupt(1), readEncoderB, CHANGE);
}

void loop() {
if(millis() - start > fin)
{
start = millis();
ATOMIC_BLOCK(ATOMIC_FORCEON) { // protect from interrupts
snap_pulse = pulse;
pulse=0;
}
rpm = snap_pulse * konstant;
Serial.println(snap_pulse);
Serial.println(rpm);
}

}

void readEncoderA()
{
pinA = PIND&(0x04);
pinB = PIND&(0x08);
pulse += (pinA == pinB) ? +1 : -1 ;
}
void readEncoderB()
{
pinA = PIND&(0x04);
pinB = PIND&(0x08);
pulse += (pinA == pinB) ? -1 : +1 ;
}

I agree 1024ppr is a lot of resolution for an RPM meter. 1ppr might be too low though.

this encoder is supposed to be spinning up to 3000rpm,

I need just a single direction

At 50 rev/second. I would like to hear more about the application before deciding that one pulse per revolution is not enough resolution.

With interrupt set to CHANGE you're getting over 100000 pulses per second at 3000 RPM, try it with FALLING instead (51200 PPS), still way too many IMHO. And, if you only want RPM, only read 1 channel, (A or B).

Here's a one channel version. I tested it with my 300ppr encoder to 5000rpm which is halfway to your pulse rate.

Your max pulse rate is about every 20us. This interrupt takes about 5us so it's safe.

The short protected bit of code is about 1 us, so we won't miss any pulses.

And you can take as long as you like doing things with snap_pulse as it's a snapshot of the value that won't change while you are looking at it :slight_smile:

Note unsigned int means max 65000 pulses per sample period. So don't go over 1 second sampling period.

unsigned long start;

volatile unsigned int pulse, snap_pulse;
const int ppr = 300; // 1 channel pulses per rev. 1 pulse = 1 rising edge
const int upDatesPerSec = 1;
const int fin = 1000/upDatesPerSec;
const float konstant = (60.0 * upDatesPerSec) / ppr;
int rpm;

void setup() {
Serial.begin(1000000);
pinMode(0, INPUT_PULLUP);
attachInterrupt(digitalPinToInterrupt(0), readEncoderA, RISING);
}

void loop() {
if(millis() - start > fin)
{
start = millis();
noInterrupts ();// temporary protect pulse from interrupts
snap_pulse = pulse;
pulse=0;
interrupts (); // turn back on. Up to 1 stored will be processed now.
rpm = snap_pulse * konstant;
Serial.println(" pulses: " + String(snap_pulse) + " rpm: "+ String(rpm));
}

}

void readEncoderA()
{
pulse += 1;
}

Excuse me for my late reply, real life was knocking at my door for a couple of days. merry christmas everyone.
the 1ppm point is very valid- i'll need to also get a real live angular position i can plot, so the crazy 2048p/r is definitely too high, but i'd like to at least be able to pinpoint at a 1 degree resolution (so around 360p/r, which isn't too high)

I'm still working on the code to get the speed as high as possible, will report back with results and what i ended up using

jimmer- i don't see which pin i need to connect my interrupting signal wire to, could you enlighten me? since i'm using an encoder that outputs up to 9v, connecting it to 0 seems to be a bad idea