Skip to content

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

@h-suzuki-isp h-suzuki-isp force-pushed the add_generic_pubsub_test branch from fb6f275 to 24f670a Compare March 21, 2024 02:35
Signed-off-by: h-suzuki-isp <[email protected]>
@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.

@christophebedard christophebedard self-requested a review March 26, 2024 00:06
Signed-off-by: h-suzuki-isp <[email protected]>
@h-suzuki-isp h-suzuki-isp force-pushed the add_generic_pubsub_test branch from f5f8f94 to ebac435 Compare March 26, 2024 01:51
@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.

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
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