search

gazebosim / gazebo-classic

Gazebo classic. For the latest version, see https://github.com/gazebosim/gz-sim
http://classic.gazebosim.org/
Other
1.17k stars 477 forks source link

Simplify the process of using gazebo as a library even more #1132

Closed osrf-migration closed 10 years ago

osrf-migration commented 10 years ago

Original report (archived issue) by Andrew Hundt (Bitbucket: ahundt).

per pull request #929, having the main version take char** and int parameters makes it a bit more difficult to use if you already process the command line arguments in a different way. Could the setup* functions also be overloaded as follows? 

#!c++

setupServer(std::vector<std::string>& args)

It would also be awesome if the expectations for the parameters for those functions were documented in the meantime. That is, what are the strings expected to contain?

I believe the answer is a list of paths to plugins to load, but I'm not 100% certain.

osrf-migration commented 10 years ago

Original comment by Andrew Hundt (Bitbucket: ahundt).

osrf-migration commented 10 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).

The argc and argv that are passed to setupServer are then passed to the SystemPlugin::Load function. So changing the setupServer interface would probably also require changing the SystemPlugin::Load interface.

osrf-migration commented 10 years ago

Original comment by Carlos Agüero (Bitbucket: caguero, GitHub: caguero).

The parameters in setupServer() or setupClient() are only used by the system plugins. In other words, the system plugin is the only one that will understand the parameters. So, there are no expectations for the parameters.

Is there any reason to overload setupServer and setupClient instead of using the current signature? You can always convert your std::vector to char** and int.

osrf-migration commented 10 years ago

Original comment by Andrew Hundt (Bitbucket: ahundt).

Where can I find more information about system plugins?

The question behind the issue and questions is that I'm attempting to create a simple main() that initializes and runs gazebo as a library, rather than creating a program that runs as plugins to gazebo.

I parse my own arguments for that application. Right now I do have a function that converts an object with a begin() end() iterator pair into a set of char** and int, but it seems like it would be convenient to provide an implementation for that in the API, rather than requiring individuals to implement it. Actually, what would be even better is a real API function with parameters containing more specific information. Then the current version of the SetupServer() and SetupClient() calls could then do the parsing into the real API format I mention above.

That would really be the best case.

osrf-migration commented 10 years ago

Original comment by Carlos Agüero (Bitbucket: caguero, GitHub: caguero).

This is the best source of information about plugins that I know:

http://gazebosim.org/wiki/Tutorials/plugins

I have the feeling that you don't need the system plugin because you're parsing your arguments in your own main(). If this is the case, you don't have to pass any arguments into setupServer() or setupClient() (there are default values).

I still don't see a strong reason to overload the current setupServer() or setupClient() with an additional signature. Every user might store the command line parameters in its own internal data structures in multiples ways and we cannot overload the functions for every option. We provide a well known signature for command line arguments and the client should adapt the parameters to the signature.

osrf-migration commented 10 years ago

Original comment by Andrew Hundt (Bitbucket: ahundt).

Thank you for the link.

I think you may have misunderstood my new suggestion.

I agree that it doesn't make sense to handle every possible way to pass command line parameters. Please leave aside the specific parameter type I use for a moment. I think that generally, you want gazebo to be very easy for people to use, and similarly, I think there are other ways to implement setupServer and friends that will help take additional steps towards that goal, because the current argc,argv parameters are a bit opaque and not as easy to use as they could be.

One possibility is a normal API be created for these functions, where you pass a range of shared libraries to load to one parameter, and additional parameters for whatever other information types. That way there doesn't need to be any assumption that you are parsing command line arguments, because it is an unusual format for general C++ code that is rarely used aside from parsing main().

This means implementing in the exact same style normal standard functions or objects are implemented, like anything in common::*. Then, if desired, a single overload for the command line arguments version could be implemented to use that API since I expect it would still be necessary.

I haven't looked at all the code so forgive my not being 100% explicit and substituting type2 and type3 for additional parameters aside from shared libraries to load.

possible example:

#!c++

SetupServer(std::vector<std::string> pluginsToLoad, type2 arg2, type3 arg3);

