ArduPilot / ardupilot

ArduPlane, ArduCopter, ArduRover, ArduSub source
http://ardupilot.org/
GNU General Public License v3.0
10.95k stars 17.48k forks source link

Plane::send_nav_controller_output uses incorrect airspeed units #7932

Open lekston opened 6 years ago

lekston commented 6 years ago

Issue details

Plane::send_nav_controller_output reports airspeed error in [cm] whereas mavlink specifies it in [m]. I did a (very brief) search in Mission Planner, qGroundControl and Droid Planner/Tower to check that they do not use this message so I believe there should be no problems with correcting it.

Version

3.8.4

Platform

[ ] All [ ] AntennaTracker [ ] Copter [x] Plane [ ] Rover [ ] Submarine

Naterater commented 6 years ago

I'd like airspeed in general to be in m/s with 2 decimal places instead of cm/s. That might require some scaling elsewhere though.

As a reference, about 15 parameters use cm/s (mostly quadplane parameters). The other ~50 parameters use meters.

Takeoff and landing speeds are in m/s, wind speeds are in m/s, EKF values are in m/s. I would think that cruise, ground, and air speed should be the same way.

magicrub commented 6 years ago

I agree that m/s is easier than cm/s. They were put in that way for a good reason: floating point math is expensive on 8bit cpus. However, that time has passed and now the problem is param names are hard to change for compatibility reasons.

However, what @lekston is saying is we are not obeying the MAVLink spec and transmitting the data in wrong units. That is something that needs to get fixed.

magicrub commented 6 years ago

pending PR: https://github.com/ArduPilot/ardupilot/pull/7933

Naterater commented 6 years ago

Does it also need to be corrected in Mission Planner?

magicrub commented 6 years ago

A (very brief) search in Mission Planner, qGroundControl and Droid Planner/Tower they do not use this message

needs to be confirmed before that PR gets merged

lekston commented 6 years ago

I did a brief search in their repositories and I found no "mavlink_navcontroller" nor "mavlink_msg_nav_controller" in there.

meee1 commented 6 years ago

Missionplanner does infact use it https://github.com/ArduPilot/MissionPlanner/blob/master/ExtLibs/ArduPilot/CurrentState.cs#L2316

lekston commented 6 years ago

My apologies for overlooking this. Thanks Michael! I will know where to look next time.

amilcarlucas commented 3 years ago

ping?