Skip to content
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

[rcl] Improve handling of dynamic discovery #1023

Merged
merged 41 commits into from Apr 8, 2023
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
1bb953e
Get discovery preferences from the environment
gbiggs Aug 2, 2022
8ecb6b3
Support specification of discovery range and static peers
gbiggs Aug 7, 2022
6d4f0e6
Add some debug helpers
gbiggs Sep 7, 2022
f557eea
Cleanup the LOCALHOST_ONLY deprecation a bit.
clalancette Oct 7, 2022
02015ff
Rewrite parsing of static peers.
clalancette Oct 7, 2022
4529112
Fix some silly bugs.
clalancette Oct 13, 2022
8b99c1f
Use names instead of integers for discovery range env vars
mxgrey Feb 6, 2023
0d117f6
Add support for dynamic allocation
arjo129 Mar 2, 2023
137ea61
Add a warning if ROS_AUTOMATIC_DISCOVERY_OFF is set and STATIC_PEERS are
arjo129 Mar 3, 2023
0bbaabd
Update to latest rmw API
mxgrey Mar 9, 2023
5225a54
Uncrustify
mxgrey Mar 9, 2023
f7d4487
Merge branch 'rolling' into gbiggs/discovery-peers-specification
sloretz Mar 24, 2023
7deb711
Update for rmw_discovery_options_t changes
sloretz Mar 28, 2023
911932f
Address feedback: use strncmp
arjo129 Mar 28, 2023
df48726
Address feedback: Log levels
arjo129 Mar 28, 2023
ac342ad
Address feedback: remove TODO
arjo129 Mar 28, 2023
ffd5000
Address feedback: rename function
arjo129 Mar 28, 2023
c3f1c77
ddress feedback: Docstring
arjo129 Mar 28, 2023
81fa1f0
Address feedback: comment
arjo129 Mar 28, 2023
211d4a6
Add `RCL_RET_ERROR` if fini fails.
arjo129 Mar 28, 2023
18c8800
`snprintf`->`rcutils_snprintf`
arjo129 Mar 28, 2023
185d5ae
Rename tests
arjo129 Mar 28, 2023
06efd9d
rcl_get_discovery_automatic_range -> rcl_get_automatic_discovery_rang…
sloretz Mar 28, 2023
1945a0c
Annotate tests
arjo129 Mar 29, 2023
4260e6f
More comments
arjo129 Mar 29, 2023
d3bb058
Style
arjo129 Mar 29, 2023
fe50db2
Style
arjo129 Mar 29, 2023
b644644
Uncrustify
arjo129 Mar 29, 2023
32e6cce
Constness and warning
arjo129 Mar 29, 2023
cd2eeb8
address TODO about IP address validation in the static peers
wjwwood Mar 29, 2023
5b568ca
NOT_SET and SYSTEM_DEFAULT values
sloretz Mar 29, 2023
a610f99
refactor discovery options to handle env vars better and simplify str…
wjwwood Mar 29, 2023
4c4248f
fixup docs
wjwwood Mar 29, 2023
12e55f0
(re)improve the discovery range debug message
wjwwood Mar 29, 2023
4101c90
Set discovery options to NOT_SET to detect user changse
sloretz Mar 31, 2023
2cb6d41
lint
sloretz Mar 31, 2023
90ba86d
Merge branch 'rolling' into gbiggs/discovery-peers-specification
sloretz Mar 31, 2023
07ad46f
strncpy_s on windows
sloretz Mar 31, 2023
bf024b8
Merge branch 'rolling' into gbiggs/discovery-peers-specification
sloretz Apr 6, 2023
ee00c56
Change default range to SUBNET, and allow configuring it at build time
sloretz Apr 6, 2023
e5e8877
Merge branch 'rolling' into gbiggs/discovery-peers-specification
sloretz Apr 7, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions rcl/CMakeLists.txt
Expand Up @@ -38,6 +38,7 @@ set(${PROJECT_NAME}_sources
src/rcl/client.c
src/rcl/common.c
src/rcl/context.c
src/rcl/discovery_options.c
src/rcl/domain_id.c
src/rcl/event.c
src/rcl/expand_topic_name.c
Expand Down
88 changes: 88 additions & 0 deletions rcl/include/rcl/discovery_options.h
@@ -0,0 +1,88 @@
// Copyright 2022 Open Source Robotics Foundation, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

/// @file

#ifndef RCL__DISCOVERY_OPTIONS_H_
#define RCL__DISCOVERY_OPTIONS_H_

#ifdef __cplusplus
extern "C"
{
#endif

#include "rcl/types.h"
#include "rcl/visibility_control.h"

#include "rcutils/allocator.h"

#include "rmw/discovery_options.h"

/// Determine how the user wishes to discover other ROS nodes automatically.
/**
* Checks an environment variable to determine how far automatic discovery should be allowed to
* propagate: not at all, the local machine only, or however far the automatic discovery mechanism
* used by the RMW implementation can propagate on the network (e.g. for multicast-based discovery,
* this will be the local subnet).
*
* \param[out] discovery_options Must not be NULL.
* \return #RCL_RET_INVALID_ARGUMENT if an argument is invalid, or
* \return #RCL_RET_ERROR if an unexpected error happened, or
* \return #RCL_RET_OK.
*/
RCL_PUBLIC
rcl_ret_t
rcl_get_discovery_automatic_range(rmw_discovery_options_t * discovery_options);

/// Convert the automatic discovery range value to a string for easy printing.
/**
* The string buffer passed to this function should be at least 40 bytes. If it is less (as
* indicated by the size parameter) the stringified enumeration value will be truncated.
wjwwood marked this conversation as resolved.
Show resolved Hide resolved
*
* \param[in] destination The string buffer to print into. Must not be NULL.
* \param[in] size Must be the size of the destination buffer, including space for a NULL byte.
* \param[in] discovery_options Must not be NULL.
* \return #RCL_RET_INVALID_ARGUMENT if an argument is invalid, or
* \return #RCL_RET_ERROR if an unexpected error happened, or
* \return #RCL_RET_OK.
*/
RCL_PUBLIC
rcl_ret_t
rcl_automatic_discovery_range_to_string(
char * destination,
size_t size,
rmw_discovery_options_t * discovery_options);
wjwwood marked this conversation as resolved.
Show resolved Hide resolved

/// Determine how the user wishes to discover other ROS nodes via statically-configured peers.
/**
* Checks an environment variable to determine the hosts that the user wants to communicate with,
* in addition to localhost.
* TODO(gbiggs): Where should the IPs be validated?
wjwwood marked this conversation as resolved.
Show resolved Hide resolved
*
* \param[out] discovery_options Must not be NULL.
* \return #RCL_RET_INVALID_ARGUMENT if an argument is invalid, or
* \return #RCL_RET_ERROR if an unexpected error happened, or
* \return #RCL_RET_OK.
*/
RCL_PUBLIC
rcl_ret_t
rcl_get_discovery_static_peers(
rmw_discovery_options_t * discovery_options,
rcutils_allocator_t * allocator);
wjwwood marked this conversation as resolved.
Show resolved Hide resolved

#ifdef __cplusplus
}
#endif

#endif // RCL__DISCOVERY_OPTIONS_H_
178 changes: 178 additions & 0 deletions rcl/src/rcl/discovery_options.c
@@ -0,0 +1,178 @@
// Copyright 2022 Open Source Robotics Foundation, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

#include "rcl/discovery_options.h"

#include <stdlib.h>
#include <string.h>

#include "rcutils/allocator.h"
#include "rcutils/env.h"
#include "rcutils/logging_macros.h"
#include "rcutils/split.h"
#include "rcutils/types/string_array.h"

#include "rcl/error_handling.h"
#include "rcl/types.h"

static const char * const RCL_STATIC_PEERS_ENV_VAR = "ROS_STATIC_PEERS";
static const char * const RCL_AUTOMATIC_DISCOVERY_RANGE_ENV_VAR = "ROS_AUTOMATIC_DISCOVERY_RANGE";

rcl_ret_t
rcl_get_discovery_automatic_range(rmw_discovery_options_t * discovery_options)
{
const char * ros_automatic_discovery_range_env_val = NULL;
const char * get_env_error_str = NULL;

RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_INVALID_ARGUMENT);
RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_ERROR);
RCL_CHECK_ARGUMENT_FOR_NULL(discovery_options, RCL_RET_INVALID_ARGUMENT);

get_env_error_str = rcutils_get_env(
RCL_AUTOMATIC_DISCOVERY_RANGE_ENV_VAR,
&ros_automatic_discovery_range_env_val);
if (NULL != get_env_error_str) {
RCL_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Error getting env var '%s': %s", RCL_AUTOMATIC_DISCOVERY_RANGE_ENV_VAR,
get_env_error_str);
return RCL_RET_ERROR;
}
if (ros_automatic_discovery_range_env_val == NULL) {
discovery_options->automatic_discovery_range = RMW_AUTOMATIC_DISCOVERY_RANGE_LOCALHOST;
} else if (strcmp(ros_automatic_discovery_range_env_val, "OFF") == 0) {
discovery_options->automatic_discovery_range = RMW_AUTOMATIC_DISCOVERY_RANGE_OFF;
} else if (strcmp(ros_automatic_discovery_range_env_val, "LOCALHOST") == 0) {
discovery_options->automatic_discovery_range = RMW_AUTOMATIC_DISCOVERY_RANGE_LOCALHOST;
} else if (strcmp(ros_automatic_discovery_range_env_val, "SUBNET") == 0) {
discovery_options->automatic_discovery_range = RMW_AUTOMATIC_DISCOVERY_RANGE_SUBNET;
} else {
RCUTILS_LOG_WARN_NAMED(
ROS_PACKAGE_NAME,
"Invalid value %s specified for '%s''; assuming localhost only",
ros_automatic_discovery_range_env_val,
RCL_AUTOMATIC_DISCOVERY_RANGE_ENV_VAR);

discovery_options->automatic_discovery_range = RMW_AUTOMATIC_DISCOVERY_RANGE_LOCALHOST;
}

