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

vehicle_command: add initial frame support (GLOBAL & GLOBAL_RELATIVE_ALT) #23080

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

dagar
Copy link
Member

@dagar dagar commented May 1, 2024

This adds initial frame support to vehicle_command so that things like DO_REPOSITION can specify relative or absolute altitude. Ideally this would be updated to strictly respect the specification and reject anything other than GLOBAL/GLOBAL_RELATIVE_ALT, but for now I've left it open to hopefully not break any compatibility until we can verify.

In the future this can be nicely expanded to support things like MAV_FRAME_LOCAL_NED, MAV_FRAME_GLOBAL_TERRAIN_ALT, MAV_FRAME_BODY_FRD etc.

@dagar dagar requested a review from MaEtUgR May 1, 2024 22:22
@dagar dagar force-pushed the pr-cmd_frame branch 3 times, most recently from be2c751 to a3ded08 Compare May 1, 2024 23:39
@dagar
Copy link
Member Author

dagar commented May 1, 2024

I've updated this to actually reject any COMMAND_INT that has a frame other than GLOBAL or GLOBAL_RELATIVE_ALT, however longer term I think it would be more appropriate to push this out to the final handler (eg DO_REPOSITION) where we can have proper granularity for supported/unsupported frames for each command. Additionally we have no way to distinguish the frame being not set (MAV_FRAME 0) from MAV_FRAME_GLOBAL and some commands don't actually require a frame.

Copy link
Member

@MaEtUgR MaEtUgR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some suggestions, will add a commit.

src/modules/mavlink/mavlink_receiver.cpp Outdated Show resolved Hide resolved
src/modules/mavlink/mavlink_receiver.cpp Outdated Show resolved Hide resolved
msg/VehicleCommand.msg Outdated Show resolved Hide resolved
src/modules/mavlink/mavlink_receiver.cpp Outdated Show resolved Hide resolved
@MaEtUgR
Copy link
Member

MaEtUgR commented May 2, 2024

I rebased on main and added my suggestions. I'm not 100% sure if it breaks any flawed but popular use cases that we need to transition. For sure we need testing to make sure there's nothing obvious.

@MaEtUgR
Copy link
Member

MaEtUgR commented May 2, 2024

In my SILT SIH tests the usual stuff with QGC like takeoff, land, goto, orbit all work fine but when I put together a relative altitude goto it flies slowly into the ground like the original issue said. The new logic gets called and add home altitude and parameter but that somehow doesn't matter. Maybe my test is set up wrongly 👀

@MaEtUgR
Copy link
Member

MaEtUgR commented May 2, 2024

The addition of the home altitude really doesn't end up in the triplet
image
I'll have to check what's going on later.

@github-actions github-actions bot added the stale label Jun 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DO_REPOSITION ignores frame, always assumes AMSL
2 participants