[commands] Revamp Interruptible (#4192)

wpilibNewCommands/src/test/native/cpp/frc2/command/CommandRequirementsTest.cpp

@@ -7,6 +7,7 @@
 #include "CommandTestBase.h"
 #include "frc2/command/CommandScheduler.h"
 #include "frc2/command/ConditionalCommand.h"
+#include "frc2/command/FunctionalCommand.h"
 #include "frc2/command/InstantCommand.h"
 #include "frc2/command/ParallelCommandGroup.h"
 #include "frc2/command/ParallelDeadlineGroup.h"
@@ -49,26 +50,35 @@ TEST_F(CommandRequirementsTest, RequirementUninterruptible) {
 
   TestSubsystem requirement;
 
-  MockCommand command1({&requirement});
-  MockCommand command2({&requirement});
+  int initCounter = 0;
+  int exeCounter = 0;
+  int endCounter = 0;
 
-  EXPECT_CALL(command1, Initialize());
-  EXPECT_CALL(command1, Execute()).Times(2);
-  EXPECT_CALL(command1, End(true)).Times(0);
-  EXPECT_CALL(command1, End(false)).Times(0);
+  std::unique_ptr<Command> command1 =
+      FunctionalCommand([&initCounter] { initCounter++; },
+                        [&exeCounter] { exeCounter++; },
+                        [&endCounter](bool interruptible) { endCounter++; },
+                        [] { return false; }, {&requirement})
+          .WithInterruptBehavior(
+              Command::InterruptionBehavior::kCancelIncoming);
+  MockCommand command2({&requirement});
 
   EXPECT_CALL(command2, Initialize()).Times(0);
   EXPECT_CALL(command2, Execute()).Times(0);
   EXPECT_CALL(command2, End(true)).Times(0);
   EXPECT_CALL(command2, End(false)).Times(0);
 
-  scheduler.Schedule(false, &command1);
+  scheduler.Schedule(command1.get());
+  EXPECT_EQ(1, initCounter);
   scheduler.Run();
-  EXPECT_TRUE(scheduler.IsScheduled(&command1));
+  EXPECT_EQ(1, exeCounter);
+  EXPECT_TRUE(scheduler.IsScheduled(command1.get()));
   scheduler.Schedule(&command2);
-  EXPECT_TRUE(scheduler.IsScheduled(&command1));
+  EXPECT_TRUE(scheduler.IsScheduled(command1.get()));
   EXPECT_FALSE(scheduler.IsScheduled(&command2));
   scheduler.Run();
+  EXPECT_EQ(2, exeCounter);
+  EXPECT_EQ(0, endCounter);
 }
 
 TEST_F(CommandRequirementsTest, DefaultCommandRequirementError) {

wpilibNewCommands/src/test/native/cpp/frc2/command/CommandTestBase.h

@@ -10,6 +10,7 @@
 #include <frc/simulation/DriverStationSim.h>
 
 #include "frc2/command/CommandGroupBase.h"
+#include "frc2/command/CommandHelper.h"
 #include "frc2/command/CommandScheduler.h"
 #include "frc2/command/SetUtilities.h"
 #include "frc2/command/SubsystemBase.h"
@@ -26,7 +27,10 @@ class CommandTestBase : public ::testing::Test {
   class TestSubsystem : public SubsystemBase {};
 
  protected:
-  class MockCommand : public Command {
+  /**
+   * NOTE: Moving mock objects causes EXPECT_CALL to not work correctly!
+   */
+  class MockCommand : public CommandHelper<CommandBase, MockCommand> {
    public:
     MOCK_CONST_METHOD0(GetRequirements, wpi::SmallSet<Subsystem*, 4>());
     MOCK_METHOD0(IsFinished, bool());
@@ -65,7 +69,7 @@ class CommandTestBase : public ::testing::Test {
           .WillRepeatedly(::testing::Return(m_requirements));
     }
 
-    MockCommand(const MockCommand& other) : Command{other} {}
+    MockCommand(const MockCommand& other) : CommandHelper{other} {}
 
     void SetFinished(bool finished) {
       EXPECT_CALL(*this, IsFinished())
@@ -77,11 +81,6 @@ class CommandTestBase : public ::testing::Test {
       scheduler.Cancel(this);
     }
 
-   protected:
-    std::unique_ptr<Command> TransferOwnership() && {  // NOLINT
-      return std::make_unique<MockCommand>(std::move(*this));
-    }
-
    private:
     wpi::SmallSet<Subsystem*, 4> m_requirements;
   };

wpilibNewCommands/src/test/native/cpp/frc2/command/SchedulingRecursionTest.cpp

@@ -3,43 +3,51 @@
 // the WPILib BSD license file in the root directory of this project.
 
 #include "CommandTestBase.h"
+#include "frc2/command/Command.h"
 #include "frc2/command/CommandHelper.h"
 #include "frc2/command/RunCommand.h"
 #include "gtest/gtest.h"
 
 using namespace frc2;
 
-class SchedulingRecursionTest : public CommandTestBaseWithParam<bool> {};
+class SchedulingRecursionTest
+    : public CommandTestBaseWithParam<Command::InterruptionBehavior> {};
 
 class SelfCancellingCommand
     : public CommandHelper<CommandBase, SelfCancellingCommand> {
  public:
-  SelfCancellingCommand(CommandScheduler* scheduler, Subsystem* requirement)
-      : m_scheduler(scheduler) {
+  SelfCancellingCommand(CommandScheduler* scheduler, Subsystem* requirement,
+                        Command::InterruptionBehavior interruptionBehavior =
+                            Command::InterruptionBehavior::kCancelSelf)
+      : m_scheduler(scheduler), m_interrupt(interruptionBehavior) {
     AddRequirements(requirement);
   }
 
   void Initialize() override { m_scheduler->Cancel(this); }
 
+  InterruptionBehavior GetInterruptionBehavior() const override {
+    return m_interrupt;
+  }
+
  private:
   CommandScheduler* m_scheduler;
+  InterruptionBehavior m_interrupt;
 };
 
 /**
  * Checks <a
  * href="https://github.com/wpilibsuite/allwpilib/issues/4259">wpilibsuite/allwpilib#4259</a>.
  */
-TEST_P(SchedulingRecursionTest, CancelFromInitialize) {
+TEST_F(SchedulingRecursionTest, CancelFromInitialize) {
   CommandScheduler scheduler = GetScheduler();
   bool hasOtherRun = false;
   TestSubsystem requirement;
-  SelfCancellingCommand selfCancels{&scheduler, &requirement};
+  auto selfCancels = SelfCancellingCommand(&scheduler, &requirement);
   RunCommand other =
       RunCommand([&hasOtherRun] { hasOtherRun = true; }, {&requirement});
 
-  scheduler.Schedule(GetParam(), &selfCancels);
+  scheduler.Schedule(&selfCancels);
   scheduler.Run();
-  // interruptibility of new arrival isn't checked
   scheduler.Schedule(&other);
 
   EXPECT_FALSE(scheduler.IsScheduled(&selfCancels));
@@ -48,16 +56,18 @@ TEST_P(SchedulingRecursionTest, CancelFromInitialize) {
   EXPECT_TRUE(hasOtherRun);
 }
 
-TEST_P(SchedulingRecursionTest, DefaultCommand) {
+TEST_P(SchedulingRecursionTest,
+       DefaultCommandGetsRescheduledAfterSelfCanceling) {
   CommandScheduler scheduler = GetScheduler();
   bool hasOtherRun = false;
   TestSubsystem requirement;
-  SelfCancellingCommand selfCancels{&scheduler, &requirement};
+  auto selfCancels =
+      SelfCancellingCommand(&scheduler, &requirement, GetParam());
   RunCommand other =
       RunCommand([&hasOtherRun] { hasOtherRun = true; }, {&requirement});
   scheduler.SetDefaultCommand(&requirement, std::move(other));
 
-  scheduler.Schedule(GetParam(), &selfCancels);
+  scheduler.Schedule(&selfCancels);
   scheduler.Run();
   scheduler.Run();
   EXPECT_FALSE(scheduler.IsScheduled(&selfCancels));
@@ -93,5 +103,7 @@ TEST_F(SchedulingRecursionTest, CancelFromEnd) {
   EXPECT_FALSE(scheduler.IsScheduled(&selfCancels));
 }
 
-INSTANTIATE_TEST_SUITE_P(SchedulingRecursionTests, SchedulingRecursionTest,
-                         testing::Bool());
+INSTANTIATE_TEST_SUITE_P(
+    SchedulingRecursionTests, SchedulingRecursionTest,
+    testing::Values(Command::InterruptionBehavior::kCancelSelf,
+                    Command::InterruptionBehavior::kCancelIncoming));