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

ROS integration test updates #2970

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

ROS integration test updates #2970

wants to merge 9 commits into from

Conversation

bkueng
Copy link
Member

@bkueng bkueng commented Dec 11, 2023

Doc updates for

We can also move the library integration tests into a separate page under test_and_ci.

@hamishwillee
Copy link
Collaborator

We can also move the library integration tests into a separate page under test_and_ci.

I think that makes sense, and cross link.
The doc also needs more clarity on how it works in CI, and links to the tests in github.
My assumption (?) is that eventually this will become the infrastructure to write tests for PX4 systems that use both a companion computer with ROS and PX4 - in other words I write a new mode that does stuff, this allows me to test it. Is that valid?

I think we need these to have a parent doc "Integration Testing". Thoughts?

@bkueng
Copy link
Member Author

bkueng commented Dec 13, 2023

My assumption (?) is that eventually this will become the infrastructure to write tests for PX4 systems that use both a companion computer with ROS and PX4 - in other words I write a new mode that does stuff, this allows me to test it. Is that valid?

Yes it could be. How that would look like exactly isn't clear to me either though. Important will be that the tests run fast enough so we can have many.

@hamishwillee
Copy link
Collaborator

@bkueng I've added structure to the integration tests part as discussed, moving the ROS1/MAVROS based tests into a new doc and clearly marking as deprecated.

We really do need that "Integrated PX4 and Companion" test story to be fleshed out. Fortunately I'm only here for my good looks, so someone who knows what they are doing will have to sort that out.

Anyway, can you please sanity check this and if you're OK with it we can merge when your autopilot PR goes in.

Copy link
Member Author

@bkueng bkueng left a comment

Choose a reason for hiding this comment

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

We really do need that "Integrated PX4 and Companion" test story to be fleshed out. Fortunately I'm only here for my good looks, so someone who knows what they are doing will have to sort that out.

You're just a bit too quick, it will happen eventually (I hope :). We barely even have some setpoints defined, and even less modes.

en/test_and_ci/README.md Outdated Show resolved Hide resolved
en/SUMMARY.md Outdated Show resolved Hide resolved
@hamishwillee
Copy link
Collaborator

Thanks. Ready when you are!

Copy link

No flaws found

@hamishwillee
Copy link
Collaborator

@bkueng ANy plans to merge the PR this belongs to?

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