tylerjw
commented
2 years ago These are my DRAFT notes about this topic. I will update this as I have more time to read and think about how this interface should be designed.
Constructors should not fail
Right now parameters for MoveIt are spread across several different constructors, PlanningComponent being one of them.
Instead, we should refactor this to use generate_parameter_library from a single yaml file to make it easier to search for parameters. Then the struct from this library should be passed into the constructor to PlanningComponent.
Secondly, the joint model group is gotten from the robot model and the group name. This should just be a parameter to the constructor. As the joint model group is a raw pointer the constructor should still throw if it is a nullptr. Eventually, when we refactor jmg to not be a nullptr this throw can be removed from the constructor.
Default destructor is fine
Actions that result in Data that can be made a separate independent thing
- Planning pipeline parameter loading should be separate from the constructor of PlanningComponent and passed in as a parameter.
Methods that could just be functions
- getNamedTargetStates uses group_name from constructor to get joint_model_group from RobotModel (read from moveitcpp) and then calls joint_modelgroup->getDefaultStateNames and returns the result.
std::vector<std::string> get_named_target_states(auto robot_model, auto group_name) {
if (auto jmg = robot_model->getJointModelGroup(group_name);
jmg != nullptr) {
return jmg->getDefaultStateNames();
}
return {};
}This should maybe return a std::optional as there is a difference between a group_name existing and having no named states and a group_name not existing.
std::optional<std::vector<std::string>>> get_named_target_states(
auto robot_model, auto group_name) {
if (auto jmg = robot_model->getJointModelGroup(group_name);
jmg != nullptr) {
return jmg->getDefaultStateNames();
}
return std::nullopt;
}Another way to look at it is this calls a function on joint_model_group and returns the result if joint_model_group is not a nullptr. Pointers are a type of monads as they can point to memory or nullptr.
template<typename T, typename F, typename Ret>
Ret call_or(T* ptr, F method, Ret value) {
if (ptr != nullptr) {
return ptr->(*method)();
}
return value;
}
auto get_named_target_states(auto robot_model, auto group_name) {
return call_or(
robot_model->getJointModelGroup(group_name),
&JointModelGroup::getDefaultStateNames,
std::vector<std::string>{});
}
This class: https://github.com/ros-planning/moveit2/blob/main/moveit_ros/planning/moveit_cpp/include/moveit/moveit_cpp/planning_component.h
This class builds a
::planning_interface::MotionPlanRequestthrough a bunch of setters that modify the internal state ofPlanningComponentand then theplan()function combines those into the request message and sends it and handles the response.The API for building a
MotionPlanRequestshould be more composable and flexible to make it more extensible and usable and not tied to the action of sending the request and handing the response.Goals of this project:
MotionPlanRequestAs an API breakage, this change only targets Rolling and could be backported to Humble only afterward. We should consider tick-tocking the old API.