search

moveit / srdfdom

Semantic Robot Description Format
BSD 3-Clause "New" or "Revised" License
12 stars 68 forks source link

ROS2 port #48

Closed mlautman closed 4 years ago

mlautman commented 5 years ago

This ports srdfdom to ROS2. Both python and C++ tests pass

mlautman commented 5 years ago

This was based on #45 by @vmayoral

mlautman commented 5 years ago

@rhaschke

mlautman commented 5 years ago

@henningkayser Please review

mlautman commented 5 years ago

@rhaschke

I suggest to split reformatting from actual changes. Right now, it's hard to review this PR at all.

I have split up the PR into separate commits for formatting and porting

Travis is not yet succeeding.

This is because moveit_ci is still broken: The command "docker pull moveit/moveit:crystal-ci" failed with error code 1.

Porting srdfdom is one step in getting moveit_ci ported to ROS2.

rhaschke commented 5 years ago

moveit_ci is still broken

I think, the only thing you need is to provide the corresponding docker container. Didn't you work on that already?

Porting srdfdom is one step in getting moveit_ci ported to ROS2.

srdfdom is completely unrelated to moveit_ci.

mlautman commented 5 years ago

moveit_ci is still broken

I think, the only thing you need is to provide the corresponding docker container. Didn't you work on that already?

It is a WIP. We realized that quite a few of the dependencies (eg srdfdom) weren't building correctly so we are porting them now.

Porting srdfdom is one step in getting moveit_ci ported to ROS2.

srdfdom is completely unrelated to moveit_ci.

Not exactly. The travis.yml for srdfdom clones moveit_ci and runs travis.sh from that repo. Porting everything is going to be kind of a chicken and the egg problem so we choose to start with the dependencies

mlautman commented 5 years ago

@vmayoral Can you give this a quick review?

rhaschke commented 5 years ago

Porting everything is going to be kind of a chicken and the egg problem.

There is a clear dependency chain, isn't it?

mlautman commented 5 years ago

moveit_ci pulls in https://raw.githubusercontent.com/ros-planning/moveit/$ROS_DISTRO-devel/moveit.rosinstall

It also tries do download the docker image for MoveIt which isn't for crystal yet. If we get the base moveit docker image building and update the rosinstall (without CI passing) then the chain will be broken and we'll be all clear to port moveit_ci

rhaschke commented 5 years ago

What is pulled in by moveit_ci is defined in .travis.yml. If you want to break the circle start there. Another dependency is indeed the docker image. For now, I suggest to remove/disable the wstool call and installing of MoveIt dependencies there as well. This was introduced in the Dockerfile to speed up Travis. travis.sh itself updates those dependencies as well.