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

Support passing yaml parameter files via commandline #253

Merged
merged 18 commits into from
Jun 5, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions rcl/include/rcl/arguments.h
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,55 @@ rcl_arguments_get_unparsed(
rcl_allocator_t allocator,
int ** output_unparsed_indices);

/// Return the number of parameter yaml files given in the arguments.
/**
* <hr>
* Attribute | Adherence
* ------------------ | -------------
* Allocates Memory | No
* Thread-Safe | No
* Uses Atomics | No
* Lock-Free | Yes
*
* \param[in] args An arguments structure that has been parsed.
* \return number of yaml files, or
* \return -1 if args is `NULL` or zero initialized.
*/
RCL_PUBLIC
RCL_WARN_UNUSED
int
rcl_arguments_get_param_files_count(
const rcl_arguments_t * args);


/// Return a list of yaml parameter file paths specified on the command line.
/**
* <hr>
* Attribute | Adherence
* ------------------ | -------------
* Allocates Memory | Yes
* Thread-Safe | No
* Uses Atomics | No
* Lock-Free | Yes
*
* \param[in] arguments An arguments structure that has been parsed.
* \param[in] allocator A valid allocator.
* \param[out] parameter_files An allocated array of paramter file names.
* This array must be deallocated by the caller using the given allocator.
* The output is NULL if there were no paramter files.
* \return `RCL_RET_OK` if everything goes correctly, or
* \return `RCL_RET_INVALID_ARGUMENT` if any function arguments are invalid, or
* \return `RCL_RET_BAD_ALLOC` if allocating memory failed, or
* \return `RCL_RET_ERROR` if an unspecified error occurs.
*/
RCL_PUBLIC
RCL_WARN_UNUSED
rcl_ret_t
rcl_arguments_get_param_files(
const rcl_arguments_t * arguments,
rcl_allocator_t allocator,
char *** parameter_files);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like you should unlock an achievement for using ***. But, how do you know how long parameter_files is after calling?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never mind, I guess it always matches the arguments->impl->num_param_files_args.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

feel like you should unlock an achievement for using ***

Haha, achievement goes to @sloretz :

const char ** nonros_argv[]);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


