Skip to content

Commit

Permalink
refactor: remove all args but id
Browse files Browse the repository at this point in the history
  • Loading branch information
sfoster1 committed Apr 24, 2024
1 parent 895c84c commit 28a2d16
Show file tree
Hide file tree
Showing 11 changed files with 35 additions and 404 deletions.
8 changes: 0 additions & 8 deletions api/src/opentrons/protocol_engine/clients/sync_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,19 +130,11 @@ def load_labware(
def reload_labware(
self,
labware_id: str,
load_name: str,
namespace: str,
version: int,
display_name: Optional[str] = None,
) -> commands.ReloadLabwareResult:
"""Execute a ReloadLabware command and return the result."""
request = commands.ReloadLabwareCreate(
params=commands.ReloadLabwareParams(
labwareId=labware_id,
loadName=load_name,
namespace=namespace,
version=version,
displayName=display_name,
)
)
result = self._transport.execute_command(request=request)
Expand Down
44 changes: 0 additions & 44 deletions api/src/opentrons/protocol_engine/commands/reload_labware.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,23 +29,6 @@ class ReloadLabwareParams(BaseModel):
labwareId: str = Field(
..., description="The already-loaded labware instance to update."
)
loadName: str = Field(
...,
description="Name used to reference a labware definition.",
)
namespace: str = Field(
...,
description="The namespace the labware definition belongs to.",
)
version: int = Field(
...,
description="The labware definition version.",
)
displayName: Optional[str] = Field(
None,
description="An optional user-specified display name "
"or label for this labware.",
)


class ReloadLabwareResult(BaseModel):
Expand All @@ -55,10 +38,6 @@ class ReloadLabwareResult(BaseModel):
...,
description="An ID to reference this labware in subsequent commands. Same as the one in the parameters.",
)
definition: LabwareDefinition = Field(
...,
description="The full definition data for this labware.",
)
offsetId: Optional[str] = Field(
# Default `None` instead of `...` so this field shows up as non-required in
# OpenAPI. The server is allowed to omit it or make it null.
Expand Down Expand Up @@ -89,33 +68,10 @@ async def execute(self, params: ReloadLabwareParams) -> ReloadLabwareResult:
"""Reload the definition and calibration data for a specific labware."""
reloaded_labware = await self._equipment.reload_labware(
labware_id=params.labwareId,
load_name=params.loadName,
namespace=params.namespace,
version=params.version,
)

# note: this check must be kept because somebody might specify the trash loadName
if (
labware_validation.is_flex_trash(params.loadName)
and isinstance(reloaded_labware.location, DeckSlotLocation)
and self._state_view.geometry.get_slot_column(
reloaded_labware.location.slotName
)
!= 3
):
raise LabwareIsNotAllowedInLocationError(
f"{params.loadName} is not allowed in slot {reloaded_labware.location.slotName}"
)

if isinstance(reloaded_labware.location, OnLabwareLocation):
self._state_view.labware.raise_if_labware_cannot_be_stacked(
top_labware_definition=reloaded_labware.definition,
bottom_labware_id=reloaded_labware.location.labwareId,
)

return ReloadLabwareResult(
labwareId=params.labwareId,
definition=reloaded_labware.definition,
offsetId=reloaded_labware.offsetId,
)

Expand Down
31 changes: 4 additions & 27 deletions api/src/opentrons/protocol_engine/execution/equipment.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ class LoadedLabwareData:
class ReloadedLabwareData:
"""The result of a reload labware procedure."""

definition: LabwareDefinition
location: LabwareLocation
offsetId: Optional[str]

Expand Down Expand Up @@ -180,47 +179,25 @@ async def load_labware(
labware_id=labware_id, definition=definition, offsetId=offset_id
)

async def reload_labware(
self, labware_id: str, load_name: str, namespace: str, version: int
) -> ReloadedLabwareData:
async def reload_labware(self, labware_id: str) -> ReloadedLabwareData:
"""Reload an already-loaded labware. This cannot change the labware location.
Args:
labware_id: The ID of the already-loaded labware.
load_name: The labware's load name.
namespace: The labware's namespace.
version: The labware's version.
Raises:
LabwareNotLoadedError: If `labware_id` does not reference a loaded labware.
"""
definition_uri = uri_from_details(
load_name=load_name,
namespace=namespace,
version=version,
)
location = self._state_store.labware.get_location(labware_id)

try:
# Try to use existing definition in state.
definition = self._state_store.labware.get_definition_by_uri(definition_uri)
except LabwareDefinitionDoesNotExistError:
definition = await self._labware_data_provider.get_labware_definition(
load_name=load_name,
namespace=namespace,
version=version,
)

# Allow propagation of ModuleNotLoadedError.
location = self._state_store.labware.get_location(labware_id)
definition_uri = self._state_store.labware.get_definition_uri(labware_id)
offset_id = self.find_applicable_labware_offset_id(
labware_definition_uri=definition_uri,
labware_location=location,
)

return ReloadedLabwareData(
definition=definition, location=location, offsetId=offset_id
)
return ReloadedLabwareData(location=location, offsetId=offset_id)

async def load_pipette(
self,
Expand Down
7 changes: 6 additions & 1 deletion api/src/opentrons/protocol_engine/state/labware.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ def handle_action(self, action: Action) -> None:

def _handle_command(self, command: Command) -> None:
"""Modify state in reaction to a command."""
if isinstance(command.result, (LoadLabwareResult, ReloadLabwareResult)):
if isinstance(command.result, LoadLabwareResult):
# If the labware load refers to an offset, that offset must actually exist.
if command.result.offsetId is not None:
assert command.result.offsetId in self._state.labware_offsets_by_id
Expand Down Expand Up @@ -204,6 +204,11 @@ def _handle_command(self, command: Command) -> None:
displayName=command.params.displayName,
)

elif isinstance(command.result, ReloadLabwareResult):
labware_id = command.params.labwareId
new_offset_id = command.result.offsetId
self._state.labware_by_id[labware_id].offsetId = new_offset_id

elif isinstance(command.result, MoveLabwareResult):
labware_id = command.params.labwareId
new_location = command.params.newLocation
Expand Down
41 changes: 10 additions & 31 deletions api/src/opentrons/protocol_engine/state/tips.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,6 @@
from enum import Enum
from typing import Dict, Optional, List, Union

from opentrons_shared_data.labware.labware_definition import LabwareDefinition

from opentrons.hardware_control.nozzle_manager import NozzleMap

from .abstract_store import HasState, HandlesActions
from ..actions import (
Action,
Expand All @@ -17,7 +13,6 @@
from ..commands import (
Command,
LoadLabwareResult,
ReloadLabwareResult,
PickUpTip,
PickUpTipResult,
DropTipResult,
Expand All @@ -29,6 +24,8 @@
)
from ..error_recovery_policy import ErrorRecoveryType

from opentrons.hardware_control.nozzle_manager import NozzleMap


class TipRackWellState(Enum):
"""The state of a single tip in a tip rack's well."""
Expand Down Expand Up @@ -103,24 +100,21 @@ def handle_action(self, action: Action) -> None:
well_name
] = TipRackWellState.CLEAN

def _add_new_tiprack(self, labware_id: str, definition: LabwareDefinition) -> None:
self._state.tips_by_labware_id[labware_id] = {
well_name: TipRackWellState.CLEAN
for column in definition.ordering
for well_name in column
}
self._state.column_by_labware_id[labware_id] = [
column for column in definition.ordering
]

def _handle_succeeded_command(self, command: Command) -> None:
if (
isinstance(command.result, LoadLabwareResult)
and command.result.definition.parameters.isTiprack
):
labware_id = command.result.labwareId
definition = command.result.definition
self._add_new_tiprack(labware_id, definition)
self._state.tips_by_labware_id[labware_id] = {
well_name: TipRackWellState.CLEAN
for column in definition.ordering
for well_name in column
}
self._state.column_by_labware_id[labware_id] = [
column for column in definition.ordering
]

elif isinstance(command.result, PickUpTipResult):
labware_id = command.params.labwareId
Expand All @@ -136,21 +130,6 @@ def _handle_succeeded_command(self, command: Command) -> None:
pipette_id = command.params.pipetteId
self._state.length_by_pipette_id.pop(pipette_id, None)

elif isinstance(command.result, ReloadLabwareResult):
if (
command.result.definition.parameters.isTiprack
and command.result.labwareId not in self._state.tips_by_labware_id
):
self._add_new_tiprack(
command.result.labwareId, command.result.definition
)
elif (
not command.result.definition.parameters.isTiprack
and command.result.labwareId in self._state.tips_by_labware_id
):
self._state.tips_by_labware_id.pop(command.result.labwareId)
self._state.column_by_labware_id.pop(command.result.labwareId)

def _handle_failed_command(
self,
action: FailCommandAction,
Expand Down
11 changes: 1 addition & 10 deletions api/tests/opentrons/protocol_engine/clients/test_sync_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,32 +164,23 @@ def test_load_labware(
def test_reload_labware(
decoy: Decoy,
transport: ChildThreadTransport,
tip_rack_def: LabwareDefinition,
subject: SyncClient,
) -> None:
"""It should execute a reload labware command."""
expected_request = commands.ReloadLabwareCreate(
params=commands.ReloadLabwareParams(
labwareId="some-labware-id",
loadName="some_labware",
namespace="opentrons",
version=1,
displayName="some_display_name",
)
)

expected_result = commands.ReloadLabwareResult(
labwareId="some-labware-id", definition=tip_rack_def, offsetId=None
labwareId="some-labware-id", offsetId=None
)
decoy.when(transport.execute_command(request=expected_request)).then_return(
expected_result
)
result = subject.reload_labware(
labware_id="some-labware-id",
namespace="opentrons",
load_name="some_labware",
version=1,
display_name="some_display_name",
)
assert result == expected_result

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,23 +46,11 @@ async def test_reload_labware_implementation(

data = ReloadLabwareParams(
labwareId="my-labware-id",
loadName="some-load-name",
namespace="opentrons-test",
version=1,
displayName="My custom display name",
)

decoy.when(
await equipment.reload_labware(
labware_id="my-labware-id",
load_name="some-load-name",
namespace="opentrons-test",
version=1,
)
).then_return(
decoy.when(await equipment.reload_labware(labware_id="my-labware-id",)).then_return(
ReloadedLabwareData(
location=DeckSlotLocation(slotName=DeckSlotName.SLOT_4),
definition=well_plate_def,
offsetId="labware-offset-id",
)
)
Expand All @@ -71,7 +59,6 @@ async def test_reload_labware_implementation(

assert result == ReloadLabwareResult(
labwareId="my-labware-id",
definition=well_plate_def,
offsetId="labware-offset-id",
)

Expand All @@ -87,55 +74,13 @@ async def test_reload_labware_raises_labware_does_not_exist(

data = ReloadLabwareParams(
labwareId="my-labware-id",
loadName="some-load-name",
namespace="opentrons-test",
version=1,
displayName="My custom display name",
)

decoy.when(
await equipment.reload_labware(
labware_id="my-labware-id",
load_name="some-load-name",
namespace="opentrons-test",
version=1,
)
).then_raise(LabwareNotLoadedError("What labware is this!"))

with pytest.raises(LabwareNotLoadedError):
await subject.execute(data)


async def test_load_labware_raises_location_not_allowed(
decoy: Decoy,
equipment: EquipmentHandler,
state_view: StateView,
well_plate_def: LabwareDefinition,
) -> None:
"""A ReloadLabware command should raise if the flex trash definition is not in a valid slot."""
subject = ReloadLabwareImplementation(equipment=equipment, state_view=state_view)
decoy.when(labware_validation.is_flex_trash("some-load-name")).then_return(True)
decoy.when(
await equipment.reload_labware(
labware_id="my-labware-id",
load_name="some-load-name",
namespace="opentrons-test",
version=1,
)
).then_return(
ReloadedLabwareData(
location=DeckSlotLocation(slotName=DeckSlotName.SLOT_A3),
definition=well_plate_def,
offsetId="labware-offset-id",
)
)
data = ReloadLabwareParams(
labwareId="my-labware-id",
loadName="some-load-name",
namespace="opentrons-test",
version=1,
displayName="My custom display name",
)

with pytest.raises(LabwareIsNotAllowedInLocationError):
await subject.execute(data)
7 changes: 0 additions & 7 deletions api/tests/opentrons/protocol_engine/state/command_fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -581,22 +581,15 @@ def create_prepare_to_aspirate_command(pipette_id: str) -> cmd.PrepareToAspirate

def create_reload_labware_command(
labware_id: str,
definition: LabwareDefinition,
offset_id: Optional[str],
display_name: Optional[str],
) -> cmd.ReloadLabware:
"""Create a completed ReloadLabware command."""
params = cmd.ReloadLabwareParams(
loadName=definition.parameters.loadName,
namespace=definition.namespace,
version=definition.version,
labwareId=labware_id,
displayName=display_name,
)

result = cmd.ReloadLabwareResult(
labwareId=labware_id,
definition=definition,
offsetId=offset_id,
)

Expand Down
Loading

0 comments on commit 28a2d16

Please sign in to comment.