nimbuscontrols / EIPScanner

Free implementation of EtherNet/IP in C++
https://eipscanner.readthedocs.io/en/latest/
MIT License
244 stars 99 forks source link

Unable to talk to two devices simulataneously #66

Open ashwinmudigonda2 opened 3 years ago

ashwinmudigonda2 commented 3 years ago

I have 2 EthernetIP devices (both TURCK models) with 2 distinct IP addresses. I am able to talk to each of them separately and set bits. But when I try to talk to both of them in the same program, it does not work.

Here is the Connection1 details from the EDS file:

$
[Connection Manager] Connection1 = 0x04010002, $ 0-15 = supported transport classes $ -------------------------------------- $ @16 = trigger: cyclic $ 17 = trigger: change of state $ 18 = trigger: application $ 19 = trigger: reserved $ -------------------------------------- $ 20-23 = trigger: reserved $ -------------------------------------- $ 24 = transport type: listen-only $ 25 = transport type: input-only $ @26 = transport type: exclusive-owner $ 27 = transport type: redundant-owner $ -------------------------------------- $ 28-30 = reserved $ 31 = Client = 0 / Server = 1 $ -------------------------------------- 0x44640405, $ @0 = O->T fixed size supported $ 1 = O->T variable size supported $ @2 = T->O fixed size supported $ 3 = T->O variable size supported $ --------------------------------------- $ 4-5 = O->T number of bytes per slot (obsolete) $ 6-7 = T->O number of bytes per slot (obsolete) $ --------------------------------------- $ 8-10 = O->T Real time transfer format $ 11 = reserved $ -------------------------------------- $ 12-14 = T->O Real time transfer format $ 15 = reserved $ -------------------------------------- $ 16 = O->T connection type: NULL $ 17 = O->T connection type: MULTICAST $ @18 = O->T connection type: POINT2POINT $ 19 = O->T connection type: reserved $ -------------------------------------- $ 20 = T->O connection type: NULL $ @21 = T->O connection type: MULTICAST $ @22 = T->O connection type: POINT2POINT $ 23 = T->O connection type: reserved $ -------------------------------------- $ 24 = O->T priority: LOW $ 25 = O->T priority: HIGH $ @26 = O->T priority: SCHEDULED $ 27 = O->T priority: reserved $ -------------------------------------- $ 28 = T->O priority: LOW $ 29 = T->O priority: HIGH $ @30 = T->O priority: SCHEDULED $ 31 = T->O priority: reserved $ -------------------------------------- ,,Assem104, $ O->T RPI, size in bytes, format ,,Assem103, $ T->O RPI, size in bytes, format ,, $ config #1 size in bytes, format ,Assem106, $ config #2 size in bytes, format "Exclusive Owner", $ Connection Name "", $ Fixed data size, input inst. 103, output inst. 104 "20 04 24 6A 2C 68 2C 67"; $ Path

Here are the settings that I modified within ImplicitMessagingExample.cpp:

            parameters.connectionPath = {0x20, 0x04, 0x24, 0x6A, 0x2C, 0x68, 0x2C, 0x67};
            parameters.o2tRealTimeFormat = true;
            parameters.originatorVendorId = 501; // VendCode from VIPA EDS.
            parameters.originatorSerialNumber = 5038;
            parameters.t2oNetworkConnectionParams |= NetworkConnectionParams::P2P;
            parameters.t2oNetworkConnectionParams |= NetworkConnectionParams::SCHEDULED_PRIORITY;
            parameters.t2oNetworkConnectionParams |= 10;
            parameters.o2tNetworkConnectionParams |= NetworkConnectionParams::P2P;
            parameters.o2tNetworkConnectionParams |= NetworkConnectionParams::SCHEDULED_PRIORITY;
            parameters.o2tNetworkConnectionParams |= 10;

            parameters.o2tRPI = 1000000;
            parameters.t2oRPI = 1000000;
            parameters.transportTypeTrigger |= NetworkConnectionParams::CLASS1;

I modified the ImplictMessagingExample.cpp by wrapping the initialization, open, close, set and get bits into a class and associated functions.

Example:

    eipWrapper::EIPWrapper wrapper1("192.168.1.111");
    wrapper1.open();
    std::this_thread::sleep_for (std::chrono::seconds(3));
    eipWrapper::EIPWrapper wrapper2("192.168.1.112");
    wrapper2.open();

This does not throw any errors. Output:

[INFO] Registered session 102
[INFO] Send request: service=0x54 epath=[classId=6 objectId=1]
[INFO] Connection state: 0
[INFO] Open IO connection O2T_ID=3494509568 T2O_ID=224198657 SerialNumber 1
[INFO] Open UDP socket to send data to 192.168.1.111:2222
*****
[INFO] Registered session 102
[INFO] Send request: service=0x54 epath=[classId=6 objectId=1]
[INFO] Connection state: 0
[INFO] Open IO connection O2T_ID=3495502080 T2O_ID=224198658 SerialNumber 2
[INFO] Open UDP socket to send data to 192.168.1.112:2222
*****
[INFO] Close connection connection T2O_ID=224198657
[INFO] Send request: service=0x4e epath=[classId=6 objectId=1]
[INFO] Close connection connection T2O_ID=224198658
[INFO] Send request: service=0x4e epath=[classId=6 objectId=1]
[INFO] Unregistered session 102
[INFO] Unregistered session 102

However, when I try to set the 0th bit for one of the devices.

[INFO] Registered session 102
[INFO] Send request: service=0x54 epath=[classId=6 objectId=1]
[INFO] Connection state: 0
[INFO] Open IO connection O2T_ID=3530795776 T2O_ID=779026433 SerialNumber 1
[INFO] Open UDP socket to send data to 192.168.1.111:2222
*****
[INFO] Registered session 102
[INFO] Send request: service=0x54 epath=[classId=6 objectId=1]
[INFO] Connection state: 0
[INFO] Open IO connection O2T_ID=3531788800 T2O_ID=779026434 SerialNumber 2
[INFO] Open UDP socket to send data to 192.168.1.112:2222
*****
[INFO] Received: secNum=2 data=[0][0][0][0][0][0][0][0][0][0]
[INFO] Received: secNum=3 data=[0][0][0][0][0][0][0][0][0][0]
[INFO] Received: secNum=4 data=[0][0][0][0][0][0][0][0][0][0]
[ERROR] Received data from unknown connection T2O_ID=779026434
[WARNING] Connection O2T_ID=3530795776 has fixed size 10 bytes but 0 bytes are to send. Don't send this data.
[INFO] Current bytes before: [0][0][0][0][0][0][0][0][0][0]
[INFO] Current bytes after: [0][0][1][0][0][0][0][0][0][0]
[INFO] data size: 10
[WARNING] Connection SeriaNumber=1 is closed by timeout4100000:4000000
[INFO] Closed
[WARNING] Attempt to close an already closed connection
[INFO] Close connection connection T2O_ID=779026434
[INFO] Send request: service=0x4e epath=[classId=6 objectId=1]
[INFO] Unregistered session 102
[INFO] Unregistered session 102

I tried to launch each device in its own process. I had a bit toggle every second for a second. The moment the second device received the bit, I see this on the first one's log:

[DEBUG] Received data from connection T2O_ID=1703018497
[INFO] Received: secNum=1 data=[0][0][0][0][0][0][0][0][0][0]
[WARNING] Connection SeriaNumber=1 is closed by timeout4099000:4000000
[INFO] Closed
[DEBUG] Close socket fd=4

And the first one stops responding.

Expected behavior I expect the library to be able to set the bits on each of the devices independently.

Environment (please complete the following information):

chruetli commented 1 year ago

@ashwinmudigonda2 Did you find a solution here?

chruetli commented 1 year ago

I did a deep dive into the library code and I think I found the reason why multiple instances doesn't work. I think the reason is a missunderstanding/missinterpretation of UDP sockets. UDP sockets are - in contrast to TCP socket - not bound to any remote address. Looking at the lib one can see that for every instance a UDP socket with local port 2222 is created. As soon as the second instance is started and it's socket is opened, the first socket will starve to death btw to timeout ([WARNING] Connection SeriaNumber=1 is closed by timeout). The socket of the second instance will consume the whole traffic to port 2222 (as we see [ERROR] Received data from unknown connection T2O_ID=....)

The solution sounds simple, just use one socket for all connections and use recvfrom() to optain the senders address.

