Hello, we are trying to use a sequential command group, but we need an initialize method in that command. Is that a thing or will we need to find a different solution? Thanks!
Can you elaborate on what you’re trying to do? A constructor might be enough, or perhaps theres a better pattern you could follow that doesnt require it at all.
Why not just add another command to the start of the sequence that does what you need?
we are trying to generate a trajectory on the fly using our current position and some other point on the field. The problem we are running into when trying to still use the PPSwerveControllerCommand all of the path generation runs in the classloader. This means that when we move, the pid then overcorrects trying to get from the current position of the robot to the starting position of the trajectory and it doesn’t take our current position because the trajectory has already been made, any suggestions?
Our current fix is to rewrite the class, but that means we have a different On the Fly Auto command that we use during teleop, and another command for Auto during Auto, The on the fly works, but we are wondering if there is a more elegant solution that would allow us to reuse a single command for both. While also not having a ceiling as we implement A star for teleop on the fly auto
An on-the-fly trajectory-following command that takes the current position into account seems like the best plan.
Could you expand on why that’s not suitable for use during auto? Is it a matter of slowing down auto unnecessarily with the initialisation time?
We were originally just aiming to use the PPSwerveControllerCommand, as that allows us to use markers and stop points and all of those lovely features without having to rewrite that code ourselves. We were hoping we wouldn’t have to keep them decoupled and go back to exclusively using the PPSwerveControllerCommand instead of rewriting a command, where the trajectory is generated on init of our command, and then executed using a PPSwerveController in our drive sub. We don’t know how to generate a trajectory on the fly such that it isn’t generated in the class loader, and is generated on button press so that it updates with the current drive pose, and makes a new trajectory using that pose.
Not quite sure what you mean. You can easily “superimpose” your trajectory to your robot by simply applying desired pose to the odometry, essentially telling the trajectory - “I am at that pose”. That means when starting, trajectory controller will consider your current pose to be correct, whatever you told it to. For instance, you can read the first pose from the trajectory file, and say - “I am there”. Then when you run the trajectory, the robot will believe you’re at the beginning of a trajectory, rather than somewhere else. if you need an example for that, I can provide that.
If you want to do corrective actions during trajectory run, you can actually do so as well. For that you can subclass PPSwerveControllerCommand into a “regular” command. That means you would need to overload all of the “normal” command methods and constructor, but not forget to do a “super…” call on them, so it will run whatever logic PPSwerveControllerCommand suppose to do there (especially “execute” and “isFinished”). Again, if you need an example, I can provide that as well (ours is done on the Ramsete chassis, but swerve one is done pretty much the same way).
Update: As Oblarg points out below, this trick doesn’t work because we’re trying to override a method that is (now) final.
I can’t see the code for your on-the-fly command, but I’d guess it looks something like:
class OnTheFly extends SequentialCommandGroup {
Destination m_destination;
public OnTheFly(Destination destination, ...other args...) {
m_destination = destination;
}
@Override
public void initialize() {
PathPlannerTrajectory trajectory = generateTrajectory(m_destination);
Command command = new PPSwerveControllerCommand(trajectory, ...other args...);
addCommands(command);
// Must call super method when overriding command group life cycle method.
super.initialize();
}
};
You can’t override the lifecycle methods of command groups; it’s a huge footgun because the things break if you fail to call super.
You should solve this problem with composition, not subclassing.
I agree that overriding the life-cycle methods of command groups should be done only with great caution. In this case, we overrode only one lifecycle method and we did call super
, but your point is well taken.
I’m pretty sure in 2023 those methods are final
and you just flatly cannot do this.
You’re right. It’s final
. That’s what I get for posting untested code.
But you could certainly write a utility class to bridge this gap. I perhaps should have written something more like this.
To shed some more light, what we are doing currently is(be ready for some horrible psuedo code, I don’t have access to the repo right now.)
on button press(new On the fly command)
in auto(run auto command)
our auto command, uses the PPSwerveControllerCommand, with similar structure to that of a wpi ramsete command.
Our on the fly command uses the PPSwerveController, with all of the command generated by us, i.e. sampling the command at different times and running that. On init makes the trajectory between current pose and specified end pose.
The generation of the trajectory is harder because we don’t want to generate that trajectory in robot container, it just doesn’t seem right, and we can’t generate it in the constructor, as the constructor doesn’t seem to run when we call “new” on button press. (note we are using a sequential command group to set the motor speeds to 0 after the trajectory has finished, as path planner does not do that by default and sometimes the pids don’t get within tolerance.)
This isn’t a problem in auto because between the time the class is loaded and the class is ran, our position doesn’t change, but during teleop, we don’t know how to generate the trajectory in the sequential command group using the integrated PPSwerveControllerCommand, as there is no place to generate the trajectory, as we don’t want to do it in robot container, we can’t do it in the constructor, and we can’t mutate the PPSwerveControllerCommand to generate it for us at least, that wasn’t the intent.
For these reasons, we don’t believe it is possible or really, elegant without either A. making a class that returns the new trajectory, and passing something like new Trajectoryclass.generateTrajectory(Pose2d endpose) which I don’t even know if you can access a method in that way using the new keyword. OR B. just using 2 separate classes. Or C. Extending PPSwerveControllerCommand, and using init to generate our trajectory and then calling super on everything else(or however that works out syntactically)
I don’t know how much sense this makes, let me know If i can provide clarity, I will post the code later if necessary. Thank you.
Write a static factory method that returns the fully-composed command at runtime.
How would you supply the PPSwerveControllerCommand with the trajectory in this implementation?
The same way you would do it if you were making an “ordinary” command in RobotContainer, just inside of a method body.
I’m not familiar with the PathPlanner library in particular, but if you can create a command at boot time you can also do it at runtime.
Oh, that makes sense, I think I was misunderstanding, thank you!
We did essentially this, and realized the problem. What does seem to work for us:
- create a function which creates the command you need
- wrap that in a ProxyCommand.
So your function might be roughly:
Command makeTrajFollow() {
Pose2d currPose = getPose();
// …
return new TrajCommand( …);
}
And then where you construct the command (eg bound to a button):
button.onTrue( new ProxyCommand(() → makeTrajFollow()))
The “makeTrajFollow” will get called when the button is pressed so it can fetch the actual current Pose and go from there.
You can skip the ProxyCommand if you have the whole end composition itself returned from a factory. This way you don’t clutter your binding statements with new
, as an added benefit.