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.

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

Add payload tx/rx timeouts to DDS #24309

Merged
merged 4 commits into from
Feb 14, 2025
Merged

Add payload tx/rx timeouts to DDS #24309

merged 4 commits into from
Feb 14, 2025

Conversation

alexcekay
Copy link
Member

@alexcekay alexcekay commented Feb 10, 2025

Solved Problem

We had certain situations where the DDS Agent did stop to send payload to the DDS client.
In that situation the DDS client could still ping the agent and thus the currently existing robustness measurements did not trigger a reconnect.

Solution

Added two new parameters:

  • UXRCE_DDS_RX_TO
    • Defines after how many seconds of not receiving RX payload the connection is reset
    • Is disabled by default
  • UXRCE_DDS_TX_TO
    • Defines after how many seconds of not sending TX payload the connection is reset
    • Is disabled by default

The already existing TX/RX payload measurements are used to check if payload is still sent/received.
In addition I refactored the DDS logic into a few new methods, as the main loop got really hard to read.

Changelog Entry

For release notes:

Feature Added TX/RX payload timeouts to DDS

Alternatives

  • I will look into the root cause, why the Agent performed so badly in our case. This investigation however will take more time and this is a general robustness measurement that can also help in other situations.
  • It would be possible to automatically enable/disable this feature by generating booleans depending if pubs/subs are configured in dds_topics.yaml. The timeout to set however depends on the pub rate of the topic, which could be dynamic. To avoid decreasing the robustness, the parameter allows the user to set the timeout that is actually required for the system.

Test coverage

  • Tested the following on the bench
    • The ping reset still works, when the new parameters are not used
    • The TX/RX resets work independent of each other
    • The CPU load is not visibly increased

Copy link

github-actions bot commented Feb 10, 2025

🔎 FLASH Analysis

px4_fmu-v5x [Total VM Diff: 672 byte (0.03 %)]
    FILE SIZE        VM SIZE    
--------------  -------------- 
+0.0%    +672  +0.0%    +672    .text
  +1.1%    +386  +1.1%    +386    ../../src/modules/uxrce_dds_client/uxrce_dds_client.cpp
  +0.1%    +173  +0.1%    +173    [section .text]
  +0.1%     +98  +0.1%     +98    ROMFS/nsh_romfsimg.c
  +0.1%     +16  +0.1%     +16    ../../src/lib/parameters/parameters.cpp
  +0.2%      +3  +0.2%      +3    ../../src/systemcmds/ver/ver.cpp
  -0.1%      -4  -0.1%      -4    ../../src/modules/vtol_att_control/vtol_type.cpp
+0.0%     +56  [ = ]       0    .debug_abbrev
   +11%     +56  [ = ]       0    ../../src/lib/version/version.c
+0.0%     +16  [ = ]       0    .debug_aranges
  -5.0%      -8  [ = ]       0    ../../src/lib/version/version.c
  +3.9%     +24  [ = ]       0    ../../src/modules/uxrce_dds_client/uxrce_dds_client.cpp
+0.0%    +104  [ = ]       0    .debug_frame
+0.0% +5.03Ki  [ = ]       0    .debug_info
  +0.0%     +16  [ = ]       0    ../../src/drivers/batt_smbus/batt_smbus.cpp
  +0.0%     +16  [ = ]       0    ../../src/drivers/cdcacm_autostart/cdcacm_autostart.cpp
  +0.0%     +16  [ = ]       0    ../../src/drivers/distance_sensor/lightware_laser_i2c/lightware_laser_i2c.cpp
  +0.0%     +16  [ = ]       0    ../../src/drivers/dshot/DShot.cpp
  +0.0%     +16  [ = ]       0    ../../src/drivers/heater/heater.cpp
  +0.0%     +16  [ = ]       0    ../../src/drivers/lights/rgbled/rgbled.cpp
  +0.0%     +16  [ = ]       0    ../../src/drivers/lights/rgbled_is31fl3195/rgbled_is31fl3195.cpp
  +0.0%     +16  [ = ]       0    ../../src/drivers/lights/rgbled_lp5562/rgbled_lp5562.cpp
  +0.0%     +16  [ = ]       0    ../../src/drivers/lights/rgbled_ncp5623c/rgbled_ncp5623c.cpp
  +0.0%     +16  [ = ]       0    ../../src/drivers/osd/msp_osd/msp_osd.cpp
  +0.0%     +16  [ = ]       0    ../../src/drivers/power_monitor/ina226/ina226.cpp
  +0.1%     +16  [ = ]       0    ../../src/drivers/power_monitor/ina226/ina226_main.cpp
  +0.0%     +16  [ = ]       0    ../../src/drivers/power_monitor/ina228/ina228.cpp
  +0.1%     +16  [ = ]       0    ../../src/drivers/power_monitor/ina228/ina228_main.cpp
  +0.0%     +16  [ = ]       0    ../../src/drivers/power_monitor/ina238/ina238.cpp
  +0.1%     +16  [ = ]       0    ../../src/drivers/power_monitor/ina238/ina238_main.cpp
  +0.0%     +16  [ = ]       0    ../../src/drivers/power_monitor/pm_selector_auterion/PowerMonitorSelectorAuterion.cpp
  +0.0%     +16  [ = ]       0    ../../src/drivers/pwm_out/PWMOut.cpp
  +0.0%     +16  [ = ]       0    ../../src/drivers/px4io/px4io.cpp
  +0.0%     +16  [ = ]       0    ../../src/drivers/rc/crsf_rc/CrsfRc.cpp
 -100.0% +4.71Ki  [ = ]       0    [171 Others]
+0.0%    +454  [ = ]       0    .debug_line
  -1.3%     -25  [ = ]       0    ../../src/lib/version/version.c
  +0.9%    +483  [ = ]       0    ../../src/modules/uxrce_dds_client/uxrce_dds_client.cpp
  -0.4%      -4  [ = ]       0    task/task_cancelpt.c
+0.0%    +366  [ = ]       0    .debug_loc
  -0.1%     -15  [ = ]       0    ../../src/drivers/px4io/px4io.cpp
  +0.2%     +15  [ = ]       0    ../../src/drivers/rc/crsf_rc/CrsfRc.cpp
  +0.1%     +15  [ = ]       0    ../../src/drivers/uavcan/remoteid.cpp
  -0.0%     -13  [ = ]       0    ../../src/drivers/uavcan/uavcan_main.cpp
  +0.0%     +29  [ = ]       0    ../../src/drivers/uavcan/uavcan_servers.cpp
  -0.1%     -15  [ = ]       0    ../../src/lib/avoidance/ObstacleAvoidance.cpp
  -0.1%     -30  [ = ]       0    ../../src/lib/collision_prevention/CollisionPrevention.cpp
  -0.1%     -16  [ = ]       0    ../../src/lib/mixer_module/mixer_module.cpp
  +0.2%     +15  [ = ]       0    ../../src/modules/camera_feedback/CameraFeedback.cpp
  +0.0%     +15  [ = ]       0    ../../src/modules/commander/Commander.cpp
  +0.0%      +1  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/externalChecks.cpp
  +0.1%     +13  [ = ]       0    ../../src/modules/commander/failure_detector/FailureDetector.cpp
  +0.0%     +15  [ = ]       0    ../../src/modules/flight_mode_manager/tasks/Auto/FlightTaskAuto.cpp
  -0.0%     -13  [ = ]       0    ../../src/modules/flight_mode_manager/tasks/AutoFollowTarget/FlightTaskAutoFollowTarget.cpp
  +0.2%     +15  [ = ]       0    ../../src/modules/flight_mode_manager/tasks/Transition/FlightTaskTransition.cpp
  +0.2%     +15  [ = ]       0    ../../src/modules/flight_mode_manager/tasks/Utility/Sticks.cpp
  -0.1%     -15  [ = ]       0    ../../src/modules/fw_att_control/FixedwingAttitudeControl.cpp
  -0.0%     -16  [ = ]       0    ../../src/modules/fw_pos_control/FixedwingPositionControl.cpp
  +0.0%     +15  [ = ]       0    ../../src/modules/fw_rate_control/FixedwingRateControl.cpp
  -0.3%     -15  [ = ]       0    ../../src/modules/land_detector/FixedwingLandDetector.cpp
 -100.0%    +351  [ = ]       0    [11 Others]
+0.0%     +80  [ = ]       0    .debug_ranges
  -2.6%      -8  [ = ]       0    ../../src/lib/version/version.c
  +0.8%     +88  [ = ]       0    ../../src/modules/uxrce_dds_client/uxrce_dds_client.cpp
+0.1% +2.14Ki  [ = ]       0    .debug_str
  +0.1%     +32  [ = ]       0    ../../src/drivers/batt_smbus/batt_smbus.cpp
   +12% +2.11Ki  [ = ]       0    ../../src/modules/uxrce_dds_client/uxrce_dds_client.cpp
-0.5%      -1  [ = ]       0    .shstrtab
+0.0%    +141  [ = ]       0    .strtab
  -8.1%     -32  [ = ]       0    ../../src/lib/version/version.c
  +8.2%     +12  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/manualControlCheck.cpp
  +3.7%    +129  [ = ]       0    ../../src/modules/uxrce_dds_client/uxrce_dds_client.cpp
  +0.0%     +32  [ = ]       0    [section .strtab]
+0.0%    +144  [ = ]       0    .symtab
  -7.0%     -64  [ = ]       0    ../../src/lib/version/version.c
   +11%     +16  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/manualControlCheck.cpp
  +0.3%     +16  [ = ]       0    ../../src/modules/fw_pos_control/FixedwingPositionControl.cpp
  +4.2%    +128  [ = ]       0    ../../src/modules/uxrce_dds_client/uxrce_dds_client.cpp
  +0.1%     +48  [ = ]       0    [section .symtab]
-6.0%    -672  [ = ]       0    [Unmapped]
+0.0% +8.49Ki  +0.0%    +672    TOTAL

px4_fmu-v6x [Total VM Diff: 808 byte (0.04 %)]
    FILE SIZE        VM SIZE    
--------------  -------------- 
+0.0%    +744  +0.0%    +744    .text
  +1.1%    +386  +1.1%    +386    ../../src/modules/uxrce_dds_client/uxrce_dds_client.cpp
  +0.1%    +171  +0.1%    +171    [section .text]
  +0.4%    +104  +0.4%    +104    ../../src/lib/parameters/parameters.cpp
  +0.1%     +87  +0.1%     +87    ROMFS/nsh_romfsimg.c
  -0.1%      -4  -0.1%      -4    ../../src/modules/vtol_att_control/vtol_type.cpp
[ = ]       0  +0.1%     +64    .bss
  [ = ]       0  +6.9%     +60    [section .bss]
  [ = ]       0  +1.4%      +8    ../../src/lib/parameters/parameters.cpp
  [ = ]       0  -0.1%      -4    ../../platforms/nuttx/src/px4/common/board_dma_alloc.c
+0.0%     +56  [ = ]       0    .debug_abbrev
   +11%     +56  [ = ]       0    ../../src/lib/version/version.c
+0.0%     +16  [ = ]       0    .debug_aranges
  -5.0%      -8  [ = ]       0    ../../src/lib/version/version.c
  +3.9%     +24  [ = ]       0    ../../src/modules/uxrce_dds_client/uxrce_dds_client.cpp
+0.0%    +116  [ = ]       0    .debug_frame
+0.0% +4.85Ki  [ = ]       0    .debug_info
  +0.0%     +16  [ = ]       0    ../../src/drivers/cdcacm_autostart/cdcacm_autostart.cpp
  +0.0%     +16  [ = ]       0    ../../src/drivers/distance_sensor/lightware_laser_i2c/lightware_laser_i2c.cpp
  +0.0%     +16  [ = ]       0    ../../src/drivers/dshot/DShot.cpp
  +0.0%     +16  [ = ]       0    ../../src/drivers/heater/heater.cpp
  +0.0%     +16  [ = ]       0    ../../src/drivers/lights/rgbled/rgbled.cpp
  +0.0%     +16  [ = ]       0    ../../src/drivers/lights/rgbled_is31fl3195/rgbled_is31fl3195.cpp
  +0.0%     +16  [ = ]       0    ../../src/drivers/lights/rgbled_lp5562/rgbled_lp5562.cpp
  +0.0%     +16  [ = ]       0    ../../src/drivers/lights/rgbled_ncp5623c/rgbled_ncp5623c.cpp
  +0.0%     +16  [ = ]       0    ../../src/drivers/osd/msp_osd/msp_osd.cpp
  +0.0%     +16  [ = ]       0    ../../src/drivers/power_monitor/ina226/ina226.cpp
  +0.1%     +16  [ = ]       0    ../../src/drivers/power_monitor/ina226/ina226_main.cpp
  +0.0%     +16  [ = ]       0    ../../src/drivers/power_monitor/ina228/ina228.cpp
  +0.1%     +16  [ = ]       0    ../../src/drivers/power_monitor/ina228/ina228_main.cpp
  +0.0%     +16  [ = ]       0    ../../src/drivers/power_monitor/ina238/ina238.cpp
  +0.1%     +16  [ = ]       0    ../../src/drivers/power_monitor/ina238/ina238_main.cpp
  +0.0%     +16  [ = ]       0    ../../src/drivers/power_monitor/pm_selector_auterion/PowerMonitorSelectorAuterion.cpp
  +0.0%     +16  [ = ]       0    ../../src/drivers/pwm_out/PWMOut.cpp
  +0.0%     +16  [ = ]       0    ../../src/drivers/px4io/px4io.cpp
  +0.0%     +16  [ = ]       0    ../../src/drivers/rc_input/RCInput.cpp
  +0.0%     +16  [ = ]       0    ../../src/drivers/uavcan/actuators/esc.cpp
 -100.0% +4.54Ki  [ = ]       0    [160 Others]
+0.0%    +530  [ = ]       0    .debug_line
  +0.3%     +66  [ = ]       0    ../../src/lib/parameters/parameters.cpp
  -1.3%     -25  [ = ]       0    ../../src/lib/version/version.c
  +0.9%    +483  [ = ]       0    ../../src/modules/uxrce_dds_client/uxrce_dds_client.cpp
  +0.5%      +6  [ = ]       0    task/task_cancelpt.c
-0.0%      -5  [ = ]       0    .debug_loc
  +0.2%     +15  [ = ]       0    ../../src/drivers/osd/msp_osd/msp_osd.cpp
  -0.1%     -15  [ = ]       0    ../../src/drivers/px4io/px4io.cpp
  -0.0%      -1  [ = ]       0    ../../src/drivers/uavcan/uavcan_servers.cpp
  +0.1%     +15  [ = ]       0    ../../src/lib/collision_prevention/CollisionPrevention.cpp
  +0.1%     +16  [ = ]       0    ../../src/lib/mixer_module/mixer_module.cpp
  -0.2%     -52  [ = ]       0    ../../src/lib/parameters/parameters.cpp
  -0.2%     -15  [ = ]       0    ../../src/lib/rtl/rtl_time_estimator.cpp
  -0.2%     -15  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/batteryCheck.cpp
  +0.1%     +13  [ = ]       0    ../../src/modules/commander/failure_detector/FailureDetector.cpp
  -0.2%     -15  [ = ]       0    ../../src/modules/flight_mode_manager/FlightModeManager.cpp
  -0.0%     -13  [ = ]       0    ../../src/modules/flight_mode_manager/tasks/AutoFollowTarget/FlightTaskAutoFollowTarget.cpp
  -0.1%     -15  [ = ]       0    ../../src/modules/fw_att_control/FixedwingAttitudeControl.cpp
  -0.0%     -16  [ = ]       0    ../../src/modules/fw_autotune_attitude_control/fw_autotune_attitude_control.cpp
  -0.0%     -31  [ = ]       0    ../../src/modules/fw_pos_control/FixedwingPositionControl.cpp
  +0.1%     +15  [ = ]       0    ../../src/modules/gyro_calibration/GyroCalibration.cpp
  -0.3%     -15  [ = ]       0    ../../src/modules/land_detector/FixedwingLandDetector.cpp
  -0.1%     -15  [ = ]       0    ../../src/modules/navigator/mission_base.cpp
  -0.2%     -15  [ = ]       0    ../../src/modules/navigator/rtl_direct.cpp
  -0.2%     -15  [ = ]       0    ../../src/modules/navigator/rtl_direct_mission_land.cpp
  -0.5%     -15  [ = ]       0    ../../src/modules/navigator/rtl_mission_fast.cpp
 -100.0%    +184  [ = ]       0    [8 Others]
