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

ESC check: Avoid unsigned timestamp underflow in telemtry timeout #24069

Merged
merged 2 commits into from
Dec 3, 2024

Conversation

Perrrewi
Copy link
Contributor

@Perrrewi Perrrewi commented Dec 3, 2024

Solved Problem

During several flights a warning of ESC telemetry missing appear.

Solution

Changing the logic of the if statement to avoid unsigned int underflow.

Test coverage

Tested on a drone with QGC

Context

The tasks are prioritized and the driver has a higher priority than the commander checks. If anything delays driver, then the difference between the absolute time now and the latest published sample time from the esc status esc_status.timestamp might cause an unsigned integer underflow as the esc_status.timestamp becomes greater than now.

To test this hypothesis a sleep was introduced as below:
Screenshot from 2024-12-03 14-29-50
with following printout:
Screenshot from 2024-12-03 14-29-01

By changing the formula as in the commit, the if statement would fail ones the esc_status.timestamp stop being published, and if there is a delay the new line of code will avoid the wrapping of the negative subtraction:
Screenshot from 2024-12-03 14-30-09

Even with 1 s of sleep there was no telemetry missing message:
Screenshot from 2024-12-03 14-31-49

Copy link

github-actions bot commented Dec 3, 2024

FLASH Analysis

px4_fmu-v5x
    FILE SIZE        VM SIZE    
--------------  -------------- 
-0.0%      -1  [ = ]       0    [Unmapped]
-0.0%     -15  [ = ]       0    .debug_line
  -0.4%     -15  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/escCheck.cpp
-0.0%     -36  [ = ]       0    .debug_loc
  -0.0%      -4  [ = ]       0    [section .debug_loc]
  -0.7%     -32  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/escCheck.cpp
-0.0%     -52  [ = ]       0    TOTAL

px4_fmu-v6x
    FILE SIZE        VM SIZE    
--------------  -------------- 
-0.0%      -1  [ = ]       0    [Unmapped]
-0.0%     -15  [ = ]       0    .debug_line
  -0.4%     -15  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/escCheck.cpp
-0.0%     -36  [ = ]       0    .debug_loc
  -0.0%      -4  [ = ]       0    [section .debug_loc]
  -0.7%     -32  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/escCheck.cpp
-0.0%     -52  [ = ]       0    TOTAL

Updated: 2024-12-03T16:18:32

@MaEtUgR MaEtUgR changed the title Avoid unsigned integer underflow ESC telemtry timeout check: Avoid unsigned timestamp underflow Dec 3, 2024
@MaEtUgR MaEtUgR self-requested a review December 3, 2024 15:24
Copy link
Member

@MaEtUgR MaEtUgR left a comment

Choose a reason for hiding this comment

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

Thanks for debugging this issue 🙏

@MaEtUgR MaEtUgR changed the title ESC telemtry timeout check: Avoid unsigned timestamp underflow ESC check: Avoid unsigned timestamp underflow in telemtry timeout Dec 3, 2024
@MaEtUgR MaEtUgR merged commit dfa48f9 into main Dec 3, 2024
46 of 51 checks passed
@MaEtUgR MaEtUgR deleted the pernilla/avoid-unsigned-int-underflow branch December 3, 2024 16:14
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.

2 participants