diff --git a/bin/secop-server b/bin/secop-server index 6866b34..ed218af 100755 --- a/bin/secop-server +++ b/bin/secop-server @@ -75,6 +75,11 @@ def parseArgv(argv): action='store_true', help='check cfg files only', default=False) + parser.add_argument('-r', + '--relaxed', + action='store_true', + help='no checking of problematic behaviour', + default=False) return parser.parse_args(argv) @@ -85,6 +90,9 @@ def main(argv=None): args = parseArgv(argv[1:]) loglevel = 'debug' if args.verbose else ('error' if args.quiet else 'info') + if args.relaxed: + generalConfig.defaults['lazy_number_validation'] = True + generalConfig.defaults['disable_value_range_check'] = True generalConfig.init(args.gencfg) logger.init(loglevel) diff --git a/secop/datatypes.py b/secop/datatypes.py index 4de2306..ada53f6 100644 --- a/secop/datatypes.py +++ b/secop/datatypes.py @@ -35,25 +35,20 @@ from secop.lib.enum import Enum from secop.parse import Parser from secop.properties import HasProperties, Property -# Only export these classes for 'from secop.datatypes import *' -__all__ = [ - 'DataType', 'get_datatype', - 'FloatRange', 'IntRange', 'ScaledInteger', - 'BoolType', 'EnumType', - 'BLOBType', 'StringType', 'TextType', - 'TupleOf', 'ArrayOf', 'StructOf', - 'CommandType', 'StatusType', -] - # *DEFAULT* limits for IntRange/ScaledIntegers transport serialisation DEFAULT_MIN_INT = -16777216 DEFAULT_MAX_INT = 16777216 UNLIMITED = 1 << 64 # internal limit for integers, is probably high enough for any datatype size -generalConfig.defaults['lazy_number_validation'] = True # should be changed to False later +generalConfig.defaults['lazy_number_validation'] = False Parser = Parser() +class DiscouragedConversion(BadValueError): + """the discouraged conversion string - > float happened""" + log_message = True + + # base class for all DataTypes class DataType(HasProperties): """base class for all data types""" @@ -64,7 +59,7 @@ class DataType(HasProperties): def __call__(self, value): """check if given value (a python obj) is valid for this datatype - returns the value or raises an appropriate exception""" + returns the (possibly converted) value or raises an appropriate exception""" raise NotImplementedError def from_string(self, text): @@ -193,12 +188,15 @@ class FloatRange(DataType): def __call__(self, value): try: - if generalConfig.lazy_number_validation: - value = float(value) # for legacy code - else: - value += 0.0 # do not accept strings here + value += 0.0 # do not accept strings here except Exception: - raise BadValueError('Can not convert %r to float' % value) from None + try: + value = float(value) + except Exception: + raise BadValueError('Can not convert %r to float' % value) from None + if not generalConfig.lazy_number_validation: + raise DiscouragedConversion('automatic string to float conversion no longer supported') from None + # map +/-infty to +/-max possible number value = clamp(-sys.float_info.max, value, sys.float_info.max) @@ -236,6 +234,18 @@ class FloatRange(DataType): return ' '.join([self.fmtstr % value, unit]) return self.fmtstr % value + def problematic_range(self, target_type): + """check problematic range + + returns True when self.min or self.max is given, not 0 and equal to the same limit on target_type. + """ + value_info = self.get_info() + target_info = target_type.get_info() + minval = value_info.get('min') # None when -infinite + maxval = value_info.get('max') # None when +infinite + return ((minval and minval == target_info.get('min')) or + (maxval and maxval == target_info.get('max'))) + def compatible(self, other): if not isinstance(other, (FloatRange, ScaledInteger)): raise BadValueError('incompatible datatypes') @@ -269,13 +279,16 @@ class IntRange(DataType): def __call__(self, value): try: - if generalConfig.lazy_number_validation: - fvalue = float(value) # for legacy code - else: - fvalue = value + 0.0 # do not accept strings here + fvalue = value + 0.0 # do not accept strings here value = int(value) except Exception: - raise BadValueError('Can not convert %r to int' % value) from None + try: + fvalue = float(value) + value = int(value) + except Exception: + raise BadValueError('Can not convert %r to int' % value) from None + if not generalConfig.lazy_number_validation: + raise DiscouragedConversion('automatic string to float conversion no longer supported') 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)) @@ -305,13 +318,15 @@ class IntRange(DataType): return '%d' % value def compatible(self, other): - if isinstance(other, IntRange): + if isinstance(other, (IntRange, FloatRange, ScaledInteger)): other(self.min) other(self.max) return - # this will accept some EnumType, BoolType - for i in range(self.min, self.max + 1): - other(i) + if isinstance(other, (EnumType, BoolType)): + # the following loop will not cycle more than the number of Enum elements + for i in range(self.min, self.max + 1): + other(i) + raise BadValueError('incompatible datatypes') class ScaledInteger(DataType): @@ -376,12 +391,14 @@ class ScaledInteger(DataType): def __call__(self, value): try: - if generalConfig.lazy_number_validation: - value = float(value) # for legacy code - else: - value += 0.0 # do not accept strings here + value += 0.0 # do not accept strings here except Exception: - raise BadValueError('Can not convert %r to float' % value) from None + try: + value = float(value) + except Exception: + raise BadValueError('Can not convert %r to float' % value) from None + if not generalConfig.lazy_number_validation: + raise DiscouragedConversion('automatic string to float conversion no longer supported') from None prec = max(self.scale, abs(value * self.relative_resolution), self.absolute_resolution) if self.min - prec <= value <= self.max + prec: @@ -1128,37 +1145,26 @@ def floatargs(kwds): DATATYPES = dict( bool = lambda **kwds: BoolType(), - int = lambda min, max, **kwds: IntRange(minval=min, maxval=max), - scaled = lambda scale, min, max, **kwds: ScaledInteger(scale=scale, minval=min*scale, maxval=max*scale, **floatargs(kwds)), - double = lambda min=None, max=None, **kwds: FloatRange(minval=min, maxval=max, **floatargs(kwds)), - blob = lambda maxbytes, minbytes=0, **kwds: BLOBType(minbytes=minbytes, maxbytes=maxbytes), - string = lambda minchars=0, maxchars=None, isUTF8=False, **kwds: StringType(minchars=minchars, maxchars=maxchars, isUTF8=isUTF8), - array = lambda maxlen, members, minlen=0, pname='', **kwds: ArrayOf(get_datatype(members, pname), minlen=minlen, maxlen=maxlen), - tuple = lambda members, pname='', **kwds: TupleOf(*tuple((get_datatype(t, pname) for t in members))), - enum = lambda members, pname='', **kwds: EnumType(pname, members=members), - struct = lambda members, optional=None, pname='', **kwds: StructOf(optional, **dict((n, get_datatype(t, pname)) for n, t in list(members.items()))), - command = lambda argument=None, result=None, pname='', **kwds: CommandType(get_datatype(argument, pname), get_datatype(result)), - limit = lambda members, pname='', **kwds: LimitsType(get_datatype(members, pname)), ) diff --git a/secop/io.py b/secop/io.py index 20b2059..620e01f 100644 --- a/secop/io.py +++ b/secop/io.py @@ -49,7 +49,7 @@ class HasIodev(Module): def __init__(self, name, logger, opts, srv): iodev = opts.get('iodev') - Module.__init__(self, name, logger, opts, srv) + super().__init__(name, logger, opts, srv) if self.uri: opts = {'uri': self.uri, 'description': 'communication device for %s' % name, 'export': False} diff --git a/secop/modules.py b/secop/modules.py index 82a436d..6e2bf29 100644 --- a/secop/modules.py +++ b/secop/modules.py @@ -29,10 +29,10 @@ from collections import OrderedDict from functools import wraps from secop.datatypes import ArrayOf, BoolType, EnumType, FloatRange, \ - IntRange, StatusType, StringType, TextType, TupleOf + IntRange, StatusType, StringType, TextType, TupleOf, DiscouragedConversion from secop.errors import BadValueError, ConfigError, \ ProgrammingError, SECoPError, SilentError, secop_error -from secop.lib import formatException, mkthread, UniqueObject +from secop.lib import formatException, mkthread, UniqueObject, generalConfig from secop.lib.enum import Enum from secop.params import Accessible, Command, Parameter from secop.poller import BasicPoller, Poller @@ -40,6 +40,8 @@ from secop.properties import HasProperties, Property from secop.logging import RemoteLogHandler, HasComlog +generalConfig.defaults['disable_value_range_check'] = False # check for problematic value range by default + Done = UniqueObject('already set') """a special return value for a read/write function @@ -475,9 +477,13 @@ class Module(HasAccessibles): changed = pobj.value != value try: # store the value even in case of error - # TODO: we should neither check limits nor convert string to float here pobj.value = pobj.datatype(value) except Exception as e: + if isinstance(e, DiscouragedConversion): + if DiscouragedConversion.log_message: + self.log.error(str(e)) + self.log.error('you may disable this behaviour by running the server with --relaxed') + DiscouragedConversion.log_message = False if not err: # do not overwrite given error err = e if err: @@ -682,10 +688,30 @@ class Readable(Module): class Writable(Readable): """basic writable module""" - + disable_value_range_check = Property('disable value range check', BoolType(), default=False) target = Parameter('target value of the module', default=0, readonly=False, datatype=FloatRange(unit='$')) + def __init__(self, name, logger, cfgdict, srv): + super().__init__(name, logger, cfgdict, srv) + value_dt = self.parameters['value'].datatype + target_dt = self.parameters['target'].datatype + try: + # this handles also the cases where the limits on the value are more + # restrictive than on the target + target_dt.compatible(value_dt) + except Exception: + if type(value_dt) == type(target_dt): + raise ConfigError('the target range extends beyond the value range') from None + raise ProgrammingError('the datatypes of target and value are not compatible') from None + if isinstance(value_dt, FloatRange): + if (not self.disable_value_range_check and not generalConfig.disable_value_range_check + and value_dt.problematic_range(target_dt)): + self.log.error('the value range must be bigger than the target range!') + self.log.error('you may disable this error message by running the server with --relaxed') + self.log.error('or by setting the disable_value_range_check property of the module to True') + raise ConfigError('the value range must be bigger than the target range') + class Drivable(Writable): """basic drivable module""" diff --git a/test/test_datatypes.py b/test/test_datatypes.py index 0acb5df..097190f 100644 --- a/test/test_datatypes.py +++ b/test/test_datatypes.py @@ -28,7 +28,8 @@ import pytest from secop.datatypes import ArrayOf, BLOBType, BoolType, \ CommandType, ConfigError, DataType, Enum, EnumType, FloatRange, \ IntRange, ProgrammingError, ScaledInteger, StatusType, \ - StringType, StructOf, TextType, TupleOf, get_datatype + StringType, StructOf, TextType, TupleOf, get_datatype, \ + DiscouragedConversion from secop.lib import generalConfig @@ -677,5 +678,5 @@ def test_lazy_validation(dt): generalConfig.defaults['lazy_number_validation'] = True dt('0') generalConfig.defaults['lazy_number_validation'] = False - with pytest.raises(ValueError): + with pytest.raises(DiscouragedConversion): dt('0') diff --git a/test/test_modules.py b/test/test_modules.py index e7e037a..794147d 100644 --- a/test/test_modules.py +++ b/test/test_modules.py @@ -24,13 +24,14 @@ import pytest -from secop.datatypes import BoolType, FloatRange, StringType, IntRange +from secop.datatypes import BoolType, FloatRange, StringType, IntRange, ScaledInteger from secop.errors import ProgrammingError, ConfigError from secop.modules import Communicator, Drivable, Readable, Module from secop.params import Command, Parameter from secop.poller import BasicPoller from secop.lib.multievent import MultiEvent from secop.rwhandler import ReadHandler, WriteHandler +from secop.lib import generalConfig class DispatcherStub: @@ -54,7 +55,7 @@ class DispatcherStub: class LoggerStub: def debug(self, fmt, *args): print(fmt % args) - info = warning = exception = debug + info = warning = exception = error = debug handlers = [] @@ -88,6 +89,7 @@ def test_ModuleMagic(): a1 = Parameter('a1', datatype=BoolType(), default=False) a2 = Parameter('a2', datatype=BoolType(), default=True) value = Parameter(datatype=StringType(), default='first') + target = Parameter(datatype=StringType(), default='') @Command(argument=BoolType(), result=BoolType()) def cmd2(self, arg): @@ -136,6 +138,7 @@ def test_ModuleMagic(): return arg value = Parameter(datatype=FloatRange(unit='deg')) + target = Parameter(datatype=FloatRange(), default=0) a1 = Parameter(datatype=FloatRange(unit='$/s'), readonly=False) b2 = Parameter('', datatype=BoolType(), default=True, poll=True, readonly=False, initwrite=True) @@ -174,7 +177,7 @@ def test_ModuleMagic(): # check for inital updates working properly o1 = Newclass1('o1', logger, {'.description':''}, srv) - expectedBeforeStart = {'target': 0.0, 'status': (Drivable.Status.IDLE, ''), + expectedBeforeStart = {'target': '', 'status': (Drivable.Status.IDLE, ''), 'param1': False, 'param2': 1.0, 'a1': 0.0, 'a2': True, 'pollinterval': 5.0, 'value': 'first'} assert updates.pop('o1') == expectedBeforeStart @@ -191,6 +194,7 @@ def test_ModuleMagic(): o2 = Newclass2('o2', logger, {'.description':'', 'a1': 2.7}, srv) # no update for b2, as this has to be written expectedBeforeStart['a1'] = 2.7 + expectedBeforeStart['target'] = 0.0 assert updates.pop('o2') == expectedBeforeStart o2.earlyInit() event = MultiEvent() @@ -214,7 +218,7 @@ def test_ModuleMagic(): # check '$' in unit works properly assert o2.parameters['a1'].datatype.unit == 'mm/s' cfg = Newclass2.configurables - assert set(cfg.keys()) == {'export', 'group', 'description', + assert set(cfg.keys()) == {'export', 'group', 'description', 'disable_value_range_check', 'meaning', 'visibility', 'implementation', 'interface_classes', 'target', 'stop', 'status', 'param1', 'param2', 'cmd', 'a2', 'pollinterval', 'b2', 'cmd2', 'value', 'a1'} @@ -575,3 +579,65 @@ def test_no_read_write(): assert obj.write_param('egg') == 'egg' assert obj.param == 'egg' assert updates == {'obj': {'param': 'egg'}} + + +def test_incompatible_value_target(): + class Mod1(Drivable): + value = Parameter('', FloatRange(0, 10), default=0) + target = Parameter('', FloatRange(0, 11), default=0) + + class Mod2(Drivable): + value = Parameter('', FloatRange(), default=0) + target = Parameter('', StringType(), default='') + + class Mod3(Drivable): + value = Parameter('', FloatRange(), default=0) + target = Parameter('', ScaledInteger(1, 0, 10), default=0) + + srv = ServerStub({}) + + with pytest.raises(ConfigError): + obj = Mod1('obj', logger, {'description': ''}, srv) # pylint: disable=unused-variable + + with pytest.raises(ProgrammingError): + obj = Mod2('obj', logger, {'description': ''}, srv) + + obj = Mod3('obj', logger, {'description': ''}, srv) + + +def test_problematic_value_range(): + class Mod(Drivable): + value = Parameter('', FloatRange(0, 10), default=0) + target = Parameter('', FloatRange(0, 10), default=0) + + srv = ServerStub({}) + + obj = Mod('obj', logger, {'description': '', 'value.max': 10.1}, srv) # pylint: disable=unused-variable + + with pytest.raises(ConfigError): + obj = Mod('obj', logger, {'description': ''}, srv) + + class Mod2(Drivable): + value = Parameter('', FloatRange(), default=0) + target = Parameter('', FloatRange(), default=0) + + obj = Mod2('obj', logger, {'description': ''}, srv) + obj = Mod2('obj', logger, {'description': '', 'target.min': 0, 'target.max': 10}, srv) + + with pytest.raises(ConfigError): + obj = Mod('obj', logger, { + 'value.min': 0, 'value.max': 10, + 'target.min': 0, 'target.max': 10, 'description': ''}, srv) + + obj = Mod('obj', logger, {'disable_value_range_check': True, + 'value.min': 0, 'value.max': 10, + 'target.min': 0, 'target.max': 10, 'description': ''}, srv) + + generalConfig.defaults['disable_value_range_check'] = True + + class Mod4(Drivable): + value = Parameter('', FloatRange(0, 10), default=0) + target = Parameter('', FloatRange(0, 10), default=0) + obj = Mod4('obj', logger, { + 'value.min': 0, 'value.max': 10, + 'target.min': 0, 'target.max': 10, 'description': ''}, srv)