Pyads integration with modifications by Patrick#288
Pyads integration with modifications by Patrick#288stlehmann wants to merge 6 commits intoBeckhoff:masterfrom
Conversation
We’re seeing increasing demand for a shared library version of the standalone AdsLib. Here we go and mark the external C functions, so we can build a shared library in the next step. Suggested-by: Stefan Lehmann <stlm@posteo.de> Signed-off-by: Patrick Bruenn <p.bruenn@beckhoff.com>
More and more users want a shared library of the standalone version. Here we go and finally add it to our build. Suggested-by: Stefan Lehmann <stlm@posteo.de> Signed-off-by: Patrick Bruenn <p.bruenn@beckhoff.com>
In my opinion, the "struct AdsNotificationHeader" in the TwinCAT SDK is broken. It relies on "#pragma pack" and forces unaligned access on 64-bit machines. A long time ago, I decided to fix it by reordering the structure, which is only used on the client side and is not sent "over the wire". However, projects like pyADS[1], which want to dynamically choose whether to use the standalone AdsLib or the TwinCAT router, needed to patch this. Let's give them a compile-time option to do so, and use it ourselves when we build a shared library, which is most likely used "in parallel" with the TwinCAT variant. [1] https://github.com/stlehmann/pyads Signed-off-by: Patrick Bruenn <p.bruenn@beckhoff.com>
|
Thats good news. I took your version and just moved the wrapper from standalone to the generic code, because compiled with TcAdsDll we actually have these functions, too. I wonder if we even need them in the dynamically linked version. However, thats seems the clean approach. So could you double check https://github.com/Beckhoff/ADS/tree/patrickbr/for-stlehmann-pyads-integration again and if its working I will merge it and publish a new version which includes some other minor fixes aswell. 6ddad87 EDIT: just wanted to mention @stlehmann to force a notification |
|
@pbruenn this works like a charm. The patch passed all pyads tests. Looking forward to the new version. Thanks for the greate work. Let me know if you need any further help. Cheers 👍 |
I moved the ifdefs a little to only include the relevant functions (not the namespaced ones). Hope this still works. The change is now on master. |
|
This has been resolved by 77953d5 |
2 participants
This is a modified version of #285 that was proposed by @pbruenn to resolve #284. It replaces #285.