Closed Ace314159 closed 3 years ago
This change looks good. We (Azure Edge Robotics) were making an equivalent fix.
@Ace314159 In parallel, We are working on MoveIt2 for Windows - https://github.com/ros-planning/moveit2/issues/504.
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.
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.
@Ace314159 great catch!
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.
I changed 2 things to get this building on Windows and working with MoveIt 2.
empty_vector_
when build the tests. This is because theWINDOWS_EXPORT_ALL_SYMBOLS
CMake property doesn't work for static variables, so in such cases manual visibility control headers are required.ament_export_dependencies