fix(commander): gate heading innovation check on global origin#27420
Open
onelittlechildawa wants to merge 1 commit into
Open
fix(commander): gate heading innovation check on global origin#27420onelittlechildawa wants to merge 1 commit into
onelittlechildawa wants to merge 1 commit into
Conversation
github-actions
Bot
added
kind:bug
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates estimator arming checks to pass local position context into checkEstimatorStatus() and conditionally enforce heading requirements based on whether a global reference is set.
Changes:
- Extend
checkEstimatorStatus()to acceptvehicle_local_position_s(lpos) for richer decision-making. - Gate the preflight heading innovation failure check on
lpos.xy_global. - Update the call site to pass
lposinto the estimator status check.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/modules/commander/HealthAndArmingChecks/checks/estimatorCheck.hpp | Adds lpos parameter to checkEstimatorStatus() declaration. |
| src/modules/commander/HealthAndArmingChecks/checks/estimatorCheck.cpp | Passes lpos into checkEstimatorStatus() and uses lpos.xy_global to gate heading preflight failure. |
| const estimator_status_s &estimator_status, const vehicle_local_position_s &lpos, | ||
| NavModes required_groups) | ||
| { | ||
| // Heading is required to arm for all modes that need any form of local position, plus FW AUTO_TAKEOFF |
Comment on lines
+148
to
+149
| // Only require a heading reference when a global origin is set (i.e. global ops are intended) | ||
| if (!context.isArmed() && estimator_status.pre_flt_fail_innov_heading && lpos.xy_global) { |
| const estimator_status_s &estimator_status, const vehicle_local_position_s &lpos, | ||
| NavModes required_groups) | ||
| { | ||
| // Heading is required to arm for all modes that need any form of local position, plus FW AUTO_TAKEOFF |
Signed-off-by: onelittlechildawa <onelittlechild@outlook.com>
onelittlechildawa
force-pushed
the
fix/heading-innov-global-origin
branch
from
2d0517a to
442bae7
Compare
This file contains hidden or 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
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.
Summary
Follow-up to #27371.
Keep heading innovation preflight failures consistent with the global-origin gating added for heading-reference checks.
Problem
#27371 relaxed heading-reference enforcement for local-only operation, but one heading preflight failure path still did not account for whether a global origin exists.
This could still deny arming for local-only setups with a heading-related preflight error.
Solution
Apply the same global-origin requirement to the heading innovation preflight failure path.
Globally referenced operation keeps the stricter heading validation, while local-only operation remains allowed.