Pyads integration with modifications by Patrick#288
Open
stlehmann wants to merge 6 commits intoBeckhoff:masterfrom
Open
Pyads integration with modifications by Patrick#288stlehmann wants to merge 6 commits intoBeckhoff:masterfrom
stlehmann 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>
Member
|
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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a modified version of #285 that was proposed by @pbruenn to resolve #284. It replaces #285.