PID controller for DC-motor

Other Hardware Motors, Mechanics, Power and CNC
1

Hello!
I am trying to make a "steer-by-wire" system using 2 potentiometers and a DC-motor. Im having trouble getting it to work after implementing PID (or PI). (It worked pretty good with only the "P" part).
It works kinda okay sometimes for a split secound or two, and then it just becomes "unresponsive". Do you know what could be wrong?

Thanks in advance!

Here is my code:

int enablePin = 9;
int motorLeft = 5;
int motorRight = 6;
int sensorPosition;                                // Input position of potentiometer
int motorPosition;                                 // Position of potentiometer connected to motor
int motorValue;                                     // Output value that the PID calculates.
int timeNow, timeThen, dT;
float P_error, I_error, D_error, lastError;

double Pexp = 3;
double Iexp = 0.01;
double Dexp = 0;


void setup() {
  analogWrite(enablePin, 255);
  Serial.begin(9600);
}


void loop() {
  sensorPosition = analogRead(A0);
  motorPosition = analogRead(A1);

  P_error = sensorPosition - motorPosition;
  motorValue = calcPID(P_error);

  if (motorValue < 0) {
    analogWrite(motorLeft, constrain((-motorValue), 0, 255));
  } else if (motorValue > 0) {
    analogWrite(motorRight, constrain(motorValue, 0, 255));
  }

 delay(100);

}

int calcPID(int P_error) {
  timeNow = millis();
  dT = (timeNow - timeThen);

  I_error += P_error * dT;

  //D_error = (P_error - lastError)/dT;

  int motorValue = (Pexp * P_error) + (Iexp * I_error) + (Dexp * D_error);

  lastError = P_error;
  timeThen = timeNow;

  return motorValue;
}
2

Post a wiring diagram (hand drawn is fine) and links to all the components.

Note that "dT" is usually measured in seconds, rather than milliseconds.

  dT = (timeNow - timeThen);

  I_error += P_error * dT;

And on an Arduino Uno or the like, these values will rapidly overflow and become alternately negative and positive, as millis() returns a 32 bit unsigned long time value. Declare timeNow and timeThen to be unsigned long instead.

int timeNow, timeThen, dT;
3

Thanks for your reply!
Im running an Uno board
I changed the dT to sec:

dT = (timeNow - timeThen) * 0.001;

Here comes the wiring diagram too, im using a TH-bridge SN754410NE:

wiring_diagram
wiring_diagram1034×762 171 KB

4

I think I figured out my initial problem by adding this to the first part of the if-statement and vice versa (for the "right turn" part):

analogWrite(motorRight, 0);

if (motorValue < 0 and motorPosition > 2) {
    analogWrite(motorRight, 0); 
    analogWrite(motorLeft, constrain(-motorValue, 0, 255));
    Serial.print("                                   V:  "), Serial.println(constrain(-motorValue, 0, 255));
  } else if (motorValue > 0 and motorPosition < 1022) {
    analogWrite(motorLeft, 0); 
    analogWrite(motorRight, constrain(motorValue, 0, 255));
    Serial.print("                                             H:  "), Serial.println(constrain(motorValue, 0, 255));
  }