calculate rpm motor and speed motor using PID

Hi forum

I have one problem. I am confused 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 to rotate another cycle and 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");
}