Instead of requiring rclcpp to handle this case opensplice_cmake_module could simply provide an environment hook which ensures that the domain id is set for OpenSplice.

@dirk-thomas Someone could also unset ROS_DOMAIN_ID. In which case connext would work, defaulting to 0 and OpenSplice would not.

Someone could also unset OSPL_URI after sourcing the environment and that would also break. I guess there is always a way to break the system if you try 😉

That's OpenSplice specific, the ROS_DOMAIN_ID is ROS specific and having it set or not set should result in the same behavior, regardless of the implementation being used. So I think the argument that you can break the system if you try is not an argument which applies to this pr.

As to where we ensure the ROS_DOMAIN_ID is set for OpenSplice, I put it in rclcpp because we read the ROS_DOMAIN_ID in rclcpp. I can look at moving it to the rmw_opensplice implementation of create_node.

I moved it to rmw_opensplice_cpp, the benefit being that I can scope the use of this more tightly. So now if the ROS_DOMAIN_ID is not set, it sets it, creates the participant factory, and then unsets it again. If the ROS_DOMAIN_ID is set, but different from the domain_id that was passed in (something we might want to expose through rclcpp in the future), then it sets it to what was passed in, storing the current value, and restores it after it finishes creating the domain participant factory.

I still need to test this locally.

I also added a check to see if the OSPL_URI is set, and returns a relevant error message if it is not. Previously if OSPL_URI was not set you just get "failed to create participant".

I've tested it locally:

william@farl ~
% bash
william@farl:~
$ source ./ros2/install/setup.bash
william@farl:~
$ talker__rmw_opensplice_cpp
Publishing: 'Hello World: 1'
Publishing: 'Hello World: 2'
Publishing: 'Hello World: 3'
Publishing: 'Hello World: 4'
Publishing: 'Hello World: 5'
^Csignal_handler(2)
william@farl:~
$ ROS_DOMAIN_ID=1 talker__rmw_opensplice_cpp
Publishing: 'Hello World: 1'
Publishing: 'Hello World: 2'
Publishing: 'Hello World: 3'
Publishing: 'Hello World: 4'
Publishing: 'Hello World: 5'
^Csignal_handler(2)
william@farl:~
$ exit

william@farl ~
% bash
william@farl:~
$ source ros2/install/setup.bash
william@farl:~
$ listener__rmw_opensplice_cpp
I heard: [Hello World: 1]
I heard: [Hello World: 2]
I heard: [Hello World: 3]
I heard: [Hello World: 4]
I heard: [Hello World: 5]
^Csignal_handler(2)
william@farl:~
$ ROS_DOMAIN_ID=2 listener__rmw_opensplice_cpp
^Csignal_handler(2)
william@farl:~
$ exit

I still need to test it manually on Windows, but the build is passing on Windows: http://ci.ros2.org/job/ros2_batch_ci_windows/377/

It has test failures, but I'll have to dig into them to know if that's expected or not.

This works on Linux, but is still not working correctly on Windows, I'll look into it today and make a decision about its feasibility as a fix on Windows.

I've given up on getting it to work around on Windows, as I do on the other platforms. So now the Windows env hook sets the ROS_DOMAIN_ID is not set to 0 and errors out if it is not set when you run the program.

I'll do uncrustify and squash after lunch, CI for now:

• http://ci.ros2.org/job/ros2_batch_ci_linux/367/
• http://ci.ros2.org/job/ros2_batch_ci_osx/274/
• http://ci.ros2.org/job/ros2_batch_ci_windows/410/

CI looks good. The test failures on Windows seem to be unrelated. I just finished manually running the examples on Windows as well, setting, changing, and unsetting the ROS_DOMAIN_ID in turn.

I think this is ready for review. I'll squash after review.

LGTM

I "fixed" the indentation level of the xml file from 3 to 4 spaces since that is the format osplconf uses when writing files: 44278bd