Controlling a dc motor via pid and rpm sensor.

I don’t know why I am having so much trouble with this.
I am trying to control a dc brushed motor via an electronic speed control with the pwm output from pin D6 on a UNO.

Rpm feedback is via an optical sensor to pin D2.

For now, the Setpoint for the PID is hard coded but will be variable eventually.

I have the following rpm script which works perfectly.

volatile float time = 0;
volatile float time_last = 0;
volatile int rpm_array[5] = {0,0,0,0,0};

void setup()
{
  //Digital Pin 2 Set As An Interrupt
 attachInterrupt(0, fan_interrupt, FALLING);

 Serial.begin(9600);
 analogWrite(6, 60);
}

//Main Loop To Calculate RPM and Update LCD Display
void loop()
{
  int rpm = 0;

 
 
  
  while(1){    

     //Slow Down The LCD Display Updates
  delay(400);
  
   //Update The RPM
  if(time > 0)
  {
    //5 Sample Moving Average To Smooth Out The Data
      rpm_array[0] = rpm_array[1];
      rpm_array[1] = rpm_array[2];
      rpm_array[2] = rpm_array[3];
      rpm_array[3] = rpm_array[4];
      rpm_array[4] = 60*(1000000/(time*36));    
    //Last 5 Average RPM Counts Eqauls....
      rpm = (rpm_array[0] + rpm_array[1] + rpm_array[2] + rpm_array[3] + rpm_array[4]) / 5;
       Serial.println(rpm);
  }
 
 }
}

//Capture The IR Break-Beam Interrupt
void fan_interrupt()
{
   time = (micros() - time_last); 
   time_last = micros();
}

I have tried all sorts of ways of controlling the motor using feedback from the rpm but success eludes me. Using the PID_Basic example, I tried to merge this with the above sketch and came up with this.

#include <PID_v1.h>

//Define Variables we'll be connecting to
double Setpoint, Input, Output;

//Specify the links and initial tuning parameters
PID myPID(&Input, &Output, &Setpoint, 2, 5, 1, DIRECT);

//For tacho.
volatile float time = 0;
volatile float time_last = 0;
volatile int rpm_array[5] = {0, 0, 0, 0, 0};
int rpm = 0;

void setup()
{
  //Digital Pin 2 Set As An Interrupt for tacho.
  attachInterrupt(0, sensorInterrupt, FALLING);


  //initialize the variables we're linked to
  Input = rpm;
  Setpoint = 30;

  //turn the PID on
  myPID.SetMode(AUTOMATIC);


  Serial.begin(9600);
}

void loop()
{
  getSetPoint();
  getStartStop();
  switchOnOff();
  readRpm();
  sensorInterrupt();
  sendOutputToEsc();
  debug();
}

void getSetPoint()
{
  Setpoint = 30;

}

void getStartStop()
{

}

void switchOnOff()
{

}

void readRpm()
{
  rpm = 0;
  
  while (1) {

    //Slow Down The LCD Display Updates
    delay(40);

    //Update The RPM
    if (time > 0)
    {
      //5 Sample Moving Average To Smooth Out The Data
      rpm_array[0] = rpm_array[1];
      rpm_array[1] = rpm_array[2];
      rpm_array[2] = rpm_array[3];
      rpm_array[3] = rpm_array[4];
      rpm_array[4] = 60 * (1000000 / (time * 36));
      //Last 5 Average RPM Counts Eqauls....
      rpm = (rpm_array[0] + rpm_array[1] + rpm_array[2] + rpm_array[3] + rpm_array[4]) / 5;
     
    }

  }
}

void sensorInterrupt()
{
  time = (micros() - time_last);
  time_last = micros();
}

void sendOutputToEsc()
{
  Input = rpm;
  myPID.Compute();
  analogWrite(6, Output);
}

void debug()
{
Serial.print("rpm ");
Serial.print(rpm);
Serial.print("  Input ");
Serial.print(Input);
Serial.print("  Setpoint ");
Serial.print(Setpoint);
Serial.print("  Output ");
Serial.print(Output);
Serial.print("  A0 ");
Serial.println(analogRead(0));
}

The result is nothing. No serial output, no movement on the motor, nothing. I assume the sketch is stuck in a loop and I’m guessing it is the sensorInterrupt that’s causing it. (Note: GUESSING).
I do not know how to move forward from here.

while (1) {

You never break out of the while loop. What happens if you just update the new rpm value in loop.

You should be disabling interrupts, making a copy of the value of time from the ISR, and then re-enable interrupts so that the value it cannot change while being read.

Try

#include <PID_v1.h>

//Define Variables we'll be connecting to
double Setpoint, Input, Output;

//Specify the links and initial tuning parameters
PID myPID(&Input, &Output, &Setpoint, 2, 5, 1, DIRECT);

//For tacho.
volatile float time = 0;
volatile float time_last = 0;
volatile int rpm_array[5] = {0, 0, 0, 0, 0};
int rpm = 0;

void setup()
{
  //Digital Pin 2 Set As An Interrupt for tacho.
  attachInterrupt(0, sensorInterrupt, FALLING);


  //initialize the variables we're linked to
  Input = rpm;
  Setpoint = 30;

  //turn the PID on
  myPID.SetMode(AUTOMATIC);


  Serial.begin(9600);
}

void loop()
{
  getSetPoint();
  getStartStop();
  switchOnOff();
  readRpm();
  sensorInterrupt();
  sendOutputToEsc();
  debug();
}

void getSetPoint()
{
  Setpoint = 30;

}

void getStartStop()
{

}

void switchOnOff()
{

}

void readRpm()
{
  rpm = 0;
  
 // while (1) { eliminate while loop

    //Slow Down The LCD Display Updates
    delay(40);

    //Update The RPM
      noInterrupts();
      unsigned long copyTime = time:
      interrupts();
    if (copyTime > 0)
    {
      //5 Sample Moving Average To Smooth Out The Data
      rpm_array[0] = rpm_array[1];
      rpm_array[1] = rpm_array[2];
      rpm_array[2] = rpm_array[3];
      rpm_array[3] = rpm_array[4];
      //rpm_array[4] = 60 * (1000000 / (time * 36));
      rpm_array[4] = 60 * (1000000 / (copyTime * 36));
      //Last 5 Average RPM Counts Eqauls....
      rpm = (rpm_array[0] + rpm_array[1] + rpm_array[2] + rpm_array[3] + rpm_array[4]) / 5;
     
    }

 // }
}

void sensorInterrupt()
{
  time = (micros() - time_last);
  time_last = micros();
}

void sendOutputToEsc()
{
  Input = rpm;
  myPID.Compute();
  analogWrite(6, Output);
}

void debug()
{
Serial.print("rpm ");
Serial.print(rpm);
Serial.print("  Input ");
Serial.print(Input);
Serial.print("  Setpoint ");
Serial.print(Setpoint);
Serial.print("  Output ");
Serial.print(Output);
Serial.print("  A0 ");
Serial.println(analogRead(0));
}

I’m giving you a lot of information I hope it helps for your situation.

PID_V1 has a problem but for most it isn’t a huge issue. But in your case I see it affecting your results and you should know why and how I fixed it for my balancing robot.

concerning PID_V1
First thing to note is the default sample time is 100 milliseconds. so no matter how fast you can sample your input for minute changes, if you don’t change this then you will only calculate every 100 milliseconds to change your motor PWM value.

 SampleTime = 100;							//default Controller Sample Time is 0.1 seconds

Second thing to note is that this sample time is exactly what you set it. you can’t be faster or you will skip that sample and return false! if you are too slow the PID math is off.

bool PID::Compute()
{
   if(!inAuto) return false;
   unsigned long now = millis();
   unsigned long timeChange = (now - lastTime);
   if(timeChange>=SampleTime)
   {
      /*Compute all the working error variables*/
      /*Compute PID Output*/
      /*Remember some variables for next time*/
	 return true;
   }
   else return false;
}

Note that Ki and Kd are altered to include the fixed sample time

/* SetSampleTime(...) *********************************************************
 * sets the period, in Milliseconds, at which the calculation is performed	
 ******************************************************************************/
void PID::SetSampleTime(int NewSampleTime)
{
   if (NewSampleTime > 0)
   {
      double ratio  = (double)NewSampleTime
                      / (double)SampleTime;
      ki *= ratio;
      kd /= ratio;
      SampleTime = (unsigned long)NewSampleTime;
   }
}

your problem could be similar to mine you input isn’t always triggered at the same exact time so you are going to skip samples or wait too long and the new sample is stale!

With Traditional PID the time is not fixed it is measured and used in the calculation as a differential. But for temperature it is easily read so having a fixed time is fine but reading a sample from an RPM sensor can be an unknown duration.

If you would like to use a PID routine that doesn’t require fixed times make these changes to PID_V1 note that with this change SetSampleTime is now a minimum sample time and for your case it can be set to 1 millisecond as the minimum.

/* Compute() **********************************************************************
*     This, as they say, is where the magic happens.  this function should be called
*   every time "void loop()" executes.  the function will decide for itself whether a new
*   pid Output needs to be computed.  returns true when the output is computed,
*   false when nothing has been done.
**********************************************************************************/ 

bool PID::Compute()
{
	static int SkipCtr;
	unsigned long now = millis();
	unsigned long timeChange = (now - lastTime);
	if(!inAuto){
		SkipCtr = 21; // Soft Start Skipps ki and kd for 20 cycles
		integral = 0;
		pre_error = 0;
		return false;
	}
	if(timeChange>=SampleTime) // Sample time is now Minimum Sample Time 
	{
		double DTerm = 0;
		double derivative = 0;
		// kp -  proportional gain
		// ki -  Integral gain
		// kd -  derivative gain
		// dt -  loop interval time
		// outMax - maximum value of manipulated variable
		// outMin - minimum value of manipulated variable

		//Compute all the working error variables
		double input = *myInput;

		// Calculate error
		double error = *mySetpoint - input;

		// Proportional term
		double PTerm = kp * error;
		SkipCtr--;
		if(SkipCtr <= 1){
			SkipCtr= 1;
			// Integral term
			integral += error * (double) (timeChange*.001); // uses real delta T not a fixed delta T
			ITerm = ki * integral;
			if((ITerm > outMax)||(ITerm < outMin) ) integral -= error * (double) (timeChange*.001); // Prevemts Windup
			/*
			///////////////////////////////////////////////////////////////////////////////////////////////////
			We were not talking about that but it's something clever:
			....Jerky robot, that's because the Kd * de(t)/dt term goes to infinity when you change the set-point. Try instead of using the angle error, e(t), use the negative of the actual value change, dangle(t)/dt. they are equivalent but this way you avoid infinity errors. 

			Kd*de(t)/dt = -Kd*dangle(t)/dt
			///////////////////////////////////////////////////////////////////////////////////////////////////
			*/
			// Derivative term using error change
			//derivative = (error - pre_error)  / (double)(timeChange*.001); // uses real delta T not a fixed delta T
			//DTerm = kd * derivative;
			// Derivative term using angle change
			derivative = (input - lastInput)  / (double)(timeChange*.001); // uses real delta T not a fixed delta T
			DTerm = -kd * derivative;
		}
		//Compute PID Output
		double output = PTerm + ITerm + DTerm;

		if(output > outMax) output = outMax;
		else if(output < outMin) output = outMin;
		*myOutput = output;

		//Remember some variables for next time
		pre_error = error;
		lastInput = input;
		lastTime = now;
		/*
		// Debugging 
		for (static long QTimer = millis(); (long)( millis() - QTimer ) >= 100; QTimer = millis() ) {  // one line Spam Delay at 100 miliseconds
		Serial.print(F("\tInput ")); Serial.print(input);
		Serial.print(F("\mySetpoint ")); Serial.print(mySetpoint);
		Serial.print(F("\tDelta T ")); Serial.print(timeChange);
		Serial.print(F("\tPTerm ")); Serial.print(PTerm);
		Serial.print(F("\tITerm ")); Serial.print(ITerm);
		Serial.print(F("\tKd ")); Serial.print(kd);
		Serial.print(F("\tderivative ")); Serial.print(derivative);
		Serial.print(F("\tDTerm ")); Serial.print(DTerm);
		Serial.print(F("\tOutput ")); Serial.print(output);
		Serial.println();
		}	  
		*/
		return true;

	}
	else return false;
}

/* SetTunings(...)*************************************************************
* This function allows the controller's dynamic performance to be adjusted. 
* it's called automatically from the constructor, but tunings can also
* be adjusted on the fly during normal operation
******************************************************************************/ 
void PID::SetTunings(double Kp, double Ki, double Kd)
{
	if (Kp<0 || Ki<0 || Kd<0) return;

	dispKp = Kp; dispKi = Ki; dispKd = Kd;

	double SampleTimeInSec = ((double)SampleTime)/1000;  
	kp = Kp;
	ki = Ki ;// ki = Ki  * SampleTimeInSec;
	kd = Kd ;// kd = Kd/ SampleTimeInSec;

	if(controllerDirection ==REVERSE)
	{
		kp = (0 - kp);
		ki = (0 - ki);
		kd = (0 - kd);
	}
}

/* SetSampleTime(...) *********************************************************
* sets the period, in Milliseconds, at which the calculation is performed	
******************************************************************************/
void PID::SetSampleTime(int NewSampleTime)
{
	if (NewSampleTime > 0)
	{
		double ratio  = (double)NewSampleTime
			/ (double)SampleTime;
		//    ki *= ratio;
		//    kd /= ratio;
		SampleTime = (unsigned long)NewSampleTime;
	}
}

As for your code:
with or without the altered PID routine, you want to calculate you PID and output as fast as you can after getting a good reading
so Something like:

void sensorInterrupt()
{
  time = micros();
}
void readRpm() {
  static int i;
  if (time) {
    rpm_array[i] = 60 * (1000000 / ((time - time_last) * 36)); //  Calculate this instance of rpm    NOTE: the time differential is moved to here
    rpm = (rpm_array[0] + rpm_array[1] + rpm_array[2] + rpm_array[3] + rpm_array[4]) / 5;
    sendOutputToEsc(); // <<<<<<<<<<<<<<<<<  We have a reading Lets respond to it as fast as we can and make adjustments
    i++; // move to the next array for the next reading
    if (i == 5) i = 0; // return to beginning of array
    time_last = time; // store the time for the next reading
    time = 0; // set time to zero to wait for the next rpm trigger.
  }
}

Get rid of all delays and if you need a spam timer for the LCD or Serial use a no delay timer. Now you can fire debug() as fast as you can and it only triggers 10 times a second

void debug()
{
  static unsigned long SpamTimer;
  if (unsigned long)(millis() - SpamTimer) >= 100) {
    SpamTimer = millis();
    Serial.print("rpm ");
    Serial.print(rpm);
    Serial.print("  Input ");
    Serial.print(Input);
    Serial.print("  Setpoint ");
    Serial.print(Setpoint);
    Serial.print("  Output ");
    Serial.print(Output);
    Serial.print("  A0 ");
    Serial.println(analogRead(0));
  }
}
You guys are awesome, thanks.

