-
Notifications
You must be signed in to change notification settings - Fork 163
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
Set default logger level from command line #256
Changes from 15 commits
714751b
dc35947
08b6266
10b4056
6479269
73b987a
eb72a1a
77b1a8c
7d5c90d
0cf31b5
9ac0f8d
3d5cba6
27fca18
ed56679
e923448
0ef1505
b626c0a
3307ff7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,8 @@ | |
#include "rcl/lexer_lookahead.h" | ||
#include "rcl/validate_topic_name.h" | ||
#include "rcutils/allocator.h" | ||
#include "rcutils/error_handling.h" | ||
#include "rcutils/logging.h" | ||
#include "rcutils/logging_macros.h" | ||
#include "rcutils/strdup.h" | ||
#include "rmw/validate_namespace.h" | ||
|
@@ -51,12 +53,22 @@ _rcl_parse_remap_rule( | |
rcl_allocator_t allocator, | ||
rcl_remap_t * output_rule); | ||
|
||
/// Parse an argument that may or may not be a parameter file rule. | ||
/// The syntax of the file name is not validated. | ||
/// \param[in] arg the argument to parse | ||
/// \param[in] allocator an allocator to use | ||
/// \param[in,out] param_file string that could be a parameter file name | ||
/// \return RCL_RET_OK if the rule was parsed correctly, or | ||
/// \return RCL_RET_INVALID_PARAM_RULE if the argument is not a valid rule, or | ||
/// \return RCL_RET_BAD_ALLOC if an allocation failed, or | ||
/// \return RLC_RET_ERROR if an unspecified error occurred. | ||
/// \internal | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nitpick: since this is not parsed by doxygen, I'd recommend using normal comments, e.g. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. while I was mostly just copying the existing formatting, I had imagined from the /internal tag that we were using doxygen formatting to optionally support building internal docs. But even without that could it still be worthwhile keeping the doxygen formatting in case one day we move them to the header? Or do you find it easier to read as a standard comment? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well if you want to use doxygen style that’s fine, though we should turn on the internal docs feature then. But I’m sure this is something I mentioned on the other prs, but this doesn’t match the doxygen style for most of rcl. So if you’re going to touch it, I’d recommend the style described in our style guide under comments and doc comments: https://github.com/ros2/ros2/wiki/Developer-Guide#c-1 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yep, happy to change the file to use /** style. and will look at turning on the internal docs generation |
||
RCL_LOCAL | ||
rcl_ret_t | ||
_rcl_parse_param_rule( | ||
_rcl_parse_param_file_rule( | ||
const char * arg, | ||
rcl_allocator_t allocator, | ||
char ** output_rule); | ||
char ** param_file); | ||
|
||
rcl_ret_t | ||
rcl_arguments_get_param_files( | ||
|
@@ -98,6 +110,22 @@ rcl_arguments_get_param_files_count( | |
return args->impl->num_param_files_args; | ||
} | ||
|
||
/// Parse an argument that may or may not be a log level rule. | ||
/// \param[in] arg the argument to parse | ||
/// \param[in] allocator an allocator to use | ||
/// \param[in,out] log_level parsed log level represented by `RCUTILS_LOG_SEVERITY` enum | ||
/// \return RCL_RET_OK if a valid log level was parsed, or | ||
/// \return RCL_RET_INVALID_LOG_LEVEL_RULE if the argument is not a valid rule, or | ||
/// \return RCL_RET_BAD_ALLOC if an allocation failed, or | ||
/// \return RLC_RET_ERROR if an unspecified error occurred. | ||
/// \internal | ||
RCL_LOCAL | ||
rcl_ret_t | ||
_rcl_parse_log_level_rule( | ||
const char * arg, | ||
rcl_allocator_t allocator, | ||
int * log_level); | ||
|
||
rcl_ret_t | ||
rcl_parse_arguments( | ||
int argc, | ||
|
@@ -129,6 +157,7 @@ rcl_parse_arguments( | |
rcl_arguments_impl_t * args_impl = args_output->impl; | ||
args_impl->num_remap_rules = 0; | ||
args_impl->remap_rules = NULL; | ||
args_impl->log_level = -1; | ||
args_impl->unparsed_args = NULL; | ||
args_impl->num_unparsed_args = 0; | ||
args_impl->parameter_files = NULL; | ||
|
@@ -157,29 +186,50 @@ rcl_parse_arguments( | |
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(); | ||
// Attempt to parse argument as parameter file rule | ||
args_impl->parameter_files[args_impl->num_param_files_args] = NULL; | ||
if ( | ||
RCL_RET_OK == _rcl_parse_param_rule( | ||
RCL_RET_OK == _rcl_parse_param_file_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)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note to reviewers: this block has been restructured to not have else statements. Error messages must be cleared after each attempt to avoid overwriting error messages on the next argument. This is an example output from out tests, with debug enabled:
|
||
args_impl->num_param_files_args); | ||
continue; | ||
} | ||
RCUTILS_LOG_DEBUG_NAMED(ROS_PACKAGE_NAME, | ||
"Couldn't parse arg %d (%s) as parameter file rule. Error: %s", i, argv[i], | ||
rcl_get_error_string()); | ||
rcl_reset_error(); | ||
|
||
// Attempt to parse argument as remap rule | ||
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->num_remap_rules); | ||
} else { | ||
RCUTILS_LOG_DEBUG_NAMED(ROS_PACKAGE_NAME, "arg %d (%s) error '%s'", i, argv[i], | ||
rcl_get_error_string()); | ||
rcl_reset_error(); | ||
args_impl->unparsed_args[args_impl->num_unparsed_args] = i; | ||
++(args_impl->num_unparsed_args); | ||
continue; | ||
} | ||
RCUTILS_LOG_DEBUG_NAMED(ROS_PACKAGE_NAME, | ||
"Couldn't parse arg %d (%s) as remap rule. Error: %s", i, argv[i], rcl_get_error_string()); | ||
rcl_reset_error(); | ||
|
||
// Attempt to parse argument as log level configuration | ||
int log_level; | ||
if (RCL_RET_OK == _rcl_parse_log_level_rule(argv[i], allocator, &log_level)) { | ||
args_impl->log_level = log_level; | ||
continue; | ||
} | ||
RCUTILS_LOG_DEBUG_NAMED(ROS_PACKAGE_NAME, | ||
"Couldn't parse arg %d (%s) as log level rule. Error: %s", i, argv[i], | ||
rcl_get_error_string()); | ||
rcl_reset_error(); | ||
|
||
// Argument wasn't parsed by any rule | ||
args_impl->unparsed_args[args_impl->num_unparsed_args] = i; | ||
++(args_impl->num_unparsed_args); | ||
} | ||
|
||
// Shrink remap_rules array to match number of successfully parsed rules | ||
|
@@ -933,6 +983,29 @@ _rcl_parse_remap_begin_remap_rule( | |
return ret; | ||
} | ||
|
||
rcl_ret_t | ||
_rcl_parse_log_level_rule( | ||
const char * arg, | ||
rcl_allocator_t allocator, | ||
int * log_level) | ||
{ | ||
RCL_CHECK_ARGUMENT_FOR_NULL(arg, RCL_RET_INVALID_ARGUMENT, allocator); | ||
RCL_CHECK_ARGUMENT_FOR_NULL(log_level, RCL_RET_INVALID_ARGUMENT, allocator); | ||
|
||
if (strncmp(RCL_LOG_LEVEL_ARG_RULE, arg, strlen(RCL_LOG_LEVEL_ARG_RULE)) != 0) { | ||
RCL_SET_ERROR_MSG("Argument does not start with '" RCL_LOG_LEVEL_ARG_RULE "'", allocator); | ||
return RCL_RET_INVALID_LOG_LEVEL_RULE; | ||
} | ||
rcutils_ret_t ret = rcutils_logging_severity_level_from_string( | ||
arg + strlen(RCL_LOG_LEVEL_ARG_RULE), allocator, log_level); | ||
if (RCUTILS_RET_OK == ret) { | ||
return RCL_RET_OK; | ||
} | ||
RCL_SET_ERROR_MSG("Argument does not use a valid severity level", allocator); | ||
return RCL_RET_INVALID_LOG_LEVEL_RULE; | ||
} | ||
|
||
|
||
rcl_ret_t | ||
_rcl_parse_remap_rule( | ||
const char * arg, | ||
|
@@ -969,24 +1042,25 @@ _rcl_parse_remap_rule( | |
} | ||
|
||
rcl_ret_t | ||
_rcl_parse_param_rule( | ||
_rcl_parse_param_file_rule( | ||
const char * arg, | ||
rcl_allocator_t allocator, | ||
char ** output_rule) | ||
char ** param_file) | ||
{ | ||
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) { | ||
const size_t param_prefix_len = strlen(RCL_PARAM_FILE_ARG_RULE); | ||
if (strncmp(RCL_PARAM_FILE_ARG_RULE, 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) { | ||
*param_file = allocator.allocate(sizeof(char) * (outlen + 1), allocator.state); | ||
if (NULL == param_file) { | ||
RCUTILS_SAFE_FWRITE_TO_STDERR("Failed to allocate memory for parameters file path\n"); | ||
return RCL_RET_BAD_ALLOC; | ||
} | ||
snprintf(*output_rule, outlen + 1, "%s", arg + param_prefix_len); | ||
snprintf(*param_file, outlen + 1, "%s", arg + param_prefix_len); | ||
return RCL_RET_OK; | ||
} | ||
RCL_SET_ERROR_MSG("Argument does not start with '" RCL_PARAM_FILE_ARG_RULE "'", allocator); | ||
return RCL_RET_INVALID_PARAM_RULE; | ||
} | ||
|
||
|
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.
What's the expected behavior if
__log_level:=
is used more than once in command line arguments? Error? Last one wins? First one wins?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.
Last one, documentation added in 0ef1505