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

Multicopter position control: Invalid setpoint can windup integrator #19041

Merged
merged 3 commits into from
Jan 19, 2022

Conversation

MaEtUgR
Copy link
Member

@MaEtUgR MaEtUgR commented Jan 19, 2022

Describe problem solved by this pull request
Settings a specific combination of invalid trajectory_setpoint can lead to a flyaway.
image
This was found during simulation debugging for #18988

I found that depending on the setpoint combination the velocity integrator can wind up because the calculations are ran with the invalid setpoint. Even though the results of the "invalid calculation" are not use the integrator state inside the controller is updated and leads to the problem.

Describe your solution

  1. I added a minimal unit test to reproduce the error case.
  2. I made the checks for valid inputs and combinations very explicit at the beginning and don't run any calculations if they do not pass the check.
  3. The existing tests and the new one pass.

Test data / coverage
Added unit test and manual test case in SITL with which I originally found the issue work as expected with this pr.

…ination unit test

This bug was by chance found during simulation testing and debugging.
The unit test is to easily reproduce and cover this case.
The following output instead of printing the
action "stop and wait" just once:

WARN  [mc_pos_control] invalid setpoints
WARN  [mc_pos_control] invalid setpoints
WARN  [mc_pos_control] Failsafe: stop and wait
WARN  [mc_pos_control] invalid setpoints
WARN  [mc_pos_control] Failsafe: stop and wait
WARN  [mc_pos_control] invalid setpoints
WARN  [mc_pos_control] Failsafe: stop and wait
Copy link
Member

@bresch bresch left a comment

Choose a reason for hiding this comment

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

Looks good

@MaEtUgR MaEtUgR merged commit c9f7c20 into master Jan 19, 2022
@MaEtUgR MaEtUgR deleted the invalid-setpoint-integrator-windup branch January 19, 2022 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants