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

Send_position_target_global_int: Set mavlink altitude frame to be target's frame #17344

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

hendjoshsr71
Copy link
Member

@hendjoshsr71 hendjoshsr71 commented May 4, 2021

This changes the send_position_target_global_int message to send the target's altitude mavlink frame in the same frame as the target's location frame.

Example: below uses waypoints in terrain, ASML, and above home frames.
Note this currently ends up being filled by AC_WP_Nav::get_wp_nav() which always sets the target to terrain or ASML. To get the above home frame filled in would require a bit work since that is not used in WP_Nav.

Master
master_set_altitude_frame_position_target_global_int

After PR
set_altitude_frame_position_target_global_int

ArduCopter/GCS_Mavlink.cpp Outdated Show resolved Hide resolved
@rmackay9
Copy link
Contributor

I think this is probably most useful in Guided mode where the caller may want to check that what they've sent in has been consumed correctly.

@hendjoshsr71 hendjoshsr71 force-pushed the pr/add_frames_to_pos_target_global branch from 359e13a to 6f8b7b5 Compare June 30, 2021 19:36
@hendjoshsr71 hendjoshsr71 force-pushed the pr/add_frames_to_pos_target_global branch from 6f8b7b5 to 50ce467 Compare July 8, 2021 06:32
@hendjoshsr71
Copy link
Member Author

hendjoshsr71 commented Jul 8, 2021

EDIT: moving all the below to separate PRs.
- Based on a request in discord, added send_position_target_global_int to Sub
- moved the message to gcs_common
- TODO: add the autotests

ArduSub receiving the message
image

@hendjoshsr71
Copy link
Member Author

hendjoshsr71 commented Sep 3, 2021

Updated this so that the commanded altitude frames for guided and auto modes can also report relative to home in addition to terrain & absolute frames.

Rover Autotest may fail: I think this is due to the issue with terrain in CI as it passes locally. There is an argument to be made that there isn't a lot of purpose to testing rover for differing altitude frames since it ignores altitude.

@hendjoshsr71
Copy link
Member Author

hendjoshsr71 commented Sep 28, 2021

I've disabled the terrain frame autotest for now in this PR after fixing the altitude frame conversion in autotest. See this issue for why #18788

Mission Below: Sets Relative Frame 50m then 100m then 50m, Terrain 100m then 150m then 100m, then Absolute Frame 1200m then 1250m

Master: Red Position_Target.Altitude
master_pos_target_alt_Frames

After PR: Green Position_Target.Altitude
after_pos_target_alt_Frames

@hendjoshsr71
Copy link
Member Author

@peterbarker Any thoughts on disabling set_position_target_zzz terrain frame testing as I do in this PR because of issue #18788 ? Or on my autotest changes here?

@hendjoshsr71 hendjoshsr71 force-pushed the pr/add_frames_to_pos_target_global branch from 39ce23d to 239d075 Compare February 7, 2022 00:36
@khancyr khancyr self-assigned this Jun 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants