Beckhoff / ADS

Beckhoff protocol to communicate with TwinCAT devices.
MIT License
493 stars 194 forks source link

AdsDevice Reconnection #169

Closed steeljav closed 1 year ago

steeljav commented 1 year ago

I am developing an ADS client application that will run as a normal (non-privileged) user on a TC/BSD CX5240. The device will be switched from run to config, stopped and restarted, etc and the client application must handle the ADS errors generated by these events.

I have reviewed the previous issues that addressed this, but the uses cases appear to be slightly different. I am loosely following the logic provided in the example in the ADS lib and connecting and reading data similar to this:

static auto connectToADS(args) {
    return std::make_unique<AdsDevice>(ipAddress, netID, AMSPORT_R0_PLC_TC3);
}
auto route = connectToADS(remoteNetId, remoteIpV4);

AdsHandle varHandle = route->GetHandle("ApplicationGVL.myStruct");

auto error = route->ReadReqEx2(ADSIGRP_SYM_VALBYHND,
                              *varHandle,
                              sizeof(struct myStruct),
                              &my_struct,
                              &bytesRead);

My question is when the PLC goes away and error == 1861 or similar, what is the correct way to re-establish the AdsDevice? I have tested logic similar to the following and it works, but is this correct or am I missing something?

if (error == 1861 || other error codes indicating disconnect) {
    while (!connected) {
        route->~AdsDevice();  // re-connection works with or without this. Not sure if it is necessary.
        route = connectToADS(remoteNetId, remoteIpV4);
        // sleep for 1 sec
    }
}

Thank you.

pbruenn commented 1 year ago

Looks reasonable to me. I would only replace

- route->~AdsDevice();  // re-connection works with or without this. Not sure if it is necessary.
+ // force old connection to be closed before we try a reconnect
+ route->reset(nullptr); 
  route = connectToADS(remoteNetId, remoteIpV4);

Depending on your application, you might find a way to let route go out of scope, so you won't need any explicit call to destruct the old device connection.

steeljav commented 1 year ago

Ok great. Thank you!