Code review changes from Michael. Includes:

- SetBlocking returns the old blocking state value. Added test.
- Fix some style issues
- Make completion function on the dispatcher optional, to maintain some
compatibility with the previous API

Author: AlexanderWells-diamond

docs/reference/api.rst

@@ -377,10 +377,16 @@ record creation function.
 
 .. function:: SetBlocking(blocking)
 
-    This can be used to globally set the default `blocking` flag, which will
-    apply to all records created after this point. This allows blocking to be
+    This can be used to globally set the default of the `blocking` flag, which
+    will apply to all records created after this point. This allows blocking to be
     easily set/unset when creating groups of records.
 
+    Returns the previous value of the `blocking` flag, which enables code like this::
+
+        old_blocking = SetBlocking(new_blocking)
+        create_records()
+        SetBlocking(old_blocking)
+
     This does not change the blocking value for any already created records.
 
     .. seealso::

softioc/asyncio_dispatcher.py

@@ -32,18 +32,19 @@ def aioJoin(worker=worker, loop=self.loop):
         else:
             self.loop = loop
 
-    def __call__(self, func, completion, func_args=(), completion_args=()):
+    def __call__(
+            self,
+            func,
+            func_args=(),
+            completion = None,
+            completion_args=()):
         async def async_wrapper():
             try:
-                if inspect.iscoroutinefunction(func):
-                    await func(*func_args)
-                else:
-                    ret = func(*func_args)
-                    # Handle the case of a synchronous function that returns a
-                    # coroutine, like the lambda for on_update_name does
-                    if inspect.isawaitable(ret):
-                        await ret
-                completion(*completion_args)
+                ret = func(*func_args)
+                if inspect.isawaitable(ret):
+                    await ret
+                if completion:
+                    completion(*completion_args)
             except Exception:
                 logging.exception("Exception when awaiting callback")
         asyncio.run_coroutine_threadsafe(async_wrapper(), self.loop)

softioc/cothread_dispatcher.py

@@ -10,8 +10,14 @@ def __init__(self):
         # processing doesn't interfere with other callback processing.
         self.__dispatcher = cothread.cothread._Callback()
 
-    def __call__(self, func, completion, func_args=(), completion_args=()):
+    def __call__(
+            self,
+            func,
+            func_args=(),
+            completion = None,
+            completion_args=()):
         def wrapper():
             func(*func_args)
-            completion(*completion_args)
+            if completion:
+                completion(*completion_args)
         self.__dispatcher(wrapper)

softioc/device.py

@@ -4,7 +4,6 @@
 import ctypes
 from ctypes import *
 import numpy
-from threading import Event
 
 from . import alarm
 from . import fields
@@ -19,17 +18,19 @@
 
 
 # This is set from softioc.iocInit
-# dispatcher(func, *args) will queue a callback to happen
 dispatcher = None
 
 # Global blocking flag, used to mark asynchronous (False) or synchronous (True)
 # processing modes for Out records.
 # Default False to maintain behaviour from previous versions.
 blocking = False
 
-def SetBlocking(val):
+# Set the current global blocking flag, and return the previous value.
+def SetBlocking(new_val):
     global blocking
-    blocking = val
+    old_val = blocking
+    blocking = new_val
+    return old_val
 
 
 # EPICS processing return codes
@@ -163,7 +164,6 @@ def __init__(self, name, **kargs):
         if self._blocking:
             self._callback = create_callback_capsule()
 
-
         self.__super.__init__(name, **kargs)
 
     def init_record(self, record):
@@ -188,7 +188,6 @@ def __completion(self, record):
         '''Signals that all on_update processing is finished'''
         if self._blocking:
             signal_processing_complete(record, self._callback)
-        pass
 
     def _process(self, record):
         '''Processing suitable for output records.  Performs immediate value
@@ -218,10 +217,9 @@ def _process(self, record):
                 record.PACT = self._blocking
                 dispatcher(
                     self.__on_update,
-                    self.__completion,
                     func_args=(python_value,),
-                    completion_args=(record,)
-                )
+                    completion = self.__completion,
+                    completion_args=(record,))
 
             return EPICS_OK
 

softioc/imports.py

@@ -35,8 +35,7 @@ def signal_processing_complete(record, callback):
     _extension.signal_processing_complete(
         record.PRIO,
         record.record.value,
-        callback
-    )
+        callback)
 
 def expect_success(status, function, args):
     assert status == 0, 'Expected success'

tests/test_records.py

@@ -536,6 +536,20 @@ def test_blocking_global_flag_creates_attributes(self):
         bo3 = builder.boolOut("OUTREC3", blocking=True)
         self.check_record_blocking_attributes(bo3)
 
+    def test_blocking_returns_old_state(self):
+        """Test that SetBlocking returns the previously set value"""
+        old_val = SetBlocking(True)
+        assert old_val is False  # Default is False
+
+        old_val = SetBlocking(False)
+        assert old_val is True
+
+        # Test it correctly maintains state when passed the current value
+        old_val = SetBlocking(False)
+        assert old_val is False
+        old_val = SetBlocking(False)
+        assert old_val is False
+
     def blocking_test_func(self, device_name, conn):
 
         builder.SetDeviceName(device_name)