calculate rpm motor and speed motor using PID

Hi forum

I have one problem. I dont understand that I could calculate the rpm motor and PID but when the motor encoder disc attach to the optical sensor the motor could rotate one cycle and then stop. Then continue rotate another cycle it stop again. Please advice.My ouput pin motor is 9 and intterupt pin attach to the optical sensor is Pin no 2.
I attach the code please advice.

#include <PID_v1.h>
double Setpoint, Input, Output;
volatile int rpmcount = 0;
double rpm = 0;
unsigned long lastmillis = 0;
int outputPin = 9; // motor connected to digital pin 9 output
// Tuning parameters
float Kp=0; //Initial Proportional Gain 
float Ki=10; //Initial Integral Gain 
float Kd=0; //Initial Differential Gain 

// Specify the links and initial tuning parameters
PID myPID(&rpm, &Output, &Setpoint,Kp,Ki,Kd, DIRECT);
const int sampleRate = 1;
unsigned long lastMessage = 0;
unsigned long serialTime;


void setup()
{
Setpoint = 100;
Serial.begin(9600); 
 attachInterrupt(0, rpm_motor, FALLING);//interrupt cero (0) is on pin two(2).
 myPID.SetMode(AUTOMATIC); // Turn on the PID loop as automatic control
  myPID.SetSampleTime(sampleRate); // Sets the sample rate
  lastMessage = millis(); // timestamp

}



void loop()
{

 myPID.Compute(); // Run the PID loop
analogWrite(outputPin, Output); 
 if (millis() - lastmillis == 1000){  //Uptade every one second, this will be equal to reading frecuency (Hz).
 
 detachInterrupt(0);    //Disable interrupt when calculating
 
 
 rpm = rpmcount * 60;  // Convert frecuency to RPM, note: this works for one interruption per full rotation. For two interrups per full rotation use rpmcount * 30.
                        //input pid
 Serial.print("RPM =\t"); //print the word "RPM" and tab.
 Serial.print(rpm); // print the rpm value.
 Serial.print("\t Hz=\t"); //print the word "Hz".
 Serial.println(rpmcount); //print revolutions per second or Hz. And print new line or enter.
 
 rpmcount = 0; // Restart the RPM counter
 lastmillis = millis(); // Uptade lasmillis
 attachInterrupt(0, rpm_motor, FALLING); //enable interrupt
  }
  if(millis()>serialTime)
  {
    SerialReceive();
    SerialSend();
    serialTime+=500;
  }  
}
void rpm_motor(){ // this code will be executed every time the interrupt 0 (pin2) gets low.
  rpmcount++;
}

 

/********************************************
 * Serial Communication functions / helpers
 ********************************************/


union {                // This Data structure lets
  byte asBytes[24];    // us take the byte array
  float asFloat[6];    // sent from processing and
}                      // easily convert it to a
foo;                   // float array



// getting float values from processing into the arduino
// was no small task.  the way this program does it is
// as follows:
//  * a float takes up 4 bytes.  in processing, convert
//    the array of floats we want to send, into an array
//    of bytes.
//  * send the bytes to the arduino
//  * use a data structure known as a union to convert
//    the array of bytes back into an array of floats

//  the bytes coming from the arduino follow the following
//  format:
//  0: 0=Manual, 1=Auto, else = ? error ?
//  1: 0=Direct, 1=Reverse, else = ? error ?
//  2-5: float setpoint
//  6-9: float input
//  10-13: float output  
//  14-17: float P_Param
//  18-21: float I_Param
//  22-245: float D_Param
void SerialReceive()
{

  // read the bytes sent from Processing
  int index=0;
  byte Auto_Man = -1;
  byte Direct_Reverse = -1;
  while(Serial.available()&&index<26)
  {
    if(index==0) Auto_Man = Serial.read();
    else if(index==1) Direct_Reverse = Serial.read();
    else foo.asBytes[index-2] = Serial.read();
    index++;
  } 
  
  // if the information we got was in the correct format, 
  // read it into the system
  if(index==26  && (Auto_Man==0 || Auto_Man==1)&& (Direct_Reverse==0 || Direct_Reverse==1))
  {
    Setpoint=double(foo.asFloat[0]);
    //Input=double(foo.asFloat[1]);       // * the user has the ability to send the 
                                          //   value of "Input"  in most cases (as 
                                          //   in this one) this is not needed.
    if(Auto_Man==0)                       // * only change the output if we are in 
    {                                     //   manual mode.  otherwise we'll get an
      Output=double(foo.asFloat[2]);      //   output blip, then the controller will 
    }                                     //   overwrite.
    
    double p, i, d;                       // * read in and set the controller tunings
    p = double(foo.asFloat[3]);           //
    i = double(foo.asFloat[4]);           //
    d = double(foo.asFloat[5]);           //
    myPID.SetTunings(p, i, d);            //
    
    if(Auto_Man==0) myPID.SetMode(MANUAL);// * set the controller mode
    else myPID.SetMode(AUTOMATIC);             //
    
    if(Direct_Reverse==0) myPID.SetControllerDirection(DIRECT);// * set the controller Direction
    else myPID.SetControllerDirection(REVERSE);          //
  }
  Serial.flush();                         // * clear any random data from the serial buffer
}

// unlike our tiny microprocessor, the processing ap
// has no problem converting strings into floats, so
// we can just send strings.  much easier than getting
// floats from processing to here no?
void SerialSend()
{
  Serial.print("PID ");
  Serial.print(Setpoint);   
  Serial.print(" ");
  Serial.print(rpm);   
  Serial.print(" ");
  Serial.print(Output);   
  Serial.print(" ");
  Serial.print(myPID.GetKp());   
  Serial.print(" ");
  Serial.print(myPID.GetKi());   
  Serial.print(" ");
  Serial.print(myPID.GetKd());   
  Serial.print(" ");
  if(myPID.GetMode()==AUTOMATIC) Serial.print("Automatic");
  else Serial.print("Manual");  
  Serial.print(" ");
  if(myPID.GetDirection()==DIRECT) Serial.println("Direct");
  else Serial.println("Reverse");
}

Does this one the best way ?
( or should “==” be “>=”)
“if (millis() - lastmillis == 1000)” …

If I change the code if (millis() - lastmillis == 1000 to if (millis() - lastmillis = 1000) it shows error Ivalue required as left operand of assignment. Need your advice is that anything problem with the rpm = rpmcount * 60; when I uncommand this code the motor continue running as usual when the encoder disc attach to the optical sensor unfortunately its doesnt calculate the RPM value.

I ment to say : test for greater than, NOT for equal

hi forum
I already test greater than if (millis() - lastmillis >= 1000) but when the disc encoder motor attach to the optical sensor it rotate one cycle and stop for few seconds then rotate one cycle and stop again. Can you advice me what is the problem from my code please.

keep the ">=" ! (using == will hit just if you're lucky)
You want output pr. 1/2 sec.-> Incr. Serial.output speed to 115200 baud, so nothing is overrun

Seems like the problem is connected to the PID calculations as it returns 0 when you expect something else..
Try (for fun) to insert an additional sentence; "Output=value;" (select a suitable nbr. for value)
this statement just AFTER the "myPid.compute"

hi forum

I had tried with output =200 , setpoint is 100 and serial ouput to 115200 the motor disc not stopping when attached at the optical sensor but the calculation rpm shows RPM 1200 with 20 hz which doesnt calculate the proper reading. Can you advice please. thank you.

you call "Serialreceive.."
Sure there is something - every time ?
Your code take for granted that data is ALWAYS available, and make adjustments even if no new data.
........
else: you can speed up by testing for most likely outcome first..