Cattledog, your mods fixed the loop problem, thanks. And I take your point about delays. Should be banned;-)

zhomeslice, you have predicted what I have been struggling with for weeks. No matter what approach I have taken using PID, I get erratic results. Your time and effort to explain this is greatly appreciated.

I did want to get away from the fixed sample time but when I altered PID_v1.cpp I get the folowing errors.
[code}
Arduino: 1.6.7 (Linux), Board: "Arduino/Genuino Uno"

/home/anne/Arduino/libraries/PID/PID_v1.cpp: In member function 'bool PID::Compute()':
/home/anne/Arduino/libraries/PID/PID_v1.cpp:56:3: error: 'integral' was not declared in this scope
  integral = 0;
  ^
/home/anne/Arduino/libraries/PID/PID_v1.cpp:57:3: error: 'pre_error' was not declared in this scope
  pre_error = 0;
  ^
/home/anne/Arduino/libraries/PID/PID_v1.cpp:83:4: error: 'integral' was not declared in this scope
   integral += error * (double) (timeChange*.001); // uses real delta T not a fixed delta T
   ^
/home/anne/Arduino/libraries/PID/PID_v1.cpp:109:3: error: 'pre_error' was not declared in this scope
  pre_error = error;
  ^
exit status 1
Error compiling.

 This report would have more information with
 "Show verbose output during compilation"
 enabled in File > Preferences.

