-
Notifications
You must be signed in to change notification settings - Fork 167
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
pass node to client and service destroy funcs and enable tests for connext #67
Conversation
bbd8b09
to
df0c45d
Compare
df0c45d
to
ba9369a
Compare
if(rmw_implementation STREQUAL "rmw_opensplice_cpp") | ||
if( | ||
rmw_implementation STREQUAL "rmw_opensplice_cpp" | ||
OR rmw_implementation STREQUAL "rmw_connext_cpp" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The OR
should be trailing the previous line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, maybe this whole check should be removed now. I started these changes before FastRTPS got some upgrades. I'm testing that out right now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a different problem with FastRTPS, but I think I'll invert the logic now that FastRTPS is the only one with an issue and I'll open an issue to track the problems with FastRTPS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed the logic (596c0ec) and opened issues to track why:
@@ -44,6 +44,9 @@ | |||
# define CLASSNAME(NAME, SUFFIX) NAME | |||
#endif | |||
|
|||
bool is_connext = | |||
std::string(rmw_get_implementation_identifier()).find("connext") != std::string::npos; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this find
specific enough or should it better check for find("rmw_connext") == 0
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is good enough, but I'll change it anyways: 0d9ecc1
db13530
to
54ea614
Compare
* check allocator validity before using it * use allocator_is_valid and remove remnant from c_utilities * print to stderr only if RCUTILS_REPORT_ERROR_HANDLING_ERRORS defined * use public RCUTILS_SAFE_FWRITE_TO_STDERR
* Check if ament index resource exists before accessing
Connects to ros2/rmw_connext#168.