diff --git a/commandsv3/src/main/java/org/wpilib/command3/Scheduler.java b/commandsv3/src/main/java/org/wpilib/command3/Scheduler.java index f55f5c3679..474d66f25e 100644 --- a/commandsv3/src/main/java/org/wpilib/command3/Scheduler.java +++ b/commandsv3/src/main/java/org/wpilib/command3/Scheduler.java @@ -131,7 +131,7 @@ public final class Scheduler implements ProtobufSerializable { private final Stack m_currentCommandAncestry = new Stack<>(); /** The periodic callbacks to run, outside of the command structure. */ - private final List m_periodicCallbacks = new ArrayList<>(); + private final SequencedMap m_periodicCallbacks = new LinkedHashMap<>(); /** Event loop for trigger bindings. */ private final EventLoop m_eventLoop = new EventLoop(); @@ -269,16 +269,18 @@ public final class Scheduler implements ProtobufSerializable { * unrecoverable infinite loop! * * @param callback the callback to sideload + * @see #addPeriodic(Runnable) */ public void sideload(Consumer callback) { var coroutine = new Coroutine(this, m_scope, callback); - m_periodicCallbacks.add(coroutine); + var scope = BindingScope.createNarrowestScope(this); + m_periodicCallbacks.put(scope, coroutine); } /** - * Adds a task to run repeatedly for as long as the scheduler runs. This internally handles the - * looping and control yielding necessary for proper function. The callback will run at the same - * periodic frequency as the scheduler. + * Adds a periodic callback to run as part of the scheduler. The callback should not manipulate or + * control any mechanisms, but can be used to log information, update data (such as simulations or + * LED data buffers), or perform some other helpful task. * *

For example: * @@ -290,7 +292,24 @@ public final class Scheduler implements ProtobufSerializable { * }); * } * + *

{@code addPeriodic} is a convenience method that is identical to using {@link + * #sideload(Consumer)} with an unending {@code while} loop: + * + *

{@code
+   * // An addPeriodic call:
+   * scheduler.addPeriodic(() -> leds.setData(ledDataBuffer));
+   *
+   * // Is equivalent to this sideload call:
+   * scheduler.sideload(coroutine -> {
+   *   while (true) {
+   *     leds.setData(ledDataBuffer)
+   *     coroutine.yield();
+   *   }
+   * });
+   * }
+ * * @param callback the periodic function to run + * @see #sideload(Consumer) */ public void addPeriodic(Runnable callback) { sideload( @@ -641,8 +660,10 @@ public final class Scheduler implements ProtobufSerializable { } private void runPeriodicSideloads() { + m_periodicCallbacks.entrySet().removeIf(e -> !e.getKey().active()); + // Update periodic callbacks - for (Coroutine coroutine : m_periodicCallbacks) { + for (Coroutine coroutine : m_periodicCallbacks.values()) { coroutine.mount(); try { coroutine.runToYieldPoint(); @@ -652,7 +673,7 @@ public final class Scheduler implements ProtobufSerializable { } // And remove any periodic callbacks that have completed - m_periodicCallbacks.removeIf(Coroutine::isDone); + m_periodicCallbacks.entrySet().removeIf(e -> e.getValue().isDone()); } private void runCommands() { diff --git a/commandsv3/src/test/java/org/wpilib/command3/SchedulerSideloadFunctionTests.java b/commandsv3/src/test/java/org/wpilib/command3/SchedulerSideloadFunctionTests.java index c6b0ff92aa..8d32bff285 100644 --- a/commandsv3/src/test/java/org/wpilib/command3/SchedulerSideloadFunctionTests.java +++ b/commandsv3/src/test/java/org/wpilib/command3/SchedulerSideloadFunctionTests.java @@ -50,12 +50,21 @@ class SchedulerSideloadFunctionTests extends CommandTestBase { } @Test - void childCommandEscapesViaSideload() { + void childCommandCannotEscapeViaSideload() { + var sideloadRan = new AtomicBoolean(false); var child = Command.noRequirements(Coroutine::park).named("Child"); var parent = Command.noRequirements( parentCoroutine -> { - m_scheduler.sideload(sideloadCoroutine -> sideloadCoroutine.fork(child)); + // Because this sideloads inside a command, and the command immediately exits, + // the sideload will be removed from the scheduler immediately after being added. + // And because sideloads are run _before_ commands are, it would only have been + // able to run in the next scheduler run (ie, if the parent command had yielded). + m_scheduler.sideload( + sideloadCoroutine -> { + sideloadRan.set(true); + sideloadCoroutine.fork(child); + }); }) .named("Parent"); @@ -65,9 +74,11 @@ class SchedulerSideloadFunctionTests extends CommandTestBase { assertFalse( m_scheduler.isScheduledOrRunning(child), "the sideload to schedule the child should not have run yet"); + assertFalse(sideloadRan.get(), "the sideload should not have run yet"); m_scheduler.run(); - assertTrue(m_scheduler.isRunning(child), "child should have started running"); + assertFalse(sideloadRan.get(), "the sideload should not have run"); + assertFalse(m_scheduler.isRunning(child), "child should not have run"); } @Test