Tweaks to build the whole project on Windows with CMake/MSVC 19 in 2026

[twdragon]

• Fixed the PLC port definitions mistakenly left in the BSD section in the test program
• Added CMake tweaks to successfully build the project on Windows natively using git/MSVC/PowerShell
• Implemented TcAdsDll_ROOT CMake variable to let the build system catch the libraries from non-standard locations. Tested on non-native installation in Windows 10.0.19045 and SDK 10.0.26100.0

[juarezr-mvtec]

Hello, I am not the maintainer, but I authored the FindTcAdsDll.cmake file that you are editing. I suggest that you should keep the support for finding TcAdsDll in old TwinCAT versions ? The way you are doing it is correct but will only work for the newest versions.
So instead of changing the variable value just try make it an array :
if (CMAKE_SIZEOF_VOID_P EQUAL 8)
set(TcAdsDll_IMPLIB_DIR "${TcAdsDll_ROOT_DIR}/Lib/x64" "${TcAdsDll_ROOT_DIR}/x64/lib")
set(TcAdsDll_DLL_DIR "${TcAdsDll_ROOT_DIR}/x64" "${TcAdsDll_ROOT_DIR}/../../Common64")
elseif (CMAKE_SIZEOF_VOID_P EQUAL 4)
set(TcAdsDll_IMPLIB_DIR "${TcAdsDll_ROOT_DIR}/Lib")
set(TcAdsDll_DLL_DIR "${TcAdsDll_ROOT_DIR}" "${TcAdsDll_ROOT_DIR}/../../Common32")
endif()

The find_library and find_file PATHS argument take an array anyways.

Additionally, since the default location is no longer C:/TwinCAT/.. in the new version so we can probably change it to something like this:

if (WIN32 AND NOT DEFINED TcAdsDll_ROOT)
# Typical install locations on Windows
set(_TcAdsDll_PATH "$ENV{SystemDrive}/TwinCAT/AdsApi/TcAdsDll")
if( NOT EXISTS ${_TcAdsDll_PATH})
# New TwinCAT 3.5+ default location
set(_TcAdsDll_PATH "$ENV{ProgramFiles(x86)}/Beckhoff/TwinCAT/AdsApi/TcAdsDll")
endif()
else ()

Try them out, tell me if it works.

[twdragon]

@juarezr-mvtec thanks for the suggestion! Unfortunately, direct introduction of array-like path lists did not work, so I used separate variables.

[twdragon]

@juarezr-mvtec I will also try to fix the Linux pipeline, but I cannot promise to do it now, so likely it will be a separate PR

[juarezr-mvtec]

Hello @twdragon

The problem is that we are passing the variable PATHS with "" like :

find_library(TcAdsDll_IMPLIB
NAMES TcAdsDll.lib
PATHS "${TcAdsDll_IMPLIB_DIR}"
NO_DEFAULT_PATH
)

this makes find_library consider TcAdsDll_IMPLIB_DIR as a single string instead of a list. So removing the "" fixes the issue with passing multiple paths. I tested this on my setup (old version path) and this is working for me:

if (CMAKE_SIZEOF_VOID_P EQUAL 8)
    set(TcAdsDll_IMPLIB_DIR "${TcAdsDll_ROOT_DIR}/Lib/x64" "${TcAdsDll_ROOT_DIR}/x64/lib")
    set(TcAdsDll_DLL_DIR "${TcAdsDll_ROOT_DIR}/x64" "${TcAdsDll_ROOT_DIR}/../../Common64")
elseif (CMAKE_SIZEOF_VOID_P EQUAL 4)
    set(TcAdsDll_IMPLIB_DIR "${TcAdsDll_ROOT_DIR}/Lib")
    set(TcAdsDll_DLL_DIR "${TcAdsDll_ROOT_DIR}" "${TcAdsDll_ROOT_DIR}/../../Common32")
endif ()
find_library(TcAdsDll_IMPLIB
        NAMES TcAdsDll.lib
        PATHS ${TcAdsDll_IMPLIB_DIR}
        NO_DEFAULT_PATH
)
find_file(TcAdsDll_LIBRARY
        NAMES TcAdsDll.dll
        PATHS ${TcAdsDll_DLL_DIR}
        NO_DEFAULT_PATH
)

