SequentialCommandGroup.isFinished() Unexpected Behavior

[GregoryLi360]

Describe the bug SequentialCommandGroup.isFinished() does not return true when all commands in the command group are finished.

To Reproduce Steps to reproduce the behavior:

1. Create a new SequentialCommandGroup
2. Schedule the SequentialCommandGroup

3. Check if the SequentialCommandGroup is finished when all commands in it are finished

   • Link to code:

     
     var commandGroup = new InstantCommand().andThen(new InstantCommand());
     commandGroup.setName("instant commands group");
     kB.onTrue(commandGroup);

kA.onTrue(new InstantCommand(() -> { System.out.println("command group finished: " + commandGroup.isFinished()); }));

**Expected behavior**
SequentialCommandGroup.isFinished() should return true when all commands in the SequentialCommandGroup are finished.

**Desktop (please complete the following information):**
 - OS: MacOS 14
 - Project Information: 
 WPILib Information:
Project Version: 2024.3.1
VS Code Version: 1.85.1
WPILib Extension Version: 2024.1.1
C++ Extension Version: 1.19.1
Java Extension Version: 1.26.2023121408
Java Debug Extension Version: 0.55.2023121302
Java Dependencies Extension Version 0.23.2023120100
Java Version: 17
Java Location: ~/wpilib/2024/jdk
Vendor Libraries:
    NavX (2024.1.0)
    CTRE-Phoenix (v6) (24.1.0)
    REVLib (2024.2.0)
    WPILib-New-Commands (1.0.0)
    photonlib (v2024.2.8)

[Starlight220]

Calling isFinished after end is undefined behavior. What are you trying to do?

[GregoryLi360]

I’m not sure why that would be undefined behavior.

I am trying to create an inner SequentialCommandGroup that will execute when the outer command computes a value that needs to use the periodic game loop. When the inner SequentialCommandGroup ends, I want the outer command to also end.

[Starlight220]

Why can't you have them composed (possibly by proxy if you understand the implications of proxying) or merged?

It's undefined because the state of a command object is defined and valid only while it is running, meaning between initialize and end.

[GregoryLi360]

I wanted the outer and inner structure so that the computed value will be contained within the scope of one simple outer command while the inner SequentialCommandGroup is a composition of reusable components.

At the core, I think SequentialCommandGroup.isFinished() not being true is causing our auto sequence to stop at that specific command.

[Starlight220]

You shouldn't be calling isFinished directly.

I wanted the outer and inner structure so that the computed value will be contained within the scope of one simple outer command while the inner SequentialCommandGroup is a composition of reusable components.

Why can't the two be composed in a sequence?

[GregoryLi360]

In that case would I have to pass a setter function into the first command?

[skajacore]

I also expected the isFinished() function to be useful for determining if a command sequence has finished. The documentation for the function is misleading. It says that the return value for this public function indicates whether the command has finished. It is a just a wrapper function for the value false and worse leads to undefined behavior?

[Starlight220]

The documentation is intended so teams know how to implement it -- it is not intended to be called by team code!

Team code shouldn't be holding references to commands at all, imo, and definitely not calling internal lifecycle methods on them. The C++ ownership semantics do this the right way.

---

If you want to do something when a sequence (or any other command) ends, there are several ways to do this without undefined behavior:

• finallyDo/handleInterrupt (for lambdas)
• put a command after in sequence, adding indirection via ProxyCommand/ScheduleCommand if needed