CIP EthernetIP protocol implementation by NithyaElango · Pull Request #187 · vmware-archive/liota

NithyaElango · 2017-11-16T18:26:54Z

CIP protocol support for Liota

pmasrani · 2017-11-16T18:33:37Z

+
+Then, EtherNet/IP related parameters required in `send()` and `receive()` like tags, datatype etc., will be mentioned while starting the server. Please refer this [example](https://github.com/lucifercr07/liota/blob/can_bus/examples/canbus/simulated_canbus.py) which reads the data from the server and send it to DCC.
+
+In SampleProp.conf, the EtherNetIP should be changed to the IP of the server.


CipEtherNetIp not EtherNetIP

pmasrani · 2017-11-16T18:33:57Z

+
+## Using EtherNetIP_DeviceComms
+
+Initially run the writer program which keeps writing data to the server, check this [simulator]( ) for the code.


something missing in brackets?

pmasrani · 2017-11-16T18:34:07Z

+## Using EtherNetIP_DeviceComms
+
+Initially run the writer program which keeps writing data to the server, check this [simulator]( ) for the code.
+Change the host to the IP of the machine where server is running in line number 6 in this [simulator]( ).


something missing in brackets?

pmasrani · 2017-11-16T18:36:14Z

+from cpppo.server.enip.getattr import attribute_operations
+import json
+
+HOST = "host"


I guess this needs to be changed every time someone wants to run the code. Instead, assume all 3 components (simulator, liota and server) running on the same machine. Use 127.0.0.1 in sampleProp.conf and here. In Readme, provide the instruction of where to change it, if every component is running on different machine.

pmasrani · 2017-11-16T18:38:02Z

+    def receive(self):
+        with self.conn:
+            try:
+                request_ = self.conn.read("Scada[0]")


I think "Scada" is the name/tag of the queue. Correct me if i am wrong. It seems to be hard-coded between liota and simulator. Make this also a configurable parameter in sampleProp.conf like IP address.

Yes, it is the tag. I will change it.

pmasrani · 2017-11-16T18:45:50Z

+log = logging.getLogger(__name__)
+
+
+class CIPEthernetIP:


CipEthernetIp instead of CIPEthernetIP.

pmasrani · 2017-11-16T18:46:39Z

@@ -0,0 +1,90 @@
+# -*- coding: utf-8 -*-


package name convention? cip_socket_graphite? I think even directory name needs to be changed. Check that.

pmasrani · 2017-11-16T18:49:32Z

+
+        self.config = read_user_config(config_path + '/sampleProp.conf')
+
+        self.ethernetIP_conn =  CipEtherNetIpDeviceComms(host=self.config['CipEtherNetIp'], port, timeout, dialect,


I don't see a reason why we are declaring all these parameters. Just a simple call CipEtherNetIpDeviceComms(host=self.config['CipEtherNetIp']) should work. Isn't it?

Yes that would work, I will change it.

pmasrani · 2017-11-16T18:50:23Z

@@ -0,0 +1,22 @@
+# Using CIP EtherNet Industrial Protocol as Transport in LIOTA
+
+LIOTA offers CIP EtherNet Industrial protocol as transport at Device end via [EtherNetIP_DeviceComms](https://github.com/lucifercr07/liota/blob/can_bus/liota/device_comms/canbus_device_comms.py)


EtherNetIP_DeviceComms? Correct the name.

pmasrani · 2017-11-16T18:51:18Z

+
+## Starting the server
+
+To get started with the CIP protocol, a server must be started in any linux machine. Install cpppo in that machine using 


Should we provide a link to cpppo library?

pmasrani · 2017-11-17T12:16:08Z

+#### [CIP EthernetIP Parameters] ####
+
+CipEtherNetIp = "127.0.0.1"
+Tag = "Scada[0]"


I think this should just be "Scada". The value from the tag is read at [0] position. That should be done by the program.

I changed this.

pmasrani · 2017-11-17T12:17:36Z

+			req = conn.write( tag, elements=elements, data=data,
+                                   tag_type=tagType)
+
+          	except socket.error as exc:


Now what happens when other errors are received? Program stops? Should we have a catch all exceptions block and print the Exception?

I am not sure what has to be done here. Should I catch all exceptions?

I checked the client file in cpppo library, shutdown is not required, closing the connection is sufficient. so changed accordingly.

pmasrani · 2017-11-17T12:18:20Z

+    def clean_up(self):
+        for metric in self.metrics:
+            metric.stop_collecting()
+	self.ethernetIP_conn._disconnect()


Did you check the indent? Did you try unloading the package?

I did unload it, when I see the program, I don't find this indentation mistake.

pmasrani · 2017-11-17T12:18:51Z

+## Using cip_ethernet_ip_device_comms
+
+Initially run the writer program which keeps writing data to the server, check this [simulator](https://github.com/NithyaElango/liota/blob/CipEthernetIP/tests/cipethernetip/cip_ethernet_ip_simulator.py) for the code.
+Modify the variable "host" in this [simulator](https://github.com/NithyaElango/liota/blob/CipEthernetIP/tests/cipethernetip/cip_ethernet_ip_simulator.py) if the server is running in a different machine. If not, the server,simulator and liota program are running in  the same machine, then nothing needs to be changed.


The variable name you have used is HOST, not "host".

Changed this.

pmasrani · 2017-11-17T12:19:31Z

+            try:
+                self.conn.shutdown(socket.SHUT_WR)
+            except:
+                pass


Can you please log the exception?

pmasrani · 2017-11-17T12:19:56Z

+                response = next(self.conn)
+                data = response['enip']['CIP']['send_data']['CPF']['item'][1]['unconnected_send']['request']['read_frag']['data'][0]
+            except AssertionError as error:
+                log.error("Failed to receive reply")


Change it to log.exception.

pmasrani · 2017-11-17T12:20:13Z

+        except AssertionError as exc:
+            log.info("Response timed out!!")
+        except socket.error as exc:
+            log.error("Couldn't send command: %s" % (exc))


Change it to log.exception

pmasrani · 2017-11-17T12:20:54Z

+            name=ethernet_device_metric_name,
+            unit=None,
+            interval=5,
+            sampling_function=lambda: read_value(self.ethernetIP_conn,self.tag)


space missing after comma? autopep8?

This is done.

pmasrani · 2017-11-17T12:22:20Z

+
+    def connect(self):
+        with client.connector(host=self.host) as self.conn:
+            if(self.conn):


Do we need the if statement here? If we are inside with, it will print the log.

Removed the if statement.

KohliDev · 2017-11-21T20:10:07Z

@@ -0,0 +1,109 @@
+# -*- coding: utf-8 -*-


rename cip_device_comms.py

CIP could be implemented over four different networks.
I have done it over ethernet, so It is known as EtherNet/IP, and thus me and Piyush named it as cip_ethernet_ip.
Is it necessary to rename?

KohliDev · 2017-11-21T20:10:21Z

+log = logging.getLogger(__name__)
+
+
+class CipEtherNetIpDeviceComms(DeviceComms):


Rename to CipDeviceComms

KohliDev · 2017-11-21T20:10:43Z

+    def receive(self, tag, index):	
+        data = self.client.receive(tag, index)
+        return data
+


Remove extra line

KohliDev · 2017-11-21T20:11:26Z

+        self.broadcast = broadcast
+        self.source_address = source_address
+
+    def connect(self):


Is there no credentials support?

No, there is no credentials support

KohliDev · 2017-11-21T20:12:35Z

+        try:
+            req = self.conn.write(self.tag, elements=self.elements, data=self.data,
+                                  tag_type=self.tag_type)
+        except AssertionError as exc:


Reason for assertion error?

In cpppo library, which is being used here, the write() function calls another function "unconnected_send()" - where there is an assert statement https://github.com/pjkundert/cpppo/blob/master/server/enip/client.py#L768 and that is the reason for the assertion error.

KohliDev · 2017-11-21T20:17:53Z

@@ -0,0 +1,62 @@
+# -*- coding: utf-8 -*-


There should be a unit test case as well.

I don't understand how this has to be done.

KohliDev · 2017-11-21T20:18:29Z

+#  ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF     #
+#  THE POSSIBILITY OF SUCH DAMAGE.                                            #
+# ----------------------------------------------------------------------------#
+


Can you add comments over here what are you trying to do in this particular test case?

KohliDev · 2017-11-21T20:18:37Z

+                        log.exception("Instance has no such attribute")		
+		time.sleep(5)
+		i = i + 1
+


Remove extra lines.

KohliDev · 2017-11-21T20:19:58Z

+from liota.device_comms.cip_ethernet_ip_device_comms import CipEtherNetIpDeviceComms
+from liota.entities.devices.simulated_device import SimulatedDevice
+
+


Can you add comments over here to explain the flow?

KohliDev · 2017-11-21T20:21:25Z

+    return value
+
+
+class PackageClass(LiotaPackage):


There should also be a normal Liota example with the package example on the usage of this protocol as we haven't moved away or neither decided to discard the current existing examples.

https://github.com/vmware/liota/tree/master/examples

You want me to write an liota example program other than a liota package for the usage of this protocol? And it should be under liota/examples right?

I wrote an example program for the same usage and placed it under liota/examples.

CIP EthernetIP protocol implementation

a631d43

pmasrani reviewed Nov 16, 2017

View reviewed changes

pmasrani mentioned this pull request Nov 16, 2017

EtherNetIP protocol support #182

Closed

root added 3 commits November 16, 2017 23:54

CIP EthernetIP protocol changes

4cae282

CIP EthernetIP protocol README changes

84c86ba

CIP EthernetIP protocol README changes

c6f5f33

pmasrani reviewed Nov 17, 2017

View reviewed changes

CIP EthernetIP protocol review comment changes

2c891e9

KohliDev reviewed Nov 21, 2017

View reviewed changes

root added 2 commits November 22, 2017 06:24

CIP EthernetIP protocol review comment changes

ff75f4e

CIP EthernetIP protocol README changes

5abdb82


		Then, EtherNet/IP related parameters required in `send()` and `receive()` like tags, datatype etc., will be mentioned while starting the server. Please refer this [example](https://github.com/lucifercr07/liota/blob/can_bus/examples/canbus/simulated_canbus.py) which reads the data from the server and send it to DCC.

		In SampleProp.conf, the EtherNetIP should be changed to the IP of the server.


		## Using EtherNetIP_DeviceComms

		Initially run the writer program which keeps writing data to the server, check this [simulator]( ) for the code.


		self.config = read_user_config(config_path + '/sampleProp.conf')

		self.ethernetIP_conn = CipEtherNetIpDeviceComms(host=self.config['CipEtherNetIp'], port, timeout, dialect,

		@@ -0,0 +1,22 @@
		# Using CIP EtherNet Industrial Protocol as Transport in LIOTA

		LIOTA offers CIP EtherNet Industrial protocol as transport at Device end via [EtherNetIP_DeviceComms](https://github.com/lucifercr07/liota/blob/can_bus/liota/device_comms/canbus_device_comms.py)


		## Starting the server

		To get started with the CIP protocol, a server must be started in any linux machine. Install cpppo in that machine using

		log = logging.getLogger(__name__)


		class CipEtherNetIpDeviceComms(DeviceComms):

		from liota.device_comms.cip_ethernet_ip_device_comms import CipEtherNetIpDeviceComms
		from liota.entities.devices.simulated_device import SimulatedDevice

Uh oh!

Conversation

NithyaElango commented Nov 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

NithyaElango commented Nov 16, 2017 •

edited

Loading