fix(api): Use the get_histogram retry mechanism instead of the AsyncResponseSerial one. (#19679)

Author: vegano1

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

@@ -462,7 +462,10 @@ def __init__(
         self._async_error_ack = async_error_ack.lower()
 
     async def send_command(
-        self, command: CommandBuilder, retries: int = 0, timeout: Optional[float] = None
+        self,
+        command: CommandBuilder,
+        retries: int | None = None,
+        timeout: float | None = None,
     ) -> str:
         """
         Send a command and return the response.
@@ -478,12 +481,12 @@ async def send_command(
         """
         return await self.send_data(
             data=command.build(),
-            retries=retries or self._number_of_retries,
+            retries=retries if retries is not None else self._number_of_retries,
             timeout=timeout,
         )
 
     async def send_data(
-        self, data: str, retries: int = 0, timeout: Optional[float] = None
+        self, data: str, retries: int | None = None, timeout: float | None = None
     ) -> str:
         """
         Send data and return the response.
@@ -501,7 +504,8 @@ async def send_data(
             "timeout", timeout
         ):
             return await self._send_data(
-                data=data, retries=retries or self._number_of_retries
+                data=data,
+                retries=retries if retries is not None else self._number_of_retries,
             )
 
     async def _send_data(self, data: str, retries: int = 0) -> str:
@@ -517,7 +521,6 @@ async def _send_data(self, data: str, retries: int = 0) -> str:
         Raises: SerialException
         """
         data_encode = data.encode()
-        retries = retries or self._number_of_retries
 
         for retry in range(retries + 1):
             log.debug(f"{self._name}: Write -> {data_encode!r}")

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

@@ -461,7 +461,12 @@ async def _get_tof_histogram_frame(
         command = GCODE.GET_TOF_MEASUREMENT.build_command().add_element(sensor.name)
         if resend:
             command.add_element("R")
-        resp = await self._connection.send_command(command)
+
+        # Note: We DONT want to auto resend the request if it fails, because the
+        # firmware will send the next frame id instead of the current one missed.
+        # So lets set `retries=0` so we only send the frame once and we can
+        # use the retry mechanism of the `get_tof_histogram` method instead.
+        resp = await self._connection.send_command(command, retries=0)
         return self.parse_get_tof_measurement(resp)
 
     async def get_tof_histogram(self, sensor: TOFSensor) -> TOFMeasurementResult:

api/tests/opentrons/drivers/asyncio/communication/test_serial_connection.py

@@ -129,6 +129,27 @@ async def test_send_command_with_retry(
     )
 
 
+async def test_send_command_with_zero_retries(
+    mock_serial_port: AsyncMock, async_subject: AsyncResponseSerialConnection, ack: str
+) -> None:
+    """It should a command once"""
+    mock_serial_port.read_until.side_effect = (b"", b"")
+
+    # Set the default number of retries to 1, we want to overide this with
+    # the retries from the subject.send_data(data, retries=0) method call.
+    async_subject._number_of_retries = 1
+
+    with pytest.raises(NoResponse):
+        # We want this to overwrite the internal `_number_of_retries`
+        await async_subject.send_data(data="send data", retries=0)
+
+    mock_serial_port.timeout_override.assert_called_once_with("timeout", None)
+    mock_serial_port.write.assert_called_once_with(data=b"send data")
+    mock_serial_port.read_until.assert_called_once_with(match=ack.encode())
+    mock_serial_port.close.assert_called_once()
+    mock_serial_port.open.assert_called_once()
+
+
 async def test_send_command_with_retry_exhausted(
     mock_serial_port: AsyncMock, subject: SerialKind
 ) -> None:

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

@@ -4,6 +4,7 @@
 from binascii import Error as BinError
 from decoy import Decoy
 from mock import AsyncMock, MagicMock
+from opentrons.drivers.asyncio.communication.errors import NoResponse
 from opentrons.drivers.asyncio.communication.serial_connection import (
     AsyncResponseSerialConnection,
 )
@@ -591,7 +592,7 @@ async def test_get_tof_histogram_frame(
     get_measurement = types.GCODE.GET_TOF_MEASUREMENT.build_command().add_element(
         types.TOFSensor.X.name
     )
-    connection.send_command.assert_any_call(get_measurement)
+    connection.send_command.assert_any_call(get_measurement, retries=0)
     connection.reset_mock()
 
     # Test cancel transfer
@@ -611,7 +612,7 @@ async def test_get_tof_histogram_frame(
         .add_element(types.TOFSensor.X.name)
         .add_element("R")
     )
-    connection.send_command.assert_any_call(get_measurement)
+    connection.send_command.assert_any_call(get_measurement, retries=0)
     connection.reset_mock()
 
     # Test invalid index response
@@ -675,7 +676,7 @@ async def test_get_tof_histogram(
         types.TOFSensor.X.name
     )
     connection.send_command.assert_any_call(manage_measurement)
-    connection.send_command.assert_any_call(get_measurement)
+    connection.send_command.assert_any_call(get_measurement, retries=0)
     connection.reset_mock()
 
     # Test invalid frame_id
@@ -699,7 +700,33 @@ async def test_get_tof_histogram(
         types.TOFSensor.X.name
     )
     connection.send_command.assert_any_call(manage_measurement)
-    connection.send_command.assert_any_call(get_measurement)
+    connection.send_command.assert_any_call(get_measurement, retries=0)
+    connection.reset_mock()
+
+    # Test resend mechanism
+    get_measurement = (
+        types.GCODE.GET_TOF_MEASUREMENT.build_command()
+        .add_element(types.TOFSensor.X.name)
+        .add_element("R")
+    )
+    payload = [p for p in get_histogram_payload(30)]
+    connection.send_command.side_effect = [
+        "M215 X:1 T:2 M:3",
+        "M225 X K:0 C:1 L:3840",
+        payload[0],
+        payload[1],
+        # We raise NoResponse on frame 3 to simulate a timeout and force a resend
+        NoResponse("", "Timeout"),
+        # After the timeout we expect the same packet to be resent
+        payload[2]
+        # Then the rest of the packets
+    ] + payload[3:]
+
+    response = await subject.get_tof_histogram(types.TOFSensor.X)
+
+    connection.send_command.assert_any_call(manage_measurement)
+    # Assert that the M226 GCODE with `R` (resend) element was sent
+    connection.send_command.assert_any_call(get_measurement, retries=0)
     connection.reset_mock()