diff --git a/src/aare/devices/tell_client.py b/src/aare/devices/tell_client.py index 833fadba..1d6452cb 100755 --- a/src/aare/devices/tell_client.py +++ b/src/aare/devices/tell_client.py @@ -278,34 +278,22 @@ class TellClient: timeout=wait_timeout, ) logger.info(f"event: {event} occurred with value: {value}") - if event == "state" and str(value) == '"Ready"': - try: - msg = self.check_command_ok( - timeout=wait_timeout, msg=f"Mount {segment}{puck}-{sample}: " - ) - logger.info(f"Check command okay response: {msg}") - return TellEventValueEnum.SUCCESS - except Exception: - raise - if event == "state" and value == '"Busy"': - logger.warning('got busy response form robot, waiting for mount to complete') - try: - msg = self.check_command_ok( - timeout=wait_timeout, msg=f"Mount {segment}{puck}-{sample}: " - ) - logger.info(f"Check command okay response: {msg}") - return TellEventValueEnum.SUCCESS - except Exception: - raise if event is None or event == "state": + # TELL reports a state transition (typically "Busy" then + # "Ready"). Event timing means we sometimes observe the + # intermediate "Busy", and the value can arrive quoted or + # unquoted, so we do not trust the event value here. Instead + # wait for the mount to finish and let check_command_ok be + # the sole authority on success/failure. + state_value = str(value).strip().strip('"\'').lower() + if state_value == "busy": + logger.warning("got busy response from robot, waiting for mount to complete") logger.info(f"event: {event} occurred with value: {value}, checking command completed okay") - self.check_command_ok( + msg = self.check_command_ok( timeout=wait_timeout, msg=f"Mount {segment}{puck}-{sample}: " ) - if value.lower() == "ready" or value.lower() == '"ready"' or value == "Ready" or str(value.lower()) == "ready" or str(value.lower()) == '"ready"': - return TellEventValueEnum.SUCCESS - else: - raise Exception(f"Unexpected event: {event} occurred with value: {value}") + logger.info(f"Check command okay response: {msg}") + return TellEventValueEnum.SUCCESS elif ( event == TellEventTypeEnum.GIPPER_DETECTION.value and value == TellEventValueEnum.NO_PIN_IN_GRIPPER.value diff --git a/tests/unit/devices/test_tell_client.py b/tests/unit/devices/test_tell_client.py index 619461a4..4c1beb6e 100644 --- a/tests/unit/devices/test_tell_client.py +++ b/tests/unit/devices/test_tell_client.py @@ -1,6 +1,8 @@ import pytest from unittest.mock import MagicMock -from aare.devices.tell_client import TellClient +from aare.common.exception_handler import TellCommunicationError +from aare.common.models import DewarAddress, SampleDewarAddress +from aare.devices.tell_client import TellClient, TellEventValueEnum from aare.devices.tell_backend import TellBackend @pytest.fixture @@ -29,4 +31,48 @@ def test_is_in_mount_position_false(mock_beamline, mock_backend): client = TellClient(mock_beamline, backend=mock_backend) assert client.is_in_mount_position() is False - #TODO add tests for status checks and other functions \ No newline at end of file + +def _mount_address(): + return SampleDewarAddress(puck=DewarAddress(segment="A", pos=3), pin=10) + + +@pytest.mark.parametrize("state_value", ["Busy", "Ready", '"Busy"', '"Ready"', "busy"]) +def test_mount_state_event_succeeds_when_command_completes( + mock_beamline, mock_backend, state_value +): + """Regression: TELL emits a "state" event (sometimes the intermediate + "Busy", quoted or unquoted) instead of a terminal event. The mount must + rely on check_command_ok rather than string-matching the event value, so a + completed command is reported as SUCCESS rather than raising.""" + mock_backend.wait_events.return_value = ("state", state_value) + mock_backend.get_result.return_value = { + "status": "completed", + "return": "A39", + "exception": None, + "id": 3017907, + } + client = TellClient(mock_beamline, backend=mock_backend) + + result = client.mount(_mount_address(), wait=True) + + assert result == TellEventValueEnum.SUCCESS + mock_backend.get_result.assert_called() # check_command_ok consulted the result + + +def test_mount_state_event_raises_when_command_not_completed( + mock_beamline, mock_backend +): + """A genuine failure still surfaces: if the command did not complete, + check_command_ok raises (MountingFailed) and mount() re-raises it as a + critical TellCommunicationError, even though the event value was "Busy".""" + mock_backend.wait_events.return_value = ("state", "Busy") + mock_backend.get_result.return_value = { + "status": "error", + "return": None, + "exception": "boom", + "id": 3017907, + } + client = TellClient(mock_beamline, backend=mock_backend) + + with pytest.raises(TellCommunicationError): + client.mount(_mount_address(), wait=True) \ No newline at end of file