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.

Sign up for GitHub

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

Jump to bottom

Use a uniform distribution to drop messages in lossy test #1827

Merged
merged 2 commits into from
Jun 30, 2022
Merged

Use a uniform distribution to drop messages in lossy test #1827

merged 2 commits into from
Jun 30, 2022

Conversation

JonasVautherin
Copy link
Collaborator

This test seems flaky to me in the CI, and I can't help but to think that maybe, sometimes, it gets into a state where dropping exactly every tenth message make it fail.

I changed it for a uniform distribution, which seems deterministic when running on my computer (not sure what the default seed is), but at least it's dropping 10% of the messages, and not exactly every tenth message.

Without this PR (I see a lot of 73):

[12:34:32|Debug] Dropped outgoing message: 44 (mavsdk_impl.cpp:359)
[12:34:32|Debug] Dropped incoming message: 51 (mavsdk_impl.cpp:251)
[12:34:33|Debug] Dropped outgoing message: 73 (mavsdk_impl.cpp:359)
[12:34:33|Debug] Dropped outgoing message: 73 (mavsdk_impl.cpp:359)
[12:34:33|Debug] Dropped outgoing message: 73 (mavsdk_impl.cpp:359)
[12:34:34|Debug] Dropped outgoing message: 73 (mavsdk_impl.cpp:359)
[12:34:34|Debug] Dropped outgoing message: 73 (mavsdk_impl.cpp:359)
[12:34:34|Debug] Dropped outgoing message: 73 (mavsdk_impl.cpp:359)
[12:34:34|Debug] Dropped outgoing message: 73 (mavsdk_impl.cpp:359)
[12:34:35|Debug] Dropped outgoing message: 73 (mavsdk_impl.cpp:359)
[12:34:35|Debug] Dropped incoming message: 44 (mavsdk_impl.cpp:251)
[12:34:35|Debug] Dropped incoming message: 73 (mavsdk_impl.cpp:251)
[12:34:36|Debug] Dropped incoming message: 73 (mavsdk_impl.cpp:251)
[12:34:36|Debug] Dropped incoming message: 73 (mavsdk_impl.cpp:251)
[12:34:37|Debug] Dropped incoming message: 73 (mavsdk_impl.cpp:251)
[12:34:37|Debug] Dropped incoming message: 73 (mavsdk_impl.cpp:251)
[12:34:38|Debug] Dropped incoming message: 73 (mavsdk_impl.cpp:251)
[12:34:38|Debug] Dropped incoming message: 73 (mavsdk_impl.cpp:251)
[12:34:39|Debug] Dropped incoming message: 73 (mavsdk_impl.cpp:251)
[12:34:39|Debug] Dropped incoming message: 73 (mavsdk_impl.cpp:251)
[12:34:40|Debug] Dropped incoming message: 73 (mavsdk_impl.cpp:251)

With this PR:

[12:33:55|Debug] Dropped outgoing message: 73 (mavsdk_impl.cpp:359)
[12:33:55|Debug] Dropped incoming message: 51 (mavsdk_impl.cpp:251)
[12:33:55|Debug] Dropped incoming message: 51 (mavsdk_impl.cpp:251)
[12:33:56|Debug] Dropped outgoing message: 73 (mavsdk_impl.cpp:359)
[12:33:56|Debug] Dropped outgoing message: 73 (mavsdk_impl.cpp:359)
[12:33:56|Debug] Dropped incoming message: 51 (mavsdk_impl.cpp:251)
[12:33:57|Debug] Dropped incoming message: 51 (mavsdk_impl.cpp:251)
[12:33:57|Debug] Dropped outgoing message: 43 (mavsdk_impl.cpp:359)
[12:33:57|Debug] Dropped incoming message: 44 (mavsdk_impl.cpp:251)
[12:33:58|Debug] Dropped incoming message: 73 (mavsdk_impl.cpp:251)
[12:33:58|Debug] Dropped outgoing message: 51 (mavsdk_impl.cpp:359)
[12:33:59|Debug] Dropped outgoing message: 51 (mavsdk_impl.cpp:359)
[12:33:59|Debug] Dropped incoming message: 73 (mavsdk_impl.cpp:251)
[12:34:00|Debug] Dropped incoming message: 73 (mavsdk_impl.cpp:251)
[12:34:00|Debug] Dropped outgoing message: 51 (mavsdk_impl.cpp:359)
[12:34:01|Debug] Dropped outgoing message: 51 (mavsdk_impl.cpp:359)
[12:34:01|Debug] Dropped outgoing message: 51 (mavsdk_impl.cpp:359)
[12:34:02|Debug] Dropped outgoing message: 51 (mavsdk_impl.cpp:359)

