-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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
mavlink: GLOBAL_POSITION_INT send without lat/lon availability #15308
Conversation
- the altitude and velocity portions of this message are still relevant without GPS lat/lon
I am highly agree with this change (I actually made similar change on my repo too..) |
msg.lat = gpos.lat * 1e7; | ||
msg.lon = gpos.lon * 1e7; | ||
// lat, lon: Latitude, Longitude in degE7 | ||
vehicle_global_position_s gpos{}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So lat/lon are just going to be 0 if they are not available?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct (once vehicle_local_position is publishing). This is the behavior @DonLakeFlyer requested that already has special handling in QGC for other platforms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would have preferred explicit handling, but as long as it's lat=lon=0 I think it's fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I'll have to add a check in MAVSDK then. And we should document it in the MAVLink spec.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about using int32_max
for both instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about using int32_max for both instead?
Yes that would work (I also suggested it), but I think it's too late with the significant existing deployed usage. We could still try to get the mavlink spec updated with the explicit INT32_MAX, but it would be in addition to the lat=lon=0 special case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, if it's already deployed then we might as well document 0/0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@LorenzMeier speak up if you are not ok with this!
FYI: ArduPilot already does 0/0. |
I haven't seen the notification for this - I strongly disagree with this, as we really shouldn't be adding ANYTHING that induces invalid data without proper flags. This really should have been done as a MAVLink packet extension with either altitude in a separate message or a flag for validity added. |
@DonLakeFlyer as discussed.
I'm not crazy about the idea of sending this continuously without a way to indicate the lat & lon aren't valid, but let me know what you'd prefer.