Simply PID loop in Arduino does not work when I send a value through serial

I have a PID loop running on an Arduino Nano that I plan to connect to a Raspberry Pi via USB. I'm doing some testing on my laptop and I am trying to make sure it works, but it doesn't. The code is posted below. You can see that I commented out the target = sin function in the loop below. When the target = rpm is commented out, and the target = sin is not. The program runs perfectly. However, when I comment out the target = sin and uncomment the target = rpm, it does not work. I have sent values to the nano in the serial monitor window, and I know that it has received the value because I see "Transmission received" in the monitor. The target value is also updated in the serial monitor when the target = sin is active, but the target value says zero in the serial monitor when I send a value to it. Could someone please explain what I am doing wrong?

#include <CytronMotorDriver.h> 


  // Configure the motor driver.
CytronMD motorR(PWM_DIR, 3, 4);  // PWM 1 = Pin D3, DIR 1 = Pin D4.

// Configure encoder pins
#define EncoderA 6 // yellow wire 
#define EncoderB 7 // green wire

volatile int posi = 0;      // position of motor
long previousT = 0;
float eprevious = 0;
float eintegral = 0;

int rpm;
int rpm_correction_factor = 1;
float wheel_circumference = 0.090;

// The setup routine runs once when you press reset.
void setup() {
  Serial.begin(9600);
  pinMode(EncoderA, INPUT);
  pinMode(EncoderB, INPUT);
  attachInterrupt(digitalPinToInterrupt(EncoderA), readEncoder, RISING); //whenever encoder phase A is HIGH, readEncoder function is called
  Serial.println("target position");
  }


// setSpeed can be any number for -255 to 255 (negative means reverse)
void loop() {
   //check if arduino has received some data. if data has arrived, use readStringUntil to get the next line. All bytes received until /n are converted to useable communication data
    if (Serial.available() > 0) {
        String received_speed = Serial.readStringUntil('\n');
        int rpm = round(rpm_correction_factor * (received_speed.toInt())/(wheel_circumference * 3.14 * 60));
        Serial.print("transmission received");
    }
  
  
  //int target = 250*sin(previousT/1e6); //test target position for PID control loop
  int target = rpm; //target position from serial port 

  //PID constants (these values may have to be adjusted)
  float kp = 1;
  float kd = 0;
  float ki = 0;

  //finding the time difference
  long currentT = micros();
  float deltaT = ((float)(currentT-previousT))/1.0e6;
  previousT = currentT;  

  int motor_position = 0;
  noInterrupts();
  motor_position = posi;
  interrupts();


  //error calculation
  int e = motor_position - target; //position and target may have to be switched depending on how the motor leads are wired

  //derivative
  float dedt = (e - eprevious)/(deltaT);

  //integral
  eintegral = eintegral + e*deltaT;

  //control signal
  float u = kp*e + kd*dedt + ki*eintegral;

  //set motor power and limit it to between -255 and +255
  //float power = fabs(u);
  float power = u;
  if(power>255){
    power = 255;
  }
  if(power<-255){
    power = -255;
  }

  motorR.setSpeed(power);
  //store previous error
  eprevious = e;

  //print target and position to serial plotter
  Serial.print(target);
  Serial.print(",");
  Serial.println(motor_position);
  //Serial.println();

 
}

void readEncoder(){
  // called when EncoderA is high; If the motor is rotating counterclockwise, EncoderB should already be high, so we would add one to the position 
  int b = digitalRead(EncoderB);
  if(b>0) {
    posi ++;
    }
  else{
    posi --;
    }
  }

it looks like the issue is something us programmers call "shadowing." You have a variable rpm that is defined at the top making it a global variable:

int rpm;
int rpm_correction_factor = 1;
float wheel_circumference = 0.090;

However, when you read from Serial, you actually redefine that variable as local to the "scope" of the if block:

    if (Serial.available() > 0) {
        String received_speed = Serial.readStringUntil('\n');
        int rpm = round(rpm_correction_factor * (received_speed.toInt())/(wheel_circumference * 3.14 * 60));
        Serial.print("transmission received");
    }

This rpm variable is not the same as your global rpm. It is only visible inside the if block. Once you leave the if block, that variable no longer exists. In essence, you never set rpm variable that you are referencing here:

int target = rpm; //target position from serial port

This rpm variable is never touched in your program, therefore, it remains 0. So the solution here is to remove the int before rpm:

    if (Serial.available() > 0) {
        String received_speed = Serial.readStringUntil('\n');
        rpm = round(rpm_correction_factor * (received_speed.toInt())/(wheel_circumference * 3.14 * 60));
        Serial.print("transmission received");
    }

This way you can only assign to the global version. I'm not sure why this didn't throw some kind of warning. I'm not an arduino guy, but make sure all warnings are enabled. This should have at least triggered an "unused variable" with -Wall. If this were gcc or clang, a more specific warning could be enabled with -Wshadow.

A few issues ...

1. Using 3.14 instead of (e.g.) M_PI
2. Sending the values in ASCII (as evidenced by the Serial.readStringUntil('\n'); further adds to the issue (better to send fixed size binary data values).
3. Although it is good to send debug information back via some debug port (e.g. the UART), the time to send/receive these perturbates the real timing that is needed for the motor control.

Having done this for a commercial robotics motor control system (control system using linux (and ROS)), sending position values to Arduino (equivalent system using FreeRTOS), the control is "sloppy/loose".

The Arduino must provide the PWM values at precise intervals. It must be able to handle missed/corrupted values sent by the RPi.

We found that the control system should [periodically] send "trajectory/velocity" values instead of position values. That is, send (e.g.) rotational velocity values (e.g. radians / second). Or, a "path", such as final floor position of the robot for the given leg of the journey.

When you send position values, you have a lot of fragile calculations to calculate the latency of the arrived values, based on port speed, departure timestamps (from the RPi) and arrival timestamps on the arduino.

Instead, have the arduino take the velocity values and calculate the PWM values itself.

Then, the arduino can calculate the PWM position values at a high rate, and only need control from the RPi if the RPi determines that the velocity/trajectory changes.

Consider this analogy ... Two people are in a car. One is the driver [Arduino] and the other is reading a map [RPi]. If the passenger says turn right at the corner, the passenger should not tell the driver how to implement the turn on a moment-by-moment basis. (e.g.) The passenger should not be sending commands such as: Turn steering wheel right to 30 degrees, apply braking force, hold for 1.3 seconds, return steering wheel to upright position, etc.