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

Add test for GenericPublisher/Subscriber #97

Merged
merged 10 commits into from Apr 5, 2024

Conversation

h-suzuki-isp
Copy link
Contributor

@h-suzuki-isp h-suzuki-isp commented Mar 18, 2024

Please, refer to ros2/rclcpp#2448.

Because I added tracepoint for GenericPublisher and GenericSubscriber, I add some tests to test_tracetools.
I have added the following tests.

  • src/
    • test_generic_ping.cpp
    • test_generic_pong.cpp
  • test/
    • test_generic_pub_sub.py
    • test_generic_subscription.py

action-ros-ci-repos-override: https://gist.githubusercontent.com/christophebedard/9a37e393935913add5ad490aaee26e7f/raw/194c464b68cd138e887f041f50f42c0cbe3f4710/ros2.repos

Signed-off-by: h-suzuki <h-suzuki@isp.co.jp>
Signed-off-by: h-suzuki <h-suzuki@isp.co.jp>
Signed-off-by: h-suzuki <h-suzuki@isp.co.jp>
Signed-off-by: h-suzuki <h-suzuki@isp.co.jp>
Signed-off-by: h-suzuki-isp <h-suzuki@isp.co.jp>
@christophebedard
Copy link
Member

christophebedard commented Mar 26, 2024

The Rpr__ros2_tracing__ubuntu_noble_amd64 job failure is expected due to the transition to Ubuntu Noble.

The reason why the ros2_tracing / test (ubuntu-22.04, rolling, source, instr-enabled, tp-included) job fails is because only this PR branch gets built; your rclcpp/rcl/rmw_* branches are not used. I've added a repos file to this PR description so that it runs with your other changes. Can you do git commit --amend --no-edit and then force push so that CI runs again? The tests seem to successfully pass locally for me when I use all the relevant branches.

Signed-off-by: h-suzuki-isp <h-suzuki@isp.co.jp>
@h-suzuki-isp
Copy link
Contributor Author

@christophebedard
Thank you for your help.
As a result of running CI again, the test pass successfully.

@christophebedard
Copy link
Member

It looks good to me, but I'm just trying to run the tests with the other rmw implementations too. For some reason I can't get RMW_IMPLEMENTATION=rmw_connextdds to work.

@christophebedard
Copy link
Member

It looks good to me, but I'm just trying to run the tests with the other rmw implementations too. For some reason I can't get RMW_IMPLEMENTATION=rmw_connextdds to work.

Oops, I didn't have rti-connext-dds-6.0.1 installed on my system, that's probably why.

h-suzuki-isp and others added 4 commits March 28, 2024 11:30
Co-authored-by: Christophe Bedard <bedard.christophe@gmail.com>
Signed-off-by: h-suzuki-isp <146712054+h-suzuki-isp@users.noreply.github.com>
Signed-off-by: h-suzuki-isp <h-suzuki@isp.co.jp>
Signed-off-by: h-suzuki-isp <h-suzuki@isp.co.jp>
Signed-off-by: h-suzuki-isp <h-suzuki@isp.co.jp>
Copy link
Member

@christophebedard christophebedard 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 iterating!

Let's now wait for full CI (under the rclcpp PR) and perhaps some final reviews for the changes to the rmw implementations.

@christophebedard
Copy link
Member

Alright, all other PRs have been merged, so I'll merge this one. See ros2/rclcpp#2448 (comment) for CI.

Thank you for the contribution @h-suzuki-isp!

@christophebedard christophebedard merged commit a689891 into ros2:rolling Apr 5, 2024
8 of 9 checks passed
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.

None yet

2 participants