I can see two possibilities to implement a solution:

  1. Create one static UDP socket receiving all traffic, the class has a list of data handlers that are called when traffic arives. (observer like)
  2. Extend the (existing) connection handler class to handle multiple connections. One object has to be created and passed to every newly created instance.

Hope this helps. I'm trying to do a "quick fix". But for a clean solution there should be some discussion.

Btw. Is there any simulator (Python or so) for devices with class 1 connections? Would be simpler for development than always using multiple HW-devices

ashwinmudigonda2 commented 1 year ago

@chruetli thanks for the analysis. I decided to pass over this library and switch to a Modbus based solution. It was easily extensible and more stable.

sriharshav commented 1 year ago

I did a deep dive into the library code and I think I found the reason why multiple instances doesn't work. I think the reason is a missunderstanding/missinterpretation of UDP sockets. UDP sockets are - in contrast to TCP socket - not bound to any remote address. Looking at the lib one can see that for every instance a UDP socket with local port 2222 is created. As soon as the second instance is started and it's socket is opened, the first socket will starve to death btw to timeout ([WARNING] Connection SeriaNumber=1 is closed by timeout). The socket of the second instance will consume the whole traffic to port 2222 (as we see [ERROR] Received data from unknown connection T2O_ID=....)

The solution sounds simple, just use one socket for all connections and use recvfrom() to optain the senders address.

I can see two possibilities to implement a solution:

  1. Create one static UDP socket receiving all traffic, the class has a list of data handlers that are called when traffic arives. (observer like)
  2. Extend the (existing) connection handler class to handle multiple connections. One object has to be created and passed to every newly created instance.

Hope this helps. I'm trying to do a "quick fix". But for a clean solution there should be some discussion.

Btw. Is there any simulator (Python or so) for devices with class 1 connections? Would be simpler for development than always using multiple HW-devices

@chruetli I am facing same issue, and your analysis is logical, do you have a quick fix?

chruetli commented 1 year ago

@sriharshav No, at the moment I cannot provide you with a quick fix.

sriharshav commented 1 year ago

I managed to get this to work. In my system, there is one device with an 8-bit addressing scheme, and another with a 16-bit addressing scheme. It was only implicit IO that I needed.

  1. As a result, I updated the constructor of the connection manager to take an std::map of the endpoint as well as a shared pointer to the message router.

  2. The connection manager's state variables are updated accordingly. messgaeRouter to messageRouterMap. In the same way, incarnationId corresponds to incarnationIdMap.

  3. The findOrCreateSocket method of forwardOpen takes the IP address of the remote end point as an input to find or create a socket.

This limits the ability to connect to multiple adapters. The UDPBoundSocket binds to the local IP address and the port. I fixed it so that the local NIC IP is passed. This way, all inbound traffic is routed based on the received connection Id. With the setBeginReceive handler, the connection Id is at index 0.

There are also general changes to forwardClose and constructors based on the updated map structure for instance variables, as noted in point 2.

It works with both Balluff EIP and Banner Bar Code simultaneously. For sending, it opens one UDP port per device, but for receiving, it reuses the same UDP bound socket for all devices.

chruetli commented 1 year ago

I the meantime I also came to a "quick" fix. I tried to extract all changes from my working code intot the following repo https://github.com/nimbuscontrols/EIPScanner/compare/master...chruetli:EIPScanner:mruetti-multiple-connections-fix. There maybe some flaws (and there are still debug-statements). @sriharshav Can you make a pull with your solution an we can discuss our solutions. It would be great to see multiple implicit connection support in the mainline of the library!

chruetli commented 1 year ago

@sriharshav One point I have problems with is the reconnect mechanism when a connection was lost. E.g. disconnecting the Ethernet cable. Does your implementation handle this case out-of-the-box?

chruetli commented 1 year ago

@sriharshav Would it be possible to see your code?

Maddin-619 commented 2 months ago

Thank you @chruetli for your "quick fix"! I tried to fix the remaining issues with the reconnect. I am not aware of the whole EIP spec and I did not understand everything, but I discoverd some lifetime issues with the UDPBoundSocket singleton. In caes of a disconnect the destructor was called and closed the socket. I think the socket should always be open. I have a quick fix of your quick fix :rofl: https://github.com/Maddin-619/EIPScanner/tree/fix/multi_connections This isn't production ready and I think we need some refactorings, but I tested it with multiple Balluff devices and everything worked flawlessly.