From e28295fc7bbec82b4925461c19bd6ebae7e8d1f4 Mon Sep 17 00:00:00 2001 From: Matt Soucy Date: Sun, 19 Aug 2018 22:43:21 -0400 Subject: [PATCH] Add dependency injection of Subsystem to Command (#1275) --- .../src/main/native/cpp/commands/Command.cpp | 19 ++++++ .../native/cpp/commands/InstantCommand.cpp | 3 + .../main/native/cpp/commands/PIDCommand.cpp | 33 +++++++++ .../main/native/cpp/commands/TimedCommand.cpp | 7 ++ .../native/include/frc/commands/Command.h | 34 ++++++++++ .../include/frc/commands/InstantCommand.h | 8 +++ .../native/include/frc/commands/PIDCommand.h | 11 +++ .../include/frc/commands/TimedCommand.h | 17 +++++ .../wpi/first/wpilibj/command/Command.java | 54 +++++++++++++++ .../first/wpilibj/command/InstantCommand.java | 17 +++++ .../wpi/first/wpilibj/command/PIDCommand.java | 67 ++++++++++++++++++- .../first/wpilibj/command/TimedCommand.java | 23 ++++++- 12 files changed, 291 insertions(+), 2 deletions(-) diff --git a/wpilibc/src/main/native/cpp/commands/Command.cpp b/wpilibc/src/main/native/cpp/commands/Command.cpp index 10ce8760d1..06e67b2ab0 100644 --- a/wpilibc/src/main/native/cpp/commands/Command.cpp +++ b/wpilibc/src/main/native/cpp/commands/Command.cpp @@ -27,6 +27,10 @@ Command::Command(const wpi::Twine& name) : Command(name, -1.0) {} Command::Command(double timeout) : Command("", timeout) {} +Command::Command(Subsystem& requirement) : Command("", -1.0) { + Requires(&requirement); +} + Command::Command(const wpi::Twine& name, double timeout) : SendableBase(false) { // We use -1.0 to indicate no timeout. if (timeout < 0.0 && timeout != -1.0) @@ -43,6 +47,21 @@ Command::Command(const wpi::Twine& name, double timeout) : SendableBase(false) { } } +Command::Command(const wpi::Twine& name, Subsystem& requirement) + : Command(name, -1.0) { + Requires(&requirement); +} + +Command::Command(double timeout, Subsystem& requirement) + : Command("", timeout) { + Requires(&requirement); +} + +Command::Command(const wpi::Twine& name, double timeout, Subsystem& requirement) + : Command(name, timeout) { + Requires(&requirement); +} + double Command::TimeSinceInitialized() const { if (m_startTime < 0.0) return 0.0; diff --git a/wpilibc/src/main/native/cpp/commands/InstantCommand.cpp b/wpilibc/src/main/native/cpp/commands/InstantCommand.cpp index aa4f7e6a16..3ee3c3ce07 100644 --- a/wpilibc/src/main/native/cpp/commands/InstantCommand.cpp +++ b/wpilibc/src/main/native/cpp/commands/InstantCommand.cpp @@ -11,4 +11,7 @@ using namespace frc; InstantCommand::InstantCommand(const wpi::Twine& name) : Command(name) {} +InstantCommand::InstantCommand(const wpi::Twine& name, Subsystem& subsystem) + : Command(name, subsystem) {} + bool InstantCommand::IsFinished() { return true; } diff --git a/wpilibc/src/main/native/cpp/commands/PIDCommand.cpp b/wpilibc/src/main/native/cpp/commands/PIDCommand.cpp index 949dccaa47..1cd9114dc0 100644 --- a/wpilibc/src/main/native/cpp/commands/PIDCommand.cpp +++ b/wpilibc/src/main/native/cpp/commands/PIDCommand.cpp @@ -41,6 +41,39 @@ PIDCommand::PIDCommand(double p, double i, double d, double period) { m_controller = std::make_shared(p, i, d, this, this, period); } +PIDCommand::PIDCommand(const wpi::Twine& name, double p, double i, double d, + double f, double period, Subsystem& requirement) + : Command(name, requirement) { + m_controller = std::make_shared(p, i, d, this, this, period); +} + +PIDCommand::PIDCommand(double p, double i, double d, double f, double period, + Subsystem& requirement) { + m_controller = + std::make_shared(p, i, d, f, this, this, period); +} + +PIDCommand::PIDCommand(const wpi::Twine& name, double p, double i, double d, + Subsystem& requirement) + : Command(name) { + m_controller = std::make_shared(p, i, d, this, this); +} + +PIDCommand::PIDCommand(const wpi::Twine& name, double p, double i, double d, + double period, Subsystem& requirement) + : Command(name) { + m_controller = std::make_shared(p, i, d, this, this, period); +} + +PIDCommand::PIDCommand(double p, double i, double d, Subsystem& requirement) { + m_controller = std::make_shared(p, i, d, this, this); +} + +PIDCommand::PIDCommand(double p, double i, double d, double period, + Subsystem& requirement) { + m_controller = std::make_shared(p, i, d, this, this, period); +} + void PIDCommand::_Initialize() { m_controller->Enable(); } void PIDCommand::_End() { m_controller->Disable(); } diff --git a/wpilibc/src/main/native/cpp/commands/TimedCommand.cpp b/wpilibc/src/main/native/cpp/commands/TimedCommand.cpp index d5c6b8c82f..a8fc06be51 100644 --- a/wpilibc/src/main/native/cpp/commands/TimedCommand.cpp +++ b/wpilibc/src/main/native/cpp/commands/TimedCommand.cpp @@ -14,4 +14,11 @@ TimedCommand::TimedCommand(const wpi::Twine& name, double timeout) TimedCommand::TimedCommand(double timeout) : Command(timeout) {} +TimedCommand::TimedCommand(const wpi::Twine& name, double timeout, + Subsystem& requirement) + : Command(name, timeout, requirement) {} + +TimedCommand::TimedCommand(double timeout, Subsystem& requirement) + : Command(timeout, requirement) {} + bool TimedCommand::IsFinished() { return IsTimedOut(); } diff --git a/wpilibc/src/main/native/include/frc/commands/Command.h b/wpilibc/src/main/native/include/frc/commands/Command.h index 777401fb91..1a74fea83a 100644 --- a/wpilibc/src/main/native/include/frc/commands/Command.h +++ b/wpilibc/src/main/native/include/frc/commands/Command.h @@ -72,6 +72,13 @@ class Command : public ErrorBase, public SendableBase { */ explicit Command(double timeout); + /** + * Creates a new command with the given timeout and a default name. + * + * @param requirement the subsystem that the command requires + */ + explicit Command(Subsystem& requirement); + /** * Creates a new command with the given name and timeout. * @@ -81,6 +88,33 @@ class Command : public ErrorBase, public SendableBase { */ Command(const wpi::Twine& name, double timeout); + /** + * Creates a new command with the given name and timeout. + * + * @param name the name of the command + * @param requirement the subsystem that the command requires + */ + Command(const wpi::Twine& name, Subsystem& requirement); + + /** + * Creates a new command with the given name and timeout. + * + * @param timeout the time (in seconds) before this command "times out" + * @param requirement the subsystem that the command requires + * @see IsTimedOut() + */ + Command(double timeout, Subsystem& requirement); + + /** + * Creates a new command with the given name and timeout. + * + * @param name the name of the command + * @param timeout the time (in seconds) before this command "times out" + * @param requirement the subsystem that the command requires + * @see IsTimedOut() + */ + Command(const wpi::Twine& name, double timeout, Subsystem& requirement); + ~Command() override = default; /** diff --git a/wpilibc/src/main/native/include/frc/commands/InstantCommand.h b/wpilibc/src/main/native/include/frc/commands/InstantCommand.h index a188bbcffc..4a73c7580d 100644 --- a/wpilibc/src/main/native/include/frc/commands/InstantCommand.h +++ b/wpilibc/src/main/native/include/frc/commands/InstantCommand.h @@ -27,6 +27,14 @@ class InstantCommand : public Command { */ explicit InstantCommand(const wpi::Twine& name); + /** + * Creates a new InstantCommand with the given name. + * + * @param name The name for this command + * @param requirement The subsystem that the command requires + */ + InstantCommand(const wpi::Twine& name, Subsystem& requirement); + InstantCommand() = default; virtual ~InstantCommand() = default; diff --git a/wpilibc/src/main/native/include/frc/commands/PIDCommand.h b/wpilibc/src/main/native/include/frc/commands/PIDCommand.h index 5df6341105..f607a1b3a6 100644 --- a/wpilibc/src/main/native/include/frc/commands/PIDCommand.h +++ b/wpilibc/src/main/native/include/frc/commands/PIDCommand.h @@ -28,6 +28,17 @@ class PIDCommand : public Command, public PIDOutput, public PIDSource { PIDCommand(double p, double i, double d); PIDCommand(double p, double i, double d, double period); PIDCommand(double p, double i, double d, double f, double period); + PIDCommand(const wpi::Twine& name, double p, double i, double d, + Subsystem& requirement); + PIDCommand(const wpi::Twine& name, double p, double i, double d, + double period, Subsystem& requirement); + PIDCommand(const wpi::Twine& name, double p, double i, double d, double f, + double period, Subsystem& requirement); + PIDCommand(double p, double i, double d, Subsystem& requirement); + PIDCommand(double p, double i, double d, double period, + Subsystem& requirement); + PIDCommand(double p, double i, double d, double f, double period, + Subsystem& requirement); virtual ~PIDCommand() = default; void SetSetpointRelative(double deltaSetpoint); diff --git a/wpilibc/src/main/native/include/frc/commands/TimedCommand.h b/wpilibc/src/main/native/include/frc/commands/TimedCommand.h index da5585ace5..30f3e3cc13 100644 --- a/wpilibc/src/main/native/include/frc/commands/TimedCommand.h +++ b/wpilibc/src/main/native/include/frc/commands/TimedCommand.h @@ -35,6 +35,23 @@ class TimedCommand : public Command { */ explicit TimedCommand(double timeout); + /** + * Creates a new TimedCommand with the given name and timeout. + * + * @param name the name of the command + * @param timeout the time (in seconds) before this command "times out" + * @param requirement the subsystem that the command requires + */ + TimedCommand(const wpi::Twine& name, double timeout, Subsystem& requirement); + + /** + * Creates a new WaitCommand with the given timeout. + * + * @param timeout the time (in seconds) before this command "times out" + * @param requirement the subsystem that the command requires + */ + TimedCommand(double timeout, Subsystem& requirement); + virtual ~TimedCommand() = default; protected: diff --git a/wpilibj/src/main/java/edu/wpi/first/wpilibj/command/Command.java b/wpilibj/src/main/java/edu/wpi/first/wpilibj/command/Command.java index 57db307961..4906312994 100644 --- a/wpilibj/src/main/java/edu/wpi/first/wpilibj/command/Command.java +++ b/wpilibj/src/main/java/edu/wpi/first/wpilibj/command/Command.java @@ -135,6 +135,45 @@ public abstract class Command extends SendableBase { m_timeout = timeout; } + /** + * Creates a new command with the given timeout and a default name. The default name is the name + * of the class. + * + * @param requirement the subsystem that this command requires + * @throws IllegalArgumentException if given a negative timeout + * @see Command#isTimedOut() isTimedOut() + */ + public Command(Subsystem requirement) { + this(); + requires(requirement); + } + + /** + * Creates a new command with the given name. + * + * @param name the name for this command + * @param requirement the subsystem that this command requires + * @throws IllegalArgumentException if name is null + */ + public Command(String name, Subsystem requirement) { + this(name); + requires(requirement); + } + + /** + * Creates a new command with the given timeout and a default name. The default name is the name + * of the class. + * + * @param timeout the time (in seconds) before this command "times out" + * @param requirement the subsystem that this command requires + * @throws IllegalArgumentException if given a negative timeout + * @see Command#isTimedOut() isTimedOut() + */ + public Command(double timeout, Subsystem requirement) { + this(timeout); + requires(requirement); + } + /** * Creates a new command with the given name and timeout. * @@ -151,6 +190,21 @@ public abstract class Command extends SendableBase { m_timeout = timeout; } + /** + * Creates a new command with the given name and timeout. + * + * @param name the name of the command + * @param timeout the time (in seconds) before this command "times out" + * @param requirement the subsystem that this command requires + * @throws IllegalArgumentException if given a negative timeout + * @throws IllegalArgumentException if given a negative timeout or name was null. + * @see Command#isTimedOut() isTimedOut() + */ + public Command(String name, double timeout, Subsystem requirement) { + this(name, timeout); + requires(requirement); + } + /** * Sets the timeout of this command. * diff --git a/wpilibj/src/main/java/edu/wpi/first/wpilibj/command/InstantCommand.java b/wpilibj/src/main/java/edu/wpi/first/wpilibj/command/InstantCommand.java index 389b78fcc8..7dc6c9bcf7 100644 --- a/wpilibj/src/main/java/edu/wpi/first/wpilibj/command/InstantCommand.java +++ b/wpilibj/src/main/java/edu/wpi/first/wpilibj/command/InstantCommand.java @@ -25,6 +25,23 @@ public class InstantCommand extends Command { super(name); } + /** + * Creates a new {@link InstantCommand InstantCommand} with the given requirement. + * @param requirement the subsystem this command requires + */ + public InstantCommand(Subsystem requirement) { + super(requirement); + } + + /** + * Creates a new {@link InstantCommand InstantCommand} with the given name and requirement. + * @param name the name for this command + * @param requirement the subsystem this command requires + */ + public InstantCommand(String name, Subsystem requirement) { + super(name, requirement); + } + @Override protected boolean isFinished() { return true; diff --git a/wpilibj/src/main/java/edu/wpi/first/wpilibj/command/PIDCommand.java b/wpilibj/src/main/java/edu/wpi/first/wpilibj/command/PIDCommand.java index 99d37a6d50..c9763e6600 100644 --- a/wpilibj/src/main/java/edu/wpi/first/wpilibj/command/PIDCommand.java +++ b/wpilibj/src/main/java/edu/wpi/first/wpilibj/command/PIDCommand.java @@ -93,7 +93,7 @@ public abstract class PIDCommand extends Command { /** * Instantiates a {@link PIDCommand} that will use the given p, i and d values. It will use the - * class name as its name.. It will also space the time between PID loop calculations to be equal + * class name as its name. It will also space the time between PID loop calculations to be equal * to the given period. * * @param p the proportional value @@ -106,6 +106,71 @@ public abstract class PIDCommand extends Command { m_controller = new PIDController(p, i, d, m_source, m_output, period); } + /** + * Instantiates a {@link PIDCommand} that will use the given p, i and d values. + * + * @param name the name of the command + * @param p the proportional value + * @param i the integral value + * @param d the derivative value + * @param requirement the subsystem that this command requires + */ + @SuppressWarnings("ParameterName") + public PIDCommand(String name, double p, double i, double d, Subsystem requirement) { + super(name, requirement); + m_controller = new PIDController(p, i, d, m_source, m_output); + } + + /** + * Instantiates a {@link PIDCommand} that will use the given p, i and d values. It will also space + * the time between PID loop calculations to be equal to the given period. + * + * @param name the name + * @param p the proportional value + * @param i the integral value + * @param d the derivative value + * @param period the time (in seconds) between calculations + * @param requirement the subsystem that this command requires + */ + @SuppressWarnings("ParameterName") + public PIDCommand(String name, double p, double i, double d, double period, + Subsystem requirement) { + super(name, requirement); + m_controller = new PIDController(p, i, d, m_source, m_output, period); + } + + /** + * Instantiates a {@link PIDCommand} that will use the given p, i and d values. It will use the + * class name as its name. + * + * @param p the proportional value + * @param i the integral value + * @param d the derivative value + * @param requirement the subsystem that this command requires + */ + @SuppressWarnings("ParameterName") + public PIDCommand(double p, double i, double d, Subsystem requirement) { + super(requirement); + m_controller = new PIDController(p, i, d, m_source, m_output); + } + + /** + * Instantiates a {@link PIDCommand} that will use the given p, i and d values. It will use the + * class name as its name. It will also space the time between PID loop calculations to be equal + * to the given period. + * + * @param p the proportional value + * @param i the integral value + * @param d the derivative value + * @param period the time (in seconds) between calculations + * @param requirement the subsystem that this command requires + */ + @SuppressWarnings("ParameterName") + public PIDCommand(double p, double i, double d, double period, Subsystem requirement) { + super(requirement); + m_controller = new PIDController(p, i, d, m_source, m_output, period); + } + /** * Returns the {@link PIDController} used by this {@link PIDCommand}. Use this if you would like * to fine tune the pid loop. diff --git a/wpilibj/src/main/java/edu/wpi/first/wpilibj/command/TimedCommand.java b/wpilibj/src/main/java/edu/wpi/first/wpilibj/command/TimedCommand.java index a05e70498c..89bb06dbcf 100644 --- a/wpilibj/src/main/java/edu/wpi/first/wpilibj/command/TimedCommand.java +++ b/wpilibj/src/main/java/edu/wpi/first/wpilibj/command/TimedCommand.java @@ -15,7 +15,7 @@ public class TimedCommand extends Command { /** * Instantiates a TimedCommand with the given name and timeout. * - * @param name the name of the command + * @param name the name of the command * @param timeout the time the command takes to run (seconds) */ public TimedCommand(String name, double timeout) { @@ -31,6 +31,27 @@ public class TimedCommand extends Command { super(timeout); } + /** + * Instantiates a TimedCommand with the given name and timeout. + * + * @param name the name of the command + * @param timeout the time the command takes to run (seconds) + * @param requirement the subsystem that this command requires + */ + public TimedCommand(String name, double timeout, Subsystem requirement) { + super(name, timeout, requirement); + } + + /** + * Instantiates a TimedCommand with the given timeout. + * + * @param timeout the time the command takes to run (seconds) + * @param requirement the subsystem that this command requires + */ + public TimedCommand(double timeout, Subsystem requirement) { + super(timeout, requirement); + } + /** * Ends command when timed out. */