diff --git a/src/main/java/com/team766/framework3/AutonomousMode.java b/src/main/java/com/team766/framework3/AutonomousMode.java index 71823b6e6..bf9b4accd 100644 --- a/src/main/java/com/team766/framework3/AutonomousMode.java +++ b/src/main/java/com/team766/framework3/AutonomousMode.java @@ -5,21 +5,16 @@ import java.util.function.Supplier; public class AutonomousMode implements AutonomousSelector.Selectable { - private final Supplier m_constructor; + private final Supplier m_constructor; private final String m_name; - public AutonomousMode(final String name, final Supplier constructor) { - m_constructor = constructor; + public AutonomousMode(final String name, final Supplier constructor) { m_name = name; - } - - public AutonomousMode( - final String name, final com.google.common.base.Supplier constructor) { - this(name, () -> new ContextImpl(constructor.get())); + m_constructor = () -> constructor.get(); } public Command instantiate() { - return m_constructor.get(); + return m_constructor.get().createCommand(); } public String name() { diff --git a/src/main/java/com/team766/framework3/Context.java b/src/main/java/com/team766/framework3/Context.java index f09366775..01e052429 100644 --- a/src/main/java/com/team766/framework3/Context.java +++ b/src/main/java/com/team766/framework3/Context.java @@ -94,18 +94,23 @@ public interface Context { /** * Run the given Procedure synchronously (the calling Procedure will not resume until this one * has finished). + * The given procedure must only reserve Mechanisms that were reserved by the calling Procedure. */ void runSync(final Procedure func); /** * Run the given Procedures at the same time. The calling Procedure will resume after all * Procedures in the group finish. + * The given procedures must only reserve Mechanisms that were reserved by the calling Procedure, + * and their reservations must not overlap. */ void runParallel(Procedure... procedures); /** * Run the given Procedures at the same time. The calling Procedure will resume once any * Procedure in the group finishes, and the others will be cancelled. + * The given procedures must only reserve Mechanisms that were reserved by the calling Procedure, + * and their reservations must not overlap. */ void runParallelRace(Procedure... procedures); } diff --git a/src/main/java/com/team766/framework3/ContextImpl.java b/src/main/java/com/team766/framework3/ContextImpl.java index a646a2f21..ca64563da 100644 --- a/src/main/java/com/team766/framework3/ContextImpl.java +++ b/src/main/java/com/team766/framework3/ContextImpl.java @@ -360,11 +360,11 @@ public void runSync(final Procedure procedure) { @Override public void runParallel(Procedure... procedures) { - var contexts = new ContextImpl[procedures.length]; + var contexts = new Command[procedures.length]; for (int i = 0; i < contexts.length; ++i) { var procedure = procedures[i]; checkProcedureReservationsSubset(procedure); - contexts[i] = new ContextImpl(procedure); + contexts[i] = procedure.createCommand(); } // NOTE: Commands.parallel will ensure procedures' reservations are disjoint. runSync(new WPILibCommandProcedure(Commands.parallel(contexts))); @@ -372,11 +372,11 @@ public void runParallel(Procedure... procedures) { @Override public void runParallelRace(Procedure... procedures) { - var contexts = new ContextImpl[procedures.length]; + var contexts = new Command[procedures.length]; for (int i = 0; i < contexts.length; ++i) { var procedure = procedures[i]; checkProcedureReservationsSubset(procedure); - contexts[i] = new ContextImpl(procedure); + contexts[i] = procedure.createCommand(); } // NOTE: Commands.race will ensure procedures' reservations are disjoint. runSync(new WPILibCommandProcedure(Commands.race(contexts))); diff --git a/src/main/java/com/team766/framework3/FunctionalInstantProcedure.java b/src/main/java/com/team766/framework3/FunctionalInstantProcedure.java index 69e038672..2a78d02ab 100644 --- a/src/main/java/com/team766/framework3/FunctionalInstantProcedure.java +++ b/src/main/java/com/team766/framework3/FunctionalInstantProcedure.java @@ -1,12 +1,11 @@ package com.team766.framework3; -import edu.wpi.first.wpilibj2.command.Subsystem; import java.util.Set; public final class FunctionalInstantProcedure extends InstantProcedure { private final Runnable runnable; - public FunctionalInstantProcedure(Set reservations, Runnable runnable) { + public FunctionalInstantProcedure(Set> reservations, Runnable runnable) { super(runnable.toString(), reservations); this.runnable = runnable; } diff --git a/src/main/java/com/team766/framework3/FunctionalProcedure.java b/src/main/java/com/team766/framework3/FunctionalProcedure.java index 092e5e015..69d13c1b8 100644 --- a/src/main/java/com/team766/framework3/FunctionalProcedure.java +++ b/src/main/java/com/team766/framework3/FunctionalProcedure.java @@ -1,13 +1,12 @@ package com.team766.framework3; -import edu.wpi.first.wpilibj2.command.Subsystem; import java.util.Set; import java.util.function.Consumer; public final class FunctionalProcedure extends Procedure { private final Consumer runnable; - public FunctionalProcedure(Set reservations, Consumer runnable) { + public FunctionalProcedure(Set> reservations, Consumer runnable) { super(runnable.toString(), reservations); this.runnable = runnable; } diff --git a/src/main/java/com/team766/framework3/InstantProcedure.java b/src/main/java/com/team766/framework3/InstantProcedure.java index 5d3262bf5..7689a5985 100644 --- a/src/main/java/com/team766/framework3/InstantProcedure.java +++ b/src/main/java/com/team766/framework3/InstantProcedure.java @@ -1,7 +1,6 @@ package com.team766.framework3; import edu.wpi.first.wpilibj2.command.Command; -import edu.wpi.first.wpilibj2.command.Subsystem; import java.util.Set; public abstract class InstantProcedure extends Procedure implements Runnable { @@ -9,7 +8,7 @@ protected InstantProcedure() { super(); } - protected InstantProcedure(String name, Set reservations) { + protected InstantProcedure(String name, Set> reservations) { super(name, reservations); } diff --git a/src/main/java/com/team766/framework3/Mechanism.java b/src/main/java/com/team766/framework3/Mechanism.java index 25a792f41..a9a2e8238 100644 --- a/src/main/java/com/team766/framework3/Mechanism.java +++ b/src/main/java/com/team766/framework3/Mechanism.java @@ -13,21 +13,19 @@ public abstract class Mechanism> extends SubsystemBase impl private R request = null; private boolean isRequestNew = false; - protected Category loggerCategory = Category.MECHANISMS; - @Override public Category getLoggerCategory() { - return loggerCategory; + return Category.MECHANISMS; } public final void setRequest(R request) { - checkContextOwnership(); + checkContextReservation(); this.request = request; isRequestNew = true; log(this.getClass().getName() + " processing request: " + request); } - protected void checkContextOwnership() { + protected void checkContextReservation() { var owningCommand = CommandScheduler.getInstance().requiring(this); if ((owningCommand == null || SchedulerMonitor.currentCommand != owningCommand) && m_runningPeriodic == null) { diff --git a/src/main/java/com/team766/framework3/Procedure.java b/src/main/java/com/team766/framework3/Procedure.java index ced0eac97..30a14c523 100644 --- a/src/main/java/com/team766/framework3/Procedure.java +++ b/src/main/java/com/team766/framework3/Procedure.java @@ -3,7 +3,6 @@ import com.google.common.collect.Sets; import com.team766.logging.Category; import edu.wpi.first.wpilibj2.command.Command; -import edu.wpi.first.wpilibj2.command.Subsystem; import java.util.Collection; import java.util.Set; @@ -23,22 +22,21 @@ private static synchronized int createNewId() { } private final String name; - private final Set reservations; - protected Category loggerCategory = Category.PROCEDURES; + private final Set> reservations; protected Procedure() { this.name = this.getClass().getName() + "/" + createNewId(); this.reservations = Sets.newHashSet(); } - protected Procedure(String name, Set reservations) { + protected Procedure(String name, Set> reservations) { this.name = name; this.reservations = reservations; } public abstract void run(Context context); - Command createCommand() { + /* package */ Command createCommand() { return new ContextImpl(this); } @@ -49,25 +47,25 @@ public final String getName() { @Override public Category getLoggerCategory() { - return loggerCategory; + return Category.PROCEDURES; } - protected final M reserve(M m) { + protected final > M reserve(M m) { reservations.add(m); return m; } - protected final void reserve(Subsystem... ms) { + protected final void reserve(Mechanism... ms) { for (var m : ms) { reservations.add(m); } } - protected final void reserve(Collection ms) { + protected final void reserve(Collection> ms) { reservations.addAll(ms); } - public final Set reservations() { + public final Set> reservations() { return reservations; } diff --git a/src/main/java/com/team766/framework3/Rule.java b/src/main/java/com/team766/framework3/Rule.java index 204721ebf..8b40a2428 100644 --- a/src/main/java/com/team766/framework3/Rule.java +++ b/src/main/java/com/team766/framework3/Rule.java @@ -1,7 +1,6 @@ package com.team766.framework3; import com.google.common.collect.Maps; -import edu.wpi.first.wpilibj2.command.Subsystem; import java.util.Collections; import java.util.Map; import java.util.Set; @@ -37,7 +36,7 @@ public class Rule { * Functional interface for creating new {@link Procedure}s. */ @FunctionalInterface - private interface ProcedureCreator { + public interface ProcedureCreator { Procedure create(); } @@ -81,7 +80,8 @@ public Builder withNewlyTriggeringProcedure(ProcedureCreator action) { return this; } - public Builder withNewlyTriggeringProcedure(Set reservations, Runnable action) { + public Builder withNewlyTriggeringProcedure( + Set> reservations, Runnable action) { this.newlyTriggeringProcedure = () -> new FunctionalInstantProcedure(reservations, action); return this; @@ -94,7 +94,7 @@ public Builder withContinuingTriggeringProcedure(ProcedureCreator action) { } public Builder withContinuingTriggeringProcedure( - Set reservations, Runnable action) { + Set> reservations, Runnable action) { this.continuingTriggeringProcedure = () -> new FunctionalInstantProcedure(reservations, action); return this; @@ -107,7 +107,7 @@ public Builder withFinishedTriggeringProcedure(ProcedureCreator action) { } public Builder withFinishedTriggeringProcedure( - Set reservations, Runnable action) { + Set> reservations, Runnable action) { this.finishedTriggeringProcedure = () -> new FunctionalInstantProcedure(reservations, action); return this; @@ -128,7 +128,7 @@ public Builder withFinishedTriggeringProcedure( private final BooleanSupplier predicate; private final Map triggerProcedures = Maps.newEnumMap(TriggerType.class); - private final Map> triggerReservations = + private final Map>> triggerReservations = Maps.newEnumMap(TriggerType.class); private TriggerType currentTriggerType = TriggerType.NONE; @@ -173,7 +173,7 @@ private Rule( } } - private Set getReservationsForProcedure(ProcedureCreator creator) { + private Set> getReservationsForProcedure(ProcedureCreator creator) { if (creator != null) { Procedure procedure = creator.create(); if (procedure != null) { @@ -211,7 +211,7 @@ public String getName() { } } - /* package */ Set getMechanismsToReserve() { + /* package */ Set> getMechanismsToReserve() { if (triggerReservations.containsKey(currentTriggerType)) { return triggerReservations.get(currentTriggerType); } diff --git a/src/main/java/com/team766/framework3/RuleEngine.java b/src/main/java/com/team766/framework3/RuleEngine.java index 88b8094b7..3a3970fde 100644 --- a/src/main/java/com/team766/framework3/RuleEngine.java +++ b/src/main/java/com/team766/framework3/RuleEngine.java @@ -3,11 +3,11 @@ import com.google.common.collect.BiMap; import com.google.common.collect.HashBiMap; import com.team766.logging.Category; +import com.team766.logging.Logger; import com.team766.logging.LoggerExceptionUtils; import com.team766.logging.Severity; import edu.wpi.first.wpilibj2.command.Command; import edu.wpi.first.wpilibj2.command.CommandScheduler; -import edu.wpi.first.wpilibj2.command.Subsystem; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedList; @@ -27,8 +27,7 @@ protected RuleEngine() {} @Override public Category getLoggerCategory() { - // TODO: Is this the right default for RuleEngine? - return Category.OPERATOR_INTERFACE; + return Category.RULES; } protected void addRule(Rule.Builder builder) { @@ -44,7 +43,7 @@ protected Rule getRuleForTriggeredProcedure(Command command) { } public final void run() { - Set mechanismsToUse = new HashSet<>(); + Set> mechanismsToUse = new HashSet<>(); // TODO: when creating a Procedure, check that the reservations are the same as // what the Rule pre-computed. @@ -57,22 +56,27 @@ public final void run() { // see if the rule is triggering/just finished triggering Rule.TriggerType triggerType = rule.getCurrentTriggerType(); if (triggerType != Rule.TriggerType.NONE) { - log("Rule " + rule.getName() + " triggering: " + triggerType); + Logger.get(Category.FRAMEWORK) + .logRaw( + Severity.DEBUG, + "Rule " + rule.getName() + " triggering: " + triggerType); int priority = rulePriorities.get(rule); // see if there are mechanisms a potential procedure would want to reserve - Set reservations = rule.getMechanismsToReserve(); - for (Subsystem mechanism : reservations) { + Set> reservations = rule.getMechanismsToReserve(); + for (Mechanism mechanism : reservations) { // see if any of the mechanisms higher priority rules will use would also be // used by this lower priority rule's procedure. if (mechanismsToUse.contains(mechanism)) { - log( - "RULE CONFLICT! Ignoring rule: " - + rule.getName() - + "; mechanism " - + mechanism.getName() - + " already reserved by higher priority rule."); + Logger.get(Category.FRAMEWORK) + .logRaw( + Severity.DEBUG, + "RULE CONFLICT! Ignoring rule: " + + rule.getName() + + "; mechanism " + + mechanism.getName() + + " already reserved by higher priority rule."); continue; } // see if a previously triggered rule is still using the mechanism @@ -87,12 +91,14 @@ public final void run() { if (existingPriority < priority /* less is more */) { // existing rule takes priority. // don't proceed with this new rule. - log( - "RULE CONFLICT! Ignoring rule: " - + rule - + "; mechanism " - + mechanism.getName() - + " already reserved by higher priority rule."); + Logger.get(Category.FRAMEWORK) + .logRaw( + Severity.DEBUG, + "RULE CONFLICT! Ignoring rule: " + + rule + + "; mechanism " + + mechanism.getName() + + " already reserved by higher priority rule."); continue; } } diff --git a/src/main/java/com/team766/framework3/WPILibCommandProcedure.java b/src/main/java/com/team766/framework3/WPILibCommandProcedure.java index a0b3176a3..2b3a15236 100644 --- a/src/main/java/com/team766/framework3/WPILibCommandProcedure.java +++ b/src/main/java/com/team766/framework3/WPILibCommandProcedure.java @@ -1,6 +1,8 @@ package com.team766.framework3; import edu.wpi.first.wpilibj2.command.Command; +import edu.wpi.first.wpilibj2.command.Subsystem; +import java.util.Set; /** * This wraps a class that confroms to WPILib's Command interface, and allows @@ -14,27 +16,43 @@ public final class WPILibCommandProcedure extends Procedure { * @param command The WPILib Command to adapt */ public WPILibCommandProcedure(final Command command) { - super(command.getName(), command.getRequirements()); + super(command.getName(), checkSubsystemsAreMechanisms(command.getRequirements())); this.command = command; } + @SuppressWarnings("unchecked") + private static Set> checkSubsystemsAreMechanisms(Set requirements) { + for (var s : requirements) { + if (!(s instanceof Mechanism)) { + throw new IllegalArgumentException( + "Maroon Framework requires the use of Mechanism instead of Subsystem"); + } + } + return (Set>) (Set) requirements; + } + @Override public void run(final Context context) { boolean interrupted = false; try { - this.command.initialize(); - if (!this.command.isFinished()) { - this.command.execute(); - while (!this.command.isFinished()) { + command.initialize(); + if (!command.isFinished()) { + command.execute(); + while (!command.isFinished()) { context.yield(); - this.command.execute(); + command.execute(); } } } catch (Throwable ex) { interrupted = true; throw ex; } finally { - this.command.end(interrupted); + command.end(interrupted); } } + + @Override + Command createCommand() { + return command; + } } diff --git a/src/main/proto/com/team766/logging/logging.proto b/src/main/proto/com/team766/logging/logging.proto index 46dca6654..01d52e841 100644 --- a/src/main/proto/com/team766/logging/logging.proto +++ b/src/main/proto/com/team766/logging/logging.proto @@ -39,6 +39,7 @@ enum Category { DRIVE = 11; GYRO = 12; ODOMETRY = 13; + RULES = 14; } // `Value` represents a dynamically typed value which can be either a number, diff --git a/src/test/java/com/team766/framework3/MechanismTest.java b/src/test/java/com/team766/framework3/MechanismTest.java index f8cc411a5..2db91c48f 100644 --- a/src/test/java/com/team766/framework3/MechanismTest.java +++ b/src/test/java/com/team766/framework3/MechanismTest.java @@ -11,7 +11,7 @@ import org.junit.jupiter.api.Test; public class MechanismTest extends TestCase3 { - /// Test sending requests to a Mechanism. Also test that checkContextOwnership succeeds when + /// Test sending requests to a Mechanism. Also test that checkContextReservation succeeds when /// called from a Procedure which reserves the Mechanism. @Test public void testRequests() { @@ -77,10 +77,10 @@ protected void run(FakeRequest request, boolean isRequestNew) { assertTrue(cmd.isDone()); } - /// Test that checkContextOwnership throws an exception when called from a Procedure which has + /// Test that checkContextReservation throws an exception when called from a Procedure which has /// not reserved the Mechanism. @Test - public void testFailedCheckContextOwnershipInProcedure() { + public void testFailedCheckContextReservationInProcedure() { class DummyMechanism extends Mechanism { protected void run(FakeRequest request, boolean isRequestNew) {} } @@ -113,10 +113,10 @@ protected void run(FakeRequest request, boolean isRequestNew) {} "DummyMechanism tried to be used by .*MechanismTest\\$\\$Lambda.* while reserved by .*FakeProcedure.*"); } - /// Test that checkContextOwnership succeeds when called from within the Mechanism's own run() + /// Test that checkContextReservation succeeds when called from within the Mechanism's own run() /// method. @Test - public void testCheckContextOwnershipInRun() { + public void testCheckContextReservationInRun() { var thrownException = new AtomicReference(); @SuppressWarnings("unused") var mech = @@ -124,7 +124,7 @@ public void testCheckContextOwnershipInRun() { @Override protected void run(FakeRequest request, boolean isRequestNew) { try { - checkContextOwnership(); + checkContextReservation(); } catch (Throwable ex) { thrownException.set(ex); }