From e5d2fcf435e50daf48363511fc6f3c85e02e580d Mon Sep 17 00:00:00 2001 From: ohad Date: Mon, 4 Mar 2024 00:41:48 -0500 Subject: [PATCH 1/3] Added new logging command classes, tests --- .../frc4048/common/logging/CommandUtil.java | 32 +++- .../logging/ParallelLoggingCommand.java | 26 ++- .../common/logging/RaceLoggingCommand.java | 56 +++++++ .../logging/SequentialLoggingCommand.java | 31 +++- .../frc4048/common/logging/LoggingTest.java | 152 ++++++++++++++++++ 5 files changed, 281 insertions(+), 16 deletions(-) create mode 100644 src/main/java/org/usfirst/frc4048/common/logging/RaceLoggingCommand.java create mode 100644 src/test/java/org/usfirst/frc4048/common/logging/LoggingTest.java diff --git a/src/main/java/org/usfirst/frc4048/common/logging/CommandUtil.java b/src/main/java/org/usfirst/frc4048/common/logging/CommandUtil.java index 0aa5edf..d628298 100644 --- a/src/main/java/org/usfirst/frc4048/common/logging/CommandUtil.java +++ b/src/main/java/org/usfirst/frc4048/common/logging/CommandUtil.java @@ -32,6 +32,18 @@ * Command pickUp = CommandUtil.sequence("ArmSequence", new OpenGripper(), new RaiseArm(), new CloseGripper()); * Command pickupAndDrive = CommandUtil.parallel("WalkWhileChewingGum", pickup, new Drive()); * + * + * and also for nesting command groups that are custom-created: + * (Note the "extends SequentialLoggingCommandGroup" - DO NOT extend regular SequentialCommandGroup or this will not work) + * 
+ *     class PickupGroup extends SequentialLoggingCommandGroup {
+ *         public PickupGroup() {
+ *             super("Pickup", new OpenGripper(), new RaiseArm(), new CloseGripper());
+ *         }
+ *     }
+ *     Command pickUp = new PickupGroup();
+ *     Command pickupAndDrive = CommandUtil.parallel("WalkWhileChewingGum", pickup, new Drive());
+ *
*/ public class CommandUtil { public static final String COMMAND_PREFIX = "Commands"; @@ -52,6 +64,8 @@ public static Command logged(Command command) { /** * Make a logging command from the given command, with a custom name hierarchy. * The logging command will be placed within the given name prefix naming hierarchy instead of at the root. + * @param namePrefix the nesting hierarchy prefix for the command. If null the command is going to log at the + * root level. If not null it will be nested in this namespace. * @param command the command to wrap * @return a {@link LoggingCommand} that wraps the given command */ @@ -60,7 +74,7 @@ public static Command logged(String namePrefix, Command command) { } /** - * Return a logged command that runs once and stops (see {@link Commands#runOnce(Runnable, Subsystem...)}. + * Return a logged command that runs once and stops (see {@link edu.wpi.first.wpilibj2.command.Commands#runOnce(Runnable, Subsystem...)}. * This will return a loggable anonymous command with the given name. * @param name the name of the resulting command * @param action the action to perform (only once) @@ -76,28 +90,30 @@ public static Command runOnce(String name, Runnable action, Subsystem... require /** * Return a loggable {@link SequentialCommandGroup} that executes the given commands sequentially. The given commands * can be any command or command group - they will be converted to loggable commands implicitly. - * @param sequenceName the name of the resaulting command group + * @param sequenceName the name of the resulting command group * @param commands the commands to put in the group * @return the created loggable command group */ public static Command sequence(String sequenceName, Command... commands) { - LoggingCommand[] loggingCommands = wrapForLogging(sequenceName, commands); - return new SequentialLoggingCommand(sequenceName, loggingCommands); + return new SequentialLoggingCommand(sequenceName, commands); } /** * Return a loggable {@link ParallelCommandGroup} that executes the given commands in parallel. The given commands * can be any command or command group - they will be converted to loggable commands implicitly. - * @param sequenceName the name of the resaulting command group + * @param sequenceName the name of the resulting command group * @param commands the commands to put in the group * @return the created loggable command group */ public static Command parallel(String sequenceName, Command... commands) { - LoggingCommand[] loggingCommands = wrapForLogging(sequenceName, commands); - return new ParallelLoggingCommand(sequenceName, loggingCommands); + return new ParallelLoggingCommand(sequenceName, commands); + } + + public static Command race(String sequenceName, Command... commands) { + return new RaceLoggingCommand(sequenceName, commands); } - private static LoggingCommand[] wrapForLogging(String prefix, Command... commands) { + public static LoggingCommand[] wrapForLogging(String prefix, Command... commands) { // Do not use streams due to efficiency LoggingCommand[] newCommands = new LoggingCommand[commands.length]; for (int i = 0; i < commands.length; i++) { diff --git a/src/main/java/org/usfirst/frc4048/common/logging/ParallelLoggingCommand.java b/src/main/java/org/usfirst/frc4048/common/logging/ParallelLoggingCommand.java index ea02509..4d626ac 100644 --- a/src/main/java/org/usfirst/frc4048/common/logging/ParallelLoggingCommand.java +++ b/src/main/java/org/usfirst/frc4048/common/logging/ParallelLoggingCommand.java @@ -1,15 +1,33 @@ package org.usfirst.frc4048.common.logging; +import edu.wpi.first.wpilibj2.command.Command; import edu.wpi.first.wpilibj2.command.ParallelCommandGroup; +import java.util.Arrays; +import java.util.List; + public class ParallelLoggingCommand extends LoggingCommand { private static final String THIS_NAME = "-this"; - private LoggingCommand[] loggingCommands; + private List loggingCommands; + + /** + * Constructor for parallel command group. + * + * @param namePrefix the name for the group - this is where the sub-commands for this group will be nested in + * @param commands the sub commands for this group (either regular commands or LoggingCommand are OK) + */ + public ParallelLoggingCommand(String namePrefix, Command... commands) { + super(namePrefix, THIS_NAME, new ParallelCommandGroup()); + LoggingCommand[] wrapped = CommandUtil.wrapForLogging(namePrefix, commands); + ((ParallelCommandGroup) getUnderlying()).addCommands(wrapped); + this.loggingCommands = Arrays.asList(wrapped); + } - public ParallelLoggingCommand(String namePrefix, LoggingCommand... commands) { - super(namePrefix, THIS_NAME, new ParallelCommandGroup(commands)); - this.loggingCommands = commands; + public final void addCommands(Command... commands) { + LoggingCommand[] wrapped = CommandUtil.wrapForLogging(getNamePrefix(), commands); + ((ParallelCommandGroup) getUnderlying()).addCommands(wrapped); + loggingCommands.addAll(Arrays.asList(wrapped)); } @Override diff --git a/src/main/java/org/usfirst/frc4048/common/logging/RaceLoggingCommand.java b/src/main/java/org/usfirst/frc4048/common/logging/RaceLoggingCommand.java new file mode 100644 index 0000000..9eea3b3 --- /dev/null +++ b/src/main/java/org/usfirst/frc4048/common/logging/RaceLoggingCommand.java @@ -0,0 +1,56 @@ +package org.usfirst.frc4048.common.logging; + +import edu.wpi.first.wpilibj2.command.Command; +import edu.wpi.first.wpilibj2.command.ParallelRaceGroup; + +import java.util.Arrays; +import java.util.List; + +public class RaceLoggingCommand extends LoggingCommand { + private static final String THIS_NAME = "-this"; + + private List loggingCommands; + + /** + * Constructor for race command group. + * + * @param namePrefix the name for the group - this is where the sub-commands for this group will be nested in + * @param commands the sub commands for this group (either regular commands or LoggingCommand are OK) + */ + public RaceLoggingCommand(String namePrefix, Command... commands) { + super(namePrefix, THIS_NAME, new ParallelRaceGroup()); + LoggingCommand[] wrapped = CommandUtil.wrapForLogging(namePrefix, commands); + ((ParallelRaceGroup) getUnderlying()).addCommands(wrapped); + this.loggingCommands = Arrays.asList(wrapped); + } + + public final void addCommands(Command... commands) { + LoggingCommand[] wrapped = CommandUtil.wrapForLogging(getNamePrefix(), commands); + ((ParallelRaceGroup) getUnderlying()).addCommands(wrapped); + loggingCommands.addAll(Arrays.asList(wrapped)); + } + + @Override + public void setName(String name) { + // Do not change the logging name for this command (it is fixed) + getUnderlying().setName(name); + } + + @Override + public void setNamePrefix(String prefix) { + super.setNamePrefix(prefix); + setChildrenPrefix(prefix); + } + + @Override + public String toString() { + return getFullyQualifiedName(); + } + + private void setChildrenPrefix(String prefix) { + // Recursively change the prefix for all child commands + for (LoggingCommand loggingCommand : loggingCommands) { + loggingCommand.setNamePrefix(prefix); + } + } +} diff --git a/src/main/java/org/usfirst/frc4048/common/logging/SequentialLoggingCommand.java b/src/main/java/org/usfirst/frc4048/common/logging/SequentialLoggingCommand.java index d4aac2b..ce4261d 100644 --- a/src/main/java/org/usfirst/frc4048/common/logging/SequentialLoggingCommand.java +++ b/src/main/java/org/usfirst/frc4048/common/logging/SequentialLoggingCommand.java @@ -1,15 +1,33 @@ package org.usfirst.frc4048.common.logging; +import edu.wpi.first.wpilibj2.command.Command; import edu.wpi.first.wpilibj2.command.SequentialCommandGroup; +import java.util.Arrays; +import java.util.List; + public class SequentialLoggingCommand extends LoggingCommand { private static final String THIS_NAME = "-this"; - private final LoggingCommand[] loggingCommands; + private final List loggingCommands; + + /** + * Constructor for sequential command group. + * + * @param namePrefix the name for the group - this is where the sub-commands for this group will be nested in + * @param commands the sub commands for this group (either regular commands or LoggingCommand are OK) + */ + public SequentialLoggingCommand(String namePrefix, Command... commands) { + super(namePrefix, THIS_NAME, new SequentialCommandGroup()); + LoggingCommand[] wrapped = CommandUtil.wrapForLogging(namePrefix, commands); + ((SequentialCommandGroup) getUnderlying()).addCommands(wrapped); + this.loggingCommands = Arrays.asList(wrapped); + } - public SequentialLoggingCommand(String namePrefix, LoggingCommand... commands) { - super(namePrefix, THIS_NAME, new SequentialCommandGroup(commands)); - this.loggingCommands = commands; + public final void addCommands(Command... commands) { + LoggingCommand[] wrapped = CommandUtil.wrapForLogging(getNamePrefix(), commands); + ((SequentialCommandGroup) getUnderlying()).addCommands(wrapped); + loggingCommands.addAll(Arrays.asList(wrapped)); } @Override @@ -29,6 +47,11 @@ public String toString() { return getFullyQualifiedName(); } + // For testing + public List getLoggingCommands() { + return loggingCommands; + } + private void setChildrenPrefix(String prefix) { // Recursively change the prefix for all child commands for (LoggingCommand loggingCommand : loggingCommands) { diff --git a/src/test/java/org/usfirst/frc4048/common/logging/LoggingTest.java b/src/test/java/org/usfirst/frc4048/common/logging/LoggingTest.java new file mode 100644 index 0000000..ce207c1 --- /dev/null +++ b/src/test/java/org/usfirst/frc4048/common/logging/LoggingTest.java @@ -0,0 +1,152 @@ +package org.usfirst.frc4048.common.logging; + +import edu.wpi.first.wpilibj2.command.Command; +import edu.wpi.first.wpilibj2.command.Commands; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +public class LoggingTest { + + @Test + public void testLoggingCommandSingle() { + LoggingCommand command = (LoggingCommand) CommandUtil.logged(Commands.runOnce(() -> System.out.println("Hello"))); + Assertions.assertNull(command.getNamePrefix()); + Assertions.assertEquals("Commands/InstantCommand", command.getFullyQualifiedName()); + } + + @Test + public void testLoggingCommandSingleWithPrefix() { + LoggingCommand command = (LoggingCommand) CommandUtil.logged("test", Commands.runOnce(() -> System.out.println("Hello"))); + Assertions.assertEquals("test", command.getNamePrefix()); + Assertions.assertEquals("Commands/test/InstantCommand", command.getFullyQualifiedName()); + } + + @Test + public void testLoggingCommandGroup() { + LoggingCommand command = (LoggingCommand) CommandUtil.logged("test", Commands.parallel(Commands.runOnce(() -> System.out.println("Hello")))); + Assertions.assertEquals("test", command.getNamePrefix()); + Assertions.assertEquals("Commands/test/ParallelCommandGroup", command.getFullyQualifiedName()); + } + + @Test + public void testSequenceInstantCommand() { + Command command = Commands.runOnce(() -> System.out.println("Hello")); + SequentialLoggingCommand sequence = (SequentialLoggingCommand) CommandUtil.sequence("sequence", command); + Assertions.assertEquals("sequence", sequence.getNamePrefix()); + Assertions.assertEquals("Commands/sequence/-this", sequence.getFullyQualifiedName()); + Assertions.assertEquals("Commands/sequence/InstantCommand", sequence.getLoggingCommands().get(0).getFullyQualifiedName()); + } + + @Test + public void testSequenceRegularCommand() { + Command command = new Command() { + @Override + public String getName() { + return "command"; + } + }; + SequentialLoggingCommand sequence = (SequentialLoggingCommand) CommandUtil.sequence("sequence", command); + Assertions.assertEquals("sequence", sequence.getNamePrefix()); + Assertions.assertEquals("Commands/sequence/-this", sequence.getFullyQualifiedName()); + Assertions.assertEquals("Commands/sequence/command", sequence.getLoggingCommands().get(0).getFullyQualifiedName()); + } + + @Test + public void testSequenceLoggingInstantCommand() { + Command command = CommandUtil.logged("command", Commands.runOnce(() -> System.out.println("Hello"))); + SequentialLoggingCommand sequence = (SequentialLoggingCommand) CommandUtil.sequence("sequence", command); + Assertions.assertEquals("sequence", sequence.getNamePrefix()); + Assertions.assertEquals("Commands/sequence/-this", sequence.getFullyQualifiedName()); + Assertions.assertEquals("Commands/sequence/command/InstantCommand", sequence.getLoggingCommands().get(0).getFullyQualifiedName()); + } + + @Test + public void testSequenceLoggingRegularCommand() { + Command command = new Command() { + @Override + public String getName() { + return "command"; + } + }; + SequentialLoggingCommand sequence = (SequentialLoggingCommand) CommandUtil.sequence("sequence", CommandUtil.logged("logged", command)); + Assertions.assertEquals("sequence", sequence.getNamePrefix()); + Assertions.assertEquals("Commands/sequence/-this", sequence.getFullyQualifiedName()); + Assertions.assertEquals("Commands/sequence/logged/command", sequence.getLoggingCommands().get(0).getFullyQualifiedName()); + } + + @Test + public void testSequenceLoggingRegularNoNamedCommand() { + Command command = new Command() { + @Override + public String getName() { + return "command"; + } + }; + SequentialLoggingCommand sequence = (SequentialLoggingCommand) CommandUtil.sequence("sequence", CommandUtil.logged(command)); + Assertions.assertEquals("sequence", sequence.getNamePrefix()); + Assertions.assertEquals("Commands/sequence/-this", sequence.getFullyQualifiedName()); + Assertions.assertEquals("Commands/sequence/null/command", sequence.getLoggingCommands().get(0).getFullyQualifiedName()); + } + + @Test + public void testSequenceOfSequence() { + Command command = new Command() { + @Override + public String getName() { + return "command"; + } + }; + SequentialLoggingCommand inner = (SequentialLoggingCommand) CommandUtil.sequence("inner", command); + SequentialLoggingCommand outer = (SequentialLoggingCommand) CommandUtil.sequence("outer", inner); + Assertions.assertEquals("outer", outer.getNamePrefix()); + Assertions.assertEquals("Commands/outer/-this", outer.getFullyQualifiedName()); + inner = (SequentialLoggingCommand) outer.getLoggingCommands().get(0); + Assertions.assertEquals("Commands/outer/inner/-this", inner.getFullyQualifiedName()); + Assertions.assertEquals("Commands/outer/inner/command", inner.getLoggingCommands().get(0).getFullyQualifiedName()); + } + + @Test + public void testSequenceOfSequenceOfLoggedCommand() { + Command command = new Command() { + @Override + public String getName() { + return "command"; + } + }; + SequentialLoggingCommand inner = (SequentialLoggingCommand) CommandUtil.sequence("inner", CommandUtil.logged("logged", command)); + SequentialLoggingCommand outer = (SequentialLoggingCommand) CommandUtil.sequence("outer", inner); + Assertions.assertEquals("outer", outer.getNamePrefix()); + Assertions.assertEquals("Commands/outer/-this", outer.getFullyQualifiedName()); + inner = (SequentialLoggingCommand) outer.getLoggingCommands().get(0); + Assertions.assertEquals("Commands/outer/inner/-this", inner.getFullyQualifiedName()); + Assertions.assertEquals("Commands/outer/inner/command", inner.getLoggingCommands().get(0).getFullyQualifiedName()); + } + + @Test + public void testCustomSequentialCommand() { + SequentialLoggingCommand customSequence = new CustomSequentialGroup(); + Assertions.assertEquals("Sequence", customSequence.getNamePrefix()); + Assertions.assertEquals("Commands/Sequence/-this", customSequence.getFullyQualifiedName()); + Assertions.assertEquals("Commands/Sequence/Command", customSequence.getLoggingCommands().get(0).getFullyQualifiedName()); + } + + @Test + public void testCustomSequenceOfSequence() { + SequentialLoggingCommand customSequence = new CustomSequentialGroup(); + SequentialLoggingCommand outer = (SequentialLoggingCommand) CommandUtil.sequence("outer", customSequence); + Assertions.assertEquals("outer", outer.getNamePrefix()); + Assertions.assertEquals("Commands/outer/Sequence/-this", outer.getLoggingCommands().get(0).getFullyQualifiedName()); + Assertions.assertEquals("Commands/outer/Sequence/Command", ((SequentialLoggingCommand) outer.getLoggingCommands().get(0)).getLoggingCommands().get(0).getFullyQualifiedName()); + } + + private static class CustomSequentialGroup extends SequentialLoggingCommand { + public CustomSequentialGroup() { + super("Sequence", new Command() { + @Override + public String getName() { + return "Command"; + } + }); + } + } +} From 2551ad23fcf2846ec24187f5304dfd86abe12430 Mon Sep 17 00:00:00 2001 From: ohad Date: Sat, 9 Mar 2024 17:13:28 -0500 Subject: [PATCH 2/3] fix bugs in deep nesting, create superclass for group, add dealine command --- .../frc4048/common/logging/CommandUtil.java | 4 +- .../common/logging/GroupLoggingCommand.java | 72 +++++++++++++++++++ .../common/logging/LoggingCommand.java | 35 +++++++-- .../ParallelDeadlineLoggingCommand.java | 41 +++++++++++ .../logging/ParallelLoggingCommand.java | 39 ++-------- .../common/logging/RaceLoggingCommand.java | 39 ++-------- .../logging/SequentialLoggingCommand.java | 44 ++---------- 7 files changed, 163 insertions(+), 111 deletions(-) create mode 100644 src/main/java/org/usfirst/frc4048/common/logging/GroupLoggingCommand.java create mode 100644 src/main/java/org/usfirst/frc4048/common/logging/ParallelDeadlineLoggingCommand.java diff --git a/src/main/java/org/usfirst/frc4048/common/logging/CommandUtil.java b/src/main/java/org/usfirst/frc4048/common/logging/CommandUtil.java index d628298..275f7d6 100644 --- a/src/main/java/org/usfirst/frc4048/common/logging/CommandUtil.java +++ b/src/main/java/org/usfirst/frc4048/common/logging/CommandUtil.java @@ -124,9 +124,7 @@ public static LoggingCommand[] wrapForLogging(String prefix, Command... commands private static LoggingCommand wrapForLogging(String prefix, Command command) { if (command instanceof LoggingCommand loggingCommand) { - // change the prefix to include the current new parent - String childPrefix = prefix + CommandUtil.NAME_SEPARATOR + loggingCommand.getNamePrefix(); - loggingCommand.setNamePrefix(childPrefix); + loggingCommand.appendNamePrefix(prefix); return loggingCommand; } else { // New command located in the given sequence root diff --git a/src/main/java/org/usfirst/frc4048/common/logging/GroupLoggingCommand.java b/src/main/java/org/usfirst/frc4048/common/logging/GroupLoggingCommand.java new file mode 100644 index 0000000..dd895a1 --- /dev/null +++ b/src/main/java/org/usfirst/frc4048/common/logging/GroupLoggingCommand.java @@ -0,0 +1,72 @@ +package org.usfirst.frc4048.common.logging; + +import edu.wpi.first.wpilibj2.command.Command; + +import java.util.ArrayList; +import java.util.List; + +/** + * Base class for logged grouped commands (parallel, race, sequential...) + */ +public abstract class GroupLoggingCommand extends LoggingCommand { + // the "leaf" name - this comes after the name prefix to make up the full command name + private static final String THIS_NAME = "-this"; + + // A copy of the children (since we can't access them from the regular groups) + private final List childLoggingCommands; + + /** + * Constructor for logged group command. + * + * @param namePrefix the prefix for the name (comes in front of hte group name) + * @param underlying the group command that is wrapped by this command + */ + public GroupLoggingCommand(String namePrefix, Command underlying) { + super(namePrefix, THIS_NAME, underlying); + childLoggingCommands = new ArrayList<>(); + } + + /** + * A special constructor to allow for the creation of the command when we can't create the underlying up front. + * It is assumed that the {@link #setUnderlying(Command)} method is called immediately following the construction. + */ + protected GroupLoggingCommand(String namePrefix) { + super(namePrefix, THIS_NAME); + childLoggingCommands = new ArrayList<>(); + } + + public final void addLoggingCommands(List commands) { + childLoggingCommands.addAll(commands); + } + + @Override + public void setName(String name) { + // Do not change the logging name for this command (it is fixed) + getUnderlying().setName(name); + } + + @Override + public void appendNamePrefix(String prefix) { + // Change the name for this command + super.appendNamePrefix(prefix); + // Change the name for the children + appendChildrenPrefix(prefix); + } + + @Override + public String toString() { + return getFullyQualifiedName(); + } + + // For testing + public List getChildLoggingCommands() { + return childLoggingCommands; + } + + private void appendChildrenPrefix(String prefix) { + // Recursively change the prefix for all child commands + for (LoggingCommand loggingCommand : childLoggingCommands) { + loggingCommand.appendNamePrefix(prefix); + } + } +} diff --git a/src/main/java/org/usfirst/frc4048/common/logging/LoggingCommand.java b/src/main/java/org/usfirst/frc4048/common/logging/LoggingCommand.java index 347378f..0a77861 100644 --- a/src/main/java/org/usfirst/frc4048/common/logging/LoggingCommand.java +++ b/src/main/java/org/usfirst/frc4048/common/logging/LoggingCommand.java @@ -8,10 +8,14 @@ import java.util.Set; public abstract class LoggingCommand extends Command { + // Prefix for the command name (appended ahead of the command name) - this can be appended to when the command gets + // embedded at another command through appendNamePrefix private String namePrefix; + // Private name for the command (last in the hierarchy for this command) - this can be changed through setName private String commandName; - private final Command underlying; - + // The delegate command where we refer all calls + private Command underlying; + // The name as it would be used in the logs private String fullyQualifiedName; // Lazy initialized log entry to make sure we don't create it until it is needed (and command is scheduled) // otherwise we get an empty line in the log visualization tool @@ -24,6 +28,23 @@ public LoggingCommand(String namePrefix, String commandName, Command underlying) resetLoggingName(namePrefix, commandName); } + /** + * A special constructor to allow for the creation of the command when we can't create the underlying up front. + * It is assumed that the {@link #setUnderlying(Command)} method is called immediately following the construction. + */ + protected LoggingCommand(String namePrefix, String commandName) { + this.namePrefix = namePrefix; + this.commandName = commandName; + resetLoggingName(namePrefix, commandName); + } + + /** + * Second phase of initialization - set the underlying command + */ + protected void setUnderlying(Command underlying) { + this.underlying = underlying; + } + @Override public void initialize() { getLoggingEntry().append(true); @@ -82,8 +103,12 @@ public String getNamePrefix() { return namePrefix; } - public void setNamePrefix(String prefix) { - this.namePrefix = prefix; + /** + * Add a new prefix in front of the current one + * @param prefix the new prefix + */ + public void appendNamePrefix(String prefix) { + this.namePrefix = prefix + CommandUtil.NAME_SEPARATOR + namePrefix; resetLoggingName(namePrefix, commandName); } @@ -111,3 +136,5 @@ private BooleanLogEntry getLoggingEntry() { return logEntry; } } + + diff --git a/src/main/java/org/usfirst/frc4048/common/logging/ParallelDeadlineLoggingCommand.java b/src/main/java/org/usfirst/frc4048/common/logging/ParallelDeadlineLoggingCommand.java new file mode 100644 index 0000000..fe4f378 --- /dev/null +++ b/src/main/java/org/usfirst/frc4048/common/logging/ParallelDeadlineLoggingCommand.java @@ -0,0 +1,41 @@ +package org.usfirst.frc4048.common.logging; + +import edu.wpi.first.wpilibj2.command.Command; +import edu.wpi.first.wpilibj2.command.ParallelDeadlineGroup; + +import java.util.Arrays; + +public class ParallelDeadlineLoggingCommand extends GroupLoggingCommand { + private LoggingCommand deadlineLoggingCommand; + + /** + * Constructor for parallel deadline command group. + * + * @param namePrefix the name for the group - this is where the sub-commands for this group will be nested in + * @param deadline the deadline command - the one that when ends, stops other commands + * @param commands the sub commands for this group (either regular commands or LoggingCommand are OK) + */ + public ParallelDeadlineLoggingCommand(String namePrefix, Command deadline, Command... commands) { + // Since we can't initialize the deadlineGroup with empty commands, use the special constructor to pass in the + // underlying afterward + super(namePrefix); + + deadlineLoggingCommand = CommandUtil.wrapForLogging(namePrefix, deadline)[0]; + LoggingCommand[] wrapped = CommandUtil.wrapForLogging(namePrefix, commands); + setUnderlying(new ParallelDeadlineGroup(deadlineLoggingCommand, wrapped)); + addLoggingCommands(Arrays.asList(wrapped)); + } + + public final void addCommands(Command... commands) { + LoggingCommand[] wrapped = CommandUtil.wrapForLogging(getNamePrefix(), commands); + ((ParallelDeadlineGroup) getUnderlying()).addCommands(wrapped); + addLoggingCommands(Arrays.asList(wrapped)); + } + + @Override + public void appendNamePrefix(String prefix) { + super.appendNamePrefix(prefix); + // Change the name for the deadline + deadlineLoggingCommand.appendNamePrefix(prefix); + } +} diff --git a/src/main/java/org/usfirst/frc4048/common/logging/ParallelLoggingCommand.java b/src/main/java/org/usfirst/frc4048/common/logging/ParallelLoggingCommand.java index 4d626ac..62fe63e 100644 --- a/src/main/java/org/usfirst/frc4048/common/logging/ParallelLoggingCommand.java +++ b/src/main/java/org/usfirst/frc4048/common/logging/ParallelLoggingCommand.java @@ -4,13 +4,8 @@ import edu.wpi.first.wpilibj2.command.ParallelCommandGroup; import java.util.Arrays; -import java.util.List; - -public class ParallelLoggingCommand extends LoggingCommand { - private static final String THIS_NAME = "-this"; - - private List loggingCommands; +public class ParallelLoggingCommand extends GroupLoggingCommand { /** * Constructor for parallel command group. * @@ -18,39 +13,17 @@ public class ParallelLoggingCommand extends LoggingCommand { * @param commands the sub commands for this group (either regular commands or LoggingCommand are OK) */ public ParallelLoggingCommand(String namePrefix, Command... commands) { - super(namePrefix, THIS_NAME, new ParallelCommandGroup()); + // Call super with an empty group, populate children afterward + super(namePrefix, new ParallelCommandGroup()); + LoggingCommand[] wrapped = CommandUtil.wrapForLogging(namePrefix, commands); ((ParallelCommandGroup) getUnderlying()).addCommands(wrapped); - this.loggingCommands = Arrays.asList(wrapped); + addLoggingCommands(Arrays.asList(wrapped)); } public final void addCommands(Command... commands) { LoggingCommand[] wrapped = CommandUtil.wrapForLogging(getNamePrefix(), commands); ((ParallelCommandGroup) getUnderlying()).addCommands(wrapped); - loggingCommands.addAll(Arrays.asList(wrapped)); - } - - @Override - public void setName(String name) { - // Do not change the logging name for this command (it is fixed) - getUnderlying().setName(name); - } - - @Override - public void setNamePrefix(String prefix) { - super.setNamePrefix(prefix); - setChildrenPrefix(prefix); - } - - @Override - public String toString() { - return getFullyQualifiedName(); - } - - private void setChildrenPrefix(String prefix) { - // Recursively change the prefix for all child commands - for (LoggingCommand loggingCommand : loggingCommands) { - loggingCommand.setNamePrefix(prefix); - } + addLoggingCommands(Arrays.asList(wrapped)); } } diff --git a/src/main/java/org/usfirst/frc4048/common/logging/RaceLoggingCommand.java b/src/main/java/org/usfirst/frc4048/common/logging/RaceLoggingCommand.java index 9eea3b3..f93a441 100644 --- a/src/main/java/org/usfirst/frc4048/common/logging/RaceLoggingCommand.java +++ b/src/main/java/org/usfirst/frc4048/common/logging/RaceLoggingCommand.java @@ -4,13 +4,8 @@ import edu.wpi.first.wpilibj2.command.ParallelRaceGroup; import java.util.Arrays; -import java.util.List; - -public class RaceLoggingCommand extends LoggingCommand { - private static final String THIS_NAME = "-this"; - - private List loggingCommands; +public class RaceLoggingCommand extends GroupLoggingCommand { /** * Constructor for race command group. * @@ -18,39 +13,17 @@ public class RaceLoggingCommand extends LoggingCommand { * @param commands the sub commands for this group (either regular commands or LoggingCommand are OK) */ public RaceLoggingCommand(String namePrefix, Command... commands) { - super(namePrefix, THIS_NAME, new ParallelRaceGroup()); + // Call super with an empty group, populate children afterward + super(namePrefix, new ParallelRaceGroup()); + LoggingCommand[] wrapped = CommandUtil.wrapForLogging(namePrefix, commands); ((ParallelRaceGroup) getUnderlying()).addCommands(wrapped); - this.loggingCommands = Arrays.asList(wrapped); + addLoggingCommands(Arrays.asList(wrapped)); } public final void addCommands(Command... commands) { LoggingCommand[] wrapped = CommandUtil.wrapForLogging(getNamePrefix(), commands); ((ParallelRaceGroup) getUnderlying()).addCommands(wrapped); - loggingCommands.addAll(Arrays.asList(wrapped)); - } - - @Override - public void setName(String name) { - // Do not change the logging name for this command (it is fixed) - getUnderlying().setName(name); - } - - @Override - public void setNamePrefix(String prefix) { - super.setNamePrefix(prefix); - setChildrenPrefix(prefix); - } - - @Override - public String toString() { - return getFullyQualifiedName(); - } - - private void setChildrenPrefix(String prefix) { - // Recursively change the prefix for all child commands - for (LoggingCommand loggingCommand : loggingCommands) { - loggingCommand.setNamePrefix(prefix); - } + addLoggingCommands(Arrays.asList(wrapped)); } } diff --git a/src/main/java/org/usfirst/frc4048/common/logging/SequentialLoggingCommand.java b/src/main/java/org/usfirst/frc4048/common/logging/SequentialLoggingCommand.java index ce4261d..97ab270 100644 --- a/src/main/java/org/usfirst/frc4048/common/logging/SequentialLoggingCommand.java +++ b/src/main/java/org/usfirst/frc4048/common/logging/SequentialLoggingCommand.java @@ -4,13 +4,8 @@ import edu.wpi.first.wpilibj2.command.SequentialCommandGroup; import java.util.Arrays; -import java.util.List; - -public class SequentialLoggingCommand extends LoggingCommand { - private static final String THIS_NAME = "-this"; - - private final List loggingCommands; +public class SequentialLoggingCommand extends GroupLoggingCommand { /** * Constructor for sequential command group. * @@ -18,44 +13,17 @@ public class SequentialLoggingCommand extends LoggingCommand { * @param commands the sub commands for this group (either regular commands or LoggingCommand are OK) */ public SequentialLoggingCommand(String namePrefix, Command... commands) { - super(namePrefix, THIS_NAME, new SequentialCommandGroup()); + // Call super with an empty group, populate children afterward + super(namePrefix, new SequentialCommandGroup()); + LoggingCommand[] wrapped = CommandUtil.wrapForLogging(namePrefix, commands); ((SequentialCommandGroup) getUnderlying()).addCommands(wrapped); - this.loggingCommands = Arrays.asList(wrapped); + addLoggingCommands(Arrays.asList(wrapped)); } public final void addCommands(Command... commands) { LoggingCommand[] wrapped = CommandUtil.wrapForLogging(getNamePrefix(), commands); ((SequentialCommandGroup) getUnderlying()).addCommands(wrapped); - loggingCommands.addAll(Arrays.asList(wrapped)); - } - - @Override - public void setName(String name) { - // Do not change the logging name for this command (it is fixed) - getUnderlying().setName(name); - } - - @Override - public void setNamePrefix(String prefix) { - super.setNamePrefix(prefix); - setChildrenPrefix(prefix); - } - - @Override - public String toString() { - return getFullyQualifiedName(); - } - - // For testing - public List getLoggingCommands() { - return loggingCommands; - } - - private void setChildrenPrefix(String prefix) { - // Recursively change the prefix for all child commands - for (LoggingCommand loggingCommand : loggingCommands) { - loggingCommand.setNamePrefix(prefix); - } + addLoggingCommands(Arrays.asList(wrapped)); } } From c5324d6d25a595f8e838fa4380cdb242b944894b Mon Sep 17 00:00:00 2001 From: ohad Date: Sat, 9 Mar 2024 17:13:43 -0500 Subject: [PATCH 3/3] add tests --- .../frc4048/common/logging/LoggingTest.java | 43 +++++++++++++------ 1 file changed, 31 insertions(+), 12 deletions(-) diff --git a/src/test/java/org/usfirst/frc4048/common/logging/LoggingTest.java b/src/test/java/org/usfirst/frc4048/common/logging/LoggingTest.java index ce207c1..9732d80 100644 --- a/src/test/java/org/usfirst/frc4048/common/logging/LoggingTest.java +++ b/src/test/java/org/usfirst/frc4048/common/logging/LoggingTest.java @@ -34,7 +34,7 @@ public void testSequenceInstantCommand() { SequentialLoggingCommand sequence = (SequentialLoggingCommand) CommandUtil.sequence("sequence", command); Assertions.assertEquals("sequence", sequence.getNamePrefix()); Assertions.assertEquals("Commands/sequence/-this", sequence.getFullyQualifiedName()); - Assertions.assertEquals("Commands/sequence/InstantCommand", sequence.getLoggingCommands().get(0).getFullyQualifiedName()); + Assertions.assertEquals("Commands/sequence/InstantCommand", sequence.getChildLoggingCommands().get(0).getFullyQualifiedName()); } @Test @@ -48,7 +48,7 @@ public String getName() { SequentialLoggingCommand sequence = (SequentialLoggingCommand) CommandUtil.sequence("sequence", command); Assertions.assertEquals("sequence", sequence.getNamePrefix()); Assertions.assertEquals("Commands/sequence/-this", sequence.getFullyQualifiedName()); - Assertions.assertEquals("Commands/sequence/command", sequence.getLoggingCommands().get(0).getFullyQualifiedName()); + Assertions.assertEquals("Commands/sequence/command", sequence.getChildLoggingCommands().get(0).getFullyQualifiedName()); } @Test @@ -57,7 +57,7 @@ public void testSequenceLoggingInstantCommand() { SequentialLoggingCommand sequence = (SequentialLoggingCommand) CommandUtil.sequence("sequence", command); Assertions.assertEquals("sequence", sequence.getNamePrefix()); Assertions.assertEquals("Commands/sequence/-this", sequence.getFullyQualifiedName()); - Assertions.assertEquals("Commands/sequence/command/InstantCommand", sequence.getLoggingCommands().get(0).getFullyQualifiedName()); + Assertions.assertEquals("Commands/sequence/command/InstantCommand", sequence.getChildLoggingCommands().get(0).getFullyQualifiedName()); } @Test @@ -71,7 +71,7 @@ public String getName() { SequentialLoggingCommand sequence = (SequentialLoggingCommand) CommandUtil.sequence("sequence", CommandUtil.logged("logged", command)); Assertions.assertEquals("sequence", sequence.getNamePrefix()); Assertions.assertEquals("Commands/sequence/-this", sequence.getFullyQualifiedName()); - Assertions.assertEquals("Commands/sequence/logged/command", sequence.getLoggingCommands().get(0).getFullyQualifiedName()); + Assertions.assertEquals("Commands/sequence/logged/command", sequence.getChildLoggingCommands().get(0).getFullyQualifiedName()); } @Test @@ -85,7 +85,7 @@ public String getName() { SequentialLoggingCommand sequence = (SequentialLoggingCommand) CommandUtil.sequence("sequence", CommandUtil.logged(command)); Assertions.assertEquals("sequence", sequence.getNamePrefix()); Assertions.assertEquals("Commands/sequence/-this", sequence.getFullyQualifiedName()); - Assertions.assertEquals("Commands/sequence/null/command", sequence.getLoggingCommands().get(0).getFullyQualifiedName()); + Assertions.assertEquals("Commands/sequence/null/command", sequence.getChildLoggingCommands().get(0).getFullyQualifiedName()); } @Test @@ -100,9 +100,9 @@ public String getName() { SequentialLoggingCommand outer = (SequentialLoggingCommand) CommandUtil.sequence("outer", inner); Assertions.assertEquals("outer", outer.getNamePrefix()); Assertions.assertEquals("Commands/outer/-this", outer.getFullyQualifiedName()); - inner = (SequentialLoggingCommand) outer.getLoggingCommands().get(0); + inner = (SequentialLoggingCommand) outer.getChildLoggingCommands().get(0); Assertions.assertEquals("Commands/outer/inner/-this", inner.getFullyQualifiedName()); - Assertions.assertEquals("Commands/outer/inner/command", inner.getLoggingCommands().get(0).getFullyQualifiedName()); + Assertions.assertEquals("Commands/outer/inner/command", inner.getChildLoggingCommands().get(0).getFullyQualifiedName()); } @Test @@ -117,9 +117,28 @@ public String getName() { SequentialLoggingCommand outer = (SequentialLoggingCommand) CommandUtil.sequence("outer", inner); Assertions.assertEquals("outer", outer.getNamePrefix()); Assertions.assertEquals("Commands/outer/-this", outer.getFullyQualifiedName()); - inner = (SequentialLoggingCommand) outer.getLoggingCommands().get(0); + inner = (SequentialLoggingCommand) outer.getChildLoggingCommands().get(0); Assertions.assertEquals("Commands/outer/inner/-this", inner.getFullyQualifiedName()); - Assertions.assertEquals("Commands/outer/inner/command", inner.getLoggingCommands().get(0).getFullyQualifiedName()); + Assertions.assertEquals("Commands/outer/inner/logged/command", inner.getChildLoggingCommands().get(0).getFullyQualifiedName()); + } + + @Test + public void testFourLevelNesting() { + Command command = new Command() { + @Override + public String getName() { + return "command"; + } + }; + SequentialLoggingCommand first = (SequentialLoggingCommand) CommandUtil.sequence("first", CommandUtil.logged("logged", command)); + SequentialLoggingCommand second = (SequentialLoggingCommand) CommandUtil.sequence("second", first); + SequentialLoggingCommand third = (SequentialLoggingCommand) CommandUtil.sequence("third", second); + SequentialLoggingCommand fourth = (SequentialLoggingCommand) CommandUtil.sequence("fourth", third); + + Assertions.assertEquals("Commands/fourth/-this", fourth.getFullyQualifiedName()); + Assertions.assertEquals("Commands/fourth/third/-this", third.getFullyQualifiedName()); + Assertions.assertEquals("Commands/fourth/third/second/-this", second.getFullyQualifiedName()); + Assertions.assertEquals("Commands/fourth/third/second/first/-this", first.getFullyQualifiedName()); } @Test @@ -127,7 +146,7 @@ public void testCustomSequentialCommand() { SequentialLoggingCommand customSequence = new CustomSequentialGroup(); Assertions.assertEquals("Sequence", customSequence.getNamePrefix()); Assertions.assertEquals("Commands/Sequence/-this", customSequence.getFullyQualifiedName()); - Assertions.assertEquals("Commands/Sequence/Command", customSequence.getLoggingCommands().get(0).getFullyQualifiedName()); + Assertions.assertEquals("Commands/Sequence/Command", customSequence.getChildLoggingCommands().get(0).getFullyQualifiedName()); } @Test @@ -135,8 +154,8 @@ public void testCustomSequenceOfSequence() { SequentialLoggingCommand customSequence = new CustomSequentialGroup(); SequentialLoggingCommand outer = (SequentialLoggingCommand) CommandUtil.sequence("outer", customSequence); Assertions.assertEquals("outer", outer.getNamePrefix()); - Assertions.assertEquals("Commands/outer/Sequence/-this", outer.getLoggingCommands().get(0).getFullyQualifiedName()); - Assertions.assertEquals("Commands/outer/Sequence/Command", ((SequentialLoggingCommand) outer.getLoggingCommands().get(0)).getLoggingCommands().get(0).getFullyQualifiedName()); + Assertions.assertEquals("Commands/outer/Sequence/-this", outer.getChildLoggingCommands().get(0).getFullyQualifiedName()); + Assertions.assertEquals("Commands/outer/Sequence/Command", ((SequentialLoggingCommand) outer.getChildLoggingCommands().get(0)).getChildLoggingCommands().get(0).getFullyQualifiedName()); } private static class CustomSequentialGroup extends SequentialLoggingCommand {