EtherNetIP protocol support by NithyaElango · Pull Request #182 · vmware-archive/liota

NithyaElango · 2017-11-13T05:18:43Z

Added EtherNetIP protocol support for liota, which uses cpppo library internally.

pmasrani

Add the package which you have written as well. Without the package, nobody can try out this code.

Also, cpppo library needs to be added to requirements.txt

pmasrani · 2017-11-14T04:48:46Z

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


Use Pep 8 convention for naming the module. https://www.python.org/dev/peps/pep-0008/#package-and-module-names
Not only for this module and class, but for all of your code.

pmasrani · 2017-11-14T04:50:01Z

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


I think this should be CipEtherentDeviceComms. Ethernet is a generic protocol.

EtherNet/IP (EtherNet Industrial Protocol) means CIP implemented over ethernet.
So I changed the class name to EtherNetIPDeviceComms which I thought would be more suitable.
Or should I make it CipEtherentDeviceComms?

https://en.wikipedia.org/wiki/EtherNet/IP is a specific implementation of CIP. I think we should call this: CipEtherNetIpDeviceComms.

pmasrani · 2017-11-14T04:50:08Z

+import logging
+
+from liota.device_comms.device_comms import DeviceComms
+from liota.lib.transports.ENIP import EtherNetIP


rename ENIP

Renamed as EtherNetIP

pmasrani · 2017-11-14T04:51:10Z

+
+
+		if host is None:
+			log.error("Host can't be none")


Remove log statement. Once you raise an exception, it will automatically get logged.

Removed the log statement.

pmasrani · 2017-11-14T04:52:28Z

+		data = self.client.read()
+		return data
+
+	def send(self, message):


Why can't we implement send() and receive()? Whatever functionality you have in write() has to be part of send() and read() has to be part of receive()

Made these changes. Now, Send() contains write() and receive () contains read().

pmasrani · 2017-11-14T04:56:22Z

+                	assert self.conn.readable( timeout=1.0 ), "Failed to receive reply"
+                	rpy = next( self.conn )
+                	data = rpy['enip']['CIP']['send_data']['CPF']['item'][1]['unconnected_send']['request']['read_frag']['data'][0]
+        		print(data)


remove print, log if necessary

Removed all print statements

pmasrani · 2017-11-14T04:57:08Z

+        		req = self.conn.read("Scada[0]")
+                	assert self.conn.readable( timeout=1.0 ), "Failed to receive reply"
+                	rpy = next( self.conn )
+                	data = rpy['enip']['CIP']['send_data']['CPF']['item'][1]['unconnected_send']['request']['read_frag']['data'][0]


Where are these magic strings coming from? Is it always the same? What if one of them is None?

This is to fetch the data from the response got from the server, which is in json format.
It is the same always.

pmasrani · 2017-11-14T04:57:31Z

+
+
+	def shutdown( self ):
+        	if not self.udp:


What if it is not UDP?

I'm not able to figure this out, could you please take a look at this - https://github.com/pjkundert/cpppo/blob/master/server/enip/client.py#L401

pmasrani · 2017-11-14T04:59:22Z

