PIDCommand Java Object life cycle

[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/s...1#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