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
MoveGroupCommander + pilz industrial motion planner not using the updated allowed collision matrix #2676
Comments
Thanks for reporting an issue. Because we're a volunteer community, providing a pull request with suggested changes is always welcomed. |
Could you add the full error message, instead of paraphrasing it? |
Edited issue to include error message (see section "Actual Behaviour") |
I have some extra debug messages to share. Below are two traces for identical plans. One however was initiated from the MoveGroup Commander
rviz
|
Now using MoveIt 1.1.4. Could the author of the new API function |
Sorry for not getting back to you. I can't test this at the moment, and it doesn't look like you posted the lines that you are actually executing. Your classes seem to make sense (although I didn't read it all), but what is the order of operations? The
If you 1) change the allowed collision matrix as shown in your code, 2) publish the change, and 3) try planning with both planners from
This one makes me think that you may have called
Are you starting up both the
If there was anything unclear in the tutorials, please note it now while it's hot on your mind and feel free to submit a PR. |
Thank you for your reply, my config is exactly as you describe above and since updating to 1.1.4 the
The error is:
I feel like I should be passing it a |
I guess it's because you're writing "get" instead of "set"? |
I am trying to "get" the id; "set" works fine. As per the code you wrote, the docstring says to pass |
My bad, I will double check/fix that soon. As a workaround for now, I would just set both the planning pipeline and planner to whatever you need before planning. |
I'm just glad it wasn't me being dumb! 😃
Video below showing my order of operations and showing the acm_pilz_bug_sm.mp4 |
|
I hope the below is what you were after? Last point in
Last point in
|
@felixvd I have a breakthrough of sorts from your debug suggestion! 👊 Planning to a joint position rather than pose works as expected. The command is below for reference. Of course for my application I need to supply poses, not joint positions but perhaps now this will help you guys pin point the root cause? 🤞 For reference: I did then change IK solver from IKFast to KDL but that doesn't make a difference |
@jschleicher @ct2034 would you be able to help identify the root cause? We are more than happy to contribute to a fix but we didn't write the library, so it would be far more efficient if Pilz engineers pointed us in the right direction. |
I made a workaround for |
@robertjbush I'm not into the collision system and have no clue, why the ACM could not be updated. The code you point to, just uses the planning scene to check for collisions. Maybe @felixvd can give a hint, what's wrong in the usage or needs to be done to update the collsiion matrix? |
@robertjbush Have you encountered this problem since? If so, is there a minimal example to reproduce the problem? |
Hi @felixvd, Ive stopped using On Thursday we have a new software engineer starting and this bug will be one of his tasks. |
The gold standard would be a .test file using one of the robots in But it looks like we were able to reproduce a similar (or the same) bug on our system. Using OMPL after disabling collisions for a part of the scene (actually the robot) works, but the LIN planning request fails. I'll look into it. |
Hi, I am checking this problem too. I was able to reproduce the issue. In our scenario, there is an object define directly from the robot's URDF so it is considered as part of self-collision, however we can disable the collision checking After reading the code, I think that the problem is that Pilz planner's check if there is self collision when computing an IK solution for the whole robot describe by the URDF, instead of just considering the chain of links relevant to the actual robot. Whether this is the right approach or not, I don't know. The solution would be to compute an IK solution without self collision validation and then let the PlannerScene validate the found plan, OMPL seems to be doing this (wild guess). From the code: Without this extra check, the behavior is as expected. The Pilz planners can generate a plan over an obstacle that has collision allowed but fail otherwise with the error message:
I tested this by just returning |
We're going to test @cambel's approach tomorrow or Friday. Thanks for sharing. I'll report back with whether it fixes our bug asap. |
Hey all, I'm also taking a look at this in place of @robertjbush. I'm a little new to MoveIt and pilz so please forgive any silly questions! @cambel I'd be interested in learning how you tested your modified code. Are there unit tests within pilz that could be updated to verify new functionality, or would we have to make the modifications, compile from source, then swap the standard pilz package out with the modified one? It sounds like MoveIt will apply collision detection at the appropriate part of the planning pipeline, so I'm wondering if the collision detection logic in pilz can be removed completely? |
Hi @aa-tom, we have installed moveit from source, so I just modified the code, compiled and test. I don't know if there is a unit test for it. As far as I understand, moveit checks collisions for the plan generated by any planner. The extra check of self collision for each IK solution sounds like a good idea if it's limited to the robot's link involved and if it can consider the update collision matrix. So, removing the self collision entirely from the pilz is one option but I guess it'd be better to have the self collision check focus on the parts of the robot that matter. |
As far as I understand, moveit checks collisions for the plan generated by any planner.
Yes and no. It's [a feature that can be disabled](https://github.com/ros-planning/moveit/blob/master/moveit_ros/planning/planning_pipeline/include/moveit/planning_pipeline/planning_pipeline.h#L100-L102), though it's enabled by default.
We usually expect planners to do collision checking on their own.
Pilz's approach is quite unsatisfying to me in that regard anyway, because they only check for self-collisions and only for LIN (not for PTP) planning.
So LIN trajectories will stop before self-collisions, but PTP trajectories will not.
Neither will stop before environment collisions because the planner straight ignores the `PlanningScene` it receives together with the planning request and instead [creates an empty one for collision checking](https://github.com/ros-planning/moveit/blob/master/moveit_planners/pilz_industrial_motion_planner/src/trajectory_functions.cpp#L574).
I'm not sure whether there are reasons for this behavior, maybe @jschleicher can comment on that.
So all in all, at the moment the feature *has* to be enabled for MoveIt anyway because of environment collisions, but it's still reasonable to pass a collision checker to the IK plugin (which could in theory still ignore it if that's what you want).
|
@v4hn @jschleicher Do you know where about in the code the I've been taking a look at some of the relevant methods, in particular |
this is the top-level entry point from MoveIt to the planner where the correct scene is passed. For the service/action interface for sequence planning it's here instead. |
Excellent, thanks. Based on a look at the code it seems like the
Does this sound sensible? *Edit: Just realised that setting the planning scene is already provided by |
@v4hn I've implemented a fix based on the above approach. What's the process for raising a PR? |
What's the process for raising a PR?
I don't get the question. Create a pull-request on github, make sure CI succeeds, and in the best case add a test that is fixed by the PR.
|
This has been fixed by @aa-tom (with help from @cambel), but is waiting for @jschleicher to review #2803. |
Description
Asked by @felixvd to raise an issue ticket (see this ROS answers question)
My environment
Problem
Action Taken So Far
Steps to reproduce
My base application is a docker container and that is detailed below.
My
planning_pipeline.launch
is standard as below:The
robot_description
is a motoman gp8 but that can replaced by any package. Collision manager python script is below:The python script where I define the
move_group
commander wrapper functions:The rest of my code is standard implementation and calls to the functions above.
Expected behaviour
Actual behaviour
/yaskawa
):The text was updated successfully, but these errors were encountered: