Beckhoff / ADS

Beckhoff protocol to communicate with TwinCAT devices.
MIT License
515 stars 197 forks source link

Use std::function to define callbacks? #142

Open Jopand opened 3 years ago

Jopand commented 3 years ago

https://github.com/Beckhoff/ADS/blob/0e0f73f709841c8ee3a0c1ff20ebb46165662d9f/AdsLib/AdsNotificationOOI.h#L5

The current callback typedef is a bit "stiff". I have to provide a static method (correct me if I'm wrong) or a lambda with nothing piped in. If I pipe anything into the callback lambda, I get compiler errors.

If it was replaced with std::function it would be more flexible. I could use non-static methods or lambdas where I can pipe data into it.

The new suggested callback typedef would then be: typedef std::function<void(const AmsAddr* pAddr, const AdsNotificationHeader* pNotification, uint32_t hUser)> PAdsNotificationFuncExConst;

marcus-sonestedt commented 2 years ago

Agree. I made a static function and then a dispatcher via a std::unordered_map from uint32_t to my std::functions.

if uint32_t had been uint64_t, or void*, it'd be a bit easier as you could use a pointer to the function or something, but having the full std::function would be even better, then we could use lambdas as well.

Sssswl commented 2 years ago

Hi, could you please elaborate or provide a code snippet to show how this could be done? Did you made the dispatcher a static function and pass it as the callback?

I'm having the same issue. I tried to pass in the class pointer as uint32_t, but then found out that the pointer is 64 bit long and the callback could not find my class member variables because the pointer got shortened.

stefanbesler commented 2 years ago

@Sssswl use a map instance like std::map<uint32_t, SomeClass> callbacks. When you register the notification you generate a hash (or simply an increasing number), which you pass as hUser and* store it in the map callbacks[hash] = &yourclass

For a code snippet see a little wrapper that I made some time ago for Qt/QML

I hope that is helpful for you

Sssswl commented 2 years ago

@stefanbesler Your code was very helpful! My main problem was just where to put this std::map. I didn't realize I could also make it static. Everything's working now.

Thank you very much for your help!