I have not personally tested it but linux should work out of the box at least according to the maintainer (see #295).
I have another pull request where I remove some of those "not tested on linux" comments but it is still waiting for a review. #300

[twdragon]

@juarezr-mvtec now it looks like:

if (WIN32 AND NOT DEFINED TcAdsDll_ROOT)
    # Typical install locations on Windows
    list(APPEND _TcAdsDll_PATH
        "$ENV{SystemDrive}/TwinCAT/AdsApi/TcAdsDll" # Old location (TwinCAT < 3.5)
        "$ENV{ProgramFiles(x86)}/Beckhoff/TwinCAT/AdsApi/TcAdsDll" # New location (TwinCAT 3.5+)
    )
else ()
    set(_TcAdsDll_PATH)
endif ()
# Find the include headers

find_path(TcAdsDll_INCLUDE_DIR
        NAMES TcAdsApi.h TcAdsDef.h
        PATHS "${_TcAdsDll_PATH_OLD}" "${_TcAdsDll_PATH_NEW}"
        PATH_SUFFIXES "Include" "include"
)
# Find all related files base on the include files location. This is done
# assuming that the files are ordered as install by TwinCat. If they are
# in some other configuration this will not work.
if (WIN32)
    cmake_path(GET TcAdsDll_INCLUDE_DIR PARENT_PATH TcAdsDll_ROOT_DIR)
    if (NOT TcAdsDll_FIND_QUIETLY)
        message(STATUS "Searching TcAdsDll in ROOT ${TcAdsDll_ROOT_DIR}")
    endif ()
    if (CMAKE_SIZEOF_VOID_P EQUAL 8)
        set(TcAdsDll_IMPLIB_DIR "${TcAdsDll_ROOT_DIR}/Lib/x64")
        list(APPEND TcAdsDll_DLL_DIR 
            "${TcAdsDll_ROOT_DIR}/x64"
            "${TcAdsDll_ROOT_DIR}/../../Common64"
        )
    elseif (CMAKE_SIZEOF_VOID_P EQUAL 4)
        set(TcAdsDll_IMPLIB_DIR "${TcAdsDll_ROOT_DIR}/Lib")
        list(APPEND TcAdsDll_DLL_DIR 
            "${TcAdsDll_ROOT_DIR}"
            "${TcAdsDll_ROOT_DIR}/../../Common32"
        )
    endif ()
    find_library(TcAdsDll_IMPLIB
            NAMES TcAdsDll.lib
            PATHS ${TcAdsDll_IMPLIB_DIR}
            NO_DEFAULT_PATH
    )
    find_file(TcAdsDll_LIBRARY
            NAMES TcAdsDll.dll
            PATHS ${TcAdsDll_DLL_DIR}
            NO_DEFAULT_PATH
    )
else ()
    find_library(TcAdsDll_LIBRARY
            NAMES TcAdsDll
    )
endif ()

Attested to work on Windows. Thanks again!

[twdragon]

Hoping to get attention from @pbruenn. It could also close #275, as it implies usage of the Winsock2 library.

[twdragon]

@juarezr-mvtec I implemented and tested it, thanks again!

[twdragon]

@juarezr-mvtec I unfortunately don't have a different version of TwinCAT libraries on my workstation, so I cannot fully test the setup

[juarezr-mvtec]

Some more changes. With the changes this should work as expected.
I download the latest TwinCAT version and I tested this now with and without setting TcAdsDll_ROOT.

Once the changes are complete I will suggest that you SQUASH all of these changes into a single commit to keep a clean git history. :)

[twdragon]

Once the changes are complete I will suggest that you SQUASH all of these changes into a single commit to keep a clean git history. :)

@juarezr-mvtec I will do)) I have not so much time to work on this repo, so I do quick and dirty

[pbruenn]

@juarezr-mvtec @twdragon

Thanks a lot to both of you — there is a lot of useful work for CMake in here.
I was on vacation and had some other things to take care of after my return, so I apologize for the long wait.

Together with Claude I took a look at both of your PRs, and we ended up picking individual commits/changes and rewriting them a bit.

Everything we took keeps you credited with a Reported-by: trailer. Next time, please add a Developer Certificate of Origin sign-off and rebase your PR into clean commits with a logical split, so that we can merge it more easily.

The result is on patrickbr/next.

Everything from here on is AI generated:

What we took

From @juarezr-mvtec's branch:

your commit	ours
Fix warning C4566 on MSVC	9796cb4 cmake: build with UTF-8 character sets on MSVC
Fix wrong target when buiding shared lib	df9a1be cmake: fix target name in shared library definitions
Small cmake issues (AdsLibTestRef part)	d616eef cmake: skip AdsLibTestRef unless it can be built
Small cmake issues (spelling part)	9f5bbff cmake: spell TwinCAT consistently
Add catch anything on AmsRouter	cc61cdc AmsRouter: catch all exceptions in AddRoute
Use std::runtime_error instead of std::exception	c3bea77 AdsException: store the message in a fixed buffer

From @twdragon's branch:

your commit	ours
Old/new TwinCAT version installation paths fix	cacc683 cmake: find TwinCAT 3.1.4026 installations
CMake module introspection fix	d272456 cmake: require the TcAdsDll import library on Windows
Tweaks to build on Windows (AMSPORT part)	b7ce0a6 AdsLibTestRef: define AMSPORT_R0_PLC_TC3 if the SDK does not
README update	97f69e6 README: point CMake users at meson first
Tweaks to build on Windows (.gitignore part)	9ee6054 gitignore: ignore CMake install trees

Four follow-ups came out of reviewing the above, which we added on top:
9e65636 (a second bug in the same AddRoute catch handler), 1563475
(the guards around the TwinCAT targets spell the STANDALONE_ONLY option
as ONLY_STANDALONE, so it never reached them), and 20b7777 plus
9d54c19 (CI never built the shared library, which is why the TcAdsLib
target name bug survived so long).

What we changed while taking it

The TwinCAT version numbers. @twdragon, the commit and its comments
described the two layouts as "TwinCAT < 3.5" and "TwinCAT >= 3.5", but
there is no TwinCAT 3.5 — we think you meant the two current builds of
TwinCAT 3.1. The move out of %SystemDrive%\TwinCAT and the rename of
x64/lib to Lib/x64 happened in 3.1.4026; everything up to and
including 3.1.4024 uses the old layout. We have relabelled the
comments as "TwinCAT 3.1.4024 and older" / "TwinCAT 3.1.4026 and newer"
and retitled the commit accordingly. Please correct us if you were
actually looking at something else, since the paths themselves came from
you and we cannot verify them here.

C4566 / the infinity symbol. We set /utf-8 for MSVC in
CMakeLists.txt instead of prefixing the literal with u8. u8""
becomes a const char8_t[] in C++20 and stops converting to
std::string, so the prefix would break the build the day we raise the
standard. Worth knowing: meson already passes /utf-8 to every Visual
Studio like compiler by itself, so only the CMake build ever needed
this.

AdsException. We kept std::exception as the base and moved the
message into a fixed buffer rather than deriving from
std::runtime_error. Three reasons, all measured:

• std::runtime_error allocates its own message on top of the
  temporary it is constructed from. Per throw that is 4 allocations
  and 344 bytes, against 3 and 301 today. The fixed buffer is 1
  allocation and 208 bytes, and the constructor becomes noexcept
  too, so throw AdsException(err) can no longer fail with
  bad_alloc instead of the error it was meant to report.
• Changing the base moves errorCode from offset 8 to offset 16. A
  caller built against the old header then reads the __cow_string
  pointer instead of the error code. Keeping std::exception keeps
  the offset.
• AdsLib/standalone/AdsLib.cpp:40 and AmsConnection.cpp:327 catch
  const std::runtime_error&. After reparenting they would start
  swallowing AdsException and turning it into a return code.

AMSPORT_R0_PLC_TC3. Same fix, but guarded with #ifndef and placed
after #include <TcAdsDef.h>, matching how GLOBALERR_TARGET_PORT and
GLOBALERR_MISSING_ROUTE are handled three lines below. Defining it
before the include redefines whatever the SDK provides, which warns if
the SDK spells the value differently, for example as 0x353.

README. Reframed, and two fixes to the snippet. We build, test and
ship this library with meson; the CMakeLists.txt is a community effort
rather than a second supported build system, and most of its changes
come from outside contributors. So instead of a Windows section
alongside the meson ones, we moved it to the end of the README and said
that plainly: use meson, move your project to meson if you can, and
here is the CMake fallback if you cannot. No offence intended to the
work — we would just rather be honest about the level of support than
promise something we will not deliver.

The snippet itself had two problems. –ClassName used U+2013 EN DASH
instead of an ASCII hyphen, so the line cannot run as pasted. And
"$TWINCAT_ADS_API_ROOT" reads a PowerShell shell variable, not the
environment variable the comment above it describes — that needs
$env:TWINCAT_ADS_API_ROOT. We also replaced the Get-CimInstance
core-count expression with a bare --parallel, which lets the native
build tool pick.

What we did not take, and why

The TcAdsDll_ROOT block in FindTcAdsDll.cmake. It does not work,
and it is not needed. The else() branch clears the variable the
preceding if just set:

WIN32=TRUE  ROOT_set=TRUE  -> _TcAdsDll_PATH=''
WIN32=TRUE  ROOT_set=FALSE -> _TcAdsDll_PATH='SystemDrive/...;ProgramFiles/...'
WIN32=FALSE ROOT_set=TRUE  -> _TcAdsDll_PATH=''

So setting TcAdsDll_ROOT always produced an empty search list. It
appears to work because CMake already honours <PackageName>_ROOT
inside find modules (CMP0074, and we require CMake 3.23) — we confirmed
a fake install tree is still found with PATHS deliberately empty. The
module's own warning has said "Try setting TcAdsDll_ROOT" since before
this patch. We kept the README documenting it, since the feature is
real, just provided by CMake.

