From 0e016453364005521ba98a6cae6b6cbcbbc09f43 Mon Sep 17 00:00:00 2001
From: thijs snijder <snijder@astron.nl>
Date: Fri, 2 Apr 2021 12:39:30 +0200
Subject: [PATCH] added most remaining exceptions for merge

---
 devices/PCC.py                      |  1 +
 devices/README.md                   |  2 +-
 devices/clients/opcua_connection.py | 53 ++++++++++++-----------------
 devices/clients/test_client.py      |  4 ---
 devices/src/attribute_wrapper.py    |  8 +++--
 devices/src/comms_client.py         | 33 +++++++++++++++---
 6 files changed, 58 insertions(+), 43 deletions(-)

diff --git a/devices/PCC.py b/devices/PCC.py
index d9f1c0430..f1d471da3 100644
--- a/devices/PCC.py
+++ b/devices/PCC.py
@@ -127,6 +127,7 @@ class PCC(hardware_device):
 		#set up the OPC ua client
 		self.OPCua_client = OPCUAConnection("opc.tcp://{}:{}/".format(self.OPC_Server_Name, self.OPC_Server_Port), "http://lofar.eu", self.OPC_Time_Out, self.Standby, self.Fault, self)
 
+
 		# map the attributes to the OPC ua comm client
 		for i in self.attr_list():
 			i.set_comm_client(self.OPCua_client)
diff --git a/devices/README.md b/devices/README.md
index 37f5db0d5..99772cd2e 100644
--- a/devices/README.md
+++ b/devices/README.md
@@ -10,7 +10,7 @@ declare what client the attribute has to use in the initialisation and provide s
 To see how to add support for new clients, see `clients/README.md`
 
 In addition it also provides an abstraction to the tango device, specifically for hardware devices. Examples of hardware devices 
-can be found in TODO and an empty template can be found in `HW_device_tempalte.py`
+can be found in TODO and an empty template can be found in `HW_device_template.py`
 
 Requires numpy 
 ```pip install numpy```
diff --git a/devices/clients/opcua_connection.py b/devices/clients/opcua_connection.py
index 56539a2a2..e88034192 100644
--- a/devices/clients/opcua_connection.py
+++ b/devices/clients/opcua_connection.py
@@ -3,23 +3,6 @@ from src.comms_client import *
 
 __all__ = ["OPCUAConnection"]
 
-# OPCua_to_numpy_dict = {
-# 	"VariantType.Boolean": numpy.bool_,
-# 	"VariantType.SByte": numpy.int8,
-# 	"VariantType.Byte": numpy.uint8,
-# 	"VariantType.Int16": numpy.int16,
-# 	"VariantType.UInt16": numpy.uint16,
-# 	"VariantType.Int32": numpy.int32,
-# 	"VariantType.UInt32": numpy.uint32,
-# 	"VariantType.Int64": numpy.int64,
-# 	"VariantType.UInt64": numpy.uint64,
-# 	"VariantType.DateTime": numpy.datetime_data, # is this the right type, does it even matter?
-# 	"VariantType.Float": numpy.float32,
-# 	"VariantType.Double": numpy.double,
-# 	"VariantType.String": numpy.str,
-# 	"VariantType.ByteString": numpy.uint8  # sequence of bytes, not a string
-# }
-
 numpy_to_OPCua_dict = {
 	numpy.bool_: opcua.ua.VariantType.Boolean,
 	numpy.int8: opcua.ua.VariantType.SByte,
@@ -72,6 +55,7 @@ class OPCUAConnection(CommClient):
 				self.name_space_index = namespace
 
 		except Exception as e:
+			#TODO remove once SDP is fixed
 			self.streams.warn_stream("Cannot determine the OPC-UA name space index.  Will try and use the default = 2.")
 			self.name_space_index = 2
 
@@ -89,11 +73,13 @@ class OPCUAConnection(CommClient):
 			self.streams.debug_stream("Connecting to server %s", self._servername())
 			self.client.connect()
 			self.connected = True
-			self.streams.debug_stream("Connected to server. Initialising.")
+			self.streams.debug_stream("Connected to %s. Initialising.", self._servername())
 			return True
 		except socket.error as e:
-			self.streams.error_stream("Could not connect to server %s: %s", self._servername(), e)
-			return False
+			#TODO
+			self.streams.error_stream("Could not connect to server %s: %s", self._servername())
+			raise Exception("") from e
+
 
 	def disconnect(self):
 		"""
@@ -123,23 +109,19 @@ class OPCUAConnection(CommClient):
 		if isinstance(annotation, dict):
 			# check if required path inarg is present
 			if annotation.get('path') is None:
-				AssertionError("OPC-ua mapping requires a path argument in the annotation")
+				raise Exception("OPC-ua mapping requires a path argument in the annotation, was given: %s", annotation)
 
 			path = annotation.get("path")  # required
-			ua_Type = annotation.get("ua_type")  # optional, if excluded must be a build in python type
 		elif isinstance(annotation, list):
 			path = annotation
 		else:
-			TypeError("OPC-ua mapping requires either a list or dict with the path")
-			return
-
-		#TODO exceptions
+			raise Exception("OPC-ua mapping requires either a list of the path or dict with the path. Was given %s type containing: %s", type(annotation), annotation)
 
 		try:
 			node = self.obj.get_child(path)
 		except Exception as e:
 			self.streams.error_stream("Could not get node: %s on server %s: %s", path, self._servername(), e)
-			raise Exception("Could not get node: %s on server %s: %s", path, self._servername(), e)
+			raise Exception("Could not get node: %s on server %s", path, self._servername()) from e
 
 		return node
 
@@ -160,6 +142,7 @@ class OPCUAConnection(CommClient):
 		MANDATORY function: is used by the attribute wrapper to get read/write functions. must return the read and write functions
 		"""
 
+
 		# process the annotation
 		node = self._setup_annotation(annotation)
 
@@ -169,15 +152,17 @@ class OPCUAConnection(CommClient):
 		# configure and return the read/write functions
 		prot_attr = ProtocolAttribute(node, dim_x, dim_y, ua_type)
 
-		node_name = str(node.get_browse_name())[len("QualifiedName(2:"):]
-		self.streams.debug_stream("connected OPC ua node {} of type {} to attribute with dimensions: {} x {} ".format(str(node_name)[:len(node_name)-1], str(ua_type)[len("VariantType."):], dim_x, dim_y))
+		try:
+			# NOTE: debug statement tries to get the qualified name, this may not always work. in that case forgo the name and just print the path
+			node_name = str(node.get_browse_name())[len("QualifiedName(2:"):]
+			self.streams.debug_stream("connected OPC ua node {} of type {} to attribute with dimensions: {} x {} ".format(str(node_name)[:len(node_name)-1], str(ua_type)[len("VariantType."):], dim_x, dim_y))
+		except:
+			pass
 
 		# return the read/write functions
 		return prot_attr.read_function, prot_attr.write_function
 
 
-
-
 class ProtocolAttribute:
 	"""
 	This class provides a small wrapper for the OPC ua read/write functions in order to better organise the code
@@ -215,3 +200,9 @@ class ProtocolAttribute:
 			self.node.set_data_value(opcua.ua.uatypes.Variant(value=value.tolist(), varianttype=self.ua_type))
 		else:
 			self.node.set_data_value(opcua.ua.uatypes.Variant(value=value, varianttype=self.ua_type))
+
+
+	def pass_func(self, value=None):
+		pass
+
+
diff --git a/devices/clients/test_client.py b/devices/clients/test_client.py
index 090437441..6bac45275 100644
--- a/devices/clients/test_client.py
+++ b/devices/clients/test_client.py
@@ -1,9 +1,5 @@
 from src.comms_client import *
 
-
-__all__ = ["example_client"]
-
-
 # <class 'numpy.bool_'>
 
 class example_client(CommClient):
diff --git a/devices/src/attribute_wrapper.py b/devices/src/attribute_wrapper.py
index 1ea654f92..8a203eb36 100644
--- a/devices/src/attribute_wrapper.py
+++ b/devices/src/attribute_wrapper.py
@@ -4,7 +4,8 @@ from tango import AttrWriteType
 import numpy
 
 from src.wrappers import only_when_on, fault_on_error
-
+import logging
+logger = logging.getLogger()
 
 
 class attribute_wrapper(attribute):
@@ -66,8 +67,9 @@ class attribute_wrapper(attribute):
 				"""
 				try:
 					return device.value_dict[self]
-				except:
-					print()
+				except Exception as e:
+					raise Exception("Attribute read_RW function error, attempted to read value_dict with key: `%s`, are you sure this exists?", self) from e
+
 
 			@only_when_on
 			@fault_on_error
diff --git a/devices/src/comms_client.py b/devices/src/comms_client.py
index b6eb28460..672ec4c39 100644
--- a/devices/src/comms_client.py
+++ b/devices/src/comms_client.py
@@ -53,11 +53,8 @@ class CommClient(Thread):
 		self.stopping = False
 		while not self.stopping:
 			# keep trying to connect
-			print("connected check")
 			if not self.connected:
-				print("not connected, try to connect")
 				if self.connect():
-					print("connected now, call on_func")
 					self.standby_func()
 				else:
 					# we retry only once, to catch exotic network issues. if the infra or hardware is down,
@@ -103,6 +100,20 @@ class CommClient(Thread):
 		The setup-attribute has access to the comms_annotation provided to the attribute wrapper to pass along to the comms client
 		as well as a reference to the attribute itself.
 
+		It should do this by first calling: _setup_annotation and setup_value_conversion to get all data necceacry to configure the read/write functions.
+		It should then return the read and write functions to the attribute.
+
+		MANDATORY:
+		annotation_outputs = _setup_annotation(annotation)
+		attribute_outputs = _setup_annotation(attribute)
+		(note: outputs are up to the user)
+
+		REQUIRED: provide read and write functions to return, there are no restrictions on how these should be provided,
+		except that the read function takes a single input value and the write function returns a single value
+
+		MANDATORY:
+		return read_function, write_function
+
 		Examples:
 		- File system:  get_mapping returns functions that read/write a fixed
 		number of bytes at a fixed location in a file. (SEEK)
@@ -110,5 +121,19 @@ class CommClient(Thread):
 		Then return the read/write functions for that node which automatically
 		convert values between Python and OPC-UA.
 		"""
-		AssertionError("the setup_attribute must be implemented and provide return a valid read/write function for the attribute")
+		raise NotImplementedError("the setup_attribute must be implemented and provide return a valid read/write function for the attribute")
+
+	def _setup_annotation(self, annotation):
+		"""
+		This function is responsible for handling the annotation data provided by the attribute to configure the read/write function the client must provide.
+		This function should be called by setup_attribute
+		"""
+		raise NotImplementedError("the _setup_annotation must be implemented, content and outputs are up to the user")
+
+	def setup_value_conversion(self, attribute):
+		"""
+		this function is responsible for setting up the value conversion between the client and the attribute.
+		This function should be called by setup_attribute
+		"""
+		raise NotImplementedError("the setup_value_conversion must be implemented, content and outputs are up to the user")
 
-- 
GitLab