Porting Advanced Lift Drag Plugin to Gazebo

[frede791]

🎉 New Feature

Summary

This is a plugin that ports the behaviour of the advanced lift drag plugin that was present in Gazebo Classic to Gazebo. The physics implementation have not changed, but the plugin has been adapted to work with the entity component system. Primary modeling differences in the advanced_lift_drag plugin from the original liftdrag_plugin include:

• quadratic formulation for drag
• side force
• flat-plate post-stall model
• aerodynamic moments about all three axes
• body rate stability derivatives
• actuator control derivatives

The objective is to provide a more accurate model of a wing than what is provided in the basic lift drag plugin.

Test it

Another PR is currently being pursued to also port the advanced plane to PX4. This should provide the ability to test the plugin on the already existing vehicles. Alternatively, one can also write their own model and use AVL to figure out the necessary parameters.

This plugin originates from a custom plugin from PX4 that was written for gazebo classic. It should be included in Gazebo in order to provide compatability with the advanced plane model in PX4. The original source code comes from https://github.com/PX4/PX4-SITL_gazebo-classic/blob/20ded0757b4f2cb362833538716caf1e938b162a/src/liftdrag_plugin/advanced_liftdrag_plugin.cpp and was written by @karthik-s-feather.

Port < Gazebo Classic advanced_lift_drag> to < Gazebo advanced_lift_drag>

Branch comparison: https://github.com/frede791/gz-sim/blob/gz-sim7/src/systems/advanced_lift_drag/AdvancedLiftDrag.cc ... https://github.com/PX4/PX4-SITL_gazebo-classic/blob/20ded0757b4f2cb362833538716caf1e938b162a/src/liftdrag_plugin/advanced_liftdrag_plugin.cpp

[frede791]

I have updated the PR description to include more detail regarding the PX4 origin and what is the plugin is aiming to achieve and also addressed the requested changes.

[arjo129]

Any chance we could get a minimal example or test?

[frede791]

Here is a flight log of a test flight I conducted with the advanced plane and the advanced lift drag plugin:
https://review.px4.io/plot_app?log=78a4f0e2-80f5-4165-96da-c18c544c9444
Does this suffice?

[frede791]

Here is an additional flight, which also includes a landing.
https://review.px4.io/plot_app?log=e84b3567-6bf8-414b-bf46-717b30210427

[arjo129]

Hi @frede791 ,
By example I mean a place where it is used so end users can see how to use it. For instance the DetachableJoint has the following example:

• https://github.com/gazebosim/gz-sim/blob/gz-sim7/examples/worlds/detachable_joint.sdf
  And the following tests:
• https://github.com/gazebosim/gz-sim/blob/gz-sim7/test/integration/detachable_joint.cc

Similarly the original liftdrag plugin has the following example and tests:

• Example: https://github.com/gazebosim/gz-sim/blob/gz-sim7/examples/worlds/lift_drag.sdf
• Test: https://github.com/gazebosim/gz-sim/blob/gz-sim7/test/integration/lift_drag_system.cc

It is important to include such minimal examples and tests so that when we change things we can catch regressions. It also allows us as reviewers to quickly test things otherwise I would be approving this blindly as many of us don't necessarily work with Pixhawk on a day-to-day basis.

If you can;t write a test, at least an example file or instructions to test would be nice. Perhaps basing it off the PR you created for the pixhawk community would be nice.

[frede791]

I've updated the formatting of the plugin files to conform with codecheck. I also included an example file that was adapted from the PX4 advanced plane and used a similar layout as in the lift drag plugin example.

[frede791]

Anything else that needs to be addressed here? @ahcorde, @arjo129

[frede791]

Since generating this plugin requires a lot of parameters taken from AVL, I have written a standalone tool that can automate this process by only requiring some physical dimensions of the vehicle. At the moment, I am planning on placing this within the PX4 codebase:
PX4/PX4-Autopilot#22204
However, it might make sense to provide this as an independent tool for Gazebo users?

[arjo129]

Apologies, I've been very busy the past few weeks and hence have not had the time to look at the tool. Perhaps you can present this tool at a simulation meeting or an aerial working group meeting. I'm not sure we should include it in here. Perhaps a tutorial linking to the tool you wrote would be good. 😄

[frede791]

The README for the tool I created is pretty self-contained but it would perhaps make sense to add in a README here which links to the tool.

[arjo129]

That sounds great. Alternatively, you can add a link to the tool in the header documentation.

…

On Sun, Oct 22, 2023, 6:28 PM Frederik Markus ***@***.***> wrote: The README for the tool I created is pretty self-contained but it would perhaps make sense to add in a README here which links to the tool. — Reply to this email directly, view it on GitHub <#2185 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAEEMQEROWASFGSY6IAUJXDYATYLRAVCNFSM6AAAAAA5PE5KYKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONZUGA2TMMJUGQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[frede791]

@arjo129 I've added in a short section in the header file that links to the tool itself. Also fixed some linting issues (that make codecheck weirdly enough didn't seem to pick up on). Let me know if there is anything else that needs improving.

[arjo129]

This is getting closer however, I'm unable to run the examples as the mesh paths are hard coded. Consider uploading the plane model to fuel.

[frede791]

@arjo129 Addressed your issues and uploaded the plane to Gazebo fuel.

[codecov]

Codecov Report

Merging #2185 (1755745) into gz-sim7 (5dbbd1b) will decrease coverage by 0.25%.
Report is 7 commits behind head on gz-sim7.
The diff coverage is 38.67%.

❗ Current head 1755745 differs from pull request most recent head c490b97. Consider uploading reports for the commit c490b97 to get more accurate results

@@             Coverage Diff             @@
##           gz-sim7    #2185      +/-   ##
===========================================
- Coverage    65.00%   64.75%   -0.25%     
===========================================
  Files          356      357       +1     
  Lines        28932    29145     +213     
===========================================
+ Hits         18808    18874      +66     
- Misses       10124    10271     +147     

Files	Coverage Δ
src/systems/lift_drag/LiftDrag.hh	100.00% <ø> (ø)
src/systems/lift_drag/LiftDrag.cc	64.36% <86.66%> (+1.29%)	⬆️
src/systems/sensors/Sensors.cc	63.14% <88.00%> (+0.46%)	⬆️
.../systems/environment_preload/EnvironmentPreload.cc	64.74% <61.61%> (-4.95%)	⬇️
...c/systems/environment_preload/VisualizationTool.cc	2.56% <2.56%> (ø)

... and 3 files with indirect coverage changes

[arjo129]

There are still some things that aren't quite right.

[arjo129]

I don't think you understood my point. The plane starts at zero velocity. You should not invalidate the config because of that. I recommend you try actually running the example rather than have me constantly correct your code.

[frede791]

I added an additional variable position that defines the starting position of the vehicle where a speed of 0 is acceptable. Passes codecheck and the example plane does not throw an error.

[arjo129]

I'd argue that you could have more than one position where zero speed is acceptable. Say you are testing a delivery scenario. Its possible that you want your plane to take off and land multiple times. I think all you need to do is not apply any force if velocity is zero. This is consistent with real world dynamics as you can't have lift or drag unless relative airspeed is zero.

[frede791]

Yes that is a valid point, so you would just return without throwing an error.

[arjo129]

Yep that is correct.

…

On Wed, Nov 1, 2023 at 4:10 PM Frederik Markus ***@***.***> wrote: Yes that is a valid point, so you would just return without throwing an error. — Reply to this email directly, view it on GitHub <#2185 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAEEMQGIYNCAG7YQ3H3KAETYCH7WLAVCNFSM6AAAAAA5PE5KYKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOBYGU3DSMJVGY> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[frede791]

removed the position variable and let the update function return without throwing an error.

[arjo129]

I just tried it. It looks great. I just have one small suggestion to improve the documentation in the example. Once you incorporate it, this looks good to me! Thanks for iterating on this PR.

Points have been addressed. Review is stale.