Coding Help from more experienced coders

Over the last couple of weeks, I and my team have been working on using a GSP sensor for our robot skills challenge. I have got the code mostly finalized but I wanted to see if anything looks off to anyone. In theory, the PD values are the only things I need to adjust before it will fully work. The code below is the most up-to-date version of my code with comments to help understand what is going on.

#include "vex.h"    // Include vex library to use vex functions
#include <cmath>    // Include cmath library to use math functions

float currentX;    // initialize variable
float currentY;    // initialize variable
float currentA;    // initialize variable
float checkX;    // initialize variable
float checkY;    // initialize variable
float checkA;    // initialize variable
float ErrorXCheck;    // initialize variable
float ErrorYCheck;    // initialize variable
float ErrorACheck;    // initialize variable
double derivative_of_error;
double prevError;
double speed;

int gpsTask(){   // define gpsQuality() function
  while(1){
    if(gpsL.quality() > gpsR.quality()) {    // check if the quality of gpsL is greater than gpsR or not
      currentX = gpsL.xPosition(inches);    // Assign value of x position of gpsL to currentX variable
      currentY = gpsL.yPosition(inches);    // Assign value of y position of gpsL to currentY variable
      currentA = gpsL.heading(degrees);    // Assign value of heading of gpsL to currentA variable
      Brain.Screen.printAt(1, 20, "Absolute X: %f inches", currentX);
      Brain.Screen.printAt(1, 40, "Absolute Y: %f inches", currentY);
      Brain.Screen.printAt(1, 60, "Absolute Rotation: %f Radians, %f Degrees", currentA * M_PI/180, currentA);
    } else {
      currentX = gpsR.xPosition(inches);    // Assign value of x position of gpsR to currentX variable
      currentY = gpsR.yPosition(inches);    // Assign value of y position of gpsR to currentY variable
      currentA = gpsR.heading(degrees);    // Assign value of heading of gpsR to currentA variable
      Brain.Screen.printAt(1, 200, "Absolute X: %f inches", currentX);
      Brain.Screen.printAt(1, 220, "Absolute Y: %f inches", currentY);
      Brain.Screen.printAt(1, 240, "Absolute Rotation: %f Degrees", currentA);
    }
  }
}

void setDriveSpeed( float LFSpeed, float RFSpeed, float RBSpeed, float LBSpeed , float RMSpeed, float LMSpeed) {    // Define setDriveSpeed() function
  LF.spin(fwd, LFSpeed, pct);    // Set speed of LF to LFSpeed
  LB.spin(fwd, LBSpeed, pct);    // Set speed of LB to LBSpeed
  RB.spin(fwd, RBSpeed, pct);    // Set speed of RB to RBSpeed
  RF.spin(fwd, RFSpeed, pct);    // Set speed of RF to RFSpeed
  LM.spin(fwd, LMSpeed, pct);    // Set speed of LM to RBSpeed
  RM.spin(fwd, RMSpeed, pct);    // Set speed of RM to RFSpeed
}

void driveStop() {    // define driveStop() function
  LF.stop(hold);    // Stop LF motor
  LB.stop(hold);    // Stop LB motor
  RB.stop(hold);    // Stop RB motor
  RF.stop(hold);    // Stop RF motor
  RM.stop(hold);    // Stop RM motor
  LM.stop(hold);    // Stop LM motor
}

float PD(double error, double kp, double kd){
  derivative_of_error = (error - prevError);
  prevError = error;

  speed = (kp * error) + (kd * derivative_of_error);

  return speed;
}

