Skip to content

Commit e2125a6

Browse files
committed
fix(api): Use the get_histogram retry mechanism instead of the AsyncResponseSerial one.
1 parent b153405 commit e2125a6

File tree

3 files changed

+43
-11
lines changed

3 files changed

+43
-11
lines changed

api/src/opentrons/drivers/asyncio/communication/serial_connection.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -462,7 +462,10 @@ def __init__(
462462
self._async_error_ack = async_error_ack.lower()
463463

464464
async def send_command(
465-
self, command: CommandBuilder, retries: int = 0, timeout: Optional[float] = None
465+
self,
466+
command: CommandBuilder,
467+
retries: Optional[int] = None,
468+
timeout: Optional[float] = None,
466469
) -> str:
467470
"""
468471
Send a command and return the response.
@@ -478,7 +481,7 @@ async def send_command(
478481
"""
479482
return await self.send_data(
480483
data=command.build(),
481-
retries=retries or self._number_of_retries,
484+
retries=retries if retries is not None else self._number_of_retries,
482485
timeout=timeout,
483486
)
484487

@@ -500,9 +503,7 @@ async def send_data(
500503
async with super().send_data_lock, self._serial.timeout_override(
501504
"timeout", timeout
502505
):
503-
return await self._send_data(
504-
data=data, retries=retries or self._number_of_retries
505-
)
506+
return await self._send_data(data=data, retries=retries)
506507

507508
async def _send_data(self, data: str, retries: int = 0) -> str:
508509
"""
@@ -517,7 +518,6 @@ async def _send_data(self, data: str, retries: int = 0) -> str:
517518
Raises: SerialException
518519
"""
519520
data_encode = data.encode()
520-
retries = retries or self._number_of_retries
521521

522522
for retry in range(retries + 1):
523523
log.debug(f"{self._name}: Write -> {data_encode!r}")

api/src/opentrons/drivers/flex_stacker/driver.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -461,7 +461,12 @@ async def _get_tof_histogram_frame(
461461
command = GCODE.GET_TOF_MEASUREMENT.build_command().add_element(sensor.name)
462462
if resend:
463463
command.add_element("R")
464-
resp = await self._connection.send_command(command)
464+
465+
# Note: We DONT want to auto resend the request if it fails, because the
466+
# firmware will send the next frame id instead of the current one missed.
467+
# So lets set `retries=0` so we only send the frame once and we can
468+
# use the retry mechanism of the `get_tof_histogram` method instead.
469+
resp = await self._connection.send_command(command, retries=0)
465470
return self.parse_get_tof_measurement(resp)
466471

467472
async def get_tof_histogram(self, sensor: TOFSensor) -> TOFMeasurementResult:

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

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
from binascii import Error as BinError
55
from decoy import Decoy
66
from mock import AsyncMock, MagicMock
7+
from opentrons.drivers.asyncio.communication.errors import NoResponse
78
from opentrons.drivers.asyncio.communication.serial_connection import (
89
AsyncResponseSerialConnection,
910
)
@@ -591,7 +592,7 @@ async def test_get_tof_histogram_frame(
591592
get_measurement = types.GCODE.GET_TOF_MEASUREMENT.build_command().add_element(
592593
types.TOFSensor.X.name
593594
)
594-
connection.send_command.assert_any_call(get_measurement)
595+
connection.send_command.assert_any_call(get_measurement, retries=0)
595596
connection.reset_mock()
596597

597598
# Test cancel transfer
@@ -611,7 +612,7 @@ async def test_get_tof_histogram_frame(
611612
.add_element(types.TOFSensor.X.name)
612613
.add_element("R")
613614
)
614-
connection.send_command.assert_any_call(get_measurement)
615+
connection.send_command.assert_any_call(get_measurement, retries=0)
615616
connection.reset_mock()
616617

617618
# Test invalid index response
@@ -675,7 +676,7 @@ async def test_get_tof_histogram(
675676
types.TOFSensor.X.name
676677
)
677678
connection.send_command.assert_any_call(manage_measurement)
678-
connection.send_command.assert_any_call(get_measurement)
679+
connection.send_command.assert_any_call(get_measurement, retries=0)
679680
connection.reset_mock()
680681

681682
# Test invalid frame_id
@@ -699,7 +700,33 @@ async def test_get_tof_histogram(
699700
types.TOFSensor.X.name
700701
)
701702
connection.send_command.assert_any_call(manage_measurement)
702-
connection.send_command.assert_any_call(get_measurement)
703+
connection.send_command.assert_any_call(get_measurement, retries=0)
704+
connection.reset_mock()
705+
706+
# Test resend mechanism
707+
get_measurement = (
708+
types.GCODE.GET_TOF_MEASUREMENT.build_command()
709+
.add_element(types.TOFSensor.X.name)
710+
.add_element("R")
711+
)
712+
payload = [p for p in get_histogram_payload(30)]
713+
connection.send_command.side_effect = [
714+
"M215 X:1 T:2 M:3",
715+
"M225 X K:0 C:1 L:3840",
716+
payload[0],
717+
payload[1],
718+
# We raise NoResponse on frame 3 to simulate a timeout and force a resend
719+
NoResponse("", "Timeout"),
720+
# After the timeout we expect the same packet to be resent
721+
payload[2]
722+
# Then the rest of the packets
723+
] + payload[3:]
724+
725+
response = await subject.get_tof_histogram(types.TOFSensor.X)
726+
727+
connection.send_command.assert_any_call(manage_measurement)
728+
# Assert that the M226 GCODE with `R` (resend) element was sent
729+
connection.send_command.assert_any_call(get_measurement, retries=0)
703730
connection.reset_mock()
704731

705732

0 commit comments

Comments
 (0)