I'd say let's see how it goes in the CI.

@JonasVautherin JonasVautherin requested a review from julianoes June 28, 2022 22:41
@JonasVautherin
Copy link
Collaborator Author

How ironic, now they seem to fail more consistently 🤣:

 [11:17:28|Debug] Falling back to gimbal protocol v1 (mission_impl.cpp:124)
[11:17:28|Debug] Dropped incoming message: 51 (mavsdk_impl.cpp:251)
[11:17:28|Debug] Dropped incoming message: 51 (mavsdk_impl.cpp:251)
[11:17:28|Debug] Dropped incoming message: 51 (mavsdk_impl.cpp:251)
[11:17:28|Debug] Dropped outgoing message: 73 (mavsdk_impl.cpp:359)
[11:17:28|Debug] Dropped outgoing message: 73 (mavsdk_impl.cpp:359)
[11:17:28|Debug] Dropped incoming message: 51 (mavsdk_impl.cpp:251)
[11:17:28|Debug] Dropped incoming message: 51 (mavsdk_impl.cpp:251)
[11:17:28|Debug] Dropped outgoing message: 43 (mavsdk_impl.cpp:359)
[11:17:29|Debug] Dropped incoming message: 44 (mavsdk_impl.cpp:251)
[11:17:29|Debug] Dropped incoming message: 73 (mavsdk_impl.cpp:251)
[11:17:30|Debug] Dropped outgoing message: 51 (mavsdk_impl.cpp:359)
[11:17:30|Debug] Dropped outgoing message: 51 (mavsdk_impl.cpp:359)
[11:17:31|Debug] Dropped incoming message: 73 (mavsdk_impl.cpp:251)
[11:17:31|Debug] Dropped incoming message: 73 (mavsdk_impl.cpp:251)
[11:17:32|Debug] Dropped outgoing message: 51 (mavsdk_impl.cpp:359)
[11:17:32|Debug] Dropped outgoing message: 51 (mavsdk_impl.cpp:359)
[11:17:33|Debug] MAVLink: critical: Operation timeout (system_impl.cpp:247)
/__w/MAVSDK/MAVSDK/src/integration_tests/mission_transfer_lossy.cpp:53: Failure
Expected equality of these values:
  result.first
    Which is: Timeout
  Mission::Result::Success
    Which is: Success

julianoes
julianoes reviewed Jun 30, 2022
View reviewed changes
Copy link
Collaborator

@julianoes julianoes left a comment •
edited
Loading

Choose a reason for hiding this comment

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

That's funny! Thanks for that. Any chance we could fix the seed to avoid flaky CI tests?

Nevermind, a fixed seed still doesn't make it reproducible as timing and thus ordering of messages can differ between CI runs.

julianoes
julianoes approved these changes Jun 30, 2022
View reviewed changes
Copy link
Collaborator

@julianoes julianoes left a comment

Choose a reason for hiding this comment

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

Nice!

@julianoes julianoes merged commit 9d65f21 into main Jun 30, 2022
@julianoes julianoes deleted the improve-mission-transfer-lossy branch June 30, 2022 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@julianoes julianoes julianoes approved these changes

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@JonasVautherin @julianoes