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.

Sign up for GitHub

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

Jump to bottom

Commander: add option to re-enable GPS fusion on link loss #24193

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Commander: add option to re-enable GPS fusion on link loss #24193

wants to merge 1 commit into from

Conversation

sfuhrer
Copy link
Contributor

@sfuhrer sfuhrer commented Jan 10, 2025 •
edited
Loading

Solved Problem

In case of GPS spoofing or jamming, or simply for testing purposes, operators can disable the GPS fusion (EKF2_GPS_CTRL) to purely rely on other means of navigation. In case of link loss, they have no more way to change system settings, and in some cases it is beneficial if then the system switches back the GPS fusion setting as it was at the moment of takeoff.

Solution

New param: COM_DLL_GPS_CTRL.

In case the data link is lost and the GPS fusion parameter (EKF2_GPS_CTRL) is changed while arming, the fusion parameter is reset to the value before arming.

Changelog Entry

For release notes:

Feature: Commander: add option to re-enable GPS fusion on link loss...

Test coverage

SITL tested: VTOL:

  • takeoff, transition to FW, Hold mode,
  • set COM_DLL_GNSS_CTL to 1
  • set EKF2_GPS_CTRL to 0
  • close the ground station
    --> vehicle will RTL and set EKF2_GPS_CTRL back to 7

@sfuhrer sfuhrer requested review from MaEtUgR and bresch January 10, 2025 14:01
@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 10, 2025 •
edited
Loading

🔎 FLASH Analysis

px4_fmu-v5x [Total VM Diff: 320 byte (0.02 %)] 
    FILE SIZE        VM SIZE    
--------------  -------------- 
+0.0%    +320  +0.0%    +320    .text
  +0.9%    +176  +0.9%    +176    ../../src/modules/commander/Commander.cpp
  +0.0%     +86  +0.0%     +86    ROMFS/nsh_romfsimg.c
  +0.0%     +50  +0.0%     +50    [section .text]
  +0.0%      +8  +0.0%      +8    ../../src/lib/parameters/parameters.cpp
+0.0%     +56  [ = ]       0    .debug_abbrev
   +11%     +56  [ = ]       0    ../../src/lib/version/version.c
+0.0%      +8  [ = ]       0    .debug_aranges
  -5.0%      -8  [ = ]       0    ../../src/lib/version/version.c
  +1.8%     +16  [ = ]       0    ../../src/modules/commander/Commander.cpp
+0.0%     +56  [ = ]       0    .debug_frame
+0.0% +3.27Ki  [ = ]       0    .debug_info
  +0.0%      +8  [ = ]       0    ../../src/drivers/batt_smbus/batt_smbus.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/cdcacm_autostart/cdcacm_autostart.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/distance_sensor/lightware_laser_i2c/lightware_laser_i2c.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/dshot/DShot.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/heater/heater.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/lights/rgbled/rgbled.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/lights/rgbled_is31fl3195/rgbled_is31fl3195.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/lights/rgbled_lp5562/rgbled_lp5562.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/lights/rgbled_ncp5623c/rgbled_ncp5623c.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/osd/msp_osd/msp_osd.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/power_monitor/ina226/ina226.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/power_monitor/ina226/ina226_main.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/power_monitor/ina228/ina228.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/power_monitor/ina228/ina228_main.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/power_monitor/ina238/ina238.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/power_monitor/ina238/ina238_main.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/power_monitor/pm_selector_auterion/PowerMonitorSelectorAuterion.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/pwm_out/PWMOut.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/px4io/px4io.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/rc/crsf_rc/CrsfRc.cpp
 -100.0% +3.11Ki  [ = ]       0    [171 Others]
+0.0%    +319  [ = ]       0    .debug_line
  -1.3%     -25  [ = ]       0    ../../src/lib/version/version.c
  +1.1%    +348  [ = ]       0    ../../src/modules/commander/Commander.cpp
  -0.4%      -4  [ = ]       0    task/task_cancelpt.c
+0.0%    +441  [ = ]       0    .debug_loc
  -0.2%     -15  [ = ]       0    ../../src/drivers/rc/crsf_rc/CrsfRc.cpp
  +0.0%     +11  [ = ]       0    ../../src/drivers/uavcan/uavcan_servers.cpp
  +1.2%    +505  [ = ]       0    ../../src/modules/commander/Commander.cpp
  -0.4%     -15  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/accelerometerCheck.cpp
  -0.6%     -15  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/baroCheck.cpp
  -0.2%     -15  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/magnetometerCheck.cpp
  -0.2%     -15  [ = ]       0    ../../src/modules/flight_mode_manager/FlightModeManager.cpp
  -0.3%     -15  [ = ]       0    ../../src/modules/flight_mode_manager/tasks/FlightTask/FlightTask.cpp
  -0.2%     -15  [ = ]       0    ../../src/modules/flight_mode_manager/tasks/Transition/FlightTaskTransition.cpp
  +0.1%     +15  [ = ]       0    ../../src/modules/fw_att_control/FixedwingAttitudeControl.cpp
  -0.1%     -15  [ = ]       0    ../../src/modules/navigator/navigator_main.cpp
  -0.1%     -15  [ = ]       0    ../../src/modules/navigator/rtl.cpp
  +0.1%     +15  [ = ]       0    ../../src/modules/sensors/vehicle_magnetometer/VehicleMagnetometer.cpp
  -0.1%     -15  [ = ]       0    ../../src/modules/sensors/voted_sensors_update.cpp
  +0.3%     +15  [ = ]       0    ../../src/modules/simulation/sensor_gps_sim/SensorGpsSim.cpp
  +0.0%     +50  [ = ]       0    [section .debug_loc]
  -0.0%     -20  [ = ]       0    src/modules/ekf2/modules__ekf2_unity.cpp
+0.0%    +128  [ = ]       0    .debug_ranges
  -2.6%      -8  [ = ]       0    ../../src/lib/version/version.c
  +1.4%    +136  [ = ]       0    ../../src/modules/commander/Commander.cpp
+0.1% +1.84Ki  [ = ]       0    .debug_str
  +0.1%     +17  [ = ]       0    ../../src/drivers/batt_smbus/batt_smbus.cpp
  +1.6% +1.86Ki  [ = ]       0    ../../src/modules/commander/Commander.cpp
  +0.0%     +11  [ = ]       0    ../../src/modules/flight_mode_manager/FlightModeManager.cpp
  -0.1%     -28  [ = ]       0    ../../src/modules/fw_autotune_attitude_control/fw_autotune_attitude_control.cpp
  -0.0%     -21  [ = ]       0    src/modules/ekf2/modules__ekf2_unity.cpp
  +0.0%      +1  [ = ]       0    src/modules/flight_mode_manager/FlightTasks_generated.cpp
+0.5%      +1  [ = ]       0    .shstrtab
+0.0%    +163  [ = ]       0    .strtab
  -8.1%     -32  [ = ]       0    ../../src/lib/version/version.c
  +4.1%    +163  [ = ]       0    ../../src/modules/commander/Commander.cpp
  +0.0%     +32  [ = ]       0    [section .strtab]
+0.0%     +64  [ = ]       0    .symtab
  -7.0%     -64  [ = ]       0    ../../src/lib/version/version.c
  +1.3%     +64  [ = ]       0    ../../src/modules/commander/Commander.cpp
  +0.3%     +16  [ = ]       0    ../../src/modules/fw_pos_control/FixedwingPositionControl.cpp
  +0.1%     +48  [ = ]       0    [section .symtab]
  -2.2%     -16  [ = ]       0    src/modules/flight_mode_manager/FlightTasks_generated.cpp
  +5.9%     +16  [ = ]       0    usbdev/cdcacm_desc.c
-2.8%    -320  [ = ]       0    [Unmapped]
+0.0% +6.31Ki  +0.0%    +320    TOTAL
px4_fmu-v6x [Total VM Diff: 296 byte (0.02 %)] 
    FILE SIZE        VM SIZE    
--------------  -------------- 
+0.0%    +296  +0.0%    +296    .text
  +0.9%    +176  +0.9%    +176    ../../src/modules/commander/Commander.cpp
  +0.0%     +71  +0.0%     +71    ROMFS/nsh_romfsimg.c
  +0.0%     +44  +0.0%     +44    [section .text]
  +0.0%      +8  +0.0%      +8    ../../src/lib/parameters/parameters.cpp
  -0.2%      -3  -0.2%      -3    ../../src/systemcmds/ver/ver.cpp
+0.0%     +56  [ = ]       0    .debug_abbrev
   +11%     +56  [ = ]       0    ../../src/lib/version/version.c
+0.0%      +8  [ = ]       0    .debug_aranges
  -5.0%      -8  [ = ]       0    ../../src/lib/version/version.c
  +1.8%     +16  [ = ]       0    ../../src/modules/commander/Commander.cpp
+0.0%     +56  [ = ]       0    .debug_frame
+0.0% +3.18Ki  [ = ]       0    .debug_info
  +0.0%      +8  [ = ]       0    ../../src/drivers/cdcacm_autostart/cdcacm_autostart.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/distance_sensor/lightware_laser_i2c/lightware_laser_i2c.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/dshot/DShot.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/heater/heater.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/lights/rgbled/rgbled.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/lights/rgbled_is31fl3195/rgbled_is31fl3195.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/lights/rgbled_lp5562/rgbled_lp5562.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/lights/rgbled_ncp5623c/rgbled_ncp5623c.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/osd/msp_osd/msp_osd.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/power_monitor/ina226/ina226.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/power_monitor/ina226/ina226_main.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/power_monitor/ina228/ina228.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/power_monitor/ina228/ina228_main.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/power_monitor/ina238/ina238.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/power_monitor/ina238/ina238_main.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/power_monitor/pm_selector_auterion/PowerMonitorSelectorAuterion.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/pwm_out/PWMOut.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/px4io/px4io.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/rc_input/RCInput.cpp
  +0.0%      +8  [ = ]       0    ../../src/drivers/uavcan/actuators/esc.cpp
 -100.0% +3.02Ki  [ = ]       0    [160 Others]
+0.0%    +319  [ = ]       0    .debug_line
  -1.3%     -25  [ = ]       0    ../../src/lib/version/version.c
  +1.1%    +348  [ = ]       0    ../../src/modules/commander/Commander.cpp
  -0.4%      -4  [ = ]       0    task/task_cancelpt.c
+0.0%    +474  [ = ]       0    .debug_loc
  -0.2%     -15  [ = ]       0    ../../src/drivers/osd/msp_osd/msp_osd.cpp
  +0.0%      +2  [ = ]       0    ../../src/drivers/uavcan/uavcan_main.cpp
  -0.1%     -15  [ = ]       0    ../../src/lib/mixer_module/mixer_module.cpp
  +0.1%     +15  [ = ]       0    ../../src/modules/airspeed_selector/airspeed_selector_main.cpp
  +1.2%    +505  [ = ]       0    ../../src/modules/commander/Commander.cpp
  -0.1%     -75  [ = ]       0    ../../src/modules/sensors/vehicle_imu/VehicleIMU.cpp
  -0.3%     -15  [ = ]       0    ../../src/modules/temperature_compensation/TemperatureCompensationModule.cpp
  +0.0%     +48  [ = ]       0    [section .debug_loc]
  +0.0%      +9  [ = ]       0    src/modules/ekf2/modules__ekf2_unity.cpp
  +0.0%     +15  [ = ]       0    src/modules/mavlink/modules__mavlink_unity.cpp
+0.0%    +127  [ = ]       0    .debug_ranges
  -2.6%      -8  [ = ]       0    ../../src/lib/version/version.c
  +1.4%    +136  [ = ]       0    ../../src/modules/commander/Commander.cpp
  -1.5%      -1  [ = ]       0    task/task_cancelpt.c
+0.1% +1.84Ki  [ = ]       0    .debug_str
  +0.1%     +17  [ = ]       0    ../../src/drivers/cdcacm_autostart/cdcacm_autostart.cpp
  +1.6% +1.86Ki  [ = ]       0    ../../src/modules/commander/Commander.cpp
  -0.1%     -28  [ = ]       0    ../../src/modules/fw_autotune_attitude_control/fw_autotune_attitude_control.cpp
  +0.0%      +1  [ = ]       0    ../../src/modules/navigator/mission_base.cpp
  +0.0%     +11  [ = ]       0    ../../src/modules/navigator/navigator_main.cpp
  -0.0%     -21  [ = ]       0    src/modules/ekf2/modules__ekf2_unity.cpp
+0.0%    +152  [ = ]       0    .strtab
  -8.1%     -32  [ = ]       0    ../../src/lib/version/version.c
  +4.1%    +163  [ = ]       0    ../../src/modules/commander/Commander.cpp
  +0.0%     +21  [ = ]       0    [section .strtab]
+0.0%     +64  [ = ]       0    .symtab
  -1.3%     -16  [ = ]       0    ../../src/drivers/distance_sensor/ll40ls/LidarLiteI2C.cpp
  -1.0%     -16  [ = ]       0    ../../src/drivers/uavcan/sensors/battery.cpp
  -1.6%     -16  [ = ]       0    ../../src/lib/dataman_client/DatamanClient.cpp
  -7.0%     -64  [ = ]       0    ../../src/lib/version/version.c
  +1.3%     +64  [ = ]       0    ../../src/modules/commander/Commander.cpp
  +2.1%     +16  [ = ]       0    ../../src/modules/flight_mode_manager/tasks/Orbit/FlightTaskOrbit.cpp
  +0.3%     +16  [ = ]       0    ../../src/modules/fw_pos_control/FixedwingPositionControl.cpp
  +0.7%     +16  [ = ]       0    ../../src/modules/mc_pos_control/MulticopterPositionControl.cpp
  +0.1%     +48  [ = ]       0    [section .symtab]
  +2.3%     +16  [ = ]       0    src/modules/flight_mode_manager/FlightTasks_generated.cpp
-0.4%    -296  [ = ]       0    [Unmapped]
+0.0% +6.25Ki  +0.0%    +296    TOTAL

Updated: 2025-01-14T11:03:23

MaEtUgR
MaEtUgR reviewed Jan 13, 2025
View reviewed changes
Copy link
Member

@MaEtUgR MaEtUgR left a comment •
edited
Loading

Choose a reason for hiding this comment

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

  1. Is this a common workflow? I wouldn't put it in main if it's only used by a hand full of people in the same group during one phase of testing. GPS denied testing seems to be a thing recently so it could be that I don't know but this is useful for a broader audience.
  2. Ideally we'd not autonomously change a parameter in air. The workflow for toggling GPS fusion in an end-user workflow should probably be using e.g. a MAVLink command or similar.

It doesn't hurt anyone (except the 416 bytes flash) but I'm not sure how many users would want, find and use this feature and that's why I'm hesitant.

@dagar dagar self-requested a review January 14, 2025 07:31
dagar
dagar requested changes Jan 14, 2025
View reviewed changes
Copy link
Member

@dagar dagar left a comment

Choose a reason for hiding this comment

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

I'm not sure I understand the need for this, but at a minimum commander can't use an EKF2 parameter via module params because then it becomes a hard dependency.

It might be less bad if implemented within ekf2.

@sfuhrer
Copy link
Contributor Author

sfuhrer commented Jan 14, 2025

Thanks both for the feedback. We will not bring this to mainline PX4 for now, and if we do in the future I'll move it over to the EKF module.

@sfuhrer sfuhrer marked this pull request as draft January 14, 2025 09:01
@sfuhrer
Commander: add option to re-enable GPS fusion on link loss
33f1f45 
New param: COM_DLL_GPS_CTRL.

In case the data link is lost and the GPS fusion parameter
(EKF2_GPS_CTRL) is changed while arming, the fusion parameter
is reset to the value before arming.

Signed-off-by: Silvan <[email protected]>
@sfuhrer sfuhrer force-pushed the pr-gps-enable-link-loss-main branch from 1313e1e to 33f1f45 Compare January 14, 2025 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MaEtUgR MaEtUgR MaEtUgR left review comments

@dagar dagar dagar requested changes

@bresch bresch Awaiting requested review from bresch

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sfuhrer @dagar @MaEtUgR