+	DeviceComms for EtherNetIP protocol
+	"""
+
+	def __init__(self, host=None, port=None, timeout=None, dialect=None, profiler=None, udp=False, broadcast=False, source_address=None):


Why is the default value of host None, if we are immediately checking for it to be not none?

I made the localhost as default, if host is empty, as mentioned in https://github.com/pjkundert/cpppo/blob/master/server/enip/client.py#L300

pmasrani · 2017-11-14T04:59:55Z

+	def __init__(self, host=None, port=None, timeout=None, dialect=None, profiler=None, udp=False, broadcast=False, source_address=None):
+
+		self.host = host
+		self.port = port


What happens when port is none? where does it connect?

When port is None, I think the default port will be used. I want you to check this once - https://github.com/pjkundert/cpppo/blob/master/server/enip/client.py#L326
Also, it is mentioned if default port is not used, use 44818 - https://github.com/pjkundert/cpppo/blob/master/server/enip/client.py#L302

As of now I have changed the port as 44818 in the code.

ok. Thanks for checking it out. If the underlying library accepts None, then yes we should accept None. Let us not hard-code it to 44818, as it is a non-standard port, and if library implementation changes the port, we will get an error.

lucifercr07 · 2017-11-14T14:00:47Z

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


Please add a README file with instructions how to use this protocol.

pmasrani · 2017-11-16T04:32:33Z

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


This is not a simulation. Let us name it as CipSocketGraphite.py to indicate it is have Cip as DeviceComms, Socket as DCCComms and Graphite as DCC.

Also add the writer code as "simulator", so that someone can test this package. In Readme, also add the complete steps of how to run the simulator and load this package.

pmasrani · 2017-11-16T04:34:04Z

+
+
+
+    def clean_up(self):


disconnect the DeviceComms as well after stopping the metric collection.

pmasrani · 2017-11-16T04:35:35Z

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


None values need not be specified here. So remove them.

Okay, will remove them

pmasrani · 2017-11-16T04:37:12Z

+
+
+	def _connect(self):
+		self.client = EtherNetIP(self.host,self.port,self.timeout,self.dialect,self.profiler,self.udp,self.broadcast,self.source_address) 


Run autopep8 to correct the formatting.

pmasrani · 2017-11-16T11:39:33Z

@@ -0,0 +1,28 @@
+import time


Copyright missing.

pmasrani · 2017-11-16T11:41:06Z

+                              tag_type=tag_type)
+
+        	except AssertionError:
+                	print "Response timed out!! Tearing Connection and Reconnecting!!!!!"


Are you actually reconnecting anywhere??

pmasrani · 2017-11-16T11:41:19Z

+                elements = len (data)
+			    tag_type = client.enip.DINT.tag_type
+			    tag = "Scada"
+			    req = conn.write( tag, elements=elements, data=data,


formatting? use autopep8

pmasrani · 2017-11-16T11:45:17Z

+import logging
+
+from liota.device_comms.device_comms import DeviceComms
+from liota.lib.transports.EtherNetIP import EtherNetIP


rename EtherNetIP to CipEtherNetIp

pmasrani · 2017-11-16T11:46:56Z

+		if data is None:
+			raise TypeError("Data can't be none")
+		else:
+			self.client.send(tag,elements,data,tagType)


formatting.

pmasrani · 2017-11-16T11:51:56Z

 LivingRoomLightTopic = "home/living-room/light"

+#### [EtherNet/IP] ####
+EtherNetIP = "EtherNet/IP IP"


Rename to CipEtherNetIp

pmasrani · 2017-11-16T11:52:23Z

 LivingRoomHumidityTopic = "home/living-room/humidity"
 LivingRoomLightTopic = "home/living-room/light"

+#### [EtherNet/IP] ####


Provide better comment like CIP Etherent/IP Device Comms parameters

pmasrani · 2017-11-16T18:53:28Z

Closing this pull request, as this was incorrectly created and a new one (#187) is raised.

NithyaElango added 13 commits November 7, 2017 19:27

Add files via upload

96d7e71

Add files via upload

f590848

Update ENIP.py

736bd01

Update ENIP.py

5cc93b1

Update ENIP.py

c0927ee

Update ENIP.py

9583df8

Update ENIP.py

76c8c97

Update ENIP.py

bb5b4bc

Update ENIP.py

b790c5e

Update ENIP.py

9404a98

Update EtherNEtIP_DeviceComms.py

4c0c38d

Update ENIP.py

f8e0d4a

Rename EtherNEtIP_DeviceComms.py to EtherNetIP_DeviceComms.py

1d8dbb4

pmasrani reviewed Nov 14, 2017

View reviewed changes

pmasrani requested a review from KohliDev November 14, 2017 05:02

lucifercr07 reviewed Nov 14, 2017

View reviewed changes

NithyaElango added 5 commits November 16, 2017 00:20

Update EtherNetIP_DeviceComms.py

5ae4e54

Update and rename ENIP.py to EtherNetIP.py

5b682c1

Create EtherNetIPSimulation.py

bfcca2e

Update EtherNetIP_DeviceComms.py

34c637f

Update EtherNetIPSimulation.py

a6d9066

pmasrani reviewed Nov 16, 2017

View reviewed changes

NithyaElango added 8 commits November 16, 2017 11:33

Rename EtherNetIPSimulation.py to CipSocketGraphite.py

9943f29

Update CipSocketGraphite.py

c7159ad

Delete CipSocketGraphite.py

b0023f4

Create CipSocketGraphite.py

901de02

Create EtherNetIP_Simulator.py

debcabc

Update requirements.txt

82e2e2c

Update CipSocketGraphite.py

296c1f3

Update CipSocketGraphite.py

4ea7ba3

NithyaElango added 5 commits November 16, 2017 16:29

Update EtherNetIP_DeviceComms.py

5d34c08

Update sampleProp.conf

6938d0c

Create README.md

0a760c7

Update README.md

171e3de

Update README.md

fca97a4

pmasrani reviewed Nov 16, 2017

View reviewed changes

pmasrani closed this Nov 16, 2017

		log = logging.getLogger(__name__)


		class EtherNetDeviceComms(DeviceComms):

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


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



		def _connect(self):
		self.client = EtherNetIP(self.host,self.port,self.timeout,self.dialect,self.profiler,self.udp,self.broadcast,self.source_address)

Uh oh!

Conversation

NithyaElango commented Nov 13, 2017

Uh oh!

pmasrani left a comment

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