ros-industrial / stomp_ros

ROS packages for the STOMP planner (split out of industrial_moveit)
Apache License 2.0
37 stars 27 forks source link

Implement StompSmoothingAdapter #7

Closed henningkayser closed 4 years ago

henningkayser commented 5 years ago

I figured that the planning request adapter was missing and the version from https://github.com/ros-planning/moveit/pull/1081 didn't seem to work for me so I implemented this one from scratch. The adapter also implements the new initialize() function for loading it from different namespaces (https://github.com/ros-planning/moveit/pull/1530).

henningkayser commented 5 years ago

@gavanderhoorn This PR is currently only supported by the MoveIt master branch. I can change this to not use initialize() but I would like to have this feature merged if possible.

simonschmeisser commented 5 years ago

I would appreciate keeping this compatible to MoveIt melodic/1.0

jrgnicho commented 5 years ago

It currently does not build because the initialize function does not override anything and it doesn't seem to be called anywhere. What you could to is to attempt to get the configuration in the constructor and throw a runtime error if that fails. The error message should be informative enough so that the developer knows that the stomp configuration needs to be setup

mamoll commented 4 years ago

@henningkayser can you address @jrgnicho's comments?

Making it compatible with melodic also seems reasonable to me.

simonschmeisser commented 4 years ago

@henningkayser I could take over and finish this PR in the coming days, just a quick question about melodic compatibility: If we remove the override it will obviously compile but then fail as it has no config loaded. I could add loading the config from some default location there, ie the same as the normal planner plugin uses for example. Or is it possible to "guess" the location that would be passed in to initialize?

simonschmeisser commented 4 years ago

I added a PR against this branch to address the remarks by @jrgnicho in https://github.com/henningkayser/stomp_ros/pull/1

jrgnicho commented 4 years ago

Replaced by #20