From a49cae809c5e6627a537599ad27933266b927d33 Mon Sep 17 00:00:00 2001 From: Ryan Blue Date: Tue, 10 Oct 2023 01:47:15 -0400 Subject: [PATCH] Remove Command* constructors --- .../main/native/cpp/frc2/command/Commands.cpp | 5 -- .../cpp/frc2/command/DeferredCommand.cpp | 38 ++++---------- .../native/cpp/frc2/command/Subsystem.cpp | 4 -- .../native/include/frc2/command/Commands.h | 10 ---- .../include/frc2/command/DeferredCommand.h | 19 +------ .../native/include/frc2/command/Subsystem.h | 10 ---- .../cpp/frc2/command/DeferredCommandTest.cpp | 49 +++++++++++-------- 7 files changed, 41 insertions(+), 94 deletions(-) diff --git a/wpilibNewCommands/src/main/native/cpp/frc2/command/Commands.cpp b/wpilibNewCommands/src/main/native/cpp/frc2/command/Commands.cpp index 3b4f14ccff9..17dc189698b 100644 --- a/wpilibNewCommands/src/main/native/cpp/frc2/command/Commands.cpp +++ b/wpilibNewCommands/src/main/native/cpp/frc2/command/Commands.cpp @@ -83,11 +83,6 @@ CommandPtr cmd::Either(CommandPtr&& onTrue, CommandPtr&& onFalse, .ToPtr(); } -CommandPtr cmd::Defer(wpi::unique_function supplier, - Requirements requirements) { - return DeferredCommand(std::move(supplier), requirements).ToPtr(); -} - CommandPtr cmd::Defer(wpi::unique_function supplier, Requirements requirements) { return DeferredCommand(std::move(supplier), requirements).ToPtr(); diff --git a/wpilibNewCommands/src/main/native/cpp/frc2/command/DeferredCommand.cpp b/wpilibNewCommands/src/main/native/cpp/frc2/command/DeferredCommand.cpp index 30c2c3e2456..2a7f16280a9 100644 --- a/wpilibNewCommands/src/main/native/cpp/frc2/command/DeferredCommand.cpp +++ b/wpilibNewCommands/src/main/native/cpp/frc2/command/DeferredCommand.cpp @@ -6,33 +6,20 @@ #include -#include "frc2/command/PrintCommand.h" +#include "frc2/command/Commands.h" using namespace frc2; -DeferredCommand::DeferredCommand(wpi::unique_function supplier, +DeferredCommand::DeferredCommand(wpi::unique_function supplier, Requirements requirements) : m_supplier{std::move(supplier)} { AddRequirements(requirements); } -DeferredCommand::DeferredCommand(wpi::unique_function supplier, - Requirements requirements) - : DeferredCommand( - [lambdaSupplier = std::move(supplier), - holder = std::optional{}]() mutable { - holder = lambdaSupplier(); - return holder->get(); - }, - requirements) {} - void DeferredCommand::Initialize() { - auto cmd = m_supplier(); - if (cmd != nullptr) { - m_command = cmd; - CommandScheduler::GetInstance().RequireUngrouped(m_command); - m_command->SetComposed(true); - } + m_command = m_supplier().Unwrap(); + CommandScheduler::GetInstance().RequireUngrouped(m_command.get()); + m_command->SetComposed(true); m_command->Initialize(); } @@ -42,7 +29,10 @@ void DeferredCommand::Execute() { void DeferredCommand::End(bool interrupted) { m_command->End(interrupted); - m_command = &m_nullCommand; + m_command = + cmd::Print("[DeferredCommand] Lifecycle function called out-of-order!") + .WithName("none") + .Unwrap(); } bool DeferredCommand::IsFinished() { @@ -52,13 +42,5 @@ bool DeferredCommand::IsFinished() { void DeferredCommand::InitSendable(wpi::SendableBuilder& builder) { Command::InitSendable(builder); builder.AddStringProperty( - "deferred", - [this] { - if (m_command != &m_nullCommand) { - return m_command->GetName(); - } else { - return std::string{"null"}; - } - }, - nullptr); + "deferred", [this] { return m_command->GetName(); }, nullptr); } diff --git a/wpilibNewCommands/src/main/native/cpp/frc2/command/Subsystem.cpp b/wpilibNewCommands/src/main/native/cpp/frc2/command/Subsystem.cpp index e226a6796ea..4c06f1f4ead 100644 --- a/wpilibNewCommands/src/main/native/cpp/frc2/command/Subsystem.cpp +++ b/wpilibNewCommands/src/main/native/cpp/frc2/command/Subsystem.cpp @@ -55,10 +55,6 @@ CommandPtr Subsystem::RunEnd(std::function run, return cmd::RunEnd(std::move(run), std::move(end), {this}); } -CommandPtr Subsystem::Defer(wpi::unique_function supplier) { - return cmd::Defer(std::move(supplier), {this}); -} - CommandPtr Subsystem::Defer(wpi::unique_function supplier) { return cmd::Defer(std::move(supplier), {this}); } diff --git a/wpilibNewCommands/src/main/native/include/frc2/command/Commands.h b/wpilibNewCommands/src/main/native/include/frc2/command/Commands.h index b8dac9f85b9..8557ec5ac88 100644 --- a/wpilibNewCommands/src/main/native/include/frc2/command/Commands.h +++ b/wpilibNewCommands/src/main/native/include/frc2/command/Commands.h @@ -141,16 +141,6 @@ CommandPtr Select(std::function selector, return SelectCommand(std::move(selector), std::move(vec)).ToPtr(); } -/** - * Runs the command supplied by the supplier. - * - * @param supplier the command supplier - * @param requirements the set of requirements for this command - */ -[[nodiscard]] -CommandPtr Defer(wpi::unique_function supplier, - Requirements requirements); - /** * Runs the command supplied by the supplier. * diff --git a/wpilibNewCommands/src/main/native/include/frc2/command/DeferredCommand.h b/wpilibNewCommands/src/main/native/include/frc2/command/DeferredCommand.h index 23bf214a572..442076d8f27 100644 --- a/wpilibNewCommands/src/main/native/include/frc2/command/DeferredCommand.h +++ b/wpilibNewCommands/src/main/native/include/frc2/command/DeferredCommand.h @@ -28,20 +28,6 @@ namespace frc2 { */ class DeferredCommand : public CommandHelper { public: - /** - * Creates a new DeferredCommand that runs the supplied command when - * initialized, and ends when it ends. Useful for lazily - * creating commands at runtime. The supplier will be called each time this - * command is initialized. The supplier must create a new Command each - * call. - * - * @param supplier The command supplier - * @param requirements The command requirements. - * - */ - DeferredCommand(wpi::unique_function supplier, - Requirements requirements); - /** * Creates a new DeferredCommand that runs the supplied command when * initialized, and ends when it ends. Useful for lazily @@ -69,8 +55,7 @@ class DeferredCommand : public CommandHelper { void InitSendable(wpi::SendableBuilder& builder) override; private: - PrintCommand m_nullCommand{"[DeferredCommand] Supplied command was null!"}; - wpi::unique_function m_supplier; - Command* m_command{&m_nullCommand}; + wpi::unique_function m_supplier; + std::unique_ptr m_command; }; } // namespace frc2 diff --git a/wpilibNewCommands/src/main/native/include/frc2/command/Subsystem.h b/wpilibNewCommands/src/main/native/include/frc2/command/Subsystem.h index 40f08bfc40c..cdac0c04661 100644 --- a/wpilibNewCommands/src/main/native/include/frc2/command/Subsystem.h +++ b/wpilibNewCommands/src/main/native/include/frc2/command/Subsystem.h @@ -151,16 +151,6 @@ class Subsystem { [[nodiscard]] CommandPtr RunEnd(std::function run, std::function end); - /** - * Constructs a DeferredCommand with the provided supplier. This subsystem is - * added as a requirement. - * - * @param supplier the command supplier. - * @return the command. - */ - [[nodiscard]] - CommandPtr Defer(wpi::unique_function supplier); - /** * Constructs a DeferredCommand with the provided supplier. This subsystem is * added as a requirement. diff --git a/wpilibNewCommands/src/test/native/cpp/frc2/command/DeferredCommandTest.cpp b/wpilibNewCommands/src/test/native/cpp/frc2/command/DeferredCommandTest.cpp index 960a4057013..e9f9d67dcab 100644 --- a/wpilibNewCommands/src/test/native/cpp/frc2/command/DeferredCommandTest.cpp +++ b/wpilibNewCommands/src/test/native/cpp/frc2/command/DeferredCommandTest.cpp @@ -5,26 +5,46 @@ #include "CommandTestBase.h" #include "frc2/command/Commands.h" #include "frc2/command/DeferredCommand.h" +#include "frc2/command/FunctionalCommand.h" using namespace frc2; class DeferredCommandTest : public CommandTestBase { public: - void DeferredFunctionsTest(bool interrupted) { - std::unique_ptr innerHolder = std::make_unique(); - MockCommand* innerCommand = innerHolder.get(); - DeferredCommand deferred{[innerCommand] { return innerCommand; }, {}}; + void DeferredFunctionsTest(bool testInterrupted) { + int initializeCount = 0; + int executeCount = 0; + int isFinishedCount = 0; + int endCount = 0; + bool finished = false; - EXPECT_CALL(*innerCommand, Initialize()); - EXPECT_CALL(*innerCommand, Execute()); - EXPECT_CALL(*innerCommand, End(interrupted)); + DeferredCommand deferred{[&] { + return FunctionalCommand{ + [&] { initializeCount++; }, + [&] { executeCount++; }, + [&](bool interrupted) { + EXPECT_EQ(interrupted, testInterrupted); + endCount++; + }, + [&] { + isFinishedCount++; + return finished; + }} + .ToPtr(); + }, + {}}; deferred.Initialize(); + EXPECT_EQ(1, initializeCount); deferred.Execute(); + EXPECT_EQ(1, executeCount); EXPECT_FALSE(deferred.IsFinished()); - innerCommand->SetFinished(true); + EXPECT_EQ(1, isFinishedCount); + finished = true; EXPECT_TRUE(deferred.IsFinished()); - deferred.End(interrupted); + EXPECT_EQ(2, isFinishedCount); + deferred.End(testInterrupted); + EXPECT_EQ(1, endCount); } }; @@ -56,14 +76,3 @@ TEST_F(DeferredCommandTest, DeferredRequirements) { EXPECT_TRUE(command.GetRequirements().contains(&subsystem)); } - -TEST_F(DeferredCommandTest, DeferredNullCommand) { - DeferredCommand command{[] { return nullptr; }, {}}; - - EXPECT_NO_THROW({ - command.Initialize(); - command.Execute(); - command.IsFinished(); - command.End(false); - }); -}