Skip to content

Commit

Permalink
fix(api): add PAPI loaded trashes to engine on load statement (#14358)
Browse files Browse the repository at this point in the history
  • Loading branch information
jbleon95 authored Jan 29, 2024
1 parent 682c57a commit 7e9844d
Show file tree
Hide file tree
Showing 11 changed files with 134 additions and 16 deletions.
10 changes: 10 additions & 0 deletions api/src/opentrons/protocol_api/_trash_bin.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,13 @@ def location(self) -> DeckSlotName:
This is intended for Opentrons internal use only and is not a guarenteed API.
"""
return self._location

@property
def area_name(self) -> str:
"""Addressable area name of the trash bin.
:meta private:
This is intended for Opentrons internal use only and is not a guarenteed API.
"""
return self._addressable_area_name
17 changes: 17 additions & 0 deletions api/src/opentrons/protocol_api/core/engine/protocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,9 @@ def append_disposal_location(
) -> None:
"""Append a disposal location object to the core"""
if isinstance(disposal_location, TrashBin):
self._engine_client.state.addressable_areas.raise_if_area_not_in_deck_configuration(
disposal_location.area_name
)
deck_conflict.check(
engine_state=self._engine_client.state,
new_trash_bin=disposal_location,
Expand All @@ -147,6 +150,20 @@ def append_disposal_location(
existing_labware_ids=list(self._labware_cores_by_id.keys()),
existing_module_ids=list(self._module_cores_by_id.keys()),
)
self._engine_client.add_addressable_area(disposal_location.area_name)
elif isinstance(disposal_location, WasteChute):
# TODO(jbl 2024-01-25) hardcoding this specific addressable area should be refactored
# when analysis is fixed up
#
# We want to tell Protocol Engine that there's a waste chute in the waste chute location when it's loaded,
# so analysis can prevent the user from doing anything that would collide with it. At the same time, we
# do not want to create a false negative when it comes to addressable area conflict. We therefore use the
# addressable area `1ChannelWasteChute` because every waste chute cutout fixture provides it and it will
# provide the engine with the information it needs.
self._engine_client.state.addressable_areas.raise_if_area_not_in_deck_configuration(
"1ChannelWasteChute"
)
self._engine_client.add_addressable_area("1ChannelWasteChute")
self._disposal_locations.append(disposal_location)

def get_disposal_locations(self) -> List[Union[Labware, TrashBin, WasteChute]]:
Expand Down
2 changes: 2 additions & 0 deletions api/src/opentrons/protocol_engine/actions/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
AddLabwareOffsetAction,
AddLabwareDefinitionAction,
AddLiquidAction,
AddAddressableAreaAction,
AddModuleAction,
FinishErrorDetails,
DoorChangeAction,
Expand All @@ -44,6 +45,7 @@
"AddLabwareOffsetAction",
"AddLabwareDefinitionAction",
"AddLiquidAction",
"AddAddressableAreaAction",
"AddModuleAction",
"DoorChangeAction",
"ResetTipsAction",
Expand Down
21 changes: 20 additions & 1 deletion api/src/opentrons/protocol_engine/actions/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,13 @@
from opentrons_shared_data.errors import EnumeratedError

from ..commands import Command, CommandCreate, CommandPrivateResult
from ..types import LabwareOffsetCreate, ModuleDefinition, Liquid, DeckConfigurationType
from ..types import (
LabwareOffsetCreate,
ModuleDefinition,
Liquid,
DeckConfigurationType,
AddressableAreaLocation,
)


@dataclass(frozen=True)
Expand Down Expand Up @@ -153,6 +159,18 @@ class AddLiquidAction:
liquid: Liquid


@dataclass(frozen=True)
class AddAddressableAreaAction:
"""Add a single addressable area to state.
This differs from the deck configuration in PlayAction which sends over a mapping of cutout fixtures.
This action will only load one addressable area and that should be pre-validated before being sent via
the action.
"""

addressable_area: AddressableAreaLocation


@dataclass(frozen=True)
class AddModuleAction:
"""Add an attached module directly to state without a location."""
Expand Down Expand Up @@ -194,6 +212,7 @@ class SetPipetteMovementSpeedAction:
AddLabwareOffsetAction,
AddLabwareDefinitionAction,
AddModuleAction,
AddAddressableAreaAction,
AddLiquidAction,
ResetTipsAction,
SetPipetteMovementSpeedAction,
Expand Down
6 changes: 6 additions & 0 deletions api/src/opentrons/protocol_engine/clients/sync_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,12 @@ def add_labware_definition(self, definition: LabwareDefinition) -> LabwareUri:
definition=definition,
)

def add_addressable_area(self, addressable_area_name: str) -> None:
"""Add an addressable area to the engine's state."""
self._transport.call_method(
"add_addressable_area", addressable_area_name=addressable_area_name
)

def add_liquid(
self, name: str, color: Optional[str], description: Optional[str]
) -> Liquid:
Expand Down
9 changes: 9 additions & 0 deletions api/src/opentrons/protocol_engine/protocol_engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
HexColor,
PostRunHardwareState,
DeckConfigurationType,
AddressableAreaLocation,
)
from .execution import (
QueueWorker,
Expand All @@ -46,6 +47,7 @@
AddLabwareOffsetAction,
AddLabwareDefinitionAction,
AddLiquidAction,
AddAddressableAreaAction,
AddModuleAction,
HardwareStoppedAction,
ResetTipsAction,
Expand Down Expand Up @@ -481,6 +483,13 @@ def add_liquid(
self._action_dispatcher.dispatch(AddLiquidAction(liquid=liquid))
return liquid

def add_addressable_area(self, addressable_area_name: str) -> None:
"""Add an addressable area to state."""
area = AddressableAreaLocation(addressableAreaName=addressable_area_name)
self._action_dispatcher.dispatch(
AddAddressableAreaAction(addressable_area=area)
)

def reset_tips(self, labware_id: str) -> None:
"""Reset the tip state of a given labware."""
# TODO(mm, 2023-03-10): Safely raise an error if the given labware isn't a
Expand Down
6 changes: 4 additions & 2 deletions api/src/opentrons/protocol_engine/state/addressable_areas.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
PotentialCutoutFixture,
DeckConfigurationType,
)
from ..actions import Action, UpdateCommandAction, PlayAction
from ..actions import Action, UpdateCommandAction, PlayAction, AddAddressableAreaAction
from .config import Config
from .abstract_store import HasState, HandlesActions

Expand Down Expand Up @@ -175,7 +175,9 @@ def handle_action(self, action: Action) -> None:
"""Modify state in reaction to an action."""
if isinstance(action, UpdateCommandAction):
self._handle_command(action.command)
if isinstance(action, PlayAction):
elif isinstance(action, AddAddressableAreaAction):
self._check_location_is_addressable_area(action.addressable_area)
elif isinstance(action, PlayAction):
current_state = self._state
if (
action.deck_configuration is not None
Expand Down
22 changes: 10 additions & 12 deletions api/tests/opentrons/protocol_api_integration/test_trashes.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@


from opentrons import protocol_api, simulate
from opentrons.protocols.api_support.types import APIVersion

import contextlib
from typing import ContextManager, Optional, Type
Expand Down Expand Up @@ -122,16 +123,6 @@ def test_trash_search() -> None:
"2.16",
"OT-2",
False,
# This should ideally raise, matching OT-2 behavior on prior Protocol API versions.
# It currently does not because Protocol API v2.15's trashes are implemented as
# addressable areas, not labware--and they're only brought into existence
# *on first use,* not at the beginning of a protocol.
#
# The good news is that even though the conflicting load will not raise like we want,
# something in the protocol will eventually raise, e.g. when a pipette goes to drop a
# tip in the fixed trash and finds that a fixed trash can't exist there because there's
# a labware.
marks=pytest.mark.xfail(strict=True, raises=pytest.fail.Exception),
),
pytest.param(
"2.16",
Expand All @@ -156,10 +147,17 @@ def test_fixed_trash_load_conflicts(
if expect_load_to_succeed:
expected_error: ContextManager[object] = contextlib.nullcontext()
else:
# If we're expecting an error, it'll be a LocationIsOccupied for 2.15 and below, otherwise
# it will fail with an IncompatibleAddressableAreaError, since slot 12 will not be in the deck config
if APIVersion.from_string(version) < APIVersion(2, 16):
error_name = "LocationIsOccupiedError"
else:
error_name = "IncompatibleAddressableAreaError"

expected_error = pytest.raises(
Exception,
# Exact message doesn't matter, as long as it's definitely a labware load conflict.
match="LocationIsOccupiedError",
# Exact message doesn't matter, as long as it's definitely a labware load or addressable area conflict.
match=error_name,
)

with expected_error:
Expand Down
17 changes: 17 additions & 0 deletions api/tests/opentrons/protocol_engine/clients/test_sync_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,23 @@ def test_add_labware_definition(
assert result == expected_labware_uri


def test_add_addressable_area(
decoy: Decoy,
transport: ChildThreadTransport,
subject: SyncClient,
) -> None:
"""It should add an addressable area."""
subject.add_addressable_area(addressable_area_name="cool-area")

decoy.verify(
transport.call_method(
"add_addressable_area",
addressable_area_name="cool-area",
),
times=1,
)


def test_add_liquid(
decoy: Decoy,
transport: ChildThreadTransport,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@
from opentrons.types import DeckSlotName

from opentrons.protocol_engine.commands import Command
from opentrons.protocol_engine.actions import UpdateCommandAction
from opentrons.protocol_engine.actions import (
UpdateCommandAction,
AddAddressableAreaAction,
)
from opentrons.protocol_engine.state import Config
from opentrons.protocol_engine.state.addressable_areas import (
AddressableAreaStore,
Expand Down Expand Up @@ -243,3 +246,17 @@ def test_addressable_area_referencing_commands_load(
"""It should check that the addressable area is in the deck config."""
subject.handle_action(UpdateCommandAction(private_result=None, command=command))
assert expected_area in subject.state.loaded_addressable_areas_by_name


def test_add_addressable_area_action(
simulated_subject: AddressableAreaStore,
) -> None:
"""It should add the addressable area to the store."""
simulated_subject.handle_action(
AddAddressableAreaAction(
addressable_area=AddressableAreaLocation(
addressableAreaName="movableTrashA1"
)
)
)
assert "movableTrashA1" in simulated_subject.state.loaded_addressable_areas_by_name
21 changes: 21 additions & 0 deletions api/tests/opentrons/protocol_engine/test_protocol_engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
ModuleModel,
Liquid,
PostRunHardwareState,
AddressableAreaLocation,
)
from opentrons.protocol_engine.execution import (
QueueWorker,
Expand All @@ -43,6 +44,7 @@
ActionDispatcher,
AddLabwareOffsetAction,
AddLabwareDefinitionAction,
AddAddressableAreaAction,
AddLiquidAction,
AddModuleAction,
PlayAction,
Expand Down Expand Up @@ -921,6 +923,25 @@ def _stub_get_definition_uri(*args: Any, **kwargs: Any) -> None:
assert result == "some/definition/uri"


def test_add_addressable_area(
decoy: Decoy,
action_dispatcher: ActionDispatcher,
subject: ProtocolEngine,
) -> None:
"""It should dispatch an AddAddressableArea action."""
subject.add_addressable_area(addressable_area_name="my_funky_area")

decoy.verify(
action_dispatcher.dispatch(
AddAddressableAreaAction(
addressable_area=AddressableAreaLocation(
addressableAreaName="my_funky_area"
)
)
)
)


def test_add_liquid(
decoy: Decoy,
action_dispatcher: ActionDispatcher,
Expand Down

0 comments on commit 7e9844d

Please sign in to comment.