refactor(robot-server,api): wire up JSON protocol runner to /sessions

api/src/opentrons/protocol_engine/protocol_engine.py

@@ -11,6 +11,7 @@
 from .state import StateStore, StateView
 from .commands import (
     CommandRequestType,
+    PendingCommandType,
     CompletedCommandType,
     FailedCommandType,
 )
@@ -26,6 +27,7 @@ class ProtocolEngine:
 
     state_store: StateStore
     _handlers: CommandHandlers
+    _resources: ResourceProviders
 
     @classmethod
     async def create(cls, hardware: HardwareAPI) -> ProtocolEngine:
@@ -38,8 +40,7 @@ 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
         )
 
         handlers = CommandHandlers.create(
@@ -48,12 +49,13 @@ async def create(cls, hardware: HardwareAPI) -> ProtocolEngine:
             state=StateView.create_view(state_store),
         )
 
-        return cls(state_store=state_store, handlers=handlers)
+        return cls(state_store=state_store, handlers=handlers, resources=resources)
 
     def __init__(
         self,
         state_store: StateStore,
         handlers: CommandHandlers,
+        resources: ResourceProviders,
     ) -> None:
         """Initialize a ProtocolEngine instance.
 
@@ -62,6 +64,7 @@ def __init__(
         """
         self.state_store = state_store
         self._handlers = handlers
