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

simulator in hardware, new module #11400

Closed
wants to merge 15 commits into from
Closed

simulator in hardware, new module #11400

wants to merge 15 commits into from

Conversation

romain-chiap
Copy link
Contributor

@romain-chiap romain-chiap commented Feb 6, 2019

new module added that allows to run a simulator for quadrotor directly on the hardware autopilot.
For more documentation visit
https://github.com/romain-chiap/PX4_SIH_QuadX

Please use PX4 Discuss or Slack to align on pull requests if necessary.

Test data / coverage
https://review.px4.io/plot_app?log=4a5b5b89-7231-4e7e-a989-f16e71aa9966

Describe problem solved by the proposed pull request
Timing delays due to the bidirectional connection from the hardware to the desktop computer are an issue in the HITL and affect the flight performance. Furthermore, a user willing to modify the quadrotor parameters (mass, inertia, maximum thrust, ...) needs to dig into JMAVSim or Gazebo to do the modification.

Describe your preferred solution
The proposed solution is to add a new module, fully compatible with PX4, called Simulator In Hardware (SIH). It implements the equations of motion of a quadrotor along with thruster force and torque, a simple drag model is also implemented. The sih module subscribes to actuator_outputs to get directly the outputs of the mixer. It also reconstructs noisy sensor data and publish them. The idea is to include the state estimator in the loop, and the controller of course.

The user can modify the drag coefficient, inertia matrix and mass of the vehicle directly from QGC parameters.

This module is fully compatible with the current PX4 Firmware. The following flight modes have been tested:

  • Manual
  • Altitude
  • Accro
  • Mission
  • Failsafe (RC signal lost)

Describe possible alternatives
The other alternatives are to use the SITL or current HITL

Additional context
This module was developed to display the quadrotor trajectory in FlightGear, to have a real time visual.
This module outputs the quad position and orientation on a serial port. The idea is to transform those serial data into UDP to communicate the position/attitude to FlightGear for the display. A Java application SerialToUDP.jar is also provided at the following link along with a simple quadrotor model for FlightGear
https://github.com/romain-chiap/PX4_SIH_QuadX

@dagar
Copy link
Member

dagar commented Feb 6, 2019

Cool! Let me dig into this some more and we can discuss it.

A few initial minor things that stand out are including it on more boards (should be almost everywhere except fmu-v2, io-v2, etc), a mechanism to start it (via param) with the right airframe (safe configuration), and metadata for the parameters.

Trivial code style issues here - http://ci.px4.io:8080/blue/organizations/jenkins/PX4%2FFirmware/detail/PR-11400/1/pipeline

@bresch
Copy link
Member

bresch commented Feb 7, 2019

It's a really interesting approach. Traditionally, HIL is used because the flight simulator is way too heavy to run on the autopilot itself.

I'll try and give you some feedback as soon as possible!

@LorenzMeier
Copy link
Member

Super cool!

@romain-chiap
Copy link
Contributor Author

I formatted the code with astyle and the metadata for the parameters.

Regarding the board configs, mc_att_control appears in 40 different cmake files, so I guess I should include it in the same files except:

  1. /px4/fmu-v2/*.cmake
  2. /px4/sitl/*.cmake
  3. any other ?

I think I should exclude it from SITL (to avoid an Inception like simulator :). Anyway, the sensors data will be conflicting with the data from SITL, so I won't include it.

I will check to add another airframe but I'm not very familiar with that.

@dagar dagar moved this from Low priority to Discussed in Devcall Feb 27, 2019
@MaEtUgR
Copy link
Member

MaEtUgR commented Feb 27, 2019

Devcall: Cool feature! Can we have a minimal automatic test to run in CI and make sure it doesn't break in the future? Can we add some documentation?

@dagar : Suggestion to use jMAVsim or gazebo to display the vehicle state. By using this MAVLink message which get sent here for the ground truth and having a mode for the simulator to just display the state instead of simulating.

@romain-chiap
Copy link
Contributor Author

romain-chiap commented Mar 5, 2019

done:

  1. sih module added in most board configs
  2. new airframe added
    • I added a parameter SYS_SIH, which is set to 1 by the airframe
    • rcS starts the sih module if SYS_HITL and SYS_SIH equal 1
  3. HIL_STATE_QUATERNION sent through MAVLink
  4. real-time implemented with hrt_call_every() and px4_sem_wait(). It works super well, thanks 👍
    • sampling time [us]: 3999 avg, 3937 min, 4059 max, <3us rms error
    • the module sih uses 10.7% of the CPU time (on auav-x2.1 board)
    • for comparison, the ekf2 module uses 11.1% of the CPU time
    • the idle task is at 50%

in progress:

  1. read HIL_STATE_QUATERNION on jMAVSim

@romain-chiap
Copy link
Contributor Author

Devcall: Cool feature! Can we have a minimal automatic test to run in CI and make sure it doesn't break in the future? Can we add some documentation?

Where can I add the documentation? This is currently in a pdf file

@MaEtUgR MaEtUgR removed the devcall label Mar 13, 2019
@MaEtUgR
Copy link
Member

MaEtUgR commented Mar 14, 2019

@romain-chiap Probably docs should go to the development guide's simulation part:
https://dev.px4.io/en/simulation/
https://github.com/PX4/Devguide/tree/master/en/simulation
You could create a page for your onboard simulation. Maybe @hamishwillee can confirm or correct.

@hamishwillee
Copy link
Contributor

@bkueng
What category and subcategory should this be in for the module documentation? Currently this fails because a custom category was used:

 make module_documentation
ninja: Entering directory `/home/ubuntu/src/Firmware/build/px4_sitl_default'
[0/1] Generating module documentation
Scanning source path ['/home/ubuntu/src/Firmware/src']
Exception in file /home/ubuntu/src/Firmware/src/modules/sih/sih.cpp
Traceback (most recent call last):
  File "/home/ubuntu/src/Firmware/Tools/px_process_module_doc.py", line 104, in <module>
    main()
  File "/home/ubuntu/src/Firmware/Tools/px_process_module_doc.py", line 84, in main
    if not scanner.ScanDir(args.src_path, parser):
  File "/home/ubuntu/src/Firmware/Tools/px4moduledoc/srcscanner.py", line 24, in ScanDir
    if not self.ScanFile(path, parser):
  File "/home/ubuntu/src/Firmware/Tools/px4moduledoc/srcscanner.py", line 49, in ScanFile
    return parser.Parse(scope, contents)
  File "/home/ubuntu/src/Firmware/Tools/px4moduledoc/srcparser.py", line 304, in Parse
    module_doc.category() + ' for ' + scope)
Exception: Invalid/unknown category sih for modules/sih
FAILED: CMakeFiles/metadata_module_documentation 

* @min 0.0
* @decimal 2
* @increment 0.1
* @group SIH
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps the group name should be Simulation In Hardware. At this point the acronym isn't as well known as SITL or HITL so while eventually SIH might be obvious, right now it is not.
You can see how all these will render here: https://hamishwillee.gitbooks.io/havdevguide/content/v/test_sih/en/advanced/parameter_reference.html#sih

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put two underscores to:

  • SIH__LAT0
  • SIH__LON0
  • SIH__H0
  • SIH__MU_X
  • SIH__MU_Y
  • SIH__MU_Z

I would like them to be grouped in the documentation. Is there a better way to do it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking of removing those six parameters as they are very specific and don't bring much if changed by the user.

@hamishwillee
Copy link
Contributor

@romain-chiap Very very cool. A few comments inline.

With respect to documentation, as @MaEtUgR says above, it should appear in the table here: http://dev.px4.io/en/simulation/#supported-simulators and it should have its own sub page linking to it.

The sub page should provide overview information in the same way as the other pages:

  • What is the simulator
  • A diagram showing the architecture
  • Why is it "special"/good
  • How to set it up and run it

Mostly this is what you have in https://github.com/romain-chiap/PX4_SIH_QuadX/wiki/Setting-up-the-Coriolis-SIH and on your readme.

You would also link to the generated parameters: https://hamishwillee.gitbooks.io/havdevguide/content/v/test_sih/en/advanced/parameter_reference.html#sih
And to your project page if you wish (I say if you wish because since this is now in PX4 codeline best not to jump people to too many places).

The PDF you link above is pretty cool (though we generate the params, so that section not so useful). It is up to you if you include that content in our docs. IMO that is quite a lot of work so ideally you would copy the file into https://github.com/PX4/Devguide/tree/master/assets/simulation and link to it from there.

I don't know how well you know gitbook, but that is what we use for our infrastructure. Essentially you create files in markdown and submit a PR to https://github.com/PX4/Devguide. Info on the toolchain is here: http://dev.px4.io/en/contribute/docs.html#adding-new-content---big-changes
Don't stress too much about this - your existing docs look pretty good and I can subedit as needed for you.

Is that enough to get started?

@romain-chiap
Copy link
Contributor Author

@hamishwillee
Thanks, I appreciate the comments.
I'm still playing with jMAVSim to have a visualization for the quad. This will need to be described in the documentation as well.
In the meantime I will have a look at gitbook and eventually open a PR when ready.

@hamishwillee
Copy link
Contributor

Great. Minimally you could draft a link to your docs and then update when you have a little more time.

Copy link
Member

@bkueng bkueng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What category and subcategory should this be in for the module documentation? Currently this fails because a custom category was used:

We can add 'simulation' to https://github.com/PX4/Firmware/blob/master/Tools/px4moduledoc/srcparser.py#L13.

src/lib/systemlib/system_params.c Outdated Show resolved Hide resolved
msg/CMakeLists.txt Outdated Show resolved Hide resolved
src/modules/mavlink/mavlink_messages.cpp Show resolved Hide resolved
Copy link
Member

@MaEtUgR MaEtUgR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

src/modules/sih/sih.cpp Outdated Show resolved Hide resolved
src/modules/sih/CMakeLists.txt Outdated Show resolved Hide resolved
src/modules/sih/sih_params.c Outdated Show resolved Hide resolved
@romain-chiap
Copy link
Contributor Author

@hamishwillee
I created PR757 on the Devguide.

Copy link
Member

@bkueng bkueng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a couple more remarks, this is IMO good to merge afterwards.

Can you also look into CI and run the style fix script?

ROMFS/px4fmu_common/init.d/rcS Outdated Show resolved Hide resolved
src/modules/mavlink/mavlink_main.cpp Outdated Show resolved Hide resolved
src/modules/sih/sih_params.c Outdated Show resolved Hide resolved
src/modules/sih/sih.hpp Outdated Show resolved Hide resolved
src/modules/sih/sih.cpp Outdated Show resolved Hide resolved
src/modules/sih/sih.cpp Outdated Show resolved Hide resolved
src/modules/sih/sih.cpp Outdated Show resolved Hide resolved
src/modules/sih/sih.cpp Outdated Show resolved Hide resolved
src/modules/sih/sih.cpp Outdated Show resolved Hide resolved
@romain-chiap
Copy link
Contributor Author

Devcall: Cool feature! Can we have a minimal automatic test to run in CI and make sure it doesn't break in the future?

@MaEtUgR
Shall we discuss that in the next devcall (tomorrow)? I have no idea how to create an automatic test.

@hamishwillee
Copy link
Contributor

Note there are also file conflicts above to be fixed.

Copy link
Member

@bkueng bkueng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@romain-chiap can you rebase?

src/modules/sih/sih.cpp Outdated Show resolved Hide resolved
src/modules/sih/sih.cpp Outdated Show resolved Hide resolved
@romain-chiap
Copy link
Contributor Author

I rebased from PX4/Firmware master.

  1. I synchronized my Fork with PX4/Firmware
  2. I rebased my branch simulator_in_hardware from my branch master
  3. I manually solved the conflicts (i.e. srcparser.py)
  4. I updated the submodules
  5. I compiled and checked the SIH was working properly

@bkueng
Copy link
Member

bkueng commented Mar 28, 2019

Something did not work as expected (you can see it from the many commits that now appear in the PR). Can you try to rebase again?

@romain-chiap
Copy link
Contributor Author

@bkueng
I need some help with rebase (or undo rebase first).

What I did:

  1. checking where I should reset git reflog
  2. resetting to the chosen HEAD git reset --hard HEAD@{52}
  3. make distclean
  4. commit
  5. git push --force

It looks good on my repository branch, but for some reason, it did not followed to the PR.

Is there a good way to fix that? (or shall I make a new "clean" PR?)

@bkueng
Copy link
Member

bkueng commented Apr 1, 2019

It looks good now, but unfortunately, there are conflicts again.
Can you rebase again, and look into CI? For example I see this failure:

../../src/modules/sih/sih.hpp:149:14: fatal error: private field '_counter' is not used [-Wunused-private-field]

        int32_t     _counter = 0;

@bkueng
Copy link
Member

bkueng commented Apr 16, 2019

Done in #11835

@bkueng bkueng closed this Apr 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Devcall
  
Discussed
Development

Successfully merging this pull request may close these issues.

None yet

8 participants