diff --git a/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/RepeatCommand.java b/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/RepeatCommand.java index 6de06c531f5..103a5c9d060 100644 --- a/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/RepeatCommand.java +++ b/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/RepeatCommand.java @@ -63,7 +63,12 @@ public boolean isFinished() { @Override public void end(boolean interrupted) { - m_command.end(interrupted); + // Make sure we didn't already call end() (which would happen if the command finished in the + // last call to our execute()) + if (!m_ended) { + m_command.end(interrupted); + m_ended = true; + } } @Override diff --git a/wpilibNewCommands/src/main/native/cpp/frc2/command/RepeatCommand.cpp b/wpilibNewCommands/src/main/native/cpp/frc2/command/RepeatCommand.cpp index 319a7af46d9..f9b57c3e198 100644 --- a/wpilibNewCommands/src/main/native/cpp/frc2/command/RepeatCommand.cpp +++ b/wpilibNewCommands/src/main/native/cpp/frc2/command/RepeatCommand.cpp @@ -39,7 +39,12 @@ bool RepeatCommand::IsFinished() { } void RepeatCommand::End(bool interrupted) { - m_command->End(interrupted); + // Make sure we didn't already call end() (which would happen if the command + // finished in the last call to our execute()) + if (!m_ended) { + m_command->End(interrupted); + m_ended = true; + } } bool RepeatCommand::RunsWhenDisabled() const { diff --git a/wpilibNewCommands/src/test/java/edu/wpi/first/wpilibj2/command/RepeatCommandTest.java b/wpilibNewCommands/src/test/java/edu/wpi/first/wpilibj2/command/RepeatCommandTest.java index f082ddb92ae..53abdd8dc9e 100644 --- a/wpilibNewCommands/src/test/java/edu/wpi/first/wpilibj2/command/RepeatCommandTest.java +++ b/wpilibNewCommands/src/test/java/edu/wpi/first/wpilibj2/command/RepeatCommandTest.java @@ -14,54 +14,69 @@ class RepeatCommandTest extends CommandTestBase implements SingleCompositionTestBase { @Test void callsMethodsCorrectly() { - var initCounter = new AtomicInteger(0); - var exeCounter = new AtomicInteger(0); - var isFinishedCounter = new AtomicInteger(0); - var endCounter = new AtomicInteger(0); - var isFinishedHook = new AtomicBoolean(false); + try (CommandScheduler scheduler = new CommandScheduler()) { + var initCounter = new AtomicInteger(0); + var exeCounter = new AtomicInteger(0); + var isFinishedCounter = new AtomicInteger(0); + var endCounter = new AtomicInteger(0); + var isFinishedHook = new AtomicBoolean(false); - final var command = - new FunctionalCommand( - initCounter::incrementAndGet, - exeCounter::incrementAndGet, - interrupted -> endCounter.incrementAndGet(), - () -> { - isFinishedCounter.incrementAndGet(); - return isFinishedHook.get(); - }) - .repeatedly(); + final var command = + new FunctionalCommand( + initCounter::incrementAndGet, + exeCounter::incrementAndGet, + interrupted -> endCounter.incrementAndGet(), + () -> { + isFinishedCounter.incrementAndGet(); + return isFinishedHook.get(); + }) + .repeatedly(); - assertEquals(0, initCounter.get()); - assertEquals(0, exeCounter.get()); - assertEquals(0, isFinishedCounter.get()); - assertEquals(0, endCounter.get()); + assertEquals(0, initCounter.get()); + assertEquals(0, exeCounter.get()); + assertEquals(0, isFinishedCounter.get()); + assertEquals(0, endCounter.get()); - CommandScheduler.getInstance().schedule(command); - assertEquals(1, initCounter.get()); - assertEquals(0, exeCounter.get()); - assertEquals(0, isFinishedCounter.get()); - assertEquals(0, endCounter.get()); + scheduler.schedule(command); + assertEquals(1, initCounter.get()); + assertEquals(0, exeCounter.get()); + assertEquals(0, isFinishedCounter.get()); + assertEquals(0, endCounter.get()); - isFinishedHook.set(false); - CommandScheduler.getInstance().run(); - assertEquals(1, initCounter.get()); - assertEquals(1, exeCounter.get()); - assertEquals(1, isFinishedCounter.get()); - assertEquals(0, endCounter.get()); + isFinishedHook.set(false); + scheduler.run(); + assertEquals(1, initCounter.get()); + assertEquals(1, exeCounter.get()); + assertEquals(1, isFinishedCounter.get()); + assertEquals(0, endCounter.get()); - isFinishedHook.set(true); - CommandScheduler.getInstance().run(); - assertEquals(1, initCounter.get()); - assertEquals(2, exeCounter.get()); - assertEquals(2, isFinishedCounter.get()); - assertEquals(1, endCounter.get()); + isFinishedHook.set(true); + scheduler.run(); + assertEquals(1, initCounter.get()); + assertEquals(2, exeCounter.get()); + assertEquals(2, isFinishedCounter.get()); + assertEquals(1, endCounter.get()); - isFinishedHook.set(false); - CommandScheduler.getInstance().run(); - assertEquals(2, initCounter.get()); - assertEquals(3, exeCounter.get()); - assertEquals(3, isFinishedCounter.get()); - assertEquals(1, endCounter.get()); + isFinishedHook.set(false); + scheduler.run(); + assertEquals(2, initCounter.get()); + assertEquals(3, exeCounter.get()); + assertEquals(3, isFinishedCounter.get()); + assertEquals(1, endCounter.get()); + + isFinishedHook.set(true); + scheduler.run(); + assertEquals(2, initCounter.get()); + assertEquals(4, exeCounter.get()); + assertEquals(4, isFinishedCounter.get()); + assertEquals(2, endCounter.get()); + + scheduler.cancel(command); + assertEquals(2, initCounter.get()); + assertEquals(4, exeCounter.get()); + assertEquals(4, isFinishedCounter.get()); + assertEquals(2, endCounter.get()); + } } @Override diff --git a/wpilibNewCommands/src/test/native/cpp/frc2/command/RepeatCommandTest.cpp b/wpilibNewCommands/src/test/native/cpp/frc2/command/RepeatCommandTest.cpp index b71598314d7..b5456fad335 100644 --- a/wpilibNewCommands/src/test/native/cpp/frc2/command/RepeatCommandTest.cpp +++ b/wpilibNewCommands/src/test/native/cpp/frc2/command/RepeatCommandTest.cpp @@ -60,6 +60,19 @@ TEST_F(RepeatCommandTest, CallsMethodsCorrectly) { EXPECT_EQ(3, exeCounter); EXPECT_EQ(3, isFinishedCounter); EXPECT_EQ(1, endCounter); + + isFinishedHook = true; + scheduler.Run(); + EXPECT_EQ(2, initCounter); + EXPECT_EQ(4, exeCounter); + EXPECT_EQ(4, isFinishedCounter); + EXPECT_EQ(2, endCounter); + + command.Cancel(); + EXPECT_EQ(2, initCounter); + EXPECT_EQ(4, exeCounter); + EXPECT_EQ(4, isFinishedCounter); + EXPECT_EQ(2, endCounter); } INSTANTIATE_SINGLE_COMMAND_COMPOSITION_TEST_SUITE(RepeatCommandTest,