various fixes

- nodestatechange callback must appear after the online attribute
  is changed
- IntRange: move range validation outside of try except
- fixes on some error names
- well defined error message 'no such class' in secop.lib.get_class
- try again 4 times when starting Tcp Server on EADDRINUSE
- fix error handling

Change-Id: I4eee9b79ea8173936b9f5193b87e006ac8fca827
Reviewed-on: https://forge.frm2.tum.de/review/c/sine2020/secop/playground/+/26171
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/client/__init__.py

@@ -493,14 +493,12 @@ class SecopClient(ProxyClient):
             self.log.warning('unhandled message: %s %s %r' % (action, ident, data))
 
     def _set_state(self, online, state=None):
-        # treat reconnecting as online!
+        # remark: reconnecting is treated as online
-        state = state or self.state
-        self.callback(None, 'nodeStateChange', online, state)
-        for mname in self.modules:
-            self.callback(mname, 'nodeStateChange', online, state)
-        # set online attribute after callbacks -> callback may check for old state
         self.online = online
-        self.state = state
+        self.state = state or self.state
+        self.callback(None, 'nodeStateChange', self.online, self.state)
+        for mname in self.modules:
+            self.callback(mname, 'nodeStateChange', self.online, self.state)
 
     def queue_request(self, action, ident=None, data=None):
         """make a request"""

secop/datatypes.py

@@ -102,7 +102,7 @@ class DataType(HasProperties):
                 self.setProperty(k, v)
             self.checkProperties()
         except Exception as e:
