Skip to content

Commit cb9a4f0

Browse files
author
amitlissack
committed
fix(api): 4.6.x set max speed fails (#8437)
* tests that demonstrate bug. * format tests. * make restore_max_speeds and restore_speeds async. closes #8436
1 parent da35f4e commit cb9a4f0

File tree

4 files changed

+89
-18
lines changed

4 files changed

+89
-18
lines changed

api/src/opentrons/drivers/smoothie_drivers/driver_3_0.py

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
import logging
1414
from os import environ
1515
from time import time
16-
from typing import Any, Dict, Optional, Union, List, Tuple
16+
from typing import Any, Dict, Optional, Union, List, Tuple, cast
1717

1818
from math import isclose
1919

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

177177
# motor speed settings
178-
self._max_speed_settings = config.default_max_speed.copy()
178+
self._max_speed_settings = cast(
179+
Dict[str, float], config.default_max_speed.copy()
180+
)
179181
self._saved_max_speed_settings = self._max_speed_settings.copy()
180182
self._combined_speed = float(DEFAULT_AXES_SPEED)
181183
self._saved_axes_speed = float(self._combined_speed)
@@ -525,13 +527,13 @@ def speed(self) -> None:
525527
def steps_per_mm(self) -> Dict[str, float]:
526528
return self._steps_per_mm
527529

528-
@contextlib.contextmanager
529-
def restore_speed(self, value: Union[float, str]):
530-
self.set_speed(value, update=False)
530+
@contextlib.asynccontextmanager
531+
async def restore_speed(self, value: Union[float, str]):
532+
await self.set_speed(value, update=False)
531533
try:
532534
yield
533535
finally:
534-
self.set_speed(self._combined_speed)
536+
await self.set_speed(self._combined_speed)
535537

536538
@staticmethod
537539
def _build_speed_command(speed: float) -> CommandBuilder:
@@ -555,13 +557,13 @@ def push_speed(self) -> None:
555557
async def pop_speed(self) -> None:
556558
await self.set_speed(self._saved_axes_speed)
557559

558-
@contextlib.contextmanager
559-
def restore_axis_max_speed(self, new_max_speeds: Dict[str, float]):
560-
self.set_axis_max_speed(new_max_speeds, update=False)
560+
@contextlib.asynccontextmanager
561+
async def restore_axis_max_speed(self, new_max_speeds: Dict[str, float]):
562+
await self.set_axis_max_speed(new_max_speeds, update=False)
561563
try:
562564
yield
563565
finally:
564-
self.set_axis_max_speed(self._max_speed_settings) # type: ignore
566+
await self.set_axis_max_speed(self._max_speed_settings)
565567

566568
async def set_axis_max_speed(
567569
self, settings: Dict[str, float], update: bool = True
@@ -576,7 +578,7 @@ async def set_axis_max_speed(
576578
bool, True to save the settings for future use
577579
"""
578580
if update:
579-
self._max_speed_settings.update(settings) # type: ignore
581+
self._max_speed_settings.update(settings)
580582

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

591593
async def pop_axis_max_speed(self) -> None:
592-
await self.set_axis_max_speed(self._saved_max_speed_settings) # type: ignore
594+
await self.set_axis_max_speed(self._saved_max_speed_settings)
593595

594596
async def set_acceleration(self, settings: Dict[str, float]) -> None:
595597
"""

api/src/opentrons/hardware_control/controller.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
from __future__ import annotations
22
import asyncio
3-
from contextlib import contextmanager, ExitStack
3+
from contextlib import contextmanager, AsyncExitStack
44
import logging
55
from typing import Any, Dict, List, Optional, Tuple, TYPE_CHECKING, Union, Sequence
66
from typing_extensions import Final
@@ -129,9 +129,9 @@ async def move(
129129
speed: float = None,
130130
axis_max_speeds: Dict[str, float] = None,
131131
):
132-
with ExitStack() as cmstack:
132+
async with AsyncExitStack() as cmstack:
133133
if axis_max_speeds:
134-
cmstack.enter_context(
134+
await cmstack.enter_async_context(
135135
self._smoothie_driver.restore_axis_max_speed(axis_max_speeds)
136136
)
137137
await self._smoothie_driver.move(

api/tests/opentrons/drivers/smoothie_drivers/test_driver.py

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,3 +328,72 @@ def test_axis_bounds(smoothie: driver_3_0.SmoothieDriver) -> None:
328328
assert bounds == {
329329
k: (v if k != "Y" else Y_BOUND_OVERRIDE) for k, v in HOMED_POSITION.items()
330330
}
331+
332+
333+
async def test_set_max_speed(
334+
smoothie: driver_3_0.SmoothieDriver, mock_connection: AsyncMock
335+
) -> None:
336+
"""It should set the max speed."""
337+
await smoothie.set_axis_max_speed({"A": 22, "B": 322})
338+
cmds = [
339+
c.kwargs["command"].build().strip()
340+
for c in mock_connection.send_command.call_args_list
341+
]
342+
assert cmds == ["M203.1 A22 B322", "M400"]
343+
344+
345+
async def test_restore_axis_max_speed(
346+
smoothie: driver_3_0.SmoothieDriver, mock_connection: AsyncMock
347+
) -> None:
348+
"""It should set then restore the max speed."""
349+
smoothie._max_speed_settings = {"A": 1, "B": 2, "C": 3, "X": 4, "Y": 5, "Z": 6}
350+
async with smoothie.restore_axis_max_speed(
351+
{"A": 6, "B": 5, "C": 4, "X": 3, "Y": 2, "Z": 1}
352+
):
353+
pass
354+
cmds = [
355+
c.kwargs["command"].build().strip()
356+
for c in mock_connection.send_command.call_args_list
357+
]
358+
assert cmds == [
359+
# Set new max speed.
360+
"M203.1 A6 B5 C4 X3 Y2 Z1",
361+
"M400",
362+
# Restore old.
363+
"M203.1 A1 B2 C3 X4 Y5 Z6",
364+
"M400",
365+
]
366+
367+
368+
async def test_speed(
369+
smoothie: driver_3_0.SmoothieDriver, mock_connection: AsyncMock
370+
) -> None:
371+
"""It should set the speed."""
372+
await smoothie.set_speed(1)
373+
cmds = [
374+
c.kwargs["command"].build().strip()
375+
for c in mock_connection.send_command.call_args_list
376+
]
377+
# 60 is (speed * seconds_per_minute)
378+
assert cmds == ["G0 F60", "M400"]
379+
380+
381+
async def test_restore_speed(
382+
smoothie: driver_3_0.SmoothieDriver, mock_connection: AsyncMock
383+
) -> None:
384+
"""It should set then restore the speed."""
385+
smoothie._combined_speed = 3
386+
async with smoothie.restore_speed(2):
387+
pass
388+
cmds = [
389+
c.kwargs["command"].build().strip()
390+
for c in mock_connection.send_command.call_args_list
391+
]
392+
assert cmds == [
393+
# Set speed.
394+
"G0 F120",
395+
"M400",
396+
# Restore old.
397+
"G0 F180",
398+
"M400",
399+
]

api/tests/opentrons/hardware_control/test_moves.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ async def test_controller_must_home(hardware_api):
2525
home.assert_called_once()
2626

2727

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

5858

59-
async def test_move(hardware_api, is_robot):
59+
async def test_move(hardware_api):
6060
abs_position = types.Point(30, 20, 10)
6161
mount = types.Mount.RIGHT
6262
target_position1 = {
@@ -197,7 +197,7 @@ async def test_critical_point_applied(hardware_api, monkeypatch, is_robot):
197197
assert await hardware_api.current_position(types.Mount.RIGHT) == target
198198

199199

200-
async def test_new_critical_point_applied(hardware_api, monkeypatch, is_robot):
200+
async def test_new_critical_point_applied(hardware_api):
201201
await hardware_api.home()
202202
hardware_api._backend._attached_instruments = {
203203
types.Mount.LEFT: {"model": None, "id": None},

0 commit comments

Comments
 (0)