Skip to content

Commit 839a358

Browse files
authored
fix(api): bail early on module gcode parse errors (#20194)
We were waiting for multiple acks from certain commands, but if those commands fail at the module's gcode parser stage (i.e. because the module isn't updated and doesn't handle that gcode) then we'll never get a second ack and we'll just wait a really long time. This is ultimately fine in that it doesn't fail the protocol, but it makes anything that uses modules that have this problem take a really really long time. ## testing - [x] on a robot with a module that doesn't handle M411, check that the module polls don't take 60 seconds to complete Closes RABR-843
1 parent 881f17e commit 839a358

File tree

2 files changed

+34
-1
lines changed

2 files changed

+34
-1
lines changed

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
ErrorResponse,
1313
BaseErrorCode,
1414
DefaultErrorCodes,
15+
UnhandledGcode,
16+
GCodeCacheFull,
1517
)
1618
from .async_serial import AsyncSerial
1719

@@ -555,6 +557,23 @@ async def _consume_responses(
555557
# A read timeout, end
556558
yield "empty-unknown", data
557559

560+
def _raise_on_parser_error(self, data: str, response: bytes) -> None:
561+
"""Raise an exception if this response contains an error from the gcode parser on the module.
562+
563+
This has to be treated specially because multiack commands won't get multiple acks if the command
564+
fails at the parse stage. The errors handled here should be kept in sync with the module gcode
565+
parse code.
566+
"""
567+
try:
568+
str_response = self.process_raw_response(
569+
command=data, response=response.replace(self._ack, b"").decode()
570+
)
571+
self.raise_on_error(response=str_response, request=data)
572+
except (UnhandledGcode, GCodeCacheFull):
573+
raise
574+
except Exception:
575+
pass
576+
558577
async def _send_one_retry(self, data: str, acks: int) -> list[str]:
559578
data_encode = data.encode("utf-8")
560579
log.debug(f"{self._name}: Write -> {data_encode!r}")
@@ -567,8 +586,10 @@ async def _send_one_retry(self, data: str, acks: int) -> list[str]:
567586
async for response_type, response in self._consume_responses(acks):
568587
if response_type == "error":
569588
async_errors.append(response)
589+
self._raise_on_parser_error(data, response)
570590
elif response_type == "response":
571591
command_acks.append(response)
592+
self._raise_on_parser_error(data, response)
572593
else:
573594
break
574595

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,7 @@ async def test_send_data_multiple_ack_some_errors(
338338
"""It should return all acks."""
339339
successful_response = "M411"
340340
data = "M411"
341-
error_response = "ERR003:test"
341+
error_response = "ERR007:test"
342342
serial_successful_response = f" {successful_response} {ack}"
343343
encoded_successful_response = serial_successful_response.encode()
344344
serial_error_response = f" {error_response} {ack}"
@@ -424,3 +424,15 @@ class CustomDefaultErrorCodes(BaseErrorCode):
424424
assert error.value.command == "G28"
425425
assert error.value.response == "ERR999:test"
426426
assert error.value.port == "test_port"
427+
428+
429+
async def test_send_data_multiple_raises_unhandled(
430+
mock_serial_port: AsyncMock, async_subject: AsyncResponseSerialConnection, ack: str
431+
) -> None:
432+
"""It shouldn't wait for both acks before raising an unhandled gcode"""
433+
mock_serial_port.read_until.side_effect = [
434+
f"ERR003:unhandled gcode {ack}".encode(),
435+
]
436+
with pytest.raises(UnhandledGcode):
437+
await async_subject._send_data_multiack(data="M411", retries=0, acks=3)
438+
mock_serial_port.read_until.assert_called_once()

0 commit comments

Comments
 (0)