moveit / moveit2

:robot: MoveIt for ROS 2
https://moveit.ai/
BSD 3-Clause "New" or "Revised" License
1.04k stars 510 forks source link

Fix circular dependency with moveit_resources (PILZ/PRBT) #885

Open henningkayser opened 2 years ago

henningkayser commented 2 years ago

Tracking issue for the MoveIt 2 side of https://github.com/ros-planning/moveit/issues/2947.

v4hn commented 2 years ago

reopening because the "fix" PR actually states itself that this is a temporary workaround and does not work towards the solution...

tylerjw commented 2 years ago

It is a solution of sorts. There is no more circular dependency. When we have time I want to migrate the tests to a different robot. Until we figure out how to have deterministic IK without depending on moveit_core we won't get away from having the test config for pilz in moveit itself. Even if we switch robots for the tests.

v4hn commented 2 years ago

There is no more circular dependency.

But that's the symptom, not the underlying issue (no deterministic IK+robot config in MoveIt&resources). Now the symptom is gone and people have even less drive to fix the real issue...

Until we figure out how to have deterministic IK without depending on moveit_core we won't get away from having the test config for pilz in moveit itself. Even if we switch robots for the tests.

I don't follow. As Robert explained nicely in the original issue adding the opw plugin would be a nice community contribution and would directly allow to make fanuc a deterministic IK config in moveit_resources.

tylerjw commented 2 years ago

I don't follow. As Robert explained nicely in the original issue adding the opw plugin would be a nice community contribution and would directly allow to make fanuc a deterministic IK config in moveit_resources.

That would be a really nice contribution. This "solution" does not prevent someone from doing that. As I stated in the other issue, I don't care about this more than other things. The primary reason I don't care about this is I don't think I am the right person to try to tackle this problem, and instead I should focus on improving things that are more in my domain.

henningkayser commented 2 years ago

I agree that we should get this fixed, supporting the opw plugin efforts makes a lot of sense to me. I'm fine with keeping this issue open until it's really fixed. The reason we picked this solution is to unblock all the other work that's going on.

v4hn commented 2 years ago

The reason we picked this solution is to unblock all the other work that's going on.

I'm still wondering why this blocked anything, but maybe I just did not read the correct issues... The only thing I can find is https://github.com/ros-planning/moveit2/commit/0b908a172d778519766486807684232e3f9bbe62 , which is so much less invasive than what you did now...