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

Add ABB IRB 1200 moveit package #77

Closed
wants to merge 8 commits into from

Conversation

ajshort
Copy link

@ajshort ajshort commented Sep 15, 2017

No description provided.

@Levi-Armstrong
Copy link
Member

@ajshort Thank you for the contribution. Can you add the components described here in the "Update Configuration Files" section?

@ajshort
Copy link
Author

ajshort commented Sep 17, 2017

@Levi-Armstrong Done in a4f67cb

@Levi-Armstrong
Copy link
Member

Great Thank you.

@gavanderhoorn
Copy link
Member

@ajshort: thanks for this. Travis seems to be failing on some missing files:

FAILURE:
[/root/catkin_ws/src/abb_experimental/abb_irb1200_support/tests/roslaunch_test_irb1200_5_90.xml]:
	while processing /root/catkin_ws/src/abb_experimental/abb_irb1200_support/launch/robot_interface_download_irb1200_5_90.launch:
Invalid roslaunch XML syntax: [Errno 2] No such file or directory: u'/root/catkin_ws/src/abb_experimental/abb_irb1200_support/launch/robot_interface_download_irb1200_5_90.launch'
wrote test file to [/root/catkin_ws/build/abb_irb1200_support/test_results/abb_irb1200_support/roslaunch-check_tests_roslaunch_test_irb1200_5_90.xml.xml]
...writing test results to /root/catkin_ws/build/abb_irb1200_support/test_results/abb_irb1200_support/roslaunch-check_tests_roslaunch_test_irb1200_7_70.xml.xml
FAILURE:
[/root/catkin_ws/src/abb_experimental/abb_irb1200_support/tests/roslaunch_test_irb1200_7_70.xml]:
	while processing /root/catkin_ws/src/abb_experimental/abb_irb1200_support/launch/robot_interface_download_irb1200_7_70.launch:
Invalid roslaunch XML syntax: [Errno 2] No such file or directory: u'/root/catkin_ws/src/abb_experimental/abb_irb1200_support/launch/robot_interface_download_irb1200_7_70.launch'
wrote test file to [/root/catkin_ws/build/abb_irb1200_support/test_results/abb_irb1200_support/roslaunch-check_tests_roslaunch_test_irb1200_7_70.xml.xml]

For robot_interface_download_irb1200_5_90.launch I'm not sure what is going on, but the 7_70 launch file is indeed not there in the support package.

Is the PR missing files?

@gavanderhoorn
Copy link
Member

gavanderhoorn commented Oct 15, 2017

Ah, you renamed the file in c3b9ce5 (removed the variant suffix). Was that on purpose?

It may seem redundant, but we like to be as explicit as possible with all of this, that's why all files that are variant-specific have a variant suffix (ie: _5_90 and _7_70).

@ajshort
Copy link
Author

ajshort commented Oct 16, 2017

Yep, I removed the suffix as the same file can be used for both variants. Hopefully I have pushed a fix - will see how the tests go.

@gavanderhoorn
Copy link
Member

@ajshort wrote:

Yep, I removed the suffix as the same file can be used for both variants. Hopefully I have pushed a fix - will see how the tests go.

Ok. As I wrote above, it may seem redundant, but we like to be as explicit about things as possible. Additionally, at the moment these files may all look the same, but that may change in the future. This way we already have the infrastructure in place, and everything (ie: support for different variants) is as independent as it can be.

@stwirth
Copy link

stwirth commented Dec 28, 2017

+1 for merging this. @gavanderhoorn did you want the both robot variants not to share the joint_names.yaml? I think it is fine as the variants only differ in size and not number of joints.

@gavanderhoorn
Copy link
Member

@stwirth wrote:

+1 for merging this. @gavanderhoorn did you want the both robot variants not to share the joint_names.yaml? I think it is fine as the variants only differ in size and not number of joints.

None of the files should be shared by variants, with the exception of meshes (geometry).

As I wrote above in an earlier comment: the files may contain the same information right now, but that is not guaranteed to stay that way, and by giving each variant its own fileset, we keep them as decoupled as possible.

@stwirth
Copy link

stwirth commented Dec 31, 2017

@gavanderhoorn understood. I made a new PR to fix this: #81.

@gavanderhoorn
Copy link
Member

gavanderhoorn commented Jan 5, 2018

Closing this one, but continuing (commits have been included in) #85.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants