From 7aa9034ac6216a3a1e30203d9dff73e8200e9ea3 Mon Sep 17 00:00:00 2001 From: amitlissack Date: Tue, 4 May 2021 13:04:41 -0400 Subject: [PATCH] feat(api): JSON protocol pipette loading support in Protocol Engine (#7766) * LoadPipetteRequest has an optional pipetteId which will override the id generation. * support pipetteId arg for LoadPipetteRequest. * SyncClient.load_pipette does not need pipette_id argument. closes #7430 --- .../protocol_engine/commands/load_pipette.py | 9 +++++++ .../protocol_engine/execution/equipment.py | 18 ++++++++++--- .../commands/test_load_pipette.py | 3 +++ .../execution/test_equipment_handler.py | 20 ++++++++++++++ .../sessions/test_live_protocol.tavern.yaml | 26 +++++++++++++++++++ .../service/session/models/test_command.py | 7 ++--- 6 files changed, 77 insertions(+), 6 deletions(-) diff --git a/api/src/opentrons/protocol_engine/commands/load_pipette.py b/api/src/opentrons/protocol_engine/commands/load_pipette.py index a85439ae269..ddef913759b 100644 --- a/api/src/opentrons/protocol_engine/commands/load_pipette.py +++ b/api/src/opentrons/protocol_engine/commands/load_pipette.py @@ -1,5 +1,8 @@ """Load pipette command request, result, and implementation models.""" from __future__ import annotations + +from typing import Optional + from pydantic import BaseModel, Field from opentrons.types import MountType @@ -19,6 +22,11 @@ class LoadPipetteRequest(BaseModel): ..., description="The mount the pipette should be present on.", ) + pipetteId: Optional[str] = Field( + None, + description="An optional ID to assign to this pipette. If None, an ID " + "will be generated." + ) def get_implementation(self) -> LoadPipetteImplementation: """Get the load pipette request's command implementation.""" @@ -44,6 +52,7 @@ async def execute(self, handlers: CommandHandlers) -> LoadPipetteResult: loaded_pipette = await handlers.equipment.load_pipette( pipette_name=self._request.pipetteName, mount=self._request.mount, + pipette_id=self._request.pipetteId ) return LoadPipetteResult(pipetteId=loaded_pipette.pipette_id) diff --git a/api/src/opentrons/protocol_engine/execution/equipment.py b/api/src/opentrons/protocol_engine/execution/equipment.py index 5856abdc97f..8d788f4c67c 100644 --- a/api/src/opentrons/protocol_engine/execution/equipment.py +++ b/api/src/opentrons/protocol_engine/execution/equipment.py @@ -1,6 +1,6 @@ """Equipment command side-effect logic.""" from dataclasses import dataclass -from typing import Tuple +from typing import Tuple, Optional from opentrons_shared_data.labware.dev_types import LabwareDefinition from opentrons.types import MountType @@ -77,8 +77,19 @@ async def load_pipette( self, pipette_name: PipetteName, mount: MountType, + pipette_id: Optional[str], ) -> LoadedPipette: - """Ensure the requested pipette is attached.""" + """Ensure the requested pipette is attached. + + Args: + pipette_name: The pipette name. + mount: The mount on which pipette must be attached. + pipette_id: An optional identifier to assign the pipette. If None, an + identifier will be generated. + + Returns: + A LoadedPipette object. + """ other_mount = mount.other_mount() other_pipette = self._state.pipettes.get_pipette_data_by_mount( other_mount, @@ -97,6 +108,7 @@ async def load_pipette( except RuntimeError as e: raise FailedToLoadPipetteError(str(e)) from e - pipette_id = self._resources.id_generator.generate_id() + pipette_id = pipette_id if pipette_id is not None else \ + self._resources.id_generator.generate_id() return LoadedPipette(pipette_id=pipette_id) diff --git a/api/tests/opentrons/protocol_engine/commands/test_load_pipette.py b/api/tests/opentrons/protocol_engine/commands/test_load_pipette.py index db2d66fe3f6..388c7ecc841 100644 --- a/api/tests/opentrons/protocol_engine/commands/test_load_pipette.py +++ b/api/tests/opentrons/protocol_engine/commands/test_load_pipette.py @@ -19,6 +19,7 @@ def test_load_pipette_request() -> None: assert request.pipetteName == "p300_single" assert request.mount == MountType.LEFT + assert request.pipetteId is None def test_load_pipette_result() -> None: @@ -37,6 +38,7 @@ async def test_load_pipette_implementation(mock_handlers: AsyncMock) -> None: request = LoadPipetteRequest( pipetteName=PipetteName.P300_SINGLE, mount=MountType.LEFT, + pipetteId="some id" ) impl = request.get_implementation() result = await impl.execute(mock_handlers) @@ -45,4 +47,5 @@ async def test_load_pipette_implementation(mock_handlers: AsyncMock) -> None: mock_handlers.equipment.load_pipette.assert_called_with( pipette_name="p300_single", mount=MountType.LEFT, + pipette_id="some id", ) diff --git a/api/tests/opentrons/protocol_engine/execution/test_equipment_handler.py b/api/tests/opentrons/protocol_engine/execution/test_equipment_handler.py index 49558cd5cf7..feb38b1b460 100644 --- a/api/tests/opentrons/protocol_engine/execution/test_equipment_handler.py +++ b/api/tests/opentrons/protocol_engine/execution/test_equipment_handler.py @@ -110,12 +110,29 @@ async def test_load_pipette_assigns_id( res = await handler.load_pipette( pipette_name=PipetteName.P300_SINGLE, mount=MountType.LEFT, + pipette_id=None, ) assert type(res) == LoadedPipette assert res.pipette_id == "unique-id" +async def test_load_pipette_uses_provided_id( + mock_resources_with_data: AsyncMock, + handler: EquipmentHandler, +) -> None: + """It should use the provided ID rather than generating an ID for the pipette.""" + res = await handler.load_pipette( + pipette_name=PipetteName.P300_SINGLE, + mount=MountType.LEFT, + pipette_id="my pipette id" + ) + + assert type(res) == LoadedPipette + assert res.pipette_id == "my pipette id" + mock_resources_with_data.id_generator.generate_id.assert_not_called() + + async def test_load_pipette_checks_checks_existence( mock_state_view: MagicMock, mock_hardware: AsyncMock, @@ -126,6 +143,7 @@ async def test_load_pipette_checks_checks_existence( await handler.load_pipette( pipette_name=PipetteName.P300_SINGLE, mount=MountType.LEFT, + pipette_id=None, ) mock_state_view.pipettes.get_pipette_data_by_mount.assert_called_with( @@ -151,6 +169,7 @@ async def test_load_pipette_checks_checks_existence_with_already_loaded( await handler.load_pipette( pipette_name=PipetteName.P300_SINGLE, mount=MountType.RIGHT, + pipette_id=None, ) mock_state_view.pipettes.get_pipette_data_by_mount.assert_called_with( @@ -181,4 +200,5 @@ async def test_load_pipette_raises_if_pipette_not_attached( await handler.load_pipette( pipette_name=PipetteName.P300_SINGLE, mount=MountType.LEFT, + pipette_id=None, ) diff --git a/robot-server/tests/integration/sessions/test_live_protocol.tavern.yaml b/robot-server/tests/integration/sessions/test_live_protocol.tavern.yaml index 555d0c1ddfb..d1160b03ec8 100644 --- a/robot-server/tests/integration/sessions/test_live_protocol.tavern.yaml +++ b/robot-server/tests/integration/sessions/test_live_protocol.tavern.yaml @@ -86,6 +86,7 @@ stages: data: &create_pipette_data pipetteName: p300_single mount: right + pipetteId: null response: status_code: 200 json: @@ -100,6 +101,31 @@ stages: completedAt: !anystr result: pipetteId: !anystr + - name: Load pipette command specifying the id + request: + url: "{host:s}:{port:d}/sessions/{session_id}/commands/execute" + method: POST + json: + data: + command: equipment.loadPipette + data: &create_pipette_data_with_id + pipetteName: p10_single + mount: left + pipetteId: my pipette + response: + status_code: 200 + json: + links: !anydict + data: + id: !anystr + data: *create_pipette_data_with_id + command: equipment.loadPipette + status: executed + createdAt: !anystr + startedAt: !anystr + completedAt: !anystr + result: + pipetteId: my pipette - name: Delete the session request: url: "{host:s}:{port:d}/sessions/{session_id}" diff --git a/robot-server/tests/service/session/models/test_command.py b/robot-server/tests/service/session/models/test_command.py index 04d5ce613cc..f5515d85b4c 100644 --- a/robot-server/tests/service/session/models/test_command.py +++ b/robot-server/tests/service/session/models/test_command.py @@ -53,7 +53,8 @@ def test_not_empty(): "command": "equipment.loadPipette", "data": { "pipetteName": "p10_single", - "mount": "left" + "mount": "left", + "pipetteId": None, } } }) @@ -61,7 +62,7 @@ def test_not_empty(): command_definitions.EquipmentCommand.load_pipette assert request.data.data == pe_commands.LoadPipetteRequest( pipetteName="p10_single", - mount=MountType.LEFT + mount=MountType.LEFT, ) dt = datetime(2000, 1, 1) @@ -81,7 +82,7 @@ def test_not_empty(): command_definitions.EquipmentCommand.load_pipette assert response.data == pe_commands.LoadPipetteRequest( pipetteName="p10_single", - mount=MountType.LEFT + mount=MountType.LEFT, ) assert response.id == "id" assert response.createdAt == dt