gazebosim / gz-sim

Open source robotics simulator. The latest version of Gazebo.
https://gazebosim.org
Apache License 2.0
619 stars 251 forks source link

Config file loading options #796

Open nkoenig opened 3 years ago

nkoenig commented 3 years ago

Environment

Description

Running this world will cause Ignition Gazebo to hang at start because the included model has a plugin.

<?xml version="1.0" ?>
<sdf version="1.6">
  <world name="default">
    <include>
      <uri>https://fuel.ignitionrobotics.org/1.0/openrobotics/models/finals staging area</uri>
    </include>
  </world>
</sdf>

I believe the problem is that Ignition Gazebo sees the plugin, which in this case is a performer detector plugin, and doesn't load any additional plugins. Adding the following plugins to the world will make gazebo behave correctly.

    <plugin
      filename="ignition-gazebo-physics-system"
      name="ignition::gazebo::systems::Physics">
    </plugin>
    <plugin
      filename="ignition-gazebo-sensors-system"
      name="ignition::gazebo::systems::Sensors">
      <render_engine>ogre2</render_engine>
    </plugin>
    <plugin
      filename="ignition-gazebo-user-commands-system"
      name="ignition::gazebo::systems::UserCommands">
    </plugin>
    <plugin
      filename="ignition-gazebo-scene-broadcaster-system"
      name="ignition::gazebo::systems::SceneBroadcaster">
    </plugin>

I think we have an opt-in approach with plugins, which can cause strange and silent failures like this. An opt-out approach might be nicer, where an expert user can explicitly say that they don't want a plugin.

chapulina commented 3 years ago

An option mentioned in https://github.com/ignitionrobotics/ign-gazebo/issues/57#issuecomment-791766697 is to only consider world plugins when checking if plugins have already been loaded.

An opt-out approach might be nicer, where an expert user can explicitly say that they don't want a plugin.

An opt-out aproach requires the user to know all the possible plugins beforehand. So if we add a new plugin that's loaded by default, the user needs a way to know that they need to exclude it.

nkoenig commented 3 years ago

The opt-out idea is meant to make the default behavior do the right thing, and thereby improve the user experience. The trade off would be a more difficult time for expert users. There are probably numerous ways to implement this approach such as:

  1. A command line argument.
  2. A special plugin, whose existence means that Ignition Gazebo should only load the specified plugins.
  3. A gazebo specific extension to SDF.
chapulina commented 3 years ago

I think we can split this into 2 separate use cases:

  1. User loads a world with only non-world plugins and expects the default world plugins to be loaded
  2. User loads a world with a few world plugins and expects other world plugins to be loaded by default alongside those

We can tackle the 1st use case with my suggestion, I don't think we have any reason not to load the default world plugins when there are model plugins loaded. This was overlooked in the initial implementation, I think we can consider it a bug and fix it in stable releases. @mjcarroll , can you think of a reason not to do this? I believe this addresses the specific use case on this issue.

The 2nd use case needs more thought. The current behaviour is not to load any world plugin by default if the user has specified at least one world plugin (except when recording or playing back a log, we always load the necessary plugins for you). This makes it complicated to append plugins to the default configuration. I think it would be useful to have a mechanism for appending plugins instead of substituting. I think we could offer both options 1. (command line) and 3. (SDF) to configure this. I'm afraid that appending by default would break current users though, so I'd recommend we offer the option to turn on appending on stable releases, and flip the default behaviour on Fortress to be more beginner-friendly.

I also think it would be handy to have an option to ignore plugins specified on SDF. For example, you're loading an SDF world that has buoyancy handcoded into it and you want to disable that without editing the file.

I also think we should make the behaviour consistent between GUI and server configs.

My suggestion:

Loading a world that has:

Up to Edifice:

From Fortress:

nkoenig commented 2 years ago

Would it be possible to prioritize a resolution for this bug? There are many models on Fuel that cause Gazebo to hang if they are included.

chapulina commented 2 years ago

I'm planning to make some time to wrap up #1012 this month :+1: Please comment there if you have a good idea on how to handle this situation: https://github.com/ignitionrobotics/ign-gazebo/pull/1012#discussion_r703767599

chapulina commented 2 years ago

I think we can split this into 2 separate use cases:

  1. User loads a world with only non-world plugins and expects the default world plugins to be loaded
  2. User loads a world with a few world plugins and expects other world plugins to be loaded by default alongside those
chapulina commented 2 years ago

I think that backporting #1383 to Citadel would be too much work, so I say we don't do it.

All that's left to close this issue is completing #1012