From 4cab53fc1cefdbc1ecab12205005c9db2926d44f Mon Sep 17 00:00:00 2001 From: amitlissack Date: Tue, 5 Jan 2021 09:46:37 -0500 Subject: [PATCH] refactor(robot-server): Robot server command responses (#7170) * rename models. * Genericize command requests and responses closes #6519 --- .../command_execution/base_executor.py | 2 +- .../command_execution/callable_executor.py | 6 +- .../session/command_execution/command.py | 31 +- .../command_execution/hardware_executor.py | 77 ---- .../service/session/models/command.py | 340 +++++++++++------- .../session/models/command_definitions.py | 12 +- .../session/session_types/base_session.py | 17 +- .../live_protocol/command_executor.py | 23 +- .../live_protocol/command_interface.py | 24 +- .../live_protocol/state_store.py | 12 +- .../protocol/execution/command_executor.py | 6 +- .../tests/service/session/models/__init__.py | 0 .../service/session/models/test_command.py | 112 ++++++ .../live_protocol/test_command_executor.py | 85 ++--- .../live_protocol/test_command_interface.py | 4 +- .../live_protocol/test_state_store.py | 110 +++--- .../execution/test_command_executor.py | 11 +- .../tests/service/session/test_router.py | 18 +- 18 files changed, 528 insertions(+), 362 deletions(-) delete mode 100644 robot-server/robot_server/service/session/command_execution/hardware_executor.py create mode 100644 robot-server/tests/service/session/models/__init__.py create mode 100644 robot-server/tests/service/session/models/test_command.py diff --git a/robot-server/robot_server/service/session/command_execution/base_executor.py b/robot-server/robot_server/service/session/command_execution/base_executor.py index 9684fe17d29..269952718a2 100644 --- a/robot-server/robot_server/service/session/command_execution/base_executor.py +++ b/robot-server/robot_server/service/session/command_execution/base_executor.py @@ -13,5 +13,5 @@ async def execute(self, command: Command) \ :raises: SessionCommandException """ raise UnsupportedCommandException( - f"'{command.content.name}' is not supported" + f"'{command.request.command}' is not supported" ) diff --git a/robot-server/robot_server/service/session/command_execution/callable_executor.py b/robot-server/robot_server/service/session/command_execution/callable_executor.py index 00abd39e123..8795eebd54f 100644 --- a/robot-server/robot_server/service/session/command_execution/callable_executor.py +++ b/robot-server/robot_server/service/session/command_execution/callable_executor.py @@ -24,14 +24,14 @@ def __init__(self, command_handler: CommandHandler): async def execute(self, command: Command) -> CompletedCommand: """Execute command""" with duration() as time_it: - name_arg = command.content.name - data = command.content.data + name_arg = command.request.command + data = command.request.data data_arg = data.dict() if data else {} await self._callable(name_arg, data_arg) return CompletedCommand( - content=command.content, + request=command.request, meta=command.meta, result=CommandResult(started_at=time_it.start, completed_at=time_it.end) diff --git a/robot-server/robot_server/service/session/command_execution/command.py b/robot-server/robot_server/service/session/command_execution/command.py index 795939b91d9..6d4c58e8d0c 100644 --- a/robot-server/robot_server/service/session/command_execution/command.py +++ b/robot-server/robot_server/service/session/command_execution/command.py @@ -1,55 +1,46 @@ from datetime import datetime from dataclasses import dataclass, field -from typing import Optional +from typing import Optional, Generic, TypeVar from robot_server.service.session.models.command import ( - CommandDataType, CommandStatus, CommandResultType) -from robot_server.service.session.models.command_definitions import \ - CommandDefinitionType + CommandStatus, RequestTypes) from robot_server.service.session.models.common import ( IdentifierType, create_identifier) from opentrons.util.helpers import utc_now -@dataclass(frozen=True) -class CommandContent: - name: CommandDefinitionType - data: CommandDataType - - @dataclass(frozen=True) class CommandMeta: identifier: IdentifierType = field(default_factory=create_identifier) created_at: datetime = field(default_factory=utc_now) +ResultTypeT = TypeVar("ResultTypeT") + + @dataclass(frozen=True) -class CommandResult: +class CommandResult(Generic[ResultTypeT]): started_at: datetime completed_at: datetime status: CommandStatus = CommandStatus.executed - data: Optional[CommandResultType] = None + data: Optional[ResultTypeT] = None @dataclass(frozen=True) class Command: - content: CommandContent + request: RequestTypes meta: CommandMeta = field(default_factory=CommandMeta) @dataclass(frozen=True) class CompletedCommand: - content: CommandContent + request: RequestTypes meta: CommandMeta result: CommandResult -def create_command(name: CommandDefinitionType, - data: CommandDataType) -> Command: +def create_command(request: RequestTypes) -> Command: """Create a command object""" return Command( - content=CommandContent( - name=name, - data=data - ) + request=request ) diff --git a/robot-server/robot_server/service/session/command_execution/hardware_executor.py b/robot-server/robot_server/service/session/command_execution/hardware_executor.py deleted file mode 100644 index cde8bb37e5b..00000000000 --- a/robot-server/robot_server/service/session/command_execution/hardware_executor.py +++ /dev/null @@ -1,77 +0,0 @@ -import typing -from opentrons.hardware_control import ThreadManager - -from . import Command, CompletedCommand, CommandResult -from . base_executor import CommandExecutor -from ..errors import UnsupportedCommandException -from ..models.command import ( - CommandDataType) -from ..models.command_definitions import CommandDefinition, RobotCommand -from robot_server.util import duration - - -COMMAND_HANDLER = typing.Callable[ - [ThreadManager, CommandDataType], typing.Awaitable] - - -async def home_all_motors(hardware, *args): - await hardware.home() - - -async def toggle_lights(hardware: ThreadManager, *args): - light_state = hardware.get_lights() - await hardware.set_lights(rails=not light_state.get('rails', False)) - - -class HardwareExecutor(CommandExecutor): - """A command executor that executes direct hardware commands""" - - COMMAND_TO_FUNC: typing.Dict[CommandDefinition, COMMAND_HANDLER] = { - RobotCommand.home_all_motors: home_all_motors, - RobotCommand.toggle_lights: toggle_lights - } - - def __init__(self, - hardware: ThreadManager, - command_filter: typing.Optional[typing.Set[RobotCommand]]): - """ - Constructor - - :param hardware: hardware api access - :param command_filter: a set of supported commands. All commands will - be supported if None - """ - self._hardware = hardware - self._command_filter = command_filter - - async def execute(self, command: Command) -> CompletedCommand: - func = self.get_handler(command.content.name) - if func: - with duration() as timed: - await func(self._hardware, command.content.data) - else: - raise UnsupportedCommandException( - f"Command '{command.content.name}' is not supported." - ) - return CompletedCommand( - content=command.content, - meta=command.meta, - result=CommandResult(started_at=timed.start, - completed_at=timed.end) - ) - - def get_handler(self, command_name: CommandDefinition) \ - -> typing.Optional[COMMAND_HANDLER]: - """Get the handler for the command type""" - if self._command_filter is not None: - if command_name not in self._command_filter: - return None - return self.COMMAND_TO_FUNC.get(command_name) - - -class DefaultHardwareExecutor(HardwareExecutor): - """The default command executor""" - def __init__(self, hardware: ThreadManager): - super().__init__(hardware, {RobotCommand.home_all_motors, - RobotCommand.home_pipette, - RobotCommand.toggle_lights}) diff --git a/robot-server/robot_server/service/session/models/command.py b/robot-server/robot_server/service/session/models/command.py index 0f2dffc793a..028942613a8 100644 --- a/robot-server/robot_server/service/session/models/command.py +++ b/robot-server/robot_server/service/session/models/command.py @@ -1,25 +1,46 @@ +"""Modeling of session commands. + +Creating a new command type requires these steps: +1) Creation of a CommandDefinition enum identifying the command +type. + +2) If necessary, define a data payload model. + +3) If necessary, define a result payload model. + +4) Create specialized `SessionCommandRequest` and `SessionCommandResponse` +types using the CommandDefinition Literal(s), data, and result payload models. +If there are no data and result models, then add the CommandDefinition to +`CommandsEmptyData` type. + +5) If not using `CommandsEmptyData` then add specialized request and response +types to `RequestTypes` and `ResponseTypes`. +""" from datetime import datetime from enum import Enum import typing +from typing_extensions import Literal +from pydantic import BaseModel, Field +from pydantic.generics import GenericModel + from opentrons_shared_data.labware.dev_types import LabwareDefinition from opentrons_shared_data.pipette.dev_types import PipetteName -from typing_extensions import Literal +from opentrons.util.helpers import utc_now -from robot_server.service.session.models.command_definitions import \ - CommandDefinitionType, RobotCommand, ProtocolCommand, EquipmentCommand, \ - PipetteCommand, CalibrationCommand, DeckCalibrationCommand, \ - CheckCalibrationCommand +from robot_server.service.session.models.command_definitions import ( + ProtocolCommand, EquipmentCommand, PipetteCommand, CalibrationCommand, + DeckCalibrationCommand, CheckCalibrationCommand, CommandDefinitionType) from robot_server.service.session.models.common import ( EmptyModel, JogPosition, IdentifierType, OffsetVector) -from pydantic import BaseModel, Field from robot_server.service.legacy.models.control import Mount from robot_server.service.json_api import ( ResponseModel, RequestModel, ResponseDataModel) -from opentrons.util.helpers import utc_now -class LoadLabwareRequest(BaseModel): +class LoadLabwareRequestData(BaseModel): + """Data field in an equipment.loadLabware command request.""" + location: int = Field( ..., description="Deck slot", ge=1, lt=12) @@ -37,30 +58,36 @@ class LoadLabwareRequest(BaseModel): description="The labware definition version") -class LoadLabwareResponse(BaseModel): +class LoadLabwareResponseData(BaseModel): + """Result field in an equipment.loadLabware command response.""" + labwareId: IdentifierType definition: LabwareDefinition calibration: OffsetVector -class LoadInstrumentRequest(BaseModel): +class LoadInstrumentRequestData(BaseModel): + """Data field in an equipment.loadInstrument command request.""" + instrumentName: PipetteName = Field( ..., description="The name of the instrument model") mount: Mount -class LoadInstrumentResponse(BaseModel): +class LoadInstrumentResponseData(BaseModel): + """Result field in an equipment.loadInstrument command response.""" + instrumentId: IdentifierType -class PipetteRequestBase(BaseModel): +class PipetteRequestDataBase(BaseModel): pipetteId: str labwareId: str wellId: str -class LiquidRequest(PipetteRequestBase): +class LiquidRequestData(PipetteRequestDataBase): volume: float = Field( ..., description="Amount of liquid in uL. Must be greater than 0 and less " @@ -81,162 +108,231 @@ class LiquidRequest(PipetteRequestBase): ) -class SetHasCalibrationBlockRequest(BaseModel): +class SetHasCalibrationBlockRequestData(BaseModel): hasBlock: bool = Field( ..., description="whether or not there is a calibration block present") -CommandDataType = typing.Union[ - SetHasCalibrationBlockRequest, - JogPosition, - LiquidRequest, - PipetteRequestBase, - LoadLabwareRequest, - LoadInstrumentRequest, - EmptyModel -] - -# A Union of all command result types -CommandResultType = typing.Union[ - LoadLabwareResponse, - LoadInstrumentResponse, -] - - class CommandStatus(str, Enum): - """The command status""" + """The command status.""" executed = "executed" queued = "queued" failed = "failed" -class BasicSessionCommand(BaseModel): - """A session command""" - command: CommandDefinitionType = Field( +CommandT = typing.TypeVar('CommandT', bound=CommandDefinitionType) +RequestDataT = typing.TypeVar('RequestDataT', bound=BaseModel) +ResponseDataT = typing.TypeVar('ResponseDataT') + + +class SessionCommandRequest( + GenericModel, + typing.Generic[CommandT, RequestDataT, ResponseDataT] +): + """A session command request.""" + command: CommandT = Field( ..., description="The command description") + data: RequestDataT = Field( + ..., + description="The command data" + ) + def make_response( + self, + identifier: str, + status: CommandStatus, + created_at: datetime, + started_at: typing.Optional[datetime], + completed_at: typing.Optional[datetime], + result: typing.Optional[ResponseDataT] + ) -> 'SessionCommandResponse[CommandT, RequestDataT, ResponseDataT]': + """Create a SessionCommandResponse object.""" + return SessionCommandResponse( + command=self.command, + data=self.data, + id=identifier, + status=status, + createdAt=created_at, + startedAt=started_at, + completedAt=completed_at, + result=result) + + +class SessionCommandResponse( + ResponseDataModel, + GenericModel, + typing.Generic[CommandT, RequestDataT, ResponseDataT] +): + """A session command response.""" + command: CommandT + data: RequestDataT + status: CommandStatus + createdAt: datetime = Field(..., default_factory=utc_now) + startedAt: typing.Optional[datetime] + completedAt: typing.Optional[datetime] + result: typing.Optional[ResponseDataT] = None + + +CommandsEmptyData = Literal[ + ProtocolCommand.start_run, + ProtocolCommand.start_simulate, + ProtocolCommand.cancel, + ProtocolCommand.pause, + ProtocolCommand.resume, + CalibrationCommand.load_labware, + CalibrationCommand.move_to_tip_rack, + CalibrationCommand.move_to_point_one, + CalibrationCommand.move_to_deck, + CalibrationCommand.move_to_reference_point, + CalibrationCommand.pick_up_tip, + CalibrationCommand.confirm_tip_attached, + CalibrationCommand.invalidate_tip, + CalibrationCommand.save_offset, + CalibrationCommand.exit, + CalibrationCommand.invalidate_last_action, + DeckCalibrationCommand.move_to_point_two, + DeckCalibrationCommand.move_to_point_three, + CheckCalibrationCommand.compare_point, + CheckCalibrationCommand.switch_pipette, + CheckCalibrationCommand.return_tip, + CheckCalibrationCommand.transition +] +"""The command definitions requiring no data and result types.""" -class EmptySessionCommand(BasicSessionCommand): - data: EmptyModel +SimpleCommandRequest = SessionCommandRequest[ + CommandsEmptyData, + EmptyModel, + EmptyModel] +"""A command request containing no data and result type.""" -class RobotCommandRequest(EmptySessionCommand): - command: Literal[ - RobotCommand.home_all_motors, - RobotCommand.home_pipette, - RobotCommand.toggle_lights - ] +SimpleCommandResponse = SessionCommandResponse[ + CommandsEmptyData, + EmptyModel, + EmptyModel +] +"""Response to :class:`~SimpleCommandRequest`""" -class ProtocolCommandRequest(EmptySessionCommand): - command: Literal[ - ProtocolCommand.start_run, - ProtocolCommand.start_simulate, - ProtocolCommand.cancel, - ProtocolCommand.pause, - ProtocolCommand.resume - ] +LoadLabwareRequest = SessionCommandRequest[ + Literal[EquipmentCommand.load_labware], + LoadLabwareRequestData, + LoadLabwareResponseData +] -class LoadLabwareRequestM(BasicSessionCommand): - command: Literal[EquipmentCommand.load_labware] - data: LoadLabwareRequest +LoadLabwareResponse = SessionCommandResponse[ + Literal[EquipmentCommand.load_labware], + LoadLabwareRequestData, + LoadLabwareResponseData +] -class LoadInstrumentRequestM(BasicSessionCommand): - command: Literal[EquipmentCommand.load_instrument] - data: LoadInstrumentRequest +LoadInstrumentRequest = SessionCommandRequest[ + Literal[EquipmentCommand.load_instrument], + LoadInstrumentRequestData, + LoadInstrumentResponseData +] -class LiquidRequestM(BasicSessionCommand): - command: Literal[ - PipetteCommand.aspirate, - PipetteCommand.dispense - ] - data: LiquidRequest +LoadInstrumentResponse = SessionCommandResponse[ + Literal[EquipmentCommand.load_instrument], + LoadInstrumentRequestData, + LoadInstrumentResponseData +] -class TipRequestM(BasicSessionCommand): - command: Literal[ - PipetteCommand.drop_tip, - PipetteCommand.pick_up_tip - ] - data: PipetteRequestBase +LiquidRequest = SessionCommandRequest[ + Literal[PipetteCommand.aspirate, + PipetteCommand.dispense], + LiquidRequestData, + EmptyModel +] -class CalibrationRequest(EmptySessionCommand): - command: Literal[ - CalibrationCommand.load_labware, - CalibrationCommand.move_to_tip_rack, - CalibrationCommand.move_to_point_one, - CalibrationCommand.move_to_deck, - CalibrationCommand.move_to_reference_point, - CalibrationCommand.pick_up_tip, - CalibrationCommand.confirm_tip_attached, - CalibrationCommand.invalidate_tip, - CalibrationCommand.save_offset, - CalibrationCommand.exit, - CalibrationCommand.invalidate_last_action, - ] +LiquidResponse = SessionCommandResponse[ + Literal[PipetteCommand.aspirate, + PipetteCommand.dispense], + LiquidRequestData, + EmptyModel +] -class JogRequest(BasicSessionCommand): - command: Literal[CalibrationCommand.jog] - data: JogPosition +TipRequest = SessionCommandRequest[ + Literal[PipetteCommand.drop_tip, + PipetteCommand.pick_up_tip], + PipetteRequestDataBase, + EmptyModel +] + + +TipResponse = SessionCommandResponse[ + Literal[PipetteCommand.drop_tip, + PipetteCommand.pick_up_tip], + PipetteRequestDataBase, + EmptyModel +] -class SetHasCalibrationBlockRequestM(BasicSessionCommand): - command: Literal[CalibrationCommand.set_has_calibration_block] - data: SetHasCalibrationBlockRequest + +JogRequest = SessionCommandRequest[ + Literal[CalibrationCommand.jog], + JogPosition, + EmptyModel +] -class DeckCalibrationCommandRequest(EmptySessionCommand): - command: Literal[ - DeckCalibrationCommand.move_to_point_two, - DeckCalibrationCommand.move_to_point_three - ] +JogResponse = SessionCommandResponse[ + Literal[CalibrationCommand.jog], + JogPosition, + EmptyModel +] -class CheckCalibrationCommandRequest(EmptySessionCommand): - command: Literal[ - CheckCalibrationCommand.compare_point, - CheckCalibrationCommand.switch_pipette, - CheckCalibrationCommand.return_tip, - CheckCalibrationCommand.transition - ] +SetHasCalibrationBlockRequest = SessionCommandRequest[ + Literal[CalibrationCommand.set_has_calibration_block], + SetHasCalibrationBlockRequestData, + EmptyModel +] -class SessionCommand(ResponseDataModel): - """A session command response""" - command: CommandDefinitionType - data: CommandDataType - status: CommandStatus - createdAt: datetime = Field(..., default_factory=utc_now) - startedAt: typing.Optional[datetime] - completedAt: typing.Optional[datetime] - result: typing.Optional[CommandResultType] = None +SetHasCalibrationBlockResponse = SessionCommandResponse[ + Literal[CalibrationCommand.set_has_calibration_block], + SetHasCalibrationBlockRequestData, + EmptyModel] RequestTypes = typing.Union[ - RobotCommandRequest, - ProtocolCommandRequest, - LoadLabwareRequestM, - LoadInstrumentRequestM, - LiquidRequestM, - TipRequestM, - CalibrationRequest, + SimpleCommandRequest, + LoadLabwareRequest, + LoadInstrumentRequest, + LiquidRequest, + TipRequest, JogRequest, - SetHasCalibrationBlockRequestM, - DeckCalibrationCommandRequest, - CheckCalibrationCommandRequest + SetHasCalibrationBlockRequest, ] +"""Union of all request types""" + +ResponseTypes = typing.Union[ + SimpleCommandResponse, + LoadLabwareResponse, + LoadInstrumentResponse, + LiquidResponse, + TipResponse, + JogResponse, + SetHasCalibrationBlockResponse, +] +"""Union of all response types""" + -# Session command requests/responses CommandRequest = RequestModel[ RequestTypes ] +"""The command request model.""" + CommandResponse = ResponseModel[ - SessionCommand + ResponseTypes ] +"""The command response model.""" diff --git a/robot-server/robot_server/service/session/models/command_definitions.py b/robot-server/robot_server/service/session/models/command_definitions.py index 0e82c95ca41..4ad568b06c5 100644 --- a/robot-server/robot_server/service/session/models/command_definitions.py +++ b/robot-server/robot_server/service/session/models/command_definitions.py @@ -1,8 +1,14 @@ +""" +Command type definitions. + +Definitions should be grouped into thematic namespaces. +""" import typing from enum import Enum class CommandDefinition(str, Enum): + """The base of command definition enumerations.""" def __new__(cls, value): """Create a string enum.""" namespace = cls.namespace() @@ -17,9 +23,9 @@ def __new__(cls, value): @staticmethod def namespace(): """ - This is primarily for allowing definitions to define a - namespace. The name space will be used to make the value of the - enum. It will be "{namespace}.{value}" + Override to create a namespoce for the member definitions. The + name.space will be used to make the value of the enum. It will + be "{namespace}.{value}" """ return None diff --git a/robot-server/robot_server/service/session/session_types/base_session.py b/robot-server/robot_server/service/session/session_types/base_session.py index de3e700090f..3d3e04f788a 100644 --- a/robot-server/robot_server/service/session/session_types/base_session.py +++ b/robot-server/robot_server/service/session/session_types/base_session.py @@ -64,20 +64,17 @@ async def clean_up(self): pass async def execute_command(self, command: command_models.RequestTypes) -> \ - command_models.SessionCommand: + command_models.ResponseTypes: """Execute a command.""" - command_obj = create_command(command.command, - command.data) + command_obj = create_command(command) command_result = await self.command_executor.execute(command_obj) - return command_models.SessionCommand( - id=command_obj.meta.identifier, - data=command.data, - command=command.command, + return command.make_response( + identifier=command_obj.meta.identifier, status=command_result.result.status, - createdAt=command_obj.meta.created_at, - startedAt=command_result.result.started_at, - completedAt=command_result.result.completed_at, + created_at=command_obj.meta.created_at, + started_at=command_result.result.started_at, + completed_at=command_result.result.completed_at, result=command_result.result.data, ) 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 5a981c0087d..ac233d0487b 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,12 +1,13 @@ import logging -from typing import Dict, Any +from typing import Dict, Any, Union from robot_server.service.session.command_execution import ( CommandExecutor, Command, CompletedCommand, CommandResult) from robot_server.service.session.errors import UnsupportedCommandException from robot_server.service.session.session_types.live_protocol.command_interface import CommandInterface # noqa: E501 from robot_server.service.session.session_types.live_protocol.state_store import StateStore # noqa: E501 -from robot_server.service.session.models import command_definitions as models +from robot_server.service.session.models import ( + command_definitions as models, command as command_models) from robot_server.util import duration log = logging.getLogger(__name__) @@ -44,26 +45,30 @@ async def execute(self, command: Command) -> CompletedCommand: self._store.handle_command_request(command) # handle side-effects with timing - handler = self._handler_map.get(command.content.name) + handler = self._handler_map.get(command.request.command) if handler: with duration() as timed: - data = await handler(command.content.data) + data = await handler(command.request.data) else: raise UnsupportedCommandException( - f"Command '{command.content.name}' is not supported." + f"Command '{command.request.command}' is not supported." ) - result = CommandResult(started_at=timed.start, - completed_at=timed.end, - data=data) + result = CommandResult[ + Union[ + command_models.LoadLabwareResponseData, + command_models.LoadInstrumentResponseData + ]](started_at=timed.start, + completed_at=timed.end, + data=data) # add result to state self._store.handle_command_result(command, result) # return completed command to session return CompletedCommand( - content=command.content, + request=command.request, meta=command.meta, result=result, ) diff --git a/robot-server/robot_server/service/session/session_types/live_protocol/command_interface.py b/robot-server/robot_server/service/session/session_types/live_protocol/command_interface.py index db8ddabd232..29a6912c34e 100644 --- a/robot-server/robot_server/service/session/session_types/live_protocol/command_interface.py +++ b/robot-server/robot_server/service/session/session_types/live_protocol/command_interface.py @@ -25,7 +25,8 @@ def __init__(self, hardware: ThreadManager, state_store: StateStore): async def handle_load_labware( self, - command: models.LoadLabwareRequest) -> models.LoadLabwareResponse: + command: models.LoadLabwareRequestData) ->\ + models.LoadLabwareResponseData: labware_def = get_labware_definition(load_name=command.loadName, namespace=command.namespace, @@ -34,14 +35,14 @@ async def handle_load_labware( labware_path = f'{helpers.hash_labware_def(labware_def)}.json' calibration = get.get_labware_calibration(labware_path, labware_def, '') - return models.LoadLabwareResponse(labwareId=create_identifier(), - definition=labware_def, - calibration=calibration) + return models.LoadLabwareResponseData(labwareId=create_identifier(), + definition=labware_def, + calibration=calibration) async def handle_load_instrument( self, - command: models.LoadInstrumentRequest) \ - -> models.LoadInstrumentResponse: + command: models.LoadInstrumentRequestData) \ + -> models.LoadInstrumentResponseData: """Load an instrument while checking if it is connected""" mount = command.mount other_mount = mount.other_mount() @@ -63,16 +64,17 @@ async def handle_load_instrument( log.exception("Failed to cache_instruments") raise ProtocolErrorInstrument(str(e)) - return models.LoadInstrumentResponse(instrumentId=create_identifier()) + return models.LoadInstrumentResponseData( + instrumentId=create_identifier()) - async def handle_aspirate(self, command: models.PipetteRequestBase): + async def handle_aspirate(self, command: models.PipetteRequestDataBase): raise UnsupportedCommandException("") - async def handle_dispense(self, command: models.PipetteRequestBase): + async def handle_dispense(self, command: models.PipetteRequestDataBase): raise UnsupportedCommandException("") - async def handle_pick_up_tip(self, command: models.PipetteRequestBase): + async def handle_pick_up_tip(self, command: models.PipetteRequestDataBase): raise UnsupportedCommandException("") - async def handle_drop_tip(self, command: models.PipetteRequestBase): + async def handle_drop_tip(self, command: models.PipetteRequestDataBase): raise UnsupportedCommandException("") diff --git a/robot-server/robot_server/service/session/session_types/live_protocol/state_store.py b/robot-server/robot_server/service/session/session_types/live_protocol/state_store.py index 83e8a3d1bf6..89554b689b2 100644 --- a/robot-server/robot_server/service/session/session_types/live_protocol/state_store.py +++ b/robot-server/robot_server/service/session/session_types/live_protocol/state_store.py @@ -53,7 +53,7 @@ def handle_command_result( Update the state upon completion of a handled command. """ self._command_results_map[command.meta.identifier] = result - handler = self._handler_map.get(command.content.name) + handler = self._handler_map.get(command.request.command) if handler: handler(command, result) @@ -61,8 +61,9 @@ def handle_load_labware(self, command: Command, result: CommandResult) -> None: """Update state according to load_labware() command result.""" - result_data = cast(models.LoadLabwareResponse, result.data) - command_data = cast(models.LoadLabwareRequest, command.content.data) + result_data = cast(models.LoadLabwareResponseData, result.data) + command_data = cast(models.LoadLabwareRequestData, + command.request.data) self._labware[result_data.labwareId] = LabwareEntry( definition=result_data.definition, calibration=result_data.calibration, @@ -72,8 +73,9 @@ def handle_load_instrument(self, command: Command, result: CommandResult) -> None: """Store result of load instrument""" - result_data = cast(models.LoadInstrumentResponse, result.data) - command_data = cast(models.LoadInstrumentRequest, command.content.data) + result_data = cast(models.LoadInstrumentResponseData, result.data) + command_data = cast(models.LoadInstrumentRequestData, + command.request.data) self._instruments[result_data.instrumentId] = InstrumentEntry( mount=command_data.mount.to_hw_mount(), name=command_data.instrumentName diff --git a/robot-server/robot_server/service/session/session_types/protocol/execution/command_executor.py b/robot-server/robot_server/service/session/session_types/protocol/execution/command_executor.py index 090a6fe3446..4dabeee0669 100644 --- a/robot-server/robot_server/service/session/session_types/protocol/execution/command_executor.py +++ b/robot-server/robot_server/service/session/session_types/protocol/execution/command_executor.py @@ -99,7 +99,7 @@ def create_worker(configuration: SessionConfiguration, async def execute(self, command: Command) -> CompletedCommand: """Command processing""" - command_def = command.content.name + command_def = command.request.command if command_def not in self.STATE_COMMAND_MAP.get( self.current_state, {} ): @@ -119,14 +119,14 @@ async def execute(self, command: Command) -> CompletedCommand: self._events.append( models.ProtocolSessionEvent( source=models.EventSource.session_command, - event=command.content.name, + event=command.request.command, commandId=command.meta.identifier, timestamp=timed.end, ) ) return CompletedCommand( - content=command.content, + request=command.request, meta=command.meta, result=CommandResult(started_at=timed.start, completed_at=timed.end)) diff --git a/robot-server/tests/service/session/models/__init__.py b/robot-server/tests/service/session/models/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/robot-server/tests/service/session/models/test_command.py b/robot-server/tests/service/session/models/test_command.py new file mode 100644 index 00000000000..4b7df3a8ece --- /dev/null +++ b/robot-server/tests/service/session/models/test_command.py @@ -0,0 +1,112 @@ +from datetime import datetime +import pytest +from pydantic import ValidationError + +from robot_server.service.session.models import command, command_definitions + + +@pytest.mark.parametrize( + argnames="command_def", + argvalues=[ + command_definitions.ProtocolCommand.start_run, + command_definitions.CalibrationCommand.move_to_deck, + command_definitions.CheckCalibrationCommand.compare_point, + ]) +def test_empty(command_def: command_definitions.CommandDefinition): + """Test creation of empty command request and response.""" + request = command.CommandRequest.parse_obj({ + "data": { + "command": command_def.value, + "data": {} + } + }) + assert request.data.command == command_def + assert request.data.data == command.EmptyModel() + + 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=None + ) + + assert response.command == command_def + assert response.data == command.EmptyModel() + assert response.id == "id" + assert response.createdAt == dt + assert response.startedAt is None + assert response.completedAt is None + 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.loadInstrument", + "data": { + "instrumentName": "p10_single", + "mount": "left" + } + } + }) + assert request.data.command == \ + command_definitions.EquipmentCommand.load_instrument + assert request.data.data == command.LoadInstrumentRequestData( + instrumentName="p10_single", + mount=command.Mount.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=command.LoadInstrumentResponseData( + instrumentId=command.IdentifierType("123") + ) + ) + + assert response.command == \ + command_definitions.EquipmentCommand.load_instrument + assert response.data == command.LoadInstrumentRequestData( + instrumentName="p10_single", + mount=command.Mount.left + ) + assert response.id == "id" + assert response.createdAt == dt + assert response.startedAt is None + assert response.completedAt is None + assert response.result == command.LoadInstrumentResponseData( + instrumentId=command.IdentifierType("123") + ) + + +@pytest.mark.parametrize( + argnames="command_def", + argvalues=[ + command_definitions.EquipmentCommand.load_labware, + command_definitions.EquipmentCommand.load_instrument, + command_definitions.PipetteCommand.aspirate, + command_definitions.PipetteCommand.dispense, + command_definitions.PipetteCommand.drop_tip, + command_definitions.PipetteCommand.pick_up_tip, + command_definitions.CalibrationCommand.jog, + command_definitions.CalibrationCommand.set_has_calibration_block + ]) +def test_requires_data(command_def: command_definitions.CommandDefinition): + """Test creation of command requiring data will fail with empty body.""" + with pytest.raises(ValidationError): + command.CommandRequest.parse_obj({ + "data": { + "command": command_def.value, + "data": {} + } + }) 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 index b8a62c7a470..3246a40c955 100644 --- 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 @@ -4,7 +4,9 @@ from robot_server.service.session.models import command_definitions from robot_server.service.legacy.models.control import Mount from robot_server.service.session.command_execution.command import \ - CommandContent, CommandResult + CommandResult +from robot_server.service.session.models.command import LoadLabwareRequest, \ + LiquidRequest, TipRequest, SimpleCommandRequest, LoadInstrumentRequest from robot_server.service.session.models.common import EmptyModel from robot_server.service.session.session_types.live_protocol.command_executor\ import LiveProtocolCommandExecutor @@ -48,36 +50,38 @@ def command_executor(mock_command_interface, mock_state_store)\ async def test_load_labware(command_executor, mock_command_interface): - expected_response = models.LoadLabwareResponse(labwareId="your labware", - definition={}, - calibration=(1, 2, 3)) + expected_response = models.LoadLabwareResponseData( + labwareId="your labware", + definition={}, + calibration=(1, 2, 3)) async def handler(command): return expected_response mock_command_interface.handle_load_labware.side_effect = handler - command = models.LoadLabwareRequest(location=1, - loadName="hello", - displayName="niceName", - version=1, - namespace="test") + command = models.LoadLabwareRequestData(location=1, + loadName="hello", + displayName="niceName", + version=1, + namespace="test") result = await command_executor.execute( - Command(content=CommandContent( - name=command_definitions.EquipmentCommand.load_labware, - data=command)) + Command( + request=LoadLabwareRequest( + command=command_definitions.EquipmentCommand.load_labware, + data=command)) ) mock_command_interface.handle_load_labware.assert_called_once_with(command) assert result.result.data == expected_response - assert result.content == CommandContent( - name=command_definitions.EquipmentCommand.load_labware, + assert result.request == LoadLabwareRequest( + command=command_definitions.EquipmentCommand.load_labware, data=command) async def test_load_instrument(command_executor, mock_command_interface): - expected_response = models.LoadInstrumentResponse( + expected_response = models.LoadInstrumentResponseData( instrumentId="your instrument") async def handler(command): @@ -85,11 +89,11 @@ async def handler(command): mock_command_interface.handle_load_instrument.side_effect = handler - command = models.LoadInstrumentRequest(instrumentName="p1000_single", - mount=Mount.left) + command = models.LoadInstrumentRequestData(instrumentName="p1000_single", + mount=Mount.left) result = await command_executor.execute( - Command(content=CommandContent( - name=command_definitions.EquipmentCommand.load_instrument, + Command(request=LoadInstrumentRequest( + command=command_definitions.EquipmentCommand.load_instrument, data=command)) ) @@ -97,8 +101,8 @@ async def handler(command): command) assert result.result.data == expected_response - assert result.content == CommandContent( - name=command_definitions.EquipmentCommand.load_instrument, + assert result.request == LoadInstrumentRequest( + command=command_definitions.EquipmentCommand.load_instrument, data=command) @@ -113,7 +117,7 @@ async def test_liquid_commands(command_executor, mock_command_interface, async def handler(command): return None - command_data = models.LiquidRequest( + command_data = models.LiquidRequestData( pipetteId="", labwareId="", wellId="", flowRate=2, volume=1, offsetFromBottom=2) @@ -121,8 +125,8 @@ async def handler(command): mock_command_interface.handle_dispense.side_effect = handler result = await command_executor.execute( - Command(content=CommandContent( - name=command_type, + Command(request=LiquidRequest( + command=command_type, data=command_data)) ) @@ -130,8 +134,8 @@ async def handler(command): command_data) assert result.result.data is None - assert result.content == CommandContent( - name=command_type, + assert result.request == LiquidRequest( + command=command_type, data=command_data) @@ -146,15 +150,15 @@ async def test_tip_commands(command_executor, mock_command_interface, async def handler(command): return None - command_data = models.PipetteRequestBase( + command_data = models.PipetteRequestDataBase( pipetteId="", labwareId="", wellId="") mock_command_interface.handle_pick_up_tip.side_effect = handler mock_command_interface.handle_drop_tip.side_effect = handler result = await command_executor.execute( - Command(content=CommandContent( - name=command_type, + Command(request=TipRequest( + command=command_type, data=command_data)) ) @@ -162,8 +166,8 @@ async def handler(command): command_data) assert result.result.data is None - assert result.content == CommandContent( - name=command_type, + assert result.request == TipRequest( + command=command_type, data=command_data) @@ -175,16 +179,17 @@ async def handle_command(c): return 23 # Mock out the command handler map - command_executor._handler_map = {"test_command": handle_command} + command_executor._handler_map = { + command_definitions.ProtocolCommand.start_run: handle_command} return command_executor async def test_create_command_in_state_store(state_store_command_executor, mock_state_store): - c = Command(content=CommandContent( - name="test_command", - data=EmptyModel() - )) + c = Command( + request=SimpleCommandRequest( + command=command_definitions.ProtocolCommand.start_run, + data=EmptyModel())) await state_store_command_executor.execute(c) mock_state_store.handle_command_request.assert_called_once_with(c) @@ -192,10 +197,10 @@ async def test_create_command_in_state_store(state_store_command_executor, async def test_command_result_in_state_store(state_store_command_executor, mock_state_store): - c = Command(content=CommandContent( - name="test_command", - data=EmptyModel() - )) + c = Command(request=SimpleCommandRequest( + command=command_definitions.ProtocolCommand.start_run, + data=EmptyModel())) + await state_store_command_executor.execute(c) mock_state_store.handle_command_result.assert_called() diff --git a/robot-server/tests/service/session/session_types/live_protocol/test_command_interface.py b/robot-server/tests/service/session/session_types/live_protocol/test_command_interface.py index 0367a71d5b5..d1ec0faf482 100644 --- a/robot-server/tests/service/session/session_types/live_protocol/test_command_interface.py +++ b/robot-server/tests/service/session/session_types/live_protocol/test_command_interface.py @@ -28,7 +28,7 @@ def labware_calibration_mock(): @pytest.fixture def load_labware_cmd(): - return models.LoadLabwareRequest( + return models.LoadLabwareRequestData( location=1, loadName="labware-load-name", displayName="labware display name", @@ -39,7 +39,7 @@ def load_labware_cmd(): @pytest.fixture def load_instrument_cmd(): - return models.LoadInstrumentRequest( + return models.LoadInstrumentRequestData( instrumentName='p50_single', mount=Mount.left ) diff --git a/robot-server/tests/service/session/session_types/live_protocol/test_state_store.py b/robot-server/tests/service/session/session_types/live_protocol/test_state_store.py index 9dd95cea172..c3a4005dc51 100644 --- a/robot-server/tests/service/session/session_types/live_protocol/test_state_store.py +++ b/robot-server/tests/service/session/session_types/live_protocol/test_state_store.py @@ -3,7 +3,7 @@ from opentrons import types -from robot_server.service.session.models import command_definitions +from robot_server.service.session.models import command_definitions as cmddef from robot_server.service.legacy.models.control import Mount from robot_server.service.session.models import command as models from robot_server.service.session.command_execution \ @@ -15,14 +15,16 @@ def test_handle_command_request(): store = StateStore() command = create_command( - name=command_definitions.EquipmentCommand.load_labware, - data=models.LoadLabwareRequest( - location=1, - loadName="labware-load-name", - displayName="labware display name", - namespace="opentrons test", - version=1, - ), + request=models.LoadLabwareRequest( + command=cmddef.EquipmentCommand.load_labware, + data=models.LoadLabwareRequestData( + location=1, + loadName="labware-load-name", + displayName="labware display name", + namespace="opentrons test", + version=1, + ), + ) ) store.handle_command_request(command) @@ -33,14 +35,16 @@ def test_store_has_handle_command_response_method(): with patch.object(StateStore, "handle_load_labware"): store = StateStore() command = create_command( - name=command_definitions.EquipmentCommand.load_labware, - data=models.LoadLabwareRequest( - location=1, - loadName="labware-load-name", - displayName="labware display name", - namespace="opentrons test", - version=1, - ), + request=models.LoadLabwareRequest( + command=cmddef.EquipmentCommand.load_labware, + data=models.LoadLabwareRequestData( + location=1, + loadName="labware-load-name", + displayName="labware display name", + namespace="opentrons test", + version=1, + ), + ) ) command_result = CommandResult( started_at=command.meta.created_at, @@ -54,19 +58,34 @@ def test_store_has_handle_command_response_method(): @pytest.mark.parametrize( - argnames="command_name, handler", + argnames="command_request, handler", argvalues=[[ - command_definitions.EquipmentCommand.load_labware, + models.LoadLabwareRequest( + command=cmddef.EquipmentCommand.load_labware, + data=models.LoadLabwareRequestData( + location=1, + loadName="a", + displayName="a", + namespace="a", + version=1 + ) + ), "handle_load_labware"], [ - command_definitions.EquipmentCommand.load_instrument, + models.LoadInstrumentRequest( + command=cmddef.EquipmentCommand.load_instrument, + data=models.LoadInstrumentRequestData( + instrumentName="p50_multi", + mount=Mount.left, + ) + ), "handle_load_instrument"]] ) -def test_command_result_state_handler(command_name, handler): +def test_command_result_state_handler(command_request, handler): with patch.object(StateStore, handler) as handler_mock: store = StateStore() - command = create_command(name=command_name, data=None) + command = create_command(request=command_request) command_result = CommandResult( started_at=command.meta.created_at, completed_at=command.meta.created_at, @@ -77,21 +96,24 @@ def test_command_result_state_handler(command_name, handler): def test_load_labware_update(): store = StateStore() - labware = command_definitions.EquipmentCommand.load_labware - command = create_command(name=labware, - data=models.LoadLabwareRequest( - location=1, - loadName="labware-load-name", - displayName="labware display name", - namespace="opentrons test", - version=1, - )) + command = create_command( + request=models.LoadLabwareRequest( + command=cmddef.EquipmentCommand.load_labware, + data=models.LoadLabwareRequestData( + location=1, + loadName="labware-load-name", + displayName="labware display name", + namespace="opentrons test", + version=1, + )) + ) command_result = CommandResult( started_at=command.meta.created_at, completed_at=command.meta.created_at, - data=models.LoadLabwareResponse(labwareId="1234", - definition={"myLabware": "definition"}, - calibration=(1, 2, 3))) + data=models.LoadLabwareResponseData( + labwareId="1234", + definition={"myLabware": "definition"}, + calibration=(1, 2, 3))) assert store.get_labware_by_id(command_result.data.labwareId) is None store.handle_command_result(command, command_result) assert store.get_labware_by_id(command_result.data.labwareId) == \ @@ -102,20 +124,22 @@ def test_load_labware_update(): def test_load_instrument_update(): store = StateStore() - instrument = command_definitions.EquipmentCommand.load_instrument - command = create_command(name=instrument, - data=models.LoadInstrumentRequest( - instrumentName='p10_single', - mount=Mount.left) - ) + command = create_command( + request=models.LoadInstrumentRequest( + command=cmddef.EquipmentCommand.load_instrument, + data=models.LoadInstrumentRequestData( + instrumentName='p10_single', + mount=Mount.left) + ) + ) command_result = CommandResult( started_at=command.meta.created_at, completed_at=command.meta.created_at, - data=models.LoadInstrumentResponse(instrumentId="1234")) + data=models.LoadInstrumentResponseData(instrumentId="1234")) assert store.get_instrument_by_id(command_result.data.instrumentId) is None assert store.get_instrument_by_mount( - command.content.data.mount.to_hw_mount() + command.request.data.mount.to_hw_mount() ) is None store.handle_command_result(command, command_result) @@ -125,5 +149,5 @@ def test_load_instrument_update(): assert store.get_instrument_by_id(command_result.data.instrumentId) == \ expected_instrument assert store.get_instrument_by_mount( - command.content.data.mount.to_hw_mount() + command.request.data.mount.to_hw_mount() ) == expected_instrument diff --git a/robot-server/tests/service/session/session_types/protocol/execution/test_command_executor.py b/robot-server/tests/service/session/session_types/protocol/execution/test_command_executor.py index 1dc197abc26..1288b4e3cf7 100644 --- a/robot-server/tests/service/session/session_types/protocol/execution/test_command_executor.py +++ b/robot-server/tests/service/session/session_types/protocol/execution/test_command_executor.py @@ -2,8 +2,9 @@ from datetime import datetime import pytest -from robot_server.service.session.command_execution.command import Command, CommandContent # noqa: E501 +from robot_server.service.session.command_execution.command import Command from robot_server.service.session.errors import UnsupportedCommandException +from robot_server.service.session.models.command import SimpleCommandRequest from robot_server.service.session.models.command_definitions import \ ProtocolCommand from robot_server.service.session.models.common import EmptyModel @@ -74,8 +75,8 @@ async def test_command_state_reject(loop, protocol_command_executor.current_state = current_state for protocol_command in ProtocolCommand: - command = Command(content=CommandContent( - name=protocol_command, + command = Command(request=SimpleCommandRequest( + command=protocol_command, data=EmptyModel()) ) if protocol_command in accepted_commands: @@ -100,8 +101,8 @@ async def test_execute(loop, command, worker_method_name, with patch.object(ProtocolCommandExecutor, "STATE_COMMAND_MAP", new={protocol_command_executor.current_state: ProtocolCommand}): # noqa: E501 - protocol_command = Command(content=CommandContent( - name=command, + protocol_command = Command(request=SimpleCommandRequest( + command=command, data=EmptyModel()) ) await protocol_command_executor.execute(protocol_command) diff --git a/robot-server/tests/service/session/test_router.py b/robot-server/tests/service/session/test_router.py index 316b8f04c6e..429d02c8561 100644 --- a/robot-server/tests/service/session/test_router.py +++ b/robot-server/tests/service/session/test_router.py @@ -10,9 +10,11 @@ from robot_server.service.session.command_execution import CommandExecutor, \ Command from robot_server.service.session.command_execution.command import \ - CommandResult, CompletedCommand, CommandContent, CommandMeta, CommandStatus + CommandResult, CompletedCommand, CommandMeta, CommandStatus from robot_server.service.session.errors import SessionCreationException, \ UnsupportedCommandException, CommandExecutionException +from robot_server.service.session.models.command import JogRequest, \ + SimpleCommandRequest from robot_server.service.session.models.common import EmptyModel, JogPosition from robot_server.service.session.models.command_definitions import \ CalibrationCommand @@ -53,8 +55,8 @@ def command_created_at(): def patch_create_command(command_id, command_created_at): session_path = "robot_server.service.session.session_types" with patch(f"{session_path}.base_session.create_command") as p: - p.side_effect = lambda c, n: Command( - content=CommandContent(c, n), + p.side_effect = lambda c: Command( + request=c, meta=CommandMeta(command_id, command_created_at)) yield p @@ -64,7 +66,7 @@ def mock_command_executor(): mock = MagicMock(spec=CommandExecutor) async def func(command): - return CompletedCommand(content=command.content, + return CompletedCommand(request=command.request, meta=command.meta, result=CommandResult( status=CommandStatus.executed, @@ -313,8 +315,8 @@ def test_execute_command(api_client, mock_command_executor.execute.assert_called_once_with( Command( - content=CommandContent( - name=CalibrationCommand.jog, + request=JogRequest( + command=CalibrationCommand.jog, data=JogPosition(vector=(1, 2, 3,)) ), meta=CommandMeta(identifier=command_id, @@ -370,8 +372,8 @@ def test_execute_command_no_body(api_client, mock_command_executor.execute.assert_called_once_with( Command( - content=CommandContent( - name=CalibrationCommand.load_labware, + request=SimpleCommandRequest( + command=CalibrationCommand.load_labware, data=EmptyModel()), meta=CommandMeta(command_id, command_created_at) )