[cmd] Fix RepeatCommand calling end() twice

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

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 {

wpilibNewCommands/src/test/java/edu/wpi/first/wpilibj2/command/RepeatCommandTest.java

@@ -14,54 +14,69 @@ class RepeatCommandTest extends CommandTestBase
     implements SingleCompositionTestBase<RepeatCommand> {
   @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

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,