-
Notifications
You must be signed in to change notification settings - Fork 121
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Impossible to drop connection with wrong connection parameters #70
Comments
My only concern with the second method is the situations where it might not auto-reconnect. For example, when connecting to an IBH or Hemholz netlink, a connection error connecting to a certain slot may mean the PLC at that MPI address (since the slot can map to MPI addresses) and we rely on nodeS7 to auto-reconnect in those situations without pushing that connection monitoring work to the application that uses it. Same with the IP address - it's hard to know whether the IP address is wrong or if the PLC just isn't reachable at the time, but we rely on it auto-connecting either way. How would your code work in these situations, and how would that work with your module? |
As far as I can test here, currently it won't reconnect if the IP address is not reachable, and that's why I thought the second option would be more in sync to the current behavior. But maybe it's the case we have to change this too, then, and implement the first fix for it. Right now I handle this case on my implementation (by trying to reconnect when the callback returns with an error) In any case, I can handle both, as far as the behavior is always consistent. |
After some loong time, I could finally come back to this. I've created a PR (#73) implementing the first solution, as it's safer in the way we have much less chance of breaking somebody's code by changing behavior. |
@plcpeople could you publish it on npm, please? You're the one with powers for that :) |
I've found an odd condition, where nodeS7 keeps trying to reconnect forever, and there's no way to stop it.
I've simulated the following with an S7-1200:
If you create a connection with a wrong parameter (e.g. wrong slot), we'll open the TCP socket,
onTCPConnect()
ist called, but then the connection is rejected after the ISO-on-TCP connection because of the wrong parameter.error
andclose
events are emitted, and they both correctly triggers the cleanup process by callingconnectionReset()
andresetNow()
.The main problem happens with the
connectTimeout
fromonTCPConnect()
: There, thepacketTimeout()
gets called with he"connect"
parameter, and this callsconnectNow()
again after some time. But as the parameters are wrong, the same error will happen again, and this will keep happening forever. There's no way to stop it but to kill the process.I see two possible solutions here:
dropConnection()
to stop the loop:dropConnection()
is the way we currently have to close the communication, so we could either:dropConnection()
signalizing we're dead, so 'connectNow()' would just skip it, ordropConnection()
clean the timer frompacketTimeout()
, preventing the connection reset to happen againconnectTimeout
timer fromonTCPConnect()
if we get anerror
event (on theconnectError()
).The second solution seems more correct in my point of view, as it should stop doing anything if the connection callback is called with an error.
What do you think about it? I can create then a PR with the fix for this using the solution chosen.
P.S.: This seems to be the cause of the issues st-one-io/node-red-contrib-s7#26 and st-one-io/node-red-contrib-s7#29
The text was updated successfully, but these errors were encountered: