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

Set Gazebo world in flight plan #2172

Conversation

tomvand
Copy link
Contributor

@tomvand tomvand commented Nov 7, 2017

Since the flight plan specifies the "mission" of the UAV, including waypoint coordinates etc., it seems to be a better place to select the Gazebo world than the airframe file.

With this pull request, an additional attribute gazebo_world can be added to the top-level flight_plan XML element. When the Gazebo world is specified in the flight plan, it takes priority over the define in the airframe file or the default empty.world.
This allows a single airframe file to be used in different environments.

A "gazebo_world" attribute can optionally be added to the flight_plan
XML element, this attribute specifies the world that should be
loaded by Gazebo. This attribute produces a FLIGHT_PLAN_GAZEBO_WORLD
define in flight_plan.h. FLIGHT_PLAN_GAZEBO_WORLD is left undefined
if it is not specified in the flight plan XML.
If a world is specified by the flightplan (using the gazebo_world
attribute), this world will be loaded instead of the one defined
in the airframe file or the default empty.world.
@dewagter
Copy link
Member

dewagter commented Nov 8, 2017

looks good, sound great.

@gautierhattenberger
Copy link
Member

I'm not a big fan of adding a specific reference to a virtual environment to the DTD. You may want to use something else than Gazebo, like Morse, and I don't want to end up with an attribute for each solution.
Here is an other option: load the Gazebo FDM module from the flight plan and configure your NPS_GAZEBO_WORLD from there.

 <modules>
   <module name="fdm" type="gazebo">
      <define name="NPS_GAZEBO_WORLD" value="some.world"/>
   </module>
 </modules>

The FDM will be only loaded for nps target, and if already defined in airframe file, options will be merged.

@kirkscheper
Copy link
Member

The idea here is that the environment is coupled with the real environment. E.g. if the flightplan states that the environment is inside the cyberzoo (our optitrack motion tracking area) that the cyberzoo environment will be autoloaded (unless overwritten in the airframe).

IMHO It's quite a cool feature.

If you are very much against the new tag, then maybe @tomvand can search the FLIGHT_PLAN_NAME for a specific word (like optitrack) or something. This name is usually copy pasted through all flight_plans that are in the cyberzoo. I do prefer the tag however as it is a more directed decision form the user to autoload the desired environment.

@gautierhattenberger
Copy link
Member

The modules I mentioned in the code above are defined in the flight plan, not the airframe. So it should do what you are looking for. If it is the case, I would prefer this solution rather than changing the code generator for more options.

@kirkscheper
Copy link
Member

Ah, I didn't know that you could add modules in the flight plan! You learn something every day... I think that would be fine.

@gautierhattenberger
Copy link
Member

yet another missing documentation.... :(

@flixr
Copy link
Member

flixr commented Nov 8, 2017

Another thing to consider would be to pass the gazebo world as a commandline parameter when starting the simulation (and falling back to the definition of the module when not given)...

@tomvand
Copy link
Contributor Author

tomvand commented Nov 9, 2017

@flixr That would require a change in the command line when another flight plan is used, which is what I'm trying to avoid.

@gautierhattenberger Cool, I was not aware this was possible. I tried your solution but ran into some practical problems: the world name is interpreted as a variable instead of a string and the flight_plan.dtd does not allow a type="string" attribute in the define element. As a quick workaround I stringified the world name and then your solution seems to work fine. The only small issue I could find is that flight plans with the <module> solution can no longer be used with jsbsim airframes, while flightplans with the gazebo_world attribute still can.

@gautierhattenberger
Copy link
Member

If it is good enough for now, I'd like to keep the module approach, and we open an issue to allow the type attribute and to allow define and configure independently of a module, just like the firmware section of an airframe. I can't guarantee when I can implement it, but it will eventually cover your two remarks.

@flixr
Copy link
Member

flixr commented Nov 9, 2017

@tomvand I have to admit that I didn't try paparazzi with gazebo yet... but I don't quite understand why a you wouldn't be able to specify the gazebo work in the commandline regardless of which flight plan you use.
IMHO you should be able to choose the gazebo world (and initial location in that world) independently of the flight plan.

Of course this doesn't (and indeed should not) preclude the definition of a "default" gazebo world for a flight plan.

@tomvand
Copy link
Contributor Author

tomvand commented Nov 10, 2017

@gautierhattenberger I just realized that I can simply put the define inside the flight plan's <header> section. Seems to be the simplest solution and does not require any modification to the flight plan parser (although independent define and configure elements would still be a nice addition).
I will remove the changes to the parser but keep the changes in nps_fdm_gazebo so that the flight plan's define does not conflict with the one in the airframe.

@flixr It's this default world for the flight plan that I'm trying to implement here. At the moment, the world can only be set from the airframe file, but most of the time I forget to change it when I switch to a different flight plan which is why I want to put the world in the flight plan itself. When the world is controlled from the command line, I would also need to remember to change that, which is why I haven't looked into that option yet.
I agree that it would be good if the flight plan's world can in turn be overridden from the command line, but this is not currently a priority for me. It also leads to a similar trade-off as above: Gazebo flags need to be added to the NPS cli (in nps_main_parse_options()) which is also used by other fdms.

Instead, the world can be specified from the flight plan by adding the FLIGHT_PLAN_GAZEBO_WORLD define to the <header> section.
@dewagter
Copy link
Member

why do you need both a

  • NPS_GAZEBO_WORLD
  • FLIGHT_PLAN_GAZEBO_WORLD

@gautierhattenberger
Copy link
Member

I think we can close this PR as it is not relevant anymore

@tomvand
Copy link
Contributor Author

tomvand commented Nov 13, 2017

@dewagter, @gautierhattenberger If NPS_GAZEBO_WORLD is already defined from the airframe, redefining it in the flight plan would lead to conflicts. If two defines is the correct way to handle this, then this PR should be merged so that nps_fdm_gazebo.cpp also checks the FLIGHT_PLAN_GAZEBO_WORLD define. If only a single define should be used, I need to update the fdm_gazebo documentation to state that it should be defined in the flight plan and not the airframe.

@gautierhattenberger
Copy link
Member

I mostly want to avoid adding unnecessary options, but I'm not using this, so if it really helps to keep the two I can live with that

Changed documentation to specify that NPS_GAZEBO_WORLD should be
defined in the flight plan instead of the airframe file.

Fixed doxygen formatting of fdm_gazebo.xml description and fixed
whitespace issues (mixed tabs and spaces).
@tomvand
Copy link
Contributor Author

tomvand commented Nov 15, 2017

Since both of you want to avoid adding unnecessary options, I've removed the FLIGHT_PLAN_GAZEBO_WORLD define (the only remaining change to nps_fdm_gazebo.cpp is that it includes the flight plan header). Since the aircraft model is now obtained from the airframe file instead of the world file (#2135), I think it is no longer necessary to define the world inside the airframe xml. Instead, I've updated the documentation to specify that the world should be defined in the flight plan. I've also used this opportunity to clean up the formatting of the description.

@gautierhattenberger gautierhattenberger merged commit 85c2b23 into paparazzi:master Nov 15, 2017
@tomvand tomvand deleted the gazebo_world_from_flightplan_devel branch December 1, 2017 09:24
biancabndris pushed a commit to biancabndris/paparazzi that referenced this pull request Aug 29, 2018
* update documentation
* include flight_plan.h in nps_fdm_gazebo.cpp
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

5 participants