-            raise ProgrammingError(str(e))
+            raise ProgrammingError(str(e)) from None
 
     def get_info(self, **kwds):
         """prepare dict for export or repr
@@ -194,7 +194,7 @@ class FloatRange(DataType):
         try:
             value = float(value)
         except Exception:
-            raise BadValueError('Can not __call__ %r to float' % value)
+            raise BadValueError('Can not __call__ %r to float' % value) from None
         # map +/-infty to +/-max possible number
         value = clamp(-sys.float_info.max, value, sys.float_info.max)
 
@@ -267,12 +267,12 @@ class IntRange(DataType):
         try:
             fvalue = float(value)
             value = int(value)
-            if not self.min <= value <= self.max or round(fvalue) != fvalue:
-                raise BadValueError('%r should be an int between %d and %d' %
-                                    (value, self.min, self.max))
-            return value
         except Exception:
-            raise BadValueError('Can not convert %r to int' % value)
+            raise BadValueError('Can not convert %r to int' % value) from None
+        if not self.min <= value <= self.max or round(fvalue) != fvalue:
+            raise BadValueError('%r should be an int between %d and %d' %
+                                (value, self.min, self.max))
+        return value
 
     def __repr__(self):
         args = (self.min, self.max)
@@ -371,7 +371,7 @@ class ScaledInteger(DataType):
         try:
             value = float(value)
         except Exception:
-            raise BadValueError('Can not convert %r to float' % value)
+            raise BadValueError('Can not convert %r to float' % value) from None
         prec = max(self.scale, abs(value * self.relative_resolution),
                    self.absolute_resolution)
         if self.min - prec <= value <= self.max + prec:
@@ -454,7 +454,7 @@ class EnumType(DataType):
         try:
             return self._enum[value]
         except (KeyError, TypeError):  # TypeError will be raised when value is not hashable
-            raise BadValueError('%r is not a member of enum %r' % (value, self._enum))
+            raise BadValueError('%r is not a member of enum %r' % (value, self._enum)) from None
 
     def from_string(self, text):
         return self(text)
@@ -533,7 +533,7 @@ class BLOBType(DataType):
             if self.minbytes < other.minbytes or self.maxbytes > other.maxbytes:
                 raise BadValueError('incompatible datatypes')
         except AttributeError:
-            raise BadValueError('incompatible datatypes')
+            raise BadValueError('incompatible datatypes') from None
 
 
 class StringType(DataType):
@@ -572,7 +572,7 @@ class StringType(DataType):
             try:
                 value.encode('ascii')
             except UnicodeEncodeError:
-                raise BadValueError('%r contains non-ascii character!' % value)
+                raise BadValueError('%r contains non-ascii character!' % value) from None
         size = len(value)
         if size < self.minchars:
             raise BadValueError(
@@ -606,7 +606,7 @@ class StringType(DataType):
                     self.isUTF8 > other.isUTF8:
                 raise BadValueError('incompatible datatypes')
         except AttributeError:
-            raise BadValueError('incompatible datatypes')
+            raise BadValueError('incompatible datatypes') from None
 
 
 # TextType is a special StringType intended for longer texts (i.e. embedding \n),
@@ -618,7 +618,7 @@ class TextType(StringType):
     def __init__(self, maxchars=None):
         if maxchars is None:
             maxchars = UNLIMITED
-        super(TextType, self).__init__(0, maxchars)
+        super().__init__(0, maxchars)
 
     def __repr__(self):
         if self.maxchars == UNLIMITED:
@@ -771,7 +771,7 @@ class ArrayOf(DataType):
                 raise BadValueError('incompatible datatypes')
             self.members.compatible(other.members)
         except AttributeError:
-            raise BadValueError('incompatible datatypes')
+            raise BadValueError('incompatible datatypes') from None
 
 
 class TupleOf(DataType):
@@ -811,7 +811,7 @@ class TupleOf(DataType):
             return tuple(sub(elem)
                          for sub, elem in zip(self.members, value))
         except Exception as exc:
-            raise BadValueError('Can not validate:', str(exc))
+            raise BadValueError('Can not validate:', str(exc)) from None
 
     def export_value(self, value):
         """returns a python object fit for serialisation"""
@@ -894,7 +894,7 @@ class StructOf(DataType):
             return ImmutableDict((str(k), self.members[k](v))
                                  for k, v in list(value.items()))
         except Exception as exc:
-            raise BadValueError('Can not validate %s: %s' % (repr(value), str(exc)))
+            raise BadValueError('Can not validate %s: %s' % (repr(value), str(exc))) from None
 
     def export_value(self, value):
         """returns a python object fit for serialisation"""
@@ -924,7 +924,7 @@ class StructOf(DataType):
             if mandatory:
                 raise BadValueError('incompatible datatypes')
         except (AttributeError, TypeError, KeyError):
-            raise BadValueError('incompatible datatypes')
+            raise BadValueError('incompatible datatypes') from None
 
 
 class CommandType(DataType):
@@ -987,7 +987,7 @@ class CommandType(DataType):
             if self.result != other.result:  # not both are None
                 other.result.compatible(self.result)
         except AttributeError:
-            raise BadValueError('incompatible datatypes')
+            raise BadValueError('incompatible datatypes') from None
 
 
 # internally used datatypes (i.e. only for programming the SEC-node)
@@ -1164,9 +1164,9 @@ def get_datatype(json, pname=''):
             kwargs = json.copy()
             base = kwargs.pop('type')
         except (TypeError, KeyError, AttributeError):
-            raise BadValueError('a data descriptor must be a dict containing a "type" key, not %r' % json)
+            raise BadValueError('a data descriptor must be a dict containing a "type" key, not %r' % json) from None
 
     try:
         return DATATYPES[base](pname=pname, **kwargs)
     except Exception as e:
-        raise BadValueError('invalid data descriptor: %r (%s)' % (json, str(e)))
+        raise BadValueError('invalid data descriptor: %r (%s)' % (json, str(e))) from None

secop/errors.py

@@ -143,7 +143,7 @@ EXCEPTIONS = dict(
     NoSuchCommand=NoSuchCommandError,
     CommandFailed=CommandFailedError,
     CommandRunning=CommandRunningError,
-    Readonly=ReadOnlyError,
+    ReadOnly=ReadOnlyError,
     BadValue=BadValueError,
     CommunicationFailed=CommunicationFailedError,
     HardwareError=HardwareError,
@@ -151,7 +151,8 @@ EXCEPTIONS = dict(
     IsError=IsErrorError,
     Disabled=DisabledError,
     SyntaxError=ProtocolError,
-    NotImplementedError=NotImplementedError,
+    NotImplemented=NotImplementedError,
+    ProtocolError=ProtocolError,
     InternalError=InternalError,
     # internal short versions (candidates for spec)
     Protocol=ProtocolError,

secop/lib/__init__.py

@@ -105,7 +105,10 @@ def get_class(spec):
     else:
         # rarely needed by now....
         module = importlib.import_module('secop.' + modname)
-    return getattr(module, classname)
+    try:
+        return getattr(module, classname)
+    except AttributeError:
+        raise AttributeError('no such class') from None
 
 
 def mkthread(func, *args, **kwds):

secop/modules.py

@@ -99,10 +99,13 @@ class HasAccessibles(HasProperties):
             rfunc_handler = pobj.handler.get_read_func(cls, pname) if pobj.handler else None
             wrapped = hasattr(rfunc, '__wrapped__')
             if rfunc_handler:
-                if rfunc and not wrapped:
+                if 'read_' + pname in cls.__dict__:
-                    raise ProgrammingError("parameter '%s' can not have a handler "
+                    if pname in cls.__dict__:
-                                           "and read_%s" % (pname, pname))
+                        raise ProgrammingError("parameter '%s' can not have a handler "
-                rfunc = rfunc_handler
+                                               "and read_%s" % (pname, pname))
+                    # read_<pname> overwrites inherited handler
+                else:
+                    rfunc = rfunc_handler
                 wrapped = False
 
             # create wrapper except when read function is already wrapped
@@ -388,7 +391,7 @@ class Module(HasAccessibles):
                 if k in self.parameters or k in self.propertyDict:
                     setattr(self, k, v)
                     cfgdict.pop(k)
-            except (ValueError, TypeError):
+            except (ValueError, TypeError) as e:
                 # self.log.exception(formatExtendedStack())
                 errors.append('module %s, parameter %s: %s' % (self.name, k, e))
 

secop/params.py

@@ -39,10 +39,6 @@ class Accessible(HasProperties):
 
     kwds = None  # is a dict if it might be used as Override
 
-    def __init__(self, **kwds):
-        super().__init__()
-        self.init(kwds)
-
     def init(self, kwds):
         # do not use self.propertyValues.update here, as no invalid values should be
         # assigned to properties, even not before checkProperties
@@ -101,10 +97,10 @@ class Parameter(Accessible):
 
     description = Property(
         'mandatory description of the parameter', TextType(),
-        extname='description', mandatory=True)
+        extname='description', mandatory=True, export='always')
     datatype = Property(
         'datatype of the Parameter (SECoP datainfo)', DataTypeType(),
-        extname='datainfo', mandatory=True)
+        extname='datainfo', mandatory=True, export='always')
     readonly = Property(
         'not changeable via SECoP (default True)', BoolType(),
         extname='readonly', default=True, export='always')
@@ -165,7 +161,7 @@ class Parameter(Accessible):
     readerror = None
 
     def __init__(self, description=None, datatype=None, inherit=True, *, unit=None, constant=None, **kwds):
-        super().__init__(**kwds)
+        super().__init__()
         if datatype is not None:
             if not isinstance(datatype, DataType):
                 if isinstance(datatype, type) and issubclass(datatype, DataType):
@@ -178,6 +174,8 @@ class Parameter(Accessible):
             if 'default' in kwds:
                 self.default = datatype(kwds['default'])
 
+        self.init(kwds)  # datatype must be defined before we can treat dataset properties like fmtstr or unit
+
         if description is not None:
             self.description = inspect.cleandoc(description)
 
@@ -187,6 +185,7 @@ class Parameter(Accessible):
         self._constant = constant
 
     def __get__(self, instance, owner):
+        # not used yet
         if instance is None:
             return self
         return instance.parameters[self.name].value
@@ -282,7 +281,7 @@ class Command(Accessible):
 
     description = Property(
         'description of the Command', TextType(),
-        extname='description', export=True, mandatory=True)
+        extname='description', export='always', mandatory=True)
     group = Property(
         'optional command group of the command.', StringType(),
         extname='group', export=True, default='')
@@ -312,7 +311,8 @@ class Command(Accessible):
     func = None
 
     def __init__(self, argument=False, *, result=None, inherit=True, **kwds):
-        super().__init__(**kwds)
+        super().__init__()
+        self.init(kwds)
         if result or kwds or isinstance(argument, DataType) or not callable(argument):
             # normal case
             if argument is False and result:

secop/poller.py

@@ -202,7 +202,7 @@ class Poller(PollerBase):
                 module.pollOneParam(pname)
                 done = True
                 lastdue = due
-                due = max(lastdue + mininterval, now + min(self.maxwait, mininterval * 0.5))
+            due = max(lastdue + mininterval, now + min(self.maxwait, mininterval * 0.5))
             # replace due, lastdue with new values and sort in
             heapreplace(queue, (due, lastdue, pollitem))
         return 0
@@ -227,9 +227,10 @@ class Poller(PollerBase):
         (where n is the number of regular parameters).
         """
         if not self:
