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

ADSB: improve testability and small refactor #24127

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

sfuhrer
Copy link
Contributor

@sfuhrer sfuhrer commented Dec 18, 2024

Solved Problem

Some of the unit tests for ADSB failed in our fork of PX4. My best guess is that it has something to do with timing. I don't understand how it is working atm without passing the "current time" to the unit test when the ADSB functionality we're testing is depending on the exact timing of the incoming faked conflicts.

Solution

In unit tests define time of reception of new/removed conflict input.

Changelog Entry

For release notes:

Improvement: ADSB: improve testability and small refactor

@sfuhrer sfuhrer requested a review from MaEtUgR December 18, 2024 16:32
@sfuhrer
Copy link
Contributor Author

sfuhrer commented Dec 18, 2024

FYI @asimopunov

sfuhrer and others added 4 commits December 18, 2024 19:20
That enables us to time the unit tests better plus saves some flash and CPU.

Signed-off-by: Silvan Fuhrer <[email protected]>
Saves some flash and CPU by not having to call hrt_absolute_time().

Signed-off-by: Silvan Fuhrer <[email protected]>
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 looking into this. Previously the unit tests called hrt_absolute_time() on the machine that is executing the tests and hence on most runners it would just be fast enough to pass until it's not...

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