NAMES TcAdsDll -> NAMES TcAdsDll.lib. We tested both against a
fake TcAdsDll.lib; they resolve identically. Hardcoding the suffix
only de-generalises the module for MinGW, so we left the original.

The 32 bit lib / Lib pair. Commented as old versus new
locations, but Windows paths are case-insensitive, so both entries are
the same directory. The 64 bit pair (x64/lib versus Lib/x64) is
genuinely different and we took that one.

Removing the NO_DEFAULT_PATH comment. It explains a non-obvious
flag — without it find_library picks the 32 bit import library
regardless of the configuration. We kept the comment.

Removing FindTcAdsDll.cmake only tested on WINDOWS and the Linux
TODOs. The justification was that Linux now works, but it does not
yet: with TcAdsDll installed, cmake -DBUILD_SHARED_LIBS=ON still fails
because BHF_ADS_EXPORT_C is applied to the TcAdsLib target in
AdsLib/CMakeLists.txt, which wraps our declarations in extern "C"
where they collide with the SDK's own declarations:

AdsLib/AdsLib.h:34:6: error: conflicting declaration of C function
'long int AdsSyncReadReqEx2(long int, const AmsAddr*, ...)'

Once that is fixed the warning can go. Happy to take a patch for it —
it needs a decision on whether the TwinCAT variant should export the C
API at all.

For next time

Three things that would make patches much quicker to merge:

1. Run the codestyle check before pushing. ./tools/run-uncrustify.sh check — this is the same thing CI runs. A couple of the C++ hunks
   would have failed it, and FindTcAdsDll.cmake picked up trailing
   whitespace on four lines.

2. One concern per commit. "Small cmake issues" bundled a real build
   fix, a comment removal we disagreed with and a spelling correction;
   we had to split it into two and drop the middle. Likewise the
   introspection fixes landed as several commits fixing each other.
   Small, self-contained commits can be reviewed, bisected and reverted
   independently — and the ones we agree with can be merged without
   waiting for the ones we do not. Subject in the imperative under about
   50 characters, body wrapped at 72 explaining what and why. A
   Fixes: <12-char-sha> ("subject") tag when fixing a known commit is
   very welcome.

3. Signed-off-by:. None of the commits carried one, and
   CONTRIBUTING requires the Developer Certificate of Origin on every
   commit. We cannot add it on your behalf, which is why these landed
   with you as Reported-by: rather than as author. If you would like
   authorship, please resend with your sign-off and we will preserve it.

Thanks again — the Windows side of this project is much better off for
it.

[juarezr-mvtec]

Hello @pbruenn, thanks for taking the changes I will just close my pull request then.

Another question, so what is supported to be the cpp standard of the project ? On the previous cmake version it was set to cpp14 so I just keep that (top cmake sets CMAKE_CXX_STANDARD on line 19) . On the readme you mention gcc 10.2 which default to cpp14 as the supported standard so it makes sense to set it to cpp14, but there are also "stable" support of cpp17 so it might make sense to rise it, definitely no support of cpp20 if you want to keep that old toolchain compatibility.

Once that is fixed the warning can go. Happy to take a patch for it —
it needs a decision on whether the TwinCAT variant should export the C
API at all.

I would say that yes, that if AdsLib is compile as a shared library (either linux or windows) it will be nice to export a C API that can be use. That would mean that when loading the StandAlone or TwinCAT version of the AdsLib will work the same from the consumer application perspective. On the StandAlone version gives the full communication protocol while on the TwinCAT version you are basically just proving a thin layer between TcAdsDll in most cases.

However, doing so will require that you use different interface, with different function names (e.g., add a prefix AdsLib_X... before C function names) to avoid clashes with what is already exported on TcAdsDll. Additionally, you might have to duplicate certain structures and constant values (e.g., error codes). You will also need an export header of public symbols that is cross compatible with Windows and a better separation of the public and private interfaces of AdsLib. It will be a major refactor and a mayor version break. The benefits is that doing so will stabilize your interfaces (ABI forward compatibility) and make it much easier to make future changes and summit contributions without fear of any small change will break a customers old project. It will also make it much easier for US to push patches to already deploy system by just updating the shared library instead of force recompile of the binary which in this day an age is something that is very important. This is what i was trying to express when i wrote #301.

[pbruenn]

Hi @juarezr-mvtec,
the changes are merged now, I didn't add a new version label just yet as I have some more patches in the pipeline.

Regarding C++ standards. I have to say I became very conservative regarding c++ standards and will only bump the minimal version if I really need a new feature from more modern standards.

Regadring the shared library thing I answered it in your issue to be able to close this PR here if @twdragon agrees we merged everything important from it.

[twdragon]

Regadring the shared library thing I answered it in your #301 (comment) to be able to close this PR here if @twdragon agrees we merged everything important from it.

@pbruenn sure. I think we can close this PR, as you have a selection of changes. I agree. Thanks for merging!