Beckhoff / ADS

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

Batch Delete Notifications Failing #36

Closed gechelberger closed 7 years ago

gechelberger commented 8 years ago

According to the documentation for TC3 here:

http://infosys.beckhoff.de/content/1033/tc3_adsdeviceplc/html/tcadsdeviceplc_indexadsservice.htm?id=10419422066105251481

I should be able to delete a batch of notification handles in a single request. I can close them individually using AdsDelDevNotification, but closing 500 notifications on a single target/port takes up to a second when theoretically we should be able to do so in ~2ms.

// given valid handles from AdsAddDevNotification(...) std::uint32_t handle1, handle2, ...

std::vectorstd::uint32_t handles = { handle1, handle2 }; std::vectorstd::uint32_t responses(2);

std::uint32_t len = handle.size() * sizeof(std::uint32_t);

long code = AdsSyncReadWriteEx2( local_port, &target, 0xF086, //ADSIGRP_SUMUP_DELDEVNOTE, handles.size(), len, responses.data(), len, handles.data(), nullptr);

// tested against TC3.1.4020.10 on a CX5020 // code == 0 // responses[0] = 0x704 // responses[1] = 0x704

Am I misusing the index group/read write request, is this an issue with the AdsLib, or is this just an implementation bug/never implemented in the twincat runtime server?

pbruenn commented 8 years ago

ADSIGRP_SUMUP_DELDEVNOTE is not implemented in AdsLib, and I guess it isn't in TwinCAT, too.

To reduce wait time you could set the port timeout to 0 and ignore the AdsSyncDelDeviceNotificationReqEx error code (would be ADSERR_CLIENT_SYNCTIMEOUT)

AdsSyncSetTimeoutEx(local_port, 0);
for (auto h: handles) {
    AdsSyncDelDeviceNotificationReqEx(local_port, &target, h);
}

Or if you just want to clean up the easy way, make sure you allocated all notifications from the same local_port, and just close the port. The cleanup code will take care and delete the notifications for you.

AdsSyncSetTimeoutEx(local_port_notifications, 0);
AdsPortCloseEx(local_port_notifications);

But keep in mind there is a TCP connection in the background (I expect you are not working on localhost). So make sure the AdsDelRoute() is not called to early, or the EtherCAT Master might not get all AdsDelDevNotificationRequests.

gechelberger commented 8 years ago

Thanks for the response.

It looks to me like calling AdsDelRoute shouldn't be a worry as long as it's done on the same thread as AdsPortCloseEx.

Calling AdsPortCloseEx will definitely clean up all of the handles properly, but it looks like it still does that synchronously by walking the notifications opened on a port, telling it's dispatcher to erase them, which individually calls DeleteNotification to send the relevent DELNOTE AmsRequest on the AmsConnection for that dispatcher.

Do you know where I could submit a bug report/feature request for the SUMUP_DELDEVNOTE support in the twincat server itself?

pbruenn commented 8 years ago

You are right about the cleanup sequence. Setting the timeout to 0 would just simulate "asynchronous" closing, by not waiting for the response. In that case you can be sure that the operating systems sockets write() was called, but you can't be sure the data was already send to the wire. So if you call AdsPortCloseEx() to early, it would shutdown the socket before the whole buffer was send. I am not sure about your use case but maybe std::async(AdsPortCloseEx, local_port) fits better, if you just don't want to wait inside your main thread.

In general your local support office would handle TwinCAT bug reports and feature requests. The product manager for TwinCAT ADS is Sven Oberschmidt, he would have answered your question here, but currently he is on vacation.

gechelberger commented 8 years ago

Thanks. I missed the implication yesterday and this seems like it definitely mitigates my problem substantially, though you are correct that it does mean that I need to be careful about cleaning up routes.

Might I also suggest then that in AmsPort::Open or AmsPort::Close you reset the port's timeout to Default.

As to support, I've had really great experiences in the past working with Beckhoff's field technicians, but emailing support.usa@beckhoff has rarely gotten me results. I think it was 2 years ago that I got in touch about bugs in this sample that are still there today:

http://infosys.beckhoff.com/content/1033/tcsample_vc/html/tcadsdll_api_cpp_sample18.htm?id=50301061431851636012

Specifically, the release handles packet should be using index group 0xf006 rather than 0xf020, and the return code checks that follow are referencing the return code buffer from the original handle acquisition response rather than the release response (pBuffRes rather than pBuffRelRes).

pbruenn commented 7 years ago

I re-triggered your bug report in the TC2 example, too.