-
Notifications
You must be signed in to change notification settings - Fork 17.6k
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 Guided Alt Command: Use frame instead of bools for setting alt frame #28479
Plane Guided Alt Command: Use frame instead of bools for setting alt frame #28479
Conversation
@IamPete1 Mind taking a look? This is starting to move towards treating altitude frame booleans as private. It will make this PR smaller: #26328 (comment) |
d4bbf2f
to
8b15739
Compare
Switched to |
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.
Please outline what testing you have done on this PR.
I did in the description. Do you want me to upload logs, copy the exact terminal output, a video, or something more specific? |
I'm working on adding an autotest in The test methodology I used for this was not hitting the API I thought it was and I want this checked in CI before touching it. See #28487 |
* And switch to mavlink_coordinate_frame_to_location_alt_frame Signed-off-by: Ryan Friedman <[email protected]>
8b15739
to
02b497f
Compare
So PH and I weren't going to scope creep you on that bit :-) OTOH, now that you've started - why not early-return on the alt-mask check too? It's going to be a nice function after that. |
If you have a way to easily test it and are comfortable, maybe we skip CI 🤣 The reason I don't early return on the alt mask is there will be other masks to test against coming up. The alt can be controlled in the upcoming path guidance mode at the same time as other fields in this function. |
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.
This looks correct to me.
Purpose
When a plane receives an altitude command from mavlink SET_POSITION_TARGET_GLOBAL_INT, we were using the booleans in the Location field to set the frame. This switches to using the altitude setter with the frame which is more explicit.
We now convert the frames with
mavlink_coordinate_frame_to_location_alt_frame
.Conveniently, deleting code saves 40b flash.
Tests performed
./Tools/autotest/autotest.py test.Plane
.It seems like this might be covered by
SetpointGlobalPos
, but that test is not run by Plane.I also used the
changealt
andchangealt_abs
command inGUIDED
mode while flying in SITL to verify we can change altitude. Those commands do not use this interface.I do not have a great way to test this as is. Suggestions welcome!
Size Compare Durandal