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

Plane::send_nav_controller_output uses incorrect airspeed units #7932

Open
lekston opened this issue Mar 15, 2018 · 9 comments
Open

Plane::send_nav_controller_output uses incorrect airspeed units #7932

lekston opened this issue Mar 15, 2018 · 9 comments

Comments

@lekston
Copy link
Contributor

lekston commented Mar 15, 2018

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

pending PR: #7933

@Naterater
Copy link
Contributor

Does it also need to be corrected in Mission Planner?

@magicrub
Copy link
Contributor

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
Copy link
Contributor Author

lekston commented Mar 16, 2018

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

@meee1
Copy link
Contributor

meee1 commented Mar 20, 2018

@lekston
Copy link
Contributor Author

lekston commented Mar 20, 2018

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

@amilcarlucas
Copy link
Contributor

ping?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants