Fixes for usage on windows

[Ace314159]

I changed 2 things to get this building on Windows and working with MoveIt 2.

1. There were linking errors of being unable to find the member empty_vector_ when build the tests. This is because the WINDOWS_EXPORT_ALL_SYMBOLS CMake property doesn't work for static variables, so in such cases manual visibility control headers are required.
2. When building moveit_core, including model.h gave an error of being unable to find tinyxml2.h. I was able to fix this by using ament_export_dependencies

[ooeygui]

This change looks good. We (Azure Edge Robotics) were making an equivalent fix.

[ooeygui]

@Ace314159 In parallel, We are working on MoveIt2 for Windows - moveit/moveit2#504.

[codecov]

Codecov Report

Merging #91 (1e712ba) into ros2 (c6ea3aa) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             ros2      #91   +/-   ##
=======================================
  Coverage   68.21%   68.21%           
=======================================
  Files           3        3           
  Lines         607      607           
=======================================
  Hits          414      414           
  Misses        193      193           

Impacted Files	Coverage Δ
include/srdfdom/model.h	100.00% <ø> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c6ea3aa...1e712ba. Read the comment docs.

[Ace314159]

There was one more change I had to make that I didn't realize. Loading the MoveIt Rviz plugins fails because it is unable to find srdfdom.dll in the path. It seems to only look for it in the bin folder when it is in the lib folder. This last commit installs the dll in the bin folder and that fixes it.

[ooeygui]

@Ace314159 great catch!

[JafarAbdi]

Thanks for the fix and sorry for the delay!

This looks good to me, unfortunately, we still don't have a standard/documented way to export the symbols in ros-planning repos

I'll wait for a few hours to see if other maintainer have a concern otherwise I'll merge it

[JafarAbdi]

Do we still need this .? adding SRDFDOM_EXPORT to SRDFWriter should make this obsolete, right .?

[Ace314159]

Yeah, I think that should make it obsolete. We would need to add SRDFDOM_PUBLIC because the export should only occur while building and it should import when another library includes it.

However, you would need to manually add SRDFDOM_PUBLIC to all new classes, and that might be easy to forget. I still think the global export should be used since it's much simpler than tagging all new classes and more closely matches the behavior of other compilers.

[JafarAbdi]

Should we use GenerateExportHeader .? Do you know what are the drawbacks .? I'm happy to open a PR against your branch

[Ace314159]

There has been some discussion here: moveit/moveit2#286 (review), and I think using the visibility headers instead of the cmake function would be better. To quote from links from that discussion:

In summary, I prefer the boilerplate header we use (e.g. https://github.com/ros2/rclcpp/blob/231b991098d685ff019c44edb80a8ff170d65e4a/rclcpp/include/rclcpp/visibility_control.hpp#L25-L54) to the boilerplate generated code that cmake has to generate every time you build. Neither change, but the static one is nicer in my opinion because you don't waste time generating it each time and because it lets static code analysis (or like autocomplete for text editors) work without having to first build the package and have the build/install space for that package on the path for your tool. Also, if you were to port a package to a different build system (e.g. bazel or something) then you don't have to also port the behavior of generate_export_header.

[ooeygui]

We have a document on Visibility on Windows: https://aka.ms/ros/visibility. For many cases, the cmake macro for Windows will work; but there are limitations - such as when there is a static export from a class.