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

Tools: ros2: Add geopose test #25642

Merged
merged 2 commits into from
Sep 10, 2024

Conversation

Ryanf55
Copy link
Collaborator

@Ryanf55 Ryanf55 commented Nov 26, 2023

Purpose

  • Add geopose test
  • Add missing deps
  • Reduce some duplication

How to test

source /opt/ros/humble/setup.bash
colcon build --packages-up-to ardupilot_dds_tests
colcon --log-level DEBUG test --packages-select ardupilot_dds_tests --event-handlers=console_direct+

Known issue

The heading reported by ROS2 never matches CMAC's configured heading. It's not clear whether the heading configured in locations.txt is magnetic or true north, but it's more off than that.

@Ryanf55 Ryanf55 requested a review from srmainwaring November 26, 2023 16:50
@Ryanf55 Ryanf55 marked this pull request as draft November 26, 2023 16:50
@srmainwaring
Copy link
Contributor

srmainwaring commented Dec 3, 2023

Test hangs locally. When hitting ctrl+c, it will slowly kill all the tests, and then you get info:

Is it just the new test that is causing the hang, or all of them?

We already have tests covering best effort and reliable streams, so don't think that is the issue?

topic QoS
ap/time RELIABLE
ap/navsat/navsat0 BEST_EFFORT
ap/geopose/filtered BEST_EFFORT

So yea, micro_ros_agent was not found because I forgot to source, but that shouldn't cause the test to hang forever.

Ah - just saw that tacked to the end. We could add a timeout on each test, otherwise it will just wait for the agent service. Or better, add a condition that the agent be present (and a timeout).

@Ryanf55
Copy link
Collaborator Author

Ryanf55 commented Dec 4, 2023

Test hangs locally. When hitting ctrl+c, it will slowly kill all the tests, and then you get info:

Is it just the new test that is causing the hang, or all of them?

We already have tests covering best effort and reliable streams, so don't think that is the issue?

topic QoS
ap/time RELIABLE
ap/navsat/navsat0 BEST_EFFORT
ap/geopose/filtered BEST_EFFORT

So yea, micro_ros_agent was not found because I forgot to source, but that shouldn't cause the test to hang forever.

Ah - just saw that tacked to the end. We could add a timeout on each test, otherwise it will just wait for the agent service. Or better, add a condition that the agent be present (and a timeout).

Yea, a timeout would be great. Even with proper directory layout, the geopose test still fails and all the others pass. Locally, running the launch files, I get geopose data. Do you have any idea why that channel is not working when run through colcon test?

@Ryanf55 Ryanf55 added this to the DDS 4.6 milestone May 7, 2024
@Ryanf55 Ryanf55 force-pushed the dds-add-geopose-test branch from 87c8d75 to bab6179 Compare May 25, 2024 00:20
@Ryanf55
Copy link
Collaborator Author

Ryanf55 commented Jun 5, 2024

Test hangs locally. When hitting ctrl+c, it will slowly kill all the tests, and then you get info:

Is it just the new test that is causing the hang, or all of them?

We already have tests covering best effort and reliable streams, so don't think that is the issue?

topic QoS
ap/time RELIABLE
ap/navsat/navsat0 BEST_EFFORT
ap/geopose/filtered BEST_EFFORT

So yea, micro_ros_agent was not found because I forgot to source, but that shouldn't cause the test to hang forever.

Ah - just saw that tacked to the end. We could add a timeout on each test, otherwise it will just wait for the agent service. Or better, add a condition that the agent be present (and a timeout).

Any recommendations on how to implement the best approach? I'd really like to get better test coverage of this interface in CI.

@Ryanf55 Ryanf55 force-pushed the dds-add-geopose-test branch 2 times, most recently from 3321f12 to c7e6bbe Compare September 8, 2024 20:42
@Ryanf55 Ryanf55 marked this pull request as ready for review September 8, 2024 20:43
* Add missing deps
* Reduce some duplication

Signed-off-by: Ryan Friedman <[email protected]>
@Ryanf55 Ryanf55 force-pushed the dds-add-geopose-test branch from c7e6bbe to 14e2eda Compare September 8, 2024 20:47
@Ryanf55
Copy link
Collaborator Author

Ryanf55 commented Sep 8, 2024

I got the test to pass locally, there were a few typos. Instead of just checking we got a message, this also checks the position is correct.

@peterbarker peterbarker merged commit 33d75f1 into ArduPilot:master Sep 10, 2024
94 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants