Windows Port (WIP) #238

lilustga · 2020-07-30T21:17:55Z

All of moveit2 now builds on windows. This is a work in progress PR intended for feedback.

Please explain the changes you made, including a reference to the related issue if applicable

Checklist

Required by CI: Code is auto formatted using clang-format
Extend the tutorials / documentation reference
Document API changes relevant to the user in the MIGRATION.md notes
Create tests, which fail without this PR reference
Include a screenshot if changing a GUI
While waiting for someone to review your request, please help review another open pull request to support the maintainers

henningkayser · 2020-08-04T12:41:03Z

moveit_core/background_processing/CMakeLists.txt

@@ -2,6 +2,7 @@ set(MOVEIT_LIB_NAME moveit_background_processing)

 add_library(${MOVEIT_LIB_NAME} SHARED src/background_processing.cpp)
 set_target_properties(${MOVEIT_LIB_NAME} PROPERTIES VERSION "${${PROJECT_NAME}_VERSION}")
+set_target_properties(${MOVEIT_LIB_NAME} PROPERTIES WINDOWS_EXPORT_ALL_SYMBOLS TRUE)


Can't we set this as a global property? (like https://github.com/ros-planning/navigation2/pull/1704/files#diff-3f9675091693ca07439ad55ceba95229R48)

@henningkayser PR #285 set's this as a global property

henningkayser

Great progress! It's also nice that there are not too many source file changes required. Did you successfully run the demo on this yet?

henningkayser · 2020-08-04T12:43:03Z

moveit_core/collision_detection/CMakeLists.txt

@@ -70,6 +72,12 @@ if(BUILD_TESTING)
  )
 endif()

+# Causes the visibility macros to use dllexport rather than dllimport,
+# which is appropriate when building the dll but not consuming it.
+target_compile_definitions(${MOVEIT_LIB_NAME}


nit, instructions like this would fit well in a single line

henningkayser · 2020-08-04T12:43:20Z

moveit_core/collision_detection/include/moveit/collision_detection/visibility_control.hpp

@@ -0,0 +1,70 @@
+// Copyright (c) 2019, Open Source Robotics Foundation, Inc.


Suggested change

// Copyright (c) 2019, Open Source Robotics Foundation, Inc.

// Copyright (c) 2020, Open Source Robotics Foundation, Inc.

henningkayser · 2020-08-04T12:49:06Z

moveit_core/collision_detection/include/moveit/collision_detection/visibility_control.hpp

+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef COLLISION_DETECTION__VISIBILITY_CONTROL_HPP_


Is there a way to simplify the visibility logic even more? It looks like these constants don't have to be defined for each class, so maybe we could use a single header for this.

Could we also maybe leverage rcpputils? https://github.com/ros2/rcpputils/blob/master/include/rcpputils/visibility_control.hpp

@henningkayser We need one macro per shared lib. However, we could put them all in one header if that's preferable. It could be one large file in moveit_common which would be include logic for every shared lib.

Could we also maybe leverage rcpputils? https://github.com/ros2/rcpputils/blob/master/include/rcpputils/visibility_control.hpp

@mikeferguson I'm not sure what you mean by leveraging rcpputils. It uses the same logic for symbol visibility but the RCPPUTILS_PUBLIC macro will switch between import and export based on whether rcpputils is the library currently being built.

lilustga · 2020-08-05T01:43:47Z

Great progress! It's also nice that there are not too many source file changes required. Did you successfully run the demo on this yet?

I did run the demo successfully! It appears to plan successfully and I see the visualization of the arm moving in Rviz. The only issue I've noticed is an error loading the dll for the rviz planning plugin. I'll be debugging that next.

Failed to load library C:\moveit_ws\install\moveit_ros_visualization\bin\moveit_planning_scene_rviz_plugin.dll

davetcoleman · 2020-08-18T23:23:37Z

@lilustga I'm excited to try this out, as I'm working more and more on a Windows machine now. Do you have any recent updates? I'd love to test this once the merge conflicts are resolved.

p.s. I made your title more concise and standard ;-)

tylerjw · 2020-09-03T19:04:41Z

I tried using this and am failing in the rosdep step and then I get an error when trying to run catkin. I admit I don't know what I'm doing in windows. Here are the outputs I'm seeing in case someone else knows a way around these:

rosdep errors:

c:\moveit_ws>rosdep install --from-paths src --ignore-src -r -y
ERROR: the following packages/stacks could not have their rosdep keys resolved
to system dependencies:
rospack: No definition of [python3-rosdep-modules] for OS [windows]
msft_ros_vcpkg: Cannot locate rosdep definition for [vcpkg]
robotiq_3f_gripper_control: No definition of [socketcan_interface] for OS [windows]
moveit_kinematics: No definition of [liburdfdom-tools] for OS [windows]
diagnostic_common_diagnostics: No definition of [hddtemp] for OS [windows]
python_orocos_kdl: No definition of [python3-future] for OS [windows]
moveit_tutorials: Cannot locate rosdep definition for [panda_moveit_config]
code_coverage: No definition of [lcov] for OS [windows]
eigenpy: No definition of [doxygen] for OS [windows]
spacenav_node: No definition of [libspnav-dev] for OS [windows]
ps3joy: No definition of [bluez] for OS [windows]
rviz_visual_tools: No definition of [libqt5x11extras5-dev] for OS [windows]
msft_ros2_env: Cannot locate rosdep definition for [ament_cmake]
wiimote: No definition of [cwiid] for OS [windows]
Continuing to install resolvable dependencies...
ERROR: the following rosdeps failed to install
  vcpkg: Vcpkg is not installed

cmake errors:

==> Processing catkin package: 'catkin'
==> cmake c:\moveit_ws\src\catkin -DCATKIN_DEVEL_PREFIX=c:\moveit_ws\devel_isolated\catkin -DCMAKE_INSTALL_PREFIX=c:\opt\ros\melodic\x64 -DCMAKE_BUILD_TYPE=RelWithDebInfo -DCMAKE_PREFIX_PATH=C:/opt/ros/melodic/x64;C:/opt/rosdeps/x64 -DCMAKE_VERBOSE_MAKEFILE=ON -DPYTHON_VERSION=2.7 -DPYTHON_EXECUTABLE=C:\opt\python27amd64\python.exe -DPYTHON_LIBRARIES=C:\opt\python27amd64\Libs -DCATKIN_SKIP_TESTING=ON -G NMake Makefiles in 'c:\moveit_ws\build_isolated\catkin'
-- Using CATKIN_DEVEL_PREFIX: C:/moveit_ws/devel_isolated/catkin
-- Using CMAKE_PREFIX_PATH: C:/opt/ros/melodic/x64;C:/opt/rosdeps/x64
-- Using PYTHON_EXECUTABLE: C:/opt/python27amd64/python.exe
-- Using default Python package layout
-- Could NOT find PY_em (missing: PY_EM)
CMake Error at cmake/empy.cmake:30 (message):
  Unable to find either executable 'empy' or Python module 'em'...  try
  installing the package 'python-empy'

lilustga · 2020-09-03T19:08:42Z

Hi @tylerjw,

Which version of ROS are you using?

tylerjw · 2020-09-03T19:11:19Z

melodic, as that is what was recommended on moveit.ros.org, should I be using noetic?

🤦 that's ROS1, feel free to laugh at me.

tylerjw · 2020-09-03T19:12:20Z

I need to figure out how to test this... which means I need to install foxy on windows first...

lilustga · 2020-09-03T19:32:04Z

@tylerjw you can use chocolatey to install a foxy prerelease:
choco upgrade ros-foxy-desktop -y --pre

I'm testing now as well, previously I ported moveit2 to Windows on eloquent where I did my original testing.

tylerjw · 2020-09-03T19:48:46Z

I get errors when I try that:

ros-foxy-desktop
By upgrading you accept licenses for the packages.
ros-foxy-desktop is not installed. Installing...
ros-foxy-desktop not installed. The package was not found with the source(s) listed.
 Source(s): 'https://chocolatey.org/api/v2/'
 NOTE: When you specify explicit sources, it overrides default sources.
If the package version is a prerelease and you didn't specify `--pre`,
 the package may not be found.
Please see https://chocolatey.org/docs/troubleshooting for more
 assistance.

Chocolatey upgraded 0/1 packages. 1 packages failed.
 See the log for details (C:\ProgramData\chocolatey\logs\chocolatey.log).

Failures
 - ros-foxy-desktop - ros-foxy-desktop not installed. The package was not found with the source(s) listed.
 Source(s): 'https://chocolatey.org/api/v2/'
 NOTE: When you specify explicit sources, it overrides default sources.
If the package version is a prerelease and you didn't specify `--pre`,
 the package may not be found.
Please see https://chocolatey.org/docs/troubleshooting for more
 assistance.

lilustga · 2020-09-03T19:53:24Z

I get errors when I try that:

ros-foxy-desktop
By upgrading you accept licenses for the packages.
ros-foxy-desktop is not installed. Installing...
ros-foxy-desktop not installed. The package was not found with the source(s) listed.
 Source(s): 'https://chocolatey.org/api/v2/'
 NOTE: When you specify explicit sources, it overrides default sources.