return RCL_RET_OK;
}

RCL_PUBLIC
rcl_ret_t
rcl_automatic_discovery_range_to_string(
char * destination,
size_t size,
rmw_discovery_options_t * discovery_options)
{
RCL_CHECK_ARGUMENT_FOR_NULL(discovery_options, RCL_RET_INVALID_ARGUMENT);
RCL_CHECK_ARGUMENT_FOR_NULL(destination, RCL_RET_INVALID_ARGUMENT);

switch (discovery_options->automatic_discovery_range) {
case RMW_AUTOMATIC_DISCOVERY_RANGE_OFF:
snprintf(
wjwwood marked this conversation as resolved.
Show resolved Hide resolved
destination,
size,
"RMW_AUTOMATIC_DISCOVERY_RANGE_OFF (%d)",
discovery_options->automatic_discovery_range);
break;
case RMW_AUTOMATIC_DISCOVERY_RANGE_LOCALHOST:
snprintf(
destination,
size,
"RMW_AUTOMATIC_DISCOVERY_RANGE_LOCALHOST (%d)",
discovery_options->automatic_discovery_range);
break;
case RMW_AUTOMATIC_DISCOVERY_RANGE_SUBNET:
snprintf(
destination,
size,
"RMW_AUTOMATIC_DISCOVERY_RANGE_SUBNET (%d)",
discovery_options->automatic_discovery_range);
break;
case RMW_AUTOMATIC_DISCOVERY_RANGE_DEFAULT:
default:
snprintf(
destination,
size,
"RMW_AUTOMATIC_DISCOVERY_RANGE_DEFAULT (%d)",
discovery_options->automatic_discovery_range);
break;
}

return RCL_RET_OK;
}

rcl_ret_t
rcl_get_discovery_static_peers(
rmw_discovery_options_t * discovery_options,
rcutils_allocator_t * allocator)
{
const char * ros_peers_env_val = NULL;
const char * get_env_error_str = NULL;

RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_INVALID_ARGUMENT);
RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_ERROR);
RCL_CHECK_ARGUMENT_FOR_NULL(discovery_options, RCL_RET_INVALID_ARGUMENT);
RCL_CHECK_ARGUMENT_FOR_NULL(allocator, RCL_RET_INVALID_ARGUMENT);

