Add get_actual_qos() for subscriptions

[mm318]

Currently, publishers have the get_actual_qos() feature/function. It would make sense for subscriptions to have them, too.

Depends on ros2/rmw_implementation#61
Depends on ros2/rmw_fastrtps#287
Depends on ros2/rmw_connext#358
Depends on ros2/rmw_opensplice#271
Blocks ros2/rcl#455
Blocks ros2/rclcpp#754

[mm318]

@emersonknapp - please run the following CI job:

• Gist file: https://gist.githubusercontent.com/mm318/c119e49fca046fce6a3ec74552ac91a3/raw/a621734acbed40a8d21fcfb0b6357f068370a624/ros2.repos
• Additional BUILD flags: --packages-up-to rclcpp
• Additional TEST flags: --packages-up-to rclcpp
• CI Job: ci_launcher

Please also enable the use of OpenSplice.

[emersonknapp]

• Linux
• Linux-aarch64
• macOS
• Windows

[mm318]

Hi @emersonknapp, can you please re-run the CI when you get a chance? Thanks!

[mm318]

Hi @ivanpauno, it looks like we don't have anybody on our side today who can start a CI run for us. Can you please kick off a CI run for this PR? Thanks!

[ivanpauno]

• Linux
• Linux-aarch64
• macOS
• Windows

[ivanpauno]

rmw_connext is failing to build because of a visibility problem (fastrtps and opensplice were built ok).

The error is:
09:51:07      Creating library C:/J/workspace/ci_windows/ws/build/rmw_connext_cpp/Release/rmw_connext_cpp.lib and object C:/J/workspace/ci_windows/ws/build/rmw_connext_cpp/Release/rmw_connext_cpp.exp
09:51:07 rmw_publisher.obj : error LNK2019: unresolved external symbol "void __cdecl dds_qos_to_rmw_qos<struct DDS_DataWriterQos>(struct DDS_DataWriterQos const &,struct rmw_qos_profile_t *)" (??$dds_qos_to_rmw_qos@UDDS_DataWriterQos@@@@YAXAEBUDDS_DataWriterQos@@PEAUrmw_qos_profile_t@@@Z) referenced in function rmw_publisher_get_actual_qos [C:\J\workspace\ci_windows\ws\build\rmw_connext_cpp\rmw_connext_cpp.vcxproj]
09:51:07 rmw_subscription.obj : error LNK2019: unresolved external symbol "void __cdecl dds_qos_to_rmw_qos<struct DDS_DataReaderQos>(struct DDS_DataReaderQos const &,struct rmw_qos_profile_t *)" (??$dds_qos_to_rmw_qos@UDDS_DataReaderQos@@@@YAXAEBUDDS_DataReaderQos@@PEAUrmw_qos_profile_t@@@Z) referenced in function rmw_subscription_get_actual_qos [C:\J\workspace\ci_windows\ws\build\rmw_connext_cpp\rmw_connext_cpp.vcxproj]
09:51:07 C:\J\workspace\ci_windows\ws\build\rmw_connext_cpp\Release\rmw_connext_cpp.dll : fatal error LNK1120: 2 unresolved externals [C:\J\workspace\ci_windows\ws\build\rmw_connext_cpp\rmw_connext_cpp.vcxproj]

I guess that when you explicitly specialize a template you have to use the visibility macros (RMW_CONNEXT_SHARED_CPP_PUBLIC in this case):
https://github.com/aws-ros-dev/rmw_connext/blob/2fc13d6968d85160dd4568c0da1ecb9c8e8a607b/rmw_connext_shared_cpp/src/qos.cpp#L309-L315

I'm not pretty sure if that will solve the problem.
In opensplice the problem didn't appear, because the function was part of the same library.

PS: If you don't have a windows environment available, let me know and I will try to solve the problem.

[ivanpauno]

I tested it on windows, the correct way of doing it is:

template RMW_CONNEXT_SHARED_CPP_PUBLIC
void dds_qos_to_rmw_qos<DDS::DataWriterQos>(
  const DDS::DataWriterQos & dds_qos,
  rmw_qos_profile_t * qos);

The visibility macro has to be after the template keyword (see this).

Note: rmw_fastrtps_shared_cpp is built as an static library, so the problem didn't appear there. rmw_connext_shared_cpp is built as a shared library.

[mm318]

So RMW_CONNEXT_SHARED_CPP_PUBLIC needs to go in the .cpp file as opposed to the .hpp file? I think I'll put it in the .hpp file as well to be consistent with the rest of the other function declarations.

[ivanpauno]

So RMW_CONNEXT_SHARED_CPP_PUBLIC needs to go in the .cpp file as opposed to the .hpp file? I think I'll put it in the .hpp file as well to be consistent with the rest of the other function declarations.

I think that visibility macros can only by used in the template specializations (and not in the template itself). And the template specialization should stay in the cpp file. I don't know if a workaround is possible.

[mm318]

Putting the visibility macro on the explicit template instantiation seemed to have solved the linker error. Thank you!

Putting the visibility macro on the template declaration in the .hpp file didn't seem to hurt either, no errors or warnings. Is it okay to keep it there?

[ivanpauno]

Putting the visibility macro on the explicit template instantiation seemed to have solved the linker error. Thank you!

Putting the visibility macro on the template declaration in the .hpp file didn't seem to hurt either, no errors or warnings. Is it okay to keep it there?

If it doesn't do nothing, I prefer not adding it.

The example 2 of the link I posted before is a good alternative. Add the following to the .hpp:

extern template RMW_CONNEXT_SHARED_CPP_PUBLIC
void dds_qos_to_rmw_qos<DDS::DataWriterQos>(
  const DDS::DataWriterQos & dds_qos,
  rmw_qos_profile_t * qos);

extern template RMW_CONNEXT_SHARED_CPP_PUBLIC
void dds_qos_to_rmw_qos<DDS::DataReaderQos>(
  const DDS::DataReaderQos & dds_qos,
  rmw_qos_profile_t * qos);

That's more explicit, as you know what template specializations are available, and the visibility macro is only needed in the header.

Edit: The Example 2 wasn't exactly that. I tested it, and it works.

[mm318]

@ivanpauno, I've updated the other pull requests with your suggestion. Can you please kick off a CI run again? Thanks!

[ivanpauno]

• Linux
• Linux-aarch64
• macOS
• Windows

[mm318]

Thanks!