Problem with motor control

Hello experts,

I have a DC Motor connected via a Cytron MD 13-S board to an Arduino Mega 2560.
It has an optical encoder wherein the A channel is connected to pin 2 and B to pin 3.

The cytron has a pwm pin which is connected to pin 4 of the Arduino and a motor direction pin which is connected to pin 7 of the Arduino. All VCCs and grounds are properly connected.

I’ve written a sketch to move the motor a specific degrees upon receiving input from the serial monitor.

Everything works EXCEPT when I give a position (say 30 degrees) from the serial monitor, the motor moves by 30 degrees but then keeps going back and forth by 0.1 degrees till the end of time. Can something be done to make it stop cold until a new input arrives? Is there something wrong in the sketch that is making it do so?

#include <PID_v1.h>
#define MotEnable 4 //Motor Enamble pin Runs on PWM signal
#define MotDir  7 // Motor Direction pin
#define  encoderPinA  2   // right
#define  encoderPinB  3   // left


String readString; //This while store the user input data
int User_Input = 0; // This while convert input string into integer


volatile int32_t lastEncoded = 0; // Here updated value of encoder store.
volatile int32_t encoderPos = 0; // Raw encoder value
int32_t PPR = 134000;  // Encoder Pulse per revolution.
int angle = 360; // Maximum degree of motion.
int32_t REV = 0;          // Set point REQUIRED ENCODER VALUE
static boolean rotating = false;    // debounce management
// interrupt service routine vars
boolean A_set = false;
boolean B_set = false;



double kp = 5 , ki = 1 , kd = 0.01;             // modify for optimal performance
double input = 0, output = 0, setpoint = 0;
PID myPID(&input, &output, &setpoint, kp, ki, kd, DIRECT);




void setup() {
  pinMode(MotEnable, OUTPUT);
  pinMode(MotDir, OUTPUT);

  Serial.begin(9600); //initialize serial comunication

  pinMode(encoderPinA, INPUT);
  pinMode(encoderPinB, INPUT);

  digitalWrite(encoderPinA, HIGH); //turn pullup resistor on
  digitalWrite(encoderPinB, HIGH); //turn pullup resistor on


  // encoder pin on interrupt 0 (pin 2)
  attachInterrupt(0, doEncoderA, CHANGE);
  // encoder pin on interrupt 1 (pin 3)
  attachInterrupt(1, doEncoderB, CHANGE);


  TCCR3B = TCCR3B & 0b11111000 | 1;  // set 31KHz PWM to prevent motor noise

  myPID.SetMode(AUTOMATIC);   //set PID in Auto mode
  myPID.SetSampleTime(1);  // refresh rate of PID controller
  myPID.SetOutputLimits(-125, 125); // this is the MAX PWM value to move motor, here change in value reflect change in speed of motor.
}

void loop() {
  while (Serial.available()) { //Check if the serial data is available.
    delay(3);                  // a small delay
    char c = Serial.read();  // storing input data
    readString += c;         // accumulate each of the characters in readString
  }

  if (readString.length() > 0) { //Verify that the variable contains information

    Serial.println(readString.toInt());  //printing the input data in integer form
    User_Input = readString.toInt();   // here input data is store in integer form

  }

  REV = map (User_Input, 0, 360, 0, PPR); // mapping degree into pulse


  Serial.print("this is REV - ");
  Serial.println(REV);               // printing REV value

  setpoint = REV;                    //PID while work to achive this value consider as SET value
  input = encoderPos ;           // data from encoder consider as a Process value
  Serial.print("encoderPos - ");
  Serial.println(encoderPos);
  myPID.Compute();                 // calculate new output
  pwmOut(output);
}
void pwmOut(int out) {
  if (out > 0) {                         // if REV > encoderPos motor move in forward direction.
    analogWrite(MotEnable, out);         // Enabling motor enable pin to reach the desire angle
    forward();                           // calling motor to move forward
  }
  else {
    analogWrite(MotEnable, abs(out));          // if REV < encoderPos motor move in forward direction.
    reverse();                            // calling motor to move reverse
  }


  readString = ""; // Cleaning User input, ready for new Input


}

// Interrupt on A changing state
void doEncoderA() {
  // debounce
  if ( rotating ) delay (1);  // wait a little until the bouncing is done

  // Test transition, did things really change?
  if ( digitalRead(encoderPinA) != A_set ) { // debounce once more
    A_set = !A_set;

    // adjust counter + if A leads B
    if ( A_set && !B_set )
      encoderPos += 1;

    rotating = false;  // no more debouncing until loop() hits again
  }
}

// Interrupt on B changing state, same as A above
void doEncoderB() {
  if ( rotating ) delay (1);
  if ( digitalRead(encoderPinB) != B_set ) {
    B_set = !B_set;
    //  adjust counter - 1 if B leads A
    if ( B_set && !A_set )
      encoderPos -= 1;

    rotating = false;
  }
}








void forward () {
  digitalWrite(MotDir, HIGH);

}

void reverse () {
  digitalWrite(MotDir, LOW);


}
void finish () {
  analogWrite(MotEnable, 0);

}

Just guessing as I am not an expert but have you tried changing

void finish () {
  analogWrite(MotEnable, 0);

}

To

void finish () {
  analogWrite(MotEnable, LOW);

}

An analogWrite(x, 0) actually forces a digitalWrite(x, LOW) anyway.

edit, from the source:

void analogWrite(uint8_t pin, int val)
{
	pinMode(pin, OUTPUT);
	if (val == 0)
	{
		digitalWrite(pin, LOW);
	}
	else if (val == 255)
        {
              etc...

I resolved the issue. After playing with Kp, Ki and Kd values and reducing pwm (none of which worked) I figured the issue was encoder resolution. 134,000 clicks per revolution means setpoints are several thousand and arduino cannot act fast enough to stop the motor at an exact count.

I reduced the encoder resolution (by dividing the encoderPos by 18 and also the setpoint by an equal 1/18. Now it works perfectly!