wip: ughhh

api/src/opentrons/protocol_engine/commands/command_mapper.py

@@ -0,0 +1,5 @@
+"""Map command request and model types to other values."""
+
+
+class CommandMapper:
+    pass

api/src/opentrons/protocol_engine/execution/__init__.py

@@ -1,11 +1,13 @@
 """Command execution module."""
 
+from .command_executor import CommandExecutor
 from .command_handlers import CommandHandlers
 from .equipment import EquipmentHandler, LoadedLabware, LoadedPipette
 from .movement import MovementHandler
 from .pipetting import PipettingHandler
 
 __all__ = [
+    "CommandExecutor",
     "CommandHandlers",
     "EquipmentHandler",
     "LoadedLabware",

api/src/opentrons/protocol_engine/execution/command_executor.py

@@ -0,0 +1,79 @@
+"""Command side-effect execution logic container."""
+from __future__ import annotations
+
+from opentrons.hardware_control.api import API as HardwareAPI
+
+from ..resources import ResourceProviders
+from ..state import StateStore
+from ..commands import Command, CommandRequest
+from .equipment import EquipmentHandler
+from .movement import MovementHandler
+from .pipetting import PipettingHandler
+
+
+class CommandExecutor:
+    """CommandExecutor container class.
+
+    CommandExecutor manages various child handlers that define procedures to
+    execute the side-effects of commands.
+    """
+
+    def __init__(
+        self,
+        hardware: HardwareAPI,
+        state_store: StateStore,
+        resources: ResourceProviders,
+    ) -> None:
+        """Initialize the CommandExecutor with access to its dependencies."""
+        self._hardware = hardware
+        self._state_store = state_store
+        self._resources = resources
+
+    def create_command(self, request: CommandRequest) -> Command:
+        raise NotImplementedError("CommandExecutor not implemented")
+
+    def to_running(self, command: Command) -> Command:
+        raise NotImplementedError("CommandExecutor not implemented")
+
+    async def execute(self, command: Command) -> Command:
+        raise NotImplementedError("CommandExecutor not implemented")
+
+    # async def execute(self, command: Command) -> CommandResult:
+    #     state = self._state_store.state_view
+    #     hardware = self._hardware
+    #     resources = self._resources
+
+    #     equipment = EquipmentHandler(
+    #         state=state,
+    #         hardware=hardware,
+    #         resources=resources,
+    #     )
+
+    #     movement = MovementHandler(state=state, hardware=hardware)
+
+    #     pipetting = PipettingHandler(
+    #         state=state,
+    #         hardware=hardware,
+    #         movement_handler=movement,
+    #     )
+
+    #     command_impl = command.get_impl(
+    #         equipment=equipment,
+    #         movement=movement,
+    #         pipetting=pipetting,
+    #     )
+
+    # @property
+    # def equipment(self) -> EquipmentHandler:
+    #     """Access equipment handling procedures."""
+    #     return self._equipment
+
+    # @property
+    # def movement(self) -> MovementHandler:
+    #     """Access movement handling procedures."""
+    #     return self._movement
+
+    # @property
+    # def pipetting(self) -> PipettingHandler:
+    #     """Access pipetting handling procedures."""
+    #     return self._pipetting

api/src/opentrons/protocol_engine/execution/command_handlers.py

@@ -34,10 +34,7 @@ def __init__(
             resources=resources,
         )
 
-        movement = MovementHandler(
-            state=state,
-            hardware=hardware
-        )
+        movement = MovementHandler(state=state, hardware=hardware)
 
         pipetting = PipettingHandler(
             state=state,

api/src/opentrons/protocol_engine/protocol_engine.py

@@ -6,7 +6,7 @@
 from opentrons.util.helpers import utc_now
 
 from .errors import ProtocolEngineError, UnexpectedProtocolError
-from .execution import CommandHandlers
+from .execution import CommandExecutor
 from .resources import ResourceProviders
 from .state import State, StateStore, StateView
 from .commands import Command, CommandRequest, CommandStatus
@@ -22,6 +22,7 @@ class ProtocolEngine:
 
     _hardware: HardwareAPI
     _state_store: StateStore
+    _executor: CommandExecutor
     _resources: ResourceProviders
 
     @classmethod
@@ -35,15 +36,28 @@ async def create(cls, hardware: HardwareAPI) -> ProtocolEngine:
         fixed_labware = await resources.deck_data.get_deck_fixed_labware(deck_def)
 
         state_store = StateStore(
-            deck_definition=deck_def, deck_fixed_labware=fixed_labware
+            deck_definition=deck_def,
+            deck_fixed_labware=fixed_labware,
         )
 
-        return cls(state_store=state_store, resources=resources, hardware=hardware)
+        executor = CommandExecutor(
+            hardware=hardware,
+            state_store=state_store,
+            resources=resources,
+        )
+
+        return cls(
+            hardware=hardware,
+            state_store=state_store,
+            executor=executor,
+            resources=resources,
+        )
 
     def __init__(
         self,
         hardware: HardwareAPI,
         state_store: StateStore,
+        executor: CommandExecutor,
         resources: ResourceProviders,
     ) -> None:
         """Initialize a ProtocolEngine instance.
@@ -53,6 +67,7 @@ def __init__(
         """
         self._hardware = hardware
         self._state_store = state_store
+        self._executor = executor
         self._resources = resources
 
     @property
@@ -64,88 +79,78 @@ def get_state(self) -> State:
         """Get the engine's underlying state."""
         return self._state_store.get_state()
 
-    async def execute_command(
-        self,
-        request: CommandRequest,
-        command_id: Optional[str] = None,
-    ) -> Command:
-        """Execute a command request, waiting for it to complete."""
-        command = self.add_command(request, command_id)
-        result = await self.execute_command_by_id(command.id)
-
-        return result
+    # async def execute_command_by_id(self, command_id: str) -> Command:
+    #     """Execute a protocol engine command by its identifier."""
+    #     command = self.state_view.commands.get_command_by_id(command_id)
+    #     started_at = utc_now()
+    #     command = command.copy(
+    #         update={
+    #             "startedAt": started_at,
+    #             "status": CommandStatus.RUNNING,
+    #         }
+    #     )
+
+    #     self._state_store.handle_command(command)
+
+    #     # execute the command
+    #     try:
+    #         result = await self._executor.execute(command)
+    #         completed_at = utc_now()
+    #         command = command.copy(
+    #             update={
+    #                 "result": result,
+    #                 "completedAt": completed_at,
+    #                 "status": CommandStatus.EXECUTED,
+    #             }
+    #         )
+
+    #     except Exception as error:
+    #         completed_at = utc_now()
+    #         if not isinstance(error, ProtocolEngineError):
+    #             error = UnexpectedProtocolError(error)
+
+    #         command = command.copy(
+    #             update={
+    #                 # TODO(mc, 2021-06-21): return structured error details
+    #                 "error": str(error),
+    #                 "completedAt": completed_at,
+    #                 "status": CommandStatus.FAILED,
+    #             }
+    #         )
+
+    #     # store the done command
+    #     self._state_store.handle_command(command)
+
+    #     return command
+
+    def add_command(self, request: CommandRequest) -> Command:
+        """Add a command to ProtocolEngine."""
+        command = self._executor.create_command(request)
+        self._state_store.handle_command(command)
+        return command
 
     async def execute_command_by_id(self, command_id: str) -> Command:
         """Execute a protocol engine command by its identifier."""
-        command = self.state_view.commands.get_command_by_id(command_id)
-        command_impl = command.get_implementation()
-        started_at = utc_now()
-        command = command.copy(
-            update={
-                "startedAt": started_at,
-                "status": CommandStatus.RUNNING,
-            }
-        )
+        queued_command = self.state_view.commands.get_command_by_id(command_id)
+        running_command = self._executor.to_running(queued_command)
 
-        self._state_store.handle_command(command)
+        self._state_store.handle_command(running_command)
 
-        # TODO(mc, 2021-06-16): refactor command execution after command
-        # models have been simplified to delegate to CommandExecutor. This
-        # should involve ditching the relatively useless CommandHandler class
-        handlers = CommandHandlers(
-            hardware=self._hardware,
-            resources=self._resources,
-            state=self.state_view,
-        )
+        completed_command = await self._executor.execute(running_command)
 
-        # execute the command
-        try:
-            result = await command_impl.execute(handlers)
-            completed_at = utc_now()
-            command = command.copy(
-                update={
-                    "result": result,
-                    "completedAt": completed_at,
-                    "status": CommandStatus.EXECUTED,
-                }
-            )
-
-        except Exception as error:
-            completed_at = utc_now()
-            if not isinstance(error, ProtocolEngineError):
-                error = UnexpectedProtocolError(error)
-
-            command = command.copy(
-                update={
-                    # TODO(mc, 2021-06-21): return structured error details
-                    "error": str(error),
-                    "completedAt": completed_at,
-                    "status": CommandStatus.FAILED,
-                }
-            )
-
-        # store the done command
-        self._state_store.handle_command(command)
+        self._state_store.handle_command(completed_command)
 
-        return command
+        return completed_command
 
-    def add_command(
-        self,
-        request: CommandRequest,
-        command_id: Optional[str] = None,
-    ) -> Command:
-        """Add a command to ProtocolEngine."""
-        # TODO(mc, 2021-06-14): ID generation and timestamp generation need to
-        # be redone / reconsidered. Too much about command execution has leaked
-        # into the root ProtocolEngine class, so we should delegate downwards.
-        command_id = command_id or self._resources.id_generator.generate_id()
-        created_at = utc_now()
-        command_impl = request.get_implementation()
-        command = command_impl.create_command(
-            command_id=command_id,
-            created_at=created_at,
-        )
+    async def execute_command(self, request: CommandRequest) -> Command:
+        """Execute a command request, waiting for it to complete."""
+        queued_command = self._executor.create_command(request)
+        running_command = self._executor.to_running(queued_command)
 
-        self._state_store.handle_command(command)
+        self._state_store.handle_command(running_command)
 
-        return command
+        completed_command = await self._executor.execute(running_command)
+
+        self._state_store.handle_command(completed_command)
+
+        return completed_command

api/tests/opentrons/protocol_engine/clients/test_child_thread_transport.py

@@ -3,7 +3,7 @@
 import pytest
 from asyncio import AbstractEventLoop
 from datetime import datetime
-from decoy import Decoy, matchers
+from decoy import Decoy
 from functools import partial
 
 
@@ -12,12 +12,6 @@
 from opentrons.protocol_engine.clients.transports import ChildThreadTransport
 
 
-@pytest.fixture
-def decoy() -> Decoy:
-    """Create a Decoy state container for this test suite."""
-    return Decoy()
-
-
 @pytest.fixture
 async def engine(decoy: Decoy) -> ProtocolEngine:
     """Get a stubbed out ProtocolEngine."""

api/tests/opentrons/protocol_engine/commands/conftest.py

@@ -1,3 +1,4 @@
+"""Fixtures for protocol engine command tests."""
 import pytest
 from decoy import Decoy
 

api/tests/opentrons/protocol_engine/conftest.py

@@ -13,7 +13,6 @@
 from opentrons.hardware_control.api import API as HardwareController
 
 from opentrons.protocol_engine import (
-    ProtocolEngine,
     StateStore,
     StateView,
     CommandHandlers,
@@ -169,17 +168,3 @@ def store(standard_deck_def: DeckDefinitionV2) -> StateStore:
         deck_definition=standard_deck_def,
         deck_fixed_labware=[],
     )
-
-
-@pytest.fixture
-def engine(
-    mock_hardware: AsyncMock,
-    mock_state_store: MagicMock,
-    mock_resources: AsyncMock,
-) -> ProtocolEngine:
-    """Get a ProtocolEngine with its dependencies mocked out."""
-    return ProtocolEngine(
-        hardware=mock_hardware,
-        state_store=mock_state_store,
-        resources=mock_resources,
-    )