ottowayi / pycomm3

A Python Ethernet/IP library for communicating with Allen-Bradley PLCs.
MIT License
407 stars 89 forks source link

[FEATURE] - Support standard path format/notation #232

Closed tlf30 closed 2 years ago

tlf30 commented 2 years ago

Hello, Would it be possible to add support for normal path notation used in other platforms. For example, using a comma (',') instead of a slash. An example: Current format: 192.168.1.10/bp/4 Proposed format: 192.168.1.10,1,4

Perhaps this can be done in a way to not break existing compatibility, for example, just let a comma work in place of a slash?

Thanks, Trevor

tlf30 commented 2 years ago

Also, it would be great to have a way to not have any assumed port/address were added to the path. For example, a path of 192.168.1.10 should just be 192.168.1.10, not 192.168.1.10/1/0 which breaks many messages when trying to just send to an ethernet device. Perhaps this can be an option on the CIPDriver?

ottowayi commented 2 years ago

yeah I have no problem with this at all. I picked / or \ because that is how paths are displayed in Logix/Linx but forgot about the comma syntax used by other applications (or the message configuration dialog). It would just involve modifying this function and this one. I think it could as simple as adding .replace(',', '/') right after it does the backslash replace. Those functions should probably be consolidated, but I'll look into that more when I get further into my rewrite. I'm currently working on rewriting nearly the entire project, but I'm not sure when that will be done.

Only the LogixDriver automatically adds the backplane and slot 0 if only an IP address is supplied, the CIPDriver does not and is what should be used when connecting to non-PLC devices. If you're using the LogixDriver and generic_message but want to send the message to non-PLC device, you can use the route_path argument with connected=False and unconnected_send=True. The generic messaging will be done a bit differently after the rewrite, currently it expects the user to know too much about CIP routing and setting the right arguments.

If you want to make this change and submit a PR, you are welcome to. If not, I'll leave this issue open until I can do it, but I'm not sure when that will be.

tlf30 commented 2 years ago

Thank you @ottowayi

The comment from the CIPDriver constructor indicated that the path would be modified:

 def __init__(self, path: str, *args, **kwargs):
        """
        :param path: CIP path to intended target

            The path may contain 3 forms:

            - IP Address Only (``10.20.30.100``) - Use for a ControlLogix PLC is in slot 0 or if connecting to a CompactLogix or Micro800 PLC.
            - IP Address/Slot (``10.20.30.100/1``) - (ControlLogix) if PLC is not in slot 0
            - CIP Routing Path (``1.2.3.4/backplane/2/enet/6.7.8.9/backplane/0``) - Use for more complex routing.

            .. note::

                Both the IP Address and IP Address/Slot options are shortcuts, they will be replaced with the
                CIP path automatically.  The ``enet`` / ``backplane`` (or ``bp``) segments are symbols for the CIP routing
                port numbers and will be replaced with the correct value.

        """

I am not having luck connected directly to a 1756-ENBT/A module from the CIPDriver (Molex EthernetIP Tool works just fine though), but I will keep playing with it.

ottowayi commented 2 years ago

Yeah that docstring is inaccurate, the CIPDriver should not be making any modifications to the path. I just verified it against one of my ENBT's:

>>> from pycomm3 import CIPDriver, ClassCode, Services, ModuleIdentityObject
>>> enbt = CIPDriver('10.1.128.45')
>>> enbt.open()
True
>>> enbt.generic_message(
...          class_code=ClassCode.identity_object,
...          instance=b"\x01",
...          service=Services.get_attributes_all,
...          data_type=ModuleIdentityObject,
... )
Tag(tag='generic', value={'vendor': 'Rockwell Automation/Allen-Bradley', 'product_type': 'Communications Adapter', 'product_code': 58, 'revision': {'major': 6, 'minor': 1}, 'status': b'`\x00', 'serial': '002fff93', 'product_name': '1756-ENBT/A'}, type=ModuleIdentityObject(UINT(name='vendor'), UINT(name='product_type'), UINT(name='product_code'), Revision(name='revision'), BYTES(name='status'), UDINT(name='serial'), SHORT_STRING(name='product_name')), error=None)
>>>

The CIP driver is a bit basic, like lacking the identity method that the Logix one has, so I had to make the request manually. I'll be adding more features during the rewrite. If you can share logs I can maybe help you find what's wrong.

tlf30 commented 2 years ago

Ah, good to know, thank you. I will need to investigate why my enbt is not working then. Yep, my mistake, I needed to set routed_path to False when directly connecting.

ottowayi commented 2 years ago

lol that's what I mean, too many options that need to be configured just right or they won't work. Glad you got it working at least

ottowayi commented 2 years ago

commas support added in 1.2.10