Skip to content

Commit

Permalink
fix(api): fix infinite homing and z drops around fw update (#14597)
Browse files Browse the repository at this point in the history
This PR fixes two issues.
1. Often, after updating the firmware of the head or the gripper, homing
that subsystem's z axis would not cause the motor to move. The root
cause for this is in the firmware, but lessening the distance that we
home the axes also makes the errors happen after 2 minutes instead of
20.
2. Sometimes, after updating the firmware of the head or the gripper,
that axis would fall down. This is in fact pretty much the same issue -
it's to do with mechanics around how we handle the emergency brakes -
and is also fixed in firmware; but disabling these axes when we're about
to update them will help.

Also, wrap the entirety of `cache_instruments` in the motion lock rather
than just sections, which would sometimes lead to part of the calls
happening long before the rest of them.

Closes RQA-2301, RQA-2429

---------

Co-authored-by: ahiuchingau <[email protected]>
  • Loading branch information
sfoster1 and ahiuchingau authored Mar 4, 2024
1 parent 25c1e89 commit ebb287b
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1151,7 +1151,7 @@ async def watch(self, loop: asyncio.AbstractEventLoop) -> None:
def axis_bounds(self) -> OT3AxisMap[Tuple[float, float]]:
"""Get the axis bounds."""
# TODO (AL, 2021-11-18): The bounds need to be defined
phony_bounds = (0, 10000)
phony_bounds = (0, 500)
return {
Axis.Z_L: phony_bounds,
Axis.Z_R: phony_bounds,
Expand Down
31 changes: 19 additions & 12 deletions api/src/opentrons/hardware_control/ot3api.py
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,10 @@ async def update_firmware(
) -> AsyncIterator[UpdateStatus]:
"""Start the firmware update for one or more subsystems and return update progress iterator."""
subsystems = subsystems or set()
if SubSystem.head in subsystems:
await self.disengage_axes([Axis.Z_L, Axis.Z_R])
if SubSystem.gripper in subsystems:
await self.disengage_axes([Axis.Z_G])
# start the updates and yield the progress
async with self._motion_lock:
try:
Expand Down Expand Up @@ -653,10 +657,11 @@ async def cache_instruments(
Scan the attached instruments, take necessary configuration actions,
and set up hardware controller internal state if necessary.
"""
skip_configure = await self._cache_instruments(require)
if not skip_configure or not self._configured_since_update:
self._log.info("Reconfiguring instrument cache")
await self._configure_instruments()
async with self._motion_lock:
skip_configure = await self._cache_instruments(require)
if not skip_configure or not self._configured_since_update:
self._log.info("Reconfiguring instrument cache")
await self._configure_instruments()

async def _cache_instruments( # noqa: C901
self, require: Optional[Dict[top_types.Mount, PipetteName]] = None
Expand All @@ -676,11 +681,11 @@ async def _cache_instruments( # noqa: C901
# We should also check version here once we're comfortable.
if not pipette_load_name.supported_pipette(name):
raise RuntimeError(f"{name} is not a valid pipette name")
async with self._motion_lock:
# we're not actually checking the required instrument except in the context
# of simulation and it feels like a lot of work for this function
# actually be doing.
found = await self._backend.get_attached_instruments(checked_require)

# we're not actually checking the required instrument except in the context
# of simulation and it feels like a lot of work for this function
# actually be doing.
found = await self._backend.get_attached_instruments(checked_require)

if OT3Mount.GRIPPER in found.keys():
# Is now a gripper, ask if it's ok to skip
Expand Down Expand Up @@ -726,7 +731,7 @@ async def _cache_instruments( # noqa: C901
async def _configure_instruments(self) -> None:
"""Configure instruments"""
await self.set_gantry_load(self._gantry_load_from_instruments())
await self.refresh_positions()
await self.refresh_positions(acquire_lock=False)
await self.reset_tip_detectors(False)
self._configured_since_update = True

Expand Down Expand Up @@ -983,9 +988,11 @@ async def current_position_ot3(
OT3Mount.from_mount(mount), self._current_position, critical_point
)

async def refresh_positions(self) -> None:
async def refresh_positions(self, acquire_lock: bool = True) -> None:
"""Request and update both the motor and encoder positions from backend."""
async with self._motion_lock:
async with contextlib.AsyncExitStack() as stack:
if acquire_lock:
await stack.enter_async_context(self._motion_lock)
await self._backend.update_motor_status()
await self._cache_current_position()
await self._cache_encoder_position()
Expand Down

0 comments on commit ebb287b

Please sign in to comment.