From 623bfc7bc70635a29ebe42587747a45c34b6efc9 Mon Sep 17 00:00:00 2001 From: amit lissack Date: Tue, 29 Dec 2020 10:02:23 -0500 Subject: [PATCH 1/6] rename models. --- .../service/session/models/command.py | 54 +++++++++---------- .../live_protocol/command_interface.py | 24 +++++---- .../live_protocol/state_store.py | 10 ++-- .../live_protocol/test_command_executor.py | 27 +++++----- .../live_protocol/test_command_interface.py | 4 +- .../live_protocol/test_state_store.py | 17 +++--- 6 files changed, 71 insertions(+), 65 deletions(-) diff --git a/robot-server/robot_server/service/session/models/command.py b/robot-server/robot_server/service/session/models/command.py index 0f2dffc793a..b1776e06f1e 100644 --- a/robot-server/robot_server/service/session/models/command.py +++ b/robot-server/robot_server/service/session/models/command.py @@ -19,7 +19,7 @@ from opentrons.util.helpers import utc_now -class LoadLabwareRequest(BaseModel): +class LoadLabwareRequestData(BaseModel): location: int = Field( ..., description="Deck slot", ge=1, lt=12) @@ -37,30 +37,30 @@ class LoadLabwareRequest(BaseModel): description="The labware definition version") -class LoadLabwareResponse(BaseModel): +class LoadLabwareResponseData(BaseModel): labwareId: IdentifierType definition: LabwareDefinition calibration: OffsetVector -class LoadInstrumentRequest(BaseModel): +class LoadInstrumentRequestData(BaseModel): instrumentName: PipetteName = Field( ..., description="The name of the instrument model") mount: Mount -class LoadInstrumentResponse(BaseModel): +class LoadInstrumentResponseData(BaseModel): 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,26 +81,26 @@ 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, + SetHasCalibrationBlockRequestData, JogPosition, - LiquidRequest, - PipetteRequestBase, - LoadLabwareRequest, - LoadInstrumentRequest, + LiquidRequestData, + PipetteRequestDataBase, + LoadLabwareRequestData, + LoadInstrumentRequestData, EmptyModel ] # A Union of all command result types CommandResultType = typing.Union[ - LoadLabwareResponse, - LoadInstrumentResponse, + LoadLabwareResponseData, + LoadInstrumentResponseData, ] @@ -140,30 +140,30 @@ class ProtocolCommandRequest(EmptySessionCommand): ] -class LoadLabwareRequestM(BasicSessionCommand): +class LoadLabwareRequest(BasicSessionCommand): command: Literal[EquipmentCommand.load_labware] - data: LoadLabwareRequest + data: LoadLabwareRequestData -class LoadInstrumentRequestM(BasicSessionCommand): +class LoadInstrumentRequest(BasicSessionCommand): command: Literal[EquipmentCommand.load_instrument] - data: LoadInstrumentRequest + data: LoadInstrumentRequestData -class LiquidRequestM(BasicSessionCommand): +class LiquidRequest(BasicSessionCommand): command: Literal[ PipetteCommand.aspirate, PipetteCommand.dispense ] - data: LiquidRequest + data: LiquidRequestData -class TipRequestM(BasicSessionCommand): +class TipRequest(BasicSessionCommand): command: Literal[ PipetteCommand.drop_tip, PipetteCommand.pick_up_tip ] - data: PipetteRequestBase + data: PipetteRequestDataBase class CalibrationRequest(EmptySessionCommand): @@ -189,7 +189,7 @@ class JogRequest(BasicSessionCommand): class SetHasCalibrationBlockRequestM(BasicSessionCommand): command: Literal[CalibrationCommand.set_has_calibration_block] - data: SetHasCalibrationBlockRequest + data: SetHasCalibrationBlockRequestData class DeckCalibrationCommandRequest(EmptySessionCommand): @@ -222,10 +222,10 @@ class SessionCommand(ResponseDataModel): RequestTypes = typing.Union[ RobotCommandRequest, ProtocolCommandRequest, - LoadLabwareRequestM, - LoadInstrumentRequestM, - LiquidRequestM, - TipRequestM, + LoadLabwareRequest, + LoadInstrumentRequest, + LiquidRequest, + TipRequest, CalibrationRequest, JogRequest, SetHasCalibrationBlockRequestM, 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..f25f095b1ad 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 @@ -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.content.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.content.data) self._instruments[result_data.instrumentId] = InstrumentEntry( mount=command_data.mount.to_hw_mount(), name=command_data.instrumentName 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..72d08c17e4b 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 @@ -48,20 +48,21 @@ 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, @@ -77,7 +78,7 @@ async def handler(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,8 +86,8 @@ 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, @@ -113,7 +114,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) @@ -146,7 +147,7 @@ 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 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..60e3f8560cb 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 @@ -16,7 +16,7 @@ def test_handle_command_request(): store = StateStore() command = create_command( name=command_definitions.EquipmentCommand.load_labware, - data=models.LoadLabwareRequest( + data=models.LoadLabwareRequestData( location=1, loadName="labware-load-name", displayName="labware display name", @@ -34,7 +34,7 @@ def test_store_has_handle_command_response_method(): store = StateStore() command = create_command( name=command_definitions.EquipmentCommand.load_labware, - data=models.LoadLabwareRequest( + data=models.LoadLabwareRequestData( location=1, loadName="labware-load-name", displayName="labware display name", @@ -79,7 +79,7 @@ def test_load_labware_update(): store = StateStore() labware = command_definitions.EquipmentCommand.load_labware command = create_command(name=labware, - data=models.LoadLabwareRequest( + data=models.LoadLabwareRequestData( location=1, loadName="labware-load-name", displayName="labware display name", @@ -89,9 +89,10 @@ def test_load_labware_update(): 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) == \ @@ -104,14 +105,14 @@ def test_load_instrument_update(): store = StateStore() instrument = command_definitions.EquipmentCommand.load_instrument command = create_command(name=instrument, - data=models.LoadInstrumentRequest( + 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( From 68439e2b9218e8048840419396b7ba8bb1a19e4e Mon Sep 17 00:00:00 2001 From: amit lissack Date: Tue, 29 Dec 2020 11:25:12 -0500 Subject: [PATCH 2/6] tests pass. --- .../command_execution/base_executor.py | 2 +- .../command_execution/callable_executor.py | 6 +- .../session/command_execution/command.py | 22 +---- .../command_execution/hardware_executor.py | 77 --------------- .../service/session/models/command.py | 41 ++++---- .../session/session_types/base_session.py | 3 +- .../live_protocol/command_executor.py | 8 +- .../live_protocol/state_store.py | 6 +- .../protocol/execution/command_executor.py | 6 +- .../live_protocol/test_command_executor.py | 58 +++++------ .../live_protocol/test_state_store.py | 97 ++++++++++++------- .../execution/test_command_executor.py | 11 ++- .../tests/service/session/test_router.py | 18 ++-- 13 files changed, 147 insertions(+), 208 deletions(-) delete mode 100644 robot-server/robot_server/service/session/command_execution/hardware_executor.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..b3a470b94f6 100644 --- a/robot-server/robot_server/service/session/command_execution/command.py +++ b/robot-server/robot_server/service/session/command_execution/command.py @@ -3,20 +3,12 @@ from typing import Optional from robot_server.service.session.models.command import ( - CommandDataType, CommandStatus, CommandResultType) -from robot_server.service.session.models.command_definitions import \ - CommandDefinitionType + CommandStatus, CommandResultType, 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) @@ -33,23 +25,19 @@ class CommandResult: @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 b1776e06f1e..c2d5d3249ca 100644 --- a/robot-server/robot_server/service/session/models/command.py +++ b/robot-server/robot_server/service/session/models/command.py @@ -13,6 +13,7 @@ from robot_server.service.session.models.common import ( EmptyModel, JogPosition, IdentifierType, OffsetVector) from pydantic import BaseModel, Field +from pydantic.generics import GenericModel from robot_server.service.legacy.models.control import Mount from robot_server.service.json_api import ( ResponseModel, RequestModel, ResponseDataModel) @@ -111,18 +112,18 @@ class CommandStatus(str, Enum): failed = "failed" -class BasicSessionCommand(BaseModel): +RequestDataT = typing.TypeVar('RequestDataT', bound=BaseModel) + + +class SessionCommandRequest(GenericModel, typing.Generic[RequestDataT]): """A session command""" command: CommandDefinitionType = Field( ..., description="The command description") + data: RequestDataT -class EmptySessionCommand(BasicSessionCommand): - data: EmptyModel - - -class RobotCommandRequest(EmptySessionCommand): +class RobotCommandRequest(SessionCommandRequest[EmptyModel]): command: Literal[ RobotCommand.home_all_motors, RobotCommand.home_pipette, @@ -130,7 +131,7 @@ class RobotCommandRequest(EmptySessionCommand): ] -class ProtocolCommandRequest(EmptySessionCommand): +class ProtocolCommandRequest(SessionCommandRequest[EmptyModel]): command: Literal[ ProtocolCommand.start_run, ProtocolCommand.start_simulate, @@ -140,33 +141,29 @@ class ProtocolCommandRequest(EmptySessionCommand): ] -class LoadLabwareRequest(BasicSessionCommand): +class LoadLabwareRequest(SessionCommandRequest[LoadLabwareRequestData]): command: Literal[EquipmentCommand.load_labware] - data: LoadLabwareRequestData -class LoadInstrumentRequest(BasicSessionCommand): +class LoadInstrumentRequest(SessionCommandRequest[LoadInstrumentRequestData]): command: Literal[EquipmentCommand.load_instrument] - data: LoadInstrumentRequestData -class LiquidRequest(BasicSessionCommand): +class LiquidRequest(SessionCommandRequest[LiquidRequestData]): command: Literal[ PipetteCommand.aspirate, PipetteCommand.dispense ] - data: LiquidRequestData -class TipRequest(BasicSessionCommand): +class TipRequest(SessionCommandRequest[PipetteRequestDataBase]): command: Literal[ PipetteCommand.drop_tip, PipetteCommand.pick_up_tip ] - data: PipetteRequestDataBase -class CalibrationRequest(EmptySessionCommand): +class CalibrationRequest(SessionCommandRequest[EmptyModel]): command: Literal[ CalibrationCommand.load_labware, CalibrationCommand.move_to_tip_rack, @@ -182,24 +179,24 @@ class CalibrationRequest(EmptySessionCommand): ] -class JogRequest(BasicSessionCommand): +class JogRequest(SessionCommandRequest[JogPosition]): command: Literal[CalibrationCommand.jog] - data: JogPosition -class SetHasCalibrationBlockRequestM(BasicSessionCommand): +class SetHasCalibrationBlockRequestM( + SessionCommandRequest[SetHasCalibrationBlockRequestData] +): command: Literal[CalibrationCommand.set_has_calibration_block] - data: SetHasCalibrationBlockRequestData -class DeckCalibrationCommandRequest(EmptySessionCommand): +class DeckCalibrationCommandRequest(SessionCommandRequest[EmptyModel]): command: Literal[ DeckCalibrationCommand.move_to_point_two, DeckCalibrationCommand.move_to_point_three ] -class CheckCalibrationCommandRequest(EmptySessionCommand): +class CheckCalibrationCommandRequest(SessionCommandRequest[EmptyModel]): command: Literal[ CheckCalibrationCommand.compare_point, CheckCalibrationCommand.switch_pipette, 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..78d91121332 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 @@ -66,8 +66,7 @@ async def clean_up(self): async def execute_command(self, command: command_models.RequestTypes) -> \ command_models.SessionCommand: """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( 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..dd15e86bb11 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 @@ -44,14 +44,14 @@ 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, @@ -63,7 +63,7 @@ async def execute(self, command: Command) -> CompletedCommand: # 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/state_store.py b/robot-server/robot_server/service/session/session_types/live_protocol/state_store.py index f25f095b1ad..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) @@ -63,7 +63,7 @@ def handle_load_labware(self, """Update state according to load_labware() command result.""" result_data = cast(models.LoadLabwareResponseData, result.data) command_data = cast(models.LoadLabwareRequestData, - command.content.data) + command.request.data) self._labware[result_data.labwareId] = LabwareEntry( definition=result_data.definition, calibration=result_data.calibration, @@ -75,7 +75,7 @@ def handle_load_instrument(self, """Store result of load instrument""" result_data = cast(models.LoadInstrumentResponseData, result.data) command_data = cast(models.LoadInstrumentRequestData, - command.content.data) + 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/session_types/live_protocol/test_command_executor.py b/robot-server/tests/service/session/session_types/live_protocol/test_command_executor.py index 72d08c17e4b..38e1d76f749 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, RobotCommandRequest, LoadInstrumentRequest from robot_server.service.session.models.common import EmptyModel from robot_server.service.session.session_types.live_protocol.command_executor\ import LiveProtocolCommandExecutor @@ -64,16 +66,17 @@ async def handler(command): 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) @@ -89,8 +92,8 @@ async def handler(command): 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)) ) @@ -98,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) @@ -122,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)) ) @@ -131,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) @@ -154,8 +157,8 @@ async def handler(command): 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)) ) @@ -163,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) @@ -176,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.RobotCommand.toggle_lights: 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=RobotCommandRequest( + command=command_definitions.RobotCommand.toggle_lights, + data=EmptyModel())) await state_store_command_executor.execute(c) mock_state_store.handle_command_request.assert_called_once_with(c) @@ -193,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=RobotCommandRequest( + command=command_definitions.RobotCommand.toggle_lights, + 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_state_store.py b/robot-server/tests/service/session/session_types/live_protocol/test_state_store.py index 60e3f8560cb..0c01fb61478 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 @@ -15,14 +15,16 @@ def test_handle_command_request(): store = StateStore() command = create_command( - name=command_definitions.EquipmentCommand.load_labware, - data=models.LoadLabwareRequestData( - location=1, - loadName="labware-load-name", - displayName="labware display name", - namespace="opentrons test", - version=1, - ), + request=models.LoadLabwareRequest( + command=command_definitions.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.LoadLabwareRequestData( - location=1, - loadName="labware-load-name", - displayName="labware display name", - namespace="opentrons test", - version=1, - ), + request=models.LoadLabwareRequest( + command=command_definitions.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=command_definitions.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=command_definitions.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, @@ -78,14 +97,17 @@ 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.LoadLabwareRequestData( - location=1, - loadName="labware-load-name", - displayName="labware display name", - namespace="opentrons test", - version=1, - )) + command = create_command( + request=models.LoadLabwareRequest( + command=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, @@ -104,11 +126,14 @@ 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.LoadInstrumentRequestData( - instrumentName='p10_single', - mount=Mount.left) - ) + command = create_command( + request=models.LoadInstrumentRequest( + command=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, @@ -116,7 +141,7 @@ def test_load_instrument_update(): 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) @@ -126,5 +151,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..612a419cc8f 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 ProtocolCommandRequest 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=ProtocolCommandRequest( + 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=ProtocolCommandRequest( + 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..ed213850725 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, \ + LoadLabwareRequest, CalibrationRequest 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=CalibrationRequest( + command=CalibrationCommand.load_labware, data=EmptyModel()), meta=CommandMeta(command_id, command_created_at) ) From 9fd2c9bc6db05af6c3e10a133c19dccc171317c7 Mon Sep 17 00:00:00 2001 From: amit lissack Date: Wed, 30 Dec 2020 11:23:34 -0500 Subject: [PATCH 3/6] Genericize command requests and responses --- .../session/command_execution/command.py | 11 +- .../service/session/models/command.py | 257 ++++++++++++------ .../session/session_types/base_session.py | 14 +- .../live_protocol/command_executor.py | 15 +- .../live_protocol/test_command_executor.py | 12 +- .../live_protocol/test_state_store.py | 16 +- .../execution/test_command_executor.py | 6 +- .../tests/service/session/test_router.py | 4 +- 8 files changed, 212 insertions(+), 123 deletions(-) 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 b3a470b94f6..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,9 +1,9 @@ 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 ( - CommandStatus, CommandResultType, RequestTypes) + CommandStatus, RequestTypes) from robot_server.service.session.models.common import ( IdentifierType, create_identifier) from opentrons.util.helpers import utc_now @@ -15,12 +15,15 @@ class CommandMeta: 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) diff --git a/robot-server/robot_server/service/session/models/command.py b/robot-server/robot_server/service/session/models/command.py index c2d5d3249ca..8f0f828b617 100644 --- a/robot-server/robot_server/service/session/models/command.py +++ b/robot-server/robot_server/service/session/models/command.py @@ -2,22 +2,22 @@ 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 pydantic.generics import GenericModel 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 LoadLabwareRequestData(BaseModel): @@ -112,122 +112,207 @@ class CommandStatus(str, Enum): failed = "failed" +CommandT = typing.TypeVar('CommandT', bound=CommandDefinitionType) RequestDataT = typing.TypeVar('RequestDataT', bound=BaseModel) +ResponseDataT = typing.TypeVar('ResponseDataT') -class SessionCommandRequest(GenericModel, typing.Generic[RequestDataT]): +class SessionCommandRequest( + GenericModel, + typing.Generic[CommandT, RequestDataT, ResponseDataT] +): """A session command""" - command: CommandDefinitionType = Field( + 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]': + 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 + + +# The command definitions requiring no data and result types. +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 +] + + +class SimpleCommandRequest( + SessionCommandRequest[CommandsEmptyData, + EmptyModel, + EmptyModel]): + """A command containing no data and result type""" + pass + + +SimpleCommandResponse = SessionCommandResponse[ + CommandsEmptyData, + EmptyModel, + EmptyModel +] -class RobotCommandRequest(SessionCommandRequest[EmptyModel]): - command: Literal[ - RobotCommand.home_all_motors, - RobotCommand.home_pipette, - RobotCommand.toggle_lights - ] +LoadLabwareRequest = SessionCommandRequest[ + Literal[EquipmentCommand.load_labware], + LoadLabwareRequestData, + LoadLabwareResponseData +] -class ProtocolCommandRequest(SessionCommandRequest[EmptyModel]): - command: Literal[ - ProtocolCommand.start_run, - ProtocolCommand.start_simulate, - ProtocolCommand.cancel, - ProtocolCommand.pause, - ProtocolCommand.resume - ] +LoadLabwareResponse = SessionCommandResponse[ + Literal[EquipmentCommand.load_labware], + LoadLabwareRequestData, + LoadLabwareResponseData +] -class LoadLabwareRequest(SessionCommandRequest[LoadLabwareRequestData]): - command: Literal[EquipmentCommand.load_labware] +LoadInstrumentRequest = SessionCommandRequest[ + Literal[EquipmentCommand.load_instrument], + LoadInstrumentRequestData, + LoadInstrumentResponseData +] -class LoadInstrumentRequest(SessionCommandRequest[LoadInstrumentRequestData]): - command: Literal[EquipmentCommand.load_instrument] +LoadInstrumentResponse = SessionCommandResponse[ + Literal[EquipmentCommand.load_instrument], + LoadInstrumentRequestData, + LoadInstrumentResponseData +] -class LiquidRequest(SessionCommandRequest[LiquidRequestData]): - command: Literal[ - PipetteCommand.aspirate, - PipetteCommand.dispense - ] +LiquidRequest = SessionCommandRequest[ + Literal[PipetteCommand.aspirate, + PipetteCommand.dispense], + LiquidRequestData, + EmptyModel +] -class TipRequest(SessionCommandRequest[PipetteRequestDataBase]): - command: Literal[ - PipetteCommand.drop_tip, - PipetteCommand.pick_up_tip - ] +LiquidResponse = SessionCommandResponse[ + Literal[PipetteCommand.aspirate, + PipetteCommand.dispense], + LiquidRequestData, + EmptyModel +] -class CalibrationRequest(SessionCommandRequest[EmptyModel]): - 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, - ] +TipRequest = SessionCommandRequest[ + Literal[PipetteCommand.drop_tip, + PipetteCommand.pick_up_tip], + PipetteRequestDataBase, + EmptyModel +] -class JogRequest(SessionCommandRequest[JogPosition]): - command: Literal[CalibrationCommand.jog] +TipResponse = SessionCommandResponse[ + Literal[PipetteCommand.drop_tip, + PipetteCommand.pick_up_tip], + PipetteRequestDataBase, + EmptyModel +] -class SetHasCalibrationBlockRequestM( - SessionCommandRequest[SetHasCalibrationBlockRequestData] -): - command: Literal[CalibrationCommand.set_has_calibration_block] +JogRequest = SessionCommandRequest[ + Literal[CalibrationCommand.jog], + JogPosition, + EmptyModel +] -class DeckCalibrationCommandRequest(SessionCommandRequest[EmptyModel]): - command: Literal[ - DeckCalibrationCommand.move_to_point_two, - DeckCalibrationCommand.move_to_point_three - ] +JogResponse = SessionCommandResponse[ + Literal[CalibrationCommand.jog], + JogPosition, + EmptyModel +] -class CheckCalibrationCommandRequest(SessionCommandRequest[EmptyModel]): - 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, + SimpleCommandRequest, LoadLabwareRequest, LoadInstrumentRequest, LiquidRequest, TipRequest, - CalibrationRequest, JogRequest, - SetHasCalibrationBlockRequestM, - DeckCalibrationCommandRequest, - CheckCalibrationCommandRequest + SetHasCalibrationBlockRequest, +] + +ResponseTypes = typing.Union[ + SimpleCommandResponse, + LoadLabwareResponse, + LoadInstrumentResponse, + LiquidResponse, + TipResponse, + JogResponse, + SetHasCalibrationBlockResponse, ] # Session command requests/responses @@ -235,5 +320,5 @@ class SessionCommand(ResponseDataModel): RequestTypes ] CommandResponse = ResponseModel[ - SessionCommand + ResponseTypes ] 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 78d91121332..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,19 +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_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 dd15e86bb11..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__) @@ -54,9 +55,13 @@ async def execute(self, command: Command) -> CompletedCommand: 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) 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 38e1d76f749..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 @@ -6,7 +6,7 @@ from robot_server.service.session.command_execution.command import \ CommandResult from robot_server.service.session.models.command import LoadLabwareRequest, \ - LiquidRequest, TipRequest, RobotCommandRequest, LoadInstrumentRequest + 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 @@ -180,15 +180,15 @@ async def handle_command(c): # Mock out the command handler map command_executor._handler_map = { - command_definitions.RobotCommand.toggle_lights: handle_command} + 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( - request=RobotCommandRequest( - command=command_definitions.RobotCommand.toggle_lights, + request=SimpleCommandRequest( + command=command_definitions.ProtocolCommand.start_run, data=EmptyModel())) await state_store_command_executor.execute(c) @@ -197,8 +197,8 @@ 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(request=RobotCommandRequest( - command=command_definitions.RobotCommand.toggle_lights, + c = Command(request=SimpleCommandRequest( + command=command_definitions.ProtocolCommand.start_run, data=EmptyModel())) await state_store_command_executor.execute(c) 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 0c01fb61478..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 \ @@ -16,7 +16,7 @@ def test_handle_command_request(): store = StateStore() command = create_command( request=models.LoadLabwareRequest( - command=command_definitions.EquipmentCommand.load_labware, + command=cmddef.EquipmentCommand.load_labware, data=models.LoadLabwareRequestData( location=1, loadName="labware-load-name", @@ -36,7 +36,7 @@ def test_store_has_handle_command_response_method(): store = StateStore() command = create_command( request=models.LoadLabwareRequest( - command=command_definitions.EquipmentCommand.load_labware, + command=cmddef.EquipmentCommand.load_labware, data=models.LoadLabwareRequestData( location=1, loadName="labware-load-name", @@ -61,7 +61,7 @@ def test_store_has_handle_command_response_method(): argnames="command_request, handler", argvalues=[[ models.LoadLabwareRequest( - command=command_definitions.EquipmentCommand.load_labware, + command=cmddef.EquipmentCommand.load_labware, data=models.LoadLabwareRequestData( location=1, loadName="a", @@ -73,7 +73,7 @@ def test_store_has_handle_command_response_method(): "handle_load_labware"], [ models.LoadInstrumentRequest( - command=command_definitions.EquipmentCommand.load_instrument, + command=cmddef.EquipmentCommand.load_instrument, data=models.LoadInstrumentRequestData( instrumentName="p50_multi", mount=Mount.left, @@ -96,10 +96,9 @@ def test_command_result_state_handler(command_request, handler): def test_load_labware_update(): store = StateStore() - labware = command_definitions.EquipmentCommand.load_labware command = create_command( request=models.LoadLabwareRequest( - command=labware, + command=cmddef.EquipmentCommand.load_labware, data=models.LoadLabwareRequestData( location=1, loadName="labware-load-name", @@ -125,10 +124,9 @@ def test_load_labware_update(): def test_load_instrument_update(): store = StateStore() - instrument = command_definitions.EquipmentCommand.load_instrument command = create_command( request=models.LoadInstrumentRequest( - command=instrument, + command=cmddef.EquipmentCommand.load_instrument, data=models.LoadInstrumentRequestData( instrumentName='p10_single', mount=Mount.left) 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 612a419cc8f..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 @@ -4,7 +4,7 @@ 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 ProtocolCommandRequest +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 @@ -75,7 +75,7 @@ async def test_command_state_reject(loop, protocol_command_executor.current_state = current_state for protocol_command in ProtocolCommand: - command = Command(request=ProtocolCommandRequest( + command = Command(request=SimpleCommandRequest( command=protocol_command, data=EmptyModel()) ) @@ -101,7 +101,7 @@ 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(request=ProtocolCommandRequest( + protocol_command = Command(request=SimpleCommandRequest( command=command, data=EmptyModel()) ) diff --git a/robot-server/tests/service/session/test_router.py b/robot-server/tests/service/session/test_router.py index ed213850725..429d02c8561 100644 --- a/robot-server/tests/service/session/test_router.py +++ b/robot-server/tests/service/session/test_router.py @@ -14,7 +14,7 @@ from robot_server.service.session.errors import SessionCreationException, \ UnsupportedCommandException, CommandExecutionException from robot_server.service.session.models.command import JogRequest, \ - LoadLabwareRequest, CalibrationRequest + SimpleCommandRequest from robot_server.service.session.models.common import EmptyModel, JogPosition from robot_server.service.session.models.command_definitions import \ CalibrationCommand @@ -372,7 +372,7 @@ def test_execute_command_no_body(api_client, mock_command_executor.execute.assert_called_once_with( Command( - request=CalibrationRequest( + request=SimpleCommandRequest( command=CalibrationCommand.load_labware, data=EmptyModel()), meta=CommandMeta(command_id, command_created_at) From 75efde6021e6991657816648bde8bc66c6c3ebe2 Mon Sep 17 00:00:00 2001 From: amit lissack Date: Mon, 4 Jan 2021 08:59:08 -0500 Subject: [PATCH 4/6] remove unused types. --- .../service/session/models/command.py | 56 +++++++++---------- 1 file changed, 26 insertions(+), 30 deletions(-) diff --git a/robot-server/robot_server/service/session/models/command.py b/robot-server/robot_server/service/session/models/command.py index 8f0f828b617..f209dce4247 100644 --- a/robot-server/robot_server/service/session/models/command.py +++ b/robot-server/robot_server/service/session/models/command.py @@ -21,6 +21,8 @@ class LoadLabwareRequestData(BaseModel): + """Data field in an equipment.loadLabware command request.""" + location: int = Field( ..., description="Deck slot", ge=1, lt=12) @@ -39,12 +41,16 @@ class LoadLabwareRequestData(BaseModel): class LoadLabwareResponseData(BaseModel): + """Result field in an equipment.loadLabware command response.""" + labwareId: IdentifierType definition: LabwareDefinition calibration: OffsetVector class LoadInstrumentRequestData(BaseModel): + """Data field in an equipment.loadInstrument command request.""" + instrumentName: PipetteName = Field( ..., description="The name of the instrument model") @@ -52,6 +58,8 @@ class LoadInstrumentRequestData(BaseModel): class LoadInstrumentResponseData(BaseModel): + """Result field in an equipment.loadInstrument command response.""" + instrumentId: IdentifierType @@ -88,25 +96,8 @@ class SetHasCalibrationBlockRequestData(BaseModel): description="whether or not there is a calibration block present") -CommandDataType = typing.Union[ - SetHasCalibrationBlockRequestData, - JogPosition, - LiquidRequestData, - PipetteRequestDataBase, - LoadLabwareRequestData, - LoadInstrumentRequestData, - EmptyModel -] - -# A Union of all command result types -CommandResultType = typing.Union[ - LoadLabwareResponseData, - LoadInstrumentResponseData, -] - - class CommandStatus(str, Enum): - """The command status""" + """The command status.""" executed = "executed" queued = "queued" failed = "failed" @@ -121,7 +112,7 @@ class SessionCommandRequest( GenericModel, typing.Generic[CommandT, RequestDataT, ResponseDataT] ): - """A session command""" + """A session command request.""" command: CommandT = Field( ..., description="The command description") @@ -139,6 +130,7 @@ def make_response( 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, @@ -155,7 +147,7 @@ class SessionCommandResponse( GenericModel, typing.Generic[CommandT, RequestDataT, ResponseDataT] ): - """A session command response""" + """A session command response.""" command: CommandT data: RequestDataT status: CommandStatus @@ -165,7 +157,6 @@ class SessionCommandResponse( result: typing.Optional[ResponseDataT] = None -# The command definitions requiring no data and result types. CommandsEmptyData = Literal[ ProtocolCommand.start_run, ProtocolCommand.start_simulate, @@ -190,14 +181,14 @@ class SessionCommandResponse( CheckCalibrationCommand.return_tip, CheckCalibrationCommand.transition ] +"""The command definitions requiring no data and result types.""" -class SimpleCommandRequest( - SessionCommandRequest[CommandsEmptyData, - EmptyModel, - EmptyModel]): - """A command containing no data and result type""" - pass +SimpleCommandRequest = SessionCommandRequest[ + CommandsEmptyData, + EmptyModel, + EmptyModel] +"""A command request containing no data and result type.""" SimpleCommandResponse = SessionCommandResponse[ @@ -205,6 +196,7 @@ class SimpleCommandRequest( EmptyModel, EmptyModel ] +"""Response to :class:`~SimpleCommandRequest`""" LoadLabwareRequest = SessionCommandRequest[ @@ -289,8 +281,7 @@ class SimpleCommandRequest( SetHasCalibrationBlockResponse = SessionCommandResponse[ - Literal[ - CalibrationCommand.set_has_calibration_block], + Literal[CalibrationCommand.set_has_calibration_block], SetHasCalibrationBlockRequestData, EmptyModel] @@ -304,6 +295,7 @@ class SimpleCommandRequest( JogRequest, SetHasCalibrationBlockRequest, ] +"""Union of all request types""" ResponseTypes = typing.Union[ SimpleCommandResponse, @@ -314,11 +306,15 @@ class SimpleCommandRequest( JogResponse, SetHasCalibrationBlockResponse, ] +"""Union of all response types""" + -# Session command requests/responses CommandRequest = RequestModel[ RequestTypes ] +"""The command request model.""" + CommandResponse = ResponseModel[ ResponseTypes ] +"""The command response model.""" From 0426993af9df1a6527c747c3b5bd8e60f3b9a9db Mon Sep 17 00:00:00 2001 From: amit lissack Date: Mon, 4 Jan 2021 11:21:21 -0500 Subject: [PATCH 5/6] comments --- .../service/session/models/command.py | 18 ++++++++++++++++++ .../session/models/command_definitions.py | 12 +++++++++--- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/robot-server/robot_server/service/session/models/command.py b/robot-server/robot_server/service/session/models/command.py index f209dce4247..028942613a8 100644 --- a/robot-server/robot_server/service/session/models/command.py +++ b/robot-server/robot_server/service/session/models/command.py @@ -1,3 +1,21 @@ +"""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 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 From acbb1aa4164304ba3318a583acad140fa03a3c02 Mon Sep 17 00:00:00 2001 From: amit lissack Date: Mon, 4 Jan 2021 11:43:29 -0500 Subject: [PATCH 6/6] command model tests. --- .../tests/service/session/models/__init__.py | 0 .../service/session/models/test_command.py | 112 ++++++++++++++++++ 2 files changed, 112 insertions(+) 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/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": {} + } + })