better exmple:

#!c++

template<typename PluginRange>
SetupServer(PluginRange pluginsToLoad, type2 arg2, type3 arg3);

this version could handle pluginsToLoad as std::vector<string>, std::pair<char**,char**>, std::list<std::string>, etc with a single function implementation (i.e. does not require any explicit overloads, aside from the single implicit template overload).

Does that make sense / explain my thoughts more clearly?

Side notes:

  1. with an iterator + template you can cover most command line parameter formats possibilities easily
  2. see boost.range for an explanation of ranges, though not confined to the boost range
osrf-migration commented 10 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).

I think I'm starting to get lost.

You're creating your own main() that initializes and runs gazebo as a library. I assume it is similar in principle to examples/stand_alone/custom_main/custom_main.cc?

However, you are parsing your own command-line parameters to that program. So you don't want to just pass argc/argv since it has irrelevant parameters. I think you don't need to worry about passing argc/argv unless you're loading system plugins. Are you loading system plugins from this stand-alone program?

osrf-migration commented 10 years ago

Original comment by Ian Chen (Bitbucket: Ian Chen, GitHub: iche033).

We don't have an example of loading a system plugin from a main program yet. Not sure how you're doing it but You'll need to call gazebo::addPlugin(...) before gazebo::setupServer(..)

osrf-migration commented 10 years ago

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).

@iche033 is correct. You can call gazebo::addPlugin prior to gazebo::setupServer to create all the SystemPlugins that you need.

Each plugin may, or may not, consume command-line parameters. Currently, setupServer passes the argc and argv values to each SystemPlugin.

@ahundt , I believe your suggestion is to add an alternate setupServer that takes a vector of arguments instead of the argc and argv pair. Your initial comment has an example of this. We could also have a std::map<std::string, std::string> instead of a vector.

So, would the following work?

#!c++

int main(int argc, char **argv)
{
    std::vector<std::string> myArgs;
    myArgs.push_back("--param1");
    myArgs.push_back("value_for_param1");

    gazebo::addPlugin("path_to_my_system_plugin");
    gazebo::setupServer(myArgs);

    ....
}
osrf-migration commented 10 years ago

Original comment by Carlos Agüero (Bitbucket: caguero, GitHub: caguero).

osrf-migration commented 10 years ago

Original comment by Carlos Agüero (Bitbucket: caguero, GitHub: caguero).

@ahundt, Is the proposal made by @nkoenig OK with you? Essentially overloading setupServer() with a vector that will contain all the arguments for the system plugin. You will need to load the plugin before as @iche033 pointed out.

osrf-migration commented 10 years ago

Original comment by Andrew Hundt (Bitbucket: ahundt).

Yeah, that sounds good. If you didn't want to restrict it to vectors you could accept any type T that contains .begin() and .end(), but either way I think it would work well and reduce the boilerplate.

osrf-migration commented 10 years ago

Original comment by Carlos Agüero (Bitbucket: caguero, GitHub: caguero).

@ahundt, I've created a pull request #1065 for this issue.

Could you try this branch and let me know if it works for you?

osrf-migration commented 10 years ago

Original comment by Carlos Agüero (Bitbucket: caguero, GitHub: caguero).

See pullrequest #1068

osrf-migration commented 8 years ago

Original comment by John Hsu (Bitbucket: hsu, GitHub: hsu).

what happened to gazebo::setupClient in default (gazebo7 candidate)? thanks.

osrf-migration commented 8 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).

try gazebo::client::setup in gazebo_client.hh

osrf-migration commented 8 years ago

Original comment by John Hsu (Bitbucket: hsu, GitHub: hsu).

thanks, does this info belong in Migration.md or doxygen for gazebo_client.hh?

osrf-migration commented 8 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).

There's a little bit written in the migration guide from gazebo5 to gazebo6:

osrf-migration commented 8 years ago

Original comment by John Hsu (Bitbucket: hsu, GitHub: hsu).

When I couldn't find the function, my instinct was to search (via grep) Migration.md and the headers, how about pull request #2068?

osrf-migration commented 8 years ago

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).

osrf-migration commented 8 years ago

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).