Navigator: Add takeoff complete check when using local pos est #14226
Conversation
The current check only works with global position estimates, so if using local estimation, takeoff mode never finishes.
| } | ||
| const auto global_altitude_acceptable = | ||
| _navigator->get_global_position()->alt > altitude_amsl - altitude_acceptance_radius; | ||
| const auto local_altitude_acceptable = |
There was a problem hiding this comment.
I'm not sure that this is safe in general. As a first step what about only checking local acceptable if _mission_item.altitude_is_relative?
There was a problem hiding this comment.
You're probably right. This implementation is very crude and not properly tested, just kind of a "what I had to do to be able to fly". I'll check out the altitude is relative thing.
There was a problem hiding this comment.
When I'm flying SITL, pushing pose data in via the "external vision" input and setting EKF2_AID_MASK to 24 (vision only, no gps), so that my local estimate is valid but global invalid (checked with ekf2 status), it seems that _mission_item.altitude_is_relative is false. So I don't think I can use that to decide which check to use.
There was a problem hiding this comment.
I wanted to bring up the same concern, the things that come to my mind to make sure would be:
- a local origin that doesn't have z component 0 where the drone is currently standing (we should make that a default in SITL to catch these bugs early on)
- having GPS and loosing it again in which case something in the setup e.g. ground station might still be sending absolute altitude
With thealtitude_is_relativeit might work the right way.
|
Would you all like me to make further effort on this, or will you take over? I'm not sure if I should consider myself the assignee of this or not. |
|
I re-requested reviews so we can decide how to proceed. |
| _waypoint_position_reached = true; | ||
| } | ||
| const auto global_altitude_acceptable = | ||
| _navigator->get_global_position()->alt > altitude_amsl - altitude_acceptance_radius; |
There was a problem hiding this comment.
Check validity before using global?
There was a problem hiding this comment.
Definitely. I'll rebase and revisit this PR soon.
|
I'll close this, since it's gotten so old. I reckon this probably has changed upstream since I opened this. |
|
Thanks! This part of the code is still very similar but the original issue #14193 was solved already. In case there's something else this fixes then we could take it up again. |
Related to (possibly needed by) #14193.