-
Notifications
You must be signed in to change notification settings - Fork 138
Multi-Robot setup and detection #259
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
Multi-Robot setup and detection #259
Conversation
ipa-srd-rd
commented
Sep 25, 2018
- added a robot namespace for multi-robot cases, empty in single-robot case
- changed some box values for the robot because they'd block the laser scanners
<gazebo> | ||
<plugin name="ros_control" filename="libhwi_switch_gazebo_ros_control.so"> | ||
<robotNamespace>${name}</robotNamespace> | ||
<robotNamespace>$(arg robot_namespace)/${name}</robotNamespace> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the default (""
) changes the current behavior
i.e. it adds a leading /
I don't know the consequences, but I'd rather like to see that it defaults to current behavior without any change
this review comment applies for all base.urdf.xacro files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was my initial intention as well but I didn't see any bad implications of this implementation. Alternatively I could remove the /
but then the argument robot_namespace
needs a slash at the end. This seems wrong to me as well. If there is a better solution let me know, but I can't really think of one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
foreseeing bad implications without having tested every possible usecase (mojin usecases) is tricky to tell
while not ideal, your alternative is less intrusive for everyone not using multirobot setup
<geometry> | ||
<mesh filename="package://raw_description/meshes/base/raw_base_long.stl"/> | ||
<!--box size="1.04 0.58 0.31" /--> | ||
<!--<box size="0.93 0.49 0.31" />--> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why the changes in a commented line?
this review comment applies to all files in this pr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because that's how my IDE does a comment in xml using shortcuts. I thought it wouldn't matter but I can change it back if it does.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no I mean, why the values have changed while still being commented?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the commented box geometry was used because @ipa-srd-rd relied on the robot's collision mesh being detected in other robots scans
obviously, the current mesh does not have the plastic side panels modeled and the current box mesh is covers the field of view of the laser scanners
best solution would be to add the plastic side panels to the base.urdf.xacro (as new links)