Add option to set logger severity level using a config file #347
Conversation
Signed-off-by: JafarAbdi <cafer.abdi@gmail.com>
JafarAbdi
force-pushed
the
pr-logging_config_file
branch
from
a63135b to
c9f4a41
Compare
There was a problem hiding this comment.
some questions and comments,
- environment variable prevails logging level arguments? I am not sure which one prevails to the other.
- it should be able to set
defaultlevel as well in the configuration file? - probably it would be nice to follow up tutorials how to configure logging level via
RCUTILS_LOGGING_CONFIG_FILE?
| // call to this function due to RCUTILS_LOGGING_AUTOINIT Check for the | ||
| // Check for the environment variable for selecting logging level | ||
| const char * logging_config_filename; | ||
| ret_str = rcutils_get_env("RCUTILS_LOGGING_CONFIG_FILE", &logging_config_filename); |
There was a problem hiding this comment.
how about define for RCUTILS_LOGGING_CONFIG_FILE, so that we can refer to it.
src/logging.c
Outdated
| char logger_name[50]; | ||
| char severity[10]; // fatal error debug info warn case insensitive |
There was a problem hiding this comment.
how about allocate memory dynamically? I think that how it is done when parsing the arguments logging level.
There was a problem hiding this comment.
Good point, I did some googling, looks like having scanf allocate memory is a posix extension which mean that it may not work on windows (I'm not sure)
@fujitatomoya is there such a facility to set logging levels through environment variables? I don't think so, in which case this question would be moot for now. (correct me if I'm wrong here please) |
No, i do not think so. not via environmental variable, but we can change it with ros arguments. so i just thought that it should be discussed as design before implementation? |
JafarAbdi
force-pushed
the
pr-logging_config_file
branch
from
a10624e to
442033b
Compare
No, logging level arguments prevail the environment variable ones, so if we have logger1 set to debug in the
Done in the latest commit
I'm working on it now |
|
I'll add tests for this PR later this week |
You mean this one, right .? |
|
@JafarAbdi I mean the docblock for the function being modified: https://github.com/ros2/rcutils/blob/master/include/rcutils/logging.h#L49-L107 |
Add ability to specify the logger severity using a config file similar to ROS1's
ROSCONSOLE_CONFIG_FILEenv-variable, this can be done byexport RCUTILS_LOGGING_CONFIG_FILE=path_to_config_fileThis's a demo:
logging-2021-09-23_15.52.29.mp4