[commands] Refactor ProxyScheduleCommand, SelectCommand into ProxyCommand (#4534)

wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/Command.java

@@ -286,14 +286,14 @@ public interface Command {
   }
 
   /**
-   * Decorates this command to run "by proxy" by wrapping it in a {@link ProxyScheduleCommand}. This
-   * is useful for "forking off" from command groups when the user does not wish to extend the
+   * Decorates this command to run "by proxy" by wrapping it in a {@link ProxyCommand}. This is
+   * useful for "forking off" from command groups when the user does not wish to extend the
    * command's requirements to the entire command group.
    *
    * @return the decorated command
    */
-  default ProxyScheduleCommand asProxy() {
-    return new ProxyScheduleCommand(this);
+  default ProxyCommand asProxy() {
+    return new ProxyCommand(this);
   }
 
   /**

wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/ProxyCommand.java

@@ -0,0 +1,76 @@
+// Copyright (c) FIRST and other WPILib contributors.
+// Open Source Software; you can modify and/or share it under the terms of
+// the WPILib BSD license file in the root directory of this project.
+
+package edu.wpi.first.wpilibj2.command;
+
+import static edu.wpi.first.wpilibj.util.ErrorMessages.requireNonNullParam;
+
+import java.util.function.Supplier;
+
+/**
+ * Schedules the given command when this command is initialized, and ends when it ends. Useful for
+ * forking off from CommandGroups. If this command is interrupted, it will cancel the command.
+ *
+ * <p>This class is provided by the NewCommands VendorDep
+ */
+public class ProxyCommand extends CommandBase {
+  private final Supplier<Command> m_supplier;
+  private Command m_command;
+
+  /**
+   * Creates a new ProxyCommand that schedules the supplied command when initialized, and ends when
+   * it is no longer scheduled. Useful for lazily creating commands at runtime.
+   *
+   * @param supplier the command supplier
+   */
+  public ProxyCommand(Supplier<Command> supplier) {
+    m_supplier = requireNonNullParam(supplier, "supplier", "ProxyCommand");
+  }
+
+  /**
+   * Creates a new ProxyCommand that schedules the given command when initialized, and ends when it
+   * is no longer scheduled.
+   *
+   * @param command the command to run by proxy
+   */
+  public ProxyCommand(Command command) {
+    this(() -> command);
+  }
+
+  @Override
+  public void initialize() {
+    m_command = m_supplier.get();
+    m_command.schedule();
+  }
+
+  @Override
+  public void end(boolean interrupted) {
+    if (interrupted) {
+      m_command.cancel();
+    }
+    m_command = null;
+  }
+
+  @Override
+  public void execute() {}
+
+  @Override
+  public boolean isFinished() {
+    // because we're between `initialize` and `end`, `m_command` is necessarily not null
+    // but if called otherwise and m_command is null,
+    // it's UB, so we can do whatever we want -- like return true.
+    return m_command == null || !m_command.isScheduled();
+  }
+
+  /**
+   * Whether the given command should run when the robot is disabled. Override to return true if the
+   * command should run when disabled.
+   *
+   * @return true. Otherwise, this proxy would cancel commands that do run when disabled.
+   */
+  @Override
+  public boolean runsWhenDisabled() {
+    return true;
+  }
+}

wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/ProxyScheduleCommand.java

@@ -22,7 +22,10 @@ public class ProxyScheduleCommand extends CommandBase {
    * when they are all no longer scheduled.
    *
    * @param toSchedule the commands to schedule
+   * @deprecated Replace with {@link ProxyCommand}, composing multiple of them in a {@link
+   *     ParallelRaceGroup} if needed.
    */
+  @Deprecated
   public ProxyScheduleCommand(Command... toSchedule) {
     m_toSchedule = Set.of(toSchedule);
   }

wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/SelectCommand.java

@@ -63,7 +63,9 @@ public class SelectCommand extends CommandBase {
    * Creates a new selectcommand.
    *
    * @param toRun a supplier providing the command to run
+   * @deprecated Replace with {@link ProxyCommand}
    */
+  @Deprecated
   public SelectCommand(Supplier<Command> toRun) {
     m_commands = null;
     m_selector = null;

wpilibNewCommands/src/main/native/cpp/frc2/command/Command.cpp

@@ -12,7 +12,6 @@
 #include "frc2/command/ParallelDeadlineGroup.h"
 #include "frc2/command/ParallelRaceGroup.h"
 #include "frc2/command/PerpetualCommand.h"
-#include "frc2/command/ProxyScheduleCommand.h"
 #include "frc2/command/RepeatCommand.h"
 #include "frc2/command/SequentialCommandGroup.h"
 #include "frc2/command/WaitCommand.h"

wpilibNewCommands/src/main/native/cpp/frc2/command/CommandPtr.cpp

@@ -13,7 +13,7 @@
 #include "frc2/command/ParallelDeadlineGroup.h"
 #include "frc2/command/ParallelRaceGroup.h"
 #include "frc2/command/PrintCommand.h"
-#include "frc2/command/ProxyScheduleCommand.h"
+#include "frc2/command/ProxyCommand.h"
 #include "frc2/command/RepeatCommand.h"
 #include "frc2/command/SequentialCommandGroup.h"
 #include "frc2/command/WaitCommand.h"
@@ -37,7 +37,7 @@ CommandPtr CommandPtr::Repeatedly() && {
 
 CommandPtr CommandPtr::AsProxy() && {
   AssertValid();
-  m_ptr = std::make_unique<ProxyScheduleCommand>(std::move(m_ptr));
+  m_ptr = std::make_unique<ProxyCommand>(std::move(m_ptr));
   return std::move(*this);
 }
 

wpilibNewCommands/src/main/native/cpp/frc2/command/Commands.cpp

@@ -10,9 +10,7 @@
 #include "frc2/command/ParallelCommandGroup.h"
 #include "frc2/command/ParallelDeadlineGroup.h"
 #include "frc2/command/ParallelRaceGroup.h"
-#include "frc2/command/PerpetualCommand.h"
 #include "frc2/command/PrintCommand.h"
-#include "frc2/command/ProxyScheduleCommand.h"
 #include "frc2/command/RunCommand.h"
 #include "frc2/command/SequentialCommandGroup.h"
 #include "frc2/command/WaitCommand.h"

wpilibNewCommands/src/main/native/cpp/frc2/command/ProxyCommand.cpp

@@ -0,0 +1,37 @@
+// Copyright (c) FIRST and other WPILib contributors.
+// Open Source Software; you can modify and/or share it under the terms of
+// the WPILib BSD license file in the root directory of this project.
+
+#include "frc2/command/ProxyCommand.h"
+
+using namespace frc2;
+
+ProxyCommand::ProxyCommand(wpi::unique_function<Command*()> supplier)
+    : m_supplier(std::move(supplier)) {}
+
+ProxyCommand::ProxyCommand(Command* command)
+    : m_supplier([command] { return command; }) {}
+
+ProxyCommand::ProxyCommand(std::unique_ptr<Command> command)
+    : m_supplier([command = std::move(command)] { return command.get(); }) {}
+
+void ProxyCommand::Initialize() {
+  m_command = m_supplier();
+  m_command->Schedule();
+}
+
+void ProxyCommand::End(bool interrupted) {
+  if (interrupted) {
+    m_command->Cancel();
+  }
+  m_command = nullptr;
+}
+
+void ProxyCommand::Execute() {}
+
+bool ProxyCommand::IsFinished() {
+  // because we're between `initialize` and `end`, `m_command` is necessarily
+  // not null but if called otherwise and m_command is null, it's UB, so we can
+  // do whatever we want -- like return true.
+  return m_command == nullptr || !m_command->IsScheduled();
+}

wpilibNewCommands/src/main/native/cpp/frc2/command/ProxyScheduleCommand.cpp

@@ -15,13 +15,6 @@ ProxyScheduleCommand::ProxyScheduleCommand(Command* toSchedule) {
   SetInsert(m_toSchedule, {&toSchedule, 1});
 }
 
-ProxyScheduleCommand::ProxyScheduleCommand(
-    std::unique_ptr<Command>&& toSchedule)
-    : m_owning(std::move(toSchedule)) {
-  Command* ptr = m_owning.get();
-  SetInsert(m_toSchedule, {&ptr, 1});
-}
-
 void ProxyScheduleCommand::Initialize() {
   for (auto* command : m_toSchedule) {
     command->Schedule();

wpilibNewCommands/src/main/native/include/frc2/command/Command.h

@@ -25,7 +25,6 @@ std::string GetTypeName(const T& type) {
 }
 
 class PerpetualCommand;
-class ProxyScheduleCommand;
 
 /**
  * A state machine representing a complete action to be performed by the robot.
@@ -225,7 +224,7 @@ safe) semantics.
 
   /**
    * Decorates this command to run "by proxy" by wrapping it in a
-   * ProxyScheduleCommand. This is useful for "forking off" from command groups
+   * ProxyCommand. This is useful for "forking off" from command groups
    * when the user does not wish to extend the command's requirements to the
    * entire command group.
    *

wpilibNewCommands/src/main/native/include/frc2/command/CommandPtr.h

@@ -49,7 +49,7 @@ class CommandPtr final {
 
   /**
    * Decorates this command to run "by proxy" by wrapping it in a
-   * ProxyScheduleCommand. This is useful for "forking off" from command groups
+   * ProxyCommand. This is useful for "forking off" from command groups
    * when the user does not wish to extend the command's requirements to the
    * entire command group.
    *

wpilibNewCommands/src/main/native/include/frc2/command/ProxyCommand.h

@@ -0,0 +1,68 @@
+// Copyright (c) FIRST and other WPILib contributors.
+// Open Source Software; you can modify and/or share it under the terms of
+// the WPILib BSD license file in the root directory of this project.
+
+#pragma once
+
+#include <memory>
+#include <span>
+
+#include <wpi/FunctionExtras.h>
+
+#include "frc2/command/CommandBase.h"
+#include "frc2/command/CommandHelper.h"
+#include "frc2/command/SetUtilities.h"
+
+namespace frc2 {
+/**
+ * Schedules the given command when this command is initialized, and ends when
+ * it ends. Useful for forking off from CommandGroups. If this command is
+ * interrupted, it will cancel the command.
+ *
+ * <p>This class is provided by the NewCommands VendorDep
+ */
+class ProxyCommand : public CommandHelper<CommandBase, ProxyCommand> {
+ public:
+  /**
+   * Creates a new ProxyCommand that schedules the supplied command when
+   * initialized, and ends when it is no longer scheduled. Useful for lazily
+   * creating commands at runtime.
+   *
+   * @param supplier the command supplier
+   */
+  explicit ProxyCommand(wpi::unique_function<Command*()> supplier);
+
+  /**
+   * Creates a new ProxyCommand that schedules the given command when
+   * initialized, and ends when it is no longer scheduled.
+   *
+   * @param command the command to run by proxy
+   */
+  explicit ProxyCommand(Command* command);
+
+  /**
+   * Creates a new ProxyCommand that schedules the given command when
+   * initialized, and ends when it is no longer scheduled.
+   *
+   * <p>Note that this constructor passes ownership of the given command to the
+   * returned ProxyCommand.
+   *
+   * @param command the command to schedule
+   */
+  explicit ProxyCommand(std::unique_ptr<Command> command);
+
+  ProxyCommand(ProxyCommand&& other) = default;
+
+  void Initialize() override;
+
+  void End(bool interrupted) override;
+
+  void Execute() override;
+
+  bool IsFinished() override;
+
+ private:
+  wpi::unique_function<Command*()> m_supplier;
+  Command* m_command = nullptr;
+};
+}  // namespace frc2

wpilibNewCommands/src/main/native/include/frc2/command/ProxyScheduleCommand.h

@@ -8,6 +8,7 @@
 #include <span>
 
 #include <wpi/SmallVector.h>
+#include <wpi/deprecated.h>
 
 #include "frc2/command/CommandBase.h"
 #include "frc2/command/CommandHelper.h"
@@ -30,22 +31,14 @@ class ProxyScheduleCommand
    * initialized, and ends when they are all no longer scheduled.
    *
    * @param toSchedule the commands to schedule
+   * @deprecated Replace with {@link ProxyCommand},
+   * composing multiple of them in a {@link ParallelRaceGroup} if needed.
    */
+  WPI_DEPRECATED("Replace with ProxyCommand")
   explicit ProxyScheduleCommand(std::span<Command* const> toSchedule);
 
   explicit ProxyScheduleCommand(Command* toSchedule);
 
-  /**
-   * Creates a new ProxyScheduleCommand that schedules the given command when
-   * initialized, and ends when it is no longer scheduled.
-   *
-   * <p>Note that this constructor passes ownership of the given command to the
-   * returned ProxyScheduleCommand.
-   *
-   * @param toSchedule the command to schedule
-   */
-  explicit ProxyScheduleCommand(std::unique_ptr<Command>&& toSchedule);
-
   ProxyScheduleCommand(ProxyScheduleCommand&& other) = default;
 
   void Initialize() override;

wpilibNewCommands/src/main/native/include/frc2/command/SelectCommand.h

@@ -108,7 +108,10 @@ class SelectCommand : public CommandHelper<CommandBase, SelectCommand<Key>> {
    * Creates a new selectcommand.
    *
    * @param toRun a supplier providing the command to run
+   * @deprecated Replace with {@link ProxyCommand},
+   * composing multiple of them in a {@link ParallelRaceGroup} if needed.
    */
+  WPI_DEPRECATED("Replace with ProxyCommand")
   explicit SelectCommand(std::function<Command*()> toRun)
       : m_toRun{std::move(toRun)} {}
 

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

@@ -11,14 +11,14 @@ import static org.mockito.Mockito.verify;
 import java.util.concurrent.atomic.AtomicBoolean;
 import org.junit.jupiter.api.Test;
 
-class ProxyScheduleCommandTest extends CommandTestBase {
+class ProxyCommandTest extends CommandTestBase {
   @Test
-  void proxyScheduleCommandScheduleTest() {
+  void proxyCommandScheduleTest() {
     try (CommandScheduler scheduler = new CommandScheduler()) {
       MockCommandHolder command1Holder = new MockCommandHolder(true);
       Command command1 = command1Holder.getMock();
 
-      ProxyScheduleCommand scheduleCommand = new ProxyScheduleCommand(command1);
+      ProxyCommand scheduleCommand = new ProxyCommand(command1);
 
       scheduler.schedule(scheduleCommand);
 
@@ -27,13 +27,13 @@ class ProxyScheduleCommandTest extends CommandTestBase {
   }
 
   @Test
-  void proxyScheduleCommandEndTest() {
+  void proxyCommandEndTest() {
     try (CommandScheduler scheduler = CommandScheduler.getInstance()) {
       AtomicBoolean cond = new AtomicBoolean();
 
       WaitUntilCommand command = new WaitUntilCommand(cond::get);
 
-      ProxyScheduleCommand scheduleCommand = new ProxyScheduleCommand(command);
+      ProxyCommand scheduleCommand = new ProxyCommand(command);
 
       scheduler.schedule(scheduleCommand);
 

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

@@ -7,20 +7,20 @@
 #include "CommandTestBase.h"
 #include "frc2/command/CommandPtr.h"
 #include "frc2/command/InstantCommand.h"
-#include "frc2/command/ProxyScheduleCommand.h"
+#include "frc2/command/ProxyCommand.h"
 #include "frc2/command/WaitUntilCommand.h"
 
 using namespace frc2;
-class ProxyScheduleCommandTest : public CommandTestBase {};
+class ProxyCommandTest : public CommandTestBase {};
 
-TEST_F(ProxyScheduleCommandTest, NonOwningCommandSchedule) {
+TEST_F(ProxyCommandTest, NonOwningCommandSchedule) {
   CommandScheduler& scheduler = CommandScheduler::GetInstance();
 
   bool scheduled = false;
 
   InstantCommand toSchedule([&scheduled] { scheduled = true; }, {});
 
-  ProxyScheduleCommand command(&toSchedule);
+  ProxyCommand command(&toSchedule);
 
   scheduler.Schedule(&command);
   scheduler.Run();
@@ -28,14 +28,14 @@ TEST_F(ProxyScheduleCommandTest, NonOwningCommandSchedule) {
   EXPECT_TRUE(scheduled);
 }
 
-TEST_F(ProxyScheduleCommandTest, NonOwningCommandEnd) {
+TEST_F(ProxyCommandTest, NonOwningCommandEnd) {
   CommandScheduler& scheduler = CommandScheduler::GetInstance();
 
   bool finished = false;
 
   WaitUntilCommand toSchedule([&finished] { return finished; });
 
-  ProxyScheduleCommand command(&toSchedule);
+  ProxyCommand command(&toSchedule);
 
   scheduler.Schedule(&command);
   scheduler.Run();
@@ -47,7 +47,7 @@ TEST_F(ProxyScheduleCommandTest, NonOwningCommandEnd) {
   EXPECT_FALSE(scheduler.IsScheduled(&command));
 }
 
-TEST_F(ProxyScheduleCommandTest, OwningCommandSchedule) {
+TEST_F(ProxyCommandTest, OwningCommandSchedule) {
   CommandScheduler& scheduler = CommandScheduler::GetInstance();
 
   bool scheduled = false;
@@ -61,7 +61,7 @@ TEST_F(ProxyScheduleCommandTest, OwningCommandSchedule) {
   EXPECT_TRUE(scheduled);
 }
 
-TEST_F(ProxyScheduleCommandTest, OwningCommandEnd) {
+TEST_F(ProxyCommandTest, OwningCommandEnd) {
   CommandScheduler& scheduler = CommandScheduler::GetInstance();
 
   bool finished = false;