mavlink / mavros

MAVLink to ROS gateway with proxy for Ground Control Station
Other
847 stars 983 forks source link

Default config files wrongly set parameters for sys and time nodes #1930

Closed beniaminopozzan closed 3 months ago

beniaminopozzan commented 4 months ago

Issue details

The current default configuration files mavros/launch/px4_config.yaml and mavros/launch/apm_config.yaml do not properly set the parameters for the node sys and time

MAVROS version and platform

Mavros: 2.6.0 ROS: Humble Ubuntu: 22.04

Autopilot type and version

[x] ArduPilot [x] PX4

Version: N/A

Node logs

N/A

Diagnostics

N/A

Check ID

N/A

Expected behavior

The default configuration files (here for PX4 as example) set the timesync_rate to 10Hz: https://github.com/mavlink/mavros/blob/ded6c2ed9330c35893150b58bb0e9b19d574000f/mavros/launch/px4_config.yaml#L7C1-L13C87 However, that parameter is not actually used by the SystemTime plugin as can be seen with ros2 param dump /mavros/time:

/mavros/time:
  ros__parameters:
    convergence_window: 500
    max_consecutive_high_deviation: 10
    max_consecutive_high_rtt: 10
    max_deviation_sample: 10
    max_rtt_sample: 10
    qos_overrides:
      /parameter_events:
        publisher:
          depth: 1000
          durability: volatile
          history: keep_last
          reliability: reliable
    system_time_rate: 0.0
    time_ref_source: fcu
    timesync_alpha_final: 0.003
    timesync_alpha_initial: 0.05
    timesync_beta_final: 0.003
    timesync_beta_initial: 0.05
    timesync_mode: MAVLINK
    timesync_rate: 0.0
    use_sim_time: false

This is because timesync_rate should be placed under /mavros/*/time instad of /mavros/*/conn. In the end, all parameters of /mavros/*/conn should be moved either under /mavros/*/time or /mavros/*/sys in the config file

vooon commented 4 months ago

Hmm, yes, you're right. Bug from ROS1 migration...