wpilibsuite / allwpilib

Official Repository of WPILibJ and WPILibC
https://wpilib.org/
Other
1.05k stars 611 forks source link

Using andThen on SequentialCommandGroup crashes #6524

Closed railroad890 closed 4 months ago

railroad890 commented 4 months ago

Describe the bug When using andThen on a previously created SequentialCommandGroup object, the code crashes, indicating an Originally composed at: error. Full stack trace:

Error at frc.robot.RobotContainer.emptyCommandTest(RobotContainer.java:77): Unhandled exception: java.lang.Exception: Originally composed at:
    at edu.wpi.first.wpilibj2.command.CommandScheduler.registerComposedCommands(CommandScheduler.java:599)
    at edu.wpi.first.wpilibj2.command.SequentialCommandGroup.addCommands(SequentialCommandGroup.java:47)
    at edu.wpi.first.wpilibj2.command.SequentialCommandGroup.<init>(SequentialCommandGroup.java:33)
    at edu.wpi.first.wpilibj2.command.Command.andThen(Command.java:252)
    at frc.robot.RobotContainer.emptyCommandTest(RobotContainer.java:77)
    at frc.robot.RobotContainer.<init>(RobotContainer.java:46)
    at frc.robot.Robot.robotInit(Robot.java:34)
    at edu.wpi.first.wpilibj.TimedRobot.startCompetition(TimedRobot.java:107)
    at edu.wpi.first.wpilibj.RobotBase.runRobot(RobotBase.java:365)
    at edu.wpi.first.wpilibj.RobotBase.lambda$startRobot$0(RobotBase.java:433)
    at java.base/java.lang.Thread.run(Thread.java:840)

To Reproduce Steps to reproduce the behavior:

  1. Create a new SequentialCommandGroup object.
  2. Use .andThen() on the SecquentialCommandGroup object

Expected behavior A new SequentialCommandGroup to be returned

Desktop:

Additional context When calling the same functions as andThen manually, the code does not crash. The code I used to test this is at the link above.

Starlight220 commented 4 months ago

You're composing the same command instance twice -- that correctly throws. (There's a bug that truncates the error message into a less helpful one, but that's reported and being fixed).

My recommendation is to never store commands in variables; it's a recipe for this type of mistake.

Another recommendation is to add all commands to the sequence at once rather than one at a time -- it's more succinct and doesn't allocate a bunch of objects that make trouble for the GC.

Read the documentation about command compositions, there's a lot of helpful info there.