Tell_client: fix state event handling, update tests

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

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
+
+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)