use custom log_file name as per the user setting

[Tanishq30052002]

closes ros2/launch_ros#380

[fujitatomoya]

test would be ideal to make sure the specified log file name is generated with ros2 launch --log-file-name command.

[Tanishq30052002]

test would be ideal to make sure the specified log file name is generated with ros2 launch --log-file-name command.

@fujitatomoya
I am not clear what you mean here.

[christophebedard]

Also, please take a look at the test results here: https://build.ros2.org/job/Rpr__launch__ubuntu_noble_amd64/164/testReport/. There are some flake8 linter issues and some test failures that seem to be related to your change. You can run the tests locally

[Tanishq30052002]

@christophebedard i have reolved all the issues and resolved the failing test.

@fujitatomoya can you please review it once again?

[fujitatomoya]

@Tanishq30052002 thanks for working on this, had a couple of minor comments.

[fujitatomoya]

test would be ideal to make sure the specified log file name is generated with ros2 launch --log-file-name command.

@fujitatomoya I am not clear what you mean here.

i am just suggesting unit tests for CI/CD to make sure nothing is broken during development.
since this PR changes LaunchService and LaunchConfig, i think we can add test to ensure file_name is properly handled in those classes. and as user interface, that would be nice to add test to https://github.com/ros2/launch_ros/tree/rolling/ros2launch/test. but i do not see any actually test cases exist for using ros2launch, so this one is probably nice to have.

[Tanishq30052002]

am just suggesting unit tests for CI/CD to make sure nothing is broken during development.
since this PR changes LaunchService and LaunchConfig, i think we can add test to ensure file_name is properly handled in those classes. and as user interface, that would be nice to add test to

ok, sounds good. I will create test for the same.
currently functionality is working and it is working as expected with failing any current tests

[Tanishq30052002]

am just suggesting unit tests for CI/CD to make sure nothing is broken during development.
since this PR changes LaunchService and LaunchConfig, i think we can add test to ensure file_name is properly handled in those classes. and as user interface, that would be nice to add test to

ok, sounds good. I will create test for the same. currently functionality is working and it is working as expected with failing any current tests

@fujitatomoya i have written tests for launch service and launch config. you may review it once and let me know if something more can be added.

[fujitatomoya]

@Tanishq30052002 we are getting closer, and thanks for being patient and iterating with me. I have only a couple of minor things, could you please have a look? (thanks for adding test though.)

[fujitatomoya]

@Tanishq30052002 i am not convinced to take this current change. please review my comments and let's not break the user space.

[Tanishq30052002]

@Tanishq30052002 i am not convinced to take this current change. please review my comments and let's not break the user space.

I got your point, I will try to resolve these issues

[Tanishq30052002]

@fujitatomoya i have reverted back changes and also squash multiple commits to keep the history clean.
Also there are some changes in test of logging which are neccessory for testing cutom file name logging.