I am an idiot. :confused:
I moved the backup file and no errors.

Now here’s the latest code which serial prints this:
rpm -3385 Input -3385.00 Setpoint 30.00 Output 255.00 A0 690
rpm -3385 Input -3385.00 Setpoint 30.00 Output 255.00 A0 689

#include <PID_v1.h>

//Define Variables we'll be connecting to
double Setpoint, Input, Output;

//Specify the links and initial tuning parameters
PID myPID(&Input, &Output, &Setpoint, 2, 5, 1, DIRECT);

//For tacho.
volatile float time = 0;
volatile float time_last = 0;
volatile int rpm_array[5] = {0, 0, 0, 0, 0};
int rpm = 0;

void setup()
{
  //Digital Pin 2 Set As An Interrupt for tacho.
  attachInterrupt(0, sensorInterrupt, FALLING);


  //initialize the variables we're linked to
  Input = rpm;
  Setpoint = 30;

  //turn the PID on
  myPID.SetMode(AUTOMATIC);


  Serial.begin(9600);
}

void loop()
{
  getSetPoint();
  getStartStop();
  switchOnOff();
  readRpm();
  sensorInterrupt();
  sendOutputToEsc();
  debug();
}

void getSetPoint()
{
  Setpoint = 30;

}

void getStartStop()
{

}

void switchOnOff()
{

}

void sensorInterrupt()
{
  time = micros();
}

void readRpm()
{
  static int i;
  if (time) {
    rpm_array[i] = 60 * (1000000 / ((time - time_last) * 36)); //  Calculate this instance of rpm    NOTE: the time differential is moved to here
    rpm = (rpm_array[0] + rpm_array[1] + rpm_array[2] + rpm_array[3] + rpm_array[4]) / 5;
    sendOutputToEsc(); // <<<<<<<<<<<<<<<<<  We have a reading Lets respond to it as fast as we can and make adjustments
    i++; // move to the next array for the next reading
    if (i == 5) i = 0; // return to beginning of array
    time_last = time; // store the time for the next reading
    time = 0; // set time to zero to wait for the next rpm trigger.
  }
}

void sendOutputToEsc()
{
  Input = rpm;
  myPID.Compute();
  analogWrite(6, Output);
}

void debug()
{
  static unsigned long SpamTimer;
  if ((unsigned long)(millis() - SpamTimer) >= 400) {
    SpamTimer = millis();
Serial.print("rpm ");
Serial.print(rpm);
Serial.print("  Input ");
Serial.print(Input);
Serial.print("  Setpoint ");
Serial.print(Setpoint);
Serial.print("  Output ");
Serial.print(Output);
Serial.print("  A0 ");
Serial.println(analogRead(0));
}
}

These are the ALTERED PID_V1 files.
Note the major change is that you are not restricted to a specific time interval.
you can set the minimum time interval with SetSampleTime( int );
I documented the changes in the source code.

Sorry for your errors

move the old files to another directory outside your library for future use if you really want them :slight_smile:

paste these files in there place

While slightly unrelated to speed controller here is my balancing robot with a little bit of PID tuning advice and this uses the very same PID loops attached below!!!
PID Tuning of a balancing robot

PID_v1.cpp (9.01 KB)

MPU6050 - Copy.h (3.08 KB)

your variables are not quite right for what you are handling

volatile unsigned long time = 0; // only one that needs to be volatile because of the interrupt
unsigned long time_last = 0;
unsigned long rpm_array[5] = {0, 0, 0, 0, 0};
int rpm = 0;

micros() and millis() returns unsigned long values and int may bet rounded or roll over to negative

Thanks again Zhomeslice. Your input is greatly appreciated.

I am having trouble incorporating SetSampleTime(1);
I get the following error;

Arduino: 1.6.7 (Linux), Board: "Arduino/Genuino Uno"

wireless_seeder_controller_server-PID_v2:9: error: expected constructor, destructor, or type conversion before '(' token
 SetSampleTime(1);
              ^
exit status 1
expected constructor, destructor, or type conversion before '(' token

  This report would have more information with
  "Show verbose output during compilation"
  enabled in File > Preferences.

I altered the variable as you suggested and used your PID library files. Still not getting any rpm.
Serial output:

rpm -1011 Input -1011.00 Setpoint 30.00 Output 255.00 A0 605

