refactor(protocol-engine): rework command models for HTTP API schema (#…

…7993) Co-authored-by: Sanniti Pimpley <[email protected]> Co-authored-by: Max Marrone <[email protected]>
Opentrons · Jun 26, 2021 · 3f74c4a · 3f74c4a
1 parent 64011e0
commit 3f74c4a
Show file tree

Hide file tree

Showing 76 changed files with 2,491 additions and 2,419 deletions.
diff --git a/api/Makefile b/api/Makefile
@@ -61,7 +61,8 @@ ot_tests_to_typecheck := \
 	tests/opentrons/protocol_api_experimental \
 	tests/opentrons/protocol_engine \
 	tests/opentrons/motion_planning \
-	tests/opentrons/file_runner
+	tests/opentrons/file_runner \
+	tests/opentrons/protocols/runner
 
 # Defined separately than the clean target so the wheel file doesn’t have to
 # depend on a PHONY target

diff --git a/api/src/opentrons/file_runner/command_queue_worker.py b/api/src/opentrons/file_runner/command_queue_worker.py
@@ -1,14 +1,14 @@
 """CommandQueueWorker definition. Implements JSON protocol flow control."""
 import asyncio
-from typing import Awaitable, Optional, Tuple
+from typing import Awaitable, Optional
 
-from opentrons import protocol_engine
+from opentrons.protocol_engine import ProtocolEngine
 
 
 class CommandQueueWorker:
     """Execute a `ProtocolEngine`'s queued commands in the background."""
 
-    def __init__(self, protocol_engine: protocol_engine.ProtocolEngine) -> None:
+    def __init__(self, protocol_engine: ProtocolEngine) -> None:
         """Construct a CommandQueueWorker.
 
         Args:
@@ -86,13 +86,13 @@ async def wait_to_be_idle(self) -> None:
 
     def _next_command(
         self,
-    ) -> Optional[Tuple[str, protocol_engine.commands.CommandRequestType]]:
+    ) -> Optional[str]:
         if self._keep_running:
             # Will be None if the engine has no commands left.
-            return self._engine.state_view.commands.get_next_request()
+            return self._engine.state_view.commands.get_next_command()
         else:
             return None
 
     async def _play_async(self) -> None:
-        for command_id, request in iter(self._next_command, None):
-            await self._engine.execute_command(request=request, command_id=command_id)
+        for command_id in iter(self._next_command, None):
+            await self._engine.execute_command_by_id(command_id=command_id)
diff --git a/api/src/opentrons/protocol_engine/__init__.py b/api/src/opentrons/protocol_engine/__init__.py
@@ -5,10 +5,10 @@
 protocol state and side-effects like robot movements.
 """
 
+from .create_protocol_engine import create_protocol_engine
 from .protocol_engine import ProtocolEngine
