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

Include planned home position in MissionImportData #2115

Draft
wants to merge 6 commits into
base: v1.4
Choose a base branch
from

Conversation

RomanBapst
Copy link

@RomanBapst RomanBapst commented Aug 14, 2023

The planned home position is useful for moving a planned mission to the current location of the vehicle. Using the first mission item (as was done until now in the PX4 integration tests) is not correct for a VTOL mission containing a VTOL takeoff item.

Note:
Ardupilot missions contain the planned home location as the first mission item AFAIK. So the information in that case is duplicated.

…tion

such that the mission can be properly moved to the vehicle location
Copy link
Collaborator

@julianoes julianoes left a comment

Choose a reason for hiding this comment

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

Nice thanks. Couple of comments inline.

Comment on lines +2 to +3
* brief Demonstrates how to move a mission loaded from a .plan file such that the planned home
* position if present or the first waypoint is colocated with the vehicle position.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* brief Demonstrates how to move a mission loaded from a .plan file such that the planned home
* position if present or the first waypoint is colocated with the vehicle position.
* Demonstrates how to move a mission loaded from a .plan file such that the planned home
* position (if present) or the first waypoint is colocated with the vehicle position.

Comment on lines 92 to 93
REQUIRE(std::isfinite(position.latitude_deg));
REQUIRE(std::isfinite(position.longitude_deg));
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's not a test but and example, right?

offset_y = import_res.second.mission_items[0].y -
static_cast<int32_t>(1e7 * position.longitude_deg);
}
for (auto &item : import_res.second.mission_items) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably needs tools/fix_style.sh . applied.

@RomanBapst
Copy link
Author

@julianoes Thanks for the review, I wasn't actually done with this, just wanted to get it to compile (thanks to @JonasVautherin for helping with that). I can now wrap it up hopefully.

@julianoes julianoes marked this pull request as draft September 12, 2023 02:20
Signed-off-by: RomanBapst <[email protected]>
@julianoes
Copy link
Collaborator

@RomanBapst are you still planning to finish this? I think it would have to go into v2 though which is about to be released.

@RomanBapst
Copy link
Author

@julianoes Yes, I would still like to bring this in. I was plagued by compiling issues which turned out to be related to my cmake version, as @JonasVautherin could not reproduce. I got that under control but then never got to push it over the finish line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants