Advice for getting a stable rpm reading

Hi,

I'm controlling the speed of a 12VDC motor using PWM via potentiometer input on my Arduino Uno. The motor is connected to an H bridge module. I'd like to read the motor rpm which is in the range of 0-6000 max. Accuracy at lower rpm (below 1000) is not that important for my application. The ultimate goal is to use this rpm input for PID to maintain the speed of the motor.

For the rpm reading i'm using the slotted LM393 IR speed sensor module to read a disc with a single slot.
I've added a 100nf - 250v capacitor between GND and the DO pin on the speed sensor module as advised on this page

If I set the speed of the motor via the pot, the rpm given in the serial monitor appears stable-ish, then jumps a couple of hundred rpm, remains there for a few more readings, only to then drop back to a similar reading to before. Does this stability need to be improved for using it with PID? If so, is there something which can be done to improve it? I've attached the code below.

The sketch i'm using is said to be an improved version of the rpm code found in the Arduino Playground. See the last post in this thread. I've deleted the associated code for the LCD, as I don't have one to use. I've then combined it with my basic speed control code for the motor.

Any help on improving the stability of the readings would be much appreciated.

Thanks

Michael

/*
  This code measures the RPM of a DC motor (or anything really) using a HALL sensor connected to PIN 21 and outputs data on a LCD display and also some debugging info on the serial
  if I ever want to create a nice chart based on the measurements.
  
  I made quite a bit of an effort to make it as precise as possible. The RPM is sampled with 4 microsecond resolution so it should be enough for any household DC motor.
*/


int potPin=A0; // Assign potentiometer to pin A0
int RPWM_Output=5; // Assign RPWM to Arduino PWM output pin 5
int LPWM_Output=6; // Assign LPWM to Arduino PWM output pin 6
int readPotValue; // Declare potentiometer read value variable
int writePotValue; // Variable to write motor speed
volatile byte rev;
volatile long dTime, timeold;
unsigned int rpm, freq;


void setup(){

  Serial.begin(9600);
  pinMode(potPin,INPUT); // Set potentiometer as an input
  pinMode(RPWM_Output,OUTPUT); // set RPWM as a PWM output
  pinMode(LPWM_Output,OUTPUT); // set LPWM as a PWM output
  attachInterrupt(0, revInterrupt, RISING);        //pin 2 is our interrupt
  dTime=0, rev=0, rpm=0, freq=0;
  timeold=0;
}

void loop(){

  readPotValue = analogRead(potPin); // Read potentiometer and put value in readPotValue
  writePotValue = map(readPotValue,0,1023,0,255); // Use map function to calculate writePotValue
  analogWrite(RPWM_Output, 0); // Write a PWM value of zero to LPWM (Pin6)
  analogWrite(LPWM_Output, writePotValue); // Write PWM signal to RPWM (Pin5)
  
  if (rev > 20)
   {
    cli();                           //disable interrupts while we're calculating
    if (dTime > 0)                   //check for timer overflow
    {
      rev-=1;                        //subtract one since the first revolution is not measured
      rpm=(60000000*rev)/(dTime);
      freq=rpm/60;
      Serial.print(rev);
      Serial.print(" ");
      Serial.print(rpm);            //a bit of serial for the debugging (not really needed at this point, perhaps one day for some graphs)
      Serial.print(" ");
      Serial.println(dTime);
      rev=0;
    }
    sei();                          //re-enable interrupts
   }
}

void revInterrupt (){
  if (rev == 0) 
  {
    timeold=micros();              //first measurement is unreliable since the interrupts were disabled
    rev++;
  }
  else 
  {
    dTime=(micros()-timeold);      //'micros()' is not incrementing while inside the interrupt so it should be safe like this right?
    rev++;
  }
}

I am controlling the speed of a small (N20 size) DC motor with a simple optical detector (QRE1113) which I guess behaves very like yours. My ISR code is as follows

void revDetectorISR() {
 isrRevMicros = micros();
 isrRevCount ++;
 newRevMicros = true;
}

and the main part of my code checks for newRevMicros being true (and then sets it to false).

In your code you have a long section with interrupts off. Don't do that. Just suspend the interrupts while you copy the data from the ISR variables to some working variables. This is from my program

noInterrupts();
    latestIsrMicros = isrRevMicros;
    revCount = isrRevCount;
    newRevMicros = false;
interrupts();

Also, you should not try to print while interrupts are disabled as printing requires interrupts.

I don't bother converting microseconds into RPM. My PID code just works on microsecs and my setpoint is a number of microsecs.

I find that I occasionally get a spurious interrupt that is very much shorter than normal and that is usually followed by the next interrupt at the "correct" time so I have some code that ignores the very short intervals and ensures that the value of micros() from the previous interrupt continues to be used. I don't have any comparator associated with my optical detector (I don't have space for one) and its absence might be the cause of the unwanted interrupts.

My system controls the speed to better than 1% over a range of speeds from about 40,000 µsecs per rev to 4,000µsecs per rev and regardless of load (within the capability of the motor).

...R

Thanks Robin that’s really useful.

I’ll attempt to adjust the code I posted - but I’m probably going to need some help!

Looking ahead, I don’t actually need to know the RPM. I’d just like to be able to adjust the speed of the motor with the potentiometer and then use the input from the speed sensor to maintain that set speed using PID.

Knowing that, would there be any preference for using one unit of measurement over the other?

Michael

mikeoz:
Knowing that, would there be any preference for using one unit of measurement over the other?

My preference is to use the system that requires fewest calculations so the Arduino’s time can be devoted to useful stuff :slight_smile:

…R

Makes sense. I've been rereading your post - several times!

I think I understand the code you posted and your other points. Could you explain how I'd include this in the code I posted? Just so I can understand how it works as a whole.

Thank you

Michael

Replace your ISR with mine

and then replace this

if (rev > 20)
   {
    cli();                           //disable interrupts while we're calculating
    if (dTime > 0)                   //check for timer overflow
    {
      rev-=1;                        //subtract one since the first revolution is not measured
      rpm=(60000000*rev)/(dTime);
      freq=rpm/60;
      Serial.print(rev);
      Serial.print(" ");
      Serial.print(rpm);            //a bit of serial for the debugging (not really needed at this point, perhaps one day for some graphs)
      Serial.print(" ");
      Serial.println(dTime);
      rev=0;
    }
    sei();                          //re-enable interrupts
   }

with

if (newRevMicros == true) {
   noInterrupts();
      latestIsrMicros = isrRevMicros;
      revCount = isrRevCount;
      newRevMicros = false;
   interrupts();
   // whatever other code you want to work on the new values
}

I have been fiddling around with my own program today and this is a complete version that holds my motor running rock steady at the speed set by the value in the variable targetMicros. It’s all a bit rough and ready, but it works.

// python-build-start
// action, upload
// board, arduino:avr:uno
// port, /dev/ttyACM0
// ide, 1.6.3
// python-build-end

// this demo program started life as LocSlvSoftStartCopy.h


//===============
    // Variables etc for Motor control

unsigned long setPoint = 25000; // pick a number
unsigned long stallMicros = 60000; // used when there are no pulses
unsigned long slowRevMicros = 30000; // new SoftStart - used to get going

long error;
int basePWM; // the ITerm
long errorPWM; // the PTerm - negative must be possible
long deltaInput;
int deltaPWM;

int PWMval = 0;
char currentDirn;


long curSpeed = 0;


    // variables used by the ISR
volatile unsigned long isrRevMicros = 0;
volatile unsigned long isrRevCount = 0;
volatile bool newRevMicros = true;
unsigned long debounceCountUL;
byte debounceCount = 128; // middle value

bool pseudoRevMicros = false;
bool stallDetected = false;     // new SoftStart
byte afterStallCount = 0;       // new SoftStart

unsigned long revCount;

unsigned long revMicros;
unsigned long prevRevMicros;
unsigned long prevIsrMicros;
unsigned long latestIsrMicros;

    // variables in lieu of RC input
unsigned long targetMicros = 25000;

byte PWMpinCW = 5;
byte PWMpinCCW = 6;
int kP = 32;
int kI = 32;
int kPScaleFactor = 10;
char dir = 'F';

//=================

void setup() {

    Serial.begin(500000);
    pinMode(11, INPUT_PULLUP);
    attachInterrupt(0, revDetectorISR, RISING);
    //~ attachInterrupt(0, revMicrosISR, RISING);


    pinMode(PWMpinCW, OUTPUT);
    pinMode(PWMpinCCW, OUTPUT);
    analogWrite(PWMpinCCW, 0);
    analogWrite(PWMpinCW, 0);

    Serial.println("MotorSoftStartOrig.ino");
    delay(1000);

}


//===========

void loop() { // was controlMotor()
    checkForStall();
    updatePWM();
    outputMotorPwr();
        // only call doRadiostuff() when it won't interfere with the ISR
    showDebugData(1007);
}



//================

void checkForStall() {
        // this produces a pseudo pulse time if there is no real one
    if (micros() - latestIsrMicros >= stallMicros) {
        pseudoRevMicros = true;
        stallDetected = true;       // new SoftStart
        afterStallCount = 0;
        //~ Serial.print("Stalled "); Serial.println()();
    }
    else if (stallDetected == true) {       // new SoftStart
        afterStallCount ++;
        if (afterStallCount > 8) {  // stay at low speed for a few revs
            stallDetected = false;
        }
    }
}


//============

void updatePWM() {

    if(newRevMicros == true or pseudoRevMicros == true) {

            // first get the setpoint
        setPoint = targetMicros;
        if (stallDetected == true) {        // new SoftStart
            setPoint = slowRevMicros;
        }

            // then update revMicros
        prevIsrMicros = latestIsrMicros;
        if (newRevMicros == true) {
            noInterrupts();
                latestIsrMicros = isrRevMicros;
                revCount = isrRevCount;
                newRevMicros = false;
            interrupts();
        }
        else { // it was pseudoRevMicros
            latestIsrMicros = micros();
            pseudoRevMicros = false;
        }
        revMicros = latestIsrMicros - prevIsrMicros;
        if (revMicros < (setPoint >> 2)) {
            latestIsrMicros = prevIsrMicros; // restore the value
            return; // ignore very short values of revMicros as aberrations
        }

            // now figure the new pwm value
        error = revMicros - setPoint;
            // this is essentially the regular PID approach

            // first the PTerm
        errorPWM = calcPWM(error, kP);
            // attenuate the negative values?
        if (errorPWM < 0) {
            errorPWM -= ((errorPWM * kPScaleFactor) >> 4);
        }

            // then the ITerm
        basePWM += calcPWM(error, kI);
        if (basePWM > 255) {
            basePWM = 255;
        }
        if (basePWM < 0) {
            basePWM = 0;
        }
    }
}

//====================

long calcPWM(long microsVal, byte kFactor) {
        // note that division of a negative number does funny things
    long pwmCalc = microsVal * kFactor;
    if (pwmCalc < 0) {
        pwmCalc = -(abs(pwmCalc) / setPoint);
    }
    else {
        pwmCalc = pwmCalc / setPoint;
    }
    return pwmCalc;
    }

//====================

void outputMotorPwr() {


    PWMval = basePWM + errorPWM;

    if (PWMval > 255) {
        PWMval = 255;
    }
    if (PWMval < 0) {
        PWMval = 0;
    }


        // finally, apply the PWM value
    if (dir == 'F') {
        analogWrite(PWMpinCCW, 0);
        analogWrite(PWMpinCW, (byte) PWMval);
    }
    else {
        analogWrite(PWMpinCW, 0);
        analogWrite(PWMpinCCW, (byte) PWMval);
    }
}

//=============

void showDebugData(int interval) {

    static unsigned long prevShowMillis;
    if ( millis() - prevShowMillis  >= interval ) {
        prevShowMillis = millis();

        Serial.print("\tSetpt "); Serial.print(setPoint);
        Serial.print("\trevMic "); Serial.print(revMicros);
        Serial.print("\terr "); Serial.print(error);
        Serial.print("\ttotPWM "); Serial.print(PWMval);
        Serial.print("\tbsPWM "); Serial.print(basePWM);
        Serial.print("\terPWM "); Serial.print(errorPWM);

        //~ Serial.println()();
        //~ Serial.print("baseRvCt "); Serial.print(stepBaseCount);
        //~ Serial.print("numSteps "); Serial.print(numMotorSteps);
        //~ Serial.print("revCount "); Serial.print(revCount);
        Serial.println();

    }
}

//===========

void revDetectorISR() {
    isrRevMicros = micros();
    isrRevCount ++;
    newRevMicros = true;
}

…R

That's fantastic, thanks a lot Robin.

Would I be able to use this as a base for my code - which ultimately I intend to use PID with?

First off, I'd like to just get it reading a stable measurement at a set speed. I've made a start on adjusting the code in my first post following your suggestions. Could you please help me with how I get a speed measurement once I copy the ISR data to the new variables? I'm still not entirely clear how it works after studying your complete code.

Many thanks

Michael

Code thus far:

int potPin=A0; // Assign potentiometer to pin A0
int RPWM_Output=5; // Assign RPWM to Arduino PWM output pin 5
int LPWM_Output=6; // Assign LPWM to Arduino PWM output pin 6
int readPotValue; // Declare potentiometer read value variable
int writePotValue; // Variable to write motor speed

// variables used by the ISR:
volatile unsigned long isrRevMicros = 0;
volatile unsigned long isrRevCount = 0;
volatile bool newRevMicros = true;
unsigned long latestIsrMicros;

unsigned long revCount;

void setup(){

  Serial.begin(500000);
  pinMode(potPin,INPUT); // Set potentiometer as an input
  pinMode(RPWM_Output,OUTPUT); // Set RPWM as a PWM output
  pinMode(LPWM_Output,OUTPUT); // Set LPWM as a PWM output
  attachInterrupt(0, revDetectorISR, RISING); // Pin 2 is the interrupt

}

void loop(){

  readPotValue = analogRead(potPin); // Read potentiometer and put value in readPotValue
  writePotValue = map(readPotValue,0,1023,0,255); // Use map function to calculate writePotValue
  analogWrite(RPWM_Output, 0); // Write a PWM value of zero to LPWM (Pin6)
  analogWrite(LPWM_Output, writePotValue); // Write PWM signal to RPWM (Pin5)

if (newRevMicros == true) {
   noInterrupts();                            // Suspend interrupts & copy data from ISR to working variables
      latestIsrMicros = isrRevMicros;
      revCount = isrRevCount;          
      newRevMicros = false;
   interrupts();
   // whatever other code you want to work on the new values
}

      Serial.println();              // Print speed measurement from a variable in serial monitor

}

void revDetectorISR() {
 isrRevMicros = micros();
 isrRevCount ++;
 newRevMicros = true;
}

mikeoz:
Would I be able to use this as a base for my code - which ultimately I intend to use PID with?

You may do anything you like with it. And the PID is already included if you look closely - well just PI actually, I don't think the D is needed for that application.

In my program the number in revMicros is the speed expressed in microseconds per revolution.

revMicros = latestIsrMicros - prevIsrMicros;

Simple maths will convert that to RPS or RPM if you want that.

...R

You won't believe this Robin, I actually managed to figure it out myself! :o

I've attached the updated code below. So it's essentially timing in microseconds one revolution each time?
I've had it connected to the motor and to me it seems very accurate at a set speed - judging by the serial monitor readings. At a random set speed via the potentiometer, it was reading 44944 - only the last 2 digits were occasionally fluctuating. Does that sound right to you?

Thanks,

Michael

Updated code:

int potPin=A0; // Assign potentiometer to pin A0
int RPWM_Output=5; // Assign RPWM to Arduino PWM output pin 5
int LPWM_Output=6; // Assign LPWM to Arduino PWM output pin 6
int readPotValue; // Declare potentiometer read value variable
int writePotValue; // Variable to write motor speed

// variables used by the ISR:
volatile unsigned long isrRevMicros = 0;
volatile unsigned long isrRevCount = 0;
volatile bool newRevMicros = true;

unsigned long latestIsrMicros;
unsigned long prevIsrMicros;
unsigned long revMicros;

unsigned long revCount;




void setup(){

  Serial.begin(500000);
  pinMode(potPin,INPUT); // Set potentiometer as an input
  pinMode(RPWM_Output,OUTPUT); // Set RPWM as a PWM output
  pinMode(LPWM_Output,OUTPUT); // Set LPWM as a PWM output
  attachInterrupt(0, revDetectorISR, RISING); // Pin 2 is the interrupt

}

