search

Beckhoff / ADS

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

Replace C style function pointer by a std::function object. #70

Closed vyacheslavisaev closed 5 years ago

vyacheslavisaev commented 6 years ago

Hello,

I am suggesting to replace C style function pointer which is used for the callbacks by std::function object. That would make call-back function more flexible and would help to improve ADS client design. For example: it would allow to use binded or lambda functions.

I hope there is no contr arguments which might prevent us from this change!

Thank you, Slava Isaev Cosylab d.d.

pbruenn commented 6 years ago

Hi Slava, I would absolutely like to get ride of that c-style function pointer. However AdsSyncAddDeviceNotificationReqEx() is a legacy function, so we can't simply change the signature. Are you aware of AdsLibOOI on branch dev-ooi-v2? That's an attempt to make the ADS library easier to use and more straightforward. There your change would be possible. However development on AdsLibOOI is stalled for some time since there wasn't enough interest. Regards, Patrick

vyacheslavisaev commented 6 years ago

Hi Patrick,

Thanks for pointing me to the AdsLibOOI. I will check it. We are planning to use ADS on a Linux machine. Is it also Linux compatible?

If we have to support signature of the function for the legacy needs. Can we introduce a modern function which will provide modern interface and keep legacy function as a wrapper to expose functionality for C style users? What do you think?

Thanks, Slava

pbruenn commented 6 years ago

Yes, Linux was the primary use case to develop this library.

Right now, AdsLibOOI is a wrapper on top of legacy ADS functions. However my long term plan was always to reverse this and have a C++ library with an optional C wrapper around for legacy users. That would avoid these crazy exception->error code->exception conversion you will see on the AdsLibOOI branch. And I think some core code will be simplified, too.