void goToPosition(float targetX, float targetY, float targetA, float timeoutsec) {    // define goToPosition() function
  Brain.setTimer(0,sec);    // Set timer 0 to sec
  float errorX = targetX - currentX;    // Calculate the error in X axis
  float errorY = targetY - currentY;    // Calculate the error in Y axis
  float errorA = targetA - currentA;    // Calculate the error in heading
  float lastErrorX = errorX;    // initialize variable lastErrorX with errorX
  float lastErrorY = errorY;    // initialize variable lastErrorY with errorY
  float lastErrorA = errorA;    // initialize variable lastErrorA with errorA
  double kPX = 1.35; 
  double kDX = .05;    
  double kPY = 1.35;    
  double kDY = .05;    
  double kPA = .13;    
  double kDA = 0.007;    
  float maxAngle = 20;    // initialize variable maxAngle with 60
  float maxX = 20;    // initialize variable maxX with 80
  float maxY = 20;    // initialize variable maxY with 80
  while (Brain.timer(sec) < timeoutsec) {    // Loop will run till timer 0 value is less than timeoutsec
    errorX = targetX - currentX;    // Calculate the error in X axis
    errorY = targetY - currentY;    // Calculate the error in Y axis
    errorA = targetA - currentA;    // Calculate the error in heading
    ErrorXCheck = errorX; ErrorYCheck = errorY; ErrorACheck = errorA;    // Assign values to Error variables
    
    float radA = currentA * (M_PI/180);    // Calculate radian value of currentA
    float outputX = PD(errorY * std::cos(radA) + errorX * std::sin(radA), kPX, kDX);    // Calculate outputX
    float outputY = PD(errorX * std::cos(radA) - errorY * std::sin(radA), kPY, kDY);    // Calculate outputY
    float outputA = PD(errorA * 1.5, kPA, kDA);     // Calculate outputA
    
    checkX = outputX; checkY = outputY; checkA = outputA;    // Assign values to check variables
    
    float motorSpeedLF = outputX + outputY + outputA;  //LF    // Calculate LF motor speed
    float motorSpeedRF = outputX - outputY - outputA; //RF    // Calculate RF motor speed
    float motorSpeedRB = outputX + outputY - outputA; //RB    // Calculate RB motor speed
    float motorSpeedLB = outputX - outputY + outputA;  //LB    // Calculate LB motor speed
    float motorSpeedRM = outputX - outputA; //RM    // Calculate RM motor speed
    float motorSpeedLM = outputX + outputA;  //LM    // Calculate LM motor speed
    
    if(outputA > maxAngle) {outputA = maxAngle;}    // Check if outputA is greater than maxAngle or not
    if(outputA < -maxAngle) {outputA = -maxAngle;}    // Check if outputA is less than -maxAngle or not
    if(outputY > maxY) {outputY = maxY;}    // Check if outputY is greater than maxY or not
    if(outputY < -maxY) {outputY = -maxY;}    // Check if outputY is less than -maxY or not
    if(outputX > maxX) {outputX = maxX;}    // Check if outputX is greater than maxX or not
    if(outputX < -maxX) {outputX = -maxX;}    // Check if outputX is less than -maxX or not
    
      setDriveSpeed( motorSpeedLF, motorSpeedRF, motorSpeedRB, motorSpeedLB, motorSpeedRM, motorSpeedLM);    // Call function setDriveSpeed()
    
    lastErrorX = errorX;    // Assign errorX value to lastErrorX
    lastErrorY = errorY;    // Assign errorY value to lastErrorY
    lastErrorA = errorA;    // Assign errorA value to lastErrorA
  }
  setDriveSpeed(0, 0, 0, 0, 0, 0);    // Call function setDriveSpeed()
  driveStop();    // Call function driveStop()
 }

int positionTracking() {    // Define function positionTracking()
  while(1){
    //gpsTask();

    Brain.Screen.printAt(1, 20, "Absolute X: %f inches", currentX);
    Brain.Screen.printAt(1, 40, "Absolute Y: %f inches", currentY);
    Brain.Screen.printAt(1, 60, "Absolute Rotation: %f Radians, %f Degrees", currentA * M_PI/180, currentA);
    Brain.Screen.printAt(1, 80, "Speed X: %f percent", checkX);
    Brain.Screen.printAt(1, 100, "Speed Y: %f percent", checkY);
    Brain.Screen.printAt(1, 120, "Speed angle: %f percent", checkA);
    Brain.Screen.printAt(1, 140, "Error X: %f inches", ErrorXCheck);
    Brain.Screen.printAt(1, 160, "Error Y: %f inches", ErrorYCheck);
    Brain.Screen.printAt(1, 180, "Error angle: %f inches", ErrorACheck);
    Brain.Screen.printAt(1, 180, "Inertial heading: %f degrees", Inertial.heading());
    
  }
}

Frankly, you probably should try running the code and seeing if it works first. At a first glance it looks fine, but I can’t really give you more insight than your linter can.

- m

3 Likes

I have tried to run it and for the most part the robot drive to the position but it seems to struggle with getting to the exact position and staying there. This is even more noticeable if I run the code with several places for it to go to

1 Like

First, a couple of general style points, for constants that aren’t going to change, define them outside your function as a constant at the start of the code like

const double kPX = 1.35;
const float MAX_ANGLE = 20;

etc

secondly, your code to cap the values to the maximum

if(outputA > maxAngle) {outputA = maxAngle;}

takes place after you make use of the values, so as far as I can tell that code isn’t doing anything

Regarding your issues, can you be more specific about how exactly it struggles to get to a specific position? Does it stop before it reaches the point, does it oscillate and jitter around the point, what? It’s difficult to diagnose failures in a PID loop without seeing the exact behavior. Ideally if you could send a video of the behavior, that might give some clues.

EDIT: Oh, after thinking about it a bit more, I think there’s an issue in the way you’re calculating angle error. If you’re at a heading of ten degrees and want a heading of 350 degrees, your code would interpret that as an error of 340 degrees and want you to nearly spin in a full circle clockwise. When in reality it’s an error of 20 degrees counterclockwise. Fixing this issue is left as an exercise for the reader.

4 Likes

Wait, no, I just figured out why it’s broken. You’re not doing the previous error correctly.

float PD(double error, double kp, double kd){
  derivative_of_error = (error - prevError);
  prevError = error;

  speed = (kp * error) + (kd * derivative_of_error);

  return speed;
}

You’re storing prevError as a global variable. That means you’re using the same previous error values for all three PD loops.

You run it for x with an error of 10 and the previous error is now 10.
Then when you go to run it for y with an error of 0 the previous error is 10, and it now becomes 0.
Then you run it for A with an error of 50, the previous error is 0, and it becomes 50.
Then you run it for x again with an error of, let’s say 5 now, and it thinks the previous error is 50 instead of 10 so your PD loop is saying “slow down we just moved 45 units!” when it really only moved 5.

This is making it behave incredibly unstably, as it’s trying to slow down/speed up based on the error of other axes.

You need to pass previous error using your lastErrorX or whatever variable, not use a global variable. In addition to the other less substantial changes I mentioned in the previous post.

5 Likes

Thank you I knew it was something dumb like that I will fix that and give it a try

2 Likes

This is the updated code that I made after listening to all your suggestions. I make the variables constant global variables then I made a Limit function to make things look better. The max function only caps the speed so if it’s higher it just sets it to the max speed till it becomes less than that speed. I also fixed the PD loop by assigning x,y, and angle their own variables. Does this look good now?

#include "vex.h"    
#include <cmath>    

const float maxAngle = 20;    
const float maxX = 20;    
const float maxY = 20;    
float currentX;    
float currentY;    
float currentA;    

//PD variables
  const double kPX = 1.35; 
  const double kDX = .05;    
  const double kPY = 1.35;    
  const double kDY = .05;    
  const double kPA = .13;    
  const double kDA = 0.007;    

int gpsTask(){  
  while(1){
    if(gpsL.quality() > gpsR.quality()) {   
      currentX = gpsL.xPosition(inches);   
      currentY = gpsL.yPosition(inches);   
      currentA = gpsL.heading(degrees); 
      Brain.Screen.printAt(1, 200, "Absolute X: %f inches", currentX);
      Brain.Screen.printAt(1, 220, "Absolute Y: %f inches", currentY);
      Brain.Screen.printAt(1, 240, "Absolute Rotation: %f Radians, %f Degrees", currentA * M_PI/180, currentA);
    } else {
      currentX = gpsR.xPosition(inches);  
      currentY = gpsR.yPosition(inches);   
      currentA = gpsR.heading(degrees);    
      Brain.Screen.printAt(1, 200, "Absolute X: %f inches", currentX);
      Brain.Screen.printAt(1, 220, "Absolute Y: %f inches", currentY);
      Brain.Screen.printAt(1, 240, "Absolute Rotation: %f Degrees", currentA);
    }
  }
}

void setDriveSpeed( float LFSpeed, float RFSpeed, float RBSpeed, float LBSpeed , float RMSpeed, float LMSpeed) {   
  LF.spin(fwd, LFSpeed, pct);   
  LB.spin(fwd, LBSpeed, pct);   
  RB.spin(fwd, RBSpeed, pct);   
  RF.spin(fwd, RFSpeed, pct);  
  LM.spin(fwd, LMSpeed, pct);    
  RM.spin(fwd, RMSpeed, pct);   
}

void driveStop() {    
  LF.stop(hold);   
  LB.stop(hold);   
  RB.stop(hold);    
  RF.stop(hold);   
  RM.stop(hold);   
  LM.stop(hold);    
}

double PD(double error, double kp, double kd, double previous_error)
{
	double proportional_error = error * kp;
	double derivative_error = (error - previous_error) * kd;
	double speed = proportional_error + derivative_error;
	previous_error = error;
	return speed;
}

void Limit(float output, float Max) {
  if(output > Max) {
    output = Max;
  }    
  if(output < -Max) {
     output = -Max;
  }   
}

float reduceAngleMinus180to180( float angleDeg ) {
  while(!(angleDeg >= -180 && angleDeg < 180)) {
    if( angleDeg < -180 ) { angleDeg += 360; }
    if(angleDeg >= 180) { angleDeg -= 360; }
  }
  return(angleDeg);
}

void goToPosition(float targetX, float targetY, float targetA, float timeoutsec) {
  Brain.setTimer(0,sec);  
  float errorX = targetX - currentX;    
  float errorY = targetY - currentY;    
  float errorA = reduceAngleMinus180to180(targetA) - reduceAngleMinus180to180(currentA);    
  float lastErrorX = errorX;   
  float lastErrorY = errorY;    
  float lastErrorA = errorA;    
  while (Brain.timer(sec) < timeoutsec) {    
    errorX = targetX - currentX;  
    errorY = targetY - currentY;   
    errorA = reduceAngleMinus180to180(targetA) - reduceAngleMinus180to180(currentA);  
    
    float radA = currentA * (M_PI/180);   
    float outputX = PD(errorY * std::cos(radA) + errorX * std::sin(radA), kPX, kDX, lastErrorX);
    float outputY = PD(errorX * std::cos(radA) - errorY * std::sin(radA), kPY, kDY, lastErrorY);   
    float outputA = PD(errorA * 1.5, kPA, kDA, lastErrorA);     
    
    float motorSpeedLF = outputX + outputY + outputA;  //LF    
    float motorSpeedRF = outputX - outputY - outputA; //RF    
    float motorSpeedRB = outputX + outputY - outputA; //RB   
    float motorSpeedLB = outputX - outputY + outputA;  //LB   
    float motorSpeedRM = outputX - outputA; //RM    
    float motorSpeedLM = outputX + outputA;  //LM    
    
    Limit(outputX, maxX);
    Limit(outputY, maxY);  
    Limit(outputA, maxAngle);    
    
      setDriveSpeed( motorSpeedLF, motorSpeedRF, motorSpeedRB, motorSpeedLB, motorSpeedRM, motorSpeedLM);   
    
    lastErrorX = errorX;   
    lastErrorY = errorY;    
    lastErrorA = errorA;   
  }
  setDriveSpeed(0, 0, 0, 0, 0, 0);   
  driveStop();    
 }

I think you need to get a better understanding of variable scope. This is going to get a little bit advanced, but I think you’re mixing some things up

There are (to oversimplify) two kinds of variables, data and pointers. Data is simple, it’s just like a number or something. Pointers are more complicated, because they’re not actually data, they’re just the location in memory where data is.

foo(int bar){
      bar = bar + 2;
}

int buzz = 2;
foo(buzz);

What you seem to think is happening is you send the variable buzz into the foo function, it adds 2 to it, and now buzz is 4. That’s not the case, because the thing being sent isn’t buzz, it’s just the value of buzz. So buzz is still 2.

Now, what I think has happened is you once used something that was a pointer to like a class or something more advanced. So then, if you pass a pointer to something, you’re actually just saying “here’s the location of this”, so when you make changes to it, then it’ll make changes to the original variable. I’m used to seeing the opposite error, where you don’t realize that you’re making changes to the original because it’s a pointer, but in your case you seem to think everything acts like a pointer.

So instead you should return the output from your limit function, and do it like
outputX = limit(outputX, maxX)

These videos might be helpful, and honestly I just recommend finding a basic C/C++ course on YouTube and watching it

EDIT: Also, I don’t think you understood the problem with the limit function

int foo = 100; // foo is 100
int bar = 100; // foo is 100, bar is 100

int buzz = foo + bar; // foo is 100, bar is 100, buzz is 200

if(foo > 50){
   foo = 50; // foo is 50, bar is 100, buzz is still 200
}

doThingWithBuzz(buzz); // buzz is still 200

Limiting foo to 50 after you already calculate buzz with it doesn’t do anything, because you’ve already calculated buzz with it with the value greater than the maximum.

3 Likes