diff --git a/api/Makefile b/api/Makefile index 18d02d68209..40784e6c8e7 100755 --- 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 index 63eab23a41f..a1a439b271e 100644 --- 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 index 888ee0ac351..68631378f7c 100644 --- 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 index 5ed204f833c..6eee07e1b24 100644 --- 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 index b260840dce3..a1dcf949d3a 100644 --- 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 diff --git a/api/src/opentrons/protocol_engine/commands/__init__.py b/api/src/opentrons/protocol_engine/commands/__init__.py index 766debddcc9..30502dbb2cd 100644 --- a/api/src/opentrons/protocol_engine/commands/__init__.py +++ b/api/src/opentrons/protocol_engine/commands/__init__.py @@ -13,127 +13,91 @@ and/or schema generation. """ -from typing import Union +from .command import AbstractCommandImpl, BaseCommand, BaseCommandRequest, CommandStatus +from .command_mapper import CommandMapper +from .command_unions import Command, CommandRequest, CommandResult -from .command import PendingCommand, RunningCommand, CompletedCommand, FailedCommand -from .load_labware import LoadLabwareRequest, LoadLabwareResult from .add_labware_definition import ( - AddLabwareDefinitionRequest, AddLabwareDefinitionResult + AddLabwareDefinition, + AddLabwareDefinitionRequest, + AddLabwareDefinitionData, + AddLabwareDefinitionResult, ) -from .load_pipette import LoadPipetteRequest, LoadPipetteResult -from .move_to_well import MoveToWellRequest, MoveToWellResult -from .pick_up_tip import PickUpTipRequest, PickUpTipResult -from .drop_tip import DropTipRequest, DropTipResult -from .aspirate import AspirateRequest, AspirateResult -from .dispense import DispenseRequest, DispenseResult - -CommandRequestType = Union[ +from .aspirate import Aspirate, AspirateRequest, AspirateData, AspirateResult +from .dispense import Dispense, DispenseRequest, DispenseData, DispenseResult +from .drop_tip import DropTip, DropTipRequest, DropTipData, DropTipResult +from .load_labware import ( + LoadLabware, LoadLabwareRequest, - AddLabwareDefinitionRequest, - LoadPipetteRequest, - MoveToWellRequest, - PickUpTipRequest, - DropTipRequest, - AspirateRequest, - DispenseRequest, -] - -CommandResultType = Union[ + LoadLabwareData, LoadLabwareResult, - AddLabwareDefinitionResult, +) +from .load_pipette import ( + LoadPipette, + LoadPipetteRequest, + LoadPipetteData, LoadPipetteResult, +) +from .move_to_well import ( + MoveToWell, + MoveToWellRequest, + MoveToWellData, MoveToWellResult, - PickUpTipResult, - DropTipResult, - AspirateResult, - DispenseResult, -] - -PendingCommandType = Union[ - PendingCommand[LoadLabwareRequest, LoadLabwareResult], - PendingCommand[AddLabwareDefinitionRequest, AddLabwareDefinitionResult], - PendingCommand[LoadPipetteRequest, LoadPipetteResult], - PendingCommand[MoveToWellRequest, MoveToWellResult], - PendingCommand[PickUpTipRequest, PickUpTipResult], - PendingCommand[DropTipRequest, DropTipResult], - PendingCommand[AspirateRequest, AspirateResult], - PendingCommand[DispenseRequest, DispenseResult], -] - -RunningCommandType = Union[ - RunningCommand[LoadLabwareRequest, LoadLabwareResult], - RunningCommand[AddLabwareDefinitionRequest, AddLabwareDefinitionResult], - RunningCommand[LoadPipetteRequest, LoadPipetteResult], - RunningCommand[MoveToWellRequest, MoveToWellResult], - RunningCommand[PickUpTipRequest, PickUpTipResult], - RunningCommand[DropTipRequest, DropTipResult], - RunningCommand[AspirateRequest, AspirateResult], - RunningCommand[DispenseRequest, DispenseResult], -] - -CompletedCommandType = Union[ - CompletedCommand[LoadLabwareRequest, LoadLabwareResult], - CompletedCommand[AddLabwareDefinitionRequest, AddLabwareDefinitionResult], - CompletedCommand[LoadPipetteRequest, LoadPipetteResult], - CompletedCommand[MoveToWellRequest, MoveToWellResult], - CompletedCommand[PickUpTipRequest, PickUpTipResult], - CompletedCommand[DropTipRequest, DropTipResult], - CompletedCommand[AspirateRequest, AspirateResult], - CompletedCommand[DispenseRequest, DispenseResult], -] - -FailedCommandType = Union[ - FailedCommand[LoadLabwareRequest], - FailedCommand[AddLabwareDefinitionRequest], - FailedCommand[LoadPipetteRequest], - FailedCommand[MoveToWellRequest], - FailedCommand[PickUpTipRequest], - FailedCommand[DropTipRequest], - FailedCommand[AspirateRequest], - FailedCommand[DispenseRequest], -] +) +from .pick_up_tip import PickUpTip, PickUpTipRequest, PickUpTipData, PickUpTipResult -CommandType = Union[ - PendingCommandType, - RunningCommandType, - CompletedCommandType, - FailedCommandType, -] __all__ = [ - # command lifecycle state models - "PendingCommand", - "RunningCommand", - "CompletedCommand", - "FailedCommand", - - # type unions - "CommandRequestType", - "CommandResultType", - "PendingCommandType", - "RunningCommandType", - "CompletedCommandType", - "FailedCommandType", - "CommandType", - - # equipment request/result models + # command model factory + "CommandMapper", + # command type unions + "Command", + "CommandRequest", + "CommandResult", + # base interfaces + "AbstractCommandImpl", + "BaseCommand", + "BaseCommandRequest", + "CommandStatus", + # load labware command models + "LoadLabware", "LoadLabwareRequest", + "LoadLabwareData", "LoadLabwareResult", + # add labware definition command models + "AddLabwareDefinition", "AddLabwareDefinitionRequest", + "AddLabwareDefinitionData", "AddLabwareDefinitionResult", + # load pipette command models + "LoadPipette", "LoadPipetteRequest", + "LoadPipetteData", "LoadPipetteResult", - - # pipetting request/result models + # move to well command models + "MoveToWell", "MoveToWellRequest", + "MoveToWellData", "MoveToWellResult", + # pick up tip command models + "PickUpTip", "PickUpTipRequest", + "PickUpTipData", "PickUpTipResult", + # drop tip command models + "DropTip", "DropTipRequest", + "DropTipData", "DropTipResult", + # aspirate command models + "Aspirate", "AspirateRequest", + "AspirateData", "AspirateResult", + # dispense command models + "Dispense", "DispenseRequest", + "DispenseData", "DispenseResult", ] diff --git a/api/src/opentrons/protocol_engine/commands/add_labware_definition.py b/api/src/opentrons/protocol_engine/commands/add_labware_definition.py index 00b05f054fb..74113033295 100644 --- a/api/src/opentrons/protocol_engine/commands/add_labware_definition.py +++ b/api/src/opentrons/protocol_engine/commands/add_labware_definition.py @@ -1,22 +1,19 @@ """Add labware command.""" from __future__ import annotations -from opentrons.protocol_engine.commands.command import ( - CommandImplementation, CommandHandlers -) from pydantic import BaseModel, Field +from typing import Optional, Type +from typing_extensions import Literal + from opentrons.protocols.models import LabwareDefinition +from .command import AbstractCommandImpl, BaseCommand, BaseCommandRequest +AddLabwareDefinitionCommandType = Literal["addLabwareDefinition"] -class AddLabwareDefinitionRequest(BaseModel): - """Request to add a labware definition.""" - definition: LabwareDefinition = Field( - ..., - description="The labware definition." - ) - def get_implementation(self) -> AddLabwareDefinitionImplementation: - """Get the execution implementation of the AddLabwareDefinitionRequest.""" - return AddLabwareDefinitionImplementation(self) +class AddLabwareDefinitionData(BaseModel): + """Data required to add a labware definition.""" + + definition: LabwareDefinition = Field(..., description="The labware definition.") class AddLabwareDefinitionResult(BaseModel): @@ -37,18 +34,39 @@ class AddLabwareDefinitionResult(BaseModel): class AddLabwareDefinitionImplementation( - CommandImplementation[AddLabwareDefinitionRequest, - AddLabwareDefinitionResult] + AbstractCommandImpl[AddLabwareDefinitionData, AddLabwareDefinitionResult] ): """Add labware command implementation.""" async def execute( - self, - handlers: CommandHandlers + self, data: AddLabwareDefinitionData ) -> AddLabwareDefinitionResult: """Execute the add labware definition request.""" return AddLabwareDefinitionResult( - loadName=self._request.definition.parameters.loadName, - namespace=self._request.definition.namespace, - version=self._request.definition.version + loadName=data.definition.parameters.loadName, + namespace=data.definition.namespace, + version=data.definition.version, ) + + +class AddLabwareDefinition( + BaseCommand[AddLabwareDefinitionData, AddLabwareDefinitionResult] +): + """Add labware definition command resource.""" + + commandType: AddLabwareDefinitionCommandType = "addLabwareDefinition" + data: AddLabwareDefinitionData + result: Optional[AddLabwareDefinitionResult] + + _ImplementationCls: Type[ + AddLabwareDefinitionImplementation + ] = AddLabwareDefinitionImplementation + + +class AddLabwareDefinitionRequest(BaseCommandRequest[AddLabwareDefinitionData]): + """Add labware definition command creation request.""" + + commandType: AddLabwareDefinitionCommandType = "addLabwareDefinition" + data: AddLabwareDefinitionData + + _CommandCls: Type[AddLabwareDefinition] = AddLabwareDefinition diff --git a/api/src/opentrons/protocol_engine/commands/aspirate.py b/api/src/opentrons/protocol_engine/commands/aspirate.py index 04a14f3e847..5d3b214c592 100644 --- a/api/src/opentrons/protocol_engine/commands/aspirate.py +++ b/api/src/opentrons/protocol_engine/commands/aspirate.py @@ -1,41 +1,57 @@ """Aspirate command request, result, and implementation models.""" from __future__ import annotations -from typing_extensions import final +from typing import Optional, Type +from typing_extensions import Literal -from .command import CommandImplementation, CommandHandlers -from .pipetting_common import BaseLiquidHandlingRequest, BaseLiquidHandlingResult +from .pipetting_common import BaseLiquidHandlingData, BaseLiquidHandlingResult +from .command import AbstractCommandImpl, BaseCommand, BaseCommandRequest -@final -class AspirateRequest(BaseLiquidHandlingRequest): - """A request to move to a specific well and aspirate from it.""" +AspirateCommandType = Literal["aspirate"] - def get_implementation(self) -> AspirateImplementation: - """Get the execution implementation of the AspirateRequest.""" - return AspirateImplementation(self) + +class AspirateData(BaseLiquidHandlingData): + """Data required to aspirate from a specific well.""" + + pass -@final class AspirateResult(BaseLiquidHandlingResult): """Result data from the execution of a AspirateRequest.""" pass -@final -class AspirateImplementation( - CommandImplementation[AspirateRequest, AspirateResult] -): +class AspirateImplementation(AbstractCommandImpl[AspirateData, AspirateResult]): """Aspirate command implementation.""" - async def execute(self, handlers: CommandHandlers) -> AspirateResult: + async def execute(self, data: AspirateData) -> AspirateResult: """Move to and aspirate from the requested well.""" - volume = await handlers.pipetting.aspirate( - pipette_id=self._request.pipetteId, - labware_id=self._request.labwareId, - well_name=self._request.wellName, - well_location=self._request.wellLocation, - volume=self._request.volume, + volume = await self._pipetting.aspirate( + pipette_id=data.pipetteId, + labware_id=data.labwareId, + well_name=data.wellName, + well_location=data.wellLocation, + volume=data.volume, ) return AspirateResult(volume=volume) + + +class Aspirate(BaseCommand[AspirateData, AspirateResult]): + """Aspirate command model.""" + + commandType: AspirateCommandType = "aspirate" + data: AspirateData + result: Optional[AspirateResult] + + _ImplementationCls: Type[AspirateImplementation] = AspirateImplementation + + +class AspirateRequest(BaseCommandRequest[AspirateData]): + """Create aspirate command request model.""" + + commandType: AspirateCommandType = "aspirate" + data: AspirateData + + _CommandCls: Type[Aspirate] = Aspirate diff --git a/api/src/opentrons/protocol_engine/commands/command.py b/api/src/opentrons/protocol_engine/commands/command.py index 30e5ec52a5d..421f1b82d64 100644 --- a/api/src/opentrons/protocol_engine/commands/command.py +++ b/api/src/opentrons/protocol_engine/commands/command.py @@ -1,100 +1,89 @@ """Base command data model and type definitions.""" from __future__ import annotations from abc import ABC, abstractmethod -from dataclasses import dataclass +from enum import Enum from datetime import datetime -from pydantic import BaseModel -from typing import TYPE_CHECKING, Generic, TypeVar - -from ..errors import ProtocolEngineError +from pydantic import BaseModel, Field +from pydantic.generics import GenericModel +from typing import TYPE_CHECKING, Generic, Optional, TypeVar # convenience type alias to work around type-only circular dependency if TYPE_CHECKING: - from ..execution import CommandHandlers -else: - CommandHandlers = None + from ..execution import EquipmentHandler, MovementHandler, PipettingHandler -ReqT = TypeVar("ReqT", bound=BaseModel) -ResT = TypeVar("ResT", bound=BaseModel) +CommandDataT = TypeVar("CommandDataT", bound=BaseModel) +CommandResultT = TypeVar("CommandResultT", bound=BaseModel) -@dataclass(frozen=True) -class CompletedCommand(Generic[ReqT, ResT]): - """A command that has been successfully executed.""" - created_at: datetime - started_at: datetime - completed_at: datetime - request: ReqT - result: ResT +class CommandStatus(str, Enum): + """Command execution status.""" + QUEUED = "queued" + RUNNING = "running" + SUCCEEDED = "succeeded" + FAILED = "failed" -@dataclass(frozen=True) -class FailedCommand(Generic[ReqT]): - """A command that was executed but failed.""" - created_at: datetime - started_at: datetime - failed_at: datetime - request: ReqT - error: ProtocolEngineError +class BaseCommandRequest(GenericModel, Generic[CommandDataT]): + """Base class for command creation requests. + You shouldn't use this class directly; instead, use or define + your own subclass per specific command type. + """ -@dataclass(frozen=True) -class RunningCommand(Generic[ReqT, ResT]): - """A command that is currently being executed.""" + commandType: str = Field( + ..., + description=( + "Specific command type that determines data requirements and " + "execution behavior" + ), + ) + data: CommandDataT = Field(..., description="Command execution data payload") - created_at: datetime - started_at: datetime - request: ReqT - def to_completed( - self, - result: ResT, - completed_at: datetime, - ) -> CompletedCommand[ReqT, ResT]: - """Create a CompletedCommand from a RunningCommand.""" - return CompletedCommand( - created_at=self.created_at, - started_at=self.started_at, - request=self.request, - completed_at=completed_at, - result=result - ) - - def to_failed( - self, - error: ProtocolEngineError, - failed_at: datetime, - ) -> FailedCommand[ReqT]: - """Create a FailedCommand from a RunningCommand.""" - return FailedCommand( - created_at=self.created_at, - started_at=self.started_at, - request=self.request, - failed_at=failed_at, - error=error - ) - - -@dataclass(frozen=True) -class PendingCommand(Generic[ReqT, ResT]): - """A command that has not yet been started.""" - - created_at: datetime - request: ReqT - - def to_running(self, started_at: datetime) -> RunningCommand[ReqT, ResT]: - """Create a RunningCommand from a PendingCommand.""" - return RunningCommand( - created_at=self.created_at, - request=self.request, - started_at=started_at, - ) - - -class CommandImplementation(ABC, Generic[ReqT, ResT]): - """Abstract command implementation. +class BaseCommand(GenericModel, Generic[CommandDataT, CommandResultT]): + """Base command model. + + You shouldn't use this class directly; instead, use or define + your own subclass per specific command type. + """ + + id: str = Field(..., description="Unique identifier for a particular command") + createdAt: datetime = Field(..., description="Command creation timestamp") + commandType: str = Field( + ..., + description=( + "Specific command type that determines data requirements and " + "execution behavior" + ), + ) + status: CommandStatus = Field(..., description="Command execution status") + data: CommandDataT = Field(..., description="Command execution data payload") + result: Optional[CommandResultT] = Field( + None, + description="Command execution result data, if succeeded", + ) + # TODO(mc, 2021-06-08): model ProtocolEngine errors + error: Optional[str] = Field( + None, + description="Command execution failure, if failed", + ) + startedAt: Optional[datetime] = Field( + None, + description="Command execution start timestamp, if started", + ) + completedAt: Optional[datetime] = Field( + None, + description="Command execution completed timestamp, if completed", + ) + + +class AbstractCommandImpl( + ABC, + Generic[CommandDataT, CommandResultT], +): + """Abstract command creation and execution implementation. A given command request should map to a specific command implementation, which defines how to: @@ -103,17 +92,18 @@ class CommandImplementation(ABC, Generic[ReqT, ResT]): - Execute the command, mapping data from execution into the result model """ - _request: ReqT - - def __init__(self, request: ReqT) -> None: - """Initialize a command implementation from a command request.""" - self._request = request - - def create_command(self, created_at: datetime) -> PendingCommand[ReqT, ResT]: - """Create a new command resource from the implementation's request.""" - return PendingCommand(request=self._request, created_at=created_at) + def __init__( + self, + equipment: EquipmentHandler, + movement: MovementHandler, + pipetting: PipettingHandler, + ) -> None: + """Initialize the command implementation with execution handlers.""" + self._equipment = equipment + self._movement = movement + self._pipetting = pipetting @abstractmethod - async def execute(self, handlers: CommandHandlers) -> ResT: + async def execute(self, data: CommandDataT) -> CommandResultT: """Execute the command, mapping data from execution into a response model.""" ... diff --git a/api/src/opentrons/protocol_engine/commands/command_mapper.py b/api/src/opentrons/protocol_engine/commands/command_mapper.py new file mode 100644 index 00000000000..fda5fd5cda2 --- /dev/null +++ b/api/src/opentrons/protocol_engine/commands/command_mapper.py @@ -0,0 +1,37 @@ +"""Map command request and model types to other values.""" +from datetime import datetime +from typing import Any + +from .command import CommandStatus +from .command_unions import Command, CommandRequest + + +class CommandMapper: + """Static methods to map and modify command models.""" + + @staticmethod + def map_request_to_command( + request: CommandRequest, + command_id: str, + created_at: datetime, + ) -> Command: + """Map a CommandRequest instance to a full command.""" + # TODO(mc, 2021-06-22): mypy has trouble with this automatic + # request > command mapping, figure out how to type precisely + # (or wait for a future mypy version that can figure it out). + # For now, unit tests cover mapping every request type + return request._CommandCls( + id=command_id, + createdAt=created_at, + status=CommandStatus.QUEUED, + data=request.data, # type: ignore[arg-type] + ) + + @staticmethod + def update_command(command: Command, **update: Any) -> Command: + """Map a Command to a new Command instance with updated field values. + + This is a thin wrapper around `BaseModel.copy`, but it ensures that + all updates to commands happen immutably. + """ + return command.copy(update=update) diff --git a/api/src/opentrons/protocol_engine/commands/command_unions.py b/api/src/opentrons/protocol_engine/commands/command_unions.py new file mode 100644 index 00000000000..524b7d3b403 --- /dev/null +++ b/api/src/opentrons/protocol_engine/commands/command_unions.py @@ -0,0 +1,48 @@ +"""Union types of concrete command definitions.""" +from typing import Union + +from .add_labware_definition import ( + AddLabwareDefinition, + AddLabwareDefinitionRequest, + AddLabwareDefinitionResult, +) +from .aspirate import Aspirate, AspirateRequest, AspirateResult +from .dispense import Dispense, DispenseRequest, DispenseResult +from .drop_tip import DropTip, DropTipRequest, DropTipResult +from .load_labware import LoadLabware, LoadLabwareRequest, LoadLabwareResult +from .load_pipette import LoadPipette, LoadPipetteRequest, LoadPipetteResult +from .move_to_well import MoveToWell, MoveToWellRequest, MoveToWellResult +from .pick_up_tip import PickUpTip, PickUpTipRequest, PickUpTipResult + +Command = Union[ + AddLabwareDefinition, + Aspirate, + Dispense, + DropTip, + LoadLabware, + LoadPipette, + MoveToWell, + PickUpTip, +] + +CommandRequest = Union[ + AddLabwareDefinitionRequest, + AspirateRequest, + DispenseRequest, + DropTipRequest, + LoadLabwareRequest, + LoadPipetteRequest, + MoveToWellRequest, + PickUpTipRequest, +] + +CommandResult = Union[ + AddLabwareDefinitionResult, + AspirateResult, + DispenseResult, + DropTipResult, + LoadLabwareResult, + LoadPipetteResult, + MoveToWellResult, + PickUpTipResult, +] diff --git a/api/src/opentrons/protocol_engine/commands/dispense.py b/api/src/opentrons/protocol_engine/commands/dispense.py index 13e8bc35754..eef027e0281 100644 --- a/api/src/opentrons/protocol_engine/commands/dispense.py +++ b/api/src/opentrons/protocol_engine/commands/dispense.py @@ -1,41 +1,57 @@ """Dispense command request, result, and implementation models.""" from __future__ import annotations -from typing_extensions import final +from typing import Optional, Type +from typing_extensions import Literal -from .command import CommandImplementation, CommandHandlers -from .pipetting_common import BaseLiquidHandlingRequest, BaseLiquidHandlingResult +from .pipetting_common import BaseLiquidHandlingData, BaseLiquidHandlingResult +from .command import AbstractCommandImpl, BaseCommand, BaseCommandRequest -@final -class DispenseRequest(BaseLiquidHandlingRequest): - """A request to aspirate from a specific well.""" +DispenseCommandType = Literal["dispense"] - def get_implementation(self) -> DispenseImplementation: - """Get the execution implementation of the DispenseRequest.""" - return DispenseImplementation(self) + +class DispenseData(BaseLiquidHandlingData): + """Data required to dispense to a specific well.""" + + pass -@final class DispenseResult(BaseLiquidHandlingResult): """Result data from the execution of a DispenseRequest.""" pass -@final -class DispenseImplementation( - CommandImplementation[DispenseRequest, DispenseResult] -): +class DispenseImplementation(AbstractCommandImpl[DispenseData, DispenseResult]): """Dispense command implementation.""" - async def execute(self, handlers: CommandHandlers) -> DispenseResult: + async def execute(self, data: DispenseData) -> DispenseResult: """Move to and dispense to the requested well.""" - volume = await handlers.pipetting.dispense( - pipette_id=self._request.pipetteId, - labware_id=self._request.labwareId, - well_name=self._request.wellName, - well_location=self._request.wellLocation, - volume=self._request.volume, + volume = await self._pipetting.dispense( + pipette_id=data.pipetteId, + labware_id=data.labwareId, + well_name=data.wellName, + well_location=data.wellLocation, + volume=data.volume, ) return DispenseResult(volume=volume) + + +class Dispense(BaseCommand[DispenseData, DispenseResult]): + """Dispense command model.""" + + commandType: DispenseCommandType = "dispense" + data: DispenseData + result: Optional[DispenseResult] + + _ImplementationCls: Type[DispenseImplementation] = DispenseImplementation + + +class DispenseRequest(BaseCommandRequest[DispenseData]): + """Create dispense command request model.""" + + commandType: DispenseCommandType = "dispense" + data: DispenseData + + _CommandCls: Type[Dispense] = Dispense diff --git a/api/src/opentrons/protocol_engine/commands/drop_tip.py b/api/src/opentrons/protocol_engine/commands/drop_tip.py index 78d2a149908..bdf274a0e34 100644 --- a/api/src/opentrons/protocol_engine/commands/drop_tip.py +++ b/api/src/opentrons/protocol_engine/commands/drop_tip.py @@ -1,17 +1,19 @@ """Drop tip command request, result, and implementation models.""" from __future__ import annotations from pydantic import BaseModel +from typing import Optional, Type +from typing_extensions import Literal -from .command import CommandImplementation, CommandHandlers -from .pipetting_common import BasePipettingRequest +from .pipetting_common import BasePipettingData +from .command import AbstractCommandImpl, BaseCommand, BaseCommandRequest +DropTipCommandType = Literal["dropTip"] -class DropTipRequest(BasePipettingRequest): - """A request to drop a tip in a specific well.""" - def get_implementation(self) -> DropTipImplementation: - """Get the drop tip request's command implementation.""" - return DropTipImplementation(self) +class DropTipData(BasePipettingData): + """Data required to drop a tip in a specific well.""" + + pass class DropTipResult(BaseModel): @@ -20,17 +22,34 @@ class DropTipResult(BaseModel): pass -class DropTipImplementation( - CommandImplementation[DropTipRequest, DropTipResult] -): +class DropTipImplementation(AbstractCommandImpl[DropTipData, DropTipResult]): """Drop tip command implementation.""" - async def execute(self, handlers: CommandHandlers) -> DropTipResult: + async def execute(self, data: DropTipData) -> DropTipResult: """Move to and drop a tip using the requested pipette.""" - await handlers.pipetting.drop_tip( - pipette_id=self._request.pipetteId, - labware_id=self._request.labwareId, - well_name=self._request.wellName, + await self._pipetting.drop_tip( + pipette_id=data.pipetteId, + labware_id=data.labwareId, + well_name=data.wellName, ) return DropTipResult() + + +class DropTip(BaseCommand[DropTipData, DropTipResult]): + """Drop tip command model.""" + + commandType: DropTipCommandType = "dropTip" + data: DropTipData + result: Optional[DropTipResult] + + _ImplementationCls: Type[DropTipImplementation] = DropTipImplementation + + +class DropTipRequest(BaseCommandRequest[DropTipData]): + """Drop tip command creation request model.""" + + commandType: DropTipCommandType = "dropTip" + data: DropTipData + + _CommandCls: Type[DropTip] = DropTip diff --git a/api/src/opentrons/protocol_engine/commands/load_labware.py b/api/src/opentrons/protocol_engine/commands/load_labware.py index 726a76dbd2d..9a42588d2cf 100644 --- a/api/src/opentrons/protocol_engine/commands/load_labware.py +++ b/api/src/opentrons/protocol_engine/commands/load_labware.py @@ -1,17 +1,19 @@ """Load labware command request, result, and implementation models.""" from __future__ import annotations - from pydantic import BaseModel, Field -from typing import Tuple, Optional +from typing import Optional, Tuple, Type +from typing_extensions import Literal from opentrons.protocols.models import LabwareDefinition -from ..types import LabwareLocation -from .command import CommandImplementation, CommandHandlers +from opentrons.protocol_engine.types import LabwareLocation +from .command import AbstractCommandImpl, BaseCommand, BaseCommandRequest + +LoadLabwareCommandType = Literal["loadLabware"] -class LoadLabwareRequest(BaseModel): - """A request to load a labware into a slot.""" +class LoadLabwareData(BaseModel): + """Data required to load a labware into a slot.""" location: LabwareLocation = Field( ..., @@ -32,13 +34,9 @@ class LoadLabwareRequest(BaseModel): labwareId: Optional[str] = Field( None, description="An optional ID to assign to this labware. If None, an ID " - "will be generated." + "will be generated.", ) - def get_implementation(self) -> LoadLabwareImplementation: - """Get the load labware request's command implementation.""" - return LoadLabwareImplementation(self) - class LoadLabwareResult(BaseModel): """Result data from the execution of a LoadLabwareRequest.""" @@ -58,18 +56,18 @@ class LoadLabwareResult(BaseModel): class LoadLabwareImplementation( - CommandImplementation[LoadLabwareRequest, LoadLabwareResult] + AbstractCommandImpl[LoadLabwareData, LoadLabwareResult] ): """Load labware command implementation.""" - async def execute(self, handlers: CommandHandlers) -> LoadLabwareResult: + async def execute(self, data: LoadLabwareData) -> LoadLabwareResult: """Load definition and calibration data necessary for a labware.""" - loaded_labware = await handlers.equipment.load_labware( - load_name=self._request.loadName, - namespace=self._request.namespace, - version=self._request.version, - location=self._request.location, - labware_id=self._request.labwareId + loaded_labware = await self._equipment.load_labware( + load_name=data.loadName, + namespace=data.namespace, + version=data.version, + location=data.location, + labware_id=data.labwareId, ) return LoadLabwareResult( @@ -77,3 +75,22 @@ async def execute(self, handlers: CommandHandlers) -> LoadLabwareResult: definition=loaded_labware.definition, calibration=loaded_labware.calibration, ) + + +class LoadLabware(BaseCommand[LoadLabwareData, LoadLabwareResult]): + """Load labware command resource model.""" + + commandType: LoadLabwareCommandType = "loadLabware" + data: LoadLabwareData + result: Optional[LoadLabwareResult] + + _ImplementationCls: Type[LoadLabwareImplementation] = LoadLabwareImplementation + + +class LoadLabwareRequest(BaseCommandRequest[LoadLabwareData]): + """Load labware command creation request.""" + + commandType: LoadLabwareCommandType = "loadLabware" + data: LoadLabwareData + + _CommandCls: Type[LoadLabware] = LoadLabware diff --git a/api/src/opentrons/protocol_engine/commands/load_pipette.py b/api/src/opentrons/protocol_engine/commands/load_pipette.py index b253a79ddbd..35964cb876b 100644 --- a/api/src/opentrons/protocol_engine/commands/load_pipette.py +++ b/api/src/opentrons/protocol_engine/commands/load_pipette.py @@ -1,18 +1,19 @@ """Load pipette command request, result, and implementation models.""" from __future__ import annotations - -from typing import Optional - from pydantic import BaseModel, Field +from typing import Optional, Type +from typing_extensions import Literal from opentrons.types import MountType from ..types import PipetteName -from .command import CommandImplementation, CommandHandlers +from .command import AbstractCommandImpl, BaseCommand, BaseCommandRequest + +LoadPipetteCommandType = Literal["loadPipette"] -class LoadPipetteRequest(BaseModel): - """A request to load a pipette on to a mount.""" +class LoadPipetteData(BaseModel): + """Data needed to load a pipette on to a mount.""" pipetteName: PipetteName = Field( ..., @@ -25,16 +26,12 @@ class LoadPipetteRequest(BaseModel): pipetteId: Optional[str] = Field( None, description="An optional ID to assign to this pipette. If None, an ID " - "will be generated." + "will be generated.", ) - def get_implementation(self) -> LoadPipetteImplementation: - """Get the load pipette request's command implementation.""" - return LoadPipetteImplementation(self) - class LoadPipetteResult(BaseModel): - """Result data for executing a LoadPipetteRequest.""" + """Result data for executing a LoadPipette.""" pipetteId: str = Field( ..., @@ -43,16 +40,35 @@ class LoadPipetteResult(BaseModel): class LoadPipetteImplementation( - CommandImplementation[LoadPipetteRequest, LoadPipetteResult] + AbstractCommandImpl[LoadPipetteData, LoadPipetteResult] ): """Load pipette command implementation.""" - async def execute(self, handlers: CommandHandlers) -> LoadPipetteResult: + async def execute(self, data: LoadPipetteData) -> LoadPipetteResult: """Check that requested pipette is attached and assign its identifier.""" - loaded_pipette = await handlers.equipment.load_pipette( - pipette_name=self._request.pipetteName, - mount=self._request.mount, - pipette_id=self._request.pipetteId + loaded_pipette = await self._equipment.load_pipette( + pipette_name=data.pipetteName, + mount=data.mount, + pipette_id=data.pipetteId, ) return LoadPipetteResult(pipetteId=loaded_pipette.pipette_id) + + +class LoadPipette(BaseCommand[LoadPipetteData, LoadPipetteResult]): + """Load pipette command model.""" + + commandType: LoadPipetteCommandType = "loadPipette" + data: LoadPipetteData + result: Optional[LoadPipetteResult] + + _ImplementationCls: Type[LoadPipetteImplementation] = LoadPipetteImplementation + + +class LoadPipetteRequest(BaseCommandRequest[LoadPipetteData]): + """Load pipette command creation request model.""" + + commandType: LoadPipetteCommandType = "loadPipette" + data: LoadPipetteData + + _CommandCls: Type[LoadPipette] = LoadPipette diff --git a/api/src/opentrons/protocol_engine/commands/move_to_well.py b/api/src/opentrons/protocol_engine/commands/move_to_well.py index 2f13c1d0877..efeeb40616e 100644 --- a/api/src/opentrons/protocol_engine/commands/move_to_well.py +++ b/api/src/opentrons/protocol_engine/commands/move_to_well.py @@ -1,36 +1,56 @@ """Move to well command request, result, and implementation models.""" from __future__ import annotations from pydantic import BaseModel +from typing import Optional, Type +from typing_extensions import Literal -from .command import CommandImplementation, CommandHandlers -from .pipetting_common import BasePipettingRequest +from .pipetting_common import BasePipettingData +from .command import AbstractCommandImpl, BaseCommand, BaseCommandRequest -class MoveToWellRequest(BasePipettingRequest): - """A request to move a pipette to a specific well.""" +MoveToWellCommandType = Literal["moveToWell"] - def get_implementation(self) -> MoveToWellImplementation: - """Get the move to well request's command implementation.""" - return MoveToWellImplementation(self) + +class MoveToWellData(BasePipettingData): + """Data required to move a pipette to a specific well.""" + + pass class MoveToWellResult(BaseModel): - """Result data from the execution of a MoveToWellRequest.""" + """Result data from the execution of a MoveToWell command.""" pass -class MoveToWellImplementation( - CommandImplementation[MoveToWellRequest, MoveToWellResult] -): +class MoveToWellImplementation(AbstractCommandImpl[MoveToWellData, MoveToWellResult]): """Move to well command implementation.""" - async def execute(self, handlers: CommandHandlers) -> MoveToWellResult: + async def execute(self, data: MoveToWellData) -> MoveToWellResult: """Move the requested pipette to the requested well.""" - await handlers.movement.move_to_well( - pipette_id=self._request.pipetteId, - labware_id=self._request.labwareId, - well_name=self._request.wellName, + await self._movement.move_to_well( + pipette_id=data.pipetteId, + labware_id=data.labwareId, + well_name=data.wellName, ) return MoveToWellResult() + + +class MoveToWell(BaseCommand[MoveToWellData, MoveToWellResult]): + """Move to well command model.""" + + commandType: MoveToWellCommandType = "moveToWell" + data: MoveToWellData + result: Optional[MoveToWellResult] + + _ImplementationCls: Type[MoveToWellImplementation] = MoveToWellImplementation + + +class MoveToWellRequest(BaseCommandRequest[MoveToWellData]): + """Move to well command creation request model.""" + + commandType: MoveToWellCommandType = "moveToWell" + data: MoveToWellData + + _CommandCls: Type[MoveToWell] = MoveToWell diff --git a/api/src/opentrons/protocol_engine/commands/pick_up_tip.py b/api/src/opentrons/protocol_engine/commands/pick_up_tip.py index d84c291e684..aa974f3d748 100644 --- a/api/src/opentrons/protocol_engine/commands/pick_up_tip.py +++ b/api/src/opentrons/protocol_engine/commands/pick_up_tip.py @@ -1,36 +1,56 @@ """Pick up tip command request, result, and implementation models.""" from __future__ import annotations from pydantic import BaseModel +from typing import Optional, Type +from typing_extensions import Literal -from .command import CommandImplementation, CommandHandlers -from .pipetting_common import BasePipettingRequest +from .pipetting_common import BasePipettingData +from .command import AbstractCommandImpl, BaseCommand, BaseCommandRequest -class PickUpTipRequest(BasePipettingRequest): - """A request to move a pipette to a specific well.""" +PickUpTipCommandType = Literal["pickUpTip"] - def get_implementation(self) -> PickUpTipImplementation: - """Get the pick up tip request's command implementation.""" - return PickUpTipImplementation(self) + +class PickUpTipData(BasePipettingData): + """Data needed to move a pipette to a specific well.""" + + pass class PickUpTipResult(BaseModel): - """Result data from the execution of a PickUpTipRequest.""" + """Result data from the execution of a PickUpTip.""" pass -class PickUpTipImplementation( - CommandImplementation[PickUpTipRequest, PickUpTipResult] -): +class PickUpTipImplementation(AbstractCommandImpl[PickUpTipData, PickUpTipResult]): """Pick up tip command implementation.""" - async def execute(self, handlers: CommandHandlers) -> PickUpTipResult: + async def execute(self, data: PickUpTipData) -> PickUpTipResult: """Move to and pick up a tip using the requested pipette.""" - await handlers.pipetting.pick_up_tip( - pipette_id=self._request.pipetteId, - labware_id=self._request.labwareId, - well_name=self._request.wellName, + await self._pipetting.pick_up_tip( + pipette_id=data.pipetteId, + labware_id=data.labwareId, + well_name=data.wellName, ) return PickUpTipResult() + + +class PickUpTip(BaseCommand[PickUpTipData, PickUpTipResult]): + """Pick up tip command model.""" + + commandType: PickUpTipCommandType = "pickUpTip" + data: PickUpTipData + result: Optional[PickUpTipResult] + + _ImplementationCls: Type[PickUpTipImplementation] = PickUpTipImplementation + + +class PickUpTipRequest(BaseCommandRequest[PickUpTipData]): + """Pick up tip command creation request model.""" + + commandType: PickUpTipCommandType = "pickUpTip" + data: PickUpTipData + + _CommandCls: Type[PickUpTip] = PickUpTip diff --git a/api/src/opentrons/protocol_engine/commands/pipetting_common.py b/api/src/opentrons/protocol_engine/commands/pipetting_common.py index 99181ebc66b..7d70672886d 100644 --- a/api/src/opentrons/protocol_engine/commands/pipetting_common.py +++ b/api/src/opentrons/protocol_engine/commands/pipetting_common.py @@ -4,8 +4,8 @@ from ..types import WellLocation -class BasePipettingRequest(BaseModel): - """Base class for pipetting requests that interact with wells.""" +class BasePipettingData(BaseModel): + """Base class for data payloads of commands that interact with wells.""" pipetteId: str = Field( ..., @@ -21,13 +21,13 @@ class BasePipettingRequest(BaseModel): ) -class BaseLiquidHandlingRequest(BasePipettingRequest): - """Base class for liquid handling requests.""" +class BaseLiquidHandlingData(BasePipettingData): + """Base class for data payloads of commands that handle liquid.""" volume: float = Field( ..., description="Amount of liquid in uL. Must be greater than 0 and less " - "than a pipette-specific maximum volume.", + "than a pipette-specific maximum volume.", gt=0, ) wellLocation: WellLocation = Field( diff --git a/api/src/opentrons/protocol_engine/create_protocol_engine.py b/api/src/opentrons/protocol_engine/create_protocol_engine.py new file mode 100644 index 00000000000..3a23b047b5f --- /dev/null +++ b/api/src/opentrons/protocol_engine/create_protocol_engine.py @@ -0,0 +1,58 @@ +"""Main ProtocolEngine factory.""" +from opentrons.hardware_control.api import API as HardwareAPI + +from .protocol_engine import ProtocolEngine +from .resources import ResourceProviders +from .state import StateStore +from .commands import CommandMapper +from .execution import ( + CommandExecutor, + EquipmentHandler, + MovementHandler, + PipettingHandler, +) + + +async def create_protocol_engine(hardware: HardwareAPI) -> ProtocolEngine: + """Create a ProtocolEngine instance.""" + resources = ResourceProviders() + command_mapper = CommandMapper() + + # TODO(mc, 2020-11-18): check short trash FF + # TODO(mc, 2020-11-18): consider moving into a StateStore.create factory + deck_def = await resources.deck_data.get_deck_definition() + fixed_labware = await resources.deck_data.get_deck_fixed_labware(deck_def) + + state_store = StateStore( + deck_definition=deck_def, + deck_fixed_labware=fixed_labware, + ) + + equipment_handler = EquipmentHandler( + hardware=hardware, + state_store=state_store, + resources=resources, + ) + + movement_handler = MovementHandler(hardware=hardware, state_store=state_store) + + pipetting_handler = PipettingHandler( + hardware=hardware, + state_store=state_store, + movement_handler=movement_handler, + ) + + command_executor = CommandExecutor( + equipment=equipment_handler, + movement=movement_handler, + pipetting=pipetting_handler, + resources=resources, + command_mapper=command_mapper, + ) + + return ProtocolEngine( + state_store=state_store, + command_executor=command_executor, + command_mapper=command_mapper, + resources=resources, + ) diff --git a/api/src/opentrons/protocol_engine/errors/__init__.py b/api/src/opentrons/protocol_engine/errors/__init__.py index 577a952213e..9ce7bdb4c42 100644 --- a/api/src/opentrons/protocol_engine/errors/__init__.py +++ b/api/src/opentrons/protocol_engine/errors/__init__.py @@ -40,6 +40,12 @@ class PipetteNotAttachedError(ProtocolEngineError): pass +class CommandDoesNotExistError(ProtocolEngineError): + """An error raised when referencing a command that does not exist.""" + + pass + + class LabwareDoesNotExistError(ProtocolEngineError): """An error raised when referencing a labware that does not exist.""" diff --git a/api/src/opentrons/protocol_engine/execution/__init__.py b/api/src/opentrons/protocol_engine/execution/__init__.py index a69ffdf5e46..956c0ce4ce7 100644 --- a/api/src/opentrons/protocol_engine/execution/__init__.py +++ b/api/src/opentrons/protocol_engine/execution/__init__.py @@ -1,12 +1,12 @@ """Command execution module.""" -from .command_handlers import CommandHandlers +from .command_executor import CommandExecutor from .equipment import EquipmentHandler, LoadedLabware, LoadedPipette from .movement import MovementHandler from .pipetting import PipettingHandler __all__ = [ - "CommandHandlers", + "CommandExecutor", "EquipmentHandler", "LoadedLabware", "LoadedPipette", diff --git a/api/src/opentrons/protocol_engine/execution/command_executor.py b/api/src/opentrons/protocol_engine/execution/command_executor.py new file mode 100644 index 00000000000..8c19fd0265e --- /dev/null +++ b/api/src/opentrons/protocol_engine/execution/command_executor.py @@ -0,0 +1,69 @@ +"""Command side-effect execution logic container.""" +from logging import getLogger + +from ..resources import ResourceProviders +from ..commands import Command, CommandStatus, CommandMapper +from .equipment import EquipmentHandler +from .movement import MovementHandler +from .pipetting import PipettingHandler + +log = getLogger(__name__) + + +class CommandExecutor: + """CommandExecutor container class. + + CommandExecutor manages various child handlers that define procedures to + execute the side-effects of commands. + """ + + def __init__( + self, + equipment: EquipmentHandler, + movement: MovementHandler, + pipetting: PipettingHandler, + command_mapper: CommandMapper, + resources: ResourceProviders, + ) -> None: + """Initialize the CommandExecutor with access to its dependencies.""" + self._equipment = equipment + self._movement = movement + self._pipetting = pipetting + self._command_mapper = command_mapper + self._resources = resources + + async def execute(self, command: Command) -> Command: + """Run a given command's execution procedure.""" + command_impl = command._ImplementationCls( + equipment=self._equipment, + movement=self._movement, + pipetting=self._pipetting, + ) + + result = None + error = None + status = CommandStatus.SUCCEEDED + + try: + log.debug(f"Executing {command.id}, {command.commandType}, {command.data}") + result = await command_impl.execute(command.data) # type: ignore[arg-type] + except Exception as e: + log.warn( + f"Execution of {command.id} failed", + exc_info=e, + ) + # TODO(mc, 2021-06-22): differentiate between `ProtocolEngineError`s + # and unexpected errors when the Command model is ready to accept + # structured error details + error = str(e) + status = CommandStatus.FAILED + + completed_at = self._resources.model_utils.get_timestamp() + + return self._command_mapper.update_command( + command=command, + result=result, + error=error, + status=status, + completedAt=completed_at, + ) diff --git a/api/src/opentrons/protocol_engine/execution/command_handlers.py b/api/src/opentrons/protocol_engine/execution/command_handlers.py deleted file mode 100644 index c4a3d0c3b48..00000000000 --- a/api/src/opentrons/protocol_engine/execution/command_handlers.py +++ /dev/null @@ -1,65 +0,0 @@ -"""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 StateView -from .equipment import EquipmentHandler -from .movement import MovementHandler -from .pipetting import PipettingHandler - - -class CommandHandlers: - """CommandHandlers container class. - - CommandHandlers wraps various child handlers that define procedures to - execute the side-effects of commands. - """ - - _equipment: EquipmentHandler - _movement: MovementHandler - _pipetting: PipettingHandler - - def __init__( - self, - hardware: HardwareAPI, - state: StateView, - resources: ResourceProviders, - ) -> None: - """Initialize a CommandHandlers container and its child handlers.""" - equipment = EquipmentHandler( - state=state, - hardware=hardware, - resources=resources, - ) - - movement = MovementHandler( - state=state, - hardware=hardware - ) - - pipetting = PipettingHandler( - state=state, - hardware=hardware, - movement_handler=movement, - ) - - self._equipment = equipment - self._movement = movement - self._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 diff --git a/api/src/opentrons/protocol_engine/execution/equipment.py b/api/src/opentrons/protocol_engine/execution/equipment.py index fd295800739..f68abcf1aa2 100644 --- a/api/src/opentrons/protocol_engine/execution/equipment.py +++ b/api/src/opentrons/protocol_engine/execution/equipment.py @@ -9,7 +9,7 @@ from ..errors import FailedToLoadPipetteError, LabwareDefinitionDoesNotExistError from ..resources import ResourceProviders -from ..state import StateView +from ..state import StateStore, StateView from ..types import LabwareLocation, PipetteName @@ -33,27 +33,31 @@ class EquipmentHandler: """Implementation logic for labware, pipette, and module loading.""" _hardware: HardwareAPI - _state: StateView + _state_store: StateStore _resources: ResourceProviders def __init__( self, hardware: HardwareAPI, - state: StateView, + state_store: StateStore, resources: ResourceProviders, ) -> None: """Initialize an EquipmentHandler instance.""" self._hardware = hardware - self._state = state + self._state_store = state_store self._resources = resources + @property + def _state(self) -> StateView: + return self._state_store.state_view + async def load_labware( self, load_name: str, namespace: str, version: int, location: LabwareLocation, - labware_id: Optional[str] + labware_id: Optional[str], ) -> LoadedLabware: """Load labware by assigning an identifier and pulling required data. @@ -68,8 +72,9 @@ async def load_labware( Returns: A LoadedLabware object. """ - labware_id = labware_id if labware_id else \ - self._resources.id_generator.generate_id() + labware_id = ( + labware_id if labware_id else self._resources.model_utils.generate_id() + ) try: # Try to use existing definition in state. @@ -133,7 +138,10 @@ async def load_pipette( except RuntimeError as e: raise FailedToLoadPipetteError(str(e)) from e - pipette_id = pipette_id if pipette_id is not None else \ - self._resources.id_generator.generate_id() + pipette_id = ( + pipette_id + if pipette_id is not None + else self._resources.model_utils.generate_id() + ) return LoadedPipette(pipette_id=pipette_id) diff --git a/api/src/opentrons/protocol_engine/execution/movement.py b/api/src/opentrons/protocol_engine/execution/movement.py index 9636a7b89e7..cc8b43553e4 100644 --- a/api/src/opentrons/protocol_engine/execution/movement.py +++ b/api/src/opentrons/protocol_engine/execution/movement.py @@ -3,24 +3,28 @@ from opentrons.hardware_control.api import API as HardwareAPI from ..types import DeckLocation, WellLocation -from ..state import StateView +from ..state import StateStore, StateView class MovementHandler: """Implementation logic for gantry movement.""" - _state: StateView + _state_store: StateStore _hardware: HardwareAPI def __init__( self, - state: StateView, + state_store: StateStore, hardware: HardwareAPI, ) -> None: """Initialize a MovementHandler instance.""" - self._state = state + self._state_store = state_store self._hardware = hardware + @property + def _state(self) -> StateView: + return self._state_store.state_view + async def move_to_well( self, pipette_id: str, @@ -63,5 +67,5 @@ async def move_to_well( await self._hardware.move_to( mount=hw_mount, abs_position=wp.position, - critical_point=wp.critical_point + critical_point=wp.critical_point, ) diff --git a/api/src/opentrons/protocol_engine/execution/pipetting.py b/api/src/opentrons/protocol_engine/execution/pipetting.py index cd9ccd8b195..8abbd6d0f09 100644 --- a/api/src/opentrons/protocol_engine/execution/pipetting.py +++ b/api/src/opentrons/protocol_engine/execution/pipetting.py @@ -1,7 +1,7 @@ """Pipetting command handling.""" from opentrons.hardware_control.api import API as HardwareAPI -from ..state import StateView +from ..state import StateStore, StateView from ..types import DeckLocation, WellLocation, WellOrigin from .movement import MovementHandler @@ -9,21 +9,25 @@ class PipettingHandler: """Implementation logic for liquid handling commands.""" - _state: StateView + _state_store: StateStore _hardware: HardwareAPI _movement_handler: MovementHandler def __init__( self, - state: StateView, + state_store: StateStore, hardware: HardwareAPI, movement_handler: MovementHandler, ) -> None: """Initialize a PipettingHandler instance.""" - self._state = state + self._state_store = state_store self._hardware = hardware self._movement_handler = movement_handler + @property + def _state(self) -> StateView: + return self._state_store.state_view + async def pick_up_tip( self, pipette_id: str, @@ -34,7 +38,7 @@ async def pick_up_tip( # get mount and config data from state and hardware controller hw_pipette = self._state.pipettes.get_hardware_pipette( pipette_id=pipette_id, - attached_pipettes=self._hardware.attached_instruments + attached_pipettes=self._hardware.attached_instruments, ) # use config data to get tip geometry (length, diameter, volume) @@ -57,17 +61,17 @@ async def pick_up_tip( tip_length=tip_geometry.effective_length, # TODO(mc, 2020-11-12): include these parameters in the request presses=None, - increment=None + increment=None, ) # after a successful pickup, update the hardware controller state self._hardware.set_current_tiprack_diameter( mount=hw_pipette.mount, - tiprack_diameter=tip_geometry.diameter + tiprack_diameter=tip_geometry.diameter, ) self._hardware.set_working_volume( mount=hw_pipette.mount, - tip_volume=tip_geometry.volume + tip_volume=tip_geometry.volume, ) async def drop_tip( @@ -80,7 +84,7 @@ async def drop_tip( # get mount and config data from state and hardware controller hw_pipette = self._state.pipettes.get_hardware_pipette( pipette_id=pipette_id, - attached_pipettes=self._hardware.attached_instruments + attached_pipettes=self._hardware.attached_instruments, ) # get the tip drop location @@ -101,7 +105,7 @@ async def drop_tip( await self._hardware.drop_tip( mount=hw_pipette.mount, # TODO(mc, 2020-11-12): include this parameter in the request - home_after=True + home_after=True, ) async def aspirate( @@ -116,7 +120,7 @@ async def aspirate( # get mount and config data from state and hardware controller hw_pipette = self._state.pipettes.get_hardware_pipette( pipette_id=pipette_id, - attached_pipettes=self._hardware.attached_instruments + attached_pipettes=self._hardware.attached_instruments, ) ready_to_aspirate = self._state.pipettes.get_is_ready_to_aspirate( @@ -141,7 +145,8 @@ async def aspirate( current_location = DeckLocation( pipette_id=pipette_id, labware_id=labware_id, - well_name=well_name) + well_name=well_name, + ) await self._movement_handler.move_to_well( pipette_id=pipette_id, @@ -166,7 +171,7 @@ async def dispense( """Dispense liquid to a well.""" hw_pipette = self._state.pipettes.get_hardware_pipette( pipette_id=pipette_id, - attached_pipettes=self._hardware.attached_instruments + attached_pipettes=self._hardware.attached_instruments, ) await self._movement_handler.move_to_well( diff --git a/api/src/opentrons/protocol_engine/protocol_engine.py b/api/src/opentrons/protocol_engine/protocol_engine.py index 115bc1d4cd8..519f521efd9 100644 --- a/api/src/opentrons/protocol_engine/protocol_engine.py +++ b/api/src/opentrons/protocol_engine/protocol_engine.py @@ -1,20 +1,8 @@ """ProtocolEngine class definition.""" -from __future__ import annotations -from typing import Union - -from opentrons.hardware_control.api import API as HardwareAPI -from opentrons.util.helpers import utc_now - -from .errors import ProtocolEngineError, UnexpectedProtocolError -from .execution import CommandHandlers from .resources import ResourceProviders from .state import State, StateStore, StateView -from .commands import ( - CommandRequestType, - PendingCommandType, - CompletedCommandType, - FailedCommandType, -) +from .commands import Command, CommandRequest, CommandStatus, CommandMapper +from .execution import CommandExecutor class ProtocolEngine: @@ -25,30 +13,16 @@ class ProtocolEngine: of the commands themselves. """ - _hardware: HardwareAPI _state_store: StateStore + _command_executor: CommandExecutor + _command_mapper: CommandMapper _resources: ResourceProviders - @classmethod - async def create(cls, hardware: HardwareAPI) -> ProtocolEngine: - """Create a ProtocolEngine instance.""" - resources = ResourceProviders.create() - - # TODO(mc, 2020-11-18): check short trash FF - # TODO(mc, 2020-11-18): consider moving into a StateStore.create factory - deck_def = await resources.deck_data.get_deck_definition() - fixed_labware = await resources.deck_data.get_deck_fixed_labware(deck_def) - - state_store = StateStore( - deck_definition=deck_def, deck_fixed_labware=fixed_labware - ) - - return cls(state_store=state_store, resources=resources, hardware=hardware) - def __init__( self, - hardware: HardwareAPI, state_store: StateStore, + command_executor: CommandExecutor, + command_mapper: CommandMapper, resources: ResourceProviders, ) -> None: """Initialize a ProtocolEngine instance. @@ -56,8 +30,9 @@ def __init__( This constructor does not inject provider implementations. Prefer the ProtocolEngine.create factory classmethod. """ - self._hardware = hardware self._state_store = state_store + self._command_executor = command_executor + self._command_mapper = command_mapper self._resources = resources @property @@ -69,56 +44,52 @@ def get_state(self) -> State: """Get the engine's underlying state.""" return self._state_store.get_state() - async def execute_command( - self, - request: CommandRequestType, - command_id: str, - ) -> Union[CompletedCommandType, FailedCommandType]: - """Execute a command request, waiting for it to complete.""" - cmd_impl = request.get_implementation() - created_at = utc_now() - cmd = cmd_impl.create_command(created_at).to_running(created_at) - done_cmd: Union[CompletedCommandType, FailedCommandType] - - # store the command prior to execution - self._state_store.handle_command(cmd, command_id=command_id) - - # 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, + def add_command(self, request: CommandRequest) -> Command: + """Add a command to ProtocolEngine.""" + command = self._command_mapper.map_request_to_command( + request=request, + command_id=self._resources.model_utils.generate_id(), + created_at=self._resources.model_utils.get_timestamp(), ) + self._state_store.handle_command(command) + return command - # execute the command - try: - result = await cmd_impl.execute(handlers) - completed_at = utc_now() - done_cmd = cmd.to_completed(result, completed_at) + async def execute_command_by_id(self, command_id: str) -> Command: + """Execute a protocol engine command by its identifier.""" + queued_command = self.state_view.commands.get_command_by_id(command_id) - except Exception as error: - failed_at = utc_now() - if not isinstance(error, ProtocolEngineError): - error = UnexpectedProtocolError(error) - done_cmd = cmd.to_failed(error, failed_at) + running_command = self._command_mapper.update_command( + command=queued_command, + startedAt=self._resources.model_utils.get_timestamp(), + status=CommandStatus.RUNNING, + ) - # store the done command - self._state_store.handle_command(done_cmd, command_id=command_id) + self._state_store.handle_command(running_command) + completed_command = await self._command_executor.execute(running_command) + self._state_store.handle_command(completed_command) - return done_cmd + return completed_command - def add_command(self, request: CommandRequestType) -> PendingCommandType: - """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 = self._resources.id_generator.generate_id() - created_at = utc_now() - command_impl = request.get_implementation() - command = command_impl.create_command(created_at) + async def execute_command( + self, request: CommandRequest, command_id: str + ) -> Command: + """Execute a command request, waiting for it to complete.""" + created_at = self._resources.model_utils.get_timestamp() - self._state_store.handle_command(command, command_id=command_id) + queued_command = self._command_mapper.map_request_to_command( + request=request, + command_id=command_id, + created_at=created_at, + ) - return command + running_command = self._command_mapper.update_command( + command=queued_command, + startedAt=created_at, + status=CommandStatus.RUNNING, + ) + + self._state_store.handle_command(running_command) + completed_command = await self._command_executor.execute(running_command) + self._state_store.handle_command(completed_command) + + return completed_command diff --git a/api/src/opentrons/protocol_engine/resources/__init__.py b/api/src/opentrons/protocol_engine/resources/__init__.py index 5d81fa5ea03..e3840153344 100644 --- a/api/src/opentrons/protocol_engine/resources/__init__.py +++ b/api/src/opentrons/protocol_engine/resources/__init__.py @@ -1,13 +1,13 @@ """Resources used by command execution handlers.""" from .resource_providers import ResourceProviders -from .id_generator import IdGenerator +from .model_utils import ModelUtils from .deck_data_provider import DeckDataProvider, DeckFixedLabware from .labware_data_provider import LabwareDataProvider __all__ = [ "ResourceProviders", - "IdGenerator", + "ModelUtils", "LabwareDataProvider", "DeckDataProvider", "DeckFixedLabware", diff --git a/api/src/opentrons/protocol_engine/resources/id_generator.py b/api/src/opentrons/protocol_engine/resources/id_generator.py deleted file mode 100644 index 2d01f7cdaae..00000000000 --- a/api/src/opentrons/protocol_engine/resources/id_generator.py +++ /dev/null @@ -1,14 +0,0 @@ -"""Unique ID generation provider.""" - -from uuid import uuid4 - - -class IdGenerator: - """Unique ID generation provider.""" - - def generate_id(self) -> str: - """Generate a unique identifier. - - Uses UUIDv4 for safety in a multiprocessing environment. - """ - return str(uuid4()) diff --git a/api/src/opentrons/protocol_engine/resources/model_utils.py b/api/src/opentrons/protocol_engine/resources/model_utils.py new file mode 100644 index 00000000000..482233f0b3c --- /dev/null +++ b/api/src/opentrons/protocol_engine/resources/model_utils.py @@ -0,0 +1,20 @@ +"""Unique ID generation provider.""" +from datetime import datetime, timezone +from uuid import uuid4 + + +class ModelUtils: + """Common resource model utilities provider.""" + + @staticmethod + def generate_id() -> str: + """Generate a unique identifier. + + Uses UUIDv4 for safety in a multiprocessing environment. + """ + return str(uuid4()) + + @staticmethod + def get_timestamp() -> datetime: + """Get a timestamp of the current time.""" + return datetime.now(tz=timezone.utc) diff --git a/api/src/opentrons/protocol_engine/resources/resource_providers.py b/api/src/opentrons/protocol_engine/resources/resource_providers.py index b2d633c6696..5aa9fda9946 100644 --- a/api/src/opentrons/protocol_engine/resources/resource_providers.py +++ b/api/src/opentrons/protocol_engine/resources/resource_providers.py @@ -1,7 +1,7 @@ """Resource providers.""" from __future__ import annotations -from .id_generator import IdGenerator +from .model_utils import ModelUtils from .deck_data_provider import DeckDataProvider from .labware_data_provider import LabwareDataProvider @@ -13,38 +13,20 @@ class ResourceProviders: data for engine setup and command execution. """ - _id_generator: IdGenerator + _model_utils: ModelUtils _labware_data: LabwareDataProvider _deck_data: DeckDataProvider - @classmethod - def create(cls) -> ResourceProviders: - """Create a ResourceProviders container and its children.""" - id_generator = IdGenerator() - labware_data = LabwareDataProvider() - deck_data = DeckDataProvider(labware_data=labware_data) - - return cls( - id_generator=id_generator, - labware_data=labware_data, - deck_data=deck_data, - ) - - def __init__( - self, - id_generator: IdGenerator, - labware_data: LabwareDataProvider, - deck_data: DeckDataProvider, - ) -> None: + def __init__(self) -> None: """Initialize a ResourceProviders container.""" - self._id_generator = id_generator - self._labware_data = labware_data - self._deck_data = deck_data + self._model_utils = ModelUtils() + self._labware_data = LabwareDataProvider() + self._deck_data = DeckDataProvider(labware_data=self._labware_data) @property - def id_generator(self) -> IdGenerator: - """Get the unique ID generator resource.""" - return self._id_generator + def model_utils(self) -> ModelUtils: + """Get an interface to ID and timestamp generation utilities.""" + return self._model_utils @property def labware_data(self) -> LabwareDataProvider: diff --git a/api/src/opentrons/protocol_engine/state/commands.py b/api/src/opentrons/protocol_engine/state/commands.py index e8926d954d0..582b5472cce 100644 --- a/api/src/opentrons/protocol_engine/state/commands.py +++ b/api/src/opentrons/protocol_engine/state/commands.py @@ -4,8 +4,9 @@ from dataclasses import dataclass, replace from typing import List, Optional, Tuple -from ..commands import CommandType, CommandRequestType, PendingCommand, FailedCommand -from .substore import Substore +from ..commands import Command, CommandStatus +from ..errors import CommandDoesNotExistError +from .substore import Substore, CommandReactive @dataclass(frozen=True) @@ -13,10 +14,10 @@ class CommandState: """State of all protocol engine command resources.""" # TODO(mc, 2021-06-16): OrderedDict is mutable. Switch to Sequence + Mapping - commands_by_id: OrderedDict[str, CommandType] + commands_by_id: OrderedDict[str, Command] -class CommandStore(Substore[CommandState]): +class CommandStore(Substore[CommandState], CommandReactive): """Command state container.""" _state: CommandState @@ -25,10 +26,10 @@ def __init__(self) -> None: """Initialize a CommandStore and its state.""" self._state = CommandState(commands_by_id=OrderedDict()) - def handle_command(self, command: CommandType, command_id: str) -> None: + def handle_command(self, command: Command) -> None: """Modify state in reaction to any command.""" commands_by_id = self._state.commands_by_id.copy() - commands_by_id.update({command_id: command}) + commands_by_id.update({command.id: command}) self._state = replace(self._state, commands_by_id=commands_by_id) @@ -40,12 +41,14 @@ def __init__(self, state: CommandState) -> None: """Initialize the view of command state with its underlying data.""" self._state = state - def get_command_by_id(self, uid: str) -> Optional[CommandType]: + def get_command_by_id(self, command_id: str) -> Command: """Get a command by its unique identifier.""" - # TODO(mc, 2021-06-17): raise on missing ID, to line up with other state views - return self._state.commands_by_id.get(uid) + try: + return self._state.commands_by_id[command_id] + except KeyError: + raise CommandDoesNotExistError(f"Command {command_id} does not exist") - def get_all_commands(self) -> List[Tuple[str, CommandType]]: + def get_all_commands(self) -> List[Tuple[str, Command]]: """Get a list of all commands in state, paired with their respective IDs. Entries are returned in the order of first-added command to last-added command. @@ -54,7 +57,7 @@ def get_all_commands(self) -> List[Tuple[str, CommandType]]: """ return [entry for entry in self._state.commands_by_id.items()] - def get_next_request(self) -> Optional[Tuple[str, CommandRequestType]]: + def get_next_command(self) -> Optional[str]: """Return the next request in line to be executed. Normally, this corresponds to the earliest-added command that's currently @@ -67,8 +70,8 @@ def get_next_request(self) -> Optional[Tuple[str, CommandRequestType]]: If there are no pending commands at all, returns None. """ for command_id, command in self._state.commands_by_id.items(): - if isinstance(command, FailedCommand): + if command.status == CommandStatus.FAILED: return None - elif isinstance(command, PendingCommand): - return command_id, command.request + elif command.status == CommandStatus.QUEUED: + return command_id return None diff --git a/api/src/opentrons/protocol_engine/state/geometry.py b/api/src/opentrons/protocol_engine/state/geometry.py index f99d23b4e42..28dfc535bd0 100644 --- a/api/src/opentrons/protocol_engine/state/geometry.py +++ b/api/src/opentrons/protocol_engine/state/geometry.py @@ -5,7 +5,6 @@ from opentrons.types import Point from opentrons.hardware_control.dev_types import PipetteDict -from opentrons.protocols.geometry.deck import FIXED_TRASH_ID from .. import errors from ..types import WellLocation, WellOrigin @@ -174,7 +173,12 @@ def get_tip_drop_location( ) -> WellLocation: """Get tip drop location given labware and hardware pipette.""" # return to top if labware is fixed trash - if labware_id == FIXED_TRASH_ID: + is_fixed_trash = self._labware.get_labware_has_quirk( + labware_id=labware_id, + quirk="fixedTrash", + ) + + if is_fixed_trash: return WellLocation() nominal_length = self._labware.get_tip_length(labware_id) diff --git a/api/src/opentrons/protocol_engine/state/labware.py b/api/src/opentrons/protocol_engine/state/labware.py index c10ddf0cdc4..0364acea9b1 100644 --- a/api/src/opentrons/protocol_engine/state/labware.py +++ b/api/src/opentrons/protocol_engine/state/labware.py @@ -12,11 +12,7 @@ from .. import errors from ..resources import DeckFixedLabware -from ..commands import ( - CompletedCommandType, - LoadLabwareResult, - AddLabwareDefinitionResult, -) +from ..commands import Command, LoadLabwareResult, AddLabwareDefinitionResult from ..types import LabwareLocation, Dimensions from .substore import Substore, CommandReactive @@ -77,8 +73,8 @@ def __init__( deck_definition=deck_definition, ) - def handle_completed_command(self, command: CompletedCommandType) -> None: - """Modify state in reaction to a completed command.""" + def handle_command(self, command: Command) -> None: + """Modify state in reaction to a command.""" if isinstance(command.result, LoadLabwareResult): uri = uri_from_details( namespace=command.result.definition.namespace, @@ -87,7 +83,7 @@ def handle_completed_command(self, command: CompletedCommandType) -> None: ) self._state.labware_definitions_by_uri[uri] = command.result.definition self._state.labware_by_id[command.result.labwareId] = LabwareData( - location=command.request.location, + location=command.data.location, uri=uri, calibration=command.result.calibration, ) @@ -97,7 +93,7 @@ def handle_completed_command(self, command: CompletedCommandType) -> None: load_name=command.result.loadName, version=command.result.version, ) - self._state.labware_definitions_by_uri[uri] = command.request.definition + self._state.labware_definitions_by_uri[uri] = command.data.definition class LabwareView: diff --git a/api/src/opentrons/protocol_engine/state/pipettes.py b/api/src/opentrons/protocol_engine/state/pipettes.py index 663812a32a2..24151b2cca3 100644 --- a/api/src/opentrons/protocol_engine/state/pipettes.py +++ b/api/src/opentrons/protocol_engine/state/pipettes.py @@ -10,7 +10,7 @@ from ..types import PipetteName, DeckLocation from ..commands import ( - CompletedCommandType, + Command, LoadPipetteResult, AspirateResult, DispenseResult, @@ -59,7 +59,7 @@ def __init__(self) -> None: current_location=None, ) - def handle_completed_command(self, command: CompletedCommandType) -> None: + def handle_command(self, command: Command) -> None: """Modify state in reaction to a completed command.""" if isinstance( command.result, @@ -74,9 +74,9 @@ def handle_completed_command(self, command: CompletedCommandType) -> None: self._state = replace( self._state, current_location=DeckLocation( - pipette_id=command.request.pipetteId, - labware_id=command.request.labwareId, - well_name=command.request.wellName, + pipette_id=command.data.pipetteId, + labware_id=command.data.labwareId, + well_name=command.data.wellName, ), ) @@ -86,8 +86,8 @@ def handle_completed_command(self, command: CompletedCommandType) -> None: aspirated_volume_by_id = self._state.aspirated_volume_by_id.copy() pipettes_by_id[pipette_id] = PipetteData( - pipette_name=command.request.pipetteName, - mount=command.request.mount, + pipette_name=command.data.pipetteName, + mount=command.data.mount, ) aspirated_volume_by_id[pipette_id] = 0 @@ -98,7 +98,7 @@ def handle_completed_command(self, command: CompletedCommandType) -> None: ) elif isinstance(command.result, AspirateResult): - pipette_id = command.request.pipetteId + pipette_id = command.data.pipetteId aspirated_volume_by_id = self._state.aspirated_volume_by_id.copy() previous_volume = self._state.aspirated_volume_by_id[pipette_id] @@ -111,7 +111,7 @@ def handle_completed_command(self, command: CompletedCommandType) -> None: ) elif isinstance(command.result, DispenseResult): - pipette_id = command.request.pipetteId + pipette_id = command.data.pipetteId aspirated_volume_by_id = self._state.aspirated_volume_by_id.copy() previous_volume = self._state.aspirated_volume_by_id[pipette_id] diff --git a/api/src/opentrons/protocol_engine/state/state.py b/api/src/opentrons/protocol_engine/state/state.py index ed60153533f..7ce5bfc145c 100644 --- a/api/src/opentrons/protocol_engine/state/state.py +++ b/api/src/opentrons/protocol_engine/state/state.py @@ -46,6 +46,7 @@ def __init__( ) self._lifecycle_substores: List[CommandReactive] = [ + self._command_store, self._pipette_store, self._labware_store, ] @@ -61,13 +62,10 @@ def get_state(self) -> State: """Get an immutable copy of the current engine state.""" return self._state - def handle_command(self, command: cmd.CommandType, command_id: str) -> None: + def handle_command(self, command: cmd.Command) -> None: """Modify State in reaction to a Command.""" - self._command_store.handle_command(command, command_id) - - if isinstance(command, cmd.CompletedCommand): - for substore in self._lifecycle_substores: - substore.handle_completed_command(command) + for substore in self._lifecycle_substores: + substore.handle_command(command) self._update_state() diff --git a/api/src/opentrons/protocol_engine/state/substore.py b/api/src/opentrons/protocol_engine/state/substore.py index 15055e672f0..5d79b3186dd 100644 --- a/api/src/opentrons/protocol_engine/state/substore.py +++ b/api/src/opentrons/protocol_engine/state/substore.py @@ -1,8 +1,8 @@ """Base state store classes.""" -from abc import ABC +from abc import ABC, abstractmethod from typing import Generic, TypeVar -from ..commands import CompletedCommandType +from ..commands import Command SubstateT = TypeVar("SubstateT") @@ -22,6 +22,7 @@ def state(self) -> SubstateT: class CommandReactive(ABC): """Abstract base class for an interface that reacts to commands.""" - def handle_completed_command(self, command: CompletedCommandType) -> None: - """React to a CompletedCommand.""" - pass + @abstractmethod + def handle_command(self, command: Command) -> None: + """React to a Command resource change.""" + ... diff --git a/api/src/opentrons/protocols/runner/json_proto/command_translator.py b/api/src/opentrons/protocols/runner/json_proto/command_translator.py index 7dcc276a127..45d9be6243f 100644 --- a/api/src/opentrons/protocols/runner/json_proto/command_translator.py +++ b/api/src/opentrons/protocols/runner/json_proto/command_translator.py @@ -1,18 +1,20 @@ from typing import Dict, List -import opentrons.types - -from opentrons import protocol_engine as pe +from opentrons.types import DeckSlotName, MountType from opentrons.protocols import models +from opentrons.protocol_engine import ( + commands as pe_commands, + DeckSlotLocation, + PipetteName, + WellLocation, + WellOrigin, +) class CommandTranslatorError(Exception): pass -PECommands = List[pe.commands.CommandRequestType] - - class CommandTranslator: """Class that translates commands from PD/JSON to ProtocolEngine.""" @@ -20,31 +22,35 @@ def __init__(self) -> None: """Construct a command translator""" pass - def translate(self, protocol: models.JsonProtocol) -> PECommands: + def translate( + self, + protocol: models.JsonProtocol, + ) -> List[pe_commands.CommandRequest]: """Return all Protocol Engine commands required to run the given protocol.""" - result: List[pe.commands.CommandRequestType] = [] + result: List[pe_commands.CommandRequest] = [] for pipette_id, pipette in protocol.pipettes.items(): - result += [self._translate_load_pipette(pipette_id, pipette)] + result.append(self._translate_load_pipette(pipette_id, pipette)) for definition_id, definition in protocol.labwareDefinitions.items(): - result += [self._translate_add_labware_definition(definition)] + result.append(self._translate_add_labware_definition(definition)) for labware_id, labware in protocol.labware.items(): - result += [ + result.append( self._translate_load_labware( labware_id, labware, protocol.labwareDefinitions ) - ] + ) for command in protocol.commands: - result += self._translate_command(command) + result.append(self._translate_command(command)) return result def _translate_command( - self, - command: models.json_protocol.AllCommands) -> PECommands: + self, + command: models.json_protocol.AllCommands, + ) -> pe_commands.CommandRequest: """ Args: command: The PD/JSON command to translate @@ -57,24 +63,25 @@ def _translate_command( h = CommandTranslator._COMMAND_TO_NAME[command.command] return getattr(self, h)(command) except KeyError: - raise CommandTranslatorError( - f"'{command.command}' is not recognized.") + raise CommandTranslatorError(f"'{command.command}' is not recognized.") except AttributeError: raise CommandTranslatorError( - f"Cannot find handler for '{command.command}'.") + f"Cannot find handler for '{command.command}'." + ) def _translate_add_labware_definition( - self, - labware_definition: models.LabwareDefinition - ) -> pe.commands.AddLabwareDefinitionRequest: - return pe.commands.AddLabwareDefinitionRequest(definition=labware_definition) + self, labware_definition: models.LabwareDefinition + ) -> pe_commands.AddLabwareDefinitionRequest: + return pe_commands.AddLabwareDefinitionRequest( + data=pe_commands.AddLabwareDefinitionData(definition=labware_definition) + ) def _translate_load_labware( - self, - labware_id: str, - labware: models.json_protocol.Labware, - labware_definitions: Dict[str, models.LabwareDefinition] - ) -> pe.commands.LoadLabwareRequest: + self, + labware_id: str, + labware: models.json_protocol.Labware, + labware_definitions: Dict[str, models.LabwareDefinition], + ) -> pe_commands.LoadLabwareRequest: """ Args: labware_id: @@ -87,29 +94,35 @@ def _translate_load_labware( The JSON protocol's collection of labware definitions. """ definition = labware_definitions[labware.definitionId] - return pe.commands.LoadLabwareRequest( - location=pe.DeckSlotLocation( - slot=opentrons.types.DeckSlotName.from_primitive(labware.slot) - ), - loadName=definition.parameters.loadName, - namespace=definition.namespace, - version=definition.version, - labwareId=labware_id + return pe_commands.LoadLabwareRequest( + data=pe_commands.LoadLabwareData( + location=DeckSlotLocation( + slot=DeckSlotName.from_primitive(labware.slot) + ), + loadName=definition.parameters.loadName, + namespace=definition.namespace, + version=definition.version, + labwareId=labware_id, + ) ) def _translate_load_pipette( - self, - pipette_id: str, - pipette: models.json_protocol.Pipettes) -> pe.commands.LoadPipetteRequest: - return pe.commands.LoadPipetteRequest( - pipetteName=pe.PipetteName(pipette.name), - mount=opentrons.types.MountType(pipette.mount), - pipetteId=pipette_id + self, + pipette_id: str, + pipette: models.json_protocol.Pipettes, + ) -> pe_commands.LoadPipetteRequest: + return pe_commands.LoadPipetteRequest( + data=pe_commands.LoadPipetteData( + pipetteName=PipetteName(pipette.name), + mount=MountType(pipette.mount), + pipetteId=pipette_id, + ) ) def _aspirate( - self, - command: models.json_protocol.LiquidCommand) -> PECommands: + self, + command: models.json_protocol.LiquidCommand, + ) -> pe_commands.AspirateRequest: """ Translate an aspirate JSON command to a protocol engine aspirate request. @@ -119,22 +132,23 @@ def _aspirate( Returns: AspirateRequest """ - return [ - pe.commands.AspirateRequest( + return pe_commands.AspirateRequest( + data=pe_commands.AspirateData( pipetteId=command.params.pipette, labwareId=command.params.labware, wellName=command.params.well, volume=command.params.volume, - wellLocation=pe.WellLocation( - origin=pe.WellOrigin.BOTTOM, - offset=(0, 0, command.params.offsetFromBottomMm) - ) + wellLocation=WellLocation( + origin=WellOrigin.BOTTOM, + offset=(0, 0, command.params.offsetFromBottomMm), + ), ) - ] + ) def _dispense( - self, - command: models.json_protocol.LiquidCommand) -> PECommands: + self, + command: models.json_protocol.LiquidCommand, + ) -> pe_commands.DispenseRequest: """ Translate a dispense JSON command to a protocol engine dispense request. @@ -143,37 +157,32 @@ def _dispense( Returns: DispenseRequest """ - return [ - pe.commands.DispenseRequest( + return pe_commands.DispenseRequest( + data=pe_commands.DispenseData( pipetteId=command.params.pipette, labwareId=command.params.labware, wellName=command.params.well, volume=command.params.volume, - wellLocation=pe.WellLocation( - origin=pe.WellOrigin.BOTTOM, - offset=(0, 0, command.params.offsetFromBottomMm) - ) + wellLocation=WellLocation( + origin=WellOrigin.BOTTOM, + offset=(0, 0, command.params.offsetFromBottomMm), + ), ) - ] + ) - def _air_gap( - self, - command: models.json_protocol.LiquidCommand) -> PECommands: + def _air_gap(self, command: models.json_protocol.LiquidCommand) -> None: raise NotImplementedError() - def _blowout( - self, - command: models.json_protocol.BlowoutCommand) -> PECommands: + def _blowout(self, command: models.json_protocol.BlowoutCommand) -> None: raise NotImplementedError() - def _touch_tip( - self, - command: models.json_protocol.TouchTipCommand) -> PECommands: + def _touch_tip(self, command: models.json_protocol.TouchTipCommand) -> None: raise NotImplementedError() def _pick_up( - self, - command: models.json_protocol.PickUpDropTipCommand) -> PECommands: + self, + command: models.json_protocol.PickUpDropTipCommand, + ) -> pe_commands.PickUpTipRequest: """ Translate a pick_up_tip JSON command to a protocol engine pick_up_tip request. @@ -182,17 +191,18 @@ def _pick_up( Returns: PickUpTipRequest """ - return [ - pe.commands.PickUpTipRequest( + return pe_commands.PickUpTipRequest( + data=pe_commands.PickUpTipData( pipetteId=command.params.pipette, labwareId=command.params.labware, - wellName=command.params.well + wellName=command.params.well, ) - ] + ) def _drop_tip( - self, - command: models.json_protocol.PickUpDropTipCommand) -> PECommands: + self, + command: models.json_protocol.PickUpDropTipCommand, + ) -> pe_commands.DropTipRequest: """ Translate a drop tip JSON command to a protocol engine drop tip request. @@ -202,163 +212,141 @@ def _drop_tip( Returns: DropTipRequest """ - return [ - pe.commands.DropTipRequest( + return pe_commands.DropTipRequest( + data=pe_commands.DropTipData( pipetteId=command.params.pipette, labwareId=command.params.labware, - wellName=command.params.well + wellName=command.params.well, ) - ] + ) def _move_to_slot( - self, - command: models.json_protocol.MoveToSlotCommand) -> PECommands: + self, + command: models.json_protocol.MoveToSlotCommand, + ) -> None: raise NotImplementedError() - def _delay( - self, - command: models.json_protocol.DelayCommand) -> PECommands: + def _delay(self, command: models.json_protocol.DelayCommand) -> None: raise NotImplementedError() def _magnetic_module_engage( - self, - command: models.json_protocol.MagneticModuleEngageCommand) -> PECommands: + self, + command: models.json_protocol.MagneticModuleEngageCommand, + ) -> None: raise NotImplementedError() def _magnetic_module_disengage( - self, - command: models.json_protocol.MagneticModuleDisengageCommand) -> PECommands: + self, + command: models.json_protocol.MagneticModuleDisengageCommand, + ) -> None: raise NotImplementedError() def _temperature_module_set_target( - self, - command: models.json_protocol.TemperatureModuleSetTargetCommand - ) -> PECommands: + self, + command: models.json_protocol.TemperatureModuleSetTargetCommand, + ) -> None: raise NotImplementedError() def _temperature_module_await_temperature( - self, - command: models.json_protocol.TemperatureModuleAwaitTemperatureCommand - ) -> PECommands: + self, + command: models.json_protocol.TemperatureModuleAwaitTemperatureCommand, + ) -> None: raise NotImplementedError() def _temperature_module_deactivate( - self, - command: models.json_protocol.TemperatureModuleDeactivateCommand - ) -> PECommands: + self, + command: models.json_protocol.TemperatureModuleDeactivateCommand, + ) -> None: raise NotImplementedError() def _thermocycler_set_target_block_temperature( - self, - command: models.json_protocol.ThermocyclerSetTargetBlockTemperatureCommand - ) -> PECommands: + self, + command: models.json_protocol.ThermocyclerSetTargetBlockTemperatureCommand, + ) -> None: raise NotImplementedError() def _thermocycler_set_target_lid_temperature( - self, - command: models.json_protocol.ThermocyclerSetTargetLidTemperatureCommand - ) -> PECommands: + self, command: models.json_protocol.ThermocyclerSetTargetLidTemperatureCommand + ) -> None: raise NotImplementedError() def _thermocycler_await_block_temperature( - self, - command: models.json_protocol.ThermocyclerAwaitBlockTemperatureCommand - ) -> PECommands: + self, command: models.json_protocol.ThermocyclerAwaitBlockTemperatureCommand + ) -> None: raise NotImplementedError() def _thermocycler_await_lid_temperature( - self, - command: models.json_protocol.ThermocyclerAwaitLidTemperatureCommand - ) -> PECommands: + self, + command: models.json_protocol.ThermocyclerAwaitLidTemperatureCommand, + ) -> None: raise NotImplementedError() def _thermocycler_deactivate_block( - self, - command: models.json_protocol.ThermocyclerDeactivateBlockCommand - ) -> PECommands: + self, + command: models.json_protocol.ThermocyclerDeactivateBlockCommand, + ) -> None: raise NotImplementedError() def _thermocycler_deactivate_lid( - self, - command: models.json_protocol.ThermocyclerDeactivateLidCommand - ) -> PECommands: + self, + command: models.json_protocol.ThermocyclerDeactivateLidCommand, + ) -> None: raise NotImplementedError() def _thermocycler_open_lid( - self, - command: models.json_protocol.ThermocyclerOpenLidCommand) -> PECommands: + self, + command: models.json_protocol.ThermocyclerOpenLidCommand, + ) -> None: raise NotImplementedError() def _thermocycler_close_lid( - self, - command: models.json_protocol.ThermocyclerCloseLidCommand) -> PECommands: + self, + command: models.json_protocol.ThermocyclerCloseLidCommand, + ) -> None: raise NotImplementedError() def _thermocycler_run_profile( - self, - command: models.json_protocol.ThermocyclerRunProfile) -> PECommands: + self, + command: models.json_protocol.ThermocyclerRunProfile, + ) -> None: raise NotImplementedError() def _thermocycler_await_profile_complete( - self, - command: models.json_protocol.ThermocyclerAwaitProfileCompleteCommand - ) -> PECommands: + self, + command: models.json_protocol.ThermocyclerAwaitProfileCompleteCommand, + ) -> None: raise NotImplementedError() def _move_to_well( - self, - command: models.json_protocol.MoveToWellCommand) -> PECommands: + self, + command: models.json_protocol.MoveToWellCommand, + ) -> None: raise NotImplementedError() _COMMAND_TO_NAME: Dict[str, str] = { - models.json_protocol.CommandMoveToWell: - _move_to_well.__name__, - models.json_protocol.CommandThermocyclerAwaitProfile: - _thermocycler_await_profile_complete.__name__, - models.json_protocol.CommandThermocyclerRunProfile: - _thermocycler_run_profile.__name__, - models.json_protocol.CommandThermocyclerCloseLid: - _thermocycler_close_lid.__name__, - models.json_protocol.CommandThermocyclerOpenLid: - _thermocycler_open_lid.__name__, - models.json_protocol.CommandThermocyclerDeactivateLid: - _thermocycler_deactivate_lid.__name__, - models.json_protocol.CommandThermocyclerDeactivateBlock: - _thermocycler_deactivate_block.__name__, - models.json_protocol.CommandThermocyclerSetTargetLid: - _thermocycler_set_target_lid_temperature.__name__, - models.json_protocol.CommandThermocyclerAwaitBlockTemperature: - _thermocycler_await_block_temperature.__name__, - models.json_protocol.CommandThermocyclerAwaitLidTemperature: - _thermocycler_await_lid_temperature.__name__, - models.json_protocol.CommandThermocyclerSetTargetBlock: - _thermocycler_set_target_block_temperature.__name__, - models.json_protocol.CommandTemperatureModuleDeactivate: - _temperature_module_deactivate.__name__, - models.json_protocol.CommandTemperatureModuleAwait: - _temperature_module_await_temperature.__name__, - models.json_protocol.CommandTemperatureModuleSetTarget: - _temperature_module_set_target.__name__, - models.json_protocol.CommandMagneticModuleDisengage: - _magnetic_module_disengage.__name__, - models.json_protocol.CommandMagneticModuleEngage: - _magnetic_module_engage.__name__, - models.json_protocol.CommandDelay: - _delay.__name__, - models.json_protocol.CommandMoveToSlot: - _move_to_slot.__name__, - models.json_protocol.CommandDropTip: - _drop_tip.__name__, - models.json_protocol.CommandPickUpTip: - _pick_up.__name__, - models.json_protocol.CommandTouchTip: - _touch_tip.__name__, - models.json_protocol.CommandBlowout: - _blowout.__name__, - models.json_protocol.CommandAirGap: - _air_gap.__name__, - models.json_protocol.CommandDispense: - _dispense.__name__, - models.json_protocol.CommandAspirate: - _aspirate.__name__, + models.json_protocol.CommandMoveToWell: _move_to_well.__name__, + models.json_protocol.CommandThermocyclerAwaitProfile: _thermocycler_await_profile_complete.__name__, # noqa: E501 + models.json_protocol.CommandThermocyclerRunProfile: _thermocycler_run_profile.__name__, # noqa: E501 + models.json_protocol.CommandThermocyclerCloseLid: _thermocycler_close_lid.__name__, # noqa: E501 + models.json_protocol.CommandThermocyclerOpenLid: _thermocycler_open_lid.__name__, # noqa: E501 + models.json_protocol.CommandThermocyclerDeactivateLid: _thermocycler_deactivate_lid.__name__, # noqa: E501 + models.json_protocol.CommandThermocyclerDeactivateBlock: _thermocycler_deactivate_block.__name__, # noqa: E501 + models.json_protocol.CommandThermocyclerSetTargetLid: _thermocycler_set_target_lid_temperature.__name__, # noqa: E501 + models.json_protocol.CommandThermocyclerAwaitBlockTemperature: _thermocycler_await_block_temperature.__name__, # noqa: E501 + models.json_protocol.CommandThermocyclerAwaitLidTemperature: _thermocycler_await_lid_temperature.__name__, # noqa: E501 + models.json_protocol.CommandThermocyclerSetTargetBlock: _thermocycler_set_target_block_temperature.__name__, # noqa: E501 + models.json_protocol.CommandTemperatureModuleDeactivate: _temperature_module_deactivate.__name__, # noqa: E501 + models.json_protocol.CommandTemperatureModuleAwait: _temperature_module_await_temperature.__name__, # noqa: E501 + models.json_protocol.CommandTemperatureModuleSetTarget: _temperature_module_set_target.__name__, # noqa: E501 + models.json_protocol.CommandMagneticModuleDisengage: _magnetic_module_disengage.__name__, # noqa: E501 + models.json_protocol.CommandMagneticModuleEngage: _magnetic_module_engage.__name__, # noqa: E501 + models.json_protocol.CommandDelay: _delay.__name__, + models.json_protocol.CommandMoveToSlot: _move_to_slot.__name__, + models.json_protocol.CommandDropTip: _drop_tip.__name__, + models.json_protocol.CommandPickUpTip: _pick_up.__name__, + models.json_protocol.CommandTouchTip: _touch_tip.__name__, + models.json_protocol.CommandBlowout: _blowout.__name__, + models.json_protocol.CommandAirGap: _air_gap.__name__, + models.json_protocol.CommandDispense: _dispense.__name__, + models.json_protocol.CommandAspirate: _aspirate.__name__, } diff --git a/api/tests/opentrons/file_runner/test_command_queue_worker.py b/api/tests/opentrons/file_runner/test_command_queue_worker.py index 791e9c13434..8fb9cdc0a1b 100644 --- a/api/tests/opentrons/file_runner/test_command_queue_worker.py +++ b/api/tests/opentrons/file_runner/test_command_queue_worker.py @@ -1,47 +1,19 @@ """Test.""" -from typing import List, Tuple - import pytest from mock import AsyncMock, MagicMock, call # type: ignore[attr-defined] +from typing import List, Optional from opentrons.file_runner.command_queue_worker import CommandQueueWorker -from opentrons.protocol_engine import ProtocolEngine, WellLocation, StateView -from opentrons.protocol_engine.commands import ( - PickUpTipRequest, - AspirateRequest, - DispenseRequest, - CommandRequestType, -) +from opentrons.protocol_engine import ProtocolEngine, StateView @pytest.fixture -def commands() -> List[Tuple[str, CommandRequestType]]: +def commands() -> List[str]: """Fixture.""" return [ - ( - "command-id-0", - PickUpTipRequest(pipetteId="123", labwareId="abc", wellName="def"), - ), - ( - "command-id-1", - AspirateRequest( - volume=321, - wellLocation=WellLocation(), - pipetteId="123", - labwareId="xyz", - wellName="def", - ), - ), - ( - "command-id-2", - DispenseRequest( - volume=321, - wellLocation=WellLocation(), - pipetteId="123", - labwareId="xyz", - wellName="def", - ), - ), + "command-id-0", + "command-id-1", + "command-id-2", ] @@ -54,13 +26,11 @@ def state_view() -> MagicMock: @pytest.fixture def state_view_with_commands( state_view: MagicMock, - commands: List[Tuple[str, CommandRequestType]], + commands: List[Optional[str]], ) -> MagicMock: """Create a state view fixture with pending commands.""" - # List of Tuples. Command id and command. With None terminator. - # type ignore is because mypy doesn't like concatenating lists of different types. - pending_commands = commands[:] + [None] # type: ignore - state_view.commands.get_next_request.side_effect = pending_commands + pending_commands = commands + [None] + state_view.commands.get_next_command.side_effect = pending_commands return state_view @@ -79,64 +49,68 @@ def subject(protocol_engine: AsyncMock) -> CommandQueueWorker: async def test_play_no_pending( - protocol_engine: AsyncMock, subject: CommandQueueWorker, state_view: MagicMock + protocol_engine: AsyncMock, + subject: CommandQueueWorker, + state_view: MagicMock, ) -> None: """It should not execute any commands.""" - state_view.commands.get_next_request.return_value = None + state_view.commands.get_next_command.return_value = None subject.play() await subject.wait_to_be_idle() - protocol_engine.execute_command.assert_not_called() + protocol_engine.execute_command_by_id.assert_not_called() async def test_play( protocol_engine: AsyncMock, subject: CommandQueueWorker, state_view_with_commands: MagicMock, - commands: List[Tuple[str, CommandRequestType]], + commands: List[str], ) -> None: """It should cycle through pending commands and execute them.""" subject.play() await subject.wait_to_be_idle() - expected_call_args_list = [call(request=r, command_id=i) for i, r in commands] - print("Actual calls:", protocol_engine.execute_command.call_args_list) - assert protocol_engine.execute_command.call_args_list == expected_call_args_list + expected_call_args_list = [call(command_id=command_id) for command_id in commands] + assert ( + protocol_engine.execute_command_by_id.call_args_list == expected_call_args_list + ) async def test_pause( protocol_engine: AsyncMock, subject: CommandQueueWorker, state_view_with_commands: MagicMock, - commands: List[Tuple[str, CommandRequestType]], + commands: List[str], ) -> None: """It should cycle through pending commands and execute them.""" - async def mock_execute_command( - request: CommandRequestType, - command_id: str, - ) -> None: + async def mock_execute_command(command_id: str) -> None: if command_id == str("command-id-0"): # Pause after first command subject.pause() - protocol_engine.execute_command.side_effect = mock_execute_command + protocol_engine.execute_command_by_id.side_effect = mock_execute_command subject.play() await subject.wait_to_be_idle() # Only first command was executed. - protocol_engine.execute_command.assert_called_once_with( - request=commands[0][1], command_id=commands[0][0] + protocol_engine.execute_command_by_id.assert_called_once_with( + command_id=commands[0] ) # Reset execute command mock and resume - protocol_engine.execute_command.reset_mock() + protocol_engine.execute_command_by_id.reset_mock() subject.play() await subject.wait_to_be_idle() - expected_call_args_list = [call(request=r, command_id=i) for i, r in commands[1:]] + expected_call_args_list = [ + call(command_id=command_id) for command_id in commands[1:] + ] - assert protocol_engine.execute_command.call_args_list == expected_call_args_list + assert ( + protocol_engine.execute_command_by_id.call_args_list == expected_call_args_list + ) diff --git a/api/tests/opentrons/file_runner/test_json_file_runner.py b/api/tests/opentrons/file_runner/test_json_file_runner.py index 62b804b99be..526bd1d459f 100644 --- a/api/tests/opentrons/file_runner/test_json_file_runner.py +++ b/api/tests/opentrons/file_runner/test_json_file_runner.py @@ -8,11 +8,7 @@ from opentrons.file_runner.command_queue_worker import CommandQueueWorker from opentrons.protocol_engine import ProtocolEngine, WellLocation -from opentrons.protocol_engine.commands import ( - PickUpTipRequest, - AspirateRequest, - DispenseRequest, -) +from opentrons.protocol_engine import commands as pe_commands from opentrons.protocols.models import JsonProtocol from opentrons.protocols.runner.json_proto.command_translator import CommandTranslator @@ -120,20 +116,30 @@ def test_json_runner_load_commands_to_engine( protocol_engine: ProtocolEngine, ) -> None: """It should send translated commands to protocol engine.""" - mock_cmd1 = PickUpTipRequest(pipetteId="123", labwareId="abc", wellName="def") - mock_cmd2 = AspirateRequest( - volume=321, - wellLocation=WellLocation(), - pipetteId="123", - labwareId="xyz", - wellName="def", + mock_cmd1 = pe_commands.PickUpTipRequest( + data=pe_commands.PickUpTipData( + pipetteId="123", + labwareId="abc", + wellName="def", + ) ) - mock_cmd3 = DispenseRequest( - volume=321, - wellLocation=WellLocation(), - pipetteId="123", - labwareId="xyz", - wellName="def", + mock_cmd2 = pe_commands.AspirateRequest( + data=pe_commands.AspirateData( + volume=321, + wellLocation=WellLocation(), + pipetteId="123", + labwareId="xyz", + wellName="def", + ) + ) + mock_cmd3 = pe_commands.DispenseRequest( + data=pe_commands.DispenseData( + volume=321, + wellLocation=WellLocation(), + pipetteId="123", + labwareId="xyz", + wellName="def", + ) ) decoy.when(command_translator.translate(json_protocol)).then_return( diff --git a/api/tests/opentrons/protocol_engine/clients/test_child_thread_transport.py b/api/tests/opentrons/protocol_engine/clients/test_child_thread_transport.py index 54a5f925017..89542d73c78 100644 --- a/api/tests/opentrons/protocol_engine/clients/test_child_thread_transport.py +++ b/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.""" @@ -40,22 +34,23 @@ async def test_execute_command( subject: ChildThreadTransport, ) -> None: """It should execute a command synchronously in a child thread.""" - cmd_request = commands.MoveToWellRequest( + cmd_data = commands.MoveToWellData( pipetteId="pipette-id", labwareId="labware-id", - wellName="A1" + wellName="A1", ) cmd_result = commands.MoveToWellResult() + cmd_request = commands.MoveToWellRequest(data=cmd_data) decoy.when( await engine.execute_command(request=cmd_request, command_id="cmd-id") ).then_return( - commands.CompletedCommand( - request=cmd_request, + commands.MoveToWell( + id="cmd-id", + status=commands.CommandStatus.SUCCEEDED, + data=cmd_data, result=cmd_result, - created_at=matchers.IsA(datetime), - started_at=matchers.IsA(datetime), - completed_at=matchers.IsA(datetime), + createdAt=datetime.now(), ) ) @@ -72,21 +67,22 @@ async def test_execute_command_failure( subject: ChildThreadTransport, ) -> None: """It should execute a load labware command.""" - cmd_request = commands.MoveToWellRequest( + cmd_data = commands.MoveToWellData( pipetteId="pipette-id", labwareId="labware-id", - wellName="A1" + wellName="A1", ) + cmd_request = commands.MoveToWellRequest(data=cmd_data) decoy.when( await engine.execute_command(request=cmd_request, command_id="cmd-id") ).then_return( - commands.FailedCommand( - error=ProtocolEngineError("oh no"), - request=cmd_request, - created_at=matchers.IsA(datetime), - started_at=matchers.IsA(datetime), - failed_at=matchers.IsA(datetime), + commands.MoveToWell( + id="cmd-id", + data=cmd_data, + status=commands.CommandStatus.FAILED, + error="oh no", + createdAt=datetime.now(), ) ) diff --git a/api/tests/opentrons/protocol_engine/clients/test_sync_client.py b/api/tests/opentrons/protocol_engine/clients/test_sync_client.py index 1f3e982a42d..c2a5e9d4200 100644 --- a/api/tests/opentrons/protocol_engine/clients/test_sync_client.py +++ b/api/tests/opentrons/protocol_engine/clients/test_sync_client.py @@ -49,11 +49,13 @@ def stubbed_load_labware_result( ) -> commands.LoadLabwareResult: """Set up the protocol engine with default stubbed response for load labware.""" request = commands.LoadLabwareRequest( - location=DeckSlotLocation(slot=DeckSlotName.SLOT_5), - loadName="some_labware", - namespace="opentrons", - version=1, - labwareId=None, + data=commands.LoadLabwareData( + location=DeckSlotLocation(slot=DeckSlotName.SLOT_5), + loadName="some_labware", + namespace="opentrons", + version=1, + labwareId=None, + ) ) result = commands.LoadLabwareResult( @@ -91,8 +93,10 @@ def test_load_pipette( ) -> None: """It should execute a load pipette command and return its result.""" request = commands.LoadPipetteRequest( - pipetteName=PipetteName.P300_SINGLE, - mount=MountType.RIGHT, + data=commands.LoadPipetteData( + pipetteName=PipetteName.P300_SINGLE, + mount=MountType.RIGHT, + ) ) expected_result = commands.LoadPipetteResult(pipetteId="abc123") @@ -115,7 +119,9 @@ def test_pick_up_tip( subject: SyncClient, ) -> None: """It should execute a pick up tip command.""" - request = commands.PickUpTipRequest(pipetteId="123", labwareId="456", wellName="A2") + request = commands.PickUpTipRequest( + data=commands.PickUpTipData(pipetteId="123", labwareId="456", wellName="A2") + ) response = commands.PickUpTipResult() decoy.when( @@ -133,7 +139,9 @@ def test_drop_tip( subject: SyncClient, ) -> None: """It should execute a drop up tip command.""" - request = commands.DropTipRequest(pipetteId="123", labwareId="456", wellName="A2") + request = commands.DropTipRequest( + data=commands.DropTipData(pipetteId="123", labwareId="456", wellName="A2") + ) response = commands.DropTipResult() decoy.when( @@ -152,11 +160,13 @@ def test_aspirate( ) -> None: """It should send an AspirateCommand through the transport.""" request = commands.AspirateRequest( - pipetteId="123", - labwareId="456", - wellName="A2", - wellLocation=WellLocation(origin=WellOrigin.BOTTOM, offset=(0, 0, 1)), - volume=123.45, + data=commands.AspirateData( + pipetteId="123", + labwareId="456", + wellName="A2", + wellLocation=WellLocation(origin=WellOrigin.BOTTOM, offset=(0, 0, 1)), + volume=123.45, + ) ) result_from_transport = commands.AspirateResult(volume=67.89) @@ -183,11 +193,13 @@ def test_dispense( ) -> None: """It should execute a dispense command.""" request = commands.DispenseRequest( - pipetteId="123", - labwareId="456", - wellName="A2", - wellLocation=WellLocation(origin=WellOrigin.BOTTOM, offset=(0, 0, 1)), - volume=10, + data=commands.DispenseData( + pipetteId="123", + labwareId="456", + wellName="A2", + wellLocation=WellLocation(origin=WellOrigin.BOTTOM, offset=(0, 0, 1)), + volume=10, + ) ) response = commands.DispenseResult(volume=1) diff --git a/api/tests/opentrons/protocol_engine/commands/conftest.py b/api/tests/opentrons/protocol_engine/commands/conftest.py new file mode 100644 index 00000000000..5f07082bde3 --- /dev/null +++ b/api/tests/opentrons/protocol_engine/commands/conftest.py @@ -0,0 +1,27 @@ +"""Fixtures for protocol engine command tests.""" +import pytest +from decoy import Decoy + +from opentrons.protocol_engine.execution import ( + EquipmentHandler, + MovementHandler, + PipettingHandler, +) + + +@pytest.fixture +def equipment(decoy: Decoy) -> EquipmentHandler: + """Get a mocked out EquipmentHandler.""" + return decoy.create_decoy(spec=EquipmentHandler) + + +@pytest.fixture +def movement(decoy: Decoy) -> MovementHandler: + """Get a mocked out MovementHandler.""" + return decoy.create_decoy(spec=MovementHandler) + + +@pytest.fixture +def pipetting(decoy: Decoy) -> PipettingHandler: + """Get a mocked out PipettingHandler.""" + return decoy.create_decoy(spec=PipettingHandler) diff --git a/api/tests/opentrons/protocol_engine/commands/test_add_labware_definition.py b/api/tests/opentrons/protocol_engine/commands/test_add_labware_definition.py index 2921eb45088..74c1db0578a 100644 --- a/api/tests/opentrons/protocol_engine/commands/test_add_labware_definition.py +++ b/api/tests/opentrons/protocol_engine/commands/test_add_labware_definition.py @@ -1,44 +1,36 @@ """Test add labware command.""" -from mock import AsyncMock # type: ignore[attr-defined] -from opentrons.protocols import models -from opentrons.protocol_engine.commands import ( - AddLabwareDefinitionRequest, AddLabwareDefinitionResult -) - - -def test_add_labware_request(well_plate_def: models.LabwareDefinition) -> None: - """It should have an AddLabwareDefinitionRequest model.""" - request = AddLabwareDefinitionRequest( - definition=well_plate_def, - ) - - assert request.definition == well_plate_def +from decoy import Decoy +from opentrons.protocols.models import LabwareDefinition +from opentrons.protocol_engine.execution import ( + EquipmentHandler, + MovementHandler, + PipettingHandler, +) -def test_add_labware_result() -> None: - """It should be have an AddLabwareDefinitionResult model.""" - result = AddLabwareDefinitionResult( - loadName="loadname", - namespace="ns", - version=1, - ) - - assert result.loadName == "loadname" - assert result.namespace == "ns" - assert result.version == 1 +from opentrons.protocol_engine.commands.add_labware_definition import ( + AddLabwareDefinitionData, + AddLabwareDefinitionResult, + AddLabwareDefinitionImplementation, +) async def test_add_labware_implementation( - well_plate_def: models.LabwareDefinition, - mock_handlers: AsyncMock, + decoy: Decoy, + well_plate_def: LabwareDefinition, + equipment: EquipmentHandler, + movement: MovementHandler, + pipetting: PipettingHandler, ) -> None: """An AddLabwareRequest should have an execution implementation.""" - request = AddLabwareDefinitionRequest( - definition=well_plate_def + subject = AddLabwareDefinitionImplementation( + equipment=equipment, + movement=movement, + pipetting=pipetting, ) - impl = request.get_implementation() - result = await impl.execute(mock_handlers) + data = AddLabwareDefinitionData(definition=well_plate_def) + result = await subject.execute(data) assert result == AddLabwareDefinitionResult( loadName=well_plate_def.parameters.loadName, diff --git a/api/tests/opentrons/protocol_engine/commands/test_aspirate.py b/api/tests/opentrons/protocol_engine/commands/test_aspirate.py index bf191ddd251..104d31e6e25 100644 --- a/api/tests/opentrons/protocol_engine/commands/test_aspirate.py +++ b/api/tests/opentrons/protocol_engine/commands/test_aspirate.py @@ -1,45 +1,37 @@ """Test aspirate commands.""" from decoy import Decoy -from opentrons.protocol_engine import CommandHandlers, WellLocation, WellOrigin -from opentrons.protocol_engine.commands import AspirateRequest, AspirateResult +from opentrons.protocol_engine import WellLocation, WellOrigin +from opentrons.protocol_engine.execution import ( + EquipmentHandler, + MovementHandler, + PipettingHandler, +) -def test_aspirate_request() -> None: - """It should be able to create a AspirateRequest.""" - request = AspirateRequest( - pipetteId="abc", - labwareId="123", - wellName="A3", - wellLocation=WellLocation(origin=WellOrigin.BOTTOM, offset=(0, 0, 1)), - volume=50, - ) - - assert request.pipetteId == "abc" - assert request.labwareId == "123" - assert request.wellName == "A3" - assert request.wellLocation == WellLocation( - origin=WellOrigin.BOTTOM, - offset=(0, 0, 1), - ) - assert request.volume == 50 - - -def test_aspirate_result() -> None: - """It should be able to create a AspirateResult.""" - result = AspirateResult(volume=50) - - assert result.volume == 50 +from opentrons.protocol_engine.commands.aspirate import ( + AspirateData, + AspirateResult, + AspirateImplementation, +) async def test_aspirate_implementation( decoy: Decoy, - mock_cmd_handlers: CommandHandlers, + equipment: EquipmentHandler, + movement: MovementHandler, + pipetting: PipettingHandler, ) -> None: - """A PickUpTipRequest should have an execution implementation.""" + """An Aspirate should have an execution implementation.""" + subject = AspirateImplementation( + equipment=equipment, + movement=movement, + pipetting=pipetting, + ) + location = WellLocation(origin=WellOrigin.BOTTOM, offset=(0, 0, 1)) - request = AspirateRequest( + data = AspirateData( pipetteId="abc", labwareId="123", wellName="A3", @@ -48,7 +40,7 @@ async def test_aspirate_implementation( ) decoy.when( - await mock_cmd_handlers.pipetting.aspirate( + await pipetting.aspirate( pipette_id="abc", labware_id="123", well_name="A3", @@ -57,7 +49,6 @@ async def test_aspirate_implementation( ) ).then_return(42) - impl = request.get_implementation() - result = await impl.execute(mock_cmd_handlers) + result = await subject.execute(data) assert result == AspirateResult(volume=42) diff --git a/api/tests/opentrons/protocol_engine/commands/test_command.py b/api/tests/opentrons/protocol_engine/commands/test_command.py deleted file mode 100644 index ea7bfa38429..00000000000 --- a/api/tests/opentrons/protocol_engine/commands/test_command.py +++ /dev/null @@ -1,165 +0,0 @@ -"""Tests for command state and base implementation class.""" -from datetime import datetime -from pydantic import BaseModel -from mock import AsyncMock # type: ignore[attr-defined] - -from opentrons.protocol_engine import errors -from opentrons.protocol_engine.commands.command import ( - PendingCommand, - RunningCommand, - CompletedCommand, - FailedCommand, - CommandImplementation, - CommandHandlers, -) - - -class ReqModel(BaseModel): - """Command request model.""" - - foo: str - bar: int - - -class ResModel(BaseModel): - """Command result model.""" - - baz: bool - - -class ExampleImpl(CommandImplementation[ReqModel, ResModel]): - """Example command implementation.""" - - async def execute(self, handlers: CommandHandlers) -> ResModel: - """Execute the command.""" - return ResModel(baz=True) - - -def test_pending_command( - now: datetime, - later: datetime, -) -> None: - """A command should be able to be pending.""" - request = ReqModel(foo="hello", bar=0) - - cmd = PendingCommand[ReqModel, ResModel]( - request=request, - created_at=now, - ) - - assert cmd.request == request - assert cmd.created_at == now - assert cmd.to_running(started_at=later) == RunningCommand( - request=request, - created_at=now, - started_at=later, - ) - - -def test_running_command( - now: datetime, - later: datetime, - even_later: datetime, -) -> None: - """A command should be able to be running.""" - request = ReqModel(foo="hello", bar=0) - result = ResModel(baz=True) - error = errors.ProtocolEngineError("oh no!") - - cmd = RunningCommand[ReqModel, ResModel]( - request=request, - created_at=now, - started_at=later, - ) - - assert cmd.request == request - assert cmd.created_at == now - assert cmd.started_at == later - - assert cmd.to_completed( - result=result, - completed_at=even_later, - ) == CompletedCommand( - request=request, - result=result, - created_at=now, - started_at=later, - completed_at=even_later, - ) - - assert cmd.to_failed( - error=error, - failed_at=even_later, - ) == FailedCommand( - request=request, - error=error, - created_at=now, - started_at=later, - failed_at=even_later, - ) - - -def test_completed_command( - now: datetime, - later: datetime, - even_later: datetime, -) -> None: - """A command should be able to be completed.""" - request = ReqModel(foo="hello", bar=0) - result = ResModel(baz=True) - - cmd = CompletedCommand[ReqModel, ResModel]( - request=request, - result=result, - created_at=now, - started_at=later, - completed_at=even_later, - ) - - assert cmd.request == request - assert cmd.result == result - assert cmd.created_at == now - assert cmd.started_at == later - assert cmd.completed_at == even_later - - -def test_failed_command( - now: datetime, - later: datetime, - even_later: datetime, -) -> None: - """A command should be able to be failed.""" - request = ReqModel(foo="hello", bar=0) - error = errors.ProtocolEngineError("oh no!") - - cmd = FailedCommand[ReqModel]( - request=request, - error=error, - created_at=now, - started_at=later, - failed_at=even_later, - ) - - assert cmd.request == request - assert cmd.error == error - assert cmd.created_at == now - assert cmd.started_at == later - assert cmd.failed_at == even_later - - -async def test_command_implementation( - later: datetime, - mock_handlers: AsyncMock, -) -> None: - """A command implementation should create pending command and execute.""" - request = ReqModel(foo="hello", bar=0) - impl = ExampleImpl(request) - - assert impl.create_command(later) == PendingCommand( - request=request, - created_at=later, - ) - - result = await impl.execute(mock_handlers) - - assert result == ResModel(baz=True) diff --git a/api/tests/opentrons/protocol_engine/commands/test_command_mapper.py b/api/tests/opentrons/protocol_engine/commands/test_command_mapper.py new file mode 100644 index 00000000000..0376234eb7c --- /dev/null +++ b/api/tests/opentrons/protocol_engine/commands/test_command_mapper.py @@ -0,0 +1,195 @@ +"""Smoke tests for the CommandExecutor class.""" +import pytest +from datetime import datetime +from pydantic import BaseModel +from typing import Type, cast +from opentrons.protocols.models import LabwareDefinition + +from opentrons.types import MountType +from opentrons.protocol_engine import commands +from opentrons.protocol_engine.types import ( + DeckSlotLocation, + DeckSlotName, + PipetteName, + WellLocation, +) + + +@pytest.mark.parametrize( + ("request_data", "request_cls", "command_cls"), + [ + ( + commands.AddLabwareDefinitionData.construct( + # TODO(mc, 2021-06-25): do not mock out LabwareDefinition + definition=cast(LabwareDefinition, {"mockDefinition": True}) + ), + commands.AddLabwareDefinitionRequest, + commands.AddLabwareDefinition, + ), + ( + commands.AspirateData( + pipetteId="pipette-id", + labwareId="labware-id", + wellName="well-name", + volume=42, + wellLocation=WellLocation(), + ), + commands.AspirateRequest, + commands.Aspirate, + ), + ( + commands.DispenseData( + pipetteId="pipette-id", + labwareId="labware-id", + wellName="well-name", + volume=42, + wellLocation=WellLocation(), + ), + commands.DispenseRequest, + commands.Dispense, + ), + ( + commands.DropTipData( + pipetteId="pipette-id", + labwareId="labware-id", + wellName="well-name", + ), + commands.DropTipRequest, + commands.DropTip, + ), + ( + commands.LoadLabwareData( + location=DeckSlotLocation(slot=DeckSlotName.SLOT_1), + loadName="load-name", + namespace="namespace", + version=42, + ), + commands.LoadLabwareRequest, + commands.LoadLabware, + ), + ( + commands.LoadPipetteData( + mount=MountType.LEFT, + pipetteName=PipetteName.P300_SINGLE, + ), + commands.LoadPipetteRequest, + commands.LoadPipette, + ), + ( + commands.PickUpTipData( + pipetteId="pipette-id", + labwareId="labware-id", + wellName="well-name", + ), + commands.PickUpTipRequest, + commands.PickUpTip, + ), + ( + commands.MoveToWellData( + pipetteId="pipette-id", + labwareId="labware-id", + wellName="well-name", + ), + commands.MoveToWellRequest, + commands.MoveToWell, + ), + ], +) +def test_map_request_to_command( + request_data: BaseModel, + request_cls: Type[commands.CommandRequest], + command_cls: Type[commands.Command], +) -> None: + """It should be able to create a command resource from a request.""" + subject = commands.CommandMapper() + command_id = "command-id" + created_at = datetime(year=2021, month=1, day=1, hour=1, minute=1) + request = request_cls(data=request_data) # type: ignore[arg-type] + + result = subject.map_request_to_command( + request=request, + command_id=command_id, + created_at=created_at, + ) + + assert result == command_cls( + id=command_id, + createdAt=created_at, + status=commands.CommandStatus.QUEUED, + data=request_data, # type: ignore[arg-type] + ) + + +def test_update_command_status() -> None: + """It should update a command status.""" + subject = commands.CommandMapper() + command_id = "command-id" + created_at = datetime(year=2021, month=1, day=1, hour=1, minute=1) + data = commands.MoveToWellData( + pipetteId="pipette-id", + labwareId="labware-id", + wellName="well-name", + ) + + command = commands.MoveToWell( + id=command_id, + createdAt=created_at, + status=commands.CommandStatus.QUEUED, + data=data, + ) + + result = subject.update_command( + command=command, + status=commands.CommandStatus.RUNNING, + ) + + assert result == commands.MoveToWell( + id=command_id, + createdAt=created_at, + status=commands.CommandStatus.RUNNING, + data=data, + ) + + # check that returned command is a new instance and input was not mutated + assert result is not command + + +def test_update_command_timestamps() -> None: + """It should update a command fields like status and timestamps.""" + subject = commands.CommandMapper() + command_id = "command-id" + created_at = datetime(year=2021, month=1, day=1, hour=1, minute=1) + data = commands.MoveToWellData( + pipetteId="pipette-id", + labwareId="labware-id", + wellName="well-name", + ) + command_result = commands.MoveToWellResult() + + command = commands.MoveToWell( + id=command_id, + createdAt=created_at, + status=commands.CommandStatus.QUEUED, + data=data, + ) + + result = subject.update_command( + command=command, + status=commands.CommandStatus.SUCCEEDED, + startedAt=datetime(year=2021, month=1, day=1, hour=1, minute=1), + completedAt=datetime(year=2021, month=2, day=2, hour=2, minute=2), + result=command_result, + ) + + assert result == commands.MoveToWell( + id=command_id, + createdAt=created_at, + status=commands.CommandStatus.SUCCEEDED, + data=data, + result=command_result, + startedAt=datetime(year=2021, month=1, day=1, hour=1, minute=1), + completedAt=datetime(year=2021, month=2, day=2, hour=2, minute=2), + ) + + # check that returned command is a new instance and input was not mutated + assert result is not command diff --git a/api/tests/opentrons/protocol_engine/commands/test_dispense.py b/api/tests/opentrons/protocol_engine/commands/test_dispense.py index 6b429bd5ea5..690cf7e871c 100644 --- a/api/tests/opentrons/protocol_engine/commands/test_dispense.py +++ b/api/tests/opentrons/protocol_engine/commands/test_dispense.py @@ -1,45 +1,38 @@ """Test dispense commands.""" from decoy import Decoy -from opentrons.protocol_engine import CommandHandlers, WellLocation, WellOrigin -from opentrons.protocol_engine.commands import DispenseRequest, DispenseResult +from opentrons.protocol_engine import WellLocation, WellOrigin +from opentrons.protocol_engine.execution import ( + EquipmentHandler, + MovementHandler, + PipettingHandler, +) -def test_dispense_request() -> None: - """It should be able to create a DispenseRequest.""" - request = DispenseRequest( - pipetteId="abc", - labwareId="123", - wellName="A3", - wellLocation=WellLocation(origin=WellOrigin.BOTTOM, offset=(0, 0, 1)), - volume=50, - ) - assert request.pipetteId == "abc" - assert request.labwareId == "123" - assert request.wellName == "A3" - assert request.wellLocation == WellLocation( - origin=WellOrigin.BOTTOM, - offset=(0, 0, 1), - ) - assert request.volume == 50 - - -def test_dispense_result() -> None: - """It should be able to create a DispenseResult.""" - result = DispenseResult(volume=50) - - assert result.volume == 50 +from opentrons.protocol_engine.commands.dispense import ( + DispenseData, + DispenseResult, + DispenseImplementation, +) async def test_dispense_implementation( decoy: Decoy, - mock_cmd_handlers: CommandHandlers, + equipment: EquipmentHandler, + movement: MovementHandler, + pipetting: PipettingHandler, ) -> None: """A PickUpTipRequest should have an execution implementation.""" + subject = DispenseImplementation( + equipment=equipment, + movement=movement, + pipetting=pipetting, + ) + location = WellLocation(origin=WellOrigin.BOTTOM, offset=(0, 0, 1)) - request = DispenseRequest( + data = DispenseData( pipetteId="abc", labwareId="123", wellName="A3", @@ -48,7 +41,7 @@ async def test_dispense_implementation( ) decoy.when( - await mock_cmd_handlers.pipetting.dispense( + await pipetting.dispense( pipette_id="abc", labware_id="123", well_name="A3", @@ -57,7 +50,6 @@ async def test_dispense_implementation( ) ).then_return(42) - impl = request.get_implementation() - result = await impl.execute(mock_cmd_handlers) + result = await subject.execute(data) assert result == DispenseResult(volume=42) diff --git a/api/tests/opentrons/protocol_engine/commands/test_drop_tip.py b/api/tests/opentrons/protocol_engine/commands/test_drop_tip.py index 3020e36add1..322cefcceea 100644 --- a/api/tests/opentrons/protocol_engine/commands/test_drop_tip.py +++ b/api/tests/opentrons/protocol_engine/commands/test_drop_tip.py @@ -1,49 +1,46 @@ """Test pick up tip commands.""" -from mock import AsyncMock # type: ignore[attr-defined] +from decoy import Decoy -from opentrons.protocol_engine.commands import ( - DropTipRequest, +from opentrons.protocol_engine.execution import ( + EquipmentHandler, + MovementHandler, + PipettingHandler, +) + +from opentrons.protocol_engine.commands.drop_tip import ( + DropTipData, DropTipResult, + DropTipImplementation, ) -def test_pick_up_tip_request() -> None: - """It should be able to create a DropTipRequest.""" - request = DropTipRequest( - pipetteId="abc", - labwareId="123", - wellName="A3", +async def test_drop_tip_implementation( + decoy: Decoy, + equipment: EquipmentHandler, + movement: MovementHandler, + pipetting: PipettingHandler, +) -> None: + """A DropTip command should have an execution implementation.""" + subject = DropTipImplementation( + equipment=equipment, + movement=movement, + pipetting=pipetting, ) - assert request.pipetteId == "abc" - assert request.labwareId == "123" - assert request.wellName == "A3" - - -def test_pick_up_tip_result() -> None: - """It should be able to create a DropTipResult.""" - # NOTE(mc, 2020-11-17): this model has no properties at this time - result = DropTipResult() - - assert result - - -async def test_pick_up_tip_implementation(mock_handlers: AsyncMock) -> None: - """A DropTipRequest should have an execution implementation.""" - mock_handlers.pipetting.drop_tip.return_value = None - - request = DropTipRequest( + data = DropTipData( pipetteId="abc", labwareId="123", wellName="A3", ) - impl = request.get_implementation() - result = await impl.execute(mock_handlers) + result = await subject.execute(data) assert result == DropTipResult() - mock_handlers.pipetting.drop_tip.assert_called_with( - pipette_id="abc", - labware_id="123", - well_name="A3", + + decoy.verify( + await pipetting.drop_tip( + pipette_id="abc", + labware_id="123", + well_name="A3", + ) ) diff --git a/api/tests/opentrons/protocol_engine/commands/test_load_labware.py b/api/tests/opentrons/protocol_engine/commands/test_load_labware.py index 952884e105e..26d720c9631 100644 --- a/api/tests/opentrons/protocol_engine/commands/test_load_labware.py +++ b/api/tests/opentrons/protocol_engine/commands/test_load_labware.py @@ -1,77 +1,64 @@ """Test load labware commands.""" -from mock import AsyncMock # type: ignore[attr-defined] +from decoy import Decoy -from opentrons.protocols.models import LabwareDefinition from opentrons.types import DeckSlotName +from opentrons.protocols.models import LabwareDefinition from opentrons.protocol_engine.types import DeckSlotLocation -from opentrons.protocol_engine.execution import LoadedLabware -from opentrons.protocol_engine.commands import ( - LoadLabwareRequest, +from opentrons.protocol_engine.execution import ( + LoadedLabware, + EquipmentHandler, + MovementHandler, + PipettingHandler, +) +from opentrons.protocol_engine.commands.load_labware import ( + LoadLabwareData, LoadLabwareResult, + LoadLabwareImplementation, ) -def test_load_labware_request() -> None: - """It should have a LoadLabwareRequest model.""" - request = LoadLabwareRequest( - location=DeckSlotLocation(slot=DeckSlotName.SLOT_3), - loadName="some-load-name", - namespace="opentrons-test", - version=1, - labwareId="some id" - ) - - assert request.location == DeckSlotLocation(slot=DeckSlotName.SLOT_3) - assert request.loadName == "some-load-name" - assert request.namespace == "opentrons-test" - assert request.version == 1 - assert request.labwareId == "some id" - - -def test_load_labware_result(well_plate_def: LabwareDefinition) -> None: - """It should have a LoadLabwareResult model.""" - result = LoadLabwareResult( - labwareId="labware-id", - definition=well_plate_def, - calibration=(1, 2, 3), - ) - - assert result.labwareId == "labware-id" - assert result.definition == well_plate_def - assert result.calibration == (1, 2, 3) - - async def test_load_labware_implementation( + decoy: Decoy, well_plate_def: LabwareDefinition, - mock_handlers: AsyncMock, + equipment: EquipmentHandler, + movement: MovementHandler, + pipetting: PipettingHandler, ) -> None: - """A LoadLabwareRequest should have an execution implementation.""" - mock_handlers.equipment.load_labware.return_value = LoadedLabware( - labware_id="labware-id", - definition=well_plate_def, - calibration=(1, 2, 3) + """A LoadLabware command should have an execution implementation.""" + subject = LoadLabwareImplementation( + equipment=equipment, + movement=movement, + pipetting=pipetting, ) - request = LoadLabwareRequest( + data = LoadLabwareData( location=DeckSlotLocation(slot=DeckSlotName.SLOT_3), loadName="some-load-name", namespace="opentrons-test", version=1, ) - impl = request.get_implementation() - result = await impl.execute(mock_handlers) + decoy.when( + await equipment.load_labware( + location=DeckSlotLocation(slot=DeckSlotName.SLOT_3), + load_name="some-load-name", + namespace="opentrons-test", + version=1, + labware_id=None, + ) + ).then_return( + LoadedLabware( + labware_id="labware-id", + definition=well_plate_def, + calibration=(1, 2, 3), + ) + ) + + result = await subject.execute(data) assert result == LoadLabwareResult( labwareId="labware-id", definition=well_plate_def, calibration=(1, 2, 3), ) - mock_handlers.equipment.load_labware.assert_called_with( - location=DeckSlotLocation(slot=DeckSlotName.SLOT_3), - load_name="some-load-name", - namespace="opentrons-test", - version=1, - labware_id=None - ) diff --git a/api/tests/opentrons/protocol_engine/commands/test_load_pipette.py b/api/tests/opentrons/protocol_engine/commands/test_load_pipette.py index 388c7ecc841..7e6c55c69e1 100644 --- a/api/tests/opentrons/protocol_engine/commands/test_load_pipette.py +++ b/api/tests/opentrons/protocol_engine/commands/test_load_pipette.py @@ -1,51 +1,49 @@ """Test load pipette commands.""" -from mock import AsyncMock # type: ignore[attr-defined] +from decoy import Decoy from opentrons.types import MountType from opentrons.protocol_engine.types import PipetteName -from opentrons.protocol_engine.execution import LoadedPipette -from opentrons.protocol_engine.commands import ( - LoadPipetteRequest, + +from opentrons.protocol_engine.execution import ( + LoadedPipette, + EquipmentHandler, + MovementHandler, + PipettingHandler, +) +from opentrons.protocol_engine.commands.load_pipette import ( + LoadPipetteData, LoadPipetteResult, + LoadPipetteImplementation, ) -def test_load_pipette_request() -> None: - """It should have a LoadPipetteRequest model.""" - request = LoadPipetteRequest( - pipetteName=PipetteName.P300_SINGLE, - mount=MountType.LEFT, +async def test_load_pipette_implementation( + decoy: Decoy, + equipment: EquipmentHandler, + movement: MovementHandler, + pipetting: PipettingHandler, +) -> None: + """A LoadPipette command should have an execution implementation.""" + subject = LoadPipetteImplementation( + equipment=equipment, + movement=movement, + pipetting=pipetting, ) - assert request.pipetteName == "p300_single" - assert request.mount == MountType.LEFT - assert request.pipetteId is None - - -def test_load_pipette_result() -> None: - """It should have a LoadPipetteResult model.""" - result = LoadPipetteResult(pipetteId="pipette-id") - - assert result.pipetteId == "pipette-id" - - -async def test_load_pipette_implementation(mock_handlers: AsyncMock) -> None: - """A LoadPipetteRequest should have an execution implementation.""" - mock_handlers.equipment.load_pipette.return_value = LoadedPipette( - pipette_id="pipette-id", - ) - - request = LoadPipetteRequest( + data = LoadPipetteData( pipetteName=PipetteName.P300_SINGLE, mount=MountType.LEFT, - pipetteId="some id" + pipetteId="some id", ) - impl = request.get_implementation() - result = await impl.execute(mock_handlers) + + decoy.when( + await equipment.load_pipette( + pipette_name=PipetteName.P300_SINGLE, + mount=MountType.LEFT, + pipette_id="some id", + ) + ).then_return(LoadedPipette(pipette_id="pipette-id")) + + result = await subject.execute(data) assert result == LoadPipetteResult(pipetteId="pipette-id") - mock_handlers.equipment.load_pipette.assert_called_with( - pipette_name="p300_single", - mount=MountType.LEFT, - pipette_id="some id", - ) diff --git a/api/tests/opentrons/protocol_engine/commands/test_move_to_well.py b/api/tests/opentrons/protocol_engine/commands/test_move_to_well.py index f713423abe1..5c6d425867f 100644 --- a/api/tests/opentrons/protocol_engine/commands/test_move_to_well.py +++ b/api/tests/opentrons/protocol_engine/commands/test_move_to_well.py @@ -1,49 +1,45 @@ """Test move to well commands.""" -from mock import AsyncMock # type: ignore[attr-defined] +from decoy import Decoy -from opentrons.protocol_engine.commands import ( - MoveToWellRequest, +from opentrons.protocol_engine.execution import ( + EquipmentHandler, + MovementHandler, + PipettingHandler, +) + +from opentrons.protocol_engine.commands.move_to_well import ( + MoveToWellData, MoveToWellResult, + MoveToWellImplementation, ) -def test_move_to_well_request() -> None: - """It should be able to create a MoveToWellRequest.""" - request = MoveToWellRequest( - pipetteId="abc", - labwareId="123", - wellName="A3", +async def test_move_to_well_implementation( + decoy: Decoy, + equipment: EquipmentHandler, + movement: MovementHandler, + pipetting: PipettingHandler, +) -> None: + """A MoveToWell command should have an execution implementation.""" + subject = MoveToWellImplementation( + equipment=equipment, + movement=movement, + pipetting=pipetting, ) - assert request.pipetteId == "abc" - assert request.labwareId == "123" - assert request.wellName == "A3" - - -def test_move_to_well_result() -> None: - """It should be able to create a MoveToWellResult.""" - # NOTE(mc, 2020-11-17): this model has no properties at this time - result = MoveToWellResult() - - assert result - - -async def test_move_to_well_implementation(mock_handlers: AsyncMock) -> None: - """A MoveToWellRequest should have an execution implementation.""" - mock_handlers.movement.move_to_well.return_value = None - - request = MoveToWellRequest( + data = MoveToWellData( pipetteId="abc", labwareId="123", wellName="A3", ) - impl = request.get_implementation() - result = await impl.execute(mock_handlers) + result = await subject.execute(data) assert result == MoveToWellResult() - mock_handlers.movement.move_to_well.assert_called_with( - pipette_id="abc", - labware_id="123", - well_name="A3", + decoy.verify( + await movement.move_to_well( + pipette_id="abc", + labware_id="123", + well_name="A3", + ) ) diff --git a/api/tests/opentrons/protocol_engine/commands/test_pick_up_tip.py b/api/tests/opentrons/protocol_engine/commands/test_pick_up_tip.py index 853f834dbce..74989ced974 100644 --- a/api/tests/opentrons/protocol_engine/commands/test_pick_up_tip.py +++ b/api/tests/opentrons/protocol_engine/commands/test_pick_up_tip.py @@ -1,49 +1,46 @@ """Test pick up tip commands.""" -from mock import AsyncMock # type: ignore[attr-defined] +from decoy import Decoy -from opentrons.protocol_engine.commands import ( - PickUpTipRequest, +from opentrons.protocol_engine.execution import ( + EquipmentHandler, + MovementHandler, + PipettingHandler, +) + +from opentrons.protocol_engine.commands.pick_up_tip import ( + PickUpTipData, PickUpTipResult, + PickUpTipImplementation, ) -def test_pick_up_tip_request() -> None: - """It should be able to create a PickUpTipRequest.""" - request = PickUpTipRequest( - pipetteId="abc", - labwareId="123", - wellName="A3", +async def test_pick_up_tip_implementation( + decoy: Decoy, + equipment: EquipmentHandler, + movement: MovementHandler, + pipetting: PipettingHandler, +) -> None: + """A PickUpTip command should have an execution implementation.""" + subject = PickUpTipImplementation( + equipment=equipment, + movement=movement, + pipetting=pipetting, ) - assert request.pipetteId == "abc" - assert request.labwareId == "123" - assert request.wellName == "A3" - - -def test_pick_up_tip_result() -> None: - """It should be able to create a PickUpTipResult.""" - # NOTE(mc, 2020-11-17): this model has no properties at this time - result = PickUpTipResult() - - assert result - - -async def test_pick_up_tip_implementation(mock_handlers: AsyncMock) -> None: - """A PickUpTipRequest should have an execution implementation.""" - mock_handlers.pipetting.pick_up_tip.return_value = None - - request = PickUpTipRequest( + data = PickUpTipData( pipetteId="abc", labwareId="123", wellName="A3", ) - impl = request.get_implementation() - result = await impl.execute(mock_handlers) + result = await subject.execute(data) assert result == PickUpTipResult() - mock_handlers.pipetting.pick_up_tip.assert_called_with( - pipette_id="abc", - labware_id="123", - well_name="A3", + + decoy.verify( + await pipetting.pick_up_tip( + pipette_id="abc", + labware_id="123", + well_name="A3", + ) ) diff --git a/api/tests/opentrons/protocol_engine/conftest.py b/api/tests/opentrons/protocol_engine/conftest.py index 61ed9d877d9..ba8f50750be 100644 --- a/api/tests/opentrons/protocol_engine/conftest.py +++ b/api/tests/opentrons/protocol_engine/conftest.py @@ -1,117 +1,11 @@ """ProtocolEngine shared test fixtures.""" import pytest -from datetime import datetime, timedelta -from mock import AsyncMock, MagicMock # type: ignore[attr-defined] -from decoy import Decoy from opentrons_shared_data.deck import load as load_deck from opentrons_shared_data.deck.dev_types import DeckDefinitionV2 from opentrons_shared_data.labware import load_definition from opentrons.protocols.models import LabwareDefinition from opentrons.protocols.api_support.constants import STANDARD_DECK, SHORT_TRASH_DECK -from opentrons.util.helpers import utc_now -from opentrons.hardware_control.api import API as HardwareController - -from opentrons.protocol_engine import ( - ProtocolEngine, - StateStore, - StateView, - CommandHandlers, - ResourceProviders, -) - -from opentrons.protocol_engine.execution import ( - EquipmentHandler, - MovementHandler, - PipettingHandler, -) - -from opentrons.protocol_engine.resources import ( - IdGenerator, - LabwareDataProvider, - DeckDataProvider, -) - - -@pytest.fixture -def decoy() -> Decoy: - """Get a fresh Decoy state container.""" - return Decoy() - - -@pytest.fixture -def now() -> datetime: - """Get the current UTC time.""" - return utc_now() - - -@pytest.fixture -def later(now: datetime) -> datetime: - """Get a future time.""" - return utc_now() + timedelta(seconds=42) - - -@pytest.fixture -def even_later(later: datetime) -> datetime: - """Get a future time.""" - return later + timedelta(minutes=42) - - -@pytest.fixture -def mock_state_store() -> MagicMock: - """Get a mock in the shape of a StateStore.""" - return MagicMock(spec=StateStore) - - -@pytest.fixture -def mock_state_view() -> MagicMock: - """Get a mock in the shape of a StateView.""" - # TODO(mc, 2021-01-04): Replace with mock_state_view in execution/conftest.py - return MagicMock(spec=StateView) - - -@pytest.fixture -def mock_hardware() -> AsyncMock: - """Get an asynchronous mock in the shape of a HardwareController.""" - # TODO(mc, 2021-01-04): Replace with mock_hw_controller - return AsyncMock(spec=HardwareController) - - -@pytest.fixture -def mock_handlers() -> AsyncMock: - """Get an asynchronous mock in the shape of CommandHandlers.""" - # TODO(mc, 2021-01-04): Replace with mock_cmd_handlers - mock = AsyncMock() - mock.equipment = AsyncMock(spec=EquipmentHandler) - mock.movement = AsyncMock(spec=MovementHandler) - mock.pipetting = AsyncMock(spec=PipettingHandler) - - return mock - - -@pytest.fixture -def mock_cmd_handlers(decoy: Decoy) -> CommandHandlers: - """Get a mock in the shape of a CommandHandlers container.""" - return decoy.create_decoy(spec=CommandHandlers) - - -@pytest.fixture -def mock_hw_controller(decoy: Decoy) -> HardwareController: - """Get a mock in the shape of a HardwareController.""" - return decoy.create_decoy(spec=HardwareController) - - -@pytest.fixture -def mock_resources() -> AsyncMock: - """Get an asynchronous mock in the shape of ResourceProviders.""" - # TODO(mc, 2020-11-18): AsyncMock around ResourceProviders doesn't propagate - # async. mock downwards into children properly, so this has to be manually - # set up this way for now - return ResourceProviders( - id_generator=MagicMock(spec=IdGenerator), - labware_data=AsyncMock(spec=LabwareDataProvider), - deck_data=AsyncMock(spec=DeckDataProvider), - ) @pytest.fixture(scope="session") @@ -160,26 +54,3 @@ def reservoir_def() -> LabwareDefinition: 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)) - - -@pytest.fixture -def store(standard_deck_def: DeckDefinitionV2) -> StateStore: - """Get an actual StateStore.""" - return 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, - ) diff --git a/api/tests/opentrons/protocol_engine/execution/conftest.py b/api/tests/opentrons/protocol_engine/execution/conftest.py deleted file mode 100644 index 84fa758761a..00000000000 --- a/api/tests/opentrons/protocol_engine/execution/conftest.py +++ /dev/null @@ -1,11 +0,0 @@ -"""Execution test fixtures.""" -import pytest -from decoy import Decoy - -from opentrons.protocol_engine.execution.movement import MovementHandler - - -@pytest.fixture -def mock_movement_handler(decoy: Decoy) -> MovementHandler: - """Get a mock in the shape of a MovementHandler.""" - return decoy.create_decoy(spec=MovementHandler) diff --git a/api/tests/opentrons/protocol_engine/execution/test_command_executor.py b/api/tests/opentrons/protocol_engine/execution/test_command_executor.py new file mode 100644 index 00000000000..2cf38f707f2 --- /dev/null +++ b/api/tests/opentrons/protocol_engine/execution/test_command_executor.py @@ -0,0 +1,245 @@ +"""Smoke tests for the CommandExecutor class.""" +import pytest +from datetime import datetime +from decoy import Decoy +from pydantic import BaseModel +from typing import Optional, Type, cast + +from opentrons.protocol_engine.errors import ProtocolEngineError +from opentrons.protocol_engine.resources import ResourceProviders + +from opentrons.protocol_engine.commands import ( + AbstractCommandImpl, + BaseCommand, + CommandMapper, + CommandStatus, + Command, +) + +from opentrons.protocol_engine.execution import ( + CommandExecutor, + EquipmentHandler, + MovementHandler, + PipettingHandler, +) + + +@pytest.fixture +def equipment(decoy: Decoy) -> EquipmentHandler: + """Get a mocked out EquipmentHandler.""" + return decoy.create_decoy(spec=EquipmentHandler) + + +@pytest.fixture +def movement(decoy: Decoy) -> MovementHandler: + """Get a mocked out MovementHandler.""" + return decoy.create_decoy(spec=MovementHandler) + + +@pytest.fixture +def pipetting(decoy: Decoy) -> PipettingHandler: + """Get a mocked out PipettingHandler.""" + return decoy.create_decoy(spec=PipettingHandler) + + +@pytest.fixture +def resources(decoy: Decoy) -> ResourceProviders: + """Get a mocked out ResourceProviders.""" + return decoy.create_decoy(spec=ResourceProviders) + + +@pytest.fixture +def command_mapper(decoy: Decoy) -> CommandMapper: + """Get a mocked out CommandMapper.""" + return decoy.create_decoy(spec=CommandMapper) + + +@pytest.fixture +def subject( + equipment: EquipmentHandler, + movement: MovementHandler, + pipetting: PipettingHandler, + resources: ResourceProviders, + command_mapper: CommandMapper, +) -> CommandExecutor: + """Get a CommandExecutor test subject with its dependencies mocked out.""" + return CommandExecutor( + equipment=equipment, + movement=movement, + pipetting=pipetting, + resources=resources, + command_mapper=command_mapper, + ) + + +class _TestCommandData(BaseModel): + foo: str = "foo" + + +class _TestCommandResult(BaseModel): + bar: str = "bar" + + +class _TestCommandImpl(AbstractCommandImpl[_TestCommandData, _TestCommandResult]): + async def execute(self, data: _TestCommandData) -> _TestCommandResult: + raise NotImplementedError() + + +async def test_execute( + decoy: Decoy, + equipment: EquipmentHandler, + movement: MovementHandler, + pipetting: PipettingHandler, + resources: ResourceProviders, + command_mapper: CommandMapper, + subject: CommandExecutor, +) -> None: + """It should be able execute a command.""" + TestCommandImplCls = decoy.create_decoy_func(spec=_TestCommandImpl) + command_impl = decoy.create_decoy(spec=_TestCommandImpl) + + class _TestCommand(BaseCommand[_TestCommandData, _TestCommandResult]): + commandType: str = "testCommand" + data: _TestCommandData + result: Optional[_TestCommandResult] + + @property + def _ImplementationCls(self) -> Type[_TestCommandImpl]: + return TestCommandImplCls + + command_data = _TestCommandData() + command_result = _TestCommandResult() + + running_command = cast( + Command, + _TestCommand( + id="command-id", + createdAt=datetime(year=2021, month=1, day=1), + startedAt=datetime(year=2022, month=2, day=2), + status=CommandStatus.RUNNING, + data=command_data, + ), + ) + + completed_command = cast( + Command, + _TestCommand( + id="command-id", + createdAt=datetime(year=2021, month=1, day=1), + startedAt=datetime(year=2022, month=2, day=2), + completedAt=datetime(year=2023, month=3, day=3), + status=CommandStatus.SUCCEEDED, + data=command_data, + result=command_result, + ), + ) + + decoy.when( + running_command._ImplementationCls( + equipment=equipment, + movement=movement, + pipetting=pipetting, + ) + ).then_return( + command_impl # type: ignore[arg-type] + ) + + decoy.when(await command_impl.execute(command_data)).then_return(command_result) + + decoy.when(resources.model_utils.get_timestamp()).then_return( + datetime(year=2023, month=3, day=3) + ) + + decoy.when( + command_mapper.update_command( + command=running_command, + status=CommandStatus.SUCCEEDED, + completedAt=datetime(year=2023, month=3, day=3), + result=command_result, + error=None, + ) + ).then_return(completed_command) + + result = await subject.execute(running_command) + + assert result == completed_command + + +async def test_execute_raises_protocol_engine_error( + decoy: Decoy, + equipment: EquipmentHandler, + movement: MovementHandler, + pipetting: PipettingHandler, + resources: ResourceProviders, + command_mapper: CommandMapper, + subject: CommandExecutor, +) -> None: + """It should be able execute a command.""" + TestCommandImplCls = decoy.create_decoy_func(spec=_TestCommandImpl) + command_impl = decoy.create_decoy(spec=_TestCommandImpl) + + class _TestCommand(BaseCommand[_TestCommandData, _TestCommandResult]): + commandType: str = "testCommand" + data: _TestCommandData + result: Optional[_TestCommandResult] + + @property + def _ImplementationCls(self) -> Type[_TestCommandImpl]: + return TestCommandImplCls + + command_data = _TestCommandData() + command_error = ProtocolEngineError("oh no") + + running_command = cast( + Command, + _TestCommand( + id="command-id", + createdAt=datetime(year=2021, month=1, day=1), + startedAt=datetime(year=2022, month=2, day=2), + status=CommandStatus.RUNNING, + data=command_data, + ), + ) + + failed_command = cast( + Command, + _TestCommand( + id="command-id", + createdAt=datetime(year=2021, month=1, day=1), + startedAt=datetime(year=2022, month=2, day=2), + completedAt=datetime(year=2023, month=3, day=3), + status=CommandStatus.FAILED, + data=command_data, + error="oh no", + ), + ) + + decoy.when( + running_command._ImplementationCls( + equipment=equipment, + movement=movement, + pipetting=pipetting, + ) + ).then_return( + command_impl # type: ignore[arg-type] + ) + + decoy.when(await command_impl.execute(command_data)).then_raise(command_error) + + decoy.when(resources.model_utils.get_timestamp()).then_return( + datetime(year=2023, month=3, day=3) + ) + + decoy.when( + command_mapper.update_command( + command=running_command, + status=CommandStatus.FAILED, + completedAt=datetime(year=2023, month=3, day=3), + result=None, + error="oh no", + ) + ).then_return(failed_command) + + result = await subject.execute(running_command) + + assert result == failed_command diff --git a/api/tests/opentrons/protocol_engine/execution/test_equipment_handler.py b/api/tests/opentrons/protocol_engine/execution/test_equipment_handler.py index 35801ae7af6..74722435db3 100644 --- a/api/tests/opentrons/protocol_engine/execution/test_equipment_handler.py +++ b/api/tests/opentrons/protocol_engine/execution/test_equipment_handler.py @@ -2,14 +2,22 @@ import pytest from mock import AsyncMock, MagicMock # type: ignore[attr-defined] from opentrons.calibration_storage.helpers import uri_from_details -from opentrons.protocol_engine.errors import LabwareDefinitionDoesNotExistError -from opentrons.protocols.models import LabwareDefinition from opentrons.types import Mount as HwMount, MountType, DeckSlotName +from opentrons.hardware_control import API as HardwareAPI +from opentrons.protocols.models import LabwareDefinition -from opentrons.protocol_engine import errors, ResourceProviders +from opentrons.protocol_engine import errors +from opentrons.protocol_engine.errors import LabwareDefinitionDoesNotExistError from opentrons.protocol_engine.types import DeckSlotLocation, PipetteName -from opentrons.protocol_engine.state import PipetteData +from opentrons.protocol_engine.state import StateStore, PipetteData + +from opentrons.protocol_engine.resources import ( + ResourceProviders, + ModelUtils, + LabwareDataProvider, + DeckDataProvider, +) from opentrons.protocol_engine.execution.equipment import ( EquipmentHandler, @@ -18,6 +26,39 @@ ) +@pytest.fixture +def mock_state_store() -> MagicMock: + """Get a mock StateStore.""" + # TODO(mc, 2021-06-22): replace with Decoy mock + return MagicMock(spec=StateStore) + + +@pytest.fixture +def mock_state_view(mock_state_store: MagicMock) -> MagicMock: + """Get the StateView of the mock StateStore.""" + # TODO(mc, 2021-06-22): replace with Decoy mock + return mock_state_store.state_view + + +@pytest.fixture +def mock_hardware() -> AsyncMock: + """Get an asynchronous mock in the shape of a HardwareAPI.""" + # TODO(mc, 2021-06-22): Replace with Decoy mock + return AsyncMock(spec=HardwareAPI) + + +@pytest.fixture +def mock_resources() -> AsyncMock: + """Get an asynchronous mock in the shape of ResourceProviders.""" + # TODO(mc, 2021-06-22): Replace with Decoy mock + mock = AsyncMock() + mock.model_utils = MagicMock(spec=ModelUtils) + mock.labware_data = AsyncMock(spec=LabwareDataProvider) + mock.deck_data = AsyncMock(spec=DeckDataProvider) + + return mock + + @pytest.fixture def mock_resources_with_data( minimal_labware_def: LabwareDefinition, @@ -28,7 +69,7 @@ def mock_resources_with_data( minimal_labware_def ) mock_resources.labware_data.get_labware_calibration.return_value = (1, 2, 3) - mock_resources.id_generator.generate_id.return_value = "unique-id" + mock_resources.model_utils.generate_id.return_value = "unique-id" return mock_resources @@ -36,13 +77,13 @@ def mock_resources_with_data( @pytest.fixture def handler( mock_hardware: AsyncMock, - mock_state_view: MagicMock, + mock_state_store: MagicMock, mock_resources_with_data: AsyncMock, ) -> EquipmentHandler: """Get an EquipmentHandler with its dependencies mocked out.""" return EquipmentHandler( hardware=mock_hardware, - state=mock_state_view, + state_store=mock_state_store, resources=mock_resources_with_data, ) @@ -57,7 +98,7 @@ async def test_load_labware_assigns_id( load_name="load-name", namespace="opentrons-test", version=1, - labware_id=None + labware_id=None, ) assert type(res) == LoadedLabware @@ -74,12 +115,12 @@ async def test_load_labware_uses_provided_id( load_name="load-name", namespace="opentrons-test", version=1, - labware_id="my labware id" + labware_id="my labware id", ) assert type(res) == LoadedLabware assert res.labware_id == "my labware id" - mock_resources_with_data.id_generator.generate_id.assert_not_called() + mock_resources_with_data.model_utils.generate_id.assert_not_called() async def test_load_labware_gets_labware_def( @@ -89,8 +130,9 @@ async def test_load_labware_gets_labware_def( mock_state_view: MagicMock, ) -> None: """Loading labware should load the labware's defintion.""" - mock_state_view.labware.get_definition_by_uri.side_effect =\ + mock_state_view.labware.get_definition_by_uri.side_effect = ( LabwareDefinitionDoesNotExistError + ) res = await handler.load_labware( location=DeckSlotLocation(slot=DeckSlotName.SLOT_3), @@ -180,12 +222,12 @@ async def test_load_pipette_uses_provided_id( res = await handler.load_pipette( pipette_name=PipetteName.P300_SINGLE, mount=MountType.LEFT, - pipette_id="my pipette id" + pipette_id="my pipette id", ) assert type(res) == LoadedPipette assert res.pipette_id == "my pipette id" - mock_resources_with_data.id_generator.generate_id.assert_not_called() + mock_resources_with_data.model_utils.generate_id.assert_not_called() async def test_load_pipette_checks_checks_existence( diff --git a/api/tests/opentrons/protocol_engine/execution/test_movement_handler.py b/api/tests/opentrons/protocol_engine/execution/test_movement_handler.py index 1fdc407daf0..e2b6ed19664 100644 --- a/api/tests/opentrons/protocol_engine/execution/test_movement_handler.py +++ b/api/tests/opentrons/protocol_engine/execution/test_movement_handler.py @@ -7,41 +7,40 @@ from opentrons.hardware_control.types import CriticalPoint from opentrons.motion_planning import Waypoint -from opentrons.protocol_engine import StateView, DeckLocation, WellLocation, WellOrigin -from opentrons.protocol_engine.state import PipetteLocationData +from opentrons.protocol_engine import DeckLocation, WellLocation, WellOrigin +from opentrons.protocol_engine.state import StateStore, PipetteLocationData from opentrons.protocol_engine.execution.movement import MovementHandler -# TODO(mc, 2020-01-07): move to protocol_engine conftest @pytest.fixture -def mock_state_view(decoy: Decoy) -> StateView: - """Get a mock in the shape of a StateView.""" - return decoy.create_decoy(spec=StateView) +def hardware_api(decoy: Decoy) -> HardwareAPI: + """Get a mock in the shape of a HardwareAPI.""" + return decoy.create_decoy(spec=HardwareAPI) @pytest.fixture -def handler( - mock_state_view: StateView, - mock_hw_controller: HardwareAPI -) -> MovementHandler: +def state_store(decoy: Decoy) -> StateStore: + """Get a mock in the shape of a StateStore.""" + return decoy.create_decoy(spec=StateStore) + + +@pytest.fixture +def handler(state_store: StateStore, hardware_api: HardwareAPI) -> MovementHandler: """Create a PipettingHandler with its dependencies mocked out.""" - return MovementHandler( - state=mock_state_view, - hardware=mock_hw_controller - ) + return MovementHandler(state_store=state_store, hardware=hardware_api) async def test_move_to_well( decoy: Decoy, - mock_state_view: StateView, - mock_hw_controller: HardwareAPI, + state_store: StateStore, + hardware_api: HardwareAPI, handler: MovementHandler, ) -> None: """Move requests should call hardware controller with movement data.""" well_location = WellLocation(origin=WellOrigin.BOTTOM, offset=(0, 0, 1)) decoy.when( - mock_state_view.motion.get_pipette_location( + state_store.state_view.motion.get_pipette_location( pipette_id="pipette-id", current_location=None, ) @@ -53,18 +52,18 @@ async def test_move_to_well( ) decoy.when( - await mock_hw_controller.gantry_position( + await hardware_api.gantry_position( mount=Mount.LEFT, critical_point=CriticalPoint.FRONT_NOZZLE, ) ).then_return(Point(1, 1, 1)) - decoy.when( - mock_hw_controller.get_instrument_max_height(mount=Mount.LEFT) - ).then_return(42.0) + decoy.when(hardware_api.get_instrument_max_height(mount=Mount.LEFT)).then_return( + 42.0 + ) decoy.when( - mock_state_view.motion.get_movement_waypoints( + state_store.state_view.motion.get_movement_waypoints( origin=Point(1, 1, 1), origin_cp=CriticalPoint.FRONT_NOZZLE, max_travel_z=42.0, @@ -75,10 +74,7 @@ async def test_move_to_well( current_location=None, ) ).then_return( - [ - Waypoint(Point(1, 2, 3), CriticalPoint.XY_CENTER), - Waypoint(Point(4, 5, 6)) - ] + [Waypoint(Point(1, 2, 3), CriticalPoint.XY_CENTER), Waypoint(Point(4, 5, 6))] ) await handler.move_to_well( @@ -89,36 +85,32 @@ async def test_move_to_well( ) decoy.verify( - await mock_hw_controller.move_to( + await hardware_api.move_to( mount=Mount.LEFT, abs_position=Point(1, 2, 3), - critical_point=CriticalPoint.XY_CENTER + critical_point=CriticalPoint.XY_CENTER, ), - await mock_hw_controller.move_to( - mount=Mount.LEFT, - abs_position=Point(4, 5, 6), - critical_point=None + await hardware_api.move_to( + mount=Mount.LEFT, abs_position=Point(4, 5, 6), critical_point=None ), ) async def test_move_to_well_from_starting_location( decoy: Decoy, - mock_state_view: StateView, - mock_hw_controller: HardwareAPI, + state_store: StateStore, + hardware_api: HardwareAPI, handler: MovementHandler, ) -> None: """It should be able to move to a well from a start location.""" well_location = WellLocation(origin=WellOrigin.BOTTOM, offset=(0, 0, 1)) current_location = DeckLocation( - pipette_id="pipette-id", - labware_id="labware-id", - well_name="B2" + pipette_id="pipette-id", labware_id="labware-id", well_name="B2" ) decoy.when( - mock_state_view.motion.get_pipette_location( + state_store.state_view.motion.get_pipette_location( pipette_id="pipette-id", current_location=current_location, ) @@ -130,18 +122,18 @@ async def test_move_to_well_from_starting_location( ) decoy.when( - await mock_hw_controller.gantry_position( + await hardware_api.gantry_position( mount=Mount.RIGHT, critical_point=CriticalPoint.XY_CENTER, ) ).then_return(Point(1, 2, 5)) - decoy.when( - mock_hw_controller.get_instrument_max_height(mount=Mount.RIGHT) - ).then_return(42.0) + decoy.when(hardware_api.get_instrument_max_height(mount=Mount.RIGHT)).then_return( + 42.0 + ) decoy.when( - mock_state_view.motion.get_movement_waypoints( + state_store.state_view.motion.get_movement_waypoints( current_location=current_location, origin=Point(1, 2, 5), origin_cp=CriticalPoint.XY_CENTER, @@ -162,9 +154,9 @@ async def test_move_to_well_from_starting_location( ) decoy.verify( - await mock_hw_controller.move_to( + await hardware_api.move_to( mount=Mount.RIGHT, abs_position=Point(1, 2, 3), - critical_point=CriticalPoint.XY_CENTER + critical_point=CriticalPoint.XY_CENTER, ), ) diff --git a/api/tests/opentrons/protocol_engine/execution/test_pipetting_handler.py b/api/tests/opentrons/protocol_engine/execution/test_pipetting_handler.py index d78b3e4de88..f84faf99d73 100644 --- a/api/tests/opentrons/protocol_engine/execution/test_pipetting_handler.py +++ b/api/tests/opentrons/protocol_engine/execution/test_pipetting_handler.py @@ -8,8 +8,8 @@ from opentrons.hardware_control.api import API as HardwareAPI from opentrons.hardware_control.dev_types import PipetteDict -from opentrons.protocol_engine import StateView, DeckLocation, WellLocation, WellOrigin -from opentrons.protocol_engine.state import TipGeometry, HardwarePipette +from opentrons.protocol_engine import DeckLocation, WellLocation, WellOrigin +from opentrons.protocol_engine.state import StateStore, TipGeometry, HardwarePipette from opentrons.protocol_engine.execution.movement import MovementHandler from opentrons.protocol_engine.execution.pipetting import PipettingHandler @@ -31,22 +31,29 @@ def by_mount(self) -> Dict[Mount, PipetteDict]: return {Mount.LEFT: self.left_config, Mount.RIGHT: self.right_config} -# TODO(mc, 2020-01-07): move to protocol_engine conftest @pytest.fixture -def mock_state_view(decoy: Decoy) -> StateView: - """Get a mock in the shape of a StateView.""" - return decoy.create_decoy(spec=StateView) +def hardware_api(decoy: Decoy) -> HardwareAPI: + """Get a mock in the shape of a HardwareAPI.""" + return decoy.create_decoy(spec=HardwareAPI) @pytest.fixture -def mock_hw_pipettes( - mock_hw_controller: HardwareAPI -) -> MockPipettes: +def state_store(decoy: Decoy) -> StateStore: + """Get a mock in the shape of a StateStore.""" + return decoy.create_decoy(spec=StateStore) + + +@pytest.fixture +def movement_handler(decoy: Decoy) -> MovementHandler: + """Get a mock in the shape of a MovementHandler.""" + return decoy.create_decoy(spec=MovementHandler) + + +@pytest.fixture +def mock_hw_pipettes(hardware_api: HardwareAPI) -> MockPipettes: """Get mock pipette configs and attach them to the mock HW controller.""" mock_hw_pipettes = MockPipettes() - mock_hw_controller.attached_instruments = ( # type: ignore[misc] - mock_hw_pipettes.by_mount - ) + hardware_api.attached_instruments = mock_hw_pipettes.by_mount # type: ignore[misc] return mock_hw_pipettes @@ -69,29 +76,29 @@ def mock_right_pipette_config( @pytest.fixture def handler( - mock_state_view: StateView, - mock_hw_controller: HardwareAPI, - mock_movement_handler: MovementHandler, + state_store: StateStore, + hardware_api: HardwareAPI, + movement_handler: MovementHandler, ) -> PipettingHandler: """Create a PipettingHandler with its dependencies mocked out.""" return PipettingHandler( - state=mock_state_view, - hardware=mock_hw_controller, - movement_handler=mock_movement_handler, + state_store=state_store, + hardware=hardware_api, + movement_handler=movement_handler, ) async def test_handle_pick_up_tip_request( decoy: Decoy, - mock_state_view: StateView, - mock_hw_controller: HardwareAPI, - mock_movement_handler: MovementHandler, + state_store: StateStore, + hardware_api: HardwareAPI, + movement_handler: MovementHandler, mock_hw_pipettes: MockPipettes, handler: PipettingHandler, ) -> None: """It should handle a PickUpTipRequest properly.""" decoy.when( - mock_state_view.pipettes.get_hardware_pipette( + state_store.state_view.pipettes.get_hardware_pipette( pipette_id="pipette-id", attached_pipettes=mock_hw_pipettes.by_mount, ) @@ -100,7 +107,7 @@ async def test_handle_pick_up_tip_request( ) decoy.when( - mock_state_view.geometry.get_tip_geometry( + state_store.state_view.geometry.get_tip_geometry( labware_id="labware-id", well_name="B2", pipette_config=mock_hw_pipettes.left_config, @@ -120,36 +127,33 @@ async def test_handle_pick_up_tip_request( ) decoy.verify( - await mock_movement_handler.move_to_well( + await movement_handler.move_to_well( pipette_id="pipette-id", labware_id="labware-id", well_name="B2", ), - await mock_hw_controller.pick_up_tip( + await hardware_api.pick_up_tip( mount=Mount.LEFT, tip_length=50, presses=None, increment=None, ), - mock_hw_controller.set_current_tiprack_diameter( - mount=Mount.LEFT, - tiprack_diameter=5 - ), - mock_hw_controller.set_working_volume(mount=Mount.LEFT, tip_volume=300) + hardware_api.set_current_tiprack_diameter(mount=Mount.LEFT, tiprack_diameter=5), + hardware_api.set_working_volume(mount=Mount.LEFT, tip_volume=300), ) async def test_handle_drop_up_tip_request( decoy: Decoy, - mock_state_view: StateView, - mock_hw_controller: HardwareAPI, - mock_movement_handler: MovementHandler, + state_store: StateStore, + hardware_api: HardwareAPI, + movement_handler: MovementHandler, mock_hw_pipettes: MockPipettes, handler: PipettingHandler, ) -> None: """It should handle a DropTipRequest properly.""" decoy.when( - mock_state_view.pipettes.get_hardware_pipette( + state_store.state_view.pipettes.get_hardware_pipette( pipette_id="pipette-id", attached_pipettes=mock_hw_pipettes.by_mount, ) @@ -161,7 +165,7 @@ async def test_handle_drop_up_tip_request( ) decoy.when( - mock_state_view.geometry.get_tip_drop_location( + state_store.state_view.geometry.get_tip_drop_location( labware_id="labware-id", pipette_config=mock_hw_pipettes.right_config, ) @@ -174,21 +178,21 @@ async def test_handle_drop_up_tip_request( ) decoy.verify( - await mock_movement_handler.move_to_well( + await movement_handler.move_to_well( pipette_id="pipette-id", labware_id="labware-id", well_name="A1", - well_location=WellLocation(offset=(0, 0, 10)) + well_location=WellLocation(offset=(0, 0, 10)), ), - await mock_hw_controller.drop_tip(mount=Mount.RIGHT, home_after=True), + await hardware_api.drop_tip(mount=Mount.RIGHT, home_after=True), ) async def test_handle_aspirate_request_without_prep( decoy: Decoy, - mock_state_view: StateView, - mock_hw_controller: HardwareAPI, - mock_movement_handler: MovementHandler, + state_store: StateStore, + hardware_api: HardwareAPI, + movement_handler: MovementHandler, mock_hw_pipettes: MockPipettes, handler: PipettingHandler, ) -> None: @@ -196,7 +200,7 @@ async def test_handle_aspirate_request_without_prep( well_location = WellLocation(origin=WellOrigin.BOTTOM, offset=(0, 0, 1)) decoy.when( - mock_state_view.pipettes.get_hardware_pipette( + state_store.state_view.pipettes.get_hardware_pipette( pipette_id="pipette-id", attached_pipettes=mock_hw_pipettes.by_mount, ) @@ -208,7 +212,7 @@ async def test_handle_aspirate_request_without_prep( ) decoy.when( - mock_state_view.pipettes.get_is_ready_to_aspirate( + state_store.state_view.pipettes.get_is_ready_to_aspirate( pipette_id="pipette-id", pipette_config=mock_hw_pipettes.left_config, ) @@ -225,14 +229,14 @@ async def test_handle_aspirate_request_without_prep( assert volume == 25 decoy.verify( - await mock_movement_handler.move_to_well( + await movement_handler.move_to_well( pipette_id="pipette-id", labware_id="labware-id", well_name="C6", well_location=well_location, current_location=None, ), - await mock_hw_controller.aspirate( + await hardware_api.aspirate( mount=Mount.LEFT, volume=25, ), @@ -241,9 +245,9 @@ async def test_handle_aspirate_request_without_prep( async def test_handle_aspirate_request_with_prep( decoy: Decoy, - mock_state_view: StateView, - mock_hw_controller: HardwareAPI, - mock_movement_handler: MovementHandler, + state_store: StateStore, + hardware_api: HardwareAPI, + movement_handler: MovementHandler, mock_hw_pipettes: MockPipettes, handler: PipettingHandler, ) -> None: @@ -251,7 +255,7 @@ async def test_handle_aspirate_request_with_prep( well_location = WellLocation(origin=WellOrigin.BOTTOM, offset=(0, 0, 1)) decoy.when( - mock_state_view.pipettes.get_hardware_pipette( + state_store.state_view.pipettes.get_hardware_pipette( pipette_id="pipette-id", attached_pipettes=mock_hw_pipettes.by_mount, ) @@ -263,7 +267,7 @@ async def test_handle_aspirate_request_with_prep( ) decoy.when( - mock_state_view.pipettes.get_is_ready_to_aspirate( + state_store.state_view.pipettes.get_is_ready_to_aspirate( pipette_id="pipette-id", pipette_config=mock_hw_pipettes.left_config, ) @@ -280,32 +284,31 @@ async def test_handle_aspirate_request_with_prep( assert volume == 25 decoy.verify( - await mock_movement_handler.move_to_well( + await movement_handler.move_to_well( pipette_id="pipette-id", labware_id="labware-id", well_name="C6", well_location=WellLocation(origin=WellOrigin.TOP), ), - await mock_hw_controller.prepare_for_aspirate(mount=Mount.LEFT), - await mock_movement_handler.move_to_well( + await hardware_api.prepare_for_aspirate(mount=Mount.LEFT), + await movement_handler.move_to_well( pipette_id="pipette-id", labware_id="labware-id", well_name="C6", well_location=well_location, current_location=DeckLocation( - pipette_id="pipette-id", - labware_id="labware-id", - well_name="C6"), + pipette_id="pipette-id", labware_id="labware-id", well_name="C6" + ), ), - await mock_hw_controller.aspirate(mount=Mount.LEFT, volume=25), + await hardware_api.aspirate(mount=Mount.LEFT, volume=25), ) async def test_handle_dispense_request( decoy: Decoy, - mock_state_view: StateView, - mock_hw_controller: HardwareAPI, - mock_movement_handler: MovementHandler, + state_store: StateStore, + hardware_api: HardwareAPI, + movement_handler: MovementHandler, mock_hw_pipettes: MockPipettes, handler: PipettingHandler, ) -> None: @@ -313,7 +316,7 @@ async def test_handle_dispense_request( well_location = WellLocation(origin=WellOrigin.BOTTOM, offset=(0, 0, 1)) decoy.when( - mock_state_view.pipettes.get_hardware_pipette( + state_store.state_view.pipettes.get_hardware_pipette( pipette_id="pipette-id", attached_pipettes=mock_hw_pipettes.by_mount, ) @@ -335,11 +338,11 @@ async def test_handle_dispense_request( assert volume == 25 decoy.verify( - await mock_movement_handler.move_to_well( + await movement_handler.move_to_well( pipette_id="pipette-id", labware_id="labware-id", well_name="C6", well_location=well_location, ), - await mock_hw_controller.dispense(mount=Mount.RIGHT, volume=25), + await hardware_api.dispense(mount=Mount.RIGHT, volume=25), ) diff --git a/api/tests/opentrons/protocol_engine/resources/test_id_generator.py b/api/tests/opentrons/protocol_engine/resources/test_id_generator.py index 632ebbdebdc..f908760da04 100644 --- a/api/tests/opentrons/protocol_engine/resources/test_id_generator.py +++ b/api/tests/opentrons/protocol_engine/resources/test_id_generator.py @@ -1,23 +1,23 @@ -"""Simple functional tests for the IdGenerator provider.""" +"""Simple functional tests for the ModelUtils provider.""" import re -from opentrons.protocol_engine.resources import IdGenerator +from opentrons.protocol_engine.resources import ModelUtils RE_UUID = re.compile( r"^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$", - flags=re.IGNORECASE + flags=re.IGNORECASE, ) -def test_id_generator_generates_uuid() -> None: +def test_model_utils_generates_uuid() -> None: """It should generate a string matching a UUID.""" - result = IdGenerator().generate_id() + result = ModelUtils().generate_id() assert RE_UUID.match(result) -def test_id_generator_generates_unique() -> None: +def test_model_utils_generates_unique() -> None: """It should generate unique IDs.""" - results = [IdGenerator().generate_id() for i in range(1000)] + results = [ModelUtils().generate_id() for i in range(1000)] unique_results = set(results) assert len(results) == len(unique_results) diff --git a/api/tests/opentrons/protocol_engine/state/command_fixtures.py b/api/tests/opentrons/protocol_engine/state/command_fixtures.py index f90529dd02d..7fa4e57a374 100644 --- a/api/tests/opentrons/protocol_engine/state/command_fixtures.py +++ b/api/tests/opentrons/protocol_engine/state/command_fixtures.py @@ -6,66 +6,78 @@ from opentrons.types import MountType from opentrons.protocols.models import LabwareDefinition from opentrons.protocol_engine import commands as cmd -from opentrons.protocol_engine.errors import ProtocolEngineError from opentrons.protocol_engine.types import PipetteName, WellLocation, LabwareLocation def create_pending_command( - request: Optional[BaseModel] = None, -) -> cmd.PendingCommandType: - """Given a request, build a pending command model.""" + command_id: str = "command-id", + command_type: str = "command-type", + data: Optional[BaseModel] = None, +) -> cmd.Command: + """Given command data, build a pending command model.""" return cast( - cmd.PendingCommandType, - cmd.PendingCommand( - created_at=datetime(year=2021, month=1, day=1), - request=request or BaseModel(), + cmd.Command, + cmd.BaseCommand( + id=command_id, + commandType=command_type, + createdAt=datetime(year=2021, month=1, day=1), + status=cmd.CommandStatus.QUEUED, + data=data or BaseModel(), ), ) def create_running_command( - request: Optional[BaseModel] = None, -) -> cmd.RunningCommandType: - """Given a request, build a running command model.""" + command_id: str = "command-id", + command_type: str = "command-type", + data: Optional[BaseModel] = None, +) -> cmd.Command: + """Given command data, build a running command model.""" return cast( - cmd.RunningCommandType, - cmd.RunningCommand( - created_at=datetime(year=2021, month=1, day=1), - started_at=datetime(year=2022, month=2, day=2), - request=request or BaseModel(), + cmd.Command, + cmd.BaseCommand( + id=command_id, + createdAt=datetime(year=2021, month=1, day=1), + commandType=command_type, + status=cmd.CommandStatus.RUNNING, + data=data or BaseModel(), ), ) def create_failed_command( - request: Optional[BaseModel] = None, - error: Optional[ProtocolEngineError] = None, -) -> cmd.FailedCommandType: - """Given a request and error, build a failed command model.""" + command_id: str = "command-id", + command_type: str = "command-type", + data: Optional[BaseModel] = None, +) -> cmd.Command: + """Given command data, build a failed command model.""" return cast( - cmd.FailedCommandType, - cmd.FailedCommand( - created_at=datetime(year=2021, month=1, day=1), - started_at=datetime(year=2022, month=2, day=2), - failed_at=datetime(year=2023, month=3, day=3), - request=request or BaseModel(), - error=error or ProtocolEngineError(), + cmd.Command, + cmd.BaseCommand( + id=command_id, + createdAt=datetime(year=2021, month=1, day=1), + commandType=command_type, + status=cmd.CommandStatus.FAILED, + data=data or BaseModel(), ), ) def create_completed_command( - request: Optional[BaseModel] = None, + command_id: str = "command-id", + command_type: str = "command-type", + data: Optional[BaseModel] = None, result: Optional[BaseModel] = None, -) -> cmd.CompletedCommandType: - """Given a request and result, build a completed command model.""" +) -> cmd.Command: + """Given command data and result, build a completed command model.""" return cast( - cmd.CompletedCommandType, - cmd.CompletedCommand( - created_at=datetime(year=2021, month=1, day=1), - started_at=datetime(year=2022, month=2, day=2), - completed_at=datetime(year=2023, month=3, day=3), - request=request or BaseModel(), + cmd.Command, + cmd.BaseCommand( + id=command_id, + createdAt=datetime(year=2021, month=1, day=1), + commandType=command_type, + status=cmd.CommandStatus.SUCCEEDED, + data=data or BaseModel(), result=result or BaseModel(), ), ) @@ -76,48 +88,68 @@ def create_load_labware_command( location: LabwareLocation, definition: LabwareDefinition, calibration: Tuple[float, float, float], -) -> cmd.CompletedCommandType: +) -> cmd.LoadLabware: """Create a completed LoadLabware command.""" - request = cmd.LoadLabwareRequest( + data = cmd.LoadLabwareData( loadName=definition.parameters.loadName, namespace=definition.namespace, version=definition.version, location=location, labwareId=None, ) + result = cmd.LoadLabwareResult( labwareId=labware_id, definition=definition, calibration=calibration, ) - return create_completed_command(request, result) + return cmd.LoadLabware( + id="command-id", + status=cmd.CommandStatus.SUCCEEDED, + createdAt=datetime.now(), + data=data, + result=result, + ) def create_add_definition_command( definition: LabwareDefinition, -) -> cmd.CompletedCommandType: +) -> cmd.AddLabwareDefinition: """Create a completed AddLabwareDefinition command.""" - request = cmd.AddLabwareDefinitionRequest(definition=definition) + data = cmd.AddLabwareDefinitionData(definition=definition) + result = cmd.AddLabwareDefinitionResult( loadName=definition.parameters.loadName, namespace=definition.namespace, version=definition.version, ) - return create_completed_command(request, result) + return cmd.AddLabwareDefinition( + id="command-id", + status=cmd.CommandStatus.SUCCEEDED, + createdAt=datetime.now(), + data=data, + result=result, + ) def create_load_pipette_command( pipette_id: str, pipette_name: PipetteName, mount: MountType, -) -> cmd.CompletedCommandType: +) -> cmd.LoadPipette: """Get a completed LoadPipette command.""" - request = cmd.LoadPipetteRequest(pipetteName=pipette_name, mount=mount) + data = cmd.LoadPipetteData(pipetteName=pipette_name, mount=mount) result = cmd.LoadPipetteResult(pipetteId=pipette_id) - return create_completed_command(request, result) + return cmd.LoadPipette( + id="command-id", + status=cmd.CommandStatus.SUCCEEDED, + createdAt=datetime.now(), + data=data, + result=result, + ) def create_aspirate_command( @@ -126,9 +158,9 @@ def create_aspirate_command( labware_id: str = "labware-id", well_name: str = "A1", well_location: Optional[WellLocation] = None, -) -> cmd.CompletedCommandType: +) -> cmd.Aspirate: """Get a completed Aspirate command.""" - request = cmd.AspirateRequest( + data = cmd.AspirateData( pipetteId=pipette_id, labwareId=labware_id, wellName=well_name, @@ -137,7 +169,13 @@ def create_aspirate_command( ) result = cmd.AspirateResult(volume=volume) - return create_completed_command(request, result) + return cmd.Aspirate( + id="command-id", + status=cmd.CommandStatus.SUCCEEDED, + createdAt=datetime.now(), + data=data, + result=result, + ) def create_dispense_command( @@ -146,9 +184,9 @@ def create_dispense_command( labware_id: str = "labware-id", well_name: str = "A1", well_location: Optional[WellLocation] = None, -) -> cmd.CompletedCommandType: +) -> cmd.Dispense: """Get a completed Dispense command.""" - request = cmd.DispenseRequest( + data = cmd.DispenseData( pipetteId=pipette_id, labwareId=labware_id, wellName=well_name, @@ -157,16 +195,22 @@ def create_dispense_command( ) result = cmd.DispenseResult(volume=volume) - return create_completed_command(request, result) + return cmd.Dispense( + id="command-id", + status=cmd.CommandStatus.SUCCEEDED, + createdAt=datetime.now(), + data=data, + result=result, + ) def create_pick_up_tip_command( pipette_id: str, labware_id: str = "labware-id", well_name: str = "A1", -) -> cmd.CompletedCommandType: +) -> cmd.PickUpTip: """Get a completed PickUpTip command.""" - request = cmd.PickUpTipRequest( + data = cmd.PickUpTipData( pipetteId=pipette_id, labwareId=labware_id, wellName=well_name, @@ -174,16 +218,22 @@ def create_pick_up_tip_command( result = cmd.PickUpTipResult() - return create_completed_command(request, result) + return cmd.PickUpTip( + id="command-id", + status=cmd.CommandStatus.SUCCEEDED, + createdAt=datetime.now(), + data=data, + result=result, + ) def create_drop_tip_command( pipette_id: str, labware_id: str = "labware-id", well_name: str = "A1", -) -> cmd.CompletedCommandType: +) -> cmd.DropTip: """Get a completed DropTip command.""" - request = cmd.DropTipRequest( + data = cmd.DropTipData( pipetteId=pipette_id, labwareId=labware_id, wellName=well_name, @@ -191,16 +241,22 @@ def create_drop_tip_command( result = cmd.DropTipResult() - return create_completed_command(request, result) + return cmd.DropTip( + id="command-id", + status=cmd.CommandStatus.SUCCEEDED, + createdAt=datetime.now(), + data=data, + result=result, + ) def create_move_to_well_command( pipette_id: str, labware_id: str = "labware-id", well_name: str = "A1", -) -> cmd.CompletedCommandType: +) -> cmd.MoveToWell: """Get a completed MoveToWell command.""" - request = cmd.MoveToWellRequest( + data = cmd.MoveToWellData( pipetteId=pipette_id, labwareId=labware_id, wellName=well_name, @@ -208,4 +264,10 @@ def create_move_to_well_command( result = cmd.MoveToWellResult() - return create_completed_command(request, result) + return cmd.MoveToWell( + id="command-id", + status=cmd.CommandStatus.SUCCEEDED, + createdAt=datetime.now(), + data=data, + result=result, + ) diff --git a/api/tests/opentrons/protocol_engine/state/test_command_store.py b/api/tests/opentrons/protocol_engine/state/test_command_store.py index 2da2765adfd..9e2d514be2f 100644 --- a/api/tests/opentrons/protocol_engine/state/test_command_store.py +++ b/api/tests/opentrons/protocol_engine/state/test_command_store.py @@ -1,7 +1,6 @@ """Tests for the command lifecycle state.""" from collections import OrderedDict -from opentrons.protocol_engine import commands as cmd from opentrons.protocol_engine.state.commands import CommandState, CommandStore from .command_fixtures import ( @@ -13,10 +12,10 @@ def test_command_store_handles_command() -> None: """It should add a command to the store.""" - command: cmd.PendingCommand = create_pending_command() + command = create_pending_command(command_id="command-id") subject = CommandStore() - subject.handle_command(command=command, command_id="command-id") + subject.handle_command(command=command) assert subject.state == CommandState( commands_by_id=OrderedDict({"command-id": command}) @@ -26,13 +25,13 @@ def test_command_store_handles_command() -> None: def test_command_store_preserves_handle_order() -> None: """It should store commands in the order they are handled.""" # Any arbitrary 3 commands that compare non-equal (!=) to each other. - command_a: cmd.PendingCommand = create_pending_command() - command_b: cmd.RunningCommand = create_running_command() - command_c: cmd.CompletedCommand = create_completed_command() + command_a = create_pending_command(command_id="command-id-1") + command_b = create_running_command(command_id="command-id-2") + command_c = create_completed_command(command_id="command-id-1") subject = CommandStore() - subject.handle_command(command_a, "command-id-1") - subject.handle_command(command_b, "command-id-2") + subject.handle_command(command_a) + subject.handle_command(command_b) assert subject.state == CommandState( commands_by_id=OrderedDict( [ @@ -42,7 +41,7 @@ def test_command_store_preserves_handle_order() -> None: ) ) - subject.handle_command(command_c, "command-id-1") + subject.handle_command(command_c) assert subject.state == CommandState( commands_by_id=OrderedDict( [ diff --git a/api/tests/opentrons/protocol_engine/state/test_command_view.py b/api/tests/opentrons/protocol_engine/state/test_command_view.py index 31bed2e9f6b..6799d138277 100644 --- a/api/tests/opentrons/protocol_engine/state/test_command_view.py +++ b/api/tests/opentrons/protocol_engine/state/test_command_view.py @@ -1,8 +1,9 @@ """Labware state store tests.""" +import pytest from collections import OrderedDict from typing import Sequence, Tuple -from opentrons.protocol_engine import commands as cmd +from opentrons.protocol_engine import commands as cmd, errors from opentrons.protocol_engine.state.commands import CommandState, CommandView @@ -15,7 +16,7 @@ def get_command_view( - commands_by_id: Sequence[Tuple[str, cmd.CommandType]] = () + commands_by_id: Sequence[Tuple[str, cmd.Command]] = () ) -> CommandView: """Get a command view test subject.""" state = CommandState(commands_by_id=OrderedDict(commands_by_id)) @@ -25,27 +26,26 @@ def get_command_view( def test_get_command_by_id() -> None: """It should get a command by ID from state.""" - command = create_completed_command() + command = create_completed_command(command_id="command-id") subject = get_command_view(commands_by_id=[("command-id", command)]) assert subject.get_command_by_id("command-id") == command def test_get_command_bad_id() -> None: - """It should return None if a requested command ID isn't in state.""" - command = create_completed_command() + """It should raise if a requested command ID isn't in state.""" + command = create_completed_command(command_id="command-id") subject = get_command_view(commands_by_id=[("command-id", command)]) - result = subject.get_command_by_id("asdfghjkl") - - assert result is None + with pytest.raises(errors.CommandDoesNotExistError): + subject.get_command_by_id("asdfghjkl") def test_get_all_commands() -> None: """It should get all the commands from the state.""" - command_1 = create_completed_command() - command_2 = create_running_command() - command_3 = create_pending_command() + command_1 = create_completed_command(command_id="command-id-1") + command_2 = create_running_command(command_id="command-id-2") + command_3 = create_pending_command(command_id="command-id-3") subject = get_command_view( commands_by_id=[ @@ -62,7 +62,7 @@ def test_get_all_commands() -> None: ] -def test_get_next_request_returns_first_pending() -> None: +def test_get_next_command_returns_first_pending() -> None: """It should return the first command that's pending.""" pending_command = create_pending_command() running_command = create_running_command() @@ -77,18 +77,18 @@ def test_get_next_request_returns_first_pending() -> None: ] ) - assert subject.get_next_request() == ("command-id-3", pending_command.request) + assert subject.get_next_command() == "command-id-3" -def test_get_next_request_returns_none_when_no_pending() -> None: +def test_get_next_command_returns_none_when_no_pending() -> None: """It should return None if there are no pending commands to return.""" - running_command = create_running_command() - failed_command = create_failed_command() - completed_command = create_completed_command() + running_command = create_running_command(command_id="command-id-1") + completed_command = create_completed_command(command_id="command-id-2") + failed_command = create_failed_command(command_id="command-id-3") subject = get_command_view() - assert subject.get_next_request() is None + assert subject.get_next_command() is None subject = get_command_view( commands_by_id=[ @@ -98,15 +98,15 @@ def test_get_next_request_returns_none_when_no_pending() -> None: ] ) - assert subject.get_next_request() is None + assert subject.get_next_command() is None -def test_get_next_request_returns_none_when_earlier_command_failed() -> None: +def test_get_next_command_returns_none_when_earlier_command_failed() -> None: """It should return None if any prior-added command is failed.""" - pending_command = create_pending_command() - running_command = create_running_command() - failed_command = create_failed_command() - completed_command = create_completed_command() + running_command = create_running_command(command_id="command-id-1") + completed_command = create_completed_command(command_id="command-id-2") + failed_command = create_failed_command(command_id="command-id-3") + pending_command = create_pending_command(command_id="command-id-4") subject = get_command_view( commands_by_id=[ @@ -117,4 +117,4 @@ def test_get_next_request_returns_none_when_earlier_command_failed() -> None: ] ) - assert subject.get_next_request() is None + assert subject.get_next_command() is None diff --git a/api/tests/opentrons/protocol_engine/state/test_geometry_view.py b/api/tests/opentrons/protocol_engine/state/test_geometry_view.py index 49093d13540..3700e96bfdc 100644 --- a/api/tests/opentrons/protocol_engine/state/test_geometry_view.py +++ b/api/tests/opentrons/protocol_engine/state/test_geometry_view.py @@ -7,7 +7,6 @@ from opentrons_shared_data.deck.dev_types import DeckDefinitionV2 from opentrons.protocols.models import LabwareDefinition from opentrons.hardware_control.dev_types import PipetteDict -from opentrons.protocols.geometry.deck import FIXED_TRASH_ID from opentrons.types import Point, DeckSlotName from opentrons.protocol_engine import errors @@ -481,14 +480,20 @@ def test_get_tip_drop_location( def test_get_tip_drop_location_with_trash( + decoy: Decoy, labware_view: LabwareView, subject: GeometryView, ) -> None: """It should get relative drop tip location for a the fixed trash.""" pipette_config: PipetteDict = cast(PipetteDict, {"return_tip_height": 0.7}) + decoy.when( + labware_view.get_labware_has_quirk(labware_id="labware-id", quirk="fixedTrash") + ).then_return(True) + location = subject.get_tip_drop_location( - labware_id=FIXED_TRASH_ID, pipette_config=pipette_config + labware_id="labware-id", + pipette_config=pipette_config, ) assert location == WellLocation(origin=WellOrigin.TOP, offset=(0, 0, 0)) diff --git a/api/tests/opentrons/protocol_engine/state/test_labware_store.py b/api/tests/opentrons/protocol_engine/state/test_labware_store.py index adb07103bca..94d4074f8f8 100644 --- a/api/tests/opentrons/protocol_engine/state/test_labware_store.py +++ b/api/tests/opentrons/protocol_engine/state/test_labware_store.py @@ -86,7 +86,7 @@ def test_handles_load_labware( calibration=(1, 2, 3), ) - subject.handle_completed_command(command) + subject.handle_command(command) # TODO(mc, 2021-06-02): usage of ._state over .state is temporary # until store.state returns the state instead of a state view @@ -109,7 +109,7 @@ def test_handles_add_labware_defintion( version=well_plate_def.version, ) - subject.handle_completed_command(command) + subject.handle_command(command) # TODO(mc, 2021-06-02): usage of ._state over .state is temporary # until store.state returns the state instead of a state view diff --git a/api/tests/opentrons/protocol_engine/state/test_pipette_store.py b/api/tests/opentrons/protocol_engine/state/test_pipette_store.py index b28c5cc587b..9622ad50b2f 100644 --- a/api/tests/opentrons/protocol_engine/state/test_pipette_store.py +++ b/api/tests/opentrons/protocol_engine/state/test_pipette_store.py @@ -45,7 +45,7 @@ def test_handles_load_pipette(subject: PipetteStore) -> None: mount=MountType.LEFT, ) - subject.handle_completed_command(command) + subject.handle_command(command) result = subject.state @@ -68,12 +68,12 @@ def test_pipette_volume_adds_aspirate(subject: PipetteStore) -> None: volume=42, ) - subject.handle_completed_command(load_command) - subject.handle_completed_command(aspirate_command) + subject.handle_command(load_command) + subject.handle_command(aspirate_command) assert subject.state.aspirated_volume_by_id["pipette-id"] == 42 - subject.handle_completed_command(aspirate_command) + subject.handle_command(aspirate_command) assert subject.state.aspirated_volume_by_id["pipette-id"] == 84 @@ -94,17 +94,17 @@ def test_pipette_volume_subtracts_dispense(subject: PipetteStore) -> None: volume=21, ) - subject.handle_completed_command(load_command) - subject.handle_completed_command(aspirate_command) - subject.handle_completed_command(dispense_command) + subject.handle_command(load_command) + subject.handle_command(aspirate_command) + subject.handle_command(dispense_command) assert subject.state.aspirated_volume_by_id["pipette-id"] == 21 - subject.handle_completed_command(dispense_command) + subject.handle_command(dispense_command) assert subject.state.aspirated_volume_by_id["pipette-id"] == 0 - subject.handle_completed_command(dispense_command) + subject.handle_command(dispense_command) assert subject.state.aspirated_volume_by_id["pipette-id"] == 0 @@ -177,18 +177,18 @@ def test_pipette_volume_subtracts_dispense(subject: PipetteStore) -> None: ), ) def test_movement_commands_update_current_location( - command: cmd.CompletedCommandType, + command: cmd.Command, expected_location: DeckLocation, subject: PipetteStore, ) -> None: """It should save the last used pipette, labware, and well for movement commands.""" load_pipette_command = create_load_pipette_command( - pipette_id=command.request.pipetteId, # type: ignore[arg-type, union-attr] + pipette_id=command.data.pipetteId, # type: ignore[arg-type, union-attr] pipette_name=PipetteName.P300_SINGLE, mount=MountType.LEFT, ) - subject.handle_completed_command(load_pipette_command) - subject.handle_completed_command(command) + subject.handle_command(load_pipette_command) + subject.handle_command(command) assert subject.state.current_location == expected_location diff --git a/api/tests/opentrons/protocol_engine/test_create_protocol_engine.py b/api/tests/opentrons/protocol_engine/test_create_protocol_engine.py new file mode 100644 index 00000000000..ed0b6ed33a0 --- /dev/null +++ b/api/tests/opentrons/protocol_engine/test_create_protocol_engine.py @@ -0,0 +1,33 @@ +"""Smoke tests for the ProtocolEngine creation factory.""" +from opentrons.calibration_storage.helpers import uri_from_details +from opentrons_shared_data.deck.dev_types import DeckDefinitionV2 +from opentrons.protocols.models import LabwareDefinition +from opentrons.types import DeckSlotName +from opentrons.hardware_control import API as HardwareAPI +from opentrons.protocols.geometry.deck import FIXED_TRASH_ID + +from opentrons.protocol_engine import ProtocolEngine, create_protocol_engine +from opentrons.protocol_engine.types import DeckSlotLocation +from opentrons.protocol_engine.state import LabwareData + + +async def test_create_engine_initializes_state_with_deck_geometry( + hardware_api: HardwareAPI, + standard_deck_def: DeckDefinitionV2, + fixed_trash_def: LabwareDefinition, +) -> None: + """It should load deck geometry data into the store on create.""" + engine = await create_protocol_engine(hardware=hardware_api) + state = engine.state_view + + assert isinstance(engine, ProtocolEngine) + assert state.labware.get_deck_definition() == standard_deck_def + assert state.labware.get_labware_data_by_id(FIXED_TRASH_ID) == LabwareData( + location=DeckSlotLocation(slot=DeckSlotName.FIXED_TRASH), + uri=uri_from_details( + load_name=fixed_trash_def.parameters.loadName, + namespace=fixed_trash_def.namespace, + version=fixed_trash_def.version, + ), + calibration=(0, 0, 0), + ) diff --git a/api/tests/opentrons/protocol_engine/test_protocol_engine.py b/api/tests/opentrons/protocol_engine/test_protocol_engine.py index af35ec4cd44..a28f17bd444 100644 --- a/api/tests/opentrons/protocol_engine/test_protocol_engine.py +++ b/api/tests/opentrons/protocol_engine/test_protocol_engine.py @@ -1,221 +1,235 @@ """Tests for the ProtocolEngine class.""" -from datetime import datetime, timezone -from decoy import matchers -from math import isclose -from mock import AsyncMock, MagicMock # type: ignore[attr-defined] -from typing import cast - -from opentrons.calibration_storage.helpers import uri_from_details -from opentrons_shared_data.deck.dev_types import DeckDefinitionV2 -from opentrons.protocols.models import LabwareDefinition -from opentrons.types import DeckSlotName -from opentrons.protocols.geometry.deck import FIXED_TRASH_ID - -from opentrons.protocol_engine import ProtocolEngine, errors -from opentrons.protocol_engine.types import DeckSlotLocation -from opentrons.protocol_engine.commands import ( - MoveToWellRequest, - MoveToWellResult, - PendingCommand, - RunningCommand, - CompletedCommand, - FailedCommand, -) -from opentrons.protocol_engine.execution import CommandHandlers -from opentrons.protocol_engine.state import LabwareData -from opentrons.protocol_engine.commands.move_to_well import MoveToWellImplementation - - -class CloseToNow: - """Matcher for any datetime that is close to now.""" - - def __init__(self) -> None: - """Initialize a CloseToNow matcher.""" - self._now = datetime.now(tz=timezone.utc) - - 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 - ) - - def __repr__(self) -> str: - """Represent the matcher as a string.""" - return f"" - - -async def test_create_engine_initializes_state_with_deck_geometry( - mock_hardware: MagicMock, - standard_deck_def: DeckDefinitionV2, - fixed_trash_def: LabwareDefinition, -) -> None: - """It should load deck geometry data into the store on create.""" - engine = await ProtocolEngine.create(hardware=mock_hardware) - state = engine.state_view - - assert state.labware.get_deck_definition() == standard_deck_def - assert state.labware.get_labware_data_by_id(FIXED_TRASH_ID) == LabwareData( - location=DeckSlotLocation(slot=DeckSlotName.FIXED_TRASH), - uri=uri_from_details( - load_name=fixed_trash_def.parameters.loadName, - namespace=fixed_trash_def.namespace, - version=fixed_trash_def.version, - ), - calibration=(0, 0, 0), +import pytest +from datetime import datetime +from decoy import Decoy + +from opentrons.types import MountType +from opentrons.protocol_engine import ProtocolEngine, commands +from opentrons.protocol_engine.commands import CommandMapper, CommandStatus +from opentrons.protocol_engine.types import PipetteName +from opentrons.protocol_engine.execution import CommandExecutor +from opentrons.protocol_engine.resources import ResourceProviders +from opentrons.protocol_engine.state import StateStore + + +@pytest.fixture +def state_store(decoy: Decoy) -> StateStore: + """Get a mock StateStore.""" + return decoy.create_decoy(spec=StateStore) + + +@pytest.fixture +def command_executor(decoy: Decoy) -> CommandExecutor: + """Get a mock CommandExecutor.""" + return decoy.create_decoy(spec=CommandExecutor) + + +@pytest.fixture +def command_mapper(decoy: Decoy) -> CommandMapper: + """Get a mock CommandMapper.""" + return decoy.create_decoy(spec=CommandMapper) + + +@pytest.fixture +def resources(decoy: Decoy) -> ResourceProviders: + """Get mock ResourceProviders.""" + return decoy.create_decoy(spec=ResourceProviders) + + +@pytest.fixture +def subject( + state_store: StateStore, + command_executor: CommandExecutor, + command_mapper: CommandMapper, + resources: ResourceProviders, +) -> ProtocolEngine: + """Get a ProtocolEngine test subject with its dependencies stubbed out.""" + return ProtocolEngine( + state_store=state_store, + command_executor=command_executor, + command_mapper=command_mapper, + resources=resources, ) -async def test_execute_command_creates_command( - engine: ProtocolEngine, - mock_state_store: MagicMock, +def test_add_command( + decoy: Decoy, + state_store: StateStore, + command_mapper: CommandMapper, + resources: ResourceProviders, + subject: ProtocolEngine, ) -> None: - """It should create a command in the state store when executing.""" - req = MoveToWellRequest(pipetteId="123", labwareId="abc", wellName="A1") - - await engine.execute_command(req, command_id="unique-id") - mock_state_store.handle_command.assert_any_call( - RunningCommand( - created_at=cast(datetime, CloseToNow()), - started_at=cast(datetime, CloseToNow()), - request=req, - ), - command_id="unique-id", + """It should add a command to the state from a request.""" + data = commands.LoadPipetteData( + mount=MountType.LEFT, + pipetteName=PipetteName.P300_SINGLE, ) + request = commands.LoadPipetteRequest(data=data) -async def test_execute_command_calls_implementation_executor( - engine: ProtocolEngine -) -> None: - """It should create a command in the state store when executing.""" - mock_req = MagicMock(spec=MoveToWellRequest) - mock_impl = AsyncMock(spec=MoveToWellImplementation) + created_at = datetime(year=2021, month=1, day=1) - mock_req.get_implementation.return_value = mock_impl + queued_command = commands.LoadPipette( + id="command-id", + status=commands.CommandStatus.QUEUED, + createdAt=created_at, + data=data, + ) + + decoy.when(resources.model_utils.generate_id()).then_return("command-id") + decoy.when(resources.model_utils.get_timestamp()).then_return(created_at) + decoy.when( + command_mapper.map_request_to_command( + request=request, + command_id="command-id", + created_at=created_at, + ) + ).then_return(queued_command) - await engine.execute_command(mock_req, command_id="unique-id") + result = subject.add_command(request) - mock_impl.execute.assert_called_with(matchers.IsA(CommandHandlers)) + assert result == queued_command + decoy.verify(state_store.handle_command(queued_command)) -async def test_execute_command_adds_result_to_state( - engine: ProtocolEngine, - mock_state_store: MagicMock, - now: datetime, +async def test_execute_command_by_id( + decoy: Decoy, + state_store: StateStore, + command_executor: CommandExecutor, + command_mapper: CommandMapper, + resources: ResourceProviders, + subject: ProtocolEngine, ) -> None: - """It should upsert the completed command into state.""" - result = MoveToWellResult() - 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, + """It should execute an existing command in the state.""" + created_at = datetime(year=2021, month=1, day=1) + started_at = datetime(year=2022, month=2, day=2) + completed_at = datetime(year=2022, month=3, day=3) + + data = commands.LoadPipetteData( + mount=MountType.LEFT, + pipetteName=PipetteName.P300_SINGLE, ) - mock_impl.execute.return_value = result - cmd = await engine.execute_command(mock_req, command_id="unique-id") + queued_command = commands.LoadPipette( + id="command-id", + status=CommandStatus.QUEUED, + createdAt=created_at, + data=data, + ) - assert cmd == CompletedCommand( - created_at=cast(datetime, CloseToNow()), - started_at=cast(datetime, CloseToNow()), - completed_at=cast(datetime, CloseToNow()), - request=mock_req, - result=result, + running_command = commands.LoadPipette( + id="command-id", + status=CommandStatus.RUNNING, + createdAt=created_at, + startedAt=started_at, + data=data, ) - mock_state_store.handle_command.assert_called_with( - cmd, - command_id="unique-id", + executed_command = commands.LoadPipette( + id="command-id", + status=CommandStatus.SUCCEEDED, + createdAt=datetime(year=2021, month=1, day=1), + startedAt=started_at, + completedAt=completed_at, + data=data, ) + decoy.when( + state_store.state_view.commands.get_command_by_id("command-id") + ).then_return(queued_command) -async def test_execute_command_adds_error_to_state( - engine: ProtocolEngine, - mock_state_store: MagicMock, - now: datetime, -) -> None: - """It should upsert a failed command into state.""" - error = errors.ProtocolEngineError("oh no!") - 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, - ) - mock_impl.execute.side_effect = error + decoy.when(resources.model_utils.get_timestamp()).then_return(started_at) - cmd = await engine.execute_command(mock_req, command_id="unique-id") + decoy.when( + command_mapper.update_command( + command=queued_command, + status=CommandStatus.RUNNING, + startedAt=started_at, + ) + ).then_return(running_command) - assert cmd == FailedCommand( - created_at=cast(datetime, CloseToNow()), - started_at=cast(datetime, CloseToNow()), - failed_at=cast(datetime, CloseToNow()), - request=mock_req, - error=error, + decoy.when(await command_executor.execute(running_command)).then_return( + executed_command ) - mock_state_store.handle_command.assert_called_with( - cmd, - command_id="unique-id", + result = await subject.execute_command_by_id("command-id") + + assert result == executed_command + decoy.verify( + state_store.handle_command(running_command), + state_store.handle_command(executed_command), ) -async def test_execute_command_adds_unexpected_error_to_state( - engine: ProtocolEngine, - mock_state_store: MagicMock, - now: datetime, +async def test_execute_command( + decoy: Decoy, + state_store: StateStore, + command_executor: CommandExecutor, + command_mapper: CommandMapper, + resources: ResourceProviders, + subject: ProtocolEngine, ) -> None: - """It should upsert an unexpectedly failed command into state.""" - error = RuntimeError("oh no!") - 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, - ) - mock_impl.execute.side_effect = error + """It should add and execute a command from a request.""" + created_at = datetime(year=2021, month=1, day=1) + completed_at = datetime(year=2022, month=3, day=3) - cmd = await engine.execute_command(mock_req, command_id="unique-id") + data = commands.LoadPipetteData( + mount=MountType.LEFT, + pipetteName=PipetteName.P300_SINGLE, + ) - assert type(cmd.error) == errors.UnexpectedProtocolError # type: ignore[union-attr] - assert cmd.error.original_error == error # type: ignore[union-attr] + request = commands.LoadPipetteRequest(data=data) - mock_state_store.handle_command.assert_called_with( - cmd, - command_id="unique-id", + queued_command = commands.LoadPipette( + id="command-id", + status=commands.CommandStatus.QUEUED, + createdAt=created_at, + data=data, ) + running_command = commands.LoadPipette( + id="command-id", + status=commands.CommandStatus.RUNNING, + createdAt=created_at, + startedAt=created_at, + data=data, + ) -def test_add_command( - engine: ProtocolEngine, - 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" + executed_command = commands.LoadPipette( + id="command-id", + status=commands.CommandStatus.SUCCEEDED, + createdAt=created_at, + startedAt=created_at, + completedAt=completed_at, + data=data, + ) - mock_req = MagicMock(spec=MoveToWellRequest) - mock_impl = AsyncMock(spec=MoveToWellImplementation) + decoy.when(resources.model_utils.get_timestamp()).then_return(created_at) - mock_req.get_implementation.return_value = mock_impl - mock_impl.create_command.return_value = PendingCommand( - request=mock_req, created_at=now - ) + decoy.when( + command_mapper.map_request_to_command( + request=request, + created_at=created_at, + command_id="command-id", + ) + ).then_return(queued_command) - result = engine.add_command(mock_req) + decoy.when( + command_mapper.update_command( + command=queued_command, + startedAt=created_at, + status=CommandStatus.RUNNING, + ) + ).then_return(running_command) - assert result == PendingCommand( - created_at=cast(datetime, CloseToNow()), - request=mock_req, + decoy.when(await command_executor.execute(running_command)).then_return( + executed_command ) - mock_state_store.handle_command.assert_called_with( - result, - command_id="command-id", + result = await subject.execute_command(request, "command-id") + + assert result == executed_command + decoy.verify( + state_store.handle_command(running_command), + state_store.handle_command(executed_command), ) diff --git a/api/tests/opentrons/protocols/runner/json_proto/test_command_translator.py b/api/tests/opentrons/protocols/runner/json_proto/test_command_translator.py index 4da0c7641ec..caf92aea2ef 100644 --- a/api/tests/opentrons/protocols/runner/json_proto/test_command_translator.py +++ b/api/tests/opentrons/protocols/runner/json_proto/test_command_translator.py @@ -1,12 +1,16 @@ -import typing - import pytest +from typing import Any, List -from opentrons import protocol_engine as pe +from opentrons.types import DeckSlotName, MountType from opentrons.protocols import models - from opentrons.protocols.runner.json_proto.command_translator import CommandTranslator - +from opentrons.protocol_engine import ( + commands as pe_commands, + DeckSlotLocation, + PipetteName, + WellLocation, + WellOrigin, +) # todo(mc & mm, 2021-06-17): # @@ -103,7 +107,7 @@ def subject() -> CommandTranslator: return CommandTranslator() -def _assert_appear_in_order(elements: typing.Iterable, source: typing.Iterable) -> None: +def _assert_appear_in_order(elements: List[Any], source: List[Any]) -> None: """ Assert all elements appear in source, in the given order relative to each other. @@ -131,26 +135,34 @@ def test_labware( ) -> None: result = subject.translate(json_protocol) - expected_add_definition_request_1 = pe.commands.AddLabwareDefinitionRequest( - definition=models.LabwareDefinition.parse_obj(minimal_labware_def) + expected_add_definition_request_1 = pe_commands.AddLabwareDefinitionRequest( + data=pe_commands.AddLabwareDefinitionData( + definition=models.LabwareDefinition.parse_obj(minimal_labware_def) + ) ) - expected_load_request_1 = pe.commands.LoadLabwareRequest( - location=pe.DeckSlotLocation(slot=1), - loadName=minimal_labware_def["parameters"]["loadName"], - namespace=minimal_labware_def["namespace"], - version=minimal_labware_def["version"], - labwareId="tiprack1Id", + expected_load_request_1 = pe_commands.LoadLabwareRequest( + data=pe_commands.LoadLabwareData( + location=DeckSlotLocation(slot=DeckSlotName.SLOT_1), + loadName=minimal_labware_def["parameters"]["loadName"], + namespace=minimal_labware_def["namespace"], + version=minimal_labware_def["version"], + labwareId="tiprack1Id", + ) ) - expected_add_definition_request_2 = pe.commands.AddLabwareDefinitionRequest( - definition=models.LabwareDefinition.parse_obj(minimal_labware_def2) + expected_add_definition_request_2 = pe_commands.AddLabwareDefinitionRequest( + data=pe_commands.AddLabwareDefinitionData( + definition=models.LabwareDefinition.parse_obj(minimal_labware_def2) + ) ) - expected_load_request_2 = pe.commands.LoadLabwareRequest( - location=pe.DeckSlotLocation(slot=10), - loadName=minimal_labware_def2["parameters"]["loadName"], - namespace=minimal_labware_def2["namespace"], - version=minimal_labware_def2["version"], - labwareId="wellplate1Id", + expected_load_request_2 = pe_commands.LoadLabwareRequest( + data=pe_commands.LoadLabwareData( + location=DeckSlotLocation(slot=DeckSlotName.SLOT_10), + loadName=minimal_labware_def2["parameters"]["loadName"], + namespace=minimal_labware_def2["namespace"], + version=minimal_labware_def2["version"], + labwareId="wellplate1Id", + ) ) _assert_appear_in_order( @@ -170,8 +182,12 @@ def test_pipettes( ) -> None: result = subject.translate(json_protocol) - expected_request = pe.commands.LoadPipetteRequest( - pipetteName="p300_single", mount="left", pipetteId="leftPipetteId" + expected_request = pe_commands.LoadPipetteRequest( + data=pe_commands.LoadPipetteData( + pipetteName=PipetteName.P300_SINGLE, + mount=MountType.LEFT, + pipetteId="leftPipetteId", + ) ) assert expected_request in result @@ -184,18 +200,18 @@ def test_aspirate( aspirate request.""" request = subject._translate_command(aspirate_command) - assert request == [ - pe.commands.AspirateRequest( + assert request == pe_commands.AspirateRequest( + data=pe_commands.AspirateData( pipetteId=aspirate_command.params.pipette, labwareId=aspirate_command.params.labware, wellName=aspirate_command.params.well, volume=aspirate_command.params.volume, - wellLocation=pe.WellLocation( - origin=pe.WellOrigin.BOTTOM, + wellLocation=WellLocation( + origin=WellOrigin.BOTTOM, offset=(0, 0, aspirate_command.params.offsetFromBottomMm), ), ) - ] + ) def test_dispense( @@ -205,18 +221,18 @@ def test_dispense( dispense request.""" request = subject._translate_command(dispense_command) - assert request == [ - pe.commands.DispenseRequest( + assert request == pe_commands.DispenseRequest( + data=pe_commands.DispenseData( pipetteId=dispense_command.params.pipette, labwareId=dispense_command.params.labware, wellName=dispense_command.params.well, volume=dispense_command.params.volume, - wellLocation=pe.WellLocation( - origin=pe.WellOrigin.BOTTOM, + wellLocation=WellLocation( + origin=WellOrigin.BOTTOM, offset=(0, 0, dispense_command.params.offsetFromBottomMm), ), ) - ] + ) def test_drop_tip( @@ -227,13 +243,13 @@ def test_drop_tip( drop tip request.""" request = subject._translate_command(drop_tip_command) - assert request == [ - pe.commands.DropTipRequest( + assert request == pe_commands.DropTipRequest( + data=pe_commands.DropTipData( pipetteId=drop_tip_command.params.pipette, labwareId=drop_tip_command.params.labware, wellName=drop_tip_command.params.well, ) - ] + ) def test_pick_up_tip( @@ -245,10 +261,10 @@ def test_pick_up_tip( """ request = subject._translate_command(pick_up_command) - assert request == [ - pe.commands.PickUpTipRequest( + assert request == pe_commands.PickUpTipRequest( + data=pe_commands.PickUpTipData( pipetteId=pick_up_command.params.pipette, labwareId=pick_up_command.params.labware, wellName=pick_up_command.params.well, ) - ] + ) diff --git a/robot-server/robot_server/service/session/session_types/live_protocol/command_executor.py b/robot-server/robot_server/service/session/session_types/live_protocol/command_executor.py index 7e3527b5c4b..4e882809434 100644 --- a/robot-server/robot_server/service/session/session_types/live_protocol/command_executor.py +++ b/robot-server/robot_server/service/session/session_types/live_protocol/command_executor.py @@ -1,29 +1,15 @@ import logging -import typing -from opentrons.protocol_engine import ProtocolEngine, commands +from opentrons.protocol_engine import ProtocolEngine from robot_server.service.session.command_execution import ( - CommandExecutor, Command, CompletedCommand, CommandResult) -from robot_server.service.session.errors import UnsupportedCommandException, \ - CommandExecutionException -from robot_server.service.session.models import ( - command_definitions as models) + CommandExecutor, Command, CompletedCommand) log = logging.getLogger(__name__) class LiveProtocolCommandExecutor(CommandExecutor): - ACCEPTED_COMMANDS = { - models.EquipmentCommand.load_labware, - models.EquipmentCommand.load_pipette, - models.PipetteCommand.aspirate, - models.PipetteCommand.dispense, - models.PipetteCommand.pick_up_tip, - models.PipetteCommand.drop_tip, - } - def __init__( self, protocol_engine: ProtocolEngine): @@ -31,29 +17,6 @@ def __init__( async def execute(self, command: Command) -> CompletedCommand: """Execute a live protocol command.""" - if command.request.command not in self.ACCEPTED_COMMANDS: - raise UnsupportedCommandException( - f"Command '{command.request.command}' is not supported." - ) - - data = await self._protocol_engine.execute_command( - request=typing.cast( - commands.CommandRequestType, - command.request.data - ), - command_id=command.meta.identifier) - - if isinstance(data, commands.FailedCommand): - raise CommandExecutionException(reason=str(data.error)) - else: - result = CommandResult( - started_at=data.started_at, - completed_at=data.completed_at, - data=data.result) - - # return completed command to session - return CompletedCommand( - request=command.request, - meta=command.meta, - result=result, - ) + raise NotImplementedError( + "Enable useProtocolEngine feature flag to use live HTTP protocols" + ) diff --git a/robot-server/robot_server/service/session/session_types/live_protocol/session.py b/robot-server/robot_server/service/session/session_types/live_protocol/session.py index a1d5bb4d3ef..7e64011396c 100644 --- a/robot-server/robot_server/service/session/session_types/live_protocol/session.py +++ b/robot-server/robot_server/service/session/session_types/live_protocol/session.py @@ -1,7 +1,7 @@ from typing import cast from opentrons import API -from opentrons.protocol_engine import ProtocolEngine +from opentrons.protocol_engine import ProtocolEngine, create_protocol_engine from robot_server.service.session.models import session as models from robot_server.service.session.command_execution import CommandQueue, \ @@ -33,7 +33,7 @@ async def create(cls, return LiveProtocolSession( configuration=configuration, instance_meta=instance_meta, - protocol_engine=await ProtocolEngine.create( + protocol_engine=await create_protocol_engine( # Cast the ThreadManager to the wrapped API object. cast(API, configuration.hardware) ) diff --git a/robot-server/robot_server/sessions/engine_store.py b/robot-server/robot_server/sessions/engine_store.py index dd66b33f322..486c178a496 100644 --- a/robot-server/robot_server/sessions/engine_store.py +++ b/robot-server/robot_server/sessions/engine_store.py @@ -3,7 +3,7 @@ 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 opentrons.protocol_engine import ProtocolEngine, create_protocol_engine from robot_server.protocols import ProtocolResource @@ -74,7 +74,7 @@ async def create(self, protocol: Optional[ProtocolResource]) -> ProtocolEngine: # 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) + engine = await create_protocol_engine(hardware=self._hardware_api) runner = None if self._engine is not None: diff --git a/robot-server/robot_server/sessions/router.py b/robot-server/robot_server/sessions/router.py index 910d70aaa22..ffadad216cc 100644 --- a/robot-server/robot_server/sessions/router.py +++ b/robot-server/robot_server/sessions/router.py @@ -5,6 +5,7 @@ from typing_extensions import Literal from robot_server.errors import ErrorDetails, ErrorResponse + from robot_server.service.dependencies import get_current_time, get_unique_id from robot_server.service.json_api import ( RequestModel, @@ -237,6 +238,8 @@ async def create_session_action( ) # TODO(mc, 2021-06-11): support actions other than `start` + # TODO(mc, 2021-06-24): ensure the engine homes pipette plungers + # before starting the protocol run engine_store.runner.play() except SessionNotFoundError as e: diff --git a/robot-server/tests/integration/sessions/test_live_protocol.tavern.yaml b/robot-server/tests/integration/sessions/test_live_protocol.tavern.yaml deleted file mode 100644 index f6f7b9af284..00000000000 --- a/robot-server/tests/integration/sessions/test_live_protocol.tavern.yaml +++ /dev/null @@ -1,166 +0,0 @@ ---- -test_name: Live Protocol session protocol -strict: - - json:on -marks: - - usefixtures: - - run_server -stages: - - name: Create the session - request: - url: "{host:s}:{port:d}/sessions" - method: POST - json: - data: - sessionType: liveProtocol - response: - save: - json: - session_id: data.id - status_code: 201 - - name: Load well plate lw command - request: - url: "{host:s}:{port:d}/sessions/{session_id}/commands/execute" - method: POST - json: - data: - command: equipment.loadLabware - data: &labware_data - location: - slot: 2 - loadName: corning_96_wellplate_360ul_flat - namespace: Opentrons - version: 1 - labwareId: null - response: - status_code: 200 - json: - links: !anydict - data: - id: !anystr - data: *labware_data - command: equipment.loadLabware - status: executed - createdAt: !anystr - startedAt: !anystr - completedAt: !anystr - result: - labwareId: !anystr - definition: !anydict - calibration: !anylist - - name: Load tip rack command - request: - url: "{host:s}:{port:d}/sessions/{session_id}/commands/execute" - method: POST - json: - data: - command: equipment.loadLabware - data: &tiprack_data - location: - slot: 1 - loadName: opentrons_96_tiprack_300ul - namespace: Opentrons - version: 1 - labwareId: null - response: - status_code: 200 - json: - links: !anydict - data: - id: !anystr - data: *tiprack_data - command: equipment.loadLabware - status: executed - createdAt: !anystr - startedAt: !anystr - completedAt: !anystr - result: - labwareId: !anystr - definition: !anydict - calibration: !anylist - - name: Load well plate specifying labware id - request: - url: "{host:s}:{port:d}/sessions/{session_id}/commands/execute" - method: POST - json: - data: - command: equipment.loadLabware - data: &wellplate_data - location: - slot: 3 - loadName: opentrons_96_aluminumblock_nest_wellplate_100ul - namespace: Opentrons - version: 1 - labwareId: some_labware_id - response: - status_code: 200 - json: - links: !anydict - data: - id: !anystr - data: *wellplate_data - command: equipment.loadLabware - status: executed - createdAt: !anystr - startedAt: !anystr - completedAt: !anystr - result: - labwareId: some_labware_id - definition: !anydict - calibration: !anylist - - name: Load pipette command - request: - url: "{host:s}:{port:d}/sessions/{session_id}/commands/execute" - method: POST - json: - data: - command: equipment.loadPipette - data: &create_pipette_data - pipetteName: p300_single - mount: right - pipetteId: null - response: - status_code: 200 - json: - links: !anydict - data: - id: !anystr - data: *create_pipette_data - command: equipment.loadPipette - status: executed - createdAt: !anystr - startedAt: !anystr - completedAt: !anystr - result: - pipetteId: !anystr - - name: Load pipette command specifying the id - request: - url: "{host:s}:{port:d}/sessions/{session_id}/commands/execute" - method: POST - json: - data: - command: equipment.loadPipette - data: &create_pipette_data_with_id - pipetteName: p10_single - mount: left - pipetteId: my pipette - response: - status_code: 200 - json: - links: !anydict - data: - id: !anystr - data: *create_pipette_data_with_id - command: equipment.loadPipette - status: executed - createdAt: !anystr - startedAt: !anystr - completedAt: !anystr - result: - pipetteId: my pipette - - name: Delete the session - request: - url: "{host:s}:{port:d}/sessions/{session_id}" - method: DELETE - response: - status_code: 200 diff --git a/robot-server/tests/service/session/models/test_command.py b/robot-server/tests/service/session/models/test_command.py index f5515d85b4c..dd2ee36c0c4 100644 --- a/robot-server/tests/service/session/models/test_command.py +++ b/robot-server/tests/service/session/models/test_command.py @@ -1,10 +1,7 @@ from datetime import datetime import pytest -from opentrons.types import MountType from pydantic import ValidationError -from opentrons.protocol_engine import commands as pe_commands - from robot_server.service.session.models import command, command_definitions @@ -46,53 +43,6 @@ def test_empty(command_def: command_definitions.CommandDefinition): assert response.result is None -def test_not_empty(): - """Test that command request with data and result are created properly.""" - request = command.CommandRequest.parse_obj({ - "data": { - "command": "equipment.loadPipette", - "data": { - "pipetteName": "p10_single", - "mount": "left", - "pipetteId": None, - } - } - }) - assert request.data.command == \ - command_definitions.EquipmentCommand.load_pipette - assert request.data.data == pe_commands.LoadPipetteRequest( - pipetteName="p10_single", - mount=MountType.LEFT, - ) - - dt = datetime(2000, 1, 1) - - response = request.data.make_response( - identifier="id", - status=command.CommandStatus.executed, - created_at=dt, - started_at=None, - completed_at=None, - result=pe_commands.LoadPipetteResult( - pipetteId="123" - ) - ) - - assert response.command == \ - command_definitions.EquipmentCommand.load_pipette - assert response.data == pe_commands.LoadPipetteRequest( - pipetteName="p10_single", - mount=MountType.LEFT, - ) - assert response.id == "id" - assert response.createdAt == dt - assert response.startedAt is None - assert response.completedAt is None - assert response.result == pe_commands.LoadPipetteResult( - pipetteId="123" - ) - - @pytest.mark.parametrize( argnames="command_def", argvalues=[ diff --git a/robot-server/tests/service/session/session_types/live_protocol/test_command_executor.py b/robot-server/tests/service/session/session_types/live_protocol/test_command_executor.py deleted file mode 100644 index 8cf849edc9d..00000000000 --- a/robot-server/tests/service/session/session_types/live_protocol/test_command_executor.py +++ /dev/null @@ -1,179 +0,0 @@ -from datetime import datetime -from mock import MagicMock -from mock import AsyncMock -import pytest -from opentrons.protocol_engine import ProtocolEngine -from opentrons.protocol_engine import commands as pe_commands -from opentrons.protocol_engine.errors import ProtocolEngineError -from opentrons.protocol_engine.types import DeckSlotLocation -from opentrons.protocols.models import LabwareDefinition -from opentrons.types import DeckSlotName, MountType - -from robot_server.service.session.command_execution.command import ( - CommandMeta, CompletedCommand) -from robot_server.service.session.errors import ( - UnsupportedCommandException, CommandExecutionException) -from robot_server.service.session.models import command_definitions -from robot_server.service.session.models import command as command_models -from robot_server.service.session.models.common import EmptyModel -from robot_server.service.session.session_types.live_protocol.command_executor\ - import LiveProtocolCommandExecutor -from robot_server.service.session.command_execution import ( - Command, CommandResult) - - -@pytest.fixture -def mock_protocol_engine() -> MagicMock: - m = AsyncMock(spec=ProtocolEngine) - return m - - -@pytest.fixture -def command_executor(mock_protocol_engine)\ - -> LiveProtocolCommandExecutor: - return LiveProtocolCommandExecutor(protocol_engine=mock_protocol_engine) - - -@pytest.mark.parametrize( - argnames="command_type", - argvalues=[ - command_definitions.CalibrationCommand.pick_up_tip, - command_definitions.ProtocolCommand.start_run, - command_definitions.CheckCalibrationCommand.compare_point - ]) -async def test_unsupported_commands(command_type, - command_executor, - mock_protocol_engine): - """Test that non live protocol commands are rejected.""" - command_object = Command( - meta=CommandMeta(identifier="1234"), - request=command_models.SimpleCommandRequest( - command=command_type, - data=EmptyModel())) - - with pytest.raises(UnsupportedCommandException, match="is not supported"): - await command_executor.execute(command_object) - - -async def test_failed_command(command_executor, mock_protocol_engine): - """Test that protocol engine failures are caught.""" - request_body = pe_commands.LoadLabwareRequest( - location=DeckSlotLocation(slot=DeckSlotName.SLOT_2), - loadName="hello", - version=1, - namespace="test", - labwareId=None, - ) - - command_object = Command( - meta=CommandMeta(identifier="1234"), - request=command_models.LoadLabwareRequest( - command=command_definitions.EquipmentCommand.load_labware, - data=request_body)) - - protocol_engine_response = pe_commands.FailedCommand( - request=request_body, - created_at=datetime(2000, 1, 1), - started_at=datetime(2000, 1, 2), - failed_at=datetime(2000, 1, 3), - error=ProtocolEngineError("failure"), - ) - - mock_protocol_engine.execute_command.return_value =\ - protocol_engine_response - - with pytest.raises(CommandExecutionException): - await command_executor.execute(command_object) - - -async def test_load_labware( - command_executor, mock_protocol_engine, minimal_labware_def): - """Test that load labware command is executed.""" - request_body = pe_commands.LoadLabwareRequest( - location=DeckSlotLocation(slot=DeckSlotName.SLOT_2), - loadName="hello", - version=1, - namespace="test", - labwareId=None, - ) - - protocol_engine_response = pe_commands.CompletedCommand( - result=pe_commands.LoadLabwareResult( - labwareId="your labware", - definition=LabwareDefinition.parse_obj(minimal_labware_def), - calibration=(1, 2, 3)), - request=request_body, - created_at=datetime(2000, 1, 1), - started_at=datetime(2000, 1, 2), - completed_at=datetime(2000, 1, 3) - ) - - mock_protocol_engine.execute_command.return_value =\ - protocol_engine_response - - command_object = Command( - meta=CommandMeta(identifier="1234"), - request=command_models.LoadLabwareRequest( - command=command_definitions.EquipmentCommand.load_labware, - data=request_body)) - - result = await command_executor.execute(command_object) - - mock_protocol_engine.execute_command.assert_called_once_with( - request=request_body, - command_id="1234" - ) - - assert result == CompletedCommand( - request=command_object.request, - meta=command_object.meta, - result=CommandResult( - started_at=protocol_engine_response.started_at, - completed_at=protocol_engine_response.completed_at, - data=protocol_engine_response.result - ) - ) - - -async def test_load_instrument(command_executor, mock_protocol_engine): - """Test that load pipette command is executed.""" - request_body = pe_commands.LoadPipetteRequest( - pipetteName="p10_single", - mount=MountType.LEFT, - ) - - protocol_engine_response = pe_commands.CompletedCommand( - result=pe_commands.LoadPipetteResult( - pipetteId="4321" - ), - request=request_body, - created_at=datetime(2000, 1, 1), - started_at=datetime(2000, 1, 2), - completed_at=datetime(2000, 1, 3) - ) - - mock_protocol_engine.execute_command.return_value =\ - protocol_engine_response - - command_object = Command( - meta=CommandMeta(identifier="1234"), - request=command_models.LoadInstrumentRequest( - command=command_definitions.EquipmentCommand.load_pipette, - data=request_body)) - - result = await command_executor.execute(command_object) - - mock_protocol_engine.execute_command.assert_called_once_with( - request=request_body, - command_id="1234" - ) - - assert result == CompletedCommand( - request=command_object.request, - meta=command_object.meta, - result=CommandResult( - started_at=protocol_engine_response.started_at, - completed_at=protocol_engine_response.completed_at, - data=protocol_engine_response.result - ) - )