/// Return a list of arguments with ROS-specific arguments removed.
/**
* Some arguments may not have been intended as ROS arguments.
Expand Down
2 changes: 2 additions & 0 deletions rcl/include/rcl/types.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,5 +92,7 @@ typedef rmw_ret_t rcl_ret_t;
#define RCL_RET_INVALID_REMAP_RULE 1001
/// Expected one type of lexeme but got another
#define RCL_RET_WRONG_LEXEME 1002
/// Argument is not a valid parameter rule
#define RCL_RET_INVALID_PARAM_RULE 1010

#endif // RCL__TYPES_H_
135 changes: 134 additions & 1 deletion rcl/src/rcl/arguments.c
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,53 @@ _rcl_parse_remap_rule(
rcl_allocator_t allocator,
rcl_remap_t * output_rule);

RCL_LOCAL
rcl_ret_t
_rcl_parse_param_rule(
const char * arg,
rcl_allocator_t allocator,
char ** output_rule);

rcl_ret_t
rcl_arguments_get_param_files(
const rcl_arguments_t * arguments,
rcl_allocator_t allocator,
char *** parameter_files)
{
RCL_CHECK_ALLOCATOR_WITH_MSG(&allocator, "invalid allocator", return RCL_RET_INVALID_ARGUMENT);
RCL_CHECK_ARGUMENT_FOR_NULL(arguments, RCL_RET_INVALID_ARGUMENT, allocator);
RCL_CHECK_ARGUMENT_FOR_NULL(arguments->impl, RCL_RET_INVALID_ARGUMENT, allocator);
RCL_CHECK_ARGUMENT_FOR_NULL(parameter_files, RCL_RET_INVALID_ARGUMENT, allocator);
*(parameter_files) = allocator.allocate(
sizeof(char *) * arguments->impl->num_param_files_args, allocator.state);
if (NULL == *parameter_files) {
return RCL_RET_BAD_ALLOC;
}
for (int i = 0; i < arguments->impl->num_param_files_args; ++i) {
(*parameter_files)[i] = rcutils_strdup(arguments->impl->parameter_files[i], allocator);
if (NULL == *parameter_files) {
// deallocate allocated memory
for (int r = i; r >= 0; --r) {
allocator.deallocate((*parameter_files[i]), allocator.state);
}
allocator.deallocate((*parameter_files), allocator.state);
(*parameter_files) = NULL;
return RCL_RET_BAD_ALLOC;
}
}
return RCL_RET_OK;
}

int
rcl_arguments_get_param_files_count(
const rcl_arguments_t * args)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: Arguments don't need to be wrapped here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for declaration in header. Those I suppose you're just trying to always wrap or not, so maybe this is fine, just looked weird at a glance.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair point. TBH I didn't look twice and copied an existing function from that file.

int
rcl_arguments_get_count_unparsed(
const rcl_arguments_t * args);

It may be worth a follow up to clean it in the entire file.

{
if (NULL == args || NULL == args->impl) {
return -1;
}
return args->impl->num_param_files_args;
}

rcl_ret_t
rcl_parse_arguments(
int argc,
Expand Down Expand Up @@ -84,6 +131,8 @@ rcl_parse_arguments(
args_impl->remap_rules = NULL;
args_impl->unparsed_args = NULL;
args_impl->num_unparsed_args = 0;
args_impl->parameter_files = NULL;
args_impl->num_param_files_args = 0;
args_impl->allocator = allocator;

if (argc == 0) {
Expand All @@ -102,12 +151,27 @@ rcl_parse_arguments(
ret = RCL_RET_BAD_ALLOC;
goto fail;
}
args_impl->parameter_files = allocator.allocate(sizeof(char *) * argc, allocator.state);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could realloc at the end to the actual number of parameter files used.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (NULL == args_impl->parameter_files) {
ret = RCL_RET_BAD_ALLOC;
goto fail;
}

// Attempt to parse arguments as remap rules
for (int i = 0; i < argc; ++i) {
rcl_remap_t * rule = &(args_impl->remap_rules[args_impl->num_remap_rules]);
*rule = rcl_remap_get_zero_initialized();
if (RCL_RET_OK == _rcl_parse_remap_rule(argv[i], allocator, rule)) {
args_impl->parameter_files[args_impl->num_param_files_args] = NULL;
if (
RCL_RET_OK == _rcl_parse_param_rule(
argv[i], allocator, &(args_impl->parameter_files[args_impl->num_param_files_args])))
{
++(args_impl->num_param_files_args);
RCUTILS_LOG_DEBUG_NAMED(ROS_PACKAGE_NAME,
"params rule : %s\n total num param rules %d",
args_impl->parameter_files[args_impl->num_param_files_args - 1],
args_impl->num_param_files_args)
} else if (RCL_RET_OK == _rcl_parse_remap_rule(argv[i], allocator, rule)) {
++(args_impl->num_remap_rules);
} else {
RCUTILS_LOG_DEBUG_NAMED(ROS_PACKAGE_NAME, "arg %d (%s) error '%s'", i, argv[i],
Expand All @@ -131,6 +195,18 @@ rcl_parse_arguments(
allocator.deallocate(args_impl->remap_rules, allocator.state);
args_impl->remap_rules = NULL;
}
// Shrink Parameter files
if (0 == args_impl->num_param_files_args) {
allocator.deallocate(args_impl->parameter_files, allocator.state);
args_impl->parameter_files = NULL;
} else if (args_impl->num_param_files_args < argc) {
args_impl->parameter_files = rcutils_reallocf(
args_impl->parameter_files, sizeof(char *) * args_impl->num_param_files_args, &allocator);
if (NULL == args_impl->parameter_files) {
ret = RCL_RET_BAD_ALLOC;
goto fail;
}
}
// Shrink unparsed_args
if (0 == args_impl->num_unparsed_args) {
// No unparsed args
Expand Down Expand Up @@ -270,6 +346,7 @@ rcl_arguments_copy(
// Zero so it's safe to call rcl_arguments_fini() if an error occurrs while copying.
args_out->impl->num_remap_rules = 0;
args_out->impl->num_unparsed_args = 0;
args_out->impl->num_param_files_args = 0;

// Copy unparsed args
args_out->impl->unparsed_args = allocator.allocate(
Expand Down Expand Up @@ -306,6 +383,30 @@ rcl_arguments_copy(
return ret;
}
}
// Copy parameter files
if (args->impl->num_param_files_args) {
args_out->impl->parameter_files = allocator.allocate(
sizeof(char *) * args->impl->num_param_files_args, allocator.state);
if (NULL == args_out->impl->parameter_files) {
if (RCL_RET_OK != rcl_arguments_fini(args_out)) {
RCL_SET_ERROR_MSG("Error while finalizing arguments due to another error", error_alloc);
}
return RCL_RET_BAD_ALLOC;
}
args_out->impl->num_param_files_args = args->impl->num_param_files_args;
for (int i = 0; i < args->impl->num_param_files_args; ++i) {
args_out->impl->parameter_files[i] =
rcutils_strdup(args->impl->parameter_files[i], allocator);
if (NULL == args_out->impl->parameter_files[i]) {
if (RCL_RET_OK != rcl_arguments_fini(args_out)) {
RCL_SET_ERROR_MSG("Error while finalizing arguments due to another error", error_alloc);
}
return RCL_RET_BAD_ALLOC;
}
}
} else {
args_out->impl->parameter_files = NULL;
}
return RCL_RET_OK;
}

Expand Down Expand Up @@ -337,6 +438,16 @@ rcl_arguments_fini(
args->impl->num_unparsed_args = 0;
args->impl->unparsed_args = NULL;

if (args->impl->parameter_files) {
for (int p = 0; p < args->impl->num_param_files_args; ++p) {
args->impl->allocator.deallocate(
args->impl->parameter_files[p], args->impl->allocator.state);
}
args->impl->allocator.deallocate(args->impl->parameter_files, args->impl->allocator.state);
args->impl->num_param_files_args = 0;
args->impl->parameter_files = NULL;
}

args->impl->allocator.deallocate(args->impl, args->impl->allocator.state);
args->impl = NULL;
return ret;
Expand Down Expand Up @@ -857,6 +968,28 @@ _rcl_parse_remap_rule(
return ret;
}

rcl_ret_t
_rcl_parse_param_rule(
const char * arg,
rcl_allocator_t allocator,
char ** output_rule)
{
RCL_CHECK_ARGUMENT_FOR_NULL(arg, RCL_RET_INVALID_ARGUMENT, allocator);

const char * param_prefix = "__params:=";
const size_t param_prefix_len = strlen(param_prefix);
if (strncmp(param_prefix, arg, param_prefix_len) == 0) {
size_t outlen = strlen(arg) - param_prefix_len;
*output_rule = allocator.allocate(sizeof(char) * (outlen + 1), allocator.state);
if (NULL == output_rule) {
return RCL_RET_BAD_ALLOC;
}
snprintf(*output_rule, outlen + 1, "%s", arg + param_prefix_len);
return RCL_RET_OK;
}
return RCL_RET_INVALID_PARAM_RULE;
}

#ifdef __cplusplus
}
#endif
6 changes: 6 additions & 0 deletions rcl/src/rcl/arguments_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@ typedef struct rcl_arguments_impl_t
/// Length of unparsed_args.
int num_unparsed_args;

// TODO(mikaelarguedas) consider storing CLI parameter rules here
/// Array of yaml parameter file paths
char ** parameter_files;
/// Length of parameter_files.
int num_param_files_args;

/// Array of rules for name remapping.
rcl_remap_t * remap_rules;
/// Length of remap_rules.
Expand Down
73 changes: 73 additions & 0 deletions rcl/test/rcl/test_arguments.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), check_valid_vs_inval
EXPECT_TRUE(is_valid_arg("rostopic://rostopic:=rosservice"));
EXPECT_TRUE(is_valid_arg("rostopic:///rosservice:=rostopic"));
EXPECT_TRUE(is_valid_arg("rostopic:///foo/bar:=baz"));
EXPECT_TRUE(is_valid_arg("__params:=node_name"));

EXPECT_FALSE(is_valid_arg(":="));
EXPECT_FALSE(is_valid_arg("foo:="));
Expand All @@ -117,6 +118,7 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), check_valid_vs_inval
EXPECT_FALSE(is_valid_arg("foo:=/b}ar"));
EXPECT_FALSE(is_valid_arg("rostopic://:=rosservice"));
EXPECT_FALSE(is_valid_arg("rostopic::=rosservice"));
EXPECT_FALSE(is_valid_arg("__param:=node_name"));
}

TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_no_args) {
Expand Down Expand Up @@ -299,3 +301,74 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_remove_ros_args
alloc.deallocate(nonros_argv, alloc.state);
}
}

TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_param_argument_zero) {
const char * argv[] = {"process_name", "__ns:=/namespace", "random:=arg"};
int argc = sizeof(argv) / sizeof(const char *);
rcl_ret_t ret;

rcl_allocator_t alloc = rcl_get_default_allocator();
rcl_arguments_t parsed_args = rcl_get_zero_initialized_arguments();

ret = rcl_parse_arguments(argc, argv, alloc, &parsed_args);
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string_safe();

int parameter_filecount = rcl_arguments_get_param_files_count(&parsed_args);
EXPECT_EQ(0, parameter_filecount);
EXPECT_EQ(RCL_RET_OK, rcl_arguments_fini(&parsed_args));
}

TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_param_argument_single) {
const char * argv[] = {
"process_name", "__ns:=/namespace", "random:=arg", "__params:=parameter_filepath"
};
int argc = sizeof(argv) / sizeof(const char *);
rcl_ret_t ret;

rcl_allocator_t alloc = rcl_get_default_allocator();
rcl_arguments_t parsed_args = rcl_get_zero_initialized_arguments();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommend calling fini for reduced output from address sanitizer

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


ret = rcl_parse_arguments(argc, argv, alloc, &parsed_args);
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string_safe();

int parameter_filecount = rcl_arguments_get_param_files_count(&parsed_args);
EXPECT_EQ(1, parameter_filecount);
char ** parameter_files = NULL;
ret = rcl_arguments_get_param_files(&parsed_args, alloc, &parameter_files);
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string_safe();
EXPECT_STREQ("parameter_filepath", parameter_files[0]);

for (int i = 0; i < parameter_filecount; ++i) {
alloc.deallocate(parameter_files[i], alloc.state);
}
alloc.deallocate(parameter_files, alloc.state);
EXPECT_EQ(RCL_RET_OK, rcl_arguments_fini(&parsed_args));
}

TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_param_argument_multiple) {
const char * argv[] = {
"process_name", "__params:=parameter_filepath1", "__ns:=/namespace",
"random:=arg", "__params:=parameter_filepath2"
};
int argc = sizeof(argv) / sizeof(const char *);
rcl_ret_t ret;

rcl_allocator_t alloc = rcl_get_default_allocator();
rcl_arguments_t parsed_args = rcl_get_zero_initialized_arguments();

ret = rcl_parse_arguments(argc, argv, alloc, &parsed_args);
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string_safe();

int parameter_filecount = rcl_arguments_get_param_files_count(&parsed_args);
EXPECT_EQ(2, parameter_filecount);
char ** parameter_files = NULL;
ret = rcl_arguments_get_param_files(&parsed_args, alloc, &parameter_files);
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string_safe();
EXPECT_STREQ("parameter_filepath1", parameter_files[0]);
EXPECT_STREQ("parameter_filepath2", parameter_files[1]);
for (int i = 0; i < parameter_filecount; ++i) {
alloc.deallocate(parameter_files[i], alloc.state);
}
alloc.deallocate(parameter_files, alloc.state);
EXPECT_EQ(RCL_RET_OK, rcl_arguments_fini(&parsed_args));
}