+0.0%    +135  [ = ]       0    .debug_ranges
  +0.4%     +56  [ = ]       0    ../../src/lib/parameters/parameters.cpp
  -2.6%      -8  [ = ]       0    ../../src/lib/version/version.c
  +0.8%     +88  [ = ]       0    ../../src/modules/uxrce_dds_client/uxrce_dds_client.cpp
  -1.5%      -1  [ = ]       0    task/task_cancelpt.c
+0.1% +2.14Ki  [ = ]       0    .debug_str
  +0.1%     +32  [ = ]       0    ../../src/drivers/cdcacm_autostart/cdcacm_autostart.cpp
   +11% +2.11Ki  [ = ]       0    ../../src/modules/uxrce_dds_client/uxrce_dds_client.cpp
-0.5%      -1  [ = ]       0    .shstrtab
+0.0%    +141  [ = ]       0    .strtab
  -8.1%     -32  [ = ]       0    ../../src/lib/version/version.c
  +8.2%     +12  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/manualControlCheck.cpp
  +3.7%    +129  [ = ]       0    ../../src/modules/uxrce_dds_client/uxrce_dds_client.cpp
  +0.0%     +32  [ = ]       0    [section .strtab]
+0.0%    +144  [ = ]       0    .symtab
  -7.0%     -64  [ = ]       0    ../../src/lib/version/version.c
   +11%     +16  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/manualControlCheck.cpp
  +0.3%     +16  [ = ]       0    ../../src/modules/fw_pos_control/FixedwingPositionControl.cpp
  +4.2%    +128  [ = ]       0    ../../src/modules/uxrce_dds_client/uxrce_dds_client.cpp
  +0.1%     +48  [ = ]       0    [section .symtab]
-1.1%    -744  [ = ]       0    [Unmapped]
+0.0% +8.09Ki  +0.0%    +808    TOTAL

Updated: 2025-02-14T13:44:45

@alexcekay alexcekay marked this pull request as ready for review February 11, 2025 17:29
@alexcekay alexcekay requested review from dagar and niklaut February 11, 2025 17:29
@dagar
Copy link
Member

dagar commented Feb 11, 2025

Why not leave it enabled by default with a very conservative timeout?

@alexcekay
Copy link
Member Author

Why not leave it enabled by default with a very conservative timeout?

@dagar:
I did enable it now for TX by default, as this is rather safe due to the system constantly sending out data in the "normal" state. It would be required to remove a lot from dds_topics.yaml to get a system where nothing is sent. If a user really builds a system that just receives and does not send anything via DDS it is still possible to disable TX monitoring.

For RX however I don't think it is would be good to enable it by default. In my case I have a setup that does only send but not receive any data via the bridge. This would thus cause a connection reset every x seconds which delays sending.

niklaut
niklaut previously approved these changes Feb 14, 2025
Copy link
Contributor

@niklaut niklaut left a comment

Choose a reason for hiding this comment

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

Nice, just the docs are a bit inconsistent

src/modules/uxrce_dds_client/module.yaml Outdated Show resolved Hide resolved
@alexcekay alexcekay requested a review from niklaut February 14, 2025 13:39
@alexcekay alexcekay merged commit 430be08 into main Feb 14, 2025
62 of 63 checks passed
@alexcekay alexcekay deleted the pr-dds-txrx-timeout branch February 14, 2025 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants