Port to ROS 2 and add MARA files by vmayoral · Pull Request #24 · moveit/moveit_resources

vmayoral · 2019-02-26T11:47:20Z

Requesting @davetcoleman's review

vmayoral · 2019-02-26T11:57:19Z

Obviously, this PR needs to be pointed to a new ros2 branch as usual

with moveit_resources ported moveit/moveit_resources#24 this PR re-enables the testing and while allowing the submodule to still compile

vmayoral · 2019-02-27T12:00:16Z

@rhaschke?

rhaschke · 2019-02-27T18:02:59Z

I don't have time to look into any PR before March 9th.
Reading that in the title that you want to add more robot file (MARA?) to this repo, I can already tell that I don't like the idea. This repo is for unit testing primarily and should be kept as small as possible.
@davetcoleman Any other opinions?

vmayoral · 2019-02-27T18:31:47Z

@rhaschke I understand you want to keep it small but I can guess that most of the initial testing for MoveIt! 2 port will be with MARA. Shouldn't that justify adding it to the ros2 branch of it?

davetcoleman · 2019-02-28T06:17:05Z

CMakeLists.txt

-
-# devel space file
-set(MOVEIT_TEST_RESOURCES_DIR ${CMAKE_CURRENT_SOURCE_DIR})
-configure_file("include/${PROJECT_NAME}/config.h.in" "${CATKIN_DEVEL_PREFIX}/include/${PROJECT_NAME}/config.h")


@rhaschke i forget what this cmake magic was suppose to do. is it still needed?

That's needed to define MOVEIT_TEST_RESOURCES_DIR within cpp.
Should not be removed!

davetcoleman · 2019-02-28T06:17:44Z

README.md

 This repository includes various resources (URDFs, meshes, moveit_config packages) needed for MoveIt! testing.

-[![Build Status](https://travis-ci.org/ros-planning/moveit_resources.png?branch=master)](https://travis-ci.org/ros-planning/moveit_resources)
+[![Build Status](https://travis-ci.org/AcutronicRobotics/moveit_resources.png?branch=master)](https://travis-ci.org/AcutronicRobotics/moveit_resources)


please do not change the Travis badges to Acutronic

Suggested change

[![Build Status](https://travis-ci.org/AcutronicRobotics/moveit_resources.png?branch=master)](https://travis-ci.org/AcutronicRobotics/moveit_resources)

[![Build Status](https://travis-ci.org/ros-planning/moveit_resources.png?branch=master)](https://travis-ci.org/ros-planning/moveit_resources)

README.md

davetcoleman · 2019-02-28T06:19:07Z

mara_description/README.md

@@ -0,0 +1,8 @@
+# mara arm Gazebo


Suggested change

# mara arm Gazebo

# MARA URDFs

Why gazebo?

@ahcorde, can you help with this please?

please accept my code suggestion (its easy!)

davetcoleman · 2019-02-28T06:19:32Z

mara_description/launch/mara_upload.launch

@@ -0,0 +1,4 @@
+<?xml version="1.0"?>
+<launch>
+  <!-- <param name="robot_description" command="$(find xacro)/xacro --inorder '$(find mara_description)/urdf/mara_nogripper.xacro'" /> -->


this should not be commented out

@ahcorde, ping

davetcoleman · 2019-02-28T06:21:29Z

mara_description/urdf/table.obj

@@ -0,0 +1,47 @@
+# table.obj


i don't think we need a table for unit tests. please re-add this in the future if you actually have a use-case for this

@ahcorde, can you help removing this please?

davetcoleman · 2019-02-28T06:21:55Z

mara_description/urdf/table.obj

+f 4/3/5 8/2/5 6/4/5
+s 6
+f 7/1/6 1/2/6 5/3/6
+f 5/3/6 1/2/6 3/4/6


the next file is a png file... that is def not needed for tests

v4hn · 2019-02-28T14:09:49Z

The resources package is meant for testing *without* hardware and should always be included in the respective tests. Also for ros2. For testing with actual hardware, every setup I know maintains their own moveit_config anyway (because of adjusted controller timeouts, robot descriptions, ...). So testing with independent configurations and actual robots is of course very necessary, but should probably not be included in the upstream repositories.

with moveit_resources ported moveit/moveit_resources#24 this PR re-enables the testing while allowing the submodule to still compile

ahcorde · 2019-03-09T16:25:57Z

I think this PR should point to ros-planning:ros2 not to master, Am I right?.

Anyways I will remove the table.

…e robot

ahcorde · 2019-03-09T16:35:37Z

I just commit some changes. Leaving only the description of mara without additional hardware. Like fanuc or panda

vmayoral · 2019-03-10T18:17:00Z

have all your concerns been addressed now @davetcoleman?

davetcoleman · 2019-03-13T00:58:31Z

CMakeLists.txt

-set(MOVEIT_TEST_RESOURCES_DIR ${CMAKE_INSTALL_PREFIX}/${CATKIN_PACKAGE_SHARE_DESTINATION})
-configure_file("include/${PROJECT_NAME}/config.h.in" "${CATKIN_DEVEL_PREFIX}/include/${PROJECT_NAME}/.config_install.h")
+set(MOVEIT_TEST_RESOURCES_DIR "share/${PROJECT_NAME}")
+configure_file("include/${PROJECT_NAME}/config.h.in" "include/${PROJECT_NAME}/.config_install.h")


@rhaschke this looks good to me, what do you think?

Doesn't ament support devel and install space anymore?
This change, hardcoding the install location (in share), looks like breaking devel-space usage.

davetcoleman · 2019-03-13T00:58:58Z

README.md

 This repository includes various resources (URDFs, meshes, moveit_config packages) needed for MoveIt! testing.

-[![Build Status](https://travis-ci.org/ros-planning/moveit_resources.png?branch=master)](https://travis-ci.org/ros-planning/moveit_resources)
+[![Build Status](https://travis-ci.org/AcutronicRobotics/moveit_resources.png?branch=master)](https://travis-ci.org/AcutronicRobotics/moveit_resources)


Suggested change

[![Build Status](https://travis-ci.org/AcutronicRobotics/moveit_resources.png?branch=master)](https://travis-ci.org/AcutronicRobotics/moveit_resources)

[![Build Status](https://travis-ci.org/ros-planning/moveit_resources.png?branch=master)](https://travis-ci.org/ros-planning/moveit_resources)

davetcoleman · 2019-03-13T00:59:46Z

mara_description/README.md

@@ -0,0 +1,8 @@
+# mara arm Gazebo


please accept my code suggestion (its easy!)

davetcoleman · 2019-03-13T01:00:05Z

mara_description/launch/mara_upload.launch

@@ -0,0 +1,4 @@
+<?xml version="1.0"?>
+<launch>
+  <!-- <param name="robot_description" command="$(find xacro)/xacro --inorder '$(find mara_description)/urdf/mara_nogripper.xacro'" /> -->


davetcoleman · 2019-03-13T01:01:05Z

@ahcorde agreed it should target ros2 branch, I'll switch it

* Port robot_model submodule of moveit_core to ROS 2 * Clean submodule CMakeLists.txt Remove unnecesary commands listed already one level higher * robot_model, simplify CMakeLists.txt Following guidelines provided at #8 (comment) * Fix testing and comment it out for now moveit_resources hasn't been ported just yet * robot_model, cleanup code * Revert changes, reenable properties version * robot_model re-enable tests with moveit_resources ported moveit/moveit_resources#24 this PR re-enables the testing while allowing the submodule to still compile * robot_model submodule, fix logging * robot_model use LOGGER Follow consensus about logging in moveit_core submodules * Remove robot_model log.h * robot_model, apply clang format * moveit_core, robot_model: remove setDefaultIKAttempts

following from https://github.com/ros-planning/moveit_resources/blob/master/CMakeLists.txt I've made some modifications

Cmakelist fixes taking into account ros-planning

henningkayser · 2020-01-06T17:56:33Z

Closing in favor of #26

vmayoral added 2 commits February 26, 2019 11:49

Add MARA modular robot arm files

281e3e3

Port package to ROS 2

1b70db8

vmayoral added a commit to AcutronicRobotics/moveit2 that referenced this pull request Feb 26, 2019

robot_model re-enable tests

45a5eaa

with moveit_resources ported moveit/moveit_resources#24 this PR re-enables the testing and while allowing the submodule to still compile

davetcoleman reviewed Feb 28, 2019

View reviewed changes

vmayoral added a commit to AcutronicRobotics/moveit2 that referenced this pull request Mar 5, 2019

robot_model re-enable tests

70735cc

with moveit_resources ported moveit/moveit_resources#24 this PR re-enables the testing while allowing the submodule to still compile

This was referenced Mar 5, 2019

Port robot_model submodule of moveit_core to ROS 2 moveit/moveit2#10

Merged

Continuous integration moveit/moveit2#16

Closed

Removing some of the descriptions, leaving only the description of th…

1e06c86

…e robot

davetcoleman reviewed Mar 13, 2019

View reviewed changes

davetcoleman changed the base branch from master to ros2 March 13, 2019 01:01

anasarrak and others added 3 commits May 23, 2019 17:28

Cmakelist fixes taking into account ros-planning

2c1e51e

following from https://github.com/ros-planning/moveit_resources/blob/master/CMakeLists.txt I've made some modifications

Make the test compile without changes in the code

dbc2078

Merge pull request #1 from AcutronicRobotics/fixes

0faf2b7

Cmakelist fixes taking into account ros-planning

JafarAbdi mentioned this pull request Dec 17, 2019

Port CMakeLists.txt & package.xml to ROS2 #26

Merged

henningkayser closed this Jan 6, 2020

	[![Build Status](https://travis-ci.org/AcutronicRobotics/moveit_resources.png?branch=master)](https://travis-ci.org/AcutronicRobotics/moveit_resources)
	[![Build Status](https://travis-ci.org/ros-planning/moveit_resources.png?branch=master)](https://travis-ci.org/ros-planning/moveit_resources)

Conversation

vmayoral commented Feb 26, 2019

Uh oh!

vmayoral commented Feb 26, 2019

Uh oh!

vmayoral commented Feb 27, 2019

Uh oh!

rhaschke commented Feb 27, 2019

Uh oh!

vmayoral commented Feb 27, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

v4hn commented Feb 28, 2019 via email

Uh oh!

ahcorde commented Mar 9, 2019

Uh oh!

ahcorde commented Mar 9, 2019

Uh oh!

vmayoral commented Mar 10, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davetcoleman commented Mar 13, 2019

Uh oh!

henningkayser commented Jan 6, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants