From aa82bc580dc40ebf0b71ff6c4f05e8989a2a5c0f Mon Sep 17 00:00:00 2001
From: Markus Zolliker <markus.zolliker@psi.ch>
Date: Tue, 1 Mar 2022 09:52:49 +0100
Subject: [PATCH] proper return value in handler read_* methods

wrapped read_* methods must always return a value

+ do not copy __name__ attribute of handler method to wrapped method

Change-Id: I54cd4b37cf7452621ee734be393aec4611fe809b
Reviewed-on: https://forge.frm2.tum.de/review/c/sine2020/secop/playground/+/27870
Tested-by: Jenkins Automated Tests <pedersen+jenkins@frm2.tum.de>
Reviewed-by: Enrico Faulhaber <enrico.faulhaber@frm2.tum.de>
Reviewed-by: Markus Zolliker <markus.zolliker@psi.ch>
---
 secop/rwhandler.py   | 21 ++++++++++++++++-----
 test/test_handler.py | 37 ++++++++++++++++++++++++++++++++++---
 2 files changed, 50 insertions(+), 8 deletions(-)

diff --git a/secop/rwhandler.py b/secop/rwhandler.py
index aab5ecd..1727adc 100644
--- a/secop/rwhandler.py
+++ b/secop/rwhandler.py
@@ -53,11 +53,19 @@ Example 2: addressable HW parameters
         return self.get_hw_register(HW_ADDR[pname])
 """
 
-from functools import wraps
+import functools
 from secop.modules import Done
 from secop.errors import ProgrammingError
 
 
+def wraps(func):
+    """decorator to copy function attributes of wrapped function"""
+    # we modify the default here:
+    # copy __doc__ , __module___ and attributes from __dict__
+    # but not __name__ and __qualname__
+    return functools.wraps(func, assigned=('__doc__', '__module__'))
+
+
 class Handler:
     func = None
     method_names = set()  # this is shared among all instances of handlers!
@@ -120,9 +128,11 @@ class ReadHandler(Handler):
     def wrap(self, key):
         def method(module, pname=key, func=self.func):
             value = func(module, pname)
-            if value is not Done:
-                setattr(module, pname, value)
+            if value is Done:
+                return getattr(module, pname)
+            setattr(module, pname, value)
             return value
+
         return wraps(self.func)(method)
 
 
@@ -137,13 +147,14 @@ class CommonReadHandler(ReadHandler):
         self.first_key = next(iter(keys))
 
     def wrap(self, key):
-        def method(module, func=self.func):
+        def method(module, pname=key, func=self.func):
             ret = func(module)
             if ret not in (None, Done):
                 raise ProgrammingError('a method wrapped with CommonReadHandler must not return any value')
+            return getattr(module, pname)
 
         method = wraps(self.func)(method)
-        method.poll = self.poll if key == self.first_key else False
+        method.poll = self.poll and getattr(method, 'poll', True) if key == self.first_key else False
         return method
 
 
diff --git a/test/test_handler.py b/test/test_handler.py
index 5a7b48f..447be0d 100644
--- a/test/test_handler.py
+++ b/test/test_handler.py
@@ -23,7 +23,7 @@
 
 from secop.rwhandler import ReadHandler, WriteHandler, \
     CommonReadHandler, CommonWriteHandler, nopoll
-from secop.core import Module, Parameter, FloatRange
+from secop.core import Module, Parameter, FloatRange, Done
 
 
 class DispatcherStub:
@@ -104,6 +104,10 @@ def test_handler():
     assert m.b == 7
     assert data.pop() == 'b'
 
+    data.append(Done)
+    assert m.read_b() == 7
+    assert data.pop() == 'b'
+
     assert data == []
 
 
@@ -141,13 +145,16 @@ def test_common_handler():
     assert data.pop() == 'write_hdl'
 
     data.append((3, 4))
-    m.read_a()
+    assert m.read_a() == 3
     assert m.a == 3
     assert m.b == 4
     assert data.pop() == 'read_hdl'
+    data.append((5, 6))
+    assert m.read_b() == 6
+    assert data.pop() == 'read_hdl'
 
     data.append((1.1, 2.2))
-    m.read_b()
+    assert m.read_b() == 2.2
     assert m.a == 1.1
     assert m.b == 2.2
     assert data.pop() == 'read_hdl'
@@ -201,3 +208,27 @@ def test_nopoll():
 
     assert Mod4.read_a.poll is False
     assert Mod4.read_b.poll is False
+
+    class Mod5(ModuleTest):
+        a = Parameter('', FloatRange(), readonly=False)
+        b = Parameter('', FloatRange(), readonly=False)
+
+        @CommonReadHandler(['a', 'b'])
+        @nopoll
+        def read_hdl(self):
+            pass
+
+    assert Mod5.read_a.poll is False
+    assert Mod5.read_b.poll is False
+
+    class Mod6(ModuleTest):
+        a = Parameter('', FloatRange(), readonly=False)
+        b = Parameter('', FloatRange(), readonly=False)
+
+        @nopoll
+        @CommonReadHandler(['a', 'b'])
+        def read_hdl(self):
+            pass
+
+    assert Mod6.read_a.poll is False
+    assert Mod6.read_b.poll is False