improve and fix errors with parameter limits

- in order to work properly, readonly=True in limit parameters
  has to be set before creating the write_* method
- more explicit: Use e.g. target_max=Limit()
- fix an error in the loop over the base classes when creating
  the check_* method
- more concise error message when a limit is violated
+ fix an error in playground when using persistent parameters

Change-Id: Ibd557b55d6c0d9a2612cda4460b16e3c70e1bc9e
Reviewed-on: https://forge.frm2.tum.de/review/c/secop/frappy/+/31017
Tested-by: Jenkins Automated Tests <pedersen+jenkins@frm2.tum.de>
Reviewed-by: Markus Zolliker <markus.zolliker@psi.ch>

frappy/core.py

@@ -31,11 +31,11 @@ from frappy.datatypes import ArrayOf, BLOBType, BoolType, EnumType, \
 from frappy.lib.enum import Enum
 from frappy.modules import Attached, Communicator, \
     Done, Drivable, Feature, Module, Readable, Writable, HasAccessibles
-from frappy.params import Command, Parameter
+from frappy.params import Command, Parameter, Limit
 from frappy.properties import Property
 from frappy.proxy import Proxy, SecNode, proxy_class
 from frappy.io import HasIO, StringIO, BytesIO, HasIodev  # TODO: remove HasIodev (legacy stuff)
-from frappy.persistent import PersistentMixin, PersistentParam
+from frappy.persistent import PersistentMixin, PersistentParam, PersistentLimit
 from frappy.rwhandler import ReadHandler, WriteHandler, CommonReadHandler, \
     CommonWriteHandler, nopoll
 

frappy/modules.py

@@ -34,7 +34,7 @@ from frappy.errors import BadValueError, CommunicationFailedError, ConfigError,
     ProgrammingError, SECoPError, secop_error, RangeError
 from frappy.lib import formatException, mkthread, UniqueObject
 from frappy.lib.enum import Enum
-from frappy.params import Accessible, Command, Parameter
+from frappy.params import Accessible, Command, Parameter, Limit
 from frappy.properties import HasProperties, Property
 from frappy.logging import RemoteLogHandler, HasComlog
 
@@ -161,7 +161,7 @@ class HasAccessibles(HasProperties):
                     # find the base class, where the parameter <limname> is defined first.
                     # we have to check all bases, as they may not be treated yet when
                     # not inheriting from HasAccessibles
-                    base = next(b for b in reversed(base.__mro__) if limname in b.__dict__)
+                    base = next(b for b in reversed(cls.__mro__) if limname in b.__dict__)
                     if cname not in base.__dict__:
                         # there is no check method yet at this class
                         # add check function to the class where the limit was defined
@@ -431,28 +431,19 @@ class Module(HasAccessibles):
             self.valueCallbacks[pname] = []
             self.errorCallbacks[pname] = []
 
-            if not pobj.hasDatatype():
-                head, _, postfix = pname.rpartition('_')
-                if postfix not in ('min', 'max', 'limits'):
-                    errors.append(f'{pname} needs a datatype')
-                    continue
-                # when datatype is not given, properties are set automagically
-                pobj.setProperty('readonly', False)
-                baseparam = self.parameters.get(head)
+            if isinstance(pobj, Limit):
+                basepname = pname.rpartition('_')[0]
+                baseparam = self.parameters.get(basepname)
                 if not baseparam:
-                    errors.append(f'parameter {pname!r} is given, but not {head!r}')
+                    errors.append(f'limit {pname!r} is given, but not {basepname!r}')
                     continue
-                dt = baseparam.datatype
-                if dt is None:
+                if baseparam.datatype is None:
                     continue  # an error will be reported on baseparam
-                if postfix == 'limits':
-                    pobj.setProperty('datatype', TupleOf(dt, dt))
-                    pobj.setProperty('default', (dt.min, dt.max))
-                else:
-                    pobj.setProperty('datatype', dt)
-                    pobj.setProperty('default', getattr(dt, postfix))
-                if not pobj.description:
-                    pobj.setProperty('description', f'limit for {pname}')
+                pobj.set_datatype(baseparam.datatype)
+
+            if not pobj.hasDatatype():
+                errors.append(f'{pname} needs a datatype')
+                continue
 
             if pobj.value is None:
                 if pobj.needscfg:
@@ -805,11 +796,11 @@ class Module(HasAccessibles):
                 raise ValueError('remote handler not found')
         self.remoteLogHandler.set_conn_level(self, conn, level)
 
-    def checkLimits(self, value, parametername='target'):
+    def checkLimits(self, value, pname='target'):
         """check for limits
 
-        :param value: the value to be checked for <parametername>_min <= value <= <parametername>_max
-        :param parametername: parameter name, default is 'target'
+        :param value: the value to be checked for <pname>_min <= value <= <pname>_max
+        :param pname: parameter name, default is 'target'
 
         raises RangeError in case the value is not valid
 
@@ -818,14 +809,20 @@ class Module(HasAccessibles):
         when no automatic super call is desired.
         """
         try:
