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

Prevent overflow of dt in Integrator class #24202

Merged
merged 3 commits into from
Jan 21, 2025
Merged

Conversation

RomanBapst
Copy link
Contributor

Solved Problem

I was using the integrator class for a sensor which published at 12 Hz, which equals a dt of roughly 80'000 us.
The integrator dt varialbe was using a uint16, which overflows at around 65'500.

Solution

Use an uint32 variable.

Changelog Entry

For release notes:

Improvement: Prevent overflow of dt variable in Integrator class for sensors with low sampling rates.

@RomanBapst RomanBapst requested review from dagar and bresch January 13, 2025 08:20
@RomanBapst RomanBapst force-pushed the pr-fix_integrator_overflow branch from c4513df to 50d9cf9 Compare January 16, 2025 13:54
Copy link

github-actions bot commented Jan 16, 2025

🔎 FLASH Analysis

px4_fmu-v5x [Total VM Diff: -48 byte (-0 %)]
    FILE SIZE        VM SIZE    
--------------  -------------- 
+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
+0.0%     +83  [ = ]       0    .debug_info
  -0.2%      -4  [ = ]       0    ../../src/lib/version/version.c
  +0.0%     +13  [ = ]       0    ../../src/modules/sensors/vehicle_imu/VehicleIMU.cpp
  +0.0%     +13  [ = ]       0    ../../src/modules/sensors/voted_sensors_update.cpp
  +0.0%      +5  [ = ]       0    msg/topics_sources/uORBMessageFieldsGenerated.cpp
  +0.3%     +29  [ = ]       0    msg/topics_sources/vehicle_imu.cpp
  +0.0%     +13  [ = ]       0    src/modules/ekf2/modules__ekf2_unity.cpp
  +0.0%     +14  [ = ]       0    src/modules/mavlink/modules__mavlink_unity.cpp
-0.0%     -11  [ = ]       0    .debug_line
  -1.3%     -25  [ = ]       0    ../../src/lib/version/version.c
  -0.1%     -39  [ = ]       0    ../../src/modules/sensors/vehicle_imu/VehicleIMU.cpp
  +0.2%     +19  [ = ]       0    ../../src/modules/sensors/voted_sensors_update.cpp
  +0.0%     +40  [ = ]       0    src/modules/ekf2/modules__ekf2_unity.cpp
  -0.0%      -6  [ = ]       0    src/modules/mavlink/modules__mavlink_unity.cpp
+0.0%      +4  [ = ]       0    .debug_loc
  -0.0%     -20  [ = ]       0    ../../src/modules/sensors/vehicle_imu/VehicleIMU.cpp
  +0.0%     +24  [ = ]       0    src/modules/ekf2/modules__ekf2_unity.cpp
-0.0%      -8  [ = ]       0    .debug_ranges
  -2.6%      -8  [ = ]       0    ../../src/lib/version/version.c
  -0.2%     -48  [ = ]       0    ../../src/modules/sensors/vehicle_imu/VehicleIMU.cpp
  +0.2%      +8  [ = ]       0    ../../src/modules/sensors/voted_sensors_update.cpp
  +0.0%     +40  [ = ]       0    src/modules/ekf2/modules__ekf2_unity.cpp
+0.4%     +48  [ = ]       0    [Unmapped]
-0.0%     -48  -0.0%     -48    .text
  +0.0%      +8  +0.0%      +8    [section .text]
  -0.2%      -8  -0.2%      -8    ../../src/modules/sensors/voted_sensors_update.cpp
  -0.0%      -8  -0.0%      -8    src/modules/ekf2/modules__ekf2_unity.cpp
  -0.1%     -16  -0.1%     -16    ../../src/modules/sensors/vehicle_imu/VehicleIMU.cpp
  -0.0%     -24  -0.0%     -24    src/modules/mavlink/modules__mavlink_unity.cpp
+0.0%    +116  -0.0%     -48    TOTAL

px4_fmu-v6x [Total VM Diff: -48 byte (-0 %)]
    FILE SIZE        VM SIZE    
--------------  -------------- 
+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
+0.0%     +83  [ = ]       0    .debug_info
  -0.2%      -4  [ = ]       0    ../../src/lib/version/version.c
  +0.0%     +13  [ = ]       0    ../../src/modules/sensors/vehicle_imu/VehicleIMU.cpp
  +0.0%     +13  [ = ]       0    ../../src/modules/sensors/voted_sensors_update.cpp
  +0.0%      +5  [ = ]       0    msg/topics_sources/uORBMessageFieldsGenerated.cpp
  +0.3%     +29  [ = ]       0    msg/topics_sources/vehicle_imu.cpp
  +0.0%     +13  [ = ]       0    src/modules/ekf2/modules__ekf2_unity.cpp
  +0.0%     +14  [ = ]       0    src/modules/mavlink/modules__mavlink_unity.cpp
-0.0%     -11  [ = ]       0    .debug_line
  -1.3%     -25  [ = ]       0    ../../src/lib/version/version.c
  -0.1%     -39  [ = ]       0    ../../src/modules/sensors/vehicle_imu/VehicleIMU.cpp
  +0.2%     +19  [ = ]       0    ../../src/modules/sensors/voted_sensors_update.cpp
  +0.0%     +40  [ = ]       0    src/modules/ekf2/modules__ekf2_unity.cpp
  -0.0%      -6  [ = ]       0    src/modules/mavlink/modules__mavlink_unity.cpp
+0.0%     +93  [ = ]       0    .debug_loc
  -0.0%     -20  [ = ]       0    ../../src/modules/sensors/vehicle_imu/VehicleIMU.cpp
  +0.0%     +36  [ = ]       0    [section .debug_loc]
  +0.0%     +77  [ = ]       0    src/modules/ekf2/modules__ekf2_unity.cpp
-0.0%      -9  [ = ]       0    .debug_ranges
  -2.6%      -8  [ = ]       0    ../../src/lib/version/version.c
  -0.2%     -48  [ = ]       0    ../../src/modules/sensors/vehicle_imu/VehicleIMU.cpp
  +0.2%      +8  [ = ]       0    ../../src/modules/sensors/voted_sensors_update.cpp
  +0.0%     +40  [ = ]       0    src/modules/ekf2/modules__ekf2_unity.cpp
  -1.5%      -1  [ = ]       0    task/task_cancelpt.c
+0.1%     +48  [ = ]       0    [Unmapped]
-0.0%     -48  -0.0%     -48    .text
  +0.0%      +8  +0.0%      +8    [section .text]
  -0.2%      -8  -0.2%      -8    ../../src/modules/sensors/voted_sensors_update.cpp
  -0.0%      -8  -0.0%      -8    src/modules/ekf2/modules__ekf2_unity.cpp
  -0.1%     -16  -0.1%     -16    ../../src/modules/sensors/vehicle_imu/VehicleIMU.cpp
  -0.0%     -24  -0.0%     -24    src/modules/mavlink/modules__mavlink_unity.cpp
+0.0%    +204  -0.0%     -48    TOTAL

Updated: 2025-01-21T12:28:24

@RomanBapst RomanBapst force-pushed the pr-fix_integrator_overflow branch from 50d9cf9 to cdb9d7e Compare January 20, 2025 07:21
@RomanBapst RomanBapst force-pushed the pr-fix_integrator_overflow branch from cdb9d7e to ea780a7 Compare January 21, 2025 12:23
@RomanBapst RomanBapst merged commit f36b45b into main Jan 21, 2025
61 checks passed
@RomanBapst RomanBapst deleted the pr-fix_integrator_overflow branch January 21, 2025 13:48
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