-
Notifications
You must be signed in to change notification settings - Fork 13.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
Rework 1st waypoint check #23568
Merged
Merged
Rework 1st waypoint check #23568
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sfuhrer
reviewed
Aug 19, 2024
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.
There are some CI failures and I have some mini comments, otherwise I fully agree with the approach.
src/modules/navigator/MissionFeasibility/FeasibilityChecker.cpp
Outdated
Show resolved
Hide resolved
src/modules/navigator/MissionFeasibility/FeasibilityChecker.cpp
Outdated
Show resolved
Hide resolved
…ll accept mission as valid
… position instead of current position, so it is more constant during a flight
Co-authored-by: Silvan Fuhrer <[email protected]>
Signed-off-by: Silvan Fuhrer <[email protected]>
sfuhrer
force-pushed
the
redo_1st_waypoint_check
branch
from
August 20, 2024 15:01
3be4316
to
f382d6d
Compare
sfuhrer
approved these changes
Aug 21, 2024
vertiq-jordan
pushed a commit
to iq-motion-control/PX4-Autopilot
that referenced
this pull request
Aug 21, 2024
) * FeasibilityChecker: only warn when first waypoint is too far, but still accept mission as valid * feasiblityChecker: make distance to first waypoint check against home position instead of current position, so it is more constant during a flight * Apply suggestions from code review Co-authored-by: Silvan Fuhrer <[email protected]> * feasibilityCheckerTest: update tests to not fail for first waypoint check * feasibilityChecker: make comment for 1stwaypointcheck event * Feasibility check unit test: fix comment Signed-off-by: Silvan Fuhrer <[email protected]> --------- Signed-off-by: Silvan Fuhrer <[email protected]> Co-authored-by: Silvan Fuhrer <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Solved Problem
In the mission feasibility, it is checked if the 1st waypoint is too far from the current position. This means that the mission feasibility can change during flight if moving further away from the first waypoint. The mission feasibility should not be able to change depending on the current position of the AV. Especially when a long mission is flown and interrupted, the mission check should make a mission infeasible because the AV did fly away from the fist mission item.
Since the first waypoint mission check should indicate that potentially an old mission from another location is loaded in PX4 it should not be able to change the feasibility in flight
Fixes #{Github issue ID}
Solution
Changelog Entry
For release notes:
Alternatives
The home position can stil change during a flight and a even better point would be to specifically take a takeoff location which would be constant for an entire flight (AFAIK does not exist yet). Also it can still be argued if the check should make the whole mission infeasible, for simplicity, it was converted to a warning only.
Test coverage
Context
Related links, screenshot before/after, video