Change autoReconnect not working on unreachable PLC

[flacohenao]

@gfcittolin

Right now, the default value for the autoReconnect option is set to 5000 ms.

The S7Endpoint calls the .connect() method internally if the autoReconnect option > 0, wich means that if you pass 0 through the constructor you have to explicitly call .connect() wich is fine.

But when you leave it as a default (5000ms) and PLC is not reachable, there is probably an error on the 'error'event throw by the transport layer and handled by _onConnectionTimeout() function but as I described on the #98 it does not emit anything but Also the connection is not attempting to reconnect itself.

So... It doesnt make any sense to have the autoReconnect option set by default in this case.

Would be probably better if... the autoReconnect option is set on 0 by default and leave the user to config the option for the autoReconnect as they want.

OR!, simply fix the autoreconnect on Unreachables PLCS but it would result on infinite loop of reconnection attempts when PLC is not reachable...

Another approach would be set an option on deciding if you want to autoreconnect if PLC is not reachable.. Some times you just want to reconnect when something else happens but not when PLC is not reachable... (just to send notification email or something)

The design is up to you!

This is how I tested it (very simple):

let conn = new nodes7.S7Endpoint({ host: '192.168.1.1', port: 102, rack: 0, slot: 2 }); //not reachable IP/PLC
    console.log('STATUS: ', conn._connectionState); //returns 1 (CONN_CONNECTING)
    setTimeout(() => {
        console.log('STATUS: ', conn._connectionState); //returns 0 after 10 seconds past (knowing that PLC has 5000 reconnect by deault)
        //here, I just connect my PLC to network
        setTimeout(() => {
            conn.connect();
            console.log('STATUS: ', conn._connectionState); //returns 1 but this time PLC is reachable
        }, 10000);
    }, 10000);

And on the:

conn.on('connect', () => {
        console.log('Connected!!');
        console.log('STATUS: ', conn._connectionState); //returns 2 wich is CONNECTED
    });

[gfcittolin]

The idea with the S7Endpoint class is to describe a "virtual instance" of a PLC. The user provides the information needed to reach the PLC (ip, rack, slot, etc), and the S7Endpoint is responsible for creating the connections, retrying them, etc.

In the event of a network error by the time you're trying to connect the first time (e.g. a EHOSTUNREACH or a ECONNRESET), there's no way we can determine if there's a configuration error, or you're just unlucky and there's indeed a temporary network problem. In the second case, you'd need to implement a logic to retry, and that's something already built-in.

That said, I think it makes more sense to make it always reconnect (regardless the error), and then handle the actions according to the error codes. We could even think about augmenting the error or creating other events to better distinguish among the many possible failure modes

[gfcittolin]

Just added a fix for this on 1ae5ba8, seems to work well now. Could you please try it out?

[flacohenao]

Working now!

Sure, it's also possible and valid to think in implement something that help to better distinguish the error!

Closing the issue!