match rmw_fastrtps_dynamic_cpp / rmw_connext_dynamic_cpp #288
Conversation
There was a problem hiding this comment.
I guess as long as it passes CI, my question is moot and could be followed up on after merging this if @mikaelarguedas has new information about it.
| # TODO(mikaelarguedas) only Connext and FastRTPS support DDS-Security for now | ||
| if( | ||
| rmw_implementation STREQUAL "rmw_connext_cpp" OR | ||
| rmw_implementation STREQUAL "rmw_connext_dynamic_cpp" OR |
There was a problem hiding this comment.
Does connext dynamic support security? I suppose so, but I could imagine that it doesn't for some reason. @mikaelarguedas might be the best to ask about this.
There was a problem hiding this comment.
Until now the only rmw FastRTPS implementation was dynamic. Since it supported security until now the newly named rmw impl. should do the same (if nothing was broken during the refactoring).
There was a problem hiding this comment.
I don't understand. You added rmw_connext_dynamic_cpp where it wasn't there before. I don't see what it has to do with the FastRTPS refactor.
There was a problem hiding this comment.
I mistakenly was referring to the addition of FastRTPS dynamic here. I don't have any specific information about Connext dynamic and if it would support security. I would assume so but that is not based on any testing. Since Connext dynamic is currently disabled with an IGNORE in the repository the change shouldn't be a problem.
If @mikaelarguedas comments that it is not supported I am happy to remove it for the condition again.
There was a problem hiding this comment.
As Connext dynamic was already not supported and not able to send nested messages at the time support for DDS security was added, I never enabled the tests for connext_dynamic.
The change was made to be compatible with connext_dynamic as well as static as all the core of changes live in the connext shared layer and the API (static and dynamic) have been adapted accordingly: https://github.com/ros2/rmw_connext/pull/225/files.
In conclusion, I have never tested Security with connext dynamic, but as all the security code is shared between connext static and connext dynamic it "should work" if the rest of the code works.
Related to ros2/rmw_fastrtps#218.