Speed of motor with arduino and encoder

Ehi there! ArduiNoob here.

I am trying to read the speed of a motor using an Arduino Uno and a quadrature encoder.

I have tried several ways but the easiest I found was to use an interrupt to have a counter+ when the A pulse of the encoder rises (I forget about the B for now).
In order to get a time difference, I allow interrupts and then stop them after counter = e.g. 300 and calculate speed.

This seems to work but only if the calculation is done averaging many pulses. For counter = 300 the speed seems stable while for counter = 30 the speed goes up and down (attached screenshot).
I think it comes from the fact that I am using a “software” counter instead of a “hardware” counter.
I have heard this could be done using the Uno using some kind of register counter but I have zero experience and I have not been able even to read the codes I found that use the registers…

I was wondering if anyone has a quick solution or could give me guidance on where to start learning to get to the bottom of this. Or maybe some of you see that this is not doable with an Arduino…

Thanks in advance!
Lorenzo

Please refer to the code below.

More info:
speed of motor ~0.5 round per second
direction: only one way
encoder res: 12500ppr

#define outA 2 //A+ 

  float vel = 0;
  volatile long counter = 0;
  unsigned long staT = 0;
  unsigned long stoT = 0;



void setup() {
//define pins 
  pinMode(outA, INPUT_PULLUP);  

//set serial communication speed
  Serial.begin(9600);

  // encoder A channel on interrupt 0 (Arduino's pin 2)
  attachInterrupt(0, doEncoderA, RISING);

}

void loop() {
  staT = 0;
  stoT = 0;
  counter = 0;

  
  interrupts();  //allow interrupts
staT = micros(); //start counter


while (counter < 100) { //loop until 
stoT = micros();
}
  noInterrupts(); //no more interrupts

//calculate velocity
float vel = counter*1000000/(stoT-staT);


  Serial.println(vel);
}


void doEncoderA()
{

counter = counter+1;

}

Which encoder do you have? i.e. how many pulses per revolution?

Some problems:

The vel calculation must be performed with floating point arithmetic. Something like

float vel = counter*1000000.0/(stoT-staT);

vel is declared twice. The global (first) declaration is hidden by the local definition in loop().

The long counter must be declared volatile and read with interrupts off, what in turn may cause lost pulses. A volatile byte counter does not have such a problem, but limits the pulse count to 255.

wildbill:
Which encoder do you have? i.e. how many pulses per revolution?

it's 12500 ppr. I just noticed that the specs I linked were only saying 1-12500. Sorry!

DrDiettrich:
Some problems:

The vel calculation must be performed with floating point arithmetic. Something like

float vel = counter*1000000.0/(stoT-staT);

vel is declared twice. The global (first) declaration is hidden by the local definition in loop().

The long counter must be declared volatile and read with interrupts off, what in turn may cause lost pulses. A volatile byte counter does not have such a problem, but limits the pulse count to 255.

