PX4 / jMAVSim

Simple multirotor simulator with MAVLink protocol support
BSD 3-Clause "New" or "Revised" License
82 stars 207 forks source link

Fix problems with HITL mode simulation with Pixhawk running latest PX4 #41

Closed lovettchris closed 7 years ago

lovettchris commented 7 years ago

Latest PX4 px4fmu-v2 bits on Pixhawk, with a real radio control had weird problems flying crazy. With these fixes it now flies smooth as silk. Also tested SITL mode with QGC flying a mission, and it also works nicely.

bkueng commented 7 years ago

The HIL_STATE_QUATERNION serves as ground truth in SITL. Can you please test #44 instead? It should fix your problems.

LorenzMeier commented 7 years ago

I'm closing this PR in favour of #44.

lovettchris commented 7 years ago

I tried your #44 with unmodified jmavsim, and it doesn't work. The drone says "First waypoint too far, refusing mission" because GPS is toggling between 0,0,0 and real location. QGC shows map jumping back and forth.

Can you explain the architecture of simulation someplace? Because to me the low level HIL_SENSOR tells the PX4 what is really happening, the Position Estimator then publishes vehicle_attitude. If you then turn around and have the simulator send HIL_STATE_QUATERNION then the PX4 mavlink receiver publishes another vehicle_attitude which could conflict with the position estimator, and I think this is what is causing crazy flying. Isn't sending HIL_STATE_QUATERNION kind of cheeting? The PX4 should react to new vectors computed from accelerometer, gyro data only. It get's ground truth on GPS position, but sending "truth" of drone attitude is cheating. It's like flying with a optitrack camera system, and I believe that the PX4 higher level modules will get confused if they see big discontinuous jumps in vehicle_attitude messages which can happen if they are coming from two different places. It is also currently inefficient because HIL_STATE_QUATERNION also overlaps with HIL_SENSORS on the accel data. It also overlaps with HIL_GPS. Notice PX4 is expecting HIL_STATE_QUATERNION to send global position, and I'm not sure what happens when jmavsim doesn't fill in these fields of the message. This may explain why I sometimes see the map jump back and forth between 0,0,0 and the real GPS location. To me it makes sense to only send additional sensor data that is missing from HIL_SENSORS, like airspeed.

HITL goes crazy. SITL works ok. So perhaps HIL_STATE_QUATERNION should be limited to SITL mode. But I still think it's cheating :-)

bkueng commented 7 years ago

Can you please try again or describe your exact setup. Because #44 does the same thing as your changes here in HITL: it disables HIL_STATE_QUATERNION in HITL and only enables it in SITL. It works for me, I tested without any additional input connected.

You are right that in HITL we must not send HIL_STATE_QUATERNION as it would interfer with other stuff (which is the crazy thing that you experience :).

We send HIL_STATE_QUATERNION in SITL as ground truth, published in [0], and then it's just logged, but not further used in the system (and we only care about the attitude, which is why we don't set GPS and the other fields). True, it would be cheating if we used it :)

I only tested HITL yesterday for the first time, so I hope this makes sense.

For the climbing on arm problem, @julianoes is working on a proper fix.

[0] https://github.com/PX4/Firmware/blob/4b6441df0274541a0aa4c9153902e8361003afbb/src/modules/simulator/simulator_mavlink.cpp#L331

julianoes commented 7 years ago

For the climbing problems, @bkueng and I came up with: https://github.com/PX4/Firmware/pull/5716.

sytelus commented 7 years ago

Hi, I'm still wondering why we want to publish HIL_STATE_QUATERNION to autopilot either in HIL or SITL. In both modes, autopilot should never know the ground truth for the good simulation. Shouldn't autopilot just receive sensor messages and estimate pose? If it knows ground truth then we would not see the impact of noise in sensors or missing sensors, for example. Let me ask this question in other way: If we don't send HIL_STATE_QUATERNION to autopilot, what problems would we see?