Skip to content

Commit a8d78a3

Browse files
authored
fix(robot-server): dont let /instruments block (#14608)
It calls cache_instruments and that can block because it takes the motion lock, but we really don't want that. We don't mind if cache_instruments doesn't get called on the flex because it's sort of a secondary functionality, so just bail early in this case. ## Testing - Run on a robot and do some `GET /instruments` while things are homign and note that it instantly returns - Do an attach flow and note that it still returns the new instruments. Closes EXEC-298
1 parent f50e261 commit a8d78a3

File tree

5 files changed

+14
-7
lines changed

5 files changed

+14
-7
lines changed

api/src/opentrons/hardware_control/api.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,9 @@ def has_gripper(self) -> bool:
431431
return False
432432

433433
async def cache_instruments(
434-
self, require: Optional[Dict[top_types.Mount, PipetteName]] = None
434+
self,
435+
require: Optional[Dict[top_types.Mount, PipetteName]] = None,
436+
skip_if_would_block: bool = False,
435437
) -> None:
436438
"""
437439
Scan the attached instruments, take necessary configuration actions,

api/src/opentrons/hardware_control/ot3api.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -653,12 +653,16 @@ def get_all_attached_instr(self) -> Dict[OT3Mount, Optional[InstrumentDict]]:
653653

654654
# TODO (spp, 2023-01-31): add unit tests
655655
async def cache_instruments(
656-
self, require: Optional[Dict[top_types.Mount, PipetteName]] = None
656+
self,
657+
require: Optional[Dict[top_types.Mount, PipetteName]] = None,
658+
skip_if_would_block: bool = False,
657659
) -> None:
658660
"""
659661
Scan the attached instruments, take necessary configuration actions,
660662
and set up hardware controller internal state if necessary.
661663
"""
664+
if skip_if_would_block and self._motion_lock.locked():
665+
return
662666
async with self._motion_lock:
663667
skip_configure = await self._cache_instruments(require)
664668
if not skip_configure or not self._configured_since_update:

api/src/opentrons/hardware_control/protocols/instrument_configurer.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ def reset_instrument(self, mount: Optional[MountArgType] = None) -> None:
2828
async def cache_instruments(
2929
self,
3030
require: Optional[Dict[Mount, PipetteName]] = None,
31+
skip_if_would_block: bool = False,
3132
) -> None:
3233
"""
3334
Scan the attached instruments, take necessary configuration actions,

robot-server/robot_server/instruments/router.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ async def _get_attached_instruments_ot3(
216216
hardware: OT3HardwareControlAPI,
217217
) -> PydanticResponse[SimpleMultiBody[AttachedItem]]:
218218
# OT3
219-
await hardware.cache_instruments()
219+
await hardware.cache_instruments(skip_if_would_block=True)
220220
response_data = await _get_instrument_data(hardware)
221221
return await PydanticResponse.create(
222222
content=SimpleMultiBody.construct(

robot-server/tests/instruments/test_router.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ async def test_get_all_attached_instruments(
121121
subsystem=SubSystem.pipette_right,
122122
)
123123

124-
async def rehearse_instrument_retrievals() -> None:
124+
async def rehearse_instrument_retrievals(skip_if_would_block: bool = False) -> None:
125125
decoy.when(ot3_hardware_api.attached_gripper).then_return(
126126
cast(
127127
GripperDict,
@@ -188,9 +188,9 @@ async def rehearse_instrument_retrievals() -> None:
188188

189189
# We use this convoluted way of testing to verify the important point that
190190
# cache_instruments is called before fetching attached pipette and gripper data.
191-
decoy.when(await ot3_hardware_api.cache_instruments()).then_do(
192-
rehearse_instrument_retrievals
193-
)
191+
decoy.when(
192+
await ot3_hardware_api.cache_instruments(skip_if_would_block=True)
193+
).then_do(rehearse_instrument_retrievals)
194194
decoy.when(ot3_hardware_api.get_instrument_offset(mount=OT3Mount.LEFT)).then_return(
195195
PipetteOffsetSummary(
196196
offset=Point(1, 2, 3),

0 commit comments

Comments
 (0)