void loop(){

  readPotValue = analogRead(potPin); // Read potentiometer and put value in readPotValue
  writePotValue = map(readPotValue,0,1023,0,255); // Use map function to calculate writePotValue
  analogWrite(RPWM_Output, 0); // Write a PWM value of zero to LPWM (Pin6)
  analogWrite(LPWM_Output, writePotValue); // Write PWM signal to RPWM (Pin5)

 prevIsrMicros = latestIsrMicros;
if (newRevMicros == true) {
   noInterrupts();                         // Suspend interrupts & copy data from ISR to working variables
      latestIsrMicros = isrRevMicros;
      revCount = isrRevCount;          
      newRevMicros = false;
   interrupts();
 revMicros = latestIsrMicros - prevIsrMicros;   
   // whatever other code you want to work on the new values
}

      Serial.println(revMicros);              // Print revMicros value in serial monitor

}

void revDetectorISR() {
 isrRevMicros = micros();
 isrRevCount ++;
 newRevMicros = true;
}

mikeoz:
Does that sound right to you?

Your code does not have any feedback speed control system so the stability may be limited. What happens if you press against the motor shaft to slow it down? (I’m assuming the motor is small and it is safe to do that).

45,000 µsecs is about 22 rps or 1300 rpm

If you have a multimeter that can read frequency you could use that to verify the Arduino readings by putting it across the sensor outputs.

I am hoping to have a much tidier version of the motor control program in a few days

…R

Hi Robin,

Yes, no feedback yet. I was just trying to get the speed measurement working first. Now that it is, would it just be a matter of copying the PID part of your code into my sketch - or would I be better to adjust your complete sketch?

The biggest difference from your example is that I wanted to set the speed of the motor with the potentiometer and also have that as the setpoint for the PID. So wherever the potentiometer is set to, the PID will try and maintain that speed, does that sound possible?

The motor is quite large, see below:

Thanks for your help.

Michael

mikeoz:
or would I be better to adjust your complete sketch?

I suggest you try my program as it is first - if it works for you then you have a benchmark for your other experiments

The biggest difference from your example is that I wanted to set the speed of the motor with the potentiometer and also have that as the setpoint for the PID. So wherever the potentiometer is set to, the PID will try and maintain that speed, does that sound possible?

It should be straightforward to make your potentiometer change the value in my targetMicros variable - but start with a fixed value.

Your link to the image does not work. See this Image Guide

...R

Thanks Robin,

I’ve tried re attaching the image.

So when I use your complete program at first, should I keep the code exactly as it is?

Michael

12v motor.jpg

mikeoz:
So when I use your complete program at first, should I keep the code exactly as it is?

Yes. You are my guinea pig :slight_smile:

Just make sure the targetMicros value is practical for your motor.

That motor is definitely larger than mine - which is about 12mm diameter.

...R

Fair enough :wink:

So if I make the targetMicros value say 45,000. I think that was about a quarter turn of the potentiometer.

Michael

Experiment. Experiment.

It's what the Arduino is designed for.

...R

Hi Robin,

I tried the complete unaltered code and it worked! Prior to testing it I calculated the max rpm of the motor to be 6000rpm so I left the targetMicros and setpoint at 25000. I imagine the RPM of your motor is a lot quicker than mine - so I think I'll have to adjust the stallMicros value as I'd like it to still work at lower speeds if possible - say as low as 500rpm. Is this as simple as just changing the value in the stallMicros variable at the top of the program?

Many thanks

Michael

mikeoz:
I tried the complete unaltered code and it worked!

Cool - that’s the first independent test

so I think I’ll have to adjust the stallMicros value as I’d like it to still work at lower speeds if possible - say as low as 500rpm. Is this as simple as just changing the value in the stallMicros variable at the top of the program?

That sounds right. The purpose of the stallMicros is to provide a test for a very long gap that suggests the motor has stopped. So set it to a number that is significantly slower than you want the motor to work.

You will probably also want to change slowRevMicros to a slower value.

Have fun.

…R

So I could alter stallMicros to something like 600,000 which would be 100rpm. Is the slowRevMicros just the soft start speed which begins after a stall is detected and can be any value?

Thanks,

Michael

mikeoz:
So I could alter stallMicros to something like 600,000 which would be 100rpm. Is the slowRevMicros just the soft start speed which begins after a stall is detected and can be any value?

Personally would set stallMicros at about 20% slower than the minimum speed at which the motor works and I would set slowRevMicros a bit higher than the minimum speed so you can be sure it will actually run.

But you need to experiment to see what suits your situation.

...R