-            min_, max_ = getattr(self, parametername + '_limits')
+            min_, max_ = getattr(self, pname + '_limits')
+            if not min_ <= value <= max_:
+                raise RangeError(f'{pname} outside {pname}_limits')
+            return
         except AttributeError:
-            min_ = getattr(self, parametername + '_min', float('-inf'))
-            max_ = getattr(self, parametername + '_max', float('inf'))
-        if not min_ <= value <= max_:
-            if min_ > max_:
-                raise RangeError(f'invalid limits: [{min_:g}, {max_:g}]')
-            raise RangeError(f'limits violation: {value:g} outside [{min_:g}, {max_:g}]')
+            pass
+        min_ = getattr(self, pname + '_min', float('-inf'))
+        max_ = getattr(self, pname + '_max', float('inf'))
+        if min_ > max_:
+            raise RangeError(f'invalid limits: {pname}_min > {pname}_max')
+        if value < min_:
+            raise RangeError(f'{pname} below {pname}_min')
+        if value > max_:
+            raise RangeError(f'{pname} above {pname}_max')
 
 
 class Readable(Module):

frappy/params.py

@@ -514,6 +514,35 @@ class Command(Accessible):
         return result[:-1] + f', {self.func!r})' if self.func else result
 
 
+class Limit(Parameter):
+    """a special limit parameter"""
+    POSTFIXES = {'min', 'max', 'limits'}  # allowed postfixes
+
+    def __set_name__(self, owner, name):
+        super().__set_name__(owner, name)
+        head, _, postfix = name.rpartition('_')
+        if postfix not in self.POSTFIXES:
+            raise ProgrammingError(f'Limit name must end with one of {self.POSTFIXES}')
+        if 'readonly' not in self.propertyValues:
+            self.readonly = False
+        if not self.description:
+            self.description = f'limit for {head}'
+        if self.export.startswith('_') and PREDEFINED_ACCESSIBLES.get(head):
+            self.export = self.export[1:]
+
+    def set_datatype(self, datatype):
+        if self.hasDatatype():
+            return  # the programmer is responsible that a given datatype is correct
+        postfix = self.name.rpartition('_')[-1]
+        postfix = self.name.rpartition('_')[-1]
+        if postfix == 'limits':
+            self.datatype = TupleOf(datatype, datatype)
+            self.default = (datatype.min, datatype.max)
+        else:  # min, max
+            self.datatype = datatype
+            self.default = getattr(datatype, postfix)
+
+
 # list of predefined accessibles with their type
 PREDEFINED_ACCESSIBLES = {
     'value': Parameter,

frappy/persistent.py

@@ -57,7 +57,7 @@ import json
 
 from frappy.lib import generalConfig
 from frappy.datatypes import EnumType
-from frappy.params import Parameter, Property, Command
+from frappy.params import Parameter, Property, Command, Limit
 from frappy.modules import Module
 
 
@@ -67,6 +67,10 @@ class PersistentParam(Parameter):
     given = False
 
 
+class PersistentLimit(Limit, Parameter):
+    pass
+
+
 class PersistentMixin(Module):
     persistentData = None  # dict containing persistent data after startup
 

frappy/playground.py

@@ -81,6 +81,7 @@ class Dispatcher:
     def __init__(self, name, log, opts, srv):
         self.log = log
         self._modules = {}
+        self.equipment_id = opts.pop('equipment_id', name)
 
     def announce_update(self, modulename, pname, pobj):
         if pobj.readerror:

test/test_modules.py

@@ -29,7 +29,7 @@ import pytest
 from frappy.datatypes import BoolType, FloatRange, StringType, IntRange, ScaledInteger
 from frappy.errors import ProgrammingError, ConfigError, RangeError
 from frappy.modules import Communicator, Drivable, Readable, Module
-from frappy.params import Command, Parameter
+from frappy.params import Command, Parameter, Limit
 from frappy.rwhandler import ReadHandler, WriteHandler, nopoll
 from frappy.lib import generalConfig
 
@@ -795,17 +795,17 @@ stdlim = {
 
 class Lim(Module):
     a = Parameter('', FloatRange(-10, 10), readonly=False, default=0)
-    a_min = Parameter()
-    a_max = Parameter()
+    a_min = Limit()
+    a_max = Limit()
 
     b = Parameter('', FloatRange(0, None), readonly=False, default=0)
-    b_min = Parameter()
+    b_min = Limit()
 
     c = Parameter('', IntRange(None, 100), readonly=False, default=0)
-    c_max = Parameter()
+    c_max = Limit()
 
     d = Parameter('', FloatRange(-5, 5), readonly=False, default=0)
-    d_limits = Parameter()
+    d_limits = Limit()
 
     e = Parameter('', IntRange(0, 8), readonly=False, default=0)
 
@@ -872,8 +872,8 @@ def test_limit_inheritance():
                 raise ValueError('value is not a multiple of 0.25')
 
     class Mixin:
-        a_min = Parameter()
-        a_max = Parameter()
+        a_min = Limit()
+        a_max = Limit()
 
     class Mod(Mixin, Base):
         def check_a(self, value):