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