Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

End-state deviates from goal state #532

Merged
merged 8 commits into from
Feb 25, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions core/include/moveit/task_constructor/stages/connect.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ class Connect : public Connecting
{
protected:
bool compatible(const InterfaceState& from_state, const InterfaceState& to_state) const override;
bool validateEndTrajectoryDeviation(const moveit::core::JointModelGroup* jmg,
const robot_trajectory::RobotTrajectoryPtr trajectory,
const moveit::core::RobotState& goal_state, std::string& comment);

public:
enum MergeMode
Expand Down
56 changes: 56 additions & 0 deletions core/src/stages/connect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@
#include <moveit/planning_scene/planning_scene.h>
#include <moveit/trajectory_processing/time_optimal_trajectory_generation.h>

#include <angles/angles.h>

using namespace trajectory_processing;

namespace moveit {
Expand Down Expand Up @@ -132,6 +134,51 @@
return true;
}

bool Connect::validateEndTrajectoryDeviation(const moveit::core::JointModelGroup* jmg,
const robot_trajectory::RobotTrajectoryPtr trajectory,

Check failure on line 138 in core/src/stages/connect.cpp

View workflow job for this annotation

GitHub Actions / noetic-source • clang-tidy

clang-tidy: the const qualified parameter 'trajectory' is copied for each invocation; consider making it a reference (performance-unnecessary-value-param)
const moveit::core::RobotState& goal_state, std::string& comment) {
const std::size_t waypoint_count = trajectory->getWayPointCount();

if (!waypoint_count) {
comment = "Cannot validate end trajectory deviation with an empty trajectory";
return false;
}

for (const moveit::core::JointModel* jm : jmg->getJointModels()) {
const unsigned int num = jm->getVariableCount();

if (!num)
continue;

Eigen::Map<const Eigen::VectorXd> positions_goal(goal_state.getJointPositions(jm), num);
Eigen::Map<const Eigen::VectorXd> positions_last_waypoint(
trajectory->getWayPoint(waypoint_count - 1).getJointPositions(jm), num);

double min_distance;
for (std::size_t i = 0; i < num; ++i) {
if (jm->getType() == robot_state::JointModel::REVOLUTE)
min_distance = angles::shortest_angular_distance(positions_last_waypoint[i], positions_goal[i]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You must not use angles::shortest_angular_distance here, as we need to reach the exact goal state - subsequent stages rely on this fact!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hum... I'm pretty sure it removed valid solutions... Will investigate more and report back soon.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We cannot accept equality up to multiples to 360°. The goal state needs to be reached exactly. Otherwise subsequent stages will run into trouble because they relied on a different start state.

else
min_distance = positions_last_waypoint[i], positions_goal[i];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you want to compute the joint difference:

Suggested change
min_distance = positions_last_waypoint[i], positions_goal[i];
min_distance = std::abs(positions_last_waypoint[i] - positions_goal[i]);


ROS_DEBUG_STREAM_NAMED("Connect",
"angular deviation: " << min_distance << " between trajectory last waypoint and goal.");

if (std::abs(min_distance) > 1e-4) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The magic number 1e-4 should be configurable, e.g. via a stage property.

std::stringstream ss;
ss << "Deviation in joint " << jm->getName() << ": [" << positions_last_waypoint.transpose() << "] != ["
<< positions_goal.transpose() << "]";
comment = ss.str();
ROS_ERROR_STREAM_NAMED("Connect", comment);

return false;
}
}
}

return true;
}

void Connect::compute(const InterfaceState& from, const InterfaceState& to) {
const auto& props = properties();
double timeout = this->timeout();
Expand All @@ -146,6 +193,7 @@
intermediate_scenes.push_back(start);

bool success = false;
std::string deviation_comment;
std::vector<double> positions;
for (const GroupPlannerVector::value_type& pair : planner_) {
// set intermediate goal state
Expand All @@ -161,6 +209,12 @@
success = pair.second->plan(start, end, jmg, timeout, trajectory, path_constraints);
sub_trajectories.push_back(trajectory); // include failed trajectory

if (!success)
break;

// validate position deviation: trajectory last waypoint to goal
success = validateEndTrajectoryDeviation(jmg, trajectory, goal_state, deviation_comment);

if (!success)
break;

Expand All @@ -175,6 +229,8 @@
solution = makeSequential(sub_trajectories, intermediate_scenes, from, to);
if (!success) // error during sequential planning
solution->markAsFailure();
if (!deviation_comment.empty()) // deviation error
solution->setComment(deviation_comment);
connect(from, to, solution);
}

Expand Down
Loading