search

ros2 / launch

Tools for launching multiple processes and for writing tests involving multiple processes.
Apache License 2.0
126 stars 144 forks source link

add format overriding by environment variables #722

Closed SammyRamone closed 10 months ago

SammyRamone commented 1 year ago

This is a proposed fix for https://github.com/ros2/launch_ros/issues/368 Originally, I planned on reusing the RCUTILS_CONSOLE_OUTPUT_FORMAT environment variable for this. Hoewever, this is not possible since launch and rcutils rely on two different logging implementations which have different names for the same things. For example one supports {line_number} and the other {lineno}. Instead I added two environment variables OVERRIDE_LAUNCH_SCREEN_FORMAT and OVERRIDE_LAUNCH_LOG_FORMAT since screen and log formatting is handled differently in the code. I also framed the variables as override similar to https://github.com/ros2/launch/pull/713

Any feedback is welcome.

SammyRamone commented 1 year ago

friendly ping @adityapande-1995 @methylDragon @wjwwood

SammyRamone commented 11 months ago

Sorry for the long wait. This was fairly low-priority for me and I only now had time to work on it again. I added a test for the environment variables and also ensured that the other tests don't fail if the environment variable is set in the terminal in which the tests are executed.

SammyRamone commented 11 months ago

I also added a draft PR for documenting this PR https://github.com/ros2/ros2_documentation/pull/4061

SammyRamone commented 10 months ago

friendly ping @methylDragon

adityapande-1995 commented 10 months ago

@SammyRamone thanks for the patience ! The team had a discussion about this, and while we do think there should be a holistic way to configure all of this logging, this is a good feature. I'll run full CI on this.

adityapande-1995 commented 10 months ago