PIDCommand Java Object life cycle

[cpapplefamily]

We have been exploring some code options most recent the PIDCommand. We have a PIDCommand [Drive_Spin_In_Place_PID] that is created when a button on our joystick is pressed and the command [x_Test_Lookup] creates the [Drive_Spin_In_Place_PID] and starts the command. In an effort to learn what is happening we are printing the value of Drive_Spin_In_Place_PID_count and see that the number increases each time we press the button. This follows what we might expect since the PIDCommand description tells us its a command that contains it's own PID loop for short lived PID uses. I wonder can we over do it burning up memory? Do the old objects PID's created from previous button presses still exist or are they removed for garbage collection? We not Java experts but learning a lot along the way. There are some admittedly parts of the code that don't work and need to be worked through.

public class x_Test_Lookup extends Command {

Drive_Spin_In_Place_PID spinToTarget_Command ;
public x_Test_Lookup() {
// Use requires() here to declare subsystem dependencies
// eg. requires(chassis);
requires(Robot.driveTrain);
}

// Called just before this Command runs the first time
protected void initialize() {

double cX = Robot.vision.my_Get_Xcenter();
double spin = Robot.vision.my_SpinToTarget_Lookup(cX);
System.out.println("spin = " + spin);
spinToTarget_Command = new Drive_Spin_In_Place_PID(spin*-1,false);
spinToTarget_Command.start();

}

// Called repeatedly when this Command is scheduled to run
protected void execute() {
//double a = Robot.vision.my_Get_Xcenter();
//SmartDashboard.putNumber("ANGLE_TO_TARGET",Robot.vision.my_SpinToTarget_Lookup(a));
}

// Make this return true when this Command no longer needs to run execute()
protected boolean isFinished() {
return false;
}

// Called once after isFinished returns true
protected void end() {
spinToTarget_Command.end();
}

// Called when another command which requires one or more of the same
// subsystems is scheduled to run
protected void interrupted() {
end();
}
}


public class Drive_Spin_In_Place_PID extends PIDCommand {
double m_setpoint;
private int scancount = 0;
static int Drive_Spin_In_Place_PID_count = 0;
public Drive_Spin_In_Place_PID(double setpoint,boolean absoult) {


super("Gyro_PID_Spin"+Drive_Spin_In_Place_PID_count, .04, 0.001, 0.0000, 0.02); //0.00002, 0.000002, 0.02);//, 0.00005, 0.0002, 0.02);
getPIDController().setContinuous(false);
getPIDController().setAbsoluteTolerance(.5);
getPIDController().setOutputRange(-0.75, 0.75);

// Use requires() here to declare subsystem dependencies
// eg. requires(chassis);
requires(Robot.driveTrain);

if(absoult){
System.out.println("absoult"+Drive_Spin_In_Place_PID_count);
m_setpoint = setpoint;
}else{
System.out.println("Not absoult"+Drive_Spin_In_Place_PID_count);
m_setpoint = Robot.driveTrain.my_Get_Gyro() + setpoint;
}
Drive_Spin_In_Place_PID_count++;

}

protected double returnPIDInput() {
// Return your input value for the PID loop
// e.g. a sensor, like a potentiometer:
// yourPot.getAverageVoltage() / kYourMaxVoltage

return Robot.driveTrain.my_Get_Gyro();
}

protected void usePIDOutput(double output) {
// Use output to drive your system, like a motor
// e.g. yourMotor.set(output);
if(timeSinceInitialized() > Robot.vision.light_Settle_Time){

Robot.driveTrain.my_Drive_Mecanum(0,0, output, 0);
}

}


// Called just before this Command runs the first time
protected void initialize() {
m_setpoint = Robot.driveTrain.my_Get_Gyro() + m_setpoint;
getPIDController().enable();
getPIDController().setSetpoint(m_setpoint);
}

// Called repeatedly when this Command is scheduled to run
protected void execute() {
}

// Make this return true when this Command no longer needs to run execute()
protected boolean isFinished() {
double gyro = Robot.driveTrain.my_Get_Gyro();

if(gyro>m_setpoint-.5 && gyro< m_setpoint +.5){
scancount = scancount+1;
if(scancount>10){
return true;
}else{
System.out.println("Scancount = "+scancount);
return false;
}
}else{
scancount = 0;
return false;
}
}

// Called once after isFinished returns true
protected void end() {
Robot.driveTrain.my_Drive_Mecanum(0,0,0,0);
}

// Called when another command which requires one or more of the same
// subsystems is scheduled to run
protected void interrupted() {
end();
}
}

[cpapplefamily]

I should included the OI


a_1 = new JoystickButton(xbox_1, 1);
a_1.whenPressed(new x_Test_Lookup());

[Waz]

Yes, the Drive_Spin_In_Place_PID instances will remain in the heap until collected. This gets into an interesting area in using Java for FRC. Typically, at least I have not seen it, garbage collection is not an issue. I believe this is because we have lots of space and relatively short run duration (a match). We typically do not produce enough garbage that collection becomes an issue.

However, as I mentor students in Java programming, I want them to become good Java programmers, so I strongly encourage them to worry about it. I have seen sloppy object lifecycle management practices drive long running Java systems (days, weeks, months) into the ground.

In this particular case I think you could avoid the issue by getting rid of the x_Test_Lookup class and connecting a single Drive_Spin_In_Place_PID command instance directly to the button. Something like:
a_1 = new JoystickButton(xbox_1, 1);
a_1.whenPressed(new Drive_Spin_In_Place_PID(false));

Note that I took the first parameter off the constructor. You can move the calculation of that value from x_Test_Lookup#initialize() to Drive_Spin_In_Place_PID#initialize().

One other note on something I think I see but I have not run this so I could be off base. Since both commands require the driveTrain subsystem, the start of the Drive_Spin_In_Place_PID command will cause the x_Test_Lookup command to be interrupted. When it is interrupted, it stops executing and its interrupted() method is called, which in your code calls its own end() method (very common practice) which in turn calls end() on the current Drive_Spin_In_Place_PID instance. This will NOT cause the Drive_Spin_In_Place_PID instance to stop. It will continue until some terminating condition is met (isFinished returns true, it is interrupted, etc). So things work as you expect but perhaps not for the reasons you think.

I hope this helps,
Steve

[cpapplefamily]

Thanks for the reply and the correct java terms "heap". My questions are to lean better practice and not form bad habits with the team. I was going to suggest we look at removing the command layers to get to what we want as you suggested. The purpose of the layers was to make this dynamic to respond to vision goal traking. We could set the setpiont in the initialize () and maybe update in the execute ().
The required duplicates are likely from hashing out different atempts. It also my answer why we seen some prints from our end () method before execution.
We will clean things up and test to see where it ends up. Do you have a suggestion to how we would make this dynamic like when we use a setPointCommand to control a PIDSubsytem? Maybe we shouldn't?

[Waz]

You have inadvertently asked a trick question. Yes, you are suppose to be able to reset your set point regularly and have the PID loop respond appropriately. However, there was a bug in wpilib this past season that made this problematic (see this thread: https://www.chiefdelphi.com/forums/showthread.php?p=1559101#post1559101).

It looks like the fix was merged into wpilib in May, so if you get the very latest wpilib code, you should be okay.

Has to how to do it, that depends on why you want to do it.

If you want the re-targeting to to be at direction of the driver, putting it on a button makes sense. You could have a command that simply sets a new suggested target value on the running command (would need to save a reference to it when you create it in OI) and then check apply the value in execute().

We had a use case where we were updating a shooter targeting set point as we drove around. We had a active targeting command that would run when we wanted this behavior and its execute would deal with vision and update the running targeting command's set point as needed. Or something like that, my memory is a bit hazy on the details.

I hope this helps,
Steve