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

Port PRBT packages #101

Merged
merged 3 commits into from
Dec 3, 2021
Merged

Conversation

henningkayser
Copy link
Member

@henningkayser henningkayser commented Nov 10, 2021

This is needed for moveit/moveit2#452 to get the tests to pass. I don't think that we should merge this right away, or at least we should consider already deprecating this setup since it will not be supported in the future. See moveit/moveit#2947

ct2034 and others added 2 commits September 27, 2021 16:56
* adding RPBT config

* removing roslaunch check becasue of removed dependencies

* adding pilz pipeline config for panda

* add remaining prbt-related packages

* prbt_ikfast_manipulaor_plugin
* prbt_support
* prbt_pg70_support

* drop no longer needed dependency

* add remaining prbt-related packages

* prbt_ikfast_manipulaor_plugin
* prbt_support
* prbt_pg70_support

* add argument to enable last point execution

to speed up test time by skipping trajectory in fake execution

* last point execution for pg70 gripper

* drop system_info

since we only need urdf model for testing in simulation
to get rid of dependency to pilz_testutils

* actually use pg70 package

inside from moveit_resources instead of schunk_description

* drop depend on prbt_hardware_support

not needed in simulation

* add execution type for panda config

to allow skipping execution and jump to last point in fake execution

* fixup 4bc2ce drop system_info

* fix roslaunch and CMakeLists checks

* also adding capabilities here

* drop more dependencies

* using pg70 support from moveit_resources

* add execution_type to gripper

* drop Werror

as suggested in the review by v4hn

* update deprecated macro to INSTANTIATE_TEST_SUITE_P

* Revert "update deprecated macro to INSTANTIATE_TEST_SUITE_P"

This reverts commit 1a467cc.

* update version numbers to be consitant across the meta-package

* version increase fo pilz packages

* removing patch files

* tiny fix

* update panda config from templates

re-generated from MSA with adapted templates
see moveit 99c059f

* update fanuc package from MSA

with updated templates, see moveit commit a4527b

* drop remapping to joint_states_desired

for easier include of move_group.launch into tests.
This brings the panda_moveit_config closer to the templates in MSA.

Co-authored-by: Joachim Schleicher <J.Schleicher@pilz.de>
There is no compiled code in the package, so drop all compiler references.

The acceptance_test requires real hardware and doesn't make sense for MoveIt.

The compiled URDF test is inappropriate as well, because the repository should
only provide descriptions/configurations for testing in main moveit.
The actual test was already disabled because it relies on moveit datatypes,
but the package.xml/find_package entries remained.
@henningkayser henningkayser changed the title WIP: Pr port prbt packages WIP: Port PRBT packages Nov 10, 2021
@henningkayser henningkayser self-assigned this Nov 10, 2021
@v4hn
Copy link
Contributor

v4hn commented Nov 11, 2021

we should consider already deprecating this setup since it will not be supported in the future.

I don't know how urgent your pilz port is, but I would strongly suggest to solve the issue you link as part of the migration work.
Otherwise it will add more work to the issue later on...

@henningkayser
Copy link
Member Author

we should consider already deprecating this setup since it will not be supported in the future.

I don't know how urgent your pilz port is, but I would strongly suggest to solve the issue you link as part of the migration work. Otherwise it will add more work to the issue later on...

That's exactly what I'm thinking as well. I just wanted to have something running for testing first, because the migration branch was already a bit dusty... I was hoping that we could get Pilz into Galactic before we branch off.

@henningkayser henningkayser changed the title WIP: Port PRBT packages Port PRBT packages Nov 29, 2021
@tylerjw
Copy link
Member

tylerjw commented Nov 29, 2021

I don't think we should block the port of Pilz into moveit2 on migrating away from this. I do agree that long term we should migrate all the tests away from dead robots (including the PR2) like this one but that should be separate work.

Another thing we should solve is the circular dependency for pre-release testing that having tests depend on this repo creates. That is another thing that has come up in this conversation that I think should be solved separately. I don't see the problem generally of having moveit_test_configs or something similar in moveit itself for the tests. I don't want to block this work over that discussion either.

@henningkayser
Copy link
Member Author

With some back and forth discussions I think that we should get the PRBT setup merged anyway, just so that the tests can stay enabled while we merge the planner before the upcoming Galactic branch-off. We still need to get the robot migrated to something else, possibly the Fanuc as suggested here.

@@ -0,0 +1,5 @@
cartesian_limits:
Copy link
Member

Choose a reason for hiding this comment

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

I'm confused why there are changes to the fanuc packages in this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Cartesian (max) speed is needed for calculating LINear and CIRCular movements. They are configured as cartesian_limits.yaml and should be auto-generated by MSA. Not sure, why they were missing in the fanuc package...

Copy link
Member Author

Choose a reason for hiding this comment

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

They were missing here since we haven't supported it for MoveIt 2 yet. I think it makes sense to just add the yaml files for now, I just dropped the changes to xml launch files for Panda and Fanuc.

@henningkayser henningkayser force-pushed the pr-port_prbt_packages branch 2 times, most recently from 301d1a1 to 5e7f217 Compare December 1, 2021 18:23
Copy link
Member

@tylerjw tylerjw left a comment

Choose a reason for hiding this comment

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

@henningkayser if this is your plan for most lines added, no one is even getting close to this one.

@henningkayser
Copy link
Member Author

@henningkayser if this is your plan for most lines added, no one is even getting close to this one.

What do you mean? The majority of lines is from #43 which is simply cherry-picked for this PR. The huge number comes from the meshes that are being added for the PRBT.

@tylerjw
Copy link
Member

tylerjw commented Dec 1, 2021

What do you mean? The majority of lines is from #43 which is simply cherry-picked for this PR. The huge number comes from the meshes that are being added for the PRBT.

It was a bad attempt at a joke. I know that most of the lines are just the one generated cpp file. When reviewing this I've just been skipping over reading that because I know where it came from.

* Fixup prbt_moveit_config demo launch
* Fix package.xml dependencies
* Remove dependency to moveit_common
* Fix deprecated headers
* Revert xml launch changes to Panda, Fanuc
* Fix double format in yaml files
* Filter clang-tidy rules
@henningkayser henningkayser merged commit 85d613e into moveit:ros2 Dec 3, 2021
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 this pull request may close these issues.

None yet

6 participants