Latest code:

#include <PID_v1.h>

//Define Variables we'll be connecting to
double Setpoint, Input, Output;


//Specify the links and initial tuning parameters
PID myPID( &Input, &Output, &Setpoint, 2, 5, 1, DIRECT);
SetSampleTime(1);

//For tacho.
volatile unsigned long time = 0;
unsigned long time_last = 0;
volatile int rpm_array[5] = {0, 0, 0, 0, 0};
int rpm = 0;

void setup()
{
  //Digital Pin 2 Set As An Interrupt for tacho.
  attachInterrupt(0, sensorInterrupt, FALLING);


  //initialize the variables we're linked to
  Input = rpm;
  Setpoint = 30;

  //turn the PID on
  myPID.SetMode(AUTOMATIC);
  


  Serial.begin(9600);
}

void loop()
{
  getSetPoint();
  getStartStop();
  switchOnOff();
  readRpm();
  sensorInterrupt();
  sendOutputToEsc();
  debug();
}

void getSetPoint()
{
  Setpoint = 30;

}

void getStartStop()
{

}

void switchOnOff()
{

}

void sensorInterrupt()
{
  time = micros();
}

void readRpm()
{
  static int i;
  if (time) {
    rpm_array[i] = 60 * (1000000 / ((time - time_last) * 36)); //  Calculate this instance of rpm    NOTE: the time differential is moved to here
    rpm = (rpm_array[0] + rpm_array[1] + rpm_array[2] + rpm_array[3] + rpm_array[4]) / 5;
    sendOutputToEsc(); // <<<<<<<<<<<<<<<<<  We have a reading Lets respond to it as fast as we can and make adjustments
    i++; // move to the next array for the next reading
    if (i == 5) i = 0; // return to beginning of array
    time_last = time; // store the time for the next reading
    time = 0; // set time to zero to wait for the next rpm trigger.
  }
}

void sendOutputToEsc()
{
  Input = rpm;
  myPID.Compute();
  analogWrite(6, Output);
}

void debug()
{
  static unsigned long SpamTimer;
  if ((unsigned long)(millis() - SpamTimer) >= 400) {
    SpamTimer = millis();
Serial.print("rpm ");
Serial.print(rpm);
Serial.print("  Input ");
Serial.print(Input);
Serial.print("  Setpoint ");
Serial.print(Setpoint);
Serial.print("  Output ");
Serial.print(Output);
Serial.print("  A0 ");
Serial.println(analogRead(0));
}
}

you have defined the class as myPID

//Specify the links and initial tuning parameters
PID myPID( &Input, &Output, &Setpoint, 2, 5, 1, DIRECT);
myPID.SetSampleTime(1);

I would also move it to the setup()

  // in setup() function
  myPID.SetSampleTime(1);

  //turn the PID on
  myPID.SetMode(AUTOMATIC);

put it just before you set your PID to Automatic this is a good spot

Something else i see that will help you is myPID.SetOutputLimits( 0, 255 ); This is because analogWrite(6, Output); can only handel a number between 0 and 255

zhomeslice: you have defined the class as myPID

//Specify the links and initial tuning parameters
PID myPID( &Input, &Output, &Setpoint, 2, 5, 1, DIRECT);
myPID.SetSampleTime(1);

Do'h... So obvious when it's pointed out. Thanks.

Any idea why the tacho still doesn't output correctly?

moose4621: Any idea why the tacho still doesn't output correctly?

Not sure. Let's see what if we skipped the averaging

rpm = 60 * (1000000 / ((time - time_last) * 36));

zhomeslice:
Not sure.
Let’s see what if we skipped the averaging

rpm = 60 * (1000000 / ((time - time_last) * 36));

Now positive at least.
rpm 14340 Input 14340.00 Setpoint 30.00 Output 0.00 A0 659
rpm 14340 Input 14340.00 Setpoint 30.00 Output 255.00 A0 659
rpm 14340 Input 14340.00 Setpoint 30.00 Output 255.00 A0 659
rpm 14880 Input 14880.00 Setpoint 30.00 Output 0.00 A0 659
rpm 14340 Input 14340.00 Setpoint 30.00 Output 255.00 A0 659
rpm 14340 Input 14340.00 Setpoint 30.00 Output 255.00 A0 660
motor running at about 40rpm but very noisy.

void sensorInterrupt()
{
  time = micros();
}