If the package version is a prerelease and you didn't specify `--pre`,
 the package may not be found.
Please see https://chocolatey.org/docs/troubleshooting for more
 assistance.

Chocolatey upgraded 0/1 packages. 1 packages failed.
 See the log for details (C:\ProgramData\chocolatey\logs\chocolatey.log).

Failures
 - ros-foxy-desktop - ros-foxy-desktop not installed. The package was not found with the source(s) listed.
 Source(s): 'https://chocolatey.org/api/v2/'
 NOTE: When you specify explicit sources, it overrides default sources.
If the package version is a prerelease and you didn't specify `--pre`,
 the package may not be found.
Please see https://chocolatey.org/docs/troubleshooting for more
 assistance.

Try running this first:

mkdir c:\opt\chocolatey
set ChocolateyInstall=c:\opt\chocolatey
choco source add -n=ros-win -s="https://aka.ms/ros/public" --priority=1

tylerjw · 2020-09-03T20:46:16Z

I got that to work and then I did this:

c:\opt\ros\foxy\x64\setup.bat
mkdir c:\moveit_ws\src
cd c:\moveit_ws 
curl -o overrides.rosinstall https://raw.githubusercontent.com/ms-iot/master/moveit2.repos

Now I'm struggling with how to use vcstool in windows (I'm assuming that's the next step as wstool isn't in ros2).

davetcoleman · 2020-09-04T20:20:30Z

@lilustga would you mind resolving the conflicting files to make testing this PR easier?

lilustga · 2020-09-04T20:56:44Z

@lilustga would you mind resolving the conflicting files to make testing this PR easier?

Hi Dave,

I'm currently working on a rebased version of this PR to resolve the conflicts. I'm also fixing some new build breaks which occurred in the move from eloquent on Windows to foxy on Windows. 18/21 packages building so far.

davetcoleman · 2020-09-15T22:19:37Z

moveit2.repos

@@ -1,7 +1,7 @@
 repositories:
  moveit2:
    type: git
-    url: https://github.com/ros-planning/moveit2
+    url: https://github.com/ms-iot/moveit2


need to revert these

lilustga · 2020-09-16T01:02:27Z

@davetcoleman @tylerjw

If you want to try out the latest binary you can do it easily now because we've added moveit2 to our foxy desktop build.

To try the demo on Windows you simply need to:

Install the ros-foxy-desktop pre-release: https://ms-iot.github.io/ROSOnWindows/GettingStarted/SetupRos2.html
Run the demo:
ros2 launch run_moveit_cpp run_moveit_cpp.launch.py

davetcoleman · 2020-09-19T16:06:58Z

@lilustga we'd really like to see this PR merged (conflicts resolved, feedback addressed) so we can use the latest version of MoveIt 2 from source on Windows.

lilustga · 2020-09-22T22:29:10Z

PR #285 moves the symbol visibility cmake code into a common package (as discussed last week at the moveit2 meeting)

Note that it doesn't replace the need for explicit symbol visibility macros (i.e. visibility_control.hpp) in exceptional cases like static global variables.

I will make another PR with the rest of the fixes which are staged in ms-iot/moveit2.

… to .cpp to prevent symbol related build breaks. rclcpp added to ament target dependencies. Compile options updated for MSVC. Assimp find_package updated.

…tection

…ne before team hack day.

Ace314159 · 2021-06-09T19:28:09Z

Are there any updates regarding this PR? I'd love to use MoveIt2 on Windows. Is there anything I can do to help?

…::glew.

…t32_t, added boost::placeholders

…tor.cpp

davetcoleman · 2021-08-16T04:53:53Z

This should be closed because #530 was merged, correct?

henningkayser reviewed Aug 4, 2020

View reviewed changes

davetcoleman changed the title ~~Draft PR - Work in Progress Windows Port~~ Windows Port (WIP) Aug 18, 2020

davetcoleman reviewed Sep 15, 2020

View reviewed changes

davetcoleman changed the base branch from master to main September 15, 2020 22:21

Tobias-Fischer mentioned this pull request Sep 30, 2020

Export symbols for windows moveit/moveit#2328

Closed

6 tasks

lilustga and others added 9 commits April 20, 2021 19:54

initial commit with visibility added and some functions moved from .h…

77bc858

… to .cpp to prevent symbol related build breaks. rclcpp added to ament target dependencies. Compile options updated for MSVC. Assimp find_package updated.

Visibiltiy fixes for robot model and robot state

adbd24b

Windows bringup work in progress. Working on moveit_core/collision_de…

bf07c17

…tection

collision_detection_fcl symbol visibility. Moveit_core now builds.

b6004c9

More symbol visibility fixes and cleanups of earlier commits. Milesto…

0325aed

…ne before team hack day.

Disable warnings about stl dll link

87d34b9

modified .repos file to point to forked dependencies.

246181d

Symbol visibility for robot state.

b11a6db

MoveIt2 on Windows fixes

73c350c

seanyen and others added 12 commits April 20, 2021 20:13

fix.

406f106

fix.

4e5a474

fix.

a9c07ec

remove all ASSIMP things.

858b38e

Fixing type deduction errors.

4778da7

Fix Boost

188ff0a

moveit_kinematics boost fixes

d8d9406

More Windows Fixes

3d81711

moveit_kinematics and run_moveit_cpp fix

1e2c28f

moveit_ros_visualization symbol visibility.

d0d4fa5

Removed unnecessary deps in .repos file. Added compiler flags

0e52403

Update moveit2.repos

7188875

ooeygui force-pushed the master branch from c92ee9d to 7188875 Compare June 3, 2021 18:20

ooeygui mentioned this pull request Jun 17, 2021

Finish Windows port #504

Closed

ooeygui and others added 5 commits June 22, 2021 14:04

Mopve the needle

ae0e1d7

Added symbol visibility headers and replaced GLEW_LIBRARIES with GLEW…

cfad43a

…::glew.

undef near, far and max from windows.h, changed type from uint to uin…

734a8b7

…t32_t, added boost::placeholders

Fix for planning scene rviz plugin symbol visibility.

d61357f

Added ifndef WIN32 for kill, fork, execv commands in warehouse_connec…

b56eba1

…tor.cpp

Ace314159 mentioned this pull request Jul 1, 2021

Fixes for Windows #530

Merged

6 tasks

davetcoleman closed this Aug 16, 2021

MikeWrock pushed a commit to MikeWrock/moveit2 that referenced this pull request Aug 15, 2022

Remove redundant moveit2 from repos (moveit#238)

fe3413c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Windows Port (WIP) #238

Windows Port (WIP) #238

lilustga commented Jul 30, 2020

henningkayser Aug 4, 2020

lilustga Sep 22, 2020

henningkayser left a comment

henningkayser Aug 4, 2020

henningkayser Aug 4, 2020

henningkayser Aug 4, 2020

mikeferguson Aug 31, 2020

lilustga Sep 22, 2020

lilustga Sep 23, 2020 •

edited

Loading

lilustga commented Aug 5, 2020

davetcoleman commented Aug 18, 2020

tylerjw commented Sep 3, 2020

lilustga commented Sep 3, 2020

tylerjw commented Sep 3, 2020 •

edited

Loading

tylerjw commented Sep 3, 2020

lilustga commented Sep 3, 2020 •

edited

Loading

tylerjw commented Sep 3, 2020

lilustga commented Sep 3, 2020

tylerjw commented Sep 3, 2020

davetcoleman commented Sep 4, 2020

lilustga commented Sep 4, 2020

davetcoleman Sep 15, 2020

lilustga commented Sep 16, 2020

davetcoleman commented Sep 19, 2020

lilustga commented Sep 22, 2020

Ace314159 commented Jun 9, 2021

davetcoleman commented Aug 16, 2021

		@@ -0,0 +1,70 @@
		// Copyright (c) 2019, Open Source Robotics Foundation, Inc.

	// Copyright (c) 2019, Open Source Robotics Foundation, Inc.
	// Copyright (c) 2020, Open Source Robotics Foundation, Inc.

Windows Port (WIP) #238

Windows Port (WIP) #238

Conversation

lilustga commented Jul 30, 2020

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

henningkayser left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lilustga Sep 23, 2020 • edited Loading

Choose a reason for hiding this comment

lilustga commented Aug 5, 2020

davetcoleman commented Aug 18, 2020

tylerjw commented Sep 3, 2020

lilustga commented Sep 3, 2020

tylerjw commented Sep 3, 2020 • edited Loading

tylerjw commented Sep 3, 2020

lilustga commented Sep 3, 2020 • edited Loading

tylerjw commented Sep 3, 2020

lilustga commented Sep 3, 2020

tylerjw commented Sep 3, 2020

davetcoleman commented Sep 4, 2020

lilustga commented Sep 4, 2020

Choose a reason for hiding this comment

lilustga commented Sep 16, 2020

davetcoleman commented Sep 19, 2020

lilustga commented Sep 22, 2020

Ace314159 commented Jun 9, 2021

davetcoleman commented Aug 16, 2021

lilustga Sep 23, 2020 •

edited

Loading

tylerjw commented Sep 3, 2020 •

edited

Loading

lilustga commented Sep 3, 2020 •

edited

Loading