From ce0dcca689e1d6e9cd80bad474a9dcb8a18b2ea0 Mon Sep 17 00:00:00 2001 From: Jan David Mol <mol@astron.nl> Date: Wed, 30 Jun 2021 21:39:57 +0200 Subject: [PATCH 1/8] L2SS-295: Don't enter a spam connect-fault loop --- devices/clients/comms_client.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/devices/clients/comms_client.py b/devices/clients/comms_client.py index f189ce3fd..011e1e621 100644 --- a/devices/clients/comms_client.py +++ b/devices/clients/comms_client.py @@ -65,6 +65,9 @@ class CommClient(Thread): # signal that we're disconnected self.fault_func() + # don't enter a spam-connect loop if faults immediately occur + time.sleep(self.try_interval) + def ping(self): return -- GitLab From b49f74355a58bb3ae5d6e97240527737cbe62967 Mon Sep 17 00:00:00 2001 From: Jan David Mol <mol@astron.nl> Date: Wed, 30 Jun 2021 21:40:58 +0200 Subject: [PATCH 2/8] L2SS-295: Don't complain if ON or FAULT are called in their respective states. Just do nothing instead. --- devices/devices/hardware_device.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/devices/devices/hardware_device.py b/devices/devices/hardware_device.py index d244e79df..9baf57a80 100644 --- a/devices/devices/hardware_device.py +++ b/devices/devices/hardware_device.py @@ -90,7 +90,7 @@ class hardware_device(Device): self.set_state(DevState.STANDBY) @command() - @only_in_states([DevState.STANDBY]) + @only_in_states([DevState.STANDBY, DevState.ON]) @DebugIt() @fault_on_error() @log_exceptions() @@ -100,6 +100,10 @@ class hardware_device(Device): :return:None """ + if self.get_state() == DevState.ON: + # Already on. Don't complain. + return + self.configure_for_on() self.set_state(DevState.ON) @@ -125,7 +129,7 @@ class hardware_device(Device): self.set_state(DevState.OFF) @command() - @only_in_states([DevState.ON, DevState.INIT, DevState.STANDBY]) + @only_in_states([DevState.ON, DevState.INIT, DevState.STANDBY, DevState.FAULT]) @DebugIt() @log_exceptions() def Fault(self): @@ -138,6 +142,10 @@ class hardware_device(Device): :return:None """ + if self.get_state() == DevState.FAULT: + # Already faulting. Don't complain. + return + self.configure_for_fault() self.set_state(DevState.FAULT) -- GitLab From 897b14e2f69ed015c6cf7a9d43a4634fb96a95ef Mon Sep 17 00:00:00 2001 From: Jan David Mol <mol@astron.nl> Date: Wed, 30 Jun 2021 23:37:45 +0200 Subject: [PATCH 3/8] L2SS-295: Report on involved datatypes if we write with the wrong type somewhere in the conversion to the server. --- devices/clients/opcua_client.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/devices/clients/opcua_client.py b/devices/clients/opcua_client.py index de237d919..8ca42b41b 100644 --- a/devices/clients/opcua_client.py +++ b/devices/clients/opcua_client.py @@ -225,7 +225,15 @@ class ProtocolAttribute: # make sure it is a python array value = value.tolist() if type(value) == numpy.ndarray else value - self.node.set_data_value(opcua.ua.uatypes.Variant(value=value, varianttype=self.ua_type)) - + try: + self.node.set_data_value(opcua.ua.uatypes.Variant(value=value, varianttype=self.ua_type)) + except (TypeError, opcua.ua.uaerrors.BadTypeMismatch) as e: + if type(value) == list: + our_type = "list(%s) x (%d)" % (type(value[0]).__name__ if value else "", len(value)) + else: + our_type = "%s" % type(value) + expected_server_type = "%s x (%d,%d)" % (self.ua_type, self.dim_x, self.dim_y) + actual_server_type = "%s x %s" % (self.node.get_data_type_as_variant_type(), self.node.get_array_dimensions() or "???") + raise TypeError("Cannot write value to OPC-UA attribute '%s': tried to convert data type %s to expected server type %s, server reports type %s" % (self.node.get_display_name().to_string(), our_type, expected_server_type, actual_server_type)) from e -- GitLab From fa0abd13ab3d6d03bb34d4e578c73e3275537151 Mon Sep 17 00:00:00 2001 From: Jan David Mol <mol@astron.nl> Date: Thu, 1 Jul 2021 00:06:08 +0200 Subject: [PATCH 4/8] L2SS-295: Added comments. Better reporting for scalar values. --- devices/clients/opcua_client.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/devices/clients/opcua_client.py b/devices/clients/opcua_client.py index 8ca42b41b..7572bde14 100644 --- a/devices/clients/opcua_client.py +++ b/devices/clients/opcua_client.py @@ -228,12 +228,17 @@ class ProtocolAttribute: try: self.node.set_data_value(opcua.ua.uatypes.Variant(value=value, varianttype=self.ua_type)) except (TypeError, opcua.ua.uaerrors.BadTypeMismatch) as e: + # A type conversion went wrong or there is a type mismatch. + # + # This is either the conversion us -> opcua in our client, or client -> server. + # Report all types involved to allow assessment of the location of the error. if type(value) == list: our_type = "list(%s) x (%d)" % (type(value[0]).__name__ if value else "", len(value)) else: our_type = "%s" % type(value) - expected_server_type = "%s x (%d,%d)" % (self.ua_type, self.dim_x, self.dim_y) + is_scalar = (self.dim_x + self.dim_y) == 1 + expected_server_type = "%s %s" % (self.ua_type, "(scalar)" if is_scalar else "x (%d, %d)" % (self.dim_x, self.dim_y)) actual_server_type = "%s x %s" % (self.node.get_data_type_as_variant_type(), self.node.get_array_dimensions() or "???") raise TypeError("Cannot write value to OPC-UA attribute '%s': tried to convert data type %s to expected server type %s, server reports type %s" % (self.node.get_display_name().to_string(), our_type, expected_server_type, actual_server_type)) from e -- GitLab From 7c7078003249f0eeaffda3d3575649561ed60d21 Mon Sep 17 00:00:00 2001 From: Jan David Mol <mol@astron.nl> Date: Thu, 1 Jul 2021 00:16:17 +0200 Subject: [PATCH 5/8] L2SS-295: Don't spam logs with OPC-UA communication debug messages. Report which logger is logging to allow tweaking the spam. --- devices/common/lofar_logging.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/devices/common/lofar_logging.py b/devices/common/lofar_logging.py index 5f3696efa..55299a750 100644 --- a/devices/common/lofar_logging.py +++ b/devices/common/lofar_logging.py @@ -9,6 +9,8 @@ hostname = socket.gethostname() def configure_logger(logger: logging.Logger, log_extra=None): logger.setLevel(logging.DEBUG) + logging.getLogger("opcua").setLevel(logging.WARN) + try: from logstash_async.handler import AsynchronousLogstashHandler, LogstashFormatter @@ -27,7 +29,7 @@ def configure_logger(logger: logging.Logger, log_extra=None): # easily grep'ed, be parsed with a couple of shell commands and # easily fed into an Kibana/Elastic search system. handler = logging.StreamHandler() - formatter = logging.Formatter(fmt = '%(asctime)s.%(msecs)d %(levelname)s - HOST="{}" PID="%(process)d" TNAME="%(threadName)s" TID="%(thread)d" FILE="%(pathname)s" LINE="%(lineno)d" FUNC="%(funcName)s" MSG="%(message)s"'.format(hostname), datefmt = '%Y-%m-%dT%H:%M:%S') + formatter = logging.Formatter(fmt = '%(asctime)s.%(msecs)d %(levelname)s - HOST="{}" PID="%(process)d" TNAME="%(threadName)s" TID="%(thread)d" LOGGER="%(name)s" FILE="%(pathname)s" LINE="%(lineno)d" FUNC="%(funcName)s" MSG="%(message)s"'.format(hostname), datefmt = '%Y-%m-%dT%H:%M:%S') handler.setFormatter(formatter) logger.addHandler(handler) except Exception: -- GitLab From a4db4c9ef24bb161741c16dfbc423b18c5d3cbc4 Mon Sep 17 00:00:00 2001 From: Jan David Mol <mol@astron.nl> Date: Fri, 2 Jul 2021 09:22:07 +0200 Subject: [PATCH 6/8] L2SS-289: Added comment --- devices/common/lofar_logging.py | 1 + 1 file changed, 1 insertion(+) diff --git a/devices/common/lofar_logging.py b/devices/common/lofar_logging.py index 55299a750..d84a47467 100644 --- a/devices/common/lofar_logging.py +++ b/devices/common/lofar_logging.py @@ -9,6 +9,7 @@ hostname = socket.gethostname() def configure_logger(logger: logging.Logger, log_extra=None): logger.setLevel(logging.DEBUG) + # remove spam from the OPC-UA client connection logging.getLogger("opcua").setLevel(logging.WARN) try: -- GitLab From 209430b3348d7288d0025dd1ff346b12c0eeabaf Mon Sep 17 00:00:00 2001 From: Jan David Mol <mol@astron.nl> Date: Fri, 2 Jul 2021 10:37:20 +0200 Subject: [PATCH 7/8] L2SS-295: Log when someone requests a state we're already in, as that could be a bug. --- devices/devices/hardware_device.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/devices/devices/hardware_device.py b/devices/devices/hardware_device.py index 9baf57a80..8c348868d 100644 --- a/devices/devices/hardware_device.py +++ b/devices/devices/hardware_device.py @@ -102,6 +102,7 @@ class hardware_device(Device): """ if self.get_state() == DevState.ON: # Already on. Don't complain. + logger.warning("Requested to go to ON state, but am already in ON state.") return self.configure_for_on() @@ -144,6 +145,7 @@ class hardware_device(Device): """ if self.get_state() == DevState.FAULT: # Already faulting. Don't complain. + logger.warning("Requested to go to FAULT state, but am already in FAULT state.") return self.configure_for_fault() -- GitLab From aa0d48a4074fee4b43a42b2fe15cddfb2d6b725e Mon Sep 17 00:00:00 2001 From: Jan David Mol <mol@astron.nl> Date: Fri, 2 Jul 2021 16:09:04 +0200 Subject: [PATCH 8/8] L2SS-295: Use clearer formatting of strings. --- devices/clients/opcua_client.py | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/devices/clients/opcua_client.py b/devices/clients/opcua_client.py index 7572bde14..ac66f4706 100644 --- a/devices/clients/opcua_client.py +++ b/devices/clients/opcua_client.py @@ -227,18 +227,35 @@ class ProtocolAttribute: try: self.node.set_data_value(opcua.ua.uatypes.Variant(value=value, varianttype=self.ua_type)) + raise TypeError except (TypeError, opcua.ua.uaerrors.BadTypeMismatch) as e: # A type conversion went wrong or there is a type mismatch. # # This is either the conversion us -> opcua in our client, or client -> server. # Report all types involved to allow assessment of the location of the error. if type(value) == list: - our_type = "list(%s) x (%d)" % (type(value[0]).__name__ if value else "", len(value)) + our_type = "list({dtype}) x ({dimensions})".format( + dtype=(type(value[0]).__name__ if value else ""), + dimensions=len(value)) else: - our_type = "%s" % type(value) + our_type = "{dtype}".format( + dtype=type(value)) is_scalar = (self.dim_x + self.dim_y) == 1 - expected_server_type = "%s %s" % (self.ua_type, "(scalar)" if is_scalar else "x (%d, %d)" % (self.dim_x, self.dim_y)) - actual_server_type = "%s x %s" % (self.node.get_data_type_as_variant_type(), self.node.get_array_dimensions() or "???") - raise TypeError("Cannot write value to OPC-UA attribute '%s': tried to convert data type %s to expected server type %s, server reports type %s" % (self.node.get_display_name().to_string(), our_type, expected_server_type, actual_server_type)) from e + if is_scalar: + expected_server_type = "{dtype} (scalar)".format( + dtype=self.ua_type) + else: + expected_server_type = "{dtype} x ({dim_x}, {dim_y})".format( + dtype=self.ua_type, + dim_x=self.dim_x, + dim_y=self.dim_y) + + actual_server_type = "{dtype} {dimensions}".format( + dtype=self.node.get_data_type_as_variant_type(), + dimensions=(self.node.get_array_dimensions() or "???")) + + attribute_name = self.node.get_display_name().to_string() + + raise TypeError(f"Cannot write value to OPC-UA attribute '{attribute_name}': tried to convert data type {our_type} to expected server type {expected_server_type}, server reports type {actual_server_type}") from e -- GitLab