PX4 / PX4-SITL_gazebo-classic

Set of plugins, models and worlds to use with OSRF Gazebo Simulator in SITL and HITL.
http://dev.px4.io/simulation-gazebo.html
370 stars 786 forks source link

Changes for HITL support #122

Closed korotkoves closed 7 years ago

korotkoves commented 7 years ago

May be this info can be useful. Changing IMU rate to 250 HZ and disabling HIL_STATE_QUATERNION message allows me to successfully start Gazebo with Pixhawk in HITL mode. See also https://github.com/acsl-mipt/sitl_gazebo/commit/5e83deb3b1eeb23250b092e9a5522089791a9858 and https://github.com/acsl-mipt/sitl_gazebo/commit/96f684439d1a4d0cd62e8e48be4d0eb3a6a35961

TSC21 commented 7 years ago

@korotkoves can you please add two separate PR's for each commit you have so we can evaluate it and merge? Though I'm kinda reluctant about removing the HIL_STATE_QUATERNION. I'm working on getting Gazebo fully working with HIL both with and without ROS (by using MAVROS HIL plugin). On MAVROS we are able to publish all the available HIL MAVLink msgs - https://github.com/mavlink/mavros/blob/master/mavros/src/plugins/hil.cpp.

korotkoves commented 7 years ago

First commit needs setting parameter for setting imu rate. Don't know how set it in current master branch. Second commit simply comments out sending HIL_STATE_QUATERNION You can test HITL with https://github.com/acsl-mipt/multiple-sitl using ./start.rb --imu_rate 250 --hitl Connect QGroundControl(disable autoconnect to Pixhawk on serial line) to 127.0.0.1:17010

TSC21 commented 7 years ago

First commit needs setting parameter for setting imu rate. Don't know how set it in current master branch.

https://github.com/PX4/Firmware/blob/master/src/modules/simulator/simulator_params.c

Second commit simply comments out sending HIL_STATE_QUATERNION

Why is it required? What behavior do you get?

TSC21 commented 7 years ago

I see it sends the ground truth as the in the HIL_STATE_QUATERNION msg. So probably it interferes with what is received by the HIL_SENSOR + HIL_GPS msgs.

korotkoves commented 7 years ago

I meen that first commit changes imu rate in sitl_gazebo (data send to Pixhawk)

TSC21 commented 7 years ago

So what's the problem about that?

korotkoves commented 7 years ago

Simply economy of time. I would prefer propose simple ideas and changes as issues and complex modifications with pull requests. This issue about simple changes in code, so sorry for my rules.

TSC21 commented 7 years ago

Simple changes will only be useful if they fit on the sw architecture, are explainable and extensible.

TSC21 commented 7 years ago

Besides, commenting code is not a solution, is just an hack.

korotkoves commented 7 years ago

Sorry people, this thread is turning to private conversation :), this message will be last. @TSC21 please write next message directly to me, if you wish.

I really hardly understand why you want that I do pull requests with so tiny things :) I really don't know why limiting to 250 Hz imu works (https://dev.px4.io/en/simulation/hitl.html set this rate for jMavsim so I decide trying with Gazebo) I really don't know why turning off HIL_STATE_QUATERNION works (it works even in pure SITL) Мay be HIL_SENSOR conflicts with HIL_STATE_QUATERNION. I simply tested my assumptions and they worked :) I really don't want spent time to simple things, I already spent much time for writing posts in this thread :)

TSC21 commented 7 years ago

Thank you for your help. Closing this issue now as it "seems" to be solved.