get_env_error_str = rcutils_get_env(RCL_STATIC_PEERS_ENV_VAR, &ros_peers_env_val);
if (NULL != get_env_error_str) {
RCL_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Error getting env var '%s': %s",
RCL_STATIC_PEERS_ENV_VAR, get_env_error_str);
return RCL_RET_ERROR;
}

discovery_options->static_peers_count = 0;
if (ros_peers_env_val != NULL) {
rcutils_string_array_t array = rcutils_get_zero_initialized_string_array();
rcutils_ret_t split_ret = rcutils_split(ros_peers_env_val, ';', *allocator, &array);
if (RCUTILS_RET_OK != split_ret) {
RCL_SET_ERROR_MSG(rcutils_get_error_string().str);
return RCL_RET_ERROR;
}

discovery_options->static_peers =
allocator->zero_allocate(
array.size,
sizeof(rmw_peer_address_t),
allocator->state);

for (size_t i = 0; i < array.size; ++i) {
if (strlen(array.data[i]) > (RMW_DISCOVERY_OPTIONS_STATIC_PEERS_MAX_LENGTH - 1)) {
RCUTILS_LOG_WARN_NAMED(
ROS_PACKAGE_NAME,
"Static peer %s specified to '%s' is too long (maximum of %d); skipping",
array.data[i], RCL_STATIC_PEERS_ENV_VAR,
RMW_DISCOVERY_OPTIONS_STATIC_PEERS_MAX_LENGTH - 1);
continue;
}
strncpy(
discovery_options->static_peers[discovery_options->static_peers_count].peer_address,
array.data[i],
RMW_DISCOVERY_OPTIONS_STATIC_PEERS_MAX_LENGTH);
discovery_options->static_peers[discovery_options->static_peers_count].peer_address[
RMW_DISCOVERY_OPTIONS_STATIC_PEERS_MAX_LENGTH - 1] = '\0';
discovery_options->static_peers_count++;
}

if (RCUTILS_RET_OK != rcutils_string_array_fini(&array)) {
RCL_SET_ERROR_MSG(rcutils_get_error_string().str);
// We don't fail here because we got the work done, we will just leak memory
wjwwood marked this conversation as resolved.
Show resolved Hide resolved
}
}

return RCL_RET_OK;
}
64 changes: 63 additions & 1 deletion rcl/src/rcl/init.c
Expand Up @@ -28,6 +28,7 @@ extern "C"
#include "tracetools/tracetools.h"

