Pass client to check_response for error messages

src/sorunlib/_internal.py

@@ -7,11 +7,13 @@
 import ocs
 
 
-def check_response(response):
+def check_response(client, response):
     """Check that a response from OCS indicates successful task/process
     completion.
 
     Args:
+        client (ocs.ocs_client.OCSClient): OCS Client which returned the
+            response.
         response (ocs.ocs_client.OCSReply): Response from an OCS operation
             call.
 
@@ -21,15 +23,18 @@ def check_response(response):
 
     """
     op = response.session['op_name']
+    instance = client.instance_id
 
     if response.status == ocs.ERROR:
-        error = f"Request for Operation {op} failed.\n" + str(response)
+        error = f"Request for Operation {op} in Agent {instance} failed.\n" + \
+            str(response)
         raise RuntimeError(error)
     elif response.status == ocs.TIMEOUT:
-        error = f"Timeout reached waiting for {op} to complete." + str(
-            response)
+        error = f"Timeout reached waiting for {op} in Agent {instance} to " + \
+            "complete." + str(response)
         raise RuntimeError(error)
 
     if not response.session['success']:
-        error = 'Task failed to complete successfully.\n' + str(response)
+        error = f'Task {op} in Agent {instance} failed to complete " + \
+            "successfully.\n' + str(response)
         raise RuntimeError(error)

src/sorunlib/acu.py

@@ -10,9 +10,9 @@ def move_to(az, el):
         el (float): destination angle for the elevation axis
 
     """
-    # Az/El motion
-    resp = run.CLIENTS['acu'].go_to(az=az, el=el)
-    check_response(resp)
+    acu = run.CLIENTS['acu']
+    resp = acu.go_to(az=az, el=el)
+    check_response(acu, resp)
 
 
 def set_boresight(target):
@@ -25,12 +25,14 @@ def set_boresight(target):
         RuntimeError: If boresight is passed to a non-satp platform.
 
     """
+    acu = run.CLIENTS['acu']
+
     # Check platform type
-    resp = run.CLIENTS['acu'].monitor.status()
+    resp = acu.monitor.status()
     platform = resp.session['data']['PlatformType']
     if platform == "satp":
-        resp = run.CLIENTS['acu'].set_boresight(target=target)
-        check_response(resp)
+        resp = acu.set_boresight(target=target)
+        check_response(acu, resp)
     else:
         raise RuntimeError(f"Platform type {platform} does not support boresight motion")
 
@@ -46,7 +48,8 @@ def set_scan_params(az_speed, az_accel, reset=False):
             before applying any updates passed explicitly here.
 
     """
-    resp = run.CLIENTS['acu'].set_scan_params(az_speed=az_speed,
-                                              az_accel=az_accel,
-                                              reset=reset)
-    check_response(resp)
+    acu = run.CLIENTS['acu']
+    resp = acu.set_scan_params(az_speed=az_speed,
+                               az_accel=az_accel,
+                               reset=reset)
+    check_response(acu, resp)

src/sorunlib/seq.py

@@ -21,24 +21,26 @@ def scan(description, stop_time, width, az_drift=0, tag=None, subtype=None):
         subtype (str, optional): Operation subtype used to tag the stream.
 
     """
+    acu = run.CLIENTS['acu']
+
     # Enable SMuRF streams
     run.smurf.stream('on', subtype=subtype, tag=tag)
 
     try:
         # Grab current telescope position
-        resp = run.CLIENTS['acu'].monitor.status()
+        resp = acu.monitor.status()
         az = resp.session['data']['StatusDetailed']['Azimuth current position']
         el = resp.session['data']['StatusDetailed']['Elevation current position']
 
         # Start telescope motion
         # az_speed and az_accel assumed from ACU defaults
         # Can be modified by acu.set_scan_params()
-        resp = run.CLIENTS['acu'].generate_scan.start(az_endpoint1=az,
-                                                      az_endpoint2=az + width,
-                                                      el_endpoint1=el,
-                                                      el_endpoint2=el,
-                                                      el_speed=0,
-                                                      az_drift=az_drift)
+        resp = acu.generate_scan.start(az_endpoint1=az,
+                                       az_endpoint2=az + width,
+                                       el_endpoint1=el,
+                                       el_endpoint2=el,
+                                       el_speed=0,
+                                       az_drift=az_drift)
 
         if not resp.session:
             raise Exception(f"Generate Scan failed to start:\n  {resp}")
@@ -47,9 +49,9 @@ def scan(description, stop_time, width, az_drift=0, tag=None, subtype=None):
         run.commands.wait_until(stop_time)
     finally:
         # Stop motion
-        run.CLIENTS['acu'].generate_scan.stop()
-        resp = run.CLIENTS['acu'].generate_scan.wait(timeout=OP_TIMEOUT)
-        check_response(resp)
+        acu.generate_scan.stop()
+        resp = acu.generate_scan.wait(timeout=OP_TIMEOUT)
+        check_response(acu, resp)
 
         # Stop SMuRF streams
         run.smurf.stream('off')

src/sorunlib/smurf.py

@@ -49,7 +49,7 @@ def bias_step(tag=None, concurrent=True, settling_time=None):
         smurf.take_bias_steps.start(tag=tag)
         if not concurrent:
             resp = smurf.take_bias_steps.wait()
-            check_response(resp)
+            check_response(smurf, resp)
 
             # Allow cryo to settle
             wait = settling_time if settling_time else CRYO_WAIT
@@ -58,7 +58,7 @@ def bias_step(tag=None, concurrent=True, settling_time=None):
     if concurrent:
         for smurf in run.CLIENTS['smurf']:
             resp = smurf.take_bias_steps.wait()
