Linux adding routes to remote PLC

[Adrian-at-CrimsonAzure]

It's a little bare bones, doesn't have tests, and could probably do with some more work, but it successfully adds a route to the remote PLC on demand. I made these modifications for a project I was working on and I figured I'd share.

A rough breakdown of the data structure:

Would love some feedback.

[coveralls]

Pull Request Test Coverage Report for Build 219

• 49 of 50 (98.0%) changed or added relevant lines in 2 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.6%) to 89.819%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pyads/pyads_ex.py	47	48	97.92%

Totals
Change from base Build 203:	0.6%
Covered Lines:	644
Relevant Lines:	717

---

💛 - Coveralls

[Adrian-at-CrimsonAzure]

Little piece of code I used to test it against a PLC

import pyads

print(pyads.__file__)
my_ads_address = '169.254.104.190.1.1'
target_ip = '192.168.200.210'
target_ads = '5.6.182.28.1.1'
if pyads.add_route_to_plc(my_ads_address,target_ip,'administrator','1'):
	# test PLC connection
	pyads.set_local_address(my_ads_address)
	with pyads.Connection(target_ads, pyads.PORT_SPS1, target_ip) as plc:
		temp = plc.read_by_name('.PARK_ROBOT', pyads.PLCTYPE_BOOL)
		print(temp)

In therory, this can also be used on Windows but there isn't really a reason to use it because you can't add routes on the client side.

[stlehmann]

First, thank you for this PR. This looks like a handy extension and I think it is a good idea to add it. However, I don' t want to add any new untested code to the repo so I need to ask you to also implement some test-code for this function. Please have a look at the tests directory.

Thanks again for the effort and sorry for asking for more but it is necessary that any new features gets its automated test not only to ensure it works as described but also to make sure it doesn't fail due to futures changes.

[stlehmann]

And of course we will also need some documentation for it ;)

[kryskool]

Great Jobs
I have made suggestion, and it missing tests

[Adrian-at-CrimsonAzure]

I'll work on some tests later today, running into issues connecting to one of our Beckhoff PLCs on Linux. Anyone have any input? #77

[Adrian-at-CrimsonAzure]

Ah, Python 2. Wish I would have checked that earlier...

I don't really have experience using Python 2 and I personally don't want to rewrite this PR to support a version that is EOL in 5 months.

[stlehmann]

I personally don't want to rewrite this PR to support a version that is EOL in 5 months.

I understand that. Maybe it's about time to drop the Python2 support in the future. However, as the whole codebase is Python2 compatible I would like to keep it that way for now. I will have a closer look at your PR later on and try to recover Python2 compatibility.

[kryskool]

Hi @Adrian-at-CrimsonAuzre

you can use closing() on contextlib

replace

with socket.socket(socket.AF_INET, socket.SOCK_DGRAM) as sock:

like this

from contextlib import closing

with closing(socket.socket(socket.AF_INET, socket.SOCK_DGRAM)) as sock:

regards

[Adrian-at-CrimsonAzure]

It's not just that, I make heavy use of int.to_bytes() and int.from_bytes() which (I'm assuming) would need to become struct.pack() and struct.unpack()

[stlehmann]

Yes, struct.pack() is the way to go then. It is also faster then int.to_bytes(), though ;)

[Adrian-at-CrimsonAzure]

I've done what I can, but my dev environment is only set up for Python 3 so I can't go line by line to figure out what's wrong. I've just been guessing and running the tests in Python 2 so far.

[klauer]

Thanks, @Adrian-at-CrimsonAuzre - I'm rather excited about this functionality! It can be frustrating to always need a machine with TwinCAT just to add a route so ADS will work.

Bizarrely, I gave this PR a try and found that regardless of the password (correct or incorrect), rcvd_is_password_correct == b'\x04\x00\x00' and add_route_to_plc returns True.

Despite that positive response, I wasn't able to access the PLC over ADS from the machine I added a route to.

Then, instead of leaving adding_host_name to the default (gethostname), I specified my machine's IP address. I was then able to access the PLC over ADS.

I wonder if adding_host_name should be required?

[Adrian-at-CrimsonAzure]

@klauer The route being added successfully just means that the PLC added the route, not that the route is correct. It's strange that you were getting a success response with an incorrect password though, any chance you can post the bytes returned by the PLC when this occurs?

Making it mandatory might be a good idea, I also had an issue with a Docker container that was only resolved when I specified the server hostname. It could also be an issue with the PLC being unable to resolve the hostname, in which case sending the destination IP is probably the better solution. I think TwinCAT uses the hostname because it's not a given that your computer has a static IP.

What I've learned over the course of my project is that PLCs are weird, and how they communicate is weirder.

[klauer]

@Adrian-at-CrimsonAuzre Agreed on the overall weirdness. The incorrect password not making a difference in the response is the main thing that struck me as concerning, not so much the required IP vs hostname.

See the 'sent'/'received' lines in this gist if you want more info, along with the comment above each execution: https://gist.github.com/klauer/d1154d9e7a8a16bd398cec2bfc0a9678 Not sure there's much that can be done here. I'm glad it works, regardless.

[Adrian-at-CrimsonAzure]

@klauer On the PLCs I tried it returned a different response, but (most) PLCs don't seem to be designed with network security in mind. Maybe this is an artifact of an older version of TwinCAT installed on the PLC? Or maybe this is something you should report to Beckhoff?

[stlehmann]

@Adrian-at-CrimsonAuzre Has there been any progress in this matter recently? I've just checked that the tests are not passing correctly for Python2 and Python3.5. Also there seem to be some problems with connecting to certain PLCs.

[Adrian-at-CrimsonAzure]

@stlehmann No idea why the Python3.5 test failed, it passed on my local machine before I committed the last changes. Looks like it timed out when running online.
I'm personally not going to keep working on the Python2 implementation because I don't really have a proper dev environment for it and I don't have the time. I can, however, throw some documentation together if you'd like.
It seems that the connection issues stem from PLCs being strange, but presently it works for all of the Beckhoff PLCs I'm connecting to. Samba should probably be listed as a requirement for pyads on Linux because otherwise certain PLCs will be unable to respond to any requests from pyads without it. I'm pretty sure it has to do with NetBIOS.

[Adrian-at-CrimsonAzure]

Now Python3.5 passes. That's strange.

[kryskool]

Hi @Adrian-at-CrimsonAuzre

I thinks for Python 2.7 we need to return an exception NotImplemented and catch it on test

It's a good reason to migrate on Python 3.x if we want this functionnality

Comments ?

regards,

[Adrian-at-CrimsonAzure]

@kryskool I'm inclined to agree unless someone else wants to take up the mantle and figure out what's wrong with the Python2 implementation.

[stlehmann]

I fixed the last Python2 errors and merged the PR. There were just some bytes( statements left that broke Python2 compatibility. Could you both please test the final implementation because I am running on Windows. If everything is nice I will create a new release.

[Adrian-at-CrimsonAzure]

Woops, guess I missed some. Thanks for taking care of that.
I out adding routes and reading tags on Python3.6 and everything seems to work.

[stlehmann]

Thanks again for bringing this up and elaborating. I think that's a great new feature.