From 1737d6703a175cc592f17564db936ccfd3abcf73 Mon Sep 17 00:00:00 2001 From: Dustin Spicuzza Date: Fri, 13 Mar 2026 04:42:52 +0000 Subject: [PATCH 1/9] test: add failing tests for MagicBot command runner --- tests/test_magicbot_commands.py | 245 ++++++++++++++++++++++++++++++++ 1 file changed, 245 insertions(+) create mode 100644 tests/test_magicbot_commands.py diff --git a/tests/test_magicbot_commands.py b/tests/test_magicbot_commands.py new file mode 100644 index 0000000..d6c6b66 --- /dev/null +++ b/tests/test_magicbot_commands.py @@ -0,0 +1,245 @@ +from unittest.mock import Mock + +import commands2 +import magicbot + + +class RecordingCommand(commands2.Command): + def __init__(self, *, finished_after: int | None = None) -> None: + super().__init__() + self.finished_after = finished_after + self.initialize_calls = 0 + self.execute_calls = 0 + self.end_calls: list[bool] = [] + + def initialize(self): + self.initialize_calls += 1 + + def execute(self): + self.execute_calls += 1 + + def isFinished(self) -> bool: + return ( + self.finished_after is not None + and self.execute_calls >= self.finished_after + ) + + def end(self, interrupted: bool): + self.end_calls.append(interrupted) + + +class RaiseOnInitialize(commands2.Command): + def initialize(self): + raise RuntimeError("initialize boom") + + +class RaiseOnExecute(commands2.Command): + def execute(self): + raise RuntimeError("execute boom") + + +class RaiseOnEnd(commands2.Command): + def __init__(self): + super().__init__() + self.executed = False + + def execute(self): + self.executed = True + + def isFinished(self) -> bool: + return self.executed + + def end(self, interrupted: bool): + raise RuntimeError("end boom") + + +class RecordingRobot(magicbot.MagicRobot): + def createObjects(self): + pass + + def onException(self, forceReport: bool = False) -> None: + self.on_exception_calls += 1 + + +class UsesCommand: + cmd: "magicbot.commands.CommandRunner" + + def setup(self): + self.command = RecordingCommand() + self.should_run = False + + def execute(self): + if self.should_run: + self.cmd.run(self.command) + + +class AlsoUsesCommand: + cmd: "magicbot.commands.CommandRunner" + + def setup(self): + self.command = RecordingCommand() + self.should_run = False + + def execute(self): + if self.should_run: + self.cmd.run(self.command) + + +class ComponentRobot(RecordingRobot): + cmd: "magicbot.commands.CommandRunner" + first: UsesCommand + second: AlsoUsesCommand + + def createObjects(self): + self.on_exception_calls = 0 + self._automodes = Mock() + self._automodes.modes = {} + + +def make_runner(): + robot = RecordingRobot() + robot.on_exception_calls = 0 + runner = magicbot.commands.CommandRunner() + runner.robot = robot + return robot, runner + + +def make_component_robot() -> ComponentRobot: + robot = ComponentRobot() + robot.createObjects() + robot._create_components() + return robot + + +def test_command_runner_initializes_executes_and_finishes_once(): + _, runner = make_runner() + command = RecordingCommand(finished_after=2) + + runner.run(command) + runner.execute() + + assert command.initialize_calls == 1 + assert command.execute_calls == 1 + assert command.end_calls == [] + + runner.run(command) + runner.execute() + + assert command.initialize_calls == 1 + assert command.execute_calls == 2 + assert command.end_calls == [False] + + +def test_command_runner_keeps_alive_requested_command(): + _, runner = make_runner() + command = RecordingCommand() + + runner.run(command) + runner.execute() + runner.run(command) + runner.execute() + + assert command.initialize_calls == 1 + assert command.execute_calls == 2 + assert command.end_calls == [] + + +def test_command_runner_interrupts_command_not_requested_next_loop(): + _, runner = make_runner() + command = RecordingCommand() + + runner.run(command) + runner.execute() + runner.execute() + + assert command.initialize_calls == 1 + assert command.execute_calls == 1 + assert command.end_calls == [True] + + +def test_command_runner_deduplicates_same_command_requested_twice(): + _, runner = make_runner() + command = RecordingCommand() + + runner.run(command) + runner.run(command) + runner.execute() + + assert command.initialize_calls == 1 + assert command.execute_calls == 1 + assert command.end_calls == [] + + +def test_command_runner_executes_multiple_commands_concurrently(): + _, runner = make_runner() + first = RecordingCommand() + second = RecordingCommand() + + runner.run(first) + runner.run(second) + runner.execute() + + assert first.initialize_calls == 1 + assert first.execute_calls == 1 + assert second.initialize_calls == 1 + assert second.execute_calls == 1 + + +def test_command_runner_delegates_initialize_exception_to_robot(): + robot, runner = make_runner() + command = RaiseOnInitialize() + + runner.run(command) + runner.execute() + + assert robot.on_exception_calls == 1 + + +def test_command_runner_delegates_execute_exception_to_robot(): + robot, runner = make_runner() + command = RaiseOnExecute() + + runner.run(command) + runner.execute() + + assert robot.on_exception_calls == 1 + + +def test_command_runner_delegates_end_exception_to_robot(): + robot, runner = make_runner() + command = RaiseOnEnd() + + runner.run(command) + runner.execute() + + assert robot.on_exception_calls == 1 + + +def test_command_runner_works_as_magicbot_component(): + robot = make_component_robot() + + robot.first.should_run = True + robot.first.execute() + robot.cmd.execute() + + assert robot.first.command.initialize_calls == 1 + assert robot.first.command.execute_calls == 1 + + +def test_command_runner_supports_basic_command_group(): + _, runner = make_runner() + first = RecordingCommand(finished_after=1) + second = RecordingCommand(finished_after=1) + group = commands2.cmd.sequence(first, second) + + runner.run(group) + runner.execute() + runner.run(group) + runner.execute() + + assert first.initialize_calls == 1 + assert first.execute_calls == 1 + assert first.end_calls == [False] + assert second.initialize_calls == 1 + assert second.execute_calls == 1 + assert second.end_calls == [False] From a179e959bb8e022d9758269b15e5da7559dfd6e6 Mon Sep 17 00:00:00 2001 From: Dustin Spicuzza Date: Fri, 13 Mar 2026 04:43:24 +0000 Subject: [PATCH 2/9] feat: add MagicBot command runner skeleton --- magicbot/__init__.py | 4 ++++ magicbot/commands.py | 11 +++++++++++ pyproject.toml | 1 + 3 files changed, 16 insertions(+) create mode 100644 magicbot/commands.py diff --git a/magicbot/__init__.py b/magicbot/__init__.py index d485942..2479dce 100644 --- a/magicbot/__init__.py +++ b/magicbot/__init__.py @@ -1,3 +1,5 @@ +from . import commands +from .commands import CommandRunner from .magicrobot import MagicRobot from .magic_tunable import feedback, tunable from .magic_reset import will_reset_to @@ -11,6 +13,8 @@ ) __all__ = ( + "commands", + "CommandRunner", "MagicRobot", "feedback", "tunable", diff --git a/magicbot/commands.py b/magicbot/commands.py new file mode 100644 index 0000000..e9c99ad --- /dev/null +++ b/magicbot/commands.py @@ -0,0 +1,11 @@ +from __future__ import annotations + +import commands2 + + +class CommandRunner: + def run(self, command: commands2.Command) -> None: + raise NotImplementedError + + def execute(self) -> None: + raise NotImplementedError diff --git a/pyproject.toml b/pyproject.toml index 2ef1836..cfd0267 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -20,6 +20,7 @@ classifiers = [ dependencies = [ "toposort", "wpilib>=2026.1.1,<2027", + "robotpy-commands-v2>=2026.1.1,<2027", ] [[project.authors]] From 483329fe127cb2d05067ab7f36835deab5cc1884 Mon Sep 17 00:00:00 2001 From: Dustin Spicuzza Date: Fri, 13 Mar 2026 04:43:58 +0000 Subject: [PATCH 3/9] feat: implement keep-alive MagicBot command lifecycle --- magicbot/commands.py | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/magicbot/commands.py b/magicbot/commands.py index e9c99ad..b3a5528 100644 --- a/magicbot/commands.py +++ b/magicbot/commands.py @@ -4,8 +4,40 @@ class CommandRunner: + def __init__(self) -> None: + self._requested: set[commands2.Command] = set() + self._active: set[commands2.Command] = set() + def run(self, command: commands2.Command) -> None: - raise NotImplementedError + self._requested.add(command) def execute(self) -> None: - raise NotImplementedError + stale = [ + command + for command in self._active + if command not in self._requested + and command.getInterruptionBehavior() + != commands2.Command.InterruptionBehavior.kCancelIncoming + ] + for command in stale: + command.end(True) + self._active.remove(command) + + new_commands = [ + command for command in self._requested if command not in self._active + ] + for command in new_commands: + command.initialize() + self._active.add(command) + + finished: list[commands2.Command] = [] + for command in list(self._active): + command.execute() + if command.isFinished(): + command.end(False) + finished.append(command) + + for command in finished: + self._active.remove(command) + + self._requested.clear() From 6fa43a78474bff411c15dd422dd5bac96136f399 Mon Sep 17 00:00:00 2001 From: Dustin Spicuzza Date: Fri, 13 Mar 2026 04:44:21 +0000 Subject: [PATCH 4/9] feat: route command runner exceptions through MagicRobot --- magicbot/commands.py | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/magicbot/commands.py b/magicbot/commands.py index b3a5528..66b6cb1 100644 --- a/magicbot/commands.py +++ b/magicbot/commands.py @@ -1,9 +1,13 @@ from __future__ import annotations +from typing import Callable + import commands2 class CommandRunner: + robot: object + def __init__(self) -> None: self._requested: set[commands2.Command] = set() self._active: set[commands2.Command] = set() @@ -11,6 +15,19 @@ def __init__(self) -> None: def run(self, command: commands2.Command) -> None: self._requested.add(command) + def _discard(self, command: commands2.Command) -> None: + self._requested.discard(command) + self._active.discard(command) + + def _call(self, command: commands2.Command, func: Callable[[], object]) -> bool: + try: + func() + except Exception: + self._discard(command) + self.robot.onException() + return False + return True + def execute(self) -> None: stale = [ command @@ -20,22 +37,23 @@ def execute(self) -> None: != commands2.Command.InterruptionBehavior.kCancelIncoming ] for command in stale: - command.end(True) - self._active.remove(command) + if self._call(command, lambda command=command: command.end(True)): + self._active.remove(command) new_commands = [ command for command in self._requested if command not in self._active ] for command in new_commands: - command.initialize() - self._active.add(command) + if self._call(command, command.initialize): + self._active.add(command) finished: list[commands2.Command] = [] for command in list(self._active): - command.execute() + if not self._call(command, command.execute): + continue if command.isFinished(): - command.end(False) - finished.append(command) + if self._call(command, lambda command=command: command.end(False)): + finished.append(command) for command in finished: self._active.remove(command) From ce5b00e6ed253e9046830e1f3791c2a6cc95b1da Mon Sep 17 00:00:00 2001 From: Dustin Spicuzza Date: Fri, 13 Mar 2026 04:44:48 +0000 Subject: [PATCH 5/9] test: cover MagicBot component integration for command runner --- magicbot/magicrobot.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/magicbot/magicrobot.py b/magicbot/magicrobot.py index 22df302..e28d7e8 100644 --- a/magicbot/magicrobot.py +++ b/magicbot/magicrobot.py @@ -692,7 +692,7 @@ def _create_components(self) -> None: self._components = components def _collect_injectables(self) -> dict[str, Any]: - injectables = {} + injectables = {"robot": self} cls = type(self) for n in dir(self): From b67bd9b5e2e68fe8f9160cf8bca3a3a83908be25 Mon Sep 17 00:00:00 2001 From: Dustin Spicuzza Date: Fri, 13 Mar 2026 04:45:42 +0000 Subject: [PATCH 6/9] docs: document MagicBot command runner --- docs/magicbot.rst | 36 ++++++++++++++++++++++++++++++++++++ magicbot/commands.py | 20 ++++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/docs/magicbot.rst b/docs/magicbot.rst index 6806f06..868b931 100644 --- a/docs/magicbot.rst +++ b/docs/magicbot.rst @@ -51,3 +51,39 @@ State machines :exclude-members: members :undoc-members: :show-inheritance: + +Commands +~~~~~~~~ + +.. automodule:: magicbot.commands + :members: + :undoc-members: + :show-inheritance: + +``magicbot.commands.CommandRunner`` is a MagicBot component that executes +stored ``commands2.Command`` instances using keep-alive semantics. + +Call ``self.cmd.run(command)`` each loop that a stored command instance should +remain active. If a command is not re-requested on a later loop, it is ended as +interrupted when it is interruptible. + +Important usage constraints: + +- construct command instances once and store them on the robot/component +- do not create a new command inside ``execute()`` or a state method each loop +- components, state machines, and robot code may all call ``run(command)`` +- multiple different commands may run concurrently +- this is not ``commands2.CommandScheduler`` and does not perform full + scheduler-style requirement arbitration + +Example:: + + class Shooter: + cmd: magicbot.commands.CommandRunner + + def setup(self): + self.fire = FireCommand(...) + + def execute(self): + if should_fire(): + self.cmd.run(self.fire) diff --git a/magicbot/commands.py b/magicbot/commands.py index 66b6cb1..6a84670 100644 --- a/magicbot/commands.py +++ b/magicbot/commands.py @@ -6,6 +6,22 @@ class CommandRunner: + """ + A MagicBot component that executes ``commands2.Command`` instances using + keep-alive semantics. + + Call :meth:`run` each loop that a stored command instance should remain + active. Commands that are not re-requested on a later loop are interrupted + automatically if they are interruptible. + + Multiple different commands may be active concurrently. Repeated + :meth:`run` calls for the same command instance in a single loop are + deduplicated. + + This component intentionally does not expose or depend on + ``commands2.CommandScheduler``. + """ + robot: object def __init__(self) -> None: @@ -13,6 +29,10 @@ def __init__(self) -> None: self._active: set[commands2.Command] = set() def run(self, command: commands2.Command) -> None: + """ + Request that a stored command instance remain active for the current + robot loop. + """ self._requested.add(command) def _discard(self, command: commands2.Command) -> None: From 3306ec87902202754e4d62b823724f39c656afcb Mon Sep 17 00:00:00 2001 From: Dustin Spicuzza Date: Fri, 13 Mar 2026 05:06:37 +0000 Subject: [PATCH 7/9] Preserve command runner request order --- magicbot/commands.py | 10 ++++--- tests/test_magicbot_commands.py | 47 +++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/magicbot/commands.py b/magicbot/commands.py index 6a84670..8b29b6c 100644 --- a/magicbot/commands.py +++ b/magicbot/commands.py @@ -25,7 +25,7 @@ class CommandRunner: robot: object def __init__(self) -> None: - self._requested: set[commands2.Command] = set() + self._requested: dict[commands2.Command, None] = {} self._active: set[commands2.Command] = set() def run(self, command: commands2.Command) -> None: @@ -33,10 +33,10 @@ def run(self, command: commands2.Command) -> None: Request that a stored command instance remain active for the current robot loop. """ - self._requested.add(command) + self._requested.setdefault(command, None) def _discard(self, command: commands2.Command) -> None: - self._requested.discard(command) + self._requested.pop(command, None) self._active.discard(command) def _call(self, command: commands2.Command, func: Callable[[], object]) -> bool: @@ -68,7 +68,9 @@ def execute(self) -> None: self._active.add(command) finished: list[commands2.Command] = [] - for command in list(self._active): + for command in list(self._requested): + if command not in self._active: + continue if not self._call(command, command.execute): continue if command.isFinished(): diff --git a/tests/test_magicbot_commands.py b/tests/test_magicbot_commands.py index d6c6b66..fc334c4 100644 --- a/tests/test_magicbot_commands.py +++ b/tests/test_magicbot_commands.py @@ -28,6 +28,27 @@ def end(self, interrupted: bool): self.end_calls.append(interrupted) +class OrderedRecordingCommand(RecordingCommand): + def __init__( + self, name: str, events: list[tuple[str, str]], *, hash_value: int + ) -> None: + super().__init__() + self.name = name + self.events = events + self.hash_value = hash_value + + def __hash__(self) -> int: + return self.hash_value + + def initialize(self): + super().initialize() + self.events.append(("initialize", self.name)) + + def execute(self): + super().execute() + self.events.append(("execute", self.name)) + + class RaiseOnInitialize(commands2.Command): def initialize(self): raise RuntimeError("initialize boom") @@ -185,6 +206,32 @@ def test_command_runner_executes_multiple_commands_concurrently(): assert second.execute_calls == 1 +def test_command_runner_executes_commands_in_run_call_order(): + _, runner = make_runner() + events: list[tuple[str, str]] = [] + first = OrderedRecordingCommand("first", events, hash_value=1) + second = OrderedRecordingCommand("second", events, hash_value=8) + + runner.run(first) + runner.run(second) + runner.execute() + + assert events == [ + ("initialize", "first"), + ("initialize", "second"), + ("execute", "first"), + ("execute", "second"), + ] + + events.clear() + + runner.run(second) + runner.run(first) + runner.execute() + + assert events == [("execute", "second"), ("execute", "first")] + + def test_command_runner_delegates_initialize_exception_to_robot(): robot, runner = make_runner() command = RaiseOnInitialize() From b2030fcfd760b4dd6c2aeef8c3f476f019b7eeed Mon Sep 17 00:00:00 2001 From: Dustin Spicuzza Date: Fri, 13 Mar 2026 01:20:45 -0400 Subject: [PATCH 8/9] Remove commands import from __init__.py --- magicbot/__init__.py | 4 ---- tests/test_magicbot_commands.py | 1 + 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/magicbot/__init__.py b/magicbot/__init__.py index 2479dce..d485942 100644 --- a/magicbot/__init__.py +++ b/magicbot/__init__.py @@ -1,5 +1,3 @@ -from . import commands -from .commands import CommandRunner from .magicrobot import MagicRobot from .magic_tunable import feedback, tunable from .magic_reset import will_reset_to @@ -13,8 +11,6 @@ ) __all__ = ( - "commands", - "CommandRunner", "MagicRobot", "feedback", "tunable", diff --git a/tests/test_magicbot_commands.py b/tests/test_magicbot_commands.py index fc334c4..738b733 100644 --- a/tests/test_magicbot_commands.py +++ b/tests/test_magicbot_commands.py @@ -2,6 +2,7 @@ import commands2 import magicbot +import magicbot.commands class RecordingCommand(commands2.Command): From 62b7cd8324c7111a17e347321448016932f20c5d Mon Sep 17 00:00:00 2001 From: Dustin Spicuzza Date: Fri, 13 Mar 2026 01:36:10 -0400 Subject: [PATCH 9/9] Don't stop commands until they're finished --- magicbot/commands.py | 38 +++++++++++++-------------------- tests/test_magicbot_commands.py | 8 +++---- 2 files changed, 19 insertions(+), 27 deletions(-) diff --git a/magicbot/commands.py b/magicbot/commands.py index 8b29b6c..6608c56 100644 --- a/magicbot/commands.py +++ b/magicbot/commands.py @@ -10,9 +10,8 @@ class CommandRunner: A MagicBot component that executes ``commands2.Command`` instances using keep-alive semantics. - Call :meth:`run` each loop that a stored command instance should remain - active. Commands that are not re-requested on a later loop are interrupted - automatically if they are interruptible. + Call :meth:`run` to start a stored command instance. Once started, a + command continues running on each later loop until it reports finished. Multiple different commands may be active concurrently. Repeated :meth:`run` calls for the same command instance in a single loop are @@ -26,18 +25,17 @@ class CommandRunner: def __init__(self) -> None: self._requested: dict[commands2.Command, None] = {} - self._active: set[commands2.Command] = set() + self._active: dict[commands2.Command, None] = {} def run(self, command: commands2.Command) -> None: """ - Request that a stored command instance remain active for the current - robot loop. + Request that a stored command instance start running. """ self._requested.setdefault(command, None) def _discard(self, command: commands2.Command) -> None: self._requested.pop(command, None) - self._active.discard(command) + self._active.pop(command, None) def _call(self, command: commands2.Command, func: Callable[[], object]) -> bool: try: @@ -49,28 +47,22 @@ def _call(self, command: commands2.Command, func: Callable[[], object]) -> bool: return True def execute(self) -> None: - stale = [ - command - for command in self._active - if command not in self._requested - and command.getInterruptionBehavior() - != commands2.Command.InterruptionBehavior.kCancelIncoming - ] - for command in stale: - if self._call(command, lambda command=command: command.end(True)): - self._active.remove(command) - new_commands = [ command for command in self._requested if command not in self._active ] for command in new_commands: if self._call(command, command.initialize): - self._active.add(command) + self._active[command] = None + + commands_to_execute = [ + command for command in self._requested if command in self._active + ] + commands_to_execute.extend( + command for command in self._active if command not in self._requested + ) finished: list[commands2.Command] = [] - for command in list(self._requested): - if command not in self._active: - continue + for command in commands_to_execute: if not self._call(command, command.execute): continue if command.isFinished(): @@ -78,6 +70,6 @@ def execute(self) -> None: finished.append(command) for command in finished: - self._active.remove(command) + self._active.pop(command, None) self._requested.clear() diff --git a/tests/test_magicbot_commands.py b/tests/test_magicbot_commands.py index 738b733..ce669af 100644 --- a/tests/test_magicbot_commands.py +++ b/tests/test_magicbot_commands.py @@ -166,17 +166,17 @@ def test_command_runner_keeps_alive_requested_command(): assert command.end_calls == [] -def test_command_runner_interrupts_command_not_requested_next_loop(): +def test_command_runner_keeps_running_until_finished_without_run_call(): _, runner = make_runner() - command = RecordingCommand() + command = RecordingCommand(finished_after=2) runner.run(command) runner.execute() runner.execute() assert command.initialize_calls == 1 - assert command.execute_calls == 1 - assert command.end_calls == [True] + assert command.execute_calls == 2 + assert command.end_calls == [False] def test_command_runner_deduplicates_same_command_requested_twice():