Skip to content

Commit

Permalink
refactor(api, robot-server): removal of automatic drop tip from Flex …
Browse files Browse the repository at this point in the history
…while maintaining OT2 behavior (#14182)

Removes the automatic drop tip behavior from do_stop_and_recover for Flex protocols while retaining OT2 behavior
  • Loading branch information
CaseyBatten authored Dec 12, 2023
1 parent 3005cd4 commit 165a607
Show file tree
Hide file tree
Showing 6 changed files with 142 additions and 64 deletions.
83 changes: 61 additions & 22 deletions api/src/opentrons/protocol_engine/execution/hardware_stopper.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
from .tip_handler import TipHandler, HardwareTipHandler
from ...hardware_control.types import OT3Mount

from opentrons.protocol_engine.types import AddressableOffsetVector

log = logging.getLogger(__name__)

# TODO(mc, 2022-03-07): this constant dup'd from opentrons.protocols.geometry.deck
Expand Down Expand Up @@ -68,25 +70,38 @@ async def _drop_tip(self) -> None:
axes=[MotorAxis.X, MotorAxis.Y, MotorAxis.LEFT_Z, MotorAxis.RIGHT_Z]
)

for pipette_id, tip in attached_tips:
try:
await self._tip_handler.add_tip(pipette_id=pipette_id, tip=tip)
# TODO: Add ability to drop tip onto custom trash as well.
await self._movement_handler.move_to_well(
pipette_id=pipette_id,
labware_id=FIXED_TRASH_ID,
well_name="A1",
)
await self._tip_handler.drop_tip(
pipette_id=pipette_id,
home_after=False,
# OT-2 Will only ever use the Fixed Trash Addressable Area
if self._state_store.config.robot_type == "OT-2 Standard":
for pipette_id, tip in attached_tips:
try:
await self._tip_handler.add_tip(pipette_id=pipette_id, tip=tip)
# TODO: Add ability to drop tip onto custom trash as well.
# if API is 2.15 and below aka is should_have_fixed_trash

await self._movement_handler.move_to_addressable_area(
pipette_id=pipette_id,
addressable_area_name="fixedTrash",
offset=AddressableOffsetVector(x=0, y=0, z=0),
force_direct=False,
speed=None,
minimum_z_height=None,
)

await self._tip_handler.drop_tip(
pipette_id=pipette_id,
home_after=False,
)

except HwPipetteNotAttachedError:
# this will happen normally during protocol analysis, but
# should not happen during an actual run
log.debug(f"Pipette ID {pipette_id} no longer attached.")

else:
log.debug(
"Flex protocols do not support automatic tip dropping at this time."
)

except HwPipetteNotAttachedError:
# this will happen normally during protocol analysis, but
# should not happen during an actual run
log.debug(f"Pipette ID {pipette_id} no longer attached.")

async def do_halt(self, disengage_before_stopping: bool = False) -> None:
"""Issue a halt signal to the hardware API.
Expand All @@ -102,12 +117,36 @@ async def do_stop_and_recover(
post_run_hardware_state: PostRunHardwareState,
drop_tips_after_run: bool = False,
) -> None:
"""Stop and reset the HardwareAPI, optionally dropping tips and homing."""
if drop_tips_after_run:
await self._drop_tip()

"""Stop and reset the HardwareAPI, homing and dropping tips independently if specified."""
home_after_stop = post_run_hardware_state in (
PostRunHardwareState.HOME_AND_STAY_ENGAGED,
PostRunHardwareState.HOME_THEN_DISENGAGE,
)
await self._hardware_api.stop(home_after=home_after_stop)
if drop_tips_after_run:
await self._drop_tip()
await self._hardware_api.stop(home_after=home_after_stop)

elif home_after_stop:
if len(self._state_store.pipettes.get_all_attached_tips()) == 0:
await self._hardware_api.stop(home_after=home_after_stop)
else:
try:
ot3api = ensure_ot3_hardware(hardware_api=self._hardware_api)
if (
not self._state_store.config.use_virtual_gripper
and ot3api.has_gripper()
):
await ot3api.home_z(mount=OT3Mount.GRIPPER)
except HardwareNotSupportedError:
pass

await self._movement_handler.home(
axes=[
MotorAxis.X,
MotorAxis.Y,
MotorAxis.LEFT_Z,
MotorAxis.RIGHT_Z,
]
)
else:
await self._hardware_api.stop(home_after=home_after_stop)
33 changes: 30 additions & 3 deletions api/src/opentrons/protocol_runner/protocol_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,8 @@ def __init__(
legacy_file_reader: Optional[LegacyFileReader] = None,
legacy_context_creator: Optional[LegacyContextCreator] = None,
legacy_executor: Optional[LegacyExecutor] = None,
post_run_hardware_state: PostRunHardwareState = PostRunHardwareState.HOME_AND_STAY_ENGAGED,
drop_tips_after_run: bool = True,
) -> None:
"""Initialize the PythonAndLegacyRunner with its dependencies."""
super().__init__(protocol_engine)
Expand All @@ -133,7 +135,14 @@ def __init__(
self._legacy_executor = legacy_executor or LegacyExecutor()
# TODO(mc, 2022-01-11): replace task queue with specific implementations
# of runner interface
self._task_queue = task_queue or TaskQueue(cleanup_func=protocol_engine.finish)
self._task_queue = (
task_queue or TaskQueue()
) # cleanup_func=protocol_engine.finish))
self._task_queue.set_cleanup_func(
func=protocol_engine.finish,
drop_tips_after_run=drop_tips_after_run,
post_run_hardware_state=post_run_hardware_state,
)

async def load(
self, protocol_source: ProtocolSource, python_parse_mode: PythonParseMode
Expand Down Expand Up @@ -214,6 +223,8 @@ def __init__(
task_queue: Optional[TaskQueue] = None,
json_file_reader: Optional[JsonFileReader] = None,
json_translator: Optional[JsonTranslator] = None,
post_run_hardware_state: PostRunHardwareState = PostRunHardwareState.HOME_AND_STAY_ENGAGED,
drop_tips_after_run: bool = True,
) -> None:
"""Initialize the JsonRunner with its dependencies."""
super().__init__(protocol_engine)
Expand All @@ -223,7 +234,15 @@ def __init__(
self._json_translator = json_translator or JsonTranslator()
# TODO(mc, 2022-01-11): replace task queue with specific implementations
# of runner interface
self._task_queue = task_queue or TaskQueue(cleanup_func=protocol_engine.finish)
self._task_queue = (
task_queue or TaskQueue()
) # cleanup_func=protocol_engine.finish))
self._task_queue.set_cleanup_func(
func=protocol_engine.finish,
drop_tips_after_run=drop_tips_after_run,
post_run_hardware_state=post_run_hardware_state,
)

self._hardware_api.should_taskify_movement_execution(taskify=False)

async def load(self, protocol_source: ProtocolSource) -> None:
Expand Down Expand Up @@ -312,7 +331,9 @@ def __init__(
# TODO(mc, 2022-01-11): replace task queue with specific implementations
# of runner interface
self._hardware_api = hardware_api
self._task_queue = task_queue or TaskQueue(cleanup_func=protocol_engine.finish)
self._task_queue = task_queue or TaskQueue()
self._task_queue.set_cleanup_func(func=protocol_engine.finish)

self._hardware_api.should_taskify_movement_execution(taskify=False)

def prepare(self) -> None:
Expand Down Expand Up @@ -348,6 +369,8 @@ def create_protocol_runner(
legacy_file_reader: Optional[LegacyFileReader] = None,
legacy_context_creator: Optional[LegacyContextCreator] = None,
legacy_executor: Optional[LegacyExecutor] = None,
post_run_hardware_state: PostRunHardwareState = PostRunHardwareState.HOME_AND_STAY_ENGAGED,
drop_tips_after_run: bool = True,
) -> AnyRunner:
"""Create a protocol runner."""
if protocol_config:
Expand All @@ -361,6 +384,8 @@ def create_protocol_runner(
json_file_reader=json_file_reader,
json_translator=json_translator,
task_queue=task_queue,
post_run_hardware_state=post_run_hardware_state,
drop_tips_after_run=drop_tips_after_run,
)
else:
return PythonAndLegacyRunner(
Expand All @@ -370,6 +395,8 @@ def create_protocol_runner(
legacy_file_reader=legacy_file_reader,
legacy_context_creator=legacy_context_creator,
legacy_executor=legacy_executor,
post_run_hardware_state=post_run_hardware_state,
drop_tips_after_run=drop_tips_after_run,
)

return LiveRunner(
Expand Down
29 changes: 24 additions & 5 deletions api/src/opentrons/protocol_runner/task_queue.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,16 @@
from typing import Any, Awaitable, Callable, Optional
from typing_extensions import Protocol as Callback


log = logging.getLogger(__name__)


class CleanupFunc(Callback):
"""Expected cleanup function signature."""

def __call__(self, error: Optional[Exception]) -> Any:
def __call__(
self,
error: Optional[Exception],
) -> Any:
"""Cleanup, optionally taking an error thrown.
Return value will not be used.
Expand All @@ -26,19 +28,35 @@ class TaskQueue:
Once started, a TaskQueue may not be re-used.
"""

def __init__(self, cleanup_func: CleanupFunc) -> None:
def __init__(
self,
# cleanup_func: CleanupFunc,
) -> None:
"""Initialize the TaskQueue.
Args:
cleanup_func: A function to call at run function completion
with any error raised by the run function.
"""
self._cleanup_func: CleanupFunc = cleanup_func
self._cleanup_func: Optional[
Callable[[Optional[Exception]], Any]
] = None # CleanupFunc = cleanup_func

self._run_func: Optional[Callable[[], Any]] = None
self._run_task: Optional["asyncio.Task[None]"] = None
self._ok_to_join_event: asyncio.Event = asyncio.Event()

def set_cleanup_func(
self,
func: Callable[..., Awaitable[Any]],
**kwargs: Any,
) -> None:
"""Add the protocol cleanup task to the queue.
The "cleanup" task will be run after the "run" task.
"""
self._cleanup_func = partial(func, **kwargs)

def set_run_func(
self,
func: Callable[..., Awaitable[Any]],
Expand Down Expand Up @@ -74,4 +92,5 @@ async def _run(self) -> None:
log.exception("Exception raised by protocol")
error = e

await self._cleanup_func(error=error)
if self._cleanup_func is not None:
await self._cleanup_func(error)
Original file line number Diff line number Diff line change
Expand Up @@ -91,32 +91,23 @@ async def test_hardware_stopping_sequence(
post_run_hardware_state: PostRunHardwareState,
expected_home_after: bool,
) -> None:
"""It should stop the hardware, home the robot and perform drop tip if required."""
"""It should stop the hardware, and home the robot. Flex no longer performs automatic drop tip.."""
decoy.when(state_store.pipettes.get_all_attached_tips()).then_return(
[
("pipette-id", TipGeometry(length=1.0, volume=2.0, diameter=3.0)),
]
)

await subject.do_stop_and_recover(
drop_tips_after_run=True, post_run_hardware_state=post_run_hardware_state
drop_tips_after_run=True,
post_run_hardware_state=post_run_hardware_state,
)

decoy.verify(
await hardware_api.stop(home_after=False),
await movement.home(
axes=[MotorAxis.X, MotorAxis.Y, MotorAxis.LEFT_Z, MotorAxis.RIGHT_Z]
),
await mock_tip_handler.add_tip(
pipette_id="pipette-id",
tip=TipGeometry(length=1.0, volume=2.0, diameter=3.0),
),
await movement.move_to_well(
pipette_id="pipette-id",
labware_id="fixedTrash",
well_name="A1",
),
await mock_tip_handler.drop_tip(pipette_id="pipette-id", home_after=False),
await hardware_api.stop(home_after=expected_home_after),
)

Expand Down Expand Up @@ -210,7 +201,7 @@ async def test_hardware_stopping_sequence_with_gripper(
movement: MovementHandler,
mock_tip_handler: TipHandler,
) -> None:
"""It should stop the hardware, home the robot and perform drop tip if required."""
"""It should stop the hardware, and home the robot. Flex no longer performs automatic drop tip."""
subject = HardwareStopper(
hardware_api=ot3_hardware_api,
state_store=state_store,
Expand All @@ -224,6 +215,7 @@ async def test_hardware_stopping_sequence_with_gripper(
)
decoy.when(state_store.config.use_virtual_gripper).then_return(False)
decoy.when(ot3_hardware_api.has_gripper()).then_return(True)

await subject.do_stop_and_recover(
drop_tips_after_run=True,
post_run_hardware_state=PostRunHardwareState.HOME_AND_STAY_ENGAGED,
Expand All @@ -235,18 +227,5 @@ async def test_hardware_stopping_sequence_with_gripper(
await movement.home(
axes=[MotorAxis.X, MotorAxis.Y, MotorAxis.LEFT_Z, MotorAxis.RIGHT_Z]
),
await mock_tip_handler.add_tip(
pipette_id="pipette-id",
tip=TipGeometry(length=1.0, volume=2.0, diameter=3.0),
),
await movement.move_to_well(
pipette_id="pipette-id",
labware_id="fixedTrash",
well_name="A1",
),
await mock_tip_handler.drop_tip(
pipette_id="pipette-id",
home_after=False,
),
await ot3_hardware_api.stop(home_after=True),
)
21 changes: 13 additions & 8 deletions api/tests/opentrons/protocol_runner/test_task_queue.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,15 @@ async def test_set_run_func(decoy: Decoy) -> None:
run_func = decoy.mock(is_async=True)
cleanup_func = decoy.mock(is_async=True)

subject = TaskQueue(cleanup_func=cleanup_func)
subject = TaskQueue() # cleanup_func=cleanup_func)
subject.set_cleanup_func(func=cleanup_func)
subject.set_run_func(func=run_func)
subject.start()
await subject.join()

decoy.verify(
await run_func(),
await cleanup_func(error=None),
await cleanup_func(None),
)


Expand All @@ -25,7 +26,8 @@ async def test_passes_args(decoy: Decoy) -> None:
run_func = decoy.mock(is_async=True)
cleanup_func = decoy.mock(is_async=True)

subject = TaskQueue(cleanup_func=cleanup_func)
subject = TaskQueue() # cleanup_func=cleanup_func)
subject.set_cleanup_func(func=cleanup_func)
subject.set_run_func(func=run_func, hello="world")
subject.start()
await subject.join()
Expand All @@ -41,18 +43,20 @@ async def test_cleanup_gets_run_error(decoy: Decoy) -> None:

decoy.when(await run_func()).then_raise(error)

subject = TaskQueue(cleanup_func=cleanup_func)
subject = TaskQueue() # cleanup_func=cleanup_func)
subject.set_cleanup_func(func=cleanup_func)
subject.set_run_func(func=run_func)
subject.start()
await subject.join()

decoy.verify(await cleanup_func(error=error))
decoy.verify(await cleanup_func(error))


async def test_join_waits_for_start(decoy: Decoy) -> None:
"""It should wait until the queue is started when join is called."""
cleanup_func = decoy.mock(is_async=True)
subject = TaskQueue(cleanup_func=cleanup_func)
subject = TaskQueue() # cleanup_func=cleanup_func)
subject.set_cleanup_func(func=cleanup_func)
join_task = asyncio.create_task(subject.join())

await asyncio.sleep(0)
Expand All @@ -67,11 +71,12 @@ async def test_start_runs_stuff_once(decoy: Decoy) -> None:
run_func = decoy.mock(is_async=True)
cleanup_func = decoy.mock(is_async=True)

subject = TaskQueue(cleanup_func=cleanup_func)
subject = TaskQueue() # leanup_func=cleanup_func)
subject.set_cleanup_func(func=cleanup_func)
subject.set_run_func(func=run_func)
subject.start()
subject.start()
await subject.join()

decoy.verify(await run_func(), times=1)
decoy.verify(await cleanup_func(error=None), times=1)
decoy.verify(await cleanup_func(None), times=1)
Loading

0 comments on commit 165a607

Please sign in to comment.