code review

src/main/java/com/team766/framework3/AutonomousMode.java

@@ -5,21 +5,16 @@
 import java.util.function.Supplier;
 
 public class AutonomousMode implements AutonomousSelector.Selectable<AutonomousMode> {
-    private final Supplier<Command> m_constructor;
+    private final Supplier<Procedure> m_constructor;
     private final String m_name;
 
-    public AutonomousMode(final String name, final Supplier<Command> constructor) {
-        m_constructor = constructor;
+    public AutonomousMode(final String name, final Supplier<Procedure> constructor) {
         m_name = name;
-    }
-
-    public AutonomousMode(
-            final String name, final com.google.common.base.Supplier<Procedure> 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() {

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 with each other.
      */
     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 with each other.
      */
     void runParallelRace(Procedure... procedures);
 }

src/main/java/com/team766/framework3/ContextImpl.java

@@ -27,7 +27,7 @@
 class ContextImpl extends Command implements Context, LaunchedContext {
     // Maintains backward compatibility with the behavior of the old Maroon Framework scheduler.
     // TODO: Re-evaluate whether this should use the default Command behavior (return false).
-    static final boolean RUNS_WHEN_DISABLED = true;
+    private static final boolean RUNS_WHEN_DISABLED = true;
 
     /**
      * Represents the baton-passing state (see class comments). Instead of
@@ -324,12 +324,12 @@ public void waitFor(final BooleanSupplier predicate) {
 
     @Override
     public void waitFor(final LaunchedContext otherContext) {
-        waitFor(otherContext::isDone);
+        waitFor(otherContext::isFinished);
     }
 
     @Override
     public void waitFor(final LaunchedContext... otherContexts) {
-        waitFor(() -> Arrays.stream(otherContexts).allMatch(LaunchedContext::isDone));
+        waitFor(() -> Arrays.stream(otherContexts).allMatch(LaunchedContext::isFinished));
     }
 
     @Override
@@ -360,32 +360,32 @@ 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)));
     }
 
     @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)));
     }
 
     private void checkProcedureReservationsSubset(Procedure procedure) {
-        final var this_reservations = getRequirements();
+        final var thisReservations = getRequirements();
         for (var req : procedure.reservations()) {
-            if (!this_reservations.contains(req)) {
+            if (!thisReservations.contains(req)) {
                 throw new IllegalArgumentException(
                         getName()
                                 + " tried to run "
@@ -397,9 +397,9 @@ private void checkProcedureReservationsSubset(Procedure procedure) {
     }
 
     private void checkProcedureReservationsDisjoint(Procedure procedure) {
-        final var this_reservations = getRequirements();
+        final var thisReservations = getRequirements();
         for (var req : procedure.reservations()) {
-            if (this_reservations.contains(req)) {
+            if (thisReservations.contains(req)) {
                 throw new IllegalArgumentException(
                         getName()
                                 + " tried to launch "
@@ -419,7 +419,7 @@ public void initialize() {
 
     @Override
     public boolean isFinished() {
-        return isDone();
+        return m_state == State.DONE;
     }
 
     @Override
@@ -455,9 +455,4 @@ public void execute() {
     public boolean runsWhenDisabled() {
         return RUNS_WHEN_DISABLED;
     }
-
-    @Override
-    public boolean isDone() {
-        return m_state == State.DONE;
-    }
 }

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<Subsystem> reservations, Runnable runnable) {
+    public FunctionalInstantProcedure(Set<Mechanism<?>> reservations, Runnable runnable) {
         super(runnable.toString(), reservations);
         this.runnable = runnable;
     }

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<Context> runnable;
 
-    public FunctionalProcedure(Set<Subsystem> reservations, Consumer<Context> runnable) {
+    public FunctionalProcedure(Set<Mechanism<?>> reservations, Consumer<Context> runnable) {
         super(runnable.toString(), reservations);
         this.runnable = runnable;
     }

src/main/java/com/team766/framework3/InstantProcedure.java

@@ -1,15 +1,14 @@
 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 {
     protected InstantProcedure() {
         super();
     }
 
-    protected InstantProcedure(String name, Set<Subsystem> reservations) {
+    protected InstantProcedure(String name, Set<Mechanism<?>> reservations) {
         super(name, reservations);
     }
 

src/main/java/com/team766/framework3/LaunchedContext.java

@@ -13,7 +13,7 @@ public interface LaunchedContext {
     /**
      * Returns true if this Context has finished running, false otherwise.
      */
-    boolean isDone();
+    boolean isFinished();
 
     /**
      * Interrupt the running of this Context and force it to terminate.

src/main/java/com/team766/framework3/Mechanism.java

@@ -13,21 +13,19 @@ public abstract class Mechanism<R extends Request<?>> 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) {

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<Subsystem> reservations;
-    protected Category loggerCategory = Category.PROCEDURES;
+    private final Set<Mechanism<?>> reservations;
 
     protected Procedure() {
         this.name = this.getClass().getName() + "/" + createNewId();
         this.reservations = Sets.newHashSet();
     }
 
-    protected Procedure(String name, Set<Subsystem> reservations) {
+    protected Procedure(String name, Set<Mechanism<?>> 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 extends Subsystem> M reserve(M m) {
+    protected final <M extends Mechanism<?>> 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<? extends Subsystem> ms) {
+    protected final void reserve(Collection<? extends Mechanism<?>> ms) {
         reservations.addAll(ms);
     }
 
-    public final Set<Subsystem> reservations() {
+    public final Set<Mechanism<?>> reservations() {
         return reservations;
     }
 

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<Subsystem> reservations, Runnable action) {
+        public Builder withNewlyTriggeringProcedure(
+                Set<Mechanism<?>> reservations, Runnable action) {
             this.newlyTriggeringProcedure =
                     () -> new FunctionalInstantProcedure(reservations, action);
             return this;
@@ -94,7 +94,7 @@ public Builder withContinuingTriggeringProcedure(ProcedureCreator action) {
         }
 
         public Builder withContinuingTriggeringProcedure(
-                Set<Subsystem> reservations, Runnable action) {
+                Set<Mechanism<?>> reservations, Runnable action) {
             this.continuingTriggeringProcedure =
                     () -> new FunctionalInstantProcedure(reservations, action);
             return this;
@@ -107,7 +107,7 @@ public Builder withFinishedTriggeringProcedure(ProcedureCreator action) {
         }
 
         public Builder withFinishedTriggeringProcedure(
-                Set<Subsystem> reservations, Runnable action) {
+                Set<Mechanism<?>> 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<TriggerType, ProcedureCreator> triggerProcedures =
             Maps.newEnumMap(TriggerType.class);
-    private final Map<TriggerType, Set<Subsystem>> triggerReservations =
+    private final Map<TriggerType, Set<Mechanism<?>>> triggerReservations =
             Maps.newEnumMap(TriggerType.class);
 
     private TriggerType currentTriggerType = TriggerType.NONE;
@@ -173,7 +173,7 @@ private Rule(
         }
     }
 
-    private Set<Subsystem> getReservationsForProcedure(ProcedureCreator creator) {
+    private Set<Mechanism<?>> getReservationsForProcedure(ProcedureCreator creator) {
         if (creator != null) {
             Procedure procedure = creator.create();
             if (procedure != null) {
@@ -211,7 +211,7 @@ public String getName() {
         }
     }
 
-    /* package */ Set<Subsystem> getMechanismsToReserve() {
+    /* package */ Set<Mechanism<?>> getMechanismsToReserve() {
         if (triggerReservations.containsKey(currentTriggerType)) {
             return triggerReservations.get(currentTriggerType);
         }