#include "rcl/arguments.h"
#include "rcl/discovery_options.h"
#include "rcl/domain_id.h"
#include "rcl/error_handling.h"
#include "rcl/localhost.h"
Expand Down Expand Up @@ -155,13 +156,74 @@ rcl_init(

rmw_localhost_only_t * localhost_only =
&context->impl->init_options.impl->rmw_init_options.localhost_only;
if (RMW_LOCALHOST_ONLY_DEFAULT == *localhost_only) {
if (RMW_LOCALHOST_ONLY_DEFAULT != *localhost_only) {
RCUTILS_LOG_WARN_NAMED(
ROS_PACKAGE_NAME,
"'localhost_only' init option is deprecated. Use 'automatic_discovery_range' and "
"'static_peers' instead.");
} else {
// Get actual localhost_only value based on environment variable, if needed.
ret = rcl_get_localhost_only(localhost_only);
if (RCL_RET_OK != ret) {
fail_ret = ret;
goto fail;
}
if (RMW_LOCALHOST_ONLY_DEFAULT != *localhost_only) {
RCUTILS_LOG_WARN_NAMED(
ROS_PACKAGE_NAME,
"ROS_LOCALHOST_ONLY is deprecated. Use ROS_AUTOMATIC_DISCOVERY_RANGE_DEFAULT and "
"ROS_STATIC_PEERS instead.");
}
}

rmw_discovery_options_t * discovery_options =
&context->impl->init_options.impl->rmw_init_options.discovery_options;
if (RMW_AUTOMATIC_DISCOVERY_RANGE_DEFAULT == discovery_options->automatic_discovery_range) {
// Get actual multicast discovery value based on environment variable, if needed
wjwwood marked this conversation as resolved.
Show resolved Hide resolved
ret = rcl_get_discovery_automatic_range(discovery_options);
wjwwood marked this conversation as resolved.
Show resolved Hide resolved
if (RCL_RET_OK != ret) {
fail_ret = ret;
goto fail;
}
}

if (0 == discovery_options->static_peers_count &&
discovery_options->automatic_discovery_range != RMW_AUTOMATIC_DISCOVERY_RANGE_OFF)
{
// Get static peers.
// If off is set, it makes sense to not get any static peers.
ret = rcl_get_discovery_static_peers(discovery_options, &allocator);
if (RCL_RET_OK != ret) {
fail_ret = ret;
goto fail;
}
}

if (discovery_options->static_peers_count > 0 &&
discovery_options->automatic_discovery_range == RMW_AUTOMATIC_DISCOVERY_RANGE_OFF)
{
RCUTILS_LOG_WARN_NAMED(
ROS_PACKAGE_NAME,
"Note: ROS_AUTOMATIC_DISCOVERY_RANGE is set to OFF, but "
"found static peers in ROS_STATIC_PEERS. "
"Your settings ROS_STATIC_PEERS will be ignored.");
wjwwood marked this conversation as resolved.
Show resolved Hide resolved
}

char discovery_range_string[41]; // TODO(gbiggs) Is constexpr available in C11?
wjwwood marked this conversation as resolved.
Show resolved Hide resolved
rcl_automatic_discovery_range_to_string(discovery_range_string, 41, discovery_options);
RCUTILS_LOG_INFO_NAMED(
ROS_PACKAGE_NAME,
"Automatic discovery range is %s",
discovery_range_string);
RCUTILS_LOG_INFO_NAMED(
ROS_PACKAGE_NAME,
"Static peers count is %lu",
discovery_options->static_peers_count);

for (size_t ii = 0; ii < discovery_options->static_peers_count; ++ii) {
RCUTILS_LOG_INFO_NAMED(
ROS_PACKAGE_NAME,
"\t%s", discovery_options->static_peers[ii].peer_address);
wjwwood marked this conversation as resolved.
Show resolved Hide resolved
}

if (context->global_arguments.impl->enclave) {
Expand Down
10 changes: 6 additions & 4 deletions rcl/src/rcl/localhost.c
Expand Up @@ -41,9 +41,11 @@ rcl_get_localhost_only(rmw_localhost_only_t * localhost_only)
get_env_error_str);
return RCL_RET_ERROR;
}
*localhost_only = (ros_local_host_env_val != NULL &&
strcmp(
ros_local_host_env_val,
"1") == 0) ? RMW_LOCALHOST_ONLY_ENABLED : RMW_LOCALHOST_ONLY_DISABLED;
if (ros_local_host_env_val == NULL || ros_local_host_env_val[0] == '\0') {
*localhost_only = RMW_LOCALHOST_ONLY_DEFAULT;
} else {
*localhost_only = strcmp(
ros_local_host_env_val, "1") == 0 ? RMW_LOCALHOST_ONLY_ENABLED : RMW_LOCALHOST_ONLY_DISABLED;
wjwwood marked this conversation as resolved.
Show resolved Hide resolved
}
return RCL_RET_OK;
}
6 changes: 6 additions & 0 deletions rcl/test/CMakeLists.txt
Expand Up @@ -398,6 +398,12 @@ rcl_add_custom_gtest(test_validate_enclave_name
LIBRARIES ${PROJECT_NAME} mimick
)

rcl_add_custom_gtest(test_discovery_options
SRCS rcl/test_discovery_options.cpp
APPEND_LIBRARY_DIRS ${extra_lib_dirs}
LIBRARIES ${PROJECT_NAME}
)

rcl_add_custom_gtest(test_domain_id
SRCS rcl/test_domain_id.cpp
APPEND_LIBRARY_DIRS ${extra_lib_dirs}
Expand Down