removed add_command changes

Opentrons · May 15, 2024 · 9e4a3e8 · 9e4a3e8
1 parent 072c4e9
commit 9e4a3e8
Show file tree

Hide file tree

Showing 4 changed files with 5 additions and 280 deletions.
diff --git a/api/src/opentrons/protocol_runner/protocol_runner.py b/api/src/opentrons/protocol_runner/protocol_runner.py
@@ -20,8 +20,6 @@
     StateSummary,
     Command,
     commands as pe_commands,
-    CommandIntent,
-    CommandCreate,
 )
 from opentrons.protocols.parse import PythonParseMode
 from opentrons.util.broker import Broker
@@ -38,7 +36,6 @@
     LegacyExecutor,
     LegacyLoadInfo,
 )
-from ..ordered_set import OrderedSet
 from ..protocol_engine.errors import ProtocolCommandFailedError
 from ..protocol_engine.types import (
     PostRunHardwareState,
@@ -68,21 +65,9 @@ class AbstractRunner(ABC):
     you will need a new Runner to do another run.
     """
 
-    _queued_protocol_commands: List[CommandCreate]
-    """The IDs of queued commands, in FIFO order"""
-
-    _queued_setup_commands: List[CommandCreate]
-    """The IDs of queued setup commands, in FIFO order"""
-
-    _queued_fixit_commands: List[CommandCreate]
-    """The IDs of queued fixit commands, in FIFO order"""
-
     def __init__(self, protocol_engine: ProtocolEngine) -> None:
         self._protocol_engine = protocol_engine
         self._broker = LegacyBroker()
-        self._queued_protocol_commands = []
-        self._queued_setup_commands = []
-        self._queued_fixit_commands = []
 
     # TODO(mm, 2023-10-03): `LegacyBroker` is specific to Python protocols and JSON protocols ≤v5.
     # We'll need to extend this in order to report progress from newer JSON protocols.
@@ -142,10 +127,6 @@ async def run(
     ) -> RunResult:
         """Run a given protocol to completion."""
 
-    @abstractmethod
-    def set_command_queued(self, command: CommandCreate) -> CommandCreate:
-        """add command to queue."""
-
 
 class PythonAndLegacyRunner(AbstractRunner):
     """Protocol runner implementation for Python protocols, and JSON protocols ≤v5."""
@@ -187,10 +168,6 @@ def run_time_parameters(self) -> List[RunTimeParameter]:
             return self._parameter_context.export_parameters_for_analysis()
         return []
 
-    def set_command_queued(self, command: CommandCreate) -> CommandCreate:
-        """add command to queue."""
-        return command
-
     async def load(
         self,
         protocol_source: ProtocolSource,
@@ -307,6 +284,7 @@ def __init__(
         )
 
         self._hardware_api.should_taskify_movement_execution(taskify=False)
+        self._queued_commands: List[pe_commands.CommandCreate] = []
 
     async def load(self, protocol_source: ProtocolSource) -> None:
         """Load a JSONv6+ ProtocolSource into managed ProtocolEngine."""
@@ -353,10 +331,9 @@ async def load(self, protocol_source: ProtocolSource) -> None:
             params=pe_commands.HomeParams(axes=None)
         )
         # this command homes all axes, including pipette plugner and gripper jaw
-        self.set_command_queued(initial_home_command)
+        self._protocol_engine.add_command(request=initial_home_command)
 
-        for command in commands:
-            self.set_command_queued(command)
+        self._queued_commands = commands
 
         self._task_queue.set_run_func(func=self._add_command_and_execute)
 
@@ -380,25 +357,14 @@ async def run(  # noqa: D102
         return RunResult(commands=commands, state_summary=run_data, parameters=[])
 
     async def _add_command_and_execute(self) -> None:
-        for command in self._queued_protocol_commands:
-            # TODO(Tamar): should_add_execute_command change to only wait_for_command?
+        for command in self._queued_commands:
             result = await self._protocol_engine.add_and_execute_command(command)
             if result and result.error:
                 raise ProtocolCommandFailedError(
                     original_error=result.error,
                     message=f"{result.error.errorType}: {result.error.detail}",
                 )
 
-    def set_command_queued(self, command: CommandCreate) -> CommandCreate:
-        """add command to queue."""
-        self._add_to_queue(command)
-        return command
-
-    def _add_to_queue(self, command: CommandCreate) -> CommandCreate:
-        """Add new ID to the queued."""
-        self._queued_protocol_commands.append(command)
-        return command
-
 
 class LiveRunner(AbstractRunner):
     """Protocol runner implementation for live http protocols."""
@@ -440,32 +406,15 @@ async def run(  # noqa: D102
         commands = self._protocol_engine.state_view.commands.get_all()
         return RunResult(commands=commands, state_summary=run_data, parameters=[])
 
-    def set_command_queued(self, command: CommandCreate) -> CommandCreate:
-        """add command to queue."""
-        if command.intent == CommandIntent.SETUP:
-            self._add_to_setup_queue(command)
-        elif command.intent == CommandIntent.FIXIT:
-            self._add_to_fixit_queue(command)
-
-        return command
-
-    def _add_to_setup_queue(self, command: CommandCreate) -> None:
-        """Add a new ID to the queued setup."""
-        self._queued_setup_commands.append(command)
-
-    def _add_to_fixit_queue(self, command: CommandCreate) -> None:
-        """Add a new ID to the queued fixit."""
-        self._queued_fixit_commands.append(command)
-
 
 AnyRunner = Union[PythonAndLegacyRunner, JsonRunner, LiveRunner]
 
 
 def create_protocol_runner(
+    protocol_config: Optional[Union[JsonProtocolConfig, PythonProtocolConfig]],
     protocol_engine: ProtocolEngine,
     hardware_api: HardwareControlAPI,
     task_queue: Optional[TaskQueue] = None,
-    protocol_config: Optional[Union[JsonProtocolConfig, PythonProtocolConfig]] = None,
     json_file_reader: Optional[JsonFileReader] = None,
     json_translator: Optional[JsonTranslator] = None,
     legacy_file_reader: Optional[LegacyFileReader] = None,

diff --git a/api/src/opentrons/protocol_runner/run_orchestrator.py b/api/src/opentrons/protocol_runner/run_orchestrator.py
@@ -5,16 +5,13 @@
 from ..hardware_control import HardwareControlAPI
 from ..protocol_engine import (
     ProtocolEngine,
-    Command,
     CommandCreate,
     CommandIntent,
 )
 from ..protocol_engine.errors import CommandNotAllowedError
 from ..protocol_engine.types import PostRunHardwareState, RunTimeParamValuesType
 from ..protocol_reader import JsonProtocolConfig, PythonProtocolConfig
 
-from opentrons.protocols.parse import PythonParseMode
-
 
 class RunOrchestrator:
     _json_or_python_runner: Optional[
@@ -83,43 +80,6 @@ def build_orchestrator(
             protocol_engine=protocol_engine,
         )
 
-    def add_command(
-        self, request: CommandCreate, failed_command_id: Optional[str] = None
-    ) -> CommandCreate:
-        """Add a command to the queue.
-
-        Arguments:
-            request: The command type and payload data used to construct
-                the command in state.
-            failed_command_id: the failed command id this command is trying to fix.
-
-        Returns:
-            The full, newly queued command.
-
-        Raises:
-            SetupCommandNotAllowed: the request specified a setup command,
-                but the engine was not idle or paused.
-            RunStoppedError: the run has been stopped, so no new commands
-                may be added.
-            CommandNotAllowedError: the request specified a failed command id
-                with a non fixit command.
-        """
-        if failed_command_id and request.intent != CommandIntent.FIXIT:
-            raise CommandNotAllowedError(
-                "failed command id should be supplied with a FIXIT command."
-            )
-
-        # pass the failed_command_id somewhere
-        if request.intent == CommandIntent.SETUP:
-            return self._setup_runner.set_command_queued(request)
-        elif request.intent == CommandIntent.FIXIT:
-            return self._fixit_runner.set_command_queued(request)
-        elif (
-            request.intent == CommandIntent.PROTOCOL
-            and self._json_or_python_runner is not None
-        ):
-            return self._json_or_python_runner.set_command_queued(request)
-
     def get_protocol_runner(self) -> protocol_runner.AnyRunner:
         return self._json_or_python_runner or self._setup_runner
 

diff --git a/api/tests/opentrons/protocol_runner/test_run_orchestrator.py b/api/tests/opentrons/protocol_runner/test_run_orchestrator.py
diff --git a/api/tests/opentrons/protocol_runner/test_run_orchestrator_provider.py b/api/tests/opentrons/protocol_runner/test_run_orchestrator_provider.py
@@ -161,58 +161,3 @@ def test_build_run_orchestrator_provider(
     )
 
     assert isinstance(result, RunOrchestrator)
-
-
-@pytest.mark.parametrize(
-    "runner, command_intent, subject",
-    [
-        (
-            lazy_fixture("mock_setup_runner"),
-            pe_commands.CommandIntent.SETUP,
-            lazy_fixture("python_protocol_subject"),
-        ),
-        (
-            lazy_fixture("mock_fixit_runner"),
-            pe_commands.CommandIntent.FIXIT,
-            lazy_fixture("python_protocol_subject"),
-        ),
-        (
-            lazy_fixture("mock_setup_runner"),
-            pe_commands.CommandIntent.SETUP,
-            lazy_fixture("json_protocol_subject"),
-        ),
-        (
-            lazy_fixture("mock_fixit_runner"),
-            pe_commands.CommandIntent.FIXIT,
-            lazy_fixture("json_protocol_subject"),
-        ),
-        (
-            lazy_fixture("mock_protocol_json_runner"),
-            pe_commands.CommandIntent.PROTOCOL,
-            lazy_fixture("json_protocol_subject"),
-        ),
-        (
-            lazy_fixture("mock_protocol_python_runner"),
-            pe_commands.CommandIntent.PROTOCOL,
-            lazy_fixture("python_protocol_subject"),
-        ),
-    ],
-)
-def test_add_command(
-    subject: RunOrchestrator,
-    decoy: Decoy,
-    runner: AnyRunner,
-    command_intent: pe_commands.CommandIntent,
-) -> None:
-    """Should verify calls to set_command_queued."""
-    command_to_queue = pe_commands.HomeCreate.construct(
-        intent=command_intent, params=pe_commands.HomeParams.construct()
-    )
-    decoy.when(runner.set_command_queued(command_to_queue)).then_return(
-        command_to_queue
-    )
-    result = subject.add_command(command_to_queue)
-
-    assert result == command_to_queue
-
-    decoy.verify(runner.set_command_queued(command_to_queue))