Does it make sense to actually remove support for rmw_connext_cpp before it's also removed from the master repos? Maybe it makes more sense to add support for the new one now, and remove support for the old one when it's actually gone.

Codecov Report

Merging #253 (87dd954) into master (2b520b9) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #253   +/-   ##
=======================================
  Coverage   77.25%   77.25%           
=======================================
  Files          25       25           
  Lines         664      664           
  Branches       55       55           
=======================================
  Hits          513      513           
  Misses        131      131           
  Partials       20       20           

Flags with carried forward coverage won't be shown. Click here to find out more.

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2b520b9...87dd954. Read the comment docs.

I don't think the PR really precludes the use of rmw_connext_cpp, since all those changes are in documentation.

The code change will be needed to have tests pass once we start testing on ci.ros2.org (so far I've been testing with a modified ros2.repos).

I'm trying to coordinate all changes linked in #9 with the help of @ivanpauno and @clalancette, and I appreciate any input in making this transition as smooth as possible for everyone.

In general, I think this PR might be one of the lower priority ones from the list, so we can delay merging it until other changes have been put into place.

Does it make sense to actually remove support for rmw_connext_cpp before it's also removed from the master repos? Maybe it makes more sense to add support for the new one now, and remove support for the old one when it's actually gone.

I agree with that, but this PR doesn't seem to be removing support of rmw_connext_cpp.
It's just replacing in the tutorials rmw_connext_cpp with rmw_connextdds, and adding rmw_connextdds to the _RMW_WITH_ROS_GRAPH_INFO_TOPIC list.

We could also keep a reference to rmw_connext_cpp in the tutorials, but I don't think it's worth it.

We could also keep a reference to rmw_connext_cpp in the tutorials, but I don't think it's worth it.

If the change to the test is necessary for the switch to rmw_connextdds to move forward I'm 👍

However we should not break the tutorials by referencing not yet existing rmw implementations in the nightlies. The tutorials changes can be opened as a follow-up once rmw_connextdds is integrated and part of the artifacts produced by the buildfarm.

[ERROR] [1615413395.850699851] [rcl]: Error getting RMW implementation identifier / RMW implementation not installed (expected identifier of 'rmw_connextdds'), with error message 'failed to load shared library 'librmw_connextdds.so' due to dlopen error: librmw_connextdds.so: cannot open shared object file: No such file or directory, at /home/jenkins-agent/workspace/packaging_linux/ws/src/ros2/rcutils/src/shared_library.c:99, at /home/jenkins-agent/workspace/packaging_linux/ws/src/ros2/rmw_implementation/rmw_implementation/src/functions.cpp:74', exiting with 1., at /home/jenkins-agent/workspace/packaging_linux/ws/src/ros2/rcl/rcl/src/rcl/rmw_implementation_identifier_check.c:139

I restored rmw_connext_cpp in the tutorials. I will open a new PR to update them again once rmw_connextdds makes it into the nightly builds.

No concerns here @clalancette, thanks