Thanks! It seems the main issue is not yet solved though. =(

Can somebody explain me how a 12500 ppr encoder is constructed?

DrDiettrich:
Can somebody explain me how a 12500 ppr encoder is constructed?

http://www.creative-robotics.com/quadrature-intro

this is the explanation that made the most sense to me.
there is a wheel with many holes and a signal that goes through the holes and triggers a pulse.
The more holes you have the more ppr your encoder can provide and this is its resolution.

The two outputs are these two streams of pulses A and B which I want to read from arduino.

My issue is that my code seems to work when I average >100 pulses but I have strange behavior when I try to check the speed using only 1-10 pulses.

llea:
What are Quadrature Encoders | Creative Robotics

this is the explanation that made the most sense to me.
there is a wheel with many holes and a signal that goes through the holes and triggers a pulse.
The more holes you have the more ppr your encoder can provide and this is its resolution.

The two outputs are these two streams of pulses A and B which I want to read from arduino.

My issue is that my code seems to work when I average >100 pulses but I have strange behavior when I try to check the speed using only 1-10 pulses.

I think you will find Dr. Diettrich might be questioning your encoders pulses per revolution.
It was the second thing that I thought rather unusual, first being why quadrature.

12500 Pulse Per Round and 0.5 Round per Second ----> 6250 pulse per second. You trigger an interrupt ONLY on RISING edges, so you discard 3125 pulse per second. It remains 3215 interrupts to handle per second ----> the duration between 2 pulses is 320 us = only 5 clock cycles for a 16 MHz uc : IMO that is the issue because an attachinterrupt() requires much more.

FYI, a arduino DUE (84 MHz) embeddes 2 hardware quadrature decoder with a capabitlity of up to 42 million edges per second.

ard_newbie:
12500 Pulse Per Round and 0.5 Round per Second ----> 6250 pulse per second. You trigger an interrupt ONLY on RISING edges, so you discard 3125 pulse per second. It remains 3215 interrupts to handle per second ----> the duration between 2 pulses is 320 us = only 5 clock cycles for a 16 MHz uc : IMO that is the issue because an attachinterrupt() requires much more.

FYI, a arduino DUE (84 MHz) embeddes 2 hardware quadrature decoder with a capabitlity of up to 42 million edges per second.

so you suggest to try directly with a DUE?

Also, if I want to trigger on both edges, am I correct to use CHANGE instead of RISING?
As I understand this will make it even worse if I keep using a UNO.

And also, 5 cycles should still be ok if I use the full 16MHz right? Is it possible to use hardware counters with Timer1 as in this other discussion to get the full processor clock?

Thanks for the explanation!

bluejets:
I think you will find Dr. Diettrich might be questioning your encoders pulses per revolution.
It was the second thing that I thought rather unusual, first being why quadrature.

I am not sure I follow...

DrDiettrich:
Can somebody explain me how a 12500 ppr encoder is constructed?

Shouldn't be too hard with a gearbox. The data sheet as linked by OP indeed says

Resolution: 1 to 12.500 ppr (pulses per revolution)

This encoder apparently has a great number of versions - with different resolutions. OP apparently has the 12,500 ppr version, which at 0.5 rotations per second is some 6,250 pulses per second.

ard_newbie:
the duration between 2 pulses is 320 us = only 5 clock cycles for a 16 MHz uc

I'm afraid you lost a zero or three. 320 us is 5,120 clock ticks at 16 MHz.

Thanks wvmarle.
Do you have any hint on what I might be doing wrong? I still think my readout technique might be too slow (as pointed out previously interrupts are much slower than the 16MHz).
If you agree this might the bottle neck, do you know if there is an easy way to get it done at 16MHz using the hardware Timer1?

Speed should NOT be an issue, 3,000 pulses a second is not a serious load for an Arduino. #7 got their math wrong.

llea:

void loop() {

staT = 0;
  stoT = 0;
  counter = 0;
  interrupts();  //allow interrupts
  staT = micros(); //start counter
  while (counter < 100) { //loop until
    stoT = micros();
  }
  noInterrupts(); //no more interrupts

//calculate velocity
  float vel = counter * 1000000 / (stoT - staT);
  Serial.println(vel);
}

(code reformatted for readability)

Several issues and potential issues.

  1. interrupts are off most of the time - including when loop() runs over and you try to print, which needs interrupts. You will also lose interrupts this way. Only switch off interrupts when you must, and switch them back on as soon as you can.
  2. the while() loop checks for the value of a volatile long. That’s a potential issue: what if counter gets written to while it’s being checked by the while() loop? I don’t know enough of the internal workings of the AVR processor and the C++ compiler to know if that’s really an issue. As you check for <100 you can declare counter a byte, and there will be no problem.
  3. no need to read micros() every time you run that while loop. Just do it when the loop has finished.

Normally to read/use a volatile long variable you do like this:

noInterrupts();
long counterCopy = counter;
interrupts();

And now you can do your calculations or checks on counterCopy which is safe.

Speed should be no problem: I’m currently working on a project using an encoder with 8000ppr. Initially, we were using a 16MHz nano, which I think was handling it fine.

We’ve upgraded to a Due, but it seems that the issues we were seeing were electrical, rather than any CPU inadequacy. With less than half the interrupts, your Uno will be easily up to the task.

Rather than counting pulses within a specific time interval I suggest you get the ISR to record the value of micros() after (say) every revolution with code something like this

void doEncoderA() {
    static unsigned long pulseCount = 0;
    pulseCount ++;
    if (pulseCount >= 12500) { // or whatever number of pulses seems appropriate
       newISRmicros = micros();
       newRevData = true;
       pulseCount = 0;
    }
}

and then the code in loop() can check for a new reading like this

if (newRevData == true) {
   prevRevMicros = revMicros;  // save the current value
   noInterrupts();
      revMicros = newISRmicros;
      newRevData = false;
   interrupts();
}

The time for that number of pulses will be revMicros - prevRevMicros

...R

llea:
this is the explanation that made the most sense to me.
there is a wheel with many holes and a signal that goes through the holes and triggers a pulse.

So your wheel has 12500 holes in it? With 1mm holes this makes a wheel circumference of 25 m, a diameter of about 7m. That's why I cannot believe in as many ppm as you posted. Do you have a link to the data sheet of your encoder?

wildbill:
Speed should be no problem: I’m currently working on a project using an encoder with 8000ppr. Initially, we were using a 16MHz nano, which I think was handling it fine.

We’ve upgraded to a Due, but it seems that the issues we were seeing were electrical, rather than any CPU inadequacy. With less than half the interrupts, your Uno will be easily up to the task.

thank you wildbill and wvmarle. You are giving me hope!

I have updated the code using the suggestions of wvmarle (see code below). I also attach a screenshot of the speed readout that still look strange to me. wildbill do you know what next I can improve?

Robin thanks for your suggestion. My goal is to get to a point in which I can have a speed measurement after every pulse from the encoder. If I only measure once per revolution then I cannot see variations within the rotation. This is the same as in my code choosing 12500 counts.

DrDiettrich I do not know what the aim of your comments is. I linked the specs in the first post and I did not produce the encoder myself. Companies make them and people buy them. I can only say mine is not 7m in diameter. I could dive into deeper explanation on how things are made and why it is not 7m diameter, but I honestly do not see why I should.

Thanks guys!

PS: I still have not changed RISING to CHANGE because I do not think it matters at the moment and it only increases the number of things the arduino has to see.

 #define outA 2 //A+
  float vel = 0;
  volatile byte counter = 0;
  unsigned long staT = 0;
  unsigned long stoT = 0;



void setup() {
  //define pins 
  pinMode(outA, INPUT_PULLUP);  

  //set serial communication speed
  Serial.begin(115200);

  // encoder A channel on interrupt 0 (Arduino's pin 2)
  attachInterrupt(0, doEncoderA, RISING);
  interrupts();  //allow interrupts
}

void loop() {
  staT = 0;
  stoT = 0;
  counter = 0;
  staT = micros(); //start counter
  while (counter < 100) { //loop until
  }
  stoT = micros();
  float vel = counter * 1000000 / (stoT - staT);     //calculate velocity
  Serial.println(vel);
}

//counter 250 results in ~100ms integr time and a variation of ~5


void doEncoderA()  {
counter = counter+1;
}

You have ignored the advice in reply #11 regarding making a copy of your counter variable with interrupts disabled.

DrDiettrich Here's the encoder I'm working with: datasheet. It's nominally 2000ppr, but triggering on both A and B pulses on rising and falling edges gives 8000. It's hard to tell from the pictures, but the spec says it's 40mm in diameter. I've no idea what's inside it though.

wildbill:
You have ignored the advice in reply #11 regarding making a copy of your counter variable with interrupts disabled.

DrDiettrich Here's the encoder I'm working with: datasheet. It's nominally 2000ppr, but triggering on both A and B pulses on rising and falling edges gives 8000. It's hard to tell from the pictures, but the spec says it's 40mm in diameter. I've no idea what's inside it though.

I understood the copy of the counter was used when using a volatile long. As suggested I moved instead to volatile byte. As far as understood this should not have the same issue. Did I misunderstand?

Using a byte should be ok. Time's still passing though and there may be an additional interrupt after you store micros. Personally, I'd still stop interrupts while I took a copy. YMMV.

wildbill:
Using a byte should be ok. Time's still passing though and there may be an additional interrupt after you store micros. Personally, I'd still stop interrupts while I took a copy. YMMV.

Thanks wildbill. I added the copy and now all checks and calculations are done on a long copy of counter. The result seems the same and I still see oscillations as in the capture I attached before.
Did you encounter this before?
In your project are you measuring the speed after every pulse?