void readRpm()
{
  static int i;
  if (time) {
    //rpm_array[i] = 60 * (1000000 / ((time - time_last) * 36)); //  Calculate this instance of rpm    NOTE: the time differential is moved to here
    rpm = 60 * (1000000 / ((time - time_last) * 36));//(rpm_array[0] + rpm_array[1] + rpm_array[2] + rpm_array[3] + rpm_array[4]) / 5;
    sendOutputToEsc(); // <<<<<<<<<<<<<<<<<  We have a reading Lets respond to it as fast as we can and make adjustments
    //i++; // move to the next array for the next reading
    //if (i == 5) i = 0; // return to beginning of array
    time_last = time; // store the time for the next reading
    time = 0; // set time to zero to wait for the next rpm trigger.
  }
}
[/code

rpm 14340 Input 14340.00 Setpoint 30.00 Output 255.00 A0 660 motor running at about 40rpm but very noisy.

This is great that we are getting a value! Now we have a math problem.

The question is: how many pulses per revolution of your motor?

removed section my math was in error I'm working on a correction and will post shortly

PID related notes:

Note: averaging isn't the best when the PID can divide up a second and only influence the PID output based on what happens over each reading so averaging would influence the PID over time rather than jut tha one instance. so a wild reading isn't that bad. we will get to the PID values in more detail but here is a brief example on how you will tune it and why modify this code to read

int Kp = 2;
int Ki = 0;
int Kd = 0;
PID myPID ( &Input, &Output, &Setpoint, Kp,Ki,Kd, DIRECT ) ;

Kp will get us close to your setpoint but will never land it!!! we need to make find a stable Kp value before we move on to Ki. When you get a stable Kp only Ki will cause the outupt to shift up or down to reach setpoint Ki is additive and adds a portion of the error to the output. Kd will recover from rapid changing conditions and is only a temperary addition you may not need Kd at all especially at first when you start tuning.

I'm running out of time to go into details but lets start here and see where we are :)


After thinking about how we are getting the reading I may have a simpler way to get the results you want

volatile int PulseCtr;
void sensorInterrupt()
{
  PulseCtr++;  
}

It starts something like that .. I'll go further into this change after I get back later today

after running some simulations this has the best chance to run the widest range of readings from nothing to 500 RPM seems to be accurate

void sensorInterrupt()
{
  time += micros();
  PulseCtr++;
}

void readRpm()
{
  if (time > 0) {
    // note that 1666666.67 = (60 seonds * 1000000 microseconds)microseconds in a minute / 36 pulses in 1 revolution
    Input =  (double) 1666666.67 / (time - time_last) * PulseCtr; // double has more percision
    time_last = time; // store the time for the next reading
    PulseCtr = 0; // set pulse Ctr to zero
    time = 0; // set time to zero to wait for the next rpm trigger.
    myPID.Compute(); // way wait calculate now
    analogWrite(6, Output); // Set the PWM for the motor
    rpm = (int)Input; // for display convert to int
  }
}

I did merge sendOutputToEsc() with readRpm() so remove sendOutputToEsc from you program.
My Test program i used to verify the code :slight_smile:

volatile unsigned long timeX = 1;
unsigned long PulseCtr, time_last;
double Input;
int rpm;
int TestRPM = 1;
double TextRPMx = 1;
void setup() {
  // put your setup code here, to run once:
  Serial.begin(115200);
  Serial.println("Test Tachometer");
  PulseCtr = 0;
}

void loop() {
  // put your main code here, to run repeatedly:
  readRpm();
  static unsigned long TestTimer;
  if ((unsigned long)(micros() - TestTimer) >= (60000000 / (36 * TestRPM))) { //1 rpm test
    TestTimer = micros();
    sensorInterrupt();
    if (TestRPM <= 850) {
      TextRPMx += .2;
      TestRPM = (int) TextRPMx;
    }
    else while (1);
  }
}

void sensorInterrupt()
{
  timeX += micros();
  PulseCtr++;
}

void readRpm()
{
  static int i;
  if (timeX > 0) {
    // note that 1666666.67 = (60 seonds * 1000000 microseconds)microseconds in a minute / 36 pulses in 1 revolution
    Input =  (double) 1666666.67 / (timeX - time_last) * PulseCtr; // double has more percision
    time_last = timeX; // store the time for the next reading
    PulseCtr = 0; // set pulse Ctr to zero
    timeX = 0; // set time to zero to wait for the next rpm trigger.
    //    myPID.Compute();
    //    analogWrite(6, Output);
    static unsigned long SpamTimer;
    if ((unsigned long)(millis() - SpamTimer) >= 100) {
      SpamTimer = millis();
      Serial.print(" Test RPM: ");
      Serial.print(TestRPM);
      Serial.print(" Calculated RPM: ");
      Serial.println(Input);
    }
    rpm = (int)Input; // for display convert to int
  }
}

I am so appreciative of your assistance, thank you.

Given that max rpm is about 60, you are well within limits.

Your test program is awesome. I ran it out of interest and can understand better what is going on now.

I suspect now that I may have a hardware issue since the output is similar to before.

pm 18115 Input 18115.94 Setpoint 30.00 Output 0.00 A0 658
rpm 18115 Input 18115.94 Setpoint 30.00 Output 255.00 A0 659
rpm 17361 Input 17361.11 Setpoint 30.00 Output 0.00 A0 659

The motor runs very rough.
Current code.

#include <PID_v1.h>

//Define Variables we'll be connecting to
double Setpoint, Input, Output;


//Specify the links and initial tuning parameters
PID myPID( &Input, &Output, &Setpoint, 2, 5, 1, DIRECT);


//For tacho.
volatile unsigned long time = 0;
unsigned long time_last = 0;
volatile int rpm_array[5] = {0, 0, 0, 0, 0};
volatile int PulseCtr;
int rpm = 0;



void setup()
{
  //Digital Pin 2 Set As An Interrupt for tacho.
  attachInterrupt(0, sensorInterrupt, FALLING);


  //initialize the variables we're linked to
  Input = rpm;
  Setpoint = 30;

  myPID.SetSampleTime(0);
  myPID.SetOutputLimits( 0, 255 );

  //turn the PID on
  myPID.SetMode(AUTOMATIC);

  pinMode (0, OUTPUT);


  Serial.begin(9600);
}

void loop()
{
  getSetPoint();
  getStartStop();
  switchOnOff();
  readRpm();
  sensorInterrupt();
  //sendOutputToEsc();
  debug();
}

void getSetPoint()
{
  Setpoint = 30;

}

void getStartStop()
{

}

void switchOnOff()
{

}

void sensorInterrupt()
{
  // time = (micros() - time_last);
  //  time_last = micros();
  time += micros();
  PulseCtr++;
}


void readRpm()
{
  if (time > 0) {
    // note that 1666666.67 = (60 seonds * 1000000 microseconds)microseconds in a minute / 36 pulses in 1 revolution
    Input =  (double) 1666666.67 / (time - time_last) * PulseCtr; // double has more percision
    time_last = time; // store the time for the next reading
    PulseCtr = 0; // set pulse Ctr to zero
    time = 0; // set time to zero to wait for the next rpm trigger.
    myPID.Compute(); // way wait calculate now
    analogWrite(6, Output); // Set the PWM for the motor
    rpm = (int)Input; // for display convert to int
  }
}

void debug()
{
  static unsigned long SpamTimer;
  if ((unsigned long)(millis() - SpamTimer) >= 400) {
    SpamTimer = millis();
    Serial.print("rpm ");
    Serial.print(rpm);
    Serial.print("  Input ");
    Serial.print(Input);
    Serial.print("  Setpoint ");
    Serial.print(Setpoint);
    Serial.print("  Output ");
    Serial.print(Output);
    Serial.print("  A0 ");
    Serial.println(analogRead(0));

  }
}

I replaced

  analogWrite(6, Output); // Set the PWM for the motor

with

  analogWrite(6, 100); // Set the PWM for the motor

The motor runs nice and smooth as you would expect but made no difference to output.

I then wrote this using your pulse counter and a fixed output to esc to eliminate all the other guff.

//For tacho.
volatile unsigned long time = 0;
unsigned long time_last = 0;
volatile int PulseCtr;
int rpm = 0;
double Input;



void setup()
{
  //Digital Pin 2 Set As An Interrupt for tacho.
  attachInterrupt(0, sensorInterrupt, FALLING);
  Serial.begin(115200);
  analogWrite(6, 100);

}
void loop()
{ // put your main code here, to run repeatedly:
  sensorInterrupt();
  readRpm();
  debug();
}

void sensorInterrupt()
{
  // time = (micros() - time_last);
  //  time_last = micros();
  time += micros();
  PulseCtr++;
}

void readRpm()
{

  if (time > 0) {
    // note that 1666666.67 = (60 seonds * 1000000 microseconds)microseconds in a minute / 36 pulses in 1 revolution
    Input =  (double) 1666666.67 / (time - time_last) * PulseCtr; // double has more percision
    time_last = time; // store the time for the next reading
    PulseCtr = 0; // set pulse Ctr to zero
    time = 0; // set time to zero to wait for the next rpm trigger.
    rpm = (int)Input; // for display convert to int
  }
}

void debug()

{
  static unsigned long SpamTimer;
  if ((unsigned long)(millis() - SpamTimer) >= 100) {
    SpamTimer = millis();
    Serial.print(" RPM: ");
    Serial.println(rpm);

  }
}

Unfortunately the output is:
RPM: 24509

I keep going back to the RPM sensor test in post #1 though which works and I can’t figure out why nothing else does.

I then added a pot to the rpm sensor test to control the pwm out. The RPM sensor with this sketch is accurate throughout the range.

volatile float time = 0;
volatile float time_last = 0;
volatile int rpm_array[5] = {0, 0, 0, 0, 0};
int pot;

void setup()
{
  //Digital Pin 2 Set As An Interrupt
  attachInterrupt(0, fan_interrupt, FALLING);

  pot = analogRead(0);

  Serial.begin(9600);
  // analogWrite(6, 100);
}

//Main Loop To Calculate RPM and Update LCD Display
void loop()
{





  while (1) {


    pot = analogRead(0);
    analogWrite(6, map(pot, 0, 1023, 0, 255));

    int rpm = 0;

    //Slow Down The LCD Display Updates
    delay(400);

    //Update The RPM
    /*
      noInterrupts();
      unsigned long copyTime = time;
      interrupts();
    */

    if (time > 0)
    {

      //5 Sample Moving Average To Smooth Out The Data
      rpm_array[0] = rpm_array[1];
      rpm_array[1] = rpm_array[2];
      rpm_array[2] = rpm_array[3];
      rpm_array[3] = rpm_array[4];
      rpm_array[4] = 60 * (1000000 / (time * 36));
      //Last 5 Average RPM Counts Eqauls....
      rpm = (rpm_array[0] + rpm_array[1] + rpm_array[2] + rpm_array[3] + rpm_array[4]) / 5;

      // rpm = 60*(1000000/(time * 36));
      Serial.print(rpm);
      Serial.print("  ");
      Serial.println(pot);
    }

  }
}


//Capture The IR Break-Beam Interrupt
void fan_interrupt()
{
  time = (micros() - time_last);
  time_last = micros();
}

zhomeslice: PID_V1 has a problem but for most it isn't a huge issue. But in your case I see it affecting your results and you should know why and how I fixed it for my balancing robot.

concerning PID_V1 First thing to note is the default sample time is 100 milliseconds. so no matter how fast you can sample your input for minute changes, if you don't change this then you will only calculate every 100 milliseconds to change your motor PWM value.

That's not a "problem", it's a feature. It was designed like that intentionally, and used correctly it works perfectly. A PID should be updated at regular intervals if you expect consistent performance, as the update rate will affect the dynamic performance. You will never obtain optimum performance with a randomly varying update rate. The correct way to use PID_v1 is to either ensure that Compute gets called at or above at the specified interval (it should be called in every pass through loop()), or, better still, use a timer interrupt to call it at the desired interval. If you do either of those, it will work just fine without modifications.

Regards, Ray L.

Lets try this I moved the delta Time to the interrupt to keep the floating point numbers from causing greef

This should drive you motor and I tested it with a external input triggering the interrupt

volatile unsigned long timeX = 1;
unsigned long PulseCtr, time_last;
double Input;
int rpm;
int TestRPM = 1;
double TextRPMx = 1;
void setup() {
  pinMode(2, INPUT_PULLUP);
  // put your setup code here, to run once:
  Serial.begin(115200);
  Serial.println("aTest Tachometer");
  delay(1000);
  //Digital Pin 2 Set As An Interrupt for tacho.
  attachInterrupt(0, sensorInterrupt, FALLING);

  analogWrite(6, 100);
  PulseCtr = 0;
}

void loop() {
  // put your main code here, to run repeatedly:
  readRpm();

}

void sensorInterrupt()
{
  unsigned long Time;
  static unsigned long LastTime;
  Time = micros();
  timeX += (Time - LastTime);
  LastTime = Time;
  PulseCtr ++;
}

void readRpm()
{
  static int i;
  if (timeX > 0) {
    // note that 1666666.67 = (60 seonds * 1000000 microseconds)microseconds in a minute / 36 pulses in 1 revolution
    Input =  (double) (1666666.67 /  (double)(( (unsigned long)timeX ) *  (unsigned long)PulseCtr)); // double has more percision
//    Input =  (double) (1666666.67 /  (double)(( (unsigned long)timeX -  (unsigned long)time_last) *  (unsigned long)PulseCtr)); // double has more percision
    static unsigned long SpamTimer;
    if ((unsigned long)(millis() - SpamTimer) >= 1) {
      SpamTimer = millis();
      Serial.print(" timeX: ");
      Serial.print(timeX );
      Serial.print(" PulseCtr: ");
      Serial.print(PulseCtr);
      Serial.print(" Calculated RPM: ");
      Serial.println(Input);
    }
 //   time_last = timeX; // store the time for the next reading
    PulseCtr = 0; // set pulse Ctr to zero
    timeX = 0; // set time to zero to wait for the next rpm trigger.
    //    myPID.Compute();
    //    analogWrite(6, Output);

    rpm = (int)Input; // for display convert to int
  }
}

zhomeslice:
Lets try this I moved the delta Time to the interrupt to keep the floating point numbers from causing greef

This should drive you motor and I tested it with a external input triggering the interrupt

BINGO!

timeX: 40340 PulseCtr: 1 Calculated RPM: 41.32
timeX: 42372 PulseCtr: 1 Calculated RPM: 39.33
timeX: 40628 PulseCtr: 1 Calculated RPM: 41.02

:slight_smile:

Now to incorporate that back into PID output control.

In the meantime, I wrote this which works surprisingly well.

volatile float time = 0;
volatile float time_last = 0;
volatile int rpm_array[5] = {0, 0, 0, 0, 0};
byte rpm;
int output;
int setpoint = 30;
int printrpm;

void setup()
{
  //Digital Pin 2 Set As An Interrupt
  attachInterrupt(0, fan_interrupt, FALLING);
  analogWrite(6, 0);
  output = 0;

  Serial.begin(9600);

}


void loop()
{

  if (rpm < setpoint) {
    output = output + 1;
    if (output >= 255)
    {
      output = 255;
    }

  }
  else if (rpm > setpoint) {
    output = output - 1;
    if (output <= 0)
    {
      output = 0;
    }
  }
  analogWrite(6, output);

  //Update The RPM
  if (time > 0)
  {
    rpm = 60 * (1000000 / (time * 36));
    //5 Sample Moving Average To Smooth Out The Data
    rpm_array[0] = rpm_array[1];
    rpm_array[1] = rpm_array[2];
    rpm_array[2] = rpm_array[3];
    rpm_array[3] = rpm_array[4];
    rpm_array[4] = rpm;
    //Last 5 Average RPM Counts Eqauls....
    printrpm = (rpm_array[0] + rpm_array[1] + rpm_array[2] + rpm_array[3] + rpm_array[4]) / 5;


  }
  //Update The Rpm Count
  Serial.print("RPM: ");
  Serial.print(printrpm);
  Serial.print(" PWM out: ");
  Serial.print(output);
  Serial.print(" Setpoint ");
  Serial.println(setpoint);
}

//Capture The IR Break-Beam Interrupt
void fan_interrupt()
{
  time = (micros() - time_last);
  time_last = micros();
}