-            # nothing to do (else we might call time.sleep(float('inf')) below
+            # nothing to do (else time.sleep(float('inf')) might be called below
             started_callback()
             return
+        # if writeInitParams is not yet done, we do it here
         for module in self.modules:
             module.writeInitParams()
         # do all polls once and, at the same time, insert due info

secop/protocol/interface/tcp.py

@@ -25,6 +25,8 @@ import socket
 import socketserver
 import sys
 import threading
+import time
+import errno
 
 from secop.datatypes import BoolType, StringType
 from secop.errors import SECoPError
@@ -184,8 +186,19 @@ class TCPServer(socketserver.ThreadingTCPServer):
         port = int(options.pop('uri').split('://', 1)[-1])
         self.detailed_errors = options.pop('detailed_errors', False)
 
-        self.allow_reuse_address = True
         self.log.info("TCPServer %s binding to port %d" % (name, port))
-        socketserver.ThreadingTCPServer.__init__(
+        for ntry in range(5):
-            self, ('0.0.0.0', port), TCPRequestHandler, bind_and_activate=True)
+            try:
+                socketserver.ThreadingTCPServer.__init__(
+                    self, ('0.0.0.0', port), TCPRequestHandler, bind_and_activate=True)
+                break
+            except OSError as e:
+                if e.args[0] == errno.EADDRINUSE:  # address already in use
+                    # this may happen despite of allow_reuse_address
+                    time.sleep(0.3 * (1 << ntry))  # max accumulated sleep time: 0.3 * 31 = 9.3 sec
+                else:
+                    self.log.error('could not initialize TCP Server: %r' % e)
+                    raise
+        if ntry:
+            self.log.warning('tried again %d times after "Address already in use"' % ntry)
         self.log.info("TCPServer initiated")

secop/server.py

@@ -256,7 +256,7 @@ class Server:
                     self.modules[modname] = modobj
                 except ConfigError as e:
                     errors.append('error creating module %s:' % modname)
-                    for errtxt in e.args[0]:
+                    for errtxt in e.args[0] if isinstance(e.args[0], list) else [e.args[0]]:
                         errors.append('  ' + errtxt)
                 except Exception:
                     failure_traceback = traceback.format_exc()