+        self._resources = resources
 
     async def execute_command(
         self,
@@ -94,8 +97,16 @@ async def execute_command(
 
         return done_cmd
 
-    def add_command(self, request: CommandRequestType) -> None:
+    def add_command(self, request: CommandRequestType) -> PendingCommandType:
         """Add a command to ProtocolEngine."""
-        # TODO(spp, 2020-05-13):
-        #   Generate a UUID to be used as command_id for each command added.
-        raise NotImplementedError
+        # 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 = self._resources.id_generator.generate_id()
+        created_at = utc_now()
+        command_impl = request.get_implementation()
+        command = command_impl.create_command(created_at)
+
+        self.state_store.handle_command(command, command_id=command_id)
+
+        return command

api/tests/opentrons/file_runner/test_python_file_runner.py

@@ -10,19 +10,19 @@ def subject() -> PythonFileRunner:
     return PythonFileRunner()
 
 
-@pytest.mark.xfail(raises=NotImplementedError)
+@pytest.mark.xfail(raises=NotImplementedError, strict=True)
 def test_python_runner_play(subject: PythonFileRunner) -> None:
     """It should be able to start the run."""
     subject.play()
 
 
-@pytest.mark.xfail(raises=NotImplementedError)
+@pytest.mark.xfail(raises=NotImplementedError, strict=True)
 def test_python_runner_pause(subject: PythonFileRunner) -> None:
     """It should be able to pause the run."""
     subject.pause()
 
 
-@pytest.mark.xfail(raises=NotImplementedError)
+@pytest.mark.xfail(raises=NotImplementedError, strict=True)
 def test_python_runner_stop(subject: PythonFileRunner) -> None:
     """It should be able to stop the run."""
     subject.stop()

api/tests/opentrons/protocol_api_experimental/test_protocol_context.py

@@ -114,7 +114,7 @@ def test_load_pipette_with_bad_args(
         subject.load_pipette(pipette_name="p300_single", mount="west")
 
 
-@pytest.mark.xfail(raises=NotImplementedError)
+@pytest.mark.xfail(raises=NotImplementedError, strict=True)
 def test_load_pipette_with_tipracks_list(subject: ProtocolContext) -> None:
     """TODO: it should do something with the `tip_racks` parameter to load_pipette."""
     subject.load_pipette(
@@ -124,7 +124,7 @@ def test_load_pipette_with_tipracks_list(subject: ProtocolContext) -> None:
     )
 
 
-@pytest.mark.xfail(raises=NotImplementedError)
+@pytest.mark.xfail(raises=NotImplementedError, strict=True)
 def test_load_pipette_with_replace(subject: ProtocolContext) -> None:
     """TODO: it should do something with the `replace` parameter to load_pipette."""
     subject.load_pipette(pipette_name="p300_single", mount="left", replace=True)
@@ -189,7 +189,7 @@ def test_load_labware_default_namespace_and_version(
     assert result == Labware(labware_id="abc123", engine_client=engine_client)
 
 
-@pytest.mark.xfail(raises=NotImplementedError)
+@pytest.mark.xfail(raises=NotImplementedError, strict=True)
 def test_load_labware_with_label(subject: ProtocolContext) -> None:
     """TODO: it should do something with the `label` parameter to load_labware."""
     subject.load_labware(load_name="some_labware", location=5, label="some_label")

api/tests/opentrons/protocol_engine/conftest.py

@@ -152,17 +152,13 @@ def well_plate_def() -> LabwareDefinition:
 @pytest.fixture(scope="session")
 def reservoir_def() -> LabwareDefinition:
     """Get the definition of single-row reservoir."""
-    return LabwareDefinition.parse_obj(
-        load_definition("nest_12_reservoir_15ml", 1)
-    )
+    return LabwareDefinition.parse_obj(load_definition("nest_12_reservoir_15ml", 1))
 
 
 @pytest.fixture(scope="session")
 def tip_rack_def() -> LabwareDefinition:
     """Get the definition of Opentrons 300 uL tip rack."""
-    return LabwareDefinition.parse_obj(
-        load_definition("opentrons_96_tiprack_300ul", 1)
-    )
+    return LabwareDefinition.parse_obj(load_definition("opentrons_96_tiprack_300ul", 1))
 
 
 @pytest.fixture
@@ -177,10 +173,12 @@ def store(standard_deck_def: DeckDefinitionV2) -> StateStore:
 @pytest.fixture
 def engine(
     mock_state_store: MagicMock,
-    mock_handlers: AsyncMock
+    mock_handlers: AsyncMock,
+    mock_resources: AsyncMock,
 ) -> ProtocolEngine:
     """Get a ProtocolEngine with its dependencies mocked out."""
     return ProtocolEngine(
         state_store=mock_state_store,
         handlers=mock_handlers,
+        resources=mock_resources,
     )

api/tests/opentrons/protocol_engine/test_protocol_engine.py

@@ -33,9 +33,8 @@ def __init__(self) -> None:
 
     def __eq__(self, other: object) -> bool:
         """Check if a target object is a datetime that is close to now."""
-        return (
-            isinstance(other, datetime) and
-            isclose(self._now.timestamp(), other.timestamp(), rel_tol=5)
+        return isinstance(other, datetime) and isclose(
+            self._now.timestamp(), other.timestamp(), rel_tol=5
         )
 
     def __repr__(self) -> str:
@@ -66,7 +65,7 @@ async def test_create_engine_initializes_state_with_deck_geometry(
 
 async def test_execute_command_creates_command(
     engine: ProtocolEngine,
-    mock_state_store: MagicMock
+    mock_state_store: MagicMock,
 ) -> None:
     """It should create a command in the state store when executing."""
     req = MoveToWellRequest(pipetteId="123", labwareId="abc", wellName="A1")
@@ -76,9 +75,9 @@ async def test_execute_command_creates_command(
         RunningCommand(
             created_at=cast(datetime, CloseToNow()),
             started_at=cast(datetime, CloseToNow()),
-            request=req
+            request=req,
         ),
-        command_id="unique-id"
+        command_id="unique-id",
     )
 
 
@@ -111,7 +110,7 @@ async def test_execute_command_adds_result_to_state(
     mock_req.get_implementation.return_value = mock_impl
     mock_impl.create_command.return_value = PendingCommand(
         request=mock_req,
-        created_at=now
+        created_at=now,
     )
     mock_impl.execute.return_value = result
 
@@ -145,7 +144,7 @@ async def test_execute_command_adds_error_to_state(
     mock_req.get_implementation.return_value = mock_impl
     mock_impl.create_command.return_value = PendingCommand(
         request=mock_req,
-        created_at=now
+        created_at=now,
     )
     mock_impl.execute.side_effect = error
 
@@ -179,7 +178,7 @@ async def test_execute_command_adds_unexpected_error_to_state(
     mock_req.get_implementation.return_value = mock_impl
     mock_impl.create_command.return_value = PendingCommand(
         request=mock_req,
-        created_at=now
+        created_at=now,
     )
     mock_impl.execute.side_effect = error
 
@@ -192,3 +191,34 @@ async def test_execute_command_adds_unexpected_error_to_state(
         cmd,
         command_id="unique-id",
     )
+
+
+def test_add_command(
+    engine: ProtocolEngine,
+    mock_handlers: AsyncMock,
+    mock_state_store: MagicMock,
+    mock_resources: AsyncMock,
+    now: datetime,
+) -> None:
+    """It should add a pending command into state."""
+    mock_resources.id_generator.generate_id.return_value = "command-id"
+
+    mock_req = MagicMock(spec=MoveToWellRequest)
+    mock_impl = AsyncMock(spec=MoveToWellImplementation)
+
+    mock_req.get_implementation.return_value = mock_impl
+    mock_impl.create_command.return_value = PendingCommand(
+        request=mock_req, created_at=now
+    )
+
+    result = engine.add_command(mock_req)
+
+    assert result == PendingCommand(
+        created_at=cast(datetime, CloseToNow()),
+        request=mock_req,
+    )
+
+    mock_state_store.handle_command.assert_called_with(
+        result,
+        command_id="command-id",
+    )

robot-server/robot_server/protocols/__init__.py

@@ -2,17 +2,20 @@
 
 from opentrons.file_runner import ProtocolFileType
 
-from .router import protocols_router
+from .router import protocols_router, ProtocolNotFound
 from .dependencies import get_protocol_store
-from .protocol_store import ProtocolStore, ProtocolResource
+from .protocol_store import ProtocolStore, ProtocolResource, ProtocolNotFoundError
 
 __all__ = [
     # main protocols router
     "protocols_router",
+    # common error response details
+    "ProtocolNotFound",
     # protocol state management
     "get_protocol_store",
     "ProtocolStore",
     "ProtocolResource",
+    "ProtocolNotFoundError",
     # convenience re-exports from opentrons
     "ProtocolFileType",
 ]

robot-server/robot_server/sessions/action_models.py

@@ -15,7 +15,7 @@ class SessionActionType(str, Enum):
 class SessionActionCreateData(BaseModel):
     """Request model for new control action creation."""
 
-    controlType: SessionActionType
+    actionType: SessionActionType
 
 
 class SessionAction(ResourceModel):
@@ -30,7 +30,7 @@ class SessionAction(ResourceModel):
 
     id: str = Field(..., description="A unique identifier to reference the command.")
     createdAt: datetime = Field(..., description="When the command was created.")
-    controlType: SessionActionType = Field(
+    actionType: SessionActionType = Field(
         ...,
         description="Specific type of action, which determines behavior.",
     )

robot-server/robot_server/sessions/engine_store.py

@@ -2,7 +2,9 @@
 from typing import Optional
 
 from opentrons.hardware_control import API as HardwareAPI
+from opentrons.file_runner import AbstractFileRunner, ProtocolFile, create_file_runner
 from opentrons.protocol_engine import ProtocolEngine
+from robot_server.protocols import ProtocolResource
 
 
 class EngineConflictError(RuntimeError):
@@ -33,6 +35,7 @@ def __init__(self, hardware_api: HardwareAPI) -> None:
         """
         self._hardware_api = hardware_api
         self._engine: Optional[ProtocolEngine] = None
+        self._runner: Optional[AbstractFileRunner] = None
 
     @property
     def engine(self) -> ProtocolEngine:
@@ -46,7 +49,19 @@ def engine(self) -> ProtocolEngine:
 
         return self._engine
 
-    async def create(self) -> ProtocolEngine:
+    @property
+    def runner(self) -> AbstractFileRunner:
+        """Get the persisted AbstractFileRunner.
+
+        Raises:
+            EngineMissingError: Runner has not yet been created and persisted.
+        """
+        if self._runner is None:
+            raise EngineMissingError("Runner not yet created.")
+
+        return self._runner
+
+    async def create(self, protocol: Optional[ProtocolResource]) -> ProtocolEngine:
         """Create and store a ProtocolEngine.
 
         Returns:
@@ -56,15 +71,35 @@ async def create(self) -> ProtocolEngine:
             EngineConflictError: a ProtocolEngine is already present.
         """
         # NOTE: this async. creation happens before the `self._engine`
-        # check intentially to avoid a race condition where `self._engine` is
+        # check intentionally to avoid a race condition where `self._engine` is
         # set after the check but before the engine has finished getting created,
         # at the expense of having to potentially throw away an engine instance
         engine = await ProtocolEngine.create(hardware=self._hardware_api)
+        runner = None
 
         if self._engine is not None:
             raise EngineConflictError("Cannot load multiple sessions simultaneously.")
 
+        if protocol is not None:
+            # TODO(mc, 2021-06-11): add multi-file support. As written, other
+            # parts of the API will make sure len(files) == 0, but this will
+            # not remain true as engine sessions are built out
+            protocol_file = ProtocolFile(
+                file_path=protocol.files[0],
+                file_type=protocol.protocol_type,
+            )
+            runner = create_file_runner(
+                protocol_file=protocol_file,
+                engine=engine,
+            )
+
+            # TODO(mc, 2021-06-11): serparating load from creation is a potentially
+            # two-phase initialization. Revisit this, but for now keep the weirdness
+            # contained here in this factory method
+            runner.load()
+
         self._engine = engine
+        self._runner = runner
 
         return engine