From 4bbdbdfb48ef3acb845358b0cc0807dc56b3f2f1 Mon Sep 17 00:00:00 2001 From: Starlight220 <53231611+Starlight220@users.noreply.github.com> Date: Wed, 7 Dec 2022 07:13:31 +0200 Subject: [PATCH] [commands] Move GroupedCommands to CommandScheduler (#4728) Move the command group checking functionality from CommandGroupBase into CommandScheduler. Update references to grouping as composition for clarity (because explicitly grouping isn't the only way to do it). Deprecate the static factory methods parallel, race, and deadline in CommandGroupBase in favor of the identical ones in Commands. --- .../wpi/first/wpilibj2/command/Command.java | 126 +++++++++--------- .../first/wpilibj2/command/CommandBase.java | 2 +- .../wpilibj2/command/CommandGroupBase.java | 84 +++--------- .../wpilibj2/command/CommandScheduler.java | 94 ++++++++++++- .../wpilibj2/command/ConditionalCommand.java | 27 ++-- .../command/ParallelCommandGroup.java | 23 ++-- .../command/ParallelDeadlineGroup.java | 24 ++-- .../wpilibj2/command/ParallelRaceGroup.java | 17 +-- .../wpilibj2/command/PerpetualCommand.java | 8 +- .../first/wpilibj2/command/RepeatCommand.java | 17 ++- .../wpilibj2/command/ScheduleCommand.java | 4 +- .../first/wpilibj2/command/SelectCommand.java | 24 ++-- .../command/SequentialCommandGroup.java | 17 +-- .../wpilibj2/command/WrapperCommand.java | 13 +- .../main/native/cpp/frc2/command/Command.cpp | 14 +- .../cpp/frc2/command/CommandGroupBase.cpp | 41 ------ .../cpp/frc2/command/CommandScheduler.cpp | 32 ++++- .../cpp/frc2/command/ConditionalCommand.cpp | 9 +- .../cpp/frc2/command/ParallelCommandGroup.cpp | 8 +- .../frc2/command/ParallelDeadlineGroup.cpp | 8 +- .../cpp/frc2/command/ParallelRaceGroup.cpp | 6 +- .../cpp/frc2/command/PerpetualCommand.cpp | 6 +- .../native/cpp/frc2/command/RepeatCommand.cpp | 6 +- .../frc2/command/SequentialCommandGroup.cpp | 6 +- .../cpp/frc2/command/WrapperCommand.cpp | 6 +- .../native/include/frc2/command/Command.h | 27 +++- .../include/frc2/command/CommandGroupBase.h | 47 +------ .../native/include/frc2/command/CommandPtr.h | 4 +- .../include/frc2/command/CommandScheduler.h | 28 ++++ .../include/frc2/command/ConditionalCommand.h | 20 +-- .../frc2/command/ParallelCommandGroup.h | 22 +-- .../frc2/command/ParallelDeadlineGroup.h | 32 ++--- .../include/frc2/command/ParallelRaceGroup.h | 14 +- .../include/frc2/command/PerpetualCommand.h | 3 +- .../include/frc2/command/RepeatCommand.h | 13 +- .../include/frc2/command/ScheduleCommand.h | 8 +- .../include/frc2/command/SelectCommand.h | 29 ++-- .../frc2/command/SequentialCommandGroup.h | 20 +-- .../include/frc2/command/WrapperCommand.h | 1 - .../command/CommandGroupErrorTest.java | 17 ++- .../wpilibj2/command/CommandTestBase.java | 2 +- .../native/cpp/frc2/command/CommandTestBase.h | 1 - .../gearsbot/commands/Autonomous.java | 3 +- .../examples/gearsbot/commands/Pickup.java | 4 +- .../gearsbot/commands/PrepareToPickup.java | 3 +- .../RapidReactCommandBot.java | 2 +- 46 files changed, 450 insertions(+), 472 deletions(-) diff --git a/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/Command.java b/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/Command.java index 0fecef2338..63459c8308 100644 --- a/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/Command.java +++ b/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/Command.java @@ -67,11 +67,11 @@ public interface Command { * finishes normally, the command will be interrupted and un-scheduled. Note that the timeout only * applies to the command returned by this method; the calling command is not itself changed. * - *

Note: This decorator works by composing this command within a CommandGroup. The command - * cannot be used independently after being decorated, or be re-decorated with a different - * decorator, unless it is manually cleared from the list of grouped commands with {@link - * CommandGroupBase#clearGroupedCommand(Command)}. The decorated command can, however, be further - * decorated without issue. + *

Note: This decorator works by adding this command to a composition. The command the + * decorator was called on cannot be scheduled independently or be added to a different + * composition (namely, decorators), unless it is manually cleared from the list of composed + * commands with {@link CommandScheduler#removeComposedCommand(Command)}. The command composition + * returned from this method can be further decorated without issue. * * @param seconds the timeout duration * @return the command with the timeout added @@ -86,11 +86,11 @@ public interface Command { * that this only applies to the command returned by this method; the calling command is not * itself changed. * - *

Note: This decorator works by composing this command within a CommandGroup. The command - * cannot be used independently after being decorated, or be re-decorated with a different - * decorator, unless it is manually cleared from the list of grouped commands with {@link - * CommandGroupBase#clearGroupedCommand(Command)}. The decorated command can, however, be further - * decorated without issue. + *

Note: This decorator works by adding this command to a composition. The command the + * decorator was called on cannot be scheduled independently or be added to a different + * composition (namely, decorators), unless it is manually cleared from the list of composed + * commands with {@link CommandScheduler#removeComposedCommand(Command)}. The command composition + * returned from this method can be further decorated without issue. * * @param condition the interrupt condition * @return the command with the interrupt condition added @@ -105,11 +105,11 @@ public interface Command { * that this only applies to the command returned by this method; the calling command is not * itself changed. * - *

Note: This decorator works by composing this command within a CommandGroup. The command - * cannot be used independently after being decorated, or be re-decorated with a different - * decorator, unless it is manually cleared from the list of grouped commands with {@link - * CommandGroupBase#clearGroupedCommand(Command)}. The decorated command can, however, be further - * decorated without issue. + *

Note: This decorator works by adding this command to a composition. The command the + * decorator was called on cannot be scheduled independently or be added to a different + * composition (namely, decorators), unless it is manually cleared from the list of composed + * commands with {@link CommandScheduler#removeComposedCommand(Command)}. The command composition + * returned from this method can be further decorated without issue. * * @param condition the interrupt condition * @return the command with the interrupt condition added @@ -123,11 +123,11 @@ public interface Command { /** * Decorates this command with a runnable to run before this command starts. * - *

Note: This decorator works by composing this command within a CommandGroup. The command - * cannot be used independently after being decorated, or be re-decorated with a different - * decorator, unless it is manually cleared from the list of grouped commands with {@link - * CommandGroupBase#clearGroupedCommand(Command)}. The decorated command can, however, be further - * decorated without issue. + *

Note: This decorator works by adding this command to a composition. The command the + * decorator was called on cannot be scheduled independently or be added to a different + * composition (namely, decorators), unless it is manually cleared from the list of composed + * commands with {@link CommandScheduler#removeComposedCommand(Command)}. The command composition + * returned from this method can be further decorated without issue. * * @param toRun the Runnable to run * @param requirements the required subsystems @@ -140,11 +140,11 @@ public interface Command { /** * Decorates this command with another command to run before this command starts. * - *

Note: This decorator works by composing this command within a CommandGroup. The command - * cannot be used independently after being decorated, or be re-decorated with a different - * decorator, unless it is manually cleared from the list of grouped commands with {@link - * CommandGroupBase#clearGroupedCommand(Command)}. The decorated command can, however, be further - * decorated without issue. + *

Note: This decorator works by adding this command to a composition. The command the + * decorator was called on cannot be scheduled independently or be added to a different + * composition (namely, decorators), unless it is manually cleared from the list of composed + * commands with {@link CommandScheduler#removeComposedCommand(Command)}. The command composition + * returned from this method can be further decorated without issue. * * @param before the command to run before this one * @return the decorated command @@ -156,11 +156,11 @@ public interface Command { /** * Decorates this command with a runnable to run after the command finishes. * - *

Note: This decorator works by composing this command within a CommandGroup. The command - * cannot be used independently after being decorated, or be re-decorated with a different - * decorator, unless it is manually cleared from the list of grouped commands with {@link - * CommandGroupBase#clearGroupedCommand(Command)}. The decorated command can, however, be further - * decorated without issue. + *

Note: This decorator works by adding this command to a composition. The command the + * decorator was called on cannot be scheduled independently or be added to a different + * composition (namely, decorators), unless it is manually cleared from the list of composed + * commands with {@link CommandScheduler#removeComposedCommand(Command)}. The command composition + * returned from this method can be further decorated without issue. * * @param toRun the Runnable to run * @param requirements the required subsystems @@ -174,11 +174,11 @@ public interface Command { * Decorates this command with a set of commands to run after it in sequence. Often more * convenient/less-verbose than constructing a new {@link SequentialCommandGroup} explicitly. * - *

Note: This decorator works by composing this command within a CommandGroup. The command - * cannot be used independently after being decorated, or be re-decorated with a different - * decorator, unless it is manually cleared from the list of grouped commands with {@link - * CommandGroupBase#clearGroupedCommand(Command)}. The decorated command can, however, be further - * decorated without issue. + *

Note: This decorator works by adding this command to a composition. The command the + * decorator was called on cannot be scheduled independently or be added to a different + * composition (namely, decorators), unless it is manually cleared from the list of composed + * commands with {@link CommandScheduler#removeComposedCommand(Command)}. The command composition + * returned from this method can be further decorated without issue. * * @param next the commands to run next * @return the decorated command @@ -194,11 +194,11 @@ public interface Command { * command ends and interrupting all the others. Often more convenient/less-verbose than * constructing a new {@link ParallelDeadlineGroup} explicitly. * - *

Note: This decorator works by composing this command within a CommandGroup. The command - * cannot be used independently after being decorated, or be re-decorated with a different - * decorator, unless it is manually cleared from the list of grouped commands with {@link - * CommandGroupBase#clearGroupedCommand(Command)}. The decorated command can, however, be further - * decorated without issue. + *

Note: This decorator works by adding this command to a composition. The command the + * decorator was called on cannot be scheduled independently or be added to a different + * composition (namely, decorators), unless it is manually cleared from the list of composed + * commands with {@link CommandScheduler#removeComposedCommand(Command)}. The command composition + * returned from this method can be further decorated without issue. * * @param parallel the commands to run in parallel * @return the decorated command @@ -212,11 +212,11 @@ public interface Command { * command ends. Often more convenient/less-verbose than constructing a new {@link * ParallelCommandGroup} explicitly. * - *

Note: This decorator works by composing this command within a CommandGroup. The command - * cannot be used independently after being decorated, or be re-decorated with a different - * decorator, unless it is manually cleared from the list of grouped commands with {@link - * CommandGroupBase#clearGroupedCommand(Command)}. The decorated command can, however, be further - * decorated without issue. + *

Note: This decorator works by adding this command to a composition. The command the + * decorator was called on cannot be scheduled independently or be added to a different + * composition (namely, decorators), unless it is manually cleared from the list of composed + * commands with {@link CommandScheduler#removeComposedCommand(Command)}. The command composition + * returned from this method can be further decorated without issue. * * @param parallel the commands to run in parallel * @return the decorated command @@ -232,11 +232,11 @@ public interface Command { * command ends. Often more convenient/less-verbose than constructing a new {@link * ParallelRaceGroup} explicitly. * - *

Note: This decorator works by composing this command within a CommandGroup. The command - * cannot be used independently after being decorated, or be re-decorated with a different - * decorator, unless it is manually cleared from the list of grouped commands with {@link - * CommandGroupBase#clearGroupedCommand(Command)}. The decorated command can, however, be further - * decorated without issue. + *

Note: This decorator works by adding this command to a composition. The command the + * decorator was called on cannot be scheduled independently or be added to a different + * composition (namely, decorators), unless it is manually cleared from the list of composed + * commands with {@link CommandScheduler#removeComposedCommand(Command)}. The command composition + * returned from this method can be further decorated without issue. * * @param parallel the commands to run in parallel * @return the decorated command @@ -251,11 +251,11 @@ public interface Command { * Decorates this command to run perpetually, ignoring its ordinary end conditions. The decorated * command can still be interrupted or canceled. * - *

Note: This decorator works by composing this command within a CommandGroup. The command - * cannot be used independently after being decorated, or be re-decorated with a different - * decorator, unless it is manually cleared from the list of grouped commands with {@link - * CommandGroupBase#clearGroupedCommand(Command)}. The decorated command can, however, be further - * decorated without issue. + *

Note: This decorator works by adding this command to a composition. The command the + * decorator was called on cannot be scheduled independently or be added to a different + * composition (namely, decorators), unless it is manually cleared from the list of composed + * commands with {@link CommandScheduler#removeComposedCommand(Command)}. The command composition + * returned from this method can be further decorated without issue. * * @return the decorated command * @deprecated PerpetualCommand violates the assumption that execute() doesn't get called after @@ -273,11 +273,11 @@ public interface Command { * Decorates this command to run repeatedly, restarting it when it ends, until this command is * interrupted. The decorated command can still be canceled. * - *

Note: This decorator works by composing this command within a CommandGroup. The command - * cannot be used independently after being decorated, or be re-decorated with a different - * decorator, unless it is manually cleared from the list of grouped commands with {@link - * CommandGroupBase#clearGroupedCommand(Command)}. The decorated command can, however, be further - * decorated without issue. + *

Note: This decorator works by adding this command to a composition. The command the + * decorator was called on cannot be scheduled independently or be added to a different + * composition (namely, decorators), unless it is manually cleared from the list of composed + * commands with {@link CommandScheduler#removeComposedCommand(Command)}. The command composition + * returned from this method can be further decorated without issue. * * @return the decorated command */ @@ -287,8 +287,8 @@ public interface Command { /** * 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. + * useful for "forking off" from command compositions when the user does not wish to extend the + * command's requirements to the entire command composition. * * @return the decorated command */ @@ -391,7 +391,7 @@ public interface Command { /** * Whether or not the command is currently scheduled. Note that this does not detect whether the - * command is being run by a CommandGroup, only whether it is directly being run by the scheduler. + * command is in a composition, only whether it is directly being run by the scheduler. * * @return Whether the command is scheduled. */ diff --git a/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/CommandBase.java b/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/CommandBase.java index 25b2156c1d..30b8686d2b 100644 --- a/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/CommandBase.java +++ b/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/CommandBase.java @@ -104,6 +104,6 @@ public abstract class CommandBase implements Sendable, Command { } }); builder.addBooleanProperty( - ".isParented", () -> CommandGroupBase.getGroupedCommands().contains(this), null); + ".isParented", () -> CommandScheduler.getInstance().isComposed(this), null); } } diff --git a/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/CommandGroupBase.java b/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/CommandGroupBase.java index ad62554bcb..73a13423ee 100644 --- a/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/CommandGroupBase.java +++ b/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/CommandGroupBase.java @@ -4,75 +4,15 @@ package edu.wpi.first.wpilibj2.command; -import java.util.Collection; -import java.util.Collections; -import java.util.Set; -import java.util.WeakHashMap; - /** - * A base for CommandGroups. Statically tracks commands that have been allocated to groups to ensure - * those commands are not also used independently, which can result in inconsistent command state - * and unpredictable execution. + * A base for CommandGroups. * *

This class is provided by the NewCommands VendorDep + * + * @deprecated This class is an empty abstraction. Inherit directly from CommandBase/Command. */ +@Deprecated(forRemoval = true) public abstract class CommandGroupBase extends CommandBase { - private static final Set m_groupedCommands = - Collections.newSetFromMap(new WeakHashMap<>()); - - static void registerGroupedCommands(Command... commands) { - m_groupedCommands.addAll(Set.of(commands)); - } - - /** - * Clears the list of grouped commands, allowing all commands to be freely used again. - * - *

WARNING: Using this haphazardly can result in unexpected/undesirable behavior. Do not use - * this unless you fully understand what you are doing. - */ - public static void clearGroupedCommands() { - m_groupedCommands.clear(); - } - - /** - * Removes a single command from the list of grouped commands, allowing it to be freely used - * again. - * - *

WARNING: Using this haphazardly can result in unexpected/undesirable behavior. Do not use - * this unless you fully understand what you are doing. - * - * @param command the command to remove from the list of grouped commands - */ - public static void clearGroupedCommand(Command command) { - m_groupedCommands.remove(command); - } - - /** - * Requires that the specified commands not have been already allocated to a CommandGroup. Throws - * an {@link IllegalArgumentException} if commands have been allocated. - * - * @param commands The commands to check - */ - public static void requireUngrouped(Command... commands) { - requireUngrouped(Set.of(commands)); - } - - /** - * Requires that the specified commands not have been already allocated to a CommandGroup. Throws - * an {@link IllegalArgumentException} if commands have been allocated. - * - * @param commands The commands to check - */ - public static void requireUngrouped(Collection commands) { - if (!Collections.disjoint(commands, getGroupedCommands())) { - throw new IllegalArgumentException("Commands cannot be added to more than one CommandGroup"); - } - } - - static Set getGroupedCommands() { - return m_groupedCommands; - } - /** * Adds the given commands to the command group. * @@ -85,8 +25,10 @@ public abstract class CommandGroupBase extends CommandBase { * * @param commands the commands to include * @return the command group + * @deprecated Replace with {@link Commands#sequence(Command...)} */ - public static CommandGroupBase sequence(Command... commands) { + @Deprecated + public static SequentialCommandGroup sequence(Command... commands) { return new SequentialCommandGroup(commands); } @@ -95,8 +37,10 @@ public abstract class CommandGroupBase extends CommandBase { * * @param commands the commands to include * @return the command group + * @deprecated Replace with {@link Commands#parallel(Command...)} */ - public static CommandGroupBase parallel(Command... commands) { + @Deprecated + public static ParallelCommandGroup parallel(Command... commands) { return new ParallelCommandGroup(commands); } @@ -105,8 +49,10 @@ public abstract class CommandGroupBase extends CommandBase { * * @param commands the commands to include * @return the command group + * @deprecated Replace with {@link Commands#race(Command...)} */ - public static CommandGroupBase race(Command... commands) { + @Deprecated + public static ParallelRaceGroup race(Command... commands) { return new ParallelRaceGroup(commands); } @@ -116,8 +62,10 @@ public abstract class CommandGroupBase extends CommandBase { * @param deadline the deadline command * @param commands the commands to include * @return the command group + * @deprecated Replace with {@link Commands#deadline(Command, Command...)} */ - public static CommandGroupBase deadline(Command deadline, Command... commands) { + @Deprecated + public static ParallelDeadlineGroup deadline(Command deadline, Command... commands) { return new ParallelDeadlineGroup(deadline, commands); } } diff --git a/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/CommandScheduler.java b/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/CommandScheduler.java index 4fb89ac361..82d89f6fe1 100644 --- a/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/CommandScheduler.java +++ b/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/CommandScheduler.java @@ -26,6 +26,7 @@ import edu.wpi.first.wpilibj.event.EventLoop; import edu.wpi.first.wpilibj.livewindow.LiveWindow; import edu.wpi.first.wpilibj2.command.Command.InterruptionBehavior; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.Iterator; import java.util.LinkedHashMap; @@ -33,6 +34,7 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.WeakHashMap; import java.util.function.Consumer; /** @@ -60,6 +62,8 @@ public final class CommandScheduler implements NTSendable, AutoCloseable { return instance; } + private final Set m_composedCommands = Collections.newSetFromMap(new WeakHashMap<>()); + // A set of the currently-running commands. private final Set m_scheduledCommands = new LinkedHashSet<>(); @@ -208,10 +212,7 @@ public final class CommandScheduler implements NTSendable, AutoCloseable { return; } - if (CommandGroupBase.getGroupedCommands().contains(command)) { - throw new IllegalArgumentException( - "A command that is part of a command group cannot be independently scheduled"); - } + requireNotComposed(command); // Do nothing if the scheduler is disabled, the robot is disabled and the command doesn't // run when disabled, or the command is already scheduled. @@ -405,6 +406,8 @@ public final class CommandScheduler implements NTSendable, AutoCloseable { return; } + requireNotComposed(defaultCommand); + if (!defaultCommand.getRequirements().contains(subsystem)) { throw new IllegalArgumentException("Default commands must require their subsystem!"); } @@ -489,8 +492,8 @@ public final class CommandScheduler implements NTSendable, AutoCloseable { /** * Whether the given commands are running. Note that this only works on commands that are directly - * scheduled by the scheduler; it will not work on commands inside of CommandGroups, as the - * scheduler does not see them. + * scheduled by the scheduler; it will not work on commands inside compositions, as the scheduler + * does not see them. * * @param commands the command to query * @return whether the command is currently scheduled @@ -557,6 +560,85 @@ public final class CommandScheduler implements NTSendable, AutoCloseable { m_finishActions.add(requireNonNullParam(action, "action", "onCommandFinish")); } + /** + * Register commands as composed. An exception will be thrown if these commands are scheduled + * directly or added to a composition. + * + * @param commands the commands to register + * @throws IllegalArgumentException if the given commands have already been composed. + */ + public void registerComposedCommands(Command... commands) { + var commandSet = Set.of(commands); + requireNotComposed(commandSet); + m_composedCommands.addAll(commandSet); + } + + /** + * Clears the list of composed commands, allowing all commands to be freely used again. + * + *

WARNING: Using this haphazardly can result in unexpected/undesirable behavior. Do not use + * this unless you fully understand what you are doing. + */ + public void clearComposedCommands() { + m_composedCommands.clear(); + } + + /** + * Removes a single command from the list of composed commands, allowing it to be freely used + * again. + * + *

WARNING: Using this haphazardly can result in unexpected/undesirable behavior. Do not use + * this unless you fully understand what you are doing. + * + * @param command the command to remove from the list of grouped commands + */ + public void removeComposedCommand(Command command) { + m_composedCommands.remove(command); + } + + /** + * Requires that the specified command hasn't been already added to a composition. + * + * @param command The command to check + * @throws IllegalArgumentException if the given commands have already been composed. + */ + public void requireNotComposed(Command command) { + if (m_composedCommands.contains(command)) { + throw new IllegalArgumentException( + "Commands that have been composed may not be added to another composition or scheduled" + + "individually!"); + } + } + + /** + * Requires that the specified commands not have been already added to a composition. + * + * @param commands The commands to check + * @throws IllegalArgumentException if the given commands have already been composed. + */ + public void requireNotComposed(Collection commands) { + if (!Collections.disjoint(commands, getComposedCommands())) { + throw new IllegalArgumentException( + "Commands that have been composed may not be added to another composition or scheduled" + + "individually!"); + } + } + + /** + * Check if the given command has been composed. + * + * @param command The command to check + * @return true if composed + * @throws IllegalArgumentException if the given commands have already been composed. + */ + public boolean isComposed(Command command) { + return getComposedCommands().contains(command); + } + + Set getComposedCommands() { + return m_composedCommands; + } + @Override public void initSendable(NTSendableBuilder builder) { builder.setSmartDashboardType("Scheduler"); diff --git a/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/ConditionalCommand.java b/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/ConditionalCommand.java index 18d392aa7a..8a299b2dbb 100644 --- a/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/ConditionalCommand.java +++ b/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/ConditionalCommand.java @@ -5,22 +5,16 @@ package edu.wpi.first.wpilibj2.command; import static edu.wpi.first.wpilibj.util.ErrorMessages.requireNonNullParam; -import static edu.wpi.first.wpilibj2.command.CommandGroupBase.requireUngrouped; import java.util.function.BooleanSupplier; /** - * Runs one of two commands, depending on the value of the given condition when this command is - * initialized. Does not actually schedule the selected command - rather, the command is run through - * this command; this ensures that the command will behave as expected if used as part of a - * CommandGroup. Requires the requirements of both commands, again to ensure proper functioning when - * used in a CommandGroup. If this is undesired, consider using {@link ScheduleCommand}. + * A command composition that runs one of two commands, depending on the value of the given + * condition when this command is initialized. * - *

As this command contains multiple component commands within it, it is technically a command - * group; the command instances that are passed to it cannot be added to any other groups, or - * scheduled individually. - * - *

As a rule, CommandGroups require the union of the requirements of their component commands. + *

The rules for command compositions apply: command instances that are passed to it cannot be + * added to any other composition or scheduled individually, and the composition requires all + * subsystems its components require. * *

This class is provided by the NewCommands VendorDep */ @@ -38,13 +32,12 @@ public class ConditionalCommand extends CommandBase { * @param condition the condition to determine which command to run */ public ConditionalCommand(Command onTrue, Command onFalse, BooleanSupplier condition) { - requireUngrouped(onTrue, onFalse); - - CommandGroupBase.registerGroupedCommands(onTrue, onFalse); - - m_onTrue = onTrue; - m_onFalse = onFalse; + m_onTrue = requireNonNullParam(onTrue, "onTrue", "ConditionalCommand"); + m_onFalse = requireNonNullParam(onFalse, "onFalse", "ConditionalCommand"); m_condition = requireNonNullParam(condition, "condition", "ConditionalCommand"); + + CommandScheduler.getInstance().registerComposedCommands(onTrue, onFalse); + m_requirements.addAll(m_onTrue.getRequirements()); m_requirements.addAll(m_onFalse.getRequirements()); } diff --git a/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/ParallelCommandGroup.java b/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/ParallelCommandGroup.java index 40a85a714c..3b36f42eca 100644 --- a/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/ParallelCommandGroup.java +++ b/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/ParallelCommandGroup.java @@ -9,24 +9,27 @@ import java.util.HashMap; import java.util.Map; /** - * A CommandGroup that runs a set of commands in parallel, ending when the last command ends. + * A command composition that runs a set of commands in parallel, ending when the last command ends. * - *

As a rule, CommandGroups require the union of the requirements of their component commands. + *

The rules for command compositions apply: command instances that are passed to it cannot be + * added to any other composition or scheduled individually, and the composition requires all + * subsystems its components require. * *

This class is provided by the NewCommands VendorDep */ +@SuppressWarnings("removal") public class ParallelCommandGroup extends CommandGroupBase { - // maps commands in this group to whether they are still running + // maps commands in this composition to whether they are still running private final Map m_commands = new HashMap<>(); private boolean m_runWhenDisabled = true; private InterruptionBehavior m_interruptBehavior = InterruptionBehavior.kCancelIncoming; /** * Creates a new ParallelCommandGroup. The given commands will be executed simultaneously. The - * command group will finish when the last command finishes. If the CommandGroup is interrupted, - * only the commands that are still running will be interrupted. + * command composition will finish when the last command finishes. If the composition is + * interrupted, only the commands that are still running will be interrupted. * - * @param commands the commands to include in this group. + * @param commands the commands to include in this composition. */ public ParallelCommandGroup(Command... commands) { addCommands(commands); @@ -34,19 +37,17 @@ public class ParallelCommandGroup extends CommandGroupBase { @Override public final void addCommands(Command... commands) { - requireUngrouped(commands); - if (m_commands.containsValue(true)) { throw new IllegalStateException( - "Commands cannot be added to a CommandGroup while the group is running"); + "Commands cannot be added to a composition while it's running"); } - registerGroupedCommands(commands); + CommandScheduler.getInstance().registerComposedCommands(commands); for (Command command : commands) { if (!Collections.disjoint(command.getRequirements(), m_requirements)) { throw new IllegalArgumentException( - "Multiple commands in a parallel group cannot" + "require the same subsystems"); + "Multiple commands in a parallel composition cannot require the same subsystems"); } m_commands.put(command, false); m_requirements.addAll(command.getRequirements()); diff --git a/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/ParallelDeadlineGroup.java b/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/ParallelDeadlineGroup.java index e3ac8fd85f..0687ed1a9c 100644 --- a/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/ParallelDeadlineGroup.java +++ b/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/ParallelDeadlineGroup.java @@ -9,15 +9,19 @@ import java.util.HashMap; import java.util.Map; /** - * A CommandGroup that runs a set of commands in parallel, ending only when a specific command (the - * "deadline") ends, interrupting all other commands that are still running at that point. + * A command composition that runs a set of commands in parallel, ending only when a specific + * command (the "deadline") ends, interrupting all other commands that are still running at that + * point. * - *

As a rule, CommandGroups require the union of the requirements of their component commands. + *

The rules for command compositions apply: command instances that are passed to it cannot be + * added to any other composition or scheduled individually, and the composition requires all + * subsystems its components require. * *

This class is provided by the NewCommands VendorDep */ +@SuppressWarnings("removal") public class ParallelDeadlineGroup extends CommandGroupBase { - // maps commands in this group to whether they are still running + // maps commands in this composition to whether they are still running private final Map m_commands = new HashMap<>(); private boolean m_runWhenDisabled = true; private boolean m_finished = true; @@ -26,11 +30,11 @@ public class ParallelDeadlineGroup extends CommandGroupBase { /** * Creates a new ParallelDeadlineGroup. The given commands (including the deadline) will be - * executed simultaneously. The CommandGroup will finish when the deadline finishes, interrupting - * all other still-running commands. If the CommandGroup is interrupted, only the commands still + * executed simultaneously. The composition will finish when the deadline finishes, interrupting + * all other still-running commands. If the composition is interrupted, only the commands still * running will be interrupted. * - * @param deadline the command that determines when the group ends + * @param deadline the command that determines when the composition ends * @param commands the commands to be executed */ public ParallelDeadlineGroup(Command deadline, Command... commands) { @@ -56,14 +60,12 @@ public class ParallelDeadlineGroup extends CommandGroupBase { @Override public final void addCommands(Command... commands) { - requireUngrouped(commands); - if (!m_finished) { throw new IllegalStateException( - "Commands cannot be added to a CommandGroup while the group is running"); + "Commands cannot be added to a composition while it's running"); } - registerGroupedCommands(commands); + CommandScheduler.getInstance().registerComposedCommands(commands); for (Command command : commands) { if (!Collections.disjoint(command.getRequirements(), m_requirements)) { diff --git a/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/ParallelRaceGroup.java b/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/ParallelRaceGroup.java index 43172af6df..e5ba80d173 100644 --- a/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/ParallelRaceGroup.java +++ b/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/ParallelRaceGroup.java @@ -9,13 +9,16 @@ import java.util.HashSet; import java.util.Set; /** - * A CommandGroup that runs a set of commands in parallel, ending when any one of the commands ends + * A composition that runs a set of commands in parallel, ending when any one of the commands ends * and interrupting all the others. * - *

As a rule, CommandGroups require the union of the requirements of their component commands. + *

The rules for command compositions apply: command instances that are passed to it cannot be + * added to any other composition or scheduled individually, and the composition requires all + * subsystems its components require. * *

This class is provided by the NewCommands VendorDep */ +@SuppressWarnings("removal") public class ParallelRaceGroup extends CommandGroupBase { private final Set m_commands = new HashSet<>(); private boolean m_runWhenDisabled = true; @@ -27,7 +30,7 @@ public class ParallelRaceGroup extends CommandGroupBase { * "race to the finish" - the first command to finish ends the entire command, with all other * commands being interrupted. * - * @param commands the commands to include in this group. + * @param commands the commands to include in this composition. */ public ParallelRaceGroup(Command... commands) { addCommands(commands); @@ -35,19 +38,17 @@ public class ParallelRaceGroup extends CommandGroupBase { @Override public final void addCommands(Command... commands) { - requireUngrouped(commands); - if (!m_finished) { throw new IllegalStateException( - "Commands cannot be added to a CommandGroup while the group is running"); + "Commands cannot be added to a composition while it's running!"); } - registerGroupedCommands(commands); + CommandScheduler.getInstance().registerComposedCommands(commands); for (Command command : commands) { if (!Collections.disjoint(command.getRequirements(), m_requirements)) { throw new IllegalArgumentException( - "Multiple commands in a parallel group cannot" + " require the same subsystems"); + "Multiple commands in a parallel composition cannot require the same subsystems"); } m_commands.add(command); m_requirements.addAll(command.getRequirements()); diff --git a/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/PerpetualCommand.java b/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/PerpetualCommand.java index 9ef643e4a4..9fb90190a7 100644 --- a/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/PerpetualCommand.java +++ b/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/PerpetualCommand.java @@ -4,12 +4,9 @@ package edu.wpi.first.wpilibj2.command; -import static edu.wpi.first.wpilibj2.command.CommandGroupBase.registerGroupedCommands; -import static edu.wpi.first.wpilibj2.command.CommandGroupBase.requireUngrouped; - /** * A command that runs another command in perpetuity, ignoring that command's end conditions. While - * this class does not extend {@link CommandGroupBase}, it is still considered a CommandGroup, as it + * this class does not extend {@link CommandGroupBase}, it is still considered a composition, as it * allows one to compose another command within it; the command instances that are passed to it * cannot be added to any other groups, or scheduled individually. * @@ -33,8 +30,7 @@ public class PerpetualCommand extends CommandBase { * @param command the command to run perpetually */ public PerpetualCommand(Command command) { - requireUngrouped(command); - registerGroupedCommands(command); + CommandScheduler.getInstance().registerComposedCommands(command); m_command = command; m_requirements.addAll(command.getRequirements()); } diff --git a/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/RepeatCommand.java b/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/RepeatCommand.java index c1401ec626..b70f9c521c 100644 --- a/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/RepeatCommand.java +++ b/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/RepeatCommand.java @@ -4,16 +4,16 @@ package edu.wpi.first.wpilibj2.command; -import static edu.wpi.first.wpilibj2.command.CommandGroupBase.registerGroupedCommands; -import static edu.wpi.first.wpilibj2.command.CommandGroupBase.requireUngrouped; +import static edu.wpi.first.wpilibj.util.ErrorMessages.requireNonNullParam; /** * A command that runs another command repeatedly, restarting it when it ends, until this command is - * interrupted. While this class does not extend {@link CommandGroupBase}, it is still considered a - * CommandGroup, as it allows one to compose another command within it; the command instances that - * are passed to it cannot be added to any other groups, or scheduled individually. + * interrupted. Command instances that are passed to it cannot be added to any other groups, or + * scheduled individually. * - *

As a rule, CommandGroups require the union of the requirements of their component commands. + *

The rules for command compositions apply: command instances that are passed to it cannot be + * added to any other composition or scheduled individually,and the composition requires all + * subsystems its components require. * *

This class is provided by the NewCommands VendorDep */ @@ -28,9 +28,8 @@ public class RepeatCommand extends CommandBase { * @param command the command to run repeatedly */ public RepeatCommand(Command command) { - requireUngrouped(command); - registerGroupedCommands(command); - m_command = command; + m_command = requireNonNullParam(command, "command", "RepeatCommand"); + CommandScheduler.getInstance().registerComposedCommands(command); m_requirements.addAll(command.getRequirements()); } diff --git a/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/ScheduleCommand.java b/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/ScheduleCommand.java index 98062f9a90..a61be2f336 100644 --- a/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/ScheduleCommand.java +++ b/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/ScheduleCommand.java @@ -8,8 +8,8 @@ import java.util.Set; /** * Schedules the given commands when this command is initialized. Useful for forking off from - * CommandGroups. Note that if run from a CommandGroup, the group will not know about the status of - * the scheduled commands, and will treat this command as finishing instantly. + * CommandGroups. Note that if run from a composition, the composition will not know about the + * status of the scheduled commands, and will treat this command as finishing instantly. * *

This class is provided by the NewCommands VendorDep */ diff --git a/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/SelectCommand.java b/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/SelectCommand.java index b3cd97b56d..a020c1fc9f 100644 --- a/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/SelectCommand.java +++ b/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/SelectCommand.java @@ -5,24 +5,17 @@ package edu.wpi.first.wpilibj2.command; import static edu.wpi.first.wpilibj.util.ErrorMessages.requireNonNullParam; -import static edu.wpi.first.wpilibj2.command.CommandGroupBase.requireUngrouped; import java.util.Map; import java.util.function.Supplier; /** - * Runs one of a selection of commands, either using a selector and a key to command mapping, or a - * supplier that returns the command directly at runtime. Does not actually schedule the selected - * command - rather, the command is run through this command; this ensures that the command will - * behave as expected if used as part of a CommandGroup. Requires the requirements of all included - * commands, again to ensure proper functioning when used in a CommandGroup. If this is undesired, - * consider using {@link ScheduleCommand}. + * A command composition that runs one of a selection of commands, either using a selector and a key + * to command mapping, or a supplier that returns the command directly at runtime. * - *

As this command contains multiple component commands within it, it is technically a command - * group; the command instances that are passed to it cannot be added to any other groups, or - * scheduled individually. - * - *

As a rule, CommandGroups require the union of the requirements of their component commands. + *

The rules for command compositions apply: command instances that are passed to it cannot be + * added to any other composition or scheduled individually, and the composition requires all + * subsystems its components require. * *

This class is provided by the NewCommands VendorDep */ @@ -41,13 +34,12 @@ public class SelectCommand extends CommandBase { * @param selector the selector to determine which command to run */ public SelectCommand(Map commands, Supplier selector) { - requireUngrouped(commands.values()); - - CommandGroupBase.registerGroupedCommands(commands.values().toArray(new Command[] {})); - m_commands = requireNonNullParam(commands, "commands", "SelectCommand"); m_selector = requireNonNullParam(selector, "selector", "SelectCommand"); + CommandScheduler.getInstance() + .registerComposedCommands(commands.values().toArray(new Command[] {})); + m_toRun = null; for (Command command : m_commands.values()) { diff --git a/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/SequentialCommandGroup.java b/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/SequentialCommandGroup.java index 4d1f44cefa..6de02e65f1 100644 --- a/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/SequentialCommandGroup.java +++ b/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/SequentialCommandGroup.java @@ -8,12 +8,15 @@ import java.util.ArrayList; import java.util.List; /** - * A CommandGroups that runs a list of commands in sequence. + * A command composition that runs a list of commands in sequence. * - *

As a rule, CommandGroups require the union of the requirements of their component commands. + *

The rules for command compositions apply: command instances that are passed to it cannot be + * added to any other composition or scheduled individually, and the composition requires all + * subsystems its components require. * *

This class is provided by the NewCommands VendorDep */ +@SuppressWarnings("removal") public class SequentialCommandGroup extends CommandGroupBase { private final List m_commands = new ArrayList<>(); private int m_currentCommandIndex = -1; @@ -22,9 +25,9 @@ public class SequentialCommandGroup extends CommandGroupBase { /** * Creates a new SequentialCommandGroup. The given commands will be run sequentially, with the - * CommandGroup finishing when the last command finishes. + * composition finishing when the last command finishes. * - * @param commands the commands to include in this group. + * @param commands the commands to include in this composition. */ public SequentialCommandGroup(Command... commands) { addCommands(commands); @@ -32,14 +35,12 @@ public class SequentialCommandGroup extends CommandGroupBase { @Override public final void addCommands(Command... commands) { - requireUngrouped(commands); - if (m_currentCommandIndex != -1) { throw new IllegalStateException( - "Commands cannot be added to a CommandGroup while the group is running"); + "Commands cannot be added to a composition while it's running"); } - registerGroupedCommands(commands); + CommandScheduler.getInstance().registerComposedCommands(commands); for (Command command : commands) { m_commands.add(command); diff --git a/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/WrapperCommand.java b/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/WrapperCommand.java index de676d865d..3ae055a874 100644 --- a/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/WrapperCommand.java +++ b/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/WrapperCommand.java @@ -4,17 +4,15 @@ package edu.wpi.first.wpilibj2.command; -import static edu.wpi.first.wpilibj2.command.CommandGroupBase.registerGroupedCommands; -import static edu.wpi.first.wpilibj2.command.CommandGroupBase.requireUngrouped; - import java.util.Set; /** * A class used internally to wrap commands while overriding a specific method; all other methods * will call through to the wrapped command. * - *

Wrapped commands may only be used through the wrapper, trying to directly schedule them or add - * them to a group will throw an exception. + *

The rules for command compositions apply: command instances that are passed to it cannot be + * added to any other composition or scheduled individually, and the composition requires all + * subsystems its components require. */ public abstract class WrapperCommand extends CommandBase { protected final Command m_command; @@ -23,11 +21,10 @@ public abstract class WrapperCommand extends CommandBase { * Wrap a command. * * @param command the command being wrapped. Trying to directly schedule this command or add it to - * a group will throw an exception. + * a composition will throw an exception. */ protected WrapperCommand(Command command) { - requireUngrouped(command); - registerGroupedCommands(command); + CommandScheduler.getInstance().registerComposedCommands(command); m_command = command; } diff --git a/wpilibNewCommands/src/main/native/cpp/frc2/command/Command.cpp b/wpilibNewCommands/src/main/native/cpp/frc2/command/Command.cpp index 45c7febf88..b0b88e4a1b 100644 --- a/wpilibNewCommands/src/main/native/cpp/frc2/command/Command.cpp +++ b/wpilibNewCommands/src/main/native/cpp/frc2/command/Command.cpp @@ -25,7 +25,7 @@ Command::~Command() { } Command& Command::operator=(const Command& rhs) { - m_isGrouped = false; + m_isComposed = false; return *this; } @@ -135,12 +135,20 @@ std::string Command::GetName() const { void Command::SetName(std::string_view name) {} +bool Command::IsComposed() const { + return m_isComposed; +} + +void Command::SetComposed(bool isComposed) { + m_isComposed = isComposed; +} + bool Command::IsGrouped() const { - return m_isGrouped; + return IsComposed(); } void Command::SetGrouped(bool grouped) { - m_isGrouped = grouped; + SetComposed(grouped); } namespace frc2 { diff --git a/wpilibNewCommands/src/main/native/cpp/frc2/command/CommandGroupBase.cpp b/wpilibNewCommands/src/main/native/cpp/frc2/command/CommandGroupBase.cpp index e92cc6720d..eb9c293e4d 100644 --- a/wpilibNewCommands/src/main/native/cpp/frc2/command/CommandGroupBase.cpp +++ b/wpilibNewCommands/src/main/native/cpp/frc2/command/CommandGroupBase.cpp @@ -5,44 +5,3 @@ #include "frc2/command/CommandGroupBase.h" using namespace frc2; - -bool CommandGroupBase::RequireUngrouped(const Command& command) { - if (command.IsGrouped()) { - throw FRC_MakeError( - frc::err::CommandIllegalUse, - "Commands cannot be added to more than one CommandGroup"); - } - return true; -} - -bool CommandGroupBase::RequireUngrouped(const Command* command) { - return RequireUngrouped(*command); -} - -bool CommandGroupBase::RequireUngrouped( - std::span> commands) { - bool allUngrouped = true; - for (auto&& command : commands) { - allUngrouped &= !command.get()->IsGrouped(); - } - if (!allUngrouped) { - throw FRC_MakeError( - frc::err::CommandIllegalUse, - "Commands cannot be added to more than one CommandGroup"); - } - return allUngrouped; -} - -bool CommandGroupBase::RequireUngrouped( - std::initializer_list commands) { - bool allUngrouped = true; - for (auto&& command : commands) { - allUngrouped &= !command->IsGrouped(); - } - if (!allUngrouped) { - throw FRC_MakeError( - frc::err::CommandIllegalUse, - "Commands cannot be added to more than one CommandGroup"); - } - return allUngrouped; -} diff --git a/wpilibNewCommands/src/main/native/cpp/frc2/command/CommandScheduler.cpp b/wpilibNewCommands/src/main/native/cpp/frc2/command/CommandScheduler.cpp index f096e493bf..049e9c5d98 100644 --- a/wpilibNewCommands/src/main/native/cpp/frc2/command/CommandScheduler.cpp +++ b/wpilibNewCommands/src/main/native/cpp/frc2/command/CommandScheduler.cpp @@ -19,7 +19,6 @@ #include #include -#include "frc2/command/CommandGroupBase.h" #include "frc2/command/CommandPtr.h" #include "frc2/command/Subsystem.h" @@ -118,12 +117,8 @@ void CommandScheduler::Schedule(Command* command) { return; } - if (command->IsGrouped()) { - throw FRC_MakeError(frc::err::CommandIllegalUse, - "A command that is part of a command group " - "cannot be independently scheduled"); - return; - } + RequireUngrouped(command); + if (m_impl->disabled || (frc::RobotState::IsDisabled() && !command->RunsWhenDisabled()) || m_impl->scheduledCommands.contains(command)) { @@ -307,6 +302,7 @@ void CommandScheduler::SetDefaultCommand(Subsystem* subsystem, throw FRC_MakeError(frc::err::CommandIllegalUse, "{}", "Default commands must require their subsystem!"); } + RequireUngrouped(defaultCommand.get()); SetDefaultCommandImpl(subsystem, std::move(defaultCommand).Unwrap()); } @@ -436,6 +432,28 @@ void CommandScheduler::OnCommandFinish(Action action) { m_impl->finishActions.emplace_back(std::move(action)); } +void CommandScheduler::RequireUngrouped(const Command* command) { + if (command->IsComposed()) { + throw FRC_MakeError( + frc::err::CommandIllegalUse, + "Commands cannot be added to more than one CommandGroup"); + } +} + +void CommandScheduler::RequireUngrouped( + std::span> commands) { + for (auto&& command : commands) { + RequireUngrouped(command.get()); + } +} + +void CommandScheduler::RequireUngrouped( + std::initializer_list commands) { + for (auto&& command : commands) { + RequireUngrouped(command); + } +} + void CommandScheduler::InitSendable(nt::NTSendableBuilder& builder) { builder.SetSmartDashboardType("Scheduler"); builder.SetUpdateTable( diff --git a/wpilibNewCommands/src/main/native/cpp/frc2/command/ConditionalCommand.cpp b/wpilibNewCommands/src/main/native/cpp/frc2/command/ConditionalCommand.cpp index 30ef410741..9de3ce9ae5 100644 --- a/wpilibNewCommands/src/main/native/cpp/frc2/command/ConditionalCommand.cpp +++ b/wpilibNewCommands/src/main/native/cpp/frc2/command/ConditionalCommand.cpp @@ -10,15 +10,14 @@ ConditionalCommand::ConditionalCommand(std::unique_ptr&& onTrue, std::unique_ptr&& onFalse, std::function condition) : m_condition{std::move(condition)} { - if (!CommandGroupBase::RequireUngrouped({onTrue.get(), onFalse.get()})) { - return; - } + CommandScheduler::GetInstance().RequireUngrouped( + {onTrue.get(), onFalse.get()}); m_onTrue = std::move(onTrue); m_onFalse = std::move(onFalse); - m_onTrue->SetGrouped(true); - m_onFalse->SetGrouped(true); + m_onTrue->SetComposed(true); + m_onFalse->SetComposed(true); m_runsWhenDisabled &= m_onTrue->RunsWhenDisabled(); m_runsWhenDisabled &= m_onFalse->RunsWhenDisabled(); diff --git a/wpilibNewCommands/src/main/native/cpp/frc2/command/ParallelCommandGroup.cpp b/wpilibNewCommands/src/main/native/cpp/frc2/command/ParallelCommandGroup.cpp index 53f0ca7634..99e0845c8d 100644 --- a/wpilibNewCommands/src/main/native/cpp/frc2/command/ParallelCommandGroup.cpp +++ b/wpilibNewCommands/src/main/native/cpp/frc2/command/ParallelCommandGroup.cpp @@ -63,11 +63,7 @@ Command::InterruptionBehavior ParallelCommandGroup::GetInterruptionBehavior() void ParallelCommandGroup::AddCommands( std::vector>&& commands) { - for (auto&& command : commands) { - if (!RequireUngrouped(*command)) { - return; - } - } + CommandScheduler::GetInstance().RequireUngrouped(commands); if (isRunning) { throw FRC_MakeError(frc::err::CommandIllegalUse, @@ -77,7 +73,7 @@ void ParallelCommandGroup::AddCommands( for (auto&& command : commands) { if (RequirementsDisjoint(this, command.get())) { - command->SetGrouped(true); + command->SetComposed(true); AddRequirements(command->GetRequirements()); m_runWhenDisabled &= command->RunsWhenDisabled(); if (command->GetInterruptionBehavior() == diff --git a/wpilibNewCommands/src/main/native/cpp/frc2/command/ParallelDeadlineGroup.cpp b/wpilibNewCommands/src/main/native/cpp/frc2/command/ParallelDeadlineGroup.cpp index 35d5f0fb4a..93d42baad2 100644 --- a/wpilibNewCommands/src/main/native/cpp/frc2/command/ParallelDeadlineGroup.cpp +++ b/wpilibNewCommands/src/main/native/cpp/frc2/command/ParallelDeadlineGroup.cpp @@ -60,9 +60,7 @@ Command::InterruptionBehavior ParallelDeadlineGroup::GetInterruptionBehavior() void ParallelDeadlineGroup::AddCommands( std::vector>&& commands) { - if (!RequireUngrouped(commands)) { - return; - } + CommandScheduler::GetInstance().RequireUngrouped(commands); if (!m_finished) { throw FRC_MakeError(frc::err::CommandIllegalUse, @@ -72,7 +70,7 @@ void ParallelDeadlineGroup::AddCommands( for (auto&& command : commands) { if (RequirementsDisjoint(this, command.get())) { - command->SetGrouped(true); + command->SetComposed(true); AddRequirements(command->GetRequirements()); m_runWhenDisabled &= command->RunsWhenDisabled(); if (command->GetInterruptionBehavior() == @@ -90,7 +88,7 @@ void ParallelDeadlineGroup::AddCommands( void ParallelDeadlineGroup::SetDeadline(std::unique_ptr&& deadline) { m_deadline = deadline.get(); - m_deadline->SetGrouped(true); + m_deadline->SetComposed(true); m_commands.emplace_back(std::move(deadline), false); AddRequirements(m_deadline->GetRequirements()); m_runWhenDisabled &= m_deadline->RunsWhenDisabled(); diff --git a/wpilibNewCommands/src/main/native/cpp/frc2/command/ParallelRaceGroup.cpp b/wpilibNewCommands/src/main/native/cpp/frc2/command/ParallelRaceGroup.cpp index 9a62b41e02..df658bcc73 100644 --- a/wpilibNewCommands/src/main/native/cpp/frc2/command/ParallelRaceGroup.cpp +++ b/wpilibNewCommands/src/main/native/cpp/frc2/command/ParallelRaceGroup.cpp @@ -50,9 +50,7 @@ Command::InterruptionBehavior ParallelRaceGroup::GetInterruptionBehavior() void ParallelRaceGroup::AddCommands( std::vector>&& commands) { - if (!RequireUngrouped(commands)) { - return; - } + CommandScheduler::GetInstance().RequireUngrouped(commands); if (isRunning) { throw FRC_MakeError(frc::err::CommandIllegalUse, @@ -62,7 +60,7 @@ void ParallelRaceGroup::AddCommands( for (auto&& command : commands) { if (RequirementsDisjoint(this, command.get())) { - command->SetGrouped(true); + command->SetComposed(true); AddRequirements(command->GetRequirements()); m_runWhenDisabled &= command->RunsWhenDisabled(); if (command->GetInterruptionBehavior() == diff --git a/wpilibNewCommands/src/main/native/cpp/frc2/command/PerpetualCommand.cpp b/wpilibNewCommands/src/main/native/cpp/frc2/command/PerpetualCommand.cpp index 0c199f2d05..2d0af1e153 100644 --- a/wpilibNewCommands/src/main/native/cpp/frc2/command/PerpetualCommand.cpp +++ b/wpilibNewCommands/src/main/native/cpp/frc2/command/PerpetualCommand.cpp @@ -7,11 +7,9 @@ using namespace frc2; PerpetualCommand::PerpetualCommand(std::unique_ptr&& command) { - if (!CommandGroupBase::RequireUngrouped(*command)) { - return; - } + CommandScheduler::GetInstance().RequireUngrouped(command.get()); m_command = std::move(command); - m_command->SetGrouped(true); + m_command->SetComposed(true); AddRequirements(m_command->GetRequirements()); } diff --git a/wpilibNewCommands/src/main/native/cpp/frc2/command/RepeatCommand.cpp b/wpilibNewCommands/src/main/native/cpp/frc2/command/RepeatCommand.cpp index 33d2639cb2..9fcbe68711 100644 --- a/wpilibNewCommands/src/main/native/cpp/frc2/command/RepeatCommand.cpp +++ b/wpilibNewCommands/src/main/native/cpp/frc2/command/RepeatCommand.cpp @@ -7,11 +7,9 @@ using namespace frc2; RepeatCommand::RepeatCommand(std::unique_ptr&& command) { - if (!CommandGroupBase::RequireUngrouped(*command)) { - return; - } + CommandScheduler::GetInstance().RequireUngrouped(command.get()); m_command = std::move(command); - m_command->SetGrouped(true); + m_command->SetComposed(true); AddRequirements(m_command->GetRequirements()); } diff --git a/wpilibNewCommands/src/main/native/cpp/frc2/command/SequentialCommandGroup.cpp b/wpilibNewCommands/src/main/native/cpp/frc2/command/SequentialCommandGroup.cpp index ad25eff1c2..532614e242 100644 --- a/wpilibNewCommands/src/main/native/cpp/frc2/command/SequentialCommandGroup.cpp +++ b/wpilibNewCommands/src/main/native/cpp/frc2/command/SequentialCommandGroup.cpp @@ -60,9 +60,7 @@ Command::InterruptionBehavior SequentialCommandGroup::GetInterruptionBehavior() void SequentialCommandGroup::AddCommands( std::vector>&& commands) { - if (!RequireUngrouped(commands)) { - return; - } + CommandScheduler::GetInstance().RequireUngrouped(commands); if (m_currentCommandIndex != invalid_index) { throw FRC_MakeError(frc::err::CommandIllegalUse, @@ -71,7 +69,7 @@ void SequentialCommandGroup::AddCommands( } for (auto&& command : commands) { - command->SetGrouped(true); + command->SetComposed(true); AddRequirements(command->GetRequirements()); m_runWhenDisabled &= command->RunsWhenDisabled(); if (command->GetInterruptionBehavior() == diff --git a/wpilibNewCommands/src/main/native/cpp/frc2/command/WrapperCommand.cpp b/wpilibNewCommands/src/main/native/cpp/frc2/command/WrapperCommand.cpp index a6e7468f7f..ff31b7261f 100644 --- a/wpilibNewCommands/src/main/native/cpp/frc2/command/WrapperCommand.cpp +++ b/wpilibNewCommands/src/main/native/cpp/frc2/command/WrapperCommand.cpp @@ -9,11 +9,9 @@ using namespace frc2; WrapperCommand::WrapperCommand(std::unique_ptr&& command) { - if (!CommandGroupBase::RequireUngrouped(*command)) { - return; - } + CommandScheduler::GetInstance().RequireUngrouped(command.get()); m_command = std::move(command); - m_command->SetGrouped(true); + m_command->SetComposed(true); } void WrapperCommand::Initialize() { diff --git a/wpilibNewCommands/src/main/native/include/frc2/command/Command.h b/wpilibNewCommands/src/main/native/include/frc2/command/Command.h index 5ba47363b9..e1f145d2e9 100644 --- a/wpilibNewCommands/src/main/native/include/frc2/command/Command.h +++ b/wpilibNewCommands/src/main/native/include/frc2/command/Command.h @@ -302,9 +302,9 @@ safe) semantics. void Cancel(); /** - * Whether or not the command is currently scheduled. Note that this does not - * detect whether the command is being run by a CommandGroup, only whether it - * is directly being run by the scheduler. + * Whether or not the command is currently scheduled. Note that this does not + * detect whether the command is in a composition, only whether it is directly + * being run by the scheduler. * * @return Whether the command is scheduled. */ @@ -324,13 +324,32 @@ safe) semantics. * Whether the command is currently grouped in a command group. Used as extra * insurance to prevent accidental independent use of grouped commands. */ + bool IsComposed() const; + + /** + * Sets whether the command is currently composed in a command composition. + * Can be used to "reclaim" a command if a composition is no longer going to + * use it. NOT ADVISED! + */ + void SetComposed(bool isComposed); + + /** + * Whether the command is currently grouped in a command group. Used as extra + * insurance to prevent accidental independent use of grouped commands. + * + * @deprecated Moved to IsComposed() + */ + WPI_DEPRECATED("Moved to IsComposed()") bool IsGrouped() const; /** * Sets whether the command is currently grouped in a command group. Can be * used to "reclaim" a command if a group is no longer going to use it. NOT * ADVISED! + * + * @deprecated Moved to SetComposed() */ + WPI_DEPRECATED("Moved to SetComposed()") void SetGrouped(bool grouped); /** @@ -379,7 +398,7 @@ safe) semantics. */ virtual std::unique_ptr TransferOwnership() && = 0; - bool m_isGrouped = false; + bool m_isComposed = false; }; /** diff --git a/wpilibNewCommands/src/main/native/include/frc2/command/CommandGroupBase.h b/wpilibNewCommands/src/main/native/include/frc2/command/CommandGroupBase.h index c1aaaca78b..9fe65fa37a 100644 --- a/wpilibNewCommands/src/main/native/include/frc2/command/CommandGroupBase.h +++ b/wpilibNewCommands/src/main/native/include/frc2/command/CommandGroupBase.h @@ -4,61 +4,24 @@ #pragma once -#include #include -#include #include +#include + #include "frc2/command/CommandBase.h" namespace frc2 { /** - * A base for CommandGroups. Statically tracks commands that have been - * allocated to groups to ensure those commands are not also used independently, - * which can result in inconsistent command state and unpredictable execution. + * A base for CommandGroups. * * This class is provided by the NewCommands VendorDep + * @deprecated This class is an empty abstraction. Inherit directly from + * CommandBase. */ class CommandGroupBase : public CommandBase { public: - /** - * Requires that the specified command not have been already allocated to a - * CommandGroup. Reports an error if the command is already grouped. - * - * @param command The command to check - * @return True if all the command is ungrouped. - */ - static bool RequireUngrouped(const Command& command); - - /** - * Requires that the specified command not have been already allocated to a - * CommandGroup. Reports an error if the command is already grouped. - * - * @param command The command to check - * @return True if all the command is ungrouped. - */ - static bool RequireUngrouped(const Command* command); - - /** - * Requires that the specified commands not have been already allocated to a - * CommandGroup. Reports an error if any of the commands are already grouped. - * - * @param commands The commands to check - * @return True if all the commands are ungrouped. - */ - static bool RequireUngrouped( - std::span> commands); - - /** - * Requires that the specified commands not have been already allocated to a - * CommandGroup. Reports an error if any of the commands are already grouped. - * - * @param commands The commands to check - * @return True if all the commands are ungrouped. - */ - static bool RequireUngrouped(std::initializer_list commands); - /** * Adds the given commands to the command group. * diff --git a/wpilibNewCommands/src/main/native/include/frc2/command/CommandPtr.h b/wpilibNewCommands/src/main/native/include/frc2/command/CommandPtr.h index 2390e9dca2..70800f4ffa 100644 --- a/wpilibNewCommands/src/main/native/include/frc2/command/CommandPtr.h +++ b/wpilibNewCommands/src/main/native/include/frc2/command/CommandPtr.h @@ -252,8 +252,8 @@ class CommandPtr final { /** * Whether or not the command is currently scheduled. Note that this does not - * detect whether the command is being run by a CommandGroup, only whether it - * is directly being run by the scheduler. + * detect whether the command is in a composition, only whether it is directly + * being run by the scheduler. * * @return Whether the command is scheduled. */ diff --git a/wpilibNewCommands/src/main/native/include/frc2/command/CommandScheduler.h b/wpilibNewCommands/src/main/native/include/frc2/command/CommandScheduler.h index 2e0b09b0d8..c0c09c1140 100644 --- a/wpilibNewCommands/src/main/native/include/frc2/command/CommandScheduler.h +++ b/wpilibNewCommands/src/main/native/include/frc2/command/CommandScheduler.h @@ -360,6 +360,34 @@ class CommandScheduler final : public nt::NTSendable, */ void OnCommandFinish(Action action); + /** + * Requires that the specified command hasn't been already added to a + * composition. + * + * @param command The command to check + * @throws if the given commands have already been composed. + */ + void RequireUngrouped(const Command* command); + + /** + * Requires that the specified commands not have been already added to a + * composition. + * + * @param commands The commands to check + * @throws if the given commands have already been composed. + */ + void RequireUngrouped(std::span> commands); + + /** + * Requires that the specified commands not have been already added to a + * composition. + * + * @param commands The commands to check + * @throws IllegalArgumentException if the given commands have already been + * composed. + */ + void RequireUngrouped(std::initializer_list commands); + void InitSendable(nt::NTSendableBuilder& builder) override; private: diff --git a/wpilibNewCommands/src/main/native/include/frc2/command/ConditionalCommand.h b/wpilibNewCommands/src/main/native/include/frc2/command/ConditionalCommand.h index 507ce9a1de..61c5d1ddc3 100644 --- a/wpilibNewCommands/src/main/native/include/frc2/command/ConditionalCommand.h +++ b/wpilibNewCommands/src/main/native/include/frc2/command/ConditionalCommand.h @@ -9,25 +9,17 @@ #include #include "frc2/command/CommandBase.h" -#include "frc2/command/CommandGroupBase.h" #include "frc2/command/CommandHelper.h" namespace frc2 { /** - * Runs one of two commands, depending on the value of the given condition when - * this command is initialized. Does not actually schedule the selected command - * - rather, the command is run through this command; this ensures that the - * command will behave as expected if used as part of a CommandGroup. Requires - * the requirements of both commands, again to ensure proper functioning when - * used in a CommandGroup. If this is undesired, consider using - * ScheduleCommand. + * A command composition that runs one of two commands, depending on the value + * of the given condition when this command is initialized. * - *

As this command contains multiple component commands within it, it is - * technically a command group; the command instances that are passed to it - * cannot be added to any other groups, or scheduled individually. - * - *

As a rule, CommandGroups require the union of the requirements of their - * component commands. + *

The rules for command compositions apply: command instances that are + * passed to it are owned by the composition and cannot be added to any other + * composition or scheduled individually, and the composition requires all + * subsystems its components require. * * This class is provided by the NewCommands VendorDep * diff --git a/wpilibNewCommands/src/main/native/include/frc2/command/ParallelCommandGroup.h b/wpilibNewCommands/src/main/native/include/frc2/command/ParallelCommandGroup.h index cf7059b858..69c99f0de0 100644 --- a/wpilibNewCommands/src/main/native/include/frc2/command/ParallelCommandGroup.h +++ b/wpilibNewCommands/src/main/native/include/frc2/command/ParallelCommandGroup.h @@ -18,11 +18,13 @@ namespace frc2 { /** - * A CommandGroup that runs a set of commands in parallel, ending when the last - * command ends. + * A command composition that runs a set of commands in parallel, ending when + * the last command ends. * - *

As a rule, CommandGroups require the union of the requirements of their - * component commands. + *

The rules for command compositions apply: command instances that are + * passed to it are owned by the composition and cannot be added to any other + * composition or scheduled individually, and the composition requires all + * subsystems its components require. * * This class is provided by the NewCommands VendorDep */ @@ -30,23 +32,23 @@ class ParallelCommandGroup : public CommandHelper { public: /** - * Creates a new ParallelCommandGroup. The given commands will be executed + * Creates a new ParallelCommandGroup. The given commands will be executed * simultaneously. The command group will finish when the last command - * finishes. If the CommandGroup is interrupted, only the commands that are + * finishes. If the composition is interrupted, only the commands that are * still running will be interrupted. * - * @param commands the commands to include in this group. + * @param commands the commands to include in this composition. */ explicit ParallelCommandGroup( std::vector>&& commands); /** - * Creates a new ParallelCommandGroup. The given commands will be executed + * Creates a new ParallelCommandGroup. The given commands will be executed * simultaneously. The command group will finish when the last command - * finishes. If the CommandGroup is interrupted, only the commands that are + * finishes. If the composition is interrupted, only the commands that are * still running will be interrupted. * - * @param commands the commands to include in this group. + * @param commands the commands to include in this composition. */ template As a rule, CommandGroups require the union of the requirements of their - * component commands. + *

The rules for command compositions apply: command instances that are + * passed to it are owned by the composition and cannot be added to any other + * composition or scheduled individually, and the composition requires all + * subsystems its components require. * * This class is provided by the NewCommands VendorDep */ @@ -31,25 +33,25 @@ class ParallelDeadlineGroup : public CommandHelper { public: /** - * Creates a new ParallelDeadlineGroup. The given commands (including the - * deadline) will be executed simultaneously. The CommandGroup will finish - * when the deadline finishes, interrupting all other still-running commands. - * If the CommandGroup is interrupted, only the commands still running will be + * Creates a new ParallelDeadlineGroup. The given commands (including the + * deadline) will be executed simultaneously. The composition will finish when + * the deadline finishes, interrupting all other still-running commands. If + * the composition is interrupted, only the commands still running will be * interrupted. * - * @param deadline the command that determines when the group ends + * @param deadline the command that determines when the composition ends * @param commands the commands to be executed */ ParallelDeadlineGroup(std::unique_ptr&& deadline, std::vector>&& commands); /** - * Creates a new ParallelDeadlineGroup. The given commands (including the - * deadline) will be executed simultaneously. The CommandGroup will finish - * when the deadline finishes, interrupting all other still-running commands. - * If the CommandGroup is interrupted, only the commands still running will be + * Creates a new ParallelDeadlineGroup. The given commands (including the + * deadline) will be executed simultaneously. The composition will finish when + * the deadline finishes, interrupting all other still-running commands. If + * the composition is interrupted, only the commands still running will be * interrupted. * - * @param deadline the command that determines when the group ends + * @param deadline the command that determines when the composition ends * @param commands the commands to be executed */ template As a rule, CommandGroups require the union of the requirements of their - * component commands. + *

The rules for command compositions apply: command instances that are + * passed to it are owned by the composition and cannot be added to any other + * composition or scheduled individually, and the composition requires all + * subsystems its components require. * * This class is provided by the NewCommands VendorDep */ @@ -30,11 +32,11 @@ class ParallelRaceGroup : public CommandHelper { public: /** - * Creates a new ParallelCommandRace. The given commands will be executed + * Creates a new ParallelCommandRace. The given commands will be executed * simultaneously, and will "race to the finish" - the first command to finish * ends the entire command, with all other commands being interrupted. * - * @param commands the commands to include in this group. + * @param commands the commands to include in this composition. */ explicit ParallelRaceGroup(std::vector>&& commands); diff --git a/wpilibNewCommands/src/main/native/include/frc2/command/PerpetualCommand.h b/wpilibNewCommands/src/main/native/include/frc2/command/PerpetualCommand.h index 2d57a26b49..297cb64f55 100644 --- a/wpilibNewCommands/src/main/native/include/frc2/command/PerpetualCommand.h +++ b/wpilibNewCommands/src/main/native/include/frc2/command/PerpetualCommand.h @@ -13,13 +13,12 @@ #include #include "frc2/command/CommandBase.h" -#include "frc2/command/CommandGroupBase.h" #include "frc2/command/CommandHelper.h" namespace frc2 { /** * A command that runs another command in perpetuity, ignoring that command's - * end conditions. While this class does not extend {@link CommandGroupBase}, + * end conditions. While this class does not extend frc2::CommandGroupBase, * it is still considered a CommandGroup, as it allows one to compose another * command within it; the command instances that are passed to it cannot be * added to any other groups, or scheduled individually. diff --git a/wpilibNewCommands/src/main/native/include/frc2/command/RepeatCommand.h b/wpilibNewCommands/src/main/native/include/frc2/command/RepeatCommand.h index 1f696dc72d..e0d2ab2e2c 100644 --- a/wpilibNewCommands/src/main/native/include/frc2/command/RepeatCommand.h +++ b/wpilibNewCommands/src/main/native/include/frc2/command/RepeatCommand.h @@ -13,19 +13,18 @@ #include #include "frc2/command/CommandBase.h" -#include "frc2/command/CommandGroupBase.h" #include "frc2/command/CommandHelper.h" namespace frc2 { /** * A command that runs another command repeatedly, restarting it when it ends, - * until this command is interrupted. While this class does not extend {@link - * CommandGroupBase}, it is still considered a CommandGroup, as it allows one to - * compose another command within it; the command instances that are passed to - * it cannot be added to any other groups, or scheduled individually. + * until this command is interrupted. Command instances that are passed to it + * cannot be added to any other groups, or scheduled individually. * - *

As a rule, CommandGroups require the union of the requirements of their - * component commands. + *

The rules for command compositions apply: command instances that are + * passed to it are owned by the composition and cannot be added to any other + * composition or scheduled individually, and the composition requires all + * subsystems its components require. * *

This class is provided by the NewCommands VendorDep */ diff --git a/wpilibNewCommands/src/main/native/include/frc2/command/ScheduleCommand.h b/wpilibNewCommands/src/main/native/include/frc2/command/ScheduleCommand.h index 4e71f31b54..63cc3b331e 100644 --- a/wpilibNewCommands/src/main/native/include/frc2/command/ScheduleCommand.h +++ b/wpilibNewCommands/src/main/native/include/frc2/command/ScheduleCommand.h @@ -14,10 +14,10 @@ namespace frc2 { /** - * Schedules the given commands when this command is initialized. Useful for - * forking off from CommandGroups. Note that if run from a CommandGroup, the - * group will not know about the status of the scheduled commands, and will - * treat this command as finishing instantly. + * Schedules the given commands when this command is initialized. Useful for + * forking off from CommandGroups. Note that if run from a composition, the + * composition will not know about the status of the scheduled commands, and + * will treat this command as finishing instantly. * * This class is provided by the NewCommands VendorDep */ diff --git a/wpilibNewCommands/src/main/native/include/frc2/command/SelectCommand.h b/wpilibNewCommands/src/main/native/include/frc2/command/SelectCommand.h index d2b9ec39e7..d35f2f4d02 100644 --- a/wpilibNewCommands/src/main/native/include/frc2/command/SelectCommand.h +++ b/wpilibNewCommands/src/main/native/include/frc2/command/SelectCommand.h @@ -16,25 +16,18 @@ #include #include "frc2/command/CommandBase.h" -#include "frc2/command/CommandGroupBase.h" #include "frc2/command/PrintCommand.h" namespace frc2 { /** - * Runs one of a selection of commands, either using a selector and a key to - * command mapping, or a supplier that returns the command directly at runtime. - * Does not actually schedule the selected command - rather, the command is run - * through this command; this ensures that the command will behave as expected - * if used as part of a CommandGroup. Requires the requirements of all included - * commands, again to ensure proper functioning when used in a CommandGroup. If - * this is undesired, consider using ScheduleCommand. + * A command composition that runs one of a selection of commands, either using + * a selector and a key to command mapping, or a supplier that returns the + * command directly at runtime. * - *

As this command contains multiple component commands within it, it is - * technically a command group; the command instances that are passed to it - * cannot be added to any other groups, or scheduled individually. - * - *

As a rule, CommandGroups require the union of the requirements of their - * component commands. + *

The rules for command compositions apply: command instances that are + * passed to it are owned by the composition and cannot be added to any other + * composition or scheduled individually, and the composition requires all + * subsystems its components require. * * This class is provided by the NewCommands VendorDep */ @@ -61,9 +54,7 @@ class SelectCommand : public CommandHelper> { ...); for (auto&& command : foo) { - if (!CommandGroupBase::RequireUngrouped(*command.second)) { - return; - } + CommandScheduler::GetInstance().RequireUngrouped(command.second.get()); } for (auto&& command : foo) { @@ -82,9 +73,7 @@ class SelectCommand : public CommandHelper> { std::vector>>&& commands) : m_selector{std::move(selector)} { for (auto&& command : commands) { - if (!CommandGroupBase::RequireUngrouped(*command.second)) { - return; - } + CommandScheduler::GetInstance().RequireUngrouped(command.second.get()); } for (auto&& command : commands) { diff --git a/wpilibNewCommands/src/main/native/include/frc2/command/SequentialCommandGroup.h b/wpilibNewCommands/src/main/native/include/frc2/command/SequentialCommandGroup.h index 255bbea671..7bf2a68b08 100644 --- a/wpilibNewCommands/src/main/native/include/frc2/command/SequentialCommandGroup.h +++ b/wpilibNewCommands/src/main/native/include/frc2/command/SequentialCommandGroup.h @@ -24,10 +24,12 @@ namespace frc2 { const size_t invalid_index = std::numeric_limits::max(); /** - * A CommandGroups that runs a list of commands in sequence. + * A command composition that runs a list of commands in sequence. * - *

As a rule, CommandGroups require the union of the requirements of their - * component commands. + *

The rules for command compositions apply: command instances that are + * passed to it are owned by the composition and cannot be added to any other + * composition or scheduled individually, and the composition requires all + * subsystems its components require. * * This class is provided by the NewCommands VendorDep */ @@ -35,21 +37,21 @@ class SequentialCommandGroup : public CommandHelper { public: /** - * Creates a new SequentialCommandGroup. The given commands will be run - * sequentially, with the CommandGroup finishing when the last command + * Creates a new SequentialCommandGroup. The given commands will be run + * sequentially, with the composition finishing when the last command * finishes. * - * @param commands the commands to include in this group. + * @param commands the commands to include in this composition. */ explicit SequentialCommandGroup( std::vector>&& commands); /** - * Creates a new SequentialCommandGroup. The given commands will be run - * sequentially, with the CommandGroup finishing when the last command + * Creates a new SequentialCommandGroup. The given commands will be run + * sequentially, with the composition finishing when the last command * finishes. * - * @param commands the commands to include in this group. + * @param commands the commands to include in this composition. */ template #include "frc2/command/CommandBase.h" -#include "frc2/command/CommandGroupBase.h" #include "frc2/command/CommandHelper.h" namespace frc2 { diff --git a/wpilibNewCommands/src/test/java/edu/wpi/first/wpilibj2/command/CommandGroupErrorTest.java b/wpilibNewCommands/src/test/java/edu/wpi/first/wpilibj2/command/CommandGroupErrorTest.java index 4fb44a50f6..ba4a77b829 100644 --- a/wpilibNewCommands/src/test/java/edu/wpi/first/wpilibj2/command/CommandGroupErrorTest.java +++ b/wpilibNewCommands/src/test/java/edu/wpi/first/wpilibj2/command/CommandGroupErrorTest.java @@ -24,16 +24,15 @@ class CommandGroupErrorTest extends CommandTestBase { @Test void commandInGroupExternallyScheduledTest() { - try (CommandScheduler scheduler = new CommandScheduler()) { - MockCommandHolder command1Holder = new MockCommandHolder(true); - Command command1 = command1Holder.getMock(); - MockCommandHolder command2Holder = new MockCommandHolder(true); - Command command2 = command2Holder.getMock(); + MockCommandHolder command1Holder = new MockCommandHolder(true); + Command command1 = command1Holder.getMock(); + MockCommandHolder command2Holder = new MockCommandHolder(true); + Command command2 = command2Holder.getMock(); - new ParallelCommandGroup(command1, command2); + new ParallelCommandGroup(command1, command2); - assertThrows(IllegalArgumentException.class, () -> scheduler.schedule(command1)); - } + assertThrows( + IllegalArgumentException.class, () -> CommandScheduler.getInstance().schedule(command1)); } @Test @@ -42,7 +41,7 @@ class CommandGroupErrorTest extends CommandTestBase { assertDoesNotThrow(() -> command.withTimeout(10).until(() -> false)); assertThrows(IllegalArgumentException.class, () -> command.withTimeout(10)); - CommandGroupBase.clearGroupedCommand(command); + CommandScheduler.getInstance().removeComposedCommand(command); assertDoesNotThrow(() -> command.withTimeout(10)); } } diff --git a/wpilibNewCommands/src/test/java/edu/wpi/first/wpilibj2/command/CommandTestBase.java b/wpilibNewCommands/src/test/java/edu/wpi/first/wpilibj2/command/CommandTestBase.java index ecd3e9b6d4..1d46fb5654 100644 --- a/wpilibNewCommands/src/test/java/edu/wpi/first/wpilibj2/command/CommandTestBase.java +++ b/wpilibNewCommands/src/test/java/edu/wpi/first/wpilibj2/command/CommandTestBase.java @@ -22,7 +22,7 @@ public class CommandTestBase { CommandScheduler.getInstance().cancelAll(); CommandScheduler.getInstance().enable(); CommandScheduler.getInstance().getActiveButtonLoop().clear(); - CommandGroupBase.clearGroupedCommands(); + CommandScheduler.getInstance().clearComposedCommands(); setDSEnabled(true); } diff --git a/wpilibNewCommands/src/test/native/cpp/frc2/command/CommandTestBase.h b/wpilibNewCommands/src/test/native/cpp/frc2/command/CommandTestBase.h index d06423e69c..a1ab1de820 100644 --- a/wpilibNewCommands/src/test/native/cpp/frc2/command/CommandTestBase.h +++ b/wpilibNewCommands/src/test/native/cpp/frc2/command/CommandTestBase.h @@ -9,7 +9,6 @@ #include -#include "frc2/command/CommandGroupBase.h" #include "frc2/command/CommandHelper.h" #include "frc2/command/CommandScheduler.h" #include "frc2/command/SetUtilities.h" diff --git a/wpilibjExamples/src/main/java/edu/wpi/first/wpilibj/examples/gearsbot/commands/Autonomous.java b/wpilibjExamples/src/main/java/edu/wpi/first/wpilibj/examples/gearsbot/commands/Autonomous.java index 599ed69a0b..44a7ce7d60 100644 --- a/wpilibjExamples/src/main/java/edu/wpi/first/wpilibj/examples/gearsbot/commands/Autonomous.java +++ b/wpilibjExamples/src/main/java/edu/wpi/first/wpilibj/examples/gearsbot/commands/Autonomous.java @@ -8,6 +8,7 @@ import edu.wpi.first.wpilibj.examples.gearsbot.subsystems.Claw; import edu.wpi.first.wpilibj.examples.gearsbot.subsystems.Drivetrain; import edu.wpi.first.wpilibj.examples.gearsbot.subsystems.Elevator; import edu.wpi.first.wpilibj.examples.gearsbot.subsystems.Wrist; +import edu.wpi.first.wpilibj2.command.Commands; import edu.wpi.first.wpilibj2.command.SequentialCommandGroup; /** The main autonomous command to pickup and deliver the soda to the box. */ @@ -22,6 +23,6 @@ public class Autonomous extends SequentialCommandGroup { new Place(claw, wrist, elevator), new SetDistanceToBox(0.60, drive), // new DriveStraight(-2), // Use Encoders if ultrasonic is broken - parallel(new SetWristSetpoint(-45, wrist), new CloseClaw(claw))); + Commands.parallel(new SetWristSetpoint(-45, wrist), new CloseClaw(claw))); } } diff --git a/wpilibjExamples/src/main/java/edu/wpi/first/wpilibj/examples/gearsbot/commands/Pickup.java b/wpilibjExamples/src/main/java/edu/wpi/first/wpilibj/examples/gearsbot/commands/Pickup.java index 4c7d267c55..4e6eafd4ca 100644 --- a/wpilibjExamples/src/main/java/edu/wpi/first/wpilibj/examples/gearsbot/commands/Pickup.java +++ b/wpilibjExamples/src/main/java/edu/wpi/first/wpilibj/examples/gearsbot/commands/Pickup.java @@ -7,6 +7,7 @@ package edu.wpi.first.wpilibj.examples.gearsbot.commands; import edu.wpi.first.wpilibj.examples.gearsbot.subsystems.Claw; import edu.wpi.first.wpilibj.examples.gearsbot.subsystems.Elevator; import edu.wpi.first.wpilibj.examples.gearsbot.subsystems.Wrist; +import edu.wpi.first.wpilibj2.command.Commands; import edu.wpi.first.wpilibj2.command.SequentialCommandGroup; /** @@ -23,6 +24,7 @@ public class Pickup extends SequentialCommandGroup { public Pickup(Claw claw, Wrist wrist, Elevator elevator) { addCommands( new CloseClaw(claw), - parallel(new SetWristSetpoint(-45, wrist), new SetElevatorSetpoint(0.25, elevator))); + Commands.parallel( + new SetWristSetpoint(-45, wrist), new SetElevatorSetpoint(0.25, elevator))); } } diff --git a/wpilibjExamples/src/main/java/edu/wpi/first/wpilibj/examples/gearsbot/commands/PrepareToPickup.java b/wpilibjExamples/src/main/java/edu/wpi/first/wpilibj/examples/gearsbot/commands/PrepareToPickup.java index 1ae599228d..3c0661de24 100644 --- a/wpilibjExamples/src/main/java/edu/wpi/first/wpilibj/examples/gearsbot/commands/PrepareToPickup.java +++ b/wpilibjExamples/src/main/java/edu/wpi/first/wpilibj/examples/gearsbot/commands/PrepareToPickup.java @@ -7,6 +7,7 @@ package edu.wpi.first.wpilibj.examples.gearsbot.commands; import edu.wpi.first.wpilibj.examples.gearsbot.subsystems.Claw; import edu.wpi.first.wpilibj.examples.gearsbot.subsystems.Elevator; import edu.wpi.first.wpilibj.examples.gearsbot.subsystems.Wrist; +import edu.wpi.first.wpilibj2.command.Commands; import edu.wpi.first.wpilibj2.command.SequentialCommandGroup; /** Make sure the robot is in a state to pickup soda cans. */ @@ -21,6 +22,6 @@ public class PrepareToPickup extends SequentialCommandGroup { public PrepareToPickup(Claw claw, Wrist wrist, Elevator elevator) { addCommands( new OpenClaw(claw), - parallel(new SetWristSetpoint(0, wrist), new SetElevatorSetpoint(0, elevator))); + Commands.parallel(new SetWristSetpoint(0, wrist), new SetElevatorSetpoint(0, elevator))); } } diff --git a/wpilibjExamples/src/main/java/edu/wpi/first/wpilibj/examples/rapidreactcommandbot/RapidReactCommandBot.java b/wpilibjExamples/src/main/java/edu/wpi/first/wpilibj/examples/rapidreactcommandbot/RapidReactCommandBot.java index bb550ce76e..879f2c227f 100644 --- a/wpilibjExamples/src/main/java/edu/wpi/first/wpilibj/examples/rapidreactcommandbot/RapidReactCommandBot.java +++ b/wpilibjExamples/src/main/java/edu/wpi/first/wpilibj/examples/rapidreactcommandbot/RapidReactCommandBot.java @@ -4,7 +4,7 @@ package edu.wpi.first.wpilibj.examples.rapidreactcommandbot; -import static edu.wpi.first.wpilibj2.command.CommandGroupBase.parallel; +import static edu.wpi.first.wpilibj2.command.Commands.parallel; import edu.wpi.first.wpilibj.examples.rapidreactcommandbot.Constants.AutoConstants; import edu.wpi.first.wpilibj.examples.rapidreactcommandbot.Constants.OIConstants;