From 1655e252fcc2f4235632ad1f49a318dca0ee7cad Mon Sep 17 00:00:00 2001 From: Markus Zolliker Date: Wed, 24 Jun 2020 14:15:35 +0200 Subject: [PATCH] fix handling of StructOf datatype - change secop.client.SecopClient to use native types instead of strings for its setParameter and execCommand methods. - secop-gui: for now, setParameter accept strings for complex types. this should be changed to use native type in an other change - fix bugs in parser.py + SecopClient: make visible in an error message that the error was generated on the SEC node + fix a bug when a command is called with 0 as argument Change-Id: Id87d4678311ef8cf43a25153254d36127e16c6d9 Reviewed-on: https://forge.frm2.tum.de/review/c/sine2020/secop/playground/+/23299 Tested-by: Jenkins Automated Tests Reviewed-by: Enrico Faulhaber Reviewed-by: Markus Zolliker --- secop/client/__init__.py | 6 +++--- secop/datatypes.py | 29 ++++++----------------------- secop/gui/mainwindow.py | 8 +++++--- secop/parse.py | 31 +++++++++++++++++++++---------- secop/protocol/dispatcher.py | 2 +- test/test_parse.py | 20 ++++++++++++++++++-- 6 files changed, 54 insertions(+), 42 deletions(-) diff --git a/secop/client/__init__.py b/secop/client/__init__.py index 7b8a402..27d53fd 100644 --- a/secop/client/__init__.py +++ b/secop/client/__init__.py @@ -520,7 +520,7 @@ class SecopClient(ProxyClient): action, _, data = entry[2] # pylint: disable=unpacking-non-sequence if action.startswith(ERRORPREFIX): errcls = self.error_map(data[0]) - raise errcls(data[1]) + raise errcls('on SEC-Node: ' + data[1]) return entry[2] # reply def request(self, action, ident=None, data=None): @@ -552,7 +552,7 @@ class SecopClient(ProxyClient): def setParameter(self, module, parameter, value): self.connect() # make sure we are connected datatype = self.modules[module]['parameters'][parameter]['datatype'] - value = datatype.export_value(datatype.from_string(value)) + value = datatype.export_value(value) self.request(WRITEREQUEST, self.identifier[module, parameter], value) return self.cache[module, parameter] @@ -560,7 +560,7 @@ class SecopClient(ProxyClient): self.connect() # make sure we are connected datatype = self.modules[module]['commands'][command]['datatype'].argument if datatype: - argument = datatype.export_value(datatype.from_string(argument)) + argument = datatype.export_value(argument) else: if argument is not None: raise secop.errors.BadValueError('command has no argument') diff --git a/secop/datatypes.py b/secop/datatypes.py index 0090ce2..9ff6b92 100644 --- a/secop/datatypes.py +++ b/secop/datatypes.py @@ -824,9 +824,6 @@ class StructOf(DataType): if not isinstance(subtype, DataType): raise ProgrammingError( 'StructOf only works with named DataType objs as keyworded arguments!') - if not isinstance(name, str): - raise ProgrammingError( - 'StructOf only works with named DataType objs as keyworded arguments!') for name in self.optional: if name not in members: raise ProgrammingError( @@ -850,16 +847,11 @@ class StructOf(DataType): ['%s=%s' % (n, repr(st)) for n, st in list(self.members.items())]), opt) def __call__(self, value): - """return the validated value or raise - - in principle, we should make it some sort of frozen dict - """ + """return the validated value or raise""" try: - # XXX: handle optional elements !!! - if len(list(value.keys())) != len(list(self.members.keys())): - raise BadValueError( - 'Illegal number of Arguments! Need %d arguments.' % - len(list(self.members.keys()))) + missing = set(self.members) - set(value) - set(self.optional) + if missing: + raise BadValueError('missing values for keys %r' % list(missing)) # validate elements and return as dict return ImmutableDict((str(k), self.members[k](v)) for k, v in list(value.items())) @@ -868,21 +860,13 @@ class StructOf(DataType): def export_value(self, value): """returns a python object fit for serialisation""" - if len(list(value.keys())) != len(list(self.members.keys())): - raise BadValueError( - 'Illegal number of Arguments! Need %d arguments.' % len( - list(self.members.keys()))) + self(value) # check validity return dict((str(k), self.members[k].export_value(v)) for k, v in list(value.items())) def import_value(self, value): """returns a python object from serialisation""" - if len(list(value.keys())) != len(list(self.members.keys())): - raise BadValueError( - 'Illegal number of Arguments! Need %d arguments.' % len( - list(self.members.keys()))) - return ImmutableDict((str(k), self.members[k].import_value(v)) - for k, v in list(value.items())) + return self({str(k): self.members[k].import_value(v) for k, v in value.items()}) def from_string(self, text): value, rem = Parser.parse(text) @@ -1143,5 +1127,4 @@ def get_datatype(json, pname=''): try: return DATATYPES[base](pname=pname, **kwargs) except Exception as e: - print(base, kwargs, repr(e)) raise BadValueError('invalid data descriptor: %r (%s)' % (json, str(e))) diff --git a/secop/gui/mainwindow.py b/secop/gui/mainwindow.py index afda457..b8b69e7 100644 --- a/secop/gui/mainwindow.py +++ b/secop/gui/mainwindow.py @@ -75,14 +75,16 @@ class QSECNode(QObject): return self.modules[module]['parameters'][parameter] def setParameter(self, module, parameter, value): - self.conn.setParameter(module, parameter, value) + # TODO: change the widgets for complex types to no longer use strings + datatype = self.conn.modules[module]['parameters'][parameter]['datatype'] + self.conn.setParameter(module, parameter, datatype.from_string(value)) def getParameter(self, module, parameter): return self.conn.getParameter(module, parameter, True) - def execCommand(self, module, command, arg): + def execCommand(self, module, command, argument): try: - return self.conn.execCommand(module, command, arg) + return self.conn.execCommand(module, command, argument) except Exception as e: return 'ERROR: %r' % e, {} diff --git a/secop/parse.py b/secop/parse.py index 0ce2a8b..4180471 100644 --- a/secop/parse.py +++ b/secop/parse.py @@ -21,7 +21,7 @@ # ***************************************************************************** """parser. used for config files and the gui -can't use ast.literal_eval as it can't handle enums :( +can't use ast.literal_eval as we want less strict syntax (strings without quotes) parsing rules: (...) -> tuple @@ -34,9 +34,13 @@ text -> string 'text' -> string "text" -> string -further convertions are done by the validator of the datatype.... +further conversions are done by the validator of the datatype.... """ +# TODO: should be refactored to use Exceptions instead of None in return tuple +# also it would be better to use functions instead of a class + + from collections import OrderedDict @@ -80,7 +84,7 @@ class Parser: # check escapes! if text[idx - 1] == '\\': continue - return text[1:idx], text[idx + 1:] + return text[1:idx], text[idx + 1:].strip() # unquoted strings are terminated by comma or whitespace idx = 0 @@ -88,7 +92,7 @@ class Parser: if text[idx] in '\x09 ,.;:()[]{}<>-+*/\\!"§$%&=?#~+*\'´`^°|-': break idx += 1 - return text[:idx] or None, text[idx:] + return text[:idx] or None, text[idx:].strip() def parse_tuple(self, orgtext): text = orgtext.strip() @@ -98,17 +102,21 @@ class Parser: # convert to closing bracket bra = ')]>'['([<'.index(bra)] reslist = [] - # search for cosing bracket, collecting results + # search for closing bracket, collecting results text = text[1:] while text: if bra not in text: return None, text res, rem = self.parse_sub(text) if res is None: + print('remtuple %r %r %r' % (rem, text, bra)) + if rem[0] == bra: + # allow trailing separator + return tuple(reslist), rem[1:].strip() return None, text reslist.append(res) if rem[0] == bra: - return tuple(reslist), rem[1:] + return tuple(reslist), rem[1:].strip() # eat separator if rem[0] in ',;': text = rem[1:] @@ -122,24 +130,27 @@ class Parser: return None, orgtext # keep ordering result = OrderedDict() - # search for cosing bracket, collecting results + # search for closing bracket, collecting results # watch for key=value or key:value pairs, separated by , text = text[1:] while '}' in text: # first part is always a string key, rem = self.parse_string(text) - if not key: + if key is None: + if rem[0] == '}': + # allow trailing separator + return result, rem[1:].strip() return None, orgtext if rem[0] not in ':=': return None, rem # eat separator text = rem[1:] value, rem = self.parse_sub(text) - if not value: + if value is None: return None, orgtext result[key] = value if rem[0] == '}': - return result, rem[1:] + return result, rem[1:].strip() if rem[0] not in ',;': return None, rem diff --git a/secop/protocol/dispatcher.py b/secop/protocol/dispatcher.py index c1577db..3c6e903 100644 --- a/secop/protocol/dispatcher.py +++ b/secop/protocol/dispatcher.py @@ -212,7 +212,7 @@ class Dispatcher: # now call func # note: exceptions are handled in handle_request, not here! func = getattr(moduleobj, 'do_' + cmdname) - res = func(argument) if argument else func() + res = func() if argument is None else func(argument) # pipe through cmdspec.datatype.result if cmdspec.datatype.result: diff --git a/test/test_parse.py b/test/test_parse.py index ebe9544..ebd9268 100644 --- a/test/test_parse.py +++ b/test/test_parse.py @@ -23,6 +23,7 @@ from collections import OrderedDict +from ast import literal_eval import pytest @@ -46,7 +47,7 @@ def test_Parser_parse_number(parser): def test_Parser_parse_string(parser): assert parser.parse_string('%') == (None, '%') assert parser.parse_string('a') == ('a', '') - assert parser.parse_string('Hello World!') == ('Hello', ' World!') + assert parser.parse_string('Hello World!') == ('Hello', 'World!') assert parser.parse_string('Hello