Skip to content

Commit

Permalink
fix(api): Skip updating position estimators for axes that are not pre…
Browse files Browse the repository at this point in the history
…sent (#16804)
  • Loading branch information
SyntaxColoring authored Nov 14, 2024
1 parent 20c98e6 commit 91b40ae
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 27 deletions.
10 changes: 5 additions & 5 deletions api/src/opentrons/hardware_control/ot3api.py
Original file line number Diff line number Diff line change
Expand Up @@ -775,12 +775,12 @@ async def _update_position_estimation(
"""
Function to update motor estimation for a set of axes
"""
if axes is None:
axes = [ax for ax in Axis]

if axes:
checked_axes = [ax for ax in axes if ax in Axis]
else:
checked_axes = [ax for ax in Axis]
await self._backend.update_motor_estimation(checked_axes)
axes = [ax for ax in axes if self._backend.axis_is_present(ax)]

await self._backend.update_motor_estimation(axes)

# Global actions API
def pause(self, pause_type: PauseType) -> None:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ async def update_axis_position_estimations(self, axes: Sequence[Axis]) -> None:
"""Update the specified axes' position estimators from their encoders.
This will allow these axes to make a non-home move even if they do not currently have
a position estimation (unless there is no tracked poition from the encoders, as would be
a position estimation (unless there is no tracked position from the encoders, as would be
true immediately after boot).
Axis encoders have less precision than their position estimators. Calling this function will
Expand All @@ -19,6 +19,8 @@ async def update_axis_position_estimations(self, axes: Sequence[Axis]) -> None:
This function updates only the requested axes. If other axes have bad position estimation,
moves that require those axes or attempts to get the position of those axes will still fail.
Axes that are not currently available (like a plunger for a pipette that is not connected)
will be ignored.
"""
...

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,11 @@ class UpdatePositionEstimatorsParams(BaseModel):
"""Payload required for an UpdatePositionEstimators command."""

axes: List[MotorAxis] = Field(
..., description="The axes for which to update the position estimators."
...,
description=(
"The axes for which to update the position estimators."
" Any axes that are not physically present will be ignored."
),
)


Expand Down
29 changes: 21 additions & 8 deletions api/tests/opentrons/hardware_control/test_ot3_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -2037,23 +2037,36 @@ def set_mock_plunger_configs() -> None:


@pytest.mark.parametrize(
"axes",
[[Axis.X], [Axis.X, Axis.Y], [Axis.X, Axis.Y, Axis.P_L], None],
("axes_in", "axes_present", "expected_axes"),
[
([Axis.X, Axis.Y], [Axis.X, Axis.Y], [Axis.X, Axis.Y]),
([Axis.X, Axis.Y], [Axis.Y, Axis.Z_L], [Axis.Y]),
(None, list(Axis), list(Axis)),
(None, [Axis.Y, Axis.Z_L], [Axis.Y, Axis.Z_L]),
],
)
async def test_update_position_estimation(
ot3_hardware: ThreadManager[OT3API],
hardware_backend: OT3Simulator,
axes: List[Axis],
axes_in: List[Axis],
axes_present: List[Axis],
expected_axes: List[Axis],
) -> None:
def _axis_is_present(axis: Axis) -> bool:
return axis in axes_present

with patch.object(
hardware_backend,
"update_motor_estimation",
AsyncMock(spec=hardware_backend.update_motor_estimation),
) as mock_update:
await ot3_hardware._update_position_estimation(axes)
if axes is None:
axes = [ax for ax in Axis]
mock_update.assert_called_once_with(axes)
) as mock_update, patch.object(
hardware_backend,
"axis_is_present",
Mock(spec=hardware_backend.axis_is_present),
) as mock_axis_is_present:
mock_axis_is_present.side_effect = _axis_is_present
await ot3_hardware._update_position_estimation(axes_in)
mock_update.assert_called_once_with(expected_axes)


async def test_refresh_positions(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -369,12 +369,6 @@ const buildBlowoutCommands = (
),
},
},
{
commandType: 'prepareToAspirate',
params: {
pipetteId: pipetteId ?? MANAGED_PIPETTE_ID,
},
},
Z_HOME,
]
: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,11 +152,7 @@ def _listener_filter(arbitration_id: ArbitrationId) -> bool:
log.warning("Update motor position estimation timed out")
raise CommandTimedOutError(
"Update motor position estimation timed out",
detail={
"missing-nodes": ", ".join(
node.name for node in set(nodes).difference(set(data))
)
},
detail={"missing-node": node.name},
)

return data
2 changes: 1 addition & 1 deletion shared-data/command/schemas/10.json
Original file line number Diff line number Diff line change
Expand Up @@ -4624,7 +4624,7 @@
"type": "object",
"properties": {
"axes": {
"description": "The axes for which to update the position estimators.",
"description": "The axes for which to update the position estimators. Any axes that are not physically present will be ignored.",
"type": "array",
"items": {
"$ref": "#/definitions/MotorAxis"
Expand Down

0 comments on commit 91b40ae

Please sign in to comment.