Skip to content

Commit

Permalink
fix(api): 4.6.x set max speed fails (#8437)
Browse files Browse the repository at this point in the history
* tests that demonstrate bug.

* format tests.

* make restore_max_speeds and restore_speeds async.

closes #8436
  • Loading branch information
amitlissack committed Sep 30, 2021
1 parent da35f4e commit cb9a4f0
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 18 deletions.
26 changes: 14 additions & 12 deletions api/src/opentrons/drivers/smoothie_drivers/driver_3_0.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import logging
from os import environ
from time import time
from typing import Any, Dict, Optional, Union, List, Tuple
from typing import Any, Dict, Optional, Union, List, Tuple, cast

from math import isclose

Expand Down Expand Up @@ -175,7 +175,9 @@ def __init__(
self.engaged_axes = {ax: True for ax in AXES}

# motor speed settings
self._max_speed_settings = config.default_max_speed.copy()
self._max_speed_settings = cast(
Dict[str, float], config.default_max_speed.copy()
)
self._saved_max_speed_settings = self._max_speed_settings.copy()
self._combined_speed = float(DEFAULT_AXES_SPEED)
self._saved_axes_speed = float(self._combined_speed)
Expand Down Expand Up @@ -525,13 +527,13 @@ def speed(self) -> None:
def steps_per_mm(self) -> Dict[str, float]:
return self._steps_per_mm

@contextlib.contextmanager
def restore_speed(self, value: Union[float, str]):
self.set_speed(value, update=False)
@contextlib.asynccontextmanager
async def restore_speed(self, value: Union[float, str]):
await self.set_speed(value, update=False)
try:
yield
finally:
self.set_speed(self._combined_speed)
await self.set_speed(self._combined_speed)

@staticmethod
def _build_speed_command(speed: float) -> CommandBuilder:
Expand All @@ -555,13 +557,13 @@ def push_speed(self) -> None:
async def pop_speed(self) -> None:
await self.set_speed(self._saved_axes_speed)

@contextlib.contextmanager
def restore_axis_max_speed(self, new_max_speeds: Dict[str, float]):
self.set_axis_max_speed(new_max_speeds, update=False)
@contextlib.asynccontextmanager
async def restore_axis_max_speed(self, new_max_speeds: Dict[str, float]):
await self.set_axis_max_speed(new_max_speeds, update=False)
try:
yield
finally:
self.set_axis_max_speed(self._max_speed_settings) # type: ignore
await self.set_axis_max_speed(self._max_speed_settings)

async def set_axis_max_speed(
self, settings: Dict[str, float], update: bool = True
Expand All @@ -576,7 +578,7 @@ async def set_axis_max_speed(
bool, True to save the settings for future use
"""
if update:
self._max_speed_settings.update(settings) # type: ignore
self._max_speed_settings.update(settings)

command = _command_builder().add_gcode(gcode=GCODE.SET_MAX_SPEED)
for axis, value in sorted(settings.items()):
Expand All @@ -589,7 +591,7 @@ def push_axis_max_speed(self) -> None:
self._saved_max_speed_settings = self._max_speed_settings.copy()

async def pop_axis_max_speed(self) -> None:
await self.set_axis_max_speed(self._saved_max_speed_settings) # type: ignore
await self.set_axis_max_speed(self._saved_max_speed_settings)

async def set_acceleration(self, settings: Dict[str, float]) -> None:
"""
Expand Down
6 changes: 3 additions & 3 deletions api/src/opentrons/hardware_control/controller.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from __future__ import annotations
import asyncio
from contextlib import contextmanager, ExitStack
from contextlib import contextmanager, AsyncExitStack
import logging
from typing import Any, Dict, List, Optional, Tuple, TYPE_CHECKING, Union, Sequence
from typing_extensions import Final
Expand Down Expand Up @@ -129,9 +129,9 @@ async def move(
speed: float = None,
axis_max_speeds: Dict[str, float] = None,
):
with ExitStack() as cmstack:
async with AsyncExitStack() as cmstack:
if axis_max_speeds:
cmstack.enter_context(
await cmstack.enter_async_context(
self._smoothie_driver.restore_axis_max_speed(axis_max_speeds)
)
await self._smoothie_driver.move(
Expand Down
69 changes: 69 additions & 0 deletions api/tests/opentrons/drivers/smoothie_drivers/test_driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -328,3 +328,72 @@ def test_axis_bounds(smoothie: driver_3_0.SmoothieDriver) -> None:
assert bounds == {
k: (v if k != "Y" else Y_BOUND_OVERRIDE) for k, v in HOMED_POSITION.items()
}


async def test_set_max_speed(
smoothie: driver_3_0.SmoothieDriver, mock_connection: AsyncMock
) -> None:
"""It should set the max speed."""
await smoothie.set_axis_max_speed({"A": 22, "B": 322})
cmds = [
c.kwargs["command"].build().strip()
for c in mock_connection.send_command.call_args_list
]
assert cmds == ["M203.1 A22 B322", "M400"]


async def test_restore_axis_max_speed(
smoothie: driver_3_0.SmoothieDriver, mock_connection: AsyncMock
) -> None:
"""It should set then restore the max speed."""
smoothie._max_speed_settings = {"A": 1, "B": 2, "C": 3, "X": 4, "Y": 5, "Z": 6}
async with smoothie.restore_axis_max_speed(
{"A": 6, "B": 5, "C": 4, "X": 3, "Y": 2, "Z": 1}
):
pass
cmds = [
c.kwargs["command"].build().strip()
for c in mock_connection.send_command.call_args_list
]
assert cmds == [
# Set new max speed.
"M203.1 A6 B5 C4 X3 Y2 Z1",
"M400",
# Restore old.
"M203.1 A1 B2 C3 X4 Y5 Z6",
"M400",
]


async def test_speed(
smoothie: driver_3_0.SmoothieDriver, mock_connection: AsyncMock
) -> None:
"""It should set the speed."""
await smoothie.set_speed(1)
cmds = [
c.kwargs["command"].build().strip()
for c in mock_connection.send_command.call_args_list
]
# 60 is (speed * seconds_per_minute)
assert cmds == ["G0 F60", "M400"]


async def test_restore_speed(
smoothie: driver_3_0.SmoothieDriver, mock_connection: AsyncMock
) -> None:
"""It should set then restore the speed."""
smoothie._combined_speed = 3
async with smoothie.restore_speed(2):
pass
cmds = [
c.kwargs["command"].build().strip()
for c in mock_connection.send_command.call_args_list
]
assert cmds == [
# Set speed.
"G0 F120",
"M400",
# Restore old.
"G0 F180",
"M400",
]
6 changes: 3 additions & 3 deletions api/tests/opentrons/hardware_control/test_moves.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ async def test_controller_must_home(hardware_api):
home.assert_called_once()


async def test_home_specific_sim(hardware_api, monkeypatch, is_robot):
async def test_home_specific_sim(hardware_api):
await hardware_api.home()
await hardware_api.move_to(types.Mount.RIGHT, types.Point(0, 10, 20))
# Avoid the autoretract when moving two difference instruments
Expand Down Expand Up @@ -56,7 +56,7 @@ async def test_retract(hardware_api):
}


async def test_move(hardware_api, is_robot):
async def test_move(hardware_api):
abs_position = types.Point(30, 20, 10)
mount = types.Mount.RIGHT
target_position1 = {
Expand Down Expand Up @@ -197,7 +197,7 @@ async def test_critical_point_applied(hardware_api, monkeypatch, is_robot):
assert await hardware_api.current_position(types.Mount.RIGHT) == target


async def test_new_critical_point_applied(hardware_api, monkeypatch, is_robot):
async def test_new_critical_point_applied(hardware_api):
await hardware_api.home()
hardware_api._backend._attached_instruments = {
types.Mount.LEFT: {"model": None, "id": None},
Expand Down

0 comments on commit cb9a4f0

Please sign in to comment.