-            check_response(resp)
+            check_response(smurf, resp)
 
 
 def iv_curve(tag=None, concurrent=True, settling_time=None):
@@ -81,7 +81,7 @@ def iv_curve(tag=None, concurrent=True, settling_time=None):
         smurf.take_iv.start(tag=tag)
         if not concurrent:
             resp = smurf.take_iv.wait()
-            check_response(resp)
+            check_response(smurf, resp)
 
             # Allow cryo to settle
             wait = settling_time if settling_time else CRYO_WAIT
@@ -90,7 +90,7 @@ def iv_curve(tag=None, concurrent=True, settling_time=None):
     if concurrent:
         for smurf in run.CLIENTS['smurf']:
             resp = smurf.take_iv.wait()
-            check_response(resp)
+            check_response(smurf, resp)
 
 
 def uxm_setup(concurrent=True, settling_time=0):
@@ -110,15 +110,15 @@ def uxm_setup(concurrent=True, settling_time=0):
         smurf.uxm_setup.start()
         if not concurrent:
             resp = smurf.uxm_setup.wait()
-            check_response(resp)
+            check_response(smurf, resp)
 
             # Allow cryo to settle
             time.sleep(settling_time)
 
     if concurrent:
         for smurf in run.CLIENTS['smurf']:
             resp = smurf.uxm_setup.wait()
-            check_response(resp)
+            check_response(smurf, resp)
 
 
 def uxm_relock(test_mode=False, concurrent=True, settling_time=0):
@@ -140,15 +140,15 @@ def uxm_relock(test_mode=False, concurrent=True, settling_time=0):
         smurf.uxm_relock.start(test_mode=test_mode)
         if not concurrent:
             resp = smurf.uxm_relock.wait()
-            check_response(resp)
+            check_response(smurf, resp)
 
             # Allow cryo to settle
             time.sleep(settling_time)
 
     if concurrent:
         for smurf in run.CLIENTS['smurf']:
             resp = smurf.uxm_relock.wait()
-            check_response(resp)
+            check_response(smurf, resp)
 
 
 def bias_dets(concurrent=True, settling_time=0):
@@ -168,15 +168,15 @@ def bias_dets(concurrent=True, settling_time=0):
         smurf.bias_dets.start()
         if not concurrent:
             resp = smurf.bias_dets.wait()
-            check_response(resp)
+            check_response(smurf, resp)
 
             # Allow cryo to settle
             time.sleep(settling_time)
 
     if concurrent:
         for smurf in run.CLIENTS['smurf']:
             resp = smurf.bias_dets.wait()
-            check_response(resp)
+            check_response(smurf, resp)
 
 
 def take_bgmap(tag=None, concurrent=True, settling_time=0):
@@ -198,15 +198,15 @@ def take_bgmap(tag=None, concurrent=True, settling_time=0):
         smurf.take_bgmap.start(tag=tag)
         if not concurrent:
             resp = smurf.take_bgmap.wait()
-            check_response(resp)
+            check_response(smurf, resp)
 
             # Allow cryo to settle
             time.sleep(settling_time)
 
     if concurrent:
         for smurf in run.CLIENTS['smurf']:
             resp = smurf.take_bgmap.wait()
-            check_response(resp)
+            check_response(smurf, resp)
 
 
 def take_noise(tag=None, concurrent=True, settling_time=0):
@@ -228,15 +228,15 @@ def take_noise(tag=None, concurrent=True, settling_time=0):
         smurf.take_noise.start(tag=tag)
         if not concurrent:
             resp = smurf.take_noise.wait()
-            check_response(resp)
+            check_response(smurf, resp)
 
             # Allow cryo to settle
             time.sleep(settling_time)
 
     if concurrent:
         for smurf in run.CLIENTS['smurf']:
             resp = smurf.take_noise.wait()
-            check_response(resp)
+            check_response(smurf, resp)
 
 
 def stream(state, tag=None, subtype=None):
@@ -257,4 +257,4 @@ def stream(state, tag=None, subtype=None):
         for smurf in run.CLIENTS['smurf']:
             smurf.stream.stop()
             resp = smurf.stream.wait()
-            check_response(resp)
+            check_response(smurf, resp)

tests/test__internal.py

@@ -21,23 +21,29 @@ def create_session(op_name, success=None):
     return session.encoded()
 
 
-invalid_responses = [(OCSReply(ocs.TIMEOUT,
-                               'msg',
-                               create_session('test', success=True))),
-                     (OCSReply(ocs.ERROR,
-                               'msg',
-                               create_session('test')))]
+# Very partial mock as we only need the instance_id to exist.
+class MockClient:
+    def __init__(self):
+        self.instance_id = 'test-id'
+
+
+invalid_responses = [(MockClient(), OCSReply(ocs.TIMEOUT,
+                                             'msg',
+                                             create_session('test', success=True))),
+                     (MockClient(), OCSReply(ocs.ERROR,
+                                             'msg',
+                                             create_session('test')))]
 
 valid_responses = [
-    (OCSReply(ocs.OK, 'msg', create_session('test', success=True)))]
+    (MockClient(), OCSReply(ocs.OK, 'msg', create_session('test', success=True)))]
 
 
-@pytest.mark.parametrize("response", invalid_responses)
-def test_check_response_raises(response):
+@pytest.mark.parametrize("client,response", invalid_responses)
+def test_check_response_raises(client, response):
     with pytest.raises(RuntimeError):
-        check_response(response)
+        check_response(client, response)
 
 
-@pytest.mark.parametrize("response", valid_responses)
-def test_check_response(response):
-    check_response(response)
+@pytest.mark.parametrize("client,response", valid_responses)
+def test_check_response(client, response):
+    check_response(client, response)