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.

Sign up for GitHub

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

Jump to bottom

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

Open
henningkayser opened this issue Dec 3, 2021 · 6 comments · Fixed by #909
Open

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

henningkayser opened this issue Dec 3, 2021 · 6 comments · Fixed by #909
Assignees
@henningkayser
Labels
infrastructure

Comments

@henningkayser
Copy link
Member

Tracking issue for the MoveIt 2 side of moveit/moveit#2947.

  • The PILZ planner should use a supported robot rather than PRBT
  • The circular dependency with moveit_resources should be fixed
@henningkayser henningkayser self-assigned this Dec 3, 2021
@henningkayser henningkayser mentioned this issue Dec 3, 2021
Merged
@tylerjw tylerjw mentioned this issue Dec 9, 2021
Merged
1 task
@tylerjw tylerjw self-assigned this Dec 9, 2021
vatanaksoytezer pushed a commit to vatanaksoytezer/moveit2 that referenced this issue Dec 13, 2021
Temporarily move moveit_resources under target workspace due moveit#885
7305c40
tylerjw pushed a commit that referenced this issue Dec 13, 2021
Docker - Temporarily move moveit_resources under target workspace due…
0b908a1 
… to #885 (#915)
tylerjw added a commit to tylerjw/moveit2 that referenced this issue Dec 13, 2021
@tylerjw
Revert "Docker - Temporarily move moveit_resources under target works…
3fd73f5 
…pace due to moveit#885 (moveit#915)"

This reverts commit 0b908a1.
tylerjw added a commit to tylerjw/moveit2 that referenced this issue Dec 22, 2021
@tylerjw
Revert "Docker - Temporarily move moveit_resources under target works…
baf7a9c 
…pace due to moveit#885 (moveit#915)"

This reverts commit 0b908a1.
henningkayser pushed a commit that referenced this issue Dec 22, 2021
@tylerjw @ct2034
@v4hn @sjahr
Add PRBT test dependencies for PILZ planner (#909)
09b2972 
* Adding PRBT config
* Port prbt packages to ROS 2
* Move PRBT into test_configs directory
* Fix pre-commit for pilz test_config
* Revert "Docker - Temporarily move moveit_resources under target workspace due to #885 (#915)"
* Reset repos file entry for moveit_resources
* prbt_support: drop all test code

Co-authored-by: Christian Henkel <post@henkelchristian.de>
Co-authored-by: Michael Görner <me@v4hn.de>
Co-authored-by: Sebastian Jahr <sebastian.jahr@picknik.ai>
@v4hn v4hn reopened this Dec 23, 2021
@v4hn
Copy link
Contributor

v4hn commented Dec 23, 2021

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

@tylerjw
Copy link
Member

tylerjw commented Dec 23, 2021

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
Copy link
Contributor

v4hn commented Dec 23, 2021

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
Copy link
Member

tylerjw commented Dec 23, 2021

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
Copy link
Member Author

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
Copy link
Contributor

v4hn commented Dec 23, 2021 via email

@tylerjw tylerjw removed their assignment Aug 23, 2023
@henningkayser henningkayser moved this to 📋 Backlog in MoveIt Sep 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@henningkayser henningkayser

Labels
infrastructure
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Pilz test_resources tylerjw/moveit2
3 participants
@v4hn @tylerjw @henningkayser