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

Using ExecuteTaskSolutionCapability to execute a solution is overriding the joint state values #353

Closed
JafarAbdi opened this issue Apr 14, 2022 · 5 comments · Fixed by #498

Comments

@JafarAbdi
Copy link
Member

This is happening because in execute_task_solution_capability.cpp#L179 it modifies the state in the planning scene monitor which overrides the values from /joint_states, /tf, /tf_static topics

Possible fixes could be

1- Reset the robot_state's joint_state and multi_dof_joint_state for scene_diff before applying the planning scene

auto scene_diff = sub_traj.scene_diff; // Not ideal since we will copy the whole scene_diff before reseting joint_state & multi_dof_joint_state
scene_diff.robot_state.joint_state = sensor_msgs::msg::JointState();
scene_diff.robot_state.multi_dof_joint_state = sensor_msgs::msg::MultiDOFJointState();

2- In task.cpp#L268-L269 after we fill up the message we reset those variables

Let me know which solution you prefer so I open a PR with the patch

@JafarAbdi
Copy link
Member Author

@v4hn @rhaschke do you any preferences?

JafarAbdi added a commit to JafarAbdi/moveit_task_constructor that referenced this issue May 23, 2022
@rhaschke
Copy link
Contributor

I guess the differences between actual joint state values (after execution) and those in scene_diff are marginal, right?
I would prefer the second option: the planning scene diff shouldn't comprise joint values in first place.
Sorry for the delay in replying.

JafarAbdi added a commit to JafarAbdi/moveit_task_constructor that referenced this issue Jun 5, 2022
JafarAbdi added a commit to JafarAbdi/moveit_task_constructor that referenced this issue Jun 23, 2022
JafarAbdi added a commit to JafarAbdi/moveit_task_constructor that referenced this issue Aug 22, 2022
JafarAbdi added a commit to JafarAbdi/moveit_task_constructor that referenced this issue Dec 5, 2022
sea-bass pushed a commit to sea-bass/moveit_task_constructor that referenced this issue Mar 10, 2023
sjahr pushed a commit to sjahr/moveit_task_constructor that referenced this issue May 3, 2023
Implement generic pose randomizer

add new property to configure controller info for traj execution

Don't override joint state in planning scene monitor when executing task's solution - Fix: moveit#353

Refactor creating an executable trajectory in a helper lambda

Make mtc execute solution capability apply scene diffs before executing trajectory

Run goalCallback in a seperate asynchronously

Use modern cmake

Explicitly instantiate computeGeneric templates

Add install interface for the include directory

Replace boost::function with std::function
sjahr pushed a commit to sjahr/moveit_task_constructor that referenced this issue May 15, 2023
sjahr pushed a commit to sjahr/moveit_task_constructor that referenced this issue May 15, 2023
sjahr pushed a commit to sjahr/moveit_task_constructor that referenced this issue May 15, 2023
…ng trajectory

Don't override joint state in planning scene monitor when executing task's solution - Fix: moveit#353

Refactor creating an executable trajectory in a helper lambda

Make mtc execute solution capability apply scene diffs before executing trajectory
sjahr pushed a commit to sjahr/moveit_task_constructor that referenced this issue May 23, 2023
…ng trajectory

Don't override joint state in planning scene monitor when executing task's solution - Fix: moveit#353

Refactor creating an executable trajectory in a helper lambda

Make mtc execute solution capability apply scene diffs before executing trajectory
sjahr pushed a commit to sjahr/moveit_task_constructor that referenced this issue May 23, 2023
…ng trajectory

Don't override joint state in planning scene monitor when executing task's solution - Fix: moveit#353

Refactor creating an executable trajectory in a helper lambda

Make mtc execute solution capability apply scene diffs before executing trajectory

Fix compiler issues

Clang-tidy fixes

Copy planning scene to modify it
@sjahr sjahr linked a pull request Oct 23, 2023 that will close this issue
@sjahr
Copy link
Contributor

sjahr commented Oct 27, 2023

I think this is addressed with #498. Please re-open if not.

@sjahr sjahr closed this as completed Oct 27, 2023
@mechwiz
Copy link
Contributor

mechwiz commented Nov 8, 2023

While #498 addresses the issue in option 2, that is only true if the execute function is called within MTC. However, if a goal is sent to the execution_task_solution server directly (separate from within the execute function in MTC), then the joint values in that goal's trajectories will still override the joint state values. Hence I think option 1 is really the way to go to address this issue.

@rhaschke
Copy link
Contributor

rhaschke commented Nov 8, 2023

The problem with #498 is simply that the cleanup of the scene_diff's joint states is only performed in Task::execute, but not in the message generation. I filed PR #504, which should solve that issue.

rhaschke added a commit that referenced this issue Mar 7, 2024
Joints are handled in trajectories.
Scene diffs should not modify joints during execution.
Fixes #353.
rhaschke added a commit to ubi-agni/moveit_task_constructor that referenced this issue Mar 8, 2024
Joints are handled in trajectories.
Scene diffs should not modify joints during execution.
Fixes moveit#353.

Alternative to moveit#504. The previous solution, to always clear the joint states
during message generation, broke the visualization in rviz.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants