Skip to content

Commander: strictly separate GNSS checks from global position ones#27374

Open
sfuhrer wants to merge 2 commits into
mainfrom
chore/renmae-COM_ARM_WO_GPS
Open

Commander: strictly separate GNSS checks from global position ones#27374
sfuhrer wants to merge 2 commits into
mainfrom
chore/renmae-COM_ARM_WO_GPS

Conversation

@sfuhrer
Copy link
Copy Markdown
Contributor

@sfuhrer sfuhrer commented May 18, 2026

Solved Problem

Name of COM_ARM_WO_GPS is confusing and partially wrong.

Solution

  • Rename COM_ARM_WO_GPS to COM_ARM_WO_GPOS.
  • Only use it to reject arming when global position is invalid, do not convolute it with GNSS quality checks

This follows up on #26863: If the system is expected to never be flown without a valid GNSS, SYS_HAS_NUM_GNSS should be set to >0. The system then rejects arming, and can failsafe if it looses it in-air (separate config, COM_GNSSLOSS_ACT).

Changelog Entry

For release notes:

Chore: Commander: strictly separate GNSS checks from global position ones

Alternatives

Test coverage

Untested.

sfuhrer added 2 commits May 18, 2026 10:06
@sfuhrer
Commander: EstimatorCheck: use SYS_HAS_NUM_GNSS instead of COM_ARM_WO…
ed4ac89 
…_GPS for GPS checks

Reject arming if SYS_HAS_NUM_GNSS is >0 and a GNSS quality check is failing.
Do not distinguish between modes, treat all modes the same.
Never publish a warning only.

Signed-off-by: Silvan <silvan@auterion.com>
@sfuhrer
Commander: rename COM_ARM_WO_GPS to COM_ARM_WO_GPOS
1f72189 
Strictly separate GNSS checks from global position.
Arming logic around needing global position: add warning level,
as the paramter already had it.

Signed-off-by: Silvan <silvan@auterion.com>
@github-actions github-actions Bot added risk:safety-critical May affect arming, failsafe, control, navigation, or flight safety. scope:commander Arming, modes, failsafe, health checks, or vehicle state. scope:parameters Parameter definitions, metadata, migration, or defaults. scope:docs labels May 18, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 18, 2026

💡 Commit messages could be improved

Not blocking, but these commit messages could use some cleanup.

Commit Message Suggestion
ed4ac89534 Commander: EstimatorCheck: use SYS_HAS_NUM_GNSS instead of COM_ARM_WO_GPS for GPS checks Missing conventional commit format (e.g. "feat(ekf2): add something")
1f721892bd Commander: rename COM_ARM_WO_GPS to COM_ARM_WO_GPOS Missing conventional commit format (e.g. "feat(ekf2): add something")

See the commit message convention for details.

This comment will be automatically removed once the issues are resolved.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 18, 2026

❌ PR title needs conventional commit format

Expected format: type(scope): description (conventional commits).

Your title:

Commander: strictly separate GNSS checks from global position ones

Suggested fix:

feat(commander): strictly separate GNSS checks from global position ones

To fix this: click the ✏️ next to the PR title at the top of this page and update it.

See CONTRIBUTING.md for details.

This comment will be automatically removed once the issue is resolved.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 18, 2026

🔎 FLASH Analysis

px4_fmu-v5x [Total VM Diff: 144 byte (0.01 %)] 
    FILE SIZE        VM SIZE    
 --------------  -------------- 
  +0.0%    +144  +0.0%    +144    .text
    +0.1%    +112  +0.1%    +112    [section .text]
    +9.4%     +68  +9.4%     +68    SystemChecks::checkAndReport()
    [NEW]     +56  [NEW]     +56    CSWTCH.255
    +5.1%     +56  +5.1%     +56    param_modify_on_import()
    [NEW]     +32  [NEW]     +32    CSWTCH.251
    [NEW]      +4  [NEW]      +4    CSWTCH.817
    [DEL]      -4  [DEL]      -4    CSWTCH.820
   -10.5%     -16 -10.5%     -16    SystemChecks::SystemChecks()
   -19.0%     -16 -19.0%     -16    SystemChecks::updateParamsImpl()
    -0.0%     -16  -0.0%     -16    g_cromfs_image
    [DEL]     -32  [DEL]     -32    CSWTCH.252
    -2.6%     -44  -2.6%     -44    EstimatorChecks::checkEstimatorStatus()
    [DEL]     -56  [DEL]     -56    CSWTCH.256
  -0.0%     -54  [ = ]       0    .debug_abbrev
  -0.0%      -8  [ = ]       0    .debug_frame
  -0.0% -1.31Ki  [ = ]       0    .debug_info
  -0.0%     -20  [ = ]       0    .debug_line
   -60.0%      -3  [ = ]       0    [Unmapped]
    -0.0%     -17  [ = ]       0    [section .debug_line]
  +0.0%    +292  [ = ]       0    .debug_loclists
  +0.0%     +14  [ = ]       0    .debug_rnglists
     +50%      +1  [ = ]       0    [Unmapped]
    +0.0%     +13  [ = ]       0    [section .debug_rnglists]
  -0.0%    -950  [ = ]       0    .debug_str
  -1.8%    -144  [ = ]       0    [Unmapped]
  -0.0% -2.02Ki  +0.0%    +144    TOTAL
px4_fmu-v6x [Total VM Diff: 200 byte (0.01 %)] 
    FILE SIZE        VM SIZE    
 --------------  -------------- 
  +0.0%    +200  +0.0%    +200    .text
    +0.1%    +128  +0.1%    +128    [section .text]
    +9.4%     +68  +9.4%     +68    SystemChecks::checkAndReport()
    [NEW]     +56  [NEW]     +56    CSWTCH.255
    +5.1%     +56  +5.1%     +56    param_modify_on_import()
    [NEW]     +32  [NEW]     +32    CSWTCH.251
    +0.0%     +24  +0.0%     +24    g_cromfs_image
    [NEW]      +4  [NEW]      +4    CSWTCH.817
    [DEL]      -4  [DEL]      -4    CSWTCH.820
   -10.5%     -16 -10.5%     -16    SystemChecks::SystemChecks()
   -19.0%     -16 -19.0%     -16    SystemChecks::updateParamsImpl()
    [DEL]     -32  [DEL]     -32    CSWTCH.252
    -2.6%     -44  -2.6%     -44    EstimatorChecks::checkEstimatorStatus()
    [DEL]     -56  [DEL]     -56    CSWTCH.256
  -0.0%     -54  [ = ]       0    .debug_abbrev
  -0.0%      -8  [ = ]       0    .debug_frame
  -0.0% -1.31Ki  [ = ]       0    .debug_info
  -0.0%     -12  [ = ]       0    .debug_line
    +250%      +5  [ = ]       0    [Unmapped]
    -0.0%     -17  [ = ]       0    [section .debug_line]
  +0.0%    +292  [ = ]       0    .debug_loclists
  +0.0%     +14  [ = ]       0    .debug_rnglists
    [NEW]      +1  [ = ]       0    [Unmapped]
    +0.0%     +13  [ = ]       0    [section .debug_rnglists]
  -0.0%    -950  [ = ]       0    .debug_str
  -4.4%    -200  [ = ]       0    [Unmapped]
  -0.0% -2.01Ki  +0.0%    +200    TOTAL

Updated: 2026-05-18T09:23:25

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 18, 2026

No broken links found in changed files.

dakejahl
dakejahl reviewed May 18, 2026
View reviewed changes
Comment thread src/modules/commander/HealthAndArmingChecks/checks/estimatorCheck.cpp

} else {
required_modes.message_modes = required_modes.fail_modes = NavModes::None;
log_level = events::Log::Disabled;
Copy link
Copy Markdown
Contributor

@dakejahl dakejahl May 18, 2026

Choose a reason for hiding this comment

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

The description for SYS_HAS_NUM_GNSS says it will warn, and it does below via mavlink_log_warning, shouldn't we mark this as warn for all modes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dakejahl dakejahl dakejahl left review comments

@bresch bresch bresch approved these changes

Assignees

No one assigned

Labels

risk:safety-critical May affect arming, failsafe, control, navigation, or flight safety. scope:commander Arming, modes, failsafe, health checks, or vehicle state. scope:docs scope:parameters Parameter definitions, metadata, migration, or defaults.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sfuhrer @bresch @dakejahl