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

Enabling non-namespace use of Gazebo robot #187

Closed
wants to merge 4 commits into from

Conversation

rhaschke
Copy link
Contributor

@rhaschke rhaschke commented Oct 27, 2021

So far, the Gazebo robot was always started in the arm_id=panda namespace. However, the real robot is not running in a namespace. To increase coherence, this PR (building on top of #183 to avoid merge conflicts) adds two commits:

  • introducing the ns argument to panda_arm.urdf.xacro
  • refactoring franka_gazebo/panda.launch to allow an empty namespace

This essentially decouples the namespace from the arm_id and - for example - allows spawning two arms as follows:

roslaunch gazebo_ros empty_world.launch world_name:=worlds/empty.world use_sim_time:=true
roslaunch franka_gazebo panda.launch gazebo:=false arm_id:=panda1 ns:=left y:=0
roslaunch franka_gazebo panda.launch gazebo:=false arm_id:=panda2 ns:=right y:=1

@rhaschke
Copy link
Contributor Author

rhaschke commented Oct 27, 2021

Note that using a namespace different than arm_id triggers this warning:

if (this->arm_id_ != robot_namespace) {
ROS_WARN_STREAM_NAMED(
"franka_hw_sim",
"Caution: Robot names differ! Read 'arm_id: "
<< this->arm_id_ << "' from parameter server but URDF defines '<robotNamespace>"
<< robot_namespace << "</robotNamespace>'. Will use '" << this->arm_id_ << "'!");
}

... which was introduced only recently by @gollth in f2f82b2. I think this commit already resolved the main problems to separate the name of the robot (arm_id) from the ROS namespace it is running in. In other words, I think this warning is superfluous.

rhaschke added a commit to rhaschke/franka_ros that referenced this pull request Oct 27, 2021
rhaschke added a commit to rhaschke/franka_ros that referenced this pull request Oct 28, 2021
rhaschke added a commit to rhaschke/franka_ros that referenced this pull request Nov 7, 2021
The robotNamespace is conceptually different from arm_id.
While the latter just acts as a name prefix for links and joints,
the <robotNamespace> tag specifies the name of the ROS namespace, which is used
for key robot components like the joint_states topic, the controller_manager, etc.

To allow for an empty namespace, the default needs to be specified as empty.
Otherwise, i.e. having a non-empty default, specifying ns:="" on the commandline
will fall back to the default despite an (empty) value was provided.
By default, we don't start in a namespace to match the real-robot behavior.
Unfortunately, we cannot just specify an empty ns="" in roslaunch, but we
need to distinguish the two cases empty/non-empty namespace.
To avoid code duplication, the relevant code was factored out into panda_ns.launch.xml,
which is include w/ or w/o a namespace.
The file-ending is deliberatively choosen as .launch.xml to hide the file from bash completion.
Instead of arm_id/arm_id_xxx use arm_id/xxx
@rhaschke
Copy link
Contributor Author

rhaschke commented Nov 8, 2021

Rebased after merge of #183.

@gollth
Copy link
Contributor

gollth commented Nov 8, 2021

Hmm, not sure about this one.

I think the main confusion here is the distinction between topic namespaces and URDF prefixes, which are two distinct features. Let's try to not mix the two.

Passing a topic namespace into an URDF seems already flawed to me. Ideally a URDF does not need to care in which namespace it is loaded. It is valid in any namespace.

The only reason I see where we would need the namespace in the URDF anyways would be for

<gazebo>
  <plugin name="gazebo_ros_control" filename="libgazebo_ros_control.so">
    <robotNamespace>$(arg arm_id)</robotNamespace> <!-- HERE -->
    <controlPeriod>0.001</controlPeriod>
    <robotSimType>franka_gazebo/FrankaHWSim</robotSimType>
  </plugin>
</gazebo>

The documentation for this <robotNamespace> states:

The ROS namespace to be used for this instance of the plugin, defaults to robot name in URDF/SDF

So can we just leave it out completely? Then we don't have any association between ROS topic namespaces in the URDF anymore:

<gazebo>
  <plugin name="gazebo_ros_control" filename="libgazebo_ros_control.so">
    <controlPeriod>0.001</controlPeriod>
    <robotSimType>franka_gazebo/FrankaHWSim</robotSimType>
  </plugin>
</gazebo>

Regarding the naming between franka_gazebo and franka_control I think you have a fair point, that the two should match. When we let gazebo figure out the namespace by itself, we can even remove the <group ns="$(arg arm_id)"> entirely from the panda.launch file. This has two advantages:

  • No special logic for dealing with empty namespaces like you suggested
  • Give user code (launch files) the possibility to declare their namespaces, e.g.:
<launch>
  <group ns="panda_left">
    <include file="$(find franka_gazebo)/launch/panda.launch">
      <arg name="arm_id"     value="panda_1" />
    </include>
  </group>

  <group ns="panda_right">
    <include file="$(find franka_gazebo)/launch/panda.launch">
      <arg name="arm_id"     value="panda_2" />
    </include>
  </group>
</include>

Notice how the namespace could potentially even be different than the arm_id

I drafted a possible solution here if you want to have a look. Would that be compatible with MoveIT then?


This essentially decouples the namespace from the arm_id and - for example - allows spawning two arms as follows:

BTW. this is already possible, see https://frankaemika.github.io/docs/franka_ros.html#customization

@gollth
Copy link
Contributor

gollth commented Nov 8, 2021

Note that using a namespace different than arm_id triggers this warning:

Regarding this check I also agree, that, in any case this will be outdated then. The main idea behind this was to give the User a clearer warning if the namespaces don't match other than the cryptic Controller Spawner couldn't find the expected controller_manager ROS interface. when the URDF could not be loaded.

However now I see, that if this warning would be printed, then the URDF had to be found in the first place to define the FrankaHWSim plugin...

@rhaschke
Copy link
Contributor Author

rhaschke commented Nov 8, 2021

I think the main confusion here is the distinction between topic namespaces and URDF prefixes, which are two distinct features. Let's try to not mix the two.

Good that we agree on this crucial point. The problem is that the old code linked these two features, using the URDF prefix arm_id also for the topic namespaces generated by Gazebo:

<robotNamespace>$(arg arm_id)</robotNamespace>
<controlPeriod>0.001</controlPeriod>

Thus, the central contribution of this PR is to change this line. The rest is required to forward the ns argument to that spot.
I like your idea to "let Gazebo figure out the namespace by itself". However, your draft doesn't achieve that: Your solution relies on the fact that Gazebo itself is started within the corresponding namespace. However, this rules out the possibility to spawn multiple robots with different arm IDs and namespaces into the same Gazebo instance.
I don't see an option other than specifying the Gazebo namespace in the URDF. One could try to use the namespace of the robot spawner, but this would require changes to gazebo_ros_control.

@gollth
Copy link
Contributor

gollth commented Nov 9, 2021

Don't know if I understand your point entirely.

However, your draft doesn't achieve that

For me it does: If I start this launchfile with the changes from the above commit, I can start both pandas in separate namespaces (which can even be different from the arm_ids) and Gazebo in its own (root) namespace:

<?xml version="1.0"?>
<launch>

  <include file="$(find gazebo_ros)/launch/empty_world.launch" >
    <!-- Start paused, simulation will be started, when Pandas were loaded -->
    <arg name="paused" value="true"/>
    <arg name="use_sim_time" value="true"/>
  </include>

  <group ns="panda_left">
    <include file="$(find franka_gazebo)/launch/panda.launch">
      <arg name="arm_id"     value="panda_1" />
      <arg name="y"          value="-0.5" />
      <arg name="gazebo"     value="false" />
      <arg name="paused"     value="true" />
    </include>
  </group>

  <group ns="panda_right">
    <include file="$(find franka_gazebo)/launch/panda.launch">
      <arg name="arm_id"     value="panda_2" />
      <arg name="y"          value="0.5" />
      <arg name="gazebo"     value="false" />
      <arg name="paused"     value="false" />
    </include>
  </group>

</launch>

Output of rosnode list confirms that:

/gazebo
/gazebo_gui
/panda_left/interactive_marker
/panda_left/joint_state_desired_publisher
/panda_left/joint_state_publisher
/panda_left/panda_1_controller_spawner
/panda_left/panda_1_gripper_spawner
/panda_left/robot_state_publisher
/panda_right/joint_state_desired_publisher
/panda_right/joint_state_publisher
/panda_right/panda_2_controller_spawner
/panda_right/panda_2_gripper_spawner
/panda_right/robot_state_publisher
/rosout

Can you elaborate what you mean by "Your solution relies on the fact that Gazebo itself is started within the corresponding namespace"?

@rhaschke
Copy link
Contributor Author

rhaschke commented Nov 9, 2021

You are totally right. I just verified that the Gazebo spawner indeed considers the namespace it is started in. My statement in #187 (comment) was based on the (unverified - sorry, I was in a rush) assumption that this wouldn't be the case. Your solution is much better then. Closing here in favour of #196.

@rhaschke rhaschke closed this Nov 9, 2021
@gollth
Copy link
Contributor

gollth commented Nov 9, 2021

No worries, thanks for the contribution

@rickstaa
Copy link
Contributor

rickstaa commented Sep 3, 2023

Note that using a namespace different than arm_id triggers this warning:

if (this->arm_id_ != robot_namespace) {
ROS_WARN_STREAM_NAMED(
"franka_hw_sim",
"Caution: Robot names differ! Read 'arm_id: "
<< this->arm_id_ << "' from parameter server but URDF defines '<robotNamespace>"
<< robot_namespace << "</robotNamespace>'. Will use '" << this->arm_id_ << "'!");
}

... which was introduced only recently by @gollth in f2f82b2. I think this commit already resolved the main problems to separate the name of the robot (arm_id) from the ROS namespace it is running in. In other words, I think this warning is superfluous.

However now I see, that if this warning would be printed, then the URDF had to be found in the first place to define the FrankaHWSim plugin...

@gollth, @Maverobot I created a pull request that removes the redundant ROS namespace warning from FrankaHWSim. Please feel free to ask for changes if you think of a better solution.

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

3 participants