Update cmake support for AdsLib#295
Conversation
2262ce5 to
ff91c39
Compare
pbruenn
left a comment
There was a problem hiding this comment.
Hi @izonfreak ,
thanks for this PR. I can't really review the CMake changes, as they are more of a community effort. I chose Meson over CMake because, at the time, it seemed much cleaner to me (and I still think so today).
That said, the reality is that CMake is used in many more places, and I understand the importance of supporting it. Therefore, I'm happy to merge the CMake--related changes as they are, provided that my small CMake test continues to pass:
Lines 113 to 114 in 6788651
However, please split the other minor changes into separate commits, and then we can merge this PR, thanks.
p.s.: I just updated the CONTRIBUTING file to make clear we prefer logical split commits.
| rowNames{ { | ||
| "1", "2", "5", "10", "20", "50", "100", "200", "500", | ||
| "\u221E" // infinity symbol | ||
| u8"\u221E" // infinity symbol |
There was a problem hiding this comment.
Please keep this separate from the CMake changes and put it into its own commit explaining why this is needed.
| } | ||
| if (SOCK_STREAM == type) { | ||
| if (::connect(m_Socket, rp->ai_addr, rp->ai_addrlen)) { | ||
| if (::connect(m_Socket, rp->ai_addr, static_cast<int>(rp->ai_addrlen))) { |
There was a problem hiding this comment.
Again, please separate this from the CMake changes and put it into its own commit and explain why we need the change.
| .vscode/ | ||
|
|
||
| # Clion | ||
| .idea/ |
There was a problem hiding this comment.
Well, I know this file is already quite heavy and I already forgot forgotten about it, if until you didn't remember until you reminded me.
I don't want to bloat it even more with IDE specific stuff.
Git supports $XDG_CONFIG_HOME/git/ignore which is much better suited for developer specific IDE files. You won't even need to copy & paste your .gitignore settings into every project anymore.
https://git-scm.com/docs/gitignore
Hello @pbruenn, I will move the other small changes to their own pull request, but they are mostly fixes so that the sources compile without warnings in the different toolchains i am testing with. Before we merge I do have some questions mostly about lib names because i saw it on another issue.
Are my name assumptions correct ? The reason for the different names is so that they can all be install together in the same location like the image bellow. The ones with (d) are the debug version.
cmake can find the right version and file base on the consuming project configuration:
Ands is use like: |
|
Hi @izonfreak, On TC/BSD it's: On Linux and FreeBSD our binaries are called:
We don't set SONAME at the moment, neither on Linux or FreeBSD: At the moment I increment the version manually in two places debian/changelog and AdsTool/main.cpp see 6788651 Normally, debian/changelog is the source of our versions, so I suggest you use If you have access to mybeckhoff and can download "Beckhoff RT Linux" you can install it into a VM. KVM works fine and Hyper-V should, too. |
This commit modernized the cmake implementations and fixes issues when
building with the MSVC (Microsoft Visual Compiler). Building with clang
and GCC compilers was also tested and supported.
This cmake implementation allows building both variants of
the AdsLib (standalone and with twincat rounter) on a single cmake
project and is also possible to install both on the same directory (for
easy import) in both debug and release builds.
The changes included on this commit are as follows:
- Increase cmake min version to 3.23
- Set the current AdsLib version in project(). Read directly from
debian/changelog file.
- Add basic module FindTcAdsDll.cmake for locating the TcAdsDll (windows
only). Linux support needed.
- Allow building both shared and static library version. Simply set
BUILD_SHARED_LIBS to ON or OFF according to your needs.
- Add generation of a cmake config file (AdsConfig.cmake) on install and
enebale using find_package on the consuming project. One only needs to
specify the Ads_ROOT variable.
- Use GNU-style installation directories.
- Provide library headers under include/ads
- Improve import target naming.
- AdsLib : Standalone variant of the AdsLib.
- TcAdsLib : TwinCAT rounter variant of the AdsLib. Only if TcAdsDll
is found and available. Otherwise the target is no created and a
warning message is display.
- Fatal error if BUILD_SHARED_LIBS=ON when using the microsoft visual
studio compiler. The reson is that the AdsLib does not provide the
needed import and export decorators.
- Allow building tests using cmake.
- Allow building AdsTool using cmake in both standalone and twincat
rounter variants.
- Add build options:
- WITH_TEST : When OFF the project does not create/build targets for the
different unitests and integration tests.
- WITH_EXAMPLES : When OFF the project does not create/build targets
for the project examples.
- STANDALONE_ONLY : When ON create/build only the AdsLib stand alone
version (does seach for TwinCat TcAdsDll)
Steps for testing/building.
- Install cmake min v3.23
- Configure the cmake project with the needed options:
cmake -G "Ninja"
-B %BUILD_DIR%\release
-DCMAKE_INSTALL_PREFIX=%INSTALL_DIR%
-DCMAKE_BUILD_TYPE=Release
-DCMAKE_PREFIX_PATH=C:\TwinCAT\AdsApi\TcAdsDll
-DCMAKE_POSITION_INDEPENDENT_CODE=ON
-DBUILD_SHARED_LIBS=OFF
-DWITH_TESTS=OFF
-DWITH_EXAMPLES=OFF
-DSTANDALONE_ONLY=OFF
- Build & install:
cmake --build %BUILD_DIR%\release
cmake --build %BUILD_DIR%\release --target install
ff91c39 to
61b27b0
Compare
|
Hello @pbruenn, the requested changes have been implemented. The only thing that i omitted is adding support for finding libTcAdsDll.so until I can download and test the twincat runtime on linux and make sure that it works. I might have time for this later this year. |


This commit modernized the cmake implementations and fixes issues when building with the MSVC (Microsoft Visual Compiler). Building with clang and GCC compilers was also tested and supported.
This cmake implementation allows building both variants of the AdsLib (standalone and with twincat rounter) on a single cmake project and is also possible to install both on the same directory (for easy import) in both debug and release builds.
The changes included on this commit are as follows:
Steps for testing/building.
cmake -G "Ninja"
-B %BUILD_DIR%\release
-DCMAKE_INSTALL_PREFIX=%INSTALL_DIR%
-DCMAKE_BUILD_TYPE=Release
-DCMAKE_PREFIX_PATH=C:\TwinCAT\AdsApi\TcAdsDll
-DCMAKE_POSITION_INDEPENDENT_CODE=ON
-DBUILD_SHARED_LIBS=OFF
-DWITH_TESTS=OFF
-DWITH_EXAMPLES=OFF
-DSTANDALONE_ONLY=OFF
cmake --build %BUILD_DIR%\release
cmake --build %BUILD_DIR%\release --target install