-from .state import StateStore, StateView, LabwareData
-from .execution import CommandHandlers
-from .resources import ResourceProviders
+from .errors import ProtocolEngineError
+from .state import State, StateView, LabwareData
 from .types import (
     DeckLocation,
     DeckSlotLocation,
@@ -19,16 +19,15 @@
 )
 
 __all__ = [
-    # main export
+    # main factory and interface exports
+    "create_protocol_engine",
     "ProtocolEngine",
+    # error types
+    "ProtocolEngineError",
     # state interfaces and models
-    "StateStore",
+    "State",
     "StateView",
     "LabwareData",
-    # command execution interfaces
-    "CommandHandlers",
-    # resource management interfaces
-    "ResourceProviders",
     # type definitions and other value models
     "DeckLocation",
     "DeckSlotLocation",

diff --git a/api/src/opentrons/protocol_engine/clients/sync_client.py b/api/src/opentrons/protocol_engine/clients/sync_client.py
@@ -35,10 +35,12 @@ def load_labware(
     ) -> commands.LoadLabwareResult:
         """Execute a LoadLabwareRequest and return the result."""
         request = commands.LoadLabwareRequest(
-            location=location,
-            loadName=load_name,
-            namespace=namespace,
-            version=version,
+            data=commands.LoadLabwareData(
+                location=location,
+                loadName=load_name,
+                namespace=namespace,
+                version=version,
+            )
         )
         result = self._transport.execute_command(
             request=request,
@@ -54,8 +56,10 @@ def load_pipette(
     ) -> commands.LoadPipetteResult:
         """Execute a LoadPipetteRequest and return the result."""
         request = commands.LoadPipetteRequest(
-            pipetteName=pipette_name,
-            mount=mount,
+            data=commands.LoadPipetteData(
+                pipetteName=pipette_name,
+                mount=mount,
+            )
         )
         result = self._transport.execute_command(
             request=request,
@@ -72,7 +76,11 @@ def pick_up_tip(
     ) -> commands.PickUpTipResult:
         """Execute a PickUpTipRequest and return the result."""
         request = commands.PickUpTipRequest(
-            pipetteId=pipette_id, labwareId=labware_id, wellName=well_name
+            data=commands.PickUpTipData(
+                pipetteId=pipette_id,
+                labwareId=labware_id,
+                wellName=well_name,
+            )
         )
         result = self._transport.execute_command(
             request=request,
@@ -89,7 +97,11 @@ def drop_tip(
     ) -> commands.DropTipResult:
         """Execute a DropTipRequest and return the result."""
         request = commands.DropTipRequest(
-            pipetteId=pipette_id, labwareId=labware_id, wellName=well_name
+            data=commands.DropTipData(
+                pipetteId=pipette_id,
+                labwareId=labware_id,
+                wellName=well_name,
+            )
         )
         result = self._transport.execute_command(
             request=request, command_id=self._create_command_id()
@@ -106,15 +118,16 @@ def aspirate(
     ) -> commands.AspirateResult:
         """Execute an ``AspirateRequest``, returning the result."""
         request = commands.AspirateRequest(
-            pipetteId=pipette_id,
-            labwareId=labware_id,
-            wellName=well_name,
-            wellLocation=well_location,
-            volume=volume,
+            data=commands.AspirateData(
+                pipetteId=pipette_id,
+                labwareId=labware_id,
+                wellName=well_name,
+                wellLocation=well_location,
+                volume=volume,
+            )
         )
         result = self._transport.execute_command(
-            request=request,
-            command_id=self._create_command_id()
+            request=request, command_id=self._create_command_id()
         )
 
         return cast(commands.AspirateResult, result)
@@ -129,11 +142,13 @@ def dispense(
     ) -> commands.DispenseResult:
         """Execute a ``DispenseRequest``, returning the result."""
         request = commands.DispenseRequest(
-            pipetteId=pipette_id,
-            labwareId=labware_id,
-            wellName=well_name,
-            wellLocation=well_location,
-            volume=volume,
+            data=commands.DispenseData(
+                pipetteId=pipette_id,
+                labwareId=labware_id,
+                wellName=well_name,
+                wellLocation=well_location,
+                volume=volume,
+            )
         )
         result = self._transport.execute_command(
             request=request, command_id=self._create_command_id()

diff --git a/api/src/opentrons/protocol_engine/clients/transports.py b/api/src/opentrons/protocol_engine/clients/transports.py
@@ -3,8 +3,9 @@
 from asyncio import AbstractEventLoop, run_coroutine_threadsafe
 
 from ..protocol_engine import ProtocolEngine
+from ..errors import ProtocolEngineError
 from ..state import StateView
-from ..commands import CommandRequestType, CommandResultType, FailedCommand
+from ..commands import CommandRequest, CommandResult
 
 
 class AbstractSyncTransport(ABC):
@@ -19,9 +20,9 @@ def state(self) -> StateView:
     @abstractmethod
     def execute_command(
         self,
-        request: CommandRequestType,
+        request: CommandRequest,
         command_id: str,
-    ) -> CommandResultType:
+    ) -> CommandResult:
         """Execute a ProtocolEngine command, blocking until the command completes.
 
         Args:
@@ -65,16 +66,20 @@ def state(self) -> StateView:
 
     def execute_command(
         self,
-        request: CommandRequestType,
+        request: CommandRequest,
         command_id: str,
-    ) -> CommandResultType:
+    ) -> CommandResult:
         """Execute a command synchronously on the main thread."""
-        command_state = run_coroutine_threadsafe(
+        command = run_coroutine_threadsafe(
             self._engine.execute_command(request=request, command_id=command_id),
             loop=self._loop,
         ).result()
 
-        if isinstance(command_state, FailedCommand):
-            raise command_state.error
+        if command.error is not None:
+            # TODO(mc, 2021-06-21): refactor when command.error is error details
+            # rather than a string
+            raise ProtocolEngineError(command.error)
 
-        return command_state.result
+        assert command.result is not None, f"Expected Command {command} to have result"
+
+        return command.result