Beckhoff / AdsToJava

This library is intended for use in ADS client applications written in the Java programming language.
MIT License
8 stars 2 forks source link

Time out when adding route to not responding TC device #10

Open martinste1n opened 4 months ago

martinste1n commented 4 months ago

Hi,

first thank you very much for the effort you put into this library. It has been a tremendous help!

Nevertheless we are facing an in issue with the following function: https://github.com/Beckhoff/AdsToJava/blob/5a385ffef249f660dc500547ad7f7d3904143204/src/main/java/de/beckhoff/jni/tcads/AdsCallDllFunction.java#L1481

Actually I'm not sure if can be fixed here or in the general ads-lib.

Problem We connect to multiple TC devices and everything works just as expected as long as the machine are online. Yet when the machines are offline / not reachable the call to AdsCallDllFunction.adsAddLocalRoute will block indefinetly. We do not receive a timeout exception or similar.

Here is an excerpt of the code in which we face the issue:

long portopen = AdsCallDllFunction.adsPortOpenEx();
        if (portopen ==0) {
             throw new Exception(String.format("[%s] Could not open Port", devId));
        }
        OpenConnect.getInstance().logger.info("Open Port: " + portopen);

        this.sourceAmsPort = portopen;

        this.amsaddr = new AmsAddr();
        this.amsaddr.setPort((int) this.targetAmsPort);
        this.amsaddr.setNetIdStringEx(this.targetAmsNetId);

        if(isReachable(this.targetIPAddress)) {
            // Since the AMS routing isn't handled by the TwinCAT AMS Router, we need to
            // tell the AdsLib which IP address is associated with the AMS NetId.
            OpenConnect.getInstance().logger
                    .info("Add local route for " + this.targetIPAddress + " with net id " + this.amsaddr.getNetIdString());

            long err = AdsCallDllFunction.adsAddLocalRoute(this.amsaddr.getNetId(), this.targetIPAddress);

            if (err != 0) {
                OpenConnect.getInstance().logger.info("Error: Open communication: 0x" + Long.toHexString(err));
                throw new Exception("Error: Open communication: 0x" + Long.toHexString(err));
            } else {
                OpenConnect.getInstance().logger.info("Success: Open communication!");
                OpenConnect.getInstance().logger.info("Connect to " + this.amsaddr.getNetIdString() + ":"
                        + this.amsaddr.getPort() + " from " + this.sourceAmsNetId + ":" + this.sourceAmsPort);
                //wait for connection to be established
                Thread.sleep(10000);
                this.connected = true;
                OpenConnect.getInstance().logger.info("Beckhoff client connected");
            }

        }else {
            String msg = String.format("Error: Host %s (%s:%s) is not reachable", this.devId, this.targetIPAddress, this.targetAmsPort);
            OpenConnect.getInstance().logger.info(msg) ;
            throw new Exception(msg);
        }
    }

    private boolean isReachable(String hostIP) {
        //return true;
        Process p1;
        String OS = System.getProperty("os.name").toLowerCase();
        String pingCountChar = OS.contains("win")? "n":"c";
        try {
            p1 = java.lang.Runtime.getRuntime().exec(String.format("ping -%s 4 %s",pingCountChar, hostIP));
            int returnVal = p1.waitFor();
           return returnVal==0;
        } catch (Exception e) {
            OpenConnect.getInstance().logger.error("Error while testing if device is reachable",e);
            return false;
        }

    }

We already implemented our own way to test if the machine is online before we try to connect, yet there are edge cases where the machine is just available yet it wont respond to request yet. If this case arise, the call will block and the whole thread will block. In Java there is no option to interrupt the thread while the JNI Call is ongoing. Therefore we need to kill the whole application, which is not suitable for our use cases.

Possible Solution In our case the optimal solution would be an timeout, after which an exception is thrown or an error code is returned. Yet, I don't know if this can be done inside the ads2java lib or needs to be implemented in the ads-lib additionally.

Thanks in advance! Martin

vossjannik commented 2 months ago

Hi @martinste1n ,

AdsCallDllFunction.adsAddLocalRoute calls this function in the Linux ADS library: https://github.com/Beckhoff/ADS/blob/20d4a67970a6b954b32f6e51caf622da26adc13c/AdsLib/standalone/AmsRouter.cpp#L27

That function does not have a timeout parameter. Please create an issue in the ADS project. If they add a timeout parameter, i can make it accessible in Java.

Maybe you can set a system-wide timeout for DNS lookups? That might be a workaround for your use case.

Best regards, Jannik