Ensure command ends

wpilibsuite · Jul 29, 2023 · 84254c9 · 84254c9
1 parent 3b08c13
commit 84254c9
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 5 deletions.
diff --git a/wpilibNewCommands/src/main/native/cpp/frc2/command/CommandScheduler.cpp b/wpilibNewCommands/src/main/native/cpp/frc2/command/CommandScheduler.cpp
@@ -6,6 +6,7 @@
 
 #include <cstdio>
 #include <queue>
+
 #include <frc/RobotBase.h>
 #include <frc/RobotState.h>
 #include <frc/TimedRobot.h>

diff --git a/wpilibNewCommands/src/test/java/edu/wpi/first/wpilibj2/command/SchedulingRecursionTest.java b/wpilibNewCommands/src/test/java/edu/wpi/first/wpilibj2/command/SchedulingRecursionTest.java
@@ -25,6 +25,7 @@ class SchedulingRecursionTest extends CommandTestBase {
   void cancelFromInitialize(InterruptionBehavior interruptionBehavior) {
     try (CommandScheduler scheduler = new CommandScheduler()) {
       AtomicBoolean hasOtherRun = new AtomicBoolean();
+      AtomicBoolean hasEnded = new AtomicBoolean();
       Subsystem requirement = new Subsystem() {};
       Command selfCancels =
           new Command() {
@@ -37,6 +38,11 @@ public void initialize() {
               scheduler.cancel(this);
             }
 
+            @Override
+            public void end(boolean interrupted) {
+              hasEnded.set(true);
+            }
+
             @Override
             public InterruptionBehavior getInterruptionBehavior() {
               return interruptionBehavior;
@@ -52,6 +58,7 @@ public InterruptionBehavior getInterruptionBehavior() {
           });
       assertFalse(scheduler.isScheduled(selfCancels));
       assertTrue(scheduler.isScheduled(other));
+      assertTrue(hasEnded.get());
       scheduler.run();
       assertTrue(hasOtherRun.get());
     }
@@ -62,6 +69,7 @@ public InterruptionBehavior getInterruptionBehavior() {
   void defaultCommandGetsRescheduledAfterSelfCanceling(InterruptionBehavior interruptionBehavior) {
     try (CommandScheduler scheduler = new CommandScheduler()) {
       AtomicBoolean hasOtherRun = new AtomicBoolean();
+      AtomicBoolean hasEnded = new AtomicBoolean();
       Subsystem requirement = new Subsystem() {};
       Command selfCancels =
           new Command() {
@@ -74,6 +82,11 @@ public void initialize() {
               scheduler.cancel(this);
             }
 
+            @Override
+            public void end(boolean interrupted) {
+              hasEnded.set(true);
+            }
+
             @Override
             public InterruptionBehavior getInterruptionBehavior() {
               return interruptionBehavior;
@@ -90,6 +103,7 @@ public InterruptionBehavior getInterruptionBehavior() {
       scheduler.run();
       assertFalse(scheduler.isScheduled(selfCancels));
       assertTrue(scheduler.isScheduled(other));
+      assertTrue(hasEnded.get());
       scheduler.run();
       assertTrue(hasOtherRun.get());
     }
@@ -252,7 +266,7 @@ public void end(boolean interrupted) {
     }
   }
 
-@ParameterizedTest
+  @ParameterizedTest
   @EnumSource(InterruptionBehavior.class)
   void scheduleInitializeFromDefaultCommand(InterruptionBehavior interruptionBehavior) {
     try (CommandScheduler scheduler = new CommandScheduler()) {

diff --git a/wpilibNewCommands/src/test/native/cpp/frc2/command/SchedulingRecursionTest.cpp b/wpilibNewCommands/src/test/native/cpp/frc2/command/SchedulingRecursionTest.cpp
@@ -16,21 +16,27 @@ class SchedulingRecursionTest
 class SelfCancellingCommand
     : public CommandHelper<Command, SelfCancellingCommand> {
  public:
-  SelfCancellingCommand(CommandScheduler* scheduler, Subsystem* requirement,
+  SelfCancellingCommand(CommandScheduler* scheduler, bool& hasEnded,
+                        Subsystem* requirement,
                         Command::InterruptionBehavior interruptionBehavior =
                             Command::InterruptionBehavior::kCancelSelf)
-      : m_scheduler(scheduler), m_interrupt(interruptionBehavior) {
+      : m_scheduler(scheduler),
+        m_hasEnded(hasEnded),
+        m_interrupt(interruptionBehavior) {
     AddRequirements(requirement);
   }
 
   void Initialize() override { m_scheduler->Cancel(this); }
 
+  void End(bool interrupted) override { m_hasEnded = true; }
+
   InterruptionBehavior GetInterruptionBehavior() const override {
     return m_interrupt;
   }
 
  private:
   CommandScheduler* m_scheduler;
+  bool& m_hasEnded;
   InterruptionBehavior m_interrupt;
 };
 
@@ -41,8 +47,9 @@ class SelfCancellingCommand
 TEST_F(SchedulingRecursionTest, CancelFromInitialize) {
   CommandScheduler scheduler = GetScheduler();
   bool hasOtherRun = false;
+  bool hasEnded = false;
   TestSubsystem requirement;
-  auto selfCancels = SelfCancellingCommand(&scheduler, &requirement);
+  auto selfCancels = SelfCancellingCommand(&scheduler, hasEnded, &requirement);
   RunCommand other =
       RunCommand([&hasOtherRun] { hasOtherRun = true; }, {&requirement});
 
@@ -52,6 +59,7 @@ TEST_F(SchedulingRecursionTest, CancelFromInitialize) {
 
   EXPECT_FALSE(scheduler.IsScheduled(&selfCancels));
   EXPECT_TRUE(scheduler.IsScheduled(&other));
+  EXPECT_TRUE(hasEnded);
   scheduler.Run();
   EXPECT_TRUE(hasOtherRun);
 }
@@ -60,9 +68,10 @@ TEST_P(SchedulingRecursionTest,
        DefaultCommandGetsRescheduledAfterSelfCanceling) {
   CommandScheduler scheduler = GetScheduler();
   bool hasOtherRun = false;
+  bool hasEnded = false;
   TestSubsystem requirement;
   auto selfCancels =
-      SelfCancellingCommand(&scheduler, &requirement, GetParam());
+      SelfCancellingCommand(&scheduler, hasEnded, &requirement, GetParam());
   RunCommand other =
       RunCommand([&hasOtherRun] { hasOtherRun = true; }, {&requirement});
   scheduler.SetDefaultCommand(&requirement, std::move(other));
@@ -72,6 +81,7 @@ TEST_P(SchedulingRecursionTest,
   scheduler.Run();
   EXPECT_FALSE(scheduler.IsScheduled(&selfCancels));
   EXPECT_TRUE(scheduler.IsScheduled(scheduler.GetDefaultCommand(&requirement)));
+  EXPECT_TRUE(hasEnded);
   scheduler.Run();
   EXPECT_TRUE(hasOtherRun);
 }