feat: add remapping argument to MoveItPy initialization #3367
Conversation
|
Hi @sea-bass @henningkayser @peterdavidfagan , Would you be able to take a look when you have some time? |
There was a problem hiding this comment.
Ideally, I wanted MoveItPy to support remapping via the remappings option specified in the launch file. However, due to the design philosophy of rclpy, this seems difficult.
Can you please elaborate on this? What's the difficulty?
I see this more as a design flaw of the MoveItPy constructor.
py_binding_tools allows any arguments (including remapping args).
I always wanted to rewrite MoveItPy to use py_binding_tools, but never found time for it...
| @@ -62,7 +64,7 @@ void initMoveitPy(py::module& m) | |||
|
|
|||
| .def(py::init([](const std::string& node_name, const std::string& name_space, | |||
| const std::vector<std::string>& launch_params_filepaths, const py::object& config_dict, | |||
| bool provide_planning_service) { | |||
| bool provide_planning_service, const py::object& remappings) { | |||
There was a problem hiding this comment.
Just use a std::map as argument here. This simplifies casting code below.
Wrapping the map into std::optional allows to handle None values.
| bool provide_planning_service, const py::object& remappings) { | |
| bool provide_planning_service, const std::optional<std::map<std::string, std::string>>& remappings) { |
| if (py::isinstance<py::list>(remappings)) | ||
| { | ||
| for (auto item : remappings) | ||
| { | ||
| py::tuple item_tuple = item.cast<py::tuple>(); | ||
| if (item_tuple.size() != 2) | ||
| { | ||
| throw std::runtime_error("Remapping must be a list of tuples with 2 elements"); | ||
| } | ||
| launch_arguments.push_back("--remap"); | ||
| launch_arguments.push_back(item_tuple[0].cast<std::string>() + | ||
| ":=" + item_tuple[1].cast<std::string>()); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
| if (py::isinstance<py::list>(remappings)) | |
| { | |
| for (auto item : remappings) | |
| { | |
| py::tuple item_tuple = item.cast<py::tuple>(); | |
| if (item_tuple.size() != 2) | |
| { | |
| throw std::runtime_error("Remapping must be a list of tuples with 2 elements"); | |
| } | |
| launch_arguments.push_back("--remap"); | |
| launch_arguments.push_back(item_tuple[0].cast<std::string>() + | |
| ":=" + item_tuple[1].cast<std::string>()); | |
| } | |
| } | |
| if (remappings.has_value()) | |
| { | |
| for (const auto &[key, value]: remappings) | |
| { | |
| launch_arguments.push_back("--remap"); | |
| launch_arguments.push_back(key + ":=" + value); | |
| } | |
| } |
Description
Previously, the topics started by MoveItPy could not be remapped.
To address this, I added an argument for remapping, allowing remapping to be handled within Python scripts.
Ideally, I wanted MoveItPy to support remapping via the remappings option specified in the launch file. However, due to the design philosophy of rclpy, this seems difficult. As a compromise, I have